Refactor SourceType handling to enum

src/AppInstallerCLICore/Commands/DscSourceResource.cpp

@@ -107,7 +107,7 @@ namespace AppInstaller::CLI
                     {
                         Output.Exist(true);
                         Output.Argument(source.Arg);
-                        Output.Type(source.Type);
+                        Output.Type(std::string{ ToString(source.Type) });
                         Output.TrustLevel(TrustLevelStringFromFlags(source.TrustLevel));
                         Output.Explicit(source.Explicit);
 
@@ -140,6 +140,7 @@ namespace AppInstaller::CLI
 
                 if (Input.Type())
                 {
+                    THROW_HR_IF(APPINSTALLER_CLI_ERROR_INVALID_CL_ARGUMENTS, !TryConvertToSourceTypeEnum(Input.Type().value()).has_value());
                     SubContext->Args.AddArg(Execution::Args::Type::SourceType, Input.Type().value());
                 }
 
@@ -495,7 +496,7 @@ namespace AppInstaller::CLI
             SourceResourceObject output;
             output.SourceName(source.Name);
             output.Argument(source.Arg);
-            output.Type(source.Type);
+            output.Type(std::string{ ToString(source.Type) });
             output.TrustLevel(TrustLevelStringFromFlags(source.TrustLevel));
             output.Explicit(source.Explicit);
 

src/AppInstallerCLICore/Commands/SourceCommand.cpp

@@ -94,6 +94,30 @@ namespace AppInstaller::CLI
 
     void SourceAddCommand::ValidateArgumentsInternal(Args& execArgs) const
     {
+        if (execArgs.Contains(Execution::Args::Type::SourceType))
+        {
+            std::string_view sourceTypeArg = execArgs.GetArg(Execution::Args::Type::SourceType);
+            auto validOptions = std::vector<Utility::LocIndString>{
+                Utility::LocIndString{ Repository::ToString(Repository::SourceType::PreIndexedPackage) },
+                Utility::LocIndString{ Repository::ToString(Repository::SourceType::Rest) } };
+#ifndef AICLI_DISABLE_TEST_HOOKS
+            validOptions.emplace_back(Utility::LocIndString{ Repository::ToString(Repository::SourceType::ConfigurableTest) });
+#endif
+
+            auto sourceType = Repository::TryConvertToSourceTypeEnum(sourceTypeArg);
+            bool isUserAddType = sourceType.has_value() &&
+                (sourceType.value() == Repository::SourceType::PreIndexedPackage
+                 || sourceType.value() == Repository::SourceType::Rest
+#ifndef AICLI_DISABLE_TEST_HOOKS
+                 || sourceType.value() == Repository::SourceType::ConfigurableTest
+#endif
+                );
+            if (!isUserAddType)
+            {
+                throw CommandException(Resource::String::InvalidArgumentValueError(ArgumentCommon::ForType(Execution::Args::Type::SourceType).Name, Utility::Join(","_liv, validOptions)));
+            }
+        }
+
         if (execArgs.Contains(Execution::Args::Type::SourceTrustLevel))
         {
             try

src/AppInstallerCLICore/ConfigurationWingetDscModuleUnitValidation.cpp

@@ -119,7 +119,7 @@ namespace AppInstaller::CLI::Configuration
             {
                 if (Utility::CaseInsensitiveEquals(wellKnownSource.Name, source.Name) &&
                     Utility::CaseInsensitiveEquals(wellKnownSource.Arg, source.Arg) &&
-                    Utility::CaseInsensitiveEquals(wellKnownSource.Type, source.Type))
+                    Utility::CaseInsensitiveEquals(Repository::ToString(wellKnownSource.Type), source.Type))
                 {
                     return true;
                 }

src/AppInstallerCLICore/PackageCollection.cpp

@@ -127,7 +127,9 @@ namespace AppInstaller::CLI
                 sourceDetails.Identifier = Utility::LocIndString{ detailsNode[ss.PackagesJson_Source_Identifier].asString() };
                 sourceDetails.Name = detailsNode[ss.PackagesJson_Source_Name].asString();
                 sourceDetails.Arg = detailsNode[ss.PackagesJson_Source_Argument].asString();
-                sourceDetails.Type = detailsNode[ss.PackagesJson_Source_Type].asString();
+                auto sourceType = TryConvertToSourceTypeEnum(detailsNode[ss.PackagesJson_Source_Type].asString());
+                THROW_HR_IF(APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE, !sourceType.has_value());
+                sourceDetails.Type = sourceType.value();
 
                 PackageCollection::Source source{ std::move(sourceDetails) };
                 for (const auto& packageNode : sourceNode[ss.PackagesJson_Packages])
@@ -238,7 +240,7 @@ namespace AppInstaller::CLI
             sourceDetailsNode[ss.PackagesJson_Source_Name] = source.Details.Name;
             sourceDetailsNode[ss.PackagesJson_Source_Argument] = source.Details.Arg;
             sourceDetailsNode[ss.PackagesJson_Source_Identifier] = source.Details.Identifier;
-            sourceDetailsNode[ss.PackagesJson_Source_Type] = source.Details.Type;
+            sourceDetailsNode[ss.PackagesJson_Source_Type] = std::string{ ToString(source.Details.Type) };
             sourceNode[ss.PackagesJson_Source_Details] = std::move(sourceDetailsNode);
 
             sourceNode[ss.PackagesJson_Packages] = Json::Value{ Json::ValueType::arrayValue };

src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp

@@ -1495,7 +1495,7 @@ namespace AppInstaller::CLI::Workflow
 
         ConfigurationUnit CreateWinGetSourceUnit(const PackageCollection::Source& source, std::wstring_view unitType)
         {
-            std::string sourceUnitId = source.Details.Name + '_' + source.Details.Type;
+            std::string sourceUnitId = source.Details.Name + '_' + std::string{ Repository::ToString(source.Details.Type) };
             std::wstring sourceUnitIdWide = Utility::ConvertToUTF16(sourceUnitId);
 
             ConfigurationUnit unit;
@@ -1512,7 +1512,7 @@ namespace AppInstaller::CLI::Workflow
             ValueSet settings;
             settings.Insert(s_Setting_WinGetSource_Name, PropertyValue::CreateString(Utility::ConvertToUTF16(source.Details.Name)));
             settings.Insert(s_Setting_WinGetSource_Arg, PropertyValue::CreateString(Utility::ConvertToUTF16(source.Details.Arg)));
-            settings.Insert(s_Setting_WinGetSource_Type, PropertyValue::CreateString(Utility::ConvertToUTF16(source.Details.Type)));
+            settings.Insert(s_Setting_WinGetSource_Type, PropertyValue::CreateString(Utility::ConvertToUTF16(Repository::ToString(source.Details.Type))));
             if (WI_IsFlagSet(source.Details.TrustLevel, Repository::SourceTrustLevel::Trusted))
             {
                 settings.Insert(s_Setting_WinGetSource_TrustLevel, PropertyValue::CreateString(L"trusted"));

src/AppInstallerCLICore/Workflows/SourceFlow.cpp

@@ -50,7 +50,7 @@ namespace AppInstaller::CLI::Workflow
         auto sourceList = context.Get<Execution::Data::SourceList>();
         std::string_view name = context.Args.GetArg(Args::Type::SourceName);
         std::string_view arg = context.Args.GetArg(Args::Type::SourceArg);
-        std::string_view type = context.Args.GetArg(Args::Type::SourceType);
+        Repository::SourceType type = Repository::TryConvertToSourceTypeEnum(context.Args.GetArg(Args::Type::SourceType)).value_or(Repository::Source::GetDefaultSourceType());
 
         // In the absence of a specified type, the default is Microsoft.PreIndexed.Package for comparison.
         // The default type assignment to the source takes place during the add operation (Source::Add in Repository.cpp).
@@ -59,10 +59,6 @@ namespace AppInstaller::CLI::Workflow
         // For example, the following commands would be allowed, but they acts as different alias to same source:
         //      winget source add "mysource1" "https:\\mysource" --trust - level trusted
         //      winget source add "mysource2" "https:\\mysource" --trust - level trusted
-        if (type.empty())
-        {
-            type = Repository::Source::GetDefaultSourceType();
-        }
 
         for (const auto& details : sourceList)
         {
@@ -83,7 +79,7 @@ namespace AppInstaller::CLI::Workflow
                 }
             }
 
-            if (!details.Arg.empty() && details.Arg == arg && details.Type == type)
+            if (!details.Arg.empty() && details.Arg == arg && type == details.Type)
             {
                 context.Reporter.Error() << Resource::String::SourceAddAlreadyExistsDifferentName << std::endl <<
                     "  "_liv << details.Name << " -> "_liv << details.Arg << std::endl;
@@ -118,7 +114,7 @@ namespace AppInstaller::CLI::Workflow
         {
             std::string_view name = context.Args.GetArg(Args::Type::SourceName);
             std::string_view arg = context.Args.GetArg(Args::Type::SourceArg);
-            std::string_view type = context.Args.GetArg(Args::Type::SourceType);
+            Repository::SourceType type = Repository::TryConvertToSourceTypeEnum(context.Args.GetArg(Args::Type::SourceType)).value_or(Repository::Source::GetDefaultSourceType());
 
             Repository::SourceTrustLevel trustLevel = Repository::SourceTrustLevel::None;
             if (context.Args.Contains(Execution::Args::Type::SourceTrustLevel))
@@ -183,7 +179,7 @@ namespace AppInstaller::CLI::Workflow
             Execution::TableOutput<2> table(context.Reporter, { Resource::String::SourceListField, Resource::String::SourceListValue });
 
             table.OutputLine({ Resource::LocString(Resource::String::SourceListName), source.Name });
-            table.OutputLine({ Resource::LocString(Resource::String::SourceListType), source.Type });
+            table.OutputLine({ Resource::LocString(Resource::String::SourceListType), std::string{ Repository::ToString(source.Type) } });
             table.OutputLine({ Resource::LocString(Resource::String::SourceListArg), source.Arg });
             table.OutputLine({ Resource::LocString(Resource::String::SourceListData), source.Data });
             table.OutputLine({ Resource::LocString(Resource::String::SourceListIdentifier), source.Identifier });
@@ -401,7 +397,7 @@ namespace AppInstaller::CLI::Workflow
             {
                 SourceFromPolicy s;
                 s.Name = source.Name;
-                s.Type = source.Type;
+                s.Type = std::string{ Repository::ToString(source.Type) };
                 s.Arg = source.Arg;
                 s.Data = source.Data;
                 s.Identifier = source.Identifier;

src/AppInstallerCLIE2ETests/DSCv3SourceResourceCommand.cs

@@ -318,6 +318,23 @@ public void Source_Set_SimpleRepeated()
             AssertDiffState(diff, []);
         }
 
+        /// <summary>
+        /// Calls `set` on the `source` resource with an invalid source type.
+        /// </summary>
+        [Test]
+        public void Source_Set_InvalidType()
+        {
+            SourceResourceData resourceData = new SourceResourceData()
+            {
+                Name = DefaultSourceName,
+                Argument = DefaultSourceArgDirect,
+                Type = "InvalidType",
+            };
+
+            var result = RunDSCv3Command(SourceResource, SetFunction, resourceData);
+            Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS));
+        }
+
         /// <summary>
         /// Calls `set` on the `source` resource to ensure that it is not present.
         /// </summary>

src/AppInstallerCLIE2ETests/SourceCommand.cs

@@ -164,6 +164,50 @@ public void SourceAddWithInvalidURL()
             Assert.That(result.StdOut, Does.Contain("An unexpected error occurred while executing the command"));
         }
 
+        /// <summary>
+        /// Test source add with empty type.
+        /// </summary>
+        [Test]
+        public void SourceAddWithEmptyType()
+        {
+            var result = TestCommon.RunAICLICommand("source add", $"SourceWithEmptyType {Constants.TestSourceUrl} --type \"\"");
+            Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS));
+            Assert.That(result.StdOut, Does.Contain("The value provided for the `type` argument is invalid"));
+            Assert.That(result.StdOut, Does.Contain("Microsoft.PreIndexed.Package,Microsoft.Rest"));
+        }
+
+        /// <summary>
+        /// Test source add with invalid type.
+        /// </summary>
+        [Test]
+        public void SourceAddWithInvalidType()
+        {
+            var result = TestCommon.RunAICLICommand("source add", $"SourceWithInvalidType {Constants.TestSourceUrl} --type InvalidType");
+            Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.ERROR_INVALID_CL_ARGUMENTS));
+            Assert.That(result.StdOut, Does.Contain("The value provided for the `type` argument is invalid"));
+            Assert.That(result.StdOut, Does.Contain("Microsoft.PreIndexed.Package,Microsoft.Rest"));
+        }
+
+        /// <summary>
+        /// Test source add without type.
+        /// </summary>
+        [Test]
+        public void SourceAddWithoutType()
+        {
+            // Remove the test source to avoid duplicate source url error.
+            TestCommon.RunAICLICommand("source remove", Constants.TestSourceName);
+
+            var result = TestCommon.RunAICLICommand("source add", $"SourceWithoutType {Constants.TestSourceUrl}");
+            Assert.That(result.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK));
+            Assert.That(result.StdOut, Does.Contain("Done"));
+
+            var listResult = TestCommon.RunAICLICommand("source list", "-n SourceWithoutType");
+            Assert.That(listResult.ExitCode, Is.EqualTo(Constants.ErrorCode.S_OK));
+            Assert.That(listResult.StdOut, Does.Contain("Microsoft.PreIndexed.Package"));
+
+            TestCommon.RunAICLICommand("source remove", "-n SourceWithoutType");
+        }
+
         /// <summary>
         /// Test source add with http url.
         /// </summary>

