Retain non-null installer fields when copying over root values (#491)

mdanish-kh · web-flow · commit 9fd1078397f0 · 2023-12-13T14:58:13.000-08:00
diff --git a/src/WingetCreateCLI/Commands/BaseCommand.cs b/src/WingetCreateCLI/Commands/BaseCommand.cs
@@ -422,6 +422,7 @@ protected static void RemoveEmptyStringFieldsInManifests(Manifests manifests)
 
         /// <summary>
         /// Shifts common installer fields from manifest root to installer level.
+        /// If value is already defined at the installer level, then it should not be overwritten.
         /// </summary>
         /// <param name="installerManifest">Wrapper object containing the installer manifest object models.</param>
         protected static void ShiftRootFieldsToInstallerLevel(InstallerManifest installerManifest)
@@ -439,8 +440,13 @@ protected static void ShiftRootFieldsToInstallerLevel(InstallerManifest installe
                 {
                     foreach (var installer in installerManifest.Installers)
                     {
-                        // Copy the value to installer level
-                        installer.GetType().GetProperty(property.Name).SetValue(installer, rootValue);
+                        var installerProperty = installer.GetType().GetProperty(property.Name);
+
+                        // Only set the value if it is null at the installer level
+                        if (installerProperty.GetValue(installer) == null)
+                        {
+                            installerProperty.SetValue(installer, rootValue);
+                        }
                     }
 
                     // Set root value to null
diff --git a/src/WingetCreateCLI/Commands/UpdateCommand.cs b/src/WingetCreateCLI/Commands/UpdateCommand.cs
@@ -460,13 +460,6 @@ public Manifests DeserializeManifestContentAndApplyInitialUpdate(List<string> la
                 UpdatePropertyForLocaleManifests(nameof(LocaleManifest.PackageVersion), this.Version, localeManifests);
             }
 
-            // TODO: Move relevant metadata from root node to installer node.
-            if (installerManifest.InstallerType != null)
-            {
-                installerManifest.Installers.ForEach(i => i.InstallerType = installerManifest.InstallerType);
-                installerManifest.InstallerType = null;
-            }
-
             return manifests;
         }
 
diff --git a/src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.OverwriteNullInstallerFields.yaml b/src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.OverwriteNullInstallerFields.yaml
@@ -0,0 +1,88 @@
+PackageIdentifier: TestPublisher.OverwriteNullInstallerFields
+PackageVersion: 0.1.2
+PackageName: Overwrite installer level fields by root fields
+Publisher: Test publisher
+License: MIT
+ShortDescription: A manifest that verifies that installer level fields are overwritten by root fields.
+Description: |-
+  Expected flow:
+
+  1) Installer level fields are overwritten by root fields at the start of the update.
+  2) The update flow modifies the installer level fields if needed. (e.g. ProductCode in case of MSI upgrade)
+  3) At the end of the update, the common installer fields are moved to the root level.
+InstallerLocale: en-US
+InstallerType: zip
+NestedInstallerType: exe
+NestedInstallerFiles:
+- RelativeFilePath: WingetCreateTestExeInstaller.exe
+  PortableCommandAlias: PortableCommandAlias1
+AppsAndFeaturesEntries:
+- DisplayName: TestDisplayName1
+  Publisher: TestPublisher1
+  DisplayVersion: 1.0.1
+  ProductCode: TestProductCode1
+  UpgradeCode: TestUpgradeCode1
+  InstallerType: msi
+InstallerSwitches:
+  Silent: /silent1
+  SilentWithProgress: /silentwithprogress1
+Dependencies:
+  PackageDependencies:
+  - PackageIdentifier: TestPackageDependency1
+  MinimumVersion: 1.0.1
+  WindowsFeatures:
+  - TestWindowsFeature1
+  ExternalDependencies:
+  - TestExternalDependency1
+  WindowsLibraries:
+  - TestWindowsLibrary1
+ExpectedReturnCodes:
+  - InstallerReturnCode: 1001
+    ReturnResponse: installInProgress
+MinimumOSVersion: 10.0.22000.0
+PackageFamilyName: TestPackageFamilyName1
+Platform:
+- Windows.Desktop
+Scope: machine
+UpgradeBehavior: install
+ElevationRequirement: elevationRequired
+Commands:
+  - fakeCommand1
+Protocols:
+  - fakeProtocol1
+FileExtensions:
+  - .exe
+# Uncomment when installer model gets updated to support these fields
+#Markets:
+#  AllowedMarkets:
+#   - fakeAllowedMarket
+#  ExcludedMarkets:
+#    - fakeExcludedMarket
+InstallerAbortsTerminal: true
+InstallLocationRequired: true
+RequireExplicitUpgrade: true
+UnsupportedOSArchitectures:
+  - arm64
+DisplayInstallWarnings: true
+InstallerSuccessCodes:
+  - 1
+UnsupportedArguments:
+  - log
+  - location
+InstallationMetadata:
+  DefaultInstallLocation: "%ProgramFiles%\\TestApp1"
+  Files:
+  - RelativeFilePath: "main1.exe"
+    FileSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82
+    FileType: launch
+    InvocationParameter: "/arg1"
+Installers:
+  - Architecture: x64
+    InstallerUrl: https://fakedomain.com/WingetCreateTestZipInstaller.zip
+    InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+  - Architecture: x86
+    InstallerUrl: https://fakedomain.com/WingetCreateTestZipInstaller.zip
+    InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
+PackageLocale: en-US
+ManifestType: singleton
+ManifestVersion: 1.4.0
diff --git a/src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.RetainInstallerFields.yaml b/src/WingetCreateTests/WingetCreateTests/Resources/TestPublisher.RetainInstallerFields.yaml
@@ -1,21 +1,24 @@
-PackageIdentifier: TestPublisher.OverrideInstallerFields
+PackageIdentifier: TestPublisher.RetainInstallerFields
 PackageVersion: 0.1.2
