Skip to content

Commit

Permalink
Store information result in RestSourceReference for reuse (microsoft#…
Browse files Browse the repository at this point in the history 
…5112)

## Change
When the `RestSourceReference` retrieves the information, store it and
pass it along to `RestClient::Create` so that it can reuse the value
rather than calling again.
  • Loading branch information
@JohnMcPMS
JohnMcPMS authored Jan 8, 2025
1 parent c925c39 commit d8819d0
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 35 deletions.
16 changes: 12 additions & 4 deletions src/AppInstallerCLITests/CustomHeader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ namespace
})delimiter");
}

// In RestClient.cpp tests
extern RestClient CreateRestClient(
const std::string& restApi,
const std::optional<std::string>& customHeader,
std::string_view caller,
const Http::HttpClientHelper& helper,
const Authentication::AuthenticationArguments& authArgs = {});

TEST_CASE("RestClient_CustomHeader", "[RestSource][CustomHeader]")
{
utility::string_t sample = _XPLATSTR(
Expand All @@ -55,7 +63,7 @@ TEST_CASE("RestClient_CustomHeader", "[RestSource][CustomHeader]")
std::optional<std::string> customHeader = "Testing custom header";
auto header = std::make_pair<>(CustomHeaderName, JSON::GetUtilityString(customHeader.value()));
HttpClientHelper helper{ GetHeaderVerificationHandler(web::http::status_codes::OK, sample, header) };
RestClient client = RestClient::Create(utility::conversions::to_utf8string("https://restsource.com/api"), customHeader, {}, std::move(helper), {});
RestClient client = CreateRestClient(utility::conversions::to_utf8string("https://restsource.com/api"), customHeader, {}, helper);
REQUIRE(client.GetSourceIdentifier() == "Source123");
}

Expand Down Expand Up @@ -104,7 +112,7 @@ TEST_CASE("RestSourceSearch_CustomHeaderExceedingSize", "[RestSource][CustomHead
auto header = std::make_pair<>(CustomHeaderName, JSON::GetUtilityString(customHeader));
HttpClientHelper helper{ GetHeaderVerificationHandler(web::http::status_codes::OK, sampleSearchResponse, header) };

REQUIRE_THROWS_HR(RestClient::Create(utility::conversions::to_utf8string("https://restsource.com/api"), customHeader, {}, std::move(helper), {}),
REQUIRE_THROWS_HR(CreateRestClient(utility::conversions::to_utf8string("https://restsource.com/api"), customHeader, {}, helper),
APPINSTALLER_CLI_ERROR_CUSTOMHEADER_EXCEEDS_MAXLENGTH);
}

Expand All @@ -122,7 +130,7 @@ TEST_CASE("RestClient_CustomUserAgentHeader", "[RestSource][CustomHeader]")
std::string testCaller = "TestCaller";
auto header = std::make_pair<>(web::http::header_names::user_agent, JSON::GetUtilityString(Runtime::GetUserAgent(testCaller)));
HttpClientHelper helper{ GetHeaderVerificationHandler(web::http::status_codes::OK, sample, header) };
RestClient client = RestClient::Create(utility::conversions::to_utf8string("https://restsource.com/api"), {}, testCaller, std::move(helper), {});
RestClient client = CreateRestClient(utility::conversions::to_utf8string("https://restsource.com/api"), {}, testCaller, helper);
REQUIRE(client.GetSourceIdentifier() == "Source123");
}

Expand All @@ -139,6 +147,6 @@ TEST_CASE("RestClient_DefaultUserAgentHeader", "[RestSource][CustomHeader]")

auto header = std::make_pair<>(web::http::header_names::user_agent, JSON::GetUtilityString(Runtime::GetDefaultUserAgent()));
HttpClientHelper helper{ GetHeaderVerificationHandler(web::http::status_codes::OK, sample, header) };
RestClient client = RestClient::Create(utility::conversions::to_utf8string("https://restsource.com/api"), {}, {}, std::move(helper), {});
RestClient client = CreateRestClient(utility::conversions::to_utf8string("https://restsource.com/api"), {}, {}, helper);
REQUIRE(client.GetSourceIdentifier() == "Source123");
}
36 changes: 23 additions & 13 deletions src/AppInstallerCLITests/RestClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ using namespace AppInstaller::Repository::Rest::Schema;

