Generalise S3 buckets

common-lib/src/main/scala/com/gu/mediaservice/lib/BaseStore.scala

@@ -1,7 +1,7 @@
 package com.gu.mediaservice.lib
 
 import org.apache.pekko.actor.{Cancellable, Scheduler}
-import com.gu.mediaservice.lib.aws.S3
+import com.gu.mediaservice.lib.aws.{S3, S3Bucket}
 import com.gu.mediaservice.lib.config.CommonConfig
 import com.gu.mediaservice.lib.logging.GridLogging
 import org.joda.time.DateTime
@@ -14,7 +14,7 @@ import scala.concurrent.duration._
 import scala.util.control.NonFatal
 
 
-abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonConfig)(implicit ec: ExecutionContext)
+abstract class BaseStore[TStoreKey, TStoreVal](bucket: S3Bucket, config: CommonConfig)(implicit ec: ExecutionContext)
   extends GridLogging {
 
   val s3 = new S3(config)
@@ -25,15 +25,14 @@ abstract class BaseStore[TStoreKey, TStoreVal](bucket: String, config: CommonCon
   protected def getS3Object(key: String): Option[String] = s3.getObjectAsString(bucket, key)
 
   protected def getLatestS3Stream: Option[InputStream] = {
-    val objects = s3.client
-      .listObjects(bucket).getObjectSummaries.asScala
+    val objects = s3.listObjects(bucket).getObjectSummaries.asScala
       .filterNot(_.getKey == "AMAZON_SES_SETUP_NOTIFICATION")
 
     if (objects.nonEmpty) {
       val obj = objects.maxBy(_.getLastModified)
       logger.info(s"Latest key ${obj.getKey} in bucket $bucket")
 
-      val stream = s3.client.getObject(bucket, obj.getKey).getObjectContent
+      val stream = s3.getObject(bucket, obj).getObjectContent
       Some(stream)
     } else {
       logger.error(s"Bucket $bucket is empty")

common-lib/src/main/scala/com/gu/mediaservice/lib/ImageIngestOperations.scala

@@ -1,10 +1,10 @@
 package com.gu.mediaservice.lib
 
-import com.amazonaws.services.s3.model.{DeleteObjectsRequest, MultiObjectDeleteException}
+import com.amazonaws.services.s3.model.MultiObjectDeleteException
 
 import java.io.File
 import com.gu.mediaservice.lib.config.CommonConfig
-import com.gu.mediaservice.lib.aws.S3Object
+import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object}
 import com.gu.mediaservice.lib.logging.LogMarker
 import com.gu.mediaservice.model.{Instance, MimeType, Png}
 import com.typesafe.scalalogging.StrictLogging
@@ -21,7 +21,7 @@ object ImageIngestOperations {
   private def snippetForId(id: String) = id.take(6).mkString("/") + "/" + id
 }
 
-class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config: CommonConfig, isVersionedS3: Boolean = false)
+class ImageIngestOperations(imageBucket: S3Bucket, thumbnailBucket: S3Bucket, config: CommonConfig, isVersionedS3: Boolean = false)
   extends S3ImageStorage(config) with StrictLogging {
 
   import ImageIngestOperations.{fileKeyFromId, optimisedPngKeyFromId}
@@ -44,7 +44,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config
   private def storeThumbnailImage(storableImage: StorableThumbImage)
                                  (implicit logMarker: LogMarker): Future[S3Object] = {
     val instanceSpecificKey = instanceAwareThumbnailImageKey(storableImage)
-    logger.info(s"Storing thumbnail to instance specific key: $thumbnailBucket / $instanceSpecificKey")
+    logger.info(s"Storing thumbnail to instance specific key: ${thumbnailBucket.bucket} / $instanceSpecificKey")
     storeImage(thumbnailBucket, instanceSpecificKey, storableImage.file, Some(storableImage.mimeType),
       overwrite = true)
   }
@@ -57,26 +57,45 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config
       overwrite = true)
   }
 
-  private def bulkDelete(bucket: String, keys: List[String]): Future[Map[String, Boolean]] = keys match {
+  private def bulkDelete(bucket: S3Bucket, keys: List[String]): Future[Map[String, Boolean]] = keys match {
     case Nil => Future.successful(Map.empty)
-    case _ => Future {
-      try {
-        logger.info(s"Creating S3 bulkDelete request for $bucket / keys: " + keys.mkString(","))
-        client.deleteObjects(
-          new DeleteObjectsRequest(bucket).withKeys(keys: _*)
-        )
-        keys.map { key =>
-          key -> true
-        }.toMap
-      } catch {
-        case partialFailure: MultiObjectDeleteException =>
-          logger.warn(s"Partial failure when deleting images from $bucket: ${partialFailure.getMessage} ${partialFailure.getErrors}")
-          val errorKeys = partialFailure.getErrors.asScala.map(_.getKey).toSet
+    case _ =>
+      val bulkDeleteImplemented = bucket.endpoint != "storage.googleapis.com"
+      if (bulkDeleteImplemented) {
+        Future {
+          try {
+            logger.info(s"Bulk deleting S3 objects from ${bucket.bucket}: " + keys.mkString(","))
+            deleteObjects(bucket, keys)
+            keys.map { key =>
+              key -> true
+            }.toMap
+          } catch {
+            case partialFailure: MultiObjectDeleteException =>
+              logger.warn(s"Partial failure when deleting images from $bucket: ${partialFailure.getMessage} ${partialFailure.getErrors}")
+              val errorKeys = partialFailure.getErrors.asScala.map(_.getKey).toSet
+              keys.map { key =>
+                key -> !errorKeys.contains(key)
+              }.toMap
+          }
+        }
+
+      } else {
+        Future.sequence {
           keys.map { key =>
-            key -> !errorKeys.contains(key)
-          }.toMap
+            Future {
+              logger.info(s"Deleting S3 objects from ${bucket.bucket}: " + key)
+              try {
+                deleteObject(bucket, key)
+                (key, true)
+              } catch {
+                case e: Exception =>
+                  logger.debug(s"Failure when deleting images from $bucket: $key, ${e.getMessage}")
+                  (key, false)
+              }
+            }
+          }
+        }.map(_.toMap)
       }
-    }
   }
 
   def deleteOriginal(id: String)(implicit logMarker: LogMarker, instance: Instance): Future[Unit] = if(isVersionedS3) deleteVersionedImage(imageBucket, fileKeyFromId(id)) else deleteImage(imageBucket, fileKeyFromId(id))
@@ -87,7 +106,7 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config
   def deletePNGs(ids: Set[String])(implicit instance: Instance) = bulkDelete(imageBucket, ids.map(id => optimisedPngKeyFromId(id)).toList)
 
   def doesOriginalExist(id: String)(implicit instance: Instance): Boolean =
-    client.doesObjectExist(imageBucket, fileKeyFromId(id))
+    doesObjectExist(imageBucket, fileKeyFromId(id))
 
   private def instanceAwareOriginalImageKey(storableImage: StorableOriginalImage) = {
     fileKeyFromId(storableImage.id)(storableImage.instance)
@@ -107,7 +126,7 @@ sealed trait ImageWrapper {
   val instance: Instance
 }
 sealed trait StorableImage extends ImageWrapper {
-  def toProjectedS3Object(thumbBucket: String): S3Object = S3Object(
+  def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object(
     thumbBucket,
     ImageIngestOperations.fileKeyFromId(id)(instance),
     file,
@@ -119,7 +138,7 @@ sealed trait StorableImage extends ImageWrapper {
 
 case class StorableThumbImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage
 case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, lastModified: DateTime, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage {
-  override def toProjectedS3Object(thumbBucket: String): S3Object = S3Object(
+  override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object(
     thumbBucket,
     ImageIngestOperations.fileKeyFromId(id)(instance),
     file,
@@ -129,7 +148,7 @@ case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, las
   )
 }
 case class StorableOptimisedImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, instance: Instance) extends StorableImage {
-  override def toProjectedS3Object(thumbBucket: String): S3Object = S3Object(
+  override def toProjectedS3Object(thumbBucket: S3Bucket): S3Object = S3Object(
     thumbBucket,
     ImageIngestOperations.optimisedPngKeyFromId(id)(instance),
     file,

common-lib/src/main/scala/com/gu/mediaservice/lib/ImageQuarantineOperations.scala

@@ -2,13 +2,13 @@ package com.gu.mediaservice.lib
 
 import java.io.File
 import com.gu.mediaservice.lib.config.CommonConfig
-import com.gu.mediaservice.lib.aws.S3Object
+import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object}
 import com.gu.mediaservice.lib.logging.LogMarker
 import com.gu.mediaservice.model.{Instance, MimeType}
 
 import scala.concurrent.Future
 
-class ImageQuarantineOperations(quarantineBucket: String, config: CommonConfig, isVersionedS3: Boolean = false)
+class ImageQuarantineOperations(quarantineBucket: S3Bucket, config: CommonConfig, isVersionedS3: Boolean = false)
   extends S3ImageStorage(config) {
 
   def storeQuarantineImage(id: String, file: File, mimeType: Option[MimeType], meta: Map[String, String] = Map.empty)

common-lib/src/main/scala/com/gu/mediaservice/lib/ImageStorage.scala

@@ -2,11 +2,10 @@ package com.gu.mediaservice.lib
 
 import java.util.concurrent.Executors
 import java.io.File
-
 import scala.concurrent.{ExecutionContext, Future}
 import scala.concurrent.duration._
 import scala.language.postfixOps
-import com.gu.mediaservice.lib.aws.S3Object
+import com.gu.mediaservice.lib.aws.{S3Bucket, S3Object}
 import com.gu.mediaservice.lib.logging.LogMarker
 import com.gu.mediaservice.model.MimeType
 
@@ -37,9 +36,9 @@ trait ImageStorage {
   /** Store a copy of the given file and return the URI of that copy.
     * The file can safely be deleted afterwards.
     */
-  def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType],
+  def storeImage(bucket: S3Bucket, id: String, file: File, mimeType: Option[MimeType],
                  meta: Map[String, String] = Map.empty, overwrite: Boolean)
                 (implicit logMarker: LogMarker): Future[S3Object]
 
-  def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker): Future[Unit]
+  def deleteImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker): Future[Unit]
 }

common-lib/src/main/scala/com/gu/mediaservice/lib/S3ImageStorage.scala

@@ -1,6 +1,6 @@
 package com.gu.mediaservice.lib
 
-import com.gu.mediaservice.lib.aws.S3
+import com.gu.mediaservice.lib.aws.{S3, S3Bucket}
 import com.gu.mediaservice.lib.config.CommonConfig
 import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker}
 import com.gu.mediaservice.model.MimeType
@@ -14,10 +14,10 @@ import scala.concurrent.Future
 class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage with GridLogging {
 
   private val cacheSetting = Some(cacheForever)
-  def storeImage(bucket: String, id: String, file: File, mimeType: Option[MimeType],
+  def storeImage(bucket: S3Bucket, id: String, file: File, mimeType: Option[MimeType],
                  meta: Map[String, String] = Map.empty, overwrite: Boolean)
                 (implicit logMarker: LogMarker) = {
-    logger.info(logMarker, s"bucket: $bucket, id: $id, meta: $meta")
+    logger.info(logMarker, s"storeImage to bucket: ${bucket.bucket}, id: $id, meta: $meta")
     val eventualObject = if (overwrite) {
       store(bucket, id, file, mimeType, meta, cacheSetting)
     } else {
@@ -27,21 +27,21 @@ class S3ImageStorage(config: CommonConfig) extends S3(config) with ImageStorage
     eventualObject
   }
 
-  def deleteImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future {
-    client.deleteObject(bucket, id)
+  def deleteImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future {
+    deleteObject(bucket, id)
     logger.info(logMarker, s"Deleted image $id from bucket $bucket")
   }
 
-  def deleteVersionedImage(bucket: String, id: String)(implicit logMarker: LogMarker) = Future {
-    val objectVersion = client.getObjectMetadata(bucket, id).getVersionId
-    client.deleteVersion(bucket, id, objectVersion)
+  def deleteVersionedImage(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future {
+    val objectVersion = getObjectMetadata(bucket, id).getVersionId
+    deleteVersion(bucket, id, objectVersion)
     logger.info(logMarker, s"Deleted image $id from bucket $bucket (version: $objectVersion)")
   }
 
-  def deleteFolder(bucket: String, id: String)(implicit logMarker: LogMarker) = Future {
-		val files = client.listObjects(bucket, id).getObjectSummaries.asScala
+  def deleteFolder(bucket: S3Bucket, id: String)(implicit logMarker: LogMarker) = Future {
+    val files = listObjects(bucket, id).getObjectSummaries.asScala
     logger.info(s"Found ${files.size} files to delete in folder $id")
-    files.foreach(file => client.deleteObject(bucket, file.getKey))
+    files.foreach(file => deleteObject(bucket, file.getKey))
 		logger.info(logMarker, s"Deleting images in folder $id from bucket $bucket")
 	}
 

common-lib/src/main/scala/com/gu/mediaservice/lib/auth/KeyStore.scala

@@ -1,13 +1,13 @@
 package com.gu.mediaservice.lib.auth
 
 import com.gu.mediaservice.lib.BaseStore
+import com.gu.mediaservice.lib.aws.S3Bucket
 import com.gu.mediaservice.lib.config.CommonConfig
 import com.gu.mediaservice.model.Instance
 
-import scala.jdk.CollectionConverters._
 import scala.concurrent.ExecutionContext
 
-class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionContext)
+class KeyStore(bucket: S3Bucket, config: CommonConfig)(implicit ec: ExecutionContext)
   extends BaseStore[String, ApiAccessor](bucket, config)(ec) {
 
   def lookupIdentity(key: String)(implicit instance: Instance): Option[ApiAccessor] = store.get().get(instance.id + "/" + key)
@@ -17,7 +17,6 @@ class KeyStore(bucket: String, config: CommonConfig)(implicit ec: ExecutionConte
   }
 
   private def fetchAll: Map[String, ApiAccessor] = {
-    val keys = s3.client.listObjects(bucket).getObjectSummaries.asScala.map(_.getKey)
-    keys.flatMap(k => getS3Object(k).map(k -> ApiAccessor(_))).toMap
+    s3.listObjectKeys(bucket).flatMap(k => getS3Object(k).map(k -> ApiAccessor(_))).toMap
   }
 }