Support contents upload on FakeDriveService

chrome/browser/drive/fake_drive_service.cc

Index: chrome/browser/drive/fake_drive_service.cc
diff --git a/chrome/browser/drive/fake_drive_service.cc b/chrome/browser/drive/fake_drive_service.cc
index 32d9d2d1720bb47382ac018741308110a07e894f..6e7b260d8740837e672a6daa7a3a098214239c0f 100644
--- a/chrome/browser/drive/fake_drive_service.cc
+++ b/chrome/browser/drive/fake_drive_service.cc
@@ -6,7 +6,9 @@
 
 #include <string>
 
+#include "base/file_util.h"
 #include "base/logging.h"
+#include "base/md5.h"
 #include "base/message_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
@@ -96,28 +98,6 @@ bool EntryMatchWithQuery(const ResourceEntry& entry,
   return true;
 }
 
-// Gets the upload URL from the given entry. Returns an empty URL if not
-// found.
-GURL GetUploadUrl(const base::DictionaryValue& entry) {
-  std::string upload_url;
-  const base::ListValue* links = NULL;
-  if (entry.GetList("link", &links) && links) {
-    for (size_t link_index = 0;
-         link_index < links->GetSize();
-         ++link_index) {
-      const base::DictionaryValue* link = NULL;
-      std::string rel;
-      if (links->GetDictionary(link_index, &link) &&
-          link && link->GetString("rel", &rel) &&
-          rel == kUploadUrlRel &&
-          link->GetString("href", &upload_url)) {
-        break;
-      }
-    }
-  }
-  return GURL(upload_url);
-}
-
 // Returns |url| without query parameter.
 GURL RemoveQueryParameter(const GURL& url) {
   GURL::Replacements replacements;
@@ -127,16 +107,49 @@ GURL RemoveQueryParameter(const GURL& url) {
 
 }  // namespace
 
+struct FakeDriveService::UploadSession {
+  std::string content_type;
+  int64 content_length;
+  std::string parent_resource_id;
+  std::string resource_id;
+  std::string etag;
+  std::string title;
+
+  int64 uploaded_size;
+
+  UploadSession()
+      : content_length(0),
+        uploaded_size(0) {}
+
+  UploadSession(
+      std::string content_type,
+      int64 content_length,
+      std::string parent_resource_id,
+      std::string resource_id,
+      std::string etag,
+      std::string title)
+    : content_type(content_type),
+      content_length(content_length),
+      parent_resource_id(parent_resource_id),
+      resource_id(resource_id),
+      etag(etag),
+      title(title),
+      uploaded_size(0) {
+  }
+};
+
 FakeDriveService::FakeDriveService()
     : largest_changestamp_(0),
       published_date_seq_(0),
+      next_upload_sequence_number_(0),
       default_max_results_(0),
       resource_id_count_(0),
       resource_list_load_count_(0),
       change_list_load_count_(0),
       directory_load_count_(0),
       about_resource_load_count_(0),
-      offline_(false) {
+      offline_(false),
+      weak_factory_(this) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
@@ -548,7 +561,6 @@ CancelCallback FakeDriveService::DownloadFile(
   // Write "x"s of the file size specified in the entry.
   std::string file_size_string;
   entry->GetString("docs$size.$t", &file_size_string);
-  // TODO(satorux): To be correct, we should update docs$md5Checksum.$t here.
   int64 file_size = 0;
   if (base::StringToInt64(file_size_string, &file_size)) {
     base::BinaryValue* content_binary_data;
@@ -892,27 +904,26 @@ CancelCallback FakeDriveService::InitiateUploadNewFile(
     return CancelCallback();
   }
 
-  // Content length should be zero, as we'll create an empty file first. The
-  // content will be added in ResumeUpload().
-  const base::DictionaryValue* new_entry = AddNewEntry(content_type,
-                                                       "",  // content_data
-                                                       parent_resource_id,
-                                                       title,
-                                                       false,  // shared_with_me
-                                                       "file");
-  if (!new_entry) {
+  if (parent_resource_id != GetRootResourceId() &&
+      !FindEntryByResourceId(parent_resource_id)) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback, HTTP_NOT_FOUND, GURL()));
     return CancelCallback();
   }
-  const GURL upload_url = GetUploadUrl(*new_entry);
-  DCHECK(upload_url.is_valid());
+
+  GURL session_url = net::AppendQueryParameter(
+      GetNewUploadSessionUrl(), "mode", "newfile");
+  upload_session_[session_url] =
+      UploadSession(content_type, content_length,
+                    parent_resource_id,
+                    "",  // resource_id
+                    "",  // etag
+                    title);
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
-      base::Bind(callback, HTTP_SUCCESS,
-                 net::AppendQueryParameter(upload_url, "mode", "newfile")));
+      base::Bind(callback, HTTP_SUCCESS, session_url));
   return CancelCallback();
 }
 