src/AppInstallerCLITests/ARPChanges.cpp

@@ -102,7 +102,7 @@ struct ARPTestContext : public Context
         };
 
         // Inject our source
-        TestHook_SetSourceFactoryOverride(std::string{ Repository::Microsoft::PredefinedInstalledSourceFactory::Type() }, SourceFactory);
+        TestHook_SetSourceFactoryOverride(SourceType::PredefinedInstalled, SourceFactory);
 
         Source = std::make_shared<TestSource>();
         Source->SearchFunction = [&](const SearchRequest& request)

src/AppInstallerCLITests/CompositeSource.cpp

@@ -330,7 +330,7 @@ struct CompositeWithTrackingTestSetup : public CompositeTestSetup
     CompositeWithTrackingTestSetup() : TrackingFactory([&](const SourceDetails&) { return Tracking; })
     {
         Tracking = std::make_shared<SQLiteIndexSource>(SourceDetails{}, SQLiteIndex::CreateNew(SQLITE_MEMORY_DB_CONNECTION_TARGET));
-        TestHook_SetSourceFactoryOverride(std::string{ PackageTrackingCatalogSourceFactory::Type() }, TrackingFactory);
+        TestHook_SetSourceFactoryOverride(SourceType::PackageTracking, TrackingFactory);
     }
 
     ~CompositeWithTrackingTestSetup()