refactor: BlockPutSettings.CidPrefix

Removes duplicated fields and replaces them with cid.Prefix

Codec, MhType and MhLength were  already in prefix, and we already return
prefix. A lot of duplicated values and code responsible for syncing them
did not really need to exist.

Author: lidel

options/block.go

@@ -2,18 +2,15 @@ package options
 
 import (
 	"fmt"
+
 	cid "github.com/ipfs/go-cid"
 	mc "github.com/multiformats/go-multicodec"
 	mh "github.com/multiformats/go-multihash"
 )
 
 type BlockPutSettings struct {
-	StoreCodec string
-	// FIXME: Rename to Format (and possibly mark as deprecated).
-	Codec    string
-	MhType   uint64
-	MhLength int
-	Pin      bool
+	CidPrefix cid.Prefix
+	Pin       bool
 }
 
 type BlockRmSettings struct {
@@ -23,70 +20,29 @@ type BlockRmSettings struct {
 type BlockPutOption func(*BlockPutSettings) error
 type BlockRmOption func(*BlockRmSettings) error
 
-func BlockPutOptions(opts ...BlockPutOption) (*BlockPutSettings, cid.Prefix, error) {
+func BlockPutOptions(opts ...BlockPutOption) (*BlockPutSettings, error) {
+	var cidPrefix cid.Prefix
+
+	// Baseline is CIDv1 raw sha2-255-32 (can be tweaked later via opts)
+	cidPrefix.Version = 1
+	cidPrefix.Codec = uint64(mc.Raw)
+	cidPrefix.MhType = mh.SHA2_256
+	cidPrefix.MhLength = -1 // -1 means len is to be calculated during mh.Sum()
+
 	options := &BlockPutSettings{
-		Codec:      "",
-		StoreCodec: "",
-		MhType:     mh.SHA2_256,
-		MhLength:   -1,
-		Pin:        false,
+		CidPrefix: cidPrefix,
+		Pin:       false,
 	}
 
+	// Apply any overrides
 	for _, opt := range opts {
 		err := opt(options)
 		if err != nil {
-			return nil, cid.Prefix{}, err
-		}
-	}
-
-	if options.Codec != "" && options.StoreCodec != "" {
-		return nil, cid.Prefix{}, fmt.Errorf("incompatible format (%s) and store-codec options set (%s)",
-			options.Codec, options.StoreCodec)
-	}
-
-	if options.Codec == "" && options.StoreCodec == "" {
-		// FIXME(BLOCKING): Do we keep the old default v0 here?
-		options.Codec = "v0"
-		// FIXME(BLOCKING): Review how to handle "protobuf". For now we simplify the code only with "v0".
-	}
-
-	var pref cid.Prefix
-	pref.Version = 1
-
-	// Old format option.
-	if options.Codec != "" {
-		if options.Codec == "v0" {
-			if options.MhType != mh.SHA2_256 || (options.MhLength != -1 && options.MhLength != 32) {
-				return nil, cid.Prefix{}, fmt.Errorf("only sha2-255-32 is allowed with CIDv0")
-			}
-			pref.Version = 0
-		}
-
-		// FIXME(BLOCKING): Do we actually want to consult the CID codecs table
-		//  even with the old --format options? Or do we always want to check
-		//  the multicodec one?
-		cidCodec, ok := cid.Codecs[options.Codec]
-		if !ok {
-			return nil, cid.Prefix{}, fmt.Errorf("unrecognized format: %s", options.Codec)
-		}
-		pref.Codec = cidCodec
-	} else {
-		// New store-codec options. We handle it as it's done for `ipfs dag put`.
-		var storeCodec mc.Code
-		if err := storeCodec.Set(options.StoreCodec); err != nil {
-			return nil, cid.Prefix{}, err
+			return nil, err
 		}
-		pref.Codec = uint64(storeCodec)
 	}
 
-	// FIXME: The entire codec manipulation/validation needs to be encapsulated
-	//  outside this funtion to clearly demark that it is the only option we are
-	//  overwriting here.
-
-	pref.MhType = options.MhType
-	pref.MhLength = options.MhLength
-
-	return options, pref, nil
+	return options, nil
 }
 
 func BlockRmOptions(opts ...BlockRmOption) (*BlockRmSettings, error) {
@@ -107,31 +63,84 @@ type blockOpts struct{}
 
 var Block blockOpts
 
-// Format is an option for Block.Put which specifies the multicodec to use to
-// serialize the object. Default is "v0"
-func (blockOpts) Format(codec string) BlockPutOption {
+// CidCodec is the modern option for Block.Put which specifies the multicodec to use
+// in the CID returned by the Block.Put operation.
+// It uses correct codes from go-multicodec and replaces the old Format now with CIDv1 as the default.
+func (blockOpts) CidCodec(codecName string) BlockPutOption {
 	return func(settings *BlockPutSettings) error {
-		settings.Codec = codec
+		if codecName == "" {
+			return nil
+		}
+		code, err := codeFromName(codecName)
+		if err != nil {
+			return err
+		}
+		settings.CidPrefix.Codec = uint64(code)
 		return nil
 	}
 }
 
-// StoreCodec is an option for Block.Put which specifies the multicodec to use to
-// serialize the object. It replaces the old Format now with CIDv1 as the default.
-func (blockOpts) StoreCodec(storeCodec string) BlockPutOption {
+// Map string to code from go-multicodec
+func codeFromName(codecName string) (mc.Code, error) {
+	var cidCodec mc.Code
+	err := cidCodec.Set(codecName)
+	return cidCodec, err
+}
+
+// Format is a legacy option for Block.Put which specifies the multicodec to
+// use to serialize the object.
+// Provided for backward-compatibility only. Use CidCodec instead.
+func (blockOpts) Format(format string) BlockPutOption {
 	return func(settings *BlockPutSettings) error {
-		settings.StoreCodec = storeCodec
+		if format == "" {
+			return nil
+		}
+		// Opt-in CIDv0 support for backward-compatibility
+		if format == "v0" {
+			settings.CidPrefix.Version = 0
+		}
+
+		// Fixup a legacy (invalid) names for dag-pb (0x70)
+		if format == "v0" || format == "protobuf" {
+			format = "dag-pb"
+		}
+
+		// Fixup invalid name for dag-cbor (0x71)
+		if format == "cbor" {
+			format = "dag-cbor"
+		}
+
+		// Set code based on name passed as "format"
+		code, err := codeFromName(format)
+		if err != nil {
+			return err
+		}
+		settings.CidPrefix.Codec = uint64(code)
+
+		// If CIDv0, ensure all parameters are compatible
+		// (in theory go-cid would validate this anyway, but we want to provide better errors)
+		pref := settings.CidPrefix
+		if pref.Version == 0 {
+			if pref.Codec != uint64(mc.DagPb) {
+				return fmt.Errorf("only dag-pb is allowed with CIDv0")
+			}
+			if pref.MhType != mh.SHA2_256 || (pref.MhLength != -1 && pref.MhLength != 32) {
+				return fmt.Errorf("only sha2-255-32 is allowed with CIDv0")
+			}
+		}
+
 		return nil
 	}
+
 }
 
 // Hash is an option for Block.Put which specifies the multihash settings to use
 // when hashing the object. Default is mh.SHA2_256 (0x12).
 // If mhLen is set to -1, default length for the hash will be used
 func (blockOpts) Hash(mhType uint64, mhLen int) BlockPutOption {
 	return func(settings *BlockPutSettings) error {
-		settings.MhType = mhType
-		settings.MhLength = mhLen
+		settings.CidPrefix.MhType = mhType
+		settings.CidPrefix.MhLength = mhLen
 		return nil
 	}
 }

tests/block.go

@@ -17,15 +17,18 @@ import (
 )
 
 var (
-	pbCid    = "QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN"
-	cborCid  = "bafyreicnga62zhxnmnlt6ymq5hcbsg7gdhqdu6z4ehu3wpjhvqnflfy6nm"
-	cborKCid = "bafyr2qgsohbwdlk7ajmmbb4lhoytmest4wdbe5xnexfvtxeatuyqqmwv3fgxp3pmhpc27gwey2cct56gloqefoqwcf3yqiqzsaqb7p4jefhcw"
+	pbCidV0  = "QmZULkCELmmk5XNfCgTnCyFgAVxBRBXyDHGGMVoLFLiXEN"                                                                 // dag-pb
+	pbCid    = "bafybeiffndsajwhk3lwjewwdxqntmjm4b5wxaaanokonsggenkbw6slwk4"                                                    // dag-pb
+	cborCid  = "bafyreicnga62zhxnmnlt6ymq5hcbsg7gdhqdu6z4ehu3wpjhvqnflfy6nm"                                                    // dag-cbor
+	cborKCid = "bafyr2qgsohbwdlk7ajmmbb4lhoytmest4wdbe5xnexfvtxeatuyqqmwv3fgxp3pmhpc27gwey2cct56gloqefoqwcf3yqiqzsaqb7p4jefhcw" // dag-cbor keccak-512
 )
 
+// dag-pb
 func pbBlock() io.Reader {
 	return bytes.NewReader([]byte{10, 12, 8, 2, 18, 6, 104, 101, 108, 108, 111, 10, 24, 6})
 }
 
+// dag-cbor
 func cborBlock() io.Reader {
 	return bytes.NewReader([]byte{101, 72, 101, 108, 108, 111})
 }
@@ -39,8 +42,10 @@ func (tp *TestSuite) TestBlock(t *testing.T) {
 	})
 
 	t.Run("TestBlockPut", tp.TestBlockPut)
-	t.Run("TestBlockPutFormat", tp.TestBlockPutFormat)
-	t.Run("TestBlockPutStoreCodec", tp.TestBlockPutStoreCodec)
+	t.Run("TestBlockPutCidCodec: dag-cbor", tp.TestBlockPutCidCodecDagCbor)
+	t.Run("TestBlockPutFormat (legacy): cbor → dag-cbor", tp.TestBlockPutFormatDagCbor)
+	t.Run("TestBlockPutFormat (legacy): protobuf → dag-pb", tp.TestBlockPutFormatDagPb)
+	t.Run("TestBlockPutFormat (legacy): v0 → CIDv0", tp.TestBlockPutFormatV0)
 	t.Run("TestBlockPutHash", tp.TestBlockPutHash)
 	t.Run("TestBlockGet", tp.TestBlockGet)
 	t.Run("TestBlockRm", tp.TestBlockRm)
@@ -66,7 +71,9 @@ func (tp *TestSuite) TestBlockPut(t *testing.T) {
 	}
 }
 
-func (tp *TestSuite) TestBlockPutFormat(t *testing.T) {
+// Format is deprecated, it used invalid codec names.
+// Confirm 'cbor' gets fixed to 'dag-cbor'
+func (tp *TestSuite) TestBlockPutFormatDagCbor(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	api, err := tp.makeAPI(ctx)
@@ -84,15 +91,55 @@ func (tp *TestSuite) TestBlockPutFormat(t *testing.T) {
 	}
 }
 
-func (tp *TestSuite) TestBlockPutStoreCodec(t *testing.T) {
+// Format is deprecated, it used invalid codec names.
+// Confirm 'protobuf' got fixed to 'dag-pb'
+func (tp *TestSuite) TestBlockPutFormatDagPb(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 	api, err := tp.makeAPI(ctx)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	res, err := api.Block().Put(ctx, cborBlock(), opt.Block.StoreCodec("dag-cbor"))
+	res, err := api.Block().Put(ctx, pbBlock(), opt.Block.Format("protobuf"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if res.Path().Cid().String() != pbCid {
+		t.Errorf("got wrong cid: %s", res.Path().Cid().String())
+	}
+}
+
+// Format is deprecated, it used invalid codec names.
+// Confirm fake codec 'v0' got fixed to CIDv0 (with implicit dag-pb codec)
+func (tp *TestSuite) TestBlockPutFormatV0(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	api, err := tp.makeAPI(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	res, err := api.Block().Put(ctx, pbBlock(), opt.Block.Format("v0"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if res.Path().Cid().String() != pbCidV0 {
+		t.Errorf("got wrong cid: %s", res.Path().Cid().String())
+	}
+}
+
+func (tp *TestSuite) TestBlockPutCidCodecDagCbor(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	api, err := tp.makeAPI(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	res, err := api.Block().Put(ctx, cborBlock(), opt.Block.CidCodec("dag-cbor"))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -102,6 +149,24 @@ func (tp *TestSuite) TestBlockPutStoreCodec(t *testing.T) {
 	}
 }
 
+func (tp *TestSuite) TestBlockPutCidCodecDagPb(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	api, err := tp.makeAPI(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	res, err := api.Block().Put(ctx, pbBlock(), opt.Block.CidCodec("dag-pb"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if res.Path().Cid().String() != pbCid {
+		t.Errorf("got wrong cid: %s", res.Path().Cid().String())
+	}
+}
+
 func (tp *TestSuite) TestBlockPutHash(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -114,7 +179,7 @@ func (tp *TestSuite) TestBlockPutHash(t *testing.T) {
 		ctx,
 		cborBlock(),
 		opt.Block.Hash(mh.KECCAK_512, -1),
-		opt.Block.Format("cbor"),
+		opt.Block.CidCodec("dag-cbor"),
 	)
 	if err != nil {
 		t.Fatal(err)