From 6042d4d882022456b07a77a97b57cfedd3c007f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 3 Jun 2021 11:30:27 +0100 Subject: [PATCH] node/bindnode: start running all schema tests We add node/tests.SchemaTestAll to simplify this task, meaning we don't need to duplicate all test func declarations in node/bindnode. SchemaTestAll is also flexible enough to allow running multiple sub-tests per schema test in the future. There were two remaining places in node/tests that still weren't using ipld.DeepEqual, so fix those. Finally, bindnode needed a couple of changes to fully support ipld.DeepEqual. Most notable is iteration over maps, which required a bit of a refactor to keep ordered keys. --- node/bindnode/node.go | 104 ++++++++++++++++++++++++++++++----- node/bindnode/schema_test.go | 52 ++++++++++++++++++ node/tests/schema.go | 62 +++++++++++++++++++++ node/tests/schemaMaps.go | 2 +- node/tests/testcase.go | 2 +- 5 files changed, 207 insertions(+), 15 deletions(-) create mode 100644 node/bindnode/schema_test.go create mode 100644 node/tests/schema.go diff --git a/node/bindnode/node.go b/node/bindnode/node.go index b25b0eb..54d495c 100644 --- a/node/bindnode/node.go +++ b/node/bindnode/node.go @@ -109,7 +109,26 @@ func inferGoType(typ schema.Type) reflect.Type { if typ.ValueIsNullable() { vtyp = reflect.PtrTo(vtyp) } - return reflect.MapOf(ktyp, vtyp) + // We need an extra field to keep the map ordered, + // since IPLD maps must have stable iteration order. + // We could sort when iterating, but that's expensive. + // Keeping the insertion order is easy and intuitive. + // + // struct { + // Keys []K + // Values map[K]V + // } + goFields := []reflect.StructField{ + { + Name: "Keys", + Type: reflect.SliceOf(ktyp), + }, + { + Name: "Values", + Type: reflect.MapOf(ktyp, vtyp), + }, + } + return reflect.StructOf(goFields) case *schema.TypeList: etyp := inferGoType(typ.ValueType()) if typ.ValueIsNullable() { @@ -117,6 +136,7 @@ func inferGoType(typ schema.Type) reflect.Type { } return reflect.SliceOf(etyp) case *schema.TypeUnion: + // We need an extra field to record what member we stored. type goUnion struct { Index int // 0..len(typ.Members)-1 Value interface{} @@ -260,20 +280,21 @@ func (w *_node) LookupByString(key string) (ipld.Node, error) { return node, nil case *schema.TypeMap: var kval reflect.Value + valuesVal := w.val.FieldByName("Values") switch ktyp := typ.KeyType().(type) { case *schema.TypeString: kval = reflect.ValueOf(key) default: asm := &_assembler{ schemaType: ktyp, - val: reflect.New(w.val.Type().Key()).Elem(), + val: reflect.New(valuesVal.Type().Key()).Elem(), } if err := (*_assemblerRepr)(asm).AssignString(key); err != nil { return nil, err } kval = asm.val } - fval := w.val.MapIndex(kval) + fval := valuesVal.MapIndex(kval) if !fval.IsValid() { // not found return nil, ipld.ErrNotExists{ // TODO @@ -377,7 +398,11 @@ func (w *_node) MapIterator() ipld.MapIterator { val: w.val, } case *schema.TypeMap: - panic("TODO: ") + return &_mapIterator{ + schemaType: typ, + keysVal: w.val.FieldByName("Keys"), + valuesVal: w.val.FieldByName("Values"), + } } return nil } @@ -405,7 +430,7 @@ func (w *_node) Length() int64 { case *schema.TypeUnion: return 1 } - fallthrough // map + return int64(w.val.FieldByName("Keys").Len()) case ipld.Kind_List: return int64(w.val.Len()) } @@ -530,12 +555,15 @@ func (w *_assembler) BeginMap(sizeHint int64) (ipld.MapAssembler, error) { }, nil case *schema.TypeMap: val := w.nonPtrVal() - if val.IsNil() { - val.Set(reflect.MakeMap(val.Type())) + keysVal := val.FieldByName("Keys") + valuesVal := val.FieldByName("Values") + if valuesVal.IsNil() { + valuesVal.Set(reflect.MakeMap(valuesVal.Type())) } return &_mapAssembler{ schemaType: typ, - val: val, + keysVal: keysVal, + valuesVal: valuesVal, finish: w.finish, }, nil case *schema.TypeUnion: @@ -841,7 +869,8 @@ func (w *_structAssembler) ValuePrototype(k string) ipld.NodePrototype { type _mapAssembler struct { schemaType *schema.TypeMap - val reflect.Value // non-pointer + keysVal reflect.Value // non-pointer + valuesVal reflect.Value // non-pointer finish func() error // TODO: more state checks @@ -854,17 +883,21 @@ type _mapAssembler struct { func (w *_mapAssembler) AssembleKey() ipld.NodeAssembler { w.curKey = _assembler{ schemaType: w.schemaType.KeyType(), - val: reflect.New(w.val.Type().Key()).Elem(), + val: reflect.New(w.valuesVal.Type().Key()).Elem(), } return &w.curKey } func (w *_mapAssembler) AssembleValue() ipld.NodeAssembler { kval := w.curKey.val - val := reflect.New(w.val.Type().Elem()).Elem() + val := reflect.New(w.valuesVal.Type().Elem()).Elem() finish := func() error { // fmt.Println(kval.Interface(), val.Interface()) - w.val.SetMapIndex(kval, val) + + // TODO: check for duplicates in keysVal + w.keysVal.Set(reflect.Append(w.keysVal, kval)) + + w.valuesVal.SetMapIndex(kval, val) return nil } return &_assembler{ @@ -893,7 +926,7 @@ func (w *_mapAssembler) Finish() error { } func (w *_mapAssembler) KeyPrototype() ipld.NodePrototype { - return &_prototype{schemaType: w.schemaType.KeyType(), goType: w.val.Type().Key()} + return &_prototype{schemaType: w.schemaType.KeyType(), goType: w.valuesVal.Type().Key()} } func (w *_mapAssembler) ValuePrototype(k string) ipld.NodePrototype { @@ -1050,6 +1083,45 @@ func (w *_structIterator) Done() bool { return w.nextIndex >= len(w.fields) } +type _mapIterator struct { + schemaType *schema.TypeMap + keysVal reflect.Value // non-pointer + valuesVal reflect.Value // non-pointer + nextIndex int + + // these are only used in repr.go + reprEnd int +} + +func (w *_mapIterator) Next() (key, value ipld.Node, _ error) { + if w.Done() { + return nil, nil, ipld.ErrIteratorOverread{} + } + goKey := w.keysVal.Index(w.nextIndex) + val := w.valuesVal.MapIndex(goKey) + w.nextIndex++ + + key = &_node{ + schemaType: w.schemaType.KeyType(), + val: goKey, + } + if w.schemaType.ValueIsNullable() { + if val.IsNil() { + return key, ipld.Null, nil + } + val = val.Elem() + } + node := &_node{ + schemaType: w.schemaType.ValueType(), + val: val, + } + return key, node, nil +} + +func (w *_mapIterator) Done() bool { + return w.nextIndex >= w.keysVal.Len() +} + type _listIterator struct { schemaType *schema.TypeList val reflect.Value // non-pointer @@ -1063,6 +1135,12 @@ func (w *_listIterator) Next() (index int64, value ipld.Node, _ error) { idx := int64(w.nextIndex) val := w.val.Index(w.nextIndex) w.nextIndex++ + if w.schemaType.ValueIsNullable() { + if val.IsNil() { + return idx, ipld.Null, nil + } + val = val.Elem() + } return idx, &_node{schemaType: w.schemaType.ValueType(), val: val}, nil } diff --git a/node/bindnode/schema_test.go b/node/bindnode/schema_test.go new file mode 100644 index 0000000..69ce096 --- /dev/null +++ b/node/bindnode/schema_test.go @@ -0,0 +1,52 @@ +package bindnode_test + +import ( + "strings" + "testing" + + "github.com/ipld/go-ipld-prime" + "github.com/ipld/go-ipld-prime/node/bindnode" + "github.com/ipld/go-ipld-prime/node/tests" + "github.com/ipld/go-ipld-prime/schema" +) + +// For now, we simply run all schema tests with PrototypeOnlySchema. +// In the future, forSchemaTest might return multiple engines. + +func forSchemaTest(name string) []tests.EngineSubtest { + return []tests.EngineSubtest{{ + Engine: &bindEngine{}, + }} +} + +func TestSchema(t *testing.T) { + t.Parallel() + + tests.SchemaTestAll(t, forSchemaTest) +} + +var _ tests.Engine = (*bindEngine)(nil) + +type bindEngine struct { + ts schema.TypeSystem +} + +func (e *bindEngine) Init(t *testing.T, ts schema.TypeSystem) { + e.ts = ts +} + +func (e *bindEngine) PrototypeByName(name string) ipld.NodePrototype { + wantRepr := strings.HasSuffix(name, ".Repr") + if wantRepr { + name = strings.TrimSuffix(name, ".Repr") + } + schemaType := e.ts.TypeByName(name) + if schemaType == nil { + return nil + } + proto := bindnode.PrototypeOnlySchema(schemaType) + if wantRepr { + proto = proto.(bindnode.TypedPrototype).Representation() + } + return proto +} diff --git a/node/tests/schema.go b/node/tests/schema.go new file mode 100644 index 0000000..d7f7c75 --- /dev/null +++ b/node/tests/schema.go @@ -0,0 +1,62 @@ +package tests + +import "testing" + +// use a table instead of a map, to get a consistent order + +var allSchemaTests = []struct { + name string + fn func(*testing.T, Engine) +}{ + {"ListsContainingMaybe", SchemaTestListsContainingMaybe}, + {"ListsContainingLists", SchemaTestListsContainingLists}, + {"MapsContainingMaybe", SchemaTestMapsContainingMaybe}, + {"MapsContainingMaps", SchemaTestMapsContainingMaps}, + {"MapsWithComplexKeys", SchemaTestMapsWithComplexKeys}, + {"String", SchemaTestString}, + {"RequiredFields", SchemaTestRequiredFields}, + {"StructNesting", SchemaTestStructNesting}, + {"StructReprStringjoin", SchemaTestStructReprStringjoin}, + {"StructReprTuple", SchemaTestStructReprTuple}, + {"StructsContainingMaybe", SchemaTestStructsContainingMaybe}, + {"UnionKeyed", SchemaTestUnionKeyed}, + {"UnionKeyedComplexChildren", SchemaTestUnionKeyedComplexChildren}, + {"UnionKeyedReset", SchemaTestUnionKeyedReset}, + {"UnionKinded", SchemaTestUnionKinded}, + {"UnionStringprefix", SchemaTestUnionStringprefix}, +} + +type EngineSubtest struct { + Name string // subtest name + Engine Engine +} + +func SchemaTestAll(t *testing.T, forTest func(name string) []EngineSubtest) { + for _, test := range allSchemaTests { + test := test // do not reuse the range variable + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + subtests := forTest(test.name) + if len(subtests) == 0 { + t.Skip("no engine provided to SchemaTestAll") + } + if len(subtests) == 1 { + sub := subtests[0] + if sub.Name != "" { + t.Fatal("a single engine shouldn't be named") + } + test.fn(t, sub.Engine) + return + } + for _, sub := range subtests { + if sub.Name == "" { + t.Fatal("multiple engines should be named") + } + t.Run(sub.Name, func(t *testing.T) { + test.fn(t, sub.Engine) + }) + } + }) + } +} diff --git a/node/tests/schemaMaps.go b/node/tests/schemaMaps.go index 8faa616..49c6521 100644 --- a/node/tests/schemaMaps.go +++ b/node/tests/schemaMaps.go @@ -156,7 +156,7 @@ func SchemaTestMapsContainingMaps(t *testing.T, engine Engine) { }) }) withNode(must.Node(n.LookupByString("none")), func(n ipld.Node) { - Wish(t, n, ShouldEqual, ipld.Null) + Wish(t, ipld.DeepEqual(n, ipld.Null), ShouldEqual, true) }) _, err := n.LookupByString("miss") Wish(t, err, ShouldBeSameTypeAs, ipld.ErrNotExists{}) diff --git a/node/tests/testcase.go b/node/tests/testcase.go index 4dbfd47..25ee0e9 100644 --- a/node/tests/testcase.go +++ b/node/tests/testcase.go @@ -124,7 +124,7 @@ func (tcase testcase) Test(t *testing.T, np, npr ipld.NodePrototype) { } if n2 != nil { t.Run("type-create and repr-create match", func(t *testing.T) { - Wish(t, n, ShouldEqual, n2) + Wish(t, ipld.DeepEqual(n, n2), ShouldEqual, true) }) } -- GitLab