drive: Stop using FileCacheEntry related methods in FileCache
Remove ResourceMetadataStorage::PutCacheEntry, RemoveCacheEntry and GetCacheEntryIterator. (GetCacheEntry is kept for tests)
FileCache::ClearAll is only responsible to delete files.
BUG=275271
TEST=unit_tests
Review URL: https://codereview.chromium.org/296463002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271664 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/drive/file_cache.cc b/chrome/browser/chromeos/drive/file_cache.cc
index 5fa5d82..86dcedb 100644
--- a/chrome/browser/chromeos/drive/file_cache.cc
+++ b/chrome/browser/chromeos/drive/file_cache.cc
@@ -82,14 +82,16 @@
DVLOG(1) << "Freeing up disk space for " << num_bytes;
// Remove all entries unless specially marked.
- scoped_ptr<ResourceMetadataStorage::CacheEntryIterator> it =
- storage_->GetCacheEntryIterator();
+ scoped_ptr<ResourceMetadataStorage::Iterator> it = storage_->GetIterator();
for (; !it->IsAtEnd(); it->Advance()) {
- const FileCacheEntry& entry = it->GetValue();
- if (!entry.is_pinned() &&
- !entry.is_dirty() &&
- !mounted_files_.count(it->GetID()))
- storage_->RemoveCacheEntry(it->GetID());
+ if (it->GetValue().file_specific_info().has_cache_state() &&
+ !it->GetValue().file_specific_info().cache_state().is_pinned() &&
+ !it->GetValue().file_specific_info().cache_state().is_dirty() &&
+ !mounted_files_.count(it->GetID())) {
+ ResourceEntry entry(it->GetValue());
+ entry.mutable_file_specific_info()->clear_cache_state();
+ storage_->PutEntry(entry);
+ }
}
if (it->HasError())
return false;
@@ -98,12 +100,14 @@
base::FileEnumerator enumerator(cache_file_directory_,
false, // not recursive
base::FileEnumerator::FILES);
- FileCacheEntry entry;
+ ResourceEntry entry;
for (base::FilePath current = enumerator.Next(); !current.empty();
current = enumerator.Next()) {
std::string id = GetIdFromPath(current);
- FileError error = storage_->GetCacheEntry(id, &entry);
- if (error == FILE_ERROR_NOT_FOUND)
+ FileError error = storage_->GetEntry(id, &entry);
+ if (error == FILE_ERROR_NOT_FOUND ||
+ (error == FILE_ERROR_OK &&
+ !entry.file_specific_info().cache_state().is_present()))
base::DeleteFile(current, false /* recursive */);
else if (error != FILE_ERROR_OK)
return false;
@@ -118,11 +122,11 @@
AssertOnSequencedWorkerPool();
DCHECK(cache_file_path);
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
- if (!cache_entry.is_present())
+ if (!entry.file_specific_info().cache_state().is_present())
return FILE_ERROR_NOT_FOUND;
*cache_file_path = GetCacheFilePath(id);
@@ -135,6 +139,11 @@
FileOperationType file_operation_type) {
AssertOnSequencedWorkerPool();
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
+ if (error != FILE_ERROR_OK)
+ return error;
+
int64 file_size = 0;
if (file_operation_type == FILE_OPERATION_COPY) {
if (!base::GetFileSize(source_path, &file_size)) {
@@ -171,49 +180,47 @@
}
// Now that file operations have completed, update metadata.
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
- if (error != FILE_ERROR_OK && error != FILE_ERROR_NOT_FOUND)
- return error;
- cache_entry.set_md5(md5);
- cache_entry.set_is_present(true);
+ FileCacheEntry* cache_state =
+ entry.mutable_file_specific_info()->mutable_cache_state();
+ cache_state->set_md5(md5);
+ cache_state->set_is_present(true);
if (md5.empty())
- cache_entry.set_is_dirty(true);
- return storage_->PutCacheEntry(id, cache_entry);
+ cache_state->set_is_dirty(true);
+ return storage_->PutEntry(entry);
}
FileError FileCache::Pin(const std::string& id) {
AssertOnSequencedWorkerPool();
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
- if (error != FILE_ERROR_OK && error != FILE_ERROR_NOT_FOUND)
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
+ if (error != FILE_ERROR_OK)
return error;
- cache_entry.set_is_pinned(true);
- return storage_->PutCacheEntry(id, cache_entry);
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_is_pinned(
+ true);
+ return storage_->PutEntry(entry);
}
FileError FileCache::Unpin(const std::string& id) {
AssertOnSequencedWorkerPool();
// Unpinning a file means its entry must exist in cache.
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
// Now that file operations have completed, update metadata.
- if (cache_entry.is_present()) {
- cache_entry.set_is_pinned(false);
- error = storage_->PutCacheEntry(id, cache_entry);
- if (error != FILE_ERROR_OK)
- return error;
+ if (entry.file_specific_info().cache_state().is_present()) {
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_is_pinned(
+ false);
} else {
// Remove the existing entry if we are unpinning a non-present file.
- error = storage_->RemoveCacheEntry(id);
- if (error != FILE_ERROR_OK)
- return error;
+ entry.mutable_file_specific_info()->clear_cache_state();
}
+ error = storage_->PutEntry(entry);
+ if (error != FILE_ERROR_OK)
+ return error;
// Now it's a chance to free up space if needed.
FreeDiskSpaceIfNeededFor(0);
@@ -227,10 +234,12 @@
DCHECK(cache_file_path);
// Get cache entry associated with the id and md5
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
+ if (!entry.file_specific_info().cache_state().is_present())
+ return FILE_ERROR_NOT_FOUND;
if (mounted_files_.count(id))
return FILE_ERROR_INVALID_OPERATION;
@@ -258,18 +267,18 @@
// Marking a file dirty means its entry and actual file blob must exist in
// cache.
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
- if (!cache_entry.is_present()) {
+ if (!entry.file_specific_info().cache_state().is_present()) {
LOG(WARNING) << "Can't mark dirty a file that wasn't cached: " << id;
return FILE_ERROR_NOT_FOUND;
}
- cache_entry.set_is_dirty(true);
- cache_entry.clear_md5();
- error = storage_->PutCacheEntry(id, cache_entry);
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_is_dirty(true);
+ entry.mutable_file_specific_info()->mutable_cache_state()->clear_md5();
+ error = storage_->PutEntry(entry);
if (error != FILE_ERROR_OK)
return error;
@@ -294,19 +303,19 @@
if (IsOpenedForWrite(id))
return FILE_ERROR_IN_USE;
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
- if (!cache_entry.is_present())
+ if (!entry.file_specific_info().cache_state().is_present())
return FILE_ERROR_NOT_FOUND;
const std::string& md5 = util::GetMd5Digest(GetCacheFilePath(id));
if (md5.empty())
return FILE_ERROR_NOT_FOUND;
- cache_entry.set_md5(md5);
- return storage_->PutCacheEntry(id, cache_entry);
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_md5(md5);
+ return storage_->PutEntry(entry);
}
FileError FileCache::ClearDirty(const std::string& id) {
@@ -317,11 +326,11 @@
// Clearing a dirty file means its entry and actual file blob must exist in
// cache.
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
- if (!cache_entry.is_present()) {
+ if (!entry.file_specific_info().cache_state().is_present()) {
LOG(WARNING) << "Can't clear dirty state of a file that wasn't cached: "
<< id;
return FILE_ERROR_NOT_FOUND;
@@ -329,26 +338,29 @@
// If a file is not dirty (it should have been marked dirty via OpenForWrite),
// clearing its dirty state is an invalid operation.
- if (!cache_entry.is_dirty()) {
+ if (!entry.file_specific_info().cache_state().is_dirty()) {
LOG(WARNING) << "Can't clear dirty state of a non-dirty file: " << id;
return FILE_ERROR_INVALID_OPERATION;
}
- cache_entry.set_is_dirty(false);
- return storage_->PutCacheEntry(id, cache_entry);
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_is_dirty(
+ false);
+ return storage_->PutEntry(entry);
}
FileError FileCache::Remove(const std::string& id) {
AssertOnSequencedWorkerPool();
- FileCacheEntry cache_entry;
+ ResourceEntry entry;
// If entry doesn't exist, nothing to do.
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ FileError error = storage_->GetEntry(id, &entry);
if (error == FILE_ERROR_NOT_FOUND)
return FILE_ERROR_OK;
if (error != FILE_ERROR_OK)
return error;
+ if (!entry.file_specific_info().has_cache_state())
+ return FILE_ERROR_OK;
// Cannot delete a mounted file.
if (mounted_files_.count(id))
@@ -360,23 +372,13 @@
return FILE_ERROR_FAILED;
// Now that all file operations have completed, remove from metadata.
- return storage_->RemoveCacheEntry(id);
+ entry.mutable_file_specific_info()->clear_cache_state();
+ return storage_->PutEntry(entry);
}
bool FileCache::ClearAll() {
AssertOnSequencedWorkerPool();
- // Remove entries on the metadata.
- scoped_ptr<ResourceMetadataStorage::CacheEntryIterator> it =
- storage_->GetCacheEntryIterator();
- for (; !it->IsAtEnd(); it->Advance()) {
- if (storage_->RemoveCacheEntry(it->GetID()) != FILE_ERROR_OK)
- return false;
- }
-
- if (it->HasError())
- return false;
-
// Remove files.
base::FileEnumerator enumerator(cache_file_directory_,
false, // not recursive
@@ -393,13 +395,13 @@
// Older versions do not clear MD5 when marking entries dirty.
// Clear MD5 of all dirty entries to deal with old data.
- scoped_ptr<ResourceMetadataStorage::CacheEntryIterator> it =
- storage_->GetCacheEntryIterator();
+ scoped_ptr<ResourceMetadataStorage::Iterator> it = storage_->GetIterator();
for (; !it->IsAtEnd(); it->Advance()) {
- if (it->GetValue().is_dirty()) {
- FileCacheEntry new_entry(it->GetValue());
- new_entry.clear_md5();
- if (storage_->PutCacheEntry(it->GetID(), new_entry) != FILE_ERROR_OK)
+ if (it->GetValue().file_specific_info().cache_state().is_dirty()) {
+ ResourceEntry new_entry(it->GetValue());
+ new_entry.mutable_file_specific_info()->mutable_cache_state()->
+ clear_md5();
+ if (storage_->PutEntry(new_entry) != FILE_ERROR_OK)
return false;
}
}
@@ -439,14 +441,15 @@
for (base::FilePath current = enumerator.Next(); !current.empty();
current = enumerator.Next()) {
const std::string& id = GetIdFromPath(current);
- FileCacheEntry entry;
- FileError error = storage_->GetCacheEntry(id, &entry);
- if (error == FILE_ERROR_OK) {
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
+ if (error != FILE_ERROR_OK && error != FILE_ERROR_NOT_FOUND)
+ return false;
+ if (error == FILE_ERROR_OK &&
+ entry.file_specific_info().cache_state().is_present()) {
// This file is managed by FileCache, no need to recover it.
continue;
}
- if (error != FILE_ERROR_NOT_FOUND)
- return false;
// If a cache entry which is non-dirty and has matching MD5 is found in
// |recovered_cache_entries|, it means the current file is already uploaded
@@ -529,9 +532,9 @@
std::string id = GetIdFromPath(file_path);
- // Get cache entry associated with the id and md5
- FileCacheEntry cache_entry;
- FileError error = storage_->GetCacheEntry(id, &cache_entry);
+ // Get the entry associated with the id.
+ ResourceEntry entry;
+ FileError error = storage_->GetEntry(id, &entry);
if (error != FILE_ERROR_OK)
return error;
diff --git a/chrome/browser/chromeos/drive/file_cache.h b/chrome/browser/chromeos/drive/file_cache.h
index c85be5d..fe0b76c 100644
--- a/chrome/browser/chromeos/drive/file_cache.h
+++ b/chrome/browser/chromeos/drive/file_cache.h
@@ -114,7 +114,7 @@
// Removes the specified cache entry and delete cache files if available.
FileError Remove(const std::string& id);
- // Removes all the files in the cache directory and cache entries in DB.
+ // Removes all the files in the cache directory.
bool ClearAll();
// Initializes the cache. Returns true on success.
diff --git a/chrome/browser/chromeos/drive/file_cache_unittest.cc b/chrome/browser/chromeos/drive/file_cache_unittest.cc
index 0f38245..bbb9b883 100644
--- a/chrome/browser/chromeos/drive/file_cache_unittest.cc
+++ b/chrome/browser/chromeos/drive/file_cache_unittest.cc
@@ -60,12 +60,6 @@
return cache->RenameCacheFilesToNewFormat();
}
- FileError GetCacheEntry(FileCache* cache,
- const std::string& id,
- FileCacheEntry* cache_entry) {
- return cache->storage_->GetCacheEntry(id, cache_entry);
- }
-
content::TestBrowserThreadBundle thread_bundle_;
base::ScopedTempDir temp_dir_;
base::FilePath cache_files_dir_;
@@ -83,6 +77,9 @@
dir_source_root.AppendASCII("chrome/test/data/chromeos/drive/image.png");
// Store files. This file should not be moved.
+ ResourceEntry entry;
+ entry.set_local_id("id_foo");
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store("id_foo", "md5", src_path,
FileCache::FILE_OPERATION_COPY));
@@ -131,6 +128,10 @@
// Store a file as a 'temporary' file and remember the path.
const std::string id_tmp = "id_tmp", md5_tmp = "md5_tmp";
+
+ ResourceEntry entry;
+ entry.set_local_id(id_tmp);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
ASSERT_EQ(FILE_ERROR_OK,
cache_->Store(id_tmp, md5_tmp, src_file,
FileCache::FILE_OPERATION_COPY));
@@ -139,6 +140,9 @@
// Store a file as a pinned file and remember the path.
const std::string id_pinned = "id_pinned", md5_pinned = "md5_pinned";
+ entry.Clear();
+ entry.set_local_id(id_pinned);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
ASSERT_EQ(FILE_ERROR_OK,
cache_->Store(id_pinned, md5_pinned, src_file,
FileCache::FILE_OPERATION_COPY));
@@ -153,11 +157,12 @@
EXPECT_TRUE(cache_->FreeDiskSpaceIfNeededFor(kNeededBytes));
// Only 'temporary' file gets removed.
- FileCacheEntry entry;
- EXPECT_EQ(FILE_ERROR_NOT_FOUND, GetCacheEntry(cache_.get(), id_tmp, &entry));
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id_tmp, &entry));
+ EXPECT_FALSE(entry.file_specific_info().cache_state().is_present());
EXPECT_FALSE(base::PathExists(tmp_path));
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id_pinned, &entry));
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id_pinned, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_present());
EXPECT_TRUE(base::PathExists(pinned_path));
// Returns false when disk space cannot be freed.
@@ -177,6 +182,9 @@
temp_dir_.path().AppendASCII(kCacheFileDirectory);
// Try to get an existing file from cache.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(id, md5, src_file_path,
FileCache::FILE_OPERATION_COPY));
base::FilePath cache_file_path;
@@ -191,6 +199,9 @@
// Get file from cache with different id.
id = "id2";
+ entry.Clear();
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_NOT_FOUND, cache_->GetFile(id, &cache_file_path));
// Pin a non-existent file.
@@ -222,13 +233,15 @@
std::string md5(base::MD5String(src_contents));
// Store a file.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
id, md5, src_file_path, FileCache::FILE_OPERATION_COPY));
- FileCacheEntry cache_entry;
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
- EXPECT_TRUE(cache_entry.is_present());
- EXPECT_EQ(md5, cache_entry.md5());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_present());
+ EXPECT_EQ(md5, entry.file_specific_info().cache_state().md5());
base::FilePath cache_file_path;
EXPECT_EQ(FILE_ERROR_OK, cache_->GetFile(id, &cache_file_path));
@@ -243,10 +256,10 @@
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
id, std::string(), src_file_path, FileCache::FILE_OPERATION_COPY));
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
- EXPECT_TRUE(cache_entry.is_present());
- EXPECT_TRUE(cache_entry.md5().empty());
- EXPECT_TRUE(cache_entry.is_dirty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_present());
+ EXPECT_TRUE(entry.file_specific_info().cache_state().md5().empty());
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_dirty());
// No free space available.
fake_free_disk_space_getter_->set_default_value(0);
@@ -264,38 +277,42 @@
std::string md5(base::MD5String(src_contents));
// Store a file.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
id, md5, src_file_path, FileCache::FILE_OPERATION_COPY));
- FileCacheEntry cache_entry;
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
- EXPECT_FALSE(cache_entry.is_pinned());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_FALSE(entry.file_specific_info().cache_state().is_pinned());
// Pin the existing file.
EXPECT_EQ(FILE_ERROR_OK, cache_->Pin(id));
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
- EXPECT_TRUE(cache_entry.is_pinned());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_pinned());
// Unpin the file.
EXPECT_EQ(FILE_ERROR_OK, cache_->Unpin(id));
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
- EXPECT_FALSE(cache_entry.is_pinned());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_FALSE(entry.file_specific_info().cache_state().is_pinned());
// Pin a non-present file.
std::string id_non_present = "id_non_present";
+ entry.Clear();
+ entry.set_local_id(id_non_present);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Pin(id_non_present));
- EXPECT_EQ(FILE_ERROR_OK,
- GetCacheEntry(cache_.get(), id_non_present, &cache_entry));
- EXPECT_TRUE(cache_entry.is_pinned());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id_non_present, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_pinned());
// Unpin the previously pinned non-existent file.
EXPECT_EQ(FILE_ERROR_OK, cache_->Unpin(id_non_present));
- EXPECT_EQ(FILE_ERROR_NOT_FOUND,
- GetCacheEntry(cache_.get(), id_non_present, &cache_entry));
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id_non_present, &entry));
+ EXPECT_FALSE(entry.file_specific_info().has_cache_state());
// Unpin a file that doesn't exist in cache and is not pinned.
EXPECT_EQ(FILE_ERROR_NOT_FOUND, cache_->Unpin("id_non_existent"));
@@ -310,6 +327,9 @@
std::string md5(base::MD5String(src_contents));
// Store a file.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
id, md5, src_file_path, FileCache::FILE_OPERATION_COPY));
@@ -333,14 +353,16 @@
ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file));
const std::string id = "id";
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
ASSERT_EQ(FILE_ERROR_OK, cache_->Store(id, "md5", src_file,
FileCache::FILE_OPERATION_COPY));
// Entry is not dirty nor opened.
EXPECT_FALSE(cache_->IsOpenedForWrite(id));
- FileCacheEntry entry;
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_FALSE(entry.is_dirty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_FALSE(entry.file_specific_info().cache_state().is_dirty());
// Open (1).
scoped_ptr<base::ScopedClosureRunner> file_closer1;
@@ -348,8 +370,8 @@
EXPECT_TRUE(cache_->IsOpenedForWrite(id));
// Entry is dirty.
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_TRUE(entry.is_dirty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_dirty());
// Open (2).
scoped_ptr<base::ScopedClosureRunner> file_closer2;
@@ -376,6 +398,9 @@
EXPECT_TRUE(google_apis::test_util::WriteStringToFile(src_file_path,
contents_before));
std::string id("id1");
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(id, base::MD5String(contents_before),
src_file_path,
FileCache::FILE_OPERATION_COPY));
@@ -396,14 +421,14 @@
file_closer.reset();
// MD5 was cleared by OpenForWrite().
- FileCacheEntry entry;
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_TRUE(entry.md5().empty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().md5().empty());
// Update MD5.
EXPECT_EQ(FILE_ERROR_OK, cache_->UpdateMd5(id));
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_EQ(base::MD5String(contents_after), entry.md5());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_EQ(base::MD5String(contents_after),
+ entry.file_specific_info().cache_state().md5());
}
TEST_F(FileCacheTest, ClearDirty) {
@@ -412,6 +437,9 @@
ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file));
const std::string id = "id";
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
ASSERT_EQ(FILE_ERROR_OK, cache_->Store(id, "md5", src_file,
FileCache::FILE_OPERATION_COPY));
@@ -420,9 +448,8 @@
EXPECT_EQ(FILE_ERROR_OK, cache_->OpenForWrite(id, &file_closer));
// Entry is dirty.
- FileCacheEntry entry;
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_TRUE(entry.is_dirty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_dirty());
// Cannot clear the dirty bit of an opened entry.
EXPECT_EQ(FILE_ERROR_IN_USE, cache_->ClearDirty(id));
@@ -432,8 +459,8 @@
EXPECT_EQ(FILE_ERROR_OK, cache_->ClearDirty(id));
// Entry is not dirty.
- EXPECT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &entry));
- EXPECT_FALSE(entry.is_dirty());
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->GetEntry(id, &entry));
+ EXPECT_FALSE(entry.file_specific_info().cache_state().is_dirty());
}
TEST_F(FileCacheTest, Remove) {
@@ -445,6 +472,9 @@
std::string md5(base::MD5String(src_contents));
// First store a file to cache.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
base::FilePath src_file;
ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file));
EXPECT_EQ(FILE_ERROR_OK, cache_->Store(
@@ -502,21 +532,18 @@
const std::string md5("abcdef0123456789");
// Store an existing file.
+ ResourceEntry entry;
+ entry.set_local_id(id);
+ EXPECT_EQ(FILE_ERROR_OK, metadata_storage_->PutEntry(entry));
base::FilePath src_file;
ASSERT_TRUE(base::CreateTemporaryFileInDir(temp_dir_.path(), &src_file));
ASSERT_EQ(FILE_ERROR_OK,
cache_->Store(id, md5, src_file, FileCache::FILE_OPERATION_COPY));
- // Verify that the cache entry is created.
- FileCacheEntry cache_entry;
- ASSERT_EQ(FILE_ERROR_OK, GetCacheEntry(cache_.get(), id, &cache_entry));
-
// Clear cache.
EXPECT_TRUE(cache_->ClearAll());
// Verify that the cache is removed.
- EXPECT_EQ(FILE_ERROR_NOT_FOUND,
- GetCacheEntry(cache_.get(), id, &cache_entry));
EXPECT_TRUE(base::IsDirectoryEmpty(cache_files_dir_));
}
diff --git a/chrome/browser/chromeos/drive/resource_metadata.cc b/chrome/browser/chromeos/drive/resource_metadata.cc
index 979cdb8..90ab4c96 100644
--- a/chrome/browser/chromeos/drive/resource_metadata.cc
+++ b/chrome/browser/chromeos/drive/resource_metadata.cc
@@ -387,8 +387,17 @@
if (!new_parent.file_info().is_directory())
return FILE_ERROR_NOT_A_DIRECTORY;
+ // Do not overwrite cache states.
+ // Cache state should be changed via FileCache.
+ ResourceEntry updated_entry(entry);
+ if (old_entry.file_specific_info().has_cache_state()) {
+ *updated_entry.mutable_file_specific_info()->mutable_cache_state() =
+ old_entry.file_specific_info().cache_state();
+ } else if (updated_entry.file_specific_info().has_cache_state()) {
+ updated_entry.mutable_file_specific_info()->clear_cache_state();
+ }
// Remove from the old parent and add it to the new parent with the new data.
- return PutEntryUnderDirectory(entry);
+ return PutEntryUnderDirectory(updated_entry);
}
FileError ResourceMetadata::GetSubDirectoriesRecursively(
diff --git a/chrome/browser/chromeos/drive/resource_metadata_storage.cc b/chrome/browser/chromeos/drive/resource_metadata_storage.cc
index c1b8a58..13c88f0 100644
--- a/chrome/browser/chromeos/drive/resource_metadata_storage.cc
+++ b/chrome/browser/chromeos/drive/resource_metadata_storage.cc
@@ -240,62 +240,6 @@
return !it_->status().ok();
}
-ResourceMetadataStorage::CacheEntryIterator::CacheEntryIterator(
- scoped_ptr<leveldb::Iterator> it) : it_(it.Pass()) {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(it_);
-
- it_->SeekToFirst();
- AdvanceInternal();
-}
-
-ResourceMetadataStorage::CacheEntryIterator::~CacheEntryIterator() {
- base::ThreadRestrictions::AssertIOAllowed();
-}
-
-bool ResourceMetadataStorage::CacheEntryIterator::IsAtEnd() const {
- base::ThreadRestrictions::AssertIOAllowed();
- return !it_->Valid();
-}
-
-const std::string& ResourceMetadataStorage::CacheEntryIterator::GetID() const {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(!IsAtEnd());
- return id_;
-}
-
-const FileCacheEntry&
-ResourceMetadataStorage::CacheEntryIterator::GetValue() const {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(!IsAtEnd());
- return entry_;
-}
-
-void ResourceMetadataStorage::CacheEntryIterator::Advance() {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(!IsAtEnd());
-
- it_->Next();
- AdvanceInternal();
-}
-
-bool ResourceMetadataStorage::CacheEntryIterator::HasError() const {
- base::ThreadRestrictions::AssertIOAllowed();
- return !it_->status().ok();
-}
-
-void ResourceMetadataStorage::CacheEntryIterator::AdvanceInternal() {
- for (; it_->Valid(); it_->Next()) {
- // Skip unparsable broken entries.
- // TODO(hashimoto): Broken entries should be cleaned up at some point.
- if (IsCacheEntryKey(it_->key()) &&
- entry_.ParseFromArray(it_->value().data(), it_->value().size())) {
- id_ = GetIdFromCacheEntryKey(it_->key());
- break;
- }
- }
-}
-
// static
bool ResourceMetadataStorage::UpgradeOldDB(
const base::FilePath& directory_path,
@@ -665,6 +609,8 @@
return FILE_ERROR_FAILED;
}
batch.Put(GetCacheEntryKey(id), serialized_entry);
+ } else {
+ batch.Delete(GetCacheEntryKey(id));
}
// Put the entry itself.
@@ -708,6 +654,8 @@
if (cache_error == FILE_ERROR_OK) {
*out_entry->mutable_file_specific_info()->mutable_cache_state() =
cache_entry;
+ } else {
+ out_entry->mutable_file_specific_info()->clear_cache_state();
}
return FILE_ERROR_OK;
}
@@ -785,24 +733,6 @@
return LevelDBStatusToFileError(it->status());
}
-FileError ResourceMetadataStorage::PutCacheEntry(const std::string& id,
- const FileCacheEntry& entry) {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(!id.empty());
-
- std::string serialized_entry;
- if (!entry.SerializeToString(&serialized_entry)) {
- DLOG(ERROR) << "Failed to serialize the entry.";
- return FILE_ERROR_FAILED;
- }
-
- const leveldb::Status status = resource_map_->Put(
- leveldb::WriteOptions(),
- leveldb::Slice(GetCacheEntryKey(id)),
- leveldb::Slice(serialized_entry));
- return LevelDBStatusToFileError(status);
-}
-
FileError ResourceMetadataStorage::GetCacheEntry(const std::string& id,
FileCacheEntry* out_entry) {
base::ThreadRestrictions::AssertIOAllowed();
@@ -819,25 +749,6 @@
FILE_ERROR_OK : FILE_ERROR_FAILED;
}
-FileError ResourceMetadataStorage::RemoveCacheEntry(const std::string& id) {
- base::ThreadRestrictions::AssertIOAllowed();
- DCHECK(!id.empty());
-
- const leveldb::Status status = resource_map_->Delete(
- leveldb::WriteOptions(),
- leveldb::Slice(GetCacheEntryKey(id)));
- return LevelDBStatusToFileError(status);
-}
-
-scoped_ptr<ResourceMetadataStorage::CacheEntryIterator>
-ResourceMetadataStorage::GetCacheEntryIterator() {
- base::ThreadRestrictions::AssertIOAllowed();
-
- scoped_ptr<leveldb::Iterator> it(
- resource_map_->NewIterator(leveldb::ReadOptions()));
- return make_scoped_ptr(new CacheEntryIterator(it.Pass()));
-}
-
ResourceMetadataStorage::RecoveredCacheInfo::RecoveredCacheInfo()
: is_dirty(false) {}
diff --git a/chrome/browser/chromeos/drive/resource_metadata_storage.h b/chrome/browser/chromeos/drive/resource_metadata_storage.h
index 44392b3..7d38f70c 100644
--- a/chrome/browser/chromeos/drive/resource_metadata_storage.h
+++ b/chrome/browser/chromeos/drive/resource_metadata_storage.h
@@ -166,18 +166,9 @@
FileError GetChildren(const std::string& parent_id,
std::vector<std::string>* children);
- // Puts the cache entry to this storage.
- FileError PutCacheEntry(const std::string& id, const FileCacheEntry& entry);
-
// Gets a cache entry stored in this storage.
FileError GetCacheEntry(const std::string& id, FileCacheEntry* out_entry);
- // Removes a cache entry from this storage.
- FileError RemoveCacheEntry(const std::string& id);
-
- // Returns an object to iterate over cache entries stored in this storage.
- scoped_ptr<CacheEntryIterator> GetCacheEntryIterator();
-
// Returns the local ID associated with the given resource ID.
FileError GetIdByResourceId(const std::string& resource_id,
std::string* out_id);
diff --git a/chrome/browser/chromeos/drive/resource_metadata_storage_unittest.cc b/chrome/browser/chromeos/drive/resource_metadata_storage_unittest.cc
index cf5571e..1414560 100644
--- a/chrome/browser/chromeos/drive/resource_metadata_storage_unittest.cc
+++ b/chrome/browser/chromeos/drive/resource_metadata_storage_unittest.cc
@@ -156,14 +156,6 @@
EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
}
- // Insert some cache entries.
- std::map<std::string, FileCacheEntry> cache_entries;
- cache_entries[keys[0]].set_md5("aaaaaa");
- cache_entries[keys[1]].set_md5("bbbbbb");
- for (std::map<std::string, FileCacheEntry>::iterator it =
- cache_entries.begin(); it != cache_entries.end(); ++it)
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(it->first, it->second));
-
// Iterate and check the result.
std::map<std::string, ResourceEntry> found_entries;
scoped_ptr<ResourceMetadataStorage::Iterator> it = storage_->GetIterator();
@@ -179,72 +171,6 @@
EXPECT_EQ(1U, found_entries.count(keys[i]));
}
-TEST_F(ResourceMetadataStorageTest, PutCacheEntry) {
- FileCacheEntry entry;
- const std::string key1 = "abcdefg";
- const std::string key2 = "abcd";
- const std::string md5_1 = "foo";
- const std::string md5_2 = "bar";
-
- // Put cache entries.
- entry.set_md5(md5_1);
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(key1, entry));
- entry.set_md5(md5_2);
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(key2, entry));
-
- // Get cache entires.
- EXPECT_EQ(FILE_ERROR_OK, storage_->GetCacheEntry(key1, &entry));
- EXPECT_EQ(md5_1, entry.md5());
- EXPECT_EQ(FILE_ERROR_OK, storage_->GetCacheEntry(key2, &entry));
- EXPECT_EQ(md5_2, entry.md5());
-
- // Remove cache entries.
- EXPECT_EQ(FILE_ERROR_OK, storage_->RemoveCacheEntry(key1));
- EXPECT_EQ(FILE_ERROR_NOT_FOUND, storage_->GetCacheEntry(key1, &entry));
-
- EXPECT_EQ(FILE_ERROR_OK, storage_->RemoveCacheEntry(key2));
- EXPECT_EQ(FILE_ERROR_NOT_FOUND, storage_->GetCacheEntry(key2, &entry));
-}
-
-TEST_F(ResourceMetadataStorageTest, CacheEntryIterator) {
- // Prepare data.
- std::map<std::string, FileCacheEntry> entries;
- FileCacheEntry cache_entry;
-
- cache_entry.set_md5("aA");
- entries["entry1"] = cache_entry;
- cache_entry.set_md5("bB");
- entries["entry2"] = cache_entry;
- cache_entry.set_md5("cC");
- entries["entry3"] = cache_entry;
- cache_entry.set_md5("dD");
- entries["entry4"] = cache_entry;
-
- for (std::map<std::string, FileCacheEntry>::iterator it = entries.begin();
- it != entries.end(); ++it)
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(it->first, it->second));
-
- // Insert some dummy entries.
- ResourceEntry entry;
- entry.set_local_id("entry1");
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
- entry.set_local_id("entry2");
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
-
- // Iterate and check the result.
- scoped_ptr<ResourceMetadataStorage::CacheEntryIterator> it =
- storage_->GetCacheEntryIterator();
- ASSERT_TRUE(it);
- size_t num_entries = 0;
- for (; !it->IsAtEnd(); it->Advance()) {
- EXPECT_EQ(1U, entries.count(it->GetID()));
- EXPECT_EQ(entries[it->GetID()].md5(), it->GetValue().md5());
- ++num_entries;
- }
- EXPECT_FALSE(it->HasError());
- EXPECT_EQ(entries.size(), num_entries);
-}
-
TEST_F(ResourceMetadataStorageTest, GetIdByResourceId) {
const std::string local_id = "local_id";
const std::string resource_id = "resource_id";
@@ -308,13 +234,6 @@
}
}
- // Put some dummy cache entries.
- for (size_t i = 0; i < arraysize(parents_id); ++i) {
- FileCacheEntry cache_entry;
- EXPECT_EQ(FILE_ERROR_OK,
- storage_->PutCacheEntry(parents_id[i], cache_entry));
- }
-
// Try to get children.
for (size_t i = 0; i < children_name_id.size(); ++i) {
std::vector<std::string> children;
@@ -465,8 +384,6 @@
ResourceEntry entry;
entry.set_local_id(key1);
EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
- FileCacheEntry cache_entry;
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(key1, cache_entry));
// Set newer version, upgrade and reopen DB.
SetDBVersion(ResourceMetadataStorage::kDBVersion + 1);
@@ -483,7 +400,6 @@
storage_->GetLargestChangestamp(&largest_changestamp));
EXPECT_EQ(0, largest_changestamp);
EXPECT_EQ(FILE_ERROR_NOT_FOUND, storage_->GetEntry(key1, &entry));
- EXPECT_EQ(FILE_ERROR_NOT_FOUND, storage_->GetCacheEntry(key1, &cache_entry));
}
TEST_F(ResourceMetadataStorageTest, WrongPath) {
@@ -498,19 +414,12 @@
}
TEST_F(ResourceMetadataStorageTest, RecoverCacheEntriesFromTrashedResourceMap) {
- // Put some cache entries.
- FileCacheEntry cache_entry;
- cache_entry.set_md5("md5_foo");
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry("id_foo", cache_entry));
- cache_entry.set_md5("md5_bar");
- cache_entry.set_is_dirty(true);
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry("id_bar", cache_entry));
-
// Put entry with id_foo.
ResourceEntry entry;
entry.set_local_id("id_foo");
entry.set_base_name("foo");
entry.set_title("foo");
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_md5("md5_foo");
EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
// Put entry with id_bar as a id_foo's child.
@@ -518,6 +427,8 @@
entry.set_parent_local_id("id_foo");
entry.set_base_name("bar");
entry.set_title("bar");
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_md5("md5_bar");
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_is_dirty(true);
EXPECT_EQ(FILE_ERROR_OK, storage_->PutEntry(entry));
// Remove parent-child relationship to make the DB invalid.
@@ -596,11 +507,6 @@
PutChild(key2, name3, key3);
EXPECT_TRUE(CheckValidity());
- // Add some cache entries.
- FileCacheEntry cache_entry;
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(key1, cache_entry));
- EXPECT_EQ(FILE_ERROR_OK, storage_->PutCacheEntry(key2, cache_entry));
-
// Remove key2.
RemoveChild(key1, name2);
EXPECT_FALSE(CheckValidity());
diff --git a/chrome/browser/chromeos/drive/resource_metadata_unittest.cc b/chrome/browser/chromeos/drive/resource_metadata_unittest.cc
index 3df3214..9f96e68b 100644
--- a/chrome/browser/chromeos/drive/resource_metadata_unittest.cc
+++ b/chrome/browser/chromeos/drive/resource_metadata_unittest.cc
@@ -352,6 +352,35 @@
resource_metadata_->RefreshEntry(new_entry));
}
+TEST_F(ResourceMetadataTest, RefreshEntry_DoNotOverwriteCacheState) {
+ ResourceEntry entry;
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->GetResourceEntryByPath(
+ base::FilePath::FromUTF8Unsafe("drive/root/dir1/file4"), &entry));
+
+ // Try to set MD5 with RefreshEntry.
+ entry.mutable_file_specific_info()->mutable_cache_state()->set_md5("md5");
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->RefreshEntry(entry));
+
+ // Cache state is unchanged.
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->GetResourceEntryByPath(
+ base::FilePath::FromUTF8Unsafe("drive/root/dir1/file4"), &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().md5().empty());
+
+ // Pin the file.
+ EXPECT_EQ(FILE_ERROR_OK, cache_->Pin(entry.local_id()));
+
+ // Try to clear the cache state with RefreshEntry.
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->GetResourceEntryByPath(
+ base::FilePath::FromUTF8Unsafe("drive/root/dir1/file4"), &entry));
+ entry.mutable_file_specific_info()->clear_cache_state();
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->RefreshEntry(entry));
+
+ // Cache state is not cleared.
+ EXPECT_EQ(FILE_ERROR_OK, resource_metadata_->GetResourceEntryByPath(
+ base::FilePath::FromUTF8Unsafe("drive/root/dir1/file4"), &entry));
+ EXPECT_TRUE(entry.file_specific_info().cache_state().is_pinned());
+}
+
TEST_F(ResourceMetadataTest, GetSubDirectoriesRecursively) {
std::set<base::FilePath> sub_directories;
