Skip to content

Commit

Permalink
feat(query): add tests for IndexingService.Query function (#87)
Browse files Browse the repository at this point in the history
Resolves storacha/project-tracking#171

Adds tests for the main `IndexingService.Query` function. The happy path
test is useful to understand the behavior of the service fetching the
different types of claims.

**Note for reviewers:** the PR is not as big as it seems. I changed some
interfaces dealing with `url.URL` to use `*url.URL`, which makes client
code a bit more readable and is more idiomatic. A lot of files were
touched just to update the type of the parameter/variable.

## Considerations

I think the happy path test is the most valuable. I also added some
basic tests for error conditions in the main dependencies. I thought of
adding a test confirming the function filters non-relevant claims, but
that is implemented by the `providerindex` package and tested there.

Please let me know if you think I'm missing any other interesting tests.

## Known issues

The addition of `MockProviderIndex` causes an import cycle in the tests
of the `providerindex` package. These tests import the `mocks` package
and the new mock imports the `providerindex` package to implement the
mock.

I think the best solution would be to change the mock generation
configuration so that mocks are created in the same package as the
interfaces they are mocking. That would avoid the cycle while allowing
tests to live in the same package as the functions they are testing
(which in turn enables testing non-exported members).

I'll take care of that in a separate PR (#88).
  • Loading branch information
volmedo authored Jan 16, 2025
1 parent 02a3619 commit c841822
Show file tree
Hide file tree
Showing 28 changed files with 950 additions and 65 deletions.
17 changes: 11 additions & 6 deletions .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,21 @@ filename: "mock_{{.InterfaceName}}.go"
dir: "{{.InterfaceDir}}"
mockname: "Mock{{.InterfaceName}}"
packages:
github.com/storacha/indexing-service/pkg/service/providerindex:
github.com/storacha/indexing-service/pkg/service/blobindexlookup:
interfaces:
ContentToClaimsMapper:
LegacyClaimsFinder:
BlobIndexLookup:
github.com/storacha/indexing-service/pkg/service/contentclaims:
config:
filename: "mock_ContentClaims{{.InterfaceName}}.go"
mockname: "MockContentClaims{{.InterfaceName}}"
interfaces:
Finder:
config:
filename: mock_ContentClaimsFinder.go
mockname: MockContentClaimsFinder
Service:
github.com/storacha/indexing-service/pkg/service/providerindex:
interfaces:
ContentToClaimsMapper:
LegacyClaimsFinder:
ProviderIndex:
github.com/storacha/indexing-service/pkg/types:
interfaces:
ContentClaimsCache:
Expand Down
9 changes: 6 additions & 3 deletions pkg/internal/jobwalker/singlewalk/singlewalk.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,15 @@ func SingleWalker[Job, State any](ctx context.Context, initial []Job, initialSta
return state.Access(), ctx.Err()
default:
}
next := initial[len(initial)-1]
initial = initial[:len(initial)-1]
handler(ctx, next, func(j Job) error {
next := stack[len(stack)-1]
stack = stack[:len(stack)-1]
err := handler(ctx, next, func(j Job) error {
stack = append(stack, j)
return nil
}, state)
if err != nil {
return state.Access(), err
}
}
return state.Access(), nil
}
2 changes: 1 addition & 1 deletion pkg/service/blobindexlookup/cachinglookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func WithCache(blobIndexLookup BlobIndexLookup, shardedDagIndexCache types.Shard
}
}

func (b *cachingLookup) Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
func (b *cachingLookup) Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL *url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
// attempt to read index from cache and return it if succesful
index, err := b.shardDagIndexCache.Get(ctx, contextID)
if err == nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/service/blobindexlookup/cachinglookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestWithCache__Find(t *testing.T) {
// Create ClaimLookup instance
cl := blobindexlookup.WithCache(lookup, mockStore, providerCacher)

index, err := cl.Find(context.Background(), tc.contextID, provider, *testutil.TestURL, nil)
index, err := cl.Find(context.Background(), tc.contextID, provider, testutil.TestURL, nil)
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
Expand Down Expand Up @@ -181,7 +181,7 @@ type mockBlobIndexLookup struct {
err error
}

func (m *mockBlobIndexLookup) Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
func (m *mockBlobIndexLookup) Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL *url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
return m.index, m.err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/service/blobindexlookup/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ type BlobIndexLookup interface {
// 3. return the index
// 4. asyncronously, add records to the ProviderStore from the parsed blob index so that we can avoid future queries to IPNI for
// other multihashes in the index
Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error)
Find(ctx context.Context, contextID types.EncodedContextID, provider model.ProviderResult, fetchURL *url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error)
}
108 changes: 108 additions & 0 deletions pkg/service/blobindexlookup/mock_BlobIndexLookup.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/service/blobindexlookup/simplelookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func NewBlobIndexLookup(httpClient *http.Client) BlobIndexLookup {
}