@@ -949,15 +960,19 @@ CancelCallback FakeDriveService::InitiateUploadExistingFile(
         base::Bind(callback, HTTP_PRECONDITION, GURL()));
     return CancelCallback();
   }
-  entry->SetString("docs$size.$t", "0");
 
-  const GURL upload_url = GetUploadUrl(*entry);
-  DCHECK(upload_url.is_valid());
+  GURL session_url = net::AppendQueryParameter(
+      GetNewUploadSessionUrl(), "mode", "existing");
+  upload_session_[session_url] =
+      UploadSession(content_type, content_length,
+                    "",  // parent_resource_id
+                    resource_id,
+                    entry_etag,
+                    "" /* title */);
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
-      base::Bind(callback, HTTP_SUCCESS,
-                 net::AppendQueryParameter(upload_url, "mode", "existing")));
+      base::Bind(callback, HTTP_SUCCESS, session_url));
   return CancelCallback();
 }
 
@@ -968,6 +983,38 @@ CancelCallback FakeDriveService::GetUploadStatus(
     const UploadRangeCallback& callback) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
+
+  if (!ContainsKey(upload_session_, upload_url)) {

[hashimoto, 2013/06/21 04:16:21]

nit: Hmn, I didn't know that we had this function in stl_util.h.
We usually use std::map::count to do this kind of check.
What is the merit of this function over upload_session_.count(upload_url) > 0?

[tzik, 2013/06/21 05:06:52]

Changed this to count().
They are exactly same in their functionality or performance.

+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, UploadRangeResponse(HTTP_NOT_FOUND, 0, 0),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
+
+  const UploadSession& session = upload_session_[upload_url];
+  if (session.uploaded_size == session.content_length) {
+    if (session.resource_id.empty()) {
+      base::MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(callback, UploadRangeResponse(HTTP_CREATED, 0, 0),
+                     base::Passed(scoped_ptr<ResourceEntry>())));

[hashimoto, 2013/06/21 04:16:21]

Why are you returning null pointers even in success cases?

BTW, is this part needed to be in this change?
Please add tests for this method if you change the implementation.

[tzik, 2013/06/21 05:06:52]

IIUC, the server doesn't return resource entry for this request. Let me check
it.

[tzik, 2013/06/21 05:35:04]

Done. I move this part of change to another CL.

+      return CancelCallback();
+    }
+
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback, UploadRangeResponse(HTTP_SUCCESS, 0, 0),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
+
+  base::MessageLoop::current()->PostTask(
+      FROM_HERE,
+      base::Bind(callback,
+                 UploadRangeResponse(HTTP_RESUME_INCOMPLETE,
+                                     0, session.uploaded_size),
+                 base::Passed(scoped_ptr<ResourceEntry>())));
   return CancelCallback();
 }
 
@@ -984,8 +1031,6 @@ CancelCallback FakeDriveService::ResumeUpload(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(!callback.is_null());
 
-  scoped_ptr<ResourceEntry> result_entry;
-
   if (offline_) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
@@ -993,42 +1038,37 @@ CancelCallback FakeDriveService::ResumeUpload(
                    UploadRangeResponse(GDATA_NO_CONNECTION,
                                        start_position,
                                        end_position),
-                   base::Passed(&result_entry)));
+                   base::Passed(scoped_ptr<ResourceEntry>())));
     return CancelCallback();
   }
 
