Improve matching for ZIP with multiple nested installers (#529)

src/WingetCreateCore/Common/PackageParser.cs

@@ -216,6 +216,10 @@ public static void UpdateInstallerNodesAsync(List<InstallerMetadata> installerMe
                 {
                     parseFailedInstallerUrls.Add(installerMetadata.InstallerUrl);
                 }
+
+                // In case of multiple nested installers in the archive, we expect the new installers to have duplicates
+                // Remove these duplicates to avoid multiple matches.
+                installerMetadata.NewInstallers = installerMetadata.NewInstallers.Distinct().ToList();
             }
 
             int numOfNewInstallers = installerMetadataList.Sum(x => x.NewInstallers.Count);
@@ -262,6 +266,24 @@ public static void UpdateInstallerNodesAsync(List<InstallerMetadata> installerMe
                     // If a match is found, add match to dictionary and remove for list of existingInstallers
                     if (existingInstallerMatch != null)
                     {
+                        // Remove the nested installers from the new installer that are not present in the existing installer.
+                        if (newInstaller.NestedInstallerFiles != null && existingInstallerMatch.NestedInstallerFiles != null)
+                        {
+                            var matchedFiles = newInstaller.NestedInstallerFiles
+                                .Where(nif =>
+                                {
+                                    var fileName = Path.GetFileName(nif.RelativeFilePath);
+
+                                    // If the flow reaches here, there's guaranteed to be a matching file name
+                                    // Any mismatches would've been detected earlier in the update flow.
+                                    return existingInstallerMatch.NestedInstallerFiles.Any(eif =>
+                                        Path.GetFileName(eif.RelativeFilePath) == fileName);
+                                })
+                                .ToList();
+
+                            newInstaller.NestedInstallerFiles = matchedFiles;
+                        }
+
                         installerMatchDict.Add(existingInstallerMatch, newInstaller);
                         existingInstallers.Remove(existingInstallerMatch);
                     }

src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.RetainInstallerFields.yaml

@@ -9,16 +9,13 @@ Description: |-
 
   1) For the first installer, all root level fields are copied over and root fields are set to null.
   2) For the second installer, installer level fields are preserved since they are not null.
-  3) InstallerType and NestedInstallerType are common across both installers, so they are moved to the root level at the end of the update.
-
-  TODO: Use different NestedInstallerType and RelativeFilePath for each installer once logic for handling multiple nested installers is improved.
-  Reference: https://github.com/microsoft/winget-create/issues/392
+  3) InstallerType is common across both installers, so it is moved to the root level at the end of the update.
 InstallerLocale: en-US
 InstallerType: zip
 NestedInstallerType: exe
 NestedInstallerFiles:
 - RelativeFilePath: WingetCreateTestExeInstaller.exe
-  PortableCommandAlias: TestAlias
+  PortableCommandAlias: TestExeAlias
 AppsAndFeaturesEntries:
 - DisplayName: TestDisplayName1
   Publisher: TestPublisher1
@@ -87,10 +84,10 @@ Installers:
     InstallerType: zip
     InstallerUrl: https://fakedomain.com/WingetCreateTestZipInstaller.zip
     InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
-    NestedInstallerType: exe
+    NestedInstallerType: msi
     NestedInstallerFiles:
-      - RelativeFilePath: WingetCreateTestExeInstaller.exe
-        PortableCommandAlias: TestAlias
+      - RelativeFilePath: WingetCreateTestMsiInstaller.msi
+        PortableCommandAlias: TestMsiAlias
     AppsAndFeaturesEntries:
       - DisplayName: TestDisplayName2
         Publisher: TestPublisher2

src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.ZipMultipleNestedInstallers.yaml

@@ -0,0 +1,61 @@
+PackageIdentifier: TestPublisher.MultipleNestedInstallers
+PackageVersion: 0.1.2
+PackageName: Test zip app
+Publisher: Test publisher
+License: MIT
+ShortDescription: A manifest used for testing whether multiple nested installers are handled correctly.
+Description: The zip archive contains extra nested installers that are intentionally left out in the manifest to test the behavior.
+Installers:
+- Architecture: x64
+  InstallerType: zip
+  InstallerUrl: https://fakedomain.com/WingetCreateTestMultipleNestedInstallers.zip
+  InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  NestedInstallerType: portable
+  NestedInstallerFiles:
+  - RelativeFilePath: WingetCreateTestExeInstaller.exe
+    PortableCommandAlias: TestExeInstallerAlias
+  - RelativeFilePath: WingetCreateTestPortableInstaller.exe
+    PortableCommandAlias: TestPortableInstallerAlias
+  - RelativeFilePath: WingetCreateTestPortableInstaller (1).exe
+    PortableCommandAlias: TestPortableInstallerAlias1
+  - RelativeFilePath: WingetCreateTestPortableInstaller (2).exe
+    PortableCommandAlias: TestPortableInstallerAlias2
+- Architecture: x86
+  InstallerType: zip
+  InstallerUrl: https://fakedomain.com/WingetCreateTestMultipleNestedInstallers.zip
+  InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  NestedInstallerType: portable
+  NestedInstallerFiles:
+  - RelativeFilePath: WingetCreateTestPortableInstaller.exe
+    PortableCommandAlias: TestPortableInstallerAlias
+  - RelativeFilePath: WingetCreateTestPortableInstaller (1).exe
+    PortableCommandAlias: TestPortableInstallerAlias1
+- Architecture: arm
+  InstallerType: zip
+  InstallerUrl: https://fakedomain.com/WingetCreateTestMultipleNestedInstallers.zip
+  InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  NestedInstallerType: portable
+  NestedInstallerFiles:
+  - RelativeFilePath: WingetCreateTestPortableInstaller (2).exe
+    PortableCommandAlias: TestPortableInstallerAlias2
+- Architecture: arm64
+  InstallerType: zip
+  Scope: user
+  InstallerUrl: https://fakedomain.com/WingetCreateTestMultipleNestedInstallers.zip
+  InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  NestedInstallerType: exe
+  NestedInstallerFiles:
+  - RelativeFilePath: WingetCreateTestExeInstaller.exe
+    PortableCommandAlias: TestExeInstallerAlias
+- Architecture: arm64
+  InstallerType: zip
+  Scope: machine
+  InstallerUrl: https://fakedomain.com/WingetCreateTestMultipleNestedInstallers.zip
+  InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  NestedInstallerType: msi
+  NestedInstallerFiles:
+  - RelativeFilePath: WingetCreateTestMsiInstaller.msi
+    PortableCommandAlias: TestMsiInstallerAlias
+PackageLocale: en-US
+ManifestType: singleton
+ManifestVersion: 1.6.0

