diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index 53175f0afa..2c6906108d 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -584,6 +584,7 @@ Tlg tlhelp TLSCAs tombstoned +tombstoning Toolhelp transitioning trimstart diff --git a/doc/ReleaseNotes.md b/doc/ReleaseNotes.md index ad5dd4dc9a..e91ce5cb4e 100644 --- a/doc/ReleaseNotes.md +++ b/doc/ReleaseNotes.md @@ -81,3 +81,4 @@ Added a user setting (`logging.fileNameStrategy`) for controlling the default na * File and directory paths passed to `signtool.exe` and `makeappx.exe` are now quoted, fixing failures when paths contain spaces. * DSC export now correctly exports WinGet Admin Settings * `winget validate` now performs case-insensitive comparison for file extensions where applicable +* `winget source reset` now properly resets default sources instead of removing them diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp index f98ff24910..a4a7b4654e 100644 --- a/src/AppInstallerCLICore/Commands/SourceCommand.cpp +++ b/src/AppInstallerCLICore/Commands/SourceCommand.cpp @@ -283,8 +283,7 @@ namespace AppInstaller::CLI { context << Workflow::EnsureRunningAsAdmin << - Workflow::GetSourceListWithFilter << - Workflow::ResetSourceList; + Workflow::ResetNamedSource; } else { diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp index 54f71d2522..4e0e67f86b 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp @@ -366,6 +366,20 @@ namespace AppInstaller::CLI::Workflow } } + void ResetNamedSource(Execution::Context& context) + { + std::string_view name = context.Args.GetArg(Args::Type::SourceName); + + context.Reporter.Info() << Resource::String::SourceResetOne(Utility::LocIndView{ name }) << std::endl; + if (!Repository::Source::DropSource(name)) + { + context.Reporter.Error() << Resource::String::SourceListNoneFound(Utility::LocIndView{ name }) << std::endl; + AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_SOURCE_NAME_DOES_NOT_EXIST); + } + + context.Reporter.Info() << Resource::String::Done << std::endl; + } + void ResetAllSources(Execution::Context& context) { context.Reporter.Info() << Resource::String::SourceResetAll; diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.h b/src/AppInstallerCLICore/Workflows/SourceFlow.h index e3a92a04a8..4d771e72d4 100644 --- a/src/AppInstallerCLICore/Workflows/SourceFlow.h +++ b/src/AppInstallerCLICore/Workflows/SourceFlow.h @@ -66,6 +66,12 @@ namespace AppInstaller::CLI::Workflow // Outputs: None void ResetSourceList(Execution::Context& context); + // Drops a single source by name, including hidden tombstoned default sources. + // Required Args: SourceName + // Inputs: None + // Outputs: None + void ResetNamedSource(Execution::Context& context); + // Drops all sources. // Required Args: None // Inputs: None diff --git a/src/AppInstallerCLITests/SourceFlow.cpp b/src/AppInstallerCLITests/SourceFlow.cpp index 6581f92a82..013d53f652 100644 --- a/src/AppInstallerCLITests/SourceFlow.cpp +++ b/src/AppInstallerCLITests/SourceFlow.cpp @@ -150,3 +150,35 @@ TEST_CASE("OpenSource_WithCustomHeader", "[OpenSource][CustomHeader]") AppInstaller::CLI::Workflow::OpenSource()(context); REQUIRE(receivedCustomHeader); } + +TEST_CASE("SourceResetFlow_ByNameResetsTombstonedDefaultSource", "[SourceResetFlow][workflow]") +{ + SetSetting(Stream::UserSources, R"( +Sources: + - Name: winget + Type: "" + Arg: "" + Data: "" + IsTombstone: true +)"sv); + + std::ostringstream output; + TestContext context{ output, std::cin }; + auto previousThreadGlobals = context.SetForCurrentThread(); + context.Override({ EnsureRunningAsAdmin, [](TestContext&) {} }); + context.Args.AddArg(Execution::Args::Type::SourceName, "winget"sv); + + SourceResetCommand sourceReset({}); + sourceReset.Execute(context); + + INFO(output.str()); + REQUIRE(context.GetTerminationHR() == S_OK); + + auto sources = Source::GetCurrentSources(); + auto winget = std::find_if( + sources.begin(), + sources.end(), + [](const SourceDetails& sd) { return sd.Name == "winget"; }); + REQUIRE(winget != sources.end()); + REQUIRE(winget->Origin == SourceOrigin::Default); +} diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp index a92683ce71..7d0dfc4c80 100644 --- a/src/AppInstallerCLITests/Sources.cpp +++ b/src/AppInstallerCLITests/Sources.cpp @@ -78,6 +78,27 @@ constexpr std::string_view s_SingleSourceOverride = R"( Priority: 12 )"sv; +constexpr std::string_view s_WingetTombstoned = R"( +Sources: + - Name: winget + Type: "" + Arg: "" + Data: "" + IsTombstone: true +)"sv; + +constexpr std::string_view s_WingetDefaultMetadata = R"( +Sources: + - Name: winget + LastUpdate: 100 +)"sv; + +constexpr std::string_view s_WingetFontOverrideMetadata = R"( +Sources: + - Name: winget-font + LastUpdate: 100 +)"sv; + constexpr std::string_view s_SingleSourceMetadata = R"( Sources: - Name: testName @@ -755,6 +776,81 @@ TEST_CASE("RepoSources_DropAllSources", "[sources]") REQUIRE(sources[0].Origin == SourceOrigin::Default); } + +TEST_CASE("RepoSources_DropDefaultSourceByName", "[sources]") +{ + RemoveSetting(Stream::UserSources); + SetSetting(Stream::SourcesMetadata, s_WingetDefaultMetadata); + + std::vector sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + + // Verify the winget source has non-zero metadata before reset + auto wingetBefore = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget"; }); + REQUIRE(wingetBefore != sources.end()); + REQUIRE(wingetBefore->LastUpdateTime == ConvertUnixEpochToSystemClock(100)); + + DropSource("winget"); + + // Source should still be present as a Default source + sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + + auto wingetAfter = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget"; }); + REQUIRE(wingetAfter != sources.end()); + REQUIRE(wingetAfter->Origin == SourceOrigin::Default); + // Metadata should be cleared + REQUIRE(wingetAfter->LastUpdateTime == ConvertUnixEpochToSystemClock(0)); +} + +TEST_CASE("RepoSources_ResetTombstonedDefaultSourceByName", "[sources]") +{ + SetSetting(Stream::UserSources, s_WingetTombstoned); + SetSetting(Stream::SourcesMetadata, s_WingetDefaultMetadata); + + std::vector sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount - 1); + + auto wingetBefore = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget"; }); + REQUIRE(wingetBefore == sources.end()); + + DropSource("winget"); + + sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + + auto wingetAfter = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget"; }); + REQUIRE(wingetAfter != sources.end()); + REQUIRE(wingetAfter->Origin == SourceOrigin::Default); + REQUIRE(wingetAfter->LastUpdateTime == ConvertUnixEpochToSystemClock(0)); +} + +TEST_CASE("RepoSources_DropDefaultSourceOverrideByName", "[sources]") +{ + SetSetting(Stream::UserSources, s_SingleSourceOverride); + SetSetting(Stream::SourcesMetadata, s_WingetFontOverrideMetadata); + + std::vector sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + + // winget-font should be present as a User (override) source with metadata + auto fontBefore = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget-font"; }); + REQUIRE(fontBefore != sources.end()); + REQUIRE(fontBefore->Origin == SourceOrigin::User); + REQUIRE(fontBefore->LastUpdateTime == ConvertUnixEpochToSystemClock(100)); + + DropSource("winget-font"); + + // winget-font should be restored to Default with cleared metadata + sources = GetSources(); + REQUIRE(sources.size() == c_DefaultSourceCount); + + auto fontAfter = std::find_if(sources.begin(), sources.end(), [](const SourceDetails& sd) { return sd.Name == "winget-font"; }); + REQUIRE(fontAfter != sources.end()); + REQUIRE(fontAfter->Origin == SourceOrigin::Default); + REQUIRE(fontAfter->LastUpdateTime == ConvertUnixEpochToSystemClock(0)); +} + TEST_CASE("RepoSources_SearchAcrossMultipleSources", "[sources]") { TestHook_ClearSourceFactoryOverrides(); diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 5fdcccea0b..525bd9e1ba 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -1094,7 +1094,7 @@ namespace AppInstaller::Repository { SourceList sourceList; - auto source = sourceList.GetCurrentSource(name); + auto source = sourceList.GetSource(name); if (!source) { AICLI_LOG(Repo, Info, << "Named source to be dropped, but not found: " << name); @@ -1105,7 +1105,18 @@ namespace AppInstaller::Repository AICLI_LOG(Repo, Info, << "Named source to be dropped, found: " << source->Name); EnsureSourceIsRemovable(*source); - sourceList.RemoveSource(*source); + + // For default sources, overrides of default sources, and tombstones masking + // default sources, reset to clean state instead of tombstoning/removing. + if (source->Origin == SourceOrigin::Default || + (source->Origin == SourceOrigin::User && (source->IsOverride || source->IsTombstone))) + { + sourceList.ResetSource(*source); + } + else + { + sourceList.RemoveSource(*source); + } return true; } diff --git a/src/AppInstallerRepositoryCore/SourceList.cpp b/src/AppInstallerRepositoryCore/SourceList.cpp index 6cfd224764..02682c0155 100644 --- a/src/AppInstallerRepositoryCore/SourceList.cpp +++ b/src/AppInstallerRepositoryCore/SourceList.cpp @@ -240,6 +240,14 @@ namespace AppInstaller::Repository DoNotUpdateBefore = source.DoNotUpdateBefore; } + void SourceDetailsInternal::ResetMetadataFields() + { + LastUpdateTime = {}; + DoNotUpdateBefore = {}; + AcceptedAgreementFields = 0; + AcceptedAgreementsIdentifier = {}; + } + void SourceDetailsInternal::CopyOverrideFieldsFrom(const SourceDetails& overrideSource) { // These are the supported Override fields. @@ -661,6 +669,39 @@ namespace AppInstaller::Repository SaveMetadataInternal(details, true); } + void SourceList::ResetSource(const SourceDetailsInternal& detailsRef) + { + // Copy the incoming details because we might destroy the referenced structure + // when reloading the source details from settings. + SourceDetailsInternal details = detailsRef; + bool sourcesSet = false; + + for (size_t i = 0; !sourcesSet && i < 10; ++i) + { + // Remove any user-level entry for this source (tombstone or override). + auto itr = FindSource(details.Name, true); + if (itr != m_sourceList.end() && itr->Origin == SourceOrigin::User) + { + m_sourceList.erase(itr); + } + + sourcesSet = SetSourcesByOrigin(SourceOrigin::User, m_sourceList); + + if (!sourcesSet) + { + OverwriteSourceList(); + OverwriteMetadata(); + } + } + + THROW_HR_IF_MSG(E_UNEXPECTED, !sourcesSet, "Too many attempts at SetSourcesByOrigin"); + + // Reload to bring back the Default source (now that the user entry is removed). + OverwriteSourceList(); + + ResetMetadataInternal(details); + } + void SourceList::SaveMetadata(const SourceDetailsInternal& details) { SaveMetadataInternal(details); @@ -1006,4 +1047,25 @@ namespace AppInstaller::Repository THROW_HR_IF_MSG(E_UNEXPECTED, !metadataSet, "Too many attempts at SetMetadata"); } + + void SourceList::ResetMetadataInternal(const SourceDetailsInternal& details) + { + bool metadataSet = false; + + for (size_t i = 0; !metadataSet && i < 10; ++i) + { + // Load fresh metadata from storage so that other sources' metadata is preserved. + OverwriteMetadata(); + + auto target = FindSource(details.Name, true); + if (target != m_sourceList.end()) + { + target->ResetMetadataFields(); + } + + metadataSet = SetMetadata(m_sourceList); + } + + THROW_HR_IF_MSG(E_UNEXPECTED, !metadataSet, "Too many attempts at SetMetadata"); + } } diff --git a/src/AppInstallerRepositoryCore/SourceList.h b/src/AppInstallerRepositoryCore/SourceList.h index a2bd862169..d5f87aff40 100644 --- a/src/AppInstallerRepositoryCore/SourceList.h +++ b/src/AppInstallerRepositoryCore/SourceList.h @@ -25,6 +25,9 @@ namespace AppInstaller::Repository // Copies the metadata fields from this source. This only include partial metadata. void CopyMetadataFieldsFrom(const SourceDetails& source); + // Resets all metadata fields to their default (clean-install) state. + void ResetMetadataFields(); + // Copies the overridden fields from the target source to this source. This is only the supported override fields. void CopyOverrideFieldsFrom(const SourceDetails& overrideSource); @@ -70,6 +73,10 @@ namespace AppInstaller::Repository void RemoveSource(const SourceDetailsInternal& details); void EditSource(const SourceDetailsInternal& details); + // Reset a default source: removes any user-level entry (tombstone or override) + // and clears the source's metadata, restoring it to clean-install state. + void ResetSource(const SourceDetailsInternal& details); + // Save source metadata; the particular source with the metadata update is given. // The given source must already be in the internal source list. void SaveMetadata(const SourceDetailsInternal& details); @@ -108,6 +115,9 @@ namespace AppInstaller::Repository // If remove is true, the given source is being removed. void SaveMetadataInternal(const SourceDetailsInternal& details, bool remove = false); + // Clears all metadata for the named source and persists the change. + void ResetMetadataInternal(const SourceDetailsInternal& details); + std::vector m_sourceList; Settings::Stream m_userSourcesStream; Settings::Stream m_metadataStream;