-  DictionaryValue* entry = NULL;
-  entry = FindEntryByUploadUrl(RemoveQueryParameter(upload_url));
-  if (!entry) {
+  if (!ContainsKey(upload_session_, upload_url)) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    UploadRangeResponse(HTTP_NOT_FOUND,
                                        start_position,
                                        end_position),
-                   base::Passed(&result_entry)));
+                   base::Passed(scoped_ptr<ResourceEntry>())));
     return CancelCallback();
   }
 
+  UploadSession* session = &upload_session_[upload_url];
+
   // Chunks are required to be sent in such a ways that they fill from the start
   // of the not-yet-uploaded part with no gaps nor overlaps.
-  std::string current_size_string;
-  int64 current_size;
-  if (!entry->GetString("docs$size.$t", &current_size_string) ||
-      !base::StringToInt64(current_size_string, &current_size) ||
-      current_size != start_position) {
+  int64 current_size = session->uploaded_size;

[hashimoto, 2013/06/21 04:16:21]

nit: Do we need this variable?

[tzik, 2013/06/21 05:06:52]

Done.

+  if (current_size != start_position) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    UploadRangeResponse(HTTP_BAD_REQUEST,
                                        start_position,
                                        end_position),
-                   base::Passed(&result_entry)));
+                   base::Passed(scoped_ptr<ResourceEntry>())));
     return CancelCallback();
   }
 
-  entry->SetString("docs$size.$t", base::Int64ToString(end_position));
-
   if (!progress_callback.is_null()) {
     // In the real GDataWapi/Drive DriveService, progress is reported in
     // nondeterministic timing. In this fake implementation, we choose to call
@@ -1044,35 +1084,98 @@ CancelCallback FakeDriveService::ResumeUpload(
   }
 
   if (content_length != end_position) {
+    session->uploaded_size = end_position;
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(callback,
                    UploadRangeResponse(HTTP_RESUME_INCOMPLETE,
                                        start_position,
                                        end_position),
-                    base::Passed(&result_entry)));
+                   base::Passed(scoped_ptr<ResourceEntry>())));
     return CancelCallback();
   }
 
-  AddNewChangestamp(entry);
-  result_entry = ResourceEntry::CreateFrom(*entry).Pass();
+  std::string content_data;
+  if (!file_util::ReadFileToString(local_file_path, &content_data)) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   UploadRangeResponse(GDATA_FILE_ERROR,
+                                       start_position,
+                                       end_position),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
+  session->uploaded_size = end_position;
+
+  if (session->resource_id.empty()) {

[hashimoto, 2013/06/21 04:16:21]

resource_id.empty() is true iff the upload is for a new file, right?
I think having a comment about it would help future readers.

BTW, do we still need to append mode=newfile|existing parameter for URLs?

[tzik, 2013/06/21 05:06:52]

Done.

+    const DictionaryValue* new_entry = AddNewEntry(
+        session->content_type,
+        content_data,
+        session->parent_resource_id,
+        session->title,
+        false,  // shared_with_me
+        "file");
+    if (!new_entry) {
+      base::MessageLoop::current()->PostTask(
+          FROM_HERE,
+          base::Bind(callback,
+                     UploadRangeResponse(HTTP_NOT_FOUND,
+                                         start_position,
+                                         end_position),
+                     base::Passed(scoped_ptr<ResourceEntry>())));
+      return CancelCallback();
+    }
+
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   UploadRangeResponse(HTTP_CREATED,
+                                       start_position,
+                                       end_position),
+                   base::Passed(ResourceEntry::CreateFrom(*new_entry))));
+    return CancelCallback();
+  }
 
-  std::string upload_mode;
-  bool upload_mode_found =
-      net::GetValueForKeyInQuery(upload_url, "mode", &upload_mode);
-  DCHECK(upload_mode_found &&
-         (upload_mode == "newfile" || upload_mode == "existing"));
+  DictionaryValue* entry = FindEntryByResourceId(session->resource_id);
+  if (!entry) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   UploadRangeResponse(HTTP_NOT_FOUND,
+                                       start_position,
+                                       end_position),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
+
+  std::string entry_etag;
+  entry->GetString("gd$etag", &entry_etag);
+  if (entry_etag.empty() || session->etag != entry_etag) {
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(callback,
+                   UploadRangeResponse(HTTP_PRECONDITION,
+                                       start_position,
+                                       end_position),
+                   base::Passed(scoped_ptr<ResourceEntry>())));
+    return CancelCallback();
+  }
 
