Handle tiffs correctly

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

@@ -20,19 +20,43 @@ class ImageIngestOperations(imageBucket: String, thumbnailBucket: String, config
 
   import ImageIngestOperations.{fileKeyFromId, optimisedPngKeyFromId}
 
-  def storeOriginal(id: String, file: File, mimeType: Option[MimeType], meta: Map[String, String] = Map.empty)
-                   (implicit logMarker: LogMarker): Future[S3Object] =
-    storeImage(imageBucket, fileKeyFromId(id), file, mimeType, meta)
-
-  def storeThumbnail(id: String, file: File, mimeType: Option[MimeType])
-                    (implicit logMarker: LogMarker): Future[S3Object] =
-    storeImage(thumbnailBucket, fileKeyFromId(id), file, mimeType)
-
-  def storeOptimisedPng(id: String, file: File)
+  def store(storableImage: StorableImage)
+           (implicit logMarker: LogMarker): Future[S3Object] = storableImage match {
+    case s:StorableOriginalImage => storeOriginalImage(s)
+    case s:StorableThumbImage => storeThumbnailImage(s)
+    case s:StorableOptimisedImage => storeOptimisedImage(s)
+  }
+
+  private def storeOriginalImage(storableImage: StorableImage)
+                        (implicit logMarker: LogMarker): Future[S3Object] =
+    storeImage(imageBucket, fileKeyFromId(storableImage.id), storableImage.file, Some(storableImage.mimeType), storableImage.meta)
+
+  private def storeThumbnailImage(storableImage: StorableImage)
+                         (implicit logMarker: LogMarker): Future[S3Object] =
+    storeImage(thumbnailBucket, fileKeyFromId(storableImage.id), storableImage.file, Some(storableImage.mimeType))
+
+  private def storeOptimisedImage(storableImage: StorableImage)
                        (implicit logMarker: LogMarker): Future[S3Object] =
-    storeImage(imageBucket, optimisedPngKeyFromId(id), file, Some(Png))
-  
+    storeImage(imageBucket, optimisedPngKeyFromId(storableImage.id), storableImage.file, Some(storableImage.mimeType))
+
   def deleteOriginal(id: String): Future[Unit] = if(isVersionedS3) deleteVersionedImage(imageBucket, fileKeyFromId(id)) else deleteImage(imageBucket, fileKeyFromId(id))
   def deleteThumbnail(id: String): Future[Unit] = deleteImage(thumbnailBucket, fileKeyFromId(id))
   def deletePng(id: String): Future[Unit] = deleteImage(imageBucket, optimisedPngKeyFromId(id))
 }
+
+
+
+trait StorableImage {
+  val id: String
+  val file: File
+  val mimeType: MimeType
+  val meta: Map[String, String]
+}
+case class StorableThumbImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty) extends StorableImage
+case class StorableOriginalImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty) extends StorableImage
+case class StorableOptimisedImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty) extends StorableImage
+case class StorableBrowserViewableImage(id: String, file: File, mimeType: MimeType, meta: Map[String, String] = Map.empty, mustUpload: Boolean = false) extends StorableImage {
+  def asStorableOptimisedImage = StorableOptimisedImage(id, file, mimeType, meta)
+  def asStorableThumbImage = StorableThumbImage(id, file, mimeType, meta)
+
+}

common-lib/src/main/scala/com/gu/mediaservice/lib/imaging/ImageOperations.scala

@@ -4,6 +4,8 @@ import java.io._
 
 import org.im4java.core.IMOperation
 import com.gu.mediaservice.lib.Files._
+import com.gu.mediaservice.lib.StorableThumbImage
+import com.gu.mediaservice.lib.imaging.ImageOperations.thumbMimeType
 import com.gu.mediaservice.lib.imaging.im4jwrapper.ImageMagick.{addImage, format, runIdentifyCmd}
 import com.gu.mediaservice.lib.imaging.im4jwrapper.{ExifTool, ImageMagick}
 import com.gu.mediaservice.lib.logging.GridLogging
