Remove failIfAlreadyExists from BlobContainer.copyBlob

Author: bcully

modules/repository-s3/src/internalClusterTest/java/org/elasticsearch/repositories/s3/S3RepositoryThirdPartyTests.java

@@ -240,7 +240,7 @@ public void testCopy() {
             sourceBlobContainer.writeBlob(randomPurpose(), sourceBlobName, blobBytes, true);
 
             final var targetBlobContainer = repository.blobStore().blobContainer(repository.basePath().add("target"));
-            sourceBlobContainer.copyBlob(randomPurpose(), sourceBlobName, targetBlobContainer, targetBlobName, false);
+            sourceBlobContainer.copyBlob(randomPurpose(), sourceBlobName, targetBlobContainer, targetBlobName);
 
             return sourceBlobContainer.readBlob(randomPurpose(), sourceBlobName).readAllBytes();
         });

modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java

@@ -324,31 +324,25 @@ public void writeBlobAtomic(OperationPurpose purpose, String blobName, BytesRefe
      * Server-side copy can be done for any size object, but if the object is larger than 5 GB then
      * it must be done through a series of part copy operations rather than a single blob copy.
      * See <a href="https://docs.aws.amazon.com/AmazonS3/latest/API/API_CopyObject.html">CopyObject</a>.
+     * Note that this operation will overwrite the destination if it already exists.
      * @param purpose             The purpose of the operation
      * @param sourceBlobName      The name of the blob to copy from
      * @param destinationBlobContainer The blob container to copy the blob into
      * @param destinationBlobName      The name of the blob to copy to
-     * @param failIfAlreadyExists Whether to throw a FileAlreadyExistsException if the target blob already exists
-     *                            On S3, if true, throws UnsupportedOperationException because we don't know how
-     *                            to do this atomically.
-     * @throws IOException
+     * @throws IOException        If the operation fails on the server side
      */
     @Override
     public void copyBlob(
         OperationPurpose purpose,
         String sourceBlobName,
         BlobContainer destinationBlobContainer,
-        String destinationBlobName,
-        boolean failIfAlreadyExists
+        String destinationBlobName
     ) throws IOException {
         assert BlobContainer.assertPurposeConsistency(purpose, sourceBlobName);
         assert BlobContainer.assertPurposeConsistency(purpose, destinationBlobName);
         if (destinationBlobContainer instanceof S3BlobContainer == false) {
             throw new IllegalArgumentException("target blob container must be a S3BlobContainer");
         }
-        if (failIfAlreadyExists) {
-            throw new UnsupportedOperationException("S3 blob container does not support failIfAlreadyExists");
-        }
 
         final var s3TargetBlobContainer = (S3BlobContainer) destinationBlobContainer;
 
@@ -365,7 +359,10 @@ public void copyBlob(
         try (AmazonS3Reference clientReference = blobStore.clientReference()) {
             SocketAccess.doPrivilegedVoid(() -> { clientReference.client().copyObject(copyRequest); });
         } catch (final AmazonClientException e) {
-            throw new IOException("Unable to copy object [" + sourceBlobName + "]", e);
+            throw new IOException(
+                "Unable to copy object [" + sourceBlobName + "] to [" + destinationBlobContainer + "][" + destinationBlobName + "]",
+                e
+            );
         }
     }
 

modules/repository-s3/src/test/java/org/elasticsearch/repositories/s3/S3BlobStoreContainerTests.java

@@ -386,7 +386,7 @@ public void testCopy() throws Exception {
         final ArgumentCaptor<CopyObjectRequest> captor = ArgumentCaptor.forClass(CopyObjectRequest.class);
         when(client.copyObject(captor.capture())).thenReturn(new CopyObjectResult());
 
-        sourceBlobContainer.copyBlob(randomPurpose(), sourceBlobName, targetBlobContainer, targetBlobName, false);
+        sourceBlobContainer.copyBlob(randomPurpose(), sourceBlobName, targetBlobContainer, targetBlobName);
 
         final CopyObjectRequest request = captor.getValue();
         assertEquals(sourceBucketName, request.getSourceBucketName());
@@ -396,12 +396,6 @@ public void testCopy() throws Exception {
         assertEquals(storageClass.toString(), request.getStorageClass());
         assertEquals(cannedAccessControlList, request.getCannedAccessControlList());
 
-        // failIfAlreadyExists is not currently supported on S3
-        assertThrows(
-            UnsupportedOperationException.class,
-            () -> sourceBlobContainer.copyBlob(randomPurpose(), sourceBlobName, targetBlobContainer, targetBlobName, true)
-        );
-
         closeMockClient(blobStore);
     }
 

server/src/main/java/org/elasticsearch/common/blobstore/BlobContainer.java

@@ -182,22 +182,20 @@ default void writeBlobAtomic(OperationPurpose purpose, String blobName, BytesRef
      * If copy is unavailable then throws UnsupportedOperationException.
      * It may be unavailable either because the blob container has no copy implementation
      * or because the target blob container is not on the same store as the source.
+     * If the destination blob already exists, this operation will overwrite it.
      *
      * @param purpose             The purpose of the operation
      * @param sourceBlobName      The name of the blob to copy from
      * @param destinationBlobContainer The blob container to copy the blob into
      * @param destinationBlobName      The name of the blob to copy to
-     * @param failIfAlreadyExists Whether to throw a FileAlreadyExistsException if the target blob already exists
      * @throws NoSuchFileException If the source blob does not exist
-     * @throws FileAlreadyExistsException If failIfAlreadyExists is true and the target blob already exists
-     * @throws IOException        If the operation generates an IO error (e.g., due to local copy fallback)
+     * @throws IOException        If the operation generates an IO error
      */
     default void copyBlob(
         OperationPurpose purpose,
         String sourceBlobName,
         BlobContainer destinationBlobContainer,
-        String destinationBlobName,
-        boolean failIfAlreadyExists
+        String destinationBlobName
     ) throws IOException {
         throw new UnsupportedOperationException("this blob container does not support copy");
     }

server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobContainer.java

@@ -354,8 +354,7 @@ public void copyBlob(
         OperationPurpose purpose,
         String sourceBlobName,
         BlobContainer destinationBlobContainer,
-        String destinationBlobName,
-        boolean failIfAlreadyExists
+        String destinationBlobName
     ) throws IOException {
         if (destinationBlobContainer instanceof FsBlobContainer == false) {
             throw new IllegalArgumentException("targetBlobContainer must be a FsBlobContainer");
@@ -366,7 +365,7 @@ public void copyBlob(
         final Path tempBlobPath = targetContainer.path.resolve(tempBlob);
         Files.copy(sourceBlobPath, tempBlobPath, StandardCopyOption.REPLACE_EXISTING);
         try {
-            targetContainer.moveBlobAtomic(purpose, tempBlob, destinationBlobName, failIfAlreadyExists);
+            targetContainer.moveBlobAtomic(purpose, tempBlob, destinationBlobName, false);
         } catch (IOException ex) {
             try {
                 targetContainer.deleteBlobsIgnoringIfNotExists(purpose, Iterators.single(tempBlob));

server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobContainerTests.java

@@ -389,11 +389,7 @@ public void testCopy() throws IOException {
         final var targetBlobName = randomAlphaOfLengthBetween(1, 20).toLowerCase(Locale.ROOT);
         final var contents = new BytesArray(randomByteArrayOfLength(randomIntBetween(1, 512)));
         sourceContainer.writeBlob(randomPurpose(), sourceBlobName, contents, true);
-        sourceContainer.copyBlob(randomPurpose(), sourceBlobName, targetContainer, targetBlobName, true);
-        assertThrows(
-            FileAlreadyExistsException.class,
-            () -> sourceContainer.copyBlob(randomPurpose(), sourceBlobName, targetContainer, targetBlobName, true)
-        );
+        sourceContainer.copyBlob(randomPurpose(), sourceBlobName, targetContainer, targetBlobName);
 
         var sourceContents = Streams.readFully(sourceContainer.readBlob(randomPurpose(), sourceBlobName));
         var targetContents = Streams.readFully(targetContainer.readBlob(randomPurpose(), targetBlobName));