DYN-3457: Dynamo should alert the user if there are conflicts loading packages from different locations

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

@@ -735,4 +735,10 @@ Parameter name: {0}</value>
     <value>The current user, '{0}', is not a maintainer of the package '{1}'.</value>
     <comment>{0} = user name (i.e. 'DynamoTeam'), {1} = package name (i.e. 'Clockwork for Dynamo 1.x')</comment>
   </data>
+  <data name="DuplicatedOlderPackage" xml:space="preserve">
+    <value>An older version of the package called {0} version {2} was found at {1} with version {3}. Ignoring it.</value>
+  </data>
+  <data name="InvalidPackageVersion" xml:space="preserve">
+    <value>The version of the package called {0} found at {1} is invalid (version: "{2}"). Ignoring it.</value>
+  </data>
 </root>

src/DynamoCore/Properties/Resources.resx

@@ -735,4 +735,10 @@ Parameter name: {0}</value>
     <value>The current user, '{0}', is not a maintainer of the package '{1}'.</value>
     <comment>{0} = user name (i.e. 'DynamoTeam'), {1} = package name (i.e. 'Clockwork for Dynamo 1.x')</comment>
   </data>
+  <data name="DuplicatedOlderPackage" xml:space="preserve">
+    <value>An older version of the package called {0} version {2} was found at {1} with version {3}. Ignoring it.</value>
+  </data>
+  <data name="InvalidPackageVersion" xml:space="preserve">
+    <value>The version of the package called {0} found at {1} is invalid (version: "{2}"). Ignoring it.</value>
+  </data>
 </root>

src/DynamoPackages/PackageLoader.cs

@@ -401,13 +401,13 @@ public Package ScanPackageDirectory(string directory, bool checkCertificates)
             {
                 var headerPath = Path.Combine(directory, "pkg.json");
 
-                Package discoveredPkg;
+                Package discoveredPackage;
 
                 // get the package name and the installed version
                 if (PathHelper.IsValidPath(headerPath))
                 {
-                    discoveredPkg = Package.FromJson(headerPath, AsLogger());
-                    if (discoveredPkg == null)
+                    discoveredPackage = Package.FromJson(headerPath, AsLogger());
+                    if (discoveredPackage == null)
                         throw new LibraryLoadFailedException(directory, String.Format(Properties.Resources.MalformedHeaderPackage, headerPath));
                 }
                 else
@@ -418,16 +418,45 @@ public Package ScanPackageDirectory(string directory, bool checkCertificates)
                 // prevent loading unsigned packages if the certificates are required on package dlls
                 if (checkCertificates)
                 {
-                    CheckPackageNodeLibraryCertificates(directory, discoveredPkg);
+                    CheckPackageNodeLibraryCertificates(directory, discoveredPackage);
                 }
 
-                // prevent duplicates
-                if (LocalPackages.All(pkg => pkg.Name != discoveredPkg.Name))
+                var discoveredVersion = CheckAndGetPackageVersion(discoveredPackage.VersionName, discoveredPackage.Name, discoveredPackage.RootDirectory);
+
+                var existingPackage = LocalPackages.FirstOrDefault(package => package.Name == discoveredPackage.Name);
+
+                // Is this a new package?
+                if (existingPackage == null)
                 {
-                    Add(discoveredPkg);
-                    return discoveredPkg; // success
+                    // Yes
+                    Add(discoveredPackage);
+                    return discoveredPackage; // success
                 }
-                throw new LibraryLoadFailedException(directory, String.Format(Properties.Resources.DulicatedPackage, discoveredPkg.Name, discoveredPkg.RootDirectory));
+
+                var existingVersion = CheckAndGetPackageVersion(existingPackage.VersionName, existingPackage.Name, existingPackage.RootDirectory);
+
+                // Is this a duplicated package?
+                if (existingVersion == discoveredVersion)
+                {
+                    // Duplicated with the same version
+                    throw new LibraryLoadFailedException(directory, String.Format(Properties.Resources.DulicatedPackage, discoveredPackage.Name, discoveredPackage.RootDirectory));
+                } 
+
+                // Is the existing version newer?
+                if (existingVersion > discoveredVersion)
+                {
+                    // Newer version already exist
+                    Log(String.Format(Properties.Resources.DuplicatedOlderPackage, discoveredPackage.Name, discoveredPackage.RootDirectory, existingVersion.ToString(), discoveredVersion.ToString()), WarningLevel.Moderate);
+                    return null;
+                }
+
+                // Older version exist, replace with newer version
+                Remove(existingPackage);
+                Add(discoveredPackage);
+
+                Log(String.Format(Properties.Resources.DuplicatedOlderPackage, existingPackage.Name, existingPackage.RootDirectory, discoveredVersion.ToString(), existingVersion.ToString()), WarningLevel.Moderate);
+
+                return discoveredPackage;
             }
             catch (Exception e)
             {
@@ -438,6 +467,25 @@ public Package ScanPackageDirectory(string directory, bool checkCertificates)
             return null;
         }
 
