From ed788ba49c0245cbd55c72d02bae94d0556d497d Mon Sep 17 00:00:00 2001 From: "Masih H. Derkani" Date: Thu, 14 Jul 2022 13:08:48 +0000 Subject: [PATCH] Simplify representation of optional and nullable IPLD fields Earlier versions of bindnode required pointers in order to represent optional/nullable fields even if the go type was nillable. Now that bindnode allows nillable types to represent optional/nulalble fields, remove pointer to interfaces and simply use the interface type straight up. Relates to: - https://github.com/ipld/go-ipld-prime/issues/378 --- api/v0/ingest/schema/envelope.go | 2 +- api/v0/ingest/schema/types.go | 4 ++-- api/v0/ingest/schema/types_test.go | 4 ++-- internal/ingest/ingest.go | 2 +- internal/ingest/ingest_test.go | 18 +++++++++--------- internal/ingest/linksystem.go | 4 ++-- test/typehelpers/typehelpers.go | 16 +++++----------- 7 files changed, 22 insertions(+), 28 deletions(-) diff --git a/api/v0/ingest/schema/envelope.go b/api/v0/ingest/schema/envelope.go index be7c9fb98..bb8bb383a 100644 --- a/api/v0/ingest/schema/envelope.go +++ b/api/v0/ingest/schema/envelope.go @@ -54,7 +54,7 @@ func (r *advSignatureRecord) UnmarshalRecord(buf []byte) error { func signaturePayload(ad *Advertisement, oldFormat bool) ([]byte, error) { bindex := cid.Undef.Bytes() if ad.PreviousID != nil { - bindex = (*ad.PreviousID).(cidlink.Link).Cid.Bytes() + bindex = ad.PreviousID.(cidlink.Link).Cid.Bytes() } ent := ad.Entries.(cidlink.Link).Cid.Bytes() diff --git a/api/v0/ingest/schema/types.go b/api/v0/ingest/schema/types.go index ff0dec8e5..b235030ae 100644 --- a/api/v0/ingest/schema/types.go +++ b/api/v0/ingest/schema/types.go @@ -11,7 +11,7 @@ import ( type ( Advertisement struct { - PreviousID *ipld.Link + PreviousID ipld.Link Provider string Addresses []string Signature []byte @@ -22,7 +22,7 @@ type ( } EntryChunk struct { Entries []multihash.Multihash - Next *ipld.Link + Next ipld.Link } ) diff --git a/api/v0/ingest/schema/types_test.go b/api/v0/ingest/schema/types_test.go index 2b96cc229..d0b93e977 100644 --- a/api/v0/ingest/schema/types_test.go +++ b/api/v0/ingest/schema/types_test.go @@ -135,7 +135,7 @@ func generateAdvertisement(rng *rand.Rand) *stischema.Advertisement { mhs := util.RandomMultihashes(7, rng) prev := ipld.Link(cidlink.Link{Cid: cid.NewCidV1(cid.Raw, mhs[0])}) return &stischema.Advertisement{ - PreviousID: &prev, + PreviousID: prev, Provider: mhs[1].String(), Addresses: []string{ mhs[2].String(), @@ -153,6 +153,6 @@ func generateEntryChunk(rng *rand.Rand) *stischema.EntryChunk { next := ipld.Link(cidlink.Link{Cid: cid.NewCidV1(cid.Raw, mhs[0])}) return &stischema.EntryChunk{ Entries: mhs, - Next: &next, + Next: next, } } diff --git a/internal/ingest/ingest.go b/internal/ingest/ingest.go index 76ecc0d39..735e54844 100644 --- a/internal/ingest/ingest.go +++ b/internal/ingest/ingest.go @@ -231,7 +231,7 @@ func (ing *Ingester) generalLegsBlockHook(_ peer.ID, c cid.Cid, actions legs.Seg if ad, err := ing.loadAd(c); err != nil { actions.FailSync(err) } else if ad.PreviousID != nil { - actions.SetNextSyncCid((*(ad.PreviousID)).(cidlink.Link).Cid) + actions.SetNextSyncCid(ad.PreviousID.(cidlink.Link).Cid) } else { actions.SetNextSyncCid(cid.Undef) } diff --git a/internal/ingest/ingest_test.go b/internal/ingest/ingest_test.go index da62c7ea1..3e265250d 100644 --- a/internal/ingest/ingest_test.go +++ b/internal/ingest/ingest_test.go @@ -286,7 +286,7 @@ func TestRestartDuringSync(t *testing.T) { blockedReads.add(bAd.Entries.(cidlink.Link).Cid) - bCid := *cAd.PreviousID + bCid := cAd.PreviousID ctx := context.Background() err = te.publisher.SetRoot(ctx, bCid.(cidlink.Link).Cid) @@ -367,7 +367,7 @@ func TestFailDuringSync(t *testing.T) { blockedReads.add(bAd.Entries.(cidlink.Link).Cid) - bCid := (*cAd.PreviousID) + bCid := cAd.PreviousID require.NoError(t, err) ctx := context.Background() @@ -444,8 +444,8 @@ func TestIngestDoesNotSkipAdIfFirstTryFailed(t *testing.T) { bEntChunk := bAd.Entries blockedReads.add(bAd.Entries.(cidlink.Link).Cid) - bCid := *cAd.PreviousID - aCid := *bAd.PreviousID + bCid := cAd.PreviousID + aCid := bAd.PreviousID ctx := context.Background() err = te.publisher.SetRoot(ctx, cCid.(cidlink.Link).Cid) @@ -610,7 +610,7 @@ func TestRmWithNoEntries(t *testing.T) { require.NoError(t, err) require.NotNil(t, ad.PreviousID) - prevAdNode, err := te.publisherLinkSys.Load(linking.LinkContext{}, *ad.PreviousID, schema.AdvertisementPrototype) + prevAdNode, err := te.publisherLinkSys.Load(linking.LinkContext{}, ad.PreviousID, schema.AdvertisementPrototype) require.NoError(t, err) prevAd, err := schema.UnwrapAdvertisement(prevAdNode) require.NoError(t, err) @@ -887,7 +887,7 @@ func decodeEntriesChunk(t *testing.T, store datastore.Batching, c cid.Cid) ([]mu return ec.Entries, cid.Undef } - return ec.Entries, (*ec.Next).(cidlink.Link).Cid + return ec.Entries, ec.Next.(cidlink.Link).Cid } func TestMultiplePublishers(t *testing.T) { @@ -1258,7 +1258,7 @@ func connectHosts(t *testing.T, srcHost, dstHost host.Host) { func newRandomLinkedList(t *testing.T, lsys ipld.LinkSystem, size int) (ipld.Link, []multihash.Multihash) { var out []multihash.Multihash - var nextLnk *ipld.Link + var nextLnk ipld.Link for i := 0; i < size; i++ { mhs := util.RandomMultihashes(testEntriesChunkSize, rng) chunk := &schema.EntryChunk{ @@ -1270,9 +1270,9 @@ func newRandomLinkedList(t *testing.T, lsys ipld.LinkSystem, size int) (ipld.Lin lnk, err := lsys.Store(ipld.LinkContext{}, schema.Linkproto, node) require.NoError(t, err) out = append(out, mhs...) - nextLnk = &lnk + nextLnk = lnk } - return *nextLnk, out + return nextLnk, out } func publishRandomIndexAndAdv(t *testing.T, pub legs.Publisher, lsys ipld.LinkSystem, fakeSig bool) (cid.Cid, []multihash.Multihash, peer.ID) { diff --git a/internal/ingest/linksystem.go b/internal/ingest/linksystem.go index aa1ea05bc..23aec876e 100644 --- a/internal/ingest/linksystem.go +++ b/internal/ingest/linksystem.go @@ -323,7 +323,7 @@ func (ing *Ingester) ingestAd(publisherID peer.ID, adCid cid.Cid, ad schema.Adve } if chunk != nil && chunk.Next != nil { - nextChunkCid := (*chunk.Next).(cidlink.Link).Cid + nextChunkCid := chunk.Next.(cidlink.Link).Cid // Traverse remaining entry chunks based on the entries selector that limits recursion depth. _, err = ing.sub.Sync(ctx, publisherID, nextChunkCid, ing.entriesSel, nil, legs.ScopedBlockHook(func(p peer.ID, c cid.Cid, actions legs.SegmentSyncActions) { // Load CID as entry chunk since the selector should only select entry chunk nodes. @@ -340,7 +340,7 @@ func (ing *Ingester) ingestAd(publisherID peer.ID, adCid cid.Cid, ad schema.Adve return } if chunk.Next != nil { - actions.SetNextSyncCid((*(chunk.Next)).(cidlink.Link).Cid) + actions.SetNextSyncCid(chunk.Next.(cidlink.Link).Cid) } else { actions.SetNextSyncCid(cid.Undef) } diff --git a/test/typehelpers/typehelpers.go b/test/typehelpers/typehelpers.go index 37bdc61db..322a39f5f 100644 --- a/test/typehelpers/typehelpers.go +++ b/test/typehelpers/typehelpers.go @@ -68,9 +68,7 @@ func (b RandomAdBuilder) build(t *testing.T, lsys ipld.LinkSystem, signingKey cr Metadata: metadata, } - if headLink != nil { - ad.PreviousID = &headLink - } + ad.PreviousID = headLink if !fakeSig { err := ad.Sign(signingKey) @@ -88,7 +86,7 @@ func (b RandomAdBuilder) build(t *testing.T, lsys ipld.LinkSystem, signingKey cr ctxID := []byte("test-context-id-" + fmt.Sprint(0)) ad := schema.Advertisement{ - PreviousID: &headLink, + PreviousID: headLink, Provider: p.String(), Addresses: addrs, Entries: schema.NoEntries, @@ -143,14 +141,10 @@ func (b RandomEntryChunkBuilder) Build(t *testing.T, lsys ipld.LinkSystem) datam } var err error - chunk := schema.EntryChunk{ + Next: headLink, Entries: mhs, } - if headLink != nil { - chunk.Next = &headLink - } - node, err := chunk.ToNode() require.NoError(t, err) headLink, err = lsys.Store(ipld.LinkContext{}, schema.Linkproto, node) @@ -417,8 +411,8 @@ func AllAdLinks(t *testing.T, head datamodel.Link, lsys ipld.LinkSystem) []datam out := []datamodel.Link{head} ad := AdFromLink(t, head, lsys) for ad.PreviousID != nil { - out = append(out, *ad.PreviousID) - ad = AdFromLink(t, *ad.PreviousID, lsys) + out = append(out, ad.PreviousID) + ad = AdFromLink(t, ad.PreviousID, lsys) } // Flip order so the latest is last