const std::string TestRestUri = "http://restsource.net";

RestClient CreateRestClient(
const std::string& restApi,
const std::optional<std::string>& customHeader,
std::string_view caller,
const Http::HttpClientHelper& helper,
const Authentication::AuthenticationArguments& authArgs = {})
{
return RestClient::Create(restApi, customHeader, caller, helper, RestClient::GetInformation(restApi, customHeader, caller, helper), authArgs);
}

TEST_CASE("GetLatestCommonVersion", "[RestSource]")
{
std::set<AppInstaller::Utility::Version> wingetSupportedContracts = { Version {"1.0.0"}, Version {"1.2.0"} };
Expand Down Expand Up @@ -103,7 +113,7 @@ TEST_CASE("GetInformation_Success", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
IRestClient::Information information = RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper));
IRestClient::Information information = RestClient::GetInformation(TestRestUri, {}, {}, helper);
REQUIRE(information.SourceIdentifier == "Source123");
REQUIRE(information.ServerSupportedVersions.size() == 2);
REQUIRE(information.ServerSupportedVersions.at(0) == "1.0.0");
Expand Down Expand Up @@ -165,7 +175,7 @@ TEST_CASE("GetInformation_WithAuthenticationInfo_Success", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
IRestClient::Information information = RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper));
IRestClient::Information information = RestClient::GetInformation(TestRestUri, {}, {}, helper);
REQUIRE(information.SourceIdentifier == "Source123");
REQUIRE(information.ServerSupportedVersions.size() == 1);
REQUIRE(information.ServerSupportedVersions.at(0) == "1.7.0");
Expand Down Expand Up @@ -208,7 +218,7 @@ TEST_CASE("GetInformation_Fail_AgreementsWithoutIdentifier", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper)), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, helper), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
}

TEST_CASE("GetInformation_Fail_InvalidMicrosoftEntraIdInfo", "[RestSource]")
Expand All @@ -226,7 +236,7 @@ TEST_CASE("GetInformation_Fail_InvalidMicrosoftEntraIdInfo", "[RestSource]")
}})delimiter");

HttpClientHelper helper1{ GetTestRestRequestHandler(web::http::status_codes::OK, sample1) };
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper1)), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, helper1), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);

utility::string_t sample2 = _XPLATSTR(
R"delimiter({
Expand All @@ -245,7 +255,7 @@ TEST_CASE("GetInformation_Fail_InvalidMicrosoftEntraIdInfo", "[RestSource]")
}})delimiter");

HttpClientHelper helper2{ GetTestRestRequestHandler(web::http::status_codes::OK, sample2) };
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper2)), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, helper2), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);

utility::string_t sample3 = _XPLATSTR(
R"delimiter({
Expand All @@ -263,7 +273,7 @@ TEST_CASE("GetInformation_Fail_InvalidMicrosoftEntraIdInfo", "[RestSource]")
}})delimiter");

HttpClientHelper helper3{ GetTestRestRequestHandler(web::http::status_codes::OK, sample3) };
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper3)), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
REQUIRE_THROWS_HR(RestClient::GetInformation(TestRestUri, {}, {}, helper3), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);