+        /// <summary>
+        /// Check and get the version from the version string. Throw a library load exception if anything is wrong with the version
+        /// </summary>
+        /// <param name="version">the version string</param>
+        /// <param name="name">name of the package</param>
+        /// <param name="directory">package directory</param>
+        /// <returns>Returns a valid Version</returns>
+        private Version CheckAndGetPackageVersion(string version, string name, string directory)
+        {
+            try
+            {
+                return new Version(version);
+            }
+            catch (Exception e) when (e is ArgumentException || e is FormatException || e is OverflowException)
+            {
+                throw new LibraryLoadFailedException(directory, String.Format(Properties.Resources.InvalidPackageVersion, name, directory, version));
+            }
+        }
+
         /// <summary>
         /// Check the node libraries defined in the package json file are valid and have a valid certificate
         /// </summary>

test/Libraries/PackageManagerTests/PackageLoaderTests.cs

@@ -658,6 +658,98 @@ public void HasValidStandardLibraryPath()
             Assert.AreEqual(standardDirectory, directory);
         }
 
+        [Test]
+        public void PackageLoaderLoadPackageWithBadVersion()
+        {
+            // Arrange
+            var loader = GetPackageLoader();
+            var badPackageLocation = Path.Combine(PackagesDirectory, @"BadVersion\PackageWithBadVersion");
+
+            // Act
+            var badPackage = loader.ScanPackageDirectory(badPackageLocation);
+
+            // Assert
+            Assert.IsNull(badPackage);
+            Assert.IsNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Bad package"));
+        }
+
+        [Test]
+        public void PackageLoaderLoadMultiplePackagesWithBadVersion()
+        {
+            // Arrange
+            var loader = GetPackageLoader();
+            var goodPackageLocation = Path.Combine(PackagesDirectory, @"BadVersion\PackageWithGoodVersion");
+            var badPackageLocation = Path.Combine(PackagesDirectory, @"BadVersion\PackageWithBadVersion");
+
+            // Act
+            var goodPackage = loader.ScanPackageDirectory(goodPackageLocation);
+            var badPackage = loader.ScanPackageDirectory(badPackageLocation);
+
+            // Assert
+            Assert.IsNotNull(goodPackage);
+            Assert.IsNull(badPackage);
+            Assert.IsNotNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Good package"));
+            Assert.IsNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Bad package"));
+        }
+
+        [Test]
+        public void PackageLoaderLoadMultiplePackagesWithBadVersionReversed()
+        {
+            // Arrange
+            var loader = GetPackageLoader();
+            var goodPackageLocation = Path.Combine(PackagesDirectory, @"BadVersion\PackageWithGoodVersion");
+            var badPackageLocation = Path.Combine(PackagesDirectory, @"BadVersion\PackageWithBadVersion");
+
+            // Act
+            var badPackage = loader.ScanPackageDirectory(badPackageLocation);
+            var goodPackage = loader.ScanPackageDirectory(goodPackageLocation);
+
+            // Assert
+            Assert.IsNotNull(goodPackage);
+            Assert.IsNull(badPackage);
+            Assert.IsNotNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Good package"));
+            Assert.IsNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Bad package"));
+        }
+
+
+        [Test]
+        public void PackageLoaderLoadNewPackage()
+        {
+            // Arrange
+            var loader = GetPackageLoader();
+            var oldPackageLocation = Path.Combine(PackagesDirectory, @"Version\PackageWithOldVersion");
+            var newPackageLocation = Path.Combine(PackagesDirectory, @"Version\PackageWithNewVersion");
+
+            // Act
+            var oldPackage = loader.ScanPackageDirectory(oldPackageLocation);
+            var newPackage = loader.ScanPackageDirectory(newPackageLocation);
+
+            // Assert
+            Assert.IsNotNull(oldPackage);
+            Assert.IsNotNull(newPackage);
+            Assert.IsNotNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"New package"));
+            Assert.IsNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Old package"));
+        }
+
+        [Test]
+        public void PackageLoaderLoadOldPackage()
+        {
+            // Arrange
+            var loader = GetPackageLoader();
+            var oldPackageLocation = Path.Combine(PackagesDirectory, @"Version\PackageWithOldVersion");
+            var newPackageLocation = Path.Combine(PackagesDirectory, @"Version\PackageWithNewVersion");
+
+            // Act
+            var newPackage = loader.ScanPackageDirectory(newPackageLocation);
+            var oldPackage = loader.ScanPackageDirectory(oldPackageLocation);
+
+            // Assert
+            Assert.IsNull(oldPackage);
+            Assert.IsNotNull(newPackage);
+            Assert.IsNotNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"New package"));
+            Assert.IsNull(loader.LocalPackages.FirstOrDefault(package => package.Description == @"Old package"));
+        }
+
         [Test]
         public void IsUnderPackageControlIsCorrectForValidFunctionDefinition()
         {

test/pkgs/BadVersion/PackageWithBadVersion/pkg.json

@@ -0,0 +1 @@
+{"license":"","file_hash":null,"name":"Package","version":"","description":"Bad package","group":"","keywords":null,"dependencies":[],"contents":"","engine_version":"2.1.0.7840","engine":"dynamo","engine_metadata":"","site_url":"","repository_url":"","contains_binaries":true,"node_libraries":["Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"]}

test/pkgs/BadVersion/PackageWithGoodVersion/pkg.json

@@ -0,0 +1 @@
+{"license":"","file_hash":null,"name":"Package","version":"1.0.0","description":"Good package","group":"","keywords":null,"dependencies":[],"contents":"","engine_version":"2.1.0.7840","engine":"dynamo","engine_metadata":"","site_url":"","repository_url":"","contains_binaries":true,"node_libraries":["Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"]}

test/pkgs/Version/PackageWithNewVersion/pkg.json

@@ -0,0 +1 @@
+{"license":"","file_hash":null,"name":"Package","version":"2.0.0","description":"New package","group":"","keywords":null,"dependencies":[],"contents":"","engine_version":"2.1.0.7840","engine":"dynamo","engine_metadata":"","site_url":"","repository_url":"","contains_binaries":true,"node_libraries":["Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"]}

test/pkgs/Version/PackageWithOldVersion/pkg.json

@@ -0,0 +1 @@
+{"license":"","file_hash":null,"name":"Package","version":"1.0.0","description":"Old package","group":"","keywords":null,"dependencies":[],"contents":"","engine_version":"2.1.0.7840","engine":"dynamo","engine_metadata":"","site_url":"","repository_url":"","contains_binaries":true,"node_libraries":["Package, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null"]}