Cleanup improved and code redundant code removed.

Signed-off-by: Christoph Fiehe <c.fiehe@eurodata.de>

api/publish.go

@@ -3,7 +3,6 @@ package api
 import (
 	"fmt"
 	"net/http"
-	"path/filepath"
 	"strings"
 
 	"github.com/aptly-dev/aptly/aptly"
@@ -484,20 +483,11 @@ func apiPublishUpdateSwitch(c *gin.Context) {
 		}
 
 		if b.SkipCleanup == nil || !*b.SkipCleanup {
-			publishedStorage := context.GetPublishedStorage(storage)
-
-			err = collection.CleanupPrefixComponentFiles(published.Prefix, result.UpdatedComponents(), publishedStorage, collectionFactory, out)
+			err = collection.CleanupPrefixComponentFiles(context, published, result.AddedComponents(), result.UpdatedComponents(), result.RemovedComponents(),
+				collectionFactory, out)
 			if err != nil {
 				return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
 			}
-
-			// Cleanup files belonging to a removed component by dropping the component directory from the storage backend.
-			for _, component := range result.RemovedComponents() {
-				err = publishedStorage.RemoveDirs(filepath.Join(prefix, "dists", distribution, component), out)
-				if err != nil {
-					return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
-				}
-			}
 		}
 
 		return &task.ProcessReturnValue{Code: http.StatusOK, Value: published}, nil
@@ -890,9 +880,6 @@ func apiPublishUpdate(c *gin.Context) {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
 		}
 
-		updatedComponents := result.UpdatedComponents()
-		removedComponents := result.RemovedComponents()
-
 		err = published.Publish(context.PackagePool(), context, collectionFactory, signer, out, b.ForceOverwrite)
 		if err != nil {
 			return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
@@ -904,22 +891,11 @@ func apiPublishUpdate(c *gin.Context) {
 		}
 
 		if b.SkipCleanup == nil || !*b.SkipCleanup {
-			publishedStorage := context.GetPublishedStorage(storage)
-
-			err = collection.CleanupPrefixComponentFiles(published.Prefix, updatedComponents, publishedStorage, collectionFactory, out)
+			err = collection.CleanupPrefixComponentFiles(context, published,
+				result.AddedComponents(), result.UpdatedComponents(), result.RemovedComponents(), collectionFactory, out)
 			if err != nil {
 				return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
 			}
-
-			if len(removedComponents) > 0 {
-				// Cleanup files belonging to a removed component by dropping the component directory from the storage backend.
-				for _, component := range removedComponents {
-					err = publishedStorage.RemoveDirs(filepath.Join(prefix, "dists", distribution, component), out)
-					if err != nil {
-						return &task.ProcessReturnValue{Code: http.StatusInternalServerError, Value: nil}, fmt.Errorf("unable to update: %s", err)
-					}
-				}
-			}
 		}
 
 		return &task.ProcessReturnValue{Code: http.StatusOK, Value: published}, nil

cmd/publish_switch.go

@@ -115,8 +115,8 @@ func aptlyPublishSwitch(cmd *commander.Command, args []string) error {
 
 	skipCleanup := context.Flags().Lookup("skip-cleanup").Value.Get().(bool)
 	if !skipCleanup {
-		err = collectionFactory.PublishedRepoCollection().CleanupPrefixComponentFiles(published.Prefix, components,
-			context.GetPublishedStorage(storage), collectionFactory, context.Progress())
+		err = collectionFactory.PublishedRepoCollection().CleanupPrefixComponentFiles(context, published,
+			[]string{}, components, []string{}, collectionFactory, context.Progress())
 		if err != nil {
 			return fmt.Errorf("unable to switch: %s", err)
 		}

cmd/publish_update.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"fmt"
-	"path/filepath"
 
 	"github.com/aptly-dev/aptly/deb"
 	"github.com/smira/commander"
@@ -77,20 +76,11 @@ func aptlyPublishUpdate(cmd *commander.Command, args []string) error {
 
 	skipCleanup := context.Flags().Lookup("skip-cleanup").Value.Get().(bool)
 	if !skipCleanup {
-		publishedStorage := context.GetPublishedStorage(storage)
-		err = collectionFactory.PublishedRepoCollection().CleanupPrefixComponentFiles(published.Prefix, result.UpdatedComponents(),
-			publishedStorage, collectionFactory, context.Progress())
+		err = collectionFactory.PublishedRepoCollection().CleanupPrefixComponentFiles(context, published,
+			result.AddedComponents(), result.UpdatedComponents(), result.RemovedComponents(), collectionFactory, context.Progress())
 		if err != nil {
 			return fmt.Errorf("unable to update: %s", err)
 		}
-
-		// Cleanup files belonging to a removed component by dropping the component directory from the storage backend.
-		for _, component := range result.RemovedComponents() {
-			err = publishedStorage.RemoveDirs(filepath.Join(prefix, "dists", distribution, component), context.Progress())
-			if err != nil {
-				return fmt.Errorf("unable to update: %s", err)
-			}
-		}
 	}
 
 	context.Progress().Printf("\nPublished %s repository %s has been successfully updated.\n", published.SourceKind, published.String())

deb/publish.go

@@ -1400,7 +1400,7 @@ func (collection *PublishedRepoCollection) listReferencedFilesByComponent(prefix
 	processedComponentRefs := map[string]*PackageRefList{}
 
 	for _, r := range collection.list {
-		if r.Prefix == prefix {
+		if r.Prefix == prefix && !r.MultiDist {
 			matches := false
 
 			repoComponents := r.Components()
@@ -1461,21 +1461,53 @@ func (collection *PublishedRepoCollection) listReferencedFilesByComponent(prefix
 }
 
 // CleanupPrefixComponentFiles removes all unreferenced files in published storage under prefix/component pair
-func (collection *PublishedRepoCollection) CleanupPrefixComponentFiles(prefix string, components []string,
-	publishedStorage aptly.PublishedStorage, collectionFactory *CollectionFactory, progress aptly.Progress) error {
+func (collection *PublishedRepoCollection) CleanupPrefixComponentFiles(publishedStorageProvider aptly.PublishedStorageProvider,
+	published *PublishedRepo, addedComponents, updatedComponents, removedComponents []string,
+	collectionFactory *CollectionFactory, progress aptly.Progress) error {
 
 	collection.loadList()
 
-	if progress != nil {
-		progress.Printf("Cleaning up prefix %#v components %s...\n", prefix, strings.Join(components, ", "))
+	prefix := published.Prefix
+	distribution := published.Distribution
+	multiDist := published.MultiDist
+	publishedStorage := publishedStorageProvider.GetPublishedStorage(published.Storage)
+
+	components := make([]string, 0, len(addedComponents)+len(updatedComponents)+len(removedComponents))
+	components = append(append(append(components, addedComponents...), updatedComponents...), removedComponents...)
+	sort.Strings(components)
+
+	for _, component := range removedComponents {
+		if progress != nil {
+			progress.Printf("Removing component %q from prefix %q...\n", component, prefix)
+		}
+
+		err := publishedStorage.RemoveDirs(filepath.Join(prefix, "dists", distribution, component), progress)
+		if err != nil {
+			return err
+		}
+
+		if multiDist {
+			for _, component := range removedComponents {
+				err = publishedStorage.RemoveDirs(filepath.Join(prefix, "pool", distribution, component), progress)
+				if err != nil {
+					return err
+				}
+			}
+		}
 	}
 
+	components = make([]string, 0, len(updatedComponents)+len(removedComponents))
+	components = append(append(components, addedComponents...), updatedComponents...)
+
 	referencedFiles, err := collection.listReferencedFilesByComponent(prefix, components, collectionFactory, progress)
 	if err != nil {
 		return err
 	}
 
 	for _, component := range components {
+		if progress != nil {
+			progress.Printf("Cleaning up component %q in prefix %q...\n", component, prefix)
+		}
 		sort.Strings(referencedFiles[component])
 
 		rootPath := filepath.Join(prefix, "pool", component)
@@ -1547,8 +1579,8 @@ func (collection *PublishedRepoCollection) Remove(publishedStorageProvider aptly
 		nil, collection.list[len(collection.list)-1], collection.list[:len(collection.list)-1]
 
 	if !skipCleanup && len(cleanComponents) > 0 {
-		err = collection.CleanupPrefixComponentFiles(repo.Prefix, cleanComponents,
-			publishedStorageProvider.GetPublishedStorage(storage), collectionFactory, progress)
+		err = collection.CleanupPrefixComponentFiles(publishedStorageProvider, repo, []string{}, cleanComponents, []string{},
+			collectionFactory, progress)
 		if err != nil {
 			if !force {
 				return fmt.Errorf("cleanup failed, use -force-drop to override: %s", err)