utility::string_t sample4 = _XPLATSTR(
R"delimiter({
Expand All @@ -281,7 +291,7 @@ TEST_CASE("GetInformation_Fail_InvalidMicrosoftEntraIdInfo", "[RestSource]")
Authentication::AuthenticationArguments authArgs;
authArgs.Mode = Authentication::AuthenticationMode::Silent;
Version version_1_7{ "1.7.0" };
REQUIRE_THROWS_HR(RestClient::GetSupportedInterface(TestRestUri, {}, RestClient::GetInformation(TestRestUri, {}, {}, std::move(helper4)), authArgs, version_1_7, {}), APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED);
REQUIRE_THROWS_HR(RestClient::GetSupportedInterface(TestRestUri, {}, RestClient::GetInformation(TestRestUri, {}, {}, helper4), authArgs, version_1_7, {}), APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED);
}

TEST_CASE("RestClientCreate_UnsupportedVersion", "[RestSource]")
Expand All @@ -296,7 +306,7 @@ TEST_CASE("RestClientCreate_UnsupportedVersion", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
REQUIRE_THROWS_HR(RestClient::Create("https://restsource.com/api", {}, {}, std::move(helper)), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
REQUIRE_THROWS_HR(CreateRestClient("https://restsource.com/api", {}, {}, helper), APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE);
}

TEST_CASE("RestClientCreate_UnsupportedAuthenticationMethod", "[RestSource]")
Expand All @@ -316,7 +326,7 @@ TEST_CASE("RestClientCreate_UnsupportedAuthenticationMethod", "[RestSource]")
HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
Authentication::AuthenticationArguments authArgs;
authArgs.Mode = Authentication::AuthenticationMode::Silent;
REQUIRE_THROWS_HR(RestClient::Create("https://restsource.com/api", {}, {}, std::move(helper), std::move(authArgs)), APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED);
REQUIRE_THROWS_HR(CreateRestClient("https://restsource.com/api", {}, {}, helper, authArgs), APPINSTALLER_CLI_ERROR_AUTHENTICATION_TYPE_NOT_SUPPORTED);
}

TEST_CASE("RestClientCreate_InvalidAuthenticationArguments", "[RestSource]")
Expand All @@ -339,7 +349,7 @@ TEST_CASE("RestClientCreate_InvalidAuthenticationArguments", "[RestSource]")
HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
Authentication::AuthenticationArguments authArgs;
authArgs.Mode = Authentication::AuthenticationMode::Unknown;
REQUIRE_THROWS_HR(RestClient::Create("https://restsource.com/api", {}, {}, std::move(helper), std::move(authArgs)), E_UNEXPECTED);
REQUIRE_THROWS_HR(CreateRestClient("https://restsource.com/api", {}, {}, helper, authArgs), E_UNEXPECTED);
}

TEST_CASE("RestClientCreate_1.0_Success", "[RestSource]")
Expand All @@ -354,7 +364,7 @@ TEST_CASE("RestClientCreate_1.0_Success", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
RestClient client = RestClient::Create(TestRestUri, {}, {}, std::move(helper), {});
RestClient client = CreateRestClient(TestRestUri, {}, {}, helper);
REQUIRE(client.GetSourceIdentifier() == "Source123");
}

Expand Down Expand Up @@ -391,7 +401,7 @@ TEST_CASE("RestClientCreate_1.1_Success", "[RestSource]")
}})delimiter");

HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
RestClient client = RestClient::Create(TestRestUri, {}, {}, std::move(helper));
RestClient client = CreateRestClient(TestRestUri, {}, {}, helper);
REQUIRE(client.GetSourceIdentifier() == "Source123");
auto information = client.GetSourceInformation();
REQUIRE(information.SourceAgreementsIdentifier == "agreementV1");
Expand Down Expand Up @@ -457,7 +467,7 @@ TEST_CASE("RestClientCreate_1.7_Success", "[RestSource]")
Authentication::AuthenticationArguments authArgs;
authArgs.Mode = Authentication::AuthenticationMode::Silent;
HttpClientHelper helper{ GetTestRestRequestHandler(web::http::status_codes::OK, sample) };
RestClient client = RestClient::Create(TestRestUri, {}, {}, std::move(helper), std::move(authArgs));
RestClient client = CreateRestClient(TestRestUri, {}, {}, helper, authArgs);
REQUIRE(client.GetSourceIdentifier() == "Source123");
auto information = client.GetSourceInformation();
REQUIRE(information.SourceAgreementsIdentifier == "agreementV1");
Expand Down
3 changes: 1 addition & 2 deletions src/AppInstallerCommonCore/Rest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@

namespace AppInstaller::Rest
{
utility::string_t GetRestAPIBaseUri(std::string restApiUri)
utility::string_t GetRestAPIBaseUri(std::string uri)
{
// Trim
std::string uri = restApiUri;
if (!uri.empty())
{
uri = AppInstaller::Utility::Trim(uri);
Expand Down
13 changes: 9 additions & 4 deletions src/AppInstallerRepositoryCore/Rest/RestClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace AppInstaller::Repository::Rest

namespace
{
HttpClientHelper::HttpRequestHeaders GetHeaders(std::optional<std::string> customHeader, std::string_view caller)
HttpClientHelper::HttpRequestHeaders GetHeaders(const std::optional<std::string>& customHeader, std::string_view caller)
{
HttpClientHelper::HttpRequestHeaders headers;

Expand Down Expand Up @@ -135,7 +135,7 @@ namespace AppInstaller::Repository::Rest
return *commonVersions.rbegin();
}

Schema::IRestClient::Information RestClient::GetInformation(const std::string& restApi, std::optional<std::string> customHeader, std::string_view caller, const HttpClientHelper& helper)
Schema::IRestClient::Information RestClient::GetInformation(const std::string& restApi, const std::optional<std::string>& customHeader, std::string_view caller, const HttpClientHelper& helper)
{
utility::string_t restEndpoint = AppInstaller::Rest::GetRestAPIBaseUri(restApi);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_URL, !AppInstaller::Rest::IsValidUri(restEndpoint));
Expand Down Expand Up @@ -185,14 +185,19 @@ namespace AppInstaller::Repository::Rest
THROW_HR(APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_VERSION);
}

RestClient RestClient::Create(const std::string& restApi, std::optional<std::string> customHeader, std::string_view caller, const HttpClientHelper& helper, const Authentication::AuthenticationArguments& authArgs)
RestClient RestClient::Create(
const std::string& restApi,
const std::optional<std::string>& customHeader,
std::string_view caller,
const HttpClientHelper& helper,
const Schema::IRestClient::Information& information,
const Authentication::AuthenticationArguments& authArgs)
{
utility::string_t restEndpoint = AppInstaller::Rest::GetRestAPIBaseUri(restApi);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_RESTSOURCE_INVALID_URL, !AppInstaller::Rest::IsValidUri(restEndpoint));

auto headers = GetHeaders(customHeader, caller);

IRestClient::Information information = GetInformationInternal(restEndpoint, headers, helper);
std::optional<Version> latestCommonVersion = GetLatestCommonVersion(information.ServerSupportedVersions, WingetSupportedContracts);
THROW_HR_IF(APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE, !latestCommonVersion);

Expand Down
10 changes: 8 additions & 2 deletions src/AppInstallerRepositoryCore/Rest/RestClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace AppInstaller::Repository::Rest
static std::optional<AppInstaller::Utility::Version> GetLatestCommonVersion(const std::vector<std::string>& serverSupportedVersions, const std::set<AppInstaller::Utility::Version>& wingetSupportedVersions);

// Responsible for getting the source information contracts with minimal validation. Does not try to create a rest interface out of it.
static Schema::IRestClient::Information GetInformation(const std::string& restApi, std::optional<std::string> customHeader, std::string_view caller, const Http::HttpClientHelper& helper);
static Schema::IRestClient::Information GetInformation(const std::string& restApi, const std::optional<std::string>& customHeader, std::string_view caller, const Http::HttpClientHelper& helper);

static std::unique_ptr<Schema::IRestClient> GetSupportedInterface(
const std::string& restApi,
Expand All @@ -39,7 +39,13 @@ namespace AppInstaller::Repository::Rest
const Http::HttpClientHelper& helper);

// Creates the rest client. Full validation performed (just as opening the source)
static RestClient Create(const std::string& restApi, std::optional<std::string> customHeader, std::string_view caller, const Http::HttpClientHelper& helper, const Authentication::AuthenticationArguments& authArgs = {});
static RestClient Create(
const std::string& restApi,
const std::optional<std::string>& customHeader,
std::string_view caller,
const Http::HttpClientHelper& helper,
const Schema::IRestClient::Information& information,
const Authentication::AuthenticationArguments& authArgs = {});
private:
RestClient(std::unique_ptr<Schema::IRestClient> supportedInterface, std::string sourceIdentifier);

Expand Down
21 changes: 11 additions & 10 deletions src/AppInstallerRepositoryCore/Rest/RestSourceFactory.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ namespace AppInstaller::Repository::Rest
std::shared_ptr<ISource> Open(IProgressCallback&) override
{
Initialize();
RestClient restClient = RestClient::Create(m_details.Arg, m_customHeader, m_caller, m_httpClientHelper, m_authArgs);
RestClient restClient = RestClient::Create(m_details.Arg, m_customHeader, m_caller, m_httpClientHelper, m_restClientInformation, m_authArgs);
return std::make_shared<RestSource>(m_details, m_information, std::move(restClient));
}

Expand All @@ -61,28 +61,29 @@ namespace AppInstaller::Repository::Rest
[&]()
{
m_httpClientHelper.SetPinningConfiguration(m_details.CertificatePinningConfiguration);
auto sourceInformation = RestClient::GetInformation(m_details.Arg, m_customHeader, m_caller, m_httpClientHelper);
m_restClientInformation = RestClient::GetInformation(m_details.Arg, m_customHeader, m_caller, m_httpClientHelper);

m_details.Identifier = sourceInformation.SourceIdentifier;
m_details.Identifier = m_restClientInformation.SourceIdentifier;

m_information.UnsupportedPackageMatchFields = sourceInformation.UnsupportedPackageMatchFields;
m_information.RequiredPackageMatchFields = sourceInformation.RequiredPackageMatchFields;
m_information.UnsupportedQueryParameters = sourceInformation.UnsupportedQueryParameters;
m_information.RequiredQueryParameters = sourceInformation.RequiredQueryParameters;
m_information.UnsupportedPackageMatchFields = m_restClientInformation.UnsupportedPackageMatchFields;
m_information.RequiredPackageMatchFields = m_restClientInformation.RequiredPackageMatchFields;
m_information.UnsupportedQueryParameters = m_restClientInformation.UnsupportedQueryParameters;
m_information.RequiredQueryParameters = m_restClientInformation.RequiredQueryParameters;

m_information.SourceAgreementsIdentifier = sourceInformation.SourceAgreementsIdentifier;
for (auto const& agreement : sourceInformation.SourceAgreements)
m_information.SourceAgreementsIdentifier = m_restClientInformation.SourceAgreementsIdentifier;
for (auto const& agreement : m_restClientInformation.SourceAgreements)
{
m_information.SourceAgreements.emplace_back(agreement.Label, agreement.Text, agreement.Url);
}

m_information.Authentication = sourceInformation.Authentication;
m_information.Authentication = m_restClientInformation.Authentication;
});
}

SourceDetails m_details;
Http::HttpClientHelper m_httpClientHelper;
SourceInformation m_information;
Schema::IRestClient::Information m_restClientInformation;
std::optional<std::string> m_customHeader;
std::string m_caller;
Authentication::AuthenticationArguments m_authArgs;
Expand Down

0 comments on commit d8819d0

Please sign in to comment.