refactor(CodingPattern): introduce coding pattern

We've been running into all sorts of problems upstream when trying to send
raw *Dataset's over the wire due to inconsistencies serializing to various formats.

To me this highlights a tension in the two roles that structs like Dataset, Commit,
Structure, etc. have to play: Serialization & business logic. To me this shows up the
most when trying to decide on the proper type for struct fields when a more "exotic"
type (something other than string, int, []byte, ...) is preferred to limit use cases
to specific concerns. Things like our typed DataFormat consts are a great example.
For the serialization concern a field like DataFormat would ideally just be a string.
serialization libraries like json, cbor, and gob all know how to deal with strings,
so it's less code to maintain. On the other hand DataFormat can really only be a few
things right now (CSV, JSON, CBOR), and we need tight controls on what values
DataFormat can have because we make lots of decisions on that value. For this reason
it makes lots of sense for a DataFormat field to be a custom type that errors if it
isn't a value we like. This is a "business logic" concern enforced by strong typing
patterns.

So I think the answer is to separate the concerns into two structs, one struct for the
serialization concern, and another struct for the business logic concern. I've heard
about this pattern in the wild, but haven't really implemented it myself. So, for lack of
reading I'm going to call it the "coding pattern", and it works as follows:
* two structs, one "standard", the other with the prefix "Coding"
* the first struct gets two methods: Endode and Decode, that translate to & from the
  coding variant
* All serialization should happen at the "Coding struct"

This last bullet isn't yet fully implemented in this codebase, as it's going to take
some time to migrate over. I'd like to start by switching codebases that use qri to
this new pattern, then migrating all dataset subpackages once we have time.

Author: b5

commit.go

@@ -133,7 +133,7 @@ func (cm *Commit) UnmarshalJSON(data []byte) error {
 	m := _commitMsg{}
 	if err := json.Unmarshal(data, &m); err != nil {
 		log.Debug(err.Error())
-		return fmt.Errorf("error unmarshling dataset: %s", err.Error())
+		return fmt.Errorf("error unmarshling commit: %s", err.Error())
 	}
 
 	*cm = Commit(m)
@@ -158,3 +158,50 @@ func UnmarshalCommit(v interface{}) (*Commit, error) {
 		return nil, err
 	}
 }
+
+// Encode creates a CodingCommit from a Commit instance
+func (cm Commit) Encode() *CodingCommit {
+	return &CodingCommit{
+		Author:    cm.Author,
+		Message:   cm.Message,
+		Path:      cm.Path().String(),
+		Qri:       cm.Qri.String(),
+		Signature: cm.Signature,
+		Timestamp: cm.Timestamp,
+		Title:     cm.Title,
+	}
+}
+
+// Decode creates a Commit from a CodingCommit instance
+func (cm *Commit) Decode(cc *CodingCommit) error {
+	c := Commit{
+		path:      datastore.NewKey(cc.Path),
+		Author:    cc.Author,
+		Message:   cc.Message,
+		Signature: cc.Signature,
+		Timestamp: cc.Timestamp,
+		Title:     cc.Title,
+	}
+
+	if cc.Qri == KindCommit.String() {
+		// TODO - this should respond to changes in CodingCommit
+		c.Qri = KindCommit
+	} else if cc.Qri != "" {
+		return fmt.Errorf("invalid commit 'qri' value: %s", cc.Qri)
+	}
+
+	*cm = c
+	return nil
+}
+
+// CodingCommit is a variant of Commit safe for serialization (encoding & decoding)
+// to static formats. It uses only simple go types
+type CodingCommit struct {
+	Author    *User     `json:"author,omitempty"`
+	Message   string    `json:"message,omitempty"`
+	Path      string    `json:"path,omitempty"`
+	Qri       string    `json:"qri,omitempty"`
+	Signature string    `json:"signature,omitempty"`
+	Timestamp time.Time `json:"timestamp"`
+	Title     string    `json:"title"`
+}

commit_test.go

