Fix retain package structure flow where nested directory at root level were lost (#14918)

* optimize

* comments and a test

* add subdir test and comments

* add child test

* test updates

Author: zeusongit

src/DynamoCoreWpf/ViewModels/PackageManager/PackageItemRootViewModel.cs

@@ -131,8 +131,14 @@ internal void AddChildren(PackageItemRootViewModel item)
             if (this.ChildItems.Contains(item)) return;
             this.ChildItems.Add(item);
         }
-
-        internal void AddChild(PackageItemRootViewModel elem)
+        /// <summary>
+        /// The methods is used for adding a child item to all the encountered parent folders in a nested path
+        /// and make sure all the intermediate file paths are created as separate PackageItemRootViewModel.
+        /// For example if we have a path like "\dir1\dir2\dir3" and we want to add a child item to "dir1", the method will
+        /// add "dir 3" to "dir2" and then "dir2" to "dir1".
+        /// </summary>
+        /// <param name="elem">Child item to be added.</param>
+        internal void AddChildRecursively(PackageItemRootViewModel elem)
         {
             if (elem.DependencyType.Equals(DependencyType.CustomNode)) return;
 
@@ -149,7 +155,7 @@ internal void AddChild(PackageItemRootViewModel elem)
 
             while (di.Parent != null)
             {
-                // if we already have a subfodler item with that name,
+                // if we already have a subfolder item with that name,
                 // add this element's children to its children instead of creating a new subfolder branch
                 if(existingSubFolders.Keys.Contains(elem.DirectoryName))
                 {

src/DynamoCoreWpf/ViewModels/PackageManager/PublishPackageViewModel.cs

@@ -893,6 +893,10 @@ public bool RetainFolderStructureOverride
                 }
             }
         }
+        /// <summary>
+        /// The root directory of the package
+        /// </summary>
+        private string CurrentPackageDirectory { get; set; }
         private static MetadataLoadContext sharedMetaDataLoadContext = null;
         /// <summary>
         /// A shared MetaDataLoadContext that is used for assembly inspection during package publishing.
@@ -996,7 +1000,7 @@ private void RefreshPackageContents()
 
             var items = new Dictionary<string, PackageItemRootViewModel>();
 
-            if(!String.IsNullOrEmpty(RootFolder))
+            if (!String.IsNullOrEmpty(RootFolder))
             {
                 var root = new PackageItemRootViewModel(RootFolder);
                 items[RootFolder] = root;
@@ -1022,7 +1026,7 @@ private void RefreshPackageContents()
                 }
             }
 
-            var updatedItems = BindParentToChild(items);   
+            var updatedItems = BindParentToChild(items);
 
             updatedItems.AddRange(itemsToAdd.Where(pa => pa.DependencyType.Equals(DependencyType.CustomNode)));
 
