Implement GetUploadStatusOperation on GData WAPI.

chrome/browser/google_apis/gdata_wapi_operations_unittest.cc

Index: chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
diff --git a/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc b/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
index 26397d9d35e9337fefcb1fa5c76aa04e66845132..1e80b4f25cb601e5531b56b26fd3bce19d1d32f2 100644
--- a/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
+++ b/chrome/browser/google_apis/gdata_wapi_operations_unittest.cc
@@ -225,6 +225,7 @@ class GDataWapiOperationsTest : public testing::Test {
       scoped_ptr<test_server::HttpResponse> http_response(
           new test_server::HttpResponse);
 
+      // Check an ETag.
       std::map<std::string, std::string>::const_iterator found =
           request.headers.find("If-Match");
       if (found != request.headers.end() &&
@@ -234,6 +235,16 @@ class GDataWapiOperationsTest : public testing::Test {
         return http_response.Pass();
       }
 
+      // Check if the X-Upload-Content-Length is present. If yes, store the
+      // length of the file.
+      found = request.headers.find("X-Upload-Content-Length");
+      if (found == request.headers.end() ||
+          !base::StringToInt64(found->second, &content_length_)) {
+        return scoped_ptr<test_server::HttpResponse>();
+      }
+      start_position_ = 0;
+      end_position_ = 0;
+
       http_response->set_code(test_server::SUCCESS);
       GURL upload_url;
       // POST is used for a new file, and PUT is used for an existing file.
@@ -279,29 +290,28 @@ class GDataWapiOperationsTest : public testing::Test {
           request.headers.find("Content-Range");
       if (iter == request.headers.end())
         return scoped_ptr<test_server::HttpResponse>();
-      int64 start_position = 0;
-      int64 end_position = 0;
       int64 length = 0;
       if (!ParseContentRangeHeader(iter->second,
-                                   &start_position,
-                                   &end_position,
+                                   &start_position_,
+                                   &end_position_,
                                    &length)) {
         return scoped_ptr<test_server::HttpResponse>();
       }
-
-      // Add Range header to the response, based on the values of
-      // Content-Range header in the request.
-      response->AddCustomHeader(
-          "Range",
-          "bytes=" +
-          base::Int64ToString(start_position) + "-" +
-          base::Int64ToString(end_position));
-
-      // Change the code to RESUME_INCOMPLETE if upload is not complete.
-      if (end_position + 1 < length)
-        response->set_code(test_server::RESUME_INCOMPLETE);
+      EXPECT_EQ(length, content_length_);
     }
 
+    // Add Range header to the response, based on the values of
+    // Content-Range header in the request.
+    response->AddCustomHeader(
+        "Range",
+        "bytes=" +
+        base::Int64ToString(start_position_) + "-" +
+        base::Int64ToString(end_position_));
+
+    // Change the code to RESUME_INCOMPLETE if upload is not complete.
+    if (end_position_ + 1 < content_length_)
+      response->set_code(test_server::RESUME_INCOMPLETE);
+
     return response.Pass();
   }
 
@@ -315,6 +325,12 @@ class GDataWapiOperationsTest : public testing::Test {
   scoped_ptr<GDataWapiUrlGenerator> url_generator_;
   scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
 
+  // These fields are used to keep the current upload state during a
+  // test case.
+  int64 start_position_;
+  int64 end_position_;
+  int64 content_length_;

[hashimoto, 2013/02/07 06:56:13]

Do we need to keep the values of start_position and end_position?
nit: Please initialize these variables in the ctor.

[hidehiko, 2013/02/07 07:44:07]

Yes, it is necessary to keep them.
This class is not only text fixture but also the http test server. The request
from ResumeUploadOperation updates these fields, and it is necessary to include
these "previously updated" values to the response for GetUploadOperation to
emulate the server's behavior. Added a bit more comment.


Initialized in SetUp().

[hashimoto, 2013/02/07 07:56:10]

I see, thanks.

+
   // The incoming HTTP request is saved so tests can verify the request
   // parameters like HTTP method (ex. some operations should use DELETE
   // instead of GET).
@@ -866,7 +882,8 @@ TEST_F(GDataWapiOperationsTest, UploadNewFile) {
 
 // This test exercises InitiateUploadOperation and ResumeUploadOperation for
 // a scenario of uploading a new *large* file, which requires multiple requests
-// of ResumeUploadOperation.
+// of ResumeUploadOperation. GetUploadOperation is also testes in this test

[hashimoto, 2013/02/07 06:56:13]

nit: s/testes/tested/?

[hidehiko, 2013/02/07 07:44:07]

Good catch. Fixed.

+// case.
 TEST_F(GDataWapiOperationsTest, UploadNewLargeFile) {
   const size_t kMaxNumBytes = 10;
   // This is big enough to cause multiple requests of ResumeUploadOperation
@@ -980,13 +997,49 @@ TEST_F(GDataWapiOperationsTest, UploadNewLargeFile) {
       // complete.
       EXPECT_EQ(-1, response.start_position_received);
       EXPECT_EQ(-1, response.end_position_received);
-    } else {
-      EXPECT_EQ(HTTP_RESUME_INCOMPLETE, response.code);
-      EXPECT_EQ(static_cast<int64>(start_position),
-                response.start_position_received);
-      EXPECT_EQ(static_cast<int64>(end_position),
-                response.end_position_received);
+      // The upload process is completed, so exit from the loop.
+      break;
     }
+
+    EXPECT_EQ(HTTP_RESUME_INCOMPLETE, response.code);
+    EXPECT_EQ(static_cast<int64>(start_position),
+              response.start_position_received);
+    EXPECT_EQ(static_cast<int64>(end_position),
+              response.end_position_received);
+
+    // Check the response by GetUploadStatusOperation.
+    GetUploadStatusOperation* get_upload_status_operation =
+        new GetUploadStatusOperation(
+            &operation_registry_,
+            request_context_getter_.get(),
+            base::Bind(&CopyResultFromUploadRangeCallbackAndQuit,
+                       &response,
+                       &new_entry),
+            UPLOAD_NEW_FILE,
+            FilePath::FromUTF8Unsafe("drive/newfile.txt"),
+            upload_url,
+            kUploadContent.size());

[hashimoto, 2013/02/07 06:56:13]

Could you also test the content_length==1 case?

[hidehiko, 2013/02/07 07:44:07]

Sorry, I don't understand your request now...
The content_length must be shared with the one for InitiateUploadParams. Here,
it is kUploadContent.size().

[hashimoto, 2013/02/07 07:56:10]

Sorry, not 1, -1.
In gdata_wapi_operations.h, you added a comment "|content_length| is the whole
data size to be uploaded, or -1 if it is unknown."
And it seems there is no test for the -1 case.

[kinaba, 2013/02/07 08:01:10]

The case has gone away when the streamed uploading (via DownloadObserver) is
removed.
We can (and should) erase mentions to the -1 case.

[hidehiko, 2013/02/07 08:37:56]

So, I've just removed -1 support from ResumeUploadOperation and
GetUploadStatusOperation.

+    get_upload_status_operation->Start(
+        kTestGDataAuthToken, kTestUserAgent,
+        base::Bind(&test_util::DoNothingForReAuthenticateCallback));
+    MessageLoop::current()->Run();
+
+    // METHOD_PUT should be used to upload data.
+    EXPECT_EQ(test_server::METHOD_PUT, http_request_.method);
+    // Request should go to the upload URL.
+    EXPECT_EQ(upload_url.path(), http_request_.relative_url);
+    // Content-Range header should be added.
+    EXPECT_EQ("bytes */" + base::Int64ToString(kUploadContent.size()),
+              http_request_.headers["Content-Range"]);
+    EXPECT_TRUE(http_request_.has_content);
+    EXPECT_TRUE(http_request_.content.empty());
+
+    // Check the response.
+    EXPECT_EQ(HTTP_RESUME_INCOMPLETE, response.code);
+    EXPECT_EQ(static_cast<int64>(start_position),
+              response.start_position_received);
+    EXPECT_EQ(static_cast<int64>(end_position),
+              response.end_position_received);
   }
 
   EXPECT_EQ(kUploadContent.size(), num_bytes_consumed);