Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rest source addition #765

Merged
merged 35 commits into from
Mar 16, 2021
Merged

Rest source addition #765

Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
50981dd
Adding rest changes
ashpatil-msft Feb 11, 2021
89d0d74
Fix spelling errors
ashpatil-msft Feb 11, 2021
1c17ecf
Basic search functionality
ashpatil-msft Feb 11, 2021
db1cec4
Adding cpprest
ashpatil-msft Feb 12, 2021
7e6179d
Adding cpprest nuget package
ashpatil-msft Feb 12, 2021
d0edffb
Adding necessary interfaces
ashpatil-msft Feb 17, 2021
ba6dedb
Adding http helper
ashpatil-msft Feb 17, 2021
d248255
adding constants
ashpatil-msft Feb 17, 2021
f398249
changes
ashpatil-msft Feb 18, 2021
9198ff2
Added experimental feature
ashpatil-msft Feb 18, 2021
e3b03d2
minor changes
ashpatil-msft Feb 18, 2021
e45149c
Rest changes
ashpatil-msft Feb 18, 2021
712ad8e
remove unused methods
ashpatil-msft Feb 18, 2021
edaacf0
Changes 2
ashpatil-msft Feb 19, 2021
48fc907
Moving rest files to dir
ashpatil-msft Feb 19, 2021
7004070
Changes
ashpatil-msft Feb 22, 2021
7b49b8d
changes 2
ashpatil-msft Feb 22, 2021
18ab865
remove tabs
ashpatil-msft Feb 22, 2021
a83aa71
new line
ashpatil-msft Feb 22, 2021
98276ab
CR Comments
ashpatil-msft Mar 3, 2021
9948bc3
Add experimental rest source flag check while getting sources from se…
ashpatil-msft Mar 3, 2021
d936de0
git
ashpatil-msft Mar 3, 2021
1e15740
CR comments
ashpatil-msft Mar 4, 2021
c39a849
Removed cpprestsdk nuget package
ashpatil-msft Mar 13, 2021
d3e5e8f
Merging from latest
ashpatil-msft Mar 13, 2021
74c70c4
consuming cpprestsdk in appinstaller repository core
ashpatil-msft Mar 13, 2021
df76972
spell check
ashpatil-msft Mar 13, 2021
a4a3164
Minor changes
ashpatil-msft Mar 15, 2021
98bb330
PR Comments. Adding info to settings file
ashpatil-msft Mar 15, 2021
4140cc6
adding rest source to settings schema
ashpatil-msft Mar 15, 2021
aa73642
PR comments
ashpatil-msft Mar 15, 2021
1efd16e
PR Comments
ashpatil-msft Mar 16, 2021
9e56c1a
Comments
ashpatil-msft Mar 16, 2021
bc1d8c4
comments
ashpatil-msft Mar 16, 2021
e01dfb1
todo comment
ashpatil-msft Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/actions/spelling/allow.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ cotaskmem
cout
cpp
cppreference
cpprest
cpprestsdk
cppwinrt
CPRWL
Expand Down Expand Up @@ -177,6 +178,7 @@ iostream
IOutput
IProgress
IRandom
IRest
ISAPPROVEDFOROUTPUT
ISource
isspace
Expand Down Expand Up @@ -281,6 +283,7 @@ png
posix
powershell
PPV
pplx
pragma
PRECONFIG
preindexed
Expand Down
10 changes: 10 additions & 0 deletions doc/Settings.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,13 @@ While work is in progress for import, the command is hidden behind a feature tog
"import": true
},
```

### restSource

While work is in progress for rest source support, the feature is hidden behind a feature toggle. Enabling this will not change how client works currently and will allow testing any additional rest sources added. One can enable it as below:

```
"experimentalFeatures": {
"restSource": true
},
```
5 changes: 5 additions & 0 deletions schemas/JSON/settings/settings.schema.0.2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@
"description": "Enable the import command while it is in development",
"type": "boolean",
"default": false
},
"restSource": {
"description": "Enable the rest source support while it is in development",
"type": "boolean",
"default": false
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/AppInstallerCommonCore/Errors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ namespace AppInstaller
return "Failed to install one or more imported packages";
case APPINSTALLER_CLI_ERROR_NOT_ALL_PACKAGES_FOUND:
return "Could not find one or more requested packages";
case APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE:
return "Json file is invalid";
case APPINSTALLER_CLI_ERROR_SOURCE_NOT_REMOTE:
return "The source location is not remote";
default:
return "Unknown Error Code";
}
Expand Down
4 changes: 4 additions & 0 deletions src/AppInstallerCommonCore/ExperimentalFeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ namespace AppInstaller::Settings
return User().Get<Setting::EFImport>();
case Feature::ExperimentalExport:
return User().Get<Setting::EFExport>();
case Feature::ExperimentalRestSource:
return User().Get<Setting::EFRestSource>();
default:
THROW_HR(E_UNEXPECTED);
}
Expand All @@ -56,6 +58,8 @@ namespace AppInstaller::Settings
return ExperimentalFeature{ "Import Command", "import", "https://aka.ms/winget-settings", Feature::ExperimentalImport };
case Feature::ExperimentalExport:
return ExperimentalFeature{ "Export Command", "export", "https://aka.ms/winget-settings", Feature::ExperimentalExport };
case Feature::ExperimentalRestSource:
return ExperimentalFeature{ "Rest Source Support", "restSource", "https://aka.ms/winget-settings", Feature::ExperimentalRestSource };
ashpatil-msft marked this conversation as resolved.
Show resolved Hide resolved
default:
THROW_HR(E_UNEXPECTED);
}
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCommonCore/Public/AppInstallerErrors.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
#define APPINSTALLER_CLI_ERROR_IMPORT_INSTALL_FAILED ((HRESULT)0x8a150034)
#define APPINSTALLER_CLI_ERROR_NOT_ALL_PACKAGES_FOUND ((HRESULT)0x8a150035)
#define APPINSTALLER_CLI_ERROR_JSON_INVALID_FILE ((HRESULT)0x8a150036)
#define APPINSTALLER_CLI_ERROR_SOURCE_NOT_REMOTE ((HRESULT)0x8A150037)

namespace AppInstaller
{
Expand Down
3 changes: 2 additions & 1 deletion src/AppInstallerCommonCore/Public/winget/ExperimentalFeature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ namespace AppInstaller::Settings
ExperimentalUpgrade = 0x10,
ExperimentalUninstall = 0x20,
ExperimentalImport = 0x40,
ExperimentalRestSource = 0x80,
Max, // This MUST always be after all experimental features

// Features listed after Max will not be shown with the features command
// This can be used to hide highly experimental features
ExperimentalExport = 0x80
ExperimentalExport = 0x100
};

using Feature_t = std::underlying_type_t<ExperimentalFeature::Feature>;
Expand Down
2 changes: 2 additions & 0 deletions src/AppInstallerCommonCore/Public/winget/UserSettings.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ namespace AppInstaller::Settings
EFImport,
EFExport,
TelemetryDisable,
EFRestSource,
Max
};

Expand Down Expand Up @@ -93,6 +94,7 @@ namespace AppInstaller::Settings
SETTINGMAPPING_SPECIALIZATION(Setting::EFImport, bool, bool, false, ".experimentalFeatures.import"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFExport, bool, bool, false, ".experimentalFeatures.export"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::TelemetryDisable, bool, bool, false, ".telemetry.disable"sv);
SETTINGMAPPING_SPECIALIZATION(Setting::EFRestSource, bool, bool, false, ".experimentalFeatures.restSource"sv);

// Used to deduce the SettingVariant type; making a variant that includes std::monostate and all SettingMapping types.
template <size_t... I>
Expand Down
1 change: 1 addition & 0 deletions src/AppInstallerCommonCore/UserSettings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ namespace AppInstaller::Settings
WINGET_VALIDATE_PASS_THROUGH(EFImport)
WINGET_VALIDATE_PASS_THROUGH(EFExport)
WINGET_VALIDATE_PASS_THROUGH(TelemetryDisable)
WINGET_VALIDATE_PASS_THROUGH(EFRestSource)
}

UserSettings::UserSettings() : m_type(UserSettingsType::Default)
Expand Down
27 changes: 19 additions & 8 deletions src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@
<ClCompile>
<Optimization>Disabled</Optimization>
<PreprocessorDefinitions>_NO_ASYNCRTIMP;_DEBUG;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|ARM'">true</TreatWarningAsError>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'">true</TreatWarningAsError>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">true</TreatWarningAsError>
Expand All @@ -139,7 +139,7 @@
<ItemDefinitionGroup Condition="'$(Platform)'=='Win32'">
<ClCompile>
<PreprocessorDefinitions>_NO_ASYNCRTIMP;WIN32;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">true</TreatWarningAsError>
</ClCompile>
<Link>
Expand All @@ -152,10 +152,10 @@
<FunctionLevelLinking>true</FunctionLevelLinking>
<IntrinsicFunctions>true</IntrinsicFunctions>
<PreprocessorDefinitions>_NO_ASYNCRTIMP;NDEBUG;%(PreprocessorDefinitions);CLICOREDLLBUILD</PreprocessorDefinitions>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|x64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<AdditionalIncludeDirectories Condition="'$(Configuration)|$(Platform)'=='Release|x64'">$(ProjectDir);$(ProjectDir)\Public;$(ProjectDir)..\AppInstallerCommonCore\Public;$(ProjectDir)..\JsonCppLib\json;$(ProjectDir)..\cpprestsdk\cpprestsdk\Release\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|ARM'">true</TreatWarningAsError>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'">true</TreatWarningAsError>
<TreatWarningAsError Condition="'$(Configuration)|$(Platform)'=='Release|Win32'">true</TreatWarningAsError>
Expand Down Expand Up @@ -205,6 +205,12 @@
<ClInclude Include="Microsoft\SQLiteIndex.h" />
<ClInclude Include="Microsoft\SQLiteIndexSource.h" />
<ClInclude Include="pch.h" />
<ClInclude Include="Rest\HttpClientHelper.h" />
<ClInclude Include="Rest\RestClient.h" />
<ClInclude Include="Rest\RestSource.h" />
<ClInclude Include="Rest\RestSourceFactory.h" />
<ClInclude Include="Rest\Schema\1_0\Interface.h" />
<ClInclude Include="Rest\Schema\IRestClient.h" />
<ClInclude Include="SourceFactory.h" />
<ClInclude Include="SQLiteStatementBuilder.h" />
<ClInclude Include="Public\AppInstallerRepositorySearch.h" />
Expand Down Expand Up @@ -246,6 +252,11 @@
<PrecompiledHeader>Create</PrecompiledHeader>
</ClCompile>
<ClCompile Include="RepositorySource.cpp" />
<ClCompile Include="Rest\HttpClientHelper.cpp" />
<ClCompile Include="Rest\RestClient.cpp" />
<ClCompile Include="Rest\RestSource.cpp" />
<ClCompile Include="Rest\RestSourceFactory.cpp" />
<ClCompile Include="Rest\Schema\1_0\Interface.cpp" />
<ClCompile Include="SQLiteStatementBuilder.cpp" />
<ClCompile Include="SQLiteTempTable.cpp" />
<ClCompile Include="SQLiteWrapper.cpp" />
Expand Down
42 changes: 42 additions & 0 deletions src/AppInstallerRepositoryCore/AppInstallerRepositoryCore.vcxproj.filters
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@
<Filter Include="Microsoft\Schema\1_2">
<UniqueIdentifier>{dc8b6163-e9b5-4d05-87bc-d6098afe0f92}</UniqueIdentifier>
</Filter>
<Filter Include="Rest">
<UniqueIdentifier>{7f16f6e0-12c6-4bb9-885e-3f8b666d5f74}</UniqueIdentifier>
</Filter>
<Filter Include="Rest\Schema">
<UniqueIdentifier>{15a1ee83-4118-40fe-ba43-c54d7185d505}</UniqueIdentifier>
</Filter>
<Filter Include="Rest\Schema\1_0">
<UniqueIdentifier>{8a93b62f-d065-4048-b43a-a2b14b70c330}</UniqueIdentifier>
</Filter>
</ItemGroup>
<ItemGroup>
<ClInclude Include="pch.h">
Expand Down Expand Up @@ -153,6 +162,24 @@
<ClInclude Include="Microsoft\Schema\1_2\NormalizedPackagePublisherTable.h">
<Filter>Microsoft\Schema\1_2</Filter>
</ClInclude>
<ClInclude Include="Rest\Schema\1_0\Interface.h">
<Filter>Rest\Schema\1_0</Filter>
</ClInclude>
<ClInclude Include="Rest\Schema\IRestClient.h">
<Filter>Rest\Schema</Filter>
</ClInclude>
<ClInclude Include="Rest\HttpClientHelper.h">
<Filter>Rest</Filter>
</ClInclude>
<ClInclude Include="Rest\RestClient.h">
<Filter>Rest</Filter>
</ClInclude>
<ClInclude Include="Rest\RestSource.h">
<Filter>Rest</Filter>
</ClInclude>
<ClInclude Include="Rest\RestSourceFactory.h">
<Filter>Rest</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
Expand Down Expand Up @@ -230,6 +257,21 @@
<ClCompile Include="Microsoft\Schema\1_2\SearchResultsTable_1_2.cpp">
<Filter>Microsoft\Schema\1_2</Filter>
</ClCompile>
<ClCompile Include="Rest\Schema\1_0\Interface.cpp">
<Filter>Rest\Schema\1_0</Filter>
</ClCompile>
<ClCompile Include="Rest\HttpClientHelper.cpp">
<Filter>Rest</Filter>
</ClCompile>
<ClCompile Include="Rest\RestClient.cpp">
<Filter>Rest</Filter>
</ClCompile>
<ClCompile Include="Rest\RestSource.cpp">
<Filter>Rest</Filter>
</ClCompile>
<ClCompile Include="Rest\RestSourceFactory.cpp">
<Filter>Rest</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down
36 changes: 32 additions & 4 deletions src/AppInstallerRepositoryCore/RepositorySource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "SourceFactory.h"
#include "Microsoft/PredefinedInstalledSourceFactory.h"
#include "Microsoft/PreIndexedPackageSourceFactory.h"
#include "Rest/RestSourceFactory.h"

namespace AppInstaller::Repository
{
Expand Down Expand Up @@ -206,9 +207,10 @@ namespace AppInstaller::Repository
result.emplace_back(std::move(storeDetails));
}
}
break;
break;
case SourceOrigin::User:
result = GetSourcesFromSetting(
{
std::vector<SourceDetailsInternal> userSources = GetSourcesFromSetting(
Settings::Streams::UserSources,
s_SourcesYaml_Sources,
[&](SourceDetailsInternal& details, const std::string& settingValue, const YAML::Node& source)
Expand All @@ -222,7 +224,19 @@ namespace AppInstaller::Repository
TryReadScalar(name, settingValue, source, s_SourcesYaml_Source_Identifier, details.Identifier);
return true;
});
break;

for (auto& source : userSources)
{
if (Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), source.Type)
&& !Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::ExperimentalRestSource))
{
continue;
}

result.emplace_back(std::move(source));
}
}
break;
default:
THROW_HR(E_UNEXPECTED);
}
Expand Down Expand Up @@ -327,6 +341,11 @@ namespace AppInstaller::Repository
{
return Microsoft::PredefinedInstalledSourceFactory::Create();
}
else if (Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), type)
Copy link
Contributor

@yao-msft yao-msft Mar 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), type) [](start = 21, length = 69)

From the comment above "Should always come from code, so no need for case insensitivity"
Shall we just use '==' for source type comparisons? Here and line 230 and line 630 #Closed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That comment is because those are specifically internal sources. A REST source may be configured externally, so yes, we should do case insensitive.

In reply to: 594688379 [](ancestors = 594688379)

Copy link
Contributor Author

@ashpatil-msft ashpatil-msft Mar 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we need case insensitivity check because the Rest source type doesnt come from code. I also realized that I missed adding case-insensitivity check in the rest factory. I will add it there as well. #Resolved

&& Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::ExperimentalRestSource))
{
return Rest::RestSourceFactory::Create();
}

THROW_HR(APPINSTALLER_CLI_ERROR_INVALID_SOURCE_TYPE);
}
Expand Down Expand Up @@ -607,6 +626,15 @@ namespace AppInstaller::Repository
details.LastUpdateTime = Utility::ConvertUnixEpochToSystemClock(0);
details.Origin = SourceOrigin::User;

// Check feature flag enablement for rest source.
if (Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), type)
&& !Settings::ExperimentalFeature::IsEnabled(Settings::ExperimentalFeature::Feature::ExperimentalRestSource))
{
AICLI_LOG(Repo, Error, << Settings::ExperimentalFeature::GetFeature(Settings::ExperimentalFeature::Feature::ExperimentalRestSource).Name()
<< " feature is disabled. Execution cancelled.");
THROW_HR(APPINSTALLER_CLI_ERROR_EXPERIMENTAL_FEATURE_DISABLED);
}

AddSourceFromDetails(details, progress);

AICLI_LOG(Repo, Info, << "Source created with extra data: " << details.Data);
Expand All @@ -627,7 +655,7 @@ namespace AppInstaller::Repository
AICLI_LOG(Repo, Info, << "Default source requested, but no sources configured");
return {};
}
else if(currentSources.size() == 1)
else if (currentSources.size() == 1)
{
AICLI_LOG(Repo, Info, << "Default source requested, only 1 source available, using the only source: " << currentSources[0].get().Name);
return OpenSource(currentSources[0].get().Name, progress);
Expand Down
Loading