From f02f73004823405f28f0fdc1e8ae8666a029d57c Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 11 Oct 2021 11:40:28 -0700 Subject: [PATCH 1/8] External work largely done, needs internal tracking implementation --- .../Commands/UninstallCommand.cpp | 3 +- .../Workflows/InstallFlow.cpp | 60 +++++++---- .../Workflows/InstallFlow.h | 6 ++ .../Workflows/UninstallFlow.cpp | 70 +++++++++++- .../Workflows/UninstallFlow.h | 8 +- .../AppInstallerRepositoryCore.vcxproj | 2 + ...AppInstallerRepositoryCore.vcxproj.filters | 9 ++ .../CompositeSource.cpp | 2 +- .../CompositeSource.h | 1 - .../PackageTrackingCatalog.cpp | 101 ++++++++++++++++++ .../Public/winget/PackageTrackingCatalog.h | 60 +++++++++++ 11 files changed, 298 insertions(+), 24 deletions(-) create mode 100644 src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp create mode 100644 src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h diff --git a/src/AppInstallerCLICore/Commands/UninstallCommand.cpp b/src/AppInstallerCLICore/Commands/UninstallCommand.cpp index 44e0279622..e92238f9d9 100644 --- a/src/AppInstallerCLICore/Commands/UninstallCommand.cpp +++ b/src/AppInstallerCLICore/Commands/UninstallCommand.cpp @@ -136,6 +136,7 @@ namespace AppInstaller::CLI Workflow::ReportDependencies(Resource::String::UninstallCommandReportDependencies) << Workflow::ReportExecutionStage(ExecutionStage::Execution) << Workflow::ExecuteUninstaller << - Workflow::ReportExecutionStage(ExecutionStage::PostExecution); + Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << + Workflow::RecordUninstall; } } \ No newline at end of file diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp index b82da0a59f..13ab526a24 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.cpp @@ -10,21 +10,24 @@ #include "MsiInstallFlow.h" #include "WorkflowBase.h" #include "Workflows/DependenciesFlow.h" +#include #include #include +using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; +using namespace winrt::Windows::Foundation; +using namespace winrt::Windows::Foundation::Collections; +using namespace winrt::Windows::Management::Deployment; +using namespace AppInstaller::CLI::Execution; +using namespace AppInstaller::Manifest; +using namespace AppInstaller::Repository; +using namespace AppInstaller::Settings; +using namespace AppInstaller::Utility; + + namespace AppInstaller::CLI::Workflow { - using namespace winrt::Windows::ApplicationModel::Store::Preview::InstallControl; - using namespace winrt::Windows::Foundation; - using namespace winrt::Windows::Foundation::Collections; - using namespace winrt::Windows::Management::Deployment; - using namespace AppInstaller::Utility; - using namespace AppInstaller::Manifest; - using namespace AppInstaller::Repository; - using namespace AppInstaller::Settings; - namespace { bool MightWriteToARP(InstallerTypeEnum type) @@ -610,15 +613,16 @@ namespace AppInstaller::CLI::Workflow void InstallPackageInstaller(Execution::Context& context) { context << - Workflow::ReportExecutionStage(ExecutionStage::Download) << - Workflow::DownloadInstaller << - Workflow::ReportExecutionStage(ExecutionStage::PreExecution) << - Workflow::SnapshotARPEntries << - Workflow::ReportExecutionStage(ExecutionStage::Execution) << - Workflow::ExecuteInstaller << - Workflow::ReportExecutionStage(ExecutionStage::PostExecution) << - Workflow::ReportARPChanges << - Workflow::RemoveInstaller; + ReportExecutionStage(ExecutionStage::Download) << + DownloadInstaller << + ReportExecutionStage(ExecutionStage::PreExecution) << + SnapshotARPEntries << + ReportExecutionStage(ExecutionStage::Execution) << + ExecuteInstaller << + ReportExecutionStage(ExecutionStage::PostExecution) << + ReportARPChanges << + RecordInstall << + RemoveInstaller; } void InstallSinglePackage(Execution::Context& context) @@ -910,5 +914,23 @@ namespace AppInstaller::CLI::Workflow ); } } - CATCH_LOG() + CATCH_LOG(); + + void RecordInstall(Context& context) + { + // Local manifest installs won't have a package version, and tracking them doesn't provide much + // value currently. If we ever do use our own database as a primary source of packages that we + // maintain, this decision will probably have to be reconsidered. + if (!context.Contains(Data::PackageVersion)) + { + return; + } + + auto trackingCatalog = PackageTrackingCatalog::CreateForSource(context.Get()->GetSource()); + + trackingCatalog.RecordInstall( + context.Get(), + context.Get().value(), + WI_IsFlagSet(context.GetFlags(), ContextFlag::InstallerExecutionUseUpdate)); + } } diff --git a/src/AppInstallerCLICore/Workflows/InstallFlow.h b/src/AppInstallerCLICore/Workflows/InstallFlow.h index 1fb2a5240b..3014b1b05e 100644 --- a/src/AppInstallerCLICore/Workflows/InstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/InstallFlow.h @@ -191,4 +191,10 @@ namespace AppInstaller::CLI::Workflow // Inputs: ARPSnapshot?, Manifest, PackageVersion // Outputs: None void ReportARPChanges(Execution::Context& context); + + // Records the installation to the tracking catalog. + // Required Args: None + // Inputs: PackageVersion?, Manifest, Installer + // Outputs: None + void RecordInstall(Execution::Context& context); } diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp index cc3deaedac..6f4de3d5ab 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -7,13 +7,58 @@ #include "AppInstallerMsixInfo.h" #include +#include +using namespace AppInstaller::CLI::Execution; using namespace AppInstaller::Manifest; using namespace AppInstaller::Msix; using namespace AppInstaller::Repository; namespace AppInstaller::CLI::Workflow { + namespace + { + // Helper for RecordUninstall + struct UninstallCorrelatedSources + { + struct Item + { + Utility::LocIndString Identifier; + std::shared_ptr Source; + std::string SourceIdentifier; + }; + + void AddIfRemoteAndNotPresent(const std::shared_ptr& packageVersion) + { + auto source = packageVersion->GetSource(); + const auto& details = source->GetDetails(); + if (!IsSourceRemote(details)) + { + return; + } + + for (const auto& item : Items) + { + if (item.SourceIdentifier == details.Identifier) + { + return; + } + } + + Items.emplace_back(Item{ packageVersion->GetProperty(PackageVersionProperty::Id), std::move(source), details.Identifier }); + } + + std::vector Items; + + private: + bool IsSourceRemote(const SourceDetails& details) + { + auto origin = details.Origin; + return (origin == SourceOrigin::Default || origin == SourceOrigin::GroupPolicy || origin == SourceOrigin::User); + } + }; + } + void GetUninstallInfo(Execution::Context& context) { auto installedPackageVersion = context.Get(); @@ -124,4 +169,27 @@ namespace AppInstaller::CLI::Workflow context.Reporter.Info() << Resource::String::UninstallFlowUninstallSuccess << std::endl; } -} \ No newline at end of file + + void RecordUninstall(Context& context) + { + // In order to report an uninstall to every correlated tracking catalog, we first need to find them all. + auto package = context.Get(); + UninstallCorrelatedSources correlatedSources; + + // Start with the installed version + correlatedSources.AddIfRemoteAndNotPresent(package->GetInstalledVersion()); + + // Then look through all available versions + for (const auto& versionKey : package->GetAvailableVersionKeys()) + { + correlatedSources.AddIfRemoteAndNotPresent(package->GetAvailableVersion(versionKey)); + } + + // Finally record the uninstall for each found value + for (const auto& item : correlatedSources.Items) + { + auto trackingCatalog = PackageTrackingCatalog::CreateForSource(item.Source); + trackingCatalog.RecordUninstall(item.Identifier); + } + } +} diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.h b/src/AppInstallerCLICore/Workflows/UninstallFlow.h index 8a71a23040..6db0954cf9 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.h +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.h @@ -22,4 +22,10 @@ namespace AppInstaller::CLI::Workflow // Inputs: PackageFamilyNames // Outputs: None void MsixUninstall(Execution::Context& context); -} \ No newline at end of file + + // Records the uninstall to the tracking catalog. + // Required Args: None + // Inputs: Package + // Outputs: None + void RecordUninstall(Execution::Context& context); +} diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj index 34ba45a85f..ca02ea44e5 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj @@ -266,6 +266,7 @@ + @@ -324,6 +325,7 @@ + Create diff --git a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters index c68d50fb81..771934b432 100644 --- a/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters +++ b/src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters @@ -55,6 +55,9 @@ {2cc20cdb-dcb2-4e0e-b04f-e2d838146100} + + {4459e669-cc20-4550-9f12-53e088e9170e} + @@ -240,6 +243,9 @@ Microsoft + + Public\winget + @@ -374,6 +380,9 @@ Microsoft + + Source Files + diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index c59fb0c903..6f297b3823 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -2,6 +2,7 @@ // Licensed under the MIT License. #include "pch.h" #include "CompositeSource.h" +#include "winget/PackageTrackingCatalog.h" namespace AppInstaller::Repository { @@ -470,7 +471,6 @@ namespace AppInstaller::Repository auto compositePackage = std::make_shared(std::move(match.Package)); // Create a search request to run against all available sources - // TODO: Determine if we should create a single search or one for each installed package. SearchRequest systemReferenceSearch; auto installedVersion = compositePackage->GetInstalledVersion(); diff --git a/src/AppInstallerRepositoryCore/CompositeSource.h b/src/AppInstallerRepositoryCore/CompositeSource.h index cb96e7e32f..f1ff0ba3eb 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.h +++ b/src/AppInstallerRepositoryCore/CompositeSource.h @@ -51,7 +51,6 @@ namespace AppInstaller::Repository private: // Performs a search when an installed source is present. - // Will only return packages that are installed. SearchResult SearchInstalled(const SearchRequest& request) const; // Performs a search when no installed source is present. diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp new file mode 100644 index 0000000000..92951b4b6d --- /dev/null +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -0,0 +1,101 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "winget/PackageTrackingCatalog.h" +#include "Microsoft/SQLiteIndexSource.h" + + +namespace AppInstaller::Repository +{ + namespace + { + constexpr std::string_view c_PackageTrackingDirectoryName = ""; + + std::filesystem::path GetPackageTrackingRootPath() + { + std::filesystem::path result = Runtime::GetPathTo(Runtime::PathName::LocalState); + result /= + } + } + + struct PackageTrackingCatalog::implementation + { + std::shared_ptr Source; + }; + + PackageTrackingCatalog::PackageTrackingCatalog() = default; + PackageTrackingCatalog::PackageTrackingCatalog(const PackageTrackingCatalog&) = default; + PackageTrackingCatalog& PackageTrackingCatalog::operator=(const PackageTrackingCatalog&) = default; + PackageTrackingCatalog::PackageTrackingCatalog(PackageTrackingCatalog&&) noexcept = default; + PackageTrackingCatalog& PackageTrackingCatalog::operator=(PackageTrackingCatalog&&) noexcept = default; + PackageTrackingCatalog::~PackageTrackingCatalog() = default; + + PackageTrackingCatalog PackageTrackingCatalog::CreateForSource(const std::shared_ptr& source) + { + THROW_HR_IF(E_INVALIDARG, details.Type != PreIndexedPackageSourceFactory::Type()); + + auto lock = Synchronization::CrossProcessReaderWriteLock::LockShared(CreateNameForCPRWL(details), progress); + if (!lock) + { + return {}; + } + + std::filesystem::path packageLocation = GetStatePathFromDetails(details); + packageLocation /= s_PreIndexedPackageSourceFactory_IndexFileName; + + if (!std::filesystem::exists(packageLocation)) + { + AICLI_LOG(Repo, Info, << "Data not found at " << packageLocation); + THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_DATA_MISSING); + } + + SQLiteIndex index = SQLiteIndex::Open(packageLocation.u8string(), SQLiteIndex::OpenDisposition::Read); + + // We didn't use to store the source identifier, so we compute it here in case it's + // missing from the details. + return std::make_shared(details, GetPackageFamilyNameFromDetails(details), std::move(index), std::move(lock)); + } + + SearchResult PackageTrackingCatalog::Search(const SearchRequest& request) const + { + return m_implementation->Source->Search(request); + } + + struct PackageTrackingCatalog::Version::implementation + { + }; + + PackageTrackingCatalog::Version::Version() = default; + PackageTrackingCatalog::Version::Version(const Version&) = default; + PackageTrackingCatalog::Version& PackageTrackingCatalog::Version::operator=(const Version&) = default; + PackageTrackingCatalog::Version::Version(Version&&) noexcept = default; + PackageTrackingCatalog::Version& PackageTrackingCatalog::Version::operator=(Version&&) noexcept = default; + PackageTrackingCatalog::Version::~Version() = default; + + PackageTrackingCatalog::Version::Version(std::shared_ptr&& value) : + m_implementation(std::move(value)) {} + + void PackageTrackingCatalog::Version::SetMetadata(PackageVersionMetadata metadata, const Utility::NormalizedString& value) + { + UNREFERENCED_PARAMETER(metadata); + UNREFERENCED_PARAMETER(value); + THROW_HR(E_NOTIMPL); + } + + PackageTrackingCatalog::Version PackageTrackingCatalog::RecordInstall( + const Manifest::Manifest& manifest, + const Manifest::ManifestInstaller& installer, + bool isUpgrade) + { + UNREFERENCED_PARAMETER(manifest); + UNREFERENCED_PARAMETER(installer); + UNREFERENCED_PARAMETER(isUpgrade); + THROW_HR(E_NOTIMPL); + } + + void PackageTrackingCatalog::RecordUninstall(const Utility::LocIndString& packageIdentifier) + { + UNREFERENCED_PARAMETER(packageIdentifier); + THROW_HR(E_NOTIMPL); + } +} diff --git a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h new file mode 100644 index 0000000000..7d8c201053 --- /dev/null +++ b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#pragma once +#include +#include + +#include + + +namespace AppInstaller::Repository +{ + // A catalog for tracking package actions from a given source. + struct PackageTrackingCatalog + { + PackageTrackingCatalog(); + PackageTrackingCatalog(const PackageTrackingCatalog&); + PackageTrackingCatalog& operator=(const PackageTrackingCatalog&); + PackageTrackingCatalog(PackageTrackingCatalog&&) noexcept; + PackageTrackingCatalog& operator=(PackageTrackingCatalog&&) noexcept; + ~PackageTrackingCatalog(); + + // Creates or opens the tracking catalog for the given source. + // TODO: Make creation exclusive to the refactored Source type. + static PackageTrackingCatalog CreateForSource(const std::shared_ptr& source); + + // Execute a search against the catalog. + SearchResult Search(const SearchRequest& request) const; + + // Enables more granular control over the metadata in the tracking catalog if necessary. + struct Version + { + friend PackageTrackingCatalog; + + Version(); + Version(const Version&); + Version& operator=(const Version&); + Version(Version&&) noexcept; + Version& operator=(Version&&) noexcept; + ~Version(); + + // Set the given metadata value. + void SetMetadata(PackageVersionMetadata metadata, const Utility::NormalizedString& value); + + private: + struct implementation; + Version(std::shared_ptr&& value); + std::shared_ptr m_implementation; + }; + + // Records an installation of the given package. + Version RecordInstall(const Manifest::Manifest& manifest, const Manifest::ManifestInstaller& installer, bool isUpgrade); + + // Records an uninstall of the given package. + void RecordUninstall(const Utility::LocIndString& packageIdentifier); + + private: + struct implementation; + std::shared_ptr m_implementation; + }; +} From ce4a05cd35a8a37ddbe844bc6006af3a6544e58a Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Fri, 15 Oct 2021 18:15:25 -0700 Subject: [PATCH 2/8] Creation done, needs simple writes implemented and tests --- .../Workflows/UninstallFlow.cpp | 9 +-- src/AppInstallerCLITests/Strings.cpp | 12 ++++ .../AppInstallerStrings.cpp | 57 ++++++++++++++++ .../Public/AppInstallerStrings.h | 3 + .../Public/AppInstallerSynchronization.h | 7 ++ .../Synchronization.cpp | 14 ++-- .../PackageTrackingCatalog.cpp | 67 ++++++++++++++----- .../Public/AppInstallerRepositorySource.h | 4 ++ .../RepositorySource.cpp | 5 ++ 9 files changed, 149 insertions(+), 29 deletions(-) diff --git a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp index 6f4de3d5ab..6e5b233666 100644 --- a/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/UninstallFlow.cpp @@ -32,7 +32,7 @@ namespace AppInstaller::CLI::Workflow { auto source = packageVersion->GetSource(); const auto& details = source->GetDetails(); - if (!IsSourceRemote(details)) + if (!ContainsAvailablePackages(details.Origin)) { return; } @@ -49,13 +49,6 @@ namespace AppInstaller::CLI::Workflow } std::vector Items; - - private: - bool IsSourceRemote(const SourceDetails& details) - { - auto origin = details.Origin; - return (origin == SourceOrigin::Default || origin == SourceOrigin::GroupPolicy || origin == SourceOrigin::User); - } }; } diff --git a/src/AppInstallerCLITests/Strings.cpp b/src/AppInstallerCLITests/Strings.cpp index b3aa0f20d1..19a9f80d7b 100644 --- a/src/AppInstallerCLITests/Strings.cpp +++ b/src/AppInstallerCLITests/Strings.cpp @@ -173,3 +173,15 @@ TEST_CASE("ReplaceWhileCopying", "[strings]") REQUIRE(ReplaceWhileCopying(L"A red apple", L"p", L"f") == L"A red affle"); REQUIRE(ReplaceWhileCopying(L"A red apple", L"", L"green") == L"A red apple"); } + +TEST_CASE("MakeSuitablePathPart", "[strings]") +{ + REQUIRE(MakeSuitablePathPart("A\\B") == "A_B"); + REQUIRE(MakeSuitablePathPart("A\\B/") == "A_B_"); + REQUIRE(MakeSuitablePathPart("*AB") == "_AB"); + REQUIRE(MakeSuitablePathPart(u8"f*\xF6*ldcase") == u8"f_\xF6_ldcase"); + REQUIRE(MakeSuitablePathPart(".") == "_"); + REQUIRE(MakeSuitablePathPart("..") == "._"); + REQUIRE_THROWS_HR(MakeSuitablePathPart("COM1"), E_INVALIDARG); + REQUIRE_THROWS_HR(MakeSuitablePathPart("NUL.txt"), E_INVALIDARG); +} diff --git a/src/AppInstallerCommonCore/AppInstallerStrings.cpp b/src/AppInstallerCommonCore/AppInstallerStrings.cpp index dfec1d8e68..9144321fc2 100644 --- a/src/AppInstallerCommonCore/AppInstallerStrings.cpp +++ b/src/AppInstallerCommonCore/AppInstallerStrings.cpp @@ -512,4 +512,61 @@ namespace AppInstaller::Utility FindAndReplace(result, s_MessageReplacementToken, value); return result; } + + // Follow the rules at https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file to replace + // invalid characters in a candidate path part. + std::string MakeSuitablePathPart(std::string_view candidate) + { + constexpr char replaceChar = '_'; + constexpr std::string_view illegalChars = R"(<>:"/\|?*)"; + + // First, walk the string and replace illegal characters + std::string result; + result.reserve(candidate.size()); + + ICUBreakIterator itr{ candidate, UBRK_CHARACTER }; + + while (itr.CurrentBreak() != UBRK_DONE && itr.CurrentOffset() < candidate.size()) + { + UChar32 current = itr.CurrentCodePoint(); + bool isIllegal = current < 32 || (current < 256 && illegalChars.find(static_cast(current)) != std::string::npos); + + int32_t offset = itr.CurrentBreak(); + int32_t nextOffset = itr.Next(); + + // Don't allow a . at the end of a name + if (static_cast(nextOffset) >= candidate.size()) + { + if (current == static_cast('.')) + { + isIllegal = true; + } + } + + if (isIllegal) + { + result.append(1, replaceChar); + } + else + { + size_t count = (nextOffset == UBRK_DONE ? std::string::npos : static_cast(nextOffset) - static_cast(offset)); + result.append(candidate.substr(static_cast(offset), count)); + } + } + + // Second, look for any newly formed illegal names. + // For now just error on these cases; they should not happen often. + for (const auto& illegalName : { + "."sv, "CON"sv, "PRN"sv, "AUX"sv, "NUL"sv, "COM1"sv, "COM2"sv, "COM3"sv, "COM4"sv, "COM5"sv, "COM6"sv, "COM7"sv, "COM8"sv, "COM9"sv, + "LPT1"sv, "LPT2"sv, "LPT3"sv, "LPT4"sv, "LPT5"sv, "LPT6"sv, "LPT7"sv, "LPT8"sv, "LPT9"sv }) + { + // Either equals the illegal name (starts with and same length) or starts with and the first character after is a . + if (CaseInsensitiveStartsWith(result, illegalName) && (result.size() == illegalName.size() || result[illegalName.size()] == '.')) + { + THROW_HR(E_INVALIDARG); + } + } + + return result; + } } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h b/src/AppInstallerCommonCore/Public/AppInstallerStrings.h index 71078acaa3..91309c53b7 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerStrings.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerStrings.h @@ -139,4 +139,7 @@ namespace AppInstaller::Utility // Replace message predefined token std::string FindAndReplaceMessageToken(std::string_view message, std::string_view value); + + // Converts the candidate path part into one suitable for the actual file system + std::string MakeSuitablePathPart(std::string_view candidate); } diff --git a/src/AppInstallerCommonCore/Public/AppInstallerSynchronization.h b/src/AppInstallerCommonCore/Public/AppInstallerSynchronization.h index 399aa04480..4e65a09117 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerSynchronization.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerSynchronization.h @@ -21,6 +21,11 @@ namespace AppInstaller::Synchronization // - Readers are limited to an arbitrarily chosen limit. // - Not re-entrant (although repeated read locking will work, it will consume additional slots). // - No upgrade from reader to writer. + // In order to change from reader/write, one must release and reacquire the lock: + // auto lock = CrossProcessReaderWriteLock::LockShared(same_name); + // ... Determine that an exclusive is needed ... + // lock.Release(); + // lock = CrossProcessReaderWriteLock::LockExclusive(same_name); struct CrossProcessReaderWriteLock { // Create unheld lock. @@ -43,6 +48,8 @@ namespace AppInstaller::Synchronization operator bool() const; + void Release(); + private: static CrossProcessReaderWriteLock Lock(bool shared, std::string_view name, std::chrono::milliseconds timeout, IProgressCallback* progress); diff --git a/src/AppInstallerCommonCore/Synchronization.cpp b/src/AppInstallerCommonCore/Synchronization.cpp index 70cfc2cbe1..3d62cf2ea4 100644 --- a/src/AppInstallerCommonCore/Synchronization.cpp +++ b/src/AppInstallerCommonCore/Synchronization.cpp @@ -46,10 +46,7 @@ namespace AppInstaller::Synchronization CrossProcessReaderWriteLock::~CrossProcessReaderWriteLock() { - for (auto& mutex : m_mutexesHeld) - { - ReleaseMutex(mutex.get()); - } + Release(); } CrossProcessReaderWriteLock CrossProcessReaderWriteLock::LockShared(std::string_view name) @@ -82,6 +79,15 @@ namespace AppInstaller::Synchronization return !m_mutexesHeld.empty(); } + void CrossProcessReaderWriteLock::Release() + { + for (auto& mutex : m_mutexesHeld) + { + ReleaseMutex(mutex.get()); + } + m_mutexesHeld.clear(); + } + CrossProcessReaderWriteLock CrossProcessReaderWriteLock::Lock( bool shared, std::string_view name, diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index 92951b4b6d..ec99a87f39 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -4,17 +4,27 @@ #include "winget/PackageTrackingCatalog.h" #include "Microsoft/SQLiteIndexSource.h" +using namespace std::string_literals; +using namespace AppInstaller::Repository::Microsoft; + namespace AppInstaller::Repository { namespace { - constexpr std::string_view c_PackageTrackingDirectoryName = ""; + constexpr std::string_view c_PackageTrackingFileName = "installed.db"; + + std::string CreateNameForCPRWL(const std::string& pathName) + { + return "PackageTrackingCPRWL_"s + pathName; + } - std::filesystem::path GetPackageTrackingRootPath() + std::filesystem::path GetPackageTrackingFilePath(const std::string& pathName) { std::filesystem::path result = Runtime::GetPathTo(Runtime::PathName::LocalState); - result /= + result /= pathName; + result /= c_PackageTrackingFileName; + return result; } } @@ -32,28 +42,51 @@ namespace AppInstaller::Repository PackageTrackingCatalog PackageTrackingCatalog::CreateForSource(const std::shared_ptr& source) { - THROW_HR_IF(E_INVALIDARG, details.Type != PreIndexedPackageSourceFactory::Type()); - - auto lock = Synchronization::CrossProcessReaderWriteLock::LockShared(CreateNameForCPRWL(details), progress); - if (!lock) + // Not a valid source for tracking + const std::string& sourceIdentifier = source->GetIdentifier(); + if (sourceIdentifier.empty() || !ContainsAvailablePackages(source->GetDetails().Origin)) { - return {}; + THROW_HR(E_INVALIDARG); } - std::filesystem::path packageLocation = GetStatePathFromDetails(details); - packageLocation /= s_PreIndexedPackageSourceFactory_IndexFileName; + std::string pathName = Utility::MakeSuitablePathPart(sourceIdentifier); + + std::string lockName = CreateNameForCPRWL(pathName); + auto lock = Synchronization::CrossProcessReaderWriteLock::LockShared(lockName); - if (!std::filesystem::exists(packageLocation)) + std::filesystem::path trackingDB = GetPackageTrackingFilePath(pathName); + + if (!std::filesystem::exists(trackingDB)) { - AICLI_LOG(Repo, Info, << "Data not found at " << packageLocation); - THROW_HR(APPINSTALLER_CLI_ERROR_SOURCE_DATA_MISSING); + lock.Release(); + lock = Synchronization::CrossProcessReaderWriteLock::LockExclusive(lockName); + + if (!std::filesystem::exists(trackingDB)) + { + std::filesystem::create_directories(trackingDB.parent_path()); + SQLiteIndex::CreateNew(trackingDB.u8string()); + } + + lock.Release(); + lock = Synchronization::CrossProcessReaderWriteLock::LockShared(lockName); } - SQLiteIndex index = SQLiteIndex::Open(packageLocation.u8string(), SQLiteIndex::OpenDisposition::Read); + SQLiteIndex index = SQLiteIndex::Open(trackingDB.u8string(), SQLiteIndex::OpenDisposition::Read); + + // TODO: Check schema version and upgrade as necessary when there is a relevant new schema. + // Could write this all now but it will be better tested when there is a new schema. + + // Create fake details for the source while stashing some information that might be helpful for debugging + SourceDetails details; + details.Name = "Tracking for "s + source->GetDetails().Name; + details.Origin = SourceOrigin::PackageTracking; + details.Arg = pathName; + + PackageTrackingCatalog result; + result.m_implementation = std::make_shared< PackageTrackingCatalog::implementation>(); + result.m_implementation->Source = std::make_shared(details, "*Tracking", std::move(index), std::move(lock)); - // We didn't use to store the source identifier, so we compute it here in case it's - // missing from the details. - return std::make_shared(details, GetPackageFamilyNameFromDetails(details), std::move(index), std::move(lock)); + return result; } SearchResult PackageTrackingCatalog::Search(const SearchRequest& request) const diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h index bfd533b022..962a78b9e1 100644 --- a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h @@ -23,6 +23,7 @@ namespace AppInstaller::Repository Predefined, GroupPolicy, Metadata, + PackageTracking, }; // Defines the trust level of the source. @@ -253,4 +254,7 @@ namespace AppInstaller::Repository // Saves the accepted source agreements in metadata. void SaveAcceptedSourceAgreements(const SourceDetails& source); + + // Returns true if the origin type can contain available packages. + bool ContainsAvailablePackages(SourceOrigin origin); } diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index cbb0afebc5..41966e25e0 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -626,6 +626,11 @@ namespace AppInstaller::Repository sourceList.SaveAcceptedSourceAgreements(source); } + bool ContainsAvailablePackages(SourceOrigin origin) + { + return (origin == SourceOrigin::Default || origin == SourceOrigin::GroupPolicy || origin == SourceOrigin::User); + } + bool SearchRequest::IsForEverything() const { return (!Query.has_value() && Inclusions.empty() && Filters.empty()); From 73df3c41eac02727e3dfbc3615ea4976a3cdab4f Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 18 Oct 2021 16:38:10 -0700 Subject: [PATCH 3/8] Code complete on basic writes --- .../Microsoft/SQLiteIndex.cpp | 50 ++++++++++++++-- .../Microsoft/SQLiteIndex.h | 25 +++++++- .../Microsoft/SQLiteIndexSource.h | 1 + .../Microsoft/Schema/1_0/Interface.h | 8 ++- .../Microsoft/Schema/1_0/Interface_1_0.cpp | 28 +++++---- .../Microsoft/Schema/1_0/PathPartTable.cpp | 59 ++++++++++++++++++- .../Microsoft/Schema/1_0/PathPartTable.h | 6 +- .../Microsoft/Schema/1_1/Interface.h | 6 +- .../Microsoft/Schema/1_1/Interface_1_1.cpp | 12 ++-- .../Microsoft/Schema/1_2/Interface.h | 6 +- .../Microsoft/Schema/1_2/Interface_1_2.cpp | 12 ++-- .../Microsoft/Schema/1_3/Interface.h | 4 +- .../Microsoft/Schema/1_3/Interface_1_3.cpp | 4 +- .../Microsoft/Schema/ISQLiteIndex.h | 17 ++++-- .../PackageTrackingCatalog.cpp | 56 ++++++++++++++++-- .../Public/AppInstallerRepositorySearch.h | 2 + .../RepositorySource.cpp | 1 + .../SQLiteWrapper.cpp | 11 +++- 18 files changed, 251 insertions(+), 57 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp index 459f3e3576..41918d46dc 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -136,12 +136,22 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Verbose, << "Adding manifest from file [" << manifestPath << "]"); Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(manifestPath); - return AddManifest(manifest, relativePath); + return AddManifestInternal(manifest, relativePath); } SQLiteIndex::IdType SQLiteIndex::AddManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Verbose, << "Adding manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + return AddManifestInternal(manifest, relativePath); + } + + SQLiteIndex::IdType SQLiteIndex::AddManifest(const Manifest::Manifest& manifest) + { + return AddManifestInternal(manifest, {}); + } + + SQLiteIndex::IdType SQLiteIndex::AddManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath) + { + AICLI_LOG(Repo, Verbose, << "Adding manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath.value_or("") << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_addmanifest"); @@ -159,12 +169,22 @@ namespace AppInstaller::Repository::Microsoft AICLI_LOG(Repo, Verbose, << "Updating manifest from file [" << manifestPath << "]"); Manifest::Manifest manifest = Manifest::YamlParser::CreateFromPath(manifestPath); - return UpdateManifest(manifest, relativePath); + return UpdateManifestInternal(manifest, relativePath); } bool SQLiteIndex::UpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { - AICLI_LOG(Repo, Verbose, << "Updating manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + return UpdateManifestInternal(manifest, relativePath); + } + + bool SQLiteIndex::UpdateManifest(const Manifest::Manifest& manifest) + { + return UpdateManifestInternal(manifest, {}); + } + + bool SQLiteIndex::UpdateManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath) + { + AICLI_LOG(Repo, Verbose, << "Updating manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath.value_or("") << "]"); SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_updatemanifest"); @@ -191,10 +211,25 @@ namespace AppInstaller::Repository::Microsoft void SQLiteIndex::RemoveManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) { AICLI_LOG(Repo, Verbose, << "Removing manifest for [" << manifest.Id << ", " << manifest.Version << "] at relative path [" << relativePath << "]"); + RemoveManifest(manifest); + } + void SQLiteIndex::RemoveManifest(const Manifest::Manifest& manifest) + { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_removemanifest"); - m_interface->RemoveManifest(m_dbconn, manifest, relativePath); + m_interface->RemoveManifest(m_dbconn, manifest); + + SetLastWriteTime(); + + savepoint.Commit(); + } + + void SQLiteIndex::RemoveManifestById(IdType manifestId) + { + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_removemanifestbyid"); + + m_interface->RemoveManifestById(m_dbconn, manifestId); SetLastWriteTime(); @@ -241,6 +276,11 @@ namespace AppInstaller::Repository::Microsoft return m_interface->GetManifestIdByKey(m_dbconn, id, version, channel); } + std::optional SQLiteIndex::GetManifestIdByManifest(const Manifest::Manifest& manifest) const + { + return m_interface->GetManifestIdByManifest(m_dbconn, manifest); + } + std::vector SQLiteIndex::GetVersionKeysById(IdType id) const { return m_interface->GetVersionKeysById(m_dbconn, id); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h index 297253c6c7..7a2b563569 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -77,6 +78,11 @@ namespace AppInstaller::Repository::Microsoft // Returns the manifest id. IdType AddManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath); + // Adds the manifest to the index. + // If the function succeeds, the manifest has been added. + // Returns the manifest id. + IdType AddManifest(const Manifest::Manifest& manifest); + // Updates the manifest with matching { Id, Version, Channel } in the index. // The return value indicates whether the index was modified by the function. bool UpdateManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath); @@ -85,14 +91,22 @@ namespace AppInstaller::Repository::Microsoft // The return value indicates whether the index was modified by the function. bool UpdateManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath); + // Updates the manifest with matching { Id, Version, Channel } in the index. + // The return value indicates whether the index was modified by the function. + bool UpdateManifest(const Manifest::Manifest& manifest); + // Removes the manifest with matching { Id, Version, Channel } from the index. - // Path is currently ignored. void RemoveManifest(const std::filesystem::path& manifestPath, const std::filesystem::path& relativePath); // Removes the manifest with matching { Id, Version, Channel } from the index. - // Path is currently ignored. void RemoveManifest(const Manifest::Manifest& manifest, const std::filesystem::path& relativePath); + // Removes the manifest with matching { Id, Version, Channel } from the index. + void RemoveManifest(const Manifest::Manifest& manifest); + + // Removes the manifest with the given id. + void RemoveManifestById(IdType manifestId); + // Removes data that is no longer needed for an index that is to be published. void PrepareForPackaging(); @@ -113,6 +127,9 @@ namespace AppInstaller::Repository::Microsoft // If version is empty, gets the value for the 'latest' version. std::optional GetManifestIdByKey(IdType id, std::string_view version, std::string_view channel) const; + // Gets the manifest id for the given manifest, if present. + std::optional GetManifestIdByManifest(const Manifest::Manifest& manifest) const; + // Gets all versions and channels for the given id. std::vector GetVersionKeysById(IdType id) const; @@ -133,6 +150,10 @@ namespace AppInstaller::Repository::Microsoft // Constructor used to create a new index. SQLiteIndex(const std::string& target, Schema::Version version); + // Internal functions to normalize on the relativePath being present. + IdType AddManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath); + bool UpdateManifestInternal(const Manifest::Manifest& manifest, const std::optional& relativePath); + // Sets the last write time metadata value in the index. void SetLastWriteTime(); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h index fa43ad79d7..0099c3dca4 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.h @@ -35,6 +35,7 @@ namespace AppInstaller::Repository::Microsoft SearchResult Search(const SearchRequest& request) const override; // Gets the index. + SQLiteIndex& GetIndex() { return m_index; } const SQLiteIndex& GetIndex() const { return m_index; } // Determines if the other source refers to the same as this. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h index 059e2bdd25..0df7b85d44 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h @@ -16,15 +16,17 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 // Version 1.0 Schema::Version GetVersion() const override; void CreateTables(SQLite::Connection& connection) override; - SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest) override; + void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) override; void PrepareForPackaging(SQLite::Connection& connection) override; bool CheckConsistency(const SQLite::Connection& connection, bool log) const override; SearchResult Search(const SQLite::Connection& connection, const SearchRequest& request) const override; std::optional GetPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionProperty property) const override; std::vector GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const override; std::optional GetManifestIdByKey(const SQLite::Connection& connection, SQLite::rowid_t id, std::string_view version, std::string_view channel) const override; + std::optional GetManifestIdByManifest(const SQLite::Connection& connection, const Manifest::Manifest& manifest) const override; std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const override; // Version 1.1 diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp index adbddc20e5..d211176a88 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp @@ -24,7 +24,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 namespace { // Gets an existing manifest by its rowid., if it exists. - std::optional GetExistingManifestId(SQLite::Connection& connection, const Manifest::Manifest& manifest) + std::optional GetExistingManifestId(const SQLite::Connection& connection, const Manifest::Manifest& manifest) { std::optional idId = IdTable::SelectIdByValue(connection, manifest.Id, true); if (!idId) @@ -169,7 +169,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 savepoint.Commit(); } - SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { auto manifestResult = GetExistingManifestId(connection, manifest); @@ -181,7 +181,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 auto [pathAdded, pathLeafId] = PathPartTable::EnsurePathExists(connection, relativePath, true); // If we get false from the function, this manifest path already exists in the index. - THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), !pathAdded); + THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_ALREADY_EXISTS), relativePath && !pathAdded); // Ensure that all of the 1:1 data exists. SQLite::rowid_t idId = IdTable::EnsureExists(connection, manifest.Id, true); @@ -209,7 +209,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return manifestId; } - std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { auto manifestResult = GetExistingManifestId(connection, manifest); @@ -260,7 +260,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 auto [existingPathLeafId] = ManifestTable::GetIdsById(connection, manifestId); auto [pathAdded, newPathLeafId] = PathPartTable::EnsurePathExists(connection, relativePath, true); - if (pathAdded) + if (relativePath && pathAdded) { // Path was added, so we need to update the manifest table and delete the old path ManifestTable::UpdateValueIdById(connection, manifestId, newPathLeafId); @@ -282,20 +282,25 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return { indexModified, manifestId }; } - SQLite::rowid_t Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path&) + SQLite::rowid_t Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest) { auto manifestResult = GetExistingManifestId(connection, manifest); // If the manifest doesn't actually exist, fail the remove. THROW_HR_IF(E_NOT_SET, !manifestResult); - SQLite::rowid_t manifestId = manifestResult.value(); + RemoveManifestById(connection, manifestResult.value()); + + return manifestResult.value(); + } + void Interface::RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) + { // Get the ids of the values from the manifest table auto [idId, nameId, monikerId, versionId, channelId, pathLeafId] = ManifestTable::GetIdsById(connection, manifestId); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_0"); // Remove the manifest row ManifestTable::DeleteById(connection, manifestId); @@ -315,8 +320,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 CommandsTable::DeleteIfNotNeededByManifestId(connection, manifestId); savepoint.Commit(); - - return manifestId; } void Interface::PrepareForPackaging(SQLite::Connection& connection) @@ -511,6 +514,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return StaticGetManifestIdByKey(connection, id, version, channel); } + std::optional Interface::GetManifestIdByManifest(const SQLite::Connection& connection, const Manifest::Manifest& manifest) const + { + return GetExistingManifestId(connection, manifest); + } + std::vector Interface::GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const { auto versionsAndChannels = ManifestTable::GetAllValuesById(connection, id); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp index 69b6445d75..98fc94c573 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp @@ -49,6 +49,20 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return connection.GetLastInsertRowID(); } + // Inserts the no path part into the table. + SQLite::rowid_t InsertNoPathPart(SQLite::Connection& connection) + { + SQLite::Builder::StatementBuilder builder; + builder. + InsertInto(s_PathPartTable_Table_Name). + Columns({ SQLite::RowIDName, s_PathPartTable_ParentValue_Name, s_PathPartTable_PartValue_Name }). + Values(PathPartTable::NoPathId, std::optional{}, std::string_view{}); + + builder.Execute(connection); + + return connection.GetLastInsertRowID(); + } + // Gets the parent of a given part by id. // This should only be called when the part must exist, as it will throw if not found. std::optional GetParentById(SQLite::Connection& connection, SQLite::rowid_t id) @@ -155,7 +169,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return s_PathPartTable_PartValue_Name; } - std::tuple PathPartTable::EnsurePathExists(SQLite::Connection& connection, const std::filesystem::path& relativePath, bool createIfNotFound) + std::tuple EnsurePathExists(SQLite::Connection& connection, const std::filesystem::path& relativePath, bool createIfNotFound) { THROW_HR_IF(E_INVALIDARG, !relativePath.has_relative_path()); THROW_HR_IF(E_INVALIDARG, relativePath.has_root_path()); @@ -204,6 +218,49 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return { (createIfNotFound ? partsAdded : true), parent.value() }; } + std::tuple PathPartTable::EnsurePathExists(SQLite::Connection& connection, const std::optional& relativePath, bool createIfNotFound) + { + if (relativePath) + { + return EnsurePathExists(connection, relativePath.value(), createIfNotFound); + } + + std::unique_ptr savepoint; + if (createIfNotFound) + { + savepoint = std::make_unique(SQLite::Savepoint::Create(connection, "ensurepathexists_v1_0")); + } + + bool partsAdded = false; + + std::optional noPathPart = SelectPathPart(connection, {}, {}); + + if (!noPathPart) + { + if (createIfNotFound) + { + partsAdded = true; + noPathPart = InsertNoPathPart(connection); + } + else + { + // Not found, and we were told not to create. + // Return false to indicate that the path does not exist. + return {}; + } + } + + if (savepoint) + { + savepoint->Commit(); + } + + // If we get this far, the path exists. + // If we were asked to create it, return whether we needed to or it was already present. + // If not, then true indicates that it exists. + return { (createIfNotFound ? partsAdded : true), noPathPart.value() }; + } + std::optional PathPartTable::GetPathById(const SQLite::Connection& connection, SQLite::rowid_t id) { SQLite::Builder::StatementBuilder builder; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.h index 50d061f875..5868a20bdc 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.h @@ -17,6 +17,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 // The id type using id_t = SQLite::rowid_t; + // The id used when no path is present. + constexpr static id_t NoPathId = -1; + // Creates the table with named indices. static void Create(SQLite::Connection& connection); @@ -37,7 +40,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 // The result bool will indicate whether the path was found (true), or not (false). // In all cases except createIfNotFound == false and result bool == false, the int64_t value // will be valid and the rowid of the final path part in the path. - static std::tuple EnsurePathExists(SQLite::Connection& connection, const std::filesystem::path& relativePath, bool createIfNotFound); + // If relativePath is not provided, will always map to an entry with NoPathId. + static std::tuple EnsurePathExists(SQLite::Connection& connection, const std::optional& relativePath, bool createIfNotFound); // Gets the path string using the given id as the leaf. static std::optional GetPathById(const SQLite::Connection& connection, SQLite::rowid_t id); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h index b48d6da5a0..cb8c4f628b 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h @@ -13,9 +13,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 // Version 1.0 Schema::Version GetVersion() const override; void CreateTables(SQLite::Connection& connection) override; - SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) override; void PrepareForPackaging(SQLite::Connection& connection) override; bool CheckConsistency(const SQLite::Connection& connection, bool log) const override; SearchResult Search(const SQLite::Connection& connection, const SearchRequest& request) const override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp index a899c5b0e7..8284e1b2b8 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp @@ -96,7 +96,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 savepoint.Commit(); } - SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addmanifest_v1_1"); @@ -113,7 +113,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 return manifestId; } - std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatemanifest_v1_1"); @@ -128,11 +128,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 return { indexModified, manifestId }; } - SQLite::rowid_t Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + void Interface::RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_1"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_1"); - SQLite::rowid_t manifestId = V1_0::Interface::RemoveManifest(connection, manifest, relativePath); + V1_0::Interface::RemoveManifestById(connection, manifestId); // Remove all of the new 1:N data that is no longer referenced. PackageFamilyNameTable::DeleteIfNotNeededByManifestId(connection, manifestId); @@ -144,8 +144,6 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 } savepoint.Commit(); - - return manifestId; } void Interface::PrepareForPackaging(SQLite::Connection& connection) diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h index f3b2161eec..f766c09932 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h @@ -15,9 +15,9 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 // Version 1.0 Schema::Version GetVersion() const override; void CreateTables(SQLite::Connection& connection) override; - SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) override; bool CheckConsistency(const SQLite::Connection& connection, bool log) const override; std::vector GetMultiPropertyByManifestId(const SQLite::Connection& connection, SQLite::rowid_t manifestId, PackageVersionMultiProperty property) const override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp index 236027314a..47ef53603e 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp @@ -89,7 +89,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 savepoint.Commit(); } - SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addmanifest_v1_2"); @@ -106,7 +106,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 return manifestId; } - std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatemanifest_v1_2"); @@ -121,19 +121,17 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 return { indexModified, manifestId }; } - SQLite::rowid_t Interface::RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + void Interface::RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifest_v1_2"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_2"); - SQLite::rowid_t manifestId = V1_1::Interface::RemoveManifest(connection, manifest, relativePath); + V1_1::Interface::RemoveManifestById(connection, manifestId); // Remove all of the new 1.2 data that is no longer referenced. NormalizedPackageNameTable::DeleteIfNotNeededByManifestId(connection, manifestId); NormalizedPackagePublisherTable::DeleteIfNotNeededByManifestId(connection, manifestId); savepoint.Commit(); - - return manifestId; } bool Interface::CheckConsistency(const SQLite::Connection& connection, bool log) const diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h index 0e576132ae..023a63a05c 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h @@ -15,8 +15,8 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 // Version 1.0 Schema::Version GetVersion() const override; void CreateTables(SQLite::Connection& connection) override; - SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; - std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) override; + SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; + std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; protected: // Gets a property already knowing that the manifest id is valid. diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp index 68f258ff5e..c5005ce0ad 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp @@ -31,7 +31,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 savepoint.Commit(); } - SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + SQLite::rowid_t Interface::AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "addmanifest_v1_3"); @@ -49,7 +49,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 return manifestId; } - std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) + std::pair Interface::UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "updatemanifest_v1_3"); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h index 0a0ef69efa..f4fda805a7 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h @@ -9,6 +9,7 @@ #include #include +#include namespace AppInstaller::Repository::Microsoft::Schema @@ -41,15 +42,20 @@ namespace AppInstaller::Repository::Microsoft::Schema virtual void CreateTables(SQLite::Connection& connection) = 0; // Adds the manifest at the repository relative path to the index. - virtual SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; + virtual SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) = 0; // Updates the manifest with matching { Id, Version, Channel } in the index. // The return value indicates whether the index was modified by the function. - virtual std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; + virtual std::pair UpdateManifest( + SQLite::Connection& connection, + const Manifest::Manifest& manifest, + const std::optional& relativePath) = 0; // Removes the manifest with matching { Id, Version, Channel } from the index. - // Path is currently ignored. - virtual SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::filesystem::path& relativePath) = 0; + virtual SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest) = 0; + + // Removes the manifest with the given id. + virtual void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) = 0; // Removes data that is no longer needed for an index that is to be published. virtual void PrepareForPackaging(SQLite::Connection& connection) = 0; @@ -71,6 +77,9 @@ namespace AppInstaller::Repository::Microsoft::Schema // If version is empty, gets the value for the 'latest' version. virtual std::optional GetManifestIdByKey(const SQLite::Connection& connection, SQLite::rowid_t id, std::string_view version, std::string_view channel) const = 0; + // Gets the manifest id for the given manifest, if present. + virtual std::optional GetManifestIdByManifest(const SQLite::Connection& connection, const Manifest::Manifest& manifest) const = 0; + // Gets all versions and channels for the given id. virtual std::vector GetVersionKeysById(const SQLite::Connection& connection, SQLite::rowid_t id) const = 0; diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index ec99a87f39..ad1adf5d16 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "winget/PackageTrackingCatalog.h" #include "Microsoft/SQLiteIndexSource.h" +#include "AppInstallerDateTime.h" using namespace std::string_literals; using namespace AppInstaller::Repository::Microsoft; @@ -71,7 +72,7 @@ namespace AppInstaller::Repository lock = Synchronization::CrossProcessReaderWriteLock::LockShared(lockName); } - SQLiteIndex index = SQLiteIndex::Open(trackingDB.u8string(), SQLiteIndex::OpenDisposition::Read); + SQLiteIndex index = SQLiteIndex::Open(trackingDB.u8string(), SQLiteIndex::OpenDisposition::ReadWrite); // TODO: Check schema version and upgrade as necessary when there is a relevant new schema. // Could write this all now but it will be better tested when there is a new schema. @@ -83,7 +84,7 @@ namespace AppInstaller::Repository details.Arg = pathName; PackageTrackingCatalog result; - result.m_implementation = std::make_shared< PackageTrackingCatalog::implementation>(); + result.m_implementation = std::make_shared(); result.m_implementation->Source = std::make_shared(details, "*Tracking", std::move(index), std::move(lock)); return result; @@ -96,6 +97,7 @@ namespace AppInstaller::Repository struct PackageTrackingCatalog::Version::implementation { + SQLiteIndex::IdType Id; }; PackageTrackingCatalog::Version::Version() = default; @@ -120,15 +122,57 @@ namespace AppInstaller::Repository const Manifest::ManifestInstaller& installer, bool isUpgrade) { - UNREFERENCED_PARAMETER(manifest); + // TODO: Store additional information from these if needed UNREFERENCED_PARAMETER(installer); UNREFERENCED_PARAMETER(isUpgrade); - THROW_HR(E_NOTIMPL); + + auto& index = m_implementation->Source->GetIndex(); + + // Check for an existing manifest that matches this one (could be reinstalling) + auto manifestIdOpt = index.GetManifestIdByManifest(manifest); + + if (manifestIdOpt) + { + index.UpdateManifest(manifest); + } + else + { + manifestIdOpt = index.AddManifest(manifest); + } + + SQLiteIndex::IdType manifestId = manifestIdOpt.value(); + + // Write additional metadata for package tracking + std::ostringstream strstr; + strstr << Utility::GetCurrentUnixEpoch(); + index.SetMetadataByManifestId(manifestId, PackageVersionMetadata::TrackingWriteTime, strstr.str()); + + std::shared_ptr result = std::make_shared(); + result->Id = manifestId; + return { std::move(result) }; } void PackageTrackingCatalog::RecordUninstall(const Utility::LocIndString& packageIdentifier) { - UNREFERENCED_PARAMETER(packageIdentifier); - THROW_HR(E_NOTIMPL); + auto& index = m_implementation->Source->GetIndex(); + + SearchRequest idSearch; + idSearch.Filters.emplace_back(PackageMatchField::Id, MatchType::CaseInsensitive, packageIdentifier.get()); + auto searchResult = index.Search(idSearch); + + for (const auto& match : searchResult.Matches) + { + auto versions = index.GetVersionKeysById(match.first); + + for (const auto& version : versions) + { + auto manifestId = index.GetManifestIdByKey(match.first, version.GetVersion().ToString(), version.GetChannel().ToString()); + + if (manifestId) + { + index.RemoveManifestById(manifestId.value()); + } + } + } } } diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h index e831e6846f..e435c4972f 100644 --- a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h +++ b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySearch.h @@ -161,6 +161,8 @@ namespace AppInstaller::Repository Publisher, // The locale of the package InstalledLocale, + // The write time for the given version + TrackingWriteTime, }; // Convert a PackageVersionMetadata to a string. diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 41966e25e0..8b9a19dd4a 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -684,6 +684,7 @@ namespace AppInstaller::Repository case PackageVersionMetadata::SilentUninstallCommand: return "SilentUninstallCommand"sv; case PackageVersionMetadata::Publisher: return "Publisher"sv; case PackageVersionMetadata::InstalledLocale: return "InstalledLocale"sv; + case PackageVersionMetadata::TrackingWriteTime: return "TrackingWriteTime"sv; default: return "Unknown"sv; } } diff --git a/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp b/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp index b64291e767..5e72b13f6d 100644 --- a/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp +++ b/src/AppInstallerRepositoryCore/SQLiteWrapper.cpp @@ -62,7 +62,16 @@ namespace AppInstaller::Repository::SQLite void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, std::string_view v) { - THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8)); + if (v.empty()) + { + // An empty string_view can have it's data member return nullptr, which effectively binds a null value. + // We don't want that, so instead bind an empty string, which will have a non-null data pointer. + ParameterSpecificsImpl::Bind(stmt, index, {}); + } + else + { + THROW_IF_SQLITE_FAILED(sqlite3_bind_text64(stmt, index, v.data(), v.size(), SQLITE_TRANSIENT, SQLITE_UTF8)); + } } void ParameterSpecificsImpl::Bind(sqlite3_stmt* stmt, int index, int v) From 2717ce5327c9c3c561ff33c68273e6ba8f286d7f Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 18 Oct 2021 16:56:52 -0700 Subject: [PATCH 4/8] Fix casing for spelling reasons --- .../Microsoft/Schema/1_0/Interface_1_0.cpp | 2 +- .../Microsoft/Schema/1_1/Interface_1_1.cpp | 2 +- .../Microsoft/Schema/1_2/Interface_1_2.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp index d211176a88..1e133f0ac5 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp @@ -300,7 +300,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 auto [idId, nameId, monikerId, versionId, channelId, pathLeafId] = ManifestTable::GetIdsById(connection, manifestId); - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_0"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "RemoveManifestById_v1_0"); // Remove the manifest row ManifestTable::DeleteById(connection, manifestId); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp index 8284e1b2b8..467ef031b7 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp @@ -130,7 +130,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 void Interface::RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_1"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "RemoveManifestById_v1_1"); V1_0::Interface::RemoveManifestById(connection, manifestId); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp index 47ef53603e..d0594ac750 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp @@ -123,7 +123,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 void Interface::RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "removemanifestbyid_v1_2"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "RemoveManifestById_v1_2"); V1_1::Interface::RemoveManifestById(connection, manifestId); From 699a71d507c682d8d76b920541462075f9faa742 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 19 Oct 2021 14:26:52 -0700 Subject: [PATCH 5/8] Changes for regression tests --- src/AppInstallerRepositoryCore/CompositeSource.cpp | 1 - .../Microsoft/SQLiteIndex.cpp | 6 +++--- .../Microsoft/SQLiteIndex.h | 5 ++++- .../Microsoft/Schema/1_0/Interface.h | 2 +- .../Microsoft/Schema/1_0/Interface_1_0.cpp | 4 ++-- .../Microsoft/Schema/1_0/PathPartTable.cpp | 10 ++++++++-- .../Microsoft/Schema/1_1/Interface.h | 2 +- .../Microsoft/Schema/1_1/Interface_1_1.cpp | 4 ++-- .../Microsoft/Schema/1_2/Interface.h | 2 +- .../Microsoft/Schema/1_2/Interface_1_2.cpp | 4 ++-- .../Microsoft/Schema/1_3/Interface.h | 2 +- .../Microsoft/Schema/1_3/Interface_1_3.cpp | 4 ++-- .../Microsoft/Schema/ISQLiteIndex.h | 13 ++++++++++++- .../PackageTrackingCatalog.cpp | 2 +- .../SQLiteStatementBuilder.cpp | 12 ++++++------ .../SQLiteStatementBuilder.h | 9 ++++++--- 16 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/AppInstallerRepositoryCore/CompositeSource.cpp b/src/AppInstallerRepositoryCore/CompositeSource.cpp index 6f297b3823..39915119c1 100644 --- a/src/AppInstallerRepositoryCore/CompositeSource.cpp +++ b/src/AppInstallerRepositoryCore/CompositeSource.cpp @@ -2,7 +2,6 @@ // Licensed under the MIT License. #include "pch.h" #include "CompositeSource.h" -#include "winget/PackageTrackingCatalog.h" namespace AppInstaller::Repository { diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp index 41918d46dc..caff4957f4 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.cpp @@ -25,7 +25,7 @@ namespace AppInstaller::Repository::Microsoft } } - SQLiteIndex SQLiteIndex::CreateNew(const std::string& filePath, Schema::Version version) + SQLiteIndex SQLiteIndex::CreateNew(const std::string& filePath, Schema::Version version, CreateOptions options) { AICLI_LOG(Repo, Info, << "Creating new SQLite Index [" << version << "] at '" << filePath << "'"); SQLiteIndex result{ filePath, version }; @@ -36,7 +36,7 @@ namespace AppInstaller::Repository::Microsoft // Use calculated version, as incoming version could be 'latest' result.m_version.SetSchemaVersion(result.m_dbconn); - result.m_interface->CreateTables(result.m_dbconn); + result.m_interface->CreateTables(result.m_dbconn, options); result.SetLastWriteTime(); @@ -227,7 +227,7 @@ namespace AppInstaller::Repository::Microsoft void SQLiteIndex::RemoveManifestById(IdType manifestId) { - SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "sqliteindex_removemanifestbyid"); + SQLite::Savepoint savepoint = SQLite::Savepoint::Create(m_dbconn, "SQLiteIndex_RemoveManifestById"); m_interface->RemoveManifestById(m_dbconn, manifestId); diff --git a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h index 7a2b563569..90c41b75c4 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/SQLiteIndex.h @@ -33,6 +33,9 @@ namespace AppInstaller::Repository::Microsoft // The return type of GetMetadataByManifestId using MetadataResult = Schema::ISQLiteIndex::MetadataResult; + // Options for creating a new index. + using CreateOptions = Schema::ISQLiteIndex::CreateOptions; + SQLiteIndex(const SQLiteIndex&) = delete; SQLiteIndex& operator=(const SQLiteIndex&) = delete; @@ -40,7 +43,7 @@ namespace AppInstaller::Repository::Microsoft SQLiteIndex& operator=(SQLiteIndex&&) = default; // Creates a new index database of the given version. - static SQLiteIndex CreateNew(const std::string& filePath, Schema::Version version = Schema::Version::Latest()); + static SQLiteIndex CreateNew(const std::string& filePath, Schema::Version version = Schema::Version::Latest(), CreateOptions options = CreateOptions::None); // The disposition for opening the index. enum class OpenDisposition diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h index 0df7b85d44..797f9422ef 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface.h @@ -15,7 +15,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 { // Version 1.0 Schema::Version GetVersion() const override; - void CreateTables(SQLite::Connection& connection) override; + void CreateTables(SQLite::Connection& connection, CreateOptions options) override; SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; SQLite::rowid_t RemoveManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest) override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp index 1e133f0ac5..06e81f7d09 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/Interface_1_0.cpp @@ -142,7 +142,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return { 1, 0 }; } - void Interface::CreateTables(SQLite::Connection& connection) + void Interface::CreateTables(SQLite::Connection& connection, CreateOptions options) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createtables_v1_0"); @@ -160,7 +160,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 { MonikerTable::ValueName(), false, false }, { VersionTable::ValueName(), true, false }, { ChannelTable::ValueName(), true, false }, - { PathPartTable::ValueName(), false, true } + { PathPartTable::ValueName(), false, WI_IsFlagClear(options, CreateOptions::SupportPathless) } }); TagsTable::Create_deprecated(connection); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp index 98fc94c573..5cea7d41c7 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_0/PathPartTable.cpp @@ -169,7 +169,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 return s_PathPartTable_PartValue_Name; } - std::tuple EnsurePathExists(SQLite::Connection& connection, const std::filesystem::path& relativePath, bool createIfNotFound) + std::tuple EnsurePathExistsInternal(SQLite::Connection& connection, const std::filesystem::path& relativePath, bool createIfNotFound) { THROW_HR_IF(E_INVALIDARG, !relativePath.has_relative_path()); THROW_HR_IF(E_INVALIDARG, relativePath.has_root_path()); @@ -222,7 +222,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 { if (relativePath) { - return EnsurePathExists(connection, relativePath.value(), createIfNotFound); + return EnsurePathExistsInternal(connection, relativePath.value(), createIfNotFound); } std::unique_ptr savepoint; @@ -320,6 +320,12 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_0 void PathPartTable::RemovePathById(SQLite::Connection& connection, SQLite::rowid_t id) { + // Don't bother removing the pathless id + if (id == NoPathId) + { + return; + } + SQLite::rowid_t currentPartToRemove = id; while (IsLeafPart(connection, currentPartToRemove)) { diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h index cb8c4f628b..e21536ea13 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface.h @@ -12,7 +12,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 { // Version 1.0 Schema::Version GetVersion() const override; - void CreateTables(SQLite::Connection& connection) override; + void CreateTables(SQLite::Connection& connection, CreateOptions options) override; SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp index 467ef031b7..7254e20591 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_1/Interface_1_1.cpp @@ -67,7 +67,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 return { 1, 1 }; } - void Interface::CreateTables(SQLite::Connection& connection) + void Interface::CreateTables(SQLite::Connection& connection, CreateOptions options) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createtables_v1_1"); @@ -85,7 +85,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_1 { V1_0::MonikerTable::ValueName(), false, false }, { V1_0::VersionTable::ValueName(), true, false }, { V1_0::ChannelTable::ValueName(), true, false }, - { V1_0::PathPartTable::ValueName(), false, true } + { V1_0::PathPartTable::ValueName(), false, WI_IsFlagClear(options, CreateOptions::SupportPathless) } }); V1_0::TagsTable::Create(connection); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h index f766c09932..e0eb2a783a 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface.h @@ -14,7 +14,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 // Version 1.0 Schema::Version GetVersion() const override; - void CreateTables(SQLite::Connection& connection) override; + void CreateTables(SQLite::Connection& connection, CreateOptions options) override; SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; void RemoveManifestById(SQLite::Connection& connection, SQLite::rowid_t manifestId) override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp index d0594ac750..27d12a8d58 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_2/Interface_1_2.cpp @@ -73,11 +73,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_2 return { 1, 2 }; } - void Interface::CreateTables(SQLite::Connection& connection) + void Interface::CreateTables(SQLite::Connection& connection, CreateOptions options) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createtables_v1_2"); - V1_1::Interface::CreateTables(connection); + V1_1::Interface::CreateTables(connection, options); // While the name and publisher should be linked per-locale, we are not implementing that here. // This will mean that one can match cross locale name and publisher, but the chance that this diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h index 023a63a05c..e5f2314939 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface.h @@ -14,7 +14,7 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 // Version 1.0 Schema::Version GetVersion() const override; - void CreateTables(SQLite::Connection& connection) override; + void CreateTables(SQLite::Connection& connection, CreateOptions options) override; SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; std::pair UpdateManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) override; diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp index c5005ce0ad..9456cb3bde 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/1_3/Interface_1_3.cpp @@ -20,11 +20,11 @@ namespace AppInstaller::Repository::Microsoft::Schema::V1_3 return { 1, 3 }; } - void Interface::CreateTables(SQLite::Connection& connection) + void Interface::CreateTables(SQLite::Connection& connection, CreateOptions options) { SQLite::Savepoint savepoint = SQLite::Savepoint::Create(connection, "createtables_v1_3"); - V1_2::Interface::CreateTables(connection); + V1_2::Interface::CreateTables(connection, options); V1_0::ManifestTable::AddColumn(connection, { HashVirtualTable::ValueName(), HashVirtualTable::SQLiteType() }); diff --git a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h index f4fda805a7..534bcfadba 100644 --- a/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h +++ b/src/AppInstallerRepositoryCore/Microsoft/Schema/ISQLiteIndex.h @@ -38,8 +38,17 @@ namespace AppInstaller::Repository::Microsoft::Schema // Gets the schema version that this index interface is built for. virtual Schema::Version GetVersion() const = 0; + // Options for creating the index. + enum class CreateOptions + { + // Standard + None, + // Enable support for passing in nullopt values to Add/UpdateManifest + SupportPathless, + }; + // Creates all of the version dependent tables within the database. - virtual void CreateTables(SQLite::Connection& connection) = 0; + virtual void CreateTables(SQLite::Connection& connection, CreateOptions options) = 0; // Adds the manifest at the repository relative path to the index. virtual SQLite::rowid_t AddManifest(SQLite::Connection& connection, const Manifest::Manifest& manifest, const std::optional& relativePath) = 0; @@ -95,4 +104,6 @@ namespace AppInstaller::Repository::Microsoft::Schema // Largely a utility function; should not be used to do work on behalf of the index by the caller. virtual Utility::NormalizedName NormalizeName(std::string_view name, std::string_view publisher) const = 0; }; + + DEFINE_ENUM_FLAG_OPERATORS(ISQLiteIndex::CreateOptions); } diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index ad1adf5d16..9b99f128df 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -65,7 +65,7 @@ namespace AppInstaller::Repository if (!std::filesystem::exists(trackingDB)) { std::filesystem::create_directories(trackingDB.parent_path()); - SQLiteIndex::CreateNew(trackingDB.u8string()); + SQLiteIndex::CreateNew(trackingDB.u8string(), Schema::Version::Latest(), SQLiteIndex::CreateOptions::SupportPathless); } lock.Release(); diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp index a269bf968f..9811a30c5e 100644 --- a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp +++ b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.cpp @@ -686,21 +686,21 @@ namespace AppInstaller::Repository::SQLite::Builder return *this; } - StatementBuilder& StatementBuilder::DropIndex(std::string_view table) + StatementBuilder& StatementBuilder::DropIndex(std::string_view index) { - OutputOperationAndTable(m_stream, "DROP INDEX", table); + OutputOperationAndTable(m_stream, "DROP INDEX", index); return *this; } - StatementBuilder& StatementBuilder::DropIndex(QualifiedTable table) + StatementBuilder& StatementBuilder::DropIndex(QualifiedTable index) { - OutputOperationAndTable(m_stream, "DROP INDEX", table); + OutputOperationAndTable(m_stream, "DROP INDEX", index); return *this; } - StatementBuilder& StatementBuilder::DropIndex(std::initializer_list table) + StatementBuilder& StatementBuilder::DropIndex(std::initializer_list index) { - OutputOperationAndTable(m_stream, "DROP INDEX", table); + OutputOperationAndTable(m_stream, "DROP INDEX", index); return *this; } diff --git a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h index 4fdb7e0de2..956d25933a 100644 --- a/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h +++ b/src/AppInstallerRepositoryCore/SQLiteStatementBuilder.h @@ -85,6 +85,9 @@ namespace AppInstaller::Repository::SQLite::Builder // The sqlite_schema type value for a table. constexpr std::string_view Type_Table = "table"sv; + // The sqlite_schema type value for an index. + constexpr std::string_view Type_Index = "index"sv; + // The sqlite_schema column name for the name of the object. constexpr std::string_view NameColumn = "name"sv; } @@ -358,9 +361,9 @@ namespace AppInstaller::Repository::SQLite::Builder // Begin an index deletion statement. // The initializer_list form enables the table name to be constructed from multiple parts. - StatementBuilder& DropIndex(std::string_view table); - StatementBuilder& DropIndex(QualifiedTable table); - StatementBuilder& DropIndex(std::initializer_list table); + StatementBuilder& DropIndex(std::string_view index); + StatementBuilder& DropIndex(QualifiedTable index); + StatementBuilder& DropIndex(std::initializer_list index); // Set index target table. StatementBuilder& On(std::string_view table); From 84e23b0a6e2ac129907c712045cb31c78b8edd99 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 19 Oct 2021 16:22:04 -0700 Subject: [PATCH 6/8] Add tests --- .../AppInstallerCLITests.vcxproj | 1 + .../AppInstallerCLITests.vcxproj.filters | 3 + .../PackageTrackingCatalog.cpp | 174 ++++++++++++++++++ src/AppInstallerCLITests/SQLiteIndex.cpp | 134 ++++++++++++++ .../SQLiteIndexSource.cpp | 2 +- .../PackageTrackingCatalog.cpp | 20 ++ .../Public/winget/PackageTrackingCatalog.h | 5 + 7 files changed, 338 insertions(+), 1 deletion(-) create mode 100644 src/AppInstallerCLITests/PackageTrackingCatalog.cpp diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj index 3057c3d1e9..c73cb2557f 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj @@ -198,6 +198,7 @@ + diff --git a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters index 3cbc648c79..532577b83b 100644 --- a/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters +++ b/src/AppInstallerCLITests/AppInstallerCLITests.vcxproj.filters @@ -176,6 +176,9 @@ Source Files + + Source Files + diff --git a/src/AppInstallerCLITests/PackageTrackingCatalog.cpp b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp new file mode 100644 index 0000000000..2d8c301fc4 --- /dev/null +++ b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp @@ -0,0 +1,174 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +#include "pch.h" +#include "TestCommon.h" +#include +#include +#include + +using namespace std::string_literals; +using namespace TestCommon; +using namespace AppInstaller::Manifest; +using namespace AppInstaller::Repository; +using namespace AppInstaller::Repository::Microsoft; +using namespace AppInstaller::Repository::SQLite; +using namespace AppInstaller::Utility; + +static std::shared_ptr SimpleTestSetup(const std::string& filePath, SourceDetails& details, Manifest& manifest, std::string& relativePath) +{ + SQLiteIndex index = SQLiteIndex::CreateNew(filePath, Schema::Version::Latest()); + + TestDataFile testManifest("Manifest-Good.yaml"); + manifest = YamlParser::CreateFromPath(testManifest); + + relativePath = testManifest.GetPath().filename().u8string(); + + index.AddManifest(manifest, relativePath); + + details.Name = "TestName"; + details.Type = "TestType"; + details.Arg = testManifest.GetPath().parent_path().u8string(); + details.Data = ""; + + auto result = std::make_shared(details, "*SimpleTestSetup", std::move(index)); + + PackageTrackingCatalog::RemoveForSource(result->GetIdentifier()); + + return result; +} + +TEST_CASE("TrackingCatalog_Create", "[trackingcatalog]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); +} + +TEST_CASE("TrackingCatalog_Install", "[trackingcatalog]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); + + SearchRequest request; + request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id); + + SearchResult resultBefore = catalog.Search(request); + REQUIRE(resultBefore.Matches.size() == 0); + + catalog.RecordInstall(manifest, manifest.Installers[0], false); + + SearchResult resultAfter = catalog.Search(request); + REQUIRE(resultAfter.Matches.size() == 1); + + auto trackingVersion = resultAfter.Matches[0].Package->GetLatestAvailableVersion(); + REQUIRE(trackingVersion); + + auto metadata = trackingVersion->GetMetadata(); + REQUIRE(metadata.find(PackageVersionMetadata::TrackingWriteTime) != metadata.end()); +} + +TEST_CASE("TrackingCatalog_Reinstall", "[trackingcatalog]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); + + SearchRequest request; + request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id); + + catalog.RecordInstall(manifest, manifest.Installers[0], false); + + SearchResult resultBefore = catalog.Search(request); + REQUIRE(resultBefore.Matches.size() == 1); + REQUIRE(resultBefore.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Name) == + manifest.DefaultLocalization.Get()); + + // Change name + std::string newName = "New Package Name"; + manifest.DefaultLocalization.Add(newName); + + catalog.RecordInstall(manifest, manifest.Installers[0], false); + + SearchResult resultAfter = catalog.Search(request); + REQUIRE(resultAfter.Matches.size() == 1); + REQUIRE(resultBefore.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Name) == + newName); +} + +TEST_CASE("TrackingCatalog_Upgrade", "[trackingcatalog]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); + + SearchRequest request; + request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id); + + catalog.RecordInstall(manifest, manifest.Installers[0], false); + + SearchResult resultBefore = catalog.Search(request); + REQUIRE(resultBefore.Matches.size() == 1); + REQUIRE(resultBefore.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Version) == + manifest.Version); + + // Change name + manifest.Version = "99.1.2.3"; + + catalog.RecordInstall(manifest, manifest.Installers[0], true); + + SearchResult resultAfter = catalog.Search(request); + REQUIRE(resultAfter.Matches.size() == 1); + REQUIRE(resultBefore.Matches[0].Package->GetLatestAvailableVersion()->GetProperty(PackageVersionProperty::Version) == + manifest.Version); +} + +TEST_CASE("TrackingCatalog_Uninstall", "[trackingcatalog]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + SourceDetails details; + Manifest manifest; + std::string relativePath; + std::shared_ptr source = SimpleTestSetup(tempFile, details, manifest, relativePath); + + PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); + + SearchRequest request; + request.Filters.emplace_back(PackageMatchField::Id, MatchType::Exact, manifest.Id); + + catalog.RecordInstall(manifest, manifest.Installers[0], false); + + SearchResult resultBefore = catalog.Search(request); + REQUIRE(resultBefore.Matches.size() == 1); + + catalog.RecordUninstall(LocIndString{ manifest.Id }); + + SearchResult resultAfter = catalog.Search(request); + REQUIRE(resultAfter.Matches.size() == 0); +} diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp index 3f45451854..1d1047c07a 100644 --- a/src/AppInstallerCLITests/SQLiteIndex.cpp +++ b/src/AppInstallerCLITests/SQLiteIndex.cpp @@ -90,6 +90,24 @@ SQLiteIndex SimpleTestSetup(const std::string& filePath, Manifest& manifest, std return index; } +SQLiteIndex SimpleTestSetup(const std::string& filePath, Manifest& manifest, std::optional version = {}) +{ + SQLiteIndex index = CreateTestIndex(filePath, version); + + manifest.Installers.push_back({}); + manifest.Id = "Test.Id"; + manifest.DefaultLocalization.Add("Test Name"); + manifest.Moniker = "testmoniker"; + manifest.Version = "1.0.0"; + manifest.Channel = "test"; + manifest.DefaultLocalization.Add({ "t1", "t2" }); + manifest.Installers[0].Commands = { "test1", "test2" }; + + index.AddManifest(manifest); + + return index; +} + struct IndexFields { IndexFields( @@ -717,6 +735,98 @@ TEST_CASE("SQLiteIndex_UpdateManifestChangePath", "[sqliteindex][V1_0]") REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection)); } +TEST_CASE("SQLiteIndex_UpdateManifest_Pathless", "[sqliteindex][V1_0]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + Manifest manifest; + manifest.Installers.push_back({}); + manifest.Id = "test.id"; + manifest.DefaultLocalization.Add < Localization::PackageName>("Test Name"); + manifest.Moniker = "testmoniker"; + manifest.Version = "1.0.0"; + manifest.Channel = "test"; + manifest.DefaultLocalization.Add({ "t1", "t2" }); + manifest.Installers[0].Commands = { "test1", "test2" }; + + { + SQLiteIndex index = SQLiteIndex::CreateNew(tempFile, { 1, 0 }); + + index.AddManifest(manifest); + } + + { + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(!Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::PathPartTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::TagsTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::CommandsTable::IsEmpty(connection)); + } + + { + SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite); + + // Update with no updates should return false + REQUIRE(!index.UpdateManifest(manifest)); + + manifest.DefaultLocalization.Add("description2"); + + // Update with no indexed updates should return false + REQUIRE(!index.UpdateManifest(manifest)); + + // Update with indexed changes + manifest.DefaultLocalization.Add("Test Name2"); + manifest.Moniker = "testmoniker2"; + manifest.DefaultLocalization.Add({ "t1", "t2", "t3" }); + manifest.Installers[0].Commands = {}; + + REQUIRE(index.UpdateManifest(manifest)); + } + + { + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(!Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::PathPartTable::IsEmpty(connection)); + REQUIRE(!Schema::V1_0::TagsTable::IsEmpty(connection)); + // The update removed all commands + REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection)); + } + + { + SQLiteIndex index = SQLiteIndex::Open(tempFile, SQLiteIndex::OpenDisposition::ReadWrite); + + // Now remove manifest2 + index.RemoveManifest(manifest); + } + + // Open it directly to directly test table state + Connection connection = Connection::Create(tempFile, Connection::OpenDisposition::ReadWrite); + + REQUIRE(Schema::V1_0::ManifestTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::IdTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::NameTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::MonikerTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::VersionTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::ChannelTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::TagsTable::IsEmpty(connection)); + REQUIRE(Schema::V1_0::CommandsTable::IsEmpty(connection)); +} + TEST_CASE("SQLiteIndex_UpdateManifestChangeCase", "[sqliteindex][V1_0]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; @@ -1092,6 +1202,30 @@ TEST_CASE("SQLiteIndex_PathString", "[sqliteindex]") REQUIRE(latestResult == relativePath); } +TEST_CASE("SQLiteIndex_PathlessString", "[sqliteindex]") +{ + TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; + INFO("Using temporary file named: " << tempFile.GetPath()); + + Manifest manifest; + std::string relativePath; + SQLiteIndex index = SimpleTestSetup(tempFile, manifest); + + TestPrepareForRead(index); + + SearchRequest request; + request.Query = RequestMatch(MatchType::Exact, manifest.Id); + + auto results = index.Search(request); + REQUIRE(results.Matches.size() == 1); + + auto specificResult = GetPathStringByKey(index, results.Matches[0].first, manifest.Version, manifest.Channel); + REQUIRE(specificResult == relativePath); + + auto latestResult = GetPathStringByKey(index, results.Matches[0].first, "", manifest.Channel); + REQUIRE(latestResult == relativePath); +} + TEST_CASE("SQLiteIndex_Versions", "[sqliteindex]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; diff --git a/src/AppInstallerCLITests/SQLiteIndexSource.cpp b/src/AppInstallerCLITests/SQLiteIndexSource.cpp index adfbe812bc..fe699508a8 100644 --- a/src/AppInstallerCLITests/SQLiteIndexSource.cpp +++ b/src/AppInstallerCLITests/SQLiteIndexSource.cpp @@ -12,7 +12,7 @@ using namespace AppInstaller::Repository; using namespace AppInstaller::Repository::Microsoft; using namespace AppInstaller::Repository::SQLite; -std::shared_ptr SimpleTestSetup(const std::string& filePath, SourceDetails& details, Manifest& manifest, std::string& relativePath) +static std::shared_ptr SimpleTestSetup(const std::string& filePath, SourceDetails& details, Manifest& manifest, std::string& relativePath) { SQLiteIndex index = SQLiteIndex::CreateNew(filePath, Schema::Version::Latest()); diff --git a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp index 9b99f128df..9ae4410dbb 100644 --- a/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp +++ b/src/AppInstallerRepositoryCore/PackageTrackingCatalog.cpp @@ -90,6 +90,26 @@ namespace AppInstaller::Repository return result; } + void PackageTrackingCatalog::RemoveForSource(const std::string& identifier) + { + if (identifier.empty()) + { + THROW_HR(E_INVALIDARG); + } + + std::string pathName = Utility::MakeSuitablePathPart(identifier); + + std::string lockName = CreateNameForCPRWL(pathName); + auto lock = Synchronization::CrossProcessReaderWriteLock::LockExclusive(lockName); + + std::filesystem::path trackingDB = GetPackageTrackingFilePath(pathName); + + if (std::filesystem::exists(trackingDB)) + { + std::filesystem::remove(trackingDB); + } + } + SearchResult PackageTrackingCatalog::Search(const SearchRequest& request) const { return m_implementation->Source->Search(request); diff --git a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h index 7d8c201053..f15c1ec876 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h +++ b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h @@ -23,7 +23,12 @@ namespace AppInstaller::Repository // TODO: Make creation exclusive to the refactored Source type. static PackageTrackingCatalog CreateForSource(const std::shared_ptr& source); + // Removes the package tracking catalog for a given source. + static void RemoveForSource(const std::string& identifier); + // Execute a search against the catalog. + // Note that the pacakges in the results have the versions under "available" in order to + // expose all versions contained therein (in the event that this is deemed useful at some point). SearchResult Search(const SearchRequest& request) const; // Enables more granular control over the metadata in the tracking catalog if necessary. From 56c591bd77f6d17a264a2dbd8973938533b1fedb Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 19 Oct 2021 16:31:02 -0700 Subject: [PATCH 7/8] Spelling fixes --- src/AppInstallerCLITests/PackageTrackingCatalog.cpp | 10 +++++----- .../Public/winget/PackageTrackingCatalog.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/AppInstallerCLITests/PackageTrackingCatalog.cpp b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp index 2d8c301fc4..d7772b8fb0 100644 --- a/src/AppInstallerCLITests/PackageTrackingCatalog.cpp +++ b/src/AppInstallerCLITests/PackageTrackingCatalog.cpp @@ -37,7 +37,7 @@ static std::shared_ptr SimpleTestSetup(const std::string& fil return result; } -TEST_CASE("TrackingCatalog_Create", "[trackingcatalog]") +TEST_CASE("TrackingCatalog_Create", "[tracking_catalog]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); @@ -50,7 +50,7 @@ TEST_CASE("TrackingCatalog_Create", "[trackingcatalog]") PackageTrackingCatalog catalog = PackageTrackingCatalog::CreateForSource(source); } -TEST_CASE("TrackingCatalog_Install", "[trackingcatalog]") +TEST_CASE("TrackingCatalog_Install", "[tracking_catalog]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); @@ -80,7 +80,7 @@ TEST_CASE("TrackingCatalog_Install", "[trackingcatalog]") REQUIRE(metadata.find(PackageVersionMetadata::TrackingWriteTime) != metadata.end()); } -TEST_CASE("TrackingCatalog_Reinstall", "[trackingcatalog]") +TEST_CASE("TrackingCatalog_Reinstall", "[tracking_catalog]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); @@ -114,7 +114,7 @@ TEST_CASE("TrackingCatalog_Reinstall", "[trackingcatalog]") newName); } -TEST_CASE("TrackingCatalog_Upgrade", "[trackingcatalog]") +TEST_CASE("TrackingCatalog_Upgrade", "[tracking_catalog]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); @@ -147,7 +147,7 @@ TEST_CASE("TrackingCatalog_Upgrade", "[trackingcatalog]") manifest.Version); } -TEST_CASE("TrackingCatalog_Uninstall", "[trackingcatalog]") +TEST_CASE("TrackingCatalog_Uninstall", "[tracking_catalog]") { TempFile tempFile{ "repolibtest_tempdb"s, ".db"s }; INFO("Using temporary file named: " << tempFile.GetPath()); diff --git a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h index f15c1ec876..f7a8ce99bf 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h +++ b/src/AppInstallerRepositoryCore/Public/winget/PackageTrackingCatalog.h @@ -27,7 +27,7 @@ namespace AppInstaller::Repository static void RemoveForSource(const std::string& identifier); // Execute a search against the catalog. - // Note that the pacakges in the results have the versions under "available" in order to + // Note that the packages in the results have the versions under "available" in order to // expose all versions contained therein (in the event that this is deemed useful at some point). SearchResult Search(const SearchRequest& request) const; From a449377fea48fd0794eedc850c73ea37fbabe8b5 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 25 Oct 2021 14:53:23 -0700 Subject: [PATCH 8/8] PR feedback; limit path length to 255 characters --- src/AppInstallerCLITests/Strings.cpp | 2 ++ .../AppInstallerStrings.cpp | 16 +++++++++++++++- .../Public/AppInstallerSHA256.h | 4 ++++ src/AppInstallerCommonCore/SHA256.cpp | 5 +++++ 4 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerCLITests/Strings.cpp b/src/AppInstallerCLITests/Strings.cpp index 19a9f80d7b..e4fce327be 100644 --- a/src/AppInstallerCLITests/Strings.cpp +++ b/src/AppInstallerCLITests/Strings.cpp @@ -3,6 +3,7 @@ #include "pch.h" #include "TestCommon.h" #include +#include #include using namespace std::string_view_literals; @@ -182,6 +183,7 @@ TEST_CASE("MakeSuitablePathPart", "[strings]") REQUIRE(MakeSuitablePathPart(u8"f*\xF6*ldcase") == u8"f_\xF6_ldcase"); REQUIRE(MakeSuitablePathPart(".") == "_"); REQUIRE(MakeSuitablePathPart("..") == "._"); + REQUIRE(MakeSuitablePathPart(std::string(300, ' ')) == SHA256::ConvertToString(SHA256::ComputeHash(std::string(300, ' ')))); REQUIRE_THROWS_HR(MakeSuitablePathPart("COM1"), E_INVALIDARG); REQUIRE_THROWS_HR(MakeSuitablePathPart("NUL.txt"), E_INVALIDARG); } diff --git a/src/AppInstallerCommonCore/AppInstallerStrings.cpp b/src/AppInstallerCommonCore/AppInstallerStrings.cpp index 9144321fc2..a4d3149bee 100644 --- a/src/AppInstallerCommonCore/AppInstallerStrings.cpp +++ b/src/AppInstallerCommonCore/AppInstallerStrings.cpp @@ -4,6 +4,7 @@ #include "Public/AppInstallerStrings.h" #include "Public/AppInstallerErrors.h" #include "Public/AppInstallerLogging.h" +#include "Public/AppInstallerSHA256.h" namespace AppInstaller::Utility { @@ -515,18 +516,22 @@ namespace AppInstaller::Utility // Follow the rules at https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file to replace // invalid characters in a candidate path part. + // Additionally, based on https://docs.microsoft.com/en-us/windows/win32/fileio/filesystem-functionality-comparison#limits + // limit the number of characters to 255. std::string MakeSuitablePathPart(std::string_view candidate) { constexpr char replaceChar = '_'; constexpr std::string_view illegalChars = R"(<>:"/\|?*)"; + constexpr size_t pathLengthLimit = 255; // First, walk the string and replace illegal characters std::string result; result.reserve(candidate.size()); ICUBreakIterator itr{ candidate, UBRK_CHARACTER }; + size_t resultBreakCount = 0; - while (itr.CurrentBreak() != UBRK_DONE && itr.CurrentOffset() < candidate.size()) + while (itr.CurrentBreak() != UBRK_DONE && itr.CurrentOffset() < candidate.size() && resultBreakCount <= pathLengthLimit) { UChar32 current = itr.CurrentCodePoint(); bool isIllegal = current < 32 || (current < 256 && illegalChars.find(static_cast(current)) != std::string::npos); @@ -552,6 +557,15 @@ namespace AppInstaller::Utility size_t count = (nextOffset == UBRK_DONE ? std::string::npos : static_cast(nextOffset) - static_cast(offset)); result.append(candidate.substr(static_cast(offset), count)); } + + ++resultBreakCount; + } + + // If there are too many characters for a single path; switch to a hash. + // This should basically never happen, but if it does it will prevent collisions better. + if (resultBreakCount > pathLengthLimit) + { + return SHA256::ConvertToString(SHA256::ComputeHash(candidate)); } // Second, look for any newly formed illegal names. diff --git a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h index 45a65044df..de61d94030 100644 --- a/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h +++ b/src/AppInstallerCommonCore/Public/AppInstallerSHA256.h @@ -6,6 +6,7 @@ #include #include #include +#include namespace AppInstaller::Utility { @@ -46,6 +47,9 @@ namespace AppInstaller::Utility { // Computes the hash of the given buffer immediately. static HashBuffer ComputeHash(const uint8_t* buffer, std::uint32_t cbBuffer); + // Computes the hash of the given string immediately. + static HashBuffer ComputeHash(std::string_view buffer); + // Computes the hash from a given stream. static HashBuffer ComputeHash(std::istream& in); diff --git a/src/AppInstallerCommonCore/SHA256.cpp b/src/AppInstallerCommonCore/SHA256.cpp index 9b97ec9399..f13eccfab0 100644 --- a/src/AppInstallerCommonCore/SHA256.cpp +++ b/src/AppInstallerCommonCore/SHA256.cpp @@ -135,6 +135,11 @@ namespace AppInstaller::Utility { return hasher.Get(); } + SHA256::HashBuffer SHA256::ComputeHash(std::string_view buffer) + { + return ComputeHash(reinterpret_cast(buffer.data()), static_cast(buffer.size())); + } + SHA256::HashBuffer SHA256::ComputeHash(std::istream& in) { // Throw exceptions on badbit