DYN-7587 : Clicking No still installs a conflicting package

src/DynamoCore/Core/CustomNodeManager.cs

@@ -596,6 +596,44 @@ public IEnumerable<CustomNodeInfo> AddUninitializedCustomNodesInPath(string path
             return result;
         }
 
+        /// <summary>
+        /// Determines whether custom nodes under <paramref name="customNodeDirectory"/> would conflict
+        /// with definitions already registered from a different package (the case that throws
+        /// <see cref="CustomNodePackageLoadException"/> in <see cref="SetNodeInfo"/>).
+        /// </summary>
+        /// <returns>True if any <c>.dyf</c> in the directory shares a function id with an existing
+        /// package member from another package name.</returns>
+        internal bool TryGetConflictingPackageCustomNodeInfo(
+            string customNodeDirectory,
+            bool isTestMode,
+            PackageInfo incomingPackageInfo,
+            out CustomNodeInfo conflictingExistingInfo)
+        {
+            conflictingExistingInfo = null;
+            if (string.IsNullOrEmpty(customNodeDirectory) || !Directory.Exists(customNodeDirectory) || incomingPackageInfo == null)
+                return false;
+
+            foreach (var file in Directory.EnumerateFiles(customNodeDirectory, "*.dyf"))
+            {
+                CustomNodeInfo newInfo;
+                if (!TryGetInfoFromPath(file, isTestMode, out newInfo))
+                    continue;
+
+                CustomNodeInfo existingInfo;
+                if (!NodeInfos.TryGetValue(newInfo.FunctionId, out existingInfo))
+                    continue;
+
+                if (existingInfo.IsPackageMember
+                    && existingInfo.PackageInfo != null
+                    && incomingPackageInfo.Name != existingInfo.PackageInfo.Name)
+                {
+                    conflictingExistingInfo = existingInfo;
+                    return true;
+                }
+            }
+            return false;
+        }
+
         /// <summary>
         ///     Enumerates all of the files in the search path and get's their guids.
         ///     Does not instantiate the nodes.

src/DynamoCoreWpf/Properties/Resources.en-US.resx

@@ -1342,6 +1342,12 @@ You can always redownload the package.</value>
   <data name="MessageInvalidPackage" xml:space="preserve">
     <value>Failed to load an invalid package.</value>
   </data>
+  <data name="MessagePackageInstallCancelledDueToCustomNodeConflict" xml:space="preserve">
+    <value>Installation cancelled due to custom node conflict.</value>
+  </data>
+  <data name="MessagePackageInstallRestartToCompleteCustomNodeReplace" xml:space="preserve">
+    <value>Restart Dynamo to remove the conflicting package.</value>
+  </data>
   <data name="MessageFailedToFindNodeById" xml:space="preserve">
     <value>No node could be found with that Id.</value>
   </data>

src/DynamoCoreWpf/Properties/Resources.resx

@@ -979,6 +979,12 @@ You can always redownload the package.</value>
   <data name="MessageInvalidPackage" xml:space="preserve">
     <value>Failed to load an invalid package.</value>
   </data>
+  <data name="MessagePackageInstallCancelledDueToCustomNodeConflict" xml:space="preserve">
+    <value>Installation cancelled due to custom node conflict.</value>
+  </data>
+  <data name="MessagePackageInstallRestartToCompleteCustomNodeReplace" xml:space="preserve">
+    <value>Restart Dynamo to remove the conflicting package.</value>
+  </data>
   <data name="MessageFailedToDelete" xml:space="preserve">
     <value>{0} failed to delete the package.  You may need to delete the package's root directory manually.</value>
   </data>

src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs

@@ -1143,16 +1143,61 @@ internal virtual void InstallPackage(PackageDownloadHandle packageDownloadHandle
         /// <param name="downloadPath">package download path</param>
         internal void SetPackageState(PackageDownloadHandle packageDownloadHandle, string downloadPath)
         {
-            Package dynPkg;
-            if (packageDownloadHandle.Extract(DynamoViewModel.Model, downloadPath, out dynPkg))
+            string stagingDirectory = null;
+            try
             {
+                if (!packageDownloadHandle.TryPrepareInstallation(DynamoViewModel.Model, out var dynPkg, out stagingDirectory))
+                {
+                    packageDownloadHandle.Error(Resources.MessageInvalidPackage);
+                    return;
+                }
+
+                var packagesRoot = string.IsNullOrEmpty(downloadPath)
+                    ? DynamoViewModel.Model.PathManager.DefaultPackagesDirectory
+                    : downloadPath;
+
+                if (Directory.Exists(dynPkg.CustomNodeDirectory))
+                {
+                    var incomingInfo = new PackageInfo(dynPkg.Name, new Version(dynPkg.VersionName));
+                    if (DynamoViewModel.Model.CustomNodeManager.TryGetConflictingPackageCustomNodeInfo(
+                            dynPkg.CustomNodeDirectory,
+                            DynamoModel.IsTestMode,
+                            incomingInfo,
+                            out var conflictingExisting))
+                    {
+                        var existingDyfDir = Path.GetDirectoryName(conflictingExisting.Path);
+                        var installedPkg = PackageManagerExtension.PackageLoader.LocalPackages.FirstOrDefault(
+                            p => string.Equals(p.CustomNodeDirectory, existingDyfDir, StringComparison.OrdinalIgnoreCase));
+
+                        if (installedPkg == null)
+                        {
+                            packageDownloadHandle.Error(Resources.MessagePackageInstallCancelledDueToCustomNodeConflict);
+                            return;
+                        }
+
+                        var userChoseReplace =
+                            PackageManagerExtension.PackageLoader.OnConflictingPackageLoaded(installedPkg, dynPkg);
+
+                        if (!userChoseReplace)
+                        {
+                            packageDownloadHandle.Error(Resources.MessagePackageInstallCancelledDueToCustomNodeConflict);
+                            return;
+                        }
+
+                        PackageDownloadHandle.CompleteInstallation(dynPkg, stagingDirectory, packagesRoot);
+                        packageDownloadHandle.DownloadState = PackageDownloadHandle.State.Installed;
+                        packageDownloadHandle.ErrorString = Resources.MessagePackageInstallRestartToCompleteCustomNodeReplace;
+                        return;
+                    }
+                }
+
+                PackageDownloadHandle.CompleteInstallation(dynPkg, stagingDirectory, packagesRoot);
                 PackageManagerExtension.PackageLoader.LoadPackages(new List<Package> { dynPkg });
                 packageDownloadHandle.DownloadState = PackageDownloadHandle.State.Installed;
             }
-            else
+            finally
             {
-                packageDownloadHandle.DownloadState = PackageDownloadHandle.State.Error;
-                packageDownloadHandle.Error(Resources.MessageInvalidPackage);
+                PackageDownloadHandle.DiscardStagingDirectory(stagingDirectory, DynamoViewModel.Model.Logger);
             }
         }
 

src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs

@@ -1486,8 +1486,11 @@ public static string FormatPackageVersionList(IEnumerable<Tuple<PackageHeader, P
             return String.Join("\r\n", packages.Select(x => x.Item1.name + " " + x.Item2.version));
         }
 
-        private void ConflictingCustomNodePackageLoaded(Package installed, Package conflicting)
+        private bool ConflictingCustomNodePackageLoaded(Package installed, Package conflicting)
         {
+            if (installed == null || conflicting == null)
+                return false;
+
             var message = string.Format(Resources.MessageUninstallCustomNodeToContinue,
                 installed.Name + " " + installed.VersionName, conflicting.Name + " " + conflicting.VersionName);
 
@@ -1500,7 +1503,10 @@ private void ConflictingCustomNodePackageLoaded(Package installed, Package confl
                 // mark for uninstallation
                 var settings = PackageManagerClientViewModel.DynamoViewModel.Model.PreferenceSettings;
                 installed.MarkForUninstall(settings);
+                return true;
             }
+
+            return false;
         }
 
         private void DownloadsOnCollectionChanged(object sender, NotifyCollectionChangedEventArgs args)
@@ -1633,15 +1639,15 @@ internal void RegisterTransientHandlers()
         {
             SearchResults.CollectionChanged += SearchResultsOnCollectionChanged;
             PackageManagerClientViewModel.Downloads.CollectionChanged += DownloadsOnCollectionChanged;
-            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageLoaded +=
+            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageResolutionCallback +=
                 ConflictingCustomNodePackageLoaded;
         }
 
         internal void UnregisterTransientHandlers()
         {
             SearchResults.CollectionChanged -= SearchResultsOnCollectionChanged;
             PackageManagerClientViewModel.Downloads.CollectionChanged -= DownloadsOnCollectionChanged;
-            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageLoaded -=
+            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageResolutionCallback -=
                 ConflictingCustomNodePackageLoaded;
         }
 

src/DynamoPackages/PackageDownloadHandle.cs

@@ -1,6 +1,7 @@
 using System;
 using System.IO;
 using Dynamo.Core;
+using Dynamo.Logging;
 using Dynamo.Models;
 
 using Greg.Responses;
@@ -97,49 +98,105 @@ private static string BuildInstallDirectoryString(string packagesDirectory, stri
         }
 
         /// <summary>
-        /// Extracts and parses the metadata of a downloaded package
+        /// Unzips the downloaded package to a staging directory and parses <c>pkg.json</c>.
         /// </summary>
-        /// <param name="dynamoModel">Dynamo model</param>
-        /// <param name="installDirectory">If specified, overrides Dynamo's default base folder for packages</param>
-        /// <param name="pkg">Metatda parsed from the package</param>
-        /// <returns>Whether the operation succeeded or not</returns>
-        public bool Extract(DynamoModel dynamoModel, string installDirectory, out Package pkg)
+        internal bool TryPrepareInstallation(DynamoModel dynamoModel, out Package pkg, out string stagingDirectory)
         {
+            pkg = null;
+            stagingDirectory = null;
             this.DownloadState = State.Installing;
 
-            // unzip, place files
             var unzipPath = Greg.Utility.FileUtilities.UnZip(DownloadPath);
             if (!Directory.Exists(unzipPath))
             {
                 throw new Exception(Properties.Resources.PackageEmpty);
             }
 
-            // provide handle to installed package 
+            stagingDirectory = unzipPath;
             pkg = Package.FromDirectory(unzipPath, dynamoModel.Logger);
+            return pkg != null;
+        }
 
+        /// <summary>
+        /// Copies a staged package into the Dynamo packages directory and sets <paramref name="pkg"/>.RootDirectory.
+        /// </summary>
+        /// <param name="pkg">Package metadata (initially rooted at the staging folder).</param>
+        /// <param name="stagingDirectory">Path returned from <see cref="TryPrepareInstallation"/>.</param>
+        /// <param name="packagesRootDirectory">Root packages folder (e.g. default or custom package path).</param>
+        internal static void CompleteInstallation(Package pkg, string stagingDirectory, string packagesRootDirectory)
+        {
             if (pkg == null)
             {
-                return false;
+                throw new ArgumentNullException(nameof(pkg));
+            }
+            if (string.IsNullOrEmpty(stagingDirectory))
+            {
+                throw new ArgumentException("Staging directory is required.", nameof(stagingDirectory));
             }
 
-            if (String.IsNullOrEmpty(installDirectory))
-                installDirectory = dynamoModel.PathManager.DefaultPackagesDirectory;
-
-            var installedPath = BuildInstallDirectoryString(installDirectory, pkg.Name);
+            var installedPath = BuildInstallDirectoryString(packagesRootDirectory, pkg.Name);
             Directory.CreateDirectory(installedPath);
 
-            // Now create all of the directories
-            foreach (string dirPath in Directory.GetDirectories(unzipPath, "*", SearchOption.AllDirectories))
-                Directory.CreateDirectory(dirPath.Replace(unzipPath, installedPath));
+            foreach (string dirPath in Directory.GetDirectories(stagingDirectory, "*", SearchOption.AllDirectories))
+            {
+                Directory.CreateDirectory(dirPath.Replace(stagingDirectory, installedPath));
+            }
 
-            // Copy all the files
-            foreach (string newPath in Directory.GetFiles(unzipPath, "*.*", SearchOption.AllDirectories))
-                File.Copy(newPath, newPath.Replace(unzipPath, installedPath));
+            foreach (string newPath in Directory.GetFiles(stagingDirectory, "*.*", SearchOption.AllDirectories))
+            {
+                File.Copy(newPath, newPath.Replace(stagingDirectory, installedPath));
+            }
 
-            // Update root directory to final path
             pkg.RootDirectory = installedPath;
+        }
 
-            return true;
+        /// <summary>
+        /// Deletes a staging directory created by <see cref="TryPrepareInstallation"/>.
+        /// </summary>
+        internal static void DiscardStagingDirectory(string stagingDirectory, ILogger logger)
+        {
+            if (string.IsNullOrEmpty(stagingDirectory) || !Directory.Exists(stagingDirectory))
+                return;
+
+            try
+            {
+                Directory.Delete(stagingDirectory, true);
+            }
+            catch (IOException ex)
+            {
+                logger?.Log($"Failed to delete package directory {stagingDirectory}. {ex.Message}");
+            }
+            catch (UnauthorizedAccessException ex)
+            {
+                logger?.Log($"Failed to delete package directory {stagingDirectory}. {ex.Message}");
+            }
+        }
+
+        /// <summary>
+        /// Extracts and parses the metadata of a downloaded package
+        /// </summary>
+        /// <param name="dynamoModel">Dynamo model</param>
+        /// <param name="installDirectory">If specified, overrides Dynamo's default base folder for packages</param>
+        /// <param name="pkg">Metadata parsed from the package</param>
+        /// <returns>Whether the operation succeeded or not</returns>
+        public bool Extract(DynamoModel dynamoModel, string installDirectory, out Package pkg)
+        {
+            string stagingDirectory = null;
+            try
+            {
+                if (!TryPrepareInstallation(dynamoModel, out pkg, out stagingDirectory))
+                    return false;
+
+                if (String.IsNullOrEmpty(installDirectory))
+                    installDirectory = dynamoModel.PathManager.DefaultPackagesDirectory;
+
+                CompleteInstallation(pkg, stagingDirectory, installDirectory);
+                return true;
+            }
+            finally
+            {
+                DiscardStagingDirectory(stagingDirectory, dynamoModel.Logger);
+            }
         }
 
         // cancel, install, redownload

src/DynamoPackages/PackageLoader.cs

@@ -309,10 +309,25 @@ private void TryLoadPackageIntoLibrary(Package package)
         /// with an existing package is tried to load.
         /// </summary>
         public event Action<Package, Package> ConflictingCustomNodePackageLoaded;
-        private void OnConflictingPackageLoaded(Package installed, Package conflicting)
+
+        /// <summary>
+        /// Resolver wchich runs instead of only firing <see cref="ConflictingCustomNodePackageLoaded"/>
+        /// </summary>
+        public Func<Package, Package, bool> ConflictingCustomNodePackageResolutionCallback { get; set;  }
+
+        /// <summary>
+        /// Invokes conflict resolution / notification. Does not change package load state.
+        /// </summary>
+        /// <returns>True if the user accepts replacing the installed package after restart.</returns>
+        internal bool OnConflictingPackageLoaded(Package installed, Package conflicting)
         {
-            var handler = ConflictingCustomNodePackageLoaded;
-            handler?.Invoke(installed, conflicting);
+            if (ConflictingCustomNodePackageResolutionCallback != null)
+            {
+                return ConflictingCustomNodePackageResolutionCallback(installed, conflicting);
+            }
+
+            ConflictingCustomNodePackageLoaded?.Invoke(installed, conflicting);
+            return false;
         }
 
         /// <summary>