Skip to content

Commit

Permalink
fix: allow skipping publish of existing advert (#117)
Browse files Browse the repository at this point in the history
If one or multiple users upload the same blob multiple times the
`assert/index` invocation will create an advert that already exists.
This will cause `ipni-publisher` to return `ErrAlreadyAdvertised` and
the invocation fails. This PR ignores `ErrAlreadyAdvertised` because,
that's fine, whatever.

It also adds a mutex around publish per
storacha/ipni-publisher#3

---------

Co-authored-by: Vicente Olmedo <[email protected]>
  • Loading branch information
alanshaw and volmedo authored Feb 11, 2025
1 parent ece32ff commit 9b1c5d6
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
14 changes: 13 additions & 1 deletion pkg/service/providerindex/providerindex.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package providerindex
import (
"bytes"
"context"
"errors"
"fmt"
"iter"
"slices"
"sync"

logging "github.com/ipfs/go-log/v2"
ipnifind "github.com/ipni/go-libipni/find/client"
Expand Down Expand Up @@ -36,6 +38,7 @@ type ProviderIndexService struct {
findClient ipnifind.Finder
publisher publisher.Publisher
legacyClaims LegacyClaimsFinder
mutex sync.Mutex
}

var _ ProviderIndex = (*ProviderIndexService)(nil)
Expand Down Expand Up @@ -254,9 +257,18 @@ func (pi *ProviderIndexService) Publish(ctx context.Context, provider peer.AddrI
return fmt.Errorf("caching provider results: %w", err)
}

pi.mutex.Lock()
defer pi.mutex.Unlock()

id, err := pi.publisher.Publish(ctx, provider, contextID, digests, meta)
if err != nil {
return err
if errors.Is(err, publisher.ErrAlreadyAdvertised) {
// skipping is ok in this case
log.Warnf("Skipping previously published advert")
return nil
}

return fmt.Errorf("publishing advert: %w", err)
}
log.Infof("published IPNI advert: %s", id)
return nil
Expand Down
34 changes: 34 additions & 0 deletions pkg/service/providerindex/providerindex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ package providerindex
import (
"context"
"errors"
"iter"
"slices"
"testing"

"github.com/ipni/go-libipni/find/model"
"github.com/multiformats/go-multicodec"
"github.com/multiformats/go-multihash"
"github.com/storacha/go-metadata"
"github.com/storacha/indexing-service/pkg/internal/testutil"
"github.com/storacha/indexing-service/pkg/internal/testutil/extmocks"
"github.com/storacha/indexing-service/pkg/types"
"github.com/storacha/ipni-publisher/pkg/publisher"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -221,3 +226,32 @@ func TestGetProviderResults(t *testing.T) {
require.Error(t, err)
})
}

func TestPublish(t *testing.T) {
t.Run("allow skip publish existing advert", func(t *testing.T) {
mockStore := types.NewMockProviderStore(t)
mockIpniFinder := extmocks.NewMockIpniFinder(t)
mockIpniPublisher := extmocks.NewMockIpniPublisher(t)
mockLegacyClaims := NewMockLegacyClaimsFinder(t)

providerIndex := New(mockStore, mockIpniFinder, mockIpniPublisher, mockLegacyClaims)

result := testutil.RandomLocationCommitmentProviderResult()
provider := *result.Provider
contextID := string(result.ContextID)
digest := testutil.RandomMultihash()
anyDigestSeq := mock.MatchedBy(func(digests iter.Seq[multihash.Multihash]) bool {
return true
})
meta := metadata.MetadataContext.New()
err := meta.UnmarshalBinary(result.Metadata)
require.NoError(t, err)

mockStore.EXPECT().Add(testutil.AnyContext, digest, result).Return(1, nil)
mockStore.EXPECT().SetExpirable(testutil.AnyContext, digest, false).Return(nil)
mockIpniPublisher.EXPECT().Publish(testutil.AnyContext, provider, contextID, anyDigestSeq, meta).Return(testutil.RandomCID(), publisher.ErrAlreadyAdvertised)

err = providerIndex.Publish(context.Background(), provider, contextID, slices.Values([]multihash.Multihash{digest}), meta)
require.NoError(t, err)
})
}

0 comments on commit 9b1c5d6

Please sign in to comment.