diff --git a/src/DynamoCoreWpf/DynamoCoreWpf.csproj b/src/DynamoCoreWpf/DynamoCoreWpf.csproj index 58b9ea68aaa..f2321cdf926 100644 --- a/src/DynamoCoreWpf/DynamoCoreWpf.csproj +++ b/src/DynamoCoreWpf/DynamoCoreWpf.csproj @@ -148,6 +148,7 @@ + ..\..\extern\Microsoft.Xaml.Behaviors\$(TargetFramework)\Dynamo.Microsoft.Xaml.Behaviors.dll diff --git a/src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs b/src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs index b45a5779253..d7fb5d0462c 100644 --- a/src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs +++ b/src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs @@ -25,6 +25,7 @@ using String = System.String; using NotificationObject = Dynamo.Core.NotificationObject; using Prism.Commands; +using System.Runtime.InteropServices; namespace Dynamo.PackageManager { @@ -738,6 +739,22 @@ public string CurrentWarningMessage } } + private static MetadataLoadContext sharedMetaDataLoadContext = null; + /// + /// A shared MetaDataLoadContext that is used for assembly inspection during package publishing. + /// This member is shared so the behavior is similar to the ReflectionOnlyLoadContext this is replacing. + /// TODO - eventually it would be good to move to separate publish load contexts that are cleaned up at the appropriate time(?). + /// + private static MetadataLoadContext SharedPublishLoadContext + { + get + { + sharedMetaDataLoadContext ??= InitSharedPublishLoadContext(); + return sharedMetaDataLoadContext; + } + } + + #endregion internal PublishPackageViewModel() @@ -865,11 +882,11 @@ private void ThisPropertyChanged(object sender, PropertyChangedEventArgs e) } } - public static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoViewModel, Package l) + public static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoViewModel, Package pkg) { var defs = new List(); - foreach (var x in l.LoadedCustomNodes) + foreach (var x in pkg.LoadedCustomNodes) { CustomNodeDefinition def; if (dynamoViewModel.Model.CustomNodeManager.TryGetFunctionDefinition( @@ -881,44 +898,43 @@ public static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoVie } } - var vm = new PublishPackageViewModel(dynamoViewModel) + var pkgViewModel = new PublishPackageViewModel(dynamoViewModel) { - Group = l.Group, - Description = l.Description, - Keywords = l.Keywords != null ? String.Join(" ", l.Keywords) : "", + Group = pkg.Group, + Description = pkg.Description, + Keywords = pkg.Keywords != null ? String.Join(" ", pkg.Keywords) : "", CustomNodeDefinitions = defs, - Name = l.Name, - RepositoryUrl = l.RepositoryUrl ?? "", - SiteUrl = l.SiteUrl ?? "", - Package = l, - License = l.License, - SelectedHosts = l.HostDependencies as List, - CopyrightHolder = l.CopyrightHolder, - CopyrightYear = l.CopyrightYear + Name = pkg.Name, + RepositoryUrl = pkg.RepositoryUrl ?? "", + SiteUrl = pkg.SiteUrl ?? "", + Package = pkg, + License = pkg.License, + SelectedHosts = pkg.HostDependencies as List, + CopyrightHolder = pkg.CopyrightHolder, + CopyrightYear = pkg.CopyrightYear }; // add additional files - l.EnumerateAdditionalFiles(); - foreach (var file in l.AdditionalFiles) + pkg.EnumerateAdditionalFiles(); + foreach (var file in pkg.AdditionalFiles) { - vm.AdditionalFiles.Add(file.Model.FullName); + pkgViewModel.AdditionalFiles.Add(file.Model.FullName); } - var nodeLibraryNames = l.Header.node_libraries; + var nodeLibraryNames = pkg.Header.node_libraries; var assembliesLoadedTwice = new List(); - // load assemblies into reflection only context - foreach (var file in l.EnumerateAssemblyFilesInBinDirectory()) + foreach (var file in pkg.EnumerateAssemblyFilesInPackage()) { Assembly assem; - var result = PackageLoader.TryReflectionOnlyLoadFrom(file, out assem); + var result = PackageLoader.TryMetaDataContextLoad(file, SharedPublishLoadContext, out assem); switch (result) { case AssemblyLoadingState.Success: { var isNodeLibrary = nodeLibraryNames == null || nodeLibraryNames.Contains(assem.FullName); - vm.Assemblies.Add(new PackageAssembly() + pkgViewModel.Assemblies.Add(new PackageAssembly() { IsNodeLibrary = isNodeLibrary, Assembly = assem @@ -928,7 +944,7 @@ public static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoVie case AssemblyLoadingState.NotManagedAssembly: { // if it's not a .NET assembly, we load it as an additional file - vm.AdditionalFiles.Add(file); + pkgViewModel.AdditionalFiles.Add(file); break; } case AssemblyLoadingState.AlreadyLoaded: @@ -940,24 +956,24 @@ public static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoVie } //after dependencies are loaded refresh package contents - vm.RefreshPackageContents(); - vm.UpdateDependencies(); + pkgViewModel.RefreshPackageContents(); + pkgViewModel.UpdateDependencies(); if (assembliesLoadedTwice.Any()) { - vm.UploadState = PackageUploadHandle.State.Error; - vm.ErrorString = Resources.OneAssemblyWasLoadedSeveralTimesErrorMessage + string.Join("\n", assembliesLoadedTwice); + pkgViewModel.UploadState = PackageUploadHandle.State.Error; + pkgViewModel.ErrorString = Resources.OneAssemblyWasLoadedSeveralTimesErrorMessage + string.Join("\n", assembliesLoadedTwice); } - if (l.VersionName == null) return vm; + if (pkg.VersionName == null) return pkgViewModel; - var parts = l.VersionName.Split('.'); - if (parts.Count() != 3) return vm; + var parts = pkg.VersionName.Split('.'); + if (parts.Count() != 3) return pkgViewModel; - vm.MajorVersion = parts[0]; - vm.MinorVersion = parts[1]; - vm.BuildVersion = parts[2]; - return vm; + pkgViewModel.MajorVersion = parts[0]; + pkgViewModel.MinorVersion = parts[1]; + pkgViewModel.BuildVersion = parts[2]; + return pkgViewModel; } @@ -1868,5 +1884,14 @@ internal void EnableInvalidNameWarningState(string warningMessage) CurrentWarningMessage = warningMessage; IsWarningEnabled = true; } + + private static MetadataLoadContext InitSharedPublishLoadContext() + { + // Retrieve the location of the assembly and the referenced assemblies used by the domain + var runtimeAssemblies = Directory.GetFiles(RuntimeEnvironment.GetRuntimeDirectory(), "*.dll"); + // Create PathAssemblyResolver that can resolve assemblies using the created list. + var resolver = new PathAssemblyResolver(runtimeAssemblies); + return new MetadataLoadContext(resolver); + } } } diff --git a/src/DynamoPackages/DynamoPackages.csproj b/src/DynamoPackages/DynamoPackages.csproj index ef52d420fb4..eb2b313c6eb 100644 --- a/src/DynamoPackages/DynamoPackages.csproj +++ b/src/DynamoPackages/DynamoPackages.csproj @@ -33,6 +33,7 @@ + diff --git a/src/DynamoPackages/Package.cs b/src/DynamoPackages/Package.cs index f6b1f6b9d61..54b6f27a6a2 100644 --- a/src/DynamoPackages/Package.cs +++ b/src/DynamoPackages/Package.cs @@ -327,7 +327,8 @@ public void EnumerateAdditionalFiles() AdditionalFiles.AddRange(nonDyfDllFiles); } - public IEnumerable EnumerateAssemblyFilesInBinDirectory() + //TODO can we make this internal? + public IEnumerable EnumerateAssemblyFilesInPackage() { if (String.IsNullOrEmpty(RootDirectory) || !Directory.Exists(RootDirectory)) return new List(); diff --git a/src/DynamoPackages/PackageLoader.cs b/src/DynamoPackages/PackageLoader.cs index 9dcd9d60e7f..6e28e734ae7 100644 --- a/src/DynamoPackages/PackageLoader.cs +++ b/src/DynamoPackages/PackageLoader.cs @@ -743,28 +743,38 @@ private void CheckPackageNodeLibraryCertificates(string packageDirectoryPath, Pa } /// - /// Attempt to load a managed assembly in to ReflectionOnlyLoadFrom context. + /// Attempt to load a managed assembly in to MetaDataLoad context. /// /// The filename of a DLL + /// The MetaDataLoadContext to load the package assemblies into for inspection. /// out Assembly - the passed value does not matter and will only be set if loading succeeds /// Returns Success if success, NotManagedAssembly if BadImageFormatException, AlreadyLoaded if FileLoadException - internal static AssemblyLoadingState TryReflectionOnlyLoadFrom(string filename, out Assembly assem) + internal static AssemblyLoadingState TryMetaDataContextLoad(string filename,MetadataLoadContext mlc, out Assembly assem) { - try - { - assem = Assembly.ReflectionOnlyLoadFrom(filename); - return AssemblyLoadingState.Success; - } - catch (BadImageFormatException) - { - assem = null; - return AssemblyLoadingState.NotManagedAssembly; - } - catch (FileLoadException) - { - assem = null; - return AssemblyLoadingState.AlreadyLoaded; - } + try + { + var mlcAssemblies = mlc.GetAssemblies(); + assem = mlc.LoadFromAssemblyPath(filename); + var mlcAssemblies2 = mlc.GetAssemblies(); + //if loading the assembly did not actually add a new assembly to the MLC + //then we've loaded it already, and our current behavior is to + //disable publish when a package contains the same assembly twice. + if (mlcAssemblies2.Count() == mlcAssemblies.Count()) + { + throw new FileLoadException(filename); + } + return AssemblyLoadingState.Success; + } + catch (BadImageFormatException) + { + assem = null; + return AssemblyLoadingState.NotManagedAssembly; + } + catch (FileLoadException) + { + assem = null; + return AssemblyLoadingState.AlreadyLoaded; + } } /// diff --git a/test/DynamoCoreWpfTests/PackagePathTests.cs b/test/DynamoCoreWpfTests/PackagePathTests.cs index d3c5f1aadfa..3c2f4fcb444 100644 --- a/test/DynamoCoreWpfTests/PackagePathTests.cs +++ b/test/DynamoCoreWpfTests/PackagePathTests.cs @@ -1,4 +1,4 @@ - + using System.IO; using System.Linq; using System.Reflection; @@ -233,17 +233,17 @@ public void InstalledPackagesContainsCorrectNumberOfPackages() vm.packageLoader.PackagesLoaded += pkgsLoadedDelegate; vm.packageLoader.LoadAll(vm.loadPackageParams); - Assert.AreEqual(19, vm.packageLoader.LocalPackages.Count()); + Assert.AreEqual(20, vm.packageLoader.LocalPackages.Count()); Assert.AreEqual(true, packagesLoaded); var installedPackagesViewModel = new InstalledPackagesViewModel(ViewModel, vm.packageLoader); - Assert.AreEqual(19, installedPackagesViewModel.LocalPackages.Count); + Assert.AreEqual(20, installedPackagesViewModel.LocalPackages.Count); var installedPackagesView = new Dynamo.Wpf.Controls.InstalledPackagesControl(); installedPackagesView.DataContext = installedPackagesViewModel; DispatcherUtil.DoEvents(); - Assert.AreEqual(19, installedPackagesView.SearchResultsListBox.Items.Count); + Assert.AreEqual(20, installedPackagesView.SearchResultsListBox.Items.Count); Assert.AreEqual(2, installedPackagesView.Filters.Items.Count); vm.packageLoader.PackagesLoaded -= libraryLoader.LoadPackages; @@ -267,7 +267,7 @@ public void RemoveAddPackagePathChangesInstalledPackageState() // Load packages in package path. vm.packageLoader.LoadAll(vm.loadPackageParams); - Assert.AreEqual(19, vm.packageLoader.LocalPackages.Count()); + Assert.AreEqual(20, vm.packageLoader.LocalPackages.Count()); // Remove package path. vm.DeletePathCommand.Execute(0); @@ -323,7 +323,7 @@ public void EnableCustomPackagePathsLoadsPackagesOnClosingPreferences() vm.SaveSettingCommand.Execute(null); // packages are expected to load from 'PackagesDirectory' above when toggle is turned off - Assert.AreEqual(19, vm.packageLoader.LocalPackages.Count()); + Assert.AreEqual(20, vm.packageLoader.LocalPackages.Count()); vm.packageLoader.PackagesLoaded -= libraryLoader.LoadPackages; vm.packageLoader.RequestLoadNodeLibrary -= libraryLoader.LoadLibraryAndSuppressZTSearchImport; diff --git a/test/DynamoCoreWpfTests/PublishPackageViewModelTests.cs b/test/DynamoCoreWpfTests/PublishPackageViewModelTests.cs index a40615af684..aacabea0ebe 100644 --- a/test/DynamoCoreWpfTests/PublishPackageViewModelTests.cs +++ b/test/DynamoCoreWpfTests/PublishPackageViewModelTests.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Reflection; using Dynamo; using Dynamo.Graph.Nodes.CustomNodes; using Dynamo.Graph.Workspaces; @@ -17,7 +18,7 @@ namespace DynamoCoreWpfTests public class PublishPackageViewModelTests: DynamoViewModelUnitTest { - [Test, Category("Failure")] + [Test] public void AddingDyfRaisesCanExecuteChangeOnDelegateCommand() { @@ -99,7 +100,34 @@ public void CanPublishLateInitializedJsonCustomNode() } - [Test, Category("Failure")] + [Test] + public void NewPackageDoesNotThrow_NativeBinaryIsAddedAsAdditionalFile_NotBinary() + { + string packagesDirectory = Path.Combine(TestDirectory, "pkgs"); + + var pathManager = new Mock(); + pathManager.SetupGet(x => x.PackagesDirectories).Returns(() => new List { packagesDirectory }); + + var loader = new PackageLoader(pathManager.Object); + loader.LoadAll(new LoadPackageParams + { + Preferences = ViewModel.Model.PreferenceSettings + }); + + PublishPackageViewModel vm = null; + var package = loader.LocalPackages.FirstOrDefault(x => x.Name == "package with native assembly"); + Assert.DoesNotThrow(() => + { + vm = PublishPackageViewModel.FromLocalPackage(ViewModel, package); + }); + + Assert.AreEqual(1, vm.AdditionalFiles.Count); + Assert.AreEqual(0, vm.Assemblies.Count); + + Assert.AreEqual(PackageUploadHandle.State.Ready, vm.UploadState); + } + + [Test] public void NewPackageVersionUpload_DoesNotThrowExceptionWhenDLLIsLoadedSeveralTimes() { string packagesDirectory = Path.Combine(TestDirectory, "pkgs"); @@ -123,7 +151,7 @@ public void NewPackageVersionUpload_DoesNotThrowExceptionWhenDLLIsLoadedSeveralT Assert.AreEqual(PackageUploadHandle.State.Error, vm.UploadState); } - [Test, Category("Failure")] + [Test] public void NewPackageVersionUpload_CanAddAndRemoveFiles() { string packagesDirectory = Path.Combine(TestDirectory, "pkgs"); diff --git a/test/Libraries/PackageManagerTests/PackageLoaderTests.cs b/test/Libraries/PackageManagerTests/PackageLoaderTests.cs index 46144791c91..153497466f1 100644 --- a/test/Libraries/PackageManagerTests/PackageLoaderTests.cs +++ b/test/Libraries/PackageManagerTests/PackageLoaderTests.cs @@ -141,7 +141,7 @@ public void PackageDoesNotReloadOnAbsenceOfNewPackagePath() }; loader.LoadAll(loadPackageParams); - Assert.AreEqual(19, loader.LocalPackages.Count()); + Assert.AreEqual(20, loader.LocalPackages.Count()); Assert.AreEqual(true, packagesLoaded); var entries = CurrentDynamoModel.SearchModel.Entries.ToList(); @@ -150,7 +150,7 @@ public void PackageDoesNotReloadOnAbsenceOfNewPackagePath() packagesLoaded = false; // This function is called upon addition of new package paths in the UI. loader.LoadNewCustomNodesAndPackages(new List(), CurrentDynamoModel.CustomNodeManager); - Assert.AreEqual(19, loader.LocalPackages.Count()); + Assert.AreEqual(20, loader.LocalPackages.Count()); // Assert packages are not reloaded if there are no new package paths. Assert.False(packagesLoaded); @@ -187,7 +187,7 @@ public void NoPackageNodeDuplicatesOnAddingNewPackagePath() Preferences =settings, }; loader.LoadAll(loadPackageParams); - Assert.AreEqual(19, loader.LocalPackages.Count()); + Assert.AreEqual(20, loader.LocalPackages.Count()); Assert.AreEqual(true, packagesLoaded); var entries = CurrentDynamoModel.SearchModel.Entries.ToList(); @@ -198,7 +198,7 @@ public void NoPackageNodeDuplicatesOnAddingNewPackagePath() var newPaths = new List { Path.Combine(TestDirectory, "builtinpackages testdir") }; // This function is called upon addition of new package paths in the UI. loader.LoadNewCustomNodesAndPackages(newPaths, CurrentDynamoModel.CustomNodeManager); - Assert.AreEqual(20, loader.LocalPackages.Count()); + Assert.AreEqual(21, loader.LocalPackages.Count()); // Assert packages are reloaded if there are new package paths. Assert.True(packagesLoaded); @@ -503,8 +503,8 @@ public void LoadPackagesReturnsAllValidPackagesInValidDirectory() Preferences = CurrentDynamoModel.PreferenceSettings, }); - // There are 19 packages in "Dynamo\test\pkgs" - Assert.AreEqual(19, loader.LocalPackages.Count()); + // There are 20 packages in "Dynamo\test\pkgs" + Assert.AreEqual(20, loader.LocalPackages.Count()); // Verify that interdependent packages are resolved successfully // TODO: Review these assertions. Lambdas are not using x, so they are basically just checking that test files exist. @@ -541,8 +541,8 @@ public void LoadingPackageDoesNotAffectLoadedSearchEntries() Preferences = CurrentDynamoModel.PreferenceSettings, }); - // There are 19 packages in "Dynamo\test\pkgs" - Assert.AreEqual(19, loader.LocalPackages.Count()); + // There are 20 packages in "Dynamo\test\pkgs" + Assert.AreEqual(20, loader.LocalPackages.Count()); // Simulate loading new package from PM string packageDirectory = Path.Combine(TestDirectory, @"core\packageDependencyTests\ZTTestPackage"); @@ -669,8 +669,8 @@ public void LoadingConflictingCustomNodePackageDoesNotGetLoaded() Preferences = CurrentDynamoModel.PreferenceSettings, }); - // There are 19 packages in "Dynamo\test\pkgs" - Assert.AreEqual(19, loader.LocalPackages.Count()); + // There are 20 packages in "Dynamo\test\pkgs" + Assert.AreEqual(20, loader.LocalPackages.Count()); var entries = CurrentDynamoModel.SearchModel.Entries.OfType(); diff --git a/test/pkgs/package with native assembly/bin/nativeassembly.dll b/test/pkgs/package with native assembly/bin/nativeassembly.dll new file mode 100644 index 00000000000..f37cbf13b69 Binary files /dev/null and b/test/pkgs/package with native assembly/bin/nativeassembly.dll differ diff --git a/test/pkgs/package with native assembly/pkg.json b/test/pkgs/package with native assembly/pkg.json new file mode 100644 index 00000000000..42c66b92bc5 --- /dev/null +++ b/test/pkgs/package with native assembly/pkg.json @@ -0,0 +1 @@ +{"license":"","file_hash":null,"name":"package with native assembly","version":"1.0.0","description":"package","group":"","keywords":null,"dependencies":[],"contents":"","engine_version":"2.1.0.7733","engine":"dynamo","engine_metadata":"","site_url":"","repository_url":"","contains_binaries":true,"node_libraries":[]} \ No newline at end of file