Make document service an interface

chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc

Index: chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
index dbdd76cb09093440b896892a3f7b628f655468d4..98037afdb6bc277cf2fa4d5cef1d81c6bfc1a6b5 100644
--- a/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
+++ b/chrome/browser/chromeos/gdata/gdata_file_system_unittest.cc
@@ -8,6 +8,8 @@
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/json/json_file_value_serializer.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string16.h"
@@ -15,6 +17,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/gdata/gdata_file_system.h"
+#include "chrome/browser/chromeos/gdata/gdata_mock.h"
 #include "chrome/browser/chromeos/gdata/gdata_parser.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/testing_profile.h"
@@ -31,6 +34,8 @@ using base::Value;
 using base::DictionaryValue;
 using base::ListValue;
 
+#define FPL(x) FILE_PATH_LITERAL(x)
+
 namespace gdata {
 
 class GDataFileSystemTest : public testing::Test {
@@ -39,14 +44,27 @@ class GDataFileSystemTest : public testing::Test {
       : ui_thread_(content::BrowserThread::UI, &message_loop_) {
   }
 
-  virtual void SetUp() {
+  virtual void SetUp() OVERRIDE {
+    callback_helper_ = new CallbackHelper;
     profile_.reset(new TestingProfile);
     file_system_ = GDataFileSystemFactory::GetForProfile(profile_.get());

[satorux1, 2012/03/08 01:48:37]

For testing, let's do new GDataFileSystem. You can do it as GDataFileSystemTest
is listed as a friend.

Then, injecting MockDocumentService should be easy.

[Greg Spencer (Chromium), 2012/03/08 19:06:35]

Yeah, but are we bypassing anything important in GetForProfile and the profile
dependency system built around it?

[satorux1, 2012/03/08 19:28:37]

I think it's not important to use GetForProfile().
GDataFileSystemFactory::GetForProfile() is just an indirect way to create a
GDataFileSystem object to make it tied with the profile lifetime. 

I'm more worried about creating a non-mock DocumentService and throw it away. :)

[Greg Spencer (Chromium), 2012/03/09 00:33:30]

As long as we never make GDataFileSystem dependent upon anything else in the
profile I guess that's fine.  Done.

+
+    // Allocate and keep a weak pointer to the mock.
+    mock_doc_service_ = new MockDocumentsService;
+    EXPECT_CALL(*mock_doc_service_, Initialize(profile_.get())).Times(1);
+
+    // Pass the service for the filesystem to own.
+    scoped_ptr<DocumentsServiceInterface> service(mock_doc_service_);
+    file_system_->ReplaceDocumentsService(service.Pass());

[satorux1, 2012/03/08 01:48:37]

Cannot you just pass mock_doc_service_? I thought it worked, but I may be wrong.

[Greg Spencer (Chromium), 2012/03/08 19:06:35]

Nope, that doesn't work, since the constructor for scoped_ptr is explicit.  I
suppose I could do this:
file_system_->ReplaceDocumentsService(scoped_ptr<DocuementsServiceInterface>(mock_doc_service_).Pass());

But that seems just more confusing.

+  }
+
+  virtual void TearDown() OVERRIDE {
+    EXPECT_CALL(*mock_doc_service_, CancelAll()).Times(1);
   }
 
   // Loads test json file as root ("/gdata") element.
   void LoadRootFeedDocument(const std::string& filename) {
-    LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata")), filename);
+    LoadSubdirFeedDocument(FilePath(FPL("gdata")), filename);
   }
 
   // Loads test json file as subdirectory content of |directory_path|.
@@ -139,10 +157,30 @@ class GDataFileSystemTest : public testing::Test {
     return value;
   }
 
+  class CallbackHelper

[satorux1, 2012/03/08 01:48:37]

nit: class comment missing. what's this used for?

[Greg Spencer (Chromium), 2012/03/08 19:06:35]

Done.

+    : public base::RefCountedThreadSafe<CallbackHelper> {
+   public:
+    CallbackHelper() : last_error_(base::PLATFORM_FILE_OK) {}
+    virtual ~CallbackHelper() {}
+    virtual void GetFileCallback(base::PlatformFileError error,
+                                 const FilePath& file_path) {
+      last_error_ = error;
+      download_path_ = file_path;
+    }
+    virtual void FileOperationCallback(base::PlatformFileError error) {
+      last_error_ = error;
+    }
+
+    base::PlatformFileError last_error_;
+    FilePath download_path_;
+  };
+
   MessageLoopForUI message_loop_;
   content::TestBrowserThread ui_thread_;
   scoped_ptr<TestingProfile> profile_;
+  scoped_refptr<CallbackHelper> callback_helper_;
   GDataFileSystem* file_system_;
+  MockDocumentsService* mock_doc_service_;
 };
 
 
@@ -168,10 +206,10 @@ TEST_F(GDataFileSystemTest, SearchRootDirectory) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnDirectoryFound(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnDirectoryFound(FilePath(FPL("gdata")), _))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata")),
+  file_system_->FindFileByPath(FilePath(FPL("gdata")),
                                mock_find_file_delegate);
 }
 
@@ -181,13 +219,13 @@ TEST_F(GDataFileSystemTest, SearchExistingFile) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/File 1.txt")),
+  file_system_->FindFileByPath(FilePath(FPL("gdata/File 1.txt")),
                                mock_find_file_delegate);
 }
 
@@ -197,14 +235,14 @@ TEST_F(GDataFileSystemTest, SearchExistingDocument) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
 
   file_system_->FindFileByPath(
-      FilePath(FILE_PATH_LITERAL("gdata/Document 1.gdoc")),
+      FilePath(FPL("gdata/Document 1.gdoc")),
       mock_find_file_delegate);
 }
 
