From f05034587e76ac433266d4f206a9f863ec76fe5d Mon Sep 17 00:00:00 2001 From: Henrique Dias Date: Thu, 7 Sep 2023 10:19:38 +0200 Subject: [PATCH] refactor!: remove .Remainder, coreiface.ResolvePath returns remainder --- coreiface/coreapi.go | 6 +++-- coreiface/tests/block.go | 2 +- coreiface/tests/dag.go | 2 +- coreiface/tests/path.go | 12 ++++----- gateway/blocks_backend.go | 55 +++++++++++++++++++++++---------------- gateway/gateway.go | 7 ++--- gateway/handler_codec.go | 7 ++--- path/path.go | 26 +++++++++++++----- path/path_test.go | 24 ++++++++--------- path/resolver/resolver.go | 4 +-- 10 files changed, 86 insertions(+), 59 deletions(-) diff --git a/coreiface/coreapi.go b/coreiface/coreapi.go index 19a1d5ebba..2c4b6623f7 100644 --- a/coreiface/coreapi.go +++ b/coreiface/coreapi.go @@ -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 diff --git a/coreiface/tests/block.go b/coreiface/tests/block.go index 4647165d17..e825d3e615 100644 --- a/coreiface/tests/block.go +++ b/coreiface/tests/block.go @@ -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) } diff --git a/coreiface/tests/dag.go b/coreiface/tests/dag.go index 425c8b4178..767fc3b422 100644 --- a/coreiface/tests/dag.go +++ b/coreiface/tests/dag.go @@ -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) } diff --git a/coreiface/tests/path.go b/coreiface/tests/path.go index d356de39a7..739e63e1e4 100644 --- a/coreiface/tests/path.go +++ b/coreiface/tests/path.go @@ -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) { @@ -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) { @@ -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"`) } @@ -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()) } diff --git a/gateway/blocks_backend.go b/gateway/blocks_backend.go index e5ddf87626..26d60c393e 100644 --- a/gateway/blocks_backend.go +++ b/gateway/blocks_backend.go @@ -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() @@ -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. @@ -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 @@ -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 } - 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 } 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) { @@ -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) if err != nil { return false } @@ -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) if err != nil { - return nil, err + return nil, nil, err } } 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()) } - 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 + } + + imPath, err := path.NewImmutablePath(p) + if err != nil { + return nil, nil, err } - return path.NewImmutablePath(p) + return imPath, remainder, nil } type nodeGetterToCarExporer struct { diff --git a/gateway/gateway.go b/gateway/gateway.go index 3be6c10f31..c9c663eecd 100644 --- a/gateway/gateway.go +++ b/gateway/gateway.go @@ -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 diff --git a/gateway/handler_codec.go b/gateway/handler_codec.go index df222a7c21..b5dad4b4c6 100644 --- a/gateway/handler_codec.go +++ b/gateway/handler_codec.go @@ -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) i.webError(w, r, err, http.StatusNotImplemented) return false } diff --git a/path/path.go b/path/path.go index 172f9dce53..f0b0a7aed5 100644 --- a/path/path.go +++ b/path/path.go @@ -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, "/") +} + +// 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, "/") +} + // Path is a generic, valid, and well-formed path. A valid path is shaped as follows: // // /{namespace}/{root}[/remaining/path] @@ -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{} @@ -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, "/") @@ -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{} @@ -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() } diff --git a/path/path_test.go b/path/path_test.go index 08b692f39f..e7300ff22e 100644 --- a/path/path_test.go +++ b/path/path_test.go @@ -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 { @@ -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()) } }) } diff --git a/path/resolver/resolver.go b/path/resolver/resolver.go index 57b763785c..275bc02489 100644 --- a/path/resolver/resolver.go +++ b/path/resolver/resolver.go @@ -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. @@ -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()