Merge pull request #802 from NDLANO/chore/raw-controller-scrimage

Author: amatho

image-api/package.mill

@@ -16,10 +16,6 @@ object `package` extends BaseModule, DockerComponent {
       SharedDependencies.scalaUri,
       SharedDependencies.awsS3,
       SharedDependencies.jsoup,
-      mvn"org.imgscalr:imgscalr-lib:4.2",
-      // These are not strictly needed, for most cases, but offers better handling of loading images with encoding issues
-      mvn"com.twelvemonkeys.imageio:imageio-core:3.12.0",
-      mvn"com.twelvemonkeys.imageio:imageio-jpeg:3.12.0",
       mvn"com.sksamuel.scrimage:scrimage-core:4.3.5",
       mvn"com.sksamuel.scrimage:scrimage-webp:4.3.5",
       mvn"commons-io:commons-io:2.19.0",

image-api/src/main/scala/no/ndla/imageapi/controller/RawController.scala

@@ -10,11 +10,11 @@ package no.ndla.imageapi.controller
 
 import cats.implicits.catsSyntaxEitherId
 import no.ndla.common.errors.{ValidationException, ValidationMessage}
-import no.ndla.imageapi.model.domain.ImageStream
-import no.ndla.imageapi.service.{ImageConverter, ImageStorageService, PercentPoint, PixelPoint, ReadService}
+import no.ndla.imageapi.model.domain.{ImageStream, ProcessableImageStream, UnprocessableImageStream}
+import no.ndla.imageapi.service.*
 import no.ndla.network.clients.MyNDLAApiClient
-import no.ndla.network.tapir.{AllErrors, ErrorHandling, DynamicHeaders, ErrorHelpers, TapirController}
 import no.ndla.network.tapir.TapirUtil.errorOutputsFor
+import no.ndla.network.tapir.*
 import sttp.tapir.*
 import sttp.tapir.server.ServerEndpoint
 
@@ -29,8 +29,8 @@ class RawController(using
     readService: ReadService,
     myNDLAApiClient: MyNDLAApiClient,
 ) extends TapirController {
-  import errorHelpers.*
   import errorHandling.*
+  import errorHelpers.*
   override val serviceName: String         = "raw"
   override val prefix: EndpointInput[Unit] = "image-api" / serviceName
   override val enableSwagger: Boolean      = true
@@ -39,7 +39,7 @@ class RawController(using
 
   private def toImageResponse(image: ImageStream): Either[AllErrors, (DynamicHeaders, InputStream)] = {
     val headers = DynamicHeaders.fromValue("Content-Type", image.contentType)
-    Right(headers -> image.stream)
+    Right(headers -> image.toStream)
   }
 
   def getImageFile: ServerEndpoint[Any, Eff] = endpoint
@@ -69,9 +69,7 @@ class RawController(using
     .out(inputStreamBody)
     .serverLogicPure { case (imageId, imageParams) =>
       readService.getImageFileName(imageId, imageParams.language) match {
-        case Success(Some(fileName)) =>
-          val x = getRawImage(fileName, imageParams)
-          x match {
+        case Success(Some(fileName)) => getRawImage(fileName, imageParams) match {
             case Failure(ex)  => returnLeftError(ex)
             case Success(img) => toImageResponse(img)
           }
@@ -88,18 +86,19 @@ class RawController(using
         resize
       }
     }
-    val nonResizableMimeTypes = List("image/gif", "image/svg", "image/svg+xml")
     imageStorage.get(imageName) match {
-      case Success(img) if nonResizableMimeTypes.contains(img.contentType.toLowerCase) => Success(img)
-      case Success(img)                                                                => crop(img, imageParams)
+      case Success(img: UnprocessableImageStream) => Success(img)
+      case Success(img: ProcessableImageStream)   => crop(img, imageParams)
           .flatMap(stream => dynamicCropOrResize(stream, imageParams))
           .recoverWith {
             case ex: ValidationException => Failure(ex)
             case ex                      =>
               logger.error(s"Could not crop or resize image '$imageName', got exception: '${ex.getMessage}'", ex)
               Success(img)
           }
-      case Failure(e) => Failure(e)
+      case Failure(ex) =>
+        logger.error(s"Failed to get image '$imageName' from S3", ex)
+        Failure(ex)
     }
   }
 
@@ -113,7 +112,7 @@ class RawController(using
     }
   }
 
-  private def crop(image: ImageStream, imageParams: ImageParams): Try[ImageStream] = {
+  private def crop(image: ProcessableImageStream, imageParams: ImageParams): Try[ProcessableImageStream] = {
     val unit = imageParams.cropUnit.getOrElse("percent")
     unit match {
       case "percent" =>
@@ -145,7 +144,7 @@ class RawController(using
       .isDefined || imageParams.ratio.isDefined)
   }
 
-  private def dynamicCrop(image: ImageStream, imageParams: ImageParams): Try[ImageStream] = {
+  private def dynamicCrop(image: ProcessableImageStream, imageParams: ImageParams): Try[ProcessableImageStream] = {
 
     (imageParams.focalX, imageParams.focalY, imageParams.width, imageParams.height) match {
       case (Some(fx), Some(fy), w, h) =>
@@ -154,7 +153,7 @@ class RawController(using
     }
   }
 
-  private def resize(image: ImageStream, imageParams: ImageParams): Try[ImageStream] = {
+  private def resize(image: ProcessableImageStream, imageParams: ImageParams): Try[ProcessableImageStream] = {
     (imageParams.width, imageParams.height) match {
       case (Some(width), Some(height)) => imageConverter.resize(image, width.toInt, height.toInt)
       case (Some(width), _)            => imageConverter.resizeWidth(image, width.toInt)

image-api/src/main/scala/no/ndla/imageapi/model/NDLAErrors.scala

@@ -23,7 +23,8 @@ case class ImageDeleteException(message: String, exs: Seq[Throwable])         ex
 case class ImageVariantsUploadException(message: String, exs: Seq[Throwable]) extends MultipleExceptions(message, exs)
 case class ImageConversionException(message: String)                          extends RuntimeException(message)
 case class ImageCopyException(message: String)                                extends RuntimeException(message)
-case class ImageInvalidFormat(message: String)                                extends RuntimeException(message)
+case class ImageUnprocessableFormatException(contentType: String)
+    extends RuntimeException(s"Image of '$contentType' Content-Type did not have a processable binary format")
 
 object ImageErrorHelpers {
   def fileTooBigError(using props: Props): String =

image-api/src/main/scala/no/ndla/imageapi/model/domain/ImageStream.scala

@@ -8,13 +8,44 @@
 
 package no.ndla.imageapi.model.domain
 
-import java.awt.image.BufferedImage
-import java.io.InputStream
+import com.sksamuel.scrimage.ImmutableImage
+import com.sksamuel.scrimage.nio.{JpegWriter, PngWriter}
+import com.sksamuel.scrimage.webp.WebpWriter
 
-trait ImageStream {
-  def contentType: String
-  def stream: InputStream
+import java.io.{ByteArrayInputStream, InputStream}
+import scala.util.Try
+
+sealed trait ImageStream {
+  def toStream: InputStream
   def fileName: String
-  def format: String = fileName.substring(fileName.lastIndexOf(".") + 1)
-  lazy val sourceImage: BufferedImage
+  def contentType: String
+}
+
+final case class ProcessableImageStream(
+    image: ImmutableImage,
+    override val fileName: String,
+    format: ProcessableImageFormat,
+) extends ImageStream {
+  override def toStream: InputStream = {
+    val writer = format match {
+      case ProcessableImageFormat.Jpeg => JpegWriter.Default
+      case ProcessableImageFormat.Png  => PngWriter.MaxCompression
+      case ProcessableImageFormat.Webp => WebpWriter.DEFAULT
+    }
+    val bytes = image.bytes(writer)
+    new ByteArrayInputStream(bytes)
+  }
+
+  override val contentType: String = format.toContentType
+
+  def transform(f: ImmutableImage => ImmutableImage): Try[ProcessableImageStream] =
+    Try(f(image)).map(transformed => copy(image = transformed))
+}
+
+final case class UnprocessableImageStream(
+    imageBytes: Array[Byte],
+    override val fileName: String,
+    override val contentType: String,
+) extends ImageStream {
+  override def toStream: InputStream = new ByteArrayInputStream(imageBytes)
 }

image-api/src/main/scala/no/ndla/imageapi/model/domain/ProcessableImageFormat.scala

@@ -11,7 +11,13 @@ package no.ndla.imageapi.model.domain
 import com.sksamuel.scrimage.format.Format
 import enumeratum.{Enum, EnumEntry}
 
-sealed trait ProcessableImageFormat extends EnumEntry
+sealed trait ProcessableImageFormat extends EnumEntry {
+  def toContentType: String = this match {
+    case ProcessableImageFormat.Jpeg => "image/jpeg"
+    case ProcessableImageFormat.Png  => "image/png"
+    case ProcessableImageFormat.Webp => "image/webp"
+  }
+}
 
 object ProcessableImageFormat extends Enum[ProcessableImageFormat] {
   case object Jpeg extends ProcessableImageFormat