From 31ab0bf6c8928c3f0e40e41a63795c4483aa6f87 Mon Sep 17 00:00:00 2001 From: razvanalex Date: Mon, 1 Dec 2025 16:50:03 +0200 Subject: [PATCH] [Go] Write required string fields into the buffer when using Object API (#8402) * [Go] Write required string fields into the buffer when using Object API In C++, CreateX allows to write the default "" value of a required string, when the string is not explicitly set. Object API Pack method uses this implementation of CreateX. However, in go, despite whether the field is optional or required, it is always checked against empty string allowing to create messages that fail to pass the verifier. This commits partially reverts #7719 when the string field is required. * Add test for serializing required string fields using Object API * Update generated code The Monster schema contains a key string field. For historical convenience reasons, string keys are assumed required. --- src/idl_gen_go.cpp | 15 ++++++---- tests/GoTest.sh | 1 + tests/MyGame/Example/Monster.go | 5 +--- tests/go_test.go | 53 +++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 9 deletions(-) diff --git a/src/idl_gen_go.cpp b/src/idl_gen_go.cpp index f4a03cec4..46585b3e6 100644 --- a/src/idl_gen_go.cpp +++ b/src/idl_gen_go.cpp @@ -1122,11 +1122,16 @@ class GoGenerator : public BaseGenerator { const std::string offset = field_var + "Offset"; if (IsString(field.value.type)) { - code += "\t" + offset + " := flatbuffers.UOffsetT(0)\n"; - code += "\tif t." + field_field + " != \"\" {\n"; - code += "\t\t" + offset + " = builder.CreateString(t." + field_field + - ")\n"; - code += "\t}\n"; + if (!field.IsRequired()) { + code += "\t" + offset + " := flatbuffers.UOffsetT(0)\n"; + code += "\tif t." + field_field + " != \"\" {\n"; + code += "\t\t" + offset + " = builder.CreateString(t." + field_field + + ")\n"; + code += "\t}\n"; + } else { + code += "\t" + offset + " := builder.CreateString(t." + field_field + + ")\n"; + } } else if (IsVector(field.value.type) && field.value.type.element == BASE_TYPE_UCHAR && field.value.type.enum_def == nullptr) { diff --git a/tests/GoTest.sh b/tests/GoTest.sh index b55cad577..6027d0c77 100755 --- a/tests/GoTest.sh +++ b/tests/GoTest.sh @@ -23,6 +23,7 @@ go_src=${go_path}/src ../flatc -g --gen-object-api -I include_test -o ${go_src} monster_test.fbs optional_scalars.fbs ../flatc -g --gen-object-api -I include_test/sub -o ${go_src} include_test/order.fbs ../flatc -g --gen-object-api -o ${go_src}/Pizza include_test/sub/no_namespace.fbs +../flatc -g --gen-object-api -o ${go_src} required_strings.fbs # Go requires a particular layout of files in order to link multiple packages. # Copy flatbuffer Go files to their own package directories to compile the diff --git a/tests/MyGame/Example/Monster.go b/tests/MyGame/Example/Monster.go index cc6453b1a..854db5e58 100644 --- a/tests/MyGame/Example/Monster.go +++ b/tests/MyGame/Example/Monster.go @@ -76,10 +76,7 @@ func (t *MonsterT) Pack(builder *flatbuffers.Builder) flatbuffers.UOffsetT { if t == nil { return 0 } - nameOffset := flatbuffers.UOffsetT(0) - if t.Name != "" { - nameOffset = builder.CreateString(t.Name) - } + nameOffset := builder.CreateString(t.Name) inventoryOffset := flatbuffers.UOffsetT(0) if t.Inventory != nil { inventoryOffset = builder.CreateByteString(t.Inventory) diff --git a/tests/go_test.go b/tests/go_test.go index e072705a2..05c79b458 100644 --- a/tests/go_test.go +++ b/tests/go_test.go @@ -22,6 +22,7 @@ import ( pizza "Pizza" "encoding/json" optional_scalars "optional_scalars" // refers to generated code + required_strings "required_strings" // refers to generated code order "order" "bytes" @@ -200,6 +201,10 @@ func TestAll(t *testing.T) { // Check that optional scalars works CheckOptionalScalars(t.Fatalf) + // Check that default required string fields are placed in the buffer + // using Object API + CheckRequiredStrings(t.Fatalf) + // Check that getting vector element by key works CheckByKey(t.Fatalf) @@ -2346,6 +2351,54 @@ func CheckOptionalScalars(fail func(string, ...interface{})) { expectEq("defaultEnum", obj.DefaultEnum, optional_scalars.OptionalByteTwo) } +func CheckRequiredStrings(fail func(string, ...interface{})) { + equalContent := func(strValue *string, bytesValue []byte) bool { + return (strValue == nil && bytesValue == nil) || + (strValue != nil && bytesValue != nil && *strValue == string(bytesValue)) + } + + expectSucceeds := func(obj *required_strings.FooT, strA, strB *string) { + builder := flatbuffers.NewBuilder(0) + builder.Finish(obj.Pack(builder)) + + // Check fields are correctly set + foo := required_strings.GetRootAsFoo(builder.FinishedBytes(), 0) + if got := foo.StrA(); !equalContent(strA, got) { + fail(FailString("StrA", strA, got)) + } + if got := foo.StrB(); !equalContent(strB, got) { + fail(FailString("StrB", strB, got)) + } + + // Check unpack gives the original object + obj2 := foo.UnPack() + if !reflect.DeepEqual(obj, obj2) { + fail(FailString("Pack/Unpack()", obj, obj2)) + } + } + + valueA := "value a" + valueB := "value b" + empty := "" + + expectSucceeds(&required_strings.FooT{ + StrA: valueA, + }, &valueA, &empty) + + expectSucceeds(&required_strings.FooT{ + StrB: valueB, + }, &empty, &valueB) + + expectSucceeds(&required_strings.FooT{ + StrA: valueA, + StrB: valueB, + }, &valueA, &valueB) + + expectSucceeds(&required_strings.FooT{ + StrA: empty, + }, &empty, &empty) +} + func CheckByKey(fail func(string, ...interface{})) { expectEq := func(what string, a, b interface{}) { if a != b {