diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/CopyResult.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/CopyResult.scala deleted file mode 100644 index d3d2757a0d..0000000000 --- a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/CopyResult.scala +++ /dev/null @@ -1,11 +0,0 @@ -package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3 - -sealed trait CopyResult extends Product with Serializable - -object CopyResult { - - final case object Success extends CopyResult - - final case object AlreadyExists extends CopyResult - -} diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/S3OperationResult.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/S3OperationResult.scala new file mode 100644 index 0000000000..39d42d43fb --- /dev/null +++ b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/S3OperationResult.scala @@ -0,0 +1,11 @@ +package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3 + +sealed trait S3OperationResult extends Product with Serializable + +object S3OperationResult { + + final case object Success extends S3OperationResult + + final case object AlreadyExists extends S3OperationResult + +} diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClient.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClient.scala index 4c93f6acee..760ff9c8bc 100644 --- a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClient.scala +++ b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClient.scala @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.cli import akka.http.scaladsl.model.ContentType import cats.effect.{IO, Resource} import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.StoragesConfig.S3StorageConfig -import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, CopyResult, HeadObject, PutObjectRequest} +import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, HeadObject, PutObjectRequest, S3OperationResult} import fs2.Stream import io.laserdisc.pure.s3.tagless.{Interpreter, S3AsyncClientOp} import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, AwsCredentialsProvider, DefaultCredentialsProvider, StaticCredentialsProvider} @@ -35,7 +35,7 @@ trait S3StorageClient { destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] + ): IO[S3OperationResult] def copyObjectMultiPart( sourceBucket: String, @@ -43,14 +43,14 @@ trait S3StorageClient { destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] + ): IO[S3OperationResult] def uploadFile( put: PutObjectRequest, fileData: Stream[IO, ByteBuffer] ): IO[Unit] - def updateContentType(bucket: String, key: String, contentType: ContentType): IO[Unit] + def updateContentType(bucket: String, key: String, contentType: ContentType): IO[S3OperationResult] def objectExists(bucket: String, key: String): IO[Boolean] def bucketExists(bucket: String): IO[Boolean] diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientDisabled.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientDisabled.scala index 0429ba8306..5c7900b7a6 100644 --- a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientDisabled.scala +++ b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientDisabled.scala @@ -2,7 +2,7 @@ package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.cli import akka.http.scaladsl.model.ContentType import cats.effect.IO -import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, CopyResult, HeadObject, PutObjectRequest} +import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, HeadObject, PutObjectRequest, S3OperationResult} import ch.epfl.bluebrain.nexus.delta.sdk.error.ServiceError.FeatureDisabled import fs2.Stream import software.amazon.awssdk.services.s3.model._ @@ -27,7 +27,7 @@ private[client] object S3StorageClientDisabled extends S3StorageClient { destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] = raiseDisabledErr + ): IO[S3OperationResult] = raiseDisabledErr override def objectExists(bucket: String, key: String): IO[Boolean] = raiseDisabledErr @@ -36,7 +36,8 @@ private[client] object S3StorageClientDisabled extends S3StorageClient { data: Stream[IO, ByteBuffer] ): IO[Unit] = raiseDisabledErr - override def updateContentType(bucket: String, key: String, contentType: ContentType): IO[Unit] = raiseDisabledErr + override def updateContentType(bucket: String, key: String, contentType: ContentType): IO[S3OperationResult] = + raiseDisabledErr override def bucketExists(bucket: String): IO[Boolean] = raiseDisabledErr @@ -46,7 +47,7 @@ private[client] object S3StorageClientDisabled extends S3StorageClient { destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] = raiseDisabledErr + ): IO[S3OperationResult] = raiseDisabledErr override def readFileMultipart(bucket: String, fileKey: String): Stream[IO, Byte] = throw disabledErr } diff --git a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientImpl.scala b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientImpl.scala index 58930547ec..6608701028 100644 --- a/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientImpl.scala +++ b/delta/plugins/storage/src/main/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientImpl.scala @@ -62,7 +62,7 @@ final private[client] class S3StorageClientImpl(client: S3AsyncClientOp[IO]) ext destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] = + ): IO[S3OperationResult] = approveCopy(destinationBucket, destinationKey, options.overwriteTarget).flatMap { approved => if (approved) { val requestBuilder = CopyObjectRequest @@ -77,8 +77,8 @@ final private[client] class S3StorageClientImpl(client: S3AsyncClientOp[IO]) ext .contentType(contentType.value) .metadataDirective(MetadataDirective.REPLACE) } - client.copyObject(requestWithOptions.build()).as(CopyResult.Success) - } else IO.pure(CopyResult.AlreadyExists) + client.copyObject(requestWithOptions.build()).as(S3OperationResult.Success) + } else IO.pure(S3OperationResult.AlreadyExists) } def copyObjectMultiPart( @@ -87,13 +87,13 @@ final private[client] class S3StorageClientImpl(client: S3AsyncClientOp[IO]) ext destinationBucket: String, destinationKey: String, options: CopyOptions - ): IO[CopyResult] = + ): IO[S3OperationResult] = approveCopy(destinationBucket, destinationKey, options.overwriteTarget).flatMap { approved => if (approved) { copyObjectMultiPart(sourceBucket, sourceKey, destinationBucket, destinationKey, options.newContentType).as( - CopyResult.Success + S3OperationResult.Success ) - } else IO.pure(CopyResult.AlreadyExists) + } else IO.pure(S3OperationResult.AlreadyExists) } private def copyObjectMultiPart( @@ -183,19 +183,21 @@ final private[client] class S3StorageClientImpl(client: S3AsyncClientOp[IO]) ext .compile .drain - override def updateContentType(bucket: String, key: String, contentType: ContentType): IO[Unit] = { - val requestBuilder = CopyObjectRequest - .builder() - .sourceBucket(bucket) - .sourceKey(key) - .destinationBucket(bucket) - .destinationKey(key) - .checksumAlgorithm(checksumAlgorithm) - .contentType(contentType.value) - .metadataDirective(MetadataDirective.REPLACE) - - client.copyObject(requestBuilder.build()).void - } + override def updateContentType(bucket: String, key: String, contentType: ContentType): IO[S3OperationResult] = + headObject(bucket, key).flatMap { + case head if head.contentType.contains(contentType) => IO.pure(S3OperationResult.AlreadyExists) + case _ => + val requestBuilder = CopyObjectRequest + .builder() + .sourceBucket(bucket) + .sourceKey(key) + .destinationBucket(bucket) + .destinationKey(key) + .checksumAlgorithm(checksumAlgorithm) + .contentType(contentType.value) + .metadataDirective(MetadataDirective.REPLACE) + client.copyObject(requestBuilder.build()).as(S3OperationResult.Success) + } override def bucketExists(bucket: String): IO[Boolean] = { listObjectsV2(bucket) diff --git a/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientSuite.scala b/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientSuite.scala index 6a90aeb9c6..d46153ee9c 100644 --- a/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientSuite.scala +++ b/delta/plugins/storage/src/test/scala/ch/epfl/bluebrain/nexus/delta/plugins/storage/storages/operations/s3/client/S3StorageClientSuite.scala @@ -3,7 +3,7 @@ package ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.cli import akka.http.scaladsl.model.ContentTypes import cats.effect.IO import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.StoragesConfig.S3StorageConfig -import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, CopyResult, LocalStackS3StorageClient, S3Helpers} +import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, LocalStackS3StorageClient, S3Helpers, S3OperationResult} import ch.epfl.bluebrain.nexus.testkit.mu.NexusSuite import io.laserdisc.pure.s3.tagless.S3AsyncClientOp import munit.AnyFixture @@ -33,7 +33,7 @@ class S3StorageClientSuite extends NexusSuite with LocalStackS3StorageClient.Fix result <- s3StorageClient.copyObject(bucket, key, bucket, newKey, options) head <- s3StorageClient.headObject(bucket, newKey) } yield { - assertEquals(result, CopyResult.Success) + assertEquals(result, S3OperationResult.Success) assertEquals(head.fileSize, contentLength) assertEquals(head.contentType, Some(expectedContentType)) } @@ -50,7 +50,7 @@ class S3StorageClientSuite extends NexusSuite with LocalStackS3StorageClient.Fix result <- s3StorageClient.copyObject(bucket, key, bucket, newKey, options) head <- s3StorageClient.headObject(bucket, newKey) } yield { - assertEquals(result, CopyResult.Success) + assertEquals(result, S3OperationResult.Success) assertEquals(head.fileSize, contentLength) assertEquals(head.contentType, Some(contentType)) } @@ -67,7 +67,7 @@ class S3StorageClientSuite extends NexusSuite with LocalStackS3StorageClient.Fix head <- s3StorageClient.headObject(bucket, existingTargetKey) } yield { val clue = "The file should not have been overwritten" - assertEquals(result, CopyResult.AlreadyExists) + assertEquals(result, S3OperationResult.AlreadyExists) assertEquals(head.fileSize, anotherContentLength, clue) assertEquals(head.contentType, Some(expectedContentType), clue) } @@ -84,7 +84,7 @@ class S3StorageClientSuite extends NexusSuite with LocalStackS3StorageClient.Fix head <- s3StorageClient.headObject(bucket, existingTargetKey) } yield { val clue = "The file should have been overwritten" - assertEquals(result, CopyResult.Success) + assertEquals(result, S3OperationResult.Success) assertEquals(head.fileSize, contentLength, clue) assertEquals(head.contentType, Some(contentType), clue) } @@ -96,13 +96,29 @@ class S3StorageClientSuite extends NexusSuite with LocalStackS3StorageClient.Fix givenAnS3Bucket { bucket => givenAFileInABucket(bucket, fileContents) { key => for { - _ <- s3StorageClient.updateContentType(bucket, key, contentType) - head <- s3StorageClient.headObject(bucket, key) + result <- s3StorageClient.updateContentType(bucket, key, contentType) + head <- s3StorageClient.headObject(bucket, key) } yield { + assertEquals(result, S3OperationResult.Success) assertEquals(head.contentType, Some(contentType)) } } } } + test("Do not update the content type of an existing object if it is already set to this value") { + val originalContentType = ContentTypes.`text/plain(UTF-8)` + givenAnS3Bucket { bucket => + givenAFileInABucket(bucket, fileContents) { key => + for { + result <- s3StorageClient.updateContentType(bucket, key, originalContentType) + head <- s3StorageClient.headObject(bucket, key) + } yield { + assertEquals(result, S3OperationResult.AlreadyExists) + assertEquals(head.contentType, Some(originalContentType)) + } + } + } + } + } diff --git a/ship/src/main/scala/ch/epfl/bluebrain/nexus/ship/files/FileCopier.scala b/ship/src/main/scala/ch/epfl/bluebrain/nexus/ship/files/FileCopier.scala index 5e50cff369..00fc327414 100644 --- a/ship/src/main/scala/ch/epfl/bluebrain/nexus/ship/files/FileCopier.scala +++ b/ship/src/main/scala/ch/epfl/bluebrain/nexus/ship/files/FileCopier.scala @@ -8,7 +8,7 @@ import ch.epfl.bluebrain.nexus.delta.kernel.RetryStrategy.logError import ch.epfl.bluebrain.nexus.delta.kernel.{Logger, RetryStrategy} import ch.epfl.bluebrain.nexus.delta.kernel.utils.UrlUtils import ch.epfl.bluebrain.nexus.delta.plugins.storage.files.model.FileAttributes -import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, CopyResult, S3LocationGenerator} +import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.{CopyOptions, S3LocationGenerator, S3OperationResult} import ch.epfl.bluebrain.nexus.delta.plugins.storage.storages.operations.s3.client.S3StorageClient import ch.epfl.bluebrain.nexus.delta.sourcing.model.ProjectRef import ch.epfl.bluebrain.nexus.ship.config.FileProcessingConfig @@ -74,8 +74,8 @@ object FileCopier { } else s3StorageClient.copyObject(importBucket, originKey, targetBucket, targetKey, copyOptions) }.flatMap { - case CopyResult.Success => IO.unit - case CopyResult.AlreadyExists => + case S3OperationResult.Success => IO.unit + case S3OperationResult.AlreadyExists => IO.whenA(forceContentType) { attributes.mediaType.traverse { mediaType => logger.info(s"Patching to content type $mediaType for file $patchedFileName") >>