Skip to content

Commit

Permalink
Shared Storage: Migrate database to have a per-key last-write time.
Browse files Browse the repository at this point in the history
We now intend to use per-key expiration instead of database
expiration.

This change performs a database migration to include a new column `last_used_time` in `values_mapping`. (We do not make the new
column name more specific to writes in case we decide in future to
update it on other accesses.)

We also take the opportunity to rename `per_origin_mapping`'s
`last_used_time` column to `creation_time`, to better reflect what it
is now being used for.

Follow-up(s) will implement the new expiration process based on the
new per-key `last_used_time`.

Bug: 1218540,1385197
Change-Id: Iadb50314d637a6ae933c9d7239c578aeccf44ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4029459
Reviewed-by: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Commit-Queue: Cammie Smith Barnes <cammie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1075335}
  • Loading branch information
pythagoraskitty authored and Chromium LUCI CQ committed Nov 23, 2022
1 parent 859c521 commit 043ce5f
Show file tree
Hide file tree
Showing 20 changed files with 1,216 additions and 414 deletions.
20 changes: 12 additions & 8 deletions components/services/storage/BUILD.gn
Expand Up @@ -74,6 +74,8 @@ source_set("storage") {
"shared_storage/async_shared_storage_database_impl.h",
"shared_storage/shared_storage_database.cc",
"shared_storage/shared_storage_database.h",
"shared_storage/shared_storage_database_migrations.cc",
"shared_storage/shared_storage_database_migrations.h",
"shared_storage/shared_storage_manager.cc",
"shared_storage/shared_storage_manager.h",
"shared_storage/shared_storage_options.cc",
Expand Down Expand Up @@ -160,16 +162,17 @@ bundle_data("tests_bundle_data") {
visibility = [ ":tests" ]
testonly = true
sources = [
"//components/test/data/storage/shared_storage.v0.init_too_old.sql",
"//components/test/data/storage/shared_storage.v1.empty_values_mapping.5origins.sql",
"//components/test/data/storage/shared_storage.v1.empty_values_mapping.6origins.sql",
"//components/test/data/storage/shared_storage.v1.empty_values_mapping.7origins.sql",
"//components/test/data/storage/shared_storage.v1.empty_values_mapping.8origins.sql",
"//components/test/data/storage/shared_storage.v1.init_too_new.sql",
"//components/test/data/storage/shared_storage.v1.iterator.sql",
"//components/test/data/storage/shared_storage.init_too_new.sql",
"//components/test/data/storage/shared_storage.v0.sql",
"//components/test/data/storage/shared_storage.v1.no_budget_table.sql",
"//components/test/data/storage/shared_storage.v1.single_origin.sql",
"//components/test/data/storage/shared_storage.v1.sql",
"//components/test/data/storage/shared_storage.v2.empty_values_mapping.5origins.sql",
"//components/test/data/storage/shared_storage.v2.empty_values_mapping.6origins.sql",
"//components/test/data/storage/shared_storage.v2.empty_values_mapping.7origins.sql",
"//components/test/data/storage/shared_storage.v2.empty_values_mapping.8origins.sql",
"//components/test/data/storage/shared_storage.v2.iterator.sql",
"//components/test/data/storage/shared_storage.v2.single_origin.sql",
"//components/test/data/storage/shared_storage.v2.sql",
]
outputs = [ "{{bundle_resources_dir}}/" +
"{{source_root_relative_dir}}/{{source_file_part}}" ]
Expand Down Expand Up @@ -203,6 +206,7 @@ source_set("tests") {
"service_worker/service_worker_storage_test_utils.h",
"service_worker/service_worker_storage_unittest.cc",
"shared_storage/async_shared_storage_database_impl_unittest.cc",
"shared_storage/shared_storage_database_migrations_unittest.cc",
"shared_storage/shared_storage_database_unittest.cc",
"shared_storage/shared_storage_manager_unittest.cc",
"storage_service_impl_unittest.cc",
Expand Down
Expand Up @@ -106,7 +106,7 @@ class AsyncSharedStorageDatabaseImplTest : public testing::Test {
// Return the relative file path in the "storage/" subdirectory of test data
// for the SQL file from which to initialize an async shared storage database
// instance.
virtual const char* GetRelativeFilePath() { return nullptr; }
virtual std::string GetRelativeFilePath() { return nullptr; }

std::unique_ptr<AsyncSharedStorageDatabase> Create() {
if (GetType() != DBType::kInMemory)
Expand Down Expand Up @@ -663,16 +663,19 @@ class AsyncSharedStorageDatabaseImplTest : public testing::Test {
base::SimpleTestClock clock_;
};

class AsyncSharedStorageDatabaseImplFromFileV1Test
class AsyncSharedStorageDatabaseImplFromFileTest
: public AsyncSharedStorageDatabaseImplTest {
public:
DBType GetType() override { return DBType::kFileBackedFromExisting; }

const char* GetRelativeFilePath() override { return "shared_storage.v1.sql"; }
std::string GetRelativeFilePath() override {
return GetTestFileNameForCurrentVersion();
}
};

// Test loading version 1 database.
TEST_F(AsyncSharedStorageDatabaseImplFromFileV1Test, Version1_LoadFromFile) {
// Test loading current version database.
TEST_F(AsyncSharedStorageDatabaseImplFromFileTest,
CurrentVersion_LoadFromFile) {
ASSERT_TRUE(async_database_);

// Override the clock and set to the last time in the file that is used to
Expand Down Expand Up @@ -709,9 +712,9 @@ TEST_F(AsyncSharedStorageDatabaseImplFromFileV1Test, Version1_LoadFromFile) {
}

class AsyncSharedStorageDatabaseImplFromFileV1NoBudgetTableTest
: public AsyncSharedStorageDatabaseImplFromFileV1Test {
: public AsyncSharedStorageDatabaseImplFromFileTest {
public:
const char* GetRelativeFilePath() override {
std::string GetRelativeFilePath() override {
return "shared_storage.v1.no_budget_table.sql";
}
};
Expand Down Expand Up @@ -749,14 +752,14 @@ TEST_F(AsyncSharedStorageDatabaseImplFromFileV1NoBudgetTableTest,
}

struct InitFailureTestCase {
const char* relative_file_path;
std::string relative_file_path;
InitStatus expected_status;
};

std::vector<InitFailureTestCase> GetInitFailureTestCases() {
return std::vector<InitFailureTestCase>(
{{"shared_storage.v1.init_too_new.sql", InitStatus::kTooNew},
{"shared_storage.v0.init_too_old.sql", InitStatus::kTooOld}});
{{"shared_storage.init_too_new.sql", InitStatus::kTooNew},
{GetTestFileNameForLatestDeprecatedVersion(), InitStatus::kTooOld}});
}

// Used by `testing::PrintToStringParamName()`.
Expand All @@ -775,7 +778,7 @@ class AsyncSharedStorageDatabaseImplFromFileWithFailureTest
public:
DBType GetType() override { return DBType::kFileBackedFromExisting; }

const char* GetRelativeFilePath() override {
std::string GetRelativeFilePath() override {
return GetParam().relative_file_path;
}
};
Expand All @@ -785,8 +788,7 @@ INSTANTIATE_TEST_SUITE_P(All,
testing::ValuesIn(GetInitFailureTestCases()),
testing::PrintToStringParamName());

TEST_P(AsyncSharedStorageDatabaseImplFromFileWithFailureTest,
Version1_Destroy) {
TEST_P(AsyncSharedStorageDatabaseImplFromFileWithFailureTest, Destroy) {
ASSERT_TRUE(async_database_);

// Call an operation so that the database will attempt to be lazy-initialized.
Expand Down

0 comments on commit 043ce5f

Please sign in to comment.