// Find fetches the blob index from the given fetchURL
func (s *simpleLookup) Find(ctx context.Context, _ types.EncodedContextID, _ model.ProviderResult, fetchURL url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
func (s *simpleLookup) Find(ctx context.Context, _ types.EncodedContextID, _ model.ProviderResult, fetchURL *url.URL, rng *metadata.Range) (blobindex.ShardedDagIndexView, error) {
// attempt to fetch the index from provided url
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fetchURL.String(), nil)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/blobindexlookup/simplelookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestBlobIndexLookup__Find(t *testing.T) {
defer func() { testServer.Close() }()
// Create BlobIndexLookup instance
cl := blobindexlookup.NewBlobIndexLookup(testServer.Client())
index, err := cl.Find(context.Background(), cid.Bytes(), provider, *testutil.Must(url.Parse(testServer.URL))(t), tc.rngHeader)
index, err := cl.Find(context.Background(), cid.Bytes(), provider, testutil.Must(url.Parse(testServer.URL))(t), tc.rngHeader)
if tc.expectedErr != nil {
require.ErrorContains(t, err, tc.expectedErr.Error())
} else {
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/contentclaims/cachingfinder.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func WithCache(finder Finder, cache types.ContentClaimsCache) Finder {
}

// Find attempts to fetch a claim from either the local cache or via the provided URL (caching the result if its fetched)
func (cl *cachingFinder) Find(ctx context.Context, id ipld.Link, fetchURL url.URL) (delegation.Delegation, error) {
func (cl *cachingFinder) Find(ctx context.Context, id ipld.Link, fetchURL *url.URL) (delegation.Delegation, error) {
// attempt to read claim from cache and return it if succesful
claim, err := cl.cache.Get(ctx, link.ToCID(id))
if err == nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/service/contentclaims/cachingfinder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestWithCache__Find(t *testing.T) {
// Create ClaimLookup instance
cl := contentclaims.WithCache(finder, mockCache)

claim, err := cl.Find(context.Background(), tc.claimCid, *testutil.TestURL)
claim, err := cl.Find(context.Background(), tc.claimCid, testutil.TestURL)
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
Expand Down Expand Up @@ -162,7 +162,7 @@ type mockFinder struct {
err error
}

func (m *mockFinder) Find(ctx context.Context, link ipld.Link, url url.URL) (delegation.Delegation, error) {
func (m *mockFinder) Find(ctx context.Context, link ipld.Link, url *url.URL) (delegation.Delegation, error) {
if m.err != nil {
return nil, m.err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/contentclaims/identitycidfinder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func WithIdentityCids(finder Finder) Finder {
}

// Find attempts to fetch a claim from either the permenant storage or via the provided URL
func (idf *identityCidFinder) Find(ctx context.Context, id ipld.Link, fetchURL url.URL) (delegation.Delegation, error) {
func (idf *identityCidFinder) Find(ctx context.Context, id ipld.Link, fetchURL *url.URL) (delegation.Delegation, error) {

if cidLink, ok := id.(cidlink.Link); ok {
if cidLink.Cid.Prefix().MhType == multihash.IDENTITY {
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/contentclaims/identitycidfinder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestIdentityCidFinder__Find(t *testing.T) {
// Create ClaimLookup instance
cl := contentclaims.WithIdentityCids(finder)

claim, err := cl.Find(context.Background(), tc.claimCid, *testutil.TestURL)
claim, err := cl.Find(context.Background(), tc.claimCid, testutil.TestURL)
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
Expand Down
4 changes: 2 additions & 2 deletions pkg/service/contentclaims/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

type Finder interface {
// Find and retrieve a claim via URL.
Find(ctx context.Context, claim ipld.Link, fetchURL url.URL) (delegation.Delegation, error)
Find(ctx context.Context, claim ipld.Link, fetchURL *url.URL) (delegation.Delegation, error)
}

type Service interface {
Expand All @@ -19,7 +19,7 @@ type Service interface {
// Find attempts to read the claim from the cache, falling back to retrieving
// it from storage and finally, if still not found, fetching it from the
// provided URL. The result is stored in the cache.
Find(ctx context.Context, claim ipld.Link, fetchURL url.URL) (delegation.Delegation, error)
Find(ctx context.Context, claim ipld.Link, fetchURL *url.URL) (delegation.Delegation, error)
// Cache writes the claim to the cache with default expiry.
Cache(ctx context.Context, claim delegation.Delegation) error
// Publish writes the claim to the cache, and adds it to storage.
Expand Down
16 changes: 8 additions & 8 deletions pkg/service/contentclaims/mock_ContentClaimsFinder.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c841822

Please sign in to comment.