src/WingetCreateTests/WingetCreateTests/TestConstants.cs

@@ -35,6 +35,11 @@ public static class TestConstants
         /// </summary>
         public const string TestExeInstaller = "WingetCreateTestExeInstaller.exe";
 
+        /// <summary>
+        /// File name of the test portable installer.
+        /// </summary>
+        public const string TestPortableInstaller = "WingetCreateTestPortableInstaller.exe";
+
         /// <summary>
         /// File name of the test MSI installer.
         /// </summary>

src/WingetCreateTests/WingetCreateTests/TestUtils.cs

@@ -6,6 +6,7 @@ namespace Microsoft.WingetCreateTests
     using System;
     using System.Collections.Generic;
     using System.IO;
+    using System.IO.Compression;
     using System.Linq;
     using System.Net;
     using System.Net.Http;
@@ -138,5 +139,79 @@ public static string MockDownloadFile(string filename)
             string downloadedPath = PackageParser.DownloadFileAsync(url).Result;
             return downloadedPath;
         }
+
+        /// <summary>
+        /// Creates copies of the specified resource file. If multiple copies are requested, the new files will be named with a numeric suffix.
+        /// </summary>
+        /// <param name="resource">Name of the resource file to copy.</param>
+        /// <param name="numberOfCopies">Number of copies to create.</param>
+        /// <param name="newResourceName">Optional new name for the copied resource file.</param>
+        /// <returns>List of paths to the newly created files.</returns>
+        public static List<string> CreateResourceCopy(string resource, int numberOfCopies = 1, string newResourceName = null)
+        {
+            string originalResourcePath = GetTestFile(resource);
+            string newResourcePath = originalResourcePath;
+            if (!string.IsNullOrEmpty(newResourceName))
+            {
+                newResourcePath = Path.Combine(Path.GetDirectoryName(originalResourcePath), newResourceName);
+            }
+
+            List<string> copyPaths = new();
+            for (int i = 0; i < numberOfCopies; i++)
+            {
+                string copyPath = PackageParser.GetNumericFilename(newResourcePath);
+                File.Copy(originalResourcePath, copyPath);
+                copyPaths.Add(copyPath);
+            }
+
+            return copyPaths;
+        }
+
+        /// <summary>
+        /// Adds files to an existing test zip archive.
+        /// </summary>
+        /// <param name="zipResourceName">Name of the zip resource file.</param>
+        /// <param name="filePaths">List of paths for files to be included in the zip archive.</param>
+        public static void AddFilesToZip(string zipResourceName, List<string> filePaths)
+        {
+            string zipPath = GetTestFile(zipResourceName);
+            using (ZipArchive zipArchive = ZipFile.Open(zipPath, ZipArchiveMode.Update))
+            {
+                foreach (string file in filePaths)
+                {
+                    var fileInfo = new FileInfo(file);
+                    zipArchive.CreateEntryFromFile(fileInfo.FullName, fileInfo.Name);
+                }
+            } // The zipArchive is automatically closed and disposed here
+        }
+
+        /// <summary>
+        /// Removes files from an existing test zip archive.
+        /// </summary>
+        /// <param name="zipResourceName">Name of the zip resource file.</param>
+        /// <param name="fileNames">List of file names to be removed from the zip archive.</param>
+        public static void RemoveFilesFromZip(string zipResourceName, List<string> fileNames)
+        {
+            string zipPath = GetTestFile(zipResourceName);
+            using (ZipArchive zipArchive = ZipFile.Open(zipPath, ZipArchiveMode.Update))
+            {
+                foreach (string fileName in fileNames)
+                {
+                    zipArchive.GetEntry(fileName)?.Delete();
+                }
+            } // ZipArchive is automatically closed and disposed here
+        }
+
+        /// <summary>
+        /// Delete test resources from cache directory.
+        /// </summary>
+        /// <param name="testFileNames">Name of the test files to delete.</param>
+        public static void DeleteCachedFiles(List<string> testFileNames)
+        {
+            foreach (string fileName in testFileNames)
+            {
+                File.Delete(Path.Combine(PackageParser.InstallerDownloadPath, fileName));
+            }
+        }
     }
 }