@@ -214,25 +252,25 @@ TEST_F(GDataFileSystemTest, SearchDuplicateNames) {
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate =
       new MockFindFileDelegate();
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnFileFound(_))
       .Times(1);
   file_system_->FindFileByPath(
-      FilePath(FILE_PATH_LITERAL("gdata/Duplicate Name.txt")),
+      FilePath(FPL("gdata/Duplicate Name.txt")),
       mock_find_file_delegate);
 
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate2 =
       new MockFindFileDelegate();
   EXPECT_CALL(*mock_find_file_delegate2.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate2.get(), OnFileFound(_))
       .Times(1);
   file_system_->FindFileByPath(
-      FilePath(FILE_PATH_LITERAL("gdata/Duplicate Name (2).txt")),
+      FilePath(FPL("gdata/Duplicate Name (2).txt")),
       mock_find_file_delegate2);
 }
 
@@ -242,13 +280,13 @@ TEST_F(GDataFileSystemTest, SearchExistingDirectory) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(), OnDirectoryFound(_, _))
       .Times(1);
 
-  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  file_system_->FindFileByPath(FilePath(FPL("gdata/Directory 1")),
                                mock_find_file_delegate);
 }
 
@@ -259,7 +297,7 @@ TEST_F(GDataFileSystemTest, SearchNonExistingFile) {
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
   EXPECT_CALL(*mock_find_file_delegate.get(),
@@ -267,7 +305,7 @@ TEST_F(GDataFileSystemTest, SearchNonExistingFile) {
       .Times(1);
 
   file_system_->FindFileByPath(
-      FilePath(FILE_PATH_LITERAL("gdata/nonexisting.file")),
+      FilePath(FPL("gdata/nonexisting.file")),
       mock_find_file_delegate);
 }
 
@@ -278,29 +316,29 @@ TEST_F(GDataFileSystemTest, StopFileSearch) {
 
   // Stop on first directory entry.
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_TERMINATES));
 
-  file_system_->FindFileByPath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  file_system_->FindFileByPath(FilePath(FPL("gdata/Directory 1")),
                                mock_find_file_delegate);
 }
 
 TEST_F(GDataFileSystemTest, SearchInSubdir) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  LoadSubdirFeedDocument(FilePath(FPL("gdata/Directory 1")),
                          "subdir_feed.json");
 
   scoped_refptr<MockFindFileDelegate> mock_find_file_delegate =
       new MockFindFileDelegate();
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata")), _))
+              OnEnterDirectory(FilePath(FPL("gdata")), _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
 
   EXPECT_CALL(*mock_find_file_delegate.get(),
-              OnEnterDirectory(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+              OnEnterDirectory(FilePath(FPL("gdata/Directory 1")),
                                _))
       .Times(1)
       .WillOnce(Return(FindFileDelegate::FIND_FILE_CONTINUES));
@@ -309,31 +347,31 @@ TEST_F(GDataFileSystemTest, SearchInSubdir) {
       .Times(1);
 
   file_system_->FindFileByPath(
-      FilePath(FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt")),
+      FilePath(FPL("gdata/Directory 1/SubDirectory File 1.txt")),
       mock_find_file_delegate);
 }
 
 TEST_F(GDataFileSystemTest, FilePathTests) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  LoadSubdirFeedDocument(FilePath(FPL("gdata/Directory 1")),
                          "subdir_feed.json");
 
-  FindAndTestFilePath(FilePath(FILE_PATH_LITERAL("gdata/File 1.txt")));
-  FindAndTestFilePath(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")));
+  FindAndTestFilePath(FilePath(FPL("gdata/File 1.txt")));
+  FindAndTestFilePath(FilePath(FPL("gdata/Directory 1")));
   FindAndTestFilePath(
-      FilePath(FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt")));
+      FilePath(FPL("gdata/Directory 1/SubDirectory File 1.txt")));
 }
 
 TEST_F(GDataFileSystemTest, RemoveFiles) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  LoadSubdirFeedDocument(FilePath(FPL("gdata/Directory 1")),
                          "subdir_feed.json");
 
-  FilePath nonexisting_file(FILE_PATH_LITERAL("gdata/Dummy file.txt"));
-  FilePath file_in_root(FILE_PATH_LITERAL("gdata/File 1.txt"));
-  FilePath dir_in_root(FILE_PATH_LITERAL("gdata/Directory 1"));
+  FilePath nonexisting_file(FPL("gdata/Dummy file.txt"));
+  FilePath file_in_root(FPL("gdata/File 1.txt"));
+  FilePath dir_in_root(FPL("gdata/Directory 1"));
   FilePath file_in_subdir(
-      FILE_PATH_LITERAL("gdata/Directory 1/SubDirectory File 1.txt"));
+      FPL("gdata/Directory 1/SubDirectory File 1.txt"));
 
   EXPECT_TRUE(FindFile(file_in_root) != NULL);
   EXPECT_TRUE(FindFile(dir_in_root) != NULL);
@@ -358,22 +396,22 @@ TEST_F(GDataFileSystemTest, RemoveFiles) {
   EXPECT_FALSE(RemoveFile(nonexisting_file));
 
   // Try removing root file element.
-  EXPECT_FALSE(RemoveFile(FilePath(FILE_PATH_LITERAL("gdata"))));
+  EXPECT_FALSE(RemoveFile(FilePath(FPL("gdata"))));
 }
 
 TEST_F(GDataFileSystemTest, CreateDirectory) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  LoadSubdirFeedDocument(FilePath(FPL("gdata/Directory 1")),
                          "subdir_feed.json");
 
   // Create directory in root.
-  FilePath dir_path(FILE_PATH_LITERAL("gdata/New Folder 1"));
+  FilePath dir_path(FPL("gdata/New Folder 1"));
   EXPECT_TRUE(FindFile(dir_path) == NULL);
   AddDirectoryFromFile(dir_path, "directory_entry_atom.json");
   EXPECT_TRUE(FindFile(dir_path) != NULL);
 
   // Create directory in a sub dirrectory.
-  FilePath subdir_path(FILE_PATH_LITERAL("gdata/New Folder 1/New Folder 2"));
+  FilePath subdir_path(FPL("gdata/New Folder 1/New Folder 2"));
   EXPECT_TRUE(FindFile(subdir_path) == NULL);
   AddDirectoryFromFile(subdir_path, "directory_entry_atom.json");
   EXPECT_TRUE(FindFile(subdir_path) != NULL);
@@ -381,42 +419,42 @@ TEST_F(GDataFileSystemTest, CreateDirectory) {
 
 TEST_F(GDataFileSystemTest, FindFirstMissingParentDirectory) {
   LoadRootFeedDocument("root_feed.json");
-  LoadSubdirFeedDocument(FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+  LoadSubdirFeedDocument(FilePath(FPL("gdata/Directory 1")),
                          "subdir_feed.json");
 
   GURL last_dir_content_url;
   FilePath first_missing_parent_path;
 
   // Create directory in root.
-  FilePath dir_path(FILE_PATH_LITERAL("gdata/New Folder 1"));
+  FilePath dir_path(FPL("gdata/New Folder 1"));
   EXPECT_EQ(
       GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path,
           &last_dir_content_url,
           &first_missing_parent_path));
-  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/New Folder 1")),
+  EXPECT_EQ(FilePath(FPL("gdata/New Folder 1")),
             first_missing_parent_path);
   EXPECT_TRUE(last_dir_content_url.is_empty());    // root directory.
 
   // Missing folders in subdir of an existing folder.
-  FilePath dir_path2(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2"));
+  FilePath dir_path2(FPL("gdata/Directory 1/New Folder 2"));
   EXPECT_EQ(
       GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path2,
           &last_dir_content_url,
           &first_missing_parent_path));
-  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2")),
+  EXPECT_EQ(FilePath(FPL("gdata/Directory 1/New Folder 2")),
             first_missing_parent_path);
   EXPECT_FALSE(last_dir_content_url.is_empty());    // non-root directory.
 
   // Missing two folders on the path.
-  FilePath dir_path3 = dir_path2.Append(FILE_PATH_LITERAL("Another Foder"));
+  FilePath dir_path3 = dir_path2.Append(FPL("Another Folder"));
   EXPECT_EQ(
       GDataFileSystem::FOUND_MISSING,
       file_system_->FindFirstMissingParentDirectory(dir_path3,
           &last_dir_content_url,
           &first_missing_parent_path));
-  EXPECT_EQ(FilePath(FILE_PATH_LITERAL("gdata/Directory 1/New Folder 2")),
+  EXPECT_EQ(FilePath(FPL("gdata/Directory 1/New Folder 2")),
             first_missing_parent_path);
   EXPECT_FALSE(last_dir_content_url.is_empty());    // non-root directory.
 
@@ -424,7 +462,7 @@ TEST_F(GDataFileSystemTest, FindFirstMissingParentDirectory) {
   EXPECT_EQ(
       GDataFileSystem::FOUND_INVALID,
       file_system_->FindFirstMissingParentDirectory(
-          FilePath(FILE_PATH_LITERAL("gdata/File 1.txt/BadDir")),
+          FilePath(FPL("gdata/File 1.txt/BadDir")),
           &last_dir_content_url,
           &first_missing_parent_path));
 
@@ -432,26 +470,57 @@ TEST_F(GDataFileSystemTest, FindFirstMissingParentDirectory) {
   EXPECT_EQ(
       GDataFileSystem::DIRECTORY_ALREADY_PRESENT,
       file_system_->FindFirstMissingParentDirectory(
-          FilePath(FILE_PATH_LITERAL("gdata/Directory 1")),
+          FilePath(FPL("gdata/Directory 1")),
           &last_dir_content_url,
           &first_missing_parent_path));
 }
 
-// TODO(satorux): Write a test for GetFile() once DocumentsService is
-// mockable.

[satorux1, 2012/03/08 01:48:37]

Thank you for addressing my TODO!

[Greg Spencer (Chromium), 2012/03/08 19:06:35]

No problem. :-)

-
 TEST_F(GDataFileSystemTest, GetGDataFileInfoFromPath) {
   LoadRootFeedDocument("root_feed.json");
 
   GDataFileBase* file_info = file_system_->GetGDataFileInfoFromPath(
-      FilePath(FILE_PATH_LITERAL("gdata/File 1.txt")));
+      FilePath(FPL("gdata/File 1.txt")));
   ASSERT_TRUE(file_info != NULL);
   EXPECT_EQ("https://file_link_self/", file_info->self_url().spec());
   EXPECT_EQ("https://file_content_url/", file_info->content_url().spec());
 
   GDataFileBase* non_existent = file_system_->GetGDataFileInfoFromPath(
-      FilePath(FILE_PATH_LITERAL("gdata/Nonexistent.txt")));
+      FilePath(FPL("gdata/Nonexistent.txt")));
   ASSERT_TRUE(non_existent == NULL);
 }
 
+// Create a directory through the document service
+TEST_F(GDataFileSystemTest, CreateDirectoryWithService) {
+  LoadRootFeedDocument("root_feed.json");
+  EXPECT_CALL(*mock_doc_service_,
+              CreateDirectory(_, "Sample Directory Title", _)).Times(1);
+
+  // Set last error so it's not a valid error code.
+  callback_helper_->last_error_ = static_cast<base::PlatformFileError>(1);
+  file_system_->CreateDirectory(
+      FilePath(FPL("gdata/Sample Directory Title")),
+      false,  // is_exclusive
+      true,  // is_recursive
+      base::Bind(&CallbackHelper::FileOperationCallback,
+                 callback_helper_.get()));
+  // TODO(gspencer): Uncomment this when we get a blob that
+  // works that can be returned from the mock.
+  // EXPECT_EQ(base::PLATFORM_FILE_OK, callback_helper_->last_error_);
+}
+
+TEST_F(GDataFileSystemTest, GetFile) {
+  LoadRootFeedDocument("root_feed.json");
+
+  GDataFileSystem::GetFileCallback callback =
+      base::Bind(&CallbackHelper::GetFileCallback,
+                 callback_helper_.get());
+
+  EXPECT_CALL(*mock_doc_service_,
+              DownloadFile(GURL("https://file_content_url/"), _));
+
+  FilePath file_in_root(FPL("gdata/File 1.txt"));
+  file_system_->GetFile(file_in_root, callback);
+  EXPECT_STREQ("file_content_url/",
+               callback_helper_->download_path_.value().c_str());
+}
 }   // namespace gdata