From 9f7e200738a9d53bf01ecea231291884c3e4823b Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Thu, 13 Apr 2023 11:26:47 -0700 Subject: [PATCH 01/26] Stop removing stale pins --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 28ef649ef9..d6f0dd4624 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -701,10 +701,9 @@ namespace AppInstaller::Repository } // Gets the information about the pins that exist for this package - void GetExistingPins(PinningIndex& pinningIndex, bool cleanUpStalePins) + void GetExistingPins(PinningIndex& pinningIndex) { // If the package is installed, we need to add the pin information to the available packages from any source. - // If the package is not installed, we clean up stale pin information here. for (auto& availablePackage : m_availablePackages) { auto pinKey = GetPinKey(availablePackage.GetAvailablePackage().get()); @@ -716,10 +715,6 @@ namespace AppInstaller::Repository availablePackage.SetPin(std::move(pin.value())); } } - else if (pinningIndex.GetPin(pinKey) && cleanUpStalePins) - { - pinningIndex.RemovePin(pinKey); - } } } @@ -1093,8 +1088,6 @@ namespace AppInstaller::Repository } // Adds all the pin information to the results from a search to a CompositeSource. - // This function assumes that the CompositeSource included an InstalledSource so that we - // can clean up stale pins where the package is no longer installed. void AddPinInfoToCompositeSearchResult(CompositeResult& result) { if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && !result.Matches.empty()) @@ -1105,7 +1098,7 @@ namespace AppInstaller::Repository { for (auto& match : result.Matches) { - match.Package->GetExistingPins(*pinningIndex, /* cleanUpStalePins */ true); + match.Package->GetExistingPins(*pinningIndex); } } } From 117102ccf4dbe777944e8c83dafb867fdddb08e3 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 18 Apr 2023 16:08:23 -0700 Subject: [PATCH 02/26] Store product codes on pin table --- .../Commands/PinCommand.cpp | 1 - src/AppInstallerCLICore/Resources.h | 1 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 206 +++++++++++------- src/AppInstallerCLICore/Workflows/PinFlow.h | 2 + .../Shared/Strings/en-us/winget.resw | 4 + src/AppInstallerCommonCore/Pin.cpp | 61 +++++- .../Public/winget/Pin.h | 79 +++++-- .../CompositeSource.cpp | 72 +++--- .../Microsoft/PinningIndex.cpp | 23 +- .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 57 +++-- 10 files changed, 360 insertions(+), 146 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index 44bbc460e3..fe80d65e5c 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -262,7 +262,6 @@ namespace AppInstaller::CLI Workflow::GetAllPins << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::CrossReferencePinsWithSource << Workflow::ReportPins; } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 536dd0bd37..54928c34cc 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -308,6 +308,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinResettingAll); WINGET_DEFINE_RESOURCE_STRINGID(PinResetUseForceArg); WINGET_DEFINE_RESOURCE_STRINGID(PinType); + WINGET_DEFINE_RESOURCE_STRINGID(PinVersion); WINGET_DEFINE_RESOURCE_STRINGID(PoliciesPolicy); WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverridden); WINGET_DEFINE_RESOURCE_STRINGID(PortableHashMismatchOverrideRequired); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index c52648812b..3686be5ac5 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -16,21 +16,47 @@ namespace AppInstaller::CLI::Workflow namespace { // Creates a Pin appropriate for the context based on the arguments provided - Pinning::Pin CreatePin(Execution::Context& context, std::string_view packageId, std::string_view sourceId) + Pinning::Pin CreatePin(Execution::Context& context, const Pinning::PinKey& pinKey) { if (context.Args.Contains(Execution::Args::Type::GatedVersion)) { - return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, context.Args.GetArg(Execution::Args::Type::GatedVersion)); + return Pinning::Pin::CreateGatingPin(pinKey, context.Args.GetArg(Execution::Args::Type::GatedVersion)); } else if (context.Args.Contains(Execution::Args::Type::BlockingPin)) { - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + return Pinning::Pin::CreateBlockingPin(pinKey); } else { - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + return Pinning::Pin::CreatePinningPin(pinKey); } } + + // Gets a search request that can be used to find the installed package + // that corresponds with a pin. The search is based on the extra ID + // (Product Code or Package Family Name) if it is available; otherwise + // it uses the package ID. Using the extra ID allows us to disambiguate + // in certain cases. + SearchRequest GetSearchRequestForPin(const Pinning::PinKey& pinKey) + { + SearchRequest searchRequest; + + switch (pinKey.ExtraIdType) + { + case Pinning::ExtraIdStringType::ProductCode: + searchRequest.Filters.emplace_back(PackageMatchField::ProductCode, MatchType::CaseInsensitive, pinKey.ExtraId); + break; + case Pinning::ExtraIdStringType::PackageFamilyName: + searchRequest.Filters.emplace_back(PackageMatchField::PackageFamilyName, MatchType::CaseInsensitive, pinKey.ExtraId); + break; + case Pinning::ExtraIdStringType::None: + default: + searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pinKey.PackageId); + break; + } + + return searchRequest; + } } void OpenPinningIndex::operator()(Execution::Context& context) const @@ -57,25 +83,32 @@ namespace AppInstaller::CLI::Workflow { auto package = context.Get(); std::vector pins; - std::set sources; + std::set> sourcesAndIds; auto pinningIndex = context.Get(); auto packageVersionKeys = package->GetAvailableVersionKeys(); - for (const auto& versionKey : packageVersionKeys) + auto installedVersion = package->GetInstalledVersion(); + auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); + auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); + + for (const auto& versionKey : packageVersionKeys) { auto availableVersion = package->GetAvailableVersion(versionKey); - Pinning::PinKey pinKey{ - availableVersion->GetProperty(PackageVersionProperty::Id).get(), - availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; + const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); + const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); + const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); - if (sources.insert(pinKey.SourceId).second) + for (const auto& pinKey : pinKeys) { - auto pin = pinningIndex->GetPin(pinKey); - if (pin) + if (sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) { - pins.emplace_back(std::move(pin.value())); + auto pin = pinningIndex->GetPin(pinKey); + if (pin) + { + pins.emplace_back(std::move(pin.value())); + } } } } @@ -89,62 +122,66 @@ namespace AppInstaller::CLI::Workflow auto installedVersion = context.Get(); auto installedVersionString = installedVersion->GetProperty(PackageVersionProperty::Version); + auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); + auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); std::vector pinsToAddOrUpdate; - std::set sources; + std::set> sourcesAndIds; auto pinningIndex = context.Get(); auto packageVersionKeys = package->GetAvailableVersionKeys(); for (const auto& versionKey : packageVersionKeys) - { auto availableVersion = package->GetAvailableVersion(versionKey); - Pinning::PinKey pinKey{ - availableVersion->GetProperty(PackageVersionProperty::Id).get(), - availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; - - if (!sources.insert(pinKey.SourceId).second) - { - // We already considered the pin for this source - continue; - } - - auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId); - AICLI_LOG(CLI, Info, << "Evaluating pin with type " << ToString(pin.GetType()) << " for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); - - auto existingPin = pinningIndex->GetPin(pinKey); + const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); + const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); + const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); - if (existingPin) + for (const auto& pinKey : pinKeys) { - auto packageName = availableVersion->GetProperty(PackageVersionProperty::Name); - - // Pin already exists. - // If it is the same, we do nothing. If it is different, check for the --force arg - if (pin == existingPin) + if (!sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) { - AICLI_LOG(CLI, Info, << "Pin already exists"); - context.Reporter.Info() << Resource::String::PinAlreadyExists(packageName) << std::endl; + // We already considered the pin for this source and product code/pfn continue; } - AICLI_LOG(CLI, Info, << "Another pin already exists for the package for source " << pinKey.SourceId); - if (context.Args.Contains(Execution::Args::Type::Force)) + auto pin = CreatePin(context, pinKey); + AICLI_LOG(CLI, Info, << "Evaluating pin with type " << ToString(pin.GetType()) << " for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + + auto existingPin = pinningIndex->GetPin(pinKey); + + if (existingPin) { - AICLI_LOG(CLI, Info, << "Overwriting pin due to --force argument"); - context.Reporter.Warn() << Resource::String::PinExistsOverwriting(packageName) << std::endl; - pinsToAddOrUpdate.push_back(std::move(pin)); + auto packageName = availableVersion->GetProperty(PackageVersionProperty::Name); + + // Pin already exists. + // If it is the same, we do nothing. If it is different, check for the --force arg + if (pin == existingPin) + { + AICLI_LOG(CLI, Info, << "Pin already exists"); + context.Reporter.Info() << Resource::String::PinAlreadyExists(packageName) << std::endl; + continue; + } + + AICLI_LOG(CLI, Info, << "Another pin already exists for the package for source " << pinKey.SourceId); + if (context.Args.Contains(Execution::Args::Type::Force)) + { + AICLI_LOG(CLI, Info, << "Overwriting pin due to --force argument"); + context.Reporter.Warn() << Resource::String::PinExistsOverwriting(packageName) << std::endl; + pinsToAddOrUpdate.push_back(std::move(pin)); + } + else + { + context.Reporter.Error() << Resource::String::PinExistsUseForceArg(packageName) << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); + } } else { - context.Reporter.Error() << Resource::String::PinExistsUseForceArg(packageName) << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); + pinsToAddOrUpdate.push_back(std::move(pin)); } } - else - { - pinsToAddOrUpdate.push_back(std::move(pin)); - } } if (!pinsToAddOrUpdate.empty()) @@ -163,7 +200,7 @@ namespace AppInstaller::CLI::Workflow { auto package = context.Get(); std::vector pins; - std::set sources; + std::set> sourcesAndIds; auto pinningIndex = context.Get(); bool pinExists = false; @@ -172,21 +209,28 @@ namespace AppInstaller::CLI::Workflow // that will be the only one we get version keys from. // So, we remove pins from all sources unless one was provided. auto packageVersionKeys = package->GetAvailableVersionKeys(); - for (const auto& versionKey : packageVersionKeys) + auto installedVersion = package->GetInstalledVersion(); + auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); + auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); + + for (const auto& versionKey : packageVersionKeys) { auto availableVersion = package->GetAvailableVersion(versionKey); - Pinning::PinKey pinKey{ - availableVersion->GetProperty(PackageVersionProperty::Id).get(), - availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; + const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); + const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); + const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); - if (sources.insert(pinKey.SourceId).second) + for (const auto& pinKey : pinKeys) { - if (pinningIndex->GetPin(pinKey)) + if (sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) { - AICLI_LOG(CLI, Info, << "Removing pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); - pinningIndex->RemovePin(pinKey); - pinExists = true; + if (pinningIndex->GetPin(pinKey)) + { + AICLI_LOG(CLI, Info, << "Removing pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + pinningIndex->RemovePin(pinKey); + pinExists = true; + } } } } @@ -210,29 +254,38 @@ namespace AppInstaller::CLI::Workflow return; } - // Get a mapping of source IDs to names so that we can show something nicer - std::map sourceNames; - for (const auto& source : Repository::Source::GetCurrentSources()) - { - sourceNames[source.Identifier] = source.Name; - } - - Execution::TableOutput<4> table(context.Reporter, + Execution::TableOutput<6> table(context.Reporter, { + Resource::String::SearchName, Resource::String::SearchId, + Resource::String::SearchVersion, Resource::String::SearchSource, Resource::String::PinType, - Resource::String::SearchVersion, + Resource::String::PinVersion, }); + const auto& source = context.Get(); for (const auto& pin : pins) { - table.OutputLine({ - pin.GetPackageId(), - sourceNames[pin.GetSourceId()], - std::string{ ToString(pin.GetType()) }, - pin.GetGatedVersion().ToString(), - }); + const auto& pinKey = pin.GetKey(); + auto searchRequest = GetSearchRequestForPin(pin.GetKey()); + auto searchResult = source.Search(searchRequest); + for (const auto& match : searchResult.Matches) + { + auto installedVersion = match.Package->GetInstalledVersion(); + auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); + if (availableVersion) + { + table.OutputLine({ + searchResult.Matches[0].Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Name), + pinKey.PackageId, + searchResult.Matches[0].Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Version), + availableVersion->GetProperty(PackageVersionProperty::SourceName), + std::string{ ToString(pin.GetType()) }, + pin.GetGatedVersion().ToString(), + }); + } + } } table.Complete(); @@ -275,16 +328,15 @@ namespace AppInstaller::CLI::Workflow std::vector matchingPins; for (const auto& pin : pins) - { - SearchRequest searchRequest; - searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pin.GetPackageId()); + const auto& pinKey = pin.GetKey(); + auto searchRequest = GetSearchRequestForPin(pin.GetKey()); auto searchResult = source.Search(searchRequest); // Ensure the match comes from the right source for (const auto& match : searchResult.Matches) { - auto availableVersion = match.Package->GetAvailableVersion({ pin.GetSourceId(), "", "" }); + auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); if (availableVersion) { matchingPins.push_back(pin); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index 040ea5116e..b9a15003a2 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -46,6 +46,8 @@ namespace AppInstaller::CLI::Workflow void RemovePin(Execution::Context& context); // Report the pins in a table. + // This includes searching for the corresponding installed packages + // to be able to show more info, like the package name. // Required Args: None // Inputs: Pins // Outputs: None diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index e567fa12a4..d45c571587 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1843,4 +1843,8 @@ Please specify one of them using the --source option to proceed. Waiting for another install/uninstall to complete... + + Pinned version + Table header for the version to which a package is pinned; meaning it should not update from that version. + \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 429d572102..6b95a9b7f4 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -32,25 +32,70 @@ namespace AppInstaller::Pinning if (Utility::CaseInsensitiveEquals(in, "Blocking"sv)) { return PinType::Blocking; - } + } else if (Utility::CaseInsensitiveEquals(in, "Pinning"sv)) - { + { return PinType::Pinning; - } + } else if (Utility::CaseInsensitiveEquals(in, "Gating"sv)) - { + { return PinType::Gating; - } + } else if (Utility::CaseInsensitiveEquals(in, "PinnedByManifest"sv)) - { + { return PinType::PinnedByManifest; - } + } else - { + { return PinType::Unknown; } } + PinType StrictestPinType(PinType first, PinType second) + { + return std::max(first, second); + } + + std::string_view ToString(ExtraIdStringType type) + { + switch (type) + { + case ExtraIdStringType::ProductCode: + return "ProductCode"sv; + case ExtraIdStringType::PackageFamilyName: + return "PackageFamilyName"; + case ExtraIdStringType::None: + default: + return "None"; + } + } + + std::vector GetPinKeysForAvailablePackage( + std::string_view packageId, + std::string sourceId, + const std::vector& installedProductCodes, + const std::vector& installedPackageFamilyNames) + { + std::vector pinKeys; + + for (const auto& productCode : installedProductCodes) + { + pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::ProductCode, productCode); + } + + for (const auto& packageFamilyName : installedPackageFamilyNames) + { + pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::PackageFamilyName, packageFamilyName); + } + + if (pinKeys.empty()) + { + pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::None, ""); + } + + return pinKeys; + } + Pin Pin::CreateBlockingPin(PinKey&& pinKey) { return { PinType::Blocking, std::move(pinKey) }; diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 2382e24a47..187a79ba92 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -6,63 +6,116 @@ namespace AppInstaller::Pinning { - enum class PinType + // The pin types are ordered by how "strict" they are. + // Meaning, the one that is more restrictive goes later. + // This is used to decide which pin to report if there are multiple pins. + enum class PinType : int64_t { // Unknown pin type or not pinned Unknown, // Pinned by the manifest using the RequiresExplicitUpgrade field. // Behaves the same as Pinning pins PinnedByManifest, - // The package is blocked from 'upgrade --all' and 'upgrade '. - // User has to unblock to allow update. - Blocking, // The package is excluded from 'upgrade --all', unless '--include-pinned' is added. // 'upgrade ' is not blocked. Pinning, // The package is pinned to a specific version range. Gating, + // The package is blocked from 'upgrade --all' and 'upgrade '. + // User has to unblock to allow update. + Blocking, }; std::string_view ToString(PinType type); PinType ConvertToPinTypeEnum(std::string_view in); - // The set of values needed to uniquely identify a Pin + // Determines which of two pin types is more strict. + PinType StrictestPinType(PinType first, PinType second); + + // The type of an extra string used for identifying an installed app for pinning + enum class ExtraIdStringType + { + None, + ProductCode, + PackageFamilyName, + }; + + std::string_view ToString(ExtraIdStringType type); + + // The set of values needed to uniquely identify a Pin. + // We use both a PackageId and a ProductCode or PackageFamilyName + // to be able to deal with packages that match with multiple installed apps. + // This helps us deal better with apps that we don't match properly elsewhere. struct PinKey { PinKey() {} - PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId) - : PackageId(packageId), SourceId(sourceId) {} + PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId, ExtraIdStringType extraIdType, std::string_view extraId = {}) + : PackageId(packageId), SourceId(sourceId), ExtraIdType(extraIdType), ExtraId(extraId) {} bool operator==(const PinKey& other) const { - return PackageId == other.PackageId && SourceId == other.SourceId; + return PackageId == other.PackageId + && SourceId == other.SourceId + && ExtraIdType == other.ExtraIdType + && ExtraId == other.ExtraId; } + bool operator!=(const PinKey& other) const { return !(*this == other); } + bool operator<(const PinKey& other) const { - return PackageId < other.PackageId || (PackageId == other.PackageId && SourceId < other.SourceId); + if (PackageId != other.PackageId) + { + return PackageId < other.PackageId; + } + else if (SourceId != other.SourceId) + { + return SourceId < other.SourceId; + } + else if (ExtraIdType != other.ExtraIdType) + { + return ExtraIdType < other.ExtraIdType; + } + else + { + return ExtraId < other.ExtraId; + } } - Manifest::Manifest::string_t PackageId; - std::string SourceId; + const Manifest::Manifest::string_t PackageId; + const std::string SourceId; + const ExtraIdStringType ExtraIdType = ExtraIdStringType::None; + const std::string ExtraId; }; + // Gets a list of the possible pin keys that we may use for an available package + // given the ProductCodes/PackageFamilyNames we know for the installed version. + // If there is no ProductCode/PackageFamilyName, returns a single element with no extra id. + std::vector GetPinKeysForAvailablePackage( + std::string_view packageId, + std::string sourceId, + const std::vector& installedProductCodes, + const std::vector& installedPackageFamilyNames); + struct Pin { static Pin CreateBlockingPin(PinKey&& pinKey); static Pin CreatePinningPin(PinKey&& pinKey); static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion&& gatedVersion); + static Pin CreateBlockingPin(const PinKey& pinKey) { return CreateBlockingPin(PinKey{ pinKey }); } + static Pin CreatePinningPin(const PinKey& pinKey) { return CreatePinningPin(PinKey{ pinKey }); } + static Pin CreateGatingPin(const PinKey& pinKey, const Utility::GatedVersion& gatedVersion) { return CreateGatingPin(PinKey{ pinKey }, Utility::GatedVersion{ gatedVersion }); } + PinType GetType() const { return m_type; } const PinKey& GetKey() const { return m_key; } - const Manifest::Manifest::string_t& GetPackageId() const { return m_key.PackageId; } - const std::string& GetSourceId() const { return m_key.SourceId; } const Utility::GatedVersion& GetGatedVersion() const { return m_gatedVersion; } bool operator==(const Pin& other) const; + bool operator<(const Pin& other) const { return m_key < other.m_key; } private: Pin(PinType type, PinKey&& pinKey, Utility::GatedVersion&& gatedVersion = {}) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index d6f0dd4624..2b2f27b886 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -22,14 +22,6 @@ namespace AppInstaller::Repository }; } - Pinning::PinKey GetPinKey(IPackage* availablePackage) - { - return { - availablePackage->GetProperty(PackageProperty::Id).get(), - availablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins)->GetSource().GetIdentifier() - }; - } - std::optional GetLatestAvailableVersionKeySatisfyingPin(const std::vector& availableVersionKeys, PinBehavior pinBehavior) { if (availableVersionKeys.empty()) @@ -371,11 +363,13 @@ namespace AppInstaller::Repository // Wrapper around an available package to add pinning functionality for composite packages. // Most of the methods are only here for completeness of the interface and are not actually used. + // Pins are tied to product codes and package family names, of which an installed package can + // have multiple, so an available package can also have multiple pins. struct CompositeAvailablePackage : public IPackage { CompositeAvailablePackage() {} - CompositeAvailablePackage(std::shared_ptr availablePackage, std::optional pin = {}) - : m_availablePackage(availablePackage), m_pin(pin) + CompositeAvailablePackage(std::shared_ptr availablePackage, std::set&& pins = {}) + : m_availablePackage(availablePackage), m_pins(std::move(pins)) { auto latestAvailable = m_availablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins); if (latestAvailable) @@ -394,14 +388,14 @@ namespace AppInstaller::Repository return m_availablePackage; } - const std::optional& GetPin() const + const std::set& GetPins() const { - return m_pin; + return m_pins; } - void SetPin(Pinning::Pin&& pin) + void AddPin(Pinning::Pin&& pin) { - m_pin = std::move(pin); + m_pins.emplace(std::move(pin)); } Utility::LocIndString GetProperty(PackageProperty property) const override @@ -417,15 +411,18 @@ namespace AppInstaller::Repository std::vector GetAvailableVersionKeys() const override { auto result = m_availablePackage->GetAvailableVersionKeys(); - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value()) + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning)) { - for (auto& pvk : result) + for (const auto& pin : m_pins) { - if (m_pin->GetType() == Pinning::PinType::Blocking || - m_pin->GetType() == Pinning::PinType::Pinning || - (m_pin->GetType() == Pinning::PinType::Gating && !m_pin->GetGatedVersion().IsValidVersion(pvk.Version))) + for (auto& pvk : result) { - pvk.PinnedState = m_pin->GetType(); + if (pin.GetType() == Pinning::PinType::Blocking || + pin.GetType() == Pinning::PinType::Pinning || + (pin.GetType() == Pinning::PinType::Gating && !pin.GetGatedVersion().IsValidVersion(pvk.Version))) + { + pvk.PinnedState = Pinning::StrictestPinType(pin.GetType(), pvk.PinnedState); + } } } } @@ -454,12 +451,15 @@ namespace AppInstaller::Repository { Pinning::PinType pinType = Pinning::PinType::Unknown; - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value()) + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning)) { - // A gating pin behaves the same as no pin when the version fits the gated version - if (!(pinType == Pinning::PinType::Gating && m_pin->GetGatedVersion().IsValidVersion(versionKey.Version))) + for (const auto& pin : m_pins) { - pinType = m_pin->GetType(); + // A gating pin behaves the same as no pin when the version fits the gated version + if (!(pin.GetType() == Pinning::PinType::Gating && pin.GetGatedVersion().IsValidVersion(versionKey.Version))) + { + pinType = Pinning::StrictestPinType(pinType, pin.GetType()); + } } } @@ -479,7 +479,7 @@ namespace AppInstaller::Repository { return m_sourceId == otherAvailable->m_sourceId && - m_pin == otherAvailable->m_pin && + m_pins == otherAvailable->m_pins && m_availablePackage->IsSame(otherAvailable->m_availablePackage.get()); } @@ -489,7 +489,7 @@ namespace AppInstaller::Repository private: std::string m_sourceId; std::shared_ptr m_availablePackage; - std::optional m_pin; + std::set m_pins; }; // A composite package for the CompositeSource. @@ -703,16 +703,30 @@ namespace AppInstaller::Repository // Gets the information about the pins that exist for this package void GetExistingPins(PinningIndex& pinningIndex) { + if (!m_installedPackage) + { + // Only installed packages have pins + return; + } + + auto installedProductCodes = m_installedPackage->GetInstalledVersion()->GetMultiProperty(PackageVersionMultiProperty::ProductCode); + auto installedPackageFamilyNames = m_installedPackage->GetInstalledVersion()->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); + // If the package is installed, we need to add the pin information to the available packages from any source. for (auto& availablePackage : m_availablePackages) { - auto pinKey = GetPinKey(availablePackage.GetAvailablePackage().get()); - if (m_installedPackage) + auto pinKeys = Pinning::GetPinKeysForAvailablePackage( + availablePackage.GetAvailablePackage()->GetProperty(PackageProperty::Id).get(), + availablePackage.GetAvailablePackage()->GetLatestAvailableVersion(PinBehavior::IgnorePins)->GetSource().GetIdentifier(), + installedProductCodes, + installedPackageFamilyNames); + + for (const auto& pinKey : pinKeys) { auto pin = pinningIndex.GetPin(pinKey); if (pin.has_value()) { - availablePackage.SetPin(std::move(pin.value())); + availablePackage.AddPin(std::move(pin.value())); } } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index bb307b4a7e..dd22e4a3d3 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -7,6 +7,23 @@ namespace AppInstaller::Repository::Microsoft { + namespace + { + // Gets a string representing a pin key, which is to be used for logging + std::string GetPinKeyLogString(const Pinning::PinKey& pinKey) + { + std::stringstream ss; + ss << "Package=[" << pinKey.PackageId << "] Source=[" << pinKey.SourceId << "]"; + + if (pinKey.ExtraIdType != Pinning::ExtraIdStringType::None) + { + ss << ' ' << Pinning::ToString(pinKey.ExtraIdType) << "=[" << pinKey.ExtraId << "]"; + } + + return ss.str(); + } + } + PinningIndex PinningIndex::CreateNew(const std::string& filePath, Schema::Version version) { AICLI_LOG(Repo, Info, << "Creating new Pinning Index [" << version << "] at '" << filePath << "'"); @@ -74,7 +91,7 @@ namespace AppInstaller::Repository::Microsoft PinningIndex::IdType PinningIndex::AddPin(const Pinning::Pin& pin) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Adding Pin for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "] with pin type " << Pinning::ToString(pin.GetType())); + AICLI_LOG(Repo, Verbose, << "Adding Pin for " << GetPinKeyLogString(pin.GetKey()) << " with pin type " << Pinning::ToString(pin.GetType())); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_addpin"); @@ -90,7 +107,7 @@ namespace AppInstaller::Repository::Microsoft bool PinningIndex::UpdatePin(const Pinning::Pin& pin) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Updating Pin for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "] with pin type " << Pinning::ToString(pin.GetType())); + AICLI_LOG(Repo, Verbose, << "Updating Pin for " << GetPinKeyLogString(pin.GetKey()) << " with pin type " << Pinning::ToString(pin.GetType())); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_updatepin"); @@ -121,7 +138,7 @@ namespace AppInstaller::Repository::Microsoft void PinningIndex::RemovePin(const Pinning::PinKey& pinKey) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Removing Pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + AICLI_LOG(Repo, Verbose, << "Removing Pin for package " << GetPinKeyLogString(pinKey)); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_removePin"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index 5425556d5e..710b118fcc 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -9,17 +9,23 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { namespace { - std::optional GetPinFromRow(std::string_view packageId, std::string_view sourceId, Pinning::PinType type, std::string_view version) + std::optional GetPinFromRow( + std::string_view packageId, + std::string_view sourceId, + Pinning::ExtraIdStringType extraIdType, + std::string_view extraId, + Pinning::PinType type, + std::string_view version) { switch (type) { case Pinning::PinType::Blocking: - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId, extraIdType, extraId }); case Pinning::PinType::Pinning: - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId, extraIdType, extraId }); case Pinning::PinType::Gating: - return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ version }); + return Pinning::Pin::CreateGatingPin({ packageId, sourceId, extraIdType, extraId }, Utility::GatedVersion{ version }); default: return {}; } @@ -30,6 +36,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 static constexpr std::string_view s_PinTable_Table_Name = "pin"sv; static constexpr std::string_view s_PinTable_PackageId_Column = "package_id"sv; static constexpr std::string_view s_PinTable_SourceId_Column = "source_id"sv; + static constexpr std::string_view s_PinTable_ExtraIdType_Column = "extra_id_type"sv; + static constexpr std::string_view s_PinTable_ExtraId_Column = "extra_id"sv; static constexpr std::string_view s_PinTable_Type_Column = "type"sv; static constexpr std::string_view s_PinTable_Version_Column = "version"sv; static constexpr std::string_view s_PinTable_Index = "pin_index"sv; @@ -50,15 +58,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 createTableBuilder.Column(ColumnBuilder(s_PinTable_PackageId_Column, Type::Text).NotNull()); createTableBuilder.Column(ColumnBuilder(s_PinTable_SourceId_Column, Type::Text).NotNull()); + createTableBuilder.Column(ColumnBuilder(s_PinTable_ExtraIdType_Column, Type::Int64).NotNull()); + createTableBuilder.Column(ColumnBuilder(s_PinTable_ExtraId_Column, Type::Text).NotNull()); createTableBuilder.Column(ColumnBuilder(s_PinTable_Type_Column, Type::Int64).NotNull()); - createTableBuilder.Column(ColumnBuilder(s_PinTable_Version_Column, Type::Text)); + createTableBuilder.Column(ColumnBuilder(s_PinTable_Version_Column, Type::Text).NotNull()); createTableBuilder.EndColumns(); createTableBuilder.Execute(connection); // Create an index over the pairs package,source StatementBuilder createIndexBuilder; - createIndexBuilder.CreateUniqueIndex(s_PinTable_Index).On(s_PinTable_Table_Name).Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column }); + createIndexBuilder.CreateUniqueIndex(s_PinTable_Index).On(s_PinTable_Table_Name) + .Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, s_PinTable_ExtraIdType_Column, s_PinTable_ExtraId_Column }); createIndexBuilder.Execute(connection); savepoint.Commit(); @@ -69,7 +80,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::Builder::StatementBuilder builder; builder.Select(SQLite::RowIDName).From(s_PinTable_Table_Name) .Where(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) - .And(s_PinTable_SourceId_Column).Equals((std::string_view)pinKey.SourceId); + .And(s_PinTable_SourceId_Column).Equals((std::string_view)pinKey.SourceId) + .And(s_PinTable_ExtraIdType_Column).Equals(pinKey.ExtraIdType) + .And(s_PinTable_ExtraId_Column).Equals((std::string_view)pinKey.ExtraId); SQLite::Statement select = builder.Prepare(connection); @@ -86,15 +99,20 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::rowid_t PinTable::AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) { SQLite::Builder::StatementBuilder builder; + const auto& pinKey = pin.GetKey(); builder.InsertInto(s_PinTable_Table_Name) .Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, + s_PinTable_ExtraIdType_Column, + s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .Values( - (std::string_view)pin.GetPackageId(), - pin.GetSourceId(), + (std::string_view)pinKey.PackageId, + pinKey.SourceId, + pinKey.ExtraIdType, + pinKey.ExtraId, pin.GetType(), pin.GetGatedVersion().ToString()); @@ -105,9 +123,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 bool PinTable::UpdatePinById(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin) { SQLite::Builder::StatementBuilder builder; + const auto& pinKey = pin.GetKey(); builder.Update(s_PinTable_Table_Name).Set() - .Column(s_PinTable_PackageId_Column).Equals((std::string_view)pin.GetPackageId()) - .Column(s_PinTable_SourceId_Column).Equals(pin.GetSourceId()) + .Column(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) + .Column(s_PinTable_SourceId_Column).Equals(pinKey.SourceId) + .Column(s_PinTable_ExtraIdType_Column).Equals(pinKey.ExtraIdType) + .Column(s_PinTable_ExtraId_Column).Equals(pinKey.ExtraId) .Column(s_PinTable_Type_Column).Equals(pin.GetType()) .Column(s_PinTable_Version_Column).Equals(pin.GetGatedVersion().ToString()) .Where(SQLite::RowIDName).Equals(pinId); @@ -129,6 +150,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 builder.Select({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, + s_PinTable_ExtraIdType_Column, + s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .From(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); @@ -140,8 +163,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 return {}; } - auto [packageId, sourceId, type, gatedVersion] = select.GetRow(); - return GetPinFromRow(packageId, sourceId, type, gatedVersion); + auto [packageId, sourceId, extraIdType, extraId, pinType, gatedVersion] = + select.GetRow(); + return GetPinFromRow(packageId, sourceId, extraIdType, extraId, pinType, gatedVersion); } std::vector PinTable::GetAllPins(SQLite::Connection& connection) @@ -150,6 +174,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 builder.Select({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, + s_PinTable_ExtraIdType_Column, + s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .From(s_PinTable_Table_Name); @@ -159,8 +185,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 std::vector pins; while (select.Step()) { - auto [packageId, sourceId, type, gatedVersion] = select.GetRow(); - auto pin = GetPinFromRow(packageId, sourceId, type, gatedVersion); + auto [packageId, sourceId, extraIdType, extraId, pinType, gatedVersion] = + select.GetRow(); + auto pin = GetPinFromRow(packageId, sourceId, extraIdType, extraId, pinType, gatedVersion); if (pin) { pins.push_back(std::move(pin.value())); From 8ff23c226c304f8e8d5febc7779b68324b5b2cb1 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 18 Apr 2023 16:38:45 -0700 Subject: [PATCH 03/26] Add TODOs to tests --- src/AppInstallerCLITests/CompositeSource.cpp | 6 +++--- src/AppInstallerCLITests/PinFlow.cpp | 4 ++-- src/AppInstallerCLITests/PinningIndex.cpp | 17 ++++++++--------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index 629e6f8300..efe815a5db 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1116,7 +1116,7 @@ TEST_CASE("CompositeSource_PinnedAvailable", "[CompositeSource][PinFlow]") // The result when ignoring pins is always the same expectedResult.ResultsForPinBehavior[PinBehavior::IgnorePins] = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "1.1.0" }; - PinKey pinKey("Id", setup.Available->Details.Identifier); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); @@ -1232,7 +1232,7 @@ TEST_CASE("CompositeSource_OneSourcePinned", "[CompositeSource][PinFlow]") }; { - PinKey pinKey("Id", setup.Available->Details.Identifier); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreatePinningPin(PinKey{ pinKey })); @@ -1295,7 +1295,7 @@ TEST_CASE("CompositeSource_OneSourceGated", "[CompositeSource][PinFlow]") }; { - PinKey pinKey("Id", setup.Available->Details.Identifier); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreateGatingPin(PinKey{ pinKey }, GatedVersion{ "1.*"sv })); diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index a3cc70a285..edd387b7c0 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -49,9 +49,9 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") auto pins = index.GetAllPins(); REQUIRE(pins.size() == 1); REQUIRE(pins[0].GetType() == PinType::Blocking); - REQUIRE(pins[0].GetPackageId() == "AppInstallerCliTest.TestExeInstaller"); - REQUIRE(pins[0].GetSourceId() == "*TestSource"); REQUIRE(pins[0].GetGatedVersion().ToString() == ""); + REQUIRE(pins[0].GetKey().PackageId == "AppInstallerCliTest.TestExeInstaller"); + REQUIRE(pins[0].GetKey().SourceId == "*TestSource"); std::ostringstream pinListOutput; TestContext listContext{ pinListOutput, std::cin }; diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index c0216e204d..e995b459bf 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -57,7 +57,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId" }); + Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId", ExtraIdStringType::None, "" /* TODO */ }); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -77,8 +77,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") REQUIRE(pinFromIndex.value() == pin); REQUIRE(pinFromIndex->GetType() == pin.GetType()); - REQUIRE(pinFromIndex->GetPackageId() == pin.GetPackageId()); - REQUIRE(pinFromIndex->GetSourceId() == pin.GetSourceId()); + REQUIRE(pinFromIndex->GetKey() == pin.GetKey()); } { @@ -99,8 +98,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId" }, { "1.0.*"sv }); - Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId" }); + Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId", ExtraIdStringType::None, "" /* TODO */ }, { "1.0.*"sv }); + Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId", ExtraIdStringType::None, "" /* TODO */ }); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -133,8 +132,8 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1" }); - Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2" }); + Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1", ExtraIdStringType::None, "" /* TODO */ }); + Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2", ExtraIdStringType::None, "" /* TODO */ }); // Add two pins to the index, then check that they show up when queried PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -144,7 +143,7 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") REQUIRE(index.GetAllPins().size() == 2); REQUIRE(index.GetPin(pin1.GetKey()).has_value()); REQUIRE(index.GetPin(pin2.GetKey()).has_value()); - REQUIRE(!index.GetPin({ "pkg", "src" }).has_value()); + REQUIRE(!index.GetPin({ "pkg", "src", ExtraIdStringType::None, "" /* TODO */ }).has_value()); // Reset the index, then check that there are no pins index.ResetAllPins(); @@ -158,7 +157,7 @@ TEST_CASE("PinningIndex_AddDuplicatePin", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkg", "src" }, { "1.*"sv }); + Pin pin = Pin::CreateGatingPin({ "pkg", "src", ExtraIdStringType::None, "" /* TODO */ }, { "1.*"sv }); PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); index.AddPin(pin); From a52fdc6ccd3f6a45fc8683c7d5daca59c230f86a Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 18 Apr 2023 16:44:26 -0700 Subject: [PATCH 04/26] Remove test TODOs --- src/AppInstallerCLITests/CompositeSource.cpp | 6 +++--- src/AppInstallerCLITests/PinningIndex.cpp | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index efe815a5db..8e13add468 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1116,7 +1116,7 @@ TEST_CASE("CompositeSource_PinnedAvailable", "[CompositeSource][PinFlow]") // The result when ignoring pins is always the same expectedResult.ResultsForPinBehavior[PinBehavior::IgnorePins] = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "1.1.0" }; - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); @@ -1232,7 +1232,7 @@ TEST_CASE("CompositeSource_OneSourcePinned", "[CompositeSource][PinFlow]") }; { - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreatePinningPin(PinKey{ pinKey })); @@ -1295,7 +1295,7 @@ TEST_CASE("CompositeSource_OneSourceGated", "[CompositeSource][PinFlow]") }; { - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, "" /* TODO */); + PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreateGatingPin(PinKey{ pinKey }, GatedVersion{ "1.*"sv })); diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index e995b459bf..acf8a9f6d3 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -57,7 +57,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId", ExtraIdStringType::None, "" /* TODO */ }); + Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId", ExtraIdStringType::None, "" }); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -98,8 +98,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId", ExtraIdStringType::None, "" /* TODO */ }, { "1.0.*"sv }); - Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId", ExtraIdStringType::None, "" /* TODO */ }); + Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId", ExtraIdStringType::None, ""}, { "1.0.*"sv }); + Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId", ExtraIdStringType::None, ""}); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -132,8 +132,8 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1", ExtraIdStringType::None, "" /* TODO */ }); - Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2", ExtraIdStringType::None, "" /* TODO */ }); + Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1", ExtraIdStringType::None, "" }); + Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2", ExtraIdStringType::None, "" }); // Add two pins to the index, then check that they show up when queried PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -143,7 +143,7 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") REQUIRE(index.GetAllPins().size() == 2); REQUIRE(index.GetPin(pin1.GetKey()).has_value()); REQUIRE(index.GetPin(pin2.GetKey()).has_value()); - REQUIRE(!index.GetPin({ "pkg", "src", ExtraIdStringType::None, "" /* TODO */ }).has_value()); + REQUIRE(!index.GetPin({ "pkg", "src", ExtraIdStringType::None, "" }).has_value()); // Reset the index, then check that there are no pins index.ResetAllPins(); @@ -157,7 +157,7 @@ TEST_CASE("PinningIndex_AddDuplicatePin", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkg", "src", ExtraIdStringType::None, "" /* TODO */ }, { "1.*"sv }); + Pin pin = Pin::CreateGatingPin({ "pkg", "src", ExtraIdStringType::None, "" }, { "1.*"sv }); PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); index.AddPin(pin); From 553f4290016ad889d51cca207a3770536872e4cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flor=20Chac=C3=B3n?= <14323496+florelis@users.noreply.github.com> Date: Wed, 19 Apr 2023 11:50:14 -0700 Subject: [PATCH 05/26] Update src/AppInstallerCommonCore/Public/winget/Pin.h Co-authored-by: Kaleb Luedtke --- .../Public/winget/Pin.h | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 187a79ba92..c8ce76f72d 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -67,22 +67,10 @@ namespace AppInstaller::Pinning bool operator<(const PinKey& other) const { - if (PackageId != other.PackageId) - { - return PackageId < other.PackageId; - } - else if (SourceId != other.SourceId) - { - return SourceId < other.SourceId; - } - else if (ExtraIdType != other.ExtraIdType) - { - return ExtraIdType < other.ExtraIdType; - } - else - { - return ExtraId < other.ExtraId; - } + // std::tie implements tuple comparison, wherein it checks the first item in the tuple, + // iff the first elements are equal, then the second element is used for comparison, and so on + return std::tie(PackageId, SourceId, ExtraIdType, ExtraId) < + std::tie(other.PackageId, other.SourceId, other.ExtraIdType, other.ExtraId); } const Manifest::Manifest::string_t PackageId; From 3bccb45dc36830ae12f267b7a8437be373328d2b Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 19 Apr 2023 12:18:36 -0700 Subject: [PATCH 06/26] Add ToString methods --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 4 +- src/AppInstallerCommonCore/Pin.cpp | 61 +++++++++++++------ .../Public/winget/Pin.h | 18 ++++-- .../Microsoft/PinningIndex.cpp | 23 +------ .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 2 +- 5 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 3686be5ac5..2a3a8e1d44 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -147,7 +147,7 @@ namespace AppInstaller::CLI::Workflow } auto pin = CreatePin(context, pinKey); - AICLI_LOG(CLI, Info, << "Evaluating pin with type " << ToString(pin.GetType()) << " for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + AICLI_LOG(CLI, Info, << "Evaluating Pin " << pin.ToString()); auto existingPin = pinningIndex->GetPin(pinKey); @@ -227,7 +227,7 @@ namespace AppInstaller::CLI::Workflow { if (pinningIndex->GetPin(pinKey)) { - AICLI_LOG(CLI, Info, << "Removing pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + AICLI_LOG(CLI, Info, << "Removing Pin " << pin.ToString()); pinningIndex->RemovePin(pinKey); pinExists = true; } diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 6b95a9b7f4..bc58b933cc 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -9,22 +9,9 @@ namespace AppInstaller::Pinning { using namespace std::string_view_literals; - std::string_view ToString(PinType type) + PinType StrictestPinType(PinType first, PinType second) { - switch (type) - { - case PinType::Blocking: - return "Blocking"sv; - case PinType::Pinning: - return "Pinning"sv; - case PinType::Gating: - return "Gating"sv; - case PinType::PinnedByManifest: - return "PinnedByManifest"sv; - case PinType::Unknown: - default: - return "Unknown"; - } + return std::max(first, second); } PinType ConvertToPinTypeEnum(std::string_view in) @@ -51,9 +38,22 @@ namespace AppInstaller::Pinning } } - PinType StrictestPinType(PinType first, PinType second) + std::string_view ToString(PinType type) { - return std::max(first, second); + switch (type) + { + case PinType::Blocking: + return "Blocking"sv; + case PinType::Pinning: + return "Pinning"sv; + case PinType::Gating: + return "Gating"sv; + case PinType::PinnedByManifest: + return "PinnedByManifest"sv; + case PinType::Unknown: + default: + return "Unknown"; + } } std::string_view ToString(ExtraIdStringType type) @@ -70,6 +70,33 @@ namespace AppInstaller::Pinning } } + std::string PinKey::ToString() const + { + std::stringstream ss; + ss << "Package=[" << PackageId << "] Source=[" << SourceId << "]"; + + if (ExtraIdType != Pinning::ExtraIdStringType::None) + { + ss << ' ' << Pinning::ToString(ExtraIdType) << "=[" << ExtraId << "]"; + } + + return ss.str(); + } + + std::string Pin::ToString() const + { + std::stringstream ss; + ss << m_key.ToString() << " Type=[" << Pinning::ToString(m_type) << ']'; + + if (m_type == PinType::Gating) + { + ss << " GatedVersion=[" << m_gatedVersion.ToString() << ']'; + } + + return ss.str(); + + } + std::vector GetPinKeysForAvailablePackage( std::string_view packageId, std::string sourceId, diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index c8ce76f72d..656c1d3079 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -26,12 +26,6 @@ namespace AppInstaller::Pinning Blocking, }; - std::string_view ToString(PinType type); - PinType ConvertToPinTypeEnum(std::string_view in); - - // Determines which of two pin types is more strict. - PinType StrictestPinType(PinType first, PinType second); - // The type of an extra string used for identifying an installed app for pinning enum class ExtraIdStringType { @@ -40,8 +34,14 @@ namespace AppInstaller::Pinning PackageFamilyName, }; + std::string_view ToString(PinType type); + PinType ConvertToPinTypeEnum(std::string_view in); + std::string_view ToString(ExtraIdStringType type); + // Determines which of two pin types is more strict. + PinType StrictestPinType(PinType first, PinType second); + // The set of values needed to uniquely identify a Pin. // We use both a PackageId and a ProductCode or PackageFamilyName // to be able to deal with packages that match with multiple installed apps. @@ -73,6 +73,9 @@ namespace AppInstaller::Pinning std::tie(other.PackageId, other.SourceId, other.ExtraIdType, other.ExtraId); } + // Used for logging + std::string ToString() const; + const Manifest::Manifest::string_t PackageId; const std::string SourceId; const ExtraIdStringType ExtraIdType = ExtraIdStringType::None; @@ -105,6 +108,9 @@ namespace AppInstaller::Pinning bool operator==(const Pin& other) const; bool operator<(const Pin& other) const { return m_key < other.m_key; } + // Used for logging + std::string ToString() const; + private: Pin(PinType type, PinKey&& pinKey, Utility::GatedVersion&& gatedVersion = {}) : m_type(type), m_key(std::move(pinKey)), m_gatedVersion(std::move(gatedVersion)) {} diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index dd22e4a3d3..72754cbb21 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -7,23 +7,6 @@ namespace AppInstaller::Repository::Microsoft { - namespace - { - // Gets a string representing a pin key, which is to be used for logging - std::string GetPinKeyLogString(const Pinning::PinKey& pinKey) - { - std::stringstream ss; - ss << "Package=[" << pinKey.PackageId << "] Source=[" << pinKey.SourceId << "]"; - - if (pinKey.ExtraIdType != Pinning::ExtraIdStringType::None) - { - ss << ' ' << Pinning::ToString(pinKey.ExtraIdType) << "=[" << pinKey.ExtraId << "]"; - } - - return ss.str(); - } - } - PinningIndex PinningIndex::CreateNew(const std::string& filePath, Schema::Version version) { AICLI_LOG(Repo, Info, << "Creating new Pinning Index [" << version << "] at '" << filePath << "'"); @@ -91,7 +74,7 @@ namespace AppInstaller::Repository::Microsoft PinningIndex::IdType PinningIndex::AddPin(const Pinning::Pin& pin) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Adding Pin for " << GetPinKeyLogString(pin.GetKey()) << " with pin type " << Pinning::ToString(pin.GetType())); + AICLI_LOG(Repo, Verbose, << "Adding Pin " << pin.ToString()); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_addpin"); @@ -107,7 +90,7 @@ namespace AppInstaller::Repository::Microsoft bool PinningIndex::UpdatePin(const Pinning::Pin& pin) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Updating Pin for " << GetPinKeyLogString(pin.GetKey()) << " with pin type " << Pinning::ToString(pin.GetType())); + AICLI_LOG(Repo, Verbose, << "Updating Pin " << pin.ToString()); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_updatepin"); @@ -138,7 +121,7 @@ namespace AppInstaller::Repository::Microsoft void PinningIndex::RemovePin(const Pinning::PinKey& pinKey) { std::lock_guard lockInterface{ *m_interfaceLock }; - AICLI_LOG(Repo, Verbose, << "Removing Pin for package " << GetPinKeyLogString(pinKey)); + AICLI_LOG(Repo, Verbose, << "Removing Pin " << pinKey.ToString()); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_removePin"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp index 354dca15b2..0882537366 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp @@ -14,7 +14,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 if (!result) { - AICLI_LOG(Repo, Verbose, << "Did not find a pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + AICLI_LOG(Repo, Verbose, << "Did not find pin " << pinKey.ToString()); } return result; From c25ce81870e28acc85c284dafba4f00426b8bde5 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 19 Apr 2023 13:21:22 -0700 Subject: [PATCH 07/26] Increase pinning index schema version --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 4 ++-- src/AppInstallerCLITests/PinningIndex.cpp | 2 +- .../AppInstallerRepositoryCore.vcxproj | 8 ++++---- .../AppInstallerRepositoryCore.vcxproj.filters | 16 ++++++++-------- .../Microsoft/PinningIndex.cpp | 17 +++++++++++++++-- .../{Pinning_1_0 => Pinning_1_1}/PinTable.cpp | 0 .../{Pinning_1_0 => Pinning_1_1}/PinTable.h | 0 .../PinningIndexInterface.h | 0 .../PinningIndexInterface_1_1.cpp} | 4 ++-- 9 files changed, 32 insertions(+), 19 deletions(-) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_0 => Pinning_1_1}/PinTable.cpp (100%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_0 => Pinning_1_1}/PinTable.h (100%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_0 => Pinning_1_1}/PinningIndexInterface.h (100%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_0/PinningIndexInterface_1_0.cpp => Pinning_1_1/PinningIndexInterface_1_1.cpp} (94%) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 2a3a8e1d44..cb2f4cabf8 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -50,7 +50,7 @@ namespace AppInstaller::CLI::Workflow searchRequest.Filters.emplace_back(PackageMatchField::PackageFamilyName, MatchType::CaseInsensitive, pinKey.ExtraId); break; case Pinning::ExtraIdStringType::None: - default: + default: searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pinKey.PackageId); break; } @@ -227,7 +227,7 @@ namespace AppInstaller::CLI::Workflow { if (pinningIndex->GetPin(pinKey)) { - AICLI_LOG(CLI, Info, << "Removing Pin " << pin.ToString()); + AICLI_LOG(CLI, Info, << "Removing Pin " << pinKey.ToString()); pinningIndex->RemovePin(pinKey); pinExists = true; } diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index acf8a9f6d3..7fa7a48605 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -4,7 +4,7 @@ #include "TestCommon.h" #include #include -#include +#include #include using namespace std::string_literals; diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj index a193e562f6..7b479c206b 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -277,8 +277,8 @@ - - + + @@ -368,8 +368,8 @@ - - + + diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters index a1baf663b9..c3cbf899bc 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -76,7 +76,7 @@ {d9d70cf5-ce04-4db2-a0ec-970dd0ad22b6} - + {f05e19bb-2161-4ab0-9d04-2dfa2d3eb3c6} @@ -348,11 +348,11 @@ Microsoft - - Microsoft\Schema\Pinning_1_0 + + Microsoft\Schema\Pinning_1_1 - - Microsoft\Schema\Pinning_1_0 + + Microsoft\Schema\Pinning_1_1 Public\winget @@ -560,11 +560,11 @@ Source Files - + Source Files - - Microsoft\Schema\Pinning_1_0 + + Microsoft\Schema\Pinning_1_1 Source Files diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index 72754cbb21..2eca16c10d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -3,7 +3,7 @@ #include "pch.h" #include "PinningIndex.h" #include "SQLiteStorageBase.h" -#include "Schema/Pinning_1_0/PinningIndexInterface.h" +#include "Schema/Pinning_1_1/PinningIndexInterface.h" namespace AppInstaller::Repository::Microsoft { @@ -55,7 +55,20 @@ namespace AppInstaller::Repository::Microsoft try { AICLI_LOG(Repo, Info, << "Opening existing pinning index"); - return std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); + auto pinningIndex = std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); + + // Version 1.1 of the schema is a breaking change from version 1.0. + // Since v1.0 was only available during preview, we don't need to ensure + // backwards compatibility. So, we delete any old versions of the index. + const Schema::Version DeprecatedPinningIndexVersion = { 1, 0 }; + if (pinningIndex->GetVersion() == DeprecatedPinningIndexVersion) + { + AICLI_LOG(Repo, Warning, << "Existing pinning index has a deprecated schema version. Will delete existing index"); + } + else + { + return pinningIndex; + } } CATCH_LOG(); } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp similarity index 100% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h similarity index 100% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h similarity index 100% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp similarity index 94% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp index 0882537366..f3cacb0706 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" -#include "Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h" -#include "Microsoft/Schema/Pinning_1_0/PinTable.h" +#include "Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h" +#include "Microsoft/Schema/Pinning_1_1/PinTable.h" namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { From 4ebfef2c22c7d2eae9642c4fad9b3a97fd391fb4 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 19 Apr 2023 14:52:27 -0700 Subject: [PATCH 08/26] Improve SQL schema version logging --- .../Microsoft/PinningIndex.cpp | 2 +- .../Microsoft/PortableIndex.cpp | 2 +- src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp | 2 +- .../Microsoft/Schema/Version.cpp | 9 ++++++++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index 2eca16c10d..b84a68d7fb 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -9,7 +9,7 @@ namespace AppInstaller::Repository::Microsoft { PinningIndex PinningIndex::CreateNew(const std::string& filePath, Schema::Version version) { - AICLI_LOG(Repo, Info, << "Creating new Pinning Index [" << version << "] at '" << filePath << "'"); + AICLI_LOG(Repo, Info, << "Creating new Pinning Index with version [" << version << "] at '" << filePath << "'"); PinningIndex result{ filePath, version }; SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "pinningindex_createnew"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/PortableIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PortableIndex.cpp index f3bb074715..ad22d2d302 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PortableIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PortableIndex.cpp @@ -10,7 +10,7 @@ namespace AppInstaller::Repository::Microsoft { PortableIndex PortableIndex::CreateNew(const std::string& filePath, Schema::Version version) { - AICLI_LOG(Repo, Info, << "Creating new Portable Index [" << version << "] at '" << filePath << "'"); + AICLI_LOG(Repo, Info, << "Creating new Portable Index with version [" << version << "] at '" << filePath << "'"); PortableIndex result{ filePath, version }; SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "portableindex_createnew"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp index 24df0fa6e2..1a43480f4d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -18,7 +18,7 @@ namespace AppInstaller::Repository::Microsoft { SQLiteIndex SQLiteIndex::CreateNew(const std::string& filePath, Schema::Version version, CreateOptions options) { - AICLI_LOG(Repo, Info, << "Creating new SQLite Index [" << version << "] at '" << filePath << "'"); + AICLI_LOG(Repo, Info, << "Creating new SQLite Index with version [" << version << "] at '" << filePath << "'"); SQLiteIndex result{ filePath, version }; SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "sqliteindex_createnew"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp index f5a7429596..72cc4fb4a4 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp @@ -26,6 +26,13 @@ namespace AppInstaller::Repository::Microsoft::Schema std::ostream& operator<<(std::ostream& out, const Version& version) { - return (out << version.MajorVersion << '.' << version.MinorVersion); + if (version == Version::Latest()) + { + return out << "Latest"; + } + else + { + return (out << version.MajorVersion << '.' << version.MinorVersion); + } } } From 0b84462d2081310b5317100acf74f74c8f5edbae Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 19 Apr 2023 16:50:11 -0700 Subject: [PATCH 09/26] Fix schema version --- src/AppInstallerCLITests/PinningIndex.cpp | 14 +++++++------- .../Microsoft/PinningIndex.cpp | 2 +- .../Microsoft/Schema/Pinning_1_1/PinTable.cpp | 2 +- .../Microsoft/Schema/Pinning_1_1/PinTable.h | 2 +- .../Schema/Pinning_1_1/PinningIndexInterface.h | 2 +- .../Pinning_1_1/PinningIndexInterface_1_1.cpp | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index 7fa7a48605..5c628321f3 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -68,11 +68,11 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - auto pins = Pinning_V1_0::PinTable::GetAllPins(connection); + auto pins = Pinning_V1_1::PinTable::GetAllPins(connection); REQUIRE(pins.size() == 1); REQUIRE(pins[0] == pin); - auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); + auto pinFromIndex = Pinning_V1_1::PinTable::GetPinById(connection, 1); REQUIRE(pinFromIndex.has_value()); REQUIRE(pinFromIndex.value() == pin); @@ -88,8 +88,8 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") { // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - REQUIRE(Pinning_V1_0::PinTable::GetAllPins(connection).empty()); - REQUIRE(!Pinning_V1_0::PinTable::GetPinById(connection, 1)); + REQUIRE(Pinning_V1_1::PinTable::GetAllPins(connection).empty()); + REQUIRE(!Pinning_V1_1::PinTable::GetPinById(connection, 1)); } } @@ -109,7 +109,7 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") { Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadOnly); - auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); + auto pinFromIndex = Pinning_V1_1::PinTable::GetPinById(connection, 1); REQUIRE(pinFromIndex.has_value()); REQUIRE(pinFromIndex.value() == updatedPin); } @@ -122,8 +122,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") { // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - REQUIRE(Pinning_V1_0::PinTable::GetAllPins(connection).empty()); - REQUIRE(!Pinning_V1_0::PinTable::GetPinById(connection, 1)); + REQUIRE(Pinning_V1_1::PinTable::GetAllPins(connection).empty()); + REQUIRE(!Pinning_V1_1::PinTable::GetPinById(connection, 1)); } } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index b84a68d7fb..47b543fe05 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -169,7 +169,7 @@ namespace AppInstaller::Repository::Microsoft m_version.MajorVersion == 1 || m_version.IsLatest()) { - return std::make_unique(); + return std::make_unique(); } THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp index 710b118fcc..c1b74e15f1 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp @@ -5,7 +5,7 @@ #include "SQLiteStatementBuilder.h" #include "Microsoft/Schema/IPinningIndex.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 { namespace { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h index 357ca2319b..9a283b9a63 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h @@ -6,7 +6,7 @@ #include "Microsoft/Schema/IPinningIndex.h" #include -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 { struct PinTable { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h index cfe049ebad..4b233f194d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h @@ -3,7 +3,7 @@ #pragma once #include "Microsoft/Schema/IPinningIndex.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 { struct PinningIndexInterface : public IPinningIndex { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp index f3cacb0706..663a4936d0 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp @@ -4,7 +4,7 @@ #include "Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h" #include "Microsoft/Schema/Pinning_1_1/PinTable.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 { namespace { @@ -22,16 +22,16 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 } - // Version 1.0 + // Version 1.1 Schema::Version PinningIndexInterface::GetVersion() const { - return { 1, 0 }; + return { 1, 1 }; } void PinningIndexInterface::CreateTables(SQLite::Connection& connection) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpintable_v1_0"); - Pinning_V1_0::PinTable::Create(connection); + Pinning_V1_1::PinTable::Create(connection); savepoint.Commit(); } From 7d0f3dacc3d6563b65cdfd1e4adc61afeeab0a8d Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Fri, 21 Apr 2023 15:17:39 -0700 Subject: [PATCH 10/26] Add composite source test --- src/AppInstallerCLITests/CompositeSource.cpp | 189 ++++++++++++++++--- 1 file changed, 160 insertions(+), 29 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index 8e13add468..cf760a61b5 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -122,6 +122,12 @@ struct TestPackageHelper return *this; } + TestPackageHelper& WithVersion(std::string_view version) + { + m_manifest.Version = version; + return *this; + } + TestPackageHelper& WithChannel(const std::string& channel) { m_manifest.Channel = channel; @@ -185,7 +191,7 @@ TestPackageHelper MakeAvailable(std::shared_ptr source) return { /* isInstalled */ false, source}; } -void RequireIncludes(const std::vector& filters, PackageMatchField field, MatchType type, std::optional value = {}) +bool SearchRequestIncludes(const std::vector& filters, PackageMatchField field, MatchType type, std::optional value = {}) { bool found = false; @@ -198,7 +204,12 @@ void RequireIncludes(const std::vector& filters, PackageMatc } } - REQUIRE(found); + return found; +} + +void RequireSearchRequestIncludes(const std::vector& filters, PackageMatchField field, MatchType type, std::optional value = {}) +{ + REQUIRE(SearchRequestIncludes(filters, field, type, value)); } TEST_CASE("CompositeSource_PackageFamilyName_NotAvailable", "[CompositeSource]") @@ -224,7 +235,7 @@ TEST_CASE("CompositeSource_PackageFamilyName_Available", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); @@ -260,7 +271,7 @@ TEST_CASE("CompositeSource_ProductCode_Available", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(MakeInstalled().WithPC(pc), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, pc); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, pc); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithPC(pc), Criteria()); @@ -280,7 +291,7 @@ TEST_CASE("CompositeSource_NameAndPublisher_Match", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(MakeInstalled(), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::NormalizedNameAndPublisher, MatchType::Exact); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available), Criteria()); @@ -347,7 +358,7 @@ TEST_CASE("CompositeSource_FoundByBothRootSearches", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -357,7 +368,7 @@ TEST_CASE("CompositeSource_FoundByBothRootSearches", "[CompositeSource]") setup.Available->Everything.Matches.emplace_back(availablePackage, Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(availablePackage, Criteria()); @@ -378,7 +389,7 @@ TEST_CASE("CompositeSource_OnlyAvailableFoundByRootSearch", "[CompositeSource]") CompositeTestSetup setup; setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeInstalled().WithPFN(pfn), Criteria()); @@ -388,7 +399,7 @@ TEST_CASE("CompositeSource_OnlyAvailableFoundByRootSearch", "[CompositeSource]") setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); @@ -410,7 +421,7 @@ TEST_CASE("CompositeSource_FoundByAvailableRootSearch_NotInstalled", "[Composite setup.Available->Everything.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithPFN(pfn), Criteria()); @@ -436,7 +447,7 @@ TEST_CASE("CompositeSource_UpdateWithBetterMatchCriteria", "[CompositeSource]") setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(availablePackage, Criteria()); @@ -453,7 +464,7 @@ TEST_CASE("CompositeSource_UpdateWithBetterMatchCriteria", "[CompositeSource]") // Now make the source root search find it with a better criteria setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -592,7 +603,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchAll", "[CompositeSource setup.Available->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithDefaultName(firstName), Criteria()); @@ -601,7 +612,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchAll", "[CompositeSource secondAvailable->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(secondAvailable).WithDefaultName(secondName), Criteria()); @@ -630,7 +641,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_MatchSecond", "[CompositeSou secondAvailable->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(MakeAvailable(setup.Available).WithDefaultName(secondName), Criteria()); @@ -657,7 +668,7 @@ TEST_CASE("CompositeSource_MultipleAvailableSources_ReverseMatchBoth", "[Composi setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -827,7 +838,7 @@ TEST_CASE("CompositeSource_TrackingPackageFound", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -839,12 +850,12 @@ TEST_CASE("CompositeSource_TrackingPackageFound", "[CompositeSource]") { if (request.Filters.empty()) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); } else { REQUIRE(request.Filters.size() == 1); - RequireIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); + RequireSearchRequestIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); } SearchResult result; @@ -875,7 +886,7 @@ TEST_CASE("CompositeSource_TrackingPackageFound_MetadataPopulatedFromTracking", setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -887,12 +898,12 @@ TEST_CASE("CompositeSource_TrackingPackageFound_MetadataPopulatedFromTracking", { if (request.Filters.empty()) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); } else { REQUIRE(request.Filters.size() == 1); - RequireIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); + RequireSearchRequestIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); } SearchResult result; @@ -936,7 +947,7 @@ TEST_CASE("CompositeSource_TrackingFound_AvailableNot", "[CompositeSource]") setup.Installed->Everything.Matches.emplace_back(installedPackage, Criteria()); setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -966,7 +977,7 @@ TEST_CASE("CompositeSource_TrackingFound_AvailablePath", "[CompositeSource]") setup.Installed->SearchFunction = [&](const SearchRequest& request) { - RequireIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); + RequireSearchRequestIncludes(request.Inclusions, PackageMatchField::PackageFamilyName, MatchType::Exact, pfn); SearchResult result; result.Matches.emplace_back(installedPackage, Criteria()); @@ -977,7 +988,7 @@ TEST_CASE("CompositeSource_TrackingFound_AvailablePath", "[CompositeSource]") setup.Available->SearchFunction = [&](const SearchRequest& request) { REQUIRE(request.Filters.size() == 1); - RequireIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); + RequireSearchRequestIncludes(request.Filters, PackageMatchField::Id, MatchType::CaseInsensitive, availableID); SearchResult result; result.Matches.emplace_back(availablePackage, Criteria()); @@ -1081,7 +1092,7 @@ void RequireExpectedResultsWithPin(std::shared_ptr package, const Expe } } -TEST_CASE("CompositeSource_PinnedAvailable", "[CompositeSource][PinFlow]") +TEST_CASE("CompositeSource_Pinning_AvailableVersionPinned", "[CompositeSource][PinFlow]") { // We use an installed package that has 3 available versions: v1.0.0, v1.0.1 and v1.1.0. // Installed is v1.0.1 @@ -1196,7 +1207,7 @@ TEST_CASE("CompositeSource_PinnedAvailable", "[CompositeSource][PinFlow]") RequireExpectedResultsWithPin(package, expectedResult); } -TEST_CASE("CompositeSource_OneSourcePinned", "[CompositeSource][PinFlow]") +TEST_CASE("CompositeSource_Pinning_OneSourcePinned", "[CompositeSource][PinFlow]") { // We use an installed package that has 2 available sources. // If one of them is pinned, we should still get the updates from the other one. @@ -1254,7 +1265,7 @@ TEST_CASE("CompositeSource_OneSourcePinned", "[CompositeSource][PinFlow]") RequireExpectedResultsWithPin(package, expectedResult); } -TEST_CASE("CompositeSource_OneSourceGated", "[CompositeSource][PinFlow]") +TEST_CASE("CompositeSource_Pinning_OneSourceGated", "[CompositeSource][PinFlow]") { // We use an installed package that has 2 available sources. // If one of them has a gating pin, we should still get the updates from it @@ -1316,4 +1327,124 @@ TEST_CASE("CompositeSource_OneSourceGated", "[CompositeSource][PinFlow]") auto package = result.Matches[0].Package; REQUIRE(package); RequireExpectedResultsWithPin(package, expectedResult); -} \ No newline at end of file +} + +TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlow]") +{ + // Tests the case where multiple installed packages match to a single available package. + // If one of the two installed packages is pinned, when searching we should get + // two Composite packages, with only one of them pinned. + TempFile indexFile("pinningIndex", ".db"); + TestHook::SetPinningIndex_Override pinningIndexOverride(indexFile.GetPath()); + + TestUserSettings userSettings; + userSettings.Set(true); + + std::string packageId = "packageId"; + std::string productCode1 = "product-code1"; + std::string productCode2 = "product-code2"; + + // Installed packages differ in product code and version + auto installedPackage1 = MakeInstalled().WithId(packageId).WithPC(productCode1).WithVersion("1.1"sv); + auto installedPackage2 = MakeInstalled().WithId(packageId).WithPC(productCode2).WithVersion("1.2"sv); + + CompositeTestSetup setup; + setup.Installed->SearchFunction = [&](const SearchRequest& request) + { + bool isSearchById = SearchRequestIncludes(request.Inclusions, PackageMatchField::Id, MatchType::Exact, packageId); + + SearchResult result; + if (isSearchById || SearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, productCode1)) + { + result.Matches.emplace_back(installedPackage1, Criteria()); + } + + if (isSearchById || SearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, productCode2)) + { + result.Matches.emplace_back(installedPackage2, Criteria()); + } + + return result; + }; + + // Available package has the same ID, no product code, and different version from both the installed packages; + setup.Available->SearchFunction = [&](const SearchRequest&) + { + SearchResult result; + result.Matches.emplace_back(MakeAvailable(setup.Available).WithId(packageId).WithVersion("2.0"sv), Criteria()); + return result; + }; + + // We will pin the first package only + PinKey pinKey(packageId, setup.Available->Details.Identifier, Pinning::ExtraIdStringType::ProductCode, productCode1); + auto pinningIndex = PinningIndex::OpenOrCreateDefault(); + REQUIRE(pinningIndex); + + // We will check the pinning status for both installed packages + ExpectedResultsForPinning expectedResult1; + ExpectedResultsForPinning expectedResult2; + + expectedResult1.ResultsForPinBehavior[PinBehavior::IgnorePins] + = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "2.0" }; + + // The second package is never pinned, so its result is always the same + expectedResult2.ResultsForPinBehavior[PinBehavior::IgnorePins] + = expectedResult2.ResultsForPinBehavior[PinBehavior::ConsiderPins] + = expectedResult2.ResultsForPinBehavior[PinBehavior::IncludePinned] + = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "2.0" }; + expectedResult2.AvailableVersions = { + { "AvailableTestSource1", "2.0", "", Pinning::PinType::Unknown }, + }; + + SECTION("Unpinned") + { + // If there are no pins, the result should not change if we consider them + expectedResult1.ResultsForPinBehavior[PinBehavior::ConsiderPins] + = expectedResult1.ResultsForPinBehavior[PinBehavior::IncludePinned] + = expectedResult1.ResultsForPinBehavior[PinBehavior::IgnorePins]; + expectedResult1.AvailableVersions = { + { "AvailableTestSource1", "2.0", "", Pinning::PinType::Unknown }, + }; + } + SECTION("Pinned") + { + pinningIndex->AddPin(Pin::CreatePinningPin(PinKey{ pinKey })); + + // Pinning pins are ignored with --include-pinned + expectedResult1.ResultsForPinBehavior[PinBehavior::IncludePinned] = expectedResult1.ResultsForPinBehavior[PinBehavior::IgnorePins]; + + expectedResult1.ResultsForPinBehavior[PinBehavior::ConsiderPins] = { /* IsUpdateAvailable */ false, /* LatestAvailableVersion */ {} }; + expectedResult1.AvailableVersions = { + { "AvailableTestSource1", "2.0", "", Pinning::PinType::Pinning }, + }; + } + SECTION("Blocked") + { + pinningIndex->AddPin(Pin::CreateBlockingPin(PinKey{ pinKey })); + expectedResult1.ResultsForPinBehavior[PinBehavior::ConsiderPins] = { /* IsUpdateAvailable */ false, /* LatestAvailableVersion */ {} }; + + // Blocking pins are not affected by --include-pinned + expectedResult1.ResultsForPinBehavior[PinBehavior::IncludePinned] = expectedResult1.ResultsForPinBehavior[PinBehavior::ConsiderPins]; + + expectedResult1.AvailableVersions = { + { "AvailableTestSource1", "2.0", "", Pinning::PinType::Blocking }, + }; + } + + SearchRequest searchRequest; + searchRequest.Inclusions.emplace_back(PackageMatchField::Id, MatchType::Exact, packageId); + SearchResult result = setup.Composite.Search(searchRequest); + + REQUIRE(result.Matches.size() == 2); + + // Here we assume that the order we return the packages in the installed source + // search is preserved. We'll need to change it if that stops being the case. + auto package1 = result.Matches[0].Package; + REQUIRE(package1); + + auto package2 = result.Matches[1].Package; + REQUIRE(package2); + + RequireExpectedResultsWithPin(package1, expectedResult1); + RequireExpectedResultsWithPin(package2, expectedResult2); +} From 81a7679b60c816bddf5868b6c903d6d752c7993e Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Fri, 21 Apr 2023 15:21:03 -0700 Subject: [PATCH 11/26] unneeded whitespace --- src/AppInstallerCLITests/CompositeSource.cpp | 2 +- src/AppInstallerCLITests/PinFlow.cpp | 2 +- src/AppInstallerCommonCore/Public/winget/Pin.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index cf760a61b5..d90c4b093a 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1358,7 +1358,7 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo { result.Matches.emplace_back(installedPackage1, Criteria()); } - + if (isSearchById || SearchRequestIncludes(request.Inclusions, PackageMatchField::ProductCode, MatchType::Exact, productCode2)) { result.Matches.emplace_back(installedPackage2, Criteria()); diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index edd387b7c0..f28e179547 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -35,7 +35,7 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(addContext, CreateTestSource({ TSR:: TestInstaller_Exe })); + OverrideForCompositeInstalledSource(addContext, CreateTestSource({ TSR::TestInstaller_Exe })); addContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); addContext.Args.AddArg(Execution::Args::Type::BlockingPin); diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 656c1d3079..16681080d2 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -69,8 +69,8 @@ namespace AppInstaller::Pinning { // std::tie implements tuple comparison, wherein it checks the first item in the tuple, // iff the first elements are equal, then the second element is used for comparison, and so on - return std::tie(PackageId, SourceId, ExtraIdType, ExtraId) < - std::tie(other.PackageId, other.SourceId, other.ExtraIdType, other.ExtraId); + return std::tie(PackageId, SourceId, ExtraIdType, ExtraId) < + std::tie(other.PackageId, other.SourceId, other.ExtraIdType, other.ExtraId); } // Used for logging From 4c6508658d6f17fb61f3d1caee01eb21dc28c441 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 26 Apr 2023 14:26:00 -0700 Subject: [PATCH 12/26] Latest schema log --- .../Microsoft/Schema/Version.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp index 72cc4fb4a4..f5a3049547 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Version.cpp @@ -26,13 +26,17 @@ namespace AppInstaller::Repository::Microsoft::Schema std::ostream& operator<<(std::ostream& out, const Version& version) { - if (version == Version::Latest()) + if (version.IsLatest()) { return out << "Latest"; } + else if (version.IsLatestForMajor(version.MajorVersion)) + { + return out << version.MajorVersion << ".Latest"; + } else { - return (out << version.MajorVersion << '.' << version.MinorVersion); + return out << version.MajorVersion << '.' << version.MinorVersion; } } } From e214cb48cbafb715b9fce315d9ec581eeb56ce9f Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 26 Apr 2023 17:03:38 -0700 Subject: [PATCH 13/26] Revert schema and pin key changes --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 38 +++-------------- src/AppInstallerCLITests/CompositeSource.cpp | 8 ++-- src/AppInstallerCLITests/PinningIndex.cpp | 30 ++++++------- src/AppInstallerCommonCore/Pin.cpp | 26 ++---------- .../Public/winget/Pin.h | 27 +++--------- .../AppInstallerRepositoryCore.vcxproj | 8 ++-- ...AppInstallerRepositoryCore.vcxproj.filters | 16 +++---- .../Microsoft/PinningIndex.cpp | 19 ++------- .../{Pinning_1_1 => Pinning_1_0}/PinTable.cpp | 42 ++++++------------- .../{Pinning_1_1 => Pinning_1_0}/PinTable.h | 2 +- .../PinningIndexInterface.h | 2 +- .../PinningIndexInterface_1_0.cpp} | 8 ++-- 12 files changed, 66 insertions(+), 160 deletions(-) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_1 => Pinning_1_0}/PinTable.cpp (76%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_1 => Pinning_1_0}/PinTable.h (97%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_1 => Pinning_1_0}/PinningIndexInterface.h (97%) rename src/AppInstallerRepositoryCore/Microsoft/Schema/{Pinning_1_1/PinningIndexInterface_1_1.cpp => Pinning_1_0/PinningIndexInterface_1_0.cpp} (92%) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index cb2f4cabf8..5995ae2327 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -31,32 +31,6 @@ namespace AppInstaller::CLI::Workflow return Pinning::Pin::CreatePinningPin(pinKey); } } - - // Gets a search request that can be used to find the installed package - // that corresponds with a pin. The search is based on the extra ID - // (Product Code or Package Family Name) if it is available; otherwise - // it uses the package ID. Using the extra ID allows us to disambiguate - // in certain cases. - SearchRequest GetSearchRequestForPin(const Pinning::PinKey& pinKey) - { - SearchRequest searchRequest; - - switch (pinKey.ExtraIdType) - { - case Pinning::ExtraIdStringType::ProductCode: - searchRequest.Filters.emplace_back(PackageMatchField::ProductCode, MatchType::CaseInsensitive, pinKey.ExtraId); - break; - case Pinning::ExtraIdStringType::PackageFamilyName: - searchRequest.Filters.emplace_back(PackageMatchField::PackageFamilyName, MatchType::CaseInsensitive, pinKey.ExtraId); - break; - case Pinning::ExtraIdStringType::None: - default: - searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pinKey.PackageId); - break; - } - - return searchRequest; - } } void OpenPinningIndex::operator()(Execution::Context& context) const @@ -83,7 +57,7 @@ namespace AppInstaller::CLI::Workflow { auto package = context.Get(); std::vector pins; - std::set> sourcesAndIds; + std::set sources; auto pinningIndex = context.Get(); @@ -102,7 +76,7 @@ namespace AppInstaller::CLI::Workflow for (const auto& pinKey : pinKeys) { - if (sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) + if (sources.emplace(pinKey.SourceId).second) { auto pin = pinningIndex->GetPin(pinKey); if (pin) @@ -126,7 +100,7 @@ namespace AppInstaller::CLI::Workflow auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); std::vector pinsToAddOrUpdate; - std::set> sourcesAndIds; + std::set sources; auto pinningIndex = context.Get(); @@ -140,7 +114,7 @@ namespace AppInstaller::CLI::Workflow for (const auto& pinKey : pinKeys) { - if (!sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) + if (!sources.emplace(pinKey.SourceId).second) { // We already considered the pin for this source and product code/pfn continue; @@ -200,7 +174,7 @@ namespace AppInstaller::CLI::Workflow { auto package = context.Get(); std::vector pins; - std::set> sourcesAndIds; + std::set sources; auto pinningIndex = context.Get(); bool pinExists = false; @@ -223,7 +197,7 @@ namespace AppInstaller::CLI::Workflow for (const auto& pinKey : pinKeys) { - if (sourcesAndIds.emplace(pinKey.SourceId, pinKey.ExtraId).second) + if (sources.emplace(pinKey.SourceId).second) { if (pinningIndex->GetPin(pinKey)) { diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index d90c4b093a..f35d61179a 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1127,7 +1127,7 @@ TEST_CASE("CompositeSource_Pinning_AvailableVersionPinned", "[CompositeSource][P // The result when ignoring pins is always the same expectedResult.ResultsForPinBehavior[PinBehavior::IgnorePins] = { /* IsUpdateAvailable */ true, /* LatestAvailableVersion */ "1.1.0" }; - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); + PinKey pinKey("Id", setup.Available->Details.Identifier); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); @@ -1243,7 +1243,7 @@ TEST_CASE("CompositeSource_Pinning_OneSourcePinned", "[CompositeSource][PinFlow] }; { - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); + PinKey pinKey("Id", setup.Available->Details.Identifier); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreatePinningPin(PinKey{ pinKey })); @@ -1306,7 +1306,7 @@ TEST_CASE("CompositeSource_Pinning_OneSourceGated", "[CompositeSource][PinFlow]" }; { - PinKey pinKey("Id", setup.Available->Details.Identifier, Pinning::ExtraIdStringType::None, ""); + PinKey pinKey("Id", setup.Available->Details.Identifier); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); pinningIndex->AddPin(Pin::CreateGatingPin(PinKey{ pinKey }, GatedVersion{ "1.*"sv })); @@ -1376,7 +1376,7 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo }; // We will pin the first package only - PinKey pinKey(packageId, setup.Available->Details.Identifier, Pinning::ExtraIdStringType::ProductCode, productCode1); + PinKey pinKey(packageId, setup.Available->Details.Identifier, productCode1); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index 5c628321f3..8a073d92ea 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -4,7 +4,7 @@ #include "TestCommon.h" #include #include -#include +#include #include using namespace std::string_literals; @@ -57,7 +57,7 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId", ExtraIdStringType::None, "" }); + Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId" }); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -68,11 +68,11 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - auto pins = Pinning_V1_1::PinTable::GetAllPins(connection); + auto pins = Pinning_V1_0::PinTable::GetAllPins(connection); REQUIRE(pins.size() == 1); REQUIRE(pins[0] == pin); - auto pinFromIndex = Pinning_V1_1::PinTable::GetPinById(connection, 1); + auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); REQUIRE(pinFromIndex.has_value()); REQUIRE(pinFromIndex.value() == pin); @@ -88,8 +88,8 @@ TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") { // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - REQUIRE(Pinning_V1_1::PinTable::GetAllPins(connection).empty()); - REQUIRE(!Pinning_V1_1::PinTable::GetPinById(connection, 1)); + REQUIRE(Pinning_V1_0::PinTable::GetAllPins(connection).empty()); + REQUIRE(!Pinning_V1_0::PinTable::GetPinById(connection, 1)); } } @@ -98,8 +98,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId", ExtraIdStringType::None, ""}, { "1.0.*"sv }); - Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId", ExtraIdStringType::None, ""}); + Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId"}, { "1.0.*"sv }); + Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId"}); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -109,7 +109,7 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") { Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadOnly); - auto pinFromIndex = Pinning_V1_1::PinTable::GetPinById(connection, 1); + auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); REQUIRE(pinFromIndex.has_value()); REQUIRE(pinFromIndex.value() == updatedPin); } @@ -122,8 +122,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") { // Open it directly to directly test table state Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); - REQUIRE(Pinning_V1_1::PinTable::GetAllPins(connection).empty()); - REQUIRE(!Pinning_V1_1::PinTable::GetPinById(connection, 1)); + REQUIRE(Pinning_V1_0::PinTable::GetAllPins(connection).empty()); + REQUIRE(!Pinning_V1_0::PinTable::GetPinById(connection, 1)); } } @@ -132,8 +132,8 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1", ExtraIdStringType::None, "" }); - Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2", ExtraIdStringType::None, "" }); + Pin pin1 = Pin::CreateBlockingPin({ "pkg1", "src1" }); + Pin pin2 = Pin::CreatePinningPin({ "pkg2", "src2" }); // Add two pins to the index, then check that they show up when queried PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); @@ -143,7 +143,7 @@ TEST_CASE("PinningIndex_ResetAll", "[pinningIndex]") REQUIRE(index.GetAllPins().size() == 2); REQUIRE(index.GetPin(pin1.GetKey()).has_value()); REQUIRE(index.GetPin(pin2.GetKey()).has_value()); - REQUIRE(!index.GetPin({ "pkg", "src", ExtraIdStringType::None, "" }).has_value()); + REQUIRE(!index.GetPin({ "pkg", "src" }).has_value()); // Reset the index, then check that there are no pins index.ResetAllPins(); @@ -157,7 +157,7 @@ TEST_CASE("PinningIndex_AddDuplicatePin", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkg", "src", ExtraIdStringType::None, "" }, { "1.*"sv }); + Pin pin = Pin::CreateGatingPin({ "pkg", "src" }, { "1.*"sv }); PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); index.AddPin(pin); diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index bc58b933cc..b3b8bc5623 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -56,30 +56,10 @@ namespace AppInstaller::Pinning } } - std::string_view ToString(ExtraIdStringType type) - { - switch (type) - { - case ExtraIdStringType::ProductCode: - return "ProductCode"sv; - case ExtraIdStringType::PackageFamilyName: - return "PackageFamilyName"; - case ExtraIdStringType::None: - default: - return "None"; - } - } - std::string PinKey::ToString() const { std::stringstream ss; ss << "Package=[" << PackageId << "] Source=[" << SourceId << "]"; - - if (ExtraIdType != Pinning::ExtraIdStringType::None) - { - ss << ' ' << Pinning::ToString(ExtraIdType) << "=[" << ExtraId << "]"; - } - return ss.str(); } @@ -107,17 +87,17 @@ namespace AppInstaller::Pinning for (const auto& productCode : installedProductCodes) { - pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::ProductCode, productCode); + pinKeys.emplace_back(productCode, "" /* TODO */); } for (const auto& packageFamilyName : installedPackageFamilyNames) { - pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::PackageFamilyName, packageFamilyName); + pinKeys.emplace_back(packageFamilyName, "" /* TODO */); } if (pinKeys.empty()) { - pinKeys.emplace_back(packageId, sourceId, Pinning::ExtraIdStringType::None, ""); + pinKeys.emplace_back(packageId, sourceId); } return pinKeys; diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 16681080d2..3a7d255032 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -26,38 +26,23 @@ namespace AppInstaller::Pinning Blocking, }; - // The type of an extra string used for identifying an installed app for pinning - enum class ExtraIdStringType - { - None, - ProductCode, - PackageFamilyName, - }; - std::string_view ToString(PinType type); PinType ConvertToPinTypeEnum(std::string_view in); - std::string_view ToString(ExtraIdStringType type); - // Determines which of two pin types is more strict. PinType StrictestPinType(PinType first, PinType second); // The set of values needed to uniquely identify a Pin. - // We use both a PackageId and a ProductCode or PackageFamilyName - // to be able to deal with packages that match with multiple installed apps. - // This helps us deal better with apps that we don't match properly elsewhere. struct PinKey { PinKey() {} - PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId, ExtraIdStringType extraIdType, std::string_view extraId = {}) - : PackageId(packageId), SourceId(sourceId), ExtraIdType(extraIdType), ExtraId(extraId) {} + PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId) + : PackageId(packageId), SourceId(sourceId) {} bool operator==(const PinKey& other) const { return PackageId == other.PackageId - && SourceId == other.SourceId - && ExtraIdType == other.ExtraIdType - && ExtraId == other.ExtraId; + && SourceId == other.SourceId; } bool operator!=(const PinKey& other) const @@ -69,8 +54,8 @@ namespace AppInstaller::Pinning { // std::tie implements tuple comparison, wherein it checks the first item in the tuple, // iff the first elements are equal, then the second element is used for comparison, and so on - return std::tie(PackageId, SourceId, ExtraIdType, ExtraId) < - std::tie(other.PackageId, other.SourceId, other.ExtraIdType, other.ExtraId); + return std::tie(PackageId, SourceId) < + std::tie(other.PackageId, other.SourceId); } // Used for logging @@ -78,8 +63,6 @@ namespace AppInstaller::Pinning const Manifest::Manifest::string_t PackageId; const std::string SourceId; - const ExtraIdStringType ExtraIdType = ExtraIdStringType::None; - const std::string ExtraId; }; // Gets a list of the possible pin keys that we may use for an available package diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj index 7b479c206b..a193e562f6 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -277,8 +277,8 @@ - - + + @@ -368,8 +368,8 @@ - - + + diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters index c3cbf899bc..a1baf663b9 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -76,7 +76,7 @@ {d9d70cf5-ce04-4db2-a0ec-970dd0ad22b6} - + {f05e19bb-2161-4ab0-9d04-2dfa2d3eb3c6} @@ -348,11 +348,11 @@ Microsoft - - Microsoft\Schema\Pinning_1_1 + + Microsoft\Schema\Pinning_1_0 - - Microsoft\Schema\Pinning_1_1 + + Microsoft\Schema\Pinning_1_0 Public\winget @@ -560,11 +560,11 @@ Source Files - + Source Files - - Microsoft\Schema\Pinning_1_1 + + Microsoft\Schema\Pinning_1_0 Source Files diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index 47b543fe05..5b54689d63 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -3,7 +3,7 @@ #include "pch.h" #include "PinningIndex.h" #include "SQLiteStorageBase.h" -#include "Schema/Pinning_1_1/PinningIndexInterface.h" +#include "Schema/Pinning_1_0/PinningIndexInterface.h" namespace AppInstaller::Repository::Microsoft { @@ -55,20 +55,7 @@ namespace AppInstaller::Repository::Microsoft try { AICLI_LOG(Repo, Info, << "Opening existing pinning index"); - auto pinningIndex = std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); - - // Version 1.1 of the schema is a breaking change from version 1.0. - // Since v1.0 was only available during preview, we don't need to ensure - // backwards compatibility. So, we delete any old versions of the index. - const Schema::Version DeprecatedPinningIndexVersion = { 1, 0 }; - if (pinningIndex->GetVersion() == DeprecatedPinningIndexVersion) - { - AICLI_LOG(Repo, Warning, << "Existing pinning index has a deprecated schema version. Will delete existing index"); - } - else - { - return pinningIndex; - } + return std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); } CATCH_LOG(); } @@ -169,7 +156,7 @@ namespace AppInstaller::Repository::Microsoft m_version.MajorVersion == 1 || m_version.IsLatest()) { - return std::make_unique(); + return std::make_unique(); } THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp similarity index 76% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index c1b74e15f1..8b79a0306d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -5,15 +5,13 @@ #include "SQLiteStatementBuilder.h" #include "Microsoft/Schema/IPinningIndex.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { namespace { std::optional GetPinFromRow( std::string_view packageId, std::string_view sourceId, - Pinning::ExtraIdStringType extraIdType, - std::string_view extraId, Pinning::PinType type, std::string_view version) @@ -21,11 +19,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 switch (type) { case Pinning::PinType::Blocking: - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId, extraIdType, extraId }); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); case Pinning::PinType::Pinning: - return Pinning::Pin::CreatePinningPin({ packageId, sourceId, extraIdType, extraId }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); case Pinning::PinType::Gating: - return Pinning::Pin::CreateGatingPin({ packageId, sourceId, extraIdType, extraId }, Utility::GatedVersion{ version }); + return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ version }); default: return {}; } @@ -36,8 +34,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 static constexpr std::string_view s_PinTable_Table_Name = "pin"sv; static constexpr std::string_view s_PinTable_PackageId_Column = "package_id"sv; static constexpr std::string_view s_PinTable_SourceId_Column = "source_id"sv; - static constexpr std::string_view s_PinTable_ExtraIdType_Column = "extra_id_type"sv; - static constexpr std::string_view s_PinTable_ExtraId_Column = "extra_id"sv; static constexpr std::string_view s_PinTable_Type_Column = "type"sv; static constexpr std::string_view s_PinTable_Version_Column = "version"sv; static constexpr std::string_view s_PinTable_Index = "pin_index"sv; @@ -58,8 +54,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 createTableBuilder.Column(ColumnBuilder(s_PinTable_PackageId_Column, Type::Text).NotNull()); createTableBuilder.Column(ColumnBuilder(s_PinTable_SourceId_Column, Type::Text).NotNull()); - createTableBuilder.Column(ColumnBuilder(s_PinTable_ExtraIdType_Column, Type::Int64).NotNull()); - createTableBuilder.Column(ColumnBuilder(s_PinTable_ExtraId_Column, Type::Text).NotNull()); createTableBuilder.Column(ColumnBuilder(s_PinTable_Type_Column, Type::Int64).NotNull()); createTableBuilder.Column(ColumnBuilder(s_PinTable_Version_Column, Type::Text).NotNull()); @@ -69,7 +63,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 // Create an index over the pairs package,source StatementBuilder createIndexBuilder; createIndexBuilder.CreateUniqueIndex(s_PinTable_Index).On(s_PinTable_Table_Name) - .Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, s_PinTable_ExtraIdType_Column, s_PinTable_ExtraId_Column }); + .Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column }); createIndexBuilder.Execute(connection); savepoint.Commit(); @@ -80,9 +74,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 SQLite::Builder::StatementBuilder builder; builder.Select(SQLite::RowIDName).From(s_PinTable_Table_Name) .Where(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) - .And(s_PinTable_SourceId_Column).Equals((std::string_view)pinKey.SourceId) - .And(s_PinTable_ExtraIdType_Column).Equals(pinKey.ExtraIdType) - .And(s_PinTable_ExtraId_Column).Equals((std::string_view)pinKey.ExtraId); + .And(s_PinTable_SourceId_Column).Equals((std::string_view)pinKey.SourceId); SQLite::Statement select = builder.Prepare(connection); @@ -104,15 +96,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 .Columns({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, - s_PinTable_ExtraIdType_Column, - s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .Values( (std::string_view)pinKey.PackageId, pinKey.SourceId, - pinKey.ExtraIdType, - pinKey.ExtraId, pin.GetType(), pin.GetGatedVersion().ToString()); @@ -127,8 +115,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 builder.Update(s_PinTable_Table_Name).Set() .Column(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) .Column(s_PinTable_SourceId_Column).Equals(pinKey.SourceId) - .Column(s_PinTable_ExtraIdType_Column).Equals(pinKey.ExtraIdType) - .Column(s_PinTable_ExtraId_Column).Equals(pinKey.ExtraId) .Column(s_PinTable_Type_Column).Equals(pin.GetType()) .Column(s_PinTable_Version_Column).Equals(pin.GetGatedVersion().ToString()) .Where(SQLite::RowIDName).Equals(pinId); @@ -150,8 +136,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 builder.Select({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, - s_PinTable_ExtraIdType_Column, - s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .From(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); @@ -163,9 +147,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 return {}; } - auto [packageId, sourceId, extraIdType, extraId, pinType, gatedVersion] = - select.GetRow(); - return GetPinFromRow(packageId, sourceId, extraIdType, extraId, pinType, gatedVersion); + auto [packageId, sourceId, pinType, gatedVersion] = + select.GetRow(); + return GetPinFromRow(packageId, sourceId, pinType, gatedVersion); } std::vector PinTable::GetAllPins(SQLite::Connection& connection) @@ -174,8 +158,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 builder.Select({ s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, - s_PinTable_ExtraIdType_Column, - s_PinTable_ExtraId_Column, s_PinTable_Type_Column, s_PinTable_Version_Column }) .From(s_PinTable_Table_Name); @@ -185,9 +167,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 std::vector pins; while (select.Step()) { - auto [packageId, sourceId, extraIdType, extraId, pinType, gatedVersion] = - select.GetRow(); - auto pin = GetPinFromRow(packageId, sourceId, extraIdType, extraId, pinType, gatedVersion); + auto [packageId, sourceId, pinType, gatedVersion] = + select.GetRow(); + auto pin = GetPinFromRow(packageId, sourceId, pinType, gatedVersion); if (pin) { pins.push_back(std::move(pin.value())); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h similarity index 97% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h index 9a283b9a63..357ca2319b 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -6,7 +6,7 @@ #include "Microsoft/Schema/IPinningIndex.h" #include -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { struct PinTable { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h similarity index 97% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h index 4b233f194d..cfe049ebad 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h @@ -3,7 +3,7 @@ #pragma once #include "Microsoft/Schema/IPinningIndex.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { struct PinningIndexInterface : public IPinningIndex { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp similarity index 92% rename from src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp rename to src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp index 663a4936d0..bbef14f217 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_1/PinningIndexInterface_1_1.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp @@ -1,10 +1,10 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #include "pch.h" -#include "Microsoft/Schema/Pinning_1_1/PinningIndexInterface.h" -#include "Microsoft/Schema/Pinning_1_1/PinTable.h" +#include "Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h" +#include "Microsoft/Schema/Pinning_1_0/PinTable.h" -namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { namespace { @@ -31,7 +31,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_1 void PinningIndexInterface::CreateTables(SQLite::Connection& connection) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpintable_v1_0"); - Pinning_V1_1::PinTable::Create(connection); + Pinning_V1_0::PinTable::Create(connection); savepoint.Commit(); } From 329e9a84bb35fdaff868c1a152c0bb258a5f8910 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 2 May 2023 15:25:32 -0700 Subject: [PATCH 14/26] Handle pins for installed on composite --- src/AppInstallerCommonCore/Pin.cpp | 49 ++-- .../Public/winget/Pin.h | 32 ++- .../CompositeSource.cpp | 243 ++++++++++++------ .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 4 +- 4 files changed, 198 insertions(+), 130 deletions(-) diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index b3b8bc5623..398ff902c9 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -9,9 +9,12 @@ namespace AppInstaller::Pinning { using namespace std::string_view_literals; - PinType StrictestPinType(PinType first, PinType second) + namespace { - return std::max(first, second); + // Source ID to use for the installed source; it does not match any installed source. + // This does match with the actual ID of the source, but it doesn't really matter + // as it is handled specially when we see it. + constexpr std::string_view s_installedSourceId = "*PredefinedInstalledSource"sv; } PinType ConvertToPinTypeEnum(std::string_view in) @@ -56,6 +59,11 @@ namespace AppInstaller::Pinning } } + bool IsStricter(PinType first, PinType second) + { + return first > second; + } + std::string PinKey::ToString() const { std::stringstream ss; @@ -63,6 +71,16 @@ namespace AppInstaller::Pinning return ss.str(); } + PinKey PinKey::GetPinKeyForInstalled(std::string systemReferenceString) + { + return { systemReferenceString, s_installedSourceId }; + } + + bool PinKey::IsForInstalled() const + { + return SourceId == s_installedSourceId; + } + std::string Pin::ToString() const { std::stringstream ss; @@ -74,33 +92,6 @@ namespace AppInstaller::Pinning } return ss.str(); - - } - - std::vector GetPinKeysForAvailablePackage( - std::string_view packageId, - std::string sourceId, - const std::vector& installedProductCodes, - const std::vector& installedPackageFamilyNames) - { - std::vector pinKeys; - - for (const auto& productCode : installedProductCodes) - { - pinKeys.emplace_back(productCode, "" /* TODO */); - } - - for (const auto& packageFamilyName : installedPackageFamilyNames) - { - pinKeys.emplace_back(packageFamilyName, "" /* TODO */); - } - - if (pinKeys.empty()) - { - pinKeys.emplace_back(packageId, sourceId); - } - - return pinKeys; } Pin Pin::CreateBlockingPin(PinKey&& pinKey) diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 3a7d255032..e1ef0719d7 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -30,15 +30,26 @@ namespace AppInstaller::Pinning PinType ConvertToPinTypeEnum(std::string_view in); // Determines which of two pin types is more strict. - PinType StrictestPinType(PinType first, PinType second); + bool IsStricter(PinType first, PinType second); // The set of values needed to uniquely identify a Pin. + // A Pin can apply to an installed package or to an available package. + // Pins on available packages can persist when an app is updated outside of winget, + // but it's hard to have them work when there are multiple installed packages for the same available package. + // Pins on installed packages work fine when there are multiple installed packages for the same available, + // but they break when the package is updated outside of winget. struct PinKey { PinKey() {} PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId) : PackageId(packageId), SourceId(sourceId) {} + // Gets a pin key that refers to an installed package by its ProductCode or PackageFamilyName. + // The sourceId used is a special string to distinguish from available packages. + static PinKey GetPinKeyForInstalled(std::string systemReferenceString); + + bool IsForInstalled() const; + bool operator==(const PinKey& other) const { return PackageId == other.PackageId @@ -54,26 +65,16 @@ namespace AppInstaller::Pinning { // std::tie implements tuple comparison, wherein it checks the first item in the tuple, // iff the first elements are equal, then the second element is used for comparison, and so on - return std::tie(PackageId, SourceId) < - std::tie(other.PackageId, other.SourceId); + return std::tie(PackageId, SourceId) < std::tie(other.PackageId, other.SourceId); } // Used for logging std::string ToString() const; - const Manifest::Manifest::string_t PackageId; + const std::string PackageId; const std::string SourceId; }; - // Gets a list of the possible pin keys that we may use for an available package - // given the ProductCodes/PackageFamilyNames we know for the installed version. - // If there is no ProductCode/PackageFamilyName, returns a single element with no extra id. - std::vector GetPinKeysForAvailablePackage( - std::string_view packageId, - std::string sourceId, - const std::vector& installedProductCodes, - const std::vector& installedPackageFamilyNames); - struct Pin { static Pin CreateBlockingPin(PinKey&& pinKey); @@ -89,7 +90,10 @@ namespace AppInstaller::Pinning const Utility::GatedVersion& GetGatedVersion() const { return m_gatedVersion; } bool operator==(const Pin& other) const; - bool operator<(const Pin& other) const { return m_key < other.m_key; } + bool operator<(const Pin& other) const + { + return std::make_pair(m_type, m_key) < std::make_pair(other.m_type, other.m_key); + } // Used for logging std::string ToString() const; diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 2b2f27b886..90c0421a94 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -22,36 +22,82 @@ namespace AppInstaller::Repository }; } - std::optional GetLatestAvailableVersionKeySatisfyingPin(const std::vector& availableVersionKeys, PinBehavior pinBehavior) + Pinning::PinKey GetPinKeyForAvailable(IPackage* availablePackage) + { + return { + availablePackage->GetProperty(PackageProperty::Id).get(), + availablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins)->GetSource().GetIdentifier() + }; + } + + // Gets the pinned state for an available PackageVersionKey that may have a pin, + // and optionally an additional pin that could come from the installed version. + // If there are both installed and available pins, we return the one that is the most strict. + Pinning::PinType GetPinnedStateForVersion( + const PackageVersionKey& availableVersionKey, + const std::optional& pin, + PinBehavior pinBehavior) + { + if (pinBehavior == PinBehavior::IgnorePins) + { + // No need to check anything + return Pinning::PinType::Unknown; + } + + // For the pin in the available version, we can ignore it depending on the behavior and type. + // If it is gating, we don't need to check the version as that was already done. + Pinning::PinType pinTypeFromAvailable = Pinning::PinType::Unknown; + if (availableVersionKey.PinnedState == Pinning::PinType::Blocking + || (availableVersionKey.PinnedState == Pinning::PinType::Pinning && pinBehavior != PinBehavior::IncludePinned) + || availableVersionKey.PinnedState == Pinning::PinType::Gating) + { + pinTypeFromAvailable = availableVersionKey.PinnedState; + } + + // For the pin in the installed version, we can ignore it depending on the behavior and type. + // If it is gating, we need to check the version. + Pinning::PinType pinTypeFromInstalled = Pinning::PinType::Unknown; + if (pin) + { + if (pin->GetType() == Pinning::PinType::Blocking + || (pin->GetType() == Pinning::PinType::Pinning && pinBehavior != PinBehavior::IncludePinned) + || (pin->GetType() == Pinning::PinType::Gating && !pin->GetGatedVersion().IsValidVersion(availableVersionKey.Version))) + { + pinTypeFromInstalled = pin->GetType(); + } + } + + return Pinning::IsStricter(pinTypeFromAvailable, pinTypeFromAvailable) ? pinTypeFromAvailable : pinTypeFromInstalled; + } + + // Gets the latest available version that satisfies both the available pin (already tagged on the keys) + // and the installed pin (if any). + // Version keys must be sorted with the latest first. + std::optional GetLatestAvailableVersionKeySatisfyingPin( + const std::vector& availableVersionKeys, + const std::optional& installedPin, + PinBehavior pinBehavior) { if (availableVersionKeys.empty()) { return {}; } - std::optional pvk; if (pinBehavior == PinBehavior::IgnorePins) { - pvk = availableVersionKeys.front(); + return availableVersionKeys.front(); } else { // Skip until we find a version that isn't pinned for (const auto& availableVersion : availableVersionKeys) { - if (availableVersion.PinnedState == Pinning::PinType::Blocking || - availableVersion.PinnedState == Pinning::PinType::Gating || - (availableVersion.PinnedState == Pinning::PinType::Pinning && pinBehavior != PinBehavior::IncludePinned)) + if (GetPinnedStateForVersion(availableVersion, installedPin, pinBehavior) == Pinning::PinType::Unknown) { - continue; + return availableVersion; } - - pvk = availableVersion; - break; } } - - return pvk; } // Returns true for fields that provide a strong match; one that is not based on a heuristic. @@ -361,20 +407,22 @@ namespace AppInstaller::Repository std::shared_ptr m_trackingPackageVersion; }; - // Wrapper around an available package to add pinning functionality for composite packages. + // Wrapper around a package to add pinning functionality for composite packages. // Most of the methods are only here for completeness of the interface and are not actually used. - // Pins are tied to product codes and package family names, of which an installed package can - // have multiple, so an available package can also have multiple pins. - struct CompositeAvailablePackage : public IPackage + // A pinnable package can either be an installed package or a single available package; + // we deal with composite packages on CompositePackage. + struct PinnablePackage : public IPackage { - CompositeAvailablePackage() {} - CompositeAvailablePackage(std::shared_ptr availablePackage, std::set&& pins = {}) - : m_availablePackage(availablePackage), m_pins(std::move(pins)) + PinnablePackage() {} + PinnablePackage(std::shared_ptr package, std::optional pin = {}) + : m_package(package), m_pin(pin) { - auto latestAvailable = m_availablePackage->GetLatestAvailableVersion(PinBehavior::IgnorePins); - if (latestAvailable) + // Get the source ID for available packages + auto availableVersion = m_package->GetLatestAvailableVersion(PinBehavior::IgnorePins); + if (availableVersion) { - m_sourceId = latestAvailable->GetSource().GetIdentifier(); + // TODO: Check if needed for installed + m_sourceId = availableVersion->GetSource().GetIdentifier(); } } @@ -383,47 +431,40 @@ namespace AppInstaller::Repository return m_sourceId; } - const std::shared_ptr& GetAvailablePackage() const + const std::shared_ptr& GetPackage() const { - return m_availablePackage; + return m_package; } - const std::set& GetPins() const + const std::optional& GetPin() const { - return m_pins; + return m_pin; } - void AddPin(Pinning::Pin&& pin) + void SetPin(Pinning::Pin&& pin) { - m_pins.emplace(std::move(pin)); + m_pin.emplace(std::move(pin)); } Utility::LocIndString GetProperty(PackageProperty property) const override { - return m_availablePackage->GetProperty(property); + return m_package->GetProperty(property); } std::shared_ptr GetInstalledVersion() const override { - return {}; + return m_package->GetInstalledVersion(); } std::vector GetAvailableVersionKeys() const override { - auto result = m_availablePackage->GetAvailableVersionKeys(); - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning)) + auto result = m_package->GetAvailableVersionKeys(); + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value()) { - for (const auto& pin : m_pins) + // Add pin information to all version keys + for (auto& pvk : result) { - for (auto& pvk : result) - { - if (pin.GetType() == Pinning::PinType::Blocking || - pin.GetType() == Pinning::PinType::Pinning || - (pin.GetType() == Pinning::PinType::Gating && !pin.GetGatedVersion().IsValidVersion(pvk.Version))) - { - pvk.PinnedState = Pinning::StrictestPinType(pin.GetType(), pvk.PinnedState); - } - } + pvk.PinnedState = GetPinnedStateForVersion(pvk, m_pin, PinBehavior::ConsiderPins); } } @@ -438,7 +479,7 @@ namespace AppInstaller::Repository std::shared_ptr GetLatestAvailableVersion(PinBehavior pinBehavior) const override { auto availableVersionKeys = GetAvailableVersionKeys(); - auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, pinBehavior); + auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, /* installedPin */ {}, pinBehavior); if (!latestVersionKey) { return {}; @@ -451,19 +492,12 @@ namespace AppInstaller::Repository { Pinning::PinType pinType = Pinning::PinType::Unknown; - if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning)) + if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value()) { - for (const auto& pin : m_pins) - { - // A gating pin behaves the same as no pin when the version fits the gated version - if (!(pin.GetType() == Pinning::PinType::Gating && pin.GetGatedVersion().IsValidVersion(versionKey.Version))) - { - pinType = Pinning::StrictestPinType(pinType, pin.GetType()); - } - } + pinType = GetPinnedStateForVersion(versionKey, m_pin.value(), PinBehavior::ConsiderPins); } - return { m_availablePackage->GetAvailableVersion(versionKey), pinType }; + return { m_package->GetAvailableVersion(versionKey), pinType }; } bool IsUpdateAvailable(PinBehavior) const override @@ -473,14 +507,14 @@ namespace AppInstaller::Repository bool IsSame(const IPackage* other) const override { - const CompositeAvailablePackage* otherAvailable = dynamic_cast(other); + const PinnablePackage* otherAvailable = dynamic_cast(other); if (otherAvailable) { return m_sourceId == otherAvailable->m_sourceId && - m_pins == otherAvailable->m_pins && - m_availablePackage->IsSame(otherAvailable->m_availablePackage.get()); + m_pin == otherAvailable->m_pin && + m_package->IsSame(otherAvailable->m_package.get()); } return false; @@ -488,19 +522,19 @@ namespace AppInstaller::Repository private: std::string m_sourceId; - std::shared_ptr m_availablePackage; - std::set m_pins; + std::shared_ptr m_package; + std::optional m_pin; }; // A composite package for the CompositeSource. struct CompositePackage : public IPackage { - CompositePackage(std::shared_ptr installedPackage, std::shared_ptr availablePackage = {}) : - m_installedPackage(std::move(installedPackage)) + CompositePackage(std::shared_ptr installedPackage, std::shared_ptr availablePackage = {}) { // Grab the installed version's channel to allow for filtering in calls to get available info. - if (m_installedPackage) + if (installedPackage) { + m_installedPackage.emplace(installedPackage); auto installedVersion = m_installedPackage->GetInstalledVersion(); if (installedVersion) { @@ -559,11 +593,28 @@ namespace AppInstaller::Repository std::vector GetAvailableVersionKeys() const override { std::vector result; + auto installedPin = GetInstalledPin(); for (const auto& availablePackage : m_availablePackages) { auto versionKeys = availablePackage.GetAvailableVersionKeys(); - std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); + + if (installedPin) + { + // Filter out available versions with pin from the installed package + for (const auto& versionKey : versionKeys) + { + if (GetPinnedStateForVersion(versionKey, installedPin, PinBehavior::ConsiderPins) == Pinning::PinType::Unknown) + { + result.emplace_back(versionKey); + } + } + } + else + { + // Copy all version keys if we don't need to filter from installed pin + std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); + } } // Remove all elements whose channel does not match the installed package. @@ -581,7 +632,7 @@ namespace AppInstaller::Repository std::shared_ptr GetLatestAvailableVersion(PinBehavior pinBehavior) const override { auto availableVersionKeys = GetAvailableVersionKeys(); - auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, pinBehavior); + auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, GetInstalledPin(), pinBehavior); if (!latestVersionKey) { return {}; @@ -607,6 +658,7 @@ namespace AppInstaller::Repository auto result = availablePackage.GetAvailableVersionAndPin(versionKey); if (result.first) { + result.second = GetPinnedStateForVersion(versionKey, GetInstalledPin(), PinBehavior::ConsiderPins); return result; } } @@ -634,16 +686,24 @@ namespace AppInstaller::Repository if (!otherComposite || static_cast(m_installedPackage) != static_cast(otherComposite->m_installedPackage) || - (m_installedPackage && !m_installedPackage->IsSame(otherComposite->m_installedPackage.get())) || m_availablePackages.size() != otherComposite->m_availablePackages.size()) { return false; } + if (m_installedPackage) + { + if (m_installedPackage->GetSourceId() != otherComposite->m_installedPackage->GetSourceId() || + !m_installedPackage->GetPackage()->IsSame(otherComposite->m_installedPackage->GetPackage().get())) + { + return false; + } + } + for (size_t i = 0; i < m_availablePackages.size(); ++i) { if (m_availablePackages[i].GetSourceId() != otherComposite->m_availablePackages[i].GetSourceId() || - !m_availablePackages[i].GetAvailablePackage()->IsSame(otherComposite->m_availablePackages[i].GetAvailablePackage().get())) + !m_availablePackages[i].GetPackage()->IsSame(otherComposite->m_availablePackages[i].GetPackage().get())) { return false; } @@ -658,7 +718,7 @@ namespace AppInstaller::Repository { for (const auto& availablePackage : m_availablePackages) { - if (other->IsSame(availablePackage.GetAvailablePackage().get())) + if (other->IsSame(availablePackage.GetPackage().get())) { return true; } @@ -668,9 +728,16 @@ namespace AppInstaller::Repository return false; } - const std::shared_ptr& GetInstalledPackage() const + std::shared_ptr GetInstalledPackage() const { - return m_installedPackage; + if (m_installedPackage) + { + return m_installedPackage->GetPackage(); + } + else + { + return {}; + } } const std::shared_ptr& GetTrackingPackage() const @@ -709,27 +776,26 @@ namespace AppInstaller::Repository return; } - auto installedProductCodes = m_installedPackage->GetInstalledVersion()->GetMultiProperty(PackageVersionMultiProperty::ProductCode); - auto installedPackageFamilyNames = m_installedPackage->GetInstalledVersion()->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); - - // If the package is installed, we need to add the pin information to the available packages from any source. for (auto& availablePackage : m_availablePackages) { - auto pinKeys = Pinning::GetPinKeysForAvailablePackage( - availablePackage.GetAvailablePackage()->GetProperty(PackageProperty::Id).get(), - availablePackage.GetAvailablePackage()->GetLatestAvailableVersion(PinBehavior::IgnorePins)->GetSource().GetIdentifier(), - installedProductCodes, - installedPackageFamilyNames); + Pinning::PinKey pinKey = GetPinKeyForAvailable(availablePackage.GetPackage().get()); - for (const auto& pinKey : pinKeys) + auto pin = pinningIndex.GetPin(pinKey); + if (pin.has_value()) { - auto pin = pinningIndex.GetPin(pinKey); - if (pin.has_value()) - { - availablePackage.AddPin(std::move(pin.value())); - } + availablePackage.SetPin(std::move(pin.value())); } } + + Pinning::PinKey pinKey = Pinning::PinKey::GetPinKeyForInstalled( + m_installedPackage->GetProperty(PackageProperty::Id).get() + ); + + auto pin = pinningIndex.GetPin(pinKey); + if (pin.has_value()) + { + m_installedPackage->SetPin(std::move(pin.value())); + } } private: @@ -750,14 +816,21 @@ namespace AppInstaller::Repository } } - std::shared_ptr m_installedPackage; + std::optional GetInstalledPin() const + { + return (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_installedPackage) + ? m_installedPackage->GetPin() + : std::optional{}; + } + + std::optional m_installedPackage; Utility::LocIndString m_installedChannel; Source m_trackingSource; std::shared_ptr m_trackingPackage; std::shared_ptr m_trackingPackageVersion; std::string m_overrideInstalledVersion; std::shared_ptr m_defaultAvailablePackage; - std::vector m_availablePackages; + std::vector m_availablePackages; }; // The comparator compares the ResultMatch by MatchType first, then Field in a predefined order. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp index bbef14f217..0882537366 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp @@ -22,10 +22,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 } - // Version 1.1 + // Version 1.0 Schema::Version PinningIndexInterface::GetVersion() const { - return { 1, 1 }; + return { 1, 0 }; } void PinningIndexInterface::CreateTables(SQLite::Connection& connection) From f608e853fa055b77fb7f82a031755cc3ef7ef16b Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 3 May 2023 14:07:05 -0700 Subject: [PATCH 15/26] Pin on installed --- src/AppInstallerCLICore/Argument.cpp | 2 + .../Commands/PinCommand.cpp | 4 +- src/AppInstallerCLICore/ExecutionArgs.h | 1 + src/AppInstallerCLICore/Resources.h | 2 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 242 +++++++++--------- src/AppInstallerCLICore/Workflows/PinFlow.h | 3 +- .../Shared/Strings/en-us/winget.resw | 7 + src/AppInstallerCLITests/CompositeSource.cpp | 2 +- .../CompositeSource.cpp | 2 + 9 files changed, 133 insertions(+), 132 deletions(-) diff --git a/src/AppInstallerCLICore/Argument.cpp b/src/AppInstallerCLICore/Argument.cpp index 08acd455d0..898e82c469 100644 --- a/src/AppInstallerCLICore/Argument.cpp +++ b/src/AppInstallerCLICore/Argument.cpp @@ -163,6 +163,8 @@ namespace AppInstaller::CLI return { type, "version"_liv, 'v', ArgTypeCategory::None, ArgTypeExclusiveSet::PinType }; case Execution::Args::Type::BlockingPin: return { type, "blocking"_liv, ArgTypeCategory::None, ArgTypeExclusiveSet::PinType }; + case Execution::Args::Type::PinInstalled: + return { type, "installed"_liv, ArgTypeCategory::None }; // Configuration commands case Execution::Args::Type::ConfigurationFile: diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index fe80d65e5c..e85709c793 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -62,6 +62,7 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::AcceptSourceAgreements), Argument::ForType(Args::Type::Force), Argument{ Args::Type::BlockingPin, Resource::String::PinAddBlockingArgumentDescription, ArgumentType::Flag }, + Argument{ Args::Type::PinInstalled, Resource::String::PinInstalledArgumentDescription, ArgumentType::Flag }, }; } @@ -122,7 +123,6 @@ namespace AppInstaller::CLI Workflow::GetInstalledPackageVersion << Workflow::ReportPackageIdentity << Workflow::OpenPinningIndex() << - Workflow::SearchPin << Workflow::AddPin; } @@ -139,6 +139,7 @@ namespace AppInstaller::CLI Argument::ForType(Args::Type::Exact), Argument::ForType(Args::Type::CustomHeader), Argument::ForType(Args::Type::AcceptSourceAgreements), + Argument{ Args::Type::PinInstalled, Resource::String::PinInstalledArgumentDescription, ArgumentType::Flag }, }; } @@ -307,7 +308,6 @@ namespace AppInstaller::CLI Workflow::GetAllPins << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::CrossReferencePinsWithSource << Workflow::ReportPins; } } diff --git a/src/AppInstallerCLICore/ExecutionArgs.h b/src/AppInstallerCLICore/ExecutionArgs.h index a5a7658ba6..ca9f5af6df 100644 --- a/src/AppInstallerCLICore/ExecutionArgs.h +++ b/src/AppInstallerCLICore/ExecutionArgs.h @@ -95,6 +95,7 @@ namespace AppInstaller::CLI::Execution // Pin command GatedVersion, // Differs from Version in that this supports wildcards BlockingPin, + PinInstalled, // Configuration ConfigurationFile, diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 66247d7b18..2cf798c7cd 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -320,6 +320,8 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinDoesNotExist); WINGET_DEFINE_RESOURCE_STRINGID(PinExistsOverwriting); WINGET_DEFINE_RESOURCE_STRINGID(PinExistsUseForceArg); + WINGET_DEFINE_RESOURCE_STRINGID(PinInstalledArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinInstalledSource); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinNoPinsExist); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 5995ae2327..fcdec5df6f 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -31,6 +31,52 @@ namespace AppInstaller::CLI::Workflow return Pinning::Pin::CreatePinningPin(pinKey); } } + + std::set GetPinKeysForPackage(Execution::Context& context) + { + auto package = context.Get(); + + std::set pinKeys; + + if (context.Args.Contains(Execution::Args::Type::PinInstalled)) + { + auto installedVersion = package->GetInstalledVersion(); + if (installedVersion) + { + pinKeys.emplace(Pinning::PinKey::GetPinKeyForInstalled(installedVersion->GetProperty(PackageVersionProperty::Id))); + } + } + else + { + auto packageVersionKeys = package->GetAvailableVersionKeys(); + for (const auto& versionKey : packageVersionKeys) + { + auto availableVersion = package->GetAvailableVersion(versionKey); + pinKeys.emplace( + availableVersion->GetProperty(PackageVersionProperty::Id).get(), + availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get()); + } + } + + return pinKeys; + } + + // Gets a search request that can be used to find the installed package that corresponds with a pin. + SearchRequest GetSearchRequestForPin(const Pinning::PinKey& pinKey) + { + SearchRequest searchRequest; + if (pinKey.IsForInstalled()) + { + searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::PackageFamilyName, MatchType::Exact, pinKey.PackageId)); + searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, pinKey.PackageId)); + } + else + { + searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pinKey.PackageId); + } + + return searchRequest; + } } void OpenPinningIndex::operator()(Execution::Context& context) const @@ -55,35 +101,18 @@ namespace AppInstaller::CLI::Workflow void SearchPin(Execution::Context& context) { - auto package = context.Get(); - std::vector pins; - std::set sources; + auto pinKeys = GetPinKeysForPackage(context); + auto package = context.Get(); auto pinningIndex = context.Get(); - auto packageVersionKeys = package->GetAvailableVersionKeys(); - - auto installedVersion = package->GetInstalledVersion(); - auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); - auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); - - for (const auto& versionKey : packageVersionKeys) + std::vector pins; + for (const auto& pinKey : pinKeys) { - auto availableVersion = package->GetAvailableVersion(versionKey); - const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); - const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); - const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); - - for (const auto& pinKey : pinKeys) + auto pin = pinningIndex->GetPin(pinKey); + if (pin) { - if (sources.emplace(pinKey.SourceId).second) - { - auto pin = pinningIndex->GetPin(pinKey); - if (pin) - { - pins.emplace_back(std::move(pin.value())); - } - } + pins.emplace_back(std::move(pin.value())); } } @@ -92,70 +121,57 @@ namespace AppInstaller::CLI::Workflow void AddPin(Execution::Context& context) { + auto pinKeys = GetPinKeysForPackage(context); + auto package = context.Get(); auto installedVersion = context.Get(); - - auto installedVersionString = installedVersion->GetProperty(PackageVersionProperty::Version); - auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); - auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); - - std::vector pinsToAddOrUpdate; - std::set sources; - auto pinningIndex = context.Get(); - auto packageVersionKeys = package->GetAvailableVersionKeys(); - for (const auto& versionKey : packageVersionKeys) + std::vector pinsToAddOrUpdate; + for (const auto& pinKey : pinKeys) { - auto availableVersion = package->GetAvailableVersion(versionKey); - const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); - const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); - const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); + auto pin = CreatePin(context, pinKey); + AICLI_LOG(CLI, Info, << "Evaluating Pin " << pin.ToString()); - for (const auto& pinKey : pinKeys) + auto existingPin = pinningIndex->GetPin(pinKey); + if (existingPin) { - if (!sources.emplace(pinKey.SourceId).second) + Utility::LocIndString packageNameToReport = installedVersion->GetProperty(PackageVersionProperty::Name); + if (!pinKey.IsForInstalled()) { - // We already considered the pin for this source and product code/pfn - continue; + auto availableVersion = package->GetAvailableVersion({ pinKey.SourceId, "", "" }); + if (availableVersion) + { + packageNameToReport = availableVersion->GetProperty(PackageVersionProperty::Name); + } } - auto pin = CreatePin(context, pinKey); - AICLI_LOG(CLI, Info, << "Evaluating Pin " << pin.ToString()); - - auto existingPin = pinningIndex->GetPin(pinKey); - - if (existingPin) + // Pin already exists. + // If it is the same, we do nothing. If it is different, check for the --force arg + if (pin == existingPin) { - auto packageName = availableVersion->GetProperty(PackageVersionProperty::Name); - - // Pin already exists. - // If it is the same, we do nothing. If it is different, check for the --force arg - if (pin == existingPin) - { - AICLI_LOG(CLI, Info, << "Pin already exists"); - context.Reporter.Info() << Resource::String::PinAlreadyExists(packageName) << std::endl; - continue; - } + AICLI_LOG(CLI, Info, << "Pin already exists"); + context.Reporter.Info() << Resource::String::PinAlreadyExists(packageNameToReport) << std::endl; + continue; + } - AICLI_LOG(CLI, Info, << "Another pin already exists for the package for source " << pinKey.SourceId); - if (context.Args.Contains(Execution::Args::Type::Force)) - { - AICLI_LOG(CLI, Info, << "Overwriting pin due to --force argument"); - context.Reporter.Warn() << Resource::String::PinExistsOverwriting(packageName) << std::endl; - pinsToAddOrUpdate.push_back(std::move(pin)); - } - else - { - context.Reporter.Error() << Resource::String::PinExistsUseForceArg(packageName) << std::endl; - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); - } + AICLI_LOG(CLI, Info, << "Another pin already exists for the package for source " << pinKey.SourceId); + if (context.Args.Contains(Execution::Args::Type::Force)) + { + AICLI_LOG(CLI, Info, << "Overwriting pin due to --force argument"); + context.Reporter.Warn() << Resource::String::PinExistsOverwriting(packageNameToReport) << std::endl; + pinsToAddOrUpdate.push_back(std::move(pin)); } else { - pinsToAddOrUpdate.push_back(std::move(pin)); + context.Reporter.Error() << Resource::String::PinExistsUseForceArg(packageNameToReport) << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); } } + else + { + pinsToAddOrUpdate.push_back(std::move(pin)); + } } if (!pinsToAddOrUpdate.empty()) @@ -173,8 +189,7 @@ namespace AppInstaller::CLI::Workflow void RemovePin(Execution::Context& context) { auto package = context.Get(); - std::vector pins; - std::set sources; + auto pins = context.Get(); auto pinningIndex = context.Get(); bool pinExists = false; @@ -188,25 +203,11 @@ namespace AppInstaller::CLI::Workflow auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); - for (const auto& versionKey : packageVersionKeys) + for (const auto& pin : pins) { - auto availableVersion = package->GetAvailableVersion(versionKey); - const auto availablePackageId = availableVersion->GetProperty(PackageVersionProperty::Id).get(); - const auto availableSourceId = availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get(); - const auto pinKeys = Pinning::GetPinKeysForAvailablePackage(availablePackageId, availableSourceId, installedProductCodes, installedPackageFamilyNames); - - for (const auto& pinKey : pinKeys) - { - if (sources.emplace(pinKey.SourceId).second) - { - if (pinningIndex->GetPin(pinKey)) - { - AICLI_LOG(CLI, Info, << "Removing Pin " << pinKey.ToString()); - pinningIndex->RemovePin(pinKey); - pinExists = true; - } - } - } + AICLI_LOG(CLI, Info, << "Removing Pin " << pin.GetKey().ToString()); + pinningIndex->RemovePin(pin.GetKey()); + pinExists = true; } if (!pinExists) @@ -247,18 +248,30 @@ namespace AppInstaller::CLI::Workflow for (const auto& match : searchResult.Matches) { auto installedVersion = match.Package->GetInstalledVersion(); - auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); - if (availableVersion) + Utility::LocIndString sourceName; + + if (pinKey.IsForInstalled()) { - table.OutputLine({ - searchResult.Matches[0].Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Name), - pinKey.PackageId, - searchResult.Matches[0].Package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Version), - availableVersion->GetProperty(PackageVersionProperty::SourceName), - std::string{ ToString(pin.GetType()) }, - pin.GetGatedVersion().ToString(), - }); + sourceName = Resource::LocString{ Resource::String::PinInstalledSource }; } + else + { + // This ensures we get the info from the right source if it exists on multiple + auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); + if (availableVersion) + { + sourceName = availableVersion->GetProperty(PackageVersionProperty::SourceName); + } + } + + table.OutputLine({ + installedVersion->GetProperty(PackageVersionProperty::Name), + pinKey.PackageId, + installedVersion->GetProperty(PackageVersionProperty::Version), + sourceName, + std::string{ ToString(pin.GetType()) }, + pin.GetGatedVersion().ToString(), + }); } } @@ -294,31 +307,4 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Info() << Resource::String::PinNoPinsExist << std::endl; } } - - void CrossReferencePinsWithSource(Execution::Context& context) - { - const auto& pins = context.Get(); - const auto& source = context.Get(); - - std::vector matchingPins; - for (const auto& pin : pins) - { - const auto& pinKey = pin.GetKey(); - auto searchRequest = GetSearchRequestForPin(pin.GetKey()); - auto searchResult = source.Search(searchRequest); - - // Ensure the match comes from the right source - for (const auto& match : searchResult.Matches) - { - auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); - if (availableVersion) - { - matchingPins.push_back(pin); - } - } - } - - context.Add(std::move(matchingPins)); - } - } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index b9a15003a2..0b2e978975 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -26,7 +26,8 @@ namespace AppInstaller::CLI::Workflow void GetAllPins(Execution::Context& context); // Searches for all the pins associated with a package. - // There may be several if a package is available from multiple sources. + // There may be several if a package is available from multiple sources + // or if the pin is for the installed package. // Required Args: None // Inputs: PinningIndex, Package // Outputs: Pins diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index c016075b71..741addc590 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1592,6 +1592,13 @@ Please specify one of them using the --source option to proceed. Block from upgrading until the pin is removed, preventing override arguments + + Pin a specific installed version + + + Installed + Value used in a table to indicate that a package comes from the list of packages installed in the machine + Export settings as JSON diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index f35d61179a..ab3d0948a8 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1376,7 +1376,7 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo }; // We will pin the first package only - PinKey pinKey(packageId, setup.Available->Details.Identifier, productCode1); + PinKey pinKey = PinKey::GetPinKeyForInstalled(productCode1); auto pinningIndex = PinningIndex::OpenOrCreateDefault(); REQUIRE(pinningIndex); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 90c0421a94..a13c312053 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -98,6 +98,8 @@ namespace AppInstaller::Repository } } } + + return {}; } // Returns true for fields that provide a strong match; one that is not based on a heuristic. From 3963a4abc0e6cdf2df8c53fae45a8c279055c19b Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 3 May 2023 16:00:25 -0700 Subject: [PATCH 16/26] Fix list of available versions --- src/AppInstallerCLITests/TestSource.cpp | 2 +- src/AppInstallerCLITests/TestSource.h | 2 +- .../CompositeSource.cpp | 19 ++++++++++--------- .../Microsoft/SQLiteIndexSource.cpp | 4 ++-- .../Public/winget/RepositorySearch.h | 2 +- .../Rest/RestSource.cpp | 2 +- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/AppInstallerCLITests/TestSource.cpp b/src/AppInstallerCLITests/TestSource.cpp index 8aec0e4367..8b1794dda7 100644 --- a/src/AppInstallerCLITests/TestSource.cpp +++ b/src/AppInstallerCLITests/TestSource.cpp @@ -177,7 +177,7 @@ namespace TestCommon return InstalledVersion; } - std::vector TestPackage::GetAvailableVersionKeys() const + std::vector TestPackage::GetAvailableVersionKeys(PinBehavior) const { std::vector result; for (const auto& version : AvailableVersions) diff --git a/src/AppInstallerCLITests/TestSource.h b/src/AppInstallerCLITests/TestSource.h index af6a4fc9dd..0d09f51231 100644 --- a/src/AppInstallerCLITests/TestSource.h +++ b/src/AppInstallerCLITests/TestSource.h @@ -63,7 +63,7 @@ namespace TestCommon AppInstaller::Utility::LocIndString GetProperty(AppInstaller::Repository::PackageProperty property) const override; std::shared_ptr GetInstalledVersion() const override; - std::vector GetAvailableVersionKeys() const override; + std::vector GetAvailableVersionKeys(AppInstaller::Repository::PinBehavior) const override; std::shared_ptr GetLatestAvailableVersion(AppInstaller::Repository::PinBehavior) const override; std::shared_ptr GetAvailableVersion(const AppInstaller::Repository::PackageVersionKey& versionKey) const override; bool IsUpdateAvailable(AppInstaller::Repository::PinBehavior) const override; diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index a13c312053..f1b8840b32 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -67,7 +67,7 @@ namespace AppInstaller::Repository } } - return Pinning::IsStricter(pinTypeFromAvailable, pinTypeFromAvailable) ? pinTypeFromAvailable : pinTypeFromInstalled; + return Pinning::IsStricter(pinTypeFromAvailable, pinTypeFromInstalled) ? pinTypeFromAvailable : pinTypeFromInstalled; } // Gets the latest available version that satisfies both the available pin (already tagged on the keys) @@ -458,7 +458,7 @@ namespace AppInstaller::Repository return m_package->GetInstalledVersion(); } - std::vector GetAvailableVersionKeys() const override + std::vector GetAvailableVersionKeys(PinBehavior pinBehavior) const override { auto result = m_package->GetAvailableVersionKeys(); if (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_pin.has_value()) @@ -466,7 +466,7 @@ namespace AppInstaller::Repository // Add pin information to all version keys for (auto& pvk : result) { - pvk.PinnedState = GetPinnedStateForVersion(pvk, m_pin, PinBehavior::ConsiderPins); + pvk.PinnedState = GetPinnedStateForVersion(pvk, m_pin, pinBehavior); } } @@ -480,7 +480,7 @@ namespace AppInstaller::Repository std::shared_ptr GetLatestAvailableVersion(PinBehavior pinBehavior) const override { - auto availableVersionKeys = GetAvailableVersionKeys(); + auto availableVersionKeys = GetAvailableVersionKeys(pinBehavior); auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, /* installedPin */ {}, pinBehavior); if (!latestVersionKey) { @@ -592,21 +592,22 @@ namespace AppInstaller::Repository return {}; } - std::vector GetAvailableVersionKeys() const override + std::vector GetAvailableVersionKeys(PinBehavior pinBehavior) const override { std::vector result; auto installedPin = GetInstalledPin(); for (const auto& availablePackage : m_availablePackages) { - auto versionKeys = availablePackage.GetAvailableVersionKeys(); + auto versionKeys = availablePackage.GetAvailableVersionKeys(pinBehavior); + // Filter out versions covered by the installed pin. + // Pins from available package were already considered. if (installedPin) { - // Filter out available versions with pin from the installed package for (const auto& versionKey : versionKeys) { - if (GetPinnedStateForVersion(versionKey, installedPin, PinBehavior::ConsiderPins) == Pinning::PinType::Unknown) + if (GetPinnedStateForVersion(versionKey, installedPin, pinBehavior) == Pinning::PinType::Unknown) { result.emplace_back(versionKey); } @@ -633,7 +634,7 @@ namespace AppInstaller::Repository std::shared_ptr GetLatestAvailableVersion(PinBehavior pinBehavior) const override { - auto availableVersionKeys = GetAvailableVersionKeys(); + auto availableVersionKeys = GetAvailableVersionKeys(pinBehavior); auto latestVersionKey = GetLatestAvailableVersionKeySatisfyingPin(availableVersionKeys, GetInstalledPin(), pinBehavior); if (!latestVersionKey) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp index 6d390afe05..1b47d804b4 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp @@ -263,7 +263,7 @@ namespace AppInstaller::Repository::Microsoft return {}; } - std::vector GetAvailableVersionKeys() const override + std::vector GetAvailableVersionKeys(PinBehavior) const override { std::shared_ptr source = GetReferenceSource(); std::vector versions = source->GetIndex().GetVersionKeysById(m_idId); @@ -335,7 +335,7 @@ namespace AppInstaller::Repository::Microsoft return GetLatestVersionInternal(); } - std::vector GetAvailableVersionKeys() const override + std::vector GetAvailableVersionKeys(PinBehavior) const override { return {}; } diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h index 69f31b61a3..e0cd95d492 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h @@ -349,7 +349,7 @@ namespace AppInstaller::Repository // The versions will be returned in sorted, descending order. // Ex. { 4, 3, 2, 1 } // The list may contain versions from multiple sources. - virtual std::vector GetAvailableVersionKeys() const = 0; + virtual std::vector GetAvailableVersionKeys(PinBehavior pinBehavior = PinBehavior::ConsiderPins) const = 0; // Gets a specific version of this package. virtual std::shared_ptr GetLatestAvailableVersion(PinBehavior pinBehavior) const = 0; diff --git a/src/AppInstallerRepositoryCore/Rest/RestSource.cpp b/src/AppInstallerRepositoryCore/Rest/RestSource.cpp index 2578b487fc..c61f77641d 100644 --- a/src/AppInstallerRepositoryCore/Rest/RestSource.cpp +++ b/src/AppInstallerRepositoryCore/Rest/RestSource.cpp @@ -57,7 +57,7 @@ namespace AppInstaller::Repository::Rest return {}; } - std::vector GetAvailableVersionKeys() const override + std::vector GetAvailableVersionKeys(PinBehavior) const override { std::shared_ptr source = GetReferenceSource(); std::scoped_lock versionsLock{ m_packageVersionsLock }; From a93a7d5f1996c1fe391409512f6c375a8e9eccf9 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 3 May 2023 16:17:49 -0700 Subject: [PATCH 17/26] Fix pinned state for available versions --- src/AppInstallerCLITests/CompositeSource.cpp | 4 ++-- .../CompositeSource.cpp | 19 ++++++------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index ab3d0948a8..519b859ffb 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1345,8 +1345,8 @@ TEST_CASE("CompositeSource_Pinning_MultipleInstalled", "[CompositeSource][PinFlo std::string productCode2 = "product-code2"; // Installed packages differ in product code and version - auto installedPackage1 = MakeInstalled().WithId(packageId).WithPC(productCode1).WithVersion("1.1"sv); - auto installedPackage2 = MakeInstalled().WithId(packageId).WithPC(productCode2).WithVersion("1.2"sv); + auto installedPackage1 = MakeInstalled().WithId(productCode1).WithPC(productCode1).WithVersion("1.1"sv); + auto installedPackage2 = MakeInstalled().WithId(productCode2).WithPC(productCode2).WithVersion("1.2"sv); CompositeTestSetup setup; setup.Installed->SearchFunction = [&](const SearchRequest& request) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index f1b8840b32..258f44a645 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -423,7 +423,6 @@ namespace AppInstaller::Repository auto availableVersion = m_package->GetLatestAvailableVersion(PinBehavior::IgnorePins); if (availableVersion) { - // TODO: Check if needed for installed m_sourceId = availableVersion->GetSource().GetIdentifier(); } } @@ -601,23 +600,17 @@ namespace AppInstaller::Repository { auto versionKeys = availablePackage.GetAvailableVersionKeys(pinBehavior); - // Filter out versions covered by the installed pin. - // Pins from available package were already considered. + // The version keys we have already have pin information from the available package. + // Here we also add information from the installed package. if (installedPin) { - for (const auto& versionKey : versionKeys) + for (auto& versionKey : versionKeys) { - if (GetPinnedStateForVersion(versionKey, installedPin, pinBehavior) == Pinning::PinType::Unknown) - { - result.emplace_back(versionKey); - } + versionKey.PinnedState = GetPinnedStateForVersion(versionKey, installedPin, pinBehavior); } } - else - { - // Copy all version keys if we don't need to filter from installed pin - std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); - } + + std::copy(versionKeys.begin(), versionKeys.end(), std::back_inserter(result)); } // Remove all elements whose channel does not match the installed package. From 2d56521b3d77b3417e2116db583de9d20e45c746 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 3 May 2023 17:16:38 -0700 Subject: [PATCH 18/26] Use package ID directly without installed check --- .github/actions/spelling/allow.txt | 1 + .../Commands/PinCommand.cpp | 36 ++++++++++++++++--- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 29 +++++++++------ src/AppInstallerCLICore/Workflows/PinFlow.h | 8 +---- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index cade6a429a..b765ddb03f 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -397,6 +397,7 @@ pfp PGP php PII +pinnable pinningindex pipssource PKCS diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index e85709c793..8ebf05249c 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -114,9 +114,22 @@ namespace AppInstaller::CLI void PinAddCommand::ExecuteInternal(Execution::Context& context) const { + if (context.Args.Contains(Execution::Args::Type::Id)) + { + // When we are given an ID, just pin that available package without checking for installed. + // This helps when there are matching issues, for example due to multiple side-by-side installs. + context << + Workflow::OpenSource(); + } + else + { + // If not working from just ID, try matching a single installed package + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + } + context << - Workflow::OpenSource() << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(OperationType::Pin) << @@ -186,9 +199,22 @@ namespace AppInstaller::CLI void PinRemoveCommand::ExecuteInternal(Execution::Context& context) const { + if (context.Args.Contains(Execution::Args::Type::Id)) + { + // When we are given an ID, just un-pin that available package without checking for installed. + // This helps when there are matching issues, for example due to multiple side-by-side installs. + context << + Workflow::OpenSource(); + } + else + { + // If not working from just ID, try matching a single installed package + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + } + context << - Workflow::OpenSource() << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(OperationType::Pin) << @@ -262,7 +288,7 @@ namespace AppInstaller::CLI Workflow::OpenPinningIndex(/* readOnly */ true) << Workflow::GetAllPins << Workflow::OpenSource() << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed, false, Repository::CompositeSearchBehavior::AllPackages) << Workflow::ReportPins; } diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index fcdec5df6f..0a423f93ff 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -124,8 +124,8 @@ namespace AppInstaller::CLI::Workflow auto pinKeys = GetPinKeysForPackage(context); auto package = context.Get(); - auto installedVersion = context.Get(); auto pinningIndex = context.Get(); + auto installedVersion = context.Get(); std::vector pinsToAddOrUpdate; for (const auto& pinKey : pinKeys) @@ -136,8 +136,12 @@ namespace AppInstaller::CLI::Workflow auto existingPin = pinningIndex->GetPin(pinKey); if (existingPin) { - Utility::LocIndString packageNameToReport = installedVersion->GetProperty(PackageVersionProperty::Name); - if (!pinKey.IsForInstalled()) + Utility::LocIndString packageNameToReport; + if (pinKey.IsForInstalled() && installedVersion) + { + packageNameToReport = installedVersion->GetProperty(PackageVersionProperty::Name); + } + else { auto availableVersion = package->GetAvailableVersion({ pinKey.SourceId, "", "" }); if (availableVersion) @@ -199,10 +203,6 @@ namespace AppInstaller::CLI::Workflow // So, we remove pins from all sources unless one was provided. auto packageVersionKeys = package->GetAvailableVersionKeys(); - auto installedVersion = package->GetInstalledVersion(); - auto installedProductCodes = installedVersion->GetMultiProperty(PackageVersionMultiProperty::ProductCode); - auto installedPackageFamilyNames = installedVersion->GetMultiProperty(PackageVersionMultiProperty::PackageFamilyName); - for (const auto& pin : pins) { AICLI_LOG(CLI, Info, << "Removing Pin " << pin.GetKey().ToString()); @@ -247,8 +247,9 @@ namespace AppInstaller::CLI::Workflow auto searchResult = source.Search(searchRequest); for (const auto& match : searchResult.Matches) { - auto installedVersion = match.Package->GetInstalledVersion(); + Utility::LocIndString packageName; Utility::LocIndString sourceName; + Utility::LocIndString version; if (pinKey.IsForInstalled()) { @@ -260,14 +261,22 @@ namespace AppInstaller::CLI::Workflow auto availableVersion = match.Package->GetAvailableVersion({ pinKey.SourceId, "", "" }); if (availableVersion) { + packageName = availableVersion->GetProperty(PackageVersionProperty::Name); sourceName = availableVersion->GetProperty(PackageVersionProperty::SourceName); } } + auto installedVersion = match.Package->GetInstalledVersion(); + if (installedVersion) + { + packageName = installedVersion->GetProperty(PackageVersionProperty::Name); + version = installedVersion->GetProperty(PackageVersionProperty::Version); + } + table.OutputLine({ - installedVersion->GetProperty(PackageVersionProperty::Name), + packageName, pinKey.PackageId, - installedVersion->GetProperty(PackageVersionProperty::Version), + version, sourceName, std::string{ ToString(pin.GetType()) }, pin.GetGatedVersion().ToString(), diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index 0b2e978975..2e0e55a5db 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -36,7 +36,7 @@ namespace AppInstaller::CLI::Workflow // Adds a pin for the current package. // A separate pin will be added for each source. // Required Args: None - // Inputs: PinningIndex, Package + // Inputs: PinningIndex, Package, InstalledVersion? // Outputs: None void AddPin(Execution::Context& context); @@ -59,10 +59,4 @@ namespace AppInstaller::CLI::Workflow // Inputs: None // Outputs: None void ResetAllPins(Execution::Context& context); - - // Updates the list of pins to include only those matching the current open source. - // Required Args: None - // Inputs: Pins, Source - // Outputs: None - void CrossReferencePinsWithSource(Execution::Context& context); } From f1b56fd6bd3843c0648833785ef1875b5b9724c1 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:12:58 -0700 Subject: [PATCH 19/26] Add missing spaces --- src/AppInstallerCLITests/CompositeSource.cpp | 2 +- src/AppInstallerCLITests/MsixManifest.cpp | 4 ++-- src/AppInstallerCLITests/PinningIndex.cpp | 4 ++-- src/AppInstallerCLITests/RestInterface_1_0.cpp | 2 +- src/AppInstallerCLITests/RestInterface_1_1.cpp | 4 ++-- src/AppInstallerRepositoryCore/InstalledFilesCorrelation.cpp | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/AppInstallerCLITests/CompositeSource.cpp b/src/AppInstallerCLITests/CompositeSource.cpp index 519b859ffb..e2dbc5daab 100644 --- a/src/AppInstallerCLITests/CompositeSource.cpp +++ b/src/AppInstallerCLITests/CompositeSource.cpp @@ -1187,7 +1187,7 @@ TEST_CASE("CompositeSource_Pinning_AvailableVersionPinned", "[CompositeSource][P SECTION("Gated to 1.0.*") { pinningIndex->AddPin(Pin::CreateGatingPin(PinKey{ pinKey }, GatedVersion{ "1.0.*"sv })); - expectedResult.ResultsForPinBehavior[PinBehavior::ConsiderPins] = { /* IsUpdateAvailable */ false, /* LatestAvailableVersion */ "1.0.1"}; + expectedResult.ResultsForPinBehavior[PinBehavior::ConsiderPins] = { /* IsUpdateAvailable */ false, /* LatestAvailableVersion */ "1.0.1" }; // Gating pins are not affected by --include-pinned expectedResult.ResultsForPinBehavior[PinBehavior::IncludePinned] = expectedResult.ResultsForPinBehavior[PinBehavior::ConsiderPins]; diff --git a/src/AppInstallerCLITests/MsixManifest.cpp b/src/AppInstallerCLITests/MsixManifest.cpp index bccb56665a..b15552afec 100644 --- a/src/AppInstallerCLITests/MsixManifest.cpp +++ b/src/AppInstallerCLITests/MsixManifest.cpp @@ -22,8 +22,8 @@ namespace constexpr std::string_view expectedFamilyName = "FakeInstallerForTesting_125rzkzqaqjwj"; PackageVersion expectedPackageVersion { 0xAAAABBBBCCCCDDDD }; constexpr std::string_view expectedWindowsDesktopName = "Windows.Desktop"; - OSVersion expectedWindowsDesktopMinVersion { "10.0.16299.0"}; - OSVersion expectedWindowsUniversalMinVersion { "10.0.0.0"}; + OSVersion expectedWindowsDesktopMinVersion { "10.0.16299.0" }; + OSVersion expectedWindowsUniversalMinVersion { "10.0.0.0" }; } TEST_CASE("MsixManifest_ValidateFieldsParsedFromManifestReader", "[MsixManifest]") diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp index 8a073d92ea..c4dae0409f 100644 --- a/src/AppInstallerCLITests/PinningIndex.cpp +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -98,8 +98,8 @@ TEST_CASE("PinningIndex_AddUpdateRemove", "[pinningIndex]") TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); - Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId"}, { "1.0.*"sv }); - Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId"}); + Pin pin = Pin::CreateGatingPin({ "pkgId", "srcId" }, { "1.0.*"sv }); + Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId" }); { PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); diff --git a/src/AppInstallerCLITests/RestInterface_1_0.cpp b/src/AppInstallerCLITests/RestInterface_1_0.cpp index 59eb6e7f00..6e76598d26 100644 --- a/src/AppInstallerCLITests/RestInterface_1_0.cpp +++ b/src/AppInstallerCLITests/RestInterface_1_0.cpp @@ -280,7 +280,7 @@ TEST_CASE("Search_GoodResponse", "[RestSource][Interface_1_0]") "Publisher": "git", "Versions": [ { "PackageVersion": "1.0.0" }, - { "PackageVersion": "2.0.0"}] + { "PackageVersion": "2.0.0" }] }] })delimiter"); diff --git a/src/AppInstallerCLITests/RestInterface_1_1.cpp b/src/AppInstallerCLITests/RestInterface_1_1.cpp index 0c710a6edc..babb975b26 100644 --- a/src/AppInstallerCLITests/RestInterface_1_1.cpp +++ b/src/AppInstallerCLITests/RestInterface_1_1.cpp @@ -371,7 +371,7 @@ TEST_CASE("Search_BadRequest_UnsupportedPackageMatchFields", "[RestSource][Inter "Publisher": "git", "Versions": [ { "PackageVersion": "1.0.0" }, - { "PackageVersion": "2.0.0"}] + { "PackageVersion": "2.0.0" }] }] })delimiter"); @@ -394,7 +394,7 @@ TEST_CASE("Search_GoodRequest_OnlyMarketRequired", "[RestSource][Interface_1_1]" "Publisher": "git", "Versions": [ { "PackageVersion": "1.0.0" }, - { "PackageVersion": "2.0.0"}] + { "PackageVersion": "2.0.0" }] }] })delimiter"); diff --git a/src/AppInstallerRepositoryCore/InstalledFilesCorrelation.cpp b/src/AppInstallerRepositoryCore/InstalledFilesCorrelation.cpp index db8bc35636..8278204afd 100644 --- a/src/AppInstallerRepositoryCore/InstalledFilesCorrelation.cpp +++ b/src/AppInstallerRepositoryCore/InstalledFilesCorrelation.cpp @@ -16,7 +16,7 @@ namespace AppInstaller::Repository::Correlation constexpr std::string_view s_ShellLinkFileExtension = ".lnk"sv; const std::vector> s_CandidateInstallLocationRoots = { - { Filesystem::GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%"}, + { Filesystem::GetKnownFolderPath(FOLDERID_LocalAppData), "%LOCALAPPDATA%" }, { Filesystem::GetKnownFolderPath(FOLDERID_ProgramFiles), "%PROGRAMFILES%" }, { Filesystem::GetKnownFolderPath(FOLDERID_ProgramFilesX86), "%PROGRAMFILES(X86)%" }, }; From 049f48aff79e639052224d1b6d9550b397fd781f Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:13:40 -0700 Subject: [PATCH 20/26] Update comment --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 258f44a645..670317bfe2 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -31,8 +31,10 @@ namespace AppInstaller::Repository } // Gets the pinned state for an available PackageVersionKey that may have a pin, - // and optionally an additional pin that could come from the installed version. - // If there are both installed and available pins, we return the one that is the most strict. + // and optionally an additional pin that could come from the installed version or + // be a pin we have not considered yet for the version key. + // If there are a pin both in the version key and passed as an argument, + // we return the one that is the most strict. Pinning::PinType GetPinnedStateForVersion( const PackageVersionKey& availableVersionKey, const std::optional& pin, From 2fc037c54fa520a969e2bebd0eecd0b8e9c29f2c Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:14:37 -0700 Subject: [PATCH 21/26] Use string_view --- src/AppInstallerCommonCore/Pin.cpp | 2 +- src/AppInstallerCommonCore/Public/winget/Pin.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 398ff902c9..179f2a5182 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -71,7 +71,7 @@ namespace AppInstaller::Pinning return ss.str(); } - PinKey PinKey::GetPinKeyForInstalled(std::string systemReferenceString) + PinKey PinKey::GetPinKeyForInstalled(std::string_view systemReferenceString) { return { systemReferenceString, s_installedSourceId }; } diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index e1ef0719d7..51376ce4fb 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -46,7 +46,7 @@ namespace AppInstaller::Pinning // Gets a pin key that refers to an installed package by its ProductCode or PackageFamilyName. // The sourceId used is a special string to distinguish from available packages. - static PinKey GetPinKeyForInstalled(std::string systemReferenceString); + static PinKey GetPinKeyForInstalled(std::string_view systemReferenceString); bool IsForInstalled() const; From a306756930c235aaed4190f4c0db247e9378cfd9 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:17:12 -0700 Subject: [PATCH 22/26] nullopt --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 670317bfe2..fc04d15bbb 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -818,7 +818,7 @@ namespace AppInstaller::Repository { return (ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::Pinning) && m_installedPackage) ? m_installedPackage->GetPin() - : std::optional{}; + : std::nullopt; } std::optional m_installedPackage; From d493bb3d79bc84789914822987678789fb66a0c5 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:24:22 -0700 Subject: [PATCH 23/26] Simplify comparison --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index fc04d15bbb..ca330d1470 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -689,13 +689,9 @@ namespace AppInstaller::Repository return false; } - if (m_installedPackage) + if (m_installedPackage && !m_installedPackage->GetPackage()->IsSame(otherComposite->m_installedPackage->GetPackage().get())) { - if (m_installedPackage->GetSourceId() != otherComposite->m_installedPackage->GetSourceId() || - !m_installedPackage->GetPackage()->IsSame(otherComposite->m_installedPackage->GetPackage().get())) - { - return false; - } + return false; } for (size_t i = 0; i < m_availablePackages.size(); ++i) From fe502e93fafae9b9a646c7f0a97e549165301589 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 12:24:31 -0700 Subject: [PATCH 24/26] Update comment --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index ca330d1470..6819cc8ae8 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -766,7 +766,10 @@ namespace AppInstaller::Repository { if (!m_installedPackage) { - // Only installed packages have pins + // We only care about pins for installed packages. + // It is possible to add pins for packages that are not installed + // by using the ID, but that bypasses the CompositeSource altogether + // so we don't need to check for that. return; } From a7656f92a577b3ce5aa65ee973d2ec0bb185e024 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 13:12:36 -0700 Subject: [PATCH 25/26] Include pins on non-installed --- .../CompositeSource.cpp | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 6819cc8ae8..985b5bbca9 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -764,15 +764,6 @@ namespace AppInstaller::Repository // Gets the information about the pins that exist for this package void GetExistingPins(PinningIndex& pinningIndex) { - if (!m_installedPackage) - { - // We only care about pins for installed packages. - // It is possible to add pins for packages that are not installed - // by using the ID, but that bypasses the CompositeSource altogether - // so we don't need to check for that. - return; - } - for (auto& availablePackage : m_availablePackages) { Pinning::PinKey pinKey = GetPinKeyForAvailable(availablePackage.GetPackage().get()); @@ -784,14 +775,17 @@ namespace AppInstaller::Repository } } - Pinning::PinKey pinKey = Pinning::PinKey::GetPinKeyForInstalled( - m_installedPackage->GetProperty(PackageProperty::Id).get() - ); - - auto pin = pinningIndex.GetPin(pinKey); - if (pin.has_value()) + if (m_installedPackage) { - m_installedPackage->SetPin(std::move(pin.value())); + Pinning::PinKey pinKey = Pinning::PinKey::GetPinKeyForInstalled( + m_installedPackage->GetProperty(PackageProperty::Id).get() + ); + + auto pin = pinningIndex.GetPin(pinKey); + if (pin.has_value()) + { + m_installedPackage->SetPin(std::move(pin.value())); + } } } From bd396446cb996ef59bf744f20de95a1e81a424fd Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 9 May 2023 13:31:49 -0700 Subject: [PATCH 26/26] Update comments --- .../CompositeSource.cpp | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 985b5bbca9..43d6fd477a 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -35,6 +35,9 @@ namespace AppInstaller::Repository // be a pin we have not considered yet for the version key. // If there are a pin both in the version key and passed as an argument, // we return the one that is the most strict. + // Note that for a package with both available and installed pins, we will call this + // twice: once with the available pin to set the pinned state in the version key, + // and once with the installed pin to set the final pinned state. Pinning::PinType GetPinnedStateForVersion( const PackageVersionKey& availableVersionKey, const std::optional& pin, @@ -46,30 +49,31 @@ namespace AppInstaller::Repository return Pinning::PinType::Unknown; } - // For the pin in the available version, we can ignore it depending on the behavior and type. - // If it is gating, we don't need to check the version as that was already done. - Pinning::PinType pinTypeFromAvailable = Pinning::PinType::Unknown; + // For the pin in the version version, we can ignore it depending on the behavior and type. + // If it is gating, we don't need to check the version as that was already done when the + // PinnedState info was added (and we don't have the gated version here). + Pinning::PinType pinnedStateFromVersionKey = Pinning::PinType::Unknown; if (availableVersionKey.PinnedState == Pinning::PinType::Blocking || (availableVersionKey.PinnedState == Pinning::PinType::Pinning && pinBehavior != PinBehavior::IncludePinned) || availableVersionKey.PinnedState == Pinning::PinType::Gating) { - pinTypeFromAvailable = availableVersionKey.PinnedState; + pinnedStateFromVersionKey = availableVersionKey.PinnedState; } - // For the pin in the installed version, we can ignore it depending on the behavior and type. + // For the additional pin, we can ignore it depending on the behavior and type. // If it is gating, we need to check the version. - Pinning::PinType pinTypeFromInstalled = Pinning::PinType::Unknown; + Pinning::PinType pinnedStateFromAdditionalPin = Pinning::PinType::Unknown; if (pin) { if (pin->GetType() == Pinning::PinType::Blocking || (pin->GetType() == Pinning::PinType::Pinning && pinBehavior != PinBehavior::IncludePinned) || (pin->GetType() == Pinning::PinType::Gating && !pin->GetGatedVersion().IsValidVersion(availableVersionKey.Version))) { - pinTypeFromInstalled = pin->GetType(); + pinnedStateFromAdditionalPin = pin->GetType(); } } - return Pinning::IsStricter(pinTypeFromAvailable, pinTypeFromInstalled) ? pinTypeFromAvailable : pinTypeFromInstalled; + return Pinning::IsStricter(pinnedStateFromVersionKey, pinnedStateFromAdditionalPin) ? pinnedStateFromVersionKey : pinnedStateFromAdditionalPin; } // Gets the latest available version that satisfies both the available pin (already tagged on the keys)