-PackageName: Override installer level fields by root fields
+PackageName: Retain installer level fields when copying over root values
 Publisher: Test publisher
 License: MIT
-ShortDescription: A manifest that verifies that installer level fields are overridden by root fields.
+ShortDescription: A manifest that verifies that non-null installer level fields are not overwritten by root fields.
 Description: |-
   Expected flow:
 
-  1) Installer level fields are overridden by root fields at the start of the update.
-  2) The update flow modifies the installer level fields if needed. (e.g. ProductCode in case of MSI upgrade)
-  3) At the end of the update, the common installer fields are moved to the root level.
+  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
 InstallerLocale: en-US
 InstallerType: zip
 NestedInstallerType: exe
 NestedInstallerFiles:
 - RelativeFilePath: WingetCreateTestExeInstaller.exe
-  PortableCommandAlias: PortableCommandAlias1
+  PortableCommandAlias: TestAlias
 AppsAndFeaturesEntries:
 - DisplayName: TestDisplayName1
   Publisher: TestPublisher1
@@ -78,80 +81,16 @@ InstallationMetadata:
     InvocationParameter: "/arg1"
 Installers:
   - Architecture: x64
-    InstallerType: zip
     InstallerUrl: https://fakedomain.com/WingetCreateTestZipInstaller.zip
     InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
