From 98bab020738ef46298a3599f4f98cff8f332cebf Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Thu, 9 Jun 2022 20:40:41 -0400 Subject: [PATCH 01/12] feat: ipld amend --- datamodel/amender.go | 12 +++ datamodel/node.go | 2 +- node/basicnode/any.go | 95 +++++++++++++------- node/basicnode/list.go | 173 ++++++++++++++++++++++++++++++++++--- node/basicnode/map.go | 119 +++++++++++++++++++++---- node/basicnode/map_test.go | 17 ---- 6 files changed, 340 insertions(+), 78 deletions(-) create mode 100644 datamodel/amender.go diff --git a/datamodel/amender.go b/datamodel/amender.go new file mode 100644 index 00000000..1bd29344 --- /dev/null +++ b/datamodel/amender.go @@ -0,0 +1,12 @@ +package datamodel + +// NodeAmender layers onto NodeBuilder the ability to transform all or part of a Node under construction. +type NodeAmender interface { + NodeBuilder + + // Transform takes in a Node (or a child Node of a recursive node) along with a transformation function that returns + // a new NodeAmender with the transformed results. + // + // Transform returns the previous state of the target Node. + Transform(path Path, transform func(Node) (NodeAmender, error)) (Node, error) +} diff --git a/datamodel/node.go b/datamodel/node.go index 625f472d..969f96ed 100644 --- a/datamodel/node.go +++ b/datamodel/node.go @@ -238,7 +238,7 @@ type NodePrototype interface { // volumes of data, detecting and using this feature can result in significant // performance savings. type NodePrototypeSupportingAmend interface { - AmendingBuilder(base Node) NodeBuilder + AmendingBuilder(base Node) NodeAmender // FUTURE: probably also needs a `AmendingWithout(base Node, filter func(k,v) bool) NodeBuilder`, or similar. // ("deletion" based APIs are also possible but both more complicated in interfaces added, and prone to accidentally quadratic usage.) // FUTURE: there should be some stdlib `Copy` (?) methods that automatically look for this feature, and fallback if absent. diff --git a/node/basicnode/any.go b/node/basicnode/any.go index cc248463..5eba14fb 100644 --- a/node/basicnode/any.go +++ b/node/basicnode/any.go @@ -7,8 +7,9 @@ import ( var ( //_ datamodel.Node = &anyNode{} - _ datamodel.NodePrototype = Prototype__Any{} - _ datamodel.NodeBuilder = &anyBuilder{} + _ datamodel.NodePrototype = Prototype__Any{} + _ datamodel.NodePrototypeSupportingAmend = Prototype__Any{} + _ datamodel.NodeBuilder = &anyBuilder{} //_ datamodel.NodeAssembler = &anyAssembler{} ) @@ -34,8 +35,24 @@ func Chooser(_ datamodel.Link, _ linking.LinkContext) (datamodel.NodePrototype, type Prototype__Any struct{} -func (Prototype__Any) NewBuilder() datamodel.NodeBuilder { - return &anyBuilder{} +func (p Prototype__Any) NewBuilder() datamodel.NodeBuilder { + return p.AmendingBuilder(nil) +} + +// -- NodePrototypeSupportingAmend --> + +func (p Prototype__Any) AmendingBuilder(base datamodel.Node) datamodel.NodeAmender { + ab := &anyBuilder{} + if base != nil { + ab.kind = base.Kind() + if npa, castOk := base.Prototype().(datamodel.NodePrototypeSupportingAmend); castOk { + ab.amender = npa.AmendingBuilder(base) + } else { + // This node could be either scalar or recursive + ab.baseNode = base + } + } + return ab } // -- NodeBuilder --> @@ -57,17 +74,16 @@ type anyBuilder struct { kind datamodel.Kind // Only one of the following ends up being used... - // but we don't know in advance which one, so all are embeded here. + // but we don't know in advance which one, so both are embedded here. // This uses excessive space, but amortizes allocations, and all will be // freed as soon as the builder is done. - // Builders are only used for recursives; - // scalars are simple enough we just do them directly. - // 'scalarNode' may also hold another Node of unknown prototype (possibly not even from this package), + // An amender is only used for amendable nodes, while all non-amendable nodes (both recursives and scalars) are + // stored directly. + // 'baseNode' may also hold another Node of unknown prototype (possibly not even from this package), // in which case this is indicated by 'kind==99'. - mapBuilder plainMap__Builder - listBuilder plainList__Builder - scalarNode datamodel.Node + amender datamodel.NodeAmender + baseNode datamodel.Node } func (nb *anyBuilder) Reset() { @@ -79,16 +95,18 @@ func (nb *anyBuilder) BeginMap(sizeHint int64) (datamodel.MapAssembler, error) { panic("misuse") } nb.kind = datamodel.Kind_Map - nb.mapBuilder.w = &plainMap{} - return nb.mapBuilder.BeginMap(sizeHint) + mapBuilder := Prototype.Map.NewBuilder().(*plainMap__Builder) + nb.amender = mapBuilder + return mapBuilder.BeginMap(sizeHint) } func (nb *anyBuilder) BeginList(sizeHint int64) (datamodel.ListAssembler, error) { if nb.kind != datamodel.Kind_Invalid { panic("misuse") } nb.kind = datamodel.Kind_List - nb.listBuilder.w = &plainList{} - return nb.listBuilder.BeginList(sizeHint) + listBuilder := Prototype.List.NewBuilder().(*plainList__Builder) + nb.amender = listBuilder + return listBuilder.BeginList(sizeHint) } func (nb *anyBuilder) AssignNull() error { if nb.kind != datamodel.Kind_Invalid { @@ -102,7 +120,7 @@ func (nb *anyBuilder) AssignBool(v bool) error { panic("misuse") } nb.kind = datamodel.Kind_Bool - nb.scalarNode = NewBool(v) + nb.baseNode = NewBool(v) return nil } func (nb *anyBuilder) AssignInt(v int64) error { @@ -110,7 +128,7 @@ func (nb *anyBuilder) AssignInt(v int64) error { panic("misuse") } nb.kind = datamodel.Kind_Int - nb.scalarNode = NewInt(v) + nb.baseNode = NewInt(v) return nil } func (nb *anyBuilder) AssignFloat(v float64) error { @@ -118,7 +136,7 @@ func (nb *anyBuilder) AssignFloat(v float64) error { panic("misuse") } nb.kind = datamodel.Kind_Float - nb.scalarNode = NewFloat(v) + nb.baseNode = NewFloat(v) return nil } func (nb *anyBuilder) AssignString(v string) error { @@ -126,7 +144,7 @@ func (nb *anyBuilder) AssignString(v string) error { panic("misuse") } nb.kind = datamodel.Kind_String - nb.scalarNode = NewString(v) + nb.baseNode = NewString(v) return nil } func (nb *anyBuilder) AssignBytes(v []byte) error { @@ -134,7 +152,7 @@ func (nb *anyBuilder) AssignBytes(v []byte) error { panic("misuse") } nb.kind = datamodel.Kind_Bytes - nb.scalarNode = NewBytes(v) + nb.baseNode = NewBytes(v) return nil } func (nb *anyBuilder) AssignLink(v datamodel.Link) error { @@ -142,7 +160,7 @@ func (nb *anyBuilder) AssignLink(v datamodel.Link) error { panic("misuse") } nb.kind = datamodel.Kind_Link - nb.scalarNode = NewLink(v) + nb.baseNode = NewLink(v) return nil } func (nb *anyBuilder) AssignNode(v datamodel.Node) error { @@ -150,7 +168,7 @@ func (nb *anyBuilder) AssignNode(v datamodel.Node) error { panic("misuse") } nb.kind = 99 - nb.scalarNode = v + nb.baseNode = v return nil } func (anyBuilder) Prototype() datamodel.NodePrototype { @@ -158,34 +176,49 @@ func (anyBuilder) Prototype() datamodel.NodePrototype { } func (nb *anyBuilder) Build() datamodel.Node { + if nb.amender != nil { + return nb.amender.Build() + } switch nb.kind { case datamodel.Kind_Invalid: panic("misuse") case datamodel.Kind_Map: - return nb.mapBuilder.Build() + return nb.baseNode case datamodel.Kind_List: - return nb.listBuilder.Build() + return nb.baseNode case datamodel.Kind_Null: return datamodel.Null case datamodel.Kind_Bool: - return nb.scalarNode + return nb.baseNode case datamodel.Kind_Int: - return nb.scalarNode + return nb.baseNode case datamodel.Kind_Float: - return nb.scalarNode + return nb.baseNode case datamodel.Kind_String: - return nb.scalarNode + return nb.baseNode case datamodel.Kind_Bytes: - return nb.scalarNode + return nb.baseNode case datamodel.Kind_Link: - return nb.scalarNode + return nb.baseNode case 99: - return nb.scalarNode + return nb.baseNode default: panic("unreachable") } } +// -- NodeAmender --> + +func (nb *anyBuilder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { + // If `baseNode` is set and supports amendment, apply the transformation. If it doesn't, and the root is being + // replaced, replace it. If the transformation is for a nested node in a non-amendable recursive object, panic. + if nb.amender != nil { + return nb.amender.Transform(path, transform) + } + // `Transform` should never be called for a non-amendable node + panic("misuse") +} + // -- NodeAssembler --> // ... oddly enough, we seem to be able to put off implementing this diff --git a/node/basicnode/list.go b/node/basicnode/list.go index 6f7582bb..8bb0783b 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -1,22 +1,26 @@ package basicnode import ( + "fmt" + "reflect" + "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/node/mixins" ) var ( - _ datamodel.Node = &plainList{} - _ datamodel.NodePrototype = Prototype__List{} - _ datamodel.NodeBuilder = &plainList__Builder{} - _ datamodel.NodeAssembler = &plainList__Assembler{} + _ datamodel.Node = &plainList{} + _ datamodel.NodePrototype = Prototype__List{} + _ datamodel.NodePrototypeSupportingAmend = Prototype__List{} + _ datamodel.NodeBuilder = &plainList__Builder{} + _ datamodel.NodeAssembler = &plainList__Assembler{} ) // plainList is a concrete type that provides a list-kind datamodel.Node. // It can contain any kind of value. // plainList is also embedded in the 'any' struct and usable from there. type plainList struct { - x []datamodel.Node + x []datamodel.NodeAmender } // -- Node interface methods --> @@ -31,17 +35,31 @@ func (plainList) LookupByNode(datamodel.Node) (datamodel.Node, error) { return mixins.List{TypeName: "list"}.LookupByNode(nil) } func (n *plainList) LookupByIndex(idx int64) (datamodel.Node, error) { + if v, err := n.lookupAmenderByIndex(idx); err != nil { + return nil, err + } else { + return v.Build(), nil + } +} +func (n *plainList) lookupAmenderByIndex(idx int64) (datamodel.NodeAmender, error) { if n.Length() <= idx { return nil, datamodel.ErrNotExists{Segment: datamodel.PathSegmentOfInt(idx)} } return n.x[idx], nil } func (n *plainList) LookupBySegment(seg datamodel.PathSegment) (datamodel.Node, error) { + if v, err := n.lookupAmenderBySegment(seg); err != nil { + return nil, err + } else { + return v.Build(), nil + } +} +func (n *plainList) lookupAmenderBySegment(seg datamodel.PathSegment) (datamodel.NodeAmender, error) { idx, err := seg.Index() if err != nil { return nil, datamodel.ErrInvalidSegmentForList{TroubleSegment: seg, Reason: err} } - return n.LookupByIndex(idx) + return n.lookupAmenderByIndex(idx) } func (plainList) MapIterator() datamodel.MapIterator { return nil @@ -89,7 +107,7 @@ func (itr *plainList_ListIterator) Next() (idx int64, v datamodel.Node, _ error) if itr.Done() { return -1, nil, datamodel.ErrIteratorOverread{} } - v = itr.n.x[itr.idx] + v = itr.n.x[itr.idx].Build() idx = int64(itr.idx) itr.idx++ return @@ -102,8 +120,24 @@ func (itr *plainList_ListIterator) Done() bool { type Prototype__List struct{} -func (Prototype__List) NewBuilder() datamodel.NodeBuilder { - return &plainList__Builder{plainList__Assembler{w: &plainList{}}} +func (p Prototype__List) NewBuilder() datamodel.NodeBuilder { + return p.AmendingBuilder(nil) +} + +// -- NodePrototypeSupportingAmend --> + +func (p Prototype__List) AmendingBuilder(base datamodel.Node) datamodel.NodeAmender { + var w *plainList + if base != nil { + if baseList, castOk := base.(*plainList); !castOk { + panic("misuse") + } else { + w = baseList + } + } else { + w = &plainList{} + } + return &plainList__Builder{plainList__Assembler{w: w}} } // -- NodeBuilder --> @@ -113,8 +147,8 @@ type plainList__Builder struct { } func (nb *plainList__Builder) Build() datamodel.Node { - if nb.state != laState_finished { - panic("invalid state: assembler must be 'finished' before Build can be called!") + if (nb.state != laState_initial) && (nb.state != laState_finished) { + panic("invalid state: assembly in progress must be 'finished' before Build can be called!") } return nb.w } @@ -123,6 +157,119 @@ func (nb *plainList__Builder) Reset() { nb.w = &plainList{} } +// -- NodeAmender --> + +func (nb *plainList__Builder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { + // Can only transform the root of the node or an immediate child. + if path.Len() > 2 { + panic("misuse") + } else + // Allow the root of the node to be replaced. + if path.Len() == 0 { + prevNode := nb.Build() + if newNode, err := transform(prevNode); err != nil { + return nil, err + } else if newLb, castOk := newNode.(*plainList__Builder); !castOk { + return nil, fmt.Errorf("transform: cannot transform root into incompatible type: %v", reflect.TypeOf(newLb)) + } else { + *nb.w = *newLb.w + return prevNode, nil + } + } + childSeg, _ := path.Shift() + childIdx, err := childSeg.Index() + var childAmender datamodel.NodeAmender + if err != nil { + if childSeg.String() == "-" { + // "-" indicates appending a new element to the end of the list. + childIdx = nb.w.Length() + } else { + return nil, datamodel.ErrInvalidSegmentForList{TroubleSegment: childSeg, Reason: err} + } + } else { + // Don't allow the index to be equal to the length if the segment was not "-". + if childIdx >= nb.w.Length() { + return nil, fmt.Errorf("transform: cannot navigate path segment %q at %q because it is beyond the list bounds", childSeg, path) + } + // Only lookup the segment if it was within range of the list elements. If `childIdx` is equal to the length of + // the list, then we fall-through and append an element to the end of the list. + childAmender, err = nb.w.lookupAmenderByIndex(childIdx) + if err != nil { + // Return any error other than "not exists" + if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { + return nil, fmt.Errorf("transform: child at %q did not exist)", path) + } + } + } + // The default behaviour will be to update the element at the specified index (if it exists). New list elements can + // be added in two cases: + // - If an element is being appended to the end of the list. + // - If the transformation of the target node results in a list of nodes, use the first node in the list to replace + // the target node and then "add" the rest after. This is a bit of an ugly hack but is required for compatibility + // with two conflicting sets of semantics - the current `FocusedTransform`, which (quite reasonably) does an + // in-place replacement of list elements, and JSON Patch (https://datatracker.ietf.org/doc/html/rfc6902), which + // does not specify list element replacement. The only "compliant" way to do this today is to first "remove" the + // target node and then "add" its replacement at the same index, which seems inefficient. + var prevChildVal datamodel.Node = nil + if childAmender != nil { + prevChildVal = childAmender.Build() + } + if newChildVal, err := transform(prevChildVal); err != nil { + return nil, err + } else if newChildVal == nil { + newX := make([]datamodel.NodeAmender, nb.w.Length()-1) + copy(newX, nb.w.x[:childIdx]) + copy(newX[:childIdx], nb.w.x[childIdx+1:]) + nb.w.x = newX + } else if err = nb.storeChildAmender(childIdx, newChildVal); err != nil { + return nil, err + } + return prevChildVal, nil +} + +func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.NodeAmender) error { + var elems []datamodel.NodeAmender + n := a.Build() + if (n.Kind() == datamodel.Kind_List) && (n.Length() > 0) { + elems = make([]datamodel.NodeAmender, n.Length()) + // The following logic uses a transformed list (if there is one) to perform both insertions (needed by JSON + // Patch) and replacements (needed by `FocusedTransform`), while also providing the flexibility to insert more + // than one element at a particular index in the list. + // + // Rules: + // - If appending to the end of the main list, all elements from the transformed list should be considered + // "created" because they did not exist before. + // - If updating at a particular index in the main list, however, use the first element from the transformed + // list to replace the existing element at that index in the main list, then insert the rest of the + // transformed list elements after. + // + // A special case to consider is that of a list element genuinely being a list itself. If that is the case, the + // transformation MUST wrap the element in another list so that, once unwrapped, the element can be replaced or + // inserted without affecting its semantics. Otherwise, the sub-list's elements will get expanded onto that + // index in the main list. + for i := range elems { + elem, err := n.LookupByIndex(int64(i)) + if err != nil { + return err + } + elems[i] = Prototype.Any.AmendingBuilder(elem) + } + } else { + elems = []datamodel.NodeAmender{Prototype.Any.AmendingBuilder(n)} + } + if childIdx == nb.w.Length() { + nb.w.x = append(nb.w.x, elems...) + } else { + numElems := int64(len(elems)) + newX := make([]datamodel.NodeAmender, nb.w.Length()+numElems-1) + copy(newX, nb.w.x[:childIdx]) + copy(newX[childIdx:], elems) + copy(newX[childIdx+numElems:], nb.w.x[childIdx+1:]) + nb.w.x = newX + } + return nil +} + // -- NodeAssembler --> type plainList__Assembler struct { @@ -155,7 +302,7 @@ func (na *plainList__Assembler) BeginList(sizeHint int64) (datamodel.ListAssembl sizeHint = 0 } // Allocate storage space. - na.w.x = make([]datamodel.Node, 0, sizeHint) + na.w.x = make([]datamodel.NodeAmender, 0, sizeHint) // That's it; return self as the ListAssembler. We already have all the right methods on this structure. return na, nil } @@ -291,7 +438,7 @@ func (lva *plainList__ValueAssembler) AssignLink(v datamodel.Link) error { return lva.AssignNode(&vb) } func (lva *plainList__ValueAssembler) AssignNode(v datamodel.Node) error { - lva.la.w.x = append(lva.la.w.x, v) + lva.la.w.x = append(lva.la.w.x, Prototype.Any.AmendingBuilder(v)) lva.la.state = laState_initial lva.la = nil // invalidate self to prevent further incorrect use. return nil diff --git a/node/basicnode/map.go b/node/basicnode/map.go index 9a86fc52..b3c549c0 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -2,29 +2,31 @@ package basicnode import ( "fmt" + "reflect" "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/node/mixins" ) var ( - _ datamodel.Node = &plainMap{} - _ datamodel.NodePrototype = Prototype__Map{} - _ datamodel.NodeBuilder = &plainMap__Builder{} - _ datamodel.NodeAssembler = &plainMap__Assembler{} + _ datamodel.Node = &plainMap{} + _ datamodel.NodePrototype = Prototype__Map{} + _ datamodel.NodePrototypeSupportingAmend = Prototype__Map{} + _ datamodel.NodeAmender = &plainMap__Builder{} + _ datamodel.NodeAssembler = &plainMap__Assembler{} ) // plainMap is a concrete type that provides a map-kind datamodel.Node. // It can contain any kind of value. // plainMap is also embedded in the 'any' struct and usable from there. type plainMap struct { - m map[string]datamodel.Node // string key -- even if a runtime schema wrapper is using us for storage, we must have a comparable type here, and string is all we know. - t []plainMap__Entry // table for fast iteration, order keeping, and yielding pointers to enable alloc/conv amortization. + m map[string]datamodel.NodeAmender // string key -- even if a runtime schema wrapper is using us for storage, we must have a comparable type here, and string is all we know. + t []plainMap__Entry // table for fast iteration, order keeping, and yielding pointers to enable alloc/conv amortization. } type plainMap__Entry struct { - k plainString // address of this used when we return keys as nodes, such as in iterators. Need in one place to amortize shifts to heap when ptr'ing for iface. - v datamodel.Node // identical to map values. keeping them here simplifies iteration. (in codegen'd maps, this position is also part of amortization, but in this implementation, that's less useful.) + k plainString // address of this used when we return keys as nodes, such as in iterators. Need in one place to amortize shifts to heap when ptr'ing for iface. + v datamodel.NodeAmender // identical to map values. keeping them here simplifies iteration. (in codegen'd maps, this position is also part of amortization, but in this implementation, that's less useful.) // note on alternate implementations: 'v' could also use the 'any' type, and thus amortize value allocations. the memory size trade would be large however, so we don't, here. } @@ -34,6 +36,13 @@ func (plainMap) Kind() datamodel.Kind { return datamodel.Kind_Map } func (n *plainMap) LookupByString(key string) (datamodel.Node, error) { + if a, err := n.lookupAmenderByString(key); err != nil { + return nil, err + } else { + return a.Build(), nil + } +} +func (n *plainMap) lookupAmenderByString(key string) (datamodel.NodeAmender, error) { v, exists := n.m[key] if !exists { return nil, datamodel.ErrNotExists{Segment: datamodel.PathSegmentOfString(key)} @@ -100,7 +109,7 @@ func (itr *plainMap_MapIterator) Next() (k datamodel.Node, v datamodel.Node, _ e return nil, nil, datamodel.ErrIteratorOverread{} } k = &itr.n.t[itr.idx].k - v = itr.n.t[itr.idx].v + v = itr.n.t[itr.idx].v.Build() itr.idx++ return } @@ -112,8 +121,24 @@ func (itr *plainMap_MapIterator) Done() bool { type Prototype__Map struct{} -func (Prototype__Map) NewBuilder() datamodel.NodeBuilder { - return &plainMap__Builder{plainMap__Assembler{w: &plainMap{}}} +func (p Prototype__Map) NewBuilder() datamodel.NodeBuilder { + return p.AmendingBuilder(nil) +} + +// -- NodePrototypeSupportingAmend --> + +func (p Prototype__Map) AmendingBuilder(base datamodel.Node) datamodel.NodeAmender { + var b *plainMap + if base != nil { + if baseMap, castOk := base.(*plainMap); !castOk { + panic("misuse") + } else { + b = baseMap + } + } else { + b = &plainMap{} + } + return &plainMap__Builder{plainMap__Assembler{w: b}} } // -- NodeBuilder --> @@ -123,8 +148,8 @@ type plainMap__Builder struct { } func (nb *plainMap__Builder) Build() datamodel.Node { - if nb.state != maState_finished { - panic("invalid state: assembler must be 'finished' before Build can be called!") + if (nb.state != maState_initial) && (nb.state != maState_finished) { + panic("invalid state: assembly in progress must be 'finished' before Build can be called!") } return nb.w } @@ -133,6 +158,67 @@ func (nb *plainMap__Builder) Reset() { nb.w = &plainMap{} } +// -- NodeAmender --> + +func (nb *plainMap__Builder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { + // Can only transform the root of the node or an immediate child. + if path.Len() > 2 { + panic("misuse") + } else + // Allow the root of the node to be replaced. + if path.Len() == 0 { + prevNode := nb.Build() + if newNb, err := transform(prevNode); err != nil { + return nil, err + } else if newMb, castOk := newNb.(*plainMap__Builder); !castOk { + return nil, fmt.Errorf("transform: cannot transform root into incompatible type: %v", reflect.TypeOf(newNb)) + } else { + *nb.w = *newMb.w + return prevNode, nil + } + } + childSeg, _ := path.Shift() + childKey := childSeg.String() + childAmender, err := nb.w.lookupAmenderByString(childKey) + if err != nil { + // Return any error other than "not exists" + if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { + return nil, fmt.Errorf("transform: child at %q did not exist)", path) + } + } + // Allocate storage space + if nb.w.m == nil { + nb.w.t = make([]plainMap__Entry, 0, 1) + nb.w.m = make(map[string]datamodel.NodeAmender, 1) + } + var prevChildVal datamodel.Node = nil + if childAmender != nil { + prevChildVal = childAmender.Build() + } + if newChildAmender, err := transform(prevChildVal); err != nil { + return nil, err + } else { + for idx, v := range nb.w.t { + if string(v.k) == childKey { + if newChildAmender == nil { + delete(nb.w.m, childKey) + newT := make([]plainMap__Entry, nb.w.Length()-1) + copy(newT, nb.w.t[:idx]) + copy(newT[:idx], nb.w.t[idx+1:]) + nb.w.t = newT + } else { + nb.w.t[idx].v = newChildAmender + nb.w.m[string(nb.w.t[idx].k)] = newChildAmender + } + return prevChildVal, nil + } + } + nb.w.t = append(nb.w.t, plainMap__Entry{plainString(childKey), newChildAmender}) + nb.w.m[childKey] = newChildAmender + return prevChildVal, nil + } +} + // -- NodeAssembler --> type plainMap__Assembler struct { @@ -168,7 +254,7 @@ func (na *plainMap__Assembler) BeginMap(sizeHint int64) (datamodel.MapAssembler, } // Allocate storage space. na.w.t = make([]plainMap__Entry, 0, sizeHint) - na.w.m = make(map[string]datamodel.Node, sizeHint) + na.w.m = make(map[string]datamodel.NodeAmender, sizeHint) // That's it; return self as the MapAssembler. We already have all the right methods on this structure. return na, nil } @@ -404,8 +490,9 @@ func (mva *plainMap__ValueAssembler) AssignLink(v datamodel.Link) error { } func (mva *plainMap__ValueAssembler) AssignNode(v datamodel.Node) error { l := len(mva.ma.w.t) - 1 - mva.ma.w.t[l].v = v - mva.ma.w.m[string(mva.ma.w.t[l].k)] = v + a := Prototype.Any.AmendingBuilder(v) + mva.ma.w.t[l].v = a + mva.ma.w.m[string(mva.ma.w.t[l].k)] = a mva.ma.state = maState_initial mva.ma = nil // invalidate self to prevent further incorrect use. return nil diff --git a/node/basicnode/map_test.go b/node/basicnode/map_test.go index 03e17d9d..404f5dfd 100644 --- a/node/basicnode/map_test.go +++ b/node/basicnode/map_test.go @@ -258,14 +258,6 @@ func TestMapLookupError(t *testing.T) { } func TestMapNewBuilderUsageError(t *testing.T) { - qt.Assert(t, - func() { - b := basicnode.Prototype.Map.NewBuilder() - _ = b.Build() - }, - qt.PanicMatches, - `invalid state: assembler must be 'finished' before Build can be called!`) - // construct an empty map b := basicnode.Prototype.Map.NewBuilder() ma, err := b.BeginMap(0) @@ -282,15 +274,6 @@ func TestMapNewBuilderUsageError(t *testing.T) { expect := `map{}` qt.Check(t, expect, qt.Equals, actual) - // reset will return the state to 'initial', so Build will panic once again - b.Reset() - qt.Assert(t, - func() { - _ = b.Build() - }, - qt.PanicMatches, - `invalid state: assembler must be 'finished' before Build can be called!`) - // assembling a key without a value will cause Finish to panic b.Reset() ma, err = b.BeginMap(0) From 5c2edf3ec2ccfa0121d08ff6f39bb286a57fce70 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Tue, 4 Jul 2023 22:46:40 -0400 Subject: [PATCH 02/12] feat: integrate walk <-> amend --- datamodel/amender.go | 8 ++- node/basicnode/any.go | 2 +- node/basicnode/list.go | 6 +- node/basicnode/map.go | 6 +- traversal/walk.go | 134 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 147 insertions(+), 9 deletions(-) diff --git a/datamodel/amender.go b/datamodel/amender.go index 1bd29344..e9b0bd2f 100644 --- a/datamodel/amender.go +++ b/datamodel/amender.go @@ -1,6 +1,10 @@ package datamodel -// NodeAmender layers onto NodeBuilder the ability to transform all or part of a Node under construction. +// AmendFn takes a Node and returns a NodeAmender that stores any applied transformations. The returned NodeAmender +// allows further transformations to be applied to the Node under construction. +type AmendFn func(Node) (NodeAmender, error) + +// NodeAmender adds to NodeBuilder the ability to transform all or part of a Node under construction. type NodeAmender interface { NodeBuilder @@ -8,5 +12,5 @@ type NodeAmender interface { // a new NodeAmender with the transformed results. // // Transform returns the previous state of the target Node. - Transform(path Path, transform func(Node) (NodeAmender, error)) (Node, error) + Transform(path Path, transform AmendFn) (Node, error) } diff --git a/node/basicnode/any.go b/node/basicnode/any.go index 5eba14fb..5334d56e 100644 --- a/node/basicnode/any.go +++ b/node/basicnode/any.go @@ -209,7 +209,7 @@ func (nb *anyBuilder) Build() datamodel.Node { // -- NodeAmender --> -func (nb *anyBuilder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { +func (nb *anyBuilder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { // If `baseNode` is set and supports amendment, apply the transformation. If it doesn't, and the root is being // replaced, replace it. If the transformation is for a nested node in a non-amendable recursive object, panic. if nb.amender != nil { diff --git a/node/basicnode/list.go b/node/basicnode/list.go index 8bb0783b..bf2b1330 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -159,11 +159,11 @@ func (nb *plainList__Builder) Reset() { // -- NodeAmender --> -func (nb *plainList__Builder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { +func (nb *plainList__Builder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { // Can only transform the root of the node or an immediate child. - if path.Len() > 2 { + if path.Len() > 1 { panic("misuse") - } else + } // Allow the root of the node to be replaced. if path.Len() == 0 { prevNode := nb.Build() diff --git a/node/basicnode/map.go b/node/basicnode/map.go index b3c549c0..e35bf861 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -160,11 +160,11 @@ func (nb *plainMap__Builder) Reset() { // -- NodeAmender --> -func (nb *plainMap__Builder) Transform(path datamodel.Path, transform func(datamodel.Node) (datamodel.NodeAmender, error)) (datamodel.Node, error) { +func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { // Can only transform the root of the node or an immediate child. - if path.Len() > 2 { + if path.Len() > 1 { panic("misuse") - } else + } // Allow the root of the node to be replaced. if path.Len() == 0 { prevNode := nb.Build() diff --git a/traversal/walk.go b/traversal/walk.go index 1bd4e6c2..ff7805e4 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -7,6 +7,7 @@ import ( "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/linking" "github.com/ipld/go-ipld-prime/linking/preload" + "github.com/ipld/go-ipld-prime/node/basicnode" "github.com/ipld/go-ipld-prime/traversal/selector" ) @@ -495,8 +496,14 @@ func (prog Progress) walkTransforming(n datamodel.Node, s selector.Selector, fn nk := n.Kind() switch nk { case datamodel.Kind_List: + if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingAmend); castOk { + return prog.walk_transform_iterateAmendableList(n, s, fn, s.Interests()) + } return prog.walk_transform_iterateList(n, s, fn, s.Interests()) case datamodel.Kind_Map: + if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingAmend); castOk { + return prog.walk_transform_iterateAmendableMap(n, s, fn, s.Interests()) + } return prog.walk_transform_iterateMap(n, s, fn, s.Interests()) default: return n, nil @@ -574,6 +581,69 @@ func (prog Progress) walk_transform_iterateList(n datamodel.Node, s selector.Sel return bldr.Build(), nil } +func (prog Progress) walk_transform_iterateAmendableList(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { + amender := n.Prototype().(datamodel.NodePrototypeSupportingAmend).AmendingBuilder(n) + + transform := func(ps datamodel.PathSegment, node datamodel.Node) error { + _, err := amender.Transform(datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { + return basicnode.Prototype.Any.AmendingBuilder(node), nil + }) + return err + } + + for itr := selector.NewSegmentIterator(n); !itr.Done(); { + ps, v, err := itr.Next() + if err != nil { + return nil, err + } + if attn == nil || contains(attn, ps) { + sNext, err := s.Explore(n, ps) + if err != nil { + return nil, err + } + if sNext != nil { + progNext := prog + progNext.Path = prog.Path.AppendSegment(ps) + if v.Kind() == datamodel.Kind_Link { + lnk, _ := v.AsLink() + if prog.Cfg.LinkVisitOnlyOnce { + if _, seen := prog.SeenLinks[lnk]; seen { + continue + } + prog.SeenLinks[lnk] = struct{}{} + } + progNext.LastBlock.Path = progNext.Path + progNext.LastBlock.Link = lnk + v, err = progNext.loadLink(lnk, v, n) + if err != nil { + if _, ok := err.(SkipMe); ok { + continue + } + return nil, err + } + } + + next, err := progNext.WalkTransforming(v, sNext, fn) + if err != nil { + return nil, err + } + if err := transform(ps, next); err != nil { + return nil, err + } + } else { + if err := transform(ps, v); err != nil { + return nil, err + } + } + } else { + if err := transform(ps, v); err != nil { + return nil, err + } + } + } + return amender.Build(), nil +} + func (prog Progress) walk_transform_iterateMap(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { bldr := n.Prototype().NewBuilder() mapBldr, err := bldr.BeginMap(n.Length()) @@ -640,3 +710,67 @@ func (prog Progress) walk_transform_iterateMap(n datamodel.Node, s selector.Sele } return bldr.Build(), nil } + +func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { + amender := n.Prototype().(datamodel.NodePrototypeSupportingAmend).AmendingBuilder(n) + + transform := func(ps datamodel.PathSegment, node datamodel.Node) error { + _, err := amender.Transform(datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { + return basicnode.Prototype.Any.AmendingBuilder(node), nil + }) + return err + } + + for itr := selector.NewSegmentIterator(n); !itr.Done(); { + ps, v, err := itr.Next() + if err != nil { + return nil, err + } + + if attn == nil || contains(attn, ps) { + sNext, err := s.Explore(n, ps) + if err != nil { + return nil, err + } + if sNext != nil { + progNext := prog + progNext.Path = prog.Path.AppendSegment(ps) + if v.Kind() == datamodel.Kind_Link { + lnk, _ := v.AsLink() + if prog.Cfg.LinkVisitOnlyOnce { + if _, seen := prog.SeenLinks[lnk]; seen { + continue + } + prog.SeenLinks[lnk] = struct{}{} + } + progNext.LastBlock.Path = progNext.Path + progNext.LastBlock.Link = lnk + v, err = progNext.loadLink(lnk, v, n) + if err != nil { + if _, ok := err.(SkipMe); ok { + continue + } + return nil, err + } + } + + next, err := progNext.WalkTransforming(v, sNext, fn) + if err != nil { + return nil, err + } + if err := transform(ps, next); err != nil { + return nil, err + } + } else { + if err := transform(ps, v); err != nil { + return nil, err + } + } + } else { + if err := transform(ps, v); err != nil { + return nil, err + } + } + } + return amender.Build(), nil +} From 8984f185fd105e8a89f29b0d94777f06fdc74ed7 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Thu, 6 Jul 2023 14:38:36 -0400 Subject: [PATCH 03/12] feat: add map/list amender interfaces --- datamodel/amender.go | 31 ++++++++++++ datamodel/node.go | 12 +++++ node/basicnode/list.go | 106 +++++++++++++++++++++++++++++++++++------ node/basicnode/map.go | 88 ++++++++++++++++++++++++++++++---- traversal/walk.go | 43 +++++++---------- 5 files changed, 231 insertions(+), 49 deletions(-) diff --git a/datamodel/amender.go b/datamodel/amender.go index e9b0bd2f..351455bf 100644 --- a/datamodel/amender.go +++ b/datamodel/amender.go @@ -14,3 +14,34 @@ type NodeAmender interface { // Transform returns the previous state of the target Node. Transform(path Path, transform AmendFn) (Node, error) } + +// containerAmender is an internal type for representing the interface for amendable containers (like maps and lists) +type containerAmender interface { + Empty() bool + Length() int64 + Clear() + Values() ([]Node, error) + + NodeAmender +} + +// MapAmender adds a map-like interface to NodeAmender +type MapAmender interface { + Put(key string, value Node) (bool, error) + Get(key string) (Node, error) + Remove(key string) (bool, error) + Keys() ([]string, error) + + containerAmender +} + +// ListAmender adds a list-like interface to NodeAmender +type ListAmender interface { + Get(idx int64) (Node, error) + Remove(idx int64) error + Append(values ...Node) error + Insert(idx int64, values ...Node) error + Set(idx int64, value Node) error + + containerAmender +} diff --git a/datamodel/node.go b/datamodel/node.go index 969f96ed..ee500d1d 100644 --- a/datamodel/node.go +++ b/datamodel/node.go @@ -246,6 +246,18 @@ type NodePrototypeSupportingAmend interface { // FUTURE: consider putting this (and others like it) in a `feature` package, if there begin to be enough of them and docs get crowded. } +// NodePrototypeSupportingMapAmend is a feature-detection interface that can be used on a NodePrototype to see if it's +// possible to update existing map-like nodes of this style. +type NodePrototypeSupportingMapAmend interface { + AmendingBuilder(base Node) MapAmender +} + +// NodePrototypeSupportingListAmend is a feature-detection interface that can be used on a NodePrototype to see if it's +// possible to update existing list-like nodes of this style. +type NodePrototypeSupportingListAmend interface { + AmendingBuilder(base Node) ListAmender +} + // MapIterator is an interface for traversing map nodes. // Sequential calls to Next() will yield key-value pairs; // Done() describes whether iteration should continue. diff --git a/node/basicnode/list.go b/node/basicnode/list.go index bf2b1330..cdd45eaf 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -9,11 +9,11 @@ import ( ) var ( - _ datamodel.Node = &plainList{} - _ datamodel.NodePrototype = Prototype__List{} - _ datamodel.NodePrototypeSupportingAmend = Prototype__List{} - _ datamodel.NodeBuilder = &plainList__Builder{} - _ datamodel.NodeAssembler = &plainList__Assembler{} + _ datamodel.Node = &plainList{} + _ datamodel.NodePrototype = Prototype__List{} + _ datamodel.NodePrototypeSupportingListAmend = Prototype__List{} + _ datamodel.NodeBuilder = &plainList__Builder{} + _ datamodel.NodeAssembler = &plainList__Assembler{} ) // plainList is a concrete type that provides a list-kind datamodel.Node. @@ -124,9 +124,9 @@ func (p Prototype__List) NewBuilder() datamodel.NodeBuilder { return p.AmendingBuilder(nil) } -// -- NodePrototypeSupportingAmend --> +// -- NodePrototypeSupportingListAmend --> -func (p Prototype__List) AmendingBuilder(base datamodel.Node) datamodel.NodeAmender { +func (p Prototype__List) AmendingBuilder(base datamodel.Node) datamodel.ListAmender { var w *plainList if base != nil { if baseList, castOk := base.(*plainList); !castOk { @@ -206,7 +206,7 @@ func (nb *plainList__Builder) Transform(path datamodel.Path, transform datamodel // - If an element is being appended to the end of the list. // - If the transformation of the target node results in a list of nodes, use the first node in the list to replace // the target node and then "add" the rest after. This is a bit of an ugly hack but is required for compatibility - // with two conflicting sets of semantics - the current `FocusedTransform`, which (quite reasonably) does an + // with two conflicting sets of semantics - the current `focus` and `walk`, which (quite reasonably) do an // in-place replacement of list elements, and JSON Patch (https://datatracker.ietf.org/doc/html/rfc6902), which // does not specify list element replacement. The only "compliant" way to do this today is to first "remove" the // target node and then "add" its replacement at the same index, which seems inefficient. @@ -233,15 +233,15 @@ func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.Node if (n.Kind() == datamodel.Kind_List) && (n.Length() > 0) { elems = make([]datamodel.NodeAmender, n.Length()) // The following logic uses a transformed list (if there is one) to perform both insertions (needed by JSON - // Patch) and replacements (needed by `FocusedTransform`), while also providing the flexibility to insert more + // Patch) and replacements (needed by `focus` and `walk`), while also providing the flexibility to insert more // than one element at a particular index in the list. // // Rules: - // - If appending to the end of the main list, all elements from the transformed list should be considered - // "created" because they did not exist before. - // - If updating at a particular index in the main list, however, use the first element from the transformed - // list to replace the existing element at that index in the main list, then insert the rest of the - // transformed list elements after. + // - If appending to the end of the main list, all elements from the transformed list will be individually + // appended to the end of the list. + // - If updating at a particular index in the main list, use the first element from the transformed list to + // replace the existing element at that index in the main list, then insert the rest of the transformed list + // elements after. // // A special case to consider is that of a list element genuinely being a list itself. If that is the case, the // transformation MUST wrap the element in another list so that, once unwrapped, the element can be replaced or @@ -270,6 +270,84 @@ func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.Node return nil } +func (nb *plainList__Builder) Get(idx int64) (datamodel.Node, error) { + return nb.w.LookupByIndex(idx) +} + +func (nb *plainList__Builder) Remove(idx int64) error { + _, err := nb.Transform( + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfInt(idx)}), + func(_ datamodel.Node) (datamodel.NodeAmender, error) { + return nil, nil + }, + ) + return err +} + +func (nb *plainList__Builder) Append(values ...datamodel.Node) error { + // Passing an index equal to the length of the list will append the passed values to the end of the list + return nb.Insert(nb.Length(), values...) +} + +func (nb *plainList__Builder) Insert(idx int64, values ...datamodel.Node) error { + var ps datamodel.PathSegment + if idx == nb.Length() { + ps = datamodel.PathSegmentOfString("-") // indicates appending to the end of the list + } else { + ps = datamodel.PathSegmentOfInt(idx) + } + _, err := nb.Transform( + datamodel.NewPath([]datamodel.PathSegment{ps}), + func(_ datamodel.Node) (datamodel.NodeAmender, error) { + // Put all the passed values into a new list. That will result in these values being expanded at the + // specified index. + na := nb.Prototype().NewBuilder() + la, err := na.BeginList(int64(len(values))) + if err != nil { + return nil, err + } + for _, v := range values { + if err := la.AssembleValue().AssignNode(v); err != nil { + return nil, err + } + } + if err := la.Finish(); err != nil { + return nil, err + } + return Prototype.Any.AmendingBuilder(na.Build()), nil + }, + ) + return err +} + +func (nb *plainList__Builder) Set(idx int64, value datamodel.Node) error { + return nb.Insert(idx, value) +} + +func (nb *plainList__Builder) Empty() bool { + return nb.Length() == 0 +} + +func (nb *plainList__Builder) Length() int64 { + return nb.w.Length() +} + +func (nb *plainList__Builder) Clear() { + nb.Reset() +} + +func (nb *plainList__Builder) Values() ([]datamodel.Node, error) { + values := make([]datamodel.Node, 0, nb.Length()) + for itr := nb.w.ListIterator(); !itr.Done(); { + _, v, err := itr.Next() + if err != nil { + return nil, err + } + values = append(values, v) + } + return values, nil +} + // -- NodeAssembler --> type plainList__Assembler struct { diff --git a/node/basicnode/map.go b/node/basicnode/map.go index e35bf861..09413c52 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -9,11 +9,11 @@ import ( ) var ( - _ datamodel.Node = &plainMap{} - _ datamodel.NodePrototype = Prototype__Map{} - _ datamodel.NodePrototypeSupportingAmend = Prototype__Map{} - _ datamodel.NodeAmender = &plainMap__Builder{} - _ datamodel.NodeAssembler = &plainMap__Assembler{} + _ datamodel.Node = &plainMap{} + _ datamodel.NodePrototype = Prototype__Map{} + _ datamodel.NodePrototypeSupportingMapAmend = Prototype__Map{} + _ datamodel.NodeAmender = &plainMap__Builder{} + _ datamodel.NodeAssembler = &plainMap__Assembler{} ) // plainMap is a concrete type that provides a map-kind datamodel.Node. @@ -125,9 +125,9 @@ func (p Prototype__Map) NewBuilder() datamodel.NodeBuilder { return p.AmendingBuilder(nil) } -// -- NodePrototypeSupportingAmend --> +// -- NodePrototypeSupportingMapAmend --> -func (p Prototype__Map) AmendingBuilder(base datamodel.Node) datamodel.NodeAmender { +func (p Prototype__Map) AmendingBuilder(base datamodel.Node) datamodel.MapAmender { var b *plainMap if base != nil { if baseMap, castOk := base.(*plainMap); !castOk { @@ -158,7 +158,7 @@ func (nb *plainMap__Builder) Reset() { nb.w = &plainMap{} } -// -- NodeAmender --> +// -- MapAmender --> func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { // Can only transform the root of the node or an immediate child. @@ -219,6 +219,78 @@ func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel. } } +func (nb *plainMap__Builder) Put(key string, value datamodel.Node) (bool, error) { + if prevNode, err := nb.Transform( + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), + func(_ datamodel.Node) (datamodel.NodeAmender, error) { + return Prototype.Any.AmendingBuilder(value), nil + }, + ); err != nil { + return false, err + } else { + // If there was no previous node, we just added a new node. + return prevNode == nil, nil + } +} + +func (nb *plainMap__Builder) Get(key string) (datamodel.Node, error) { + return nb.w.LookupByString(key) +} + +func (nb *plainMap__Builder) Remove(key string) (bool, error) { + if prevNode, err := nb.Transform( + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), + func(_ datamodel.Node) (datamodel.NodeAmender, error) { + return nil, nil + }, + ); err != nil { + return false, err + } else { + // If there was a previous node, we just removed it. + return prevNode != nil, nil + } +} + +func (nb *plainMap__Builder) Keys() ([]string, error) { + keys := make([]string, 0, nb.Length()) + for itr := nb.w.MapIterator(); !itr.Done(); { + k, _, err := itr.Next() + if err != nil { + return nil, err + } + keyStr, err := k.AsString() + if err != nil { + return nil, err + } + keys = append(keys, keyStr) + } + return keys, nil +} + +func (nb *plainMap__Builder) Empty() bool { + return nb.Length() == 0 +} + +func (nb *plainMap__Builder) Length() int64 { + return nb.w.Length() +} + +func (nb *plainMap__Builder) Clear() { + nb.Reset() +} + +func (nb *plainMap__Builder) Values() ([]datamodel.Node, error) { + values := make([]datamodel.Node, 0, nb.Length()) + for itr := nb.w.MapIterator(); !itr.Done(); { + _, v, err := itr.Next() + if err != nil { + return nil, err + } + values = append(values, v) + } + return values, nil +} + // -- NodeAssembler --> type plainMap__Assembler struct { diff --git a/traversal/walk.go b/traversal/walk.go index ff7805e4..812ee47f 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -7,7 +7,6 @@ import ( "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/linking" "github.com/ipld/go-ipld-prime/linking/preload" - "github.com/ipld/go-ipld-prime/node/basicnode" "github.com/ipld/go-ipld-prime/traversal/selector" ) @@ -496,12 +495,12 @@ func (prog Progress) walkTransforming(n datamodel.Node, s selector.Selector, fn nk := n.Kind() switch nk { case datamodel.Kind_List: - if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingAmend); castOk { + if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingListAmend); castOk { return prog.walk_transform_iterateAmendableList(n, s, fn, s.Interests()) } return prog.walk_transform_iterateList(n, s, fn, s.Interests()) case datamodel.Kind_Map: - if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingAmend); castOk { + if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend); castOk { return prog.walk_transform_iterateAmendableMap(n, s, fn, s.Interests()) } return prog.walk_transform_iterateMap(n, s, fn, s.Interests()) @@ -582,20 +581,17 @@ func (prog Progress) walk_transform_iterateList(n datamodel.Node, s selector.Sel } func (prog Progress) walk_transform_iterateAmendableList(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { - amender := n.Prototype().(datamodel.NodePrototypeSupportingAmend).AmendingBuilder(n) - - transform := func(ps datamodel.PathSegment, node datamodel.Node) error { - _, err := amender.Transform(datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { - return basicnode.Prototype.Any.AmendingBuilder(node), nil - }) - return err - } + listAmender := n.Prototype().(datamodel.NodePrototypeSupportingListAmend).AmendingBuilder(n) for itr := selector.NewSegmentIterator(n); !itr.Done(); { ps, v, err := itr.Next() if err != nil { return nil, err } + idx, err := ps.Index() + if err != nil { + return nil, err + } if attn == nil || contains(attn, ps) { sNext, err := s.Explore(n, ps) if err != nil { @@ -627,21 +623,21 @@ func (prog Progress) walk_transform_iterateAmendableList(n datamodel.Node, s sel if err != nil { return nil, err } - if err := transform(ps, next); err != nil { + if err := listAmender.Set(idx, next); err != nil { return nil, err } } else { - if err := transform(ps, v); err != nil { + if err := listAmender.Set(idx, v); err != nil { return nil, err } } } else { - if err := transform(ps, v); err != nil { + if err := listAmender.Set(idx, v); err != nil { return nil, err } } } - return amender.Build(), nil + return listAmender.Build(), nil } func (prog Progress) walk_transform_iterateMap(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { @@ -712,14 +708,7 @@ func (prog Progress) walk_transform_iterateMap(n datamodel.Node, s selector.Sele } func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { - amender := n.Prototype().(datamodel.NodePrototypeSupportingAmend).AmendingBuilder(n) - - transform := func(ps datamodel.PathSegment, node datamodel.Node) error { - _, err := amender.Transform(datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { - return basicnode.Prototype.Any.AmendingBuilder(node), nil - }) - return err - } + mapAmender := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend).AmendingBuilder(n) for itr := selector.NewSegmentIterator(n); !itr.Done(); { ps, v, err := itr.Next() @@ -758,19 +747,19 @@ func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s sele if err != nil { return nil, err } - if err := transform(ps, next); err != nil { + if _, err := mapAmender.Put(ps.String(), next); err != nil { return nil, err } } else { - if err := transform(ps, v); err != nil { + if _, err := mapAmender.Put(ps.String(), v); err != nil { return nil, err } } } else { - if err := transform(ps, v); err != nil { + if _, err := mapAmender.Put(ps.String(), v); err != nil { return nil, err } } } - return amender.Build(), nil + return mapAmender.Build(), nil } From e3c116ab1d549aaaada0a8269843d7e38d445e51 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Fri, 14 Jul 2023 14:02:58 -0400 Subject: [PATCH 04/12] feat: add map amender tests --- datamodel/amender.go | 14 +- node/basicnode/list.go | 35 +--- node/basicnode/map.go | 72 ++++--- node/basicnode/map_test.go | 379 +++++++++++++++++++++++++++++++++++++ traversal/walk.go | 7 +- 5 files changed, 438 insertions(+), 69 deletions(-) diff --git a/datamodel/amender.go b/datamodel/amender.go index 351455bf..abf0a987 100644 --- a/datamodel/amender.go +++ b/datamodel/amender.go @@ -20,17 +20,17 @@ type containerAmender interface { Empty() bool Length() int64 Clear() - Values() ([]Node, error) + Values() (Node, error) // returns a list node with the values NodeAmender } // MapAmender adds a map-like interface to NodeAmender type MapAmender interface { - Put(key string, value Node) (bool, error) - Get(key string) (Node, error) - Remove(key string) (bool, error) - Keys() ([]string, error) + Put(key Node, value Node) error + Get(key Node) (Node, error) + Remove(key Node) (bool, error) + Keys() (Node, error) // returns a list node with the keys containerAmender } @@ -39,8 +39,8 @@ type MapAmender interface { type ListAmender interface { Get(idx int64) (Node, error) Remove(idx int64) error - Append(values ...Node) error - Insert(idx int64, values ...Node) error + Append(values Node) error // accepts a list node + Insert(idx int64, values Node) error // accepts a list node Set(idx int64, value Node) error containerAmender diff --git a/node/basicnode/list.go b/node/basicnode/list.go index cdd45eaf..9d45eef2 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -284,12 +284,12 @@ func (nb *plainList__Builder) Remove(idx int64) error { return err } -func (nb *plainList__Builder) Append(values ...datamodel.Node) error { +func (nb *plainList__Builder) Append(values datamodel.Node) error { // Passing an index equal to the length of the list will append the passed values to the end of the list - return nb.Insert(nb.Length(), values...) + return nb.Insert(nb.Length(), values) } -func (nb *plainList__Builder) Insert(idx int64, values ...datamodel.Node) error { +func (nb *plainList__Builder) Insert(idx int64, values datamodel.Node) error { var ps datamodel.PathSegment if idx == nb.Length() { ps = datamodel.PathSegmentOfString("-") // indicates appending to the end of the list @@ -299,22 +299,7 @@ func (nb *plainList__Builder) Insert(idx int64, values ...datamodel.Node) error _, err := nb.Transform( datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { - // Put all the passed values into a new list. That will result in these values being expanded at the - // specified index. - na := nb.Prototype().NewBuilder() - la, err := na.BeginList(int64(len(values))) - if err != nil { - return nil, err - } - for _, v := range values { - if err := la.AssembleValue().AssignNode(v); err != nil { - return nil, err - } - } - if err := la.Finish(); err != nil { - return nil, err - } - return Prototype.Any.AmendingBuilder(na.Build()), nil + return Prototype.Any.AmendingBuilder(values), nil }, ) return err @@ -336,16 +321,8 @@ func (nb *plainList__Builder) Clear() { nb.Reset() } -func (nb *plainList__Builder) Values() ([]datamodel.Node, error) { - values := make([]datamodel.Node, 0, nb.Length()) - for itr := nb.w.ListIterator(); !itr.Done(); { - _, v, err := itr.Next() - if err != nil { - return nil, err - } - values = append(values, v) - } - return values, nil +func (nb *plainList__Builder) Values() (datamodel.Node, error) { + return nb.Build(), nil } // -- NodeAssembler --> diff --git a/node/basicnode/map.go b/node/basicnode/map.go index 09413c52..be7a8f59 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -202,10 +202,7 @@ func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel. if string(v.k) == childKey { if newChildAmender == nil { delete(nb.w.m, childKey) - newT := make([]plainMap__Entry, nb.w.Length()-1) - copy(newT, nb.w.t[:idx]) - copy(newT[:idx], nb.w.t[idx+1:]) - nb.w.t = newT + nb.w.t = append(nb.w.t[:idx], nb.w.t[idx+1:]...) } else { nb.w.t[idx].v = newChildAmender nb.w.m[string(nb.w.t[idx].k)] = newChildAmender @@ -219,27 +216,35 @@ func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel. } } -func (nb *plainMap__Builder) Put(key string, value datamodel.Node) (bool, error) { - if prevNode, err := nb.Transform( - datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), +func (nb *plainMap__Builder) Put(key datamodel.Node, value datamodel.Node) error { + ks, err := key.AsString() + if err != nil { + return err + } + if _, err := nb.Transform( + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(ks)}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { return Prototype.Any.AmendingBuilder(value), nil }, ); err != nil { - return false, err + return err } else { // If there was no previous node, we just added a new node. - return prevNode == nil, nil + return nil } } -func (nb *plainMap__Builder) Get(key string) (datamodel.Node, error) { - return nb.w.LookupByString(key) +func (nb *plainMap__Builder) Get(key datamodel.Node) (datamodel.Node, error) { + return nb.w.LookupByNode(key) } -func (nb *plainMap__Builder) Remove(key string) (bool, error) { +func (nb *plainMap__Builder) Remove(key datamodel.Node) (bool, error) { + ks, err := key.AsString() + if err != nil { + return false, err + } if prevNode, err := nb.Transform( - datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(ks)}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { return nil, nil }, @@ -251,20 +256,35 @@ func (nb *plainMap__Builder) Remove(key string) (bool, error) { } } -func (nb *plainMap__Builder) Keys() ([]string, error) { - keys := make([]string, 0, nb.Length()) +func (nb *plainMap__Builder) Keys() (datamodel.Node, error) { + return nb.toList(true) +} + +func (nb *plainMap__Builder) toList(keysOrValues bool) (datamodel.Node, error) { + // Create a new List node and initialize its storage + lb := Prototype.List.AmendingBuilder(nil) + _, err := lb.BeginList(nb.Length()) + if err != nil { + return nil, err + } + var idx int64 = 0 for itr := nb.w.MapIterator(); !itr.Done(); { - k, _, err := itr.Next() + k, v, err := itr.Next() if err != nil { return nil, err } - keyStr, err := k.AsString() - if err != nil { + var n datamodel.Node + if keysOrValues { + n = k + } else { + n = v + } + if err = lb.Set(idx, n); err != nil { return nil, err } - keys = append(keys, keyStr) + idx++ } - return keys, nil + return lb.Build(), nil } func (nb *plainMap__Builder) Empty() bool { @@ -279,16 +299,8 @@ func (nb *plainMap__Builder) Clear() { nb.Reset() } -func (nb *plainMap__Builder) Values() ([]datamodel.Node, error) { - values := make([]datamodel.Node, 0, nb.Length()) - for itr := nb.w.MapIterator(); !itr.Done(); { - _, v, err := itr.Next() - if err != nil { - return nil, err - } - values = append(values, v) - } - return values, nil +func (nb *plainMap__Builder) Values() (datamodel.Node, error) { + return nb.toList(false) } // -- NodeAssembler --> diff --git a/node/basicnode/map_test.go b/node/basicnode/map_test.go index 404f5dfd..121b26f5 100644 --- a/node/basicnode/map_test.go +++ b/node/basicnode/map_test.go @@ -308,3 +308,382 @@ func TestMapDupKeyError(t *testing.T) { qt.Check(t, err, qt.ErrorMatches, `cannot repeat map key "cat"`) } + +func TestMapAmendingBuilderNewNode(t *testing.T) { + // Create a map amender with an empty base node + amender := basicnode.Prototype.Map.AmendingBuilder(nil) + + err := amender.Put(basicnode.NewString("cat"), basicnode.NewString("meow")) + if err != nil { + t.Fatal(err) + } + // Retry adding the entry + err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("meow")) + if err != nil { + t.Fatal(err) + } + err = amender.Put(basicnode.NewString("dog"), basicnode.NewString("bark")) + if err != nil { + t.Fatal(err) + } + err = amender.Put(basicnode.NewString("eel"), basicnode.NewString("zap")) + if err != nil { + t.Fatal(err) + } + + // compare the printed map + mapNode := amender.Build() + actual := printer.Sprint(mapNode) + + expect := `map{ + string{"cat"}: string{"meow"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + qt.Check(t, expect, qt.Equals, actual) + + // Access values of map, using string + r, err := mapNode.LookupByString("cat") + if err != nil { + t.Fatal(err) + } + qt.Check(t, "meow", qt.Equals, must.String(r)) + + // Access values of map, using node + r, err = mapNode.LookupByNode(basicnode.NewString("dog")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "bark", qt.Equals, must.String(r)) + + // Access values of map, using PathSegment + r, err = mapNode.LookupBySegment(datamodel.ParsePathSegment("eel")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "zap", qt.Equals, must.String(r)) + + // Validate the node's prototype + np := mapNode.Prototype() + qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__Map") + + // Amend the map + err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + if err != nil { + t.Fatal(err) + } + + // Access updated value of map, using get + r, err = amender.Get(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "purr", qt.Equals, must.String(r)) + + expect = `map{ + string{"cat"}: string{"purr"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + // The original node should have been updated + actual = printer.Sprint(mapNode) + qt.Assert(t, expect, qt.Equals, actual) + + // Remove an entry + removed, err := amender.Remove(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) + + expect = `map{ + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + // The original node should have been updated + actual = printer.Sprint(mapNode) + qt.Assert(t, expect, qt.Equals, actual) + + // Should not find "cat" + r, err = amender.Get(basicnode.NewString("cat")) + if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { + t.Fatal(err) + } + + keys, err := amender.Keys() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"dog"} + 1: string{"eel"} +}` + actual = printer.Sprint(keys) + qt.Assert(t, expect, qt.Equals, actual) + + values, err := amender.Values() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"bark"} + 1: string{"zap"} +}` + actual = printer.Sprint(values) + qt.Assert(t, expect, qt.Equals, actual) +} + +func TestMapAmendingBuilderExistingNode(t *testing.T) { + b := basicnode.Prototype.Map.NewBuilder() + + // construct a map of three keys, using the MapBuilder + ma, err := b.BeginMap(3) + if err != nil { + t.Fatal(err) + } + a := ma.AssembleKey() + a.AssignString("cat") + a = ma.AssembleValue() + a.AssignString("meow") + + a, err = ma.AssembleEntry("dog") + if err != nil { + t.Fatal(err) + } + a.AssignString("bark") + + a = ma.AssembleKey() + a.AssignString("eel") + a = ma.AssembleValue() + a.AssignString("zap") + + err = ma.Finish() + if err != nil { + t.Fatal(err) + } + + // Wrap in an amending builder + amender := basicnode.Prototype.Map.AmendingBuilder(b.Build()) + + // Compare the printed map + mapNode := b.Build() + actual := printer.Sprint(mapNode) + + expect := `map{ + string{"cat"}: string{"meow"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + qt.Check(t, expect, qt.Equals, actual) + + // Access values of map, using string + r, err := mapNode.LookupByString("cat") + if err != nil { + t.Fatal(err) + } + qt.Check(t, "meow", qt.Equals, must.String(r)) + + // Access values of map, using node + r, err = mapNode.LookupByNode(basicnode.NewString("dog")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "bark", qt.Equals, must.String(r)) + + // Access values of map, using PathSegment + r, err = mapNode.LookupBySegment(datamodel.ParsePathSegment("eel")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "zap", qt.Equals, must.String(r)) + + // Validate the node's prototype + np := mapNode.Prototype() + qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__Map") + + // Amend the map + err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + if err != nil { + t.Fatal(err) + } + + // Access updated value of map, using get + r, err = amender.Get(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "purr", qt.Equals, must.String(r)) + + expect = `map{ + string{"cat"}: string{"purr"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + // The original node should have been updated + actual = printer.Sprint(mapNode) + qt.Assert(t, expect, qt.Equals, actual) + + // Remove an entry + removed, err := amender.Remove(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) + + expect = `map{ + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + // The original node should have been updated + actual = printer.Sprint(mapNode) + qt.Assert(t, expect, qt.Equals, actual) + + // Should not find "cat" + r, err = amender.Get(basicnode.NewString("cat")) + if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { + t.Fatal(err) + } + + keys, err := amender.Keys() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"dog"} + 1: string{"eel"} +}` + actual = printer.Sprint(keys) + qt.Assert(t, expect, qt.Equals, actual) + + values, err := amender.Values() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"bark"} + 1: string{"zap"} +}` + actual = printer.Sprint(values) + qt.Assert(t, expect, qt.Equals, actual) +} + +func TestMapAmendingBuilderCopiedNode(t *testing.T) { + b := basicnode.Prototype.Map.NewBuilder() + + // Construct a map of three keys, using the MapBuilder + ma, err := b.BeginMap(3) + if err != nil { + t.Fatal(err) + } + a := ma.AssembleKey() + a.AssignString("cat") + a = ma.AssembleValue() + a.AssignString("meow") + + a, err = ma.AssembleEntry("dog") + if err != nil { + t.Fatal(err) + } + a.AssignString("bark") + + a = ma.AssembleKey() + a.AssignString("eel") + a = ma.AssembleValue() + a.AssignString("zap") + + err = ma.Finish() + if err != nil { + t.Fatal(err) + } + + origMapNode := b.Build() + // Copy the map using datamodel.Copy. AssignNode will copy pointers to internal values and will not return a fully + // standalone copy. + origAmender := basicnode.Prototype.Map.AmendingBuilder(nil) + err = datamodel.Copy(origMapNode, origAmender) + if err != nil { + t.Fatal(err) + } + + // Wrap in an amending builder + newAmender := basicnode.Prototype.Map.AmendingBuilder(origAmender.Build()) + + // Compare the printed map + newMapNode := newAmender.Build() + actual := printer.Sprint(newMapNode) + + expect := `map{ + string{"cat"}: string{"meow"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + qt.Check(t, expect, qt.Equals, actual) + + // Amend the copied map + err = newAmender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + if err != nil { + t.Fatal(err) + } + + expect = `map{ + string{"cat"}: string{"purr"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + // The new node should have been updated + actual = printer.Sprint(newMapNode) + qt.Assert(t, expect, qt.Equals, actual) + + // Remove an entry + removed, err := newAmender.Remove(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) + + expect = `map{ + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + + _, err = newAmender.Get(basicnode.NewString("cat")) + if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { + t.Fatal(err) + } + + keys, err := newAmender.Keys() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"dog"} + 1: string{"eel"} +}` + actual = printer.Sprint(keys) + qt.Assert(t, expect, qt.Equals, actual) + + values, err := newAmender.Values() + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"bark"} + 1: string{"zap"} +}` + actual = printer.Sprint(values) + qt.Assert(t, expect, qt.Equals, actual) + + // The original node should not have been updated + expect = `map{ + string{"cat"}: string{"meow"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + actual = printer.Sprint(origMapNode) + qt.Assert(t, expect, qt.Equals, actual) +} diff --git a/traversal/walk.go b/traversal/walk.go index 812ee47f..dc49c458 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -3,6 +3,7 @@ package traversal import ( "errors" "fmt" + "github.com/ipld/go-ipld-prime/node/basicnode" "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/linking" @@ -747,16 +748,16 @@ func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s sele if err != nil { return nil, err } - if _, err := mapAmender.Put(ps.String(), next); err != nil { + if err := mapAmender.Put(basicnode.NewString(ps.String()), next); err != nil { return nil, err } } else { - if _, err := mapAmender.Put(ps.String(), v); err != nil { + if err := mapAmender.Put(basicnode.NewString(ps.String()), v); err != nil { return nil, err } } } else { - if _, err := mapAmender.Put(ps.String(), v); err != nil { + if err := mapAmender.Put(basicnode.NewString(ps.String()), v); err != nil { return nil, err } } From 5de44d7397edd69331d0b614db93d1f56174ebba Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sat, 15 Jul 2023 18:39:53 -0400 Subject: [PATCH 05/12] feat: add list amender tests + other fixes/cleanup --- datamodel/amender.go | 23 ++- go.mod | 1 + go.sum | 2 + node/basicnode/list.go | 80 +++++--- node/basicnode/list_test.go | 390 ++++++++++++++++++++++++++++++++++++ node/basicnode/map.go | 20 +- node/basicnode/map_test.go | 84 ++++---- traversal/walk.go | 8 +- 8 files changed, 513 insertions(+), 95 deletions(-) diff --git a/datamodel/amender.go b/datamodel/amender.go index abf0a987..6d0bc123 100644 --- a/datamodel/amender.go +++ b/datamodel/amender.go @@ -20,17 +20,17 @@ type containerAmender interface { Empty() bool Length() int64 Clear() - Values() (Node, error) // returns a list node with the values + Values() (Node, error) // returns a list Node with the values NodeAmender } // MapAmender adds a map-like interface to NodeAmender type MapAmender interface { - Put(key Node, value Node) error - Get(key Node) (Node, error) - Remove(key Node) (bool, error) - Keys() (Node, error) // returns a list node with the keys + Put(key string, value Node) error + Get(key string) (Node, error) + Remove(key string) (bool, error) + Keys() (Node, error) // returns a list Node with the keys containerAmender } @@ -39,8 +39,17 @@ type MapAmender interface { type ListAmender interface { Get(idx int64) (Node, error) Remove(idx int64) error - Append(values Node) error // accepts a list node - Insert(idx int64, values Node) error // accepts a list node + // Append will add Node(s) to the end of the list. It can accept a list Node with multiple values to append. + Append(value Node) error + // Insert will add Node(s) at the specified index and shift subsequent elements to the right. It can accept a list + // Node with multiple values to insert. + // Passing an index equal to the length of the list will add Node(s) to the end of the list like Append. + Insert(idx int64, value Node) error + // Set will add Node(s) at the specified index and shift subsequent elements to the right. It can accept a list Node + // with multiple values to insert. + // Passing an index equal to the length of the list will add Node(s) to the end of the list like Append. + // Set is different from Insert in that it will start its insertion at the specified index, overwriting it in the + // process, while Insert will only add the Node(s). Set(idx int64, value Node) error containerAmender diff --git a/go.mod b/go.mod index 10b29fab..d3afbc2c 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/multiformats/go-multihash v0.2.3 github.com/polydawn/refmt v0.89.0 github.com/warpfork/go-testmark v0.12.1 + golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 gopkg.in/yaml.v2 v2.4.0 ) diff --git a/go.sum b/go.sum index 5cbfca6f..7589b43c 100644 --- a/go.sum +++ b/go.sum @@ -59,6 +59,8 @@ github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0/go.mod h1:x6AKhvS golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= +golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 h1:MGwJjxBy0HJshjDNfLsYO8xppfqWlA5ZT9OhtUUhTNw= +golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= diff --git a/node/basicnode/list.go b/node/basicnode/list.go index 9d45eef2..f02fb735 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -4,6 +4,8 @@ import ( "fmt" "reflect" + "golang.org/x/exp/slices" + "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/node/mixins" ) @@ -160,6 +162,10 @@ func (nb *plainList__Builder) Reset() { // -- NodeAmender --> func (nb *plainList__Builder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { + return nb.transform(path, transform, true) +} + +func (nb *plainList__Builder) transform(path datamodel.Path, transform datamodel.AmendFn, replace bool) (datamodel.Node, error) { // Can only transform the root of the node or an immediate child. if path.Len() > 1 { panic("misuse") @@ -217,20 +223,20 @@ func (nb *plainList__Builder) Transform(path datamodel.Path, transform datamodel if newChildVal, err := transform(prevChildVal); err != nil { return nil, err } else if newChildVal == nil { - newX := make([]datamodel.NodeAmender, nb.w.Length()-1) - copy(newX, nb.w.x[:childIdx]) - copy(newX[:childIdx], nb.w.x[childIdx+1:]) - nb.w.x = newX - } else if err = nb.storeChildAmender(childIdx, newChildVal); err != nil { + // Ref: https://pkg.go.dev/golang.org/x/exp/slices#Delete + idx := int(childIdx) + nb.w.x[idx] = nil + nb.w.x = slices.Delete(nb.w.x, idx, idx+1) + } else if err = nb.storeChildAmender(childIdx, newChildVal, replace); err != nil { return nil, err } return prevChildVal, nil } -func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.NodeAmender) error { +func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.NodeAmender, replace bool) error { var elems []datamodel.NodeAmender n := a.Build() - if (n.Kind() == datamodel.Kind_List) && (n.Length() > 0) { + if n.Kind() == datamodel.Kind_List { elems = make([]datamodel.NodeAmender, n.Length()) // The following logic uses a transformed list (if there is one) to perform both insertions (needed by JSON // Patch) and replacements (needed by `focus` and `walk`), while also providing the flexibility to insert more @@ -258,14 +264,36 @@ func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.Node elems = []datamodel.NodeAmender{Prototype.Any.AmendingBuilder(n)} } if childIdx == nb.w.Length() { + // Operations at the end of the list are straightforward - just append, and we're done. nb.w.x = append(nb.w.x, elems...) - } else { - numElems := int64(len(elems)) - newX := make([]datamodel.NodeAmender, nb.w.Length()+numElems-1) - copy(newX, nb.w.x[:childIdx]) - copy(newX[childIdx:], elems) - copy(newX[childIdx+numElems:], nb.w.x[childIdx+1:]) - nb.w.x = newX + return nil + } + numElems := len(elems) + if numElems > 0 { + if nb.w.x == nil { + // Allocate storage space + nb.w.x = make([]datamodel.NodeAmender, numElems) + } else { + // Allocate storage space + numElemsToAdd := numElems + if replace { + // We'll be replacing an existing element and need one slot less than the total number of elements being + // added. + numElemsToAdd-- + } + nb.w.x = slices.Grow(nb.w.x, numElemsToAdd) + } + copyStartIdx := 0 + if replace { + // Use the first passed element to replace the element currently at the specified index + nb.w.x[childIdx] = elems[0] + copyStartIdx++ + } + if !replace || (numElems > 1) { + // If more elements were specified, insert them after the specified index. + // Ref: https://pkg.go.dev/golang.org/x/exp/slices#Insert + nb.w.x = slices.Insert(nb.w.x, int(childIdx)+copyStartIdx, elems[copyStartIdx:]...) + } } return nil } @@ -284,31 +312,35 @@ func (nb *plainList__Builder) Remove(idx int64) error { return err } -func (nb *plainList__Builder) Append(values datamodel.Node) error { - // Passing an index equal to the length of the list will append the passed values to the end of the list - return nb.Insert(nb.Length(), values) +func (nb *plainList__Builder) Append(value datamodel.Node) error { + return nb.Set(nb.Length(), value) +} + +func (nb *plainList__Builder) Insert(idx int64, value datamodel.Node) error { + return nb.addElements(idx, value, false) +} + +func (nb *plainList__Builder) Set(idx int64, value datamodel.Node) error { + return nb.addElements(idx, value, true) } -func (nb *plainList__Builder) Insert(idx int64, values datamodel.Node) error { +func (nb *plainList__Builder) addElements(idx int64, value datamodel.Node, replaced bool) error { var ps datamodel.PathSegment if idx == nb.Length() { ps = datamodel.PathSegmentOfString("-") // indicates appending to the end of the list } else { ps = datamodel.PathSegmentOfInt(idx) } - _, err := nb.Transform( + _, err := nb.transform( datamodel.NewPath([]datamodel.PathSegment{ps}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { - return Prototype.Any.AmendingBuilder(values), nil + return Prototype.Any.AmendingBuilder(value), nil }, + replaced, ) return err } -func (nb *plainList__Builder) Set(idx int64, value datamodel.Node) error { - return nb.Insert(idx, value) -} - func (nb *plainList__Builder) Empty() bool { return nb.Length() == 0 } diff --git a/node/basicnode/list_test.go b/node/basicnode/list_test.go index 68cb1778..ff5108fc 100644 --- a/node/basicnode/list_test.go +++ b/node/basicnode/list_test.go @@ -1,12 +1,402 @@ package basicnode_test import ( + "fmt" "testing" + qt "github.com/frankban/quicktest" + + "github.com/ipld/go-ipld-prime/datamodel" + "github.com/ipld/go-ipld-prime/fluent/qp" + "github.com/ipld/go-ipld-prime/must" "github.com/ipld/go-ipld-prime/node/basicnode" "github.com/ipld/go-ipld-prime/node/tests" + "github.com/ipld/go-ipld-prime/printer" ) func TestList(t *testing.T) { tests.SpecTestListString(t, basicnode.Prototype.List) } + +func TestListAmendingBuilderNewNode(t *testing.T) { + amender := basicnode.Prototype.List.AmendingBuilder(nil) + + err := amender.Append(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + err = amender.Append(basicnode.NewString("dog")) + if err != nil { + t.Fatal(err) + } + err = amender.Append(basicnode.NewString("eel")) + if err != nil { + t.Fatal(err) + } + listNode := amender.Build() + expect := `list{ + 0: string{"cat"} + 1: string{"dog"} + 2: string{"eel"} +}` + actual := printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Update an element at the start + err = amender.Set(0, basicnode.NewString("cow")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cow"} + 1: string{"dog"} + 2: string{"eel"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Update an element in the middle + err = amender.Set(1, basicnode.NewString("fox")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cow"} + 1: string{"fox"} + 2: string{"eel"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Update an element at the end + err = amender.Set(amender.Length(), basicnode.NewString("dog")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cow"} + 1: string{"fox"} + 2: string{"eel"} + 3: string{"dog"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Delete an element from the start + err = amender.Remove(0) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} + 1: string{"eel"} + 2: string{"dog"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Delete an element from the middle + err = amender.Remove(1) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} + 1: string{"dog"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Delete an element from the end + err = amender.Remove(1) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Insert an element at the start + err = amender.Insert(0, basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"fox"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Insert an element in the middle + err = amender.Insert(1, basicnode.NewString("dog")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"dog"} + 2: string{"fox"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Insert an element at the end + err = amender.Insert(amender.Length(), basicnode.NewString("eel")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"dog"} + 2: string{"fox"} + 3: string{"eel"} +}` + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Access values of list, using index + r, err := listNode.LookupByIndex(0) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "cat", qt.Equals, must.String(r)) + + // Access values of list, using PathSegment + r, err = listNode.LookupBySegment(datamodel.ParsePathSegment("2")) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "fox", qt.Equals, must.String(r)) + + // Access updated value of list, using get + r, err = amender.Get(3) + if err != nil { + t.Fatal(err) + } + qt.Check(t, "eel", qt.Equals, must.String(r)) + + // Validate the node's prototype + np := listNode.Prototype() + qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__List") +} + +func TestListAmendingBuilderExistingNode(t *testing.T) { + listNode, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("fox")) + qp.ListEntry(am, qp.String("cow")) + qp.ListEntry(am, qp.String("deer")) + }) + if err != nil { + t.Fatal(err) + } + expect := `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} +}` + actual := printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + amender := basicnode.Prototype.List.AmendingBuilder(listNode) + err = amender.Append(basicnode.NewString("cat")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} + 3: string{"cat"} +}` + // The original node should have been updated + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + insertElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("eel")) + qp.ListEntry(am, qp.String("dog")) + }) + if err != nil { + t.Fatal(err) + } + err = amender.Insert(3, insertElems) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} + 3: string{"eel"} + 4: string{"dog"} + 5: string{"cat"} +}` + // The original node should have been updated + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) + + err = amender.Append(basicnode.NewString("eel")) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} + 3: string{"eel"} + 4: string{"dog"} + 5: string{"cat"} + 6: string{"eel"} +}` + // The original node should have been updated + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) +} + +func TestListAmendingBuilderCopiedNode(t *testing.T) { + listNode, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("fox")) + qp.ListEntry(am, qp.String("cow")) + qp.ListEntry(am, qp.String("deer")) + }) + if err != nil { + t.Fatal(err) + } + expect := `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} +}` + amender := basicnode.Prototype.List.AmendingBuilder(nil) + // Copy the map using datamodel.Copy. AssignNode will copy pointers to internal values and will not return a fully + // standalone copy. + err = datamodel.Copy(listNode, amender) + if err != nil { + t.Fatal(err) + } + copiedNode := amender.Build() + actual := printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + setElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("cat")) + qp.ListEntry(am, qp.String("dog")) + }) + if err != nil { + t.Fatal(err) + } + err = amender.Set(0, setElems) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"dog"} + 2: string{"cow"} + 3: string{"deer"} +}` + // The new node should have been updated + actual = printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + insertElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("eel")) + qp.ListEntry(am, qp.String("fox")) + }) + if err != nil { + t.Fatal(err) + } + err = amender.Insert(1, insertElems) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"eel"} + 2: string{"fox"} + 3: string{"dog"} + 4: string{"cow"} + 5: string{"deer"} +}` + // The new node should have been updated + actual = printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + appendElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("rat")) + }) + if err != nil { + t.Fatal(err) + } + err = amender.Append(appendElems) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"eel"} + 2: string{"fox"} + 3: string{"dog"} + 4: string{"cow"} + 5: string{"deer"} + 6: string{"rat"} +}` + // The new node should have been updated + actual = printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Pass through an empty list. This should have no effect. + appendElems, err = qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) {}) + if err != nil { + t.Fatal(err) + } + err = amender.Append(appendElems) + if err != nil { + t.Fatal(err) + } + // The new node should not have been updated + actual = printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + // Pass through a list containing another list + nestedList, err := qp.BuildList(basicnode.Prototype.List, 1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.String("bat")) + }) + if err != nil { + t.Fatal(err) + } + appendElems, err = qp.BuildList(basicnode.Prototype.List, 1, func(am datamodel.ListAssembler) { + qp.ListEntry(am, qp.Node(nestedList)) + }) + err = amender.Append(appendElems) + if err != nil { + t.Fatal(err) + } + expect = `list{ + 0: string{"cat"} + 1: string{"eel"} + 2: string{"fox"} + 3: string{"dog"} + 4: string{"cow"} + 5: string{"deer"} + 6: string{"rat"} + 7: list{ + 0: string{"bat"} + } +}` + // The new node should have been updated to have a list node at the end + actual = printer.Sprint(copiedNode) + qt.Assert(t, actual, qt.Equals, expect) + + expect = `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} +}` + // The original node should not have been updated + actual = printer.Sprint(listNode) + qt.Assert(t, actual, qt.Equals, expect) +} diff --git a/node/basicnode/map.go b/node/basicnode/map.go index be7a8f59..c8b8c2cd 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -216,13 +216,9 @@ func (nb *plainMap__Builder) Transform(path datamodel.Path, transform datamodel. } } -func (nb *plainMap__Builder) Put(key datamodel.Node, value datamodel.Node) error { - ks, err := key.AsString() - if err != nil { - return err - } +func (nb *plainMap__Builder) Put(key string, value datamodel.Node) error { if _, err := nb.Transform( - datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(ks)}), + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { return Prototype.Any.AmendingBuilder(value), nil }, @@ -234,17 +230,13 @@ func (nb *plainMap__Builder) Put(key datamodel.Node, value datamodel.Node) error } } -func (nb *plainMap__Builder) Get(key datamodel.Node) (datamodel.Node, error) { - return nb.w.LookupByNode(key) +func (nb *plainMap__Builder) Get(key string) (datamodel.Node, error) { + return nb.w.LookupByString(key) } -func (nb *plainMap__Builder) Remove(key datamodel.Node) (bool, error) { - ks, err := key.AsString() - if err != nil { - return false, err - } +func (nb *plainMap__Builder) Remove(key string) (bool, error) { if prevNode, err := nb.Transform( - datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(ks)}), + datamodel.NewPath([]datamodel.PathSegment{datamodel.PathSegmentOfString(key)}), func(_ datamodel.Node) (datamodel.NodeAmender, error) { return nil, nil }, diff --git a/node/basicnode/map_test.go b/node/basicnode/map_test.go index 121b26f5..e5773806 100644 --- a/node/basicnode/map_test.go +++ b/node/basicnode/map_test.go @@ -138,7 +138,7 @@ func TestMapBuilder(t *testing.T) { anotherNode := c.Build() actual = printer.Sprint(anotherNode) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // access values of map, using string r, err := anotherNode.LookupByString("cat") @@ -313,20 +313,20 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { // Create a map amender with an empty base node amender := basicnode.Prototype.Map.AmendingBuilder(nil) - err := amender.Put(basicnode.NewString("cat"), basicnode.NewString("meow")) + err := amender.Put("cat", basicnode.NewString("meow")) if err != nil { t.Fatal(err) } // Retry adding the entry - err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("meow")) + err = amender.Put("cat", basicnode.NewString("meow")) if err != nil { t.Fatal(err) } - err = amender.Put(basicnode.NewString("dog"), basicnode.NewString("bark")) + err = amender.Put("dog", basicnode.NewString("bark")) if err != nil { t.Fatal(err) } - err = amender.Put(basicnode.NewString("eel"), basicnode.NewString("zap")) + err = amender.Put("eel", basicnode.NewString("zap")) if err != nil { t.Fatal(err) } @@ -368,13 +368,13 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__Map") // Amend the map - err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + err = amender.Put("cat", basicnode.NewString("purr")) if err != nil { t.Fatal(err) } // Access updated value of map, using get - r, err = amender.Get(basicnode.NewString("cat")) + r, err = amender.Get("cat") if err != nil { t.Fatal(err) } @@ -388,10 +388,10 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { // The original node should have been updated actual = printer.Sprint(mapNode) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // Remove an entry - removed, err := amender.Remove(basicnode.NewString("cat")) + removed, err := amender.Remove("cat") if err != nil { t.Fatal(err) } @@ -404,10 +404,10 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { // The original node should have been updated actual = printer.Sprint(mapNode) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // Should not find "cat" - r, err = amender.Get(basicnode.NewString("cat")) + r, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { t.Fatal(err) } @@ -421,7 +421,7 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { 1: string{"eel"} }` actual = printer.Sprint(keys) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) values, err := amender.Values() if err != nil { @@ -432,13 +432,12 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { 1: string{"zap"} }` actual = printer.Sprint(values) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) } func TestMapAmendingBuilderExistingNode(t *testing.T) { b := basicnode.Prototype.Map.NewBuilder() - // construct a map of three keys, using the MapBuilder ma, err := b.BeginMap(3) if err != nil { t.Fatal(err) @@ -464,7 +463,7 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { t.Fatal(err) } - // Wrap in an amending builder + // Wrap in an amending build amender := basicnode.Prototype.Map.AmendingBuilder(b.Build()) // Compare the printed map @@ -504,13 +503,13 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__Map") // Amend the map - err = amender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + err = amender.Put("cat", basicnode.NewString("purr")) if err != nil { t.Fatal(err) } // Access updated value of map, using get - r, err = amender.Get(basicnode.NewString("cat")) + r, err = amender.Get("cat") if err != nil { t.Fatal(err) } @@ -524,10 +523,10 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { // The original node should have been updated actual = printer.Sprint(mapNode) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // Remove an entry - removed, err := amender.Remove(basicnode.NewString("cat")) + removed, err := amender.Remove("cat") if err != nil { t.Fatal(err) } @@ -540,10 +539,10 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { // The original node should have been updated actual = printer.Sprint(mapNode) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // Should not find "cat" - r, err = amender.Get(basicnode.NewString("cat")) + r, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { t.Fatal(err) } @@ -557,7 +556,7 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { 1: string{"eel"} }` actual = printer.Sprint(keys) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) values, err := amender.Values() if err != nil { @@ -568,13 +567,12 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { 1: string{"zap"} }` actual = printer.Sprint(values) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) } func TestMapAmendingBuilderCopiedNode(t *testing.T) { b := basicnode.Prototype.Map.NewBuilder() - // Construct a map of three keys, using the MapBuilder ma, err := b.BeginMap(3) if err != nil { t.Fatal(err) @@ -600,21 +598,17 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { t.Fatal(err) } - origMapNode := b.Build() + mapNode := b.Build() // Copy the map using datamodel.Copy. AssignNode will copy pointers to internal values and will not return a fully // standalone copy. - origAmender := basicnode.Prototype.Map.AmendingBuilder(nil) - err = datamodel.Copy(origMapNode, origAmender) + amender := basicnode.Prototype.Map.AmendingBuilder(nil) + err = datamodel.Copy(mapNode, amender) if err != nil { t.Fatal(err) } - // Wrap in an amending builder - newAmender := basicnode.Prototype.Map.AmendingBuilder(origAmender.Build()) - - // Compare the printed map - newMapNode := newAmender.Build() - actual := printer.Sprint(newMapNode) + copiedMapNode := amender.Build() + actual := printer.Sprint(copiedMapNode) expect := `map{ string{"cat"}: string{"meow"} @@ -624,7 +618,7 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { qt.Check(t, expect, qt.Equals, actual) // Amend the copied map - err = newAmender.Put(basicnode.NewString("cat"), basicnode.NewString("purr")) + err = amender.Put("cat", basicnode.NewString("purr")) if err != nil { t.Fatal(err) } @@ -635,12 +629,12 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { string{"eel"}: string{"zap"} }` - // The new node should have been updated - actual = printer.Sprint(newMapNode) - qt.Assert(t, expect, qt.Equals, actual) + // The copied node should have been updated + actual = printer.Sprint(copiedMapNode) + qt.Assert(t, actual, qt.Equals, expect) // Remove an entry - removed, err := newAmender.Remove(basicnode.NewString("cat")) + removed, err := amender.Remove("cat") if err != nil { t.Fatal(err) } @@ -651,12 +645,12 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { string{"eel"}: string{"zap"} }` - _, err = newAmender.Get(basicnode.NewString("cat")) + _, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { t.Fatal(err) } - keys, err := newAmender.Keys() + keys, err := amender.Keys() if err != nil { t.Fatal(err) } @@ -665,9 +659,9 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { 1: string{"eel"} }` actual = printer.Sprint(keys) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) - values, err := newAmender.Values() + values, err := amender.Values() if err != nil { t.Fatal(err) } @@ -676,7 +670,7 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { 1: string{"zap"} }` actual = printer.Sprint(values) - qt.Assert(t, expect, qt.Equals, actual) + qt.Assert(t, actual, qt.Equals, expect) // The original node should not have been updated expect = `map{ @@ -684,6 +678,6 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` - actual = printer.Sprint(origMapNode) - qt.Assert(t, expect, qt.Equals, actual) + actual = printer.Sprint(mapNode) + qt.Assert(t, actual, qt.Equals, expect) } diff --git a/traversal/walk.go b/traversal/walk.go index dc49c458..e1b551a3 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -3,8 +3,6 @@ package traversal import ( "errors" "fmt" - "github.com/ipld/go-ipld-prime/node/basicnode" - "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/linking" "github.com/ipld/go-ipld-prime/linking/preload" @@ -748,16 +746,16 @@ func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s sele if err != nil { return nil, err } - if err := mapAmender.Put(basicnode.NewString(ps.String()), next); err != nil { + if err := mapAmender.Put(ps.String(), next); err != nil { return nil, err } } else { - if err := mapAmender.Put(basicnode.NewString(ps.String()), v); err != nil { + if err := mapAmender.Put(ps.String(), v); err != nil { return nil, err } } } else { - if err := mapAmender.Put(basicnode.NewString(ps.String()), v); err != nil { + if err := mapAmender.Put(ps.String(), v); err != nil { return nil, err } } From 25825c144d9fff4ef839e2069f795066f940ec93 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sat, 15 Jul 2023 18:42:45 -0400 Subject: [PATCH 06/12] chore: staticcheck fix --- node/basicnode/list_test.go | 3 +++ node/basicnode/map_test.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/node/basicnode/list_test.go b/node/basicnode/list_test.go index ff5108fc..c796403c 100644 --- a/node/basicnode/list_test.go +++ b/node/basicnode/list_test.go @@ -371,6 +371,9 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { appendElems, err = qp.BuildList(basicnode.Prototype.List, 1, func(am datamodel.ListAssembler) { qp.ListEntry(am, qp.Node(nestedList)) }) + if err != nil { + t.Fatal(err) + } err = amender.Append(appendElems) if err != nil { t.Fatal(err) diff --git a/node/basicnode/map_test.go b/node/basicnode/map_test.go index e5773806..65441233 100644 --- a/node/basicnode/map_test.go +++ b/node/basicnode/map_test.go @@ -407,7 +407,7 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { qt.Assert(t, actual, qt.Equals, expect) // Should not find "cat" - r, err = amender.Get("cat") + _, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { t.Fatal(err) } @@ -542,7 +542,7 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { qt.Assert(t, actual, qt.Equals, expect) // Should not find "cat" - r, err = amender.Get("cat") + _, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { t.Fatal(err) } From 12074effbe7e8f4b9be0c14263869a2fe00087a5 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sat, 15 Jul 2023 19:07:25 -0400 Subject: [PATCH 07/12] chore: use list amender append for map keys/values --- node/basicnode/map.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/node/basicnode/map.go b/node/basicnode/map.go index c8b8c2cd..1cb43440 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -259,7 +259,6 @@ func (nb *plainMap__Builder) toList(keysOrValues bool) (datamodel.Node, error) { if err != nil { return nil, err } - var idx int64 = 0 for itr := nb.w.MapIterator(); !itr.Done(); { k, v, err := itr.Next() if err != nil { @@ -271,10 +270,9 @@ func (nb *plainMap__Builder) toList(keysOrValues bool) (datamodel.Node, error) { } else { n = v } - if err = lb.Set(idx, n); err != nil { + if err := lb.Append(n); err != nil { return nil, err } - idx++ } return lb.Build(), nil } From b97c98cef9a535999ca50ae29d73c14bca17c8ec Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sun, 16 Jul 2023 12:42:02 -0400 Subject: [PATCH 08/12] fix: map copies of nodes before transformation --- traversal/focus_test.go | 57 +++++++++++++++++++++++++++++------------ traversal/walk_test.go | 3 ++- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/traversal/focus_test.go b/traversal/focus_test.go index 7467a8e8..30018841 100644 --- a/traversal/focus_test.go +++ b/traversal/focus_test.go @@ -192,7 +192,8 @@ func TestGetWithLinkLoading(t *testing.T) { func TestFocusedTransform(t *testing.T) { t.Run("UpdateMapEntry", func(t *testing.T) { - n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("plain"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(rootNode) + n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("plain"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "plain") qt.Check(t, must.String(prev), qt.Equals, "olde string") nb := prev.Prototype().NewBuilder() @@ -204,14 +205,15 @@ func TestFocusedTransform(t *testing.T) { // updated value should be there qt.Check(t, must.Node(n.LookupByString("plain")), nodetests.NodeContentEquals, basicnode.NewString("new string!")) // everything else should be there - qt.Check(t, must.Node(n.LookupByString("linkedString")), qt.Equals, must.Node(rootNode.LookupByString("linkedString"))) - qt.Check(t, must.Node(n.LookupByString("linkedMap")), qt.Equals, must.Node(rootNode.LookupByString("linkedMap"))) - qt.Check(t, must.Node(n.LookupByString("linkedList")), qt.Equals, must.Node(rootNode.LookupByString("linkedList"))) + qt.Check(t, must.Node(n.LookupByString("linkedString")), qt.Equals, must.Node(mapNode.LookupByString("linkedString"))) + qt.Check(t, must.Node(n.LookupByString("linkedMap")), qt.Equals, must.Node(mapNode.LookupByString("linkedMap"))) + qt.Check(t, must.Node(n.LookupByString("linkedList")), qt.Equals, must.Node(mapNode.LookupByString("linkedList"))) // everything should still be in the same order qt.Check(t, keys(n), qt.DeepEquals, []string{"plain", "linkedString", "linkedMap", "linkedList"}) }) t.Run("UpdateDeeperMap", func(t *testing.T) { - n, err := traversal.FocusedTransform(middleMapNode, datamodel.ParsePath("nested/alink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(middleMapNode) + n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("nested/alink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "nested/alink") qt.Check(t, prev, nodetests.NodeContentEquals, basicnode.NewLink(leafAlphaLnk)) return basicnode.NewString("new string!"), nil @@ -221,13 +223,14 @@ func TestFocusedTransform(t *testing.T) { // updated value should be there qt.Check(t, must.Node(must.Node(n.LookupByString("nested")).LookupByString("alink")), nodetests.NodeContentEquals, basicnode.NewString("new string!")) // everything else in the parent map should should be there! - qt.Check(t, must.Node(n.LookupByString("foo")), qt.Equals, must.Node(middleMapNode.LookupByString("foo"))) - qt.Check(t, must.Node(n.LookupByString("bar")), qt.Equals, must.Node(middleMapNode.LookupByString("bar"))) + qt.Check(t, must.Node(n.LookupByString("foo")), qt.Equals, must.Node(mapNode.LookupByString("foo"))) + qt.Check(t, must.Node(n.LookupByString("bar")), qt.Equals, must.Node(mapNode.LookupByString("bar"))) // everything should still be in the same order qt.Check(t, keys(n), qt.DeepEquals, []string{"foo", "bar", "nested"}) }) t.Run("AppendIfNotExists", func(t *testing.T) { - n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(rootNode) + n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "newpart") qt.Check(t, prev, qt.IsNil) // REVIEW: should datamodel.Absent be used here? I lean towards "no" but am unsure what's least surprising here. // An interesting thing to note about inserting a value this way is that you have no `prev.Prototype().NewBuilder()` to use if you wanted to. @@ -242,7 +245,8 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, keys(n), qt.DeepEquals, []string{"plain", "linkedString", "linkedMap", "linkedList", "newpart"}) }) t.Run("CreateParents", func(t *testing.T) { - n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(rootNode) + n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "newsection/newpart") qt.Check(t, prev, qt.IsNil) // REVIEW: should datamodel.Absent be used here? I lean towards "no" but am unsure what's least surprising here. return basicnode.NewString("new string!"), nil @@ -260,14 +264,16 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, keys(n2), qt.DeepEquals, []string{"newpart"}) }) t.Run("CreateParentsRequiresPermission", func(t *testing.T) { - _, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(rootNode) + _, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, true, qt.IsFalse) // ought not be reached return nil, nil }, false) qt.Check(t, err.Error(), qt.Equals, "transform: parent position at \"newsection\" did not exist (and createParents was false)") }) t.Run("UpdateListEntry", func(t *testing.T) { - n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("2"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + listNode := duplicateListNode(middleListNode) + n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("2"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "2") qt.Check(t, prev, nodetests.NodeContentEquals, basicnode.NewLink(leafBetaLnk)) return basicnode.NewString("new string!"), nil @@ -283,7 +289,8 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, must.Node(n.LookupByIndex(3)), nodetests.NodeContentEquals, basicnode.NewLink(leafAlphaLnk)) }) t.Run("AppendToList", func(t *testing.T) { - n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("-"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + listNode := duplicateListNode(middleListNode) + n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("-"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "4") qt.Check(t, prev, qt.IsNil) return basicnode.NewString("new string!"), nil @@ -296,14 +303,16 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, n.Length(), qt.Equals, int64(5)) }) t.Run("ListBounds", func(t *testing.T) { - _, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("4"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + listNode := duplicateListNode(middleListNode) + _, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("4"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, true, qt.IsFalse) // ought not be reached return nil, nil }, false) qt.Check(t, err, qt.ErrorMatches, "transform: cannot navigate path segment \"4\" at \"\" because it is beyond the list bounds") }) t.Run("ReplaceRoot", func(t *testing.T) { // a fairly degenerate case and no reason to do this, but should work. - n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath(""), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + listNode := duplicateListNode(middleListNode) + n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath(""), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "") qt.Check(t, prev, nodetests.NodeContentEquals, middleListNode) nb := basicnode.Prototype.Any.NewBuilder() @@ -326,10 +335,11 @@ func TestFocusedTransformWithLinks(t *testing.T) { LinkSystem: lsys, LinkTargetNodePrototypeChooser: basicnode.Chooser, } + mapNode := duplicateMapNode(rootNode) t.Run("UpdateMapBeyondLink", func(t *testing.T) { n, err := traversal.Progress{ Cfg: &cfg, - }.FocusedTransform(rootNode, datamodel.ParsePath("linkedMap/nested/nonlink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + }.FocusedTransform(mapNode, datamodel.ParsePath("linkedMap/nested/nonlink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "linkedMap/nested/nonlink") qt.Check(t, must.String(prev), qt.Equals, "zoo") qt.Check(t, progress.LastBlock.Path.String(), qt.Equals, "linkedMap") @@ -346,10 +356,11 @@ func TestFocusedTransformWithLinks(t *testing.T) { store2 = memstore.Store{} }) t.Run("UpdateNotBeyondLink", func(t *testing.T) { + mapNode := duplicateMapNode(rootNode) // This is replacing a link with a non-link. Doing so shouldn't hit storage. n, err := traversal.Progress{ Cfg: &cfg, - }.FocusedTransform(rootNode, datamodel.ParsePath("linkedMap"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + }.FocusedTransform(mapNode, datamodel.ParsePath("linkedMap"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "linkedMap") nb := prev.Prototype().NewBuilder() nb.AssignString("new string!") @@ -374,3 +385,17 @@ func keys(n datamodel.Node) []string { } return v } + +func duplicateMapNode(n datamodel.Node) datamodel.Node { + amender := basicnode.Prototype.Map.AmendingBuilder(nil) + // Make a deep copy of the node + datamodel.Copy(n, amender) + return amender.Build() +} + +func duplicateListNode(n datamodel.Node) datamodel.Node { + amender := basicnode.Prototype.List.AmendingBuilder(nil) + // Make a deep copy of the node + datamodel.Copy(n, amender) + return amender.Build() +} diff --git a/traversal/walk_test.go b/traversal/walk_test.go index e7588442..87657826 100644 --- a/traversal/walk_test.go +++ b/traversal/walk_test.go @@ -781,7 +781,8 @@ func TestWalkTransforming(t *testing.T) { s, err := ss.Selector() qt.Assert(t, err, qt.IsNil) var order int - n, err := traversal.WalkTransforming(middleMapNode, s, func(prog traversal.Progress, n datamodel.Node) (datamodel.Node, error) { + mapNode := duplicateMapNode(middleMapNode) + n, err := traversal.WalkTransforming(mapNode, s, func(prog traversal.Progress, n datamodel.Node) (datamodel.Node, error) { switch order { case 0: qt.Check(t, n, nodetests.NodeContentEquals, basicnode.NewBool(true)) From 122d13b171e5359b5e9484248d7f0601a2b89429 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sun, 16 Jul 2023 15:52:59 -0400 Subject: [PATCH 09/12] fix: move node copy inside test --- traversal/focus_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traversal/focus_test.go b/traversal/focus_test.go index 30018841..e3f8db93 100644 --- a/traversal/focus_test.go +++ b/traversal/focus_test.go @@ -335,8 +335,8 @@ func TestFocusedTransformWithLinks(t *testing.T) { LinkSystem: lsys, LinkTargetNodePrototypeChooser: basicnode.Chooser, } - mapNode := duplicateMapNode(rootNode) t.Run("UpdateMapBeyondLink", func(t *testing.T) { + mapNode := duplicateMapNode(rootNode) n, err := traversal.Progress{ Cfg: &cfg, }.FocusedTransform(mapNode, datamodel.ParsePath("linkedMap/nested/nonlink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { From b22bcc3d109fd8cb08aa31170795d85d5aea8fc9 Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Sun, 16 Jul 2023 18:01:03 -0400 Subject: [PATCH 10/12] fix: deep copy amendable nodes during walk --- traversal/focus_test.go | 57 ++++++++++++----------------------------- traversal/walk.go | 26 +++++++++++-------- traversal/walk_test.go | 3 +-- 3 files changed, 33 insertions(+), 53 deletions(-) diff --git a/traversal/focus_test.go b/traversal/focus_test.go index e3f8db93..7467a8e8 100644 --- a/traversal/focus_test.go +++ b/traversal/focus_test.go @@ -192,8 +192,7 @@ func TestGetWithLinkLoading(t *testing.T) { func TestFocusedTransform(t *testing.T) { t.Run("UpdateMapEntry", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) - n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("plain"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("plain"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "plain") qt.Check(t, must.String(prev), qt.Equals, "olde string") nb := prev.Prototype().NewBuilder() @@ -205,15 +204,14 @@ func TestFocusedTransform(t *testing.T) { // updated value should be there qt.Check(t, must.Node(n.LookupByString("plain")), nodetests.NodeContentEquals, basicnode.NewString("new string!")) // everything else should be there - qt.Check(t, must.Node(n.LookupByString("linkedString")), qt.Equals, must.Node(mapNode.LookupByString("linkedString"))) - qt.Check(t, must.Node(n.LookupByString("linkedMap")), qt.Equals, must.Node(mapNode.LookupByString("linkedMap"))) - qt.Check(t, must.Node(n.LookupByString("linkedList")), qt.Equals, must.Node(mapNode.LookupByString("linkedList"))) + qt.Check(t, must.Node(n.LookupByString("linkedString")), qt.Equals, must.Node(rootNode.LookupByString("linkedString"))) + qt.Check(t, must.Node(n.LookupByString("linkedMap")), qt.Equals, must.Node(rootNode.LookupByString("linkedMap"))) + qt.Check(t, must.Node(n.LookupByString("linkedList")), qt.Equals, must.Node(rootNode.LookupByString("linkedList"))) // everything should still be in the same order qt.Check(t, keys(n), qt.DeepEquals, []string{"plain", "linkedString", "linkedMap", "linkedList"}) }) t.Run("UpdateDeeperMap", func(t *testing.T) { - mapNode := duplicateMapNode(middleMapNode) - n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("nested/alink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(middleMapNode, datamodel.ParsePath("nested/alink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "nested/alink") qt.Check(t, prev, nodetests.NodeContentEquals, basicnode.NewLink(leafAlphaLnk)) return basicnode.NewString("new string!"), nil @@ -223,14 +221,13 @@ func TestFocusedTransform(t *testing.T) { // updated value should be there qt.Check(t, must.Node(must.Node(n.LookupByString("nested")).LookupByString("alink")), nodetests.NodeContentEquals, basicnode.NewString("new string!")) // everything else in the parent map should should be there! - qt.Check(t, must.Node(n.LookupByString("foo")), qt.Equals, must.Node(mapNode.LookupByString("foo"))) - qt.Check(t, must.Node(n.LookupByString("bar")), qt.Equals, must.Node(mapNode.LookupByString("bar"))) + qt.Check(t, must.Node(n.LookupByString("foo")), qt.Equals, must.Node(middleMapNode.LookupByString("foo"))) + qt.Check(t, must.Node(n.LookupByString("bar")), qt.Equals, must.Node(middleMapNode.LookupByString("bar"))) // everything should still be in the same order qt.Check(t, keys(n), qt.DeepEquals, []string{"foo", "bar", "nested"}) }) t.Run("AppendIfNotExists", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) - n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "newpart") qt.Check(t, prev, qt.IsNil) // REVIEW: should datamodel.Absent be used here? I lean towards "no" but am unsure what's least surprising here. // An interesting thing to note about inserting a value this way is that you have no `prev.Prototype().NewBuilder()` to use if you wanted to. @@ -245,8 +242,7 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, keys(n), qt.DeepEquals, []string{"plain", "linkedString", "linkedMap", "linkedList", "newpart"}) }) t.Run("CreateParents", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) - n, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "newsection/newpart") qt.Check(t, prev, qt.IsNil) // REVIEW: should datamodel.Absent be used here? I lean towards "no" but am unsure what's least surprising here. return basicnode.NewString("new string!"), nil @@ -264,16 +260,14 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, keys(n2), qt.DeepEquals, []string{"newpart"}) }) t.Run("CreateParentsRequiresPermission", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) - _, err := traversal.FocusedTransform(mapNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + _, err := traversal.FocusedTransform(rootNode, datamodel.ParsePath("newsection/newpart"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, true, qt.IsFalse) // ought not be reached return nil, nil }, false) qt.Check(t, err.Error(), qt.Equals, "transform: parent position at \"newsection\" did not exist (and createParents was false)") }) t.Run("UpdateListEntry", func(t *testing.T) { - listNode := duplicateListNode(middleListNode) - n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("2"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("2"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "2") qt.Check(t, prev, nodetests.NodeContentEquals, basicnode.NewLink(leafBetaLnk)) return basicnode.NewString("new string!"), nil @@ -289,8 +283,7 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, must.Node(n.LookupByIndex(3)), nodetests.NodeContentEquals, basicnode.NewLink(leafAlphaLnk)) }) t.Run("AppendToList", func(t *testing.T) { - listNode := duplicateListNode(middleListNode) - n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("-"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("-"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "4") qt.Check(t, prev, qt.IsNil) return basicnode.NewString("new string!"), nil @@ -303,16 +296,14 @@ func TestFocusedTransform(t *testing.T) { qt.Check(t, n.Length(), qt.Equals, int64(5)) }) t.Run("ListBounds", func(t *testing.T) { - listNode := duplicateListNode(middleListNode) - _, err := traversal.FocusedTransform(listNode, datamodel.ParsePath("4"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + _, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath("4"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, true, qt.IsFalse) // ought not be reached return nil, nil }, false) qt.Check(t, err, qt.ErrorMatches, "transform: cannot navigate path segment \"4\" at \"\" because it is beyond the list bounds") }) t.Run("ReplaceRoot", func(t *testing.T) { // a fairly degenerate case and no reason to do this, but should work. - listNode := duplicateListNode(middleListNode) - n, err := traversal.FocusedTransform(listNode, datamodel.ParsePath(""), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + n, err := traversal.FocusedTransform(middleListNode, datamodel.ParsePath(""), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "") qt.Check(t, prev, nodetests.NodeContentEquals, middleListNode) nb := basicnode.Prototype.Any.NewBuilder() @@ -336,10 +327,9 @@ func TestFocusedTransformWithLinks(t *testing.T) { LinkTargetNodePrototypeChooser: basicnode.Chooser, } t.Run("UpdateMapBeyondLink", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) n, err := traversal.Progress{ Cfg: &cfg, - }.FocusedTransform(mapNode, datamodel.ParsePath("linkedMap/nested/nonlink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + }.FocusedTransform(rootNode, datamodel.ParsePath("linkedMap/nested/nonlink"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "linkedMap/nested/nonlink") qt.Check(t, must.String(prev), qt.Equals, "zoo") qt.Check(t, progress.LastBlock.Path.String(), qt.Equals, "linkedMap") @@ -356,11 +346,10 @@ func TestFocusedTransformWithLinks(t *testing.T) { store2 = memstore.Store{} }) t.Run("UpdateNotBeyondLink", func(t *testing.T) { - mapNode := duplicateMapNode(rootNode) // This is replacing a link with a non-link. Doing so shouldn't hit storage. n, err := traversal.Progress{ Cfg: &cfg, - }.FocusedTransform(mapNode, datamodel.ParsePath("linkedMap"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { + }.FocusedTransform(rootNode, datamodel.ParsePath("linkedMap"), func(progress traversal.Progress, prev datamodel.Node) (datamodel.Node, error) { qt.Check(t, progress.Path.String(), qt.Equals, "linkedMap") nb := prev.Prototype().NewBuilder() nb.AssignString("new string!") @@ -385,17 +374,3 @@ func keys(n datamodel.Node) []string { } return v } - -func duplicateMapNode(n datamodel.Node) datamodel.Node { - amender := basicnode.Prototype.Map.AmendingBuilder(nil) - // Make a deep copy of the node - datamodel.Copy(n, amender) - return amender.Build() -} - -func duplicateListNode(n datamodel.Node) datamodel.Node { - amender := basicnode.Prototype.List.AmendingBuilder(nil) - // Make a deep copy of the node - datamodel.Copy(n, amender) - return amender.Build() -} diff --git a/traversal/walk.go b/traversal/walk.go index e1b551a3..566cd47d 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -494,13 +494,21 @@ func (prog Progress) walkTransforming(n datamodel.Node, s selector.Selector, fn nk := n.Kind() switch nk { case datamodel.Kind_List: - if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingListAmend); castOk { - return prog.walk_transform_iterateAmendableList(n, s, fn, s.Interests()) + if np, castOk := n.Prototype().(datamodel.NodePrototypeSupportingListAmend); castOk { + a := np.AmendingBuilder(nil) + if err := datamodel.Copy(n, a); err != nil { + return nil, err + } + return prog.walk_transform_iterateAmendableList(a, s, fn, s.Interests()) } return prog.walk_transform_iterateList(n, s, fn, s.Interests()) case datamodel.Kind_Map: - if _, castOk := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend); castOk { - return prog.walk_transform_iterateAmendableMap(n, s, fn, s.Interests()) + if np, castOk := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend); castOk { + a := np.AmendingBuilder(nil) + if err := datamodel.Copy(n, a); err != nil { + return nil, err + } + return prog.walk_transform_iterateAmendableMap(a, s, fn, s.Interests()) } return prog.walk_transform_iterateMap(n, s, fn, s.Interests()) default: @@ -579,9 +587,8 @@ func (prog Progress) walk_transform_iterateList(n datamodel.Node, s selector.Sel return bldr.Build(), nil } -func (prog Progress) walk_transform_iterateAmendableList(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { - listAmender := n.Prototype().(datamodel.NodePrototypeSupportingListAmend).AmendingBuilder(n) - +func (prog Progress) walk_transform_iterateAmendableList(listAmender datamodel.ListAmender, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { + n := listAmender.Build() for itr := selector.NewSegmentIterator(n); !itr.Done(); { ps, v, err := itr.Next() if err != nil { @@ -706,9 +713,8 @@ func (prog Progress) walk_transform_iterateMap(n datamodel.Node, s selector.Sele return bldr.Build(), nil } -func (prog Progress) walk_transform_iterateAmendableMap(n datamodel.Node, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { - mapAmender := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend).AmendingBuilder(n) - +func (prog Progress) walk_transform_iterateAmendableMap(mapAmender datamodel.MapAmender, s selector.Selector, fn TransformFn, attn []datamodel.PathSegment) (datamodel.Node, error) { + n := mapAmender.Build() for itr := selector.NewSegmentIterator(n); !itr.Done(); { ps, v, err := itr.Next() if err != nil { diff --git a/traversal/walk_test.go b/traversal/walk_test.go index 87657826..e7588442 100644 --- a/traversal/walk_test.go +++ b/traversal/walk_test.go @@ -781,8 +781,7 @@ func TestWalkTransforming(t *testing.T) { s, err := ss.Selector() qt.Assert(t, err, qt.IsNil) var order int - mapNode := duplicateMapNode(middleMapNode) - n, err := traversal.WalkTransforming(mapNode, s, func(prog traversal.Progress, n datamodel.Node) (datamodel.Node, error) { + n, err := traversal.WalkTransforming(middleMapNode, s, func(prog traversal.Progress, n datamodel.Node) (datamodel.Node, error) { switch order { case 0: qt.Check(t, n, nodetests.NodeContentEquals, basicnode.NewBool(true)) From bc8e8e07c6bb0bfabf627c9936d785c4a94752ac Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:28:20 -0400 Subject: [PATCH 11/12] fix: go 0.17.x compatibility + cleanup --- node/basicnode/any.go | 19 +++++++++- node/basicnode/list.go | 43 ++++++++++++---------- node/basicnode/list_test.go | 47 +++++++++++------------- node/basicnode/map.go | 9 ++--- node/basicnode/map_test.go | 73 +++++++++++++------------------------ traversal/walk.go | 12 +----- 6 files changed, 93 insertions(+), 110 deletions(-) diff --git a/node/basicnode/any.go b/node/basicnode/any.go index 5334d56e..a1f33554 100644 --- a/node/basicnode/any.go +++ b/node/basicnode/any.go @@ -1,6 +1,9 @@ package basicnode import ( + "fmt" + "reflect" + "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/linking" ) @@ -210,8 +213,20 @@ func (nb *anyBuilder) Build() datamodel.Node { // -- NodeAmender --> func (nb *anyBuilder) Transform(path datamodel.Path, transform datamodel.AmendFn) (datamodel.Node, error) { - // If `baseNode` is set and supports amendment, apply the transformation. If it doesn't, and the root is being - // replaced, replace it. If the transformation is for a nested node in a non-amendable recursive object, panic. + // If the root is being replaced, replace it. If the transformation is for a nested node in a non-amendable + // recursive object, panic. + if path.Len() == 0 { + prevNode := nb.Build() + if newNode, err := transform(prevNode); err != nil { + return nil, err + } else if newAb, castOk := newNode.(*anyBuilder); !castOk { + return nil, fmt.Errorf("transform: cannot transform root into incompatible type: %v", reflect.TypeOf(newAb)) + } else { + nb.amender = newAb.amender + nb.baseNode = newAb.baseNode + return prevNode, nil + } + } if nb.amender != nil { return nb.amender.Transform(path, transform) } diff --git a/node/basicnode/list.go b/node/basicnode/list.go index f02fb735..632b7a4b 100644 --- a/node/basicnode/list.go +++ b/node/basicnode/list.go @@ -4,8 +4,6 @@ import ( "fmt" "reflect" - "golang.org/x/exp/slices" - "github.com/ipld/go-ipld-prime/datamodel" "github.com/ipld/go-ipld-prime/node/mixins" ) @@ -129,17 +127,16 @@ func (p Prototype__List) NewBuilder() datamodel.NodeBuilder { // -- NodePrototypeSupportingListAmend --> func (p Prototype__List) AmendingBuilder(base datamodel.Node) datamodel.ListAmender { - var w *plainList + nb := &plainList__Builder{plainList__Assembler{w: &plainList{}}} if base != nil { if baseList, castOk := base.(*plainList); !castOk { panic("misuse") } else { - w = baseList + // Make a deep copy of the base list + datamodel.Copy(baseList, nb) } - } else { - w = &plainList{} } - return &plainList__Builder{plainList__Assembler{w: w}} + return nb } // -- NodeBuilder --> @@ -223,10 +220,10 @@ func (nb *plainList__Builder) transform(path datamodel.Path, transform datamodel if newChildVal, err := transform(prevChildVal); err != nil { return nil, err } else if newChildVal == nil { - // Ref: https://pkg.go.dev/golang.org/x/exp/slices#Delete idx := int(childIdx) nb.w.x[idx] = nil - nb.w.x = slices.Delete(nb.w.x, idx, idx+1) + // Ref: https://pkg.go.dev/golang.org/x/exp/slices#Delete + nb.w.x = append(nb.w.x[:idx], nb.w.x[idx+1:]...) } else if err = nb.storeChildAmender(childIdx, newChildVal, replace); err != nil { return nil, err } @@ -273,15 +270,6 @@ func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.Node if nb.w.x == nil { // Allocate storage space nb.w.x = make([]datamodel.NodeAmender, numElems) - } else { - // Allocate storage space - numElemsToAdd := numElems - if replace { - // We'll be replacing an existing element and need one slot less than the total number of elements being - // added. - numElemsToAdd-- - } - nb.w.x = slices.Grow(nb.w.x, numElemsToAdd) } copyStartIdx := 0 if replace { @@ -291,13 +279,28 @@ func (nb *plainList__Builder) storeChildAmender(childIdx int64, a datamodel.Node } if !replace || (numElems > 1) { // If more elements were specified, insert them after the specified index. - // Ref: https://pkg.go.dev/golang.org/x/exp/slices#Insert - nb.w.x = slices.Insert(nb.w.x, int(childIdx)+copyStartIdx, elems[copyStartIdx:]...) + nb.w.x = Insert(nb.w.x, int(childIdx)+copyStartIdx, elems[copyStartIdx:]...) } } return nil } +// Ref: https://pkg.go.dev/golang.org/x/exp/slices#Insert +func Insert(x []datamodel.NodeAmender, idx int, elems ...datamodel.NodeAmender) []datamodel.NodeAmender { + tot := len(x) + len(elems) + if tot <= cap(x) { + x2 := x[:tot] + copy(x2[idx+len(elems):], x[idx:]) + copy(x2[idx:], elems) + return x2 + } + x2 := make([]datamodel.NodeAmender, tot) + copy(x2, x[:idx]) + copy(x2[idx:], elems) + copy(x2[idx+len(elems):], x[idx:]) + return x2 +} + func (nb *plainList__Builder) Get(idx int64) (datamodel.Node, error) { return nb.w.LookupByIndex(idx) } diff --git a/node/basicnode/list_test.go b/node/basicnode/list_test.go index c796403c..9d8e3ce7 100644 --- a/node/basicnode/list_test.go +++ b/node/basicnode/list_test.go @@ -205,14 +205,14 @@ func TestListAmendingBuilderExistingNode(t *testing.T) { if err != nil { t.Fatal(err) } + newListNode := amender.Build() expect = `list{ 0: string{"fox"} 1: string{"cow"} 2: string{"deer"} 3: string{"cat"} }` - // The original node should have been updated - actual = printer.Sprint(listNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) insertElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { @@ -234,8 +234,7 @@ func TestListAmendingBuilderExistingNode(t *testing.T) { 4: string{"dog"} 5: string{"cat"} }` - // The original node should have been updated - actual = printer.Sprint(listNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) err = amender.Append(basicnode.NewString("eel")) @@ -251,7 +250,15 @@ func TestListAmendingBuilderExistingNode(t *testing.T) { 5: string{"cat"} 6: string{"eel"} }` - // The original node should have been updated + actual = printer.Sprint(newListNode) + qt.Assert(t, actual, qt.Equals, expect) + + // The original node should not have been updated + expect = `list{ + 0: string{"fox"} + 1: string{"cow"} + 2: string{"deer"} +}` actual = printer.Sprint(listNode) qt.Assert(t, actual, qt.Equals, expect) } @@ -265,20 +272,14 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { if err != nil { t.Fatal(err) } + amender := basicnode.Prototype.List.AmendingBuilder(listNode) + newListNode := amender.Build() expect := `list{ 0: string{"fox"} 1: string{"cow"} 2: string{"deer"} }` - amender := basicnode.Prototype.List.AmendingBuilder(nil) - // Copy the map using datamodel.Copy. AssignNode will copy pointers to internal values and will not return a fully - // standalone copy. - err = datamodel.Copy(listNode, amender) - if err != nil { - t.Fatal(err) - } - copiedNode := amender.Build() - actual := printer.Sprint(copiedNode) + actual := printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) setElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { @@ -298,8 +299,7 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { 2: string{"cow"} 3: string{"deer"} }` - // The new node should have been updated - actual = printer.Sprint(copiedNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) insertElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { @@ -321,8 +321,7 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { 4: string{"cow"} 5: string{"deer"} }` - // The new node should have been updated - actual = printer.Sprint(copiedNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) appendElems, err := qp.BuildList(basicnode.Prototype.List, -1, func(am datamodel.ListAssembler) { @@ -344,8 +343,7 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { 5: string{"deer"} 6: string{"rat"} }` - // The new node should have been updated - actual = printer.Sprint(copiedNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) // Pass through an empty list. This should have no effect. @@ -357,8 +355,7 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { if err != nil { t.Fatal(err) } - // The new node should not have been updated - actual = printer.Sprint(copiedNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) // Pass through a list containing another list @@ -378,6 +375,7 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { if err != nil { t.Fatal(err) } + // The new node should have been updated to have a list node at the end expect = `list{ 0: string{"cat"} 1: string{"eel"} @@ -390,16 +388,15 @@ func TestListAmendingBuilderCopiedNode(t *testing.T) { 0: string{"bat"} } }` - // The new node should have been updated to have a list node at the end - actual = printer.Sprint(copiedNode) + actual = printer.Sprint(newListNode) qt.Assert(t, actual, qt.Equals, expect) + // The original node should not have been updated expect = `list{ 0: string{"fox"} 1: string{"cow"} 2: string{"deer"} }` - // The original node should not have been updated actual = printer.Sprint(listNode) qt.Assert(t, actual, qt.Equals, expect) } diff --git a/node/basicnode/map.go b/node/basicnode/map.go index 1cb43440..de2b2045 100644 --- a/node/basicnode/map.go +++ b/node/basicnode/map.go @@ -128,17 +128,16 @@ func (p Prototype__Map) NewBuilder() datamodel.NodeBuilder { // -- NodePrototypeSupportingMapAmend --> func (p Prototype__Map) AmendingBuilder(base datamodel.Node) datamodel.MapAmender { - var b *plainMap + nb := &plainMap__Builder{plainMap__Assembler{w: &plainMap{}}} if base != nil { if baseMap, castOk := base.(*plainMap); !castOk { panic("misuse") } else { - b = baseMap + // Make a deep copy of the base map + datamodel.Copy(baseMap, nb) } - } else { - b = &plainMap{} } - return &plainMap__Builder{plainMap__Assembler{w: b}} + return nb } // -- NodeBuilder --> diff --git a/node/basicnode/map_test.go b/node/basicnode/map_test.go index 65441233..415c2919 100644 --- a/node/basicnode/map_test.go +++ b/node/basicnode/map_test.go @@ -331,15 +331,13 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { t.Fatal(err) } - // compare the printed map mapNode := amender.Build() - actual := printer.Sprint(mapNode) - expect := `map{ string{"cat"}: string{"meow"} string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` + actual := printer.Sprint(mapNode) qt.Check(t, expect, qt.Equals, actual) // Access values of map, using string @@ -380,29 +378,16 @@ func TestMapAmendingBuilderNewNode(t *testing.T) { } qt.Check(t, "purr", qt.Equals, must.String(r)) - expect = `map{ - string{"cat"}: string{"purr"} - string{"dog"}: string{"bark"} - string{"eel"}: string{"zap"} -}` - - // The original node should have been updated - actual = printer.Sprint(mapNode) - qt.Assert(t, actual, qt.Equals, expect) - // Remove an entry removed, err := amender.Remove("cat") if err != nil { t.Fatal(err) } qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) - expect = `map{ string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` - - // The original node should have been updated actual = printer.Sprint(mapNode) qt.Assert(t, actual, qt.Equals, expect) @@ -463,43 +448,41 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { t.Fatal(err) } + mapNode := b.Build() // Wrap in an amending build amender := basicnode.Prototype.Map.AmendingBuilder(b.Build()) - - // Compare the printed map - mapNode := b.Build() - actual := printer.Sprint(mapNode) - + newMapNode := amender.Build() expect := `map{ string{"cat"}: string{"meow"} string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` + actual := printer.Sprint(newMapNode) qt.Check(t, expect, qt.Equals, actual) // Access values of map, using string - r, err := mapNode.LookupByString("cat") + r, err := newMapNode.LookupByString("cat") if err != nil { t.Fatal(err) } qt.Check(t, "meow", qt.Equals, must.String(r)) // Access values of map, using node - r, err = mapNode.LookupByNode(basicnode.NewString("dog")) + r, err = newMapNode.LookupByNode(basicnode.NewString("dog")) if err != nil { t.Fatal(err) } qt.Check(t, "bark", qt.Equals, must.String(r)) // Access values of map, using PathSegment - r, err = mapNode.LookupBySegment(datamodel.ParsePathSegment("eel")) + r, err = newMapNode.LookupBySegment(datamodel.ParsePathSegment("eel")) if err != nil { t.Fatal(err) } qt.Check(t, "zap", qt.Equals, must.String(r)) // Validate the node's prototype - np := mapNode.Prototype() + np := newMapNode.Prototype() qt.Check(t, fmt.Sprintf("%T", np), qt.Equals, "basicnode.Prototype__Map") // Amend the map @@ -514,15 +497,12 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { t.Fatal(err) } qt.Check(t, "purr", qt.Equals, must.String(r)) - expect = `map{ string{"cat"}: string{"purr"} string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` - - // The original node should have been updated - actual = printer.Sprint(mapNode) + actual = printer.Sprint(newMapNode) qt.Assert(t, actual, qt.Equals, expect) // Remove an entry @@ -531,14 +511,11 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { t.Fatal(err) } qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) - expect = `map{ string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` - - // The original node should have been updated - actual = printer.Sprint(mapNode) + actual = printer.Sprint(newMapNode) qt.Assert(t, actual, qt.Equals, expect) // Should not find "cat" @@ -568,6 +545,15 @@ func TestMapAmendingBuilderExistingNode(t *testing.T) { }` actual = printer.Sprint(values) qt.Assert(t, actual, qt.Equals, expect) + + // The original node should not have been updated + expect = `map{ + string{"cat"}: string{"meow"} + string{"dog"}: string{"bark"} + string{"eel"}: string{"zap"} +}` + actual = printer.Sprint(mapNode) + qt.Assert(t, actual, qt.Equals, expect) } func TestMapAmendingBuilderCopiedNode(t *testing.T) { @@ -599,22 +585,15 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { } mapNode := b.Build() - // Copy the map using datamodel.Copy. AssignNode will copy pointers to internal values and will not return a fully - // standalone copy. - amender := basicnode.Prototype.Map.AmendingBuilder(nil) - err = datamodel.Copy(mapNode, amender) - if err != nil { - t.Fatal(err) - } - - copiedMapNode := amender.Build() - actual := printer.Sprint(copiedMapNode) + amender := basicnode.Prototype.Map.AmendingBuilder(mapNode) + newMapNode := amender.Build() expect := `map{ string{"cat"}: string{"meow"} string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` + actual := printer.Sprint(newMapNode) qt.Check(t, expect, qt.Equals, actual) // Amend the copied map @@ -622,15 +601,12 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { if err != nil { t.Fatal(err) } - expect = `map{ string{"cat"}: string{"purr"} string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` - - // The copied node should have been updated - actual = printer.Sprint(copiedMapNode) + actual = printer.Sprint(newMapNode) qt.Assert(t, actual, qt.Equals, expect) // Remove an entry @@ -639,11 +615,12 @@ func TestMapAmendingBuilderCopiedNode(t *testing.T) { t.Fatal(err) } qt.Assert(t, removed, qt.IsTrue, qt.Commentf("remove should have returned true")) - expect = `map{ string{"dog"}: string{"bark"} string{"eel"}: string{"zap"} }` + actual = printer.Sprint(newMapNode) + qt.Assert(t, actual, qt.Equals, expect) _, err = amender.Get("cat") if _, notFoundErr := err.(datamodel.ErrNotExists); !notFoundErr { diff --git a/traversal/walk.go b/traversal/walk.go index 566cd47d..402aec8d 100644 --- a/traversal/walk.go +++ b/traversal/walk.go @@ -495,20 +495,12 @@ func (prog Progress) walkTransforming(n datamodel.Node, s selector.Selector, fn switch nk { case datamodel.Kind_List: if np, castOk := n.Prototype().(datamodel.NodePrototypeSupportingListAmend); castOk { - a := np.AmendingBuilder(nil) - if err := datamodel.Copy(n, a); err != nil { - return nil, err - } - return prog.walk_transform_iterateAmendableList(a, s, fn, s.Interests()) + return prog.walk_transform_iterateAmendableList(np.AmendingBuilder(n), s, fn, s.Interests()) } return prog.walk_transform_iterateList(n, s, fn, s.Interests()) case datamodel.Kind_Map: if np, castOk := n.Prototype().(datamodel.NodePrototypeSupportingMapAmend); castOk { - a := np.AmendingBuilder(nil) - if err := datamodel.Copy(n, a); err != nil { - return nil, err - } - return prog.walk_transform_iterateAmendableMap(a, s, fn, s.Interests()) + return prog.walk_transform_iterateAmendableMap(np.AmendingBuilder(n), s, fn, s.Interests()) } return prog.walk_transform_iterateMap(n, s, fn, s.Interests()) default: From 422316a530f9f24dd6fdc2bb77b90e30342982df Mon Sep 17 00:00:00 2001 From: Mohsin Zaidi <2236875+smrz2001@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:40:08 -0400 Subject: [PATCH 12/12] fix: go mod tidy --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index d3afbc2c..10b29fab 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,6 @@ require ( github.com/multiformats/go-multihash v0.2.3 github.com/polydawn/refmt v0.89.0 github.com/warpfork/go-testmark v0.12.1 - golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 gopkg.in/yaml.v2 v2.4.0 ) diff --git a/go.sum b/go.sum index 7589b43c..5cbfca6f 100644 --- a/go.sum +++ b/go.sum @@ -59,8 +59,6 @@ github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0/go.mod h1:x6AKhvS golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= -golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 h1:MGwJjxBy0HJshjDNfLsYO8xppfqWlA5ZT9OhtUUhTNw= -golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=