Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate missing ManifestVersion for manifest from rest source and make PackageFamilyName and installer type manifest validation warning #3460

Merged
merged 6 commits into from
Jul 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -604,9 +604,6 @@
<CopyFileToFolders Include="TestData\Manifest-Good-DefaultExpectedReturnCodeInInstallerSuccessCodes.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Good-PackageFamilyNameOnExe-Ver1_2.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Good-MultipleArpVersionDeclared.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,9 +540,6 @@
<CopyFileToFolders Include="TestData\Manifest-Good-NoArpVersionDeclared.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Good-PackageFamilyNameOnExe-Ver1_2.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\Manifest-Good-SingleArpVersionDeclared.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/RestInterface_1_0.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ TEST_CASE("GetManifests_GoodResponse", "[RestSource][Interface_1_0]")
REQUIRE(manifest.Version == "3.0.0abc");
REQUIRE(manifest.Moniker == "FooBarMoniker");
REQUIRE(manifest.Channel == "");
REQUIRE(manifest.ManifestVersion == AppInstaller::Manifest::ManifestVer{ "1.0.0" });
sampleManifest.VerifyLocalizations_AllFields(manifest);
sampleManifest.VerifyInstallers_AllFields(manifest);
}
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/RestInterface_1_1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ TEST_CASE("GetManifests_GoodResponse_V1_1", "[RestSource][Interface_1_1]")
REQUIRE(manifest.Version == "3.0.0abc");
REQUIRE(manifest.Moniker == "FooBarMoniker");
REQUIRE(manifest.Channel == "");
REQUIRE(manifest.ManifestVersion == AppInstaller::Manifest::ManifestVer{ "1.1.0" });
sampleManifest.VerifyLocalizations_AllFields(manifest);
sampleManifest.VerifyInstallers_AllFields(manifest);
}
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/RestInterface_1_4.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ TEST_CASE("GetManifests_GoodResponse_V1_4", "[RestSource][Interface_1_4]")
REQUIRE(manifest.Version == "3.0.0abc");
REQUIRE(manifest.Moniker == "FooBarMoniker");
REQUIRE(manifest.Channel == "");
REQUIRE(manifest.ManifestVersion == AppInstaller::Manifest::ManifestVer{ "1.4.0" });
sampleManifest.VerifyLocalizations_AllFields(manifest);
sampleManifest.VerifyInstallers_AllFields(manifest);
}
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCLITests/RestInterface_1_5.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ TEST_CASE("GetManifests_GoodResponse_V1_5", "[RestSource][Interface_1_5]")
REQUIRE(manifest.Version == "3.0.0abc");
REQUIRE(manifest.Moniker == "FooBarMoniker");
REQUIRE(manifest.Channel == "");
REQUIRE(manifest.ManifestVersion == AppInstaller::Manifest::ManifestVer{ "1.5.0" });
sampleManifest.VerifyLocalizations_AllFields(manifest);
sampleManifest.VerifyInstallers_AllFields(manifest);
}

This file was deleted.

5 changes: 2 additions & 3 deletions src/AppInstallerCLITests/YamlManifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ TEST_CASE("ReadGoodManifests", "[ManifestValidation]")
{ "Manifest-Good-Minimum-InstallerType.yaml" },
{ "Manifest-Good-Switches.yaml" },
{ "Manifest-Good-DefaultExpectedReturnCodeInInstallerSuccessCodes.yaml" },
{ "Manifest-Good-PackageFamilyNameOnExe-Ver1_2.yaml" },
};

