Prevent custom node package containing duplicate node definition from…

… loading (#9815)

* prevent duplicate custom node package from loading

* rename

* rename

* code fixes

* code cleanup

* update wording of custom node package fail dialog

* address review comments

* add test custom node packages with test

* fix failing tests

* fix test

src/DynamoCore/Core/CustomNodeManager.cs

@@ -6,6 +6,7 @@
 using System.Xml;
 using Dynamo.Engine;
 using Dynamo.Engine.NodeToCode;
+using Dynamo.Exceptions;
 using Dynamo.Graph;
 using Dynamo.Graph.Annotations;
 using Dynamo.Graph.Connectors;
@@ -119,7 +120,7 @@ protected virtual void OnInfoUpdated(CustomNodeInfo obj)
             var handler = InfoUpdated;
             if (handler != null) handler(obj);
         }
-
+        
         /// <summary>
         ///     An event that is fired when a custom node is removed from Dynamo.
         /// </summary>
@@ -341,7 +342,7 @@ public bool AddUninitializedCustomNode(string file, bool isTestMode, out CustomN
         {
             if (TryGetInfoFromPath(file, isTestMode, out info))
             {
-                SetNodeInfo(info);
+                SetNodeInfo(info, isTestMode);
                 return true;
             }
             return false;
@@ -397,7 +398,7 @@ public IEnumerable<CustomNodeInfo> AddUninitializedCustomNodesInPath(string path
             {
                 info.IsPackageMember = isPackageMember;
 
-                SetNodeInfo(info);
+                SetNodeInfo(info, isTestMode);
                 result.Add(info);
             }
             return result;
@@ -442,7 +443,7 @@ private IEnumerable<CustomNodeInfo> ScanNodeHeadersInDirectory(string dir, bool
         /// Stores the path and function definition without initializing a node.  Overwrites
         /// the existing NodeInfo if necessary
         /// </summary>
-        private void SetNodeInfo(CustomNodeInfo newInfo)
+        private void SetNodeInfo(CustomNodeInfo newInfo, bool isTestMode)
         {
             var guids = NodeInfos.Where(x =>
                         {
@@ -455,6 +456,23 @@ private void SetNodeInfo(CustomNodeInfo newInfo)
                 NodeInfos.Remove(guid);
             }
 
+            CustomNodeInfo info;
+            if (!isTestMode && NodeInfos.TryGetValue(newInfo.FunctionId, out info))
+            {
+                var newInfoPath = Path.GetDirectoryName(newInfo.Path);
+                var infoPath = Path.GetDirectoryName(info.Path);
+                var message = string.Format(Resources.MessageCustomNodePackageFailedToLoad,
+                    infoPath, newInfoPath);
+
+                var ex = new CustomNodePackageLoadException(newInfoPath, infoPath, message);
+                Log(ex.Message, WarningLevel.Moderate);
+
+                // Log to notification view extension
+                Log(ex);
+
+                throw ex;
+            }
+
             NodeInfos[newInfo.FunctionId] = newInfo;
             OnInfoUpdated(newInfo);
         }
@@ -645,7 +663,7 @@ public bool OpenCustomNodeWorkspace(
             if (InitializeCustomNode(
                 workspaceInfo,
                 xmlDoc,
-                out customNodeWorkspace))
+                out customNodeWorkspace, isTestMode))
             {
                 workspace = customNodeWorkspace;
                 return true;
@@ -657,7 +675,7 @@ public bool OpenCustomNodeWorkspace(
         private bool InitializeCustomNode(
             WorkspaceInfo workspaceInfo,
             XmlDocument xmlDoc,
-            out CustomNodeWorkspaceModel workspace)
+            out CustomNodeWorkspaceModel workspace, bool isTestMode)
         {
             // Add custom node definition firstly so that a recursive
             // custom node won't recursively load itself.
@@ -691,21 +709,21 @@ private bool InitializeCustomNode(
                 }
             }
 
-            RegisterCustomNodeWorkspace(newWorkspace);
+            RegisterCustomNodeWorkspace(newWorkspace, isTestMode);
             workspace = newWorkspace;
             return true;
         }
 
-        private void RegisterCustomNodeWorkspace(CustomNodeWorkspaceModel newWorkspace)
+        private void RegisterCustomNodeWorkspace(CustomNodeWorkspaceModel newWorkspace, bool isTestMode = false)
         {
             RegisterCustomNodeWorkspace(
                 newWorkspace,
                 newWorkspace.CustomNodeInfo,
-                newWorkspace.CustomNodeDefinition);
+                newWorkspace.CustomNodeDefinition, isTestMode);
         }
 
         private void RegisterCustomNodeWorkspace(
-            CustomNodeWorkspaceModel newWorkspace, CustomNodeInfo info, CustomNodeDefinition definition)
+            CustomNodeWorkspaceModel newWorkspace, CustomNodeInfo info, CustomNodeDefinition definition, bool isTestMode)
         {
             loadedWorkspaceModels[newWorkspace.CustomNodeId] = newWorkspace;
             SetFunctionDefinition(definition);
@@ -717,12 +735,12 @@ private void RegisterCustomNodeWorkspace(
                 OnDefinitionUpdated(newDef);
             };
 
-            SetNodeInfo(info);
+            SetNodeInfo(info, isTestMode);
 
             newWorkspace.InfoChanged += () =>
             {
                 var newInfo = newWorkspace.CustomNodeInfo;
-                SetNodeInfo(newInfo);
+                SetNodeInfo(newInfo, isTestMode);
                 OnInfoUpdated(newInfo);
             };
 
@@ -764,7 +782,7 @@ private bool InitializeCustomNode(
                         info.ID = functionId.ToString();
                         if (migrationManager.ProcessWorkspace(info, xmlDoc, isTestMode, nodeFactory))
                         {
-                            return InitializeCustomNode(info, xmlDoc, out workspace);
+                            return InitializeCustomNode(info, xmlDoc, out workspace, isTestMode);
                         }
                     }
                 }
@@ -773,7 +791,7 @@ private bool InitializeCustomNode(
                     // TODO: Skip Json migration for now
                     WorkspaceInfo.FromJsonDocument(strInput, path, isTestMode, false, AsLogger(), out info);
                     info.ID = functionId.ToString();
-                    return InitializeCustomNode(info, null, out workspace);
+                    return InitializeCustomNode(info, null, out workspace, isTestMode);
                 }
                 else throw ex;
                 Log(string.Format(Properties.Resources.CustomNodeCouldNotBeInitialized, customNodeInfo.Name));
@@ -803,7 +821,7 @@ private bool InitializeCustomNode(
         ///     Optional identifier to be used for the custom node. By default, will make a new unique one.
         /// </param>
         /// <returns>Newly created Custom Node Workspace.</returns>
-        internal WorkspaceModel CreateCustomNode(string name, string category, string description, Guid? functionId = null)
+        internal WorkspaceModel CreateCustomNode(string name, string category, string description, Guid? functionId = null, bool isTestMode = false)
         {
             var newId = functionId ?? Guid.NewGuid();
 
@@ -820,7 +838,7 @@ internal WorkspaceModel CreateCustomNode(string name, string category, string de
             };
             var workspace = new CustomNodeWorkspaceModel(info, nodeFactory);
 
-            RegisterCustomNodeWorkspace(workspace);
+            RegisterCustomNodeWorkspace(workspace, isTestMode);
             return workspace;
         }
 
@@ -1246,7 +1264,7 @@ from output in outputs
 
                 newWorkspace.HasUnsavedChanges = true;
 
-                RegisterCustomNodeWorkspace(newWorkspace);
+                RegisterCustomNodeWorkspace(newWorkspace, isTestMode);
 
                 Debug.WriteLine("Collapsed workspace has {0} nodes and {1} connectors",
                     newWorkspace.Nodes.Count(), newWorkspace.Connectors.Count());

src/DynamoCore/DynamoCore.csproj

@@ -121,6 +121,7 @@ limitations under the License.
     <Compile Include="Engine\Profiling\ProfilingData.cs" />
     <Compile Include="Engine\Profiling\ProfilingSession.cs" />
     <Compile Include="Engine\ShortestQualifiedNameReplacer.cs" />
+    <Compile Include="Exceptions\CustomNodePackageLoadException.cs" />
     <Compile Include="Exceptions\LibraryLoadFailedException.cs" />
     <Compile Include="Graph\Workspaces\PackageDependencyInfo.cs" />
     <Compile Include="Graph\Nodes\NodeOutputData.cs" />

src/DynamoCore/Exceptions/CustomNodePackageLoadException.cs

@@ -0,0 +1,28 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace Dynamo.Exceptions
+{
+    public class CustomNodePackageLoadException : LibraryLoadFailedException
+    {
+        /// <summary>
+        /// File path of existing custom node package.
+        /// </summary>
+        public string InstalledPath { get; }
+
+        /// <summary>
+        /// Constructor.
+        /// </summary>
+        /// <param name="path">File path of failing custom node package.</param>
+        /// <param name="installedPath">File path of existing package.</param>
+        /// <param name="reason">Failure reason.</param>
+        public CustomNodePackageLoadException(string path, string installedPath, string reason)
+            : base(path, reason)
+        {
+            InstalledPath = installedPath;
+        }
+    }
+}

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

@@ -700,4 +700,10 @@ The input name should be a valid variable name, without spaces. An input type an
   <data name="PythonTemplateUserFile" xml:space="preserve">
     <value>Python template loaded from DynamoSettings.xml path</value>
   </data>
+  <data name="MessageCustomNodePackageFailedToLoad" xml:space="preserve">
+    <value>{1} cannot be loaded. 
+Installing it will conflict with one or more node definitions that already exist in {0}, which is currently loaded. 
+To install {1}, Dynamo needs to first uninstall {0}. 
+Restart Dynamo to complete the uninstall, then try and download {1} again.</value>
+  </data>
 </root>

src/DynamoCore/Properties/Resources.resx

@@ -700,4 +700,10 @@ The input name should be a valid variable name, without spaces. An input type an
   <data name="PythonTemplateDefinedByHost" xml:space="preserve">
     <value>Python template set by host integrator</value>
   </data>
+  <data name="MessageCustomNodePackageFailedToLoad" xml:space="preserve">
+    <value>{1} cannot be loaded. 
+Installing it will conflict with one or more node definitions that already exist in {0}, which is currently loaded. 
+To install {1}, Dynamo needs to first uninstall {0}. 
+Restart Dynamo to complete the uninstall, then try and download {1} again.</value>
+  </data>
 </root>

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

@@ -2199,6 +2199,14 @@ Do you want to install the latest Dynamo update?</value>
   <data name="ExtensionAlreadyPresent" xml:space="preserve">
     <value>No new tab is added, as the extension is already present in the extensions side bar.</value>
   </data>
+  <data name="MessageUninstallCustomNodeToContinue" xml:space="preserve">
+    <value>{1} cannot be loaded.
+Installing it will conflict with one or more node definitions that already exist in {0}, which is currently loaded. 
+To install {1}, Dynamo needs to first uninstall {0}. 
+Restart Dynamo to complete the uninstall, then try and download {1} again.
+
+Uninstall the following packages: {0}?</value>
+  </data>
   <data name="ProvideFeedbackButton" xml:space="preserve">
     <value> Provide Feedback</value>
   </data>

src/DynamoCoreWpf/Properties/Resources.resx

@@ -2202,6 +2202,14 @@ Want to publish a different package?</value>
   <data name="ExtensionAlreadyPresent" xml:space="preserve">
     <value>No new tab is added, as the extension is already present in the extensions side bar.</value>
   </data>
+  <data name="MessageUninstallCustomNodeToContinue" xml:space="preserve">
+    <value>{1} cannot be loaded.
+Installing it will conflict with one or more node definitions that already exist in {0}, which is currently loaded. 
+To install {1}, Dynamo needs to first uninstall {0}. 
+Restart Dynamo to complete the uninstall, then try and download {1} again.
+
+Uninstall the following packages: {0}?</value>
+  </data>
   <data name="ProvideFeedbackButton" xml:space="preserve">
     <value> Provide Feedback</value>
   </data>

src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerClientViewModel.cs

@@ -10,6 +10,7 @@
 using System.Windows;
 using System.Windows.Input;
 using Dynamo.Core;
+using Dynamo.Exceptions;
 using Dynamo.Graph.Nodes.CustomNodes;
 using Dynamo.Graph.Workspaces;
 using Dynamo.Models;
@@ -183,6 +184,13 @@ public ObservableCollection<PackageDownloadHandle> Downloads
             set { _downloads = value; }
         }
 
+        private PackageManagerExtension pmExtension;
+
+        internal PackageManagerExtension PackageManagerExtension
+        {
+            get { return pmExtension ?? (pmExtension = DynamoViewModel.Model.GetPackageManagerExtension()); }
+        }
+
         public List<PackageManagerSearchElement> CachedPackageList { get; private set; }
 
         public readonly DynamoViewModel DynamoViewModel;
@@ -393,7 +401,6 @@ private void ShowNodePublishInfo(ICollection<Tuple<CustomNodeInfo, CustomNodeDef
         {
             foreach (var f in funcDefs)
             {
-                var pmExtension = DynamoViewModel.Model.GetPackageManagerExtension();
                 var pkg = pmExtension.PackageLoader.GetOwnerPackage(f.Item1);
 
                 if (pkg != null)
@@ -490,6 +497,7 @@ internal void DownloadAndInstall(PackageDownloadHandle packageDownloadHandle, st
                         if (packageDownloadHandle.Extract(DynamoViewModel.Model, downloadPath, out dynPkg))
                         {
                             var p = Package.FromDirectory(dynPkg.RootDirectory, DynamoViewModel.Model.Logger);
+
                             pmExtension.PackageLoader.LoadPackages(new List<Package> {p});
 
                             packageDownloadHandle.DownloadState = PackageDownloadHandle.State.Installed;

src/DynamoCoreWpf/ViewModels/PackageManager/PackageManagerSearchViewModel.cs

@@ -263,8 +263,10 @@ public PackageManagerSearchViewModel(PackageManagerClientViewModel client) : thi
         {
             this.PackageManagerClientViewModel = client;
             PackageManagerClientViewModel.Downloads.CollectionChanged += DownloadsOnCollectionChanged;
+            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageLoaded += 
+                ConflictingCustomNodePackageLoaded;
         }
-
+        
         /// <summary>
         /// Sort the search results
         /// </summary>
@@ -639,6 +641,23 @@ 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)
+        {
+            var message = string.Format(Resources.MessageUninstallCustomNodeToContinue,
+                installed.Name + " " + installed.VersionName, conflicting.Name + " " + conflicting.VersionName);
+
+            var dialogResult = MessageBox.Show(message,
+                Resources.CannotDownloadPackageMessageBoxTitle,
+                MessageBoxButton.YesNo, MessageBoxImage.Error);
+
+            if (dialogResult == MessageBoxResult.Yes)
+            {
+                // mark for uninstallation
+                var settings = PackageManagerClientViewModel.DynamoViewModel.Model.PreferenceSettings;
+                installed.MarkForUninstall(settings);
+            }
+        }
+
         private void DownloadsOnCollectionChanged(object sender, NotifyCollectionChangedEventArgs args)
         {
             if (args.NewItems != null)
@@ -724,8 +743,17 @@ public void SelectPrevious()
             SelectedIndex = SelectedIndex - 1;
         }
 
+        internal void UnregisterHandlers()
+        {
+            RequestShowFileDialog -= OnRequestShowFileDialog;
+            SearchResults.CollectionChanged -= SearchResultsOnCollectionChanged;
+            PackageManagerClientViewModel.Downloads.CollectionChanged -= DownloadsOnCollectionChanged;
+            PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageLoaded -=
+                ConflictingCustomNodePackageLoaded;
+        }
+
         /// <summary>
-        ///     Performs a search using the internal SearcText as the query and
+        ///     Performs a search using the internal SearchText as the query and
         ///     updates the observable SearchResults property.
         /// </summary>
         internal void SearchAndUpdateResults()