@@ -183,16 +183,18 @@ func TestCommitUnmarshalJSON(t *testing.T) {
 	cases := []struct {
 		data   string
 		result *Commit
-		err    error
+		err    string
 	}{
-		{`{}`, &Commit{}, nil},
-		{`{ "title": "title", "message": "message"}`, &Commit{Title: "title", Message: "message"}, nil},
-		{`{ "author" : { "id": "id", "email": "email@email.com"} }`, &Commit{Author: &User{ID: "id", Email: "email@email.com"}}, nil},
+		{`{}`, &Commit{}, ""},
+		{`{ "title": "title", "message": "message"}`, &Commit{Title: "title", Message: "message"}, ""},
+		{`{ "author" : { "id": "id", "email": "email@email.com"} }`, &Commit{Author: &User{ID: "id", Email: "email@email.com"}}, ""},
+		{`{`, &Commit{Author: &User{ID: "id", Email: "email@email.com"}}, "error unmarshling commit: unexpected end of JSON input"},
 	}
 
 	for i, c := range cases {
 		cm := &Commit{}
-		if err := json.Unmarshal([]byte(c.data), cm); err != c.err {
+		err := cm.UnmarshalJSON([]byte(c.data))
+		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
 			t.Errorf("case %d error mismatch. expected: '%s', got: '%s'", i, c.err, err)
 			continue
 		}
@@ -241,3 +243,49 @@ func TestUnmarshalCommit(t *testing.T) {
 		}
 	}
 }
+
+func TestCommitCoding(t *testing.T) {
+	cases := []*Commit{
+		{},
+		{Author: &User{Email: "foo"}},
+		{Message: "foo"},
+		{path: datastore.NewKey("/foo")},
+		{Qri: KindCommit},
+		{Signature: "foo"},
+		{Timestamp: time.Date(2001, 1, 1, 1, 1, 1, 1, time.UTC)},
+		{Title: "foo"},
+	}
+
+	for i, c := range cases {
+		cd := c.Encode()
+		got := &Commit{}
+		if err := got.Decode(cd); err != nil {
+			t.Errorf("case %d unexpected error '%s'", i, err)
+			continue
+		}
+
+		if err := CompareCommits(c, got); err != nil {
+			t.Errorf("case %d mismatch: %s", i, err.Error())
+			continue
+		}
+	}
+}
+
+func TestCommitDecode(t *testing.T) {
+	cases := []struct {
+		cm  *CodingCommit
+		err string
+	}{
+		{&CodingCommit{}, ""},
+		{&CodingCommit{Qri: "foo"}, "invalid commit 'qri' value: foo"},
+	}
+
+	for i, c := range cases {
+		got := &Commit{}
+		err := got.Decode(c.cm)
+		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
+			t.Errorf("case %d error mismatch. expected: '%s', got: '%s'", i, c.err, err)
+			continue
+		}
+	}
+}

dataset.go

@@ -169,7 +169,7 @@ func (ds *Dataset) UnmarshalJSON(data []byte) error {
 	d := _dataset{}
 	if err := json.Unmarshal(data, &d); err != nil {
 		log.Debug(err.Error())
-		return fmt.Errorf("error unmarshaling dataset: %s", err.Error())
+		return fmt.Errorf("unmarshaling dataset: %s", err.Error())
 	}
 	*ds = Dataset(d)
 	return nil
@@ -193,3 +193,81 @@ func UnmarshalDataset(v interface{}) (*Dataset, error) {
 		return nil, err
 	}
 }
+
+// Encode creates a CodingDataset from a Dataset instance
+func (ds Dataset) Encode() *CodingDataset {
+	cd := &CodingDataset{
+		DataPath:     ds.DataPath,
+		Meta:         ds.Meta,
+		Path:         ds.Path().String(),
+		PreviousPath: ds.PreviousPath,
+		Qri:          ds.Qri.String(),
+		VisConfig:    ds.VisConfig,
+	}
+
+	if ds.Commit != nil {
+		cd.Commit = ds.Commit.Encode()
+	}
+	if ds.Structure != nil {
+		cd.Structure = ds.Structure.Encode()
+	}
+	if ds.Transform != nil {
+		cd.Transform = ds.Transform.Encode()
+	}
+
+	return cd
+}
+
+// Decode creates a Dataset from a CodingDataset instance
+func (ds *Dataset) Decode(cd *CodingDataset) error {
+	d := Dataset{
+		path:         datastore.NewKey(cd.Path),
+		DataPath:     cd.DataPath,
+		PreviousPath: cd.PreviousPath,
+		Meta:         cd.Meta,
+		VisConfig:    cd.VisConfig,
+	}
+
+	if cd.Qri != "" {
+		// TODO - this should react to changes in cd
+		d.Qri = KindDataset
+	}
+
+	if cd.Commit != nil {
+		d.Commit = &Commit{}
+		if err := d.Commit.Decode(cd.Commit); err != nil {
+			return err
+		}
+	}
+
+	if cd.Structure != nil {
+		d.Structure = &Structure{}
+		if err := d.Structure.Decode(cd.Structure); err != nil {
+			return err
+		}
+	}
+
+	if cd.Transform != nil {
+		d.Transform = &Transform{}
+		if err := d.Transform.Decode(cd.Transform); err != nil {
+			return err
+		}
+	}
+
+	*ds = d
+	return nil
+}
+
+// CodingDataset is a variant of Dataset safe for serialization (encoding & decoding)
+// to static formats. It uses only simple go types
+type CodingDataset struct {
+	Commit       *CodingCommit    `json:"commit,omitempty"`
+	DataPath     string           `json:"dataPath,omitempty"`
+	Meta         *Meta            `json:"meta,omitempty"`
+	Path         string           `json:"path,omitempty"`
+	PreviousPath string           `json:"previousPath,omitempty"`
+	Qri          string           `json:"qri"`
+	Structure    *CodingStructure `json:"structure"`
+	Transform    *CodingTransform `json:"transform,omitempty"`
+	VisConfig    *VisConfig       `json:"visconfig,omitempty"`
+}

