Skip to content

Commit

Permalink
Fix size on empty oneOf marshalling
Browse files Browse the repository at this point in the history
planetscale#130 did not correctly
account for the key size impacting the size. This fixes the original PR,
and adds another test case to cover the issue
  • Loading branch information
howardjohn committed Apr 8, 2024
1 parent 0393e58 commit 9cfb335
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 19 deletions.
23 changes: 16 additions & 7 deletions conformance/internal/conformance/oneof_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
package conformance

import (
"testing"

"github.com/planetscale/vtprotobuf/types/known/structpb"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/proto"
upstreamstructpb "google.golang.org/protobuf/types/known/structpb"
"testing"
)

func TestEmptyOneoff(t *testing.T) {
func TestEmptyOneof(t *testing.T) {
// Regression test for https://github.com/planetscale/vtprotobuf/issues/61
msg := &TestAllTypesProto3{OneofField: &TestAllTypesProto3_OneofNestedMessage{}}
upstream, _ := proto.Marshal(msg)
vt, _ := msg.MarshalVTStrict()
require.Equal(t, upstream, vt)
t.Run("all proto", func(t *testing.T) {
msg := &TestAllTypesProto3{OneofField: &TestAllTypesProto3_OneofNestedMessage{}}
upstream, _ := proto.Marshal(msg)
vt, _ := msg.MarshalVTStrict()
require.Equal(t, upstream, vt)
})
t.Run("structpb", func(t *testing.T) {
msg := &structpb.Value{Kind: &upstreamstructpb.Value_StructValue{}}
upstream, _ := proto.Marshal((*upstreamstructpb.Value)(msg))
vt, _ := msg.MarshalVTStrict()
require.Equal(t, upstream, vt)
})
}
4 changes: 2 additions & 2 deletions features/size/size.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ func (p *size) field(oneof bool, field *protogen.Field, sizeName string) {
}
// Empty protobufs should emit a message or compatibility with Golang protobuf;
// See https://github.com/planetscale/vtprotobuf/issues/61
// Size is always 3 so just hardcode that here
// Size is always keysize + 1 so just hardcode that here
if oneof && field.Desc.Kind() == protoreflect.MessageKind && !field.Desc.IsMap() && !field.Desc.IsList() {
p.P("} else { n += 3 }")
p.P("} else { n += ", strconv.Itoa(key + 1), " }")
} else if repeated || nullable {
p.P(`}`)
}
Expand Down
6 changes: 3 additions & 3 deletions testproto/pool/pool_with_oneof_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions testproto/unsafe/unsafe_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions types/known/structpb/struct_vtproto.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 9cfb335

Please sign in to comment.