Review changes

dagsync/ipnisync/cid_schema_hint.go

@@ -1,24 +1,77 @@
 package ipnisync
 
+import (
+	"context"
+	"errors"
+)
+
 const (
 	// CidSchemaHeader is the HTTP header used as an optional hint about the
 	// type of data requested by a CID.
 	CidSchemaHeader = "Ipni-Cid-Schema-Type"
-	// CidSchemaAd is a value for the CidSchemaHeader specifying advertiesement
-	// data is being requested.
-	CidSchemaAd = "advertisement"
+	// CidSchemaAdvertisement is a value for the CidSchemaHeader specifying
+	// advertiesement data is being requested. Referrs to Advertisement in
+	// https://github.com/ipni/go-libipni/blob/main/ingest/schema/schema.ipldsch
+	CidSchemaAdvertisement = "Advertisement"
 	// CidSchemaEntries is a value for the CidSchemaHeader specifying
 	// advertisement entries (multihash chunks) data is being requested.
-	CidSchemaEntries = "entries"
+	// Referrs to Entry chunk in
+	// https://github.com/ipni/go-libipni/blob/main/ingest/schema/schema.ipldsch
+	CidSchemaEntryChunk = "EntryChunk"
 )
 
+var ErrUnknownCidSchema = errors.New("unknown cid schema type value")
+
 // cidSchemaTypeKey is the type used for the key of CidSchemaHeader when set as
 // a context value.
 type cidSchemaTypeCtxKey string
 
-// CidSchemaCtxKey is used as the key when creating a context with a value or extracting the cid schema from a context. Examples:
+// cidSchemaCtxKey is used to get the key used to store or extract the cid
+// schema value in a context.
+const cidSchemaCtxKey cidSchemaTypeCtxKey = CidSchemaHeader
+
+// CidSchemaFromCtx extracts the CID schema name from the context. If the
+// scheam value is not set, then returns "". If the schema value is set, but is
+// not recognized, then ErrUnknownCidSchema is returned along with the value.
 //
-//	ctx := context.WithValue(ctx, CidSchemaCtxKey, CidSchemaAd)
+// Returning unrecognized values with an  error allows consumers to retrieved
+// newer values that are not recognized by an older version of this library.
+func CidSchemaFromCtx(ctx context.Context) (string, error) {
+	if ctx == nil {
+		return "", nil
+	}
+	cidSchemaType, ok := ctx.Value(cidSchemaCtxKey).(string)
+	if !ok {
+		return "", nil
+	}
+
+	var err error
+	switch cidSchemaType {
+	case CidSchemaAdvertisement, CidSchemaEntryChunk:
+	default:
+		err = ErrUnknownCidSchema
+	}
+	return cidSchemaType, err
+}
+
+// CtxWithCidSchema creates a derived context that has the specified value for
+// the CID schema type.
 //
-//	cidSchemaType, ok := ctx.Value(CidSchemaCtxKey).(string)
-const CidSchemaCtxKey cidSchemaTypeCtxKey = CidSchemaHeader
+// Setting an unrecognized value, even when an error is retruned, allows
+// producers to set context values that are not recognized by an older version
+// of this library.
+func CtxWithCidSchema(ctx context.Context, cidSchemaType string) (context.Context, error) {
+	if cidSchemaType == "" {
+		return ctx, nil
+	}
+	if ctx == nil {
+		ctx = context.Background()
+	}
+	var err error
+	switch cidSchemaType {
+	case CidSchemaAdvertisement, CidSchemaEntryChunk:
+	default:
+		err = ErrUnknownCidSchema
+	}
+	return context.WithValue(ctx, cidSchemaCtxKey, cidSchemaType), err
+}

dagsync/ipnisync/cid_schema_hint_test.go

