refactor(CreateDataset): new repo.CreateDataset func

further refactoring to separate actions & repo, repo.CreateDataset does much
of the heavy lifting for a dataset that doesn't have a transform or viz specified,
which is great for testing, and keeps all the repo manipulation aspects of creating
a dataset isolated from actions.

actions/dataset.go

@@ -22,8 +22,7 @@ type Dataset struct {
 func (act Dataset) CreateDataset(name string, ds *dataset.Dataset, data cafs.File, secrets map[string]string, pin bool) (ref repo.DatasetRef, err error) {
  log.Debugf("CreateDataset: %s", name)
  var (
- path datastore.Key
- pro *profile.Profile
+ pro *profile.Profile
  // NOTE - struct fields need to be instantiated to make assign set to
  // new pointer values
  userSet = &dataset.Dataset{
@@ -61,33 +60,7 @@ func (act Dataset) CreateDataset(name string, ds *dataset.Dataset, data cafs.Fil
  return
  }
 
- path, err = dsfs.CreateDataset(act.Store(), ds, data, act.PrivateKey(), pin)
- if err != nil {
- return
- }
-
- if ds.PreviousPath != "" && ds.PreviousPath != "/" {
- prev := repo.DatasetRef{
- ProfileID: pro.ID,
- Peername: pro.Peername,
- Name: name,
- Path: ds.PreviousPath,
- }
- if err = act.DeleteRef(prev); err != nil {
- log.Error(err.Error())
- err = nil
- }
- }
-
- ref = repo.DatasetRef{
- ProfileID: pro.ID,
- Peername: pro.Peername,
- Name: name,
- Path: path.String(),
- }
-
- if err = act.PutRef(ref); err != nil {
- log.Error(err.Error())
+ if ref, err = repo.CreateDataset(act.Repo, name, ds, data, pin); err != nil {
  return
  }
 

p2p/resolve_ref_test.go

@@ -5,21 +5,20 @@ import (
  "sync"
  "testing"
 
+ "github.com/qri-io/qri/p2p"
  "github.com/qri-io/qri/p2p/test"
  "github.com/qri-io/qri/repo"
 )
 
 func TestResolveDatasetRef(t *testing.T) {
  ctx := context.Background()
- factory := p2ptest.NewTestNodeFactory(NewTestableQriNode)
+ factory := p2ptest.NewTestNodeFactory(p2p.NewTestableQriNode)
  testPeers, err := p2ptest.NewTestDirNetwork(ctx, factory)
  if err != nil {
- t.Errorf("error creating network: %s", err.Error())
- return
+ t.Fatalf("error creating network: %s", err.Error())
  }
-
- if err := p2ptest.ConnectNodes(ctx, testPeers); err != nil {
- t.Errorf("error connecting peers: %s", err.Error())
+ if err = p2ptest.ConnectNodes(ctx, testPeers); err != nil {
+ t.Fatalf("error connecting peers: %s", err.Error())
  }
 
  // Convert from test nodes to non-test nodes.

repo/dataset.go

@@ -9,10 +9,52 @@ import (
  "strings"
 
  "github.com/ghodss/yaml"
+ "github.com/ipfs/go-datastore"
  "github.com/qri-io/cafs"
  "github.com/qri-io/dataset"
+ "github.com/qri-io/dataset/dsfs"
+ "github.com/qri-io/qri/repo/profile"
 )
 
+// CreateDataset uses dsfs to add a dataset to a repo's store, updating all
+// references within the repo if successful. CreateDataset is a lower-level
+// component of github.com/qri-io/qri/actions.CreateDataset
+func CreateDataset(r Repo, name string, ds *dataset.Dataset, body cafs.File, pin bool) (ref DatasetRef, err error) {
+ var (
+ path datastore.Key
+ pro *profile.Profile
+ )
+ if pro, err = r.Profile(); err != nil {
+ return
+ }
+ if path, err = dsfs.CreateDataset(r.Store(), ds, body, r.PrivateKey(), pin); err != nil {
+ return
+ }
+
+ if ds.PreviousPath != "" && ds.PreviousPath != "/" {
+ prev := DatasetRef{
+ ProfileID: pro.ID,
+ Peername: pro.Peername,
+ Name: name,
+ Path: ds.PreviousPath,
+ }
+
+ // should be ok to skip this error. we may not have the previous
+ // reference locally
+ _ = r.DeleteRef(prev)
+ }
+
+ ref = DatasetRef{
+ ProfileID: pro.ID,
+ Peername: pro.Peername,
+ Name: name,
+ Path: path.String(),
+ }
+
+ err = r.PutRef(ref)
+ return
+}
+
 // DatasetPodBodyFile creates a streaming data file from a DatasetPod using the following precedence:
 // * dsp.BodyBytes not being nil (requires dsp.Structure.Format be set to know data format)
 // * dsp.BodyPath being a url

repo/dataset_test.go

@@ -1,15 +1,92 @@
 package repo
 
 import (
+ "encoding/base64"
  "fmt"
  "io/ioutil"
  "net/http"
  "net/http/httptest"
  "testing"
 
+ "github.com/libp2p/go-libp2p-crypto"
+ "github.com/qri-io/cafs"
  "github.com/qri-io/dataset"
+ "github.com/qri-io/qri/repo/profile"
 )
 
+// base64-encoded Test Private Key, decoded in init
+// peerId: QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt
+var (
+ testPk = []byte(`CAASpgkwggSiAgEAAoIBAQC/7Q7fILQ8hc9g07a4HAiDKE4FahzL2eO8OlB1K99Ad4L1zc2dCg+gDVuGwdbOC29IngMA7O3UXijycckOSChgFyW3PafXoBF8Zg9MRBDIBo0lXRhW4TrVytm4Etzp4pQMyTeRYyWR8e2hGXeHArXM1R/A/SjzZUbjJYHhgvEE4OZy7WpcYcW6K3qqBGOU5GDMPuCcJWac2NgXzw6JeNsZuTimfVCJHupqG/dLPMnBOypR22dO7yJIaQ3d0PFLxiDG84X9YupF914RzJlopfdcuipI+6gFAgBw3vi6gbECEzcohjKf/4nqBOEvCDD6SXfl5F/MxoHurbGBYB2CJp+FAgMBAAECggEAaVOxe6Y5A5XzrxHBDtzjlwcBels3nm/fWScvjH4dMQXlavwcwPgKhy2NczDhr4X69oEw6Msd4hQiqJrlWd8juUg6vIsrl1wS/JAOCS65fuyJfV3Pw64rWbTPMwO3FOvxj+rFghZFQgjg/i45uHA2UUkM+h504M5Nzs6Arr/rgV7uPGR5e5OBw3lfiS9ZaA7QZiOq7sMy1L0qD49YO1ojqWu3b7UaMaBQx1Dty7b5IVOSYG+Y3U/dLjhTj4Hg1VtCHWRm3nMOE9cVpMJRhRzKhkq6gnZmni8obz2BBDF02X34oQLcHC/Wn8F3E8RiBjZDI66g+iZeCCUXvYz0vxWAQQKBgQDEJu6flyHPvyBPAC4EOxZAw0zh6SF/r8VgjbKO3n/8d+kZJeVmYnbsLodIEEyXQnr35o2CLqhCvR2kstsRSfRz79nMIt6aPWuwYkXNHQGE8rnCxxyJmxV4S63GczLk7SIn4KmqPlCI08AU0TXJS3zwh7O6e6kBljjPt1mnMgvr3QKBgQD6fAkdI0FRZSXwzygx4uSg47Co6X6ESZ9FDf6ph63lvSK5/eue/ugX6p/olMYq5CHXbLpgM4EJYdRfrH6pwqtBwUJhlh1xI6C48nonnw+oh8YPlFCDLxNG4tq6JVo071qH6CFXCIank3ThZeW5a3ZSe5pBZ8h4bUZ9H8pJL4C7yQKBgFb8SN/+/qCJSoOeOcnohhLMSSD56MAeK7KIxAF1jF5isr1TP+rqiYBtldKQX9bIRY3/8QslM7r88NNj+aAuIrjzSausXvkZedMrkXbHgS/7EAPflrkzTA8fyH10AsLgoj/68mKr5bz34nuY13hgAJUOKNbvFeC9RI5g6eIqYH0FAoGAVqFTXZp12rrK1nAvDKHWRLa6wJCQyxvTU8S1UNi2EgDJ492oAgNTLgJdb8kUiH0CH0lhZCgr9py5IKW94OSM6l72oF2UrS6PRafHC7D9b2IV5Al9lwFO/3MyBrMocapeeyaTcVBnkclz4Qim3OwHrhtFjF1ifhP9DwVRpuIg+dECgYANwlHxLe//tr6BM31PUUrOxP5Y/cj+ydxqM/z6papZFkK6Mvi/vMQQNQkh95GH9zqyC5Z/yLxur4ry1eNYty/9FnuZRAkEmlUSZ/DobhU0Pmj8Hep6JsTuMutref6vCk2n02jc9qYmJuD7iXkdXDSawbEG6f5C4MUkJ38z1t1OjA==`)
+ privKey crypto.PrivKey
+
+ testPeerProfile = &profile.Profile{
+ Peername: "peer",
+ ID: "QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt",
+ }
+)
+
+func init() {
+ data, err := base64.StdEncoding.DecodeString(string(testPk))
+ if err != nil {
+ panic(err)
+ }
+ testPk = data
+
+ privKey, err = crypto.UnmarshalPrivateKey(testPk)
+ if err != nil {
+ panic(fmt.Errorf("error unmarshaling private key: %s", err.Error()))
+ }
+ testPeerProfile.PrivKey = privKey
+}
+
+func TestCreateDataset(t *testing.T) {
+ r, err := NewMemRepo(testPeerProfile, cafs.NewMapstore(), profile.NewMemStore(), nil)
+ if err != nil {
+ t.Fatal(err.Error())
+ }
+
+ ds := &dataset.Dataset{
+ Meta: &dataset.Meta{Title: "test"},
+ Commit: &dataset.Commit{Title: "hello"},
+ Structure: &dataset.Structure{
+ Format: dataset.JSONDataFormat,
+ Schema: dataset.BaseSchemaArray,
+ },
+ }
+
+ if _, err := CreateDataset(r, "foo", &dataset.Dataset{}, nil, true); err == nil {
+ t.Error("expected bad dataset to error")
+ }
+
+ ref, err := CreateDataset(r, "foo", ds, cafs.NewMemfileBytes("body.json", []byte("[]")), true)
+ if err != nil {
+ t.Fatal(err.Error())
+ }
+ refs, err := r.References(10, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(refs) != 1 {
+ t.Errorf("ref length mismatch. expected 1, got: %d", len(refs))
+ }
+
+ ds.Meta.Title = "an update"
+ ds.PreviousPath = ref.Path
+
+ ref, err = CreateDataset(r, "foo", ds, cafs.NewMemfileBytes("body.json", []byte("[]")), true)
+ if err != nil {
+ t.Fatal(err.Error())
+ }
+ refs, err = r.References(10, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(refs) != 1 {
+ t.Errorf("ref length mismatch. expected 1, got: %d", len(refs))
+ }
+}
+
 func TestDatasetPodBodyFile(t *testing.T) {
  s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
  w.Write([]byte(`{"json":"data"}`))

repo/graph_test.go

@@ -1,30 +1,17 @@
 package repo
 
 import (
- "encoding/base64"
  "fmt"
- "github.com/qri-io/dataset/dsfs"
  "testing"
 
  "github.com/ipfs/go-datastore"
  "github.com/libp2p/go-libp2p-crypto"
  "github.com/qri-io/cafs"
  "github.com/qri-io/dataset"
+ "github.com/qri-io/dataset/dsfs"
  "github.com/qri-io/qri/repo/profile"
 )
 
-// base64-encoded Test Private Key, decoded in init
-// peerId: QmZePf5LeXow3RW5U1AgEiNbW46YnRGhZ7HPvm1UmPFPwt
-var testPk = []byte(`CAASpgkwggSiAgEAAoIBAQC/7Q7fILQ8hc9g07a4HAiDKE4FahzL2eO8OlB1K99Ad4L1zc2dCg+gDVuGwdbOC29IngMA7O3UXijycckOSChgFyW3PafXoBF8Zg9MRBDIBo0lXRhW4TrVytm4Etzp4pQMyTeRYyWR8e2hGXeHArXM1R/A/SjzZUbjJYHhgvEE4OZy7WpcYcW6K3qqBGOU5GDMPuCcJWac2NgXzw6JeNsZuTimfVCJHupqG/dLPMnBOypR22dO7yJIaQ3d0PFLxiDG84X9YupF914RzJlopfdcuipI+6gFAgBw3vi6gbECEzcohjKf/4nqBOEvCDD6SXfl5F/MxoHurbGBYB2CJp+FAgMBAAECggEAaVOxe6Y5A5XzrxHBDtzjlwcBels3nm/fWScvjH4dMQXlavwcwPgKhy2NczDhr4X69oEw6Msd4hQiqJrlWd8juUg6vIsrl1wS/JAOCS65fuyJfV3Pw64rWbTPMwO3FOvxj+rFghZFQgjg/i45uHA2UUkM+h504M5Nzs6Arr/rgV7uPGR5e5OBw3lfiS9ZaA7QZiOq7sMy1L0qD49YO1ojqWu3b7UaMaBQx1Dty7b5IVOSYG+Y3U/dLjhTj4Hg1VtCHWRm3nMOE9cVpMJRhRzKhkq6gnZmni8obz2BBDF02X34oQLcHC/Wn8F3E8RiBjZDI66g+iZeCCUXvYz0vxWAQQKBgQDEJu6flyHPvyBPAC4EOxZAw0zh6SF/r8VgjbKO3n/8d+kZJeVmYnbsLodIEEyXQnr35o2CLqhCvR2kstsRSfRz79nMIt6aPWuwYkXNHQGE8rnCxxyJmxV4S63GczLk7SIn4KmqPlCI08AU0TXJS3zwh7O6e6kBljjPt1mnMgvr3QKBgQD6fAkdI0FRZSXwzygx4uSg47Co6X6ESZ9FDf6ph63lvSK5/eue/ugX6p/olMYq5CHXbLpgM4EJYdRfrH6pwqtBwUJhlh1xI6C48nonnw+oh8YPlFCDLxNG4tq6JVo071qH6CFXCIank3ThZeW5a3ZSe5pBZ8h4bUZ9H8pJL4C7yQKBgFb8SN/+/qCJSoOeOcnohhLMSSD56MAeK7KIxAF1jF5isr1TP+rqiYBtldKQX9bIRY3/8QslM7r88NNj+aAuIrjzSausXvkZedMrkXbHgS/7EAPflrkzTA8fyH10AsLgoj/68mKr5bz34nuY13hgAJUOKNbvFeC9RI5g6eIqYH0FAoGAVqFTXZp12rrK1nAvDKHWRLa6wJCQyxvTU8S1UNi2EgDJ492oAgNTLgJdb8kUiH0CH0lhZCgr9py5IKW94OSM6l72oF2UrS6PRafHC7D9b2IV5Al9lwFO/3MyBrMocapeeyaTcVBnkclz4Qim3OwHrhtFjF1ifhP9DwVRpuIg+dECgYANwlHxLe//tr6BM31PUUrOxP5Y/cj+ydxqM/z6papZFkK6Mvi/vMQQNQkh95GH9zqyC5Z/yLxur4ry1eNYty/9FnuZRAkEmlUSZ/DobhU0Pmj8Hep6JsTuMutref6vCk2n02jc9qYmJuD7iXkdXDSawbEG6f5C4MUkJ38z1t1OjA==`)
-
-func init() {
- data, err := base64.StdEncoding.DecodeString(string(testPk))
- if err != nil {
- panic(err)
- }
- testPk = data
-}
-
 func TestGraph(t *testing.T) {
  r, err := makeTestRepo()
  if err != nil {

repo/repo.go

@@ -83,12 +83,3 @@ type SearchParams struct {
 type Searchable interface {
  Search(p SearchParams) ([]DatasetRef, error)
 }
-
-// MustProfile loads a repo's profile data, panicing if any error is encountered
-func MustProfile(r Repo) *profile.Profile {
- p, err := r.Profile()
- if err != nil {
- panic(err)
- }
- return p
-}

repo/test/test_repo.go

@@ -11,8 +11,8 @@ import (
  "github.com/ghodss/yaml"
  "github.com/libp2p/go-libp2p-crypto"
  "github.com/qri-io/cafs"
+ "github.com/qri-io/dataset"
  "github.com/qri-io/dataset/dstest"
- "github.com/qri-io/qri/actions"
  "github.com/qri-io/qri/config"
  "github.com/qri-io/qri/repo"
  "github.com/qri-io/qri/repo/profile"
@@ -43,7 +43,6 @@ func init() {
  privKey, err = crypto.UnmarshalPrivateKey(testPk)
  if err != nil {
  panic(fmt.Errorf("error unmarshaling private key: %s", err.Error()))
- return
  }
  testPeerProfile.PrivKey = privKey
 }
@@ -68,19 +67,14 @@ func NewTestRepo(rc *regclient.Client) (mr *repo.MemRepo, err error) {
  return
  }
 
- act := actions.Dataset{mr}
-
  gopath := os.Getenv("GOPATH")
  for _, k := range datasets {
-
  tc, err := dstest.NewTestCaseFromDir(fmt.Sprintf("%s/src/github.com/qri-io/qri/repo/test/testdata/%s", gopath, k))
  if err != nil {
  return nil, err
  }
 
- datafile := cafs.NewMemfileBytes(tc.BodyFilename, tc.Body)
-
- if _, err = act.CreateDataset(tc.Name, tc.Input, datafile, nil, true); err != nil {
+ if err = createDataset(mr, tc); err != nil {
  return nil, fmt.Errorf("%s error creating dataset: %s", k, err.Error())
  }
  }
@@ -109,7 +103,6 @@ func NewTestRepoFromProfileID(id profile.ID, peerNum int, dataIndex int) (repo.R
  if dataIndex == -1 || dataIndex >= len(datasets) {
  return r, nil
  }
- act := actions.Dataset{r}
 
  gopath := os.Getenv("GOPATH")
  filepath := fmt.Sprintf("%s/src/github.com/qri-io/qri/repo/test/testdata/%s", gopath, datasets[dataIndex])
@@ -118,11 +111,9 @@ func NewTestRepoFromProfileID(id profile.ID, peerNum int, dataIndex int) (repo.R
  return r, err
  }
 
- datafile := cafs.NewMemfileBytes(tc.BodyFilename, tc.Body)
- if _, err = act.CreateDataset(tc.Name, tc.Input, datafile, nil, true); err != nil {
+ if err := createDataset(r, tc); err != nil {
  return nil, fmt.Errorf("error creating dataset: %s", err.Error())
  }
-
  return r, nil
 }
 
@@ -131,6 +122,40 @@ func pkgPath(paths ...string) string {
  return filepath.Join(append([]string{gp, "src/github.com/qri-io/qri/repo/test"}, paths...)...)
 }
 
+// it's tempting to use actions.Dataset.CreateDataset here, but we can't b/c import cycle :/
+// this version of createDataset doesn't run transforms or prepare viz. Test cases
+// should be designed to avoid requiring Tranforms be run or Viz be prepped
+func createDataset(r repo.Repo, tc dstest.TestCase) (err error) {
+ var (
+ ds = tc.Input
+ pro *profile.Profile
+ // NOTE - struct fields need to be instantiated to make assign set to
+ // new pointer values
+ userSet = &dataset.Dataset{
+ Commit: &dataset.Commit{},
+ Meta: &dataset.Meta{},
+ Structure: &dataset.Structure{},
+ Transform: &dataset.Transform{},
+ Viz: &dataset.Viz{},
+ }
+ )
+ pro, err = r.Profile()
+ if err != nil {
+ return
+ }
+
+ userSet.Assign(ds)
+
+ if ds.Commit != nil {
+ // NOTE: add author ProfileID here to keep the dataset package agnostic to
+ // all identity stuff except keypair crypto
+ ds.Commit.Author = &dataset.User{ID: pro.ID.String()}
+ }
+
+ _, err = repo.CreateDataset(r, tc.Name, ds, tc.BodyFile(), true)
+ return
+}
+
 // NewMemRepoFromDir reads a director of testCases and calls createDataset
 // on each case with the given privatekey, yeilding a repo where the peer with
 // this pk has created each dataset in question
@@ -145,15 +170,14 @@ func NewMemRepoFromDir(path string) (repo.Repo, crypto.PrivKey, error) {
  if err != nil {
  return mr, pk, err
  }
- act := actions.Dataset{mr}
 
  tc, err := dstest.LoadTestCases(path)
  if err != nil {
  return mr, pk, err
  }
 
  for _, c := range tc {
- if _, err := act.CreateDataset(c.Name, c.Input, c.BodyFile(), nil, true); err != nil {
+ if err := createDataset(mr, c); err != nil {
  return mr, pk, err
  }
  }