diff --git a/coding.go b/coding.go index 952c1aa..5292c65 100644 --- a/coding.go +++ b/coding.go @@ -71,10 +71,14 @@ func fromImmutableNode(encoded *immutableProtoNode) *ProtoNode { link.Cid = c n.links[i] = link } + // we don't set n.linksDirty because the order of the links list from + // serialized form needs to be stable, until we start mutating the ProtoNode return n } func (n *ProtoNode) marshalImmutable() (*immutableProtoNode, error) { - // ensure links are sorted, but don't modify existing link slice + // ensure links are sorted, but don't modify existing link slice; even if + // linksDirty is true, the links list may not be sorted if it came from a + // badly serialized form with unsorted links links := n.Links() sort.Stable(LinkSlice(links)) nd, err := qp.BuildMap(dagpb.Type.PBNode, 2, func(ma ipld.MapAssembler) { @@ -136,8 +140,9 @@ func (n *ProtoNode) GetPBNode() *pb.PBNode { } } - // Ensure links are sorted prior to encode. They may not have come sorted if - // we deserialized a badly encoded form that didn't have links already sorted. + // Ensure links are sorted prior to encode, regardless of `linksDirty`. They + // may not have come sorted if we deserialized a badly encoded form that + // didn't have links already sorted. sort.Stable(pbLinkSlice(pbn.Links)) if len(n.data) > 0 { diff --git a/node.go b/node.go index 4856565..aed14e4 100644 --- a/node.go +++ b/node.go @@ -42,8 +42,9 @@ type immutableProtoNode struct { // this mutable protonode implementation is needed to support go-unixfs, // the only library that implements both read and write for UnixFS v1. type ProtoNode struct { - links []*format.Link - data []byte + links []*format.Link + linksDirty bool + data []byte // cache encoded/marshaled value encoded *immutableProtoNode @@ -148,22 +149,20 @@ func (n *ProtoNode) AddNodeLink(name string, that format.Node) error { // RemoveNodeLink for the same link, will not result in an identically encoded // form as the links will have been sorted. func (n *ProtoNode) AddRawLink(name string, l *format.Link) error { - n.encoded = nil n.links = append(n.links, &format.Link{ Name: name, Size: l.Size, Cid: l.Cid, }) - sort.Stable(LinkSlice(n.links)) + n.linksDirty = true // needs a sort + n.encoded = nil return nil } // RemoveNodeLink removes a link on this node by the given name. If there are // no links with this name, ErrLinkNotFound will be returned. If there are more -// thank one link with this name, they will all be removed. +// than one link with this name, they will all be removed. func (n *ProtoNode) RemoveNodeLink(name string) error { - n.encoded = nil - ref := n.links[:0] found := false @@ -180,6 +179,11 @@ func (n *ProtoNode) RemoveNodeLink(name string) error { } n.links = ref + // Even though a removal won't change sorting, this node may have come from + // a deserialized state with badly sorted links. Now that we are mutating, + // we need to ensure the resulting link list is sorted when it gets consumed. + n.linksDirty = true + n.encoded = nil return nil } @@ -223,8 +227,10 @@ func (n *ProtoNode) GetLinkedNode(ctx context.Context, ds format.DAGService, nam return lnk.GetNode(ctx, ds) } -// Copy returns a copy of the node. -// NOTE: Does not make copies of Node objects in the links. +// Copy returns a copy of the node. The resulting node will have a properly +// sorted Links list regardless of whether the original came from a badly +// serialized form that didn't have a sorted list. +// NOTE: This does not make copies of Node objects in the links. func (n *ProtoNode) Copy() format.Node { nnode := new(ProtoNode) if len(n.data) > 0 { @@ -235,6 +241,10 @@ func (n *ProtoNode) Copy() format.Node { if len(n.links) > 0 { nnode.links = make([]*format.Link, len(n.links)) copy(nnode.links, n.links) + // Sort links regardless of linksDirty state, this may have come from a + // serialized form that had badly sorted links, in which case linksDirty + // will not be true. + sort.Stable(LinkSlice(nnode.links)) } nnode.builder = n.builder @@ -333,15 +343,23 @@ func (n *ProtoNode) UnmarshalJSON(b []byte) error { } n.data = s.Data - // links may not be sorted after deserialization, but we don't change + // Links may not be sorted after deserialization, but we don't change // them until we mutate this node since we're representing the current, - // as-serialized state + // as-serialized state. So n.linksDirty is not set here. n.links = s.Links + n.encoded = nil return nil } // MarshalJSON returns a JSON representation of the node. func (n *ProtoNode) MarshalJSON() ([]byte, error) { + if n.linksDirty { + // there was a mutation involving links, make sure we sort + sort.Stable(LinkSlice(n.links)) + n.linksDirty = false + n.encoded = nil + } + out := map[string]interface{}{ "data": n.data, "links": n.links, @@ -387,6 +405,12 @@ func (n *ProtoNode) Multihash() mh.Multihash { // Links returns a copy of the node's links. func (n *ProtoNode) Links() []*format.Link { + if n.linksDirty { + // there was a mutation involving links, make sure we sort + sort.Stable(LinkSlice(n.links)) + n.linksDirty = false + n.encoded = nil + } links := make([]*format.Link, len(n.links)) copy(links, n.links) return links @@ -397,7 +421,8 @@ func (n *ProtoNode) Links() []*format.Link { func (n *ProtoNode) SetLinks(links []*format.Link) { n.links = make([]*format.Link, len(links)) copy(n.links, links) - sort.Stable(LinkSlice(n.links)) + n.linksDirty = true // needs a sort + n.encoded = nil } // Resolve is an alias for ResolveLink. @@ -429,6 +454,13 @@ func (n *ProtoNode) Tree(p string, depth int) []string { return nil } + if n.linksDirty { + // there was a mutation involving links, make sure we sort + sort.Stable(LinkSlice(n.links)) + n.linksDirty = false + n.encoded = nil + } + out := make([]string, 0, len(n.links)) for _, lnk := range n.links { out = append(out, lnk.Name)