for (auto const& testCase : TestCases)
Expand Down Expand Up @@ -300,9 +299,9 @@ TEST_CASE("ReadBadManifests", "[ManifestValidation]")
{ "Manifest-Bad-VersionMissing.yaml", "Missing required property 'Version'" },
{ "Manifest-Bad-InvalidManifestVersionValue.yaml", "Failed to validate against schema associated with property name 'ManifestVersion'" },
{ "InstallFlowTest_MSStore.yaml", "Field value is not supported. [InstallerType] Value: msstore" },
{ "Manifest-Bad-PackageFamilyNameOnMSI.yaml", "The specified installer type does not support PackageFamilyName. [InstallerType] Value: msi" },
{ "Manifest-Bad-PackageFamilyNameOnMSI.yaml", "The specified installer type does not support PackageFamilyName. [InstallerType] Value: msi", true },
{ "Manifest-Bad-ProductCodeOnMSIX.yaml", "The specified installer type does not support ProductCode. [InstallerType] Value: msix" },
{ "Manifest-Bad-InvalidUpdateBehavior.yaml", "Invalid field value. [UpdateBehavior]" },
{ "Manifest-Bad-InvalidUpdateBehavior.yaml", "Invalid field value. [UpgradeBehavior]" },
{ "Manifest-Bad-InvalidLocale.yaml", "The locale value is not a well formed bcp47 language tag." },
{ "Manifest-Bad-AppsAndFeaturesEntriesOnMSIX.yaml", "The specified installer type does not write to Apps and Features entry." },
{ "InstallFlowTest_LicenseAgreement.yaml", "Field usage requires verified publishers.", true },
Expand Down
8 changes: 4 additions & 4 deletions src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ namespace AppInstaller::Manifest

if (installer.UpdateBehavior == UpdateBehaviorEnum::Unknown)
{
resultErrors.emplace_back(ManifestError::InvalidFieldValue, "UpdateBehavior");
resultErrors.emplace_back(ManifestError::InvalidFieldValue, "UpgradeBehavior");
}

// Validate system reference strings if they are set at the installer level
// Allow PackageFamilyName to be declared with non msix installers to support nested installer scenarios after manifest version 1.1
if (manifest.ManifestVersion <= ManifestVer{ s_ManifestVersionV1_1 } && !installer.PackageFamilyName.empty() && !DoesInstallerTypeUsePackageFamilyName(installer.EffectiveInstallerType()))
// Allow PackageFamilyName to be declared with non msix installers to support nested installer scenarios. But still report as warning to notify user of this uncommon case.
if (!installer.PackageFamilyName.empty() && !DoesInstallerTypeUsePackageFamilyName(installer.EffectiveInstallerType()))
{
resultErrors.emplace_back(ManifestError::InstallerTypeDoesNotSupportPackageFamilyName, "InstallerType", InstallerTypeToString(installer.EffectiveInstallerType()));
resultErrors.emplace_back(ManifestError::InstallerTypeDoesNotSupportPackageFamilyName, "InstallerType", std::string{ InstallerTypeToString(installer.EffectiveInstallerType()) }, ValidationError::Level::Warning);
}