dataset_test.go

@@ -127,11 +127,11 @@ func TestDatasetUnmarshalJSON(t *testing.T) {
 	cases := []struct {
 		FileName string
 		result   *Dataset
-		err      error
+		err      string
 	}{
-		{"testdata/datasets/airport-codes.json", AirportCodes, nil},
-		{"testdata/datasets/continent-codes.json", ContinentCodes, nil},
-		{"testdata/datasets/hours.json", Hours, nil},
+		{"testdata/datasets/airport-codes.json", AirportCodes, ""},
+		{"testdata/datasets/continent-codes.json", ContinentCodes, ""},
+		{"testdata/datasets/hours.json", Hours, ""},
 	}
 
 	for i, c := range cases {
@@ -141,7 +141,8 @@ func TestDatasetUnmarshalJSON(t *testing.T) {
 		}
 
 		ds := &Dataset{}
-		if err := json.Unmarshal(data, ds); err != c.err {
+		err = ds.UnmarshalJSON(data)
+		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
 			t.Errorf("case %d error mismatch. expected: '%s', got: '%s'", i, c.err, err)
 			continue
 		}
@@ -163,6 +164,13 @@ func TestDatasetUnmarshalJSON(t *testing.T) {
 		t.Errorf("unmarshal didn't set proper path: %s != %s", path, strds.path)
 		return
 	}
+
+	errDs := &Dataset{}
+	if err := errDs.UnmarshalJSON([]byte(`{{{{`)); err != nil && err.Error() != "unmarshaling dataset: invalid character '{' looking for beginning of object key string" {
+		t.Errorf("unexpected error: %s", err.Error())
+	} else if err == nil {
+		t.Errorf("expected error")
+	}
 }
 
 func TestDatasetIsEmpty(t *testing.T) {
@@ -242,3 +250,53 @@ func TestUnmarshalDataset(t *testing.T) {
 		}
 	}
 }
+
+func TestDatasetCoding(t *testing.T) {
+	cases := []*Dataset{
+		{},
+		{Commit: &Commit{Title: "foo"}},
+		{DataPath: "foo"},
+		{Meta: &Meta{Title: "foo"}},
+		{PreviousPath: "foo"},
+		{Qri: KindDataset},
+		{Structure: &Structure{Format: CBORDataFormat}},
+		{Transform: &Transform{AppVersion: "foo"}},
+		{VisConfig: &VisConfig{Format: "foo"}},
+	}
+
+	for i, c := range cases {
+		cd := c.Encode()
+		got := &Dataset{}
+		err := got.Decode(cd)
+		if err != nil {
+			t.Errorf("case %d unexpected error: '%s'", i, err.Error())
+			continue
+		}
+
+		if err := CompareDatasets(c, got); err != nil {
+			t.Errorf("case %d dataset mismatch: %s", i, err.Error())
+			continue
+		}
+	}
+}
+
+func TestDatasetDecode(t *testing.T) {
+	cases := []struct {
+		cd  *CodingDataset
+		err string
+	}{
+		{&CodingDataset{}, ""},
+		{&CodingDataset{Commit: &CodingCommit{Qri: "foo"}}, "invalid commit 'qri' value: foo"},
+		{&CodingDataset{Structure: &CodingStructure{Format: "foo"}}, "invalid data format: `foo`"},
+		{&CodingDataset{Transform: &CodingTransform{Resources: []byte("foo")}}, "decoding transform resources: invalid character 'o' in literal false (expecting 'a')"},
+	}
+
+	for i, c := range cases {
+		got := &Dataset{}
+		err := got.Decode(c.cd)
+		if !(err == nil && c.err == "" || err != nil && err.Error() == c.err) {
+			t.Errorf("case %d error mismatch. expected: '%s', got: '%s'", i, c.err, err)
+			continue
+		}
+	}
+}

hash.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 
-	"github.com/jbenet/go-base58"
+	"github.com/mr-tron/base58/base58"
 	"github.com/multiformats/go-multihash"
 )