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: - ipld/go-ipld-prime#378
ipni · Jul 14, 2022 · ed788ba · ed788ba
1 parent ec4ba91
commit ed788ba
Show file tree

Hide file tree

Showing 7 changed files with 22 additions and 28 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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