From f8d654da57f825dc61dba3824e21d05f1fe47baf Mon Sep 17 00:00:00 2001 From: Eric Myhre Date: Fri, 4 Dec 2020 14:24:42 +0100 Subject: [PATCH] codegen: assembler for struct with map representation now validates all non-optional fields are present. This continues what https://github.com/ipld/go-ipld-prime/pull/111/ did and adds the same logic to the map representation. The actual state tracking works the same way (and was mostly already there). Rearranged the tests slightly. Made error messages include both field name and serial key when they differ due to a rename directive. (It's possible this error would get nicer if it used a list of StructField instead of just strings, but it would also get more complicated. Maybe revisit later.) --- CHANGELOG.md | 2 +- schema/gen/go/genStructReprMap.go | 16 ++++- ...ructNesting_test.go => testStruct_test.go} | 62 +++++++++++++++++++ .../gen/go/testStructsContainingMaybe_test.go | 23 ------- schema/typeMethods.go | 5 ++ 5 files changed, 83 insertions(+), 25 deletions(-) rename schema/gen/go/{testStructNesting_test.go => testStruct_test.go} (54%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fc37d76..32c3db26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,7 +55,7 @@ When a release tag is made, this block of bullet points will just slide down to - ... mostly. Some manual configuration may sometimes be required to make sure the generated structure wouldn't have an infinite memory size. We'll keep working on making this smoother in the future. - Field symbol overrides now work properly. (E.g., if you have a schema with a field called "type", you can make that work now. Just needs a field symbol override in the Adjunct Config when doing codegen!) - Codegen'd link types now implemented the `schema.TypedLinkNode` interface where applicable. - - Structs now actually validate all required fields are present before allowing themselves to finish building. + - Structs now actually validate all required fields are present before allowing themselves to finish building. Ditto for their map representations. - Much more testing. And we've got a nice new declarative testcase system that makes it easier to write descriptions of how data should behave (at both the typed and representation view levels), and then just call one function to run exhaustive tests to make sure it looks the same from every inspectable API. - Change: codegen now outputs a fixed set of files. (Previously, it output one file per type in your schema.) This makes codegen much more managable; if you remove a type from your schema, you don't have to chase down the orphaned file. It's also just plain less clutter to look at on the filesystem. - Demo: as proof of the kind of work that can be done now with codegen, we've implemented the IPLD Schema schema -- the schema that describes IPLD Schema declarations -- using codegen. It's pretty neat. diff --git a/schema/gen/go/genStructReprMap.go b/schema/gen/go/genStructReprMap.go index 9c8f9c61..ff683a74 100644 --- a/schema/gen/go/genStructReprMap.go +++ b/schema/gen/go/genStructReprMap.go @@ -543,7 +543,21 @@ func (g structReprMapReprBuilderGenerator) emitMapAssemblerMethods(w io.Writer) case maState_finished: panic("invalid state: Finish cannot be called on an assembler that's already finished") } - //FIXME check if all required fields are set + if ma.s & fieldBits__{{ $type | TypeSymbol }}_sufficient != fieldBits__{{ $type | TypeSymbol }}_sufficient { + err := ipld.ErrMissingRequiredField{Missing: make([]string, 0)} + {{- range $i, $field := .Type.Fields }} + {{- if not $field.IsMaybe}} + if ma.s & fieldBit__{{ $type | TypeSymbol }}_{{ $field | FieldSymbolUpper }} == 0 { + {{- if $field | $type.RepresentationStrategy.FieldHasRename }} + err.Missing = append(err.Missing, "{{ $field.Name }} (serial:\"{{ $field | $type.RepresentationStrategy.GetFieldKey }}\")") + {{- else}} + err.Missing = append(err.Missing, "{{ $field.Name }}") + {{- end}} + } + {{- end}} + {{- end}} + return err + } ma.state = maState_finished *ma.m = schema.Maybe_Value return nil diff --git a/schema/gen/go/testStructNesting_test.go b/schema/gen/go/testStruct_test.go similarity index 54% rename from schema/gen/go/testStructNesting_test.go rename to schema/gen/go/testStruct_test.go index 4454c78e..a3300ef5 100644 --- a/schema/gen/go/testStructNesting_test.go +++ b/schema/gen/go/testStruct_test.go @@ -11,6 +11,68 @@ import ( "github.com/ipld/go-ipld-prime/schema" ) +func TestRequiredFields(t *testing.T) { + t.Parallel() + + ts := schema.TypeSystem{} + ts.Init() + adjCfg := &AdjunctCfg{} + ts.Accumulate(schema.SpawnString("String")) + ts.Accumulate(schema.SpawnStruct("StructOne", + []schema.StructField{ + schema.SpawnStructField("a", "String", false, false), + schema.SpawnStructField("b", "String", false, false), + }, + schema.SpawnStructRepresentationMap(map[string]string{ + // no renames. we expect a simpler error message in this case. + }), + )) + ts.Accumulate(schema.SpawnStruct("StructTwo", + []schema.StructField{ + schema.SpawnStructField("a", "String", false, false), + schema.SpawnStructField("b", "String", false, false), + }, + schema.SpawnStructRepresentationMap(map[string]string{ + "b": "z", + }), + )) + + prefix := "struct-required-fields" + pkgName := "main" + genAndCompileAndTest(t, prefix, pkgName, ts, adjCfg, func(t *testing.T, getPrototypeByName func(string) ipld.NodePrototype) { + t.Run("building-type-without-required-fields-errors", func(t *testing.T) { + np := getPrototypeByName("StructOne") + + nb := np.NewBuilder() + ma, _ := nb.BeginMap(0) + err := ma.Finish() + + Wish(t, err, ShouldBeSameTypeAs, ipld.ErrMissingRequiredField{}) + Wish(t, err.Error(), ShouldEqual, `missing required fields: a,b`) + }) + t.Run("building-representation-without-required-fields-errors", func(t *testing.T) { + nrp := getPrototypeByName("StructOne.Repr") + + nb := nrp.NewBuilder() + ma, _ := nb.BeginMap(0) + err := ma.Finish() + + Wish(t, err, ShouldBeSameTypeAs, ipld.ErrMissingRequiredField{}) + Wish(t, err.Error(), ShouldEqual, `missing required fields: a,b`) + }) + t.Run("building-representation-with-renames-without-required-fields-errors", func(t *testing.T) { + nrp := getPrototypeByName("StructTwo.Repr") + + nb := nrp.NewBuilder() + ma, _ := nb.BeginMap(0) + err := ma.Finish() + + Wish(t, err, ShouldBeSameTypeAs, ipld.ErrMissingRequiredField{}) + Wish(t, err.Error(), ShouldEqual, `missing required fields: a,b (serial:"z")`) + }) + }) +} + func TestStructNesting(t *testing.T) { t.Parallel() diff --git a/schema/gen/go/testStructsContainingMaybe_test.go b/schema/gen/go/testStructsContainingMaybe_test.go index 90b23ce7..47555078 100644 --- a/schema/gen/go/testStructsContainingMaybe_test.go +++ b/schema/gen/go/testStructsContainingMaybe_test.go @@ -171,27 +171,4 @@ func TestStructsContainingMaybe(t *testing.T) { } }) }) - - genAndCompileAndTest(t, "stroct3", "main", ts, adjCfg, func(t *testing.T, getPrototypeByName func(string) ipld.NodePrototype) { - t.Run("insufficient", func(t *testing.T) { - nrp := getPrototypeByName("Stroct") - t.Run("typed-create", func(t *testing.T) { - b := nrp.NewBuilder() - mb, err := b.BeginMap(0) - if err != nil { - t.Fatal(err) - } - v, err := mb.AssembleEntry("f1") - if err != nil { - t.Fatal(err) - } - v.AssignString("v1") - - err = mb.Finish() - if _, ok := err.(ipld.ErrMissingRequiredField); !ok { - t.Fatalf("Expected error for missing field, got %v", err) - } - }) - }) - }) } diff --git a/schema/typeMethods.go b/schema/typeMethods.go index 41f06470..ba82484a 100644 --- a/schema/typeMethods.go +++ b/schema/typeMethods.go @@ -226,6 +226,11 @@ func (r StructRepresentation_Map) GetFieldKey(field StructField) string { return field.name } +func (r StructRepresentation_Map) FieldHasRename(field StructField) bool { + _, ok := r.renames[field.name] + return ok +} + func (r StructRepresentation_Stringjoin) GetDelim() string { return r.sep }