Skip to content

Commit ab887ef

Browse files
authored
Cache information responses from REST sources (#5726)
## Change Store and use the `/information` response from REST sources per their `cache-control` header. At the base of this change is a new type of setting stream: `Encrypted`. These settings are encrypted to the user and machine that the data is stored on. Based on the log file from running with and without a cached value, the time between close log lines dropped from 189 ms to 8 ms for the msstore source. This time saving should be in effect for the large majority of uses of WinGet.
1 parent ffda04f commit ab887ef

25 files changed

+945
-45
lines changed

.github/actions/spelling/allow.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ createpintable
5353
createportabletable
5454
createtables
5555
CRTDECL
56+
CRYPTPROTECT
5657
CTLs
5758
curated
5859
CURSORPOSITON

azure-pipelines.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ jobs:
332332
- powershell: |
333333
Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force
334334
Install-Module Microsoft.WinGet.Client -Repository PSGallery -Force
335-
Repair-WingetPackageManager -AllUsers -Latest
335+
try { Repair-WingetPackageManager -AllUsers -Latest -Verbose } catch { $_.Exception }
336336
Install-WinGetPackage -Id Microsoft.Sysinternals.PsTools -Source winget
337337
displayName: Install Sysinternals PsTools Using Winget
338338
condition: succeededOrFailed()

src/AppInstallerCLIE2ETests/ConfigureCommand.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ public void ConfigureFromTestRepo_DSCv3()
364364
public void ConfigureFindUnitProcessors()
365365
{
366366
// Find all unit processors.
367-
var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty);
367+
var result = TestCommon.RunAICLICommand("test config-find-unit-processors", string.Empty, timeOut: 120000);
368368
Assert.AreEqual(0, result.ExitCode);
369369
Assert.True(result.StdOut.Contains("Microsoft/OSInfo"));
370370

src/AppInstallerCLITests/Downloader.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,3 +109,63 @@ TEST_CASE("HttpStream_ReadLastFullPage", "[HttpStream]")
109109
REQUIRE(stream->Read(buffer.get(), static_cast<ULONG>(HttpStream::HttpLocalCache::PAGE_SIZE), &read) >= S_OK);
110110
REQUIRE(read == (stat.cbSize.QuadPart % HttpStream::HttpLocalCache::PAGE_SIZE));
111111
}
112+
113+
TEST_CASE("CacheControl", "[Downloader]")
114+
{
115+
SECTION("Empty")
116+
{
117+
CacheControlPolicy test{ L"" };
118+
REQUIRE(!test.Present);
119+
}
120+
SECTION("Space")
121+
{
122+
CacheControlPolicy test{ L" " };
123+
REQUIRE(!test.Present);
124+
}
125+
SECTION("Standard")
126+
{
127+
CacheControlPolicy test{ L"public, max-age=77287" };
128+
REQUIRE(test.Present);
129+
REQUIRE(test.Public);
130+
REQUIRE(!test.NoCache);
131+
REQUIRE(!test.NoStore);
132+
REQUIRE(test.MaxAge == 77287);
133+
}
134+
SECTION("All")
135+
{
136+
CacheControlPolicy test{ L"public, no-cache, no-store, max-age = 77" };
137+
REQUIRE(test.Present);
138+
REQUIRE(test.Public);
139+
REQUIRE(test.NoCache);
140+
REQUIRE(test.NoStore);
141+
REQUIRE(test.MaxAge == 77);
142+
}
143+
SECTION("Casing")
144+
{
145+
CacheControlPolicy test{ L"Public, Max-Age=42" };
146+
REQUIRE(test.Present);
147+
REQUIRE(test.Public);
148+
REQUIRE(!test.NoCache);
149+
REQUIRE(!test.NoStore);
150+
REQUIRE(test.MaxAge == 42);
151+
}
152+
SECTION("Unknown")
153+
{
154+
CacheControlPolicy test{ L"public, max-age=77287, not-real" };
155+
REQUIRE(test.Present);
156+
REQUIRE(test.Public);
157+
REQUIRE(!test.NoCache);
158+
REQUIRE(!test.NoStore);
159+
REQUIRE(test.MaxAge == 77287);
160+
}
161+
SECTION("MaxAge Negative")
162+
{
163+
CacheControlPolicy test{ L"max-age=-1" };
164+
REQUIRE(test.MaxAge == CacheControlPolicy::MaximumMaxAge);
165+
}
166+
SECTION("MaxAge not a number")
167+
{
168+
CacheControlPolicy test{ L"max-age=FOO" };
169+
REQUIRE(test.MaxAge == 0);
170+
}
171+
}

src/AppInstallerCLITests/RestClient.cpp

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
#include "TestCommon.h"
55
#include "TestRestRequestHandler.h"
66
#include <Rest/RestClient.h>
7+
#include <Rest/RestInformationCache.h>
78
#include <Rest/Schema/IRestClient.h>
9+
#include <Rest/Schema/InformationResponseDeserializer.h>
810
#include <AppInstallerVersions.h>
911
#include <AppInstallerErrors.h>
1012
#include <AppInstallerRuntime.h>
@@ -488,3 +490,182 @@ TEST_CASE("RestClientCreate_1.7_Success", "[RestSource]")
488490
REQUIRE(information.Authentication.MicrosoftEntraIdInfo->Resource == "GUID");
489491
REQUIRE(information.Authentication.MicrosoftEntraIdInfo->Scope == "test");
490492
}
493+
494+
// Simulate the msstore cache round trip using real world data.
495+
TEST_CASE("RestInformationCache_RoundTrip", "[RestInformationCache]")
496+
{
497+
Settings::Stream{ Settings::Stream::RestInformationCache }.Remove();
498+
499+
std::wstring endpoint = L"https://test-url-com/information";
500+
CacheControlPolicy cacheControl{ L"public, max-age=77287" };
501+
auto response = web::json::value::parse(
502+
R"delimiter({
503+
"$type": "Microsoft.Marketplace.Storefront.StoreEdgeFD.BusinessLogic.Response.PackageMetadata.PackageMetadataResponse, StoreEdgeFD",
504+
"Data": {
505+
"$type": "Microsoft.Marketplace.Storefront.StoreEdgeFD.BusinessLogic.Response.PackageMetadata.PackageMetadataData, StoreEdgeFD",
506+
"SourceIdentifier": "StoreEdgeFD",
507+
"SourceAgreements": {
508+
"$type": "Microsoft.Marketplace.Storefront.StoreEdgeFD.BusinessLogic.Response.PackageMetadata.SourceAgreements, StoreEdgeFD",
509+
"AgreementsIdentifier": "StoreEdgeFD",
510+
"Agreements": [
511+
{
512+
"$type": "Microsoft.Marketplace.Storefront.StoreEdgeFD.BusinessLogic.Response.PackageManifest.AgreementDetail, StoreEdgeFD",
513+
"AgreementLabel": "Terms of Transaction",
514+
"AgreementUrl": "https://aka.ms/microsoft-store-terms-of-transaction"
515+
}
516+
]
517+
},
518+
"ServerSupportedVersions": [ "1.0.0", "1.1.0", "1.6.0" ],
519+
"RequiredQueryParameters": [ "market" ],
520+
"RequiredPackageMatchFields": [ "market" ]
521+
}
522+
})delimiter");
523+
524+
RestInformationCache cache;
525+
cache.Cache(endpoint, {}, {}, cacheControl, response);
526+
auto cachedValue = cache.Get(endpoint, {}, {});
527+
528+
REQUIRE(cachedValue.has_value());
529+
530+
InformationResponseDeserializer deserializer;
531+
const auto expected = deserializer.Deserialize(response);
532+
const auto& actual = cachedValue.value();
533+
534+
REQUIRE(expected.SourceIdentifier == actual.SourceIdentifier);
535+
REQUIRE(expected.SourceAgreementsIdentifier == actual.SourceAgreementsIdentifier);
536+
REQUIRE(expected.SourceAgreements.size() == actual.SourceAgreements.size());
537+
REQUIRE(1 == actual.SourceAgreements.size());
538+
REQUIRE(expected.SourceAgreements[0].Label == actual.SourceAgreements[0].Label);
539+
REQUIRE(expected.SourceAgreements[0].Text == actual.SourceAgreements[0].Text);
540+
REQUIRE(expected.SourceAgreements[0].Url == actual.SourceAgreements[0].Url);
541+
542+
REQUIRE(expected.ServerSupportedVersions.size() == actual.ServerSupportedVersions.size());
543+
for (const auto& expectedVersion : expected.ServerSupportedVersions)
544+
{
545+
REQUIRE(std::find(actual.ServerSupportedVersions.begin(), actual.ServerSupportedVersions.end(), expectedVersion) != actual.ServerSupportedVersions.end());
546+
}
547+
548+
REQUIRE(expected.RequiredQueryParameters.size() == actual.RequiredQueryParameters.size());
549+
REQUIRE(1 == actual.RequiredQueryParameters.size());
550+
REQUIRE(expected.RequiredQueryParameters[0] == actual.RequiredQueryParameters[0]);
551+
552+
REQUIRE(expected.RequiredPackageMatchFields.size() == actual.RequiredPackageMatchFields.size());
553+
REQUIRE(1 == actual.RequiredPackageMatchFields.size());
554+
REQUIRE(expected.RequiredPackageMatchFields[0] == actual.RequiredPackageMatchFields[0]);
555+
}
556+
557+
web::json::value CreateInformationResponse(std::string_view identifier)
558+
{
559+
std::ostringstream stream;
560+
stream << R"({ "Data": { "SourceIdentifier": ")" << identifier << R"(", "ServerSupportedVersions": [ "1.0.0" ] } })";
561+
562+
return web::json::value::parse(stream.str());
563+
}
564+
565+
TEST_CASE("RestInformationCache_Get", "[RestInformationCache]")
566+
{
567+
Settings::Stream{ Settings::Stream::RestInformationCache }.Remove();
568+
569+
std::wstring endpoint1 = L"https://test-url1-com/information";
570+
std::wstring endpoint2 = L"https://test-url2-com/information";
571+
std::wstring endpointNotPresent = L"https://test-url-not-present-com/information";
572+
std::string header = "Header";
573+
std::string caller = "Caller";
574+
std::string publicEndpoint1Identifier = "Identifier1";
575+
std::string privateEndpoint1Identifier = "Identifier2";
576+
std::string privateEndpoint2Identifier = "Identifier3";
577+
auto publicEndpoint1Response = CreateInformationResponse(publicEndpoint1Identifier);
578+
auto privateEndpoint1Response = CreateInformationResponse(privateEndpoint1Identifier);
579+
auto privateEndpoint2Response = CreateInformationResponse(privateEndpoint2Identifier);
580+
581+
RestInformationCache cache;
582+
583+
// Cache:
584+
// 1. public and private for same endpoint
585+
cache.Cache(endpoint1, header, caller, { L"public" }, publicEndpoint1Response);
586+
cache.Cache(endpoint1, header, caller, {}, privateEndpoint1Response);
587+
// 2. another endpoint with private data (same headers)
588+
cache.Cache(endpoint2, header, caller, {}, privateEndpoint2Response);
589+
590+
SECTION("Same headers prefers private")
591+
{
592+
auto cachedValue = cache.Get(endpoint1, header, caller);
593+
REQUIRE(cachedValue.has_value());
594+
REQUIRE(privateEndpoint1Identifier == cachedValue->SourceIdentifier);
595+
}
596+
SECTION("Different headers falls back to public")
597+
{
598+
auto cachedValue = cache.Get(endpoint1, "Different", "Different");
599+
REQUIRE(cachedValue.has_value());
600+
REQUIRE(publicEndpoint1Identifier == cachedValue->SourceIdentifier);
601+
}
602+
SECTION("Second endpoint")
603+
{
604+
auto cachedValue = cache.Get(endpoint2, header, caller);
605+
REQUIRE(cachedValue.has_value());
606+
REQUIRE(privateEndpoint2Identifier == cachedValue->SourceIdentifier);
607+
}
608+
SECTION("Second endpoint different headers")
609+
{
610+
auto cachedValue = cache.Get(endpoint2, {}, {});
611+
REQUIRE(!cachedValue.has_value());
612+
}
613+
SECTION("Missing endpoint")
614+
{
615+
auto cachedValue = cache.Get(endpointNotPresent, header, caller);
616+
REQUIRE(!cachedValue.has_value());
617+
}
618+
}
619+
620+
TEST_CASE("RestInformationCache_Cache_NoStore", "[RestInformationCache]")
621+
{
622+
Settings::Stream{ Settings::Stream::RestInformationCache }.Remove();
623+
624+
std::wstring endpoint = L"https://test-url-com/information";
625+
626+
RestInformationCache cache;
627+
cache.Cache(endpoint, {}, {}, { L"no-store" }, CreateInformationResponse("Identifier"));
628+
629+
auto cachedValue = cache.Get(endpoint, {}, {});
630+
REQUIRE(!cachedValue.has_value());
631+
}
632+
633+
TEST_CASE("RestInformationCache_Cache_Expiration", "[RestInformationCache]")
634+
{
635+
Settings::Stream{ Settings::Stream::RestInformationCache }.Remove();
636+
637+
std::wstring endpoint = L"https://test-url-com/information";
638+
639+
RestInformationCache cache;
640+
cache.Cache(endpoint, {}, {}, { L"max-age=2" }, CreateInformationResponse("Identifier"));
641+
642+
auto cachedValue = cache.Get(endpoint, {}, {});
643+
REQUIRE(cachedValue.has_value());
644+
645+
std::this_thread::sleep_for(5s);
646+
647+
cachedValue = cache.Get(endpoint, {}, {});
648+
REQUIRE(!cachedValue.has_value());
649+
}
650+
651+
TEST_CASE("RestInformationCache_Cache_Overwrite", "[RestInformationCache]")
652+
{
653+
Settings::Stream{ Settings::Stream::RestInformationCache }.Remove();
654+
655+
std::wstring endpoint = L"https://test-url-com/information";
656+
std::string identifier1 = "Identifier1";
657+
std::string identifier2 = "Identifier2";
658+
659+
RestInformationCache cache;
660+
cache.Cache(endpoint, {}, {}, {}, CreateInformationResponse(identifier1));
661+
662+
auto cachedValue = cache.Get(endpoint, {}, {});
663+
REQUIRE(cachedValue.has_value());
664+
REQUIRE(identifier1 == cachedValue->SourceIdentifier);
665+
666+
cache.Cache(endpoint, {}, {}, {}, CreateInformationResponse(identifier2));
667+
668+
cachedValue = cache.Get(endpoint, {}, {});
669+
REQUIRE(cachedValue.has_value());
670+
REQUIRE(identifier2 == cachedValue->SourceIdentifier);
671+
}

src/AppInstallerCLITests/Settings.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,3 +281,29 @@ TEST_CASE("AttemptSetOnNewValue", "[settings]")
281281
settingValue = ReadEntireStream(*result);
282282
REQUIRE(value2 == settingValue);
283283
}
284+
285+
TEST_CASE("SetAndReadSettingEncrypted", "[settings]")
286+
{
287+
StreamDefinition name{ Type::Encrypted, "encrypted_test_setting" };
288+
std::string value = "This is the test setting value";
289+
290+
Stream stream{ name };
291+
REQUIRE(stream.Set(value));
292+
293+
auto result = stream.Get();
294+
REQUIRE(static_cast<bool>(result));
295+
296+
std::string settingValue = ReadEntireStream(*result);
297+
REQUIRE(value == settingValue);
298+
299+
// Ensure that the data is encrypted
300+
name.Type = Type::StandardFile;
301+
302+
Stream streamDirect{ name };
303+
304+
auto resultDirect = streamDirect.Get();
305+
REQUIRE(static_cast<bool>(resultDirect));
306+
307+
std::string settingValueDirect = ReadEntireStream(*resultDirect);
308+
REQUIRE(value != settingValueDirect);
309+
}

src/AppInstallerCLITests/Strings.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ TEST_CASE("Trim", "[strings]")
117117
REQUIRE(Trim(str.assign("Multiple words")) == "Multiple words");
118118
REQUIRE(Trim(str.assign(" Multiple words")) == "Multiple words");
119119
REQUIRE(Trim(str.assign("Much after is taken \f\n\r\t\v\v\t\r\n\f ")) == "Much after is taken");
120+
121+
REQUIRE(Trim(L" Test") == L"Test");
122+
REQUIRE(Trim(L" ") == L"");
120123
}
121124

122125
TEST_CASE("CaseInsensitiveStartsWith", "[strings]")
@@ -297,6 +300,20 @@ TEST_CASE("SplitWithSeparator", "[strings]")
297300
REQUIRE(test4.size() == 2);
298301
REQUIRE(test4[0] == "trim");
299302
REQUIRE(test4[1] == "spaces");
303+
304+
std::vector<std::wstring_view> test5 = Split(L" trim /spaces / ", '/', true);
305+
REQUIRE(test5.size() == 3);
306+
REQUIRE(test5[0] == L"trim");
307+
REQUIRE(test5[1] == L"spaces");
308+
REQUIRE(test5[2] == L"");
309+
310+
std::vector<std::wstring_view> test6 = Split(L" ", '/', true);
311+
REQUIRE(test6.size() == 1);
312+
REQUIRE(test6[0] == L"");
313+
314+
std::vector<std::string> test7 = Split("", ';');
315+
REQUIRE(test7.size() == 1);
316+
REQUIRE(test7[0] == "");
300317
}
301318

302319
TEST_CASE("ConvertGuid", "[strings]")

src/AppInstallerCLITests/TestRestRequestHandler.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ std::shared_ptr<TestRestRequestHandler> GetTestRestRequestHandler(
2323
}
2424

2525
response.headers().set_content_type(mimeType);
26+
response.headers().set_cache_control(L"no-store");
2627
response.set_status_code(statusCode);
2728
return pplx::task_from_result(response);
2829
});
@@ -38,6 +39,7 @@ std::shared_ptr<TestRestRequestHandler> GetTestRestRequestHandler(
3839
response.set_body(utf16string{});
3940

4041
response.headers().set_content_type(web::http::details::mime_types::application_json);
42+
response.headers().set_cache_control(L"no-store");
4143
response.set_status_code(handler(request));
4244
return pplx::task_from_result(response);
4345
});
@@ -65,6 +67,7 @@ std::shared_ptr<TestRestRequestHandler> GetHeaderVerificationHandler(
6567
}
6668

6769
response.headers().set_content_type(web::http::details::mime_types::application_json);
70+
response.headers().set_cache_control(L"no-store");
6871
response.set_status_code(statusCode);
6972
return pplx::task_from_result(response);
7073
});

0 commit comments

Comments
 (0)