@@ -1050,8 +1054,6 @@ private bool IsDuplicateFile(PackageItemRootViewModel item1, PackageItemRootView
 
         private List<PackageItemRootViewModel> BindParentToChild(Dictionary<string, PackageItemRootViewModel> items)
         {
-            var updatedItems = new List<PackageItemRootViewModel>();
-
             foreach (var parent in items)
             {
                 foreach(var child in items)
@@ -1060,25 +1062,53 @@ private List<PackageItemRootViewModel> BindParentToChild(Dictionary<string, Pack
                     if (IsSubPathOfDeep(parent.Value, child.Value))
                     {
                         if (child.Value.isChild) continue; // if this was picked up already, don't add it again
-                        parent.Value.AddChild(child.Value);
+                        parent.Value.AddChildRecursively(child.Value);
                         child.Value.isChild = true;
                     }
                 }
             }
 
             // Only add the folder items, they contain the files
-            updatedItems = items.Values.Where(x => !x.isChild).ToList();
+            var updatedItems = GetRootItems(items);
             return updatedItems;
         }
 
+        /// <summary>
+        /// Gets the list PackageItemRootViewModel items which will be at the root directory of the package with all the child items.
+        /// </summary>
+        /// <param name="items"></param>
+        /// <returns></returns>
+        private List<PackageItemRootViewModel> GetRootItems(Dictionary<string, PackageItemRootViewModel> items)
+        {
+            var rootItems = items.Values.Where(x => !x.isChild).ToList();
+            if (!rootItems.Any()) return rootItems;
+            var packageSourceDir = CurrentPackageDirectory ??= GetLongestCommonPrefix(items.Keys.ToArray());
+
+            var root = new PackageItemRootViewModel(packageSourceDir);
+            var updatedItems = new List<PackageItemRootViewModel>();
+            //check each root item and create any missing connections
+            foreach (var item in rootItems)
+            {
+                var itemDir = new DirectoryInfo(item.DirectoryName);
+                if (!itemDir.Parent.FullName.Equals(packageSourceDir))
+                {
+                    root.AddChildRecursively(item);
+                }
+                else
+                {
+                    root.ChildItems.Add(item);
+                }
+            }
+            return root.ChildItems.ToList();
+        }   
+
         /// <summary>
         /// Test if path2 is subpath of path1
-        /// If it is, make sure all the intermediate file paths are created as separte PackageItemRootViewModel
         /// </summary>
         /// <param name="path1"></param>
         /// <param name="path2"></param>
         /// <returns></returns>
-        private bool IsSubPathOfDeep(PackageItemRootViewModel path1, PackageItemRootViewModel path2)
+        internal bool IsSubPathOfDeep(PackageItemRootViewModel path1, PackageItemRootViewModel path2)
         {
             var di1 = new DirectoryInfo(path1.DirectoryName);
             var di2 = new DirectoryInfo(path2.DirectoryName);
@@ -1089,12 +1119,37 @@ private bool IsSubPathOfDeep(PackageItemRootViewModel path1, PackageItemRootView
                 {
                     return true;
                 }
-                else di2 = di2.Parent;
+                else
+                {
+                    if (di2.Parent.FullName.Length < di1.FullName.Length) return false;
+                    di2 = di2.Parent;
+                }
             }
 
             return false;
         }
 
+        /// <summary>
+        /// Utility method to get the common file path, this may fail for files with the same partial name.
+        /// </summary>
+        /// <param name="s">A collection of filepaths</param>
+        /// <returns></returns>
+        internal string GetLongestCommonPrefix(string[] s)
+        {
+            int k = s[0].Length;
+            for (int i = 1; i < s.Length; i++)
+            {
+                k = Math.Min(k, s[i].Length);
+                for (int j = 0; j < k; j++)
+                    if (s[i][j] != s[0][j])
+                    {
+                        k = j;
+                        break;
+                    }
+            }
+            return Path.GetDirectoryName(s[0].Substring(0, k));
+        }
+
         /// <summary>
         /// Return a list of HostComboboxEntry describing known hosts from PM.
         /// Return an empty list if PM is down.
@@ -1403,6 +1458,7 @@ internal static PublishPackageViewModel FromLocalPackage(DynamoViewModel dynamoV
                 CopyrightHolder = pkg.CopyrightHolder,
                 CopyrightYear = pkg.CopyrightYear,
                 IsPublishFromLocalPackage = true,
+                CurrentPackageDirectory = pkg.RootDirectory,
                 //default retain folder structure to true when publishing a new version from local.
                 RetainFolderStructureOverride = retainFolderStructure
             };
@@ -1636,7 +1692,7 @@ internal IEnumerable<string> GetAllFiles()
             // union with additional files
             files = files.Union(AdditionalFiles);
             // if we retain the folder structure, we don't want to lose assemblies in sub-folders
-            // othrewise we need to delete duplicate assemblies which will end up in the same `dll` folder
+            // otherwise we need to delete duplicate assemblies which will end up in the same `dll` folder
             files = RetainFolderStructureOverride && !IsPublishFromLocalPackage ?
                 files.Union(Assemblies.Select(x => x.LocalFilePath)) :
                 files.Union(Assemblies.Select(x => x.Assembly.Location));  
@@ -2232,6 +2288,10 @@ private void PublishLocally()
                     var remapper = new CustomNodePathRemapper(DynamoViewModel.Model.CustomNodeManager,
                         DynamoModel.IsTestMode);
                     var builder = new PackageDirectoryBuilder(new MutatingFileSystem(), remapper);
+                    if (string.IsNullOrEmpty(Package.RootDirectory))
+                    {
+                        Package.RootDirectory = CurrentPackageDirectory;
+                    }
                     builder.BuildRetainDirectory(Package, publishPath, updatedFiles, MarkdownFiles);
                     UploadState = PackageUploadHandle.State.Uploaded;
                 }
