fix(rename): Allow rename for dataset with no history. Update .qri-ref

base/ref.go

@@ -80,8 +80,10 @@ func ModifyDatasetRef(ctx context.Context, r repo.Repo, current, new *repo.Datas
  return err
  }
  if err := repo.CanonicalizeDatasetRef(r, current); err != nil {
- log.Debug(err.Error())
- return fmt.Errorf("error with existing reference: %s", err.Error())
+ if err != repo.ErrNoHistory {
+ log.Debug(err.Error())
+ return fmt.Errorf("error with existing reference: %s", err.Error())
+ }
  }
  err = repo.CanonicalizeDatasetRef(r, new)
  if err == nil {

cmd/rename_integration_test.go

@@ -0,0 +1,124 @@
+package cmd
+
+import (
+ "os"
+ "regexp"
+ "path/filepath"
+ "testing"
+
+ "github.com/google/go-cmp/cmp"
+)
+
+// Test rename works if dataset has no history
+func TestRenameNoHistory(t *testing.T) {
+ run := NewFSITestRunner(t, "qri_test_rename_no_history")
+ defer run.Delete()
+
+ workDir := run.CreateAndChdirToWorkDir("remove_no_history")
+
+ // Init as a linked directory
+ run.MustExec(t, "qri init --name remove_no_history --format csv")
+
+ // Read .qri-ref file, it contains the reference this directory is linked to
+ actual := run.MustReadFile(t, filepath.Join(workDir, ".qri-ref"))
+ expect := "test_peer/remove_no_history"
+ if diff := cmp.Diff(expect, actual); diff != "" {
+ t.Errorf("qri list (-want +got):\n%s", diff)
+ }
+
+ // Go up one directory
+ parentDir := filepath.Dir(workDir)
+ os.Chdir(parentDir)
+
+ // Rename
+ run.MustExec(t, "qri rename me/remove_no_history me/remove_second_name")
+
+ // Old dataset name can't be used
+ err := run.ExecCommand("qri get me/remove_no_history")
+ if err == nil {
+ t.Error("expected error, did not get one")
+ }
+ expect = "repo: not found"
+ if err.Error() != expect {
+ t.Errorf("error mismatch, expect: %s, got: %s", expect, err.Error())
+ }
+
+ // New dataset name can be used, but still has no history
+ err = run.ExecCommand("qri get me/remove_second_name")
+ if err == nil {
+ t.Error("expected error, did not get one")
+ }
+ expect = "repo: no history"
+ if err.Error() != expect {
+ t.Errorf("error mismatch, expect: %s, got: %s", expect, err.Error())
+ }
+
+ // Read .qri-ref file, it contains the new reference name
+ actual = run.MustReadFile(t, filepath.Join(workDir, ".qri-ref"))
+ expect = "test_peer/remove_second_name"
+ if diff := cmp.Diff(expect, actual); diff != "" {
+ t.Errorf("qri list (-want +got):\n%s", diff)
+ }
+
+ // Test that `qri list` will only show the new ref. Still linked to the old directory name.
+ output := run.MustExec(t, "qri list")
+ // TODO(dlong): Move temp omissions into TestRunner framework
+ regex := regexp.MustCompile("linked: .*/remove_no_history")
+ replaced := string(regex.ReplaceAll([]byte(output), []byte("linked: /tmp/remove_no_history")))
+ expect = `1 test_peer/remove_second_name
+ linked: /tmp/remove_no_history
+
+`
+ if diff := cmp.Diff(expect, replaced); diff != "" {
+ t.Errorf("qri list (-want +got):\n%s", diff)
+ }
+}
+
+// Test rename updates the qri-ref link
+func TestRenameUpdatesLink(t *testing.T) {
+ run := NewFSITestRunner(t, "qri_test_rename_update_link")
+ defer run.Delete()
+
+ workDir := run.CreateAndChdirToWorkDir("remove_update_link")
+
+ // Init as a linked directory
+ run.MustExec(t, "qri init --name remove_update_link --format csv")
+
+ // Save a version
+ run.MustExec(t, "qri save")
+
+ // Read .qri-ref file, it contains the reference this directory is linked to
+ actual := run.MustReadFile(t, filepath.Join(workDir, ".qri-ref"))
+ expect := "test_peer/remove_update_link"
+ if diff := cmp.Diff(expect, actual); diff != "" {
+ t.Errorf("qri list (-want +got):\n%s", diff)
+ }
+
+ // Go up one directory
+ parentDir := filepath.Dir(workDir)
+ os.Chdir(parentDir)
+
+ // Rename
+ run.MustExec(t, "qri rename me/remove_update_link me/remove_second_name")
+
+ // Test that `qri list` will only show the new ref. Still linked to the old directory name.
+ output := run.MustExec(t, "qri list")
+ // TODO(dlong): Move temp omissions into TestRunner framework
+ regex := regexp.MustCompile("linked: .*/remove_update_link")
+ replaced := string(regex.ReplaceAll([]byte(output), []byte("linked: /tmp/remove_update_link")))
+ expect = `1 test_peer/remove_second_name
+ linked: /tmp/remove_update_link
+ 22 B, 2 entries, 0 errors
+
+`
+ if diff := cmp.Diff(expect, replaced); diff != "" {
+ t.Errorf("qri list (-want +got):\n%s", diff)
+ }
+
+ // Read .qri-ref file, it contains the new dataset reference
+ actual = run.MustReadFile(t, filepath.Join(workDir, ".qri-ref"))
+ expect = "test_peer/remove_second_name"
+ if diff := cmp.Diff(expect, actual); diff != "" {
+ t.Errorf("read .qri-ref (-want +got):\n%s", diff)
+ }
+}

fsi/fsi.go

@@ -143,20 +143,25 @@ func (fsi *FSI) CreateLink(dirPath, refStr string) (alias string, rollback func(
  return ref.AliasString(), removeLinkAndRemoveRefFunc, err
 }
 
-// UpdateLink changes an existing link entry
-func (fsi *FSI) UpdateLink(dirPath, refStr string) (string, error) {
+// ModifyLinkDirectory changes the directory for the linked dataset reference
+func (fsi *FSI) ModifyLinkDirectory(dirPath, refStr string) error {
+ return fmt.Errorf("TODO: Implement me")
+}
+
+// ModifyLinkReference changes the reference for the existing linked working directory
+func (fsi *FSI) ModifyLinkReference(dirPath, refStr string) error {
  ref, err := repo.ParseDatasetRef(refStr)
  if err != nil {
- return "", err
+ return err
  }
-
- if err = repo.CanonicalizeDatasetRef(fsi.repo, &ref); err != nil {
- return ref.String(), err
+ if err = repo.CanonicalizeDatasetRef(fsi.repo, &ref); err != nil && err != repo.ErrNoHistory {
+ return err
  }
 
- ref.FSIPath = dirPath
- err = fsi.repo.PutRef(ref)
- return ref.String(), err
+ if _, err = writeLinkFile(dirPath, ref.AliasString()); err != nil {
+ return err
+ }
+ return nil
 }
 
 // Unlink breaks the connection between a directory and a dataset

fsi/fsi_test.go

@@ -199,7 +199,7 @@ func TestCreateLinkAgainOnceQriRefRemoved(t *testing.T) {
  }
 }
 
-func TestUpdateLink(t *testing.T) {
+func TestModifyLink(t *testing.T) {
  paths := NewTmpPaths()
  defer paths.Close()
 
@@ -208,13 +208,9 @@ func TestUpdateLink(t *testing.T) {
  if err != nil {
  t.Fatalf(err.Error())
  }
- link, err := fsi.UpdateLink(paths.firstDir, "me/test_ds@/ipfs/QmExample")
+ err = fsi.ModifyLinkReference(paths.firstDir, "me/test_ds@/ipfs/QmExample")
  if err != nil {
- t.Fatalf(err.Error())
- }
- expect := `peer/test_ds@/ipfs/QmExample`
- if link != expect {
- t.Errorf("error: link did not match, actual: %s, expect: %s", link, expect)
+ t.Errorf("expected ModifyLinkReference to succeed, got: %s", err.Error())
  }
 }
 

lib/datasets.go

@@ -536,11 +536,19 @@ func (r *DatasetRequests) Rename(p *RenameParams, res *repo.DatasetRef) (err err
  return fmt.Errorf("current name is required to rename a dataset")
  }
 
+ // Update the reference stored in the repo
  if err := base.ModifyDatasetRef(ctx, r.node.Repo, &p.Current, &p.New, true /*isRename*/); err != nil {
  return err
  }
 
- if err = base.ReadDataset(ctx, r.node.Repo, &p.New); err != nil {
+ // If the dataset is linked to a working directory, update the ref
+ if p.New.FSIPath != "" {
+ if err = r.inst.fsi.ModifyLinkReference(p.New.FSIPath, p.New.String()); err != nil {
+ return err
+ }
+ }
+
+ if err = base.ReadDataset(ctx, r.node.Repo, &p.New); err != nil && err != repo.ErrNoHistory {
  log.Debug(err.Error())
  return err
  }

lib/fsi.go

@@ -39,8 +39,9 @@ func (m *FSIMethods) LinkedRefs(p *ListParams, res *[]repo.DatasetRef) (err erro
 
 // LinkParams encapsulate parameters to the link method
 type LinkParams struct {
- Dir string
- Ref string
+ Dir string
+ Ref string
+ ToModify string
 }
 
 // CreateLink creates a connection between a working drirectory and a dataset history
@@ -61,14 +62,29 @@ func (m *FSIMethods) CreateLink(p *LinkParams, res *string) (err error) {
  return err
 }
 
-// UpdateLink creates a connection between a working drirectory and a dataset history
-func (m *FSIMethods) UpdateLink(p *LinkParams, res *string) (err error) {
+// ModifyLink changes an existing link by either updating the ref for a directory, or vice versa
+func (m *FSIMethods) ModifyLink(p *LinkParams, res *bool) (err error) {
+ // absolutize path name
+ path, err := filepath.Abs(p.Dir)
+ if err != nil {
+ return err
+ }
+
+ p.Dir = path
+
  if m.inst.rpc != nil {
- return m.inst.rpc.Call("FSIMethods.UpdateLink", p, res)
+ return m.inst.rpc.Call("FSIMethods.ModifyLink", p, res)
  }
 
- *res, err = m.inst.fsi.UpdateLink(p.Dir, p.Ref)
- return err
+ if p.ToModify == "dir" {
+ *res = true
+ return m.inst.fsi.ModifyLinkDirectory(p.Dir, p.Ref)
+ } else if p.ToModify == "ref" {
+ *res = true
+ return m.inst.fsi.ModifyLinkReference(p.Dir, p.Ref)
+ } else {
+ return fmt.Errorf("ToModify has unknown value %q", p.ToModify)
+ }
 }
 
 // Unlink rmeoves a connection between a working drirectory and a dataset history