@@ -0,0 +1,50 @@
+package ipnisync_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/ipni/go-libipni/dagsync/ipnisync"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCtxWithCidSchema(t *testing.T) {
+	ctx, err := ipnisync.CtxWithCidSchema(nil, "")
+	require.NoError(t, err)
+	require.Nil(t, ctx)
+
+	ctxOrig := context.Background()
+	ctx, err = ipnisync.CtxWithCidSchema(ctxOrig, "")
+	require.NoError(t, err)
+	require.Equal(t, ctxOrig, ctx)
+
+	ctx, err = ipnisync.CtxWithCidSchema(ctxOrig, ipnisync.CidSchemaAdvertisement)
+	require.NoError(t, err)
+	require.NotEqual(t, ctxOrig, ctx)
+
+	value, err := ipnisync.CidSchemaFromCtx(ctx)
+	require.NoError(t, err)
+	require.Equal(t, ipnisync.CidSchemaAdvertisement, value)
+
+	ctx, err = ipnisync.CtxWithCidSchema(nil, ipnisync.CidSchemaEntryChunk)
+	require.NoError(t, err)
+	value, err = ipnisync.CidSchemaFromCtx(ctx)
+	require.NoError(t, err)
+	require.Equal(t, ipnisync.CidSchemaEntryChunk, value)
+
+	value, err = ipnisync.CidSchemaFromCtx(ctxOrig)
+	require.NoError(t, err)
+	require.Empty(t, value)
+
+	const unknownVal = "unknown"
+
+	// Setting unknown value returns error as well as context with value.
+	ctx, err = ipnisync.CtxWithCidSchema(ctxOrig, unknownVal)
+	require.ErrorIs(t, err, ipnisync.ErrUnknownCidSchema)
+	require.NotNil(t, ctxOrig, ctx)
+
+	// Getting unknown value returns error as well as value.
+	value, err = ipnisync.CidSchemaFromCtx(ctx)
+	require.ErrorIs(t, err, ipnisync.ErrUnknownCidSchema)
+	require.Equal(t, unknownVal, value)
+}

dagsync/ipnisync/publisher.go

@@ -1,7 +1,6 @@
 package ipnisync
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"net/http"
@@ -45,7 +44,7 @@ var _ http.Handler = (*Publisher)(nil)
 //
 // If the publisher receives a request that contains a valid CidSchemaHeader
 // header, then the ipld.Context passed to the lsys Load function contains a
