Fix fixture bug when writing files that are a multiple of the chunk size

Author: nicktindall

modules/repository-gcs/src/internalClusterTest/java/org/elasticsearch/repositories/gcs/GoogleCloudStorageBlobStoreRepositoryTests.java

@@ -30,6 +30,7 @@
 import org.elasticsearch.common.blobstore.BlobPath;
 import org.elasticsearch.common.blobstore.BlobStore;
 import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.collect.Iterators;
 import org.elasticsearch.common.io.Streams;
 import org.elasticsearch.common.regex.Regex;
@@ -59,6 +60,7 @@
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static org.elasticsearch.common.io.Streams.readFully;
 import static org.elasticsearch.repositories.blobstore.BlobStoreTestUtil.randomPurpose;
 import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.CREDENTIALS_FILE_SETTING;
 import static org.elasticsearch.repositories.gcs.GoogleCloudStorageClientSettings.ENDPOINT_SETTING;
@@ -206,6 +208,21 @@ public void testWriteReadLarge() throws IOException {
         }
     }
 
+    public void testWriteFileMultipleOfChunkSize() throws IOException {
+        final int uploadSize = randomIntBetween(2, 4) * GoogleCloudStorageBlobStore.SDK_DEFAULT_CHUNK_SIZE;
+        try (BlobStore store = newBlobStore()) {
+            final BlobContainer container = store.blobContainer(BlobPath.EMPTY);
+            final String key = randomIdentifier();
+            byte[] initialValue = randomByteArrayOfLength(uploadSize);
+            container.writeBlob(randomPurpose(), key, new BytesArray(initialValue), true);
+
+            BytesReference reference = readFully(container.readBlob(randomPurpose(), key));
+            assertEquals(new BytesArray(initialValue), reference);
+
+            container.deleteBlobsIgnoringIfNotExists(randomPurpose(), Iterators.single(key));
+        }
+    }
+
     public static class TestGoogleCloudStoragePlugin extends GoogleCloudStoragePlugin {
 
         public TestGoogleCloudStoragePlugin(Settings settings) {

test/fixtures/gcs-fixture/src/main/java/fixture/gcs/MockGcsBlobStore.java

@@ -114,16 +114,14 @@ UpdateResponse updateResumableUpload(String uploadId, HttpHeaderParser.ContentRa
                 throw failAndThrow("Attempted to update a non-existent resumable: " + uid);
             }
 
-            if (contentRange.hasRange() == false) {
-                // Content-Range: */... is a status check https://cloud.google.com/storage/docs/performing-resumable-uploads#status-check
+            ResumableUpload valueToReturn = existing;
+
+            // Handle the request, a range indicates a chunk of data was submitted
+            if (contentRange.hasRange()) {
                 if (existing.completed) {
-                    updateResponse.set(new UpdateResponse(RestStatus.OK.getStatus(), calculateRangeHeader(blobs.get(existing.path))));
-                } else {
-                    final HttpHeaderParser.Range range = calculateRangeHeader(existing);
-                    updateResponse.set(new UpdateResponse(RESUME_INCOMPLETE, range));
+                    throw failAndThrow("Attempted to write more to a completed resumable upload");
                 }
-                return existing;
-            } else {
+
                 if (contentRange.start() > contentRange.end()) {
                     throw failAndThrow("Invalid content range " + contentRange);
                 }
@@ -143,16 +141,20 @@ UpdateResponse updateResumableUpload(String uploadId, HttpHeaderParser.ContentRa
                     existing.contents,
                     requestBody.slice(offset, requestBody.length())
                 );
-                // We just received the last chunk, update the blob and remove the resumable upload from the map
-                if (contentRange.hasSize() && updatedContent.length() == contentRange.size()) {
-                    updateBlob(existing.path(), existing.ifGenerationMatch, updatedContent);
-                    updateResponse.set(new UpdateResponse(RestStatus.OK.getStatus(), null));
-                    return existing.update(BytesArray.EMPTY, true);
-                }
-                final ResumableUpload updated = existing.update(updatedContent, false);
-                updateResponse.set(new UpdateResponse(RESUME_INCOMPLETE, calculateRangeHeader(updated)));
-                return updated;
+                valueToReturn = existing.update(updatedContent, false);
+            }
+
+            // Next we determine the response
+            if (valueToReturn.completed) {
+                updateResponse.set(new UpdateResponse(RestStatus.OK.getStatus(), calculateRangeHeader(valueToReturn)));
+            } else if (contentRange.hasSize() && contentRange.size() == valueToReturn.contents.length()) {
+                valueToReturn = existing.update(valueToReturn.contents, true);
+                updateBlob(valueToReturn.path(), valueToReturn.ifGenerationMatch(), valueToReturn.contents);
+                updateResponse.set(new UpdateResponse(RestStatus.OK.getStatus(), calculateRangeHeader(valueToReturn)));
+            } else {
+                updateResponse.set(new UpdateResponse(RESUME_INCOMPLETE, calculateRangeHeader(valueToReturn)));
             }
+            return valueToReturn;
         });
         assert updateResponse.get() != null : "Should always produce an update response";
         return updateResponse.get();