Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent custom node package containing duplicate node definition from loading #9815

Merged
merged 12 commits into from
Jul 3, 2019
20 changes: 19 additions & 1 deletion src/DynamoCore/Core/CustomNodeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -455,6 +456,23 @@ private void SetNodeInfo(CustomNodeInfo newInfo)
NodeInfos.Remove(guid);
}

CustomNodeInfo info;
if (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);
}
Expand Down
1 change: 1 addition & 0 deletions src/DynamoCore/DynamoCore.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
28 changes: 28 additions & 0 deletions src/DynamoCore/Exceptions/CustomNodePackageLoadException.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
9 changes: 9 additions & 0 deletions src/DynamoCore/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/DynamoCore/Properties/Resources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -700,4 +700,7 @@ 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. It has one or more node definitions that already exist in {0} that 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>
3 changes: 3 additions & 0 deletions src/DynamoCore/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -700,4 +700,7 @@ 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. It has one or more node definitions that already exist in {0} that 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>
11 changes: 11 additions & 0 deletions src/DynamoCoreWpf/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/DynamoCoreWpf/Properties/Resources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2199,4 +2199,9 @@ 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. It has one or more node definitions that already exist in {0} that 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: {1}?</value>
</data>
</root>
5 changes: 5 additions & 0 deletions src/DynamoCoreWpf/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -2202,4 +2202,9 @@ 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. It has one or more node definitions that already exist in {0} that 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: {1}?</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -393,7 +401,6 @@ private void ShowNodePublishInfo(ICollection<Tuple<CustomNodeInfo, CustomNodeDef
{
foreach (var f in funcDefs)
{
var pmExtension = DynamoViewModel.Model.GetPackageManagerExtension();
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
var pkg = pmExtension.PackageLoader.GetOwnerPackage(f.Item1);

if (pkg != null)
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,10 @@ public PackageManagerSearchViewModel(PackageManagerClientViewModel client) : thi
{
this.PackageManagerClientViewModel = client;
PackageManagerClientViewModel.Downloads.CollectionChanged += DownloadsOnCollectionChanged;
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
PackageManagerClientViewModel.PackageManagerExtension.PackageLoader.ConflictingCustomNodePackageLoaded +=
ConflictingCustomNodePackageLoaded;
}

/// <summary>
/// Sort the search results
/// </summary>
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions src/DynamoPackages/PackageLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,26 @@ internal void TryLoadPackageIntoLibrary(Package package)
package.Loaded = true;
this.PackgeLoaded?.Invoke(package);
}
catch (CustomNodePackageLoadException e)
{
Package originalPackage =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@QilongTang what do you think - should we log this to analytics?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth! But are you thinking about public APIs for extention author to use or internal call only managed by our team?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am thinking about the regular log Analytics calls for events - I'm not sure if this would already be logged a custom exception type or we only log uncaught exceptions?

localPackages.FirstOrDefault(x => x.CustomNodeDirectory == e.InstalledPath);
OnConflictingPackageLoaded(originalPackage, package);
}
catch (Exception e)
{
Log("Exception when attempting to load package " + package.Name + " from " + package.RootDirectory);
Log(e.GetType() + ": " + e.Message);
}
}

public event Action<Package, Package> ConflictingCustomNodePackageLoaded;
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
private void OnConflictingPackageLoaded(Package installed, Package conflicting)
{
var handler = ConflictingCustomNodePackageLoaded;
handler?.Invoke(installed, conflicting);
}

/// <summary>
/// Load the package into Dynamo (including all node libraries and custom nodes)
/// and add to LocalPackages.
Expand Down