From a32f9da73c2101963006fd9b57812ef9be0f5502 Mon Sep 17 00:00:00 2001 From: gammazero Date: Wed, 18 Jan 2023 03:35:12 -0800 Subject: [PATCH 1/2] Skip advertisements that have no metadata If an ad has no entries and no metadata, then it used only to update provider addresses and is not an error. If an ad has entries but no metadata, then an error is logged an the ad is skipped. Fixes #1148 --- internal/ingest/ingest_test.go | 65 +++++++++++++++++++++++++++++++--- internal/ingest/linksystem.go | 9 +++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/internal/ingest/ingest_test.go b/internal/ingest/ingest_test.go index 897064cb4..a80828957 100644 --- a/internal/ingest/ingest_test.go +++ b/internal/ingest/ingest_test.go @@ -1044,7 +1044,60 @@ func TestSyncTooLargeMetadata(t *testing.T) { if lnk != nil { lcid = lnk.(cidlink.Link).Cid } - require.Equal(t, lcid, cid.Undef) + require.Equal(t, cid.Undef, lcid) +} + +func TestSyncAddrUpdateOnly(t *testing.T) { + srcStore := dssync.MutexWrap(datastore.NewMapDatastore()) + h := mkTestHost() + pubHost := mkTestHost() + i, core, reg, _ := mkIngest(t, h) + defer core.Close() + defer i.Close() + pub, lsys := mkMockPublisher(t, pubHost, srcStore) + defer pub.Close() + connectHosts(t, h, pubHost) + + err := dstest.WaitForPublisher(h, defaultTestIngestConfig.PubSubTopic, pubHost.ID()) + require.NoError(t, err) + + // Test ad that has no entries and no metadata. + adCid, _, providerID, _ := publishRandomIndexAndAdvWithEntriesChunkCount(t, pub, lsys, false, 0, []byte{}) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + endCid, err := i.Sync(ctx, pubHost.ID(), nil, 0, false) + require.NoError(t, err) + + // We receive the CID that we synced. + require.Equal(t, adCid, endCid) + lcid := cid.Undef + + // Check that subscriber recorded latest sync. + lnk := i.sub.GetLatestSync(pubHost.ID()) + if lnk != nil { + lcid = lnk.(cidlink.Link).Cid + } + require.Equal(t, adCid, lcid) + + pInfo, found := reg.ProviderInfo(providerID) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, adCid, pInfo.LastAdvertisement) + + // Test ad that has entries and no metadata. + adCid, _, providerID, _ = publishRandomIndexAndAdvWithEntriesChunkCount(t, pub, lsys, false, 10, []byte{}) + endCid, err = i.Sync(ctx, pubHost.ID(), nil, 0, false) + require.NoError(t, err) + require.Equal(t, adCid, endCid) + + // Even though the ad was malformed, processing it completed and indexer + // can continue processing later ads in the chain. Check that the ad was + // processed. + pInfo, found = reg.ProviderInfo(providerID) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, adCid, pInfo.LastAdvertisement) } func TestReSyncWithDepth(t *testing.T) { @@ -1628,20 +1681,24 @@ func publishRandomIndexAndAdvWithEntriesChunkCount(t *testing.T, pub dagsync.Pub metadata = []byte("test-metadata") } addrs := []string{"/ip4/127.0.0.1/tcp/9999"} - mhsLnk, mhs := newRandomLinkedList(t, lsys, eChunkCount) adv := &schema.Advertisement{ Provider: p.String(), Addresses: addrs, - Entries: mhsLnk, ContextID: ctxID, Metadata: metadata, } + var mhs []multihash.Multihash + if eChunkCount == 0 { + adv.Entries = schema.NoEntries + } else { + adv.Entries, mhs = newRandomLinkedList(t, lsys, eChunkCount) + } + if !fakeSig { err := adv.Sign(priv) require.NoError(t, err) } - node, err := adv.ToNode() require.NoError(t, err) advLnk, err := lsys.Store(ipld.LinkContext{}, schema.Linkproto, node) diff --git a/internal/ingest/linksystem.go b/internal/ingest/linksystem.go index a8132d12c..c6ad884af 100644 --- a/internal/ingest/linksystem.go +++ b/internal/ingest/linksystem.go @@ -276,6 +276,15 @@ func (ing *Ingester) ingestAd(publisherID peer.ID, adCid cid.Cid, ad schema.Adve return nil } + if len(ad.Metadata) == 0 { + // If the ad has no metadata and no entries, then the ad is only for + // updating provider addresses. + if ad.Entries == schema.NoEntries { + return nil + } + return adIngestError{adIngestMalformedErr, fmt.Errorf("advertisement missing metadata")} + } + // If advertisement has no entries, then it is for updating metadata only. if ad.Entries == schema.NoEntries || frozen { // If this is a metadata update only, then ad will not have entries. From 2ea5cfcf989ae8cedf3abba727edf7f7d8753591 Mon Sep 17 00:00:00 2001 From: gammazero Date: Wed, 18 Jan 2023 04:09:11 -0800 Subject: [PATCH 2/2] indent errors --- internal/ingest/ingest_test.go | 2 +- internal/ingest/linksystem.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/ingest/ingest_test.go b/internal/ingest/ingest_test.go index a80828957..477cd7fd5 100644 --- a/internal/ingest/ingest_test.go +++ b/internal/ingest/ingest_test.go @@ -1047,7 +1047,7 @@ func TestSyncTooLargeMetadata(t *testing.T) { require.Equal(t, cid.Undef, lcid) } -func TestSyncAddrUpdateOnly(t *testing.T) { +func TestSyncSkipNoMetadata(t *testing.T) { srcStore := dssync.MutexWrap(datastore.NewMapDatastore()) h := mkTestHost() pubHost := mkTestHost() diff --git a/internal/ingest/linksystem.go b/internal/ingest/linksystem.go index c6ad884af..93be4b33e 100644 --- a/internal/ingest/linksystem.go +++ b/internal/ingest/linksystem.go @@ -278,11 +278,11 @@ func (ing *Ingester) ingestAd(publisherID peer.ID, adCid cid.Cid, ad schema.Adve if len(ad.Metadata) == 0 { // If the ad has no metadata and no entries, then the ad is only for - // updating provider addresses. - if ad.Entries == schema.NoEntries { - return nil + // updating provider addresses. Otherwise it is an error. + if ad.Entries != schema.NoEntries { + return adIngestError{adIngestMalformedErr, fmt.Errorf("advertisement missing metadata")} } - return adIngestError{adIngestMalformedErr, fmt.Errorf("advertisement missing metadata")} + return nil } // If advertisement has no entries, then it is for updating metadata only.