From 6d7fb5b12c837f587d2b4483d39359436f69b61d Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Wed, 27 May 2026 17:34:28 +0100 Subject: [PATCH 1/7] wip --- common/app/agents/DeeplyReadAgent.scala | 59 ++++++++----------------- common/app/common/configuration.scala | 4 ++ 2 files changed, 22 insertions(+), 41 deletions(-) diff --git a/common/app/agents/DeeplyReadAgent.scala b/common/app/agents/DeeplyReadAgent.scala index 8cd908104e8e..482d0340783f 100644 --- a/common/app/agents/DeeplyReadAgent.scala +++ b/common/app/agents/DeeplyReadAgent.scala @@ -3,14 +3,21 @@ package agents import com.gu.contentapi.client.model.v1.{Content, ElementType} import com.gu.contentapi.client.utils.CapiModelEnrichment.RenderingFormat import common._ +import conf.Configuration import contentapi.ContentApiClient import layout.DiscussionSettings import model.ContentFormat -import services.{FaciaContentConvert, OphanApi} +import services.{FaciaContentConvert, OphanApi, S3} import scala.concurrent.{ExecutionContext, Future} -import scala.util.control.NonFatal import model.dotcomrendering.Trail +import play.api.libs.json.Json + + +object DeeplyReadS3Agent extends S3 { + override lazy val bucket = Configuration.cache.bucket; + lazy val stage: String = Configuration.environment.stage.toUpperCase +} class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) extends GuLogging { @@ -29,45 +36,15 @@ class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) ex */ Future .sequence(Edition.allEditions.map { edition => - ophanApi - .getDeeplyRead(edition) - .flatMap { ophanDeeplyReadItems => - log.debug(s"Fetched ${ophanDeeplyReadItems.size} Deeply Read items for ${edition.displayName}") - val constructedTrail: Seq[Future[Option[Trail]]] = ophanDeeplyReadItems.map { ophanItem => - log.debug(s"CAPI lookup for Ophan deeply read item: ${ophanItem.toString}") - val path = removeStartingSlash(ophanItem.path) - log.debug(s"CAPI Lookup for path: $path") - val capiRequest = contentApiClient - .item(path) - .showTags("all") - .showFields("all") - .showReferences("none") - .showAtoms("none") - - contentApiClient - .getResponse(capiRequest) - .map { res => - res.content.flatMap { capiData => - log.debug(s"Retrieved CAPI data for Deeply Read item: ${path}") - deeplyReadUrlToTrail(capiData) - } - } - .recover { case NonFatal(e) => - log.error(s"Error retrieving CAPI data for Deeply Read item: ${path}. ${e.getMessage}") - None - } - } - Future - .sequence(constructedTrail) - .map { maybeTrails => - (edition, maybeTrails.flatten.take(10)) - } - - } - .recover { e => - log.error(s"Failed to fetch Deeply Read items for ${edition.displayName}. ${e.getMessage()}") - (edition, Seq.empty) - } + // TODO make s3.get async + DeeplyReadS3Agent.get(s"${DeeplyReadS3Agent.stage}/deeply-read/${edition.id.toLowerCase()}.json") match { + case Some(jsonTrail) => + Json.parse(jsonTrail).asOpt[Trail] + } +// .recover { e => +// log.error(s"Failed to fetch Deeply Read items for ${edition.displayName}. ${e.getMessage()}") +// (edition, Seq.empty) +// } }) .map(trailsList => { val map = trailsList.toMap diff --git a/common/app/common/configuration.scala b/common/app/common/configuration.scala index 83de4f3a02d7..1ae42a61265a 100644 --- a/common/app/common/configuration.scala +++ b/common/app/common/configuration.scala @@ -705,6 +705,10 @@ class GuardianConfiguration extends GuLogging { lazy val host = configuration.getStringProperty("newsletterApi.host") lazy val origin = configuration.getStringProperty("newsletterApi.origin") } + + object cache { + lazy val bucket = configuration.getMandatoryStringProperty("cache.bucket") + } } object ManifestData { From 53fb91d3d6c70184c898be14b82fcbb940955594 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki <15894063+marjisound@users.noreply.github.com> Date: Fri, 5 Jun 2026 16:01:04 +0100 Subject: [PATCH 2/7] Add S3Async to get objects asynchronously Co-authored-by: Ravi <7014230+arelra@users.noreply.github.com> --- common/app/services/S3Async.scala | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 common/app/services/S3Async.scala diff --git a/common/app/services/S3Async.scala b/common/app/services/S3Async.scala new file mode 100644 index 000000000000..56b993d7f4f8 --- /dev/null +++ b/common/app/services/S3Async.scala @@ -0,0 +1,62 @@ +package services + +import com.gu.etagcaching.aws.s3.ObjectId +import common.GuLogging +import conf.Configuration +import play.api.libs.json.{JsError, JsSuccess, Json, Reads} +import services.S3.logS3ExceptionWithDevHint +import software.amazon.awssdk.core.async.AsyncResponseTransformer +import software.amazon.awssdk.services.s3.model.{GetObjectRequest, GetObjectResponse, NoSuchKeyException, S3Exception} +import utils.AWSv2 + +import scala.concurrent.{ExecutionContext, Future} +import scala.jdk.FutureConverters._ +import scala.io.Codec + +trait S3Async extends GuLogging { + + lazy val bucket = Configuration.aws.frontendStoreBucket + + lazy private val client = AWSv2.S3Async + + def handleS3Errors[T](key: String)(future: Future[T])(implicit ec: ExecutionContext): Future[T] = { + val objectId = ObjectId(bucket, key) + future.recoverWith { + case e: NoSuchKeyException => + log.warn(s"not found at ${objectId.s3Uri}") + Future.failed(e) + case e: S3Exception => + logS3ExceptionWithDevHint(objectId, e) + Future.failed(e) + } + } + + private def getResponse( + key: String, + )(implicit codec: Codec, ec: ExecutionContext): Future[(GetObjectResponse, String)] = { + val request = GetObjectRequest.builder().bucket(bucket).key(key).build() + val responseFutureJava = client.getObject(request, AsyncResponseTransformer.toBytes[GetObjectResponse]()) + + responseFutureJava.asScala.map { responseBytes => + val objectResponse = responseBytes.response() + log.debug(s"S3 got ${objectResponse.contentLength} bytes from $key") + val content = new String(responseBytes.asByteArray(), codec.charSet) + (objectResponse, content) + } + } + + def getObjectAsJson[T: Reads](key: String)(implicit ec: ExecutionContext): Future[T] = { + val futureResponse = getResponse(key)(Codec.UTF8, ec).map(_._2).flatMap { jsonString => + val parsedJson = Json.parse(jsonString) + + parsedJson.validate[T] match { + case JsSuccess(parsedObject, _) => + Future.successful(parsedObject) + case JsError(errors) => + Future.failed(new RuntimeException(s"Failed to parse JSON for key $key. Errors: $errors")) + } + } + + handleS3Errors(key)(futureResponse) + } +} From fcb0712279e470beaace32e75d6af1fcf4ac77f6 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Fri, 5 Jun 2026 18:07:09 +0100 Subject: [PATCH 3/7] Create S3Async trait Read Trails from S3 Replace DeeplyReadAgent query of Ophan and CAPI with read S3 Co-authored-by: Marjan K <15894063+marjisound@users.noreply.github.com> --- common/app/agents/DeeplyReadAgent.scala | 42 ++++++------- common/app/model/dotcomrendering/Trail.scala | 62 +++++++------------- common/app/services/S3Async.scala | 4 +- 3 files changed, 40 insertions(+), 68 deletions(-) diff --git a/common/app/agents/DeeplyReadAgent.scala b/common/app/agents/DeeplyReadAgent.scala index 482d0340783f..0638cdbda758 100644 --- a/common/app/agents/DeeplyReadAgent.scala +++ b/common/app/agents/DeeplyReadAgent.scala @@ -7,14 +7,14 @@ import conf.Configuration import contentapi.ContentApiClient import layout.DiscussionSettings import model.ContentFormat -import services.{FaciaContentConvert, OphanApi, S3} +import services.{FaciaContentConvert, OphanApi, S3, S3Async} import scala.concurrent.{ExecutionContext, Future} import model.dotcomrendering.Trail import play.api.libs.json.Json -object DeeplyReadS3Agent extends S3 { +object DeeplyReadS3Agent extends S3Async { override lazy val bucket = Configuration.cache.bucket; lazy val stage: String = Configuration.environment.stage.toUpperCase } @@ -36,29 +36,23 @@ class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) ex */ Future .sequence(Edition.allEditions.map { edition => - // TODO make s3.get async - DeeplyReadS3Agent.get(s"${DeeplyReadS3Agent.stage}/deeply-read/${edition.id.toLowerCase()}.json") match { - case Some(jsonTrail) => - Json.parse(jsonTrail).asOpt[Trail] - } -// .recover { e => -// log.error(s"Failed to fetch Deeply Read items for ${edition.displayName}. ${e.getMessage()}") -// (edition, Seq.empty) -// } - }) - .map(trailsList => { - val map = trailsList.toMap - for { - (edition, list) <- map - } yield log.debug(s"Deeply Read in ${edition.displayName}, ${list.size} items: ${list.map(_.url).toString()}") - - val mapWithTenItems = map.filter { case (_, list) => list.size == 10 } - log.debug( - s"Updating the following ${mapWithTenItems.size} editions: ${mapWithTenItems.keys.map(_.id).toList.sorted.toString()}", - ) - - deeplyReadItems.alter(deeplyReadItems.get() ++ mapWithTenItems) + val futureTrails = DeeplyReadS3Agent.getObjectAsJson[Seq[Trail]](s"${DeeplyReadS3Agent.stage}/deeply-read/${edition.id.toLowerCase()}.json") + futureTrails.map(trailsList => { + val list = trailsList.take(10) + + val map = Map(edition -> list) + log.debug(s"Deeply Read in ${edition.displayName}, ${list.size} items: ${list.map(_.url).toString()}") + + log.debug( + s"Updating the following ${list.size} editions: ${map.keys.map(_.id).toList.sorted.toString()}", + ) + + log.warn(s"Not updating ${edition.displayName} as it has only ${list.size} items") + +// deeplyReadItems.alter(deeplyReadItems.get() ++ map) + map }) + }) } def correctPillar(pillar: String): String = if (pillar == "arts") "culture" else pillar diff --git a/common/app/model/dotcomrendering/Trail.scala b/common/app/model/dotcomrendering/Trail.scala index 184c1c6ed4d1..ee70004359a9 100644 --- a/common/app/model/dotcomrendering/Trail.scala +++ b/common/app/model/dotcomrendering/Trail.scala @@ -1,14 +1,13 @@ package model.dotcomrendering import com.github.nscala_time.time.Imports.DateTimeZone -import com.gu.commercial.branding.{Branding, BrandingType, Dimensions, Logo => CommercialLogo} +import com.gu.commercial.branding.{Branding, BrandingType, Dimensions, Foundation, PaidContent, Sponsored, Logo => CommercialLogo} import common.{Edition, LinkTo} import implicits.FaciaContentFrontendHelpers.FaciaContentFrontendHelper import layout.DiscussionSettings -import model.dotcomrendering.DotcomRenderingUtils.withoutNull import model.{Article, ContentFormat, ImageMedia, Pillar} import model.pressed.PressedContent -import play.api.libs.json.{Json, OWrites, Writes} +import play.api.libs.json.{JsNull, JsObject, JsResult, JsValue, Json, OFormat} import play.api.mvc.RequestHeader import views.support.{ImageProfile, ImgSrc, Item300, Item460, RemoveOuterParaHtml} @@ -40,50 +39,31 @@ case class Trail( object Trail { - implicit val brandingTypeWrites: Writes[BrandingType] = new Writes[BrandingType] { - def writes(bt: BrandingType) = { - Json.obj( - "name" -> bt.name, - ) - } + implicit val brandingTypeFormat: OFormat[BrandingType] = new OFormat[BrandingType] { + def reads(json: JsValue): JsResult[BrandingType] = + (json \ "name").validate[String].map { + case PaidContent.name => PaidContent + case Foundation.name => Foundation + case _ => Sponsored + } + def writes(bt: BrandingType): JsObject = Json.obj("name" -> bt.name) } - implicit val dimensionsWrites: OWrites[Dimensions] = Json.writes[Dimensions] + implicit val dimensionsFormat: OFormat[Dimensions] = Json.format[Dimensions] - implicit val logoWrites: OWrites[CommercialLogo] = Json.writes[CommercialLogo] + implicit val logoFormat: OFormat[CommercialLogo] = Json.format[CommercialLogo] - implicit val brandingWrites: OWrites[Branding] = Json.writes[Branding] + implicit val brandingFormat: OFormat[Branding] = Json.format[Branding] - implicit val discussionWrites: OWrites[DiscussionSettings] = Json.writes[DiscussionSettings] + implicit val discussionSettingsFormat: OFormat[DiscussionSettings] = Json.format[DiscussionSettings] - implicit val OnwardItemWrites: Writes[Trail] = Writes { trail => - val jsObject = Json.obj( - "url" -> trail.url, - "linkText" -> trail.linkText, - "showByline" -> trail.showByline, - "byline" -> trail.byline, - "masterImage" -> trail.masterImage, - "image" -> trail.image, - "carouselImages" -> trail.carouselImages, - "ageWarning" -> trail.ageWarning, - "isLiveBlog" -> trail.isLiveBlog, - "pillar" -> trail.pillar, - "designType" -> trail.designType, - "format" -> trail.format, - "webPublicationDate" -> trail.webPublicationDate, - "headline" -> trail.headline, - "mediaType" -> trail.mediaType, - "shortUrl" -> trail.shortUrl, - "kickerText" -> trail.kickerText, - "starRating" -> trail.starRating, - "avatarUrl" -> trail.avatarUrl, - "branding" -> trail.branding, - "discussion" -> trail.discussion, - "trailText" -> trail.trailText, - "galleryCount" -> trail.galleryCount, - ) - - withoutNull(jsObject) + implicit val trailFormat: OFormat[Trail] = { + val fmt = Json.format[Trail] + new OFormat[Trail] { + override def reads(json: JsValue): JsResult[Trail] = fmt.reads(json) + override def writes(trail: Trail): JsObject = + JsObject(fmt.writes(trail).fields.filterNot(_._2 == JsNull)) + } } // We ideally want this to be replaced by something else in the near future. Probably diff --git a/common/app/services/S3Async.scala b/common/app/services/S3Async.scala index 56b993d7f4f8..10e794b20868 100644 --- a/common/app/services/S3Async.scala +++ b/common/app/services/S3Async.scala @@ -15,7 +15,7 @@ import scala.io.Codec trait S3Async extends GuLogging { - lazy val bucket = Configuration.aws.frontendStoreBucket + lazy val bucket: String = Configuration.aws.frontendStoreBucket lazy private val client = AWSv2.S3Async @@ -48,7 +48,6 @@ trait S3Async extends GuLogging { def getObjectAsJson[T: Reads](key: String)(implicit ec: ExecutionContext): Future[T] = { val futureResponse = getResponse(key)(Codec.UTF8, ec).map(_._2).flatMap { jsonString => val parsedJson = Json.parse(jsonString) - parsedJson.validate[T] match { case JsSuccess(parsedObject, _) => Future.successful(parsedObject) @@ -56,7 +55,6 @@ trait S3Async extends GuLogging { Future.failed(new RuntimeException(s"Failed to parse JSON for key $key. Errors: $errors")) } } - handleS3Errors(key)(futureResponse) } } From f64ee4c5f0c0390a47aa27f2afb31f684faf06ef Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki <15894063+marjisound@users.noreply.github.com> Date: Fri, 3 Jul 2026 12:22:11 +0100 Subject: [PATCH 4/7] Update deeply read agent to use S3 trail data --- common/app/agents/DeeplyReadAgent.scala | 99 +++++--------------- common/app/model/dotcomrendering/Trail.scala | 10 +- 2 files changed, 32 insertions(+), 77 deletions(-) diff --git a/common/app/agents/DeeplyReadAgent.scala b/common/app/agents/DeeplyReadAgent.scala index 0638cdbda758..7f304d916184 100644 --- a/common/app/agents/DeeplyReadAgent.scala +++ b/common/app/agents/DeeplyReadAgent.scala @@ -1,99 +1,46 @@ package agents -import com.gu.contentapi.client.model.v1.{Content, ElementType} -import com.gu.contentapi.client.utils.CapiModelEnrichment.RenderingFormat import common._ import conf.Configuration -import contentapi.ContentApiClient -import layout.DiscussionSettings -import model.ContentFormat -import services.{FaciaContentConvert, OphanApi, S3, S3Async} - -import scala.concurrent.{ExecutionContext, Future} import model.dotcomrendering.Trail -import play.api.libs.json.Json +import services.S3Async +import scala.concurrent.{ExecutionContext, Future} object DeeplyReadS3Agent extends S3Async { override lazy val bucket = Configuration.cache.bucket; lazy val stage: String = Configuration.environment.stage.toUpperCase } -class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) extends GuLogging { +class DeeplyReadAgent extends GuLogging { private val deeplyReadItems = Box[Map[Edition, Seq[Trail]]](Map.empty) - def removeStartingSlash(path: String): String = { - if (path.startsWith("/")) path.stripPrefix("/") else path - } - def refresh()(implicit ec: ExecutionContext): Future[Unit] = { log.debug(s"Deeply Read Agent refresh()") - /* - We query Ophan for the deeply read URLs and use them to queryCapi - then use this information to create a sequence of trails that we cache - using a Box structure. - */ Future .sequence(Edition.allEditions.map { edition => - val futureTrails = DeeplyReadS3Agent.getObjectAsJson[Seq[Trail]](s"${DeeplyReadS3Agent.stage}/deeply-read/${edition.id.toLowerCase()}.json") - futureTrails.map(trailsList => { - val list = trailsList.take(10) - - val map = Map(edition -> list) - log.debug(s"Deeply Read in ${edition.displayName}, ${list.size} items: ${list.map(_.url).toString()}") - - log.debug( - s"Updating the following ${list.size} editions: ${map.keys.map(_.id).toList.sorted.toString()}", - ) - - log.warn(s"Not updating ${edition.displayName} as it has only ${list.size} items") - -// deeplyReadItems.alter(deeplyReadItems.get() ++ map) - map + DeeplyReadS3Agent + .getObjectAsJson[Seq[Trail]]( + s"${DeeplyReadS3Agent.stage}/deeply-read/${edition.id.toLowerCase()}.json", + ) + .map(trailsList => { + edition -> trailsList.take(10) + }) + }) + .map(trailsList => { + val map = trailsList.toMap + for { + (edition, list) <- map + } yield log.debug(s"Deeply Read in ${edition.displayName}, ${list.size} items: ${list.map(_.url).toString()}") + + val mapWithTenItems = map.filter { case (_, list) => list.size == 10 } + log.debug( + s"Updating the following ${mapWithTenItems.size} editions: ${mapWithTenItems.keys.map(_.id).toList.sorted.toString()}", + ) + + deeplyReadItems.alter(deeplyReadItems.get() ++ mapWithTenItems) }) - }) - } - - def correctPillar(pillar: String): String = if (pillar == "arts") "culture" else pillar - - def deeplyReadUrlToTrail(content: Content): Option[Trail] = { - - val contentFormat: ContentFormat = ContentFormat(content.design, content.theme, content.display) - - for { - webPublicationDate <- content.webPublicationDate - fields <- content.fields - linkText <- fields.trailText - pillar <- content.pillarName - headline <- fields.headline - shortUrl <- fields.shortUrl - } yield Trail( - url = content.webUrl, - linkText = linkText, - showByline = false, - byline = fields.byline, - masterImage = None, - image = fields.thumbnail, - carouselImages = Map.empty, - ageWarning = None, - isLiveBlog = fields.liveBloggingNow.getOrElse(false), - pillar = correctPillar(pillar.toLowerCase), - designType = content.`type`.toString, - format = contentFormat, - webPublicationDate = webPublicationDate.toString(), - headline = headline, - mediaType = None, - shortUrl = shortUrl, - kickerText = None, - starRating = None, - avatarUrl = None, - branding = None, - discussion = DiscussionSettings.fromTrail(FaciaContentConvert.contentToFaciaContent(content)), - trailText = content.fields.flatMap(_.trailText), - galleryCount = - content.elements.map(_.count(el => el.`type` == ElementType.Image && el.relation == "gallery")).filter(_ > 0), - ) } def getTrails(edition: Edition)(implicit ec: ExecutionContext): Seq[Trail] = { diff --git a/common/app/model/dotcomrendering/Trail.scala b/common/app/model/dotcomrendering/Trail.scala index ee70004359a9..afb18aadcfb6 100644 --- a/common/app/model/dotcomrendering/Trail.scala +++ b/common/app/model/dotcomrendering/Trail.scala @@ -1,7 +1,15 @@ package model.dotcomrendering import com.github.nscala_time.time.Imports.DateTimeZone -import com.gu.commercial.branding.{Branding, BrandingType, Dimensions, Foundation, PaidContent, Sponsored, Logo => CommercialLogo} +import com.gu.commercial.branding.{ + Branding, + BrandingType, + Dimensions, + Foundation, + PaidContent, + Sponsored, + Logo => CommercialLogo, +} import common.{Edition, LinkTo} import implicits.FaciaContentFrontendHelpers.FaciaContentFrontendHelper import layout.DiscussionSettings From 59bc9a54062bbea77695550582e0dea17e132976 Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki <15894063+marjisound@users.noreply.github.com> Date: Fri, 3 Jul 2026 13:03:19 +0100 Subject: [PATCH 5/7] Update trail format to include the fields --- common/app/model/dotcomrendering/Trail.scala | 99 ++++++++++++++++++-- common/test/agents/DeeplyReadAgentTest.scala | 5 +- 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/common/app/model/dotcomrendering/Trail.scala b/common/app/model/dotcomrendering/Trail.scala index afb18aadcfb6..ea2db67325b5 100644 --- a/common/app/model/dotcomrendering/Trail.scala +++ b/common/app/model/dotcomrendering/Trail.scala @@ -15,7 +15,7 @@ import implicits.FaciaContentFrontendHelpers.FaciaContentFrontendHelper import layout.DiscussionSettings import model.{Article, ContentFormat, ImageMedia, Pillar} import model.pressed.PressedContent -import play.api.libs.json.{JsNull, JsObject, JsResult, JsValue, Json, OFormat} +import play.api.libs.json.{JsNull, JsObject, JsResult, JsValue, Json, OFormat, Reads, Writes} import play.api.mvc.RequestHeader import views.support.{ImageProfile, ImgSrc, Item300, Item460, RemoveOuterParaHtml} @@ -47,6 +47,13 @@ case class Trail( object Trail { + implicit val carouselImagesReads: Reads[Map[String, Option[String]]] = + Reads[Map[String, Option[String]]] { json => + json.validate[JsObject].map { + _.fields.map { case (k, v) => k -> v.asOpt[String] }.toMap + } + } + implicit val brandingTypeFormat: OFormat[BrandingType] = new OFormat[BrandingType] { def reads(json: JsValue): JsResult[BrandingType] = (json \ "name").validate[String].map { @@ -65,12 +72,90 @@ object Trail { implicit val discussionSettingsFormat: OFormat[DiscussionSettings] = Json.format[DiscussionSettings] - implicit val trailFormat: OFormat[Trail] = { - val fmt = Json.format[Trail] - new OFormat[Trail] { - override def reads(json: JsValue): JsResult[Trail] = fmt.reads(json) - override def writes(trail: Trail): JsObject = - JsObject(fmt.writes(trail).fields.filterNot(_._2 == JsNull)) + implicit val contentFormatFormat: OFormat[ContentFormat] = new OFormat[ContentFormat] { + def reads(json: JsValue): JsResult[ContentFormat] = ContentFormat.contentFormatReads.reads(json) + def writes(cf: ContentFormat): JsObject = ContentFormat.contentFormatWrites.writes(cf).as[JsObject] + } + + implicit val trailFormat: OFormat[Trail] = new OFormat[Trail] { + override def reads(json: JsValue): JsResult[Trail] = + for { + url <- (json \ "url").validate[String] + linkText <- (json \ "linkText").validate[String] + showByline <- (json \ "showByline").validate[Boolean] + byline <- (json \ "byline").validateOpt[String] + masterImage <- (json \ "masterImage").validateOpt[String] + image <- (json \ "image").validateOpt[String] + carouselImages <- (json \ "carouselImages").validate[Map[String, Option[String]]] + ageWarning <- (json \ "ageWarning").validateOpt[String] + isLiveBlog <- (json \ "isLiveBlog").validate[Boolean] + pillar <- (json \ "pillar").validate[String] + designType <- (json \ "designType").validate[String] + format <- (json \ "format").validate[ContentFormat] + webPublicationDate <- (json \ "webPublicationDate").validate[String] + headline <- (json \ "headline").validate[String] + mediaType <- (json \ "mediaType").validateOpt[String] + shortUrl <- (json \ "shortUrl").validate[String] + kickerText <- (json \ "kickerText").validateOpt[String] + starRating <- (json \ "starRating").validateOpt[Int] + avatarUrl <- (json \ "avatarUrl").validateOpt[String] + branding <- (json \ "branding").validateOpt[Branding] + discussion <- (json \ "discussion").validate[DiscussionSettings] + trailText <- (json \ "trailText").validateOpt[String] + galleryCount <- (json \ "galleryCount").validateOpt[Int] + } yield Trail( + url, + linkText, + showByline, + byline, + masterImage, + image, + carouselImages, + ageWarning, + isLiveBlog, + pillar, + designType, + format, + webPublicationDate, + headline, + mediaType, + shortUrl, + kickerText, + starRating, + avatarUrl, + branding, + discussion, + trailText, + galleryCount, + ) + + override def writes(trail: Trail): JsObject = { + val obj = Json.obj( + "url" -> trail.url, + "linkText" -> trail.linkText, + "showByline" -> trail.showByline, + "byline" -> trail.byline, + "masterImage" -> trail.masterImage, + "image" -> trail.image, + "carouselImages" -> trail.carouselImages, + "ageWarning" -> trail.ageWarning, + "isLiveBlog" -> trail.isLiveBlog, + "pillar" -> trail.pillar, + "designType" -> trail.designType, + "format" -> trail.format, + "webPublicationDate" -> trail.webPublicationDate, + "headline" -> trail.headline, + "mediaType" -> trail.mediaType, + "shortUrl" -> trail.shortUrl, + "kickerText" -> trail.kickerText, + "starRating" -> trail.starRating, + "avatarUrl" -> trail.avatarUrl, + "branding" -> trail.branding, + "discussion" -> trail.discussion, + "trailText" -> trail.trailText, + "galleryCount" -> trail.galleryCount, + ) + JsObject(obj.fields.filterNot(_._2 == JsNull)) } } diff --git a/common/test/agents/DeeplyReadAgentTest.scala b/common/test/agents/DeeplyReadAgentTest.scala index 048ee7b95dfe..a3e143efd2e9 100644 --- a/common/test/agents/DeeplyReadAgentTest.scala +++ b/common/test/agents/DeeplyReadAgentTest.scala @@ -25,10 +25,7 @@ import test.{ "DeeplyReadAgent" should "initialise with trails being an empty Seq" in { val ophanApi = new OphanApi(wsClient) val contentApiClient = testContentApiClient - val agent = new DeeplyReadAgent( - contentApiClient = contentApiClient, - ophanApi = ophanApi, - ) + val agent = new DeeplyReadAgent Edition.allEditions.map(edition => { agent.getTrails(edition) shouldBe Seq.empty }) From de54ed76e9046125768ab55a1b92187df4ef768f Mon Sep 17 00:00:00 2001 From: Marjan Kalanaki <15894063+marjisound@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:14:32 +0100 Subject: [PATCH 6/7] Update tests using DeeplyReadAgen Co-authored-by: Ravi <7014230+arelra@users.noreply.github.com> --- common/conf/env/DEVINFRA.properties | 1 + facia/test/FaciaControllerTest.scala | 2 +- facia/test/FaciaMetaDataTest.scala | 2 +- onward/test/MostPopularControllerTest.scala | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/common/conf/env/DEVINFRA.properties b/common/conf/env/DEVINFRA.properties index f2353bc7f4e5..035a43ab2343 100644 --- a/common/conf/env/DEVINFRA.properties +++ b/common/conf/env/DEVINFRA.properties @@ -9,6 +9,7 @@ content.api.host=https://content.guardianapis.com pa.api.key=none aws.bucket=none +cache.bucket=none ophan.api.host=http://api.ophan.co.uk/api ophan.api.key=none diff --git a/facia/test/FaciaControllerTest.scala b/facia/test/FaciaControllerTest.scala index 2a0cfac8a46c..65810b3647e1 100644 --- a/facia/test/FaciaControllerTest.scala +++ b/facia/test/FaciaControllerTest.scala @@ -47,7 +47,7 @@ import scala.concurrent.{Await, Future} play.api.test.Helpers.stubControllerComponents(), wsClient, new MostViewedAgent(testContentApiClient, new OphanApi(wsClient)), - new DeeplyReadAgent(testContentApiClient, new OphanApi(wsClient)), + new DeeplyReadAgent, assets = assets, ) val articleUrl = "/environment/2012/feb/22/capitalise-low-carbon-future" diff --git a/facia/test/FaciaMetaDataTest.scala b/facia/test/FaciaMetaDataTest.scala index c8c175a69126..4752fdf8de46 100644 --- a/facia/test/FaciaMetaDataTest.scala +++ b/facia/test/FaciaMetaDataTest.scala @@ -49,7 +49,7 @@ import scala.concurrent.duration._ play.api.test.Helpers.stubControllerComponents(), wsClient, new MostViewedAgent(testContentApiClient, new OphanApi(wsClient)), - new DeeplyReadAgent(testContentApiClient, new OphanApi(wsClient)), + new DeeplyReadAgent, assets = assets, ) val frontPath = "music" diff --git a/onward/test/MostPopularControllerTest.scala b/onward/test/MostPopularControllerTest.scala index 6c914b0c3e01..0064e8096ac0 100644 --- a/onward/test/MostPopularControllerTest.scala +++ b/onward/test/MostPopularControllerTest.scala @@ -27,7 +27,7 @@ import agents.DeeplyReadAgent testContentApiClient, new GeoMostPopularAgent(testContentApiClient, ophanApi), new MostPopularAgent(testContentApiClient), - new DeeplyReadAgent(testContentApiClient, ophanApi), + new DeeplyReadAgent, play.api.test.Helpers.stubControllerComponents(), ) From 9e0db5210309d6a71d0cf030d1d7e9b9828390d1 Mon Sep 17 00:00:00 2001 From: Ravi <7014230+arelra@users.noreply.github.com> Date: Fri, 17 Jul 2026 17:00:34 +0100 Subject: [PATCH 7/7] Add a feature switch to toggle between the old and new method of getting deeply read data into the cache. Co-authored-by: Marjan K <15894063+marjisound@users.noreply.github.com> --- common/app/agents/DeeplyReadAgent.scala | 134 +++++++++++++++++- .../app/conf/switches/FeatureSwitches.scala | 11 ++ common/test/agents/DeeplyReadAgentTest.scala | 5 +- facia/test/FaciaControllerTest.scala | 2 +- facia/test/FaciaMetaDataTest.scala | 2 +- onward/test/MostPopularControllerTest.scala | 2 +- 6 files changed, 147 insertions(+), 9 deletions(-) diff --git a/common/app/agents/DeeplyReadAgent.scala b/common/app/agents/DeeplyReadAgent.scala index 7f304d916184..b5ef29e1d6e1 100644 --- a/common/app/agents/DeeplyReadAgent.scala +++ b/common/app/agents/DeeplyReadAgent.scala @@ -1,23 +1,29 @@ package agents +import com.gu.contentapi.client.model.v1.{Content, ElementType} +import com.gu.contentapi.client.utils.CapiModelEnrichment.RenderingFormat import common._ import conf.Configuration +import conf.switches.Switches.UseTrailsFromS3 import model.dotcomrendering.Trail -import services.S3Async +import services.{FaciaContentConvert, OphanApi, S3Async} import scala.concurrent.{ExecutionContext, Future} +import scala.util.control.NonFatal +import contentapi.ContentApiClient +import layout.DiscussionSettings +import model.ContentFormat object DeeplyReadS3Agent extends S3Async { override lazy val bucket = Configuration.cache.bucket; lazy val stage: String = Configuration.environment.stage.toUpperCase } -class DeeplyReadAgent extends GuLogging { +class DeeplyReadAgent(contentApiClient: ContentApiClient, ophanApi: OphanApi) extends GuLogging { private val deeplyReadItems = Box[Map[Edition, Seq[Trail]]](Map.empty) - def refresh()(implicit ec: ExecutionContext): Future[Unit] = { - log.debug(s"Deeply Read Agent refresh()") + def getTrailsFromS3(edition: Edition)(implicit ec: ExecutionContext): Future[Unit] = { Future .sequence(Edition.allEditions.map { edition => DeeplyReadS3Agent @@ -43,6 +49,125 @@ class DeeplyReadAgent extends GuLogging { }) } + def removeStartingSlash(path: String): String = { + if (path.startsWith("/")) path.stripPrefix("/") else path + } + + def correctPillar(pillar: String): String = if (pillar == "arts") "culture" else pillar + + def deeplyReadUrlToTrail(content: Content): Option[Trail] = { + + val contentFormat: ContentFormat = ContentFormat(content.design, content.theme, content.display) + + for { + webPublicationDate <- content.webPublicationDate + fields <- content.fields + linkText <- fields.trailText + pillar <- content.pillarName + headline <- fields.headline + shortUrl <- fields.shortUrl + } yield Trail( + url = content.webUrl, + linkText = linkText, + showByline = false, + byline = fields.byline, + masterImage = None, + image = fields.thumbnail, + carouselImages = Map.empty, + ageWarning = None, + isLiveBlog = fields.liveBloggingNow.getOrElse(false), + pillar = correctPillar(pillar.toLowerCase), + designType = content.`type`.toString, + format = contentFormat, + webPublicationDate = webPublicationDate.toString(), + headline = headline, + mediaType = None, + shortUrl = shortUrl, + kickerText = None, + starRating = None, + avatarUrl = None, + branding = None, + discussion = DiscussionSettings.fromTrail(FaciaContentConvert.contentToFaciaContent(content)), + trailText = content.fields.flatMap(_.trailText), + galleryCount = + content.elements.map(_.count(el => el.`type` == ElementType.Image && el.relation == "gallery")).filter(_ > 0), + ) + } + + def getTrailsFromCAPI(edition: Edition)(implicit ec: ExecutionContext): Future[Unit] = { + /* + We query Ophan for the deeply read URLs and use them to queryCapi + then use this information to create a sequence of trails that we cache + using a Box structure. + */ + Future + .sequence(Edition.allEditions.map { edition => + ophanApi + .getDeeplyRead(edition) + .flatMap { ophanDeeplyReadItems => + log.debug(s"Fetched ${ophanDeeplyReadItems.size} Deeply Read items for ${edition.displayName}") + val constructedTrail: Seq[Future[Option[Trail]]] = ophanDeeplyReadItems.map { ophanItem => + log.debug(s"CAPI lookup for Ophan deeply read item: ${ophanItem.toString}") + val path = removeStartingSlash(ophanItem.path) + log.debug(s"CAPI Lookup for path: $path") + val capiRequest = contentApiClient + .item(path) + .showTags("all") + .showFields("all") + .showReferences("none") + .showAtoms("none") + + contentApiClient + .getResponse(capiRequest) + .map { res => + res.content.flatMap { capiData => + log.debug(s"Retrieved CAPI data for Deeply Read item: ${path}") + deeplyReadUrlToTrail(capiData) + } + } + .recover { case NonFatal(e) => + log.error(s"Error retrieving CAPI data for Deeply Read item: ${path}. ${e.getMessage}") + None + } + } + Future + .sequence(constructedTrail) + .map { maybeTrails => + (edition, maybeTrails.flatten.take(10)) + } + + } + .recover { e => + log.error(s"Failed to fetch Deeply Read items for ${edition.displayName}. ${e.getMessage()}") + (edition, Seq.empty) + } + }) + .map(trailsList => { + val map = trailsList.toMap + for { + (edition, list) <- map + } yield log.debug(s"Deeply Read in ${edition.displayName}, ${list.size} items: ${list.map(_.url).toString()}") + + val mapWithTenItems = map.filter { case (_, list) => list.size == 10 } + log.debug( + s"Updating the following ${mapWithTenItems.size} editions: ${mapWithTenItems.keys.map(_.id).toList.sorted.toString()}", + ) + + deeplyReadItems.alter(deeplyReadItems.get() ++ mapWithTenItems) + }) + } + + def refresh()(implicit ec: ExecutionContext): Future[Unit] = { + if (UseTrailsFromS3.isSwitchedOn) { + // TODO revert to log.debug once we switch to S3 permanently + log.info(s"Deeply Read Agent refresh() - Using S3") + getTrailsFromS3(Edition.defaultEdition) + } else { + log.info(s"Deeply Read Agent refresh() - Using CAPI") + getTrailsFromCAPI(Edition.defaultEdition) + } + } + def getTrails(edition: Edition)(implicit ec: ExecutionContext): Seq[Trail] = { val updatedTrails = deeplyReadItems.get().getOrElse(edition, Seq.empty) if (updatedTrails.isEmpty) { @@ -50,5 +175,4 @@ class DeeplyReadAgent extends GuLogging { } updatedTrails } - } diff --git a/common/app/conf/switches/FeatureSwitches.scala b/common/app/conf/switches/FeatureSwitches.scala index db85fe11e765..d682cf5aa4e6 100644 --- a/common/app/conf/switches/FeatureSwitches.scala +++ b/common/app/conf/switches/FeatureSwitches.scala @@ -671,4 +671,15 @@ trait FeatureSwitches { exposeClientSide = true, highImpact = false, ) + + val UseTrailsFromS3 = Switch( + group = SwitchGroup.Feature, + name = "use-trails-from-s3", + description = "Use trails from S3", + owners = Seq(Owner.withEmail("dotcom.platform@theguardian.com")), + sellByDate = never, + safeState = Off, + exposeClientSide = false, + highImpact = false, + ) } diff --git a/common/test/agents/DeeplyReadAgentTest.scala b/common/test/agents/DeeplyReadAgentTest.scala index a3e143efd2e9..048ee7b95dfe 100644 --- a/common/test/agents/DeeplyReadAgentTest.scala +++ b/common/test/agents/DeeplyReadAgentTest.scala @@ -25,7 +25,10 @@ import test.{ "DeeplyReadAgent" should "initialise with trails being an empty Seq" in { val ophanApi = new OphanApi(wsClient) val contentApiClient = testContentApiClient - val agent = new DeeplyReadAgent + val agent = new DeeplyReadAgent( + contentApiClient = contentApiClient, + ophanApi = ophanApi, + ) Edition.allEditions.map(edition => { agent.getTrails(edition) shouldBe Seq.empty }) diff --git a/facia/test/FaciaControllerTest.scala b/facia/test/FaciaControllerTest.scala index 65810b3647e1..2a0cfac8a46c 100644 --- a/facia/test/FaciaControllerTest.scala +++ b/facia/test/FaciaControllerTest.scala @@ -47,7 +47,7 @@ import scala.concurrent.{Await, Future} play.api.test.Helpers.stubControllerComponents(), wsClient, new MostViewedAgent(testContentApiClient, new OphanApi(wsClient)), - new DeeplyReadAgent, + new DeeplyReadAgent(testContentApiClient, new OphanApi(wsClient)), assets = assets, ) val articleUrl = "/environment/2012/feb/22/capitalise-low-carbon-future" diff --git a/facia/test/FaciaMetaDataTest.scala b/facia/test/FaciaMetaDataTest.scala index 4752fdf8de46..c8c175a69126 100644 --- a/facia/test/FaciaMetaDataTest.scala +++ b/facia/test/FaciaMetaDataTest.scala @@ -49,7 +49,7 @@ import scala.concurrent.duration._ play.api.test.Helpers.stubControllerComponents(), wsClient, new MostViewedAgent(testContentApiClient, new OphanApi(wsClient)), - new DeeplyReadAgent, + new DeeplyReadAgent(testContentApiClient, new OphanApi(wsClient)), assets = assets, ) val frontPath = "music" diff --git a/onward/test/MostPopularControllerTest.scala b/onward/test/MostPopularControllerTest.scala index 0064e8096ac0..dfcd5d31c7db 100644 --- a/onward/test/MostPopularControllerTest.scala +++ b/onward/test/MostPopularControllerTest.scala @@ -27,7 +27,7 @@ import agents.DeeplyReadAgent testContentApiClient, new GeoMostPopularAgent(testContentApiClient, ophanApi), new MostPopularAgent(testContentApiClient), - new DeeplyReadAgent, + new DeeplyReadAgent(testContentApiClient, new OphanApi(wsClient)), play.api.test.Helpers.stubControllerComponents(), )