fix: Fixed the delete order of delete recursive (#119)

beyondstorage · Nov 26, 2021 · c40c503 · c40c503
1 parent 87fb94b
commit c40c503
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 40 deletions.
diff --git a/cmd/byctl/rm.go b/cmd/byctl/rm.go
@@ -66,34 +66,60 @@ var rmCmd = &cli.Command{
 
 			so := operations.NewSingleOperator(store)
 
-			var ch chan *operations.EmptyResult
 			if c.Bool(rmFlagMultipart) && !c.Bool(rmFlagRecursive) {
 				// Remove all multipart objects whose path is `key`
-				ch, err = so.DeleteMultipart(key)
+				ch, err := so.DeleteMultipart(key)
 				if err != nil {
 					logger.Error("delete multipart",
 						zap.String("path", key),
 						zap.Error(err))
 					continue
 				}
+
+				if ch != nil {
+					for v := range ch {
+						if v.Error != nil {
+							logger.Error("delete", zap.Error(err))
+							continue
+						}
+					}
+				}
 			} else if c.Bool(rmFlagMultipart) && c.Bool(rmFlagRecursive) {
 				// Remove all multipart objects prefixed with `key`.
-				ch, err = so.DeleteMultipartViaRecursively(key)
+				ch, err := so.DeleteMultipartViaRecursively(key)
 				if err != nil {
 					logger.Error("delete multipart recursively",
 						zap.String("path", key),
 						zap.Error(err))
 					continue
 				}
+
+				if ch != nil {
+					for v := range ch {
+						if v.Error != nil {
+							logger.Error("delete", zap.Error(err))
+							continue
+						}
+					}
+				}
 			} else if !c.Bool(rmFlagMultipart) && c.Bool(rmFlagRecursive) {
 				// recursive remove a dir.
-				ch, err = so.DeleteRecursively(key)
+				ch, err := so.DeleteRecursively(key)
 				if err != nil {
 					logger.Error("delete recursively",
 						zap.String("path", key),
 						zap.Error(err))
 					continue
 				}
+
+				if ch != nil {
+					for v := range ch {
+						if v.Error != nil {
+							logger.Error("delete", zap.Error(err))
+							continue
+						}
+					}
+				}
 			} else {
 				// remove single file
 				o, err := so.Stat(key)
@@ -116,15 +142,6 @@ var rmCmd = &cli.Command{
 					continue
 				}
 			}
-
-			if ch != nil {
-				for v := range ch {
-					if v.Error != nil {
-						logger.Error("delete", zap.Error(err))
-						continue
-					}
-				}
-			}
 		}
 		return nil
 	},

diff --git a/operations/delete.go b/operations/delete.go
@@ -116,40 +116,40 @@ func (so *SingleOperator) DeleteMultipartViaRecursively(path string) (ch chan *E
 func (so *SingleOperator) DeleteRecursively(path string) (ch chan *EmptyResult, err error) {
 	ch = make(chan *EmptyResult, 4)
 
-	och, err := so.ListRecursively(path)
-	if err != nil {
-		return nil, err
-	}
-
 	go func() {
 		defer close(ch)
 
-		wg := &sync.WaitGroup{}
+		so.deleteRecursively(ch, path)
+	}()
 
-		for or := range och {
-			if or.Error != nil {
-				ch <- &EmptyResult{Error: or.Error}
-				break
-			}
-			object := or.Object
+	return
+}
 
-			wg.Add(1)
-			err = so.pool.Submit(func() {
-				defer wg.Done()
+func (so *SingleOperator) deleteRecursively(ch chan *EmptyResult, path string) {
+	it, err := so.store.List(path, pairs.WithListMode(types.ListModeDir))
+	if err != nil {
+		ch <- &EmptyResult{Error: err}
+		return
+	}
 
-				err = so.Delete(object.Path)
-				if err != nil {
-					ch <- &EmptyResult{Error: err}
-				}
-			})
-			if err != nil {
-				ch <- &EmptyResult{Error: err}
-				break
-			}
+	for {
+		o, err := it.Next()
+		if err != nil && errors.Is(err, types.IterateDone) {
+			break
+		}
+		if err != nil {
+			ch <- &EmptyResult{Error: err}
+			break
 		}
 
-		wg.Wait()
-	}()
+		if o.Mode.IsDir() {
+			so.deleteRecursively(ch, o.Path)
+		}
 
-	return ch, nil
+		err = so.store.Delete(o.Path)
+		if err != nil {
+			ch <- &EmptyResult{Error: err}
+			return
+		}
+	}
 }