search

ipfs / go-unixfsnode

An ADL IPLD prime node that wraps go-codec-dagpb's implementation of protobuf to enable pathing
Other
11 stars 5 forks source link

Provide path for getting sizes on directory iteration #60

Closed willscott closed 1 year ago

willscott commented 1 year ago

When making a map iterator over a directory, calls to .Next() currently return a value of the FieldHash() of the dagpb link.

This change has them instead return a IterLink.

In both cases, the "exposed" interface of the returned type, AsLink() exposes the underlying hash cidLink to the entry.

By using a wrapping IterLink type, we provide an opportunity to type-cast the response, and instead of treating the response under it's returned ipld.Node interface, the client has the ability to do something of the form:

k, next, err := iterator.Next()
nextSize := next.(*iter.IterLink).Substrate.FieldSize()
willscott commented 1 year ago

ref: https://github.com/ipfs/bifrost-gateway/pull/160/files#r1288485782

willscott commented 1 year ago

I went with this so that the .AsLink() directly provides .FieldHash().AsLink() of the PBLink as a way of maintaining compatibility with current semantics.

This is (if i remember correctly) to provide the higher level semantics of directory traversal that are used for pathing through unixfs.

aschmahmann commented 1 year ago

@willscott I agree, and probably changing PBLink to allow AsLink to return a cidlink.Link would be conceptually problematic (IIUC the rough go-ipld-prime node contract is that an object is exactly one of the data model types not multiple). I wouldn't be surprised if other uses of PBLink would run into similar issues and want a standard way to access both representations of the data (all the fields, and just the CID). Again, not a blocker for me though.

rvagg commented 1 year ago

Good question @aschmahmann, I'm not sure there's a clean answer (or that I'm the best person to provide one!); except to say that I think there should be a reasonable expectation of consistent behaviour of an ipld.Node based on inspecting its Kind(). A PBLink is a Kind_Map and making it act like a link might lead to some funky (unexpected) behaviour in strange parts of a system using it.

You could look at #51 with that lens and see how the approach here is a bit cleaner. Not that #51 has obvious breakage potential, but this approach has the standard mapping of Kind:Behaviour.