@@ -114,31 +116,37 @@ class ImageOperations(playPath: String) extends GridLogging {
   val thumbUnsharpRadius = 0.5d
   val thumbUnsharpSigma = 0.5d
   val thumbUnsharpAmount = 0.8d
-  def createThumbnail(sourceFile: File, sourceMimeType: Option[MimeType], width: Int, qual: Double = 100d,
-                      tempDir: File, iccColourSpace: Option[String], colourModel: Option[String]): Future[File] = {
+  def createThumbnail(sourceFile: File,
+                      sourceMimeType: Option[MimeType],
+                      width: Int,
+                      qual: Double = 100d,
+                      tempDir: File,
+                      iccColourSpace: Option[String],
+                      colourModel: Option[String]): Future[(File, MimeType)] = {
+    val cropSource  = addImage(sourceFile)
+    val thumbnailed = thumbnail(cropSource)(width)
+    val corrected   = correctColour(thumbnailed)(iccColourSpace, colourModel)
+    val converted   = applyOutputProfile(corrected, optimised = true)
+    val stripped    = stripMeta(converted)
+    val profiled    = applyOutputProfile(stripped, optimised = true)
+    val unsharpened = unsharp(profiled)(thumbUnsharpRadius, thumbUnsharpSigma, thumbUnsharpAmount)
+    val qualified   = quality(unsharpened)(qual)
+    val addOutput   = {file:File => addDestImage(qualified)(file)}
     for {
-      outputFile <- createTempFile(s"thumb-", ".jpg", tempDir)
-      cropSource  = addImage(sourceFile)
-      thumbnailed = thumbnail(cropSource)(width)
-      corrected   = correctColour(thumbnailed)(iccColourSpace, colourModel)
-      converted   = applyOutputProfile(corrected, optimised = true)
-      stripped    = stripMeta(converted)
-      profiled    = applyOutputProfile(stripped, optimised = true)
-      unsharpened = unsharp(profiled)(thumbUnsharpRadius, thumbUnsharpSigma, thumbUnsharpAmount)
-      qualified   = quality(unsharpened)(qual)
-      addOutput   = addDestImage(qualified)(outputFile)
-      _          <- runConvertCmd(addOutput, useImageMagick = sourceMimeType.contains(Tiff))
-    } yield outputFile
+      outputFile <- createTempFile(s"thumb-", thumbMimeType.fileExtension, tempDir)
+      _          <- runConvertCmd(addOutput(outputFile), useImageMagick = sourceMimeType.contains(Tiff))
+    } yield (outputFile, thumbMimeType)
   }
 
-  def transformImage(sourceFile: File, sourceMimeType: Option[MimeType], tempDir: File): Future[File] = {
+  def transformImage(sourceFile: File, sourceMimeType: Option[MimeType], tempDir: File): Future[(File, String)] = {
     for {
-      outputFile      <- createTempFile(s"transformed-", ".png", tempDir)
+      // png suffix is used by imagemagick to infer the required type
+      outputFile      <- createTempFile(s"transformed-", Png.fileExtension, tempDir)
       transformSource = addImage(sourceFile)
       addOutput       = addDestImage(transformSource)(outputFile)
       _               <- runConvertCmd(addOutput, useImageMagick = sourceMimeType.contains(Tiff))
-      _               <- checkForOutputFileChange(outputFile)
-    } yield outputFile
+      extension       <- checkForOutputFileChange(outputFile)
+    } yield (outputFile, extension)
   }
 
   // When a layered tiff is unpacked, the temp file (blah.something) is moved
@@ -147,7 +155,7 @@ class ImageOperations(playPath: String) extends GridLogging {
   // As the file has been renamed, the file object still exists, but has the wrong name
   // We will need to put it back where it is expected to be found, and clean up the other
   // files.
-  private def checkForOutputFileChange(f: File): Future[Unit] = Future {
+  private def checkForOutputFileChange(f: File): Future[String] = Future {
     val fileBits = f.getAbsolutePath.split("\\.").toList
     val mainPart = fileBits.dropRight(1).mkString(".")
     val extension = fileBits.last
@@ -160,6 +168,7 @@ class ImageOperations(playPath: String) extends GridLogging {
       // Tidy up any other files (blah-1,2,3 etc will be created for each subsequent layer)
       cleanUpLayerFiles(mainPart, extension, 1)
     }
+    fileBits.last
   }
 
   @scala.annotation.tailrec
@@ -175,6 +184,7 @@ class ImageOperations(playPath: String) extends GridLogging {
 }
 
 object ImageOperations {
+  val thumbMimeType = Jpeg
   def identifyColourModel(sourceFile: File, mimeType: MimeType)(implicit ec: ExecutionContext): Future[Option[String]] = {
     // TODO: use mimeType to lookup other properties once we support other formats
 

docs/06-objects-of-interest/06.01-tiffs.md

@@ -0,0 +1,18 @@
+# TIFF files
+
+Tiff files have special handling for importing.
+
+Like all files, we upload the original, but create an optimised and thumbnail
+png version.
+
+For TIFF files, these pngs derive from the first image file found inside the
+container format, which, in a file named `xxx.tif` will explode out as
+`xxx.png` or `xxx.jpg`.
+
+However, there is additional complexity with tiff files which contain layered
+images.  Specifically, the layer files will explode as `xxx-n.jpg` or
+`xxx-n.png` where `n` is a numeric index.
+
+Our code looks for this special case (see `ImageOperations.checkForOutputFileChange`)
+and simply moves the layer file to the expected location.
+

image-loader/app/ImageLoaderComponents.scala

@@ -3,7 +3,7 @@ import com.gu.mediaservice.lib.play.GridComponents
 import controllers.ImageLoaderController
 import lib._
 import lib.storage.ImageLoaderStore
-import model.{Uploader, Projector}
+import model.{Projector, Uploader}
 import play.api.ApplicationLoader.Context
 import router.Routes
 

image-loader/app/lib/imaging/FileMetadataReader.scala

@@ -12,9 +12,11 @@ import com.drew.metadata.jpeg.JpegDirectory
 import com.drew.metadata.png.PngDirectory
 import com.drew.metadata.xmp.XmpDirectory
 import com.drew.metadata.{Directory, Metadata}
+import com.gu.mediaservice.lib.StorableImage
 import com.gu.mediaservice.lib.imaging.im4jwrapper.ImageMagick._
 import com.gu.mediaservice.lib.metadata.ImageMetadataConverter
 import com.gu.mediaservice.model._
+import model.upload.UploadRequest
 import org.joda.time.{DateTime, DateTimeZone}
 import org.joda.time.format.ISODateTimeFormat
 import play.api.libs.json.JsValue
@@ -54,16 +56,19 @@ object FileMetadataReader {
     for {
       metadata <- readMetadata(image)
     }
-    yield getMetadataWithICPTCHeaders(metadata, imageId) // FIXME: JPEG, JFIF, Photoshop, GPS, File
+    yield getMetadataWithIPTCHeaders(metadata, imageId) // FIXME: JPEG, JFIF, Photoshop, GPS, File
 
-  def fromICPTCHeadersWithColorInfo(image: File, imageId:String, mimeType: MimeType): Future[FileMetadata] =
+  def fromIPTCHeadersWithColorInfo(image: StorableImage): Future[FileMetadata] =
+    fromIPTCHeadersWithColorInfo(image.file, image.id, image.mimeType)
+
+  def fromIPTCHeadersWithColorInfo(image: File, imageId:String, mimeType: MimeType): Future[FileMetadata] =
     for {
       metadata <- readMetadata(image)
       colourModelInformation <- getColorModelInformation(image, metadata, mimeType)
     }
-    yield getMetadataWithICPTCHeaders(metadata, imageId).copy(colourModelInformation = colourModelInformation)
+    yield getMetadataWithIPTCHeaders(metadata, imageId).copy(colourModelInformation = colourModelInformation)
 
-  private def getMetadataWithICPTCHeaders(metadata: Metadata, imageId:String): FileMetadata =
+  private def getMetadataWithIPTCHeaders(metadata: Metadata, imageId:String): FileMetadata =
     FileMetadata(
       iptc = exportDirectory(metadata, classOf[IptcDirectory]),
       exif = exportDirectory(metadata, classOf[ExifIFD0Directory]),
@@ -217,12 +222,12 @@ object FileMetadataReader {
           "paletteSize" -> Option(metaDir.getDescription(PngDirectory.TAG_PALETTE_SIZE)),
           "iccProfileName" -> Option(metaDir.getDescription(PngDirectory.TAG_ICC_PROFILE_NAME))
         ).flattenOptions
-      case _ => val metaDir = metadata.getFirstDirectoryOfType(classOf[ExifIFD0Directory])
+      case _ => val metaDir = Option(metadata.getFirstDirectoryOfType(classOf[ExifIFD0Directory]))
         Map(
           "hasAlpha" -> hasAlpha,
           "colorType" -> maybeImageType,
-          "photometricInterpretation" -> Option(metaDir.getDescription(ExifDirectoryBase.TAG_PHOTOMETRIC_INTERPRETATION)),
-          "bitsPerSample" -> Option(metaDir.getDescription(ExifDirectoryBase.TAG_BITS_PER_SAMPLE))
+          "photometricInterpretation" -> metaDir.map(_.getDescription(ExifDirectoryBase.TAG_PHOTOMETRIC_INTERPRETATION)),
+          "bitsPerSample" -> metaDir.map(_.getDescription(ExifDirectoryBase.TAG_BITS_PER_SAMPLE))
         ).flattenOptions
     }