Get true dimensions for image metadata

article/app/model/structuredData/Image.scala

@@ -1,18 +1,21 @@
 package model.structuredData
 
-import model.{ImageAsset, ImageElement}
-import play.api.libs.json.{JsString, JsValue, Json}
+import model.ImageElement
+import play.api.libs.json.{JsValue, Json}
 import views.support.Item700
 
 object Image {
 
   def apply(picture: ImageElement): JsValue = {
-    val asset: Option[ImageAsset] = Item700.bestFor(picture.images)
+    val url = Item700.bestSrcFor(picture.images).getOrElse("")
+    val height = Item700.trueHeightFor(picture.images).getOrElse(0)
+    val width = Item700.trueWidthFor(picture.images).getOrElse(0)
+
     Json.obj(
       "@type" -> "ImageObject",
-      "url" -> JsString(Item700.bestSrcFor(picture.images).getOrElse("")),
-      "height" -> asset.fold(0)(_.height),
-      "width" -> asset.fold(0)(_.width)
+      "url" -> url,
+      "height" -> height,
+      "width" -> width
     )
   }
 

common/app/views/fragments/imageFigure.scala.html

@@ -38,11 +38,9 @@
             <meta itemprop="representativeOfPage" content="true">
         }
 
-        @defining(Item700.bestFor(picture)) { asset =>
-            <meta itemprop="url" content="@Item700.bestSrcFor(picture).getOrElse("")">
-            <meta itemprop="width" content="@asset.fold(0)(_.width)">
-            <meta itemprop="height" content="@asset.fold(0)(_.height)">
-        }
+        <meta itemprop="url" content="@Item700.bestSrcFor(picture).getOrElse("")">
+        <meta itemprop="width" content="@Item700.trueWidthFor(picture).getOrElse(0)">
+        <meta itemprop="height" content="@Item700.trueHeightFor(picture).getOrElse(0)">
 
         @defining(
             if (isMain) {

common/app/views/support/Profile.scala

@@ -43,18 +43,58 @@ sealed trait ElementProfile {
   def altTextFor(image: ImageMedia): Option[String] =
     bestFor(image).flatMap(_.altText)
 
+  def trueWidthFor(image: ImageMedia): Option[Int] = {
+    val maybeAssetWidth: Option[Int] = bestFor(image).map(_.width)
+    val maybeTrueProfileWidth: Option[Int] = width.map { pixels =>
+      Math.round(pixels * dpr.getOrElse(1d)).toInt
+    }
+
+    // If the asset and profile have valid dimensions then take the minimum (due to fit=max)
+    val resizedDimension = for {
+      assetWidth <- maybeAssetWidth
+      profileWidth <- maybeTrueProfileWidth
+    } yield Math.min(assetWidth, profileWidth)
+
+    if(isCropped && ImageServerSwitch.isSwitchedOn) maybeTrueProfileWidth
+    else if(ImageServerSwitch.isSwitchedOn) resizedDimension.orElse(maybeAssetWidth).orElse(maybeTrueProfileWidth)
+    else maybeAssetWidth
+  }
+
+  def trueHeightFor(image: ImageMedia): Option[Int] = {
+    val best = bestFor(image)
+    val maybeTrueProfileHeight: Option[Int] = height.map { pixels =>
+      Math.round(pixels * dpr.getOrElse(1d)).toInt
+    }
+
+    if(isCropped) {
+      maybeTrueProfileHeight
+    } else {
+      // If we are fit=max then the aspect ratio is maintained, so use the true width
+      for {
+        height <- best.map(_.height)
+        width <- best.map(_.width)
+        trueWidth <- trueWidthFor(image)
+        if height != 0
+      } yield  (trueWidth * height) / width
+    }
+  }
+
+  val dpr: Option[Double] = if (hidpi) {
+    if (isPng) {
+      Some(1.3)
+    } else {
+      Some(2)
+    }
+  } else None
+
+  lazy val isCropped = fitParam.endsWith("=crop")
+
   // NOTE - if you modify this in any way there is a decent chance that you decache all our images :(
   val qualityparam = if (hidpi) {"q=20"} else {"q=55"}
   val autoParam = if (autoFormat) "auto=format" else ""
   val sharpParam = "usm=12"
   val fitParam = "fit=max"
-  val dprParam = if (hidpi) {
-    if (isPng) {
-      "dpr=1.3"
-    } else {
-      "dpr=2"
-    }
-  } else {""}
+  val dprParam = dpr.map("dpr=" + _.toString).getOrElse("")
   val heightParam = height.map(pixels => s"h=$pixels").getOrElse("")
   val widthParam = width.map(pixels => s"w=$pixels").getOrElse("")
 

common/test/views/support/ProfileTest.scala

@@ -0,0 +1,56 @@
+package views.support
+
+import com.gu.contentapi.client.model.v1.AssetType
+import conf.switches.Switches._
+import model.{ImageAsset, ImageMedia}
+import org.scalatest.{BeforeAndAfter, FlatSpec, Matchers}
+
+class ProfileTest extends FlatSpec with Matchers with BeforeAndAfter {
+
+  before {
+    ImageServerSwitch.switchOn()
+  }
+
+  after {
+    ImageServerSwitch.switchOn()
+  }
+
+  object HidpiProfileSmall extends Profile(width = Some(100), height = Some(60), hidpi = true)
+
+  val imageAsset: ImageAsset = ImageAsset(
+    fields = Map("width" -> "460", "height" -> "276"),
+    mediaType = AssetType.Image.name,
+    mimeType = Some("image/jpeg"),
+    url = Some("https://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg")
+  )
+  val image: ImageMedia = ImageMedia.apply(Seq(imageAsset))
+
+  "A hidpi profile with small width" should "return double the dimensions of the profile for the true dimensions" in {
+    HidpiProfileSmall.trueWidthFor(image) should be(Some(200))
+    HidpiProfileSmall.trueHeightFor(image) should be(Some(120))
+  }
+
+  object HidpiProfileLarge extends Profile(width = Some(1000), height = Some(600), hidpi = true)
+
+  "A hidpi profile with large width" should "return the dimensions of the original asset" in {
+    HidpiProfileLarge.trueWidthFor(image) should be(Some(460))
+    HidpiProfileLarge.trueHeightFor(image) should be(Some(276))
+  }
+
+  "A non hidpi profile with small width" should "return the dimensions of the profile" in {
+    Item120.trueWidthFor(image) should be(Some(120))
+    Item120.trueHeightFor(image) should be(Some(72))
+  }
+
+  "A non hidpi profile with large width" should "return the dimensions of the original asset" in {
+    Item1200.trueWidthFor(image) should be(Some(460))
+    Item1200.trueHeightFor(image) should be(Some(276))
+  }
+
+
+  "A non hidpi profile with small width and image resizing off" should "return the dimensions of the original asset" in {
+    ImageServerSwitch.switchOff()
+    Item120.trueWidthFor(image) should be(Some(460))
+    Item120.trueHeightFor(image) should be(Some(276))
+  }
+}