From 92b78f693d73716c8f9cbc4e15cbba6a3c76c3a3 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 12 Dec 2022 14:59:28 -0800 Subject: [PATCH 01/20] Add pin index --- .../Commands/PinCommand.cpp | 99 ++++++++--- .../AppInstallerCommonCore.vcxproj | 2 + .../AppInstallerCommonCore.vcxproj.filters | 6 + src/AppInstallerCommonCore/Pin.cpp | 66 ++++++++ .../Public/AppInstallerVersions.h | 10 ++ .../Public/winget/Pin.h | 57 +++++++ .../AppInstallerRepositoryCore.vcxproj | 7 + ...AppInstallerRepositoryCore.vcxproj.filters | 24 +++ .../Microsoft/PinningIndex.cpp | 96 +++++++++++ .../Microsoft/PinningIndex.h | 54 ++++++ .../Microsoft/Schema/IPinningIndex.h | 33 ++++ .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 157 ++++++++++++++++++ .../Microsoft/Schema/Pinning_1_0/PinTable.h | 26 +++ .../Pinning_1_0/PinningIndexInterface.h | 26 +++ .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 81 +++++++++ .../Schema/Portable_1_0/PortableTable.cpp | 2 +- 16 files changed, 721 insertions(+), 25 deletions(-) create mode 100644 src/AppInstallerCommonCore/Pin.cpp create mode 100644 src/AppInstallerCommonCore/Public/winget/Pin.h create mode 100644 src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp create mode 100644 src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h create mode 100644 src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h create mode 100644 src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp create mode 100644 src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h create mode 100644 src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h create mode 100644 src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index 5cdedb26b0..bb13d7deec 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -76,15 +76,26 @@ namespace AppInstaller::CLI void PinAddCommand::Complete(Execution::Context& context, Args::Type valueType) const { + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + switch (valueType) { - case Args::Type::Query: - case Args::Type::Id: - case Args::Type::Name: - case Args::Type::Moniker: - case Args::Type::Source: + case Execution::Args::Type::Query: + context << + Workflow::RequireCompletionWordNonEmpty << + Workflow::SearchSourceForManyCompletion << + Workflow::CompleteWithMatchedField; + break; + case Execution::Args::Type::Id: + case Execution::Args::Type::Name: + case Execution::Args::Type::Moniker: + case Execution::Args::Type::Source: + case Execution::Args::Type::Tag: + case Execution::Args::Type::Command: context << - Workflow::CompleteWithSingleSemanticsForValue(valueType); + Workflow::CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); break; } } @@ -106,7 +117,14 @@ namespace AppInstaller::CLI void PinAddCommand::ExecuteInternal(Execution::Context& context) const { // TODO - Command::ExecuteInternal(context); + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << + Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << + Workflow::EnsureMatchesFromSearchResult(true) << + Workflow::SearchPin << + Workflow::AddPin; } std::vector PinRemoveCommand::GetArguments() const @@ -137,15 +155,26 @@ namespace AppInstaller::CLI void PinRemoveCommand::Complete(Execution::Context& context, Args::Type valueType) const { + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + switch (valueType) { - case Args::Type::Query: - case Args::Type::Id: - case Args::Type::Name: - case Args::Type::Moniker: - case Args::Type::Source: + case Execution::Args::Type::Query: + context << + Workflow::RequireCompletionWordNonEmpty << + Workflow::SearchSourceForManyCompletion << + Workflow::CompleteWithMatchedField; + break; + case Execution::Args::Type::Id: + case Execution::Args::Type::Name: + case Execution::Args::Type::Moniker: + case Execution::Args::Type::Source: + case Execution::Args::Type::Tag: + case Execution::Args::Type::Command: context << - Workflow::CompleteWithSingleSemanticsForValue(valueType); + Workflow::CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); break; } } @@ -157,8 +186,14 @@ namespace AppInstaller::CLI void PinRemoveCommand::ExecuteInternal(Execution::Context& context) const { - // TODO - Command::ExecuteInternal(context); + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << + Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << + Workflow::EnsureMatchesFromSearchResult(true) << + Workflow::SearchPin << + Workflow::RemovePin; } std::vector PinListCommand::GetArguments() const @@ -189,15 +224,26 @@ namespace AppInstaller::CLI void PinListCommand::Complete(Execution::Context& context, Args::Type valueType) const { + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed); + switch (valueType) { - case Args::Type::Query: - case Args::Type::Id: - case Args::Type::Name: - case Args::Type::Moniker: - case Args::Type::Source: + case Execution::Args::Type::Query: context << - Workflow::CompleteWithSingleSemanticsForValue(valueType); + Workflow::RequireCompletionWordNonEmpty << + Workflow::SearchSourceForManyCompletion << + Workflow::CompleteWithMatchedField; + break; + case Execution::Args::Type::Id: + case Execution::Args::Type::Name: + case Execution::Args::Type::Moniker: + case Execution::Args::Type::Source: + case Execution::Args::Type::Tag: + case Execution::Args::Type::Command: + context << + Workflow::CompleteWithSingleSemanticsForValueUsingExistingSource(valueType); break; } } @@ -210,7 +256,13 @@ namespace AppInstaller::CLI void PinListCommand::ExecuteInternal(Execution::Context& context) const { // TODO - Command::ExecuteInternal(context); + context << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << + Workflow::SearchSourceForMany << + Workflow::HandleSearchResultFailures << + Workflow::SearchPins << + Workflow::ReportPins; } std::vector PinResetCommand::GetArguments() const @@ -237,7 +289,6 @@ namespace AppInstaller::CLI void PinResetCommand::ExecuteInternal(Execution::Context& context) const { - // TODO - Command::ExecuteInternal(context); + context << Workflow::ResetAllPins; } } diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj index 7c51c73099..961eddb16b 100644 --- a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj +++ b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj @@ -326,6 +326,7 @@ + @@ -394,6 +395,7 @@ + diff --git a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters index 574833856a..836105d370 100644 --- a/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters +++ b/src/AppInstallerCommonCore/AppInstallerCommonCore.vcxproj.filters @@ -222,6 +222,9 @@ Public\winget + + Public\winget + @@ -392,6 +395,9 @@ Source Files + + Source Files + diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp new file mode 100644 index 0000000000..9ca422785a --- /dev/null +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "winget/Pin.h" + +namespace AppInstaller::Pinning +{ + using namespace std::string_view_literals; + + std::string_view ToString(PinType type) + { + switch (type) + { + case PinType::Blocking: + return "Blocking"sv; + case PinType::Pinning: + return "Pinning"sv; + case PinType::Gating: + return "Gating"sv; + case PinType::Unknown: + default: + return "Unknown"; + } + } + + Pin Pin::CreateBlockingPin(PinKey&& pinKey) + { + return { PinType::Blocking, std::move(pinKey) }; + } + + Pin Pin::CreatePinningPin(PinKey&& pinKey) + { + return { PinType::Pinning, std::move(pinKey) }; + } + + Pin Pin::CreateGatingPin(PinKey&& pinKey, AppInstaller::Utility::GatedVersion gatedVersion) + { + return { PinType::Gating, std::move(pinKey), gatedVersion }; + } + + PinType Pin::GetType() const + { + return m_type; + } + + const PinKey& Pin::GetKey() const + { + return m_id; + } + + const AppInstaller::Manifest::Manifest::string_t& Pin::GetPackageId() const + { + return m_id.PackageId; + } + + std::string_view Pin::GetSourceId() const + { + return m_id.SourceId; + } + + AppInstaller::Utility::GatedVersion Pin::GetGatedVersion() const + { + THROW_HR_IF(E_NOT_VALID_STATE, m_type != PinType::Gating); + return m_gatedVersion.value(); + } +} \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerVersions.h b/src/AppInstallerCommonCore/Public/AppInstallerVersions.h index 954c76a12b..aa46ad7187 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerVersions.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerVersions.h @@ -169,6 +169,16 @@ namespace AppInstaller::Utility bool m_isEmpty = false; }; + // A range of versions indicated by a version and optionally a wildcard at the end. + struct GatedVersion + { + GatedVersion(); + GatedVersion(std::string_view s); + std::string ToString() const; + private: + + }; + // A channel string; existing solely to give a type. // // Compared lexicographically. diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h new file mode 100644 index 0000000000..39daab92e8 --- /dev/null +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -0,0 +1,57 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include +#include + +namespace AppInstaller::Pinning +{ + enum class PinType + { + Unknown, + // 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. + Gating, + }; + + std::string_view ToString(PinType type); + + // The set of values needed to uniquely identify a Pin + struct PinKey + { + PinKey() {} + PinKey(const Manifest::Manifest::string_t& packageId, std::string_view sourceId) + : PackageId(packageId), SourceId(sourceId) {} + + Manifest::Manifest::string_t PackageId; + std::string SourceId; + }; + + struct Pin + { + static Pin CreateBlockingPin(PinKey&& pinKey); + static Pin CreatePinningPin(PinKey&& pinKey); + static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion gatedVersion); + + PinType GetType() const; + const PinKey& GetKey() const; + const Manifest::Manifest::string_t& GetPackageId() const; + std::string_view GetSourceId() const; + + // Only available for PinType Gating + Utility::GatedVersion GetGatedVersion() const; + + private: + Pin(PinType type, PinKey&& pinKey, std::optional gatedVersion = {}) + : m_type(type), m_id(std::move(pinKey)), m_gatedVersion(gatedVersion) {} + + PinType m_type = PinType::Unknown; + PinKey m_id; + std::optional m_gatedVersion; + }; +} diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj index 2f97d0b62e..fad805b911 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -237,6 +237,7 @@ + @@ -273,8 +274,11 @@ + + + @@ -339,6 +343,7 @@ + @@ -361,6 +366,8 @@ + + diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters index 07bfc49bc3..8ffbdc73ca 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -76,6 +76,9 @@ {d9d70cf5-ce04-4db2-a0ec-970dd0ad22b6} + + {f05e19bb-2161-4ab0-9d04-2dfa2d3eb3c6} + @@ -339,6 +342,18 @@ Rest\Schema + + Microsoft\Schema + + + Microsoft + + + Microsoft\Schema\Pinning_1_0 + + + Microsoft\Schema\Pinning_1_0 + @@ -539,6 +554,15 @@ Source Files + + Source Files + + + Source Files + + + Microsoft\Schema\Pinning_1_0 + diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp new file mode 100644 index 0000000000..ca6ffd5e22 --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -0,0 +1,96 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "PinningIndex.h" +#include "SQLiteStorageBase.h" +#include "Schema/Pinning_1_0/PinningIndexInterface.h" + +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 << "'"); + PinningIndex result{ filePath, version }; + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "pinningIndex_createNew"); + + // Use calculated version, as incoming version could be 'latest' + result.m_version.SetSchemaVersion(result.m_dbconn); + + result.m_interface->CreateTables(result.m_dbconn); + + result.SetLastWriteTime(); + + savepoint.Commit(); + + return result; + } + + 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())); + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_addPin"); + + IdType result = m_interface->AddPin(m_dbconn, pin); + + SetLastWriteTime(); + + savepoint.Commit(); + + return result; + } + + void PinningIndex::RemovePin(const Pinning::PinKey& pinKey) + { + AICLI_LOG(Repo, Verbose, << "Removing Pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + std::lock_guard lockInterface{ *m_interfaceLock }; + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_removePin"); + + m_interface->RemovePin(m_dbconn, pinKey); + + SetLastWriteTime(); + + savepoint.Commit(); + } + + std::optional PinningIndex::GetPin(const Pinning::PinKey& pinKey) + { + std::lock_guard lockInterface{ *m_interfaceLock }; + return m_interface->GetPin(m_dbconn, pinKey); + } + + std::vector PinningIndex::GetAllPins(SQLite::Connection& connection) + { + std::lock_guard lockInterface{ *m_interfaceLock }; + return m_interface->GetAllPins(connection); + } + + std::unique_ptr PinningIndex::CreateIPinningIndex() const + { + if (m_version == Schema::Version{ 1, 0 } || + m_version.MajorVersion == 1 || + m_version.IsLatest()) + { + return std::make_unique(); + } + + THROW_HR(HRESULT_FROM_WIN32(ERROR_NOT_SUPPORTED)); + } + + PinningIndex::PinningIndex(const std::string& target, SQLiteStorageBase::OpenDisposition disposition, Utility::ManagedFile&& indexFile) : + SQLiteStorageBase(target, disposition, std::move(indexFile)) + { + AICLI_LOG(Repo, Info, << "Opened Pinning Index with version [" << m_version << "], last write [" << GetLastWriteTime() << "]"); + m_interface = CreateIPinningIndex(); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_CANNOT_WRITE_TO_UPLEVEL_INDEX, disposition == SQLiteStorageBase::OpenDisposition::ReadWrite && m_version != m_interface->GetVersion()); + } + + PinningIndex::PinningIndex(const std::string& target, Schema::Version version) : SQLiteStorageBase(target, version) + { + m_interface = CreateIPinningIndex(); + m_version = m_interface->GetVersion(); + } +} \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h new file mode 100644 index 0000000000..73ce98ac9b --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -0,0 +1,54 @@ +#pragma once +#include "SQLiteWrapper.h" +#include "Microsoft/Schema/IPinningIndex.h" +#include "Microsoft/SQLiteStorageBase.h" +#include "winget/Pin.h" +#include + +namespace AppInstaller::Repository::Microsoft +{ + struct PinningIndex : SQLiteStorageBase + { + // An id that refers to a specific Pinning file. + using IdType = SQLite::rowid_t; + + PinningIndex(const PinningIndex&) = delete; + PinningIndex& operator=(const PinningIndex&) = delete; + + PinningIndex(PinningIndex&&) = default; + PinningIndex& operator=(PinningIndex&&) = default; + + // Creates a new PinningIndex database of the given version. + static PinningIndex CreateNew(const std::string& filePath, Schema::Version version = Schema::Version::Latest()); + + // Opens an existing PinningIndex database. + static PinningIndex Open(const std::string& filePath, OpenDisposition disposition, Utility::ManagedFile&& indexFile = {}) + { + return { filePath, disposition, std::move(indexFile) }; + } + + // Adds a pin to the index. + IdType AddPin(const Pinning::Pin& pin); + + // Removes a pin from the index. + void RemovePin(const Pinning::PinKey& pinKey); + + // Returns the current pin for a given package if it exists. + std::optional GetPin(const Pinning::PinKey& pinKey); + + // Returns a vector containing all the existing pins. + std::vector GetAllPins(SQLite::Connection& connection); + + private: + // Constructor used to open an existing index. + PinningIndex(const std::string& target, SQLiteStorageBase::OpenDisposition disposition, Utility::ManagedFile&& indexFile); + + // Constructor used to create a new index. + PinningIndex(const std::string& target, Schema::Version version); + + // Creates the IPinningIndex interface object for this version. + std::unique_ptr CreateIPinningIndex() const; + + std::unique_ptr m_interface; + }; +} \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h new file mode 100644 index 0000000000..12414a132a --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "SQLiteWrapper.h" +#include "Microsoft/Schema/Version.h" +#include "winget/Pin.h" + +namespace AppInstaller::Repository::Microsoft::Schema +{ + struct IPinningIndex + { + virtual ~IPinningIndex() = default; + + // Gets the schema version that this index interface is built for. + virtual Schema::Version GetVersion() const = 0; + + // Creates all of the version dependent tables within the database. + virtual void CreateTables(SQLite::Connection& connection) = 0; + + // Version 1.0 + // Adds a pin to the index. + virtual SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) = 0; + + // Removes a pin from the index. + virtual SQLite::rowid_t RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) = 0; + + // Returns the current pin for a given package if it exists. + virtual std::optional GetPin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) = 0; + + // Returns a vector containing all the existing pins. + virtual std::vector GetAllPins(SQLite::Connection& connection) = 0; + }; +} \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp new file mode 100644 index 0000000000..d87abef076 --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -0,0 +1,157 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "PinTable.h" +#include "SQLiteStatementBuilder.h" +#include "Microsoft/Schema/IPinningIndex.h" + +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 gatedVersion) + { + switch (type) + { + case Pinning::PinType::Blocking: + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + case Pinning::PinType::Pinning: + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + case Pinning::PinType::Gating: + return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ gatedVersion }); + default: + return {} + } + } + } + + using namespace std::string_view_literals; + static constexpr std::string_view s_PinTable_Table_Name = "pin"sv; + static constexpr std::string_view s_PinTable_PackageId_Column = "packageId"sv; + static constexpr std::string_view s_PinTable_SourceId_Column = "sourceId"sv; + static constexpr std::string_view s_PinTable_Type_Column = "type"sv; + static constexpr std::string_view s_PinTable_GatedVersion_Column = "gatedVersion"sv; + static constexpr std::string_view s_PinTable_Index = "pinIndex"sv; + + std::string_view PinTable::TableName() + { + return s_PinTable_Table_Name; + } + + void PinTable::Create(SQLite::Connection& connection) + { + using namespace SQLite::Builder; + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createPinTable_v1_0"); + + StatementBuilder createTableBuilder; + createTableBuilder.CreateTable(s_PinTable_Table_Name).BeginColumns(); + + 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_Type_Column, Type::Int64).NotNull()); + createTableBuilder.Column(ColumnBuilder(s_PinTable_GatedVersion_Column, Type::Text)); + + 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.Execute(connection); + + savepoint.Commit(); + } + + std::optional PinTable::SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + { + SQLite::Builder::StatementBuilder builder; + builder.Select(SQLite::RowIDName).From(s_PinTable_Table_Name) + .Where(s_PinTable_PackageId_Column).Equals(pinKey.PackageId) + .And(s_PinTable_SourceId_Column).Equals(pinKey.SourceId); + + SQLite::Statement select = builder.Prepare(connection); + + if (select.Step()) + { + return select.GetColumn(0); + } + else + { + return {}; + } + } + + SQLite::rowid_t PinTable::AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) + { + SQLite::Builder::StatementBuilder builder; + builder.InsertInto(s_PinTable_Table_Name) + .Columns({ + s_PinTable_PackageId_Column, + s_PinTable_SourceId_Column, + s_PinTable_Type_Column, + s_PinTable_GatedVersion_Column }) + .Values( + pin.GetPackageId(), + pin.GetSourceId(), + pin.GetType(), + pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : ""); + + builder.Execute(connection); + return connection.GetLastInsertRowID(); + + } + + SQLite::rowid_t PinTable::RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId) + { + SQLite::Builder::StatementBuilder builder; + builder.DeleteFrom(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); + builder.Execute(connection); + } + + std::optional PinTable::GetPinById(SQLite::Connection& connection, const SQLite::rowid_t pinId) + { + SQLite::Builder::StatementBuilder builder; + builder.Select({ + s_PinTable_PackageId_Column, + s_PinTable_SourceId_Column, + s_PinTable_Type_Column, + s_PinTable_GatedVersion_Column }) + .From(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); + + SQLite::Statement select = builder.Prepare(connection); + + if (!select.Step()) + { + return {}; + } + + auto [packageId, sourceId, type, gatedVersion] = select.GetRow(); + return GetPinFromRow(packageId, sourceId, type, gatedVersion); + } + + std::vector PinTable::GetAllPins(SQLite::Connection& connection) + { + SQLite::Builder::StatementBuilder builder; + builder.Select({ + s_PinTable_PackageId_Column, + s_PinTable_SourceId_Column, + s_PinTable_Type_Column, + s_PinTable_GatedVersion_Column }) + .From(s_PinTable_Table_Name); + + SQLite::Statement select = builder.Prepare(connection); + + std::vector pins; + while (select.Step()) + { + auto [packageId, sourceId, type, gatedVersion] = select.GetRow(); + auto pin = GetPinFromRow(packageId, sourceId, type, gatedVersion); + if (pin) { + pins.push_back(std::move(pin.value())); + } + } + + return pins; + } +} \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h new file mode 100644 index 0000000000..bd4d3ad6d3 --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "SQLiteWrapper.h" +#include "SQLiteStatementBuilder.h" +#include "Microsoft/Schema/IPinningIndex.h" +#include + +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +{ + // A table + struct PinTable + { + // Get the table name. + static std::string_view TableName(); + + // Creates the table with named indices. + static void Create(SQLite::Connection& connection); + + static std::optional SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); + static SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin); + static SQLite::rowid_t RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); + static std::optional GetPinById(SQLite::Connection& connection, const SQLite::rowid_t pinId); + static std::vector GetAllPins(SQLite::Connection& connection); + }; +} diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h new file mode 100644 index 0000000000..862e8ce1ac --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h @@ -0,0 +1,26 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include "Microsoft/Schema/IPinningIndex.h" + +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +{ + struct PinningIndexInterface : public IPinningIndex + { + // Version 1.0 + Schema::Version GetVersion() const override; + void CreateTables(SQLite::Connection& connection) override; + + private: + SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) override; + + // Removes a pin from the index. + SQLite::rowid_t RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; + + // Returns the current pin for a given package if it exists. + std::optional GetPin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; + + // Returns a vector containing all the existing pins. + std::vector GetAllPins(SQLite::Connection& connection) override; + }; +} 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 new file mode 100644 index 0000000000..44beb525d9 --- /dev/null +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp @@ -0,0 +1,81 @@ +// 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" + +namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 +{ + namespace + { + std::optional GetExistingPinId(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + { + auto result = PinTable::SelectByPinKey(connection, pinKey); + + if (!result) + { + AICLI_LOG(Repo, Verbose, << "Did not find a pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); + } + + return result; + } + + } + + // Version 1.0 + Schema::Version PinningIndexInterface::GetVersion() const + { + return { 1, 0 }; + } + + void CreateTables(SQLite::Connection& connection) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpinningtables_v1_0"); + Pinning_V1_0::PinTable::Create(connection); + savepoint.Commit(); + } + + SQLite::rowid_t PinningIndexInterface::AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) + { + auto existingPin = GetExistingPinId(connection, pin.GetKey()); + + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), existingPin.has_value()); + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addpin_v1_0"); + SQLite::rowid_t pinId = PinTable::AddPin(connection, pin); + + savepoint.Commit(); + return pinId; + } + + SQLite::rowid_t PinningIndexInterface::RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + { + auto existingPinId = GetExistingPinId(connection, pinKey); + + // If the pin doesn't exist, fail the remove + THROW_HR_IF(E_NOT_SET, !existingPinId); + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removepin_v1_0"); + PinTable::RemovePinById(connection, existingPinId.value()); + + savepoint.Commit(); + return existingPinId.value(); + } + + std::optional PinningIndexInterface::GetPin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + { + auto existingPinId = GetExistingPinId(connection, pinKey); + + if (!existingPinId) + { + return {}; + } + + return PinTable::GetPinById(connection, existingPinId.value()); + } + + std::vector PinningIndexInterface::GetAllPins(SQLite::Connection& connection) + { + return PinTable::GetAllPins(connection); + } +} \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Portable_1_0/PortableTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Portable_1_0/PortableTable.cpp index 2efca323fd..247ec33689 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Portable_1_0/PortableTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Portable_1_0/PortableTable.cpp @@ -172,7 +172,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Portable_V1_0 portableFile.SymlinkTarget = std::move(symlinkTarget); result.emplace_back(std::move(portableFile)); } - + return result; } } \ No newline at end of file From a24c0de65a72bb26adfb11c1bf584fd91774e0a1 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Wed, 14 Dec 2022 16:34:56 -0800 Subject: [PATCH 02/20] Complete pinning index; implement workflow tasks --- .../AppInstallerCLICore.vcxproj | 1 + .../AppInstallerCLICore.vcxproj.filters | 5 +- .../Commands/PinCommand.cpp | 22 +- .../ExecutionContextData.h | 19 ++ src/AppInstallerCLICore/Resources.h | 8 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 223 ++++++++++++++++++ src/AppInstallerCLICore/Workflows/PinFlow.h | 46 ++++ .../Shared/Strings/en-us/winget.resw | 26 ++ src/AppInstallerCommonCore/Errors.cpp | 2 + src/AppInstallerCommonCore/Pin.cpp | 21 +- .../Public/AppInstallerErrors.h | 1 + .../Public/AppInstallerRuntime.h | 2 + .../Public/AppInstallerVersions.h | 13 +- .../Public/winget/Pin.h | 15 +- .../Microsoft/PinningIndex.cpp | 28 ++- .../Microsoft/PinningIndex.h | 7 +- .../Microsoft/Schema/IPinningIndex.h | 7 + .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 38 ++- .../Microsoft/Schema/Pinning_1_0/PinTable.h | 4 +- .../Pinning_1_0/PinningIndexInterface.h | 8 +- .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 29 ++- 21 files changed, 487 insertions(+), 38 deletions(-) create mode 100644 src/AppInstallerCLICore/Workflows/PinFlow.cpp diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj index effaa5d8b7..e43c58fb68 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj @@ -341,6 +341,7 @@ + diff --git a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters index 53e912fc82..f2983e3f44 100644 --- a/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters +++ b/src/AppInstallerCLICore/AppInstallerCLICore.vcxproj.filters @@ -349,8 +349,11 @@ Source Files + + Workflows + - Source Files + Commands diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index bb13d7deec..3b93c96d80 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -120,9 +120,11 @@ namespace AppInstaller::CLI context << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::SearchSourceForMany << + Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << - Workflow::EnsureMatchesFromSearchResult(true) << + Workflow::EnsureOneMatchFromSearchResult(false) << + Workflow::ReportPackageIdentity << + Workflow::OpenPinningIndex << Workflow::SearchPin << Workflow::AddPin; } @@ -189,9 +191,11 @@ namespace AppInstaller::CLI context << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::SearchSourceForMany << + Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << - Workflow::EnsureMatchesFromSearchResult(true) << + Workflow::EnsureOneMatchFromSearchResult(false) << + Workflow::ReportPackageIdentity << + Workflow::OpenPinningIndex << Workflow::SearchPin << Workflow::RemovePin; } @@ -257,11 +261,11 @@ namespace AppInstaller::CLI { // TODO context << + Workflow::OpenPinningIndex << + Workflow::GetAllPins << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::SearchSourceForMany << - Workflow::HandleSearchResultFailures << - Workflow::SearchPins << + Workflow::CrossReferencePinsWithSource << Workflow::ReportPins; } @@ -289,6 +293,8 @@ namespace AppInstaller::CLI void PinResetCommand::ExecuteInternal(Execution::Context& context) const { - context << Workflow::ResetAllPins; + context << + Workflow::OpenPinningIndex << + Workflow::ResetAllPins; } } diff --git a/src/AppInstallerCLICore/ExecutionContextData.h b/src/AppInstallerCLICore/ExecutionContextData.h index 9ab78e1873..6011e4b397 100644 --- a/src/AppInstallerCLICore/ExecutionContextData.h +++ b/src/AppInstallerCLICore/ExecutionContextData.h @@ -4,6 +4,7 @@ #include #include #include +#include #include "CompletionData.h" #include "PackageCollection.h" #include "PortableInstaller.h" @@ -16,6 +17,10 @@ #include #include +namespace AppInstaller::Repository::Microsoft +{ + struct PinningIndex; +} namespace AppInstaller::CLI::Execution { @@ -55,6 +60,8 @@ namespace AppInstaller::CLI::Execution AllowedArchitectures, AllowUnknownScope, PortableInstaller, + PinningIndex, + Pins, Max }; @@ -229,5 +236,17 @@ namespace AppInstaller::CLI::Execution { using value_t = CLI::Portable::PortableInstaller; }; + + template <> + struct DataMapping + { + using value_t = std::shared_ptr; + }; + + template <> + struct DataMapping + { + using value_t = std::vector; + }; } } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 1122c02259..ffd536b62f 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -98,6 +98,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(FlagContainAdjoinedError); WINGET_DEFINE_RESOURCE_STRINGID(ForceArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(GatedVersionArgumentDescription); + WINGET_DEFINE_RESOURCE_STRINGID(GatedVersion); WINGET_DEFINE_RESOURCE_STRINGID(GetManifestResultVersionNotFound); WINGET_DEFINE_RESOURCE_STRINGID(HashCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(HashCommandShortDescription); @@ -242,14 +243,21 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinAddBlockingArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinAddCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinAddCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinAdded); + WINGET_DEFINE_RESOURCE_STRINGID(PinAlreadyExists); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinExistsOverwriting); + WINGET_DEFINE_RESOURCE_STRINGID(PinExistsUseForceArg); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinRemoveCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinRemoveCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinResetCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinResetCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinResettingAll); + WINGET_DEFINE_RESOURCE_STRINGID(PinResetUseForceArg); + WINGET_DEFINE_RESOURCE_STRINGID(PinType); WINGET_DEFINE_RESOURCE_STRINGID(PoliciesDisabled); WINGET_DEFINE_RESOURCE_STRINGID(PoliciesEnabled); WINGET_DEFINE_RESOURCE_STRINGID(PoliciesPolicy); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp new file mode 100644 index 0000000000..52cccf8138 --- /dev/null +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -0,0 +1,223 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "Resources.h" +#include "PinFlow.h" +#include "TableOutput.h" +#include "Microsoft/PinningIndex.h" +#include "Microsoft/SQLiteStorageBase.h" +#include "winget/RepositorySearch.h" + +using namespace AppInstaller::Repository; +using namespace AppInstaller::Repository::Microsoft; + +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) + { + if (context.Args.Contains(Execution::Args::Type::BlockingPin)) + { + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + } + else if (context.Args.Contains(Execution::Args::Type::GatedVersion)) + { + return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, context.Args.GetArg(Execution::Args::Type::GatedVersion)); + } + else + { + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + } + } + } + + void OpenPinningIndex(Execution::Context& context) + { + auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; + + AICLI_LOG(CLI, Info, << "Openning pinning index"); + auto pinningIndex = std::filesystem::exists(indexPath) ? + PinningIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : + PinningIndex::CreateNew(indexPath.u8string()); + + context.Add(std::make_shared(std::move(pinningIndex))); + } + + void GetAllPins(Execution::Context& context) + { + AICLI_LOG(CLI, Info, << "Getting all existing pins"); + context.Add(context.Get()->GetAllPins()); + } + + void SearchPin(Execution::Context& context) + { + AICLI_LOG(CLI, Info, << "SEARCHING PIN"); + auto package = context.Get(); + std::vector pins; + + // TODO: We should support querying the multiple sources for a package, instead of just one + auto availableVersion = package->GetLatestAvailableVersion(); + + auto pinningIndex = context.Get(); + auto pin = pinningIndex->GetPin({ + availableVersion->GetProperty(PackageVersionProperty::Id).get(), + availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }); + if (pin) + { + pins.emplace_back(std::move(pin.value())); + } + + context.Add(std::move(pins)); + } + + // Adds a pin for the current package. + // A separate pin will be added for each source. + // Required Args: None + // Inputs: PinningIndex, Package + // Outputs: None + void AddPin(Execution::Context& context) + { + auto package = context.Get(); + std::vector pins; + + // TODO: We should support querying the multiple sources for a package, instead of just one + auto availableVersion = package->GetLatestAvailableVersion(); + + Pinning::PinKey pinKey{ + availableVersion->GetProperty(PackageVersionProperty::Id).get(), + availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; + auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId); + AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package[" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); + + auto pinningIndex = context.Get(); + 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) + { + AICLI_LOG(CLI, Info, << "Pin already exists"); + context.Reporter.Info() << Resource::String::PinAlreadyExists << std::endl; + return; + } + + AICLI_LOG(CLI, Info, << "Another pin already exists for the package"); + if (context.Args.Contains(Execution::Args::Type::Force)) + { + AICLI_LOG(CLI, Info, << "Overwriting pin due to --force argument"); + context.Reporter.Warn() << Resource::String::PinExistsOverwriting << std::endl; + pinningIndex->UpdatePin(pin); + } + else + { + context.Reporter.Error() << Resource::String::PinExistsUseForceArg << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); + } + } + else + { + pinningIndex->AddPin(pin); + AICLI_LOG(CLI, Info, << "Finished adding pin"); + context.Reporter.Info() << Resource::String::PinAdded << std::endl; + } + } + + // Removes all the pins associated with a package. + void RemovePin(Execution::Context& context) + { + auto package = context.Get(); + std::vector pins; + + // TODO: We should support querying the multiple sources for a package, instead of just one + auto availableVersion = package->GetLatestAvailableVersion(); + + auto pinningIndex = context.Get(); + Pinning::PinKey pinKey{ + availableVersion->GetProperty(PackageVersionProperty::Id).get(), + availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; + if (!pinningIndex->GetPin(pinKey)) + { + // TODO: Report pin not found + } + + pinningIndex->RemovePin(pinKey); + } + + // Report the pins in a table. + // Required Args: None + // Inputs: Pins + // Outputs: None + void ReportPins(Execution::Context& context) + { + // TODO: Use package and source names + Execution::TableOutput<4> table(context.Reporter, + { + Resource::String::SearchId, + Resource::String::SearchSource, + Resource::String::PinType, + Resource::String::GatedVersion + }); + + for (const auto& pin : context.Get()) + { + // TODO: Avoid these conversions to string + table.OutputLine({ + pin.GetPackageId(), + std::string{ pin.GetSourceId() }, + std::string{ ToString(pin.GetType()) }, + pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : "", + }); + } + + table.Complete(); + } + + // Resets all the existing pins. + // Required Args: None + // Inputs: PinningIndex + // Outputs: None + void ResetAllPins(Execution::Context& context) + { + AICLI_LOG(CLI, Info, << "Resetting all pins"); + if (context.Args.Contains(Execution::Args::Type::Force)) + { + context.Reporter.Info() << Resource::String::PinResettingAll << std::endl; + auto pinningIndex = context.Get(); + pinningIndex->ResetAllPins(); + } + else + { + AICLI_LOG(CLI, Info, << "--force argument is not present"); + context.Reporter.Info() << Resource::String::PinResetUseForceArg << std::endl; + + // TODO: Report pins here + } + } + + // 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) + { + const auto& pins = context.Get(); + const auto& source = context.Get(); + + std::vector matchingPins; + std::copy_if(pins.begin(), pins.end(), std::back_inserter(matchingPins), [&](Pinning::Pin pin) { + // TODO: Filter to source + SearchRequest searchRequest; + searchRequest.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, pin.GetPackageId()); + auto searchResult = source.Search(searchRequest); + + return !searchResult.Matches.empty(); + }); + + 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 504ed5782e..bc03696ad8 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -5,4 +5,50 @@ namespace AppInstaller::CLI::Workflow { + // Opens the pinning index for use in future operations. + // Required Args: None + // Inputs: None + // Outputs: PinningIndex + void OpenPinningIndex(Execution::Context& context); + + // Gets all the pins from the index. + // Required Args: None + // Inputs: PinningIndex + // Outputs: Pins + 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. + // Required Args: None + // Inputs: PinningIndex, Package + // Outputs Pins + void SearchPin(Execution::Context& context); + + // Adds a pin for the current package. + // A separate pin will be added for each source. + // Required Args: None + // Inputs: PinningIndex, Package + // Outputs: None + void AddPin(Execution::Context& context); + + // Removes all the pins associated with a package. + void RemovePin(Execution::Context& context); + + // Report the pins in a table. + // Required Args: None + // Inputs: Pins + // Outputs: None + void ReportPins(Execution::Context& context); + + // Resets all the existing pins. + // Required Args: None + // Inputs: PinningIndex + // 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); } diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 73044e944f..cafb90b948 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1526,4 +1526,30 @@ Please specify one of them using the --source option to proceed. Block from updating until the pin is removed, preventing override arguments + + Gated version + + + Pin added successfully + + + The pin already exists + + + A pin already exists for this package. Overwriting due to the --force argument. + {Locked="--force"} + + + A pin already exists for this package. Use the --force argument to overwrite it. + {Locked="--force"} + + + Resetting all current pins. + + + Use the --force argument to reset all pins. + + + Pin type + \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp index 0dce355f0e..dbc4738e36 100644 --- a/src/AppInstallerCommonCore/Errors.cpp +++ b/src/AppInstallerCommonCore/Errors.cpp @@ -208,6 +208,8 @@ namespace AppInstaller return "Archive malware scan failed."; case APPINSTALLER_CLI_ERROR_PACKAGE_ALREADY_INSTALLED: return "Found at least one version of the package installed."; + case APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS: + return "A pin already exists for the package."; // Install errors case APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE: diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 9ca422785a..5a551d003b 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -45,17 +45,17 @@ namespace AppInstaller::Pinning const PinKey& Pin::GetKey() const { - return m_id; + return m_key; } const AppInstaller::Manifest::Manifest::string_t& Pin::GetPackageId() const { - return m_id.PackageId; + return m_key.PackageId; } std::string_view Pin::GetSourceId() const { - return m_id.SourceId; + return m_key.SourceId; } AppInstaller::Utility::GatedVersion Pin::GetGatedVersion() const @@ -63,4 +63,19 @@ namespace AppInstaller::Pinning THROW_HR_IF(E_NOT_VALID_STATE, m_type != PinType::Gating); return m_gatedVersion.value(); } + + bool Pin::operator==(const Pin& other) const + { + if (m_type != other.m_type || m_key != other.m_key) + { + return false; + } + + if (m_type != PinType::Gating) + { + return false; + } + + return m_gatedVersion == other.m_gatedVersion; + } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index 130a556b0b..7aee237d29 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -110,6 +110,7 @@ #define APPINSTALLER_CLI_ERROR_INSTALL_LOCATION_REQUIRED ((HRESULT)0x8A15005F) #define APPINSTALLER_CLI_ERROR_ARCHIVE_SCAN_FAILED ((HRESULT)0x8A150060) #define APPINSTALLER_CLI_ERROR_PACKAGE_ALREADY_INSTALLED ((HRESULT)0x8A150061) +#define APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS ((HRESULT)0x8A150062) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index 451a2fc33b..56f3cdd19e 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -62,6 +62,8 @@ namespace AppInstaller::Runtime PortableLinksUserLocation, // The location where symlinks to portable packages are stored under machine scope. PortableLinksMachineLocation, + + PinningIndex, }; // The principal that an ACE applies to. diff --git a/src/AppInstallerCommonCore/Public/AppInstallerVersions.h b/src/AppInstallerCommonCore/Public/AppInstallerVersions.h index aa46ad7187..6b4a01cd0c 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerVersions.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerVersions.h @@ -172,11 +172,16 @@ namespace AppInstaller::Utility // A range of versions indicated by a version and optionally a wildcard at the end. struct GatedVersion { - GatedVersion(); - GatedVersion(std::string_view s); - std::string ToString() const; - private: + // TODO + // For now, using dummy implementation that just holds a string + GatedVersion() {} + GatedVersion(std::string_view s) : m_tmp(s) {} + std::string ToString() const { return m_tmp; } + bool operator==(const GatedVersion& other) const { return m_tmp == other.m_tmp; } + + private: + std::string m_tmp; }; // A channel string; existing solely to give a type. diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index 39daab92e8..cee3b5e302 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -28,6 +28,15 @@ namespace AppInstaller::Pinning 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; + } + bool operator!=(const PinKey& other) const + { + return !(*this == other); + } + Manifest::Manifest::string_t PackageId; std::string SourceId; }; @@ -46,12 +55,14 @@ namespace AppInstaller::Pinning // Only available for PinType Gating Utility::GatedVersion GetGatedVersion() const; + bool operator==(const Pin& other) const; + private: Pin(PinType type, PinKey&& pinKey, std::optional gatedVersion = {}) - : m_type(type), m_id(std::move(pinKey)), m_gatedVersion(gatedVersion) {} + : m_type(type), m_key(std::move(pinKey)), m_gatedVersion(gatedVersion) {} PinType m_type = PinType::Unknown; - PinKey m_id; + PinKey m_key; std::optional m_gatedVersion; }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index ca6ffd5e22..b27104dd6d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -42,6 +42,24 @@ namespace AppInstaller::Repository::Microsoft return result; } + 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())); + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_updatePin"); + + bool result = m_interface->UpdatePin(m_dbconn, pin).first; + + if (result) + { + SetLastWriteTime(); + savepoint.Commit(); + } + + return result; + } + void PinningIndex::RemovePin(const Pinning::PinKey& pinKey) { AICLI_LOG(Repo, Verbose, << "Removing Pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); @@ -62,10 +80,16 @@ namespace AppInstaller::Repository::Microsoft return m_interface->GetPin(m_dbconn, pinKey); } - std::vector PinningIndex::GetAllPins(SQLite::Connection& connection) + std::vector PinningIndex::GetAllPins() + { + std::lock_guard lockInterface{ *m_interfaceLock }; + return m_interface->GetAllPins(m_dbconn); + } + + void PinningIndex::ResetAllPins() { std::lock_guard lockInterface{ *m_interfaceLock }; - return m_interface->GetAllPins(connection); + m_interface->ResetAllPins(m_dbconn); } std::unique_ptr PinningIndex::CreateIPinningIndex() const diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h index 73ce98ac9b..e35ff91366 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -30,6 +30,9 @@ namespace AppInstaller::Repository::Microsoft // Adds a pin to the index. IdType AddPin(const Pinning::Pin& pin); + // Updates a pin type, and gated version if needed + bool UpdatePin(const Pinning::Pin& pin); + // Removes a pin from the index. void RemovePin(const Pinning::PinKey& pinKey); @@ -37,7 +40,9 @@ namespace AppInstaller::Repository::Microsoft std::optional GetPin(const Pinning::PinKey& pinKey); // Returns a vector containing all the existing pins. - std::vector GetAllPins(SQLite::Connection& connection); + std::vector GetAllPins(); + + void ResetAllPins(); private: // Constructor used to open an existing index. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h index 12414a132a..732db3ba7e 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h @@ -21,6 +21,10 @@ namespace AppInstaller::Repository::Microsoft::Schema // Adds a pin to the index. virtual SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) = 0; + // Updates an existing pin in the index. + // The return value indicates whether the index was modified by the function. + virtual std::pair UpdatePin(SQLite::Connection& connection, const Pinning::Pin& pin) = 0; + // Removes a pin from the index. virtual SQLite::rowid_t RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) = 0; @@ -29,5 +33,8 @@ namespace AppInstaller::Repository::Microsoft::Schema // Returns a vector containing all the existing pins. virtual std::vector GetAllPins(SQLite::Connection& connection) = 0; + + // Removes all the pins from the index + virtual void ResetAllPins(SQLite::Connection& connection) = 0; }; } \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index d87abef076..c12c108a9d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -20,7 +20,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 case Pinning::PinType::Gating: return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ gatedVersion }); default: - return {} + return {}; } } } @@ -65,10 +65,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 std::optional PinTable::SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { + // TODO: The statement builder requires that the bound parameters can be converted to T&&, + // but the package/source IDs go as const T&. Find a way to avoid the weird casting. SQLite::Builder::StatementBuilder builder; builder.Select(SQLite::RowIDName).From(s_PinTable_Table_Name) - .Where(s_PinTable_PackageId_Column).Equals(pinKey.PackageId) - .And(s_PinTable_SourceId_Column).Equals(pinKey.SourceId); + .Where(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) + .And(s_PinTable_SourceId_Column).Equals((std::string_view)pinKey.SourceId); SQLite::Statement select = builder.Prepare(connection); @@ -84,6 +86,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::rowid_t PinTable::AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) { + // TODO: The statement builder requires that the bound parameters can be converted to T&&, + // but the package/source IDs go as const T&. Find a way to avoid the weird casting. SQLite::Builder::StatementBuilder builder; builder.InsertInto(s_PinTable_Table_Name) .Columns({ @@ -92,17 +96,32 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 s_PinTable_Type_Column, s_PinTable_GatedVersion_Column }) .Values( - pin.GetPackageId(), - pin.GetSourceId(), + (std::string_view)pin.GetPackageId(), + (std::string_view)pin.GetSourceId(), pin.GetType(), pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : ""); builder.Execute(connection); return connection.GetLastInsertRowID(); + } + bool PinTable::UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin) + { + // TODO: The statement builder requires that the bound parameters can be converted to T&&, + // but the package/source IDs go as const T&. Find a way to avoid the weird casting. + SQLite::Builder::StatementBuilder builder; + builder.Update(s_PinTable_Table_Name).Set() + .Column(s_PinTable_PackageId_Column).Equals((std::string_view)pin.GetPackageId()) + .Column(s_PinTable_SourceId_Column).Equals((std::string_view)pin.GetSourceId()) + .Column(s_PinTable_Type_Column).Equals(pin.GetType()) + .Column(s_PinTable_GatedVersion_Column).Equals(pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : "") + .Where(SQLite::RowIDName).Equals(pinId); + + builder.Execute(connection); + return connection.GetChanges() != 0; } - SQLite::rowid_t PinTable::RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId) + void PinTable::RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId) { SQLite::Builder::StatementBuilder builder; builder.DeleteFrom(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); @@ -154,4 +173,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 return pins; } + + void PinTable::ResetAllPins(SQLite::Connection& connection) + { + SQLite::Builder::StatementBuilder builder; + builder.DeleteFrom(s_PinTable_Table_Name); + builder.Execute(connection); + } } \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h index bd4d3ad6d3..9ba338c175 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -19,8 +19,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 static std::optional SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); static SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin); - static SQLite::rowid_t RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); + static bool UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin); + static void RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); static std::optional GetPinById(SQLite::Connection& connection, const SQLite::rowid_t pinId); static std::vector GetAllPins(SQLite::Connection& connection); + static void ResetAllPins(SQLite::Connection& connection); }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h index 862e8ce1ac..c6560d5534 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h @@ -13,14 +13,10 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 private: SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) override; - - // Removes a pin from the index. + std::pair UpdatePin(SQLite::Connection& connection, const Pinning::Pin& pin) override; SQLite::rowid_t RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; - - // Returns the current pin for a given package if it exists. std::optional GetPin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; - - // Returns a vector containing all the existing pins. std::vector GetAllPins(SQLite::Connection& connection) override; + void ResetAllPins(SQLite::Connection& connection) override; }; } 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 44beb525d9..446aee5588 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 @@ -28,9 +28,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 return { 1, 0 }; } - void CreateTables(SQLite::Connection& connection) + void PinningIndexInterface::CreateTables(SQLite::Connection& connection) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpinningtables_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createPinningTables"); Pinning_V1_0::PinTable::Create(connection); savepoint.Commit(); } @@ -41,13 +41,27 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), existingPin.has_value()); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addpin_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addPin_v1_0"); SQLite::rowid_t pinId = PinTable::AddPin(connection, pin); savepoint.Commit(); return pinId; } + std::pair PinningIndexInterface::UpdatePin(SQLite::Connection& connection, const Pinning::Pin& pin) + { + auto existingPinId = GetExistingPinId(connection, pin.GetKey()); + + // If the pin doesn't exist, fail the update + THROW_HR_IF(E_NOT_SET, !existingPinId); + + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addPin_v1_0"); + bool status = PinTable::UpdatePin(connection, existingPinId.value(), pin); + + savepoint.Commit(); + return { status, existingPinId.value() }; + } + SQLite::rowid_t PinningIndexInterface::RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { auto existingPinId = GetExistingPinId(connection, pinKey); @@ -55,7 +69,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // If the pin doesn't exist, fail the remove THROW_HR_IF(E_NOT_SET, !existingPinId); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removepin_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removePin_v1_0"); PinTable::RemovePinById(connection, existingPinId.value()); savepoint.Commit(); @@ -78,4 +92,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { return PinTable::GetAllPins(connection); } + + void PinningIndexInterface::ResetAllPins(SQLite::Connection& connection) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "resetPins_v1_0"); + PinTable::ResetAllPins(connection); + savepoint.Commit(); + } } \ No newline at end of file From 964a2014ec3d4541926d96ef889ea6c247aa40a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flor=20Chac=C3=B3n?= <14323496+florelis@users.noreply.github.com> Date: Thu, 15 Dec 2022 09:48:45 -0800 Subject: [PATCH 03/20] Apply suggestions from code review Co-authored-by: Kaleb Luedtke --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 52cccf8138..8c6cf7b129 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -37,7 +37,8 @@ namespace AppInstaller::CLI::Workflow { auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; - AICLI_LOG(CLI, Info, << "Openning pinning index"); + AICLI_LOG(CLI, Info, << "Opening pinning index"); + auto pinningIndex = std::filesystem::exists(indexPath) ? PinningIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : PinningIndex::CreateNew(indexPath.u8string()); @@ -89,7 +90,8 @@ namespace AppInstaller::CLI::Workflow availableVersion->GetProperty(PackageVersionProperty::Id).get(), availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId); - AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package[" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); + AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); + auto pinningIndex = context.Get(); auto existingPin = pinningIndex->GetPin(pinKey); From 27167a801ad11dff7ecb25f3de346133c9b4680e Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Thu, 15 Dec 2022 10:13:48 -0800 Subject: [PATCH 04/20] Spelling --- .github/actions/spelling/allow.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 2b75ac9a91..27e25d1bcc 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -253,6 +253,7 @@ INVALIDARG INVALIDSID iomanip iostream +IPinning IPortable ipmo ISAPPROVEDFOROUTPUT From ad6d007ba47d78401e5098f2c7a04e5eb3f3d804 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Thu, 15 Dec 2022 14:48:06 -0800 Subject: [PATCH 05/20] Add tests for pinning index --- .../AppInstallerCLITests.vcxproj | 1 + .../AppInstallerCLITests.vcxproj.filters | 3 + src/AppInstallerCLITests/PinningIndex.cpp | 167 ++++++++++++++++++ src/AppInstallerCommonCore/Pin.cpp | 2 +- .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 2 +- .../Microsoft/Schema/Pinning_1_0/PinTable.h | 2 +- .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 2 +- 7 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 src/AppInstallerCLITests/PinningIndex.cpp diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 06eb723967..41e73bfaaa 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -214,6 +214,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 2ced205cba..161af5859b 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -239,6 +239,9 @@ Source Files\Common + + Source Files\Repository + diff --git a/src/AppInstallerCLITests/PinningIndex.cpp b/src/AppInstallerCLITests/PinningIndex.cpp new file mode 100644 index 0000000000..b22e97c762 --- /dev/null +++ b/src/AppInstallerCLITests/PinningIndex.cpp @@ -0,0 +1,167 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include +#include +#include +#include + +using namespace std::string_literals; +using namespace TestCommon; +using namespace AppInstaller::Pinning; +using namespace AppInstaller::Repository::Microsoft; +using namespace AppInstaller::Repository::SQLite; +using namespace AppInstaller::Repository::Microsoft::Schema; + +TEST_CASE("PinningIndexCreateLatestAndReopen", "[pinningIndex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + Schema::Version versionCreated; + + // Create the index + { + PinningIndex index = PinningIndex::CreateNew(tempFile, Schema::Version::Latest()); + versionCreated = index.GetVersion(); + } + + // Reopen the index for read only + { + INFO("Trying with Read"); + PinningIndex index = PinningIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::Read); + Schema::Version versionRead = index.GetVersion(); + REQUIRE(versionRead == versionCreated); + } + + // Reopen the index for read/write + { + INFO("Trying with ReadWrite"); + PinningIndex index = PinningIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::ReadWrite); + Schema::Version versionRead = index.GetVersion(); + REQUIRE(versionRead == versionCreated); + } + + // Reopen the index for immutable read + { + INFO("Trying with Immutable"); + PinningIndex index = PinningIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::Immutable); + Schema::Version versionRead = index.GetVersion(); + REQUIRE(versionRead == versionCreated); + } +} + +TEST_CASE("PinningIndexAddEntryToTable", "[pinningIndex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + Pin pin = Pin::CreateBlockingPin({ "pkgId", "sourceId" }); + + { + PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); + index.AddPin(pin); + } + + { + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + auto pins = Pinning_V1_0::PinTable::GetAllPins(connection); + REQUIRE(pins.size() == 1); + REQUIRE(pins[0] == pin); + + auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); + REQUIRE(pinFromIndex.has_value()); + REQUIRE(pinFromIndex.value() == pin); + + REQUIRE(pinFromIndex->GetType() == pin.GetType()); + REQUIRE(pinFromIndex->GetPackageId() == pin.GetPackageId()); + REQUIRE(pinFromIndex->GetSourceId() == pin.GetSourceId()); + } + + { + PinningIndex index = PinningIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::ReadWrite); + index.RemovePin(pin.GetKey()); + } + + { + // 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)); + } +} + +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.*" }); + Pin updatedPin = Pin::CreatePinningPin({ "pkgId", "srcId" }); + + { + PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); + index.AddPin(pin); + REQUIRE(index.UpdatePin(updatedPin)); + } + + { + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadOnly); + auto pinFromIndex = Pinning_V1_0::PinTable::GetPinById(connection, 1); + REQUIRE(pinFromIndex.has_value()); + REQUIRE(pinFromIndex.value() == updatedPin); + } + + { + PinningIndex index = PinningIndex::Open(tempFile, SQLiteStorageBase::OpenDisposition::ReadWrite); + index.RemovePin(updatedPin.GetKey()); + } + + { + // 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)); + } +} + +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" }); + + // Add two pins to the index, then check that they show up when queried + PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); + index.AddPin(pin1); + index.AddPin(pin2); + + 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()); + + // Reset the index, then check that there are no pins + index.ResetAllPins(); + REQUIRE(index.GetAllPins().empty()); + REQUIRE(!index.GetPin(pin1.GetKey()).has_value()); + REQUIRE(!index.GetPin(pin2.GetKey()).has_value()); +} + +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.*" }); + + PinningIndex index = PinningIndex::CreateNew(tempFile, { 1, 0 }); + index.AddPin(pin); + + REQUIRE_THROWS(index.AddPin(pin), ERROR_ALREADY_EXISTS); +} \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 5a551d003b..8a5a521742 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -73,7 +73,7 @@ namespace AppInstaller::Pinning if (m_type != PinType::Gating) { - return false; + return true; } return m_gatedVersion == other.m_gatedVersion; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index c12c108a9d..8700b1bb29 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -63,7 +63,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 savepoint.Commit(); } - std::optional PinTable::SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + std::optional PinTable::GetByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { // TODO: The statement builder requires that the bound parameters can be converted to T&&, // but the package/source IDs go as const T&. Find a way to avoid the weird casting. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h index 9ba338c175..341335bb54 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -17,7 +17,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // Creates the table with named indices. static void Create(SQLite::Connection& connection); - static std::optional SelectByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); + static std::optional GetByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); static SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin); static bool UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin); static void RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); 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 446aee5588..d9459529d6 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 @@ -10,7 +10,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { std::optional GetExistingPinId(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { - auto result = PinTable::SelectByPinKey(connection, pinKey); + auto result = PinTable::GetByPinKey(connection, pinKey); if (!result) { From f5ea9ce55d623dfcb1c136537644900da05864cf Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Fri, 16 Dec 2022 15:42:42 -0800 Subject: [PATCH 06/20] Store version in all types of pins --- src/AppInstallerCLICore/Resources.h | 1 - src/AppInstallerCLICore/Workflows/PinFlow.cpp | 19 ++++++----- .../Shared/Strings/en-us/winget.resw | 5 +-- src/AppInstallerCommonCore/Pin.cpp | 33 ++++++++----------- .../Public/winget/Pin.h | 15 +++++---- .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 26 +++++++-------- 6 files changed, 45 insertions(+), 54 deletions(-) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index bd51b7427d..b782d358fd 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -98,7 +98,6 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(FlagContainAdjoinedError); WINGET_DEFINE_RESOURCE_STRINGID(ForceArgumentDescription); WINGET_DEFINE_RESOURCE_STRINGID(GatedVersionArgumentDescription); - WINGET_DEFINE_RESOURCE_STRINGID(GatedVersion); WINGET_DEFINE_RESOURCE_STRINGID(GetManifestResultVersionNotFound); WINGET_DEFINE_RESOURCE_STRINGID(HashCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(HashCommandShortDescription); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 8c6cf7b129..db7c0e95ba 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -16,19 +16,19 @@ 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, std::string_view packageId, std::string_view sourceId, const std::string& installedVersion) { - if (context.Args.Contains(Execution::Args::Type::BlockingPin)) + if (context.Args.Contains(Execution::Args::Type::GatedVersion)) { - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, context.Args.GetArg(Execution::Args::Type::GatedVersion)); } - else if (context.Args.Contains(Execution::Args::Type::GatedVersion)) + else if (context.Args.Contains(Execution::Args::Type::BlockingPin)) { - return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, context.Args.GetArg(Execution::Args::Type::GatedVersion)); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }, { installedVersion }); } else { - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }, { installedVersion }); } } } @@ -89,7 +89,8 @@ namespace AppInstaller::CLI::Workflow Pinning::PinKey pinKey{ availableVersion->GetProperty(PackageVersionProperty::Id).get(), availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; - auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId); + auto installedVersion = package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Version); + auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId, installedVersion); AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); @@ -160,8 +161,8 @@ namespace AppInstaller::CLI::Workflow { Resource::String::SearchId, Resource::String::SearchSource, + Resource::String::SearchVersion, Resource::String::PinType, - Resource::String::GatedVersion }); for (const auto& pin : context.Get()) @@ -170,8 +171,8 @@ namespace AppInstaller::CLI::Workflow table.OutputLine({ pin.GetPackageId(), std::string{ pin.GetSourceId() }, + std::string{ pin.GetVersionString() }, std::string{ ToString(pin.GetType()) }, - pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : "", }); } diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index c1d9175d04..4f46c9d2f4 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1541,9 +1541,6 @@ Please specify one of them using the --source option to proceed. User Settings - - Gated version - Pin added successfully @@ -1567,4 +1564,4 @@ Please specify one of them using the --source option to proceed. Pin type - + \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index 8a5a521742..acc3b2b2ba 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -3,6 +3,8 @@ #include "pch.h" #include "winget/Pin.h" +using namespace AppInstaller::Utility; + namespace AppInstaller::Pinning { using namespace std::string_view_literals; @@ -23,19 +25,19 @@ namespace AppInstaller::Pinning } } - Pin Pin::CreateBlockingPin(PinKey&& pinKey) + Pin Pin::CreateBlockingPin(PinKey&& pinKey, Version version) { - return { PinType::Blocking, std::move(pinKey) }; + return { PinType::Blocking, std::move(pinKey), version.ToString() }; } - Pin Pin::CreatePinningPin(PinKey&& pinKey) + Pin Pin::CreatePinningPin(PinKey&& pinKey, Version version) { - return { PinType::Pinning, std::move(pinKey) }; + return { PinType::Pinning, std::move(pinKey), version.ToString() }; } - Pin Pin::CreateGatingPin(PinKey&& pinKey, AppInstaller::Utility::GatedVersion gatedVersion) + Pin Pin::CreateGatingPin(PinKey&& pinKey, GatedVersion gatedVersion) { - return { PinType::Gating, std::move(pinKey), gatedVersion }; + return { PinType::Gating, std::move(pinKey), gatedVersion.ToString() }; } PinType Pin::GetType() const @@ -58,24 +60,15 @@ namespace AppInstaller::Pinning return m_key.SourceId; } - AppInstaller::Utility::GatedVersion Pin::GetGatedVersion() const + std::string_view Pin::GetVersionString() const { - THROW_HR_IF(E_NOT_VALID_STATE, m_type != PinType::Gating); - return m_gatedVersion.value(); + return m_versionString; } bool Pin::operator==(const Pin& other) const { - if (m_type != other.m_type || m_key != other.m_key) - { - return false; - } - - if (m_type != PinType::Gating) - { - return true; - } - - return m_gatedVersion == other.m_gatedVersion; + return m_type == other.m_type && + m_key == other.m_key && + m_versionString == other.m_versionString; } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index cee3b5e302..cbca0adc1b 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -43,8 +43,8 @@ namespace AppInstaller::Pinning struct Pin { - static Pin CreateBlockingPin(PinKey&& pinKey); - static Pin CreatePinningPin(PinKey&& pinKey); + static Pin CreateBlockingPin(PinKey&& pinKey, Utility::Version version = {}); + static Pin CreatePinningPin(PinKey&& pinKey, Utility::Version version = {}); static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion gatedVersion); PinType GetType() const; @@ -52,17 +52,18 @@ namespace AppInstaller::Pinning const Manifest::Manifest::string_t& GetPackageId() const; std::string_view GetSourceId() const; - // Only available for PinType Gating - Utility::GatedVersion GetGatedVersion() const; + // TODO: For now we'll keep the versions as simply a string in all cases. + // This will change once we actually start using them. + std::string_view GetVersionString() const; bool operator==(const Pin& other) const; private: - Pin(PinType type, PinKey&& pinKey, std::optional gatedVersion = {}) - : m_type(type), m_key(std::move(pinKey)), m_gatedVersion(gatedVersion) {} + Pin(PinType type, PinKey&& pinKey, std::string_view versionString) + : m_type(type), m_key(std::move(pinKey)), m_versionString(versionString) {} PinType m_type = PinType::Unknown; PinKey m_key; - std::optional m_gatedVersion; + std::string m_versionString; }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index 8700b1bb29..0b45fc4416 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -9,16 +9,16 @@ 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 gatedVersion) + std::optional GetPinFromRow(std::string_view packageId, std::string_view sourceId, Pinning::PinType type, const std::string& version) { switch (type) { case Pinning::PinType::Blocking: - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }, Utility::Version{ version }); case Pinning::PinType::Pinning: - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }, Utility::Version{ version }); case Pinning::PinType::Gating: - return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ gatedVersion }); + return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ version }); default: return {}; } @@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 static constexpr std::string_view s_PinTable_PackageId_Column = "packageId"sv; static constexpr std::string_view s_PinTable_SourceId_Column = "sourceId"sv; static constexpr std::string_view s_PinTable_Type_Column = "type"sv; - static constexpr std::string_view s_PinTable_GatedVersion_Column = "gatedVersion"sv; + static constexpr std::string_view s_PinTable_Version_Column = "version"sv; static constexpr std::string_view s_PinTable_Index = "pinIndex"sv; std::string_view PinTable::TableName() @@ -50,7 +50,7 @@ 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_Type_Column, Type::Int64).NotNull()); - createTableBuilder.Column(ColumnBuilder(s_PinTable_GatedVersion_Column, Type::Text)); + createTableBuilder.Column(ColumnBuilder(s_PinTable_Version_Column, Type::Text)); createTableBuilder.EndColumns(); createTableBuilder.Execute(connection); @@ -94,12 +94,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, s_PinTable_Type_Column, - s_PinTable_GatedVersion_Column }) + s_PinTable_Version_Column }) .Values( (std::string_view)pin.GetPackageId(), - (std::string_view)pin.GetSourceId(), + pin.GetSourceId(), pin.GetType(), - pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : ""); + pin.GetVersionString()); builder.Execute(connection); return connection.GetLastInsertRowID(); @@ -112,9 +112,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::Builder::StatementBuilder builder; builder.Update(s_PinTable_Table_Name).Set() .Column(s_PinTable_PackageId_Column).Equals((std::string_view)pin.GetPackageId()) - .Column(s_PinTable_SourceId_Column).Equals((std::string_view)pin.GetSourceId()) + .Column(s_PinTable_SourceId_Column).Equals(pin.GetSourceId()) .Column(s_PinTable_Type_Column).Equals(pin.GetType()) - .Column(s_PinTable_GatedVersion_Column).Equals(pin.GetType() == Pinning::PinType::Gating ? pin.GetGatedVersion().ToString() : "") + .Column(s_PinTable_Version_Column).Equals(pin.GetVersionString()) .Where(SQLite::RowIDName).Equals(pinId); builder.Execute(connection); @@ -135,7 +135,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, s_PinTable_Type_Column, - s_PinTable_GatedVersion_Column }) + s_PinTable_Version_Column }) .From(s_PinTable_Table_Name).Where(SQLite::RowIDName).Equals(pinId); SQLite::Statement select = builder.Prepare(connection); @@ -156,7 +156,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 s_PinTable_PackageId_Column, s_PinTable_SourceId_Column, s_PinTable_Type_Column, - s_PinTable_GatedVersion_Column }) + s_PinTable_Version_Column }) .From(s_PinTable_Table_Name); SQLite::Statement select = builder.Prepare(connection); From 0a489764d4030b6fe9b66c7c01c443843a9df020 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 20 Dec 2022 12:31:44 -0800 Subject: [PATCH 07/20] Tests for pin add --- .../Commands/PinCommand.cpp | 2 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 10 +- .../AppInstallerCLITests.vcxproj | 1 + .../AppInstallerCLITests.vcxproj.filters | 3 + src/AppInstallerCLITests/PinFlow.cpp | 192 ++++++++++++++++++ 5 files changed, 205 insertions(+), 3 deletions(-) create mode 100644 src/AppInstallerCLITests/PinFlow.cpp diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index 3b93c96d80..d212e910e1 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -123,6 +123,7 @@ namespace AppInstaller::CLI Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(false) << + Workflow::GetInstalledPackageVersion << Workflow::ReportPackageIdentity << Workflow::OpenPinningIndex << Workflow::SearchPin << @@ -194,6 +195,7 @@ namespace AppInstaller::CLI Workflow::SearchSourceForSingle << Workflow::HandleSearchResultFailures << Workflow::EnsureOneMatchFromSearchResult(false) << + Workflow::GetInstalledPackageVersion << Workflow::ReportPackageIdentity << Workflow::OpenPinningIndex << Workflow::SearchPin << diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index db7c0e95ba..0e2d6cbf8b 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -54,7 +54,6 @@ namespace AppInstaller::CLI::Workflow void SearchPin(Execution::Context& context) { - AICLI_LOG(CLI, Info, << "SEARCHING PIN"); auto package = context.Get(); std::vector pins; @@ -81,6 +80,12 @@ namespace AppInstaller::CLI::Workflow void AddPin(Execution::Context& context) { auto package = context.Get(); + auto installedVersion = context.Get(); + if (!installedVersion) + { + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); + } + std::vector pins; // TODO: We should support querying the multiple sources for a package, instead of just one @@ -89,8 +94,7 @@ namespace AppInstaller::CLI::Workflow Pinning::PinKey pinKey{ availableVersion->GetProperty(PackageVersionProperty::Id).get(), availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; - auto installedVersion = package->GetInstalledVersion()->GetProperty(PackageVersionProperty::Version); - auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId, installedVersion); + auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId, installedVersion->GetProperty(PackageVersionProperty::Version)); AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 39396da682..f6c35bb3a9 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -219,6 +219,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 7cb377815d..21072d3f32 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -278,6 +278,9 @@ Source Files\Repository + + Source Files\CLI + diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp new file mode 100644 index 0000000000..5a132e6611 --- /dev/null +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -0,0 +1,192 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "WorkflowCommon.h" +#include +#include +#include +#include + +using namespace TestCommon; +using namespace AppInstaller::CLI; +using namespace AppInstaller::CLI::Workflow; +using namespace AppInstaller::Repository::Microsoft; +using namespace AppInstaller::Pinning; + +void OverrideForOpenPinningIndex(TestContext& context, const std::filesystem::path& indexPath) +{ + context.Override({ OpenPinningIndex, [=](TestContext& context) + { + auto pinningIndex = std::filesystem::exists(indexPath) ? + PinningIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : + PinningIndex::CreateNew(indexPath.u8string()); + context.Add(std::make_shared(std::move(pinningIndex))); + } }); +} + +TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") +{ + TempFile indexFile("pinningIndex", ".db"); + + std::ostringstream pinAddOutput; + TestContext addContext{ pinAddOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(addContext); + addContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + + PinAddCommand pinAdd({}); + pinAdd.Execute(addContext); + INFO(pinAddOutput.str()); + + SECTION("Pin is saved") + { + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.size() == 1); + REQUIRE(pins[0].GetType() == PinType::Pinning); + REQUIRE(pins[0].GetPackageId() == "AppInstallerCliTest.TestExeInstaller"); + REQUIRE(pins[0].GetSourceId() == "*TestSource"); + REQUIRE(pins[0].GetVersionString() == "1.0.0.0"); + } + SECTION("Remove pin") + { + std::ostringstream pinRemoveOutput; + TestContext removeContext{ pinRemoveOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForOpenPinningIndex(removeContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(removeContext); + removeContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + + PinRemoveCommand pinRemove({}); + pinRemove.Execute(removeContext); + INFO(pinRemoveOutput.str()); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.empty()); + } + SECTION("Reset pins") + { + std::ostringstream pinResetOutput; + TestContext resetContext{ pinResetOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForOpenPinningIndex(resetContext, indexFile.GetPath()); + + SECTION("Without --force") + { + PinResetCommand pinReset({}); + pinReset.Execute(resetContext); + INFO(pinResetOutput.str()); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.size() == 1); + } + SECTION("With --force") + { + resetContext.Args.AddArg(Execution::Args::Type::Force); + + PinResetCommand pinReset({}); + pinReset.Execute(resetContext); + INFO(pinResetOutput.str()); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.empty()); + } + } + SECTION("Update pin") + { + std::ostringstream pinUpdateOutput; + TestContext updateContext{ pinUpdateOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForOpenPinningIndex(updateContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(updateContext); + updateContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + updateContext.Args.AddArg(Execution::Args::Type::BlockingPin); + + SECTION("Without --force") + { + PinAddCommand pinUpdate({}); + pinUpdate.Execute(updateContext); + INFO(pinUpdateOutput.str()); + REQUIRE_TERMINATED_WITH(updateContext, APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.size() == 1); + REQUIRE(pins[0].GetType() == PinType::Pinning); + } + SECTION("With --force") + { + updateContext.Args.AddArg(Execution::Args::Type::Force); + + PinAddCommand pinUpdate({}); + pinUpdate.Execute(updateContext); + INFO(pinUpdateOutput.str()); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.size() == 1); + REQUIRE(pins[0].GetType() == PinType::Blocking); + } + } +} + +TEST_CASE("PinFlow_Add_NotFound", "[PinFlow][workflow]") +{ + std::ostringstream pinAddOutput; + TestContext addContext{ pinAddOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForCompositeInstalledSource(addContext); + addContext.Args.AddArg(Execution::Args::Type::Query, "This package doesn't exist"sv); + + PinAddCommand pinAdd({}); + pinAdd.Execute(addContext); + INFO(pinAddOutput.str()); + + REQUIRE_TERMINATED_WITH(addContext, APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); +} + +TEST_CASE("PinFlow_Add_NotInstalled", "[PinFlow][workflow]") +{ + TempFile indexFile("pinningIndex", ".db"); + + std::ostringstream pinAddOutput; + TestContext addContext{ pinAddOutput, std::cin }; + // auto previousThreadGlobals = context.SetForCurrentThread(); + OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(addContext); + addContext.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithNothingInstalled"sv); + + PinAddCommand pinAdd({}); + pinAdd.Execute(addContext); + INFO(pinAddOutput.str()); + + REQUIRE_TERMINATED_WITH(addContext, APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); + + auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); + auto pins = index.GetAllPins(); + REQUIRE(pins.empty()); +} + +TEST_CASE("PinFlow_ListEmpty", "[PinFlow][workflow]") +{ +} + +TEST_CASE("PinFlow_ListMultiple", "[PinFlow][workflow]") +{ +} + +TEST_CASE("PinFlow_ListUninstalled", "[PinFlow][workflow]") +{ +} + +TEST_CASE("PinFlow_RemoveNonExisting", "[PinFlow][workflow]") +{ +} + +TEST_CASE("PinFlow_ResetEmpty", "[PinFlow][workflow]") +{ +} \ No newline at end of file From 2e8a05744be4fbd15dcae5d4a3f611ca6dd23ef2 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 20 Dec 2022 14:14:04 -0800 Subject: [PATCH 08/20] Add more tests --- .../Commands/PinCommand.cpp | 4 ++ src/AppInstallerCLICore/Resources.h | 2 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 29 ++++++-- src/AppInstallerCLICore/Workflows/PinFlow.h | 5 +- .../Shared/Strings/en-us/winget.resw | 10 ++- src/AppInstallerCLITests/PinFlow.cpp | 70 ++++++++++++++----- src/AppInstallerCommonCore/Errors.cpp | 2 + .../Public/AppInstallerErrors.h | 1 + 8 files changed, 99 insertions(+), 24 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index d212e910e1..a7b07eb3e9 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -297,6 +297,10 @@ namespace AppInstaller::CLI { context << Workflow::OpenPinningIndex << + Workflow::GetAllPins << + Workflow::OpenSource() << + Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << + Workflow::CrossReferencePinsWithSource << Workflow::ResetAllPins; } } diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index d2d10c2f20..e3f22e1b89 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -248,10 +248,12 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinAlreadyExists); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinDoesNotExist); WINGET_DEFINE_RESOURCE_STRINGID(PinExistsOverwriting); WINGET_DEFINE_RESOURCE_STRINGID(PinExistsUseForceArg); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinListCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinNoPinsExist); WINGET_DEFINE_RESOURCE_STRINGID(PinRemoveCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinRemoveCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinResetCommandLongDescription); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 0e2d6cbf8b..a71b12d9a1 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -146,9 +146,12 @@ namespace AppInstaller::CLI::Workflow Pinning::PinKey pinKey{ availableVersion->GetProperty(PackageVersionProperty::Id).get(), availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; + AICLI_LOG(CLI, Info, << "Removing pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); if (!pinningIndex->GetPin(pinKey)) { - // TODO: Report pin not found + AICLI_LOG(CLI, Warning, << "Pin does not exist"); + context.Reporter.Warn() << Resource::String::PinDoesNotExist(pinKey.PackageId) << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST); } pinningIndex->RemovePin(pinKey); @@ -160,6 +163,13 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void ReportPins(Execution::Context& context) { + const auto& pins = context.Get(); + if (pins.empty()) + { + context.Reporter.Info() << Resource::String::PinNoPinsExist << std::endl; + return; + } + // TODO: Use package and source names Execution::TableOutput<4> table(context.Reporter, { @@ -169,7 +179,7 @@ namespace AppInstaller::CLI::Workflow Resource::String::PinType, }); - for (const auto& pin : context.Get()) + for (const auto& pin : pins) { // TODO: Avoid these conversions to string table.OutputLine({ @@ -193,15 +203,22 @@ namespace AppInstaller::CLI::Workflow if (context.Args.Contains(Execution::Args::Type::Force)) { context.Reporter.Info() << Resource::String::PinResettingAll << std::endl; - auto pinningIndex = context.Get(); - pinningIndex->ResetAllPins(); + + if (context.Get().empty()) + { + context.Reporter.Info() << Resource::String::PinNoPinsExist << std::endl; + } + else + { + auto pinningIndex = context.Get(); + pinningIndex->ResetAllPins(); + } } else { AICLI_LOG(CLI, Info, << "--force argument is not present"); context.Reporter.Info() << Resource::String::PinResetUseForceArg << std::endl; - - // TODO: Report pins here + context << ReportPins; } } diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index bc03696ad8..35862c4940 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -32,6 +32,9 @@ namespace AppInstaller::CLI::Workflow void AddPin(Execution::Context& context); // Removes all the pins associated with a package. + // Required Args: None + // Inputs: PinningIndex, Package, InstalledPackageVersion + // Outputs: None void RemovePin(Execution::Context& context); // Report the pins in a table. @@ -42,7 +45,7 @@ namespace AppInstaller::CLI::Workflow // Resets all the existing pins. // Required Args: None - // Inputs: PinningIndex + // Inputs: PinningIndex, Pins // Outputs: None void ResetAllPins(Execution::Context& context); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 8ebed6b48e..489ee62e83 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1623,9 +1623,17 @@ Please specify one of them using the --source option to proceed. Resetting all current pins. - Use the --force argument to reset all pins. + Use the --force argument to reset all pins. The following pins would be removed: Pin type + + There is no pin for package {0} + {Locked="{0}"} {0} is a placeholder that will be replaced by a package name. The message is shown when attempting to delete a pin for a package that is not pinned. + + + There are no pins configured. + Shown when listing or modifying existing pins if there are none. + \ No newline at end of file diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index 5a132e6611..a2b527eaa7 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -30,10 +30,10 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); OverrideForCompositeInstalledSource(addContext); addContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + addContext.Args.AddArg(Execution::Args::Type::BlockingPin); PinAddCommand pinAdd({}); pinAdd.Execute(addContext); @@ -44,16 +44,28 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); auto pins = index.GetAllPins(); REQUIRE(pins.size() == 1); - REQUIRE(pins[0].GetType() == PinType::Pinning); + REQUIRE(pins[0].GetType() == PinType::Blocking); REQUIRE(pins[0].GetPackageId() == "AppInstallerCliTest.TestExeInstaller"); REQUIRE(pins[0].GetSourceId() == "*TestSource"); REQUIRE(pins[0].GetVersionString() == "1.0.0.0"); + + std::ostringstream pinListOutput; + TestContext listContext{ pinListOutput, std::cin }; + OverrideForOpenPinningIndex(listContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(listContext); + listContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + + PinListCommand pinList({}); + pinList.Execute(listContext); + + INFO(pinListOutput.str()); + REQUIRE(pinListOutput.str().find("AppInstallerCliTest.TestExeInstaller")); + REQUIRE(pinListOutput.str().find("Blocking")); } SECTION("Remove pin") { std::ostringstream pinRemoveOutput; TestContext removeContext{ pinRemoveOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenPinningIndex(removeContext, indexFile.GetPath()); OverrideForCompositeInstalledSource(removeContext); removeContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); @@ -70,8 +82,8 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") { std::ostringstream pinResetOutput; TestContext resetContext{ pinResetOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenPinningIndex(resetContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(resetContext); SECTION("Without --force") { @@ -100,11 +112,9 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") { std::ostringstream pinUpdateOutput; TestContext updateContext{ pinUpdateOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenPinningIndex(updateContext, indexFile.GetPath()); OverrideForCompositeInstalledSource(updateContext); updateContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); - updateContext.Args.AddArg(Execution::Args::Type::BlockingPin); SECTION("Without --force") { @@ -116,7 +126,7 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); auto pins = index.GetAllPins(); REQUIRE(pins.size() == 1); - REQUIRE(pins[0].GetType() == PinType::Pinning); + REQUIRE(pins[0].GetType() == PinType::Blocking); } SECTION("With --force") { @@ -129,7 +139,7 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); auto pins = index.GetAllPins(); REQUIRE(pins.size() == 1); - REQUIRE(pins[0].GetType() == PinType::Blocking); + REQUIRE(pins[0].GetType() == PinType::Pinning); } } } @@ -138,7 +148,6 @@ TEST_CASE("PinFlow_Add_NotFound", "[PinFlow][workflow]") { std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForCompositeInstalledSource(addContext); addContext.Args.AddArg(Execution::Args::Type::Query, "This package doesn't exist"sv); @@ -155,7 +164,6 @@ TEST_CASE("PinFlow_Add_NotInstalled", "[PinFlow][workflow]") std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; - // auto previousThreadGlobals = context.SetForCurrentThread(); OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); OverrideForCompositeInstalledSource(addContext); addContext.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithNothingInstalled"sv); @@ -173,20 +181,50 @@ TEST_CASE("PinFlow_Add_NotInstalled", "[PinFlow][workflow]") TEST_CASE("PinFlow_ListEmpty", "[PinFlow][workflow]") { -} + TempFile indexFile("pinningIndex", ".db"); -TEST_CASE("PinFlow_ListMultiple", "[PinFlow][workflow]") -{ -} + std::ostringstream pinListOutput; + TestContext listContext{ pinListOutput, std::cin }; + OverrideForOpenPinningIndex(listContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(listContext); -TEST_CASE("PinFlow_ListUninstalled", "[PinFlow][workflow]") -{ + PinListCommand pinList({}); + pinList.Execute(listContext); + INFO(pinListOutput.str()); + + REQUIRE(pinListOutput.str().find(Resource::LocString(Resource::String::PinNoPinsExist)) != std::string::npos); } TEST_CASE("PinFlow_RemoveNonExisting", "[PinFlow][workflow]") { + TempFile indexFile("pinningIndex", ".db"); + + std::ostringstream pinRemoveOutput; + TestContext removeContext{ pinRemoveOutput, std::cin }; + OverrideForOpenPinningIndex(removeContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(removeContext); + removeContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + + PinRemoveCommand pinRemove({}); + pinRemove.Execute(removeContext); + INFO(pinRemoveOutput.str()); + + REQUIRE_TERMINATED_WITH(removeContext, APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST); } TEST_CASE("PinFlow_ResetEmpty", "[PinFlow][workflow]") { + TempFile indexFile("pinningIndex", ".db"); + + std::ostringstream pinResetOutput; + TestContext resetContext{ pinResetOutput, std::cin }; + OverrideForOpenPinningIndex(resetContext, indexFile.GetPath()); + OverrideForCompositeInstalledSource(resetContext); + resetContext.Args.AddArg(Execution::Args::Type::Force); + + PinResetCommand pinReset({}); + pinReset.Execute(resetContext); + INFO(pinResetOutput.str()); + + REQUIRE(pinResetOutput.str().find(Resource::LocString(Resource::String::PinNoPinsExist)) != std::string::npos); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp index dbc4738e36..c27694fdd7 100644 --- a/src/AppInstallerCommonCore/Errors.cpp +++ b/src/AppInstallerCommonCore/Errors.cpp @@ -210,6 +210,8 @@ namespace AppInstaller return "Found at least one version of the package installed."; case APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS: return "A pin already exists for the package."; + case APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST: + return "There is no pin for the package."; // Install errors case APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE: diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index 7aee237d29..0ec281229d 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -111,6 +111,7 @@ #define APPINSTALLER_CLI_ERROR_ARCHIVE_SCAN_FAILED ((HRESULT)0x8A150060) #define APPINSTALLER_CLI_ERROR_PACKAGE_ALREADY_INSTALLED ((HRESULT)0x8A150061) #define APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS ((HRESULT)0x8A150062) +#define APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST ((HRESULT)0x8A150063) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) From 97b6fea5cc271b022394ffcf5223695816bb8473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flor=20Chac=C3=B3n?= <14323496+florelis@users.noreply.github.com> Date: Mon, 9 Jan 2023 14:46:55 -0800 Subject: [PATCH 09/20] Apply suggestions from code review Co-authored-by: yao-msft <50888816+yao-msft@users.noreply.github.com> --- .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 6 ++++-- .../Microsoft/Schema/Pinning_1_0/PinTable.h | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index 0b45fc4416..01479f0bf3 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -9,7 +9,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { namespace { - std::optional GetPinFromRow(std::string_view packageId, std::string_view sourceId, Pinning::PinType type, const std::string& version) + std::optional GetPinFromRow(std::string_view packageId, std::string_view sourceId, Pinning::PinType type, std::string_view version) + { switch (type) { @@ -42,7 +43,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { using namespace SQLite::Builder; - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createPinTable_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpintable_v1_0"); + StatementBuilder createTableBuilder; createTableBuilder.CreateTable(s_PinTable_Table_Name).BeginColumns(); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h index 341335bb54..007c69d5f9 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -17,7 +17,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // Creates the table with named indices. static void Create(SQLite::Connection& connection); - static std::optional GetByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); + static std::optional GetIdByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); + static SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin); static bool UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin); static void RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); From 177ee73a437ff3976b520c5050c78bfe44f6519a Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 16:19:05 -0800 Subject: [PATCH 10/20] PR comments --- .github/actions/spelling/allow.txt | 4 ++ .../Commands/PinCommand.cpp | 28 ++++++--- src/AppInstallerCLICore/Resources.h | 1 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 63 +++++++------------ src/AppInstallerCLICore/Workflows/PinFlow.h | 2 +- .../Shared/Strings/en-us/winget.resw | 5 ++ src/AppInstallerCLITests/PinFlow.cpp | 26 +------- src/AppInstallerCommonCore/Pin.cpp | 18 +++--- .../Public/AppInstallerRuntime.h | 5 +- .../Public/winget/Pin.h | 17 +++-- .../Microsoft/PinningIndex.cpp | 10 +-- .../Microsoft/PinningIndex.h | 5 +- .../Microsoft/Schema/IPinningIndex.h | 5 +- .../Microsoft/Schema/Pinning_1_0/PinTable.cpp | 38 +++++------ .../Microsoft/Schema/Pinning_1_0/PinTable.h | 20 +++++- .../Pinning_1_0/PinningIndexInterface.h | 2 +- .../Pinning_1_0/PinningIndexInterface_1_0.cpp | 20 +++--- 17 files changed, 133 insertions(+), 136 deletions(-) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 27e25d1bcc..1c87c3ff13 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -4,6 +4,7 @@ ACTIONDATA ACTIONSTART addfile addmanifest +addpin addportablefile addstore admins @@ -94,6 +95,7 @@ cpprestsdk cppwinrt CPRWL createnew +createpintable createportabletable createtables cref @@ -442,6 +444,7 @@ regex regexp removefile removemanifest +removepin removeportablefile repolibtest requeue @@ -624,6 +627,7 @@ Unregisters untimes updatefile updatemanifest +updatepin updateportablefile UPLEVEL upvote diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index a7b07eb3e9..1e2c5d0d71 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -116,7 +116,6 @@ namespace AppInstaller::CLI void PinAddCommand::ExecuteInternal(Execution::Context& context) const { - // TODO context << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << @@ -261,7 +260,6 @@ namespace AppInstaller::CLI void PinListCommand::ExecuteInternal(Execution::Context& context) const { - // TODO context << Workflow::OpenPinningIndex << Workflow::GetAllPins << @@ -275,6 +273,7 @@ namespace AppInstaller::CLI { return { Argument::ForType(Args::Type::Force), + Argument::ForType(Args::Type::Source), }; } @@ -295,12 +294,23 @@ namespace AppInstaller::CLI void PinResetCommand::ExecuteInternal(Execution::Context& context) const { - context << - Workflow::OpenPinningIndex << - Workflow::GetAllPins << - Workflow::OpenSource() << - Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << - Workflow::CrossReferencePinsWithSource << - Workflow::ResetAllPins; + context << Workflow::OpenPinningIndex; + + if (context.Args.Contains(Execution::Args::Type::Force)) + { + context << Workflow::ResetAllPins; + } + else + { + AICLI_LOG(CLI, Info, << "--force argument is not present"); + context.Reporter.Info() << Resource::String::PinResetUseForceArg << std::endl; + + context << + 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 e3f22e1b89..998074c1bc 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -258,6 +258,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinRemoveCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinResetCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinResetCommandShortDescription); + WINGET_DEFINE_RESOURCE_STRINGID(PinResetSuccessful); WINGET_DEFINE_RESOURCE_STRINGID(PinResettingAll); WINGET_DEFINE_RESOURCE_STRINGID(PinResetUseForceArg); WINGET_DEFINE_RESOURCE_STRINGID(PinType); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index a71b12d9a1..e2b626818c 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -16,7 +16,7 @@ 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, const std::string& installedVersion) + Pinning::Pin CreatePin(Execution::Context& context, std::string_view packageId, std::string_view sourceId) { if (context.Args.Contains(Execution::Args::Type::GatedVersion)) { @@ -24,11 +24,11 @@ namespace AppInstaller::CLI::Workflow } else if (context.Args.Contains(Execution::Args::Type::BlockingPin)) { - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }, { installedVersion }); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); } else { - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }, { installedVersion }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); } } } @@ -72,19 +72,10 @@ namespace AppInstaller::CLI::Workflow context.Add(std::move(pins)); } - // Adds a pin for the current package. - // A separate pin will be added for each source. - // Required Args: None - // Inputs: PinningIndex, Package - // Outputs: None void AddPin(Execution::Context& context) { auto package = context.Get(); auto installedVersion = context.Get(); - if (!installedVersion) - { - AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); - } std::vector pins; @@ -94,7 +85,7 @@ namespace AppInstaller::CLI::Workflow Pinning::PinKey pinKey{ availableVersion->GetProperty(PackageVersionProperty::Id).get(), availableVersion->GetProperty(PackageVersionProperty::SourceIdentifier).get() }; - auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId, installedVersion->GetProperty(PackageVersionProperty::Version)); + auto pin = CreatePin(context, pinKey.PackageId, pinKey.SourceId); AICLI_LOG(CLI, Info, << "Adding pin with type " << ToString(pin.GetType()) << " for package [" << pin.GetPackageId() << "] from source [" << pin.GetSourceId() << "]"); @@ -133,7 +124,6 @@ namespace AppInstaller::CLI::Workflow } } - // Removes all the pins associated with a package. void RemovePin(Execution::Context& context) { auto package = context.Get(); @@ -157,10 +147,6 @@ namespace AppInstaller::CLI::Workflow pinningIndex->RemovePin(pinKey); } - // Report the pins in a table. - // Required Args: None - // Inputs: Pins - // Outputs: None void ReportPins(Execution::Context& context) { const auto& pins = context.Get(); @@ -185,7 +171,7 @@ namespace AppInstaller::CLI::Workflow table.OutputLine({ pin.GetPackageId(), std::string{ pin.GetSourceId() }, - std::string{ pin.GetVersionString() }, + pin.GetGatedVersion().ToString(), std::string{ ToString(pin.GetType()) }, }); } @@ -193,39 +179,36 @@ namespace AppInstaller::CLI::Workflow table.Complete(); } - // Resets all the existing pins. - // Required Args: None - // Inputs: PinningIndex - // Outputs: None void ResetAllPins(Execution::Context& context) { AICLI_LOG(CLI, Info, << "Resetting all pins"); - if (context.Args.Contains(Execution::Args::Type::Force)) - { - context.Reporter.Info() << Resource::String::PinResettingAll << std::endl; + context.Reporter.Info() << Resource::String::PinResettingAll << std::endl; - if (context.Get().empty()) - { - context.Reporter.Info() << Resource::String::PinNoPinsExist << std::endl; - } - else + std::string sourceId; + if (context.Args.Contains(Execution::Args::Type::Source)) + { + auto sourceName = context.Args.GetArg(Execution::Args::Type::Source); + auto sources = Source::GetCurrentSources(); + for (const auto& source : sources) { - auto pinningIndex = context.Get(); - pinningIndex->ResetAllPins(); + if (Utility::CaseInsensitiveEquals(source.Name, sourceName)) + { + sourceId = source.Identifier; + break; + } } } + + if (context.Get()->ResetAllPins(sourceId)) + { + context.Reporter.Info() << Resource::String::PinResetSuccessful << std::endl; + } else { - AICLI_LOG(CLI, Info, << "--force argument is not present"); - context.Reporter.Info() << Resource::String::PinResetUseForceArg << std::endl; - context << ReportPins; + context.Reporter.Info() << Resource::String::PinNoPinsExist << std::endl; } } - // 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) { const auto& pins = context.Get(); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index 35862c4940..b367853d81 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -45,7 +45,7 @@ namespace AppInstaller::CLI::Workflow // Resets all the existing pins. // Required Args: None - // Inputs: PinningIndex, Pins + // Inputs: None // Outputs: None void ResetAllPins(Execution::Context& context); diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index 489ee62e83..b25e360488 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1624,6 +1624,7 @@ Please specify one of them using the --source option to proceed. Use the --force argument to reset all pins. The following pins would be removed: + {Locked="--force"} Pin type @@ -1636,4 +1637,8 @@ Please specify one of them using the --source option to proceed. There are no pins configured. Shown when listing or modifying existing pins if there are none. + + Pins resetted successfully + Shown after resetting (deleting) all the pins + \ No newline at end of file diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index a2b527eaa7..c1dd94565b 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -47,7 +47,7 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") REQUIRE(pins[0].GetType() == PinType::Blocking); REQUIRE(pins[0].GetPackageId() == "AppInstallerCliTest.TestExeInstaller"); REQUIRE(pins[0].GetSourceId() == "*TestSource"); - REQUIRE(pins[0].GetVersionString() == "1.0.0.0"); + REQUIRE(pins[0].GetGatedVersion().ToString() == ""); std::ostringstream pinListOutput; TestContext listContext{ pinListOutput, std::cin }; @@ -83,10 +83,10 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinResetOutput; TestContext resetContext{ pinResetOutput, std::cin }; OverrideForOpenPinningIndex(resetContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(resetContext); SECTION("Without --force") { + OverrideForCompositeInstalledSource(resetContext); PinResetCommand pinReset({}); pinReset.Execute(resetContext); INFO(pinResetOutput.str()); @@ -158,27 +158,6 @@ TEST_CASE("PinFlow_Add_NotFound", "[PinFlow][workflow]") REQUIRE_TERMINATED_WITH(addContext, APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); } -TEST_CASE("PinFlow_Add_NotInstalled", "[PinFlow][workflow]") -{ - TempFile indexFile("pinningIndex", ".db"); - - std::ostringstream pinAddOutput; - TestContext addContext{ pinAddOutput, std::cin }; - OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(addContext); - addContext.Args.AddArg(Execution::Args::Type::Query, "TestExeInstallerWithNothingInstalled"sv); - - PinAddCommand pinAdd({}); - pinAdd.Execute(addContext); - INFO(pinAddOutput.str()); - - REQUIRE_TERMINATED_WITH(addContext, APPINSTALLER_CLI_ERROR_NO_APPLICATIONS_FOUND); - - auto index = PinningIndex::Open(indexFile.GetPath().u8string(), SQLiteStorageBase::OpenDisposition::Read); - auto pins = index.GetAllPins(); - REQUIRE(pins.empty()); -} - TEST_CASE("PinFlow_ListEmpty", "[PinFlow][workflow]") { TempFile indexFile("pinningIndex", ".db"); @@ -219,7 +198,6 @@ TEST_CASE("PinFlow_ResetEmpty", "[PinFlow][workflow]") std::ostringstream pinResetOutput; TestContext resetContext{ pinResetOutput, std::cin }; OverrideForOpenPinningIndex(resetContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(resetContext); resetContext.Args.AddArg(Execution::Args::Type::Force); PinResetCommand pinReset({}); diff --git a/src/AppInstallerCommonCore/Pin.cpp b/src/AppInstallerCommonCore/Pin.cpp index acc3b2b2ba..d76d0f4ea4 100644 --- a/src/AppInstallerCommonCore/Pin.cpp +++ b/src/AppInstallerCommonCore/Pin.cpp @@ -25,19 +25,19 @@ namespace AppInstaller::Pinning } } - Pin Pin::CreateBlockingPin(PinKey&& pinKey, Version version) + Pin Pin::CreateBlockingPin(PinKey&& pinKey) { - return { PinType::Blocking, std::move(pinKey), version.ToString() }; + return { PinType::Blocking, std::move(pinKey) }; } - Pin Pin::CreatePinningPin(PinKey&& pinKey, Version version) + Pin Pin::CreatePinningPin(PinKey&& pinKey) { - return { PinType::Pinning, std::move(pinKey), version.ToString() }; + return { PinType::Pinning, std::move(pinKey) }; } - Pin Pin::CreateGatingPin(PinKey&& pinKey, GatedVersion gatedVersion) + Pin Pin::CreateGatingPin(PinKey&& pinKey, GatedVersion&& gatedVersion) { - return { PinType::Gating, std::move(pinKey), gatedVersion.ToString() }; + return { PinType::Gating, std::move(pinKey), std::move(gatedVersion) }; } PinType Pin::GetType() const @@ -60,15 +60,15 @@ namespace AppInstaller::Pinning return m_key.SourceId; } - std::string_view Pin::GetVersionString() const + Utility::GatedVersion Pin::GetGatedVersion() const { - return m_versionString; + return m_gatedVersion; } bool Pin::operator==(const Pin& other) const { return m_type == other.m_type && m_key == other.m_key && - m_versionString == other.m_versionString; + m_gatedVersion == other.m_gatedVersion; } } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index 3fbc0cc742..5ab209811c 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -62,10 +62,7 @@ namespace AppInstaller::Runtime PortableLinksUserLocation, // The location where symlinks to portable packages are stored under machine scope. PortableLinksMachineLocation, - // The location of the user settings json file. - UserSettingsFileLocation, - // The location of the user settings json file, anonymized using environment variables. - UserSettingsFileLocationForDisplay, + // The location where the database for pins is stored. PinningIndex, }; diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index cbca0adc1b..fec59a9174 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -43,27 +43,24 @@ namespace AppInstaller::Pinning struct Pin { - static Pin CreateBlockingPin(PinKey&& pinKey, Utility::Version version = {}); - static Pin CreatePinningPin(PinKey&& pinKey, Utility::Version version = {}); - static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion gatedVersion); + static Pin CreateBlockingPin(PinKey&& pinKey); + static Pin CreatePinningPin(PinKey&& pinKey); + static Pin CreateGatingPin(PinKey&& pinKey, Utility::GatedVersion&& gatedVersion); PinType GetType() const; const PinKey& GetKey() const; const Manifest::Manifest::string_t& GetPackageId() const; std::string_view GetSourceId() const; - - // TODO: For now we'll keep the versions as simply a string in all cases. - // This will change once we actually start using them. - std::string_view GetVersionString() const; + Utility::GatedVersion GetGatedVersion() const; bool operator==(const Pin& other) const; private: - Pin(PinType type, PinKey&& pinKey, std::string_view versionString) - : m_type(type), m_key(std::move(pinKey)), m_versionString(versionString) {} + Pin(PinType type, PinKey&& pinKey, Utility::GatedVersion&& gatedVersion = {}) + : m_type(type), m_key(std::move(pinKey)), m_gatedVersion(std::move(gatedVersion)) {} PinType m_type = PinType::Unknown; PinKey m_key; - std::string m_versionString; + Utility::GatedVersion m_gatedVersion; }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index b27104dd6d..40c1a5098d 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -12,7 +12,7 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Info, << "Creating new Pinning Index [" << version << "] at '" << filePath << "'"); PinningIndex result{ filePath, version }; - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "pinningIndex_createNew"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(result.m_dbconn, "pinningindex_createnew"); // Use calculated version, as incoming version could be 'latest' result.m_version.SetSchemaVersion(result.m_dbconn); @@ -31,7 +31,7 @@ namespace AppInstaller::Repository::Microsoft 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())); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_addPin"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_addpin"); IdType result = m_interface->AddPin(m_dbconn, pin); @@ -47,7 +47,7 @@ namespace AppInstaller::Repository::Microsoft 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())); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_updatePin"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningindex_updatepin"); bool result = m_interface->UpdatePin(m_dbconn, pin).first; @@ -86,10 +86,10 @@ namespace AppInstaller::Repository::Microsoft return m_interface->GetAllPins(m_dbconn); } - void PinningIndex::ResetAllPins() + bool PinningIndex::ResetAllPins(std::string_view sourceId) { std::lock_guard lockInterface{ *m_interfaceLock }; - m_interface->ResetAllPins(m_dbconn); + return m_interface->ResetAllPins(m_dbconn, sourceId); } std::unique_ptr PinningIndex::CreateIPinningIndex() const diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h index e35ff91366..b449864aba 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -1,3 +1,5 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. #pragma once #include "SQLiteWrapper.h" #include "Microsoft/Schema/IPinningIndex.h" @@ -42,7 +44,8 @@ namespace AppInstaller::Repository::Microsoft // Returns a vector containing all the existing pins. std::vector GetAllPins(); - void ResetAllPins(); + // Deletes all pins from a given source, or from all sources if none is specified + bool ResetAllPins(std::string_view sourceId = {}); private: // Constructor used to open an existing index. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h index 732db3ba7e..c4c96a73aa 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/IPinningIndex.h @@ -34,7 +34,8 @@ namespace AppInstaller::Repository::Microsoft::Schema // Returns a vector containing all the existing pins. virtual std::vector GetAllPins(SQLite::Connection& connection) = 0; - // Removes all the pins from the index - virtual void ResetAllPins(SQLite::Connection& connection) = 0; + // Removes all the pins from a given source, or from all sources if none is specified. + // Returns a value indicating whether any pin was deleted. + virtual bool ResetAllPins(SQLite::Connection& connection, std::string_view sourceId) = 0; }; } \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp index 01479f0bf3..5425556d5e 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.cpp @@ -15,9 +15,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 switch (type) { case Pinning::PinType::Blocking: - return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }, Utility::Version{ version }); + return Pinning::Pin::CreateBlockingPin({ packageId, sourceId }); case Pinning::PinType::Pinning: - return Pinning::Pin::CreatePinningPin({ packageId, sourceId }, Utility::Version{ version }); + return Pinning::Pin::CreatePinningPin({ packageId, sourceId }); case Pinning::PinType::Gating: return Pinning::Pin::CreateGatingPin({ packageId, sourceId }, Utility::GatedVersion{ version }); default: @@ -28,11 +28,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 using namespace std::string_view_literals; static constexpr std::string_view s_PinTable_Table_Name = "pin"sv; - static constexpr std::string_view s_PinTable_PackageId_Column = "packageId"sv; - static constexpr std::string_view s_PinTable_SourceId_Column = "sourceId"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_Type_Column = "type"sv; static constexpr std::string_view s_PinTable_Version_Column = "version"sv; - static constexpr std::string_view s_PinTable_Index = "pinIndex"sv; + static constexpr std::string_view s_PinTable_Index = "pin_index"sv; std::string_view PinTable::TableName() { @@ -45,7 +45,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpintable_v1_0"); - StatementBuilder createTableBuilder; createTableBuilder.CreateTable(s_PinTable_Table_Name).BeginColumns(); @@ -65,10 +64,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 savepoint.Commit(); } - std::optional PinTable::GetByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) + std::optional PinTable::GetIdByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { - // TODO: The statement builder requires that the bound parameters can be converted to T&&, - // but the package/source IDs go as const T&. Find a way to avoid the weird casting. SQLite::Builder::StatementBuilder builder; builder.Select(SQLite::RowIDName).From(s_PinTable_Table_Name) .Where(s_PinTable_PackageId_Column).Equals((std::string_view)pinKey.PackageId) @@ -88,8 +85,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::rowid_t PinTable::AddPin(SQLite::Connection& connection, const Pinning::Pin& pin) { - // TODO: The statement builder requires that the bound parameters can be converted to T&&, - // but the package/source IDs go as const T&. Find a way to avoid the weird casting. SQLite::Builder::StatementBuilder builder; builder.InsertInto(s_PinTable_Table_Name) .Columns({ @@ -101,22 +96,20 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 (std::string_view)pin.GetPackageId(), pin.GetSourceId(), pin.GetType(), - pin.GetVersionString()); + pin.GetGatedVersion().ToString()); builder.Execute(connection); return connection.GetLastInsertRowID(); } - bool PinTable::UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin) + bool PinTable::UpdatePinById(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin) { - // TODO: The statement builder requires that the bound parameters can be converted to T&&, - // but the package/source IDs go as const T&. Find a way to avoid the weird casting. SQLite::Builder::StatementBuilder builder; 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_Type_Column).Equals(pin.GetType()) - .Column(s_PinTable_Version_Column).Equals(pin.GetVersionString()) + .Column(s_PinTable_Version_Column).Equals(pin.GetGatedVersion().ToString()) .Where(SQLite::RowIDName).Equals(pinId); builder.Execute(connection); @@ -168,7 +161,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { auto [packageId, sourceId, type, gatedVersion] = select.GetRow(); auto pin = GetPinFromRow(packageId, sourceId, type, gatedVersion); - if (pin) { + if (pin) + { pins.push_back(std::move(pin.value())); } } @@ -176,10 +170,18 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 return pins; } - void PinTable::ResetAllPins(SQLite::Connection& connection) + bool PinTable::ResetAllPins(SQLite::Connection& connection, std::string_view sourceId) { SQLite::Builder::StatementBuilder builder; builder.DeleteFrom(s_PinTable_Table_Name); + + if (!sourceId.empty()) + { + builder.Where(s_PinTable_SourceId_Column).Equals(sourceId); + } + builder.Execute(connection); + + return connection.GetChanges() != 0; } } \ No newline at end of file diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h index 007c69d5f9..357ca2319b 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinTable.h @@ -8,7 +8,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { - // A table struct PinTable { // Get the table name. @@ -17,13 +16,28 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // Creates the table with named indices. static void Create(SQLite::Connection& connection); + // Gets the row ID for the pin, if it exists. static std::optional GetIdByPinKey(SQLite::Connection& connection, const Pinning::PinKey& pinKey); + // Adds a new pin. Returns the row ID of the added pin. static SQLite::rowid_t AddPin(SQLite::Connection& connection, const Pinning::Pin& pin); - static bool UpdatePin(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin); + + // Updates an existing pin. + // Returns a value indicating whether there were any changes. + static bool UpdatePinById(SQLite::Connection& connection, SQLite::rowid_t pinId, const Pinning::Pin& pin); + + // Removes a pin given its row ID. static void RemovePinById(SQLite::Connection& connection, SQLite::rowid_t pinId); + + // Gets a pin by its row ID if it exists. + // Used for testing static std::optional GetPinById(SQLite::Connection& connection, const SQLite::rowid_t pinId); + + // Gets all the currently existing pins. static std::vector GetAllPins(SQLite::Connection& connection); - static void ResetAllPins(SQLite::Connection& connection); + + // Resets all pins from a given source, or from all sources if none is specified. + // Returns a value indicating whether there were any changes. + static bool ResetAllPins(SQLite::Connection& connection, std::string_view sourceId = {}); }; } diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h index c6560d5534..cfe049ebad 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface.h @@ -17,6 +17,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 SQLite::rowid_t RemovePin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; std::optional GetPin(SQLite::Connection& connection, const Pinning::PinKey& pinKey) override; std::vector GetAllPins(SQLite::Connection& connection) override; - void ResetAllPins(SQLite::Connection& connection) override; + bool ResetAllPins(SQLite::Connection& connection, std::string_view sourceId) override; }; } 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 d9459529d6..ca50879474 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 @@ -10,7 +10,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 { std::optional GetExistingPinId(SQLite::Connection& connection, const Pinning::PinKey& pinKey) { - auto result = PinTable::GetByPinKey(connection, pinKey); + auto result = PinTable::GetIdByPinKey(connection, pinKey); if (!result) { @@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 void PinningIndexInterface::CreateTables(SQLite::Connection& connection) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createPinningTables"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpinningtables_v1_0"); Pinning_V1_0::PinTable::Create(connection); savepoint.Commit(); } @@ -41,7 +41,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), existingPin.has_value()); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addPin_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addpin_v1_0"); SQLite::rowid_t pinId = PinTable::AddPin(connection, pin); savepoint.Commit(); @@ -55,8 +55,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // If the pin doesn't exist, fail the update THROW_HR_IF(E_NOT_SET, !existingPinId); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addPin_v1_0"); - bool status = PinTable::UpdatePin(connection, existingPinId.value(), pin); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatepin_v1_0"); + bool status = PinTable::UpdatePinById(connection, existingPinId.value(), pin); savepoint.Commit(); return { status, existingPinId.value() }; @@ -69,7 +69,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 // If the pin doesn't exist, fail the remove THROW_HR_IF(E_NOT_SET, !existingPinId); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removePin_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removepin_v1_0"); PinTable::RemovePinById(connection, existingPinId.value()); savepoint.Commit(); @@ -93,10 +93,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 return PinTable::GetAllPins(connection); } - void PinningIndexInterface::ResetAllPins(SQLite::Connection& connection) + bool PinningIndexInterface::ResetAllPins(SQLite::Connection& connection, std::string_view sourceId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "resetPins_v1_0"); - PinTable::ResetAllPins(connection); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "resetpins_v1_0"); + bool result = PinTable::ResetAllPins(connection, sourceId); savepoint.Commit(); + + return result; } } \ No newline at end of file From 9d63c89d5ef8532d58628ab200297488d7b064f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flor=20Chac=C3=B3n?= <14323496+florelis@users.noreply.github.com> Date: Mon, 9 Jan 2023 16:25:48 -0800 Subject: [PATCH 11/20] Update src/AppInstallerRepositoryCore/Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp Co-authored-by: yao-msft <50888816+yao-msft@users.noreply.github.com> --- .../Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 ca50879474..bd55e2ba07 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 @@ -53,7 +53,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 auto existingPinId = GetExistingPinId(connection, pin.GetKey()); // If the pin doesn't exist, fail the update - THROW_HR_IF(E_NOT_SET, !existingPinId); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), !existingPinId); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatepin_v1_0"); bool status = PinTable::UpdatePinById(connection, existingPinId.value(), pin); From ef09f687a92fd97070f632256df1752174d9f3a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flor=20Chac=C3=B3n?= <14323496+florelis@users.noreply.github.com> Date: Mon, 9 Jan 2023 16:26:25 -0800 Subject: [PATCH 12/20] Apply suggestions from code review Co-authored-by: yao-msft <50888816+yao-msft@users.noreply.github.com> --- .../Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 bd55e2ba07..c0e56b7cfc 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 @@ -68,7 +68,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 auto existingPinId = GetExistingPinId(connection, pinKey); // If the pin doesn't exist, fail the remove - THROW_HR_IF(E_NOT_SET, !existingPinId); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), !existingPinId); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removepin_v1_0"); PinTable::RemovePinById(connection, existingPinId.value()); From 0aceec3263bc9572df197b3aca7b3d67d83652af Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 16:29:16 -0800 Subject: [PATCH 13/20] Spelling --- .github/actions/spelling/allow.txt | 2 ++ src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw | 2 +- .../Microsoft/Schema/Pinning_1_0/PinningIndexInterface_1_0.cpp | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/actions/spelling/allow.txt b/.github/actions/spelling/allow.txt index 1c87c3ff13..7231a00be5 100644 --- a/.github/actions/spelling/allow.txt +++ b/.github/actions/spelling/allow.txt @@ -392,6 +392,7 @@ pfp PGP php PII +pinningindex pipssource PKCS placeholders @@ -449,6 +450,7 @@ removeportablefile repolibtest requeue rescap +resetpins resheader resmimetype RESOLVESOURCE diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index b25e360488..aa92060f65 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1638,7 +1638,7 @@ Please specify one of them using the --source option to proceed. Shown when listing or modifying existing pins if there are none. - Pins resetted successfully + Pins reset successfully Shown after resetting (deleting) all the pins \ No newline at end of file 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 c0e56b7cfc..354dca15b2 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 @@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::Pinning_V1_0 void PinningIndexInterface::CreateTables(SQLite::Connection& connection) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpinningtables_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createpintable_v1_0"); Pinning_V1_0::PinTable::Create(connection); savepoint.Commit(); } From 8f3d96bf16ccb44293ca06410f60f6d4663105dc Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 16:41:05 -0800 Subject: [PATCH 14/20] PR comments --- src/AppInstallerCommonCore/Public/winget/Pin.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/AppInstallerCommonCore/Public/winget/Pin.h b/src/AppInstallerCommonCore/Public/winget/Pin.h index fec59a9174..3a617e18ba 100644 --- a/src/AppInstallerCommonCore/Public/winget/Pin.h +++ b/src/AppInstallerCommonCore/Public/winget/Pin.h @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. #pragma once -#include -#include +#include "winget/Manifest.h" +#include "AppInstallerVersions.h" namespace AppInstaller::Pinning { From 19d87b1a4f3569180bf4b362a94bde009a7373e0 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 16:46:58 -0800 Subject: [PATCH 15/20] Move log --- src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index 40c1a5098d..e9cda61dd8 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -62,8 +62,8 @@ namespace AppInstaller::Repository::Microsoft void PinningIndex::RemovePin(const Pinning::PinKey& pinKey) { - AICLI_LOG(Repo, Verbose, << "Removing Pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); std::lock_guard lockInterface{ *m_interfaceLock }; + AICLI_LOG(Repo, Verbose, << "Removing Pin for package [" << pinKey.PackageId << "] from source [" << pinKey.SourceId << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "pinningIndex_removePin"); From f452776281211c656ab8a429fa32579eca3ec10b Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 17:03:49 -0800 Subject: [PATCH 16/20] Open read only --- src/AppInstallerCLICore/Commands/PinCommand.cpp | 12 +++++++----- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 5 +++-- src/AppInstallerCLICore/Workflows/PinFlow.h | 10 +++++++++- src/AppInstallerCLITests/PinFlow.cpp | 2 +- .../Public/AppInstallerRuntime.h | 2 -- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/AppInstallerCLICore/Commands/PinCommand.cpp b/src/AppInstallerCLICore/Commands/PinCommand.cpp index 1e2c5d0d71..d3e3f0fef1 100644 --- a/src/AppInstallerCLICore/Commands/PinCommand.cpp +++ b/src/AppInstallerCLICore/Commands/PinCommand.cpp @@ -124,7 +124,7 @@ namespace AppInstaller::CLI Workflow::EnsureOneMatchFromSearchResult(false) << Workflow::GetInstalledPackageVersion << Workflow::ReportPackageIdentity << - Workflow::OpenPinningIndex << + Workflow::OpenPinningIndex() << Workflow::SearchPin << Workflow::AddPin; } @@ -196,7 +196,7 @@ namespace AppInstaller::CLI Workflow::EnsureOneMatchFromSearchResult(false) << Workflow::GetInstalledPackageVersion << Workflow::ReportPackageIdentity << - Workflow::OpenPinningIndex << + Workflow::OpenPinningIndex() << Workflow::SearchPin << Workflow::RemovePin; } @@ -261,7 +261,7 @@ namespace AppInstaller::CLI void PinListCommand::ExecuteInternal(Execution::Context& context) const { context << - Workflow::OpenPinningIndex << + Workflow::OpenPinningIndex(/* readOnly */ true) << Workflow::GetAllPins << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << @@ -294,11 +294,12 @@ namespace AppInstaller::CLI void PinResetCommand::ExecuteInternal(Execution::Context& context) const { - context << Workflow::OpenPinningIndex; if (context.Args.Contains(Execution::Args::Type::Force)) { - context << Workflow::ResetAllPins; + context << + Workflow::OpenPinningIndex() << + Workflow::ResetAllPins; } else { @@ -306,6 +307,7 @@ namespace AppInstaller::CLI context.Reporter.Info() << Resource::String::PinResetUseForceArg << std::endl; context << + Workflow::OpenPinningIndex(/* readOnly */ true) << Workflow::GetAllPins << Workflow::OpenSource() << Workflow::OpenCompositeSource(Repository::PredefinedSource::Installed) << diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index e2b626818c..fd99d436a3 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -33,14 +33,15 @@ namespace AppInstaller::CLI::Workflow } } - void OpenPinningIndex(Execution::Context& context) + void OpenPinningIndex::operator()(Execution::Context& context) const { auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; AICLI_LOG(CLI, Info, << "Opening pinning index"); + SQLiteStorageBase::OpenDisposition openDisposition = m_readOnly ? SQLiteStorageBase::OpenDisposition::Read : SQLiteStorageBase::OpenDisposition::ReadWrite; auto pinningIndex = std::filesystem::exists(indexPath) ? - PinningIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : + PinningIndex::Open(indexPath.u8string(), openDisposition) : PinningIndex::CreateNew(indexPath.u8string()); context.Add(std::make_shared(std::move(pinningIndex))); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.h b/src/AppInstallerCLICore/Workflows/PinFlow.h index b367853d81..fd4a5fce2c 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.h +++ b/src/AppInstallerCLICore/Workflows/PinFlow.h @@ -9,7 +9,15 @@ namespace AppInstaller::CLI::Workflow // Required Args: None // Inputs: None // Outputs: PinningIndex - void OpenPinningIndex(Execution::Context& context); + struct OpenPinningIndex : public WorkflowTask + { + OpenPinningIndex(bool readOnly = false) : WorkflowTask("OpenPinningIndex"), m_readOnly(readOnly) {} + + void operator()(Execution::Context& context) const override; + + private: + bool m_readOnly; + }; // Gets all the pins from the index. // Required Args: None diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index c1dd94565b..02d3bcc0f5 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -15,7 +15,7 @@ using namespace AppInstaller::Pinning; void OverrideForOpenPinningIndex(TestContext& context, const std::filesystem::path& indexPath) { - context.Override({ OpenPinningIndex, [=](TestContext& context) + context.Override({ "OpenPinningIndex", [=](TestContext& context) { auto pinningIndex = std::filesystem::exists(indexPath) ? PinningIndex::Open(indexPath.u8string(), SQLiteStorageBase::OpenDisposition::ReadWrite) : diff --git a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h index 5ab209811c..f7df896850 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerRuntime.h @@ -62,8 +62,6 @@ namespace AppInstaller::Runtime PortableLinksUserLocation, // The location where symlinks to portable packages are stored under machine scope. PortableLinksMachineLocation, - // The location where the database for pins is stored. - PinningIndex, }; // The principal that an ACE applies to. From 18ba3304c73fc9f62b9f3a5ab34d892161afea63 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Mon, 9 Jan 2023 17:20:40 -0800 Subject: [PATCH 17/20] Move openpinningindex --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 11 ++--------- .../Microsoft/PinningIndex.cpp | 15 +++++++++++++++ .../Microsoft/PinningIndex.h | 4 ++++ 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index fd99d436a3..e84bf2d6f6 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -35,15 +35,8 @@ namespace AppInstaller::CLI::Workflow void OpenPinningIndex::operator()(Execution::Context& context) const { - auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; - - AICLI_LOG(CLI, Info, << "Opening pinning index"); - SQLiteStorageBase::OpenDisposition openDisposition = m_readOnly ? SQLiteStorageBase::OpenDisposition::Read : SQLiteStorageBase::OpenDisposition::ReadWrite; - - auto pinningIndex = std::filesystem::exists(indexPath) ? - PinningIndex::Open(indexPath.u8string(), openDisposition) : - PinningIndex::CreateNew(indexPath.u8string()); - + auto openDisposition = m_readOnly ? SQLiteStorageBase::OpenDisposition::Read : SQLiteStorageBase::OpenDisposition::ReadWrite; + auto pinningIndex = PinningIndex::OpenOrCreateDefault(openDisposition); context.Add(std::make_shared(std::move(pinningIndex))); } diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index e9cda61dd8..cbcce5f867 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -26,6 +26,21 @@ namespace AppInstaller::Repository::Microsoft return result; } + PinningIndex PinningIndex::OpenOrCreateDefault(OpenDisposition openDisposition) + { + auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; + AICLI_LOG(CLI, Info, << "Opening pinning index"); + + if (std::filesystem::exists(indexPath)) + { + return PinningIndex::Open(indexPath.u8string(), openDisposition); + } + else + { + return PinningIndex::CreateNew(indexPath.u8string()); + } + } + PinningIndex::IdType PinningIndex::AddPin(const Pinning::Pin& pin) { std::lock_guard lockInterface{ *m_interfaceLock }; diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h index b449864aba..b101ecf8ee 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -29,6 +29,10 @@ namespace AppInstaller::Repository::Microsoft return { filePath, disposition, std::move(indexFile) }; } + // Opens or creates a PinningIndex database on the default path. + // openDisposition is only used when opening an existing database. + static PinningIndex OpenOrCreateDefault(OpenDisposition openDisposition = OpenDisposition::ReadWrite); + // Adds a pin to the index. IdType AddPin(const Pinning::Pin& pin); From 1f6fe6204813ccb0b59a23dce48a64dcc5a9bfec Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 10 Jan 2023 10:57:18 -0800 Subject: [PATCH 18/20] Catch errors when opening the pinning index --- src/AppInstallerCLICore/Resources.h | 1 + src/AppInstallerCLICore/Workflows/PinFlow.cpp | 9 ++++++++- .../Shared/Strings/en-us/winget.resw | 4 ++++ src/AppInstallerCommonCore/Errors.cpp | 2 ++ .../Public/AppInstallerErrors.h | 1 + .../Microsoft/PinningIndex.cpp | 20 ++++++++++++------- .../Microsoft/PinningIndex.h | 3 ++- 7 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/AppInstallerCLICore/Resources.h b/src/AppInstallerCLICore/Resources.h index 998074c1bc..432c77c270 100644 --- a/src/AppInstallerCLICore/Resources.h +++ b/src/AppInstallerCLICore/Resources.h @@ -246,6 +246,7 @@ namespace AppInstaller::CLI::Resource WINGET_DEFINE_RESOURCE_STRINGID(PinAddCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinAdded); WINGET_DEFINE_RESOURCE_STRINGID(PinAlreadyExists); + WINGET_DEFINE_RESOURCE_STRINGID(PinCannotOpenIndex); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandLongDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinCommandShortDescription); WINGET_DEFINE_RESOURCE_STRINGID(PinDoesNotExist); diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index e84bf2d6f6..6eb15e2987 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -37,7 +37,14 @@ namespace AppInstaller::CLI::Workflow { auto openDisposition = m_readOnly ? SQLiteStorageBase::OpenDisposition::Read : SQLiteStorageBase::OpenDisposition::ReadWrite; auto pinningIndex = PinningIndex::OpenOrCreateDefault(openDisposition); - context.Add(std::make_shared(std::move(pinningIndex))); + if (!pinningIndex) + { + AICLI_LOG(CLI, Error, << "Unable to open pinning index."); + context.Reporter.Info() << Resource::String::PinCannotOpenIndex << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_CANNOT_OPEN_PINNING_INDEX); + } + + context.Add(std::move(pinningIndex)); } void GetAllPins(Execution::Context& context) diff --git a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw index aa92060f65..182670813d 100644 --- a/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw +++ b/src/AppInstallerCLIPackage/Shared/Strings/en-us/winget.resw @@ -1641,4 +1641,8 @@ Please specify one of them using the --source option to proceed. Pins reset successfully Shown after resetting (deleting) all the pins + + Unable to open pin database. + Error message for when we cannot open the database containing package pins. + \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp index c27694fdd7..45fd96a2d0 100644 --- a/src/AppInstallerCommonCore/Errors.cpp +++ b/src/AppInstallerCommonCore/Errors.cpp @@ -212,6 +212,8 @@ namespace AppInstaller return "A pin already exists for the package."; case APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST: return "There is no pin for the package."; + case APPINSTALLER_CLI_ERROR_CANNOT_OPEN_PINNING_INDEX: + return "Unable to open the pin database." // Install errors case APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE: diff --git a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h index 0ec281229d..f7864f751a 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerErrors.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerErrors.h @@ -112,6 +112,7 @@ #define APPINSTALLER_CLI_ERROR_PACKAGE_ALREADY_INSTALLED ((HRESULT)0x8A150061) #define APPINSTALLER_CLI_ERROR_PIN_ALREADY_EXISTS ((HRESULT)0x8A150062) #define APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST ((HRESULT)0x8A150063) +#define APPINSTALLER_CLI_ERROR_CANNOT_OPEN_PINNING_INDEX ((HRESULT)0x8A150064) // Install errors. #define APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE ((HRESULT)0x8A150101) diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index cbcce5f867..fc9dbd0ac2 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -26,19 +26,25 @@ namespace AppInstaller::Repository::Microsoft return result; } - PinningIndex PinningIndex::OpenOrCreateDefault(OpenDisposition openDisposition) + std::shared_ptr PinningIndex::OpenOrCreateDefault(OpenDisposition openDisposition) { auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; AICLI_LOG(CLI, Info, << "Opening pinning index"); - if (std::filesystem::exists(indexPath)) + try { - return PinningIndex::Open(indexPath.u8string(), openDisposition); - } - else - { - return PinningIndex::CreateNew(indexPath.u8string()); + if (std::filesystem::exists(indexPath)) + { + return std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); + } + else + { + return std::make_shared(PinningIndex::CreateNew(indexPath.u8string())); + } } + CATCH_LOG(); + + return {}; } PinningIndex::IdType PinningIndex::AddPin(const Pinning::Pin& pin) diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h index b101ecf8ee..77180331b5 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.h @@ -31,7 +31,8 @@ namespace AppInstaller::Repository::Microsoft // Opens or creates a PinningIndex database on the default path. // openDisposition is only used when opening an existing database. - static PinningIndex OpenOrCreateDefault(OpenDisposition openDisposition = OpenDisposition::ReadWrite); + // Returns nullptr in case of error. + static std::shared_ptr OpenOrCreateDefault(OpenDisposition openDisposition = OpenDisposition::ReadWrite); // Adds a pin to the index. IdType AddPin(const Pinning::Pin& pin); From 533c2c9ffe1ff7b7c0d80bedec17d7a2ff0bc2b2 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Tue, 10 Jan 2023 11:11:18 -0800 Subject: [PATCH 19/20] Update tests after merging master --- src/AppInstallerCLICore/Workflows/PinFlow.cpp | 2 +- src/AppInstallerCLITests/PinFlow.cpp | 26 +++++++++---------- src/AppInstallerCommonCore/Errors.cpp | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/PinFlow.cpp b/src/AppInstallerCLICore/Workflows/PinFlow.cpp index 6eb15e2987..55f2294348 100644 --- a/src/AppInstallerCLICore/Workflows/PinFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/PinFlow.cpp @@ -40,7 +40,7 @@ namespace AppInstaller::CLI::Workflow if (!pinningIndex) { AICLI_LOG(CLI, Error, << "Unable to open pinning index."); - context.Reporter.Info() << Resource::String::PinCannotOpenIndex << std::endl; + context.Reporter.Error() << Resource::String::PinCannotOpenIndex << std::endl; AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_CANNOT_OPEN_PINNING_INDEX); } diff --git a/src/AppInstallerCLITests/PinFlow.cpp b/src/AppInstallerCLITests/PinFlow.cpp index 02d3bcc0f5..42989fc5da 100644 --- a/src/AppInstallerCLITests/PinFlow.cpp +++ b/src/AppInstallerCLITests/PinFlow.cpp @@ -31,8 +31,8 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; OverrideForOpenPinningIndex(addContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(addContext); - addContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + OverrideForCompositeInstalledSource(addContext, CreateTestSource({ TSR:: TestInstaller_Exe })); + addContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); addContext.Args.AddArg(Execution::Args::Type::BlockingPin); PinAddCommand pinAdd({}); @@ -52,8 +52,8 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinListOutput; TestContext listContext{ pinListOutput, std::cin }; OverrideForOpenPinningIndex(listContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(listContext); - listContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + OverrideForCompositeInstalledSource(listContext, CreateTestSource({ TSR::TestInstaller_Exe })); + listContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); PinListCommand pinList({}); pinList.Execute(listContext); @@ -67,8 +67,8 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinRemoveOutput; TestContext removeContext{ pinRemoveOutput, std::cin }; OverrideForOpenPinningIndex(removeContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(removeContext); - removeContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + OverrideForCompositeInstalledSource(removeContext, CreateTestSource({ TSR::TestInstaller_Exe })); + removeContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); PinRemoveCommand pinRemove({}); pinRemove.Execute(removeContext); @@ -86,7 +86,7 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") SECTION("Without --force") { - OverrideForCompositeInstalledSource(resetContext); + OverrideForCompositeInstalledSource(resetContext, CreateTestSource({ TSR::TestInstaller_Exe })); PinResetCommand pinReset({}); pinReset.Execute(resetContext); INFO(pinResetOutput.str()); @@ -113,8 +113,8 @@ TEST_CASE("PinFlow_Add", "[PinFlow][workflow]") std::ostringstream pinUpdateOutput; TestContext updateContext{ pinUpdateOutput, std::cin }; OverrideForOpenPinningIndex(updateContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(updateContext); - updateContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + OverrideForCompositeInstalledSource(updateContext, CreateTestSource({ TSR::TestInstaller_Exe })); + updateContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); SECTION("Without --force") { @@ -148,7 +148,7 @@ TEST_CASE("PinFlow_Add_NotFound", "[PinFlow][workflow]") { std::ostringstream pinAddOutput; TestContext addContext{ pinAddOutput, std::cin }; - OverrideForCompositeInstalledSource(addContext); + OverrideForCompositeInstalledSource(addContext, CreateTestSource({})); addContext.Args.AddArg(Execution::Args::Type::Query, "This package doesn't exist"sv); PinAddCommand pinAdd({}); @@ -165,7 +165,7 @@ TEST_CASE("PinFlow_ListEmpty", "[PinFlow][workflow]") std::ostringstream pinListOutput; TestContext listContext{ pinListOutput, std::cin }; OverrideForOpenPinningIndex(listContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(listContext); + OverrideForCompositeInstalledSource(listContext, CreateTestSource({})); PinListCommand pinList({}); pinList.Execute(listContext); @@ -181,8 +181,8 @@ TEST_CASE("PinFlow_RemoveNonExisting", "[PinFlow][workflow]") std::ostringstream pinRemoveOutput; TestContext removeContext{ pinRemoveOutput, std::cin }; OverrideForOpenPinningIndex(removeContext, indexFile.GetPath()); - OverrideForCompositeInstalledSource(removeContext); - removeContext.Args.AddArg(Execution::Args::Type::Query, "AppInstallerCliTest.TestExeInstaller"sv); + OverrideForCompositeInstalledSource(removeContext, CreateTestSource({ TSR::TestInstaller_Exe })); + removeContext.Args.AddArg(Execution::Args::Type::Query, TSR::TestInstaller_Exe.Query); PinRemoveCommand pinRemove({}); pinRemove.Execute(removeContext); diff --git a/src/AppInstallerCommonCore/Errors.cpp b/src/AppInstallerCommonCore/Errors.cpp index 45fd96a2d0..524920f0fe 100644 --- a/src/AppInstallerCommonCore/Errors.cpp +++ b/src/AppInstallerCommonCore/Errors.cpp @@ -213,7 +213,7 @@ namespace AppInstaller case APPINSTALLER_CLI_ERROR_PIN_DOES_NOT_EXIST: return "There is no pin for the package."; case APPINSTALLER_CLI_ERROR_CANNOT_OPEN_PINNING_INDEX: - return "Unable to open the pin database." + return "Unable to open the pin database."; // Install errors case APPINSTALLER_CLI_ERROR_INSTALL_PACKAGE_IN_USE: From 772c3199ca555025913a0631c6645be48f0ce634 Mon Sep 17 00:00:00 2001 From: Flor Elisa Chacon Ochoa Date: Thu, 12 Jan 2023 11:03:29 -0800 Subject: [PATCH 20/20] Delete bad pinning index file --- .../Microsoft/PinningIndex.cpp | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp index fc9dbd0ac2..aa4688dc12 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/PinningIndex.cpp @@ -29,18 +29,26 @@ namespace AppInstaller::Repository::Microsoft std::shared_ptr PinningIndex::OpenOrCreateDefault(OpenDisposition openDisposition) { auto indexPath = Runtime::GetPathTo(Runtime::PathName::LocalState) / "pinning.db"; - AICLI_LOG(CLI, Info, << "Opening pinning index"); try { if (std::filesystem::exists(indexPath)) { - return std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); - } - else - { - return std::make_shared(PinningIndex::CreateNew(indexPath.u8string())); + if (std::filesystem::is_regular_file(indexPath)) + { + try + { + AICLI_LOG(Repo, Info, << "Opening existing pinning index"); + return std::make_shared(PinningIndex::Open(indexPath.u8string(), openDisposition)); + } + CATCH_LOG(); + } + + AICLI_LOG(Repo, Info, << "Attempting to delete bad index file"); + std::filesystem::remove_all(indexPath); } + + return std::make_shared(PinningIndex::CreateNew(indexPath.u8string())); } CATCH_LOG();