-// context that has that header's value stored under the CidSchemaCtxKey key.
+// context that has that header's value retrievable with CidSchemaFromCtx.
 func NewPublisher(lsys ipld.LinkSystem, privKey ic.PrivKey, options ...Option) (*Publisher, error) {
 	opts, err := getOpts(options)
 	if err != nil {
@@ -227,7 +226,10 @@ func (p *Publisher) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	ipldCtx := ipld.LinkContext{}
 	reqType := r.Header.Get(CidSchemaHeader)
 	if reqType != "" {
-		ipldCtx.Ctx = context.WithValue(context.Background(), CidSchemaCtxKey, reqType)
+		ipldCtx.Ctx, err = CtxWithCidSchema(ipldCtx.Ctx, reqType)
+		if err != nil {
+			log.Warnw(err.Error(), "value", reqType)
+		}
 	}
 
 	item, err := p.lsys.Load(ipldCtx, cidlink.Link{Cid: c}, basicnode.Prototype.Any)

dagsync/ipnisync/sync.go

@@ -227,13 +227,9 @@ func (s *Syncer) Sync(ctx context.Context, nextCid cid.Cid, sel ipld.Node) error
 	}
 
 	// Check for valid cid schema type if set.
-	cidSchemaType, ok := ctx.Value(CidSchemaCtxKey).(string)
-	if ok {
-		switch cidSchemaType {
-		case CidSchemaAd, CidSchemaEntries:
-		default:
-			return fmt.Errorf("invalid cid schema type value: %s", cidSchemaType)
-		}
+	_, err = CidSchemaFromCtx(ctx)
+	if err != nil {
+		return err
 	}
 
 	cids, err := s.walkFetch(ctx, nextCid, xsel)
@@ -317,9 +313,9 @@ retry:
 		return err
 	}
 
-	// Value already checked in Sync.
-	reqType, ok := ctx.Value(CidSchemaCtxKey).(string)
-	if ok {
+	// Error already checked in Sync.
+	reqType, _ := CidSchemaFromCtx(ctx)
+	if reqType != "" {
 		req.Header.Set(CidSchemaHeader, reqType)
 	}
 

dagsync/ipnisync/sync_test.go

@@ -244,11 +244,10 @@ func TestRequestTypeHint(t *testing.T) {
 
 	publs.StorageReadOpener = func(lnkCtx linking.LinkContext, lnk datamodel.Link) (io.Reader, error) {
 		if lnkCtx.Ctx != nil {
-			hint, ok := lnkCtx.Ctx.Value(ipnisync.CidSchemaCtxKey).(string)
-			require.True(t, ok)
+			hint, err := ipnisync.CidSchemaFromCtx(lnkCtx.Ctx)
+			require.NoError(t, err)
 			require.NotEmpty(t, hint)
 			lastReqTypeHint = hint
-			t.Log("Request type hint:", hint)
 		} else {
 			lastReqTypeHint = ""
 		}
@@ -277,15 +276,18 @@ func TestRequestTypeHint(t *testing.T) {
 	testCid, err := cid.Decode(sampleNFTStorageCid)
 	require.NoError(t, err)
 
-	ctx := context.WithValue(context.Background(), ipnisync.CidSchemaCtxKey, ipnisync.CidSchemaAd)
+	ctx, err := ipnisync.CtxWithCidSchema(context.Background(), ipnisync.CidSchemaAdvertisement)
+	require.NoError(t, err)
 	_ = syncer.Sync(ctx, testCid, selectorparse.CommonSelector_MatchPoint)
-	require.Equal(t, ipnisync.CidSchemaAd, lastReqTypeHint)
+	require.Equal(t, ipnisync.CidSchemaAdvertisement, lastReqTypeHint)
 
-	ctx = context.WithValue(context.Background(), ipnisync.CidSchemaCtxKey, ipnisync.CidSchemaEntries)
+	ctx, err = ipnisync.CtxWithCidSchema(context.Background(), ipnisync.CidSchemaEntryChunk)
+	require.NoError(t, err)
 	_ = syncer.Sync(ctx, testCid, selectorparse.CommonSelector_MatchPoint)
-	require.Equal(t, ipnisync.CidSchemaEntries, lastReqTypeHint)
+	require.Equal(t, ipnisync.CidSchemaEntryChunk, lastReqTypeHint)
 
-	ctx = context.WithValue(context.Background(), ipnisync.CidSchemaCtxKey, "bad")
+	ctx, err = ipnisync.CtxWithCidSchema(context.Background(), "bad")
+	require.ErrorIs(t, err, ipnisync.ErrUnknownCidSchema)
 	err = syncer.Sync(ctx, testCid, selectorparse.CommonSelector_MatchPoint)
-	require.ErrorContains(t, err, "invalid cid schema type value")
+	require.ErrorIs(t, err, ipnisync.ErrUnknownCidSchema)
 }

dagsync/subscriber.go

@@ -488,7 +488,10 @@ func (s *Subscriber) SyncAdChain(ctx context.Context, peerInfo peer.AddrInfo, op
 
 	sel := ExploreRecursiveWithStopNode(depthLimit, s.adsSelectorSeq, stopLnk)
 
-	ctx = context.WithValue(ctx, ipnisync.CidSchemaCtxKey, ipnisync.CidSchemaAd)
+	ctx, err = ipnisync.CtxWithCidSchema(ctx, ipnisync.CidSchemaAdvertisement)
+	if err != nil {
+		panic(err.Error())
+	}
 	syncCount, err := hnd.handle(ctx, nextCid, sel, syncer, opts.blockHook, segdl, stopAtCid)
 	if err != nil {
 		return cid.Undef, fmt.Errorf("sync handler failed: %w", err)
@@ -572,7 +575,10 @@ func (s *Subscriber) syncEntries(ctx context.Context, peerInfo peer.AddrInfo, en
 
 	log.Debugw("Start entries sync", "peer", peerInfo.ID, "cid", entCid)
 
-	ctx = context.WithValue(ctx, ipnisync.CidSchemaCtxKey, ipnisync.CidSchemaEntries)
+	ctx, err = ipnisync.CtxWithCidSchema(ctx, ipnisync.CidSchemaEntryChunk)
+	if err != nil {
+		panic(err.Error())
+	}
 	_, err = hnd.handle(ctx, entCid, sel, syncer, bh, segdl, cid.Undef)
 	if err != nil {
 		return fmt.Errorf("sync handler failed: %w", err)
@@ -874,7 +880,10 @@ func (h *handler) asyncSyncAdChain(ctx context.Context) {
 		return
 	}
 
-	ctx = context.WithValue(ctx, ipnisync.CidSchemaCtxKey, ipnisync.CidSchemaAd)
+	ctx, err = ipnisync.CtxWithCidSchema(ctx, ipnisync.CidSchemaAdvertisement)
+	if err != nil {
+		panic(err.Error())
+	}
 	sel := ExploreRecursiveWithStopNode(adsDepthLimit, h.subscriber.adsSelectorSeq, latestSyncLink)
 	syncCount, err := h.handle(ctx, nextCid, sel, syncer, h.subscriber.generalBlockHook, h.subscriber.segDepthLimit, stopAtCid)
 	if err != nil {