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

Use AppsAndFeatures name and publisher #2042

Merged
merged 2 commits into from
Mar 24, 2022
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
19 changes: 0 additions & 19 deletions src/AppInstallerCLI.sln
Original file line number Diff line number Diff line change
Expand Up @@ -86,21 +86,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "spelling", "spelling", "{2A
EndProject
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "cpprestsdk", "cpprestsdk\cpprestsdk.vcxproj", "{866C3F06-636F-4BE8-BC24-5F86ECC606A1}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "schemas", "schemas", "{92637527-6CDA-4F4A-84FD-858793776777}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "JSON", "JSON", "{F2149997-295A-4593-9282-4C675DFEB670}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "settings", "settings", "{1487DFBB-7C53-4BD3-9B2C-9B94C6C91528}"
ProjectSection(SolutionItems) = preProject
..\schemas\JSON\settings\settings.schema.0.2.json = ..\schemas\JSON\settings\settings.schema.0.2.json
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "packages", "packages", "{F5CED6B6-C27F-4405-9033-6C273B8B129C}"
ProjectSection(SolutionItems) = preProject
..\schemas\JSON\packages\packages.schema.1.0.json = ..\schemas\JSON\packages\packages.schema.1.0.json
..\schemas\JSON\packages\packages.schema.2.0.json = ..\schemas\JSON\packages\packages.schema.2.0.json
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "policy", "policy", "{1A47951F-5C7A-4D6D-BB5F-D77484437940}"
ProjectSection(SolutionItems) = preProject
..\doc\admx\en-US\DesktopAppInstaller.adml = ..\doc\admx\en-US\DesktopAppInstaller.adml
Expand Down Expand Up @@ -1020,10 +1005,6 @@ Global
{952B513F-8A00-4D74-9271-925AFB3C6252} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
{2ACDE176-F13F-42FA-8159-C34FA3D37837} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
{866C3F06-636F-4BE8-BC24-5F86ECC606A1} = {60618CAC-2995-4DF9-9914-45C6FC02C995}
{92637527-6CDA-4F4A-84FD-858793776777} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
{F2149997-295A-4593-9282-4C675DFEB670} = {92637527-6CDA-4F4A-84FD-858793776777}
{1487DFBB-7C53-4BD3-9B2C-9B94C6C91528} = {F2149997-295A-4593-9282-4C675DFEB670}
{F5CED6B6-C27F-4405-9033-6C273B8B129C} = {F2149997-295A-4593-9282-4C675DFEB670}
{1A47951F-5C7A-4D6D-BB5F-D77484437940} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
{409CD681-22A4-469D-88AE-CB5E4836E07A} = {8D53D749-D51C-46F8-A162-9371AAA6C2E7}
EndGlobalSection
Expand Down
33 changes: 21 additions & 12 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -543,25 +543,24 @@ namespace AppInstaller::CLI::Workflow
// Also attempt to find the entry based on the manifest data
const auto& manifest = context.Get<Execution::Data::Manifest>();

SearchRequest nameAndPublisherRequest;
SearchRequest manifestSearchRequest;
AppInstaller::Manifest::Manifest::string_t defaultPublisher;
if (manifest.DefaultLocalization.Contains(Localization::Publisher))
{
defaultPublisher = manifest.DefaultLocalization.Get<Localization::Publisher>();
}

// The default localization must contain the name or we cannot do this lookup
if (manifest.DefaultLocalization.Contains(Localization::PackageName))
{
AppInstaller::Manifest::Manifest::string_t defaultName = manifest.DefaultLocalization.Get<Localization::PackageName>();
AppInstaller::Manifest::Manifest::string_t defaultPublisher;
if (manifest.DefaultLocalization.Contains(Localization::Publisher))
{
defaultPublisher = manifest.DefaultLocalization.Get<Localization::Publisher>();
}

nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, defaultName, defaultPublisher));
manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, defaultName, defaultPublisher));

for (const auto& loc : manifest.Localizations)
{
if (loc.Contains(Localization::PackageName) || loc.Contains(Localization::Publisher))
{
nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact,
manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact,
loc.Contains(Localization::PackageName) ? loc.Get<Localization::PackageName>() : defaultName,
loc.Contains(Localization::Publisher) ? loc.Get<Localization::Publisher>() : defaultPublisher));
}
Expand All @@ -575,18 +574,28 @@ namespace AppInstaller::CLI::Workflow
{
if (std::find(productCodes.begin(), productCodes.end(), installer.ProductCode) == productCodes.end())
{
nameAndPublisherRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode));
manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, installer.ProductCode));
productCodes.emplace_back(installer.ProductCode);
}
}

for (const auto& appsAndFeaturesEntry : installer.AppsAndFeaturesEntries)
{
if (!appsAndFeaturesEntry.DisplayName.empty())
{
manifestSearchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact,
appsAndFeaturesEntry.DisplayName,
appsAndFeaturesEntry.Publisher.empty() ? defaultPublisher : appsAndFeaturesEntry.Publisher));
}
}
}

SearchResult findByManifest;

// Don't execute this search if it would just find everything
if (!nameAndPublisherRequest.IsForEverything())
if (!manifestSearchRequest.IsForEverything())
{
findByManifest = arpSource.Search(nameAndPublisherRequest);
findByManifest = arpSource.Search(manifestSearchRequest);
}

// Cross reference the changes with the search results
Expand Down
70 changes: 70 additions & 0 deletions src/AppInstallerCLITests/SQLiteIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,36 @@ struct IndexFields
ProductCodes(std::move(productCodes))
{}

IndexFields(
std::string id,
std::string name,
std::string publisher,
std::string moniker,
std::string version,
std::string channel,
std::vector<NormalizedString> tags,
std::vector<NormalizedString> commands,
std::string path,
std::vector<NormalizedString> packageFamilyNames,
std::vector<NormalizedString> productCodes,
std::string arpName,
std::string arpPublisher
) :
Id(std::move(id)),
Name(std::move(name)),
Publisher(std::move(publisher)),
Moniker(std::move(moniker)),
Version(std::move(version)),
Channel(std::move(channel)),
Tags(std::move(tags)),
Commands(std::move(commands)),
Path(std::move(path)),
PackageFamilyNames(std::move(packageFamilyNames)),
ProductCodes(std::move(productCodes)),
ArpName(std::move(arpName)),
ArpPublisher(std::move(arpPublisher))
{}

std::string Id;
std::string Name;
std::string Publisher;
Expand All @@ -193,6 +223,8 @@ struct IndexFields
std::string Path;
std::vector<NormalizedString> PackageFamilyNames;
std::vector<NormalizedString> ProductCodes;
std::string ArpName;
std::string ArpPublisher;
};

SQLiteIndex SearchTestSetup(const std::string& filePath, std::initializer_list<IndexFields> data = {}, std::optional<Schema::Version> version = {})
Expand Down Expand Up @@ -230,6 +262,13 @@ SQLiteIndex SearchTestSetup(const std::string& filePath, std::initializer_list<I
manifest.Installers[i].ProductCode = d.ProductCodes[i];
}

if (!d.ArpName.empty() || !d.ArpPublisher.empty())
{
manifest.Installers[0].AppsAndFeaturesEntries.push_back({});
manifest.Installers[0].AppsAndFeaturesEntries[0].DisplayName = d.ArpName;
manifest.Installers[0].AppsAndFeaturesEntries[0].Publisher = d.ArpPublisher;
}

index.AddManifest(manifest, d.Path);
};

Expand Down Expand Up @@ -2738,6 +2777,37 @@ TEST_CASE("SQLiteIndex_NormNameAndPublisher_Complex", "[sqliteindex]")
}
}

TEST_CASE("SQLiteIndex_NormNameAndPublisher_AppsAndFeatures", "[sqliteindex]")
{
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
INFO("Using temporary file named: " << tempFile.GetPath());

std::string testName = "Name";
std::string testPublisher = "Publisher";
std::string arpTestName = "Other Thing";
std::string arpTestPublisher = "Big Company Name";

SQLiteIndex index = SearchTestSetup(tempFile, {
{ "Id1", testName, testPublisher, "Moniker", "Version", "Channel", { "Tag" }, { "Command" }, "Path1", {}, { "PC1", "PC2" }, arpTestName, arpTestPublisher },
});

Schema::Version testVersion = TestPrepareForRead(index);

SearchRequest request;
request.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact, arpTestName, arpTestPublisher));

auto results = index.Search(request);

if (AreNormalizedNameAndPublisherSupported(index, testVersion))
{
REQUIRE(results.Matches.size() == 1);
}
else
{
REQUIRE(results.Matches.empty());
}
}

TEST_CASE("SQLiteIndex_ManifestHash_Present", "[sqliteindex]")
{
TempFile tempFile{ "repolibtest_tempdb"s, ".db"s };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,37 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2
{
namespace
{
void AddNormalizedName(const Utility::NameNormalizer& normalizer, const Manifest::string_t& name, std::vector<Utility::NormalizedString>& out)
{
Utility::NormalizedString value = normalizer.NormalizeName(Utility::FoldCase(name)).Name();
if (std::find(out.begin(), out.end(), value) == out.end())
{
out.emplace_back(std::move(value));
}
}

void AddLocalizationNormalizedName(const Utility::NameNormalizer& normalizer, const Manifest::ManifestLocalization& localization, std::vector<Utility::NormalizedString>& out)
{
if (localization.Contains(Manifest::Localization::PackageName))
{
Utility::NormalizedString value = normalizer.NormalizeName(Utility::FoldCase(localization.Get<Manifest::Localization::PackageName>())).Name();
if (std::find(out.begin(), out.end(), value) == out.end())
{
out.emplace_back(std::move(value));
}
AddNormalizedName(normalizer, localization.Get<Manifest::Localization::PackageName>(), out);
}
}

void AddNormalizedPublisher(const Utility::NameNormalizer& normalizer, const Manifest::string_t& publisher, std::vector<Utility::NormalizedString>& out)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AddNormalizedPublisher

nit: looks like this implementation is same as the AddNormalizedName above, shall we combine them like AddNormalizedValue?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They aren't, they use NormalizeName and NormalizePublisher respectively.

{
Utility::NormalizedString value = normalizer.NormalizePublisher(Utility::FoldCase(publisher));
if (std::find(out.begin(), out.end(), value) == out.end())
{
out.emplace_back(std::move(value));
}
}

void AddLocalizationNormalizedPublisher(const Utility::NameNormalizer& normalizer, const Manifest::ManifestLocalization& localization, std::vector<Utility::NormalizedString>& out)
{
if (localization.Contains(Manifest::Localization::Publisher))
{
Utility::NormalizedString value = normalizer.NormalizePublisher(Utility::FoldCase(localization.Get<Manifest::Localization::Publisher>()));
if (std::find(out.begin(), out.end(), value) == out.end())
{
out.emplace_back(std::move(value));
}
AddNormalizedPublisher(normalizer, localization.Get<Manifest::Localization::Publisher>(), out);
}
}

Expand All @@ -47,6 +57,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2
AddLocalizationNormalizedName(normalizer, loc, result);
}

// In addition to the names used for our display, add the display names from the ARP entries
for (const auto& installer : manifest.Installers)
{
for (const auto& appsAndFeaturesEntry : installer.AppsAndFeaturesEntries)
{
if (!appsAndFeaturesEntry.DisplayName.empty())
{
AddNormalizedName(normalizer, appsAndFeaturesEntry.DisplayName, result);
}
}
}

return result;
}

Expand All @@ -60,6 +82,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2
AddLocalizationNormalizedPublisher(normalizer, loc, result);
}

// In addition to the publishers used for our display, add the publishers from the ARP entries
for (const auto& installer : manifest.Installers)
{
for (const auto& appsAndFeaturesEntry : installer.AppsAndFeaturesEntries)
{
if (!appsAndFeaturesEntry.Publisher.empty())
{
AddNormalizedPublisher(normalizer, appsAndFeaturesEntry.Publisher, result);
}
}
}

return result;
}
}
Expand Down