@@ -2647,7 +2707,7 @@ internal PackageItemRootViewModel GetPreBuildRootItemViewModel(string publishPat
             var docItemPreview = new PackageItemRootViewModel(docDir) { isChild = true };
 
             var pkg = new PackageItemRootViewModel(new FileInfo(Path.Combine(rootDir, "pkg.json")));
-            rootItemPreview.AddChild(pkg);
+            rootItemPreview.AddChildRecursively(pkg);
 
             foreach (var file in files)
             {
@@ -2657,12 +2717,12 @@ internal PackageItemRootViewModel GetPreBuildRootItemViewModel(string publishPat
                 if (Path.GetDirectoryName(file).EndsWith(PackageDirectoryBuilder.DocumentationDirectoryName))
                 {
                     var doc = new PackageItemRootViewModel(new FileInfo(Path.Combine(docDir, fileName)));
-                    docItemPreview.AddChild(doc);
+                    docItemPreview.AddChildRecursively(doc);
                 }
                 else if (file.EndsWith(".dyf"))
                 {
                     var dyfPreview = new PackageItemRootViewModel(fileName, Path.Combine(dyfDir, fileName));
-                    dyfItemPreview.AddChild(dyfPreview);
+                    dyfItemPreview.AddChildRecursively(dyfPreview);
                 }
                 else if (file.EndsWith(".dll") || PackageDirectoryBuilder.IsXmlDocFile(file, files) || PackageDirectoryBuilder.IsDynamoCustomizationFile(file, files))
                 {
@@ -2677,27 +2737,27 @@ internal PackageItemRootViewModel GetPreBuildRootItemViewModel(string publishPat
                     else
                     {
                         var dll = new PackageItemRootViewModel(new FileInfo(Path.Combine(binDir, fileName)));
-                        binItemPreview.AddChild(dll);
+                        binItemPreview.AddChildRecursively(dll);
                     }
                 }
                 else
                 {
                     var extra = new PackageItemRootViewModel(new FileInfo(Path.Combine(extraDir, fileName)));
-                    extraItemPreview.AddChild(extra);
+                    extraItemPreview.AddChildRecursively(extra);
                 }
             }
 
             foreach(var docFile in MarkdownFiles)
             {
                 var fileName = Path.GetFileName(docFile);
                 var doc = new PackageItemRootViewModel(new FileInfo(Path.Combine(docDir, fileName)));
-                docItemPreview.AddChild(doc);
+                docItemPreview.AddChildRecursively(doc);
             }
 
-            rootItemPreview.AddChild(dyfItemPreview);
-            rootItemPreview.AddChild(binItemPreview);
-            rootItemPreview.AddChild(extraItemPreview);
-            rootItemPreview.AddChild(docItemPreview);
+            rootItemPreview.AddChildRecursively(dyfItemPreview);
+            rootItemPreview.AddChildRecursively(binItemPreview);
+            rootItemPreview.AddChildRecursively(extraItemPreview);
+            rootItemPreview.AddChildRecursively(docItemPreview);
 
             return rootItemPreview;
         }

src/DynamoPackages/PackageDirectoryBuilder.cs

@@ -69,12 +69,13 @@ public IDirectoryInfo BuildRetainDirectory(Package package, string packagesDirec
 
             var rootPath = Path.Combine(packagesDirectory, package.Name);
             var rootDir = fileSystem.TryCreateDirectory(rootPath);
+            var sourcePackageDir = package.RootDirectory;
             package.RootDirectory = rootDir.FullName;
 
             var dyfFiles = new List<string>();
 
             RemoveUnselectedFiles(contentFiles.SelectMany(files => files).ToList(), rootDir);
-            CopyFilesIntoRetainedPackageDirectory(contentFiles, markdownFiles, rootDir, out dyfFiles);
+            CopyFilesIntoRetainedPackageDirectory(contentFiles, markdownFiles, sourcePackageDir, rootDir, out dyfFiles);
             RemoveRetainDyfFiles(contentFiles.SelectMany(files => files).ToList(), dyfFiles);  
 
             RemapRetainCustomNodeFilePaths(contentFiles.SelectMany(files => files).ToList(), dyfFiles);
@@ -212,19 +213,12 @@ private void WritePackageHeader(Package package, IDirectoryInfo rootDir)
             fileSystem.WriteAllText(headerPath, pkgHeaderStr);
         }
 
-        internal void CopyFilesIntoRetainedPackageDirectory(IEnumerable<IEnumerable<string>> contentFiles, IEnumerable<string> markdownFiles, IDirectoryInfo rootDir, out List<string> dyfFiles)
+        internal void CopyFilesIntoRetainedPackageDirectory(IEnumerable<IEnumerable<string>> contentFiles, IEnumerable<string> markdownFiles, string sourcePackageDir, IDirectoryInfo rootDir, out List<string> dyfFiles)
         {
             dyfFiles = new List<string>();
 
             foreach (var files in contentFiles)
             {
-                // We expect that files are bundled in root folders
-                // For single files, just get its folder
-                var commonPath = files.Count() > 1 ? GetLongestCommonPrefix(files.ToArray()) : Path.GetDirectoryName(files.FirstOrDefault());
-                commonPath = commonPath.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
-                var commonRootPath = Path.GetDirectoryName(commonPath);
-                if (commonRootPath == null) commonRootPath = commonPath; // already at the root
-
                 foreach (var file in files.Where(x => x != null))
                 {
                     // If the file doesn't actually exist, don't copy it
@@ -233,11 +227,12 @@ internal void CopyFilesIntoRetainedPackageDirectory(IEnumerable<IEnumerable<stri
                         continue;
                     }
 
-                    var relativePath = file.Substring(commonRootPath.Length);
+                    var relativePath = file.Substring(sourcePackageDir.Length);
 
                     // Ensure the relative path starts with a directory separator.
                     if (!string.IsNullOrEmpty(relativePath) && relativePath[0] != Path.DirectorySeparatorChar)
                     {
+                        relativePath = relativePath.TrimStart(new char[] { '/', '\\' });
                         relativePath = Path.DirectorySeparatorChar + relativePath;
                     }
 
@@ -387,29 +382,6 @@ public static string NormalizePath(string path)
                        .TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)
                        .ToUpperInvariant();
         }
-
-
-        /// <summary>
-        /// Utility method to get the common file path, this may fail for files with the same partial name.
-        /// </summary>
-        /// <param name="s">A collection of filepaths</param>
-        /// <returns></returns>
-        public static string GetLongestCommonPrefix(string[] s)
-        {
-            int k = s[0].Length;
-            for (int i = 1; i < s.Length; i++)
-            {
-                k = Math.Min(k, s[i].Length);
-                for (int j = 0; j < k; j++)
-                    if (s[i][j] != s[0][j])
-                    {
-                        k = j;
-                        break;
-                    }
-            }
-            return Path.GetDirectoryName(s[0].Substring(0, k));
-        }
-
         #endregion
 
     }

test/DynamoCoreWpfTests/PackageManager/PackageManagerUITests.cs

@@ -1829,7 +1829,7 @@ public void AddsFilesAndFoldersFromFilePathsCorrectly()
             Assert.AreEqual(additionalFiles.Count, allFiles.Count(f => !f.EndsWith(".dll")));
 
             var packageContents = vm.PackageContents;
-            Assert.AreEqual(packageContents.Count, 1); // We expect only 1 root item here
+            Assert.AreEqual(1, packageContents.Count); // We expect only 1 root item here
 
             // Assert that the PackageContents contains the correct number of items
             var allFilesAndFoldres = PackageItemRootViewModel.GetFiles(packageContents.First());
@@ -2162,7 +2162,7 @@ public void CanRemoveAllDependencyTypes()
             // This makes sense as we don't want to try to establish 'common parent' for folders that maybe too far apart in a tree structure
             rootFolder = vm.PackageContents.Where(x => x.DependencyType.Equals(DependencyType.Folder));
             Assert.AreEqual(1, rootFolder.Count());
-            Assert.AreEqual(3, PackageItemRootViewModel.GetFiles(rootFolder.First()).Count());
+            Assert.AreEqual(4, PackageItemRootViewModel.GetFiles(rootFolder.First()).Count());
 
             Assert.DoesNotThrow(() => vm.RemoveItemCommand.Execute(rootFolder.First()));
             Assert.IsFalse(vm.PackageContents.Any());
@@ -2272,6 +2272,46 @@ public void AssertPreviewPackageRetainFolderStructureEqualsPublishLocalPackageRe
             Directory.Delete(publishPath, true);
         }
 
+        [Test]
+        public void AssertPreviewPackageRetainFolderStructureEqualsPublishLocalPackageResultsForNestedFolders()
+        {
+            var packageName = "NestedPackage";
+            var pathManager = this.ViewModel.Model.PathManager as PathManager;
+            var publishPath = Path.Combine(pathManager.DefaultPackagesDirectory, packageName);
+
+            string nodePath = Path.Combine(TestDirectory, "pkgs", packageName);
+            var allFiles = Directory.GetFiles(nodePath, "*", SearchOption.AllDirectories).ToList();
+            var allFolders = Directory.GetDirectories(nodePath, "*", SearchOption.AllDirectories).ToList();
+
+            //now lets publish this package.
+            var newPkgVm = new PublishPackageViewModel(this.ViewModel);
+            newPkgVm.RetainFolderStructureOverride = true;
+            newPkgVm.AddAllFilesAfterSelection(allFiles);
+
+            var previewFilesAndFolders = PackageItemRootViewModel.GetFiles(newPkgVm.PreviewPackageContents.ToList());
+            var previewFiles = previewFilesAndFolders.Where(x => !x.DependencyType.Equals(DependencyType.Folder));
+            var previewFolders = previewFilesAndFolders.Where(x => x.DependencyType.Equals(DependencyType.Folder));
+
+            newPkgVm.Name = packageName;
+            newPkgVm.MajorVersion = "0";
+            newPkgVm.MinorVersion = "0";
+            newPkgVm.BuildVersion = "1";
+            newPkgVm.PublishLocallyCommand.Execute();
+
+            Assert.IsTrue(Directory.Exists(publishPath));
+
+            // Arrange
+            var createdFiles = Directory.GetFiles(publishPath, "*", SearchOption.AllDirectories).ToList();
+            var createdFolders = Directory.GetDirectories(publishPath, "*", SearchOption.AllDirectories).ToList();
+
+            // Assert
+            Assert.AreEqual(createdFiles.Count(), previewFiles.Count() + 1);
+            Assert.AreEqual(1, createdFolders.Count(), previewFolders.Count());  // One subfolder was created
+
+            // Clean up
+            Directory.Delete(publishPath, true);
+        }
+
         [Test]
         public void AssertPublishLocalHandleType()
         {