if (!installer.ProductCode.empty() && !DoesInstallerTypeUseProductCode(installer.EffectiveInstallerType()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@
<Filter Include="Rest\Schema\1_4\Json">
<UniqueIdentifier>{d9d70cf5-ce04-4db2-a0ec-970dd0ad22b6}</UniqueIdentifier>
</Filter>
<Filter Include="Rest\Schema\1_5">
<UniqueIdentifier>{f131c993-1136-4f4c-85a9-8606c6d297a8}</UniqueIdentifier>
</Filter>
<Filter Include="Rest\Schema\1_5\Json">
<UniqueIdentifier>{78cf34a8-b868-4393-ad9c-630940569186}</UniqueIdentifier>
</Filter>
<Filter Include="Microsoft\Schema\Pinning_1_0">
<UniqueIdentifier>{f05e19bb-2161-4ab0-9d04-2dfa2d3eb3c6}</UniqueIdentifier>
</Filter>
Expand Down Expand Up @@ -336,6 +342,12 @@
<ClInclude Include="Rest\Schema\1_4\Json\SearchResponseDeserializer.h">
<Filter>Rest\Schema\1_4\Json</Filter>
</ClInclude>
<ClInclude Include="Rest\Schema\1_5\Json\ManifestDeserializer.h">
<Filter>Rest\Schema\1_5\Json</Filter>
</ClInclude>
<ClInclude Include="Rest\Schema\1_5\Interface.h">
<Filter>Rest\Schema\1_5</Filter>
</ClInclude>
<ClInclude Include="Rest\Schema\SearchRequestComposer.h">
<Filter>Rest\Schema</Filter>
</ClInclude>
Expand Down Expand Up @@ -554,6 +566,12 @@
<ClCompile Include="Rest\Schema\1_4\Json\SearchResponseDeserializer_1_4.cpp">
<Filter>Rest\Schema\1_4\Json</Filter>
</ClCompile>
<ClCompile Include="Rest\Schema\1_5\Json\ManifestDeserializer_1_5.cpp">
<Filter>Rest\Schema\1_5\Json</Filter>
</ClCompile>
<ClCompile Include="Rest\Schema\1_5\RestInterface_1_5.cpp">
<Filter>Rest\Schema\1_5</Filter>
</ClCompile>
<ClCompile Include="Rest\Schema\SearchRequestComposer.cpp">
<Filter>Rest\Schema</Filter>
</ClCompile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json
virtual Manifest::InstallerTypeEnum ConvertToInstallerType(std::string_view in) const;

std::vector<Manifest::string_t> ConvertToManifestStringArray(const std::vector<std::string>& values) const;

virtual Manifest::ManifestVer GetManifestVersion() const;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json
for (auto& versionItem : versions.value().get())
{
Manifest::Manifest manifest;

manifest.ManifestVersion = GetManifestVersion();

manifest.Id = id.value();

std::optional<std::string> packageVersion = JSON::GetRawStringValueFromJsonNode(versionItem, JSON::GetUtilityString(PackageVersion));
Expand Down Expand Up @@ -525,4 +528,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_0::Json

return result;
}

Manifest::ManifestVer ManifestDeserializer::GetManifestVersion() const
{
return Manifest::s_ManifestVersionV1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_1::Json
virtual Manifest::ExpectedReturnCodeEnum ConvertToExpectedReturnCodeEnum(std::string_view in) const;

virtual Manifest::ManifestInstaller::ExpectedReturnCodeInfo DeserializeExpectedReturnCodeInfo(const web::json::value& expectedReturnCodeJsonObject) const;

Manifest::ManifestVer GetManifestVersion() const override;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -287,4 +287,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_1::Json

return result;
}

Manifest::ManifestVer ManifestDeserializer::GetManifestVersion() const
{
return Manifest::s_ManifestVersionV1_1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,7 @@ namespace AppInstaller::Repository::Rest::Schema::V1_4::Json
Manifest::ExpectedReturnCodeEnum ConvertToExpectedReturnCodeEnum(std::string_view in) const override;

Manifest::ManifestInstaller::ExpectedReturnCodeInfo DeserializeExpectedReturnCodeInfo(const web::json::value& expectedReturnCodeJsonObject) const override;

Manifest::ManifestVer GetManifestVersion() const override;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_4::Json

return result;
}

Manifest::ManifestVer ManifestDeserializer::GetManifestVersion() const
{
return Manifest::s_ManifestVersionV1_4;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_5::Json
struct ManifestDeserializer : public V1_4::Json::ManifestDeserializer
{
std::optional<Manifest::ManifestLocalization> DeserializeLocale(const web::json::value& localeJsonObject) const override;

protected:

Manifest::ManifestVer GetManifestVersion() const override;
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,9 @@ namespace AppInstaller::Repository::Rest::Schema::V1_5::Json

return result;
}

Manifest::ManifestVer ManifestDeserializer::GetManifestVersion() const
{
return Manifest::s_ManifestVersionV1_5;
}
}
Loading