-    NestedInstallerType: msi
-    NestedInstallerFiles:
-      - RelativeFilePath: WingetCreateTestExeInstaller.exe
-        PortableCommandAlias: PortableCommandAlias2
-    AppsAndFeaturesEntries:
-      - DisplayName: TestDisplayName2
-        Publisher: TestPublisher2
-        DisplayVersion: 1.0.2
-        ProductCode: TestProductCode2
-        UpgradeCode: TestUpgradeCode2
-        InstallerType: exe
-    InstallerSwitches:
-      Silent: /silent2
-      SilentWithProgress: /silentwithprogress2
-    Dependencies:
-      PackageDependencies:
-      - PackageIdentifier: TestPackageDependency2
-        MinimumVersion: 1.0.2
-      WindowsFeatures:
-        - TestWindowsFeature2
-      ExternalDependencies:
-        - TestExternalDependency2
-      WindowsLibraries:
-        - TestWindowsLibrary2
-    ExpectedReturnCodes:
-    - InstallerReturnCode: 1002
-      ReturnResponse: installInProgress
-    MinimumOSVersion: 10.0.17763.0
-    PackageFamilyName: TestPackageFamilyName2
-    Platform:
-    - Windows.Universal
-    Scope: user
-    UpgradeBehavior: uninstallPrevious
-    ElevationRequirement: elevatesSelf
-    Commands:
-      - fakeCommand2
-    Protocols:
-      - fakeProtocol2
-    FileExtensions:
-      - .msi
-    # Uncomment when installer model gets updated to support these fields
-    #Markets:
-    #  AllowedMarkets:
-    #   - fakeAllowedMarket
-    #  ExcludedMarkets:
-    #    - fakeExcludedMarket
-    InstallerAbortsTerminal: false
-    InstallLocationRequired: false
-    RequireExplicitUpgrade: false
-    UnsupportedOSArchitectures:
-      - arm
-    DisplayInstallWarnings: false
-    InstallerSuccessCodes:
-      - 2
-    UnsupportedArguments:
-      - log
-    InstallationMetadata:
-      DefaultInstallLocation: "%ProgramFiles%\\TestApp2"
-      Files:
-        - RelativeFilePath: "main2.exe"
-          FileSha256: 69D84CA8899800A5575CE31798293CD4FEBAB1D734A07C2E51E56A28E0DF8C82
-          FileType: launch
-          InvocationParameter: "/arg2"
   - Architecture: x86
     InstallerType: zip
     InstallerUrl: https://fakedomain.com/WingetCreateTestZipInstaller.zip
     InstallerSha256: 8A052767127A6E2058BAAE03B551A807777BB1B726650E2C7E92C3E92C8DF80D
-    NestedInstallerType: msi
+    NestedInstallerType: exe
     NestedInstallerFiles:
       - RelativeFilePath: WingetCreateTestExeInstaller.exe
-        PortableCommandAlias: PortableCommandAlias2
+        PortableCommandAlias: TestAlias
     AppsAndFeaturesEntries:
       - DisplayName: TestDisplayName2
         Publisher: TestPublisher2
@@ -188,7 +127,7 @@ Installers:
       - fakeProtocol2
     FileExtensions:
       - .msi
-    # Uncomment when installer model gets updated to support these fields
+    # TODO: Uncomment when installer model gets updated to support these fields
     #Markets:
     #  AllowedMarkets:
     #   - fakeAllowedMarket
diff --git a/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs b/src/WingetCreateTests/WingetCreateTests/UnitTests/UpdateCommandTests.cs
diff --git a/src/WingetCreateTests/WingetCreateTests/WingetCreateTests.csproj b/src/WingetCreateTests/WingetCreateTests/WingetCreateTests.csproj

Original file line number	Diff line number	Diff line change
`@@ -460,13 +460,6 @@ public Manifests DeserializeManifestContentAndApplyInitialUpdate(List<string> la`
`460`	`460`	`UpdatePropertyForLocaleManifests(nameof(LocaleManifest.PackageVersion), this.Version, localeManifests);`
`461`	`461`	`}`
`462`	`462`
`463`		`- // TODO: Move relevant metadata from root node to installer node.`
`464`		`- if (installerManifest.InstallerType != null)`
`465`		`- {`
`466`		`- installerManifest.Installers.ForEach(i => i.InstallerType = installerManifest.InstallerType);`
`467`		`- installerManifest.InstallerType = null;`
`468`		`- }`
`469`		`-`
`470`	`463`	`return manifests;`
`471`	`464`	`}`
`472`	`465`