feat(remove): add files and unlink flags to remove

plumb up to API and CLI. next up: tests

api/datasets.go

@@ -456,9 +456,15 @@ func (h *DatasetHandlers) saveHandler(w http.ResponseWriter, r *http.Request) {
 
 func (h *DatasetHandlers) removeHandler(w http.ResponseWriter, r *http.Request) {
 	p := lib.RemoveParams{
-		Ref:      HTTPPathToQriPath(r.URL.Path[len("/remove"):]),
-		Revision: rev.Rev{Field: "ds", Gen: -1},
+		Ref:            HTTPPathToQriPath(r.URL.Path[len("/remove"):]),
+		Revision:       rev.Rev{Field: "ds", Gen: -1},
+		Unlink:         r.FormValue("unlink") == "true",
+		DeleteFSIFiles: r.FormValue("delete") == "true",
 	}
+	if r.FormValue("all") == "true" {
+		p.Revision = rev.NewAllRevisions()
+	}
+
 	res := lib.RemoveResponse{}
 	if err := h.Remove(&p, &res); err != nil {
 		log.Infof("error deleting dataset: %s", err.Error())

cmd/remove.go

@@ -44,6 +44,8 @@ both qri & IPFS. Promise.`,
 
 	cmd.Flags().StringVarP(&o.RevisionsText, "revisions", "r", "", "revisions to delete")
 	cmd.Flags().BoolVarP(&o.All, "all", "a", false, "synonym for --revisions=all")
+	cmd.Flags().BoolVar(&o.DeleteFSIFiles, "files", false, "delete linked files in dataset directory")
+	cmd.Flags().BoolVar(&o.Unlink, "unlink", false, "break link to directory")
 
 	return cmd
 }
@@ -54,9 +56,11 @@ type RemoveOptions struct {
 
 	Args []string
 
-	RevisionsText string
-	All           bool
-	Revision      rev.Rev
+	RevisionsText  string
+	Revision       rev.Rev
+	All            bool
+	DeleteFSIFiles bool
+	Unlink         bool
 
 	DatasetRequests *lib.DatasetRequests
 }
@@ -100,8 +104,10 @@ func (o *RemoveOptions) Validate() error {
 func (o *RemoveOptions) Run() (err error) {
 	for _, arg := range o.Args {
 		params := lib.RemoveParams{
-			Ref:      arg,
-			Revision: o.Revision,
+			Ref:            arg,
+			Revision:       o.Revision,
+			DeleteFSIFiles: o.DeleteFSIFiles,
+			Unlink:         o.Unlink,
 		}
 
 		res := lib.RemoveResponse{}
@@ -116,6 +122,12 @@ func (o *RemoveOptions) Run() (err error) {
 		} else {
 			printSuccess(o.Out, "removed %d revisions of dataset '%s'", res.NumDeleted, res.Ref)
 		}
+		if res.Unlinked {
+			printSuccess(o.Out, "removed dataset link")
+		}
+		if res.DeletedFSIFiles {
+			printSuccess(o.Out, "deleted dataset files")
+		}
 	}
 	return nil
 }

fsi/fsi.go

@@ -133,6 +133,9 @@ func (fsi *FSI) UpdateLink(dirPath, refStr string) (string, error) {
 }
 
 // Unlink breaks the connection between a directory and a dataset
+// TODO (b5) - should this even need the refStr, since we consider the .qri-ref
+// file the canonical source of truth, shouldn't this look for that file,
+// and use the reference stored within to talk to the repo?
 func (fsi *FSI) Unlink(dirPath, refStr string) error {
 	ref, err := repo.ParseDatasetRef(refStr)
 	if err != nil {
@@ -152,6 +155,10 @@ func (fsi *FSI) Unlink(dirPath, refStr string) error {
 		return err
 	}
 
+	// attempt to remove the directory, this will error if the directory is
+	// not empty. We intentionally ignore this error
+	os.Remove(dirPath)
+
 	ref.FSIPath = ""
 	return fsi.repo.PutRef(ref)
 }

fsi/mapping.go

@@ -385,6 +385,28 @@ func WriteComponents(ds *dataset.Dataset, dirPath string) error {
 	return nil
 }
 
+// DeleteDatasetFiles removes mapped files from a directory. if the result of
+// moving all files leaves the directory empty
+func DeleteDatasetFiles(dirPath string) (removed []string, err error) {
+	_, mapping, _, err := ReadDir(dirPath)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, stat := range mapping {
+		if err := os.Remove(stat.Path); err != nil {
+			return nil, err
+		}
+		removed = append(removed, stat.Path)
+	}
+
+	// attempt to remove the directory, this will error if the directory is
+	// not empty. We intentionally ignore this error
+	os.Remove(dirPath)
+
+	return removed, nil
+}
+
 // DeleteComponents removes the list of named components from the given directory
 func DeleteComponents(removeList []string, fileMap map[string]FileStat, dirPath string) error {
 	for _, comp := range removeList {

lib/datasets.go

@@ -498,15 +498,18 @@ func (r *DatasetRequests) Rename(p *RenameParams, res *repo.DatasetRef) (err err
 
 // RemoveParams defines parameters for remove command
 type RemoveParams struct {
-	Ref string
-	// Ref      *repo.DatasetRef
-	Revision rev.Rev
+	Ref            string
+	Revision       rev.Rev
+	Unlink         bool // If true, break any FSI link
+	DeleteFSIFiles bool // If true, delete tracked files from the designated FSI link
 }
 
 // RemoveResponse gives the results of a remove
 type RemoveResponse struct {
-	Ref        string
-	NumDeleted int
+	Ref             string
+	NumDeleted      int
+	Unlinked        bool // true if the remove unlinked an FSI-linked dataset
+	DeletedFSIFiles bool // true if the remove deleted FSI-linked files
 }
 
 // Remove a dataset entirely or remove a certain number of revisions
@@ -528,18 +531,42 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
 	if ref.Path == "" && ref.Peername == "" && ref.Name == "" {
 		return fmt.Errorf("either peername/name or path is required")
 	}
-
 	if p.Revision.Field != "ds" {
 		return fmt.Errorf("can only delete whole dataset revisions, not individual fields")
 	}
+	if ref.FSIPath == "" && p.Unlink {
+		return fmt.Errorf("cannot unlink, dataset is not linked to a directory")
+	}
+	if ref.FSIPath == "" && p.DeleteFSIFiles {
+		return fmt.Errorf("can't delete files, dataset is not linked to a directory")
+	}
 
-	if p.Revision.Gen == rev.AllGenerations {
+	removeEntireDataset := func() error {
 		// Delete entire dataset for all generations.
 		if err := actions.DeleteDataset(r.node, &ref); err != nil {
 			return err
 		}
 		res.NumDeleted = rev.AllGenerations
+
+		// 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 err := r.inst.fsi.Unlink(ref.FSIPath, ref.AliasString()); err != nil {
+				return err
+			}
+			res.Unlinked = true
+		}
 		return nil
+	}
+
+	if p.Revision.Gen == rev.AllGenerations {
+		return removeEntireDataset()
 	} else if p.Revision.Gen < 1 {
 		return fmt.Errorf("invalid number of revisions to delete: %d", p.Revision.Gen)
 	}
@@ -552,11 +579,7 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
 
 	// If deleting more revisions then exist, delete the entire dataset.
 	if p.Revision.Gen >= len(log) {
-		if err := actions.DeleteDataset(r.node, &ref); err != nil {
-			return err
-		}
-		res.NumDeleted = rev.AllGenerations
-		return nil
+		return removeEntireDataset()
 	}
 
 	// Delete the specific number of revisions.
@@ -566,15 +589,21 @@ func (r *DatasetRequests) Remove(p *RemoveParams, res *RemoveResponse) error {
 	}
 	res.NumDeleted = p.Revision.Gen
 
-	// if rc := r.Registry(); rc != nil {
-	// 	dse := ds.Encode()
-	// 	// TODO - this should be set by LoadDataset
-	// 	dse.Path = ref.Path
-	// 	if e := rc.DeleteDataset(ref.Peername, ref.Name, dse, pro.PrivKey.GetPublic()); e != nil {
-	// 		// ignore registry errors
-	// 		log.Errorf("deleting dataset: %s", e.Error())
-	// 	}
-	// }
+	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
+		}
+	}
 
 	return nil
 }