Skip to content

Commit

Permalink
refactor!: remove .Remainder, coreiface.ResolvePath returns remainder
Browse files Browse the repository at this point in the history
  • Loading branch information
hacdias committed Sep 7, 2023
1 parent 3214084 commit f050345
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 59 deletions.
6 changes: 4 additions & 2 deletions coreiface/coreapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ type CoreAPI interface {
// Routing returns an implementation of Routing API
Routing() RoutingAPI

// ResolvePath resolves the path using Unixfs resolver
ResolvePath(context.Context, path.Path) (path.ImmutablePath, error)
// ResolvePath resolves the path using UnixFS resolver, and returns the resolved
// immutable path, and the remainder of the path segments that cannot be resolved
// within UnixFS.
ResolvePath(context.Context, path.Path) (path.ImmutablePath, path.Segments, error)

// ResolveNode resolves the path (if not resolved already) using Unixfs
// resolver, gets and returns the resolved Node
Expand Down
2 changes: 1 addition & 1 deletion coreiface/tests/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (tp *TestSuite) TestBlockGet(t *testing.T) {

p := path.NewIPFSPath(res.Path().Cid())

rp, err := api.ResolvePath(ctx, p)
rp, _, err := api.ResolvePath(ctx, p)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion coreiface/tests/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (tp *TestSuite) TestDagPath(t *testing.T) {
t.Fatal(err)
}

rp, err := api.ResolvePath(ctx, p)
rp, _, err := api.ResolvePath(ctx, p)
if err != nil {
t.Fatal(err)
}
Expand Down
12 changes: 6 additions & 6 deletions coreiface/tests/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ func (tp *TestSuite) TestPathRemainder(t *testing.T) {
p, err := path.Join(path.NewIPFSPath(nd.Cid()), "foo", "bar")
require.NoError(t, err)

rp1, err := api.ResolvePath(ctx, p)
_, remainder, err := api.ResolvePath(ctx, p)
require.NoError(t, err)
require.Equal(t, "/foo/bar", rp1.Remainder())
require.Equal(t, "/foo/bar", remainder.String())
}

func (tp *TestSuite) TestEmptyPathRemainder(t *testing.T) {
Expand All @@ -74,9 +74,9 @@ func (tp *TestSuite) TestEmptyPathRemainder(t *testing.T) {
err = api.Dag().Add(ctx, nd)
require.NoError(t, err)

rp1, err := api.ResolvePath(ctx, path.NewIPFSPath(nd.Cid()))
_, remainder, err := api.ResolvePath(ctx, path.NewIPFSPath(nd.Cid()))
require.NoError(t, err)
require.Empty(t, rp1.Remainder())
require.Empty(t, remainder)
}

func (tp *TestSuite) TestInvalidPathRemainder(t *testing.T) {
Expand All @@ -96,7 +96,7 @@ func (tp *TestSuite) TestInvalidPathRemainder(t *testing.T) {
p, err := path.Join(path.NewIPLDPath(nd.Cid()), "/bar/baz")
require.NoError(t, err)

_, err = api.ResolvePath(ctx, p)
_, _, err = api.ResolvePath(ctx, p)
require.NotNil(t, err)
require.ErrorContains(t, err, `no link named "bar"`)
}
Expand All @@ -122,7 +122,7 @@ func (tp *TestSuite) TestPathRoot(t *testing.T) {
p, err := path.Join(path.NewIPLDPath(nd.Cid()), "/foo")
require.NoError(t, err)

rp, err := api.ResolvePath(ctx, p)
rp, _, err := api.ResolvePath(ctx, p)
require.NoError(t, err)
require.Equal(t, rp.Cid().String(), blk.Path().Cid().String())
}
Expand Down
55 changes: 33 additions & 22 deletions gateway/blocks_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,14 +485,15 @@ func walkGatewaySimpleSelector(ctx context.Context, p path.Path, params CarParam
}

func (bb *BlocksBackend) getNode(ctx context.Context, path path.ImmutablePath) (ContentPathMetadata, format.Node, error) {
roots, lastSeg, err := bb.getPathRoots(ctx, path)
roots, lastSeg, remainder, err := bb.getPathRoots(ctx, path)
if err != nil {
return ContentPathMetadata{}, nil, err
}

md := ContentPathMetadata{
PathSegmentRoots: roots,
LastSegment: lastSeg,
PathSegmentRoots: roots,
LastSegment: lastSeg,
LastSegmentRemainder: remainder,
}

lastRoot := lastSeg.Cid()
Expand All @@ -505,7 +506,7 @@ func (bb *BlocksBackend) getNode(ctx context.Context, path path.ImmutablePath) (
return md, nd, err
}

func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath path.ImmutablePath) ([]cid.Cid, path.ImmutablePath, error) {
func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath path.ImmutablePath) ([]cid.Cid, path.ImmutablePath, []string, error) {
/*
These are logical roots where each CID represent one path segment
and resolves to either a directory or the root block of a file.
Expand All @@ -529,7 +530,10 @@ func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath path.Immu
contentPathStr := contentPath.String()
pathSegments := strings.Split(contentPathStr[6:], "/")
sp.WriteString(contentPathStr[:5]) // /ipfs or /ipns
var lastPath path.ImmutablePath
var (
lastPath path.ImmutablePath
remainder []string
)
for _, root := range pathSegments {
if root == "" {
continue
Expand All @@ -538,23 +542,24 @@ func (bb *BlocksBackend) getPathRoots(ctx context.Context, contentPath path.Immu
sp.WriteString(root)
p, err := path.NewPath(sp.String())
if err != nil {
return nil, nil, err
return nil, nil, nil, err
}

Check warning on line 546 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L545-L546

Added lines #L545 - L546 were not covered by tests
resolvedSubPath, err := bb.resolvePath(ctx, p)
resolvedSubPath, remainderSubPath, err := bb.resolvePath(ctx, p)
if err != nil {
// TODO: should we be more explicit here and is this part of the IPFSBackend contract?
// The issue here was that we returned datamodel.ErrWrongKind instead of this resolver error
if isErrNotFound(err) {
return nil, nil, resolver.ErrNoLink{Name: root, Node: lastPath.Cid()}
return nil, nil, nil, resolver.ErrNoLink{Name: root, Node: lastPath.Cid()}
}
return nil, nil, err
return nil, nil, nil, err

Check warning on line 554 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L554

Added line #L554 was not covered by tests
}
lastPath = resolvedSubPath
remainder = remainderSubPath
pathRoots = append(pathRoots, lastPath.Cid())
}

pathRoots = pathRoots[:len(pathRoots)-1]
return pathRoots, lastPath, nil
return pathRoots, lastPath, remainder, nil
}

func (bb *BlocksBackend) ResolveMutable(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
Expand Down Expand Up @@ -605,7 +610,7 @@ func (bb *BlocksBackend) GetDNSLinkRecord(ctx context.Context, hostname string)
}

func (bb *BlocksBackend) IsCached(ctx context.Context, p path.Path) bool {
rp, err := bb.resolvePath(ctx, p)
rp, _, err := bb.resolvePath(ctx, p)

Check warning on line 613 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L612-L613

Added lines #L612 - L613 were not covered by tests
if err != nil {
return false
}
Expand All @@ -615,41 +620,47 @@ func (bb *BlocksBackend) IsCached(ctx context.Context, p path.Path) bool {
}

func (bb *BlocksBackend) ResolvePath(ctx context.Context, path path.ImmutablePath) (ContentPathMetadata, error) {
roots, lastSeg, err := bb.getPathRoots(ctx, path)
roots, lastSeg, remainder, err := bb.getPathRoots(ctx, path)
if err != nil {
return ContentPathMetadata{}, err
}
md := ContentPathMetadata{
PathSegmentRoots: roots,
LastSegment: lastSeg,
PathSegmentRoots: roots,
LastSegment: lastSeg,
LastSegmentRemainder: remainder,
}
return md, nil
}

func (bb *BlocksBackend) resolvePath(ctx context.Context, p path.Path) (path.ImmutablePath, error) {
func (bb *BlocksBackend) resolvePath(ctx context.Context, p path.Path) (path.ImmutablePath, []string, error) {
var err error
if p.Namespace() == path.IPNSNamespace {
p, err = resolve.ResolveIPNS(ctx, bb.namesys, p)

Check warning on line 638 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L638

Added line #L638 was not covered by tests
if err != nil {
return nil, err
return nil, nil, err

Check warning on line 640 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L640

Added line #L640 was not covered by tests
}
}

if p.Namespace() != path.IPFSNamespace {
return nil, fmt.Errorf("unsupported path namespace: %s", p.Namespace())
return nil, nil, fmt.Errorf("unsupported path namespace: %s", p.Namespace())

Check warning on line 645 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L645

Added line #L645 was not covered by tests
}

node, rest, err := bb.resolver.ResolveToLastNode(ctx, p)
node, remainder, err := bb.resolver.ResolveToLastNode(ctx, p)
if err != nil {
return nil, err
return nil, nil, err
}

p, err = path.Join(path.NewIPFSPath(node), rest...)
p, err = path.Join(path.NewIPFSPath(node), remainder...)
if err != nil {
return nil, err
return nil, nil, err
}

Check warning on line 656 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L655-L656

Added lines #L655 - L656 were not covered by tests

imPath, err := path.NewImmutablePath(p)
if err != nil {
return nil, nil, err

Check warning on line 660 in gateway/blocks_backend.go

View check run for this annotation

Codecov / codecov/patch

gateway/blocks_backend.go#L660

Added line #L660 was not covered by tests
}

return path.NewImmutablePath(p)
return imPath, remainder, nil
}

type nodeGetterToCarExporer struct {
Expand Down
7 changes: 4 additions & 3 deletions gateway/gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,10 @@ func (d DuplicateBlocksPolicy) String() string {
}

type ContentPathMetadata struct {
PathSegmentRoots []cid.Cid
LastSegment path.ImmutablePath
ContentType string // Only used for UnixFS requests
PathSegmentRoots []cid.Cid
LastSegment path.ImmutablePath
LastSegmentRemainder path.Segments
ContentType string // Only used for UnixFS requests
}

// ByteRange describes a range request within a UnixFS file. "From" and "To" mostly
Expand Down
7 changes: 4 additions & 3 deletions gateway/handler_codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ func (i *handler) renderCodec(ctx context.Context, w http.ResponseWriter, r *htt
// If the resolved path still has some remainder, return error for now.
// TODO: handle this when we have IPLD Patch (https://ipld.io/specs/patch/) via HTTP PUT
// TODO: (depends on https://github.com/ipfs/kubo/issues/4801 and https://github.com/ipfs/kubo/issues/4782)
if resolvedPath.Remainder() != "" {
path := strings.TrimSuffix(resolvedPath.String(), resolvedPath.Remainder())
err := fmt.Errorf("%q of %q could not be returned: reading IPLD Kinds other than Links (CBOR Tag 42) is not implemented: try reading %q instead", resolvedPath.Remainder(), resolvedPath.String(), path)
if len(rq.pathMetadata.LastSegmentRemainder) != 0 {
remainderStr := rq.pathMetadata.LastSegmentRemainder.String()
path := strings.TrimSuffix(resolvedPath.String(), remainderStr)
err := fmt.Errorf("%q of %q could not be returned: reading IPLD Kinds other than Links (CBOR Tag 42) is not implemented: try reading %q instead", remainderStr, resolvedPath.String(), path)

Check warning on line 90 in gateway/handler_codec.go

View check run for this annotation

Codecov / codecov/patch

gateway/handler_codec.go#L88-L90

Added lines #L88 - L90 were not covered by tests
i.webError(w, r, err, http.StatusNotImplemented)
return false
}
Expand Down
26 changes: 20 additions & 6 deletions path/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,23 @@ const (
IPLDNamespace
)

// Segments represents segments of a path.
type Segments []string

// NewSegmentsFromString creates a new [Segments] from a string.
func NewSegmentsFromString(str string) Segments {
str = strings.TrimSuffix(str, "/")
str = strings.TrimPrefix(str, "/")
return strings.Split(str, "/")

Check warning on line 46 in path/path.go

View check run for this annotation

Codecov / codecov/patch

path/path.go#L43-L46

Added lines #L43 - L46 were not covered by tests
}

// String returns the [Segments] concatenated with the path separator "/",
// and starting with a "/". For example, if segments is an array ["foo", "bar"],
// this function will return "/foo/bar".
func (s Segments) String() string {
return "/" + strings.Join(s, "/")

Check warning on line 53 in path/path.go

View check run for this annotation

Codecov / codecov/patch

path/path.go#L52-L53

Added lines #L52 - L53 were not covered by tests
}

// Path is a generic, valid, and well-formed path. A valid path is shaped as follows:
//
// /{namespace}/{root}[/remaining/path]
Expand Down Expand Up @@ -64,7 +81,7 @@ type Path interface {
// - "/ipld/bafkqaaa" returns ["ipld", "bafkqaaa"]
// - "/ipfs/bafkqaaa/a/b/" returns ["ipfs", "bafkqaaa", "a", "b"]
// - "/ipns/dnslink.net" returns ["ipns", "dnslink.net"]
Segments() []string
Segments() Segments
}

var _ Path = path{}
Expand All @@ -82,7 +99,7 @@ func (p path) Namespace() Namespace {
return p.namespace
}

func (p path) Segments() []string {
func (p path) Segments() Segments {
// Trim slashes from beginning and end, such that we do not return empty segments.
str := strings.TrimSuffix(p.str, "/")
str = strings.TrimPrefix(str, "/")
Expand All @@ -96,9 +113,6 @@ type ImmutablePath interface {

// Cid returns the [cid.Cid] of the root object of the path.
Cid() cid.Cid

// Remainder returns the unresolved parts of the path.
Remainder() string
}

var _ Path = immutablePath{}
Expand Down Expand Up @@ -131,7 +145,7 @@ func (ip immutablePath) Namespace() Namespace {
return ip.path.Namespace()
}

func (ip immutablePath) Segments() []string {
func (ip immutablePath) Segments() Segments {
return ip.path.Segments()
}

Expand Down
24 changes: 11 additions & 13 deletions path/path_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,21 +224,20 @@ func TestNewImmutablePath(t *testing.T) {

t.Run("Succeeds on Immutable Path", func(t *testing.T) {
testCases := []struct {
path string
cid cid.Cid
remainder string
path string
cid cid.Cid
}{
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n"), ""},
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n"), "/a/b"},
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b/", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n"), "/a/b"},
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n")},
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n")},
{"/ipfs/QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n/a/b/", cid.MustParse("QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n")},

{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), ""},
{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), "/a/b"},
{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b/", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), "/a/b"},
{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},
{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},
{"/ipfs/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b/", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},

{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), ""},
{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), "/a/b"},
{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b/", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku"), "/a/b"},
{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},
{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},
{"/ipld/bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku/a/b/", cid.MustParse("bafybeihdwdcefgh4dqkjv67uzcmw7ojee6xedzdetojuzjevtenxquvyku")},
}

for _, testCase := range testCases {
Expand All @@ -249,7 +248,6 @@ func TestNewImmutablePath(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, testCase.path, ip.String())
assert.Equal(t, testCase.cid, ip.Cid())
assert.Equal(t, testCase.remainder, ip.Remainder())
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions path/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ type Resolver interface {
// last block referenced by the path, and the path segments to
// traverse from the final block boundary to the final node within the
// block.
ResolveToLastNode(ctx context.Context, fpath path.Path) (cid.Cid, []string, error)
ResolveToLastNode(ctx context.Context, fpath path.Path) (cid.Cid, path.Segments, error)
// ResolvePath fetches the node for given path. It returns the last
// item returned by ResolvePathComponents and the last link traversed
// which can be used to recover the block.
Expand Down Expand Up @@ -79,7 +79,7 @@ func NewBasicResolver(fetcherFactory fetcher.Factory) Resolver {
// ResolveToLastNode walks the given path and returns the cid of the last
// block referenced by the path, and the path segments to traverse from the
// final block boundary to the final node within the block.
func (r *basicResolver) ResolveToLastNode(ctx context.Context, fpath path.Path) (cid.Cid, []string, error) {
func (r *basicResolver) ResolveToLastNode(ctx context.Context, fpath path.Path) (cid.Cid, path.Segments, error) {
ctx, span := startSpan(ctx, "basicResolver.ResolveToLastNode", trace.WithAttributes(attribute.Stringer("Path", fpath)))
defer span.End()

Expand Down

0 comments on commit f050345

Please sign in to comment.