Files.app: Implement sending batch requests.

google_apis/drive/drive_api_requests.cc

Index: google_apis/drive/drive_api_requests.cc
diff --git a/google_apis/drive/drive_api_requests.cc b/google_apis/drive/drive_api_requests.cc
index 089f2b999af2cab357f0745f94d9a652d273f54b..742629ddd6d2f24ea550ac294aafb957606bd22d 100644
--- a/google_apis/drive/drive_api_requests.cc
+++ b/google_apis/drive/drive_api_requests.cc
@@ -9,6 +9,7 @@
 #include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/sequenced_task_runner.h"
+#include "base/strings/stringprintf.h"
 #include "base/task_runner_util.h"
 #include "base/values.h"
 #include "google_apis/drive/request_sender.h"
@@ -20,6 +21,21 @@ namespace google_apis {
 namespace drive {
 namespace {
 
+// Format of one request in batch uploading request.
+const char kBatchUploadRequestFormat[] =
+    "%s %s HTTP/1.1\n"
+    "Host: www.googleapis.com\n"

[kinaba, 2015/04/13 08:25:27]

How about getting the Host domain info from UrlGenerator, rather than hand
coding it here.

[hirono, 2015/04/13 09:16:09]

Done.

+    "X-Goog-Upload-Protocol: multipart\n"
+    "Content-Type: %s\n"
+    "\n"
+    "%s";
+
+// Request header for specifying batch upload.
+const char kBatchUploadHeader[] = "X-Goog-Upload-Protocol: batch";
+
+// Content type of HTTP request.
+const char kHttpContentType[] = "application/http";
+
 // Parses the JSON value to FileResource instance and runs |callback| on the
 // UI thread once parsing is done.
 // This is customized version of ParseJsonAndRun defined above to adapt the
@@ -977,23 +993,121 @@ BatchUploadRequest::BatchUploadRequest(
     : UrlFetchRequestBase(sender),
       sender_(sender),
       url_generator_(url_generator),
+      committed_(false),
       weak_ptr_factory_(this) {
 }
 
 BatchUploadRequest::~BatchUploadRequest() {
-  for (const auto& child : child_requests_) {
-    // Request will be deleted in RequestFinished method.
-    sender_->RequestFinished(child.request);
+  for (const auto& pair : child_requests_) {

[kinaba, 2015/04/13 08:25:27]

Maybe: id_request_pair? for readability

[hirono, 2015/04/13 09:16:09]

Removed this line for turning child_requests_ to vector.

+    // Request will be deleted in |RequestFinished| method.
+    sender_->RequestFinished(pair.second.request);
   }
 }
 
-void BatchUploadRequest::AddRequest(UrlFetchRequestBase* request) {
+void BatchUploadRequest::SetBoundaryForTesting(const std::string& boundary) {
+  boundary_ = boundary;
+}
+
+void BatchUploadRequest::AddRequest(BatchableRequestBase* request) {

[kinaba, 2015/04/13 08:25:27]

I'm not familiar with batched request, but is it ok that this AddRequest does
not record the order that child request is added (because it is pushing them to
a std::map)?

[hirono, 2015/04/13 09:16:09]

The order can affect the result. I turned child_requests_ to vector to preserve
the order.

+  DCHECK(CalledOnValidThread());
   DCHECK(request);
-  child_requests_.push_back(BatchUploadChildEntry(request));
+  DCHECK(!child_requests_.count(request));
+  DCHECK(!committed_);
+  child_requests_[request] = BatchUploadChildEntry(request);
+  request->Prepare(
+      base::Bind(&BatchUploadRequest::OnChildRequestPrepared,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 request));
+}
+
+void BatchUploadRequest::OnChildRequestPrepared(
+    RequestID request_id, DriveApiErrorCode result) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(child_requests_.count(request_id));
+  if (IsSuccessfulDriveApiErrorCode(result)) {
+    child_requests_[request_id].prepared = true;
+  } else {
+    child_requests_[request_id].request->RunCallbackOnPrematureFailure(result);
+    sender_->RequestFinished(child_requests_[request_id].request);
+    child_requests_.erase(request_id);
+  }
+  MayCompletePrepare();
 }
 
 void BatchUploadRequest::Commit() {
-  NOTIMPLEMENTED();
+  DCHECK(CalledOnValidThread());
+  DCHECK(!committed_);
+  if (child_requests_.empty()) {

[kinaba, 2015/04/13 08:25:27]

What about having this case as (D)CHECK? Does the empty case make any sense?

[hirono, 2015/04/13 09:16:09]

Turned it to CHECK, and updated the comment of function.

+    sender_->RequestFinished(this);
+    return;
+  }
+  committed_ = true;
+  MayCompletePrepare();
+}
+
+void BatchUploadRequest::Prepare(const PrepareCallback& callback) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!callback.is_null());
+  prepare_callback_ = callback;
+  MayCompletePrepare();
+}
+
+void BatchUploadRequest::Cancel() {
+  for (auto& pair : child_requests_) {
+    // Request cancel should delete the request instance.
+    pair.second.request->Cancel();
+  }
+  child_requests_.clear();
+  UrlFetchRequestBase::Cancel();
+}
+
+void BatchUploadRequest::MayCompletePrepare() {
+  if (!committed_ || prepare_callback_.is_null())
+    return;
+  for (const auto& pair : child_requests_) {
+    if (!pair.second.prepared)
+      return;
+  }
+
+  // Build multipart body here.
+  std::vector<ContentTypeAndData> parts;
+  for (const auto& pair : child_requests_) {
+    std::string type;
+    std::string data;
+    const bool result = pair.second.request->GetContentData(&type, &data);
+    // Upload request must have content data.
+    DCHECK(result);
+
+    const GURL url = pair.second.request->GetURL();
+    std::string method;
+    switch (pair.second.request->GetRequestType()) {
+      case net::URLFetcher::POST:
+        method = "POST";
+        break;
+      case net::URLFetcher::PUT:
+        method = "PUT";
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+
+    parts.push_back(ContentTypeAndData());
+    parts.back().type = kHttpContentType;
+    parts.back().data = base::StringPrintf(
+        kBatchUploadRequestFormat,
+        method.c_str(), url.path().c_str(), type.c_str(), data.c_str());
+  }
+
+  GenerateMultipartBody(MULTIPART_MIXED, boundary_, parts, &upload_content_);
+  prepare_callback_.Run(HTTP_SUCCESS);
+}
+
+bool BatchUploadRequest::GetContentData(std::string* upload_content_type,
+                                        std::string* upload_content_data) {
+  upload_content_type->assign(upload_content_.type);
+  upload_content_data->assign(upload_content_.data);
+  return true;
 }
 
 base::WeakPtr<BatchUploadRequest>
@@ -1002,16 +1116,40 @@ BatchUploadRequest::GetWeakPtrAsBatchUploadRequest() {
 }
 
 GURL BatchUploadRequest::GetURL() const {
-  NOTIMPLEMENTED();
-  return GURL();
+  return url_generator_.GetBatchUploadUrl();
+}
+
+net::URLFetcher::RequestType BatchUploadRequest::GetRequestType() const {
+  return net::URLFetcher::PUT;
+}
+
+std::vector<std::string> BatchUploadRequest::GetExtraRequestHeaders() const {
+  return { kBatchUploadHeader };
 }
 
 void BatchUploadRequest::ProcessURLFetchResults(const net::URLFetcher* source) {
-  NOTIMPLEMENTED();
+  if (!IsSuccessfulDriveApiErrorCode(GetErrorCode())) {
+    RunCallbackOnPrematureFailure(GetErrorCode());
+    sender_->RequestFinished(this);
+    return;
+  }
+
+  for (auto& entry : child_requests_) {
+    // TODO(hirono): Split the mutlipart result and return the correct code and
+    // body.
+    entry.second.request->ProcessURLFetchResults(HTTP_SERVICE_UNAVAILABLE, "");
+    // Request deletes itself after processing.
+  }
+
+  child_requests_.clear();
 }
 
 void BatchUploadRequest::RunCallbackOnPrematureFailure(DriveApiErrorCode code) {
-  NOTIMPLEMENTED();
+  for (auto pair : child_requests_) {
+    pair.second.request->RunCallbackOnPrematureFailure(code);
+    sender_->RequestFinished(pair.second.request);
+  }
+  child_requests_.clear();
 }
 
 }  // namespace drive