-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rest source addition #765
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.
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
Changes from all commits
50981dd
89d0d74
1c17ecf
db1cec4
7e6179d
d0edffb
ba6dedb
d248255
f398249
9198ff2
e3b03d2
e45149c
712ad8e
edaacf0
48fc907
7004070
7b49b8d
18ab865
a83aa71
98276ab
9948bc3
d936de0
1e15740
c39a849
d3e5e8f
74c70c4
df76972
a4a3164
98bb330
4140cc6
aa73642
1efd16e
9e56c1a
bc1d8c4
e01dfb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| #include "SourceFactory.h" | ||
| #include "Microsoft/PredefinedInstalledSourceFactory.h" | ||
| #include "Microsoft/PreIndexedPackageSourceFactory.h" | ||
| #include "Rest/RestSourceFactory.h" | ||
|
|
||
| namespace AppInstaller::Repository | ||
| { | ||
|
|
@@ -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) | ||
|
|
@@ -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); | ||
| } | ||
|
|
@@ -327,6 +341,11 @@ namespace AppInstaller::Repository | |
| { | ||
| return Microsoft::PredefinedInstalledSourceFactory::Create(); | ||
| } | ||
| else if (Utility::CaseInsensitiveEquals(Rest::RestSourceFactory::Type(), type) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From the comment above "Should always come from code, so no need for case insensitivity"
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
| } | ||
|
|
@@ -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); | ||
|
|
@@ -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); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.