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

Add heuristics for matching packages to ARP after installing #2044

Merged
merged 41 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
76a0548
Add type for ARP correlation algorithms
Mar 22, 2022
da36d8b
Add function to compute best match
Mar 23, 2022
d560e33
Add overal structure for tests
Mar 23, 2022
c0894f4
Record ARP product code after install
Mar 23, 2022
2ac0f75
Use correlation measures in post-install
Mar 24, 2022
bcfb37e
Add test cases
Mar 24, 2022
d2f4662
Add normalized name measure (very hacky...)
Mar 24, 2022
a3e9d49
Add edit distance measure
Mar 24, 2022
5574320
Cleanup data
Mar 24, 2022
6c4ec11
Add edit distance measure to tests
Mar 24, 2022
cb2b9c9
Spelling
Mar 24, 2022
fb8da65
Merge branch 'master' into matching
Mar 28, 2022
d0f4110
PR comments, cleanup & refactor
Mar 30, 2022
435d4c3
Report false matches in tests
Mar 30, 2022
58a3f29
Use FoldCase; remove edit distance weights
Mar 30, 2022
2cf7961
Cleanup test data
Mar 30, 2022
ab55bbc
Fix crashes; add logs
Apr 1, 2022
867f156
Put whole ARP entry in context
Apr 1, 2022
c01cfbb
Cleanup test data
Apr 1, 2022
49727ba
Update test logs
Apr 1, 2022
aa5afee
Use type in context
Apr 1, 2022
e75287d
Update test data
Apr 4, 2022
d58c1ef
Allow empty
Apr 5, 2022
abc38b3
Remove unused measure
Apr 6, 2022
b647a60
Reduce reporting
Apr 6, 2022
d2cc53c
Spelling
Apr 6, 2022
0e29cc4
Add empty heuristic override for ARP snapshot tests
Apr 6, 2022
7c43ebf
Hide test
Apr 6, 2022
891e678
Rename context data
Apr 8, 2022
561e21d
Refactor per PR comments; use UTF-32 for edit distance
Apr 8, 2022
70ae168
Expand test cases
Apr 8, 2022
35683f8
Remove duplicates in data
Apr 8, 2022
1087ec5
Copy code for publisher property
Apr 8, 2022
0520643
Use Publisher property in tests
Apr 8, 2022
95af4ce
Merge branch 'master' into matching
Apr 8, 2022
ce8b259
Resolve TODOs
Apr 8, 2022
314d2f1
Report time for correlation
Apr 8, 2022
b48b697
Do a single allocation for edit distance table
Apr 8, 2022
1ccd981
Spelling
Apr 8, 2022
b3c3332
Update src/AppInstallerCLITests/Correlation.cpp
lechacon Apr 8, 2022
3f49a78
Use steady_clock
Apr 8, 2022
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
2 changes: 2 additions & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ hre
hresults
htm
IAttachment
IARP
IConfiguration
idx
IFACEMETHODIMP
Expand Down Expand Up @@ -378,6 +379,7 @@ TStatus
UCase
ucasemap
UChars
ucnv
uec
uild
uintptr
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/ExecutionContextData.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ namespace AppInstaller::CLI::Execution
// On import: Sources for the imported packages
Sources,
ARPSnapshot,
ProductCodeFromARP,
CorrelatedAppsAndFeaturesEntries,
Dependencies,
DependencySource,
AllowedArchitectures,
Expand Down Expand Up @@ -192,7 +192,7 @@ namespace AppInstaller::CLI::Execution
};

template <>
struct DataMapping<Data::ProductCodeFromARP>
struct DataMapping<Data::CorrelatedAppsAndFeaturesEntries>
{
using value_t = std::vector<Manifest::AppsAndFeaturesEntry>;
};
Expand Down
69 changes: 38 additions & 31 deletions src/AppInstallerCLICore/Workflows/InstallFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,54 +524,61 @@ namespace AppInstaller::CLI::Workflow
return result;
}, true);

// We can only get the source identifier from an active source
std::string sourceIdentifier;
if (context.Contains(Execution::Data::PackageVersion))
{
sourceIdentifier = context.Get<Execution::Data::PackageVersion>()->GetProperty(PackageVersionProperty::SourceIdentifier);
}

auto arpEntry = Correlation::FindARPEntryForNewlyInstalledPackage(manifest, arpSnapshot, arpSource, sourceIdentifier);
auto correlationResult = Correlation::FindARPEntryForNewlyInstalledPackage(manifest, arpSnapshot, arpSource);

