Commit e894980c authored by Vasilii Sukhanov's avatar Vasilii Sukhanov Committed by Commit Bot

Allow foreign key creation from SQLTableBuilder.

The builder will be able to create a foreign key and migrate the child
table properly.

Bug: 1137775
Change-Id: I8ce7a979e96761bc2dcd74329c101d4318662da8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2552918
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830602}
parent a8172527
......@@ -73,7 +73,7 @@ bool operator==(const CompromisedCredentials& lhs,
}
void CompromisedCredentialsTable::Init(sql::Database* db) {
db_ = db;
db_ = db;
}
bool CompromisedCredentialsTable::CreateTableIfNecessary() {
......
......@@ -22,6 +22,9 @@ namespace password_manager {
namespace {
// Implicit index name create for faster lookups by a foreign key.
constexpr char kForeignKeyIndex[] = "foreign_key_index";
// Appends |name| to |list_of_names|, separating items with ", ".
void Append(const std::string& name, std::string* list_of_names) {
if (list_of_names->empty())
......@@ -43,6 +46,23 @@ bool CheckForeignKeyConstraints(sql::Database& db) {
}
return ret;
}
std::string ComputeColumnSuffix(bool is_primary_key,
const std::string& parent_table) {
DCHECK(!is_primary_key || parent_table.empty());
if (is_primary_key) {
return " PRIMARY KEY AUTOINCREMENT";
}
std::string suffix;
if (!parent_table.empty()) {
DCHECK(!is_primary_key);
suffix += " REFERENCES ";
suffix += parent_table;
suffix += " ON UPDATE CASCADE ON DELETE CASCADE";
}
return suffix;
}
} // namespace
// static
......@@ -51,6 +71,9 @@ unsigned constexpr SQLTableBuilder::kInvalidVersion;
struct SQLTableBuilder::Column {
std::string name;
std::string type;
// If not empty, the column is a foreign key to the primary key of
// 'parent_table'.
std::string parent_table;
// Whether this column is the table's PRIMARY KEY.
bool is_primary_key;
// Whether this column is part of the table's UNIQUE constraint.
......@@ -89,6 +112,7 @@ SQLTableBuilder::~SQLTableBuilder() = default;
void SQLTableBuilder::AddColumn(std::string name, std::string type) {
DCHECK(FindLastColumnByName(name) == columns_.rend());
columns_.push_back({std::move(name), std::move(type),
/*parent_table=*/std::string(),
/*is_primary_key=*/false, /*part_of_unique_key=*/false,
sealed_version_ + 1, kInvalidVersion,
/*gets_previous_data=*/false});
......@@ -102,9 +126,15 @@ void SQLTableBuilder::AddPrimaryKeyColumn(std::string name) {
columns_.back().is_primary_key = true;
}
void SQLTableBuilder::AddColumnToUniqueKey(std::string name, std::string type) {
void SQLTableBuilder::AddColumnToUniqueKey(std::string name,
std::string type,
std::string parent_table) {
AddColumn(std::move(name), std::move(type));
columns_.back().part_of_unique_key = true;
if (!parent_table.empty()) {
columns_.back().parent_table = std::move(parent_table);
AddIndex(kForeignKeyIndex, {columns_.back().name});
}
}
void SQLTableBuilder::RenameColumn(const std::string& old_name,
......@@ -120,6 +150,9 @@ void SQLTableBuilder::RenameColumn(const std::string& old_name,
return index.max_version == kInvalidVersion &&
base::Contains(index.columns, old_name);
}));
// Migrating a foreign key can be supported but the index on it is to be
// updated.
DCHECK_EQ(old_column->parent_table, std::string());
if (sealed_version_ != kInvalidVersion &&
old_column->min_version <= sealed_version_) {
......@@ -127,6 +160,7 @@ void SQLTableBuilder::RenameColumn(const std::string& old_name,
// just replaced, it needs to be kept for generating the migration code.
Column new_column = {new_name,
old_column->type,
old_column->parent_table,
old_column->is_primary_key,
old_column->part_of_unique_key,
sealed_version_ + 1,
......@@ -237,10 +271,9 @@ bool SQLTableBuilder::CreateTable(sql::Database* db) const {
std::string names; // Names and types of the current columns.
for (const Column& column : columns_) {
if (IsColumnInLastVersion(column)) {
std::string suffix;
if (column.is_primary_key)
suffix = " PRIMARY KEY AUTOINCREMENT";
Append(column.name + " " + column.type + suffix, &names);
std::string suffix =
ComputeColumnSuffix(column.is_primary_key, column.parent_table);
Append(column.name + " " + column.type + std::move(suffix), &names);
}
}
......@@ -353,6 +386,7 @@ bool SQLTableBuilder::MigrateToNextFrom(unsigned old_version,
if (next_column != columns_.end() && next_column->gets_previous_data) {
// (1) The column is being renamed.
DCHECK_EQ(column->type, next_column->type);
DCHECK_EQ(column->parent_table, next_column->parent_table);
DCHECK_NE(column->name, next_column->name);
Append(column->name, &old_names_of_existing_columns_without_types);
Append(next_column->name + " " + next_column->type,
......@@ -366,24 +400,22 @@ bool SQLTableBuilder::MigrateToNextFrom(unsigned old_version,
// This column was added after old_version.
if (column->is_primary_key || column->part_of_unique_key)
needs_temp_table = true;
std::string suffix;
if (column->is_primary_key) {
suffix = " PRIMARY KEY AUTOINCREMENT";
if (column->is_primary_key)
has_primary_key = true;
}
std::string suffix =
ComputeColumnSuffix(column->is_primary_key, column->parent_table);
names_of_new_columns_list.push_back(column->name + " " + column->type +
suffix);
std::move(suffix));
} else if (column->min_version <= old_version &&
(column->max_version == kInvalidVersion ||
column->max_version > old_version)) {
std::string suffix;
if (column->is_primary_key) {
suffix = " PRIMARY KEY AUTOINCREMENT";
if (column->is_primary_key)
has_primary_key = true;
}
std::string suffix =
ComputeColumnSuffix(column->is_primary_key, column->parent_table);
// This column stays.
Append(column->name, &old_names_of_existing_columns_without_types);
Append(column->name + " " + column->type + suffix,
Append(column->name + " " + column->type + std::move(suffix),
&new_names_of_existing_columns);
Append(column->name, &new_names_of_existing_columns_without_types);
}
......
......@@ -65,7 +65,12 @@ class SQLTableBuilder {
void AddPrimaryKeyColumn(std::string name);
// As AddColumn but also adds column |name| to the unique key of the table.
void AddColumnToUniqueKey(std::string name, std::string type);
// If 'parent_table' isn't empty then the column is a foreign key to
// 'parent_table' and an implicit index is created. Only one foreign key per
// table is allowed.
void AddColumnToUniqueKey(std::string name,
std::string type,
std::string parent_table = std::string());
// Renames column |old_name| to |new_name|. |new_name| can not exist already.
// |old_name| must have been added in the past. Furthermore, there must be no
......
......@@ -33,6 +33,7 @@ using TableRow = std::vector<ColumnValue>;
void CheckTableContent(sql::Database& db,
const char table_name[],
const std::vector<TableRow>& expected_rows) {
SCOPED_TRACE(testing::Message() << "table_name = " << table_name);
sql::Statement table_check(db.GetUniqueStatement(
base::StringPrintf("SELECT * FROM %s", table_name).c_str()));
for (const TableRow& row : expected_rows) {
......@@ -53,7 +54,9 @@ void CheckTableContent(sql::Database& db,
class SQLTableBuilderTest : public testing::Test {
public:
SQLTableBuilderTest() : builder_(kMyLoginTable) { Init(); }
SQLTableBuilderTest() : builder_(kMyLoginTable), child_builder_(kChildTable) {
Init();
}
~SQLTableBuilderTest() override = default;
......@@ -69,6 +72,7 @@ class SQLTableBuilderTest : public testing::Test {
sql::Database* db() { return &db_; }
SQLTableBuilder* builder() { return &builder_; }
SQLTableBuilder* child_builder() { return &child_builder_; }
private:
// Part of constructor, needs to be a void-returning function to use ASSERTs.
......@@ -80,6 +84,7 @@ class SQLTableBuilderTest : public testing::Test {
sql::Database db_;
SQLTableBuilder builder_;
SQLTableBuilder child_builder_;
};
bool SQLTableBuilderTest::IsColumnOfType(const std::string& name,
......@@ -93,12 +98,11 @@ void SQLTableBuilderTest::SetupChildTable() {
EXPECT_EQ(0u, builder()->SealVersion());
EXPECT_TRUE(builder()->CreateTable(db()));
EXPECT_TRUE(
db()->Execute(base::StringPrintf("CREATE TABLE %s (name TEXT, "
"parent_id INTEGER REFERENCES %s ON "
"UPDATE CASCADE ON DELETE CASCADE)",
kChildTable, kMyLoginTable)
.c_str()));
child_builder_.AddColumn("name", "TEXT");
child_builder_.AddColumnToUniqueKey("parent_id", "INTEGER",
/*parent_table=*/kMyLoginTable);
EXPECT_EQ(0u, child_builder_.SealVersion());
EXPECT_TRUE(child_builder_.CreateTable(db()));
}
void SQLTableBuilderTest::Init() {
......@@ -495,5 +499,80 @@ TEST_F(SQLTableBuilderTest, MigrateFrom_WithForeignKey_PreventMigration) {
EXPECT_FALSE(builder()->MigrateFrom(1, db()));
}
TEST_F(SQLTableBuilderTest, MigrateFrom_WithForeignKey_ChildTable_AddColumn) {
SetupChildTable();
EXPECT_TRUE(db()->Execute(
base::StringPrintf("INSERT INTO %s (signon_realm) VALUES ('abc.com')",
kMyLoginTable)
.c_str()));
EXPECT_TRUE(db()->Execute(
base::StringPrintf(
"INSERT INTO %s (name, parent_id) VALUES ('Abc Co.', 1)", kChildTable)
.c_str()));
child_builder()->AddColumn("new_column", "TEXT");
EXPECT_EQ(1u, child_builder()->SealVersion());
EXPECT_TRUE(child_builder()->MigrateFrom(0, db()));
EXPECT_TRUE(db()->Execute(
base::StringPrintf("UPDATE %s SET new_column='value' WHERE parent_id=1",
kChildTable)
.c_str()));
CheckTableContent(*db(), kChildTable, {{"Abc Co.", 1, "value"}});
CheckTableContent(*db(), kMyLoginTable, {{"abc.com", 1}});
// The foreign key still works.
EXPECT_FALSE(db()->Execute(
"INSERT INTO child_table (name, parent_id) VALUES ('Co.', 15)"));
}
TEST_F(SQLTableBuilderTest,
MigrateFrom_WithForeignKey_ChildTable_RenameColumn) {
SetupChildTable();
EXPECT_TRUE(db()->Execute(
base::StringPrintf("INSERT INTO %s (signon_realm) VALUES ('abc.com')",
kMyLoginTable)
.c_str()));
EXPECT_TRUE(db()->Execute(
base::StringPrintf(
"INSERT INTO %s (name, parent_id) VALUES ('Abc Co.', 1)", kChildTable)
.c_str()));
child_builder()->RenameColumn("name", "new_name");
EXPECT_EQ(1u, child_builder()->SealVersion());
EXPECT_TRUE(child_builder()->MigrateFrom(0, db()));
CheckTableContent(*db(), kChildTable, {{"Abc Co.", 1}});
CheckTableContent(*db(), kMyLoginTable, {{"abc.com", 1}});
// The foreign key still works.
EXPECT_FALSE(db()->Execute(
"INSERT INTO child_table (new_name, parent_id) VALUES ('Co.', 15)"));
}
TEST_F(SQLTableBuilderTest, MigrateFrom_WithForeignKey_ChildTable_DropColumn) {
SetupChildTable();
EXPECT_TRUE(db()->Execute(
base::StringPrintf("INSERT INTO %s (signon_realm) VALUES ('abc.com')",
kMyLoginTable)
.c_str()));
EXPECT_TRUE(db()->Execute(
base::StringPrintf(
"INSERT INTO %s (name, parent_id) VALUES ('Abc Co.', 1)", kChildTable)
.c_str()));
child_builder()->DropColumn("name");
EXPECT_EQ(1u, child_builder()->SealVersion());
EXPECT_TRUE(child_builder()->MigrateFrom(0, db()));
CheckTableContent(*db(), kChildTable, {{1}});
CheckTableContent(*db(), kMyLoginTable, {{"abc.com", 1}});
// The foreign key still works.
EXPECT_FALSE(
db()->Execute("INSERT INTO child_table (parent_id) VALUES (15)"));
}
} // namespace
} // namespace password_manager
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment