Sort multipart uploads for the same key by date and choose latest. Ad…

…d test. Former-commit-id: 66cb8fe3b2aae2dd8b89bc1890a17a122fc6693a
iterate-ch · Jun 30, 2015 · 119abe2 · 119abe2
1 parent 52577ce
commit 119abe2
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 6 deletions.
diff --git a/source/ch/cyberduck/core/s3/S3MultipartService.java b/source/ch/cyberduck/core/s3/S3MultipartService.java
@@ -30,6 +30,9 @@
 import org.jets3t.service.model.MultipartUpload;
 
 import java.text.MessageFormat;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.EnumSet;
 import java.util.List;
 
@@ -76,7 +79,15 @@ public MultipartUpload find(final Path file) throws BackgroundException {
                 }
                 throw failure;
             }
-            for(MultipartUpload upload : chunk.getUploads()) {
+            final List<MultipartUpload> uploads = Arrays.asList(chunk.getUploads());
+            // Sort with newest upload first in list
+            Collections.sort(uploads, new Comparator<MultipartUpload>() {
+                @Override
+                public int compare(final MultipartUpload o1, final MultipartUpload o2) {
+                    return -o1.getInitiatedDate().compareTo(o2.getInitiatedDate());
+                }
+            });
+            for(MultipartUpload upload : uploads) {
                 if(log.isInfoEnabled()) {
                     log.info(String.format("Found multipart upload %s for %s", upload.getUploadId(), file));
                 }

diff --git a/source/ch/cyberduck/core/s3/S3MultipartUploadService.java b/source/ch/cyberduck/core/s3/S3MultipartUploadService.java
@@ -111,6 +111,8 @@ public StorageObject upload(final Path file, final Local local, final BandwidthT
             if(status.isAppend()) {
                 multipart = multipartService.find(file);
             }
+            final List<MultipartPart> completed = new ArrayList<MultipartPart>();
+            // Not found or new upload
             if(null == multipart) {
                 if(log.isInfoEnabled()) {
                     log.info("No pending multipart upload found");
@@ -124,9 +126,11 @@ public StorageObject upload(final Path file, final Local local, final BandwidthT
                             multipart.getObjectKey(), multipart.getUploadId()));
                 }
             }
-            final List<MultipartPart> completed = new ArrayList<MultipartPart>();
-            if(status.isAppend()) {
-                completed.addAll(multipartService.list(multipart));
+            else {
+                if(status.isAppend()) {
+                    // Add already completed parts
+                    completed.addAll(multipartService.list(multipart));
+                }
             }
             try {
                 final List<Future<MultipartPart>> parts = new ArrayList<Future<MultipartPart>>();

diff --git a/test/ch/cyberduck/core/s3/S3MultipartServiceTest.java b/test/ch/cyberduck/core/s3/S3MultipartServiceTest.java
@@ -28,20 +28,22 @@
 import ch.cyberduck.core.DisabledTranscriptListener;
 import ch.cyberduck.core.Host;
 import ch.cyberduck.core.Path;
+import ch.cyberduck.core.transfer.TransferStatus;
 
 import org.jets3t.service.model.MultipartUpload;
+import org.jets3t.service.model.S3Object;
 import org.junit.Ignore;
 import org.junit.Test;
 
 import java.util.EnumSet;
 import java.util.UUID;
 
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;
 
 public class S3MultipartServiceTest extends AbstractTestCase {
 
     @Test
-    public void testFind() throws Exception {
+    public void testFindNotFound() throws Exception {
         final S3Session session = new S3Session(
                 new Host(new S3Protocol(), new S3Protocol().getDefaultHostname(),
                         new Credentials(
@@ -55,6 +57,33 @@ public void testFind() throws Exception {
         assertNull(part);
     }
 
+    @Test
+    public void testFind() throws Exception {
+        final S3Session session = new S3Session(
+                new Host(new S3Protocol(), new S3Protocol().getDefaultHostname(),
+                        new Credentials(
+                                properties.getProperty("s3.key"), properties.getProperty("s3.secret")
+                        )));
+        session.open(new DisabledHostKeyCallback(), new DisabledTranscriptListener());
+        session.login(new DisabledPasswordStore(), new DisabledLoginCallback(), new DisabledCancelCallback());
+        session.open(new DisabledHostKeyCallback(), new DisabledTranscriptListener());
+        session.login(new DisabledPasswordStore(), new DisabledLoginCallback(), new DisabledCancelCallback());
+        final Path container = new Path("test.cyberduck.ch", EnumSet.of(Path.Type.directory, Path.Type.volume));
+        final Path file = new Path(container, UUID.randomUUID().toString(), EnumSet.of(Path.Type.file));
+        final S3Object object = new S3WriteFeature(session).getDetails(file.getName(), new TransferStatus());
+        final MultipartUpload first = session.getClient().multipartStartUpload(container.getName(), object);
+        assertNotNull(first);
+        // Make sure timestamp is later.
+        Thread.sleep(2000L);
+        final MultipartUpload second = session.getClient().multipartStartUpload(container.getName(), object);
+        assertNotNull(second);
+        final S3MultipartService service = new S3MultipartService(session);
+        final MultipartUpload multipart = service.find(file);
+        assertEquals(second.getUploadId(), multipart.getUploadId());
+        assertNotNull(multipart);
+        service.delete(first);
+        service.delete(second);
+    }
 
     @Test
     public void testFindKeyWithSpace() throws Exception {