-  GDataErrorCode return_code =
-      upload_mode == "newfile" ? HTTP_CREATED : HTTP_SUCCESS;
+  entry->SetString("docs$md5Checksum.$t", base::MD5String(content_data));
+  entry->Set("test$data",
+             base::BinaryValue::CreateWithCopiedBuffer(
+                 content_data.data(), content_data.size()));
+  entry->SetString("docs$size.$t", base::Int64ToString(end_position));
+  AddNewChangestamp(entry);
 
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(callback,
-                 UploadRangeResponse(return_code,
+                 UploadRangeResponse(HTTP_SUCCESS,

[hashimoto, 2013/06/21 04:16:21]

Please return HTTP_CREATED for new files.

[tzik, 2013/06/21 05:06:52]

Uploads for new file are handled around 1111, so this is existing file case.

[hashimoto, 2013/06/21 05:41:24]

You're right, thanks.

                                      start_position,
                                      end_position),
-                 base::Passed(&result_entry)));
+                 base::Passed(ResourceEntry::CreateFrom(*entry))));
   return CancelCallback();
 }
 
@@ -1214,40 +1317,6 @@ base::DictionaryValue* FakeDriveService::FindEntryByContentUrl(
   return NULL;
 }
 
-base::DictionaryValue* FakeDriveService::FindEntryByUploadUrl(
-    const GURL& upload_url) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  base::ListValue* entries = NULL;
-  // Go through entries and return the one that matches |upload_url|.
-  if (resource_list_value_->GetList("entry", &entries)) {
-    for (size_t i = 0; i < entries->GetSize(); ++i) {
-      base::DictionaryValue* entry = NULL;
-      base::ListValue* links = NULL;
-      if (entries->GetDictionary(i, &entry) &&
-          entry->GetList("link", &links) &&
-          links) {
-        for (size_t link_index = 0;
-            link_index < links->GetSize();
-            ++link_index) {
-          base::DictionaryValue* link = NULL;
-          std::string rel;
-          std::string found_upload_url;
-          if (links->GetDictionary(link_index, &link) &&
-              link && link->GetString("rel", &rel) &&
-              rel == kUploadUrlRel &&
-              link->GetString("href", &found_upload_url) &&
-              GURL(found_upload_url) == upload_url) {
-            return entry;
-          }
-        }
-      }
-    }
-  }
-
-  return NULL;
-}
-
 std::string FakeDriveService::GetNewResourceId() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -1259,6 +1328,8 @@ void FakeDriveService::AddNewChangestamp(base::DictionaryValue* entry) {
   ++largest_changestamp_;
   entry->SetString("docs$changestamp.value",
                    base::Int64ToString(largest_changestamp_));
+  entry->SetString("gd$etag",
+                   "etag_" + base::Int64ToString(largest_changestamp_));
 }
 
 const base::DictionaryValue* FakeDriveService::AddNewEntry(
@@ -1290,9 +1361,8 @@ const base::DictionaryValue* FakeDriveService::AddNewEntry(
             content_data.c_str(), content_data.size()));
     new_entry->SetString("docs$size.$t",
                          base::Int64ToString(content_data.size()));
-    // TODO(satorux): Set the correct MD5 here.
     new_entry->SetString("docs$md5Checksum.$t",
-                         "3b4385ebefec6e743574c76bbd0575de");
+                         base::MD5String(content_data));
   }
 
   // Add "category" which sets the resource type to |entry_kind|.
@@ -1477,4 +1547,9 @@ void FakeDriveService::GetResourceListInternal(
                  base::Passed(&resource_list)));
 }
 
+GURL FakeDriveService::GetNewUploadSessionUrl() {
+  return GURL("https://upload_session_url/" +
+              base::Int64ToString(next_upload_sequence_number_++));
+}
+
 }  // namespace drive