// Store the ARP entry found to match the package to record it in the tracking catalog later
if (arpEntry)
if (correlationResult.Package)
{
// TODO: Is this the right way to add the data we have?
std::vector<AppsAndFeaturesEntry> entries;

auto metadata = arpEntry->GetMetadata();
auto metadata = correlationResult.Package->GetMetadata();

AppsAndFeaturesEntry baseEntry;
baseEntry.DisplayName = arpEntry->GetProperty(PackageVersionProperty::Name).get();
baseEntry.DisplayVersion = arpEntry->GetProperty(PackageVersionProperty::Version).get();
baseEntry.Publisher = metadata[PackageVersionMetadata::Publisher];

// Display name and publisher are also available as multi properties, but
// for ARP there will always be only 0 or 1 values.
baseEntry.DisplayName = correlationResult.Package->GetProperty(PackageVersionProperty::Name).get();
baseEntry.Publisher = correlationResult.Package->GetProperty(PackageVersionProperty::Publisher).get();
baseEntry.DisplayVersion = correlationResult.Package->GetProperty(PackageVersionProperty::Version).get();
baseEntry.InstallerType = Manifest::ConvertToInstallerTypeEnum(metadata[PackageVersionMetadata::InstalledType]);

auto productCodes = arpEntry->GetMultiProperty(PackageVersionMultiProperty::ProductCode);
auto productCodes = correlationResult.Package->GetMultiProperty(PackageVersionMultiProperty::ProductCode);
for (auto&& productCode : productCodes)
{
AppsAndFeaturesEntry entry = baseEntry;
entry.ProductCode = std::move(productCode).get();
entries.push_back(std::move(entry));
}

auto names = arpEntry->GetMultiProperty(PackageVersionMultiProperty::Name);
auto publishers = arpEntry->GetMultiProperty(PackageVersionMultiProperty::Publisher);
context.Add<Data::CorrelatedAppsAndFeaturesEntries>(std::move(entries));
}

// TODO: these should always have the same size...
if (names.size() == publishers.size())
{
for (size_t i = 0; i < names.size(); ++i)
{
AppsAndFeaturesEntry entry = baseEntry;
entry.DisplayName = std::move(names[i]).get();
entry.Publisher = std::move(publishers[i]).get();
entries.push_back(std::move(entry));
}
}
// We can only get the source identifier from an active source
std::string sourceIdentifier;
if (context.Contains(Execution::Data::PackageVersion))
{
sourceIdentifier = context.Get<Execution::Data::PackageVersion>()->GetProperty(PackageVersionProperty::SourceIdentifier);
}

context.Add<Data::ProductCodeFromARP>(std::move(entries));
IPackageVersion::Metadata arpEntryMetadata;
if (correlationResult.Package)
{
arpEntryMetadata = correlationResult.Package->GetMetadata();
}

Logging::Telemetry().LogSuccessfulInstallARPChange(
sourceIdentifier,
manifest.Id,
manifest.Version,
manifest.Channel,
correlationResult.ChangesToARP,
correlationResult.MatchesInARP,
correlationResult.CountOfIntersectionOfChangesAndMatches,
correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Name)) : "",
correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Version)) : "",
correlationResult.Package ? static_cast<std::string>(correlationResult.Package->GetProperty(PackageVersionProperty::Publisher)) : "",
correlationResult.Package ? static_cast<std::string_view>(arpEntryMetadata[PackageVersionMetadata::InstalledLocale]) : ""
);
}
CATCH_LOG();

Expand All @@ -591,11 +598,11 @@ namespace AppInstaller::CLI::Workflow
// we set its product code in the manifest we record to ensure we can
// find it in the future.
// Note that this may overwrite existing information.
if (context.Contains(Data::ProductCodeFromARP))
if (context.Contains(Data::CorrelatedAppsAndFeaturesEntries))
{
// Use a new Installer entry
manifest.Installers.emplace_back();
manifest.Installers.back().AppsAndFeaturesEntries = context.Get<Data::ProductCodeFromARP>();
manifest.Installers.back().AppsAndFeaturesEntries = context.Get<Data::CorrelatedAppsAndFeaturesEntries>();
}

auto trackingCatalog = context.Get<Data::PackageVersion>()->GetSource().GetTrackingCatalog();
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCLICore/Workflows/InstallFlow.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ namespace AppInstaller::CLI::Workflow
// and stores the product code of the ARP entry found for the package.
// Required Args: None
// Inputs: ARPSnapshot?, Manifest, PackageVersion
// Outputs: ProductCodeFromARP?
// Outputs: CorrelatedAppsAndFeaturesEntries?
void ReportARPChanges(Execution::Context& context);

// Records the installation to the tracking catalog.
// Required Args: None
// Inputs: PackageVersion?, Manifest, Installer, ProductCodeFromARP?
// Inputs: PackageVersion?, Manifest, Installer, CorrelatedAppsAndFeaturesEntries?
// Outputs: None
void RecordInstall(Execution::Context& context);
}
14 changes: 3 additions & 11 deletions src/AppInstallerCLITests/ARPChanges.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,26 +209,18 @@ struct TestContext : public Context
// consider only the exact matching.
struct TestHeuristicOverride
Copy link
Member

Choose a reason for hiding this comment

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

Really these tests should probably be removed at some point in the future, but it doesn't need to be now since you have them working.

{
struct EmptyHeuristic: public ARPCorrelationAlgorithm
{
double GetMatchingScore(const Manifest::Manifest&, const Manifest::ManifestLocalization&, const ARPEntry&) const override
{
return 0;
}
};

TestHeuristicOverride()
{
ARPCorrelationAlgorithm::OverrideInstance(&m_algorithm);
IARPMatchConfidenceAlgorithm::OverrideInstance(&m_algorithm);
}

~TestHeuristicOverride()
{
ARPCorrelationAlgorithm::ResetInstance();
IARPMatchConfidenceAlgorithm::ResetInstance();
}

private:
EmptyHeuristic m_algorithm;
EmptyMatchConfidenceAlgorithm m_algorithm;
};

TEST_CASE("ARPChanges_MSIX_Ignored", "[ARPChanges][workflow]")
Expand Down
Loading