Modified ManifestReferences.DeletePackages() to return errors.

Previously VersionHandlerImpl.ManifestReferences.DeletePackages()
would only write errors to the log but not return failures to a
caller making it more difficult (requiring log redirection)
to handle failures.

Bug: 150471207
Change-Id: I8af5f5dafa00a6fd8713b31bf6739ee092ddfc4a

Author: Stewart Miles

source/VersionHandlerImpl/src/FileUtils.cs

@@ -393,25 +393,59 @@ public static string GetPackageDirectory(string path, bool actualPath) {
             return packageDir;
         }
 
+        /// <summary>
+        /// Result of RemoveAssets().
+        /// </summary>
+        public class RemoveAssetsResult {
+
+            /// <summary>
+            /// Assets that were removed.
+            /// </summary>
+            public List<string> Removed { get; set; }
+
+            /// <summary>
+            /// Assets that failed to be removed.
+            /// </summary>
+            public List<string> RemoveFailed { get; set; }
+
+            /// <summary>
+            /// Assets that were missing.
+            /// </summary>
+            public List<string> Missing { get; set; }
+
+            /// <summary>
+            /// Whether the operation was successful.
+            /// </summary>
+            public bool Success { get { return RemoveFailed.Count == 0 && Missing.Count == 0; } }
+
+            /// <summary>
+            /// Construct an empty result.
+            /// </summary>
+            public RemoveAssetsResult() {
+                Removed = new List<string>();
+                RemoveFailed = new List<string>();
+                Missing = new List<string>();
+            }
+        }
+
         /// <summary>
         /// Remove the given set of files and their folders.
         /// </summary>
         /// <param name = "filenames">Files to be removed/</param>
         /// <param name = "logger">Logger to log results.</param>
         /// <return>True if all files are removed.  False if failed to remove any file or
         /// if any file is missing.</return>
-        public static bool RemoveAssets(IEnumerable<string> filenames, Logger logger = null) {
-            List<string> assetRemoved = new List<string>();
-            List<string> assetRemoveFailed = new List<string>();
-            List<string> assetMissing = new List<string>();
+        public static RemoveAssetsResult RemoveAssets(IEnumerable<string> filenames,
+                                                      Logger logger = null) {
+            var result = new RemoveAssetsResult();
 
             HashSet<string> folderToRemove = new HashSet<string>();
             foreach (var filename in filenames) {
                 if (File.Exists(filename)) {
                     if (AssetDatabase.DeleteAsset(filename)) {
-                        assetRemoved.Add(filename);
+                        result.Removed.Add(filename);
                     } else {
-                        assetRemoveFailed.Add(filename);
+                        result.RemoveFailed.Add(filename);
                     }
 
                     // Add folder and parent folders to be removed later.
@@ -424,7 +458,7 @@ public static bool RemoveAssets(IEnumerable<string> filenames, Logger logger = n
                         folder = Path.GetDirectoryName(folder);
                     }
                 } else {
-                    assetMissing.Add(filename);
+                    result.Missing.Add(filename);
                 }
             }
 
@@ -448,6 +482,7 @@ public static bool RemoveAssets(IEnumerable<string> filenames, Logger logger = n
                     Directory.GetDirectories(folder).Length == 0) {
                     if (!AssetDatabase.DeleteAsset(folder)) {
                         folderRemoveFailed.Add(folder);
+                        result.RemoveFailed.Add(folder);
                     }
                 }
             }
@@ -456,12 +491,12 @@ public static bool RemoveAssets(IEnumerable<string> filenames, Logger logger = n
                 logger.Log(
                     String.Format("Removed:\n{0}\nFailed to Remove:\n{1}\nMissing:\n{2}\n" +
                         "Failed to Remove Folders:\n{3}\n",
-                        String.Join("\n", assetRemoved.ToArray()),
-                        String.Join("\n", assetRemoveFailed.ToArray()),
-                        String.Join("\n", assetMissing.ToArray()),
+                        String.Join("\n", result.Removed.ToArray()),
+                        String.Join("\n", result.RemoveFailed.ToArray()),
+                        String.Join("\n", result.Missing.ToArray()),
                         String.Join("\n", folderRemoveFailed.ToArray())), level : LogLevel.Verbose);
             }
-            return assetRemoveFailed.Count == 0 && assetMissing.Count == 0;
+            return result;
         }
    }
 }

source/VersionHandlerImpl/src/VersionHandlerImpl.cs

@@ -1826,11 +1826,14 @@ public static List<ManifestReferences> FindAndReadManifestsInAssetsFolder() {
         /// <summary>
         /// Delete a subset of packages managed by Version Handler.
         /// </summary>
-        /// <param name="packages">
-        /// A HashSet of canonical name of the packages to be uninstalled</param>
-        /// <param name="force">
-        /// Force to remove all file even it is referenced by other packages.</param>
-        public static void DeletePackages(HashSet<string> packages, bool force = false) {
+        /// <param name="packages">A HashSet of canonical name of the packages to be
+        /// uninstalled</param>
+        /// <param name="force">Force to remove all file even it is referenced by other
+        /// packages.</param>
+        /// <returns>If successful returns an empty collection, a collection of files that could
+        /// not be removed otherwise.</returns>
+        public static ICollection<string> DeletePackages(HashSet<string> packages,
+                                                         bool force = false) {
             // Create a map from canonical name to ManifestReferences.
             var manifestMap = new Dictionary<string, ManifestReferences>(
                 FindAndReadManifestsInAssetsFolderByPackageName());
@@ -1859,16 +1862,18 @@ public static void DeletePackages(HashSet<string> packages, bool force = false)
             VersionHandlerImpl.Log(String.Format("Uninstalling the following packages:\n{0}\n{1}",
                     String.Join("\n", (new List<string>(packages)).ToArray()),
                     filesToKeep.Count == 0 ? "" : String.Format(
-                            "Ignore the following files referenced by other packages:\n{0}\n",
+                            "Ignoring the following files referenced by other packages:\n{0}\n",
                             String.Join("\n", (new List<string>(filesToKeep)).ToArray()))));
 
-            if (FileUtils.RemoveAssets(filesToRemove, VersionHandlerImpl.Logger)) {
+            var result = FileUtils.RemoveAssets(filesToRemove, VersionHandlerImpl.Logger);
+            if (result.Success) {
                 VersionHandlerImpl.analytics.Report("uninstallpackage/delete/success",
                         "Successfully Deleted All Files in Packages");
             } else {
                 VersionHandlerImpl.analytics.Report("uninstallpackage/delete/fail",
                         "Fail to Delete Some Files in Packages");
             }
+            return result.RemoveFailed;
         }
 
         /// <summary>