fix(remove): removing a dataset with no history should work

qri-io · Sep 12, 2019 · f0ba1a1 · f0ba1a1
1 parent ace7eac
commit f0ba1a1
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 38 deletions.
diff --git a/cmd/remove.go b/cmd/remove.go
@@ -75,19 +75,20 @@ func (o *RemoveOptions) Complete(f Factory, args []string) (err error) {
  o.Revision = rev.NewAllRevisions()
  } else {
  if o.RevisionsText == "" {
- return fmt.Errorf("--revisions flag is requried")
- }
- revisions, err := rev.ParseRevs(o.RevisionsText)
- if err != nil {
- return err
- }
- if len(revisions) != 1 {
- return fmt.Errorf("need exactly 1 revision parameter to remove")
- }
- if revisions[0] == nil {
- return fmt.Errorf("invalid nil revision")
+ o.Revision = rev.Rev{Field: "ds", Gen: 0}
+ } else {
+ revisions, err := rev.ParseRevs(o.RevisionsText)
+ if err != nil {
+ return err
+ }
+ if len(revisions) != 1 {
+ return fmt.Errorf("need exactly 1 revision parameter to remove")
+ }
+ if revisions[0] == nil {
+ return fmt.Errorf("invalid nil revision")
+ }
+ o.Revision = *revisions[0]
  }
- o.Revision = *revisions[0]
  }
  return err
 }

diff --git a/lib/datasets.go b/lib/datasets.go
@@ -527,8 +527,11 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
  return err
  }
 
- if err = repo.CanonicalizeDatasetRef(r.node.Repo, &ref); err != nil {
- return err
+ noHistory := false
+ if canonErr := repo.CanonicalizeDatasetRef(r.node.Repo, &ref); canonErr != nil && canonErr != repo.ErrNoHistory {
+ return canonErr
+ } else if canonErr == repo.ErrNoHistory {
+ noHistory = true
  }
  res.Ref = ref.String()
 
@@ -539,16 +542,30 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
  return fmt.Errorf("can't delete files, dataset is not linked to a directory")
  }
 
- removeEntireDataset := func() error {
- // removing all revisions of a dataset must unlink it
- if ref.FSIPath != "" {
- if p.DeleteFSIFiles {
- if _, err := fsi.DeleteDatasetFiles(ref.FSIPath); err != nil {
- return err
- }
- res.DeletedFSIFiles = true
+ if ref.FSIPath != "" {
+ if p.DeleteFSIFiles {
+ if _, err := fsi.DeleteDatasetFiles(ref.FSIPath); err != nil {
+ return err
  }
+ res.DeletedFSIFiles = true
+ }
+
+ // running remove on a dataset that has no history must always unlink
+ if p.Unlink || noHistory {
+ if err := r.inst.fsi.Unlink(ref.FSIPath, ref.AliasString()); err != nil {
+ return err
+ }
+ res.Unlinked = true
+ }
+ }
+
+ if noHistory {
+ return nil
+ }
 
+ removeEntireDataset := func() error {
+ // removing all revisions of a dataset must unlink it
+ if ref.FSIPath != "" && !p.Unlink {
  if err := r.inst.fsi.Unlink(ref.FSIPath, ref.AliasString()); err != nil {
  return err
  }
@@ -581,22 +598,6 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
  return removeEntireDataset()
  }
 
- if ref.FSIPath != "" {
- if p.DeleteFSIFiles {
- if _, err := fsi.DeleteDatasetFiles(ref.FSIPath); err != nil {
- return err
- }
- res.DeletedFSIFiles = true
- }
-
- if p.Unlink {
- if err := r.inst.fsi.Unlink(ref.FSIPath, ref.AliasString()); err != nil {
- return err
- }
- res.Unlinked = true
- }
- }
-
  // Delete the specific number of revisions.
  replace := log[p.Revision.Gen]
  if err := actions.ModifyDataset(r.node, &ref, &replace, false /*isRename*/); err != nil {

diff --git a/lib/datasets_test.go b/lib/datasets_test.go
@@ -652,6 +652,18 @@ func TestDatasetRequestsRemove(t *testing.T) {
  }
  defer os.RemoveAll(datasetsDir)
 
+ // initialize an example no-history dataset
+ initp := &InitFSIDatasetParams{
+ Name: "no_history",
+ Dir: datasetsDir,
+ Format: "csv",
+ Mkdir: "no_history",
+ }
+ var noHistoryName string
+ if err := fsim.InitDataset(initp, &noHistoryName); err != nil {
+ t.Fatal(err)
+ }
+
  // link cities dataset with a checkout
  checkoutp := &CheckoutParams{
  Dir: filepath.Join(datasetsDir, "cities"),
@@ -724,6 +736,10 @@ func TestDatasetRequestsRemove(t *testing.T) {
  RemoveParams{Ref: "peer/craigslist", Revision: rev.Rev{Field: "ds", Gen: 1}, Unlink: true, DeleteFSIFiles: true},
  RemoveResponse{NumDeleted: 1, Unlinked: true, DeletedFSIFiles: true},
  },
+ {"no history dataset, remove link, delete files",
+ RemoveParams{Ref: noHistoryName, Revision: rev.Rev{Field: "ds", Gen: 0}, DeleteFSIFiles: true},
+ RemoveResponse{NumDeleted: 0, Unlinked: true, DeletedFSIFiles: true},
+ },
  }
 
  for _, c := range goodCases {