Database support for GDataDirectoryService.

chrome/browser/chromeos/gdata/gdata_files.cc

Index: chrome/browser/chromeos/gdata/gdata_files.cc
===================================================================
--- chrome/browser/chromeos/gdata/gdata_files.cc	(revision 149697)
+++ chrome/browser/chromeos/gdata/gdata_files.cc	(working copy)
@@ -4,19 +4,25 @@
 
 #include "chrome/browser/chromeos/gdata/gdata_files.h"
 
+#include <leveldb/db.h>
 #include <vector>
 
 #include "base/message_loop_proxy.h"
 #include "base/platform_file.h"
+#include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
+#include "base/sequenced_task_runner.h"
 #include "base/tracked_objects.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/chromeos/gdata/gdata.pb.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 #include "chrome/browser/chromeos/gdata/gdata_wapi_parser.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/base/escape.h"
 
+using content::BrowserThread;
+
 namespace gdata {
 namespace {
 
@@ -423,12 +429,126 @@
   child_directories_.clear();
 }
 
+// GDataDirectoryServiceDB implementation.
+
+// Params for GDatadirectoryServiceDB::Create.
+struct CreateDBParams {
+  CreateDBParams(const FilePath& db_path,
+                 base::SequencedTaskRunner* blocking_task_runner)
+                 : db_path(db_path),
+                   blocking_task_runner(blocking_task_runner) {
+  }
+
+  FilePath db_path;
+  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner;
+  scoped_ptr<GDataDirectoryServiceDB> db;
+  GDataDirectoryService::SerializedMap serialized_resources;
+};
+
+// Wrapper for level db. All methods must be called on blocking thread.
+class GDataDirectoryServiceDB {

[satorux1, 2012/08/03 06:32:50]

From this name, I originally thought this is a sub class of
GDataDirectoryService. Maybe rename it to something more generic like
"ResourceMetadataDB", which matches the file name "resource_metadata.db".

BTW, we should also move this into a set of separate files. Might want to do it
in this patch or add a TODO with a bug URL

[achuithb, 2012/08/03 19:15:59]

I like ResourceMetadataDB. I've done the rename. 

I think it makes sense for GDataDirectoryResource and ResourceMetadataDB to be
in the same file because they are quite interwoven and call back into one
another. The only reason they are split into two entities is because
ResourceMetadataDB must live on the blocking pool, and GDataDirectoryResource
must live on the UI thread.

+ public:
+  GDataDirectoryServiceDB(const FilePath& db_path,
+                          base::SequencedTaskRunner* blocking_task_runner);
+
+  // Initializes the database.
+  void Init();
+
+  // Reads the database into |serialized_resources|.
+  void Read(GDataDirectoryService::SerializedMap* serialized_resources);
+
+  // Saves |serialized_resources| to the database.
+  void Save(const GDataDirectoryService::SerializedMap& serialized_resources);
+
+ private:
+  // Clears the database.
+  void Clear();
+
+  scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
+  scoped_ptr<leveldb::DB> level_db_;
+  FilePath db_path_;
+};
+
+GDataDirectoryServiceDB::GDataDirectoryServiceDB(const FilePath& db_path,
+    base::SequencedTaskRunner* blocking_task_runner)
+  : blocking_task_runner_(blocking_task_runner),
+    db_path_(db_path) {
+  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
+}
+
+// Creates, initializes and reads from the database.
+// This must be defined after GDataDirectoryServiceDB and CreateDBParams.
+static void CreateGDataDirectoryServiceDBOnBlockingPool(
+    CreateDBParams* params) {
+  DCHECK(params->blocking_task_runner->RunsTasksOnCurrentThread());
+  DCHECK(!params->db_path.empty());
+
+  params->db.reset(new GDataDirectoryServiceDB(params->db_path,
+                                               params->blocking_task_runner));
+  params->db->Init();
+  params->db->Read(&params->serialized_resources);
+}
+
+void GDataDirectoryServiceDB::Init() {
+  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(!db_path_.empty());
+
+  DVLOG(1) << "Init " << db_path_.value();
+
+  leveldb::DB* level_db = NULL;
+  leveldb::Options options;
+  options.create_if_missing = true;
+  leveldb::Status db_status = leveldb::DB::Open(options, db_path_.value(),
+                                                &level_db);
+  DCHECK(level_db);
+  DCHECK(db_status.ok());
+  level_db_.reset(level_db);
+}
+
+void GDataDirectoryServiceDB::Read(
+  GDataDirectoryService::SerializedMap* serialized_resources) {
+  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
+  DCHECK(serialized_resources);
+  DVLOG(1) << "Read " << db_path_.value();
+
+  scoped_ptr<leveldb::Iterator> iter(level_db_->NewIterator(
+        leveldb::ReadOptions()));
+  for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {
+    DVLOG(1) << "Read, resource " << iter->key().ToString();
+    serialized_resources->insert(std::make_pair(iter->key().ToString(),
+                                                iter->value().ToString()));
+  }
+}
+
+void GDataDirectoryServiceDB::Save(
+    const GDataDirectoryService::SerializedMap& serialized_resources) {
+  DCHECK(blocking_task_runner_->RunsTasksOnCurrentThread());
+
+  Clear();
+  for (GDataDirectoryService::SerializedMap::const_iterator iter =
+      serialized_resources.begin();
+      iter != serialized_resources.end(); ++iter) {
+    DVLOG(1) << "Saving resource " << iter->first << " to db";
+    level_db_->Put(leveldb::WriteOptions(),

[satorux1, 2012/08/03 06:32:50]

What if Put fails? Shouldn't we check the return value and handle the error (ex
remove the files, and emit  error log).

[achuithb, 2012/08/03 19:15:59]

I've added a LOG(error), but I'm not sure what else really needs to be done. I
can't think of why Put would fail, unless the disk was full or there were disk
errors of some other kind.

+                   leveldb::Slice(iter->first),
+                   leveldb::Slice(iter->second));
+  }
+}
+
+void GDataDirectoryServiceDB::Clear() {
+  level_db_.reset();
+  leveldb::DestroyDB(db_path_.value(), leveldb::Options());
+  Init();
+}
+
 // GDataDirectoryService class implementation.
 
 GDataDirectoryService::GDataDirectoryService()
-    : serialized_size_(0),
+    : blocking_task_runner_(NULL),
+      serialized_size_(0),
       largest_changestamp_(0),
-      origin_(UNINITIALIZED) {
+      origin_(UNINITIALIZED),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   root_.reset(new GDataDirectory(NULL, this));
   root_->set_title(kGDataRootDirectory);
   root_->SetBaseNameFromTitle();
@@ -437,12 +557,23 @@
 }
 
 GDataDirectoryService::~GDataDirectoryService() {
+  ClearRoot();
+
+  // Ensure db is closed on the blocking pool.
+  if (blocking_task_runner_ && directory_service_db_.get())
+    blocking_task_runner_->PostTask(FROM_HERE,

[satorux1, 2012/08/03 06:32:50]

I think you can use DelteSoon() instead

[achuithb, 2012/08/03 19:15:59]

Done.

+        base::Bind(&base::DeletePointer<GDataDirectoryServiceDB>,
+                   directory_service_db_.release()));
+}
+
+void GDataDirectoryService::ClearRoot() {
   // Note that children have a reference to root_,
   // so we need to delete them here.
   root_->RemoveChildren();
   RemoveEntryFromResourceMap(root_.get());
   DCHECK(resource_map_.empty());
   resource_map_.clear();
+  root_.reset();
 }
 
 void GDataDirectoryService::AddEntryToDirectory(
@@ -561,6 +692,159 @@
   }
 }
 
+void GDataDirectoryService::InitFromDB(
+    const FilePath& db_path,
+    base::SequencedTaskRunner* blocking_task_runner,
+    LoadRootFeedParams* load_params,
+    const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!db_path.empty());
+  DCHECK(blocking_task_runner);
+
+  if (directory_service_db_.get()) {
+    NOTREACHED();
+    if (!callback.is_null())
+      callback.Run();
+  }
+
+  blocking_task_runner_ = blocking_task_runner;
+
+  DVLOG(1) << "InitFromDB " << db_path.value();
+
+  CreateDBParams* create_params =
+      new CreateDBParams(db_path, blocking_task_runner);
+  blocking_task_runner_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&CreateGDataDirectoryServiceDBOnBlockingPool,
+                 create_params),
+      base::Bind(&GDataDirectoryService::InitResourceMap,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Owned(create_params),
+                 load_params,
+                 callback));
+}
+
+void GDataDirectoryService::InitResourceMap(
+    CreateDBParams* create_params,
+    LoadRootFeedParams* load_params,
+    const base::Closure& callback) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(create_params);
+  // Ensure this callback is run before we return.
+  base::ScopedClosureRunner callback_runner(callback);
+
+  directory_service_db_ = create_params->db.Pass();
+  if (create_params->serialized_resources.empty()) {
+    origin_ = INITIALIZING;
+    if (load_params)
+      load_params->load_error = GDATA_FILE_ERROR_NOT_FOUND;
+    return;
+  }
+
+  ClearRoot();
+
+  int32 version = 0;
+  ResourceMap resource_map;
+  for (SerializedMap::const_iterator iter =
+      create_params->serialized_resources.begin();

[satorux1, 2012/08/03 06:32:50]

indentation

[achuithb, 2012/08/03 19:15:59]

Done.

+      iter != create_params->serialized_resources.end(); ++iter) {
+    if (iter->first == "version") {
+      if (!base::StringToInt(iter->second, &version) ||
+          version != kProtoVersion) {

[satorux1, 2012/08/03 06:32:50]

shouldn't we set the load_error to GDATA_FILE_ERROR_FAILED?

[achuithb, 2012/08/03 19:15:59]

Done.

+        return;
+      }
+      continue;
+    }
+
+    if (iter->first == "largest_changestamp") {
+      base::StringToInt(iter->second, &largest_changestamp_);
+      DVLOG(1) << "InitResourceMap largest_changestamp_"
+               << largest_changestamp_;

[satorux1, 2012/08/03 06:32:50]

ditto. load_error?

[achuithb, 2012/08/03 19:15:59]

I think this is an error but it's not fatal, right?

+      continue;
+    }
+
+    scoped_ptr<GDataEntry> entry = FromProtoString(iter->second);
+    if (entry.get()) {
+      DVLOG(1) << "Inserting resource " << iter->first
+               << " into resource_map";
+      resource_map.insert(std::make_pair(iter->first, entry.release()));
+    } else {
+      NOTREACHED() << "Failed to parse GDataEntry for resource "
+                   << iter->first;
+    }
+  }
+
+  // Fix up parent-child relations.
+  for (ResourceMap::iterator iter = resource_map.begin();
+      iter != resource_map.end(); ++iter) {
+    GDataEntry* entry = iter->second;
+    ResourceMap::iterator parent_it =
+        resource_map.find(entry->parent_resource_id());
+    if (parent_it != resource_map.end()) {
+      GDataDirectory* parent = parent_it->second->AsGDataDirectory();
+      if (parent) {
+        DVLOG(1) << "Adding " << entry->resource_id()
+                 << " as a child of " << parent->resource_id();
+        parent->AddEntry(entry);
+      } else {
+        NOTREACHED() << "Parent is not a directory " << parent->resource_id();
+      }
+    } else if (entry->resource_id() == kGDataRootDirectoryResourceId) {
+      root_.reset(entry->AsGDataDirectory());
+      DCHECK(root_.get());
+      AddEntryToResourceMap(root_.get());
+    } else {
+      NOTREACHED() << "Missing parent id " << entry->parent_resource_id()
+                   << " for resource " << entry->resource_id();
+    }
+  }
+
+  DCHECK(root_.get());
+  DCHECK_EQ(resource_map.size(), resource_map_.size());
+  DCHECK_EQ(resource_map.size(),
+            create_params->serialized_resources.size() - 2);
+
+  origin_ = FROM_CACHE;
+}
+
+void GDataDirectoryService::SaveToDB() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (!blocking_task_runner_ || !directory_service_db_.get()) {
+    NOTREACHED();
+    return;
+  }
+
+  size_t serialized_size = 0;
+  SerializedMap serialized_resources;
+  for (ResourceMap::const_iterator iter = resource_map_.begin();
+      iter != resource_map_.end(); ++iter) {
+    GDataEntryProto proto;
+    iter->second->ToProtoFull(&proto);
+    std::string serialized_string;
+    const bool ok = proto.SerializeToString(&serialized_string);
+    DCHECK(ok);
+    if (ok) {
+      serialized_resources.insert(
+          std::make_pair(iter->first, serialized_string));
+      serialized_size += serialized_string.size();
+    }
+  }
+
+  serialized_resources.insert(std::make_pair("version",
+      base::IntToString(kProtoVersion)));
+  serialized_resources.insert(std::make_pair("largest_changestamp",

[satorux1, 2012/08/03 06:32:50]

Let's define constants for "version" and "largest_changestamp"

btw, we are assuming that these won't be used as resource IDs, but the
assumption is rather fragile. We might want to use a short prefix to ensure
uniqueness. like:

m:version
m:largest_changestamp
r:<resource_id>

[achuithb, 2012/08/03 19:15:59]

Done.

+      base::IntToString(largest_changestamp_)));
+  set_last_serialized(base::Time::Now());
+  set_serialized_size(serialized_size);
+
+  blocking_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&GDataDirectoryServiceDB::Save,
+                 base::Unretained(directory_service_db_.get()),
+                 serialized_resources));
+}
+
 // Convert to/from proto.
 
 // static
@@ -795,4 +1079,31 @@
   return true;
 }
 
+scoped_ptr<GDataEntry> GDataDirectoryService::FromProtoString(
+    const std::string& serialized_proto) {
+  scoped_ptr<GDataEntry> entry;
+
+  GDataEntryProto entry_proto;
+  if (!entry_proto.ParseFromString(serialized_proto))
+    return entry.Pass();

[hashimoto, 2012/08/03 05:17:07]

Why don't you simply pass scoped_ptr<GDataEntry>()?

[satorux1, 2012/08/03 06:32:50]

Agreed with hashimoto. That'll be clearer

[achuithb, 2012/08/03 19:15:59]

Done.

+
+  if (entry_proto.file_info().is_directory()) {

[satorux1, 2012/08/03 06:32:50]

move GDataEntryProto entry_proto; right before 'if'

[achuithb, 2012/08/03 19:15:59]

I don't understand? It's needed for ParseFromString. Did you mean
scoped_ptr<GDataEntry> entry?

[satorux1, 2012/08/03 20:19:16]

my bad. i was wrong.

+    entry.reset(new GDataDirectory(NULL, this));
+    // Call GDataEntry::FromProto.

[satorux1, 2012/08/03 06:32:50]

Please add some more comment that why GDtaDirectory::FromProto() is not used.
the reason is, Here, we don't need to handle children, right?

[achuithb, 2012/08/03 19:15:59]

Correct. Comment added.

+    if (!entry->FromProto(entry_proto)) {
+      NOTREACHED() << "FromProto (directory) failed";
+      entry.reset();
+    }
+  } else {
+    scoped_ptr<GDataFile> file(new GDataFile(NULL, this));
+    // Call GDataFile::FromProto.
+    if (file->FromProto(entry_proto)) {
+      entry.reset(file.release());
+    } else {
+      NOTREACHED() << "FromProto (file) failed";
+    }
+  }
+  return entry.Pass();
+}
+
 }  // namespace gdata