commit afe95d94c15ee4d025a982f2db7dd0a7dc8e38c0
parent 9ce12c713b8a6bc06ffa364f9fdfef095c10f45e
Author: JohnMcPMS <johnmcp@microsoft.com>
Date: Wed, 6 May 2020 23:30:23 -0700
Add default source functionality and tests (#107)
Completes the functionality previously hinted at with the default source function. This is done by initializing the source setting when it does not exist. We initialize it with an uninitialized source, which will automatically be initialized when it is opened. If the source is removed or reset by name, the source list will be empty. If the all sources are reset, thus deleting the source setting, then the next attempt to get the sources will result in reinitialization from the defaults.
Also adds another field to the source details, IsDefault. This will give us options in the future to take action on these default sources, such as updating the URL if necessary. We can write code to transform a source that matches on name and IsDefault being true without any impact to the users.
Diffstat:
8 files changed, 151 insertions(+), 83 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/SourceCommand.cpp b/src/AppInstallerCLICore/Commands/SourceCommand.cpp
@@ -165,8 +165,7 @@ namespace AppInstaller::CLI
{
context <<
Workflow::QueryUserForSourceReset <<
- Workflow::ResetAllSources <<
- Workflow::AddDefaultSources;
+ Workflow::ResetAllSources;
}
}
}
diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.cpp b/src/AppInstallerCLICore/Workflows/SourceFlow.cpp
@@ -211,10 +211,4 @@ namespace AppInstaller::CLI::Workflow
Repository::DropSource({});
context.Reporter.Info() << " Done." << std::endl;
}
-
- void AddDefaultSources(Execution::Context& context)
- {
- context.Reporter.Info() << "Adding default sources ..." << std::endl;
- context.Reporter.ExecuteWithProgress(Repository::AddDefaultSources);
- }
}
diff --git a/src/AppInstallerCLICore/Workflows/SourceFlow.h b/src/AppInstallerCLICore/Workflows/SourceFlow.h
@@ -47,27 +47,21 @@ namespace AppInstaller::CLI::Workflow
// Outputs: None
void RemoveSources(Execution::Context& context);
- // Removes the sources in SourceList.
+ // Asks the user if they are ok with dropping the sources in SourceList.
// Required Args: None
- // Inputs: None
+ // Inputs: SourceList
// Outputs: None
void QueryUserForSourceReset(Execution::Context& context);
- // Removes the sources in SourceList.
+ // Drops the sources in SourceList.
// Required Args: None
// Inputs: SourceList
// Outputs: None
void ResetSourceList(Execution::Context& context);
- // Removes the sources in SourceList.
+ // Drops all sources.
// Required Args: None
// Inputs: None
// Outputs: None
void ResetAllSources(Execution::Context& context);
-
- // Removes the sources in SourceList.
- // Required Args: None
- // Inputs: None
- // Outputs: None
- void AddDefaultSources(Execution::Context& context);
}
diff --git a/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp b/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp
@@ -94,7 +94,7 @@ namespace AppInstaller::CLI::Workflow
else
{
// Even if a name was given, there are no sources
- context.Reporter.Error() << "No sources defined; add one with 'source add'" << std::endl;
+ context.Reporter.Error() << "No sources defined; add one with 'source add' or reset to defaults with 'source reset'" << std::endl;
AICLI_TERMINATE_CONTEXT(APPINSTALLER_CLI_ERROR_NO_SOURCES_DEFINED);
}
}
diff --git a/src/AppInstallerCLIE2ETests/SourceCommand.cs b/src/AppInstallerCLIE2ETests/SourceCommand.cs
@@ -74,13 +74,6 @@ namespace AppInstallerCLIE2ETests
result = TestCommon.RunAICLICommand("source remove", $"-n {SourceTestSourceName}");
Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
Assert.True(result.StdOut.Contains("Done"));
-
- TestCommon.WaitForDeploymentFinish();
-
- // List should show no source available
- result = TestCommon.RunAICLICommand("source list", "");
- Assert.AreEqual(Constants.ErrorCode.S_OK, result.ExitCode);
- Assert.True(result.StdOut.Contains("There are no sources configured."));
}
}
}
\ No newline at end of file
diff --git a/src/AppInstallerCLITests/Sources.cpp b/src/AppInstallerCLITests/Sources.cpp
@@ -46,11 +46,13 @@ Sources:
Arg: testArg
Data: testData
LastUpdate: 0
+ IsDefault: 1
- Name: testName2
Type: testType2
Arg: testArg2
Data: testData2
LastUpdate: 1
+ IsDefault: 0
- Name: testName3
Type: testType3
Arg: testArg3
@@ -143,7 +145,9 @@ TEST_CASE("RepoSources_UserSettingDoesNotExist", "[sources]")
RemoveSetting(s_RepositorySettings_UserSources);
std::vector<SourceDetails> sources = GetSources();
- REQUIRE(sources.empty());
+ // The default source is added when no source exists
+ REQUIRE(sources.size() == 1);
+ REQUIRE(sources[0].IsDefault);
}
TEST_CASE("RepoSources_EmptySourcesList", "[sources]")
@@ -185,6 +189,7 @@ TEST_CASE("RepoSources_ThreeSources", "[sources]")
REQUIRE(sources[i].Arg == "testArg"s + suffix[i]);
REQUIRE(sources[i].Data == "testData"s + suffix[i]);
REQUIRE(sources[i].LastUpdateTime == ConvertUnixEpochToSystemClock(i));
+ REQUIRE(sources[i].IsDefault == (i == 0));
}
}
@@ -204,7 +209,7 @@ TEST_CASE("RepoSources_MissingField", "[sources]")
TEST_CASE("RepoSources_AddSource", "[sources]")
{
- RemoveSetting(s_RepositorySettings_UserSources);
+ SetSetting(s_RepositorySettings_UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();
std::string name = "thisIsTheName";
@@ -234,7 +239,7 @@ TEST_CASE("RepoSources_AddSource", "[sources]")
TEST_CASE("RepoSources_AddMultipleSources", "[sources]")
{
- RemoveSetting(s_RepositorySettings_UserSources);
+ SetSetting(s_RepositorySettings_UserSources, s_EmptySources);
std::string name = "thisIsTheName";
std::string type = "thisIsTheType";
@@ -283,7 +288,7 @@ TEST_CASE("RepoSources_UpdateSource", "[sources]")
{
using namespace std::chrono_literals;
- RemoveSetting(s_RepositorySettings_UserSources);
+ SetSetting(s_RepositorySettings_UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();
std::string name = "thisIsTheName";
@@ -333,7 +338,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]")
{
using namespace std::chrono_literals;
- RemoveSetting(s_RepositorySettings_UserSources);
+ SetSetting(s_RepositorySettings_UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();
std::string name = "thisIsTheName";
@@ -368,7 +373,7 @@ TEST_CASE("RepoSources_UpdateSourceRetries", "[sources]")
TEST_CASE("RepoSources_RemoveSource", "[sources]")
{
- RemoveSetting(s_RepositorySettings_UserSources);
+ SetSetting(s_RepositorySettings_UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();
std::string name = "thisIsTheName";
@@ -399,7 +404,6 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]")
{
using namespace std::chrono_literals;
- RemoveSetting(s_RepositorySettings_UserSources);
TestHook_ClearSourceFactoryOverrides();
std::string name = "testName";
@@ -429,3 +433,43 @@ TEST_CASE("RepoSources_UpdateOnOpen", "[sources]")
REQUIRE(sources[0].Data == data);
REQUIRE(sources[0].LastUpdateTime == ConvertUnixEpochToSystemClock(0));
}
+
+TEST_CASE("RepoSources_DropSourceByName", "[sources]")
+{
+ SetSetting(s_RepositorySettings_UserSources, s_ThreeSources);
+
+ std::vector<SourceDetails> sources = GetSources();
+ REQUIRE(sources.size() == 3);
+
+ DropSource("testName");
+
+ sources = GetSources();
+ REQUIRE(sources.size() == 2);
+
+ const char* suffix[2] = { "2", "3" };
+
+ for (size_t i = 0; i < 2; ++i)
+ {
+ INFO("Source #" << i);
+ REQUIRE(sources[i].Name == "testName"s + suffix[i]);
+ REQUIRE(sources[i].Type == "testType"s + suffix[i]);
+ REQUIRE(sources[i].Arg == "testArg"s + suffix[i]);
+ REQUIRE(sources[i].Data == "testData"s + suffix[i]);
+ REQUIRE(sources[i].LastUpdateTime == ConvertUnixEpochToSystemClock(i + 1));
+ REQUIRE(!sources[i].IsDefault);
+ }
+}
+
+TEST_CASE("RepoSources_DropAllSources", "[sources]")
+{
+ SetSetting(s_RepositorySettings_UserSources, s_ThreeSources);
+
+ std::vector<SourceDetails> sources = GetSources();
+ REQUIRE(sources.size() == 3);
+
+ DropSource({});
+
+ sources = GetSources();
+ REQUIRE(sources.size() == 1);
+ REQUIRE(sources[0].IsDefault);
+}
diff --git a/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h b/src/AppInstallerRepositoryCore/Public/AppInstallerRepositorySource.h
@@ -31,6 +31,9 @@ namespace AppInstaller::Repository
// The last time that this source was updated.
std::chrono::system_clock::time_point LastUpdateTime;
+
+ // This source is a default source; added for the user by the tool.
+ bool IsDefault = false;
};
// Interface for interacting with a source from outside of the repository lib.
@@ -54,9 +57,6 @@ namespace AppInstaller::Repository
// Adds a new source for the user.
void AddSource(std::string name, std::string type, std::string arg, IProgressCallback& progress);
- // Adds the default sources.
- void AddDefaultSources(IProgressCallback& progress);
-
// Opens an existing source.
// Passing an empty string as the name of the source will return a source that aggregates all others.
std::shared_ptr<ISource> OpenSource(std::string_view name, IProgressCallback& progress);
diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp
@@ -18,18 +18,25 @@ namespace AppInstaller::Repository
constexpr std::string_view s_SourcesYaml_Source_Arg = "Arg"sv;
constexpr std::string_view s_SourcesYaml_Source_Data = "Data"sv;
constexpr std::string_view s_SourcesYaml_Source_LastUpdate = "LastUpdate"sv;
+ constexpr std::string_view s_SourcesYaml_Source_IsDefault = "IsDefault"sv;
+
+ constexpr std::string_view s_Source_WingetCommunityDefault_Name = "winget"sv;
+ constexpr std::string_view s_Source_WingetCommunityDefault_Arg = "https://winget.azureedge.net/cache"sv;
namespace
{
// Attempts to read a single scalar value from the node.
template<typename Value>
- bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value)
+ bool TryReadScalar(std::string_view settingName, const std::string& settingValue, const YAML::Node& sourceNode, std::string_view name, Value& value, bool required = true)
{
YAML::Node valueNode = sourceNode[std::string{ name }];
if (!valueNode || !valueNode.IsScalar())
{
- AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << name << " is invalid within a source):\n" << settingValue);
+ if (required)
+ {
+ AICLI_LOG(Repo, Error, << "Setting '" << settingName << "' did not contain the expected format (" << name << " is invalid within a source):\n" << settingValue);
+ }
return false;
}
@@ -88,6 +95,16 @@ namespace AppInstaller::Repository
int64_t lastUpdateInEpoch{};
if (!TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_LastUpdate, lastUpdateInEpoch)) { return false; }
details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(lastUpdateInEpoch);
+ int32_t isDefaultNumber{};
+ if (TryReadScalar(settingName, settingValue, source, s_SourcesYaml_Source_IsDefault, isDefaultNumber, false))
+ {
+ details.IsDefault = (isDefaultNumber != 0);
+ }
+ else
+ {
+ // If older than defaults, assume it is not one.
+ details.IsDefault = false;
+ }
result.emplace_back(std::move(details));
}
@@ -126,16 +143,6 @@ namespace AppInstaller::Repository
return TryGetSourcesFromSetting(settingName).value_or(std::vector<SourceDetails>{});
}
- // If there is no setting value at all, adds the default sources.
- void AddDefaultSourcesIfNeeded(IProgressCallback& progress)
- {
- auto sourcesStream = Runtime::GetSettingStream(s_RepositorySettings_UserSources);
- if (!sourcesStream)
- {
- AddDefaultSources(progress);
- }
- }
-
// Make up for the lack of string_view support in YAML CPP.
YAML::Emitter& operator<<(YAML::Emitter& out, std::string_view sv)
{
@@ -158,6 +165,7 @@ namespace AppInstaller::Repository
out << YAML::Key << s_SourcesYaml_Source_Arg << YAML::Value << details.Arg;
out << YAML::Key << s_SourcesYaml_Source_Data << YAML::Value << details.Data;
out << YAML::Key << s_SourcesYaml_Source_LastUpdate << YAML::Value << Utility::ConvertSystemClockToUnixEpoch(details.LastUpdateTime);
+ out << YAML::Key << s_SourcesYaml_Source_IsDefault << YAML::Value << (details.IsDefault ? 1 : 0);
out << YAML::EndMap;
}
@@ -264,10 +272,77 @@ namespace AppInstaller::Repository
return false;
}
+
+ SourceDetails PrepareSourceDetailsForAdd(std::string name, std::string type, std::string arg, bool isDefault)
+ {
+ THROW_HR_IF(E_INVALIDARG, name.empty());
+
+ AICLI_LOG(Repo, Info, << "Adding source: Name[" << name << "], Type[" << type << "], Arg[" << arg << "]");
+
+ // Check all sources for the given name.
+ std::vector<SourceDetails> currentSources = GetSources();
+
+ auto itr = FindSourceByName(currentSources, name);
+ THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, itr != currentSources.end());
+
+ SourceDetails details;
+ details.Name = std::move(name);
+ details.Type = std::move(type);
+ details.Arg = std::move(arg);
+ details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0);
+ details.IsDefault = isDefault;
+
+ return details;
+ }
+
+ void AddDetailsToSetting(const SourceDetails& details, std::string_view setting)
+ {
+ AICLI_LOG(Repo, Info, << "Source created with extra data: " << details.Data);
+
+ std::vector<SourceDetails> currentSources = GetSourcesFromSetting(setting);
+ currentSources.emplace_back(details);
+
+ SetSourcesToSetting(setting, currentSources);
+ }
+
+ void AddSourceInternal(std::string name, std::string type, std::string arg, bool isDefault, IProgressCallback& progress)
+ {
+ SourceDetails details = PrepareSourceDetailsForAdd(name, type, arg, isDefault);
+
+ UpdateSourceFromDetails(details, progress);
+
+ AddDetailsToSetting(details, s_RepositorySettings_UserSources);
+ }
+
+ void AddUninitializedSourceInternal(std::string name, std::string type, std::string arg, bool isDefault)
+ {
+ SourceDetails details = PrepareSourceDetailsForAdd(name, type, arg, isDefault);
+ AddDetailsToSetting(details, s_RepositorySettings_UserSources);
+ }
+
+ // If there is no setting value at all, adds the default sources.
+ void AddDefaultSourcesIfNeeded()
+ {
+ auto sourcesStream = Runtime::GetSettingStream(s_RepositorySettings_UserSources);
+ if (!sourcesStream)
+ {
+ // We have to set an initial, empty list of sources or the add will create an infinite loop.
+ SetSourcesToSetting(s_RepositorySettings_UserSources, std::vector<SourceDetails>{});
+
+ AddUninitializedSourceInternal(
+ std::string(s_Source_WingetCommunityDefault_Name),
+ std::string(Microsoft::PreIndexedPackageSourceFactory::Type()),
+ std::string(s_Source_WingetCommunityDefault_Arg),
+ true);
+ }
+ }
}
+ // TODO: If we merge sources from multiple settings in the future, the other functions
+ // in this file all need to be enlightened with how to write to othe appropriate location.
std::vector<SourceDetails> GetSources()
{
+ AddDefaultSourcesIfNeeded();
return GetSourcesFromSetting(s_RepositorySettings_UserSources);
}
@@ -289,42 +364,11 @@ namespace AppInstaller::Repository
void AddSource(std::string name, std::string type, std::string arg, IProgressCallback& progress)
{
- THROW_HR_IF(E_INVALIDARG, name.empty());
-
- AICLI_LOG(Repo, Info, << "Adding source: Name[" << name << "], Type[" << type << "], Arg[" << arg << "]");
-
- // Check all sources for the given name.
- std::vector<SourceDetails> currentSources = GetSources();
-
- auto itr = FindSourceByName(currentSources, name);
- THROW_HR_IF(APPINSTALLER_CLI_ERROR_SOURCE_NAME_ALREADY_EXISTS, itr != currentSources.end());
-
- SourceDetails details;
- details.Name = std::move(name);
- details.Type = std::move(type);
- details.Arg = std::move(arg);
- details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0);
-
- UpdateSourceFromDetails(details, progress);
-
- AICLI_LOG(Repo, Info, << "Source created with extra data: " << details.Data);
-
- currentSources = GetSourcesFromSetting(s_RepositorySettings_UserSources);
- currentSources.emplace_back(details);
-
- SetSourcesToSetting(s_RepositorySettings_UserSources, currentSources);
- }
-
- void AddDefaultSources(IProgressCallback& progress)
- {
- UNREFERENCED_PARAMETER(progress);
- // TODO: Create list of default sources
+ AddSourceInternal(name, type, arg, false, progress);
}
std::shared_ptr<ISource> OpenSource(std::string_view name, IProgressCallback& progress)
{
- AddDefaultSourcesIfNeeded(progress);
-
std::vector<SourceDetails> currentSources = GetSources();
if (name.empty())
@@ -367,7 +411,7 @@ namespace AppInstaller::Repository
{
THROW_HR_IF(E_INVALIDARG, name.empty());
- std::vector<SourceDetails> currentSources = GetSourcesFromSetting(s_RepositorySettings_UserSources);
+ std::vector<SourceDetails> currentSources = GetSources();
auto itr = FindSourceByName(currentSources, name);
if (itr == currentSources.end())
@@ -389,7 +433,7 @@ namespace AppInstaller::Repository
{
THROW_HR_IF(E_INVALIDARG, name.empty());
- std::vector<SourceDetails> currentSources = GetSourcesFromSetting(s_RepositorySettings_UserSources);
+ std::vector<SourceDetails> currentSources = GetSources();
auto itr = FindSourceByName(currentSources, name);
if (itr == currentSources.end())
@@ -418,7 +462,7 @@ namespace AppInstaller::Repository
}
else
{
- std::vector<SourceDetails> currentSources = GetSourcesFromSetting(s_RepositorySettings_UserSources);
+ std::vector<SourceDetails> currentSources = GetSources();
auto itr = FindSourceByName(currentSources, name);
if (itr == currentSources.end())
