Michael, sending to you since you reviewed the last round of changes to this
file.
~~~~~
Hello michaeln,
I'd like you to do a code review. Please execute
g4 diff -c 8116546
or point your web browser to
http://mondrian/8116546
(this changelist has been uploaded to Mondrian)
to review the following code:
Change 8116546 by [EMAIL PROTECTED] on 2008/08/27 14:27:11 *pending*
Delete databases (not just LocalServer content) when removing an origin.
PRESUBMIT=passed
R=michaeln
[EMAIL PROTECTED]
DELTA=60 (49 added, 4 deleted, 7 changed)
OCL=8116546
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.h#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_db.cc#11
edit
60 delta lines: 49 added, 4 deleted, 7 changed
Also consider running:
g4 lint -c 8116546
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8116546 by [EMAIL PROTECTED] on 2008/08/27 14:27:11 *pending*
Delete databases (not just LocalServer content) when removing an origin.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.cc#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.h#1
edit
...
//depot/googleclient/gears/opensource/gears/base/common/permissions_db.cc#11
edit
====
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.cc#1
-
c:\Src\2L/googleclient/gears/opensource/gears/base/common/database_name_table.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/database_name_table.cc
2008-08-27 14:26:59.000000000 -0700
+++ googleclient/gears/opensource/gears/base/common/database_name_table.cc
2008-08-27 13:10:30.000000000 -0700
@@ -25,14 +25,14 @@
#include "gears/base/common/database_name_table.h"
+#include "gears/base/common/file.h"
+#include "gears/base/common/paths.h"
+
DatabaseNameTable::DatabaseNameTable(SQLDatabase *db)
: db_(db) {
}
bool DatabaseNameTable::MaybeCreateTableVersion7() {
- // TODO(shess): IsDeleted is there to let us later distinguish
- // corrupt from deleted. Not interested in changing the API at this
- // time, though.
const char *sql = "CREATE TABLE IF NOT EXISTS DatabaseNames ("
" DatabaseID INTEGER PRIMARY KEY,"
" Origin TEXT NOT NULL,"
@@ -299,3 +299,44 @@
return true;
}
+
+void DatabaseNameTable::DeleteOriginDatabases(const SecurityOrigin &origin) {
+ // First mark all the databases as deleted. Then try to remove all
+ // files backing the deleted databases.
+ //
+ // If the first step fails, continue to the second step instead of
+ // returning early. During the second step, don't return on errors
+ // either; simply continue to the next database.
+
+
+ // Step 1: Mark databases as deleted.
+ const char16 *update_sql = STRING16(
+ L"UPDATE DatabaseNames SET IsDeleted = 1 WHERE Origin = ?");
+ SQLStatement update_stmt;
+ if (SQLITE_OK == update_stmt.prepare16(db_, update_sql) &&
+ SQLITE_OK == update_stmt.bind_text16(0, origin.url().c_str()) &&
+ SQLITE_DONE == update_stmt.step()) {
+ // Hooray, it succeeded. Nothing more to do here.
+ }
+
+ // Step 2: Try to remove databases marked as deleted.
+ const char16 *select_sql = STRING16(
+ L"SELECT Basename FROM DatabaseNames"
+ L" WHERE IsDeleted = 1 AND Origin = ?");
+ SQLStatement select_stmt;
+ if (SQLITE_OK == select_stmt.prepare16(db_, select_sql) &&
+ SQLITE_OK == select_stmt.bind_text16(0, origin.url().c_str())) {
+
+ std::string16 origin_data_dir;
+ if (GetDataDirectory(origin, &origin_data_dir)) {
+ origin_data_dir += kPathSeparator;
+
+ while (SQLITE_ROW == select_stmt.step()) {
+ std::string16 basename = select_stmt.column_text16_safe(0);
+ std::string16 full_path = origin_data_dir + basename;
+ File::Delete(full_path.c_str()); // ignore return value
+ }
+ }
+ }
+
+}
====
//depot/googleclient/gears/opensource/gears/base/common/database_name_table.h#1
-
c:\Src\2L/googleclient/gears/opensource/gears/base/common/database_name_table.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/database_name_table.h
2008-08-27 14:26:59.000000000 -0700
+++ googleclient/gears/opensource/gears/base/common/database_name_table.h
2008-08-27 14:27:55.000000000 -0700
@@ -26,16 +26,17 @@
#ifndef GEARS_BASE_COMMON_DATABASE_NAME_TABLE_H__
#define GEARS_BASE_COMMON_DATABASE_NAME_TABLE_H__
+#include "gears/base/common/security_model.h"
#include "gears/base/common/sqlite_wrapper.h"
-// Map database names to basenames in the filesystem.
+// Maps database names to basenames in the filesystem.
class DatabaseNameTable {
public:
DatabaseNameTable(SQLDatabase *db);
bool MaybeCreateTableVersion7();
- // Upgrade to version 7 schema (this table did not previously
+ // Upgrades to version 7 schema (this table did not previously
// exist).
bool UpgradeToVersion7();
@@ -45,23 +46,22 @@
return MaybeCreateTableVersion7();
}
- // For a given database_name, fill basename with the name of the
+ // For a given database_name, fills basename with the name of the
// file to use in origin's directory, and returns true if
// successful.
bool GetDatabaseBasename(const char16 *origin, const char16 *database_name,
std::string16 *basename);
- // Mark the given database basename corrupt so that future calls to
+ // Marks the given database basename corrupt so that future calls to
// GetDatabaseBasename will no longer return it. The basename is
// required because another thread of control could have already
// invalidated the database for the origin.
bool MarkDatabaseCorrupt(const char16 *origin, const char16 *database_name,
const char16 *basename);
- // TODO(shess): This class should be easily able to support a
- // DeleteDatabase(origin, database_name) function. Not adding
- // immediately because we don't want to change the API for this
- // release.
+ // Deletes all databases associated with the given origin. Always
+ // attempts to delete as much as possible, so no value is returned.
+ void DeleteOriginDatabases(const SecurityOrigin &origin);
private:
// A pointer to the SQLDatabase our table lives in.
====
//depot/googleclient/gears/opensource/gears/base/common/permissions_db.cc#11 -
c:\Src\2L/googleclient/gears/opensource/gears/base/common/permissions_db.cc ====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/permissions_db.cc
2008-08-27 14:26:59.000000000 -0700
+++ googleclient/gears/opensource/gears/base/common/permissions_db.cc
2008-08-27 13:08:22.000000000 -0700
@@ -182,6 +182,10 @@
if ((type == PERMISSION_LOCAL_DATA) &&
(value == PERMISSION_DENIED || value == PERMISSION_NOT_SET)) {
+ // Remove Database content.
+ database_name_table_.DeleteOriginDatabases(origin);
+
+ // Remove LocalServer content.
WebCacheDB *webcacheDB = WebCacheDB::GetDB();
if (webcacheDB) {
webcacheDB->DeleteServersForOrigin(origin);
