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.

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

Settings streams exchange semantics #1534

Merged
merged 9 commits into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/AppInstallerCLITests/CustomHeader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ TEST_CASE("RestClient_CustomHeader", "[RestSource][CustomHeader]")

TEST_CASE("AddSource_CustomHeader", "[RestSource][CustomHeader]")
{
SetSetting(Streams::UserSources, s_EmptySources);
SetSetting(Stream::UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();

std::string customHeader = "Testing custom header with open source";
Expand All @@ -111,7 +111,7 @@ TEST_CASE("AddSource_CustomHeader", "[RestSource][CustomHeader]")

TEST_CASE("CreateSource_CustomHeader", "[RestSource][CustomHeader]")
{
SetSetting(Streams::UserSources, s_EmptySources);
SetSetting(Stream::UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();

std::string customHeader = "Testing custom header with open source";
Expand All @@ -138,7 +138,7 @@ TEST_CASE("CreateSource_CustomHeader", "[RestSource][CustomHeader]")

TEST_CASE("CreateSource_CustomHeaderNotApplicable", "[RestSource][CustomHeader]")
{
SetSetting(Streams::UserSources, s_EmptySources);
SetSetting(Stream::UserSources, s_EmptySources);
TestHook_ClearSourceFactoryOverrides();

std::string customHeader = "Testing custom header with open source";
Expand Down
8 changes: 4 additions & 4 deletions src/AppInstallerCLITests/ExperimentalFeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,23 +39,23 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]")
SECTION("Feature on")
{
std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })";
SetSetting(Streams::PrimaryUserSettings, json);
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest));
}
SECTION("Feature off")
{
std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": false } })";
SetSetting(Streams::PrimaryUserSettings, json);
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest));
}
SECTION("Invalid value")
{
std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": "string" } })";
SetSetting(Streams::PrimaryUserSettings, json);
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest));
Expand All @@ -67,7 +67,7 @@ TEST_CASE("ExperimentalFeature ExperimentalCmd", "[experimentalFeature]")
GroupPolicyTestOverride policies{ policiesKey.get() };

std::string_view json = R"({ "experimentalFeatures": { "experimentalCmd": true } })";
SetSetting(Streams::PrimaryUserSettings, json);
SetSetting(Stream::PrimaryUserSettings, json);
UserSettingsTest userSettingTest;

REQUIRE_FALSE(ExperimentalFeature::IsEnabled(ExperimentalFeature::Feature::ExperimentalCmd, userSettingTest));
Expand Down
5 changes: 3 additions & 2 deletions src/AppInstallerCLITests/PreIndexedPackageSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.
#include "pch.h"
#include "TestCommon.h"
#include "TestSettings.h"
#include <AppInstallerRepositorySource.h>
#include <AppInstallerRuntime.h>
#include <AppInstallerStrings.h>
Expand Down Expand Up @@ -56,8 +57,8 @@ std::string GetContents(const fs::path& file)

void CleanSources()
{
RemoveSetting(Streams::UserSources);
RemoveSetting(Streams::SourcesMetadata);
RemoveSetting(Stream::UserSources);
RemoveSetting(Stream::SourcesMetadata);
fs::remove_all(GetPathToFileDir());
}

Expand Down
160 changes: 132 additions & 28 deletions src/AppInstallerCLITests/Settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ TEST_CASE("ReadEmptySetting", "[settings]")
{
StreamDefinition name{ Type::Standard, "nonexistentsetting" };

auto result = GetSettingStream(name);
auto result = Stream{ name }.Get();
REQUIRE(!result);
}

Expand All @@ -24,9 +24,10 @@ TEST_CASE("SetAndReadSetting", "[settings]")
StreamDefinition name{ Type::Standard, "testsettingname" };
std::string value = "This is the test setting value";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -38,9 +39,10 @@ TEST_CASE("SetAndReadSettingInContainer", "[settings]")
StreamDefinition name{ Type::Standard, "testcontainer/testsettingname" };
std::string value = "This is the test setting value from inside a container";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -52,19 +54,20 @@ TEST_CASE("RemoveSetting", "[settings]")
StreamDefinition name{ Type::Standard, "testsettingname" };
std::string value = "This is the test setting value to be removed";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

{
auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value == settingValue);
}

RemoveSetting( name);
stream.Remove();

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(!static_cast<bool>(result));
}

Expand All @@ -73,9 +76,10 @@ TEST_CASE("SetAndReadUserFileSetting", "[settings]")
StreamDefinition name{ Type::UserFile, "userfilesetting" };
std::string value = "This is the test setting value for a user file";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -86,7 +90,7 @@ TEST_CASE("ReadEmptySecureSetting", "[settings]")
{
StreamDefinition name{ Type::Secure, "secure_nonexistentsetting" };

auto result = GetSettingStream(name);
auto result = Stream{ name }.Get();
REQUIRE(!result);
}

Expand All @@ -95,9 +99,10 @@ TEST_CASE("SetAndReadSecureSetting", "[settings]")
StreamDefinition name{ Type::Secure, "secure_testsettingname" };
std::string value = "This is the test setting value";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -109,9 +114,10 @@ TEST_CASE("SetAndReadSecureSettingInContainer", "[settings]")
StreamDefinition name{ Type::Secure, "testcontainer/secure_testsettingname" };
std::string value = "This is the test setting value from inside a container";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -123,19 +129,20 @@ TEST_CASE("RemoveSecureSetting", "[settings]")
StreamDefinition name{ Type::Secure, "secure_testsettingname" };
std::string value = "This is the test setting value to be removed";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

{
auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value == settingValue);
}

RemoveSetting(name);
stream.Remove();

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(!static_cast<bool>(result));
}

Expand All @@ -144,27 +151,29 @@ TEST_CASE("SetAndReadSecureSetting_SecureDataRemoved", "[settings]")
StreamDefinition name{ Type::Secure, "secure_testsettingname" };
std::string value = "This is the test setting value";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value == settingValue);

std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Path);
std::filesystem::remove(GetPathTo(PathName::SecureSettings) / name.Name);

REQUIRE_THROWS_HR(GetSettingStream(name), SPAPI_E_FILE_HASH_NOT_IN_CATALOG);
REQUIRE_THROWS_HR(stream.Get(), SPAPI_E_FILE_HASH_NOT_IN_CATALOG);
}

TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]")
{
StreamDefinition name{ Type::Secure, "secure_testsettingname" };
std::string value = "This is the test setting value";

SetSetting(name, value);
Stream stream{ name };
REQUIRE(stream.Set(value));

auto result = GetSettingStream(name);
auto result = stream.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
Expand All @@ -173,7 +182,102 @@ TEST_CASE("SetAndReadSecureSetting_DataTampered", "[settings]")
StreamDefinition insecureName = name;
insecureName.Type = Type::Standard;

SetSetting(insecureName, "Tampered data");
REQUIRE(Stream{ insecureName }.Set("Tampered data"));

REQUIRE_THROWS_HR(GetSettingStream(name), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR));
REQUIRE_THROWS_HR(stream.Get(), HRESULT_FROM_WIN32(ERROR_DATA_CHECKSUM_ERROR));
}

TEST_CASE("SetChangeAndReadSetting", "[settings]")
{
StreamDefinition name{ Type::Standard, "testsettingname" };
std::string value1 = "This is the test setting value1";
std::string value2 = "This is the test setting value2, which is different";
std::string value3 = "This is the test setting value3; also different";

name.Type = GENERATE(Type::Standard, Type::Secure);
INFO(ToString(name.Type));

// Set the value on stream 1
Stream stream1{ name };
REQUIRE(stream1.Set(value1));

// Read the value on stream 2 to verify
{
Stream stream2{ name };

auto result = stream2.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value1 == settingValue);

// Set the value on stream 2
REQUIRE(stream2.Set(value2));
}

// Attempt to set the value on stream 1 again
REQUIRE(!stream1.Set(value3));

// Attempting to set again should still not work
REQUIRE(!stream1.Set(value3));

// Ensure that the value remains value 2
auto result = stream1.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value2 == settingValue);

// Now that we have read it, we can update it
REQUIRE(stream1.Set(value3));

result = stream1.Get();
REQUIRE(static_cast<bool>(result));

settingValue = ReadEntireStream(*result);
REQUIRE(value3 == settingValue);
}

TEST_CASE("AttemptSetOnNewValue", "[settings]")
{
StreamDefinition name{ Type::Standard, "testsettingname" };
std::string value1 = "This is the test setting value1";
std::string value2 = "This is the test setting value2, which is different";

name.Type = GENERATE(Type::Standard, Type::Secure);
INFO(ToString(name.Type));

// Remove the stream
Stream{ name }.Remove();

Stream stream1{ name };
REQUIRE(!stream1.Get());

// Set the value on stream 2
{
Stream stream2{ name };
REQUIRE(stream2.Set(value1));
}

// Attempt to set the value on stream 1 again
REQUIRE(!stream1.Set(value2));

// Attempting to set again should still not work
REQUIRE(!stream1.Set(value2));

// Ensure that the value remains value 2
auto result = stream1.Get();
REQUIRE(static_cast<bool>(result));

std::string settingValue = ReadEntireStream(*result);
REQUIRE(value1 == settingValue);

// Now that we have read it, we can update it
REQUIRE(stream1.Set(value2));

result = stream1.Get();
REQUIRE(static_cast<bool>(result));

settingValue = ReadEntireStream(*result);
REQUIRE(value2 == settingValue);
}
Loading