From 823b38f079ea878e654c502879247f200169a9e9 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 20 Sep 2025 16:57:54 +0100 Subject: [PATCH 01/58] [upstream] Fixes FileMetadataReaderTest fails locally during British summer by removing non time zoned date formatter which was shadowing the long standing time zoned date formatter. Add a British summer time YYYY-MM-dd example to show that this formatter is locale dependant. DateTimeFormat.forPattern("yyyy-MM-dd") matches the same pattern as ISODateTimeFormat.date.withZoneUTC but removes the withZoneUTC behaviour. I do not know if that was intentional but FileMetadataReaderTest was a long standing test so this could be considered a regression. Additionally, that entire block of date formatters probably have an indeterminate outcome. --- .../gu/mediaservice/lib/metadata/ImageMetadataConverter.scala | 2 +- .../mediaservice/lib/metadata/ImageMetadataConverterTest.scala | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala index 0e73246752c..a8b13ec6b99 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverter.scala @@ -144,13 +144,13 @@ object ImageMetadataConverter extends GridLogging { DateTimeFormat.forPattern("E MMM dd HH:mm:ss.SSS 'BST' yyyy").withZone(DateTimeZone.forOffsetHours(1)), DateTimeFormat.forPattern("E MMM dd HH:mm:ss 'BST' yyyy").withZone(DateTimeZone.forOffsetHours(1)), + // TODO these group of formatters are locale dependent. Is this intentional? DateTimeFormat.forPattern("yyyyMMdd"), DateTimeFormat.forPattern("yyyyMM"), DateTimeFormat.forPattern("yyyyddMM"), DateTimeFormat.forPattern("yyyy"), DateTimeFormat.forPattern("yyyy-MM"), DateTimeFormat.forPattern("yyyy:MM:dd"), - DateTimeFormat.forPattern("yyyy-MM-dd"), // 2014-12-16 - Maybe it's just a date // no timezone provided so force UTC rather than use the machine's timezone diff --git a/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala b/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala index 792f38879ca..8f0e6d2142a 100644 --- a/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala +++ b/common-lib/src/test/scala/com/gu/mediaservice/lib/metadata/ImageMetadataConverterTest.scala @@ -340,6 +340,7 @@ class ImageMetadataConverterTest extends AnyFunSpec with Matchers { it("should clean up 'just date' dates into iso format") { ImageMetadataConverter.cleanDate("2014-12-16") shouldBe "2014-12-16T00:00:00.000Z" + ImageMetadataConverter.cleanDate("2014-08-20") shouldBe "2014-08-20T00:00:00.000Z" } it("should clean up iso dates with seconds into iso format") { From 2941b6395e4e5b755d4e5b9252ba5584279df54c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 11 Apr 2026 21:16:46 +0100 Subject: [PATCH 02/58] [upstream] Fixes Intellij sbt import after removing Dynamo SDK v1. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guava is an undeclared transitive dependency used directly in source code. 2 files use Guava APIs directly: 1. common-lib/src/main/java/com/gu/mediaservice/DeprecatedHashWrapper.java:3 — imports com.google.common.hash.HashFunction / Hashing 2. image-loader/app/lib/Downloader.scala:6 — imports com.google.common.hash.HashingOutputStream and com.google.common.io.ByteStreams But Guava is never explicitly declared in build.sbt. It only arrives transitively — almost certainly from aws-java-sdk-* (v1). --- build.sbt | 1 + 1 file changed, 1 insertion(+) diff --git a/build.sbt b/build.sbt index 6091b4ac353..4468efd9721 100644 --- a/build.sbt +++ b/build.sbt @@ -91,6 +91,7 @@ val maybeBBCLib: Option[sbt.ProjectReference] = if(bbcBuildProcess) Some(bbcProj lazy val commonLib = project("common-lib").settings( libraryDependencies ++= Seq( + "com.google.guava" % "guava" % "33.5.0-jre", "com.gu" %% "editorial-permissions-client" % "7.0.0", "com.gu" %% "pan-domain-auth-play_3-0" % "19.0.0", "software.amazon.awssdk" % "iam" % awsSdkV2Version, From 63831b30432ed70f14d5f946bf4afb8c866e12e2 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 23 Apr 2026 18:18:55 +0100 Subject: [PATCH 03/58] [upstream] Fix off by 1 in message processing attempts log output. ``` "message":"Attempt 3 of 2","logger_name":"lib.RetryHandler ``` The number of attempts is the initial attempt (1) + the number of retries. --- thrall/app/lib/RetryHandler.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/thrall/app/lib/RetryHandler.scala b/thrall/app/lib/RetryHandler.scala index d73645ef539..5445522b9cc 100644 --- a/thrall/app/lib/RetryHandler.scala +++ b/thrall/app/lib/RetryHandler.scala @@ -46,12 +46,12 @@ object RetryHandler extends GridLogging { def handleWithRetry[T](f: WithMarkers[T], retries: Int, delay: FiniteDuration): WithMarkers[T] = (marker) => { implicit val scheduler: Scheduler = actorSystem.scheduler + val attempts = retries + 1 var count = 0 - def attempt = () => { count = count + 1 val markerWithRetry = combineMarkers(marker, MarkerMap("retryCount" -> count)) - logger.info(markerWithRetry, s"Attempt $count of $retries") + logger.info(markerWithRetry, s"Attempt $count of $attempts") f(markerWithRetry) } From a481d33474d5e69eccf085d5c15ab1f5860b269d Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 16:43:29 +0100 Subject: [PATCH 04/58] [upstream] testcontainers-elasticsearch for an easier local setup with Rancher. --- build.sbt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 4468efd9721..0a11abd2125 100644 --- a/build.sbt +++ b/build.sbt @@ -125,7 +125,7 @@ lazy val commonLib = project("common-lib").settings( "software.amazon.awssdk" % "bedrockruntime" % awsSdkV2Version, "software.amazon.awssdk" % "s3vectors" % awsSdkV2Version, ws, - "org.testcontainers" % "elasticsearch" % "1.21.4" % Test, + "org.testcontainers" % "testcontainers-elasticsearch" % "2.0.2" % Test, ), dependencyOverrides += "ch.qos.logback" % "logback-classic" % "1.2.13" % Test ) @@ -181,7 +181,7 @@ lazy val thrall = playProject("thrall", 9002) "software.amazon.awssdk" % "kinesis" % awsSdkV2Version, "software.amazon.awssdk" % "dynamodb" % awsSdkV2Version, "com.gu" %% "kcl-pekko-stream" % "0.1.2", - "org.testcontainers" % "elasticsearch" % "1.19.2" % Test, + "org.testcontainers" % "testcontainers-elasticsearch" % "2.0.2" % Test, "com.google.protobuf" % "protobuf-java" % "3.19.6" ), dependencyOverrides ++= Seq( From fe64c463c4b869a49e53f17eca5a85cb1776018f Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:20:02 +0100 Subject: [PATCH 05/58] [upstream] CropController uses GridClient for it's get source image call to Media API. This call was probably the only service to service call circa 2021. GridClient appears to be how more recent service to service calls are done. Moving this call to GridClient helps to enclosure all the service to service url concerns in one place. Get SourceImage requests additional fields via media-api query parameters. Extract the media api uri to image id extraction to a function for testing. --- .../com/gu/mediaservice/GridClient.scala | 19 ++++++-- cropper/app/CropperComponents.scala | 5 ++- .../app/controllers/CropperController.scala | 44 +++---------------- cropper/app/controllers/MediaApiUrls.scala | 16 +++++++ .../test/controllers/MediaApiUrlsTest.scala | 22 ++++++++++ 5 files changed, 64 insertions(+), 42 deletions(-) create mode 100644 cropper/app/controllers/MediaApiUrls.scala create mode 100644 cropper/test/controllers/MediaApiUrlsTest.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala index bf017e31473..c3207891d53 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/GridClient.scala @@ -3,7 +3,7 @@ package com.gu.mediaservice import java.net.URL import com.gu.mediaservice.GridClient.{Error, Found, NotFound, Response} import com.gu.mediaservice.lib.config.Services -import com.gu.mediaservice.model.{Collection, Crop, Edits, Image, ImageMetadata, ImageStatusRecord, SyndicationRights} +import com.gu.mediaservice.model.{Collection, Crop, Edits, Image, ImageMetadata, ImageStatusRecord, SourceImage, SyndicationRights} import com.gu.mediaservice.model.leases.LeasesByMedia import com.gu.mediaservice.model.usage.Usage import com.typesafe.scalalogging.LazyLogging @@ -104,12 +104,13 @@ class GridClient(services: Services, originDomain: String)(implicit wsClient: WS * process before returning data. * See also https://www.playframework.com/documentation/2.6.x/ScalaWS#Configuring-Timeouts */ - def makeGetRequestAsync(url: URL, authFn: WSRequest => WSRequest, requestTimeout: Option[Duration] = None) + def makeGetRequestAsync(url: URL, authFn: WSRequest => WSRequest, requestTimeout: Option[Duration] = None, + queryStringParameters: Option[Seq[(String, String)]] = None) (implicit ec: ExecutionContext): Future[Response] = { - val request: WSRequest = wsClient.url(url.toString) + val request: WSRequest = wsClient.url(url.toString).withQueryStringParameters(queryStringParameters.getOrElse(Seq.empty): _*) val requestWithTimeout = requestTimeout.fold(request)(request.withRequestTimeout) val authorisedRequest = authFn(requestWithTimeout) - authorisedRequest.get().map { response => validateResponse(response, url)} + authorisedRequest.get().map { response => validateResponse(response, url) } } private def validateResponse( @@ -234,6 +235,16 @@ class GridClient(services: Services, originDomain: String)(implicit wsClient: WS } } + def getSourceImage(mediaId: String, authFn: WSRequest => WSRequest)(implicit ec: ExecutionContext): Future[SourceImage] = { + logger.info("attempt to get image") + val url = new URL(s"${services.apiBaseUri}/images/$mediaId") + makeGetRequestAsync(url, authFn, queryStringParameters = Some(Seq("include" -> "fileMetadata"))) map { + case Found(json, _) => json.as[SourceImage] + case nf@NotFound(_, _) => Error(nf.status, url, nf.underlying).logErrorAndThrowException() + case e@Error(_, _, _) => e.logErrorAndThrowException() + } + } + def getMetadata(mediaId: String, authFn: WSRequest => WSRequest)(implicit ec: ExecutionContext): Future[ImageMetadata] = { logger.info("attempt to get metadata") val url = new URL(s"${services.apiBaseUri}/images/$mediaId") diff --git a/cropper/app/CropperComponents.scala b/cropper/app/CropperComponents.scala index c51f554d364..5a9d790ae2f 100644 --- a/cropper/app/CropperComponents.scala +++ b/cropper/app/CropperComponents.scala @@ -1,3 +1,4 @@ +import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} import com.gu.mediaservice.lib.play.GridComponents @@ -15,7 +16,9 @@ class CropperComponents(context: Context) extends GridComponents(context, new Cr val crops = new Crops(config, store, imageOperations) val notifications = new Notifications(config) - val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, wsClient, authorisation) + private val gridClient = GridClient(config.services, config.services.cropperBaseUri)(wsClient) + + val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, authorisation, gridClient) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) diff --git a/cropper/app/controllers/CropperController.scala b/cropper/app/controllers/CropperController.scala index dc2f05aff7b..0d6a1211dfa 100644 --- a/cropper/app/controllers/CropperController.scala +++ b/cropper/app/controllers/CropperController.scala @@ -2,6 +2,7 @@ package controllers import _root_.play.api.libs.json._ import _root_.play.api.mvc.{BaseController, ControllerComponents} +import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.argo.ArgoHelpers import com.gu.mediaservice.lib.argo.model.Link import com.gu.mediaservice.lib.auth.Authentication.Principal @@ -16,7 +17,6 @@ import com.gu.mediaservice.syntax.MessageSubjects import lib._ import model._ import org.joda.time.DateTime -import play.api.libs.ws.WSClient import java.net.URI import scala.concurrent.{ExecutionContext, Future} @@ -30,8 +30,9 @@ case object ApiRequestFailed extends Exception("Failed to fetch the source") class CropperController(auth: Authentication, crops: Crops, store: CropStore, notifications: Notifications, config: CropperConfig, override val controllerComponents: ControllerComponents, - ws: WSClient, authorisation: Authorisation)(implicit val ec: ExecutionContext) - extends BaseController with MessageSubjects with ArgoHelpers { + authorisation: Authorisation, + gridClient: GridClient)(implicit val ec: ExecutionContext) + extends BaseController with MessageSubjects with ArgoHelpers with MediaApiUrls { // Stupid name clash between Argo and Play import com.gu.mediaservice.lib.argo.model.{Action => ArgoAction} @@ -163,7 +164,7 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no )(implicit logMarker: LogMarker): Future[(String, Crop)] = { for { - _ <- verify(isMediaApiUri(exportRequest.uri), InvalidSource) + _ <- verify(isMediaApiImageUri(exportRequest.uri, config.apiUri), InvalidSource) apiImage <- fetchSourceFromApi(exportRequest.uri, onBehalfOfPrincipal) _ <- verify(apiImage.valid, InvalidImage) // Image should always have dimensions, but we want to safely extract the Option @@ -183,39 +184,8 @@ class CropperController(auth: Authentication, crops: Crops, store: CropStore, no } yield (id, finalCrop) } - // TODO: lame, parse into URI object and compare host instead - def isMediaApiUri(uri: String): Boolean = uri.startsWith(config.apiUri) - - def fetchSourceFromApi(uri: String, onBehalfOfPrincipal: Authentication.OnBehalfOfPrincipal): Future[SourceImage] = { - - case class HttpClientResponse(status: Int, statusText: String, json: JsValue) - - val baseRequest = ws.url(uri) - .withQueryStringParameters("include" -> "fileMetadata") - - val request = onBehalfOfPrincipal(baseRequest) - - val responseFuture = request.get().map { r => - HttpClientResponse(r.status, r.statusText, Json.parse(r.body)) - } - - responseFuture recoverWith { - case NonFatal(e) => - logger.warn(s"HTTP request to fetch source failed: $e") - Future.failed(ApiRequestFailed) - } - - for (resp <- responseFuture) - yield { - if (resp.status == 404) { - throw ImageNotFound - } else if (resp.status != 200) { - logger.warn(s"HTTP status ${resp.status} ${resp.statusText} from $uri") - throw ApiRequestFailed - } else { - resp.json.as[SourceImage] - } - } + private def fetchSourceFromApi(uri: String, onBehalfOfPrincipal: Authentication.OnBehalfOfPrincipal): Future[SourceImage] = { + gridClient.getSourceImage(imageIdFrom(uri), onBehalfOfPrincipal) } def verify(cond: => Boolean, error: Throwable): Future[Unit] = diff --git a/cropper/app/controllers/MediaApiUrls.scala b/cropper/app/controllers/MediaApiUrls.scala new file mode 100644 index 00000000000..5ccb8f4161e --- /dev/null +++ b/cropper/app/controllers/MediaApiUrls.scala @@ -0,0 +1,16 @@ +package controllers + +trait MediaApiUrls { + + def isMediaApiImageUri(uri: String, apiUri: String): Boolean = { + val hasMediaApiPrefix = uri.startsWith(apiUri) + val suffix = uri.drop(apiUri.length) + val suffixComponents = suffix.split("/") + val hasImageSuffix = suffixComponents.length == 3 && suffixComponents(1) == "images" + hasMediaApiPrefix && hasImageSuffix + } + + def imageIdFrom(uri: String): String = { + uri.split("/").last + } +} diff --git a/cropper/test/controllers/MediaApiUrlsTest.scala b/cropper/test/controllers/MediaApiUrlsTest.scala new file mode 100644 index 00000000000..6033c6b8ab4 --- /dev/null +++ b/cropper/test/controllers/MediaApiUrlsTest.scala @@ -0,0 +1,22 @@ +package controllers + +import com.gu.mediaservice.lib.imaging.ImageOperations +import com.gu.mediaservice.model._ +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatestplus.mockito.MockitoSugar +import org.scalatest._ +import flatspec._ +import matchers._ + +class MediaApiUrlsTest extends AnyFlatSpec with Matchers with MediaApiUrls { + + "media api urls" should + "identify media API image urls" in { + isMediaApiImageUri("https://media.api.test.com/images/cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe true + isMediaApiImageUri("https://media.api.test.com/images", "https://media.api.test.com") shouldBe false + isMediaApiImageUri("images/cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe false + isMediaApiImageUri("cb5b8c05b690db2d034457b5461ef32abf29eff8", "https://media.api.test.com") shouldBe false + } + +} From 3c030776790eaee5d6a3c41599b28aa9b453cad1 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 18 Jan 2026 12:09:01 +0000 Subject: [PATCH 06/58] [upstream] Remove scala-xml version override. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The override (libraryDependencySchemes += ... VersionScheme.Always for scala-xml) was added back when the build used sbt-native-packager, which at the time pulled in scala-xml 1.1.1 while sbt 1.8.0's own toolchain used scala-xml 2.1.0 — a version clash that made sbt fail hard during the build.sbt compilation step (not the app itself). The sbt-native-packager plugin was removed from project/plugins.sbt in commit 9bc68100e2, "Skeleton of Play 2.6 and SBT 1.0 project" (Michael Barton, 2018-03-28), 64ca9482a5 (2024) — the scala-xml VersionScheme.Always override added, with a comment blaming sbt-native-packager for the conflict — despite that plugin having been gone from the build for 6 years by then. It's possible the conflict was actually triggered by some other plugin (e.g. sbt-buildinfo or sbt-riffraff-artifact) and the comment mis-attributed it, or native-packager was reintroduced transiently in a branch — worth double-checking if it matters, but either way the override outlived any actual justification. --- project/plugins.sbt | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/project/plugins.sbt b/project/plugins.sbt index 635f2c287db..841571ad763 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -8,20 +8,4 @@ addSbtPlugin("com.typesafe.sbt" % "sbt-digest" % "1.1.4") addSbtPlugin("com.typesafe.sbt" % "sbt-gzip" % "1.0.2") -/* - Without setting VersionScheme.Always here on `scala-xml`, sbt 1.8.0 will raise fatal 'version conflict' errors when - used with sbt plugins like `sbt-native-packager`, which currently use sort-of-incompatible versions of the `scala-xml` - library. sbt 1.8.0 has upgraded to Scala 2.12.17, which has itself upgraded to `scala-xml` 2.1.0 - (see https://github.com/sbt/sbt/releases/tag/v1.8.0), but `sbt-native-packager` is currently using `scala-xml` 1.1.1, - and the `scala-xml` library declares that it uses specifically 'early-semver' version compatibility (see - https://www.scala-lang.org/blog/2021/02/16/preventing-version-conflicts-with-versionscheme.html#versionscheme-librarydependencyschemes-and-sbt-150 ), - meaning that for version x.y.z, `x` & `y` *must match exactly* for versions to be considered compatible by sbt. - By setting VersionScheme.Always here on `scala-xml`, we're overriding its declared version-compatability scheme, - choosing to tolerate the risk of binary incompatibility. We consider this to be safe because when set under - `projects/` (ie *not* in `build.sbt` itself) it only affects the compilation of build.sbt, not of the application - build itself. Once the build has succeeded, there is no further risk (ie of a runtime exception due to clashing - versions of `scala-xml`). - */ -libraryDependencySchemes += "org.scala-lang.modules" %% "scala-xml" % VersionScheme.Always - addDependencyTreePlugin From 1b6a8ee71177b6f2070bcb1ea78714b3b67d3b41 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 28 Apr 2026 22:42:16 +0100 Subject: [PATCH 07/58] [upstream] Owned illustrations should render with a blue border like owned photographs. --- kahuna/public/js/image/service.js | 1 + kahuna/public/js/preview/image.html | 4 ++-- kahuna/public/js/services/image-logic.js | 12 ++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/kahuna/public/js/image/service.js b/kahuna/public/js/image/service.js index 47fd7422769..8bc24e72d04 100644 --- a/kahuna/public/js/image/service.js +++ b/kahuna/public/js/image/service.js @@ -36,6 +36,7 @@ imageService.factory('imageService', ['imageLogic', function(imageLogic) { canArchive: imageLogic.canBeArchived(image), persistedReasons: imageLogic.getPersistenceExplanation(image).join('; '), isStaffPhotographer: imageLogic.isStaffPhotographer(image), + isStaffIllustrator: imageLogic.isStaffIllustrator(image), isAgencyPick: imageLogic.isAgencyPick(image), syndicationStatus: imageLogic.getSyndicationStatus(image), syndicationReason: imageLogic.getSyndicationReason(image), diff --git a/kahuna/public/js/preview/image.html b/kahuna/public/js/preview/image.html index be14d356064..b994b5468d8 100644 --- a/kahuna/public/js/preview/image.html +++ b/kahuna/public/js/preview/image.html @@ -42,7 +42,7 @@ >
{{::ctrl.imageDescription}} @@ -53,7 +53,7 @@ >
{{::ctrl.image.data.metadata.description}} diff --git a/kahuna/public/js/services/image-logic.js b/kahuna/public/js/services/image-logic.js index e1d88ca4e59..8f08b810697 100644 --- a/kahuna/public/js/services/image-logic.js +++ b/kahuna/public/js/services/image-logic.js @@ -43,6 +43,17 @@ imageLogic.factory('imageLogic', ['imageAccessor', function(imageAccessor) { staffCategories.includes(image.data.usageRights.category); } + function isStaffIllustrator(image) { + const illustratorCategories = [ + 'staff-illustrator', + 'contract-illustrator', + 'commissioned-illustrator' + ]; + + return image.data.usageRights && + illustratorCategories.includes(image.data.usageRights.category); + } + function isAgencyPick(image) { return Object.entries(window._clientConfig.agencyPicksIngredients || {}).some(([field, values]) => values.some(value => { @@ -125,6 +136,7 @@ imageLogic.factory('imageLogic', ['imageAccessor', function(imageAccessor) { getArchivedState, getPersistenceExplanation, isStaffPhotographer, + isStaffIllustrator, isAgencyPick, getSyndicationStatus, getSyndicationReason, From 5ebc89d6bb9cad2546fd29ac9c6ac10b08e7d04d Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 28 Feb 2026 11:21:06 +0000 Subject: [PATCH 08/58] [upstream] Metadata getUsageRights end point accounts for the Edits.UsageRights not always been available on the JSON edits record. Editing the metadata of an image with no usage rights overrides seems to remove the field, causing getUsageRights to start HTTP 500 erroring. --- metadata-editor/app/controllers/EditsController.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 53f682cca28..1eb2bdb1c9a 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -205,9 +205,12 @@ class EditsController( } def getUsageRights(id: String) = auth.async { - editsStore.get(id).map { dynamoEntry => - val usageRights = (dynamoEntry \ Edits.UsageRights).as[UsageRights] - respond(usageRights) + editsStore.get(id).map { dynamoEntry: JsValue => + val mayBeUsageRights = (dynamoEntry \ Edits.UsageRights).toOption.map(_.as[UsageRights]) + mayBeUsageRights match { + case Some(usageRights: UsageRights) => respond(usageRights) + case None => respondNotFound("No usage rights overrides found") + } } recover { case NoItemFound => respondNotFound("No usage rights overrides found") } From 008b891a7e6cd763786dc1102d0f1c712aef2947 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 28 Aug 2026 16:28:31 +0100 Subject: [PATCH 09/58] [upstream] ThrallEventConsumerTest does not need to extend ElasticSearchTestBase. No need to start a container. --- thrall/test/lib/kinesis/ThrallEventConsumerTest.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/thrall/test/lib/kinesis/ThrallEventConsumerTest.scala b/thrall/test/lib/kinesis/ThrallEventConsumerTest.scala index d94651bac36..96e39a4e6c9 100644 --- a/thrall/test/lib/kinesis/ThrallEventConsumerTest.scala +++ b/thrall/test/lib/kinesis/ThrallEventConsumerTest.scala @@ -1,10 +1,13 @@ package lib.kinesis -import lib.elasticsearch.ElasticSearchTestBase +import helpers.Fixtures import org.scalatest.EitherValues +import org.scalatest.freespec.AnyFreeSpec +import org.scalatest.matchers.should.Matchers import org.scalatestplus.mockito.MockitoSugar -class ThrallEventConsumerTest extends ElasticSearchTestBase with MockitoSugar with EitherValues { + +class ThrallEventConsumerTest extends AnyFreeSpec with Matchers with Fixtures with MockitoSugar with EitherValues { "parse message" - { "parse minimal message" in { val j = @@ -16,7 +19,7 @@ class ThrallEventConsumerTest extends ElasticSearchTestBase with MockitoSugar wi |} |""".stripMargin.getBytes() val m2 = ThrallEventConsumer.parseRecord(j, java.time.Instant.EPOCH) - m2.isRight shouldEqual (true) + m2.isRight shouldEqual true m2.value.subject shouldBe "DeleteImageMessage" } } From d66ed8fa6b9cba0f3925018dd22f8e5a438f0dae Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 16:42:45 +0100 Subject: [PATCH 10/58] For simpler test setup CollectionsStore takes specific Dynamo dependencies rather than all of CommonConfig. --- collections/app/CollectionsComponents.scala | 3 ++- collections/app/store/CollectionsStore.scala | 5 +---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/collections/app/CollectionsComponents.scala b/collections/app/CollectionsComponents.scala index c679179bf6d..20aeeb5d229 100644 --- a/collections/app/CollectionsComponents.scala +++ b/collections/app/CollectionsComponents.scala @@ -4,12 +4,13 @@ import controllers.{CollectionsController, ImageCollectionsController} import lib.{CollectionsConfig, CollectionsMetrics, Notifications} import play.api.ApplicationLoader.Context import router.Routes +import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient import store.{CollectionsStore, ImageCollectionsStore} class CollectionsComponents(context: Context) extends GridComponents(context, new CollectionsConfig(_)) { final override val buildInfo = utils.buildinfo.BuildInfo - val collectionsStore = new CollectionsStore(config) + private val collectionsStore = new CollectionsStore(config.collectionsTable, config.withAWSCredentials(DynamoDbAsyncClient.builder()).build()) val imageCollectionsStore = new ImageCollectionsStore(config) val metrics = new CollectionsMetrics(config, actorSystem, applicationLifecycle) val notifications = new Notifications(config) diff --git a/collections/app/store/CollectionsStore.scala b/collections/app/store/CollectionsStore.scala index 5bf8c1f1a61..1172dcda7da 100644 --- a/collections/app/store/CollectionsStore.scala +++ b/collections/app/store/CollectionsStore.scala @@ -2,7 +2,6 @@ package store import com.gu.mediaservice.lib.collections.CollectionsManager import com.gu.mediaservice.model.{ActionData, Collection} -import lib.CollectionsConfig import org.joda.time.DateTime import org.scanamo.generic.auto.genericDerivedFormat import org.scanamo.{DynamoFormat, ScanamoAsync, Table} @@ -16,9 +15,7 @@ import org.scanamo.generic.semiauto.FieldName case class Record(id: String, collection: Collection) -class CollectionsStore(config: CollectionsConfig) extends DynamoHelpers { - override val tableName: FieldName = config.collectionsTable - lazy val client: DynamoDbAsyncClient = config.withAWSCredentials(DynamoDbAsyncClient.builder()).build() +class CollectionsStore(val tableName: String, client: DynamoDbAsyncClient) extends DynamoHelpers { import org.scanamo.generic.semiauto._ implicit val dateTimeFormat: Typeclass[DateTime] = DynamoFormat.coercedXmap[DateTime, String, IllegalArgumentException](DateTime.parse, _.toString) From 0bc87b06e2360dd7b0b69acb57f5241475eaf76e Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 16:40:23 +0100 Subject: [PATCH 11/58] CollectionsStore Test uses testcontainers localstack supplied dynamoDB. --- build.sbt | 1 + .../test/store/CollectionsStoreTest.scala | 113 ++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 collections/test/store/CollectionsStoreTest.scala diff --git a/build.sbt b/build.sbt index 0a11abd2125..8e3f2506865 100644 --- a/build.sbt +++ b/build.sbt @@ -53,6 +53,7 @@ val commonSettings = Seq( "org.scalatestplus" %% "mockito-3-4" % "3.1.4.0" % Test, "org.mockito" % "mockito-core" % "2.18.0" % Test, "org.scalamock" %% "scalamock" % "5.1.0" % Test, + "org.testcontainers" % "localstack" % "1.21.4" % Test ), dependencyOverrides ++= jacksonOverrides, diff --git a/collections/test/store/CollectionsStoreTest.scala b/collections/test/store/CollectionsStoreTest.scala new file mode 100644 index 00000000000..f4a706456a6 --- /dev/null +++ b/collections/test/store/CollectionsStoreTest.scala @@ -0,0 +1,113 @@ +package store + +import com.gu.mediaservice.model.{ActionData, Collection} +import org.joda.time.DateTime +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.time.{Millis, Seconds, Span} +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB +import org.testcontainers.utility.DockerImageName +import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient +import software.amazon.awssdk.services.dynamodb.model._ + +import java.util.UUID +import scala.concurrent.ExecutionContext.Implicits.global +import scala.concurrent.Future +import scala.jdk.CollectionConverters._ + + +class CollectionsStoreTest extends AnyFunSpec with Matchers with ScalaFutures with BeforeAndAfterAll { + + implicit val defaultPatience: PatienceConfig = PatienceConfig(timeout = Span(2, Seconds), interval = Span(100, Millis)) + + private val dynamoContainer = new LocalStackContainer(DockerImageName.parse("localstack/localstack:1.4.0")).withServices(DYNAMODB) + dynamoContainer.start() + + private val dynamoClient = DynamoDbAsyncClient.builder(). + endpointOverride(dynamoContainer.getEndpointOverride(DYNAMODB)). + region(Region.of(dynamoContainer.getRegion)). + credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create(dynamoContainer.getAccessKey, dynamoContainer.getSecretKey))).build() + + private val collectionsTable = "test-collections-table-" + UUID.randomUUID().toString + private val collectionsTableForAllTest = "test-collections-table-" + UUID.randomUUID().toString + private val store = new CollectionsStore(collectionsTable, dynamoClient) + private val storeForAllTest = new CollectionsStore(collectionsTableForAllTest, dynamoClient) + + override def beforeAll(): Unit = { + def createTableRequestFor(tableName: String): CreateTableRequest = { + val attributeDefinitions = List( + AttributeDefinition.builder.attributeName("id").attributeType(ScalarAttributeType.S).build() + ) + val keySchema = List( + KeySchemaElement.builder.attributeName("id").keyType(KeyType.HASH).build() + ) + val provisionedThroughput = ProvisionedThroughput.builder.readCapacityUnits(1L).writeCapacityUnits(1L).build() + val request = CreateTableRequest.builder + .tableName(tableName) + .attributeDefinitions(attributeDefinitions.asJava) + .keySchema(keySchema.asJava) + .provisionedThroughput(provisionedThroughput) + .build() + request + } + + dynamoClient.createTable(createTableRequestFor(collectionsTable)).get() + dynamoClient.createTable(createTableRequestFor(collectionsTableForAllTest)).get() + } + + override def afterAll(): Unit = { + super.afterAll() + dynamoContainer.stop() + } + + describe("CollectionsStore") { + val collection = Collection(List("a", "b"), ActionData("author", DateTime.now()), "description") + + it("should be able to add a collection") { + val eventualResult = store.add(collection) + whenReady(eventualResult) { c => + c.pathId should be("a/b") + c.description should be("description") + } + } + + it("should be able to get a collection") { + val eventualResult = store.get(List("a", "b")) + whenReady(eventualResult) { c => + c.get.pathId should be("a/b") + c.get.description should be("description") + } + } + + it("should be able to get all collections") { + val collection1 = Collection(List("e", "f"), ActionData("author1", DateTime.now()), "description1") + val collection2 = Collection(List("g", "h"), ActionData("author2", DateTime.now()), "description2") + + val eventualAdded = Future.sequence(Seq(storeForAllTest.add(collection1), storeForAllTest.add(collection2))) + + val eventualResult = eventualAdded.flatMap(_ => storeForAllTest.getAll) + + whenReady(eventualResult) { collections => + collections.size should be(2) + collections.map(_.pathId) should contain allOf("e/f", "g/h") + } + } + + it("should be able to remove a collection") { + val eventualResult = store.remove(List("a", "b")).flatMap(_ => store.get(List("a", "b"))) + whenReady(eventualResult) { c => + c should be(None) + } + + val eventualReadback = store.get(List("a", "b")) + whenReady(eventualReadback) { r => + r should be(None) + } + } + } +} From 29d1559852c9f3af06dd5c80d478e4660f82a2bd Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 8 Mar 2026 12:23:47 +0000 Subject: [PATCH 12/58] Tests for ImagesCollectionsStore. For simpler test setup ImageCollectionsStore takes specific dependencies rather than all of CommonConfig. --- collections/app/CollectionsComponents.scala | 2 +- .../app/store/ImageCollectionsStore.scala | 5 +- .../store/ImageCollectionsStoreTest.scala | 103 ++++++++++++++++++ 3 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 collections/test/store/ImageCollectionsStoreTest.scala diff --git a/collections/app/CollectionsComponents.scala b/collections/app/CollectionsComponents.scala index 20aeeb5d229..2393aca70e2 100644 --- a/collections/app/CollectionsComponents.scala +++ b/collections/app/CollectionsComponents.scala @@ -11,7 +11,7 @@ class CollectionsComponents(context: Context) extends GridComponents(context, ne final override val buildInfo = utils.buildinfo.BuildInfo private val collectionsStore = new CollectionsStore(config.collectionsTable, config.withAWSCredentials(DynamoDbAsyncClient.builder()).build()) - val imageCollectionsStore = new ImageCollectionsStore(config) + val imageCollectionsStore = new ImageCollectionsStore(config.imageCollectionsTable, config.withAWSCredentials(DynamoDbAsyncClient.builder()).build()) val metrics = new CollectionsMetrics(config, actorSystem, applicationLifecycle) val notifications = new Notifications(config) diff --git a/collections/app/store/ImageCollectionsStore.scala b/collections/app/store/ImageCollectionsStore.scala index 1e837e36cb2..b73c9ba5b95 100644 --- a/collections/app/store/ImageCollectionsStore.scala +++ b/collections/app/store/ImageCollectionsStore.scala @@ -16,10 +16,7 @@ import scala.concurrent.ExecutionContext.Implicits.global case class ImageRecord(id: String, collections: List[Collection]) -class ImageCollectionsStore(config: CollectionsConfig) extends DynamoHelpers { - - override val tableName = config.imageCollectionsTable - lazy val client: DynamoDbAsyncClient = config.withAWSCredentials(DynamoDbAsyncClient.builder()).build() +class ImageCollectionsStore(val tableName: String, val client: DynamoDbAsyncClient) extends DynamoHelpers { import org.scanamo.generic.semiauto._ implicit val dateTimeFormat: Typeclass[DateTime] = diff --git a/collections/test/store/ImageCollectionsStoreTest.scala b/collections/test/store/ImageCollectionsStoreTest.scala new file mode 100644 index 00000000000..994131aaf01 --- /dev/null +++ b/collections/test/store/ImageCollectionsStoreTest.scala @@ -0,0 +1,103 @@ +package store + +import com.gu.mediaservice.model.{ActionData, Collection} +import org.joda.time.DateTime +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.time.{Millis, Seconds, Span} +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB +import org.testcontainers.utility.DockerImageName +import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient +import software.amazon.awssdk.services.dynamodb.model._ + +import java.util.UUID +import scala.jdk.CollectionConverters._ + +class ImageCollectionsStoreTest extends AnyFunSpec with Matchers with ScalaFutures with BeforeAndAfterAll { + + implicit val defaultPatience: PatienceConfig = PatienceConfig(timeout = Span(5, Seconds), interval = Span(500, Millis)) + + private val dynamoContainer = new LocalStackContainer(DockerImageName.parse("localstack/localstack:1.4.0")).withServices(DYNAMODB) + dynamoContainer.start() + + private val dynamoClient = DynamoDbAsyncClient.builder(). + endpointOverride(dynamoContainer.getEndpointOverride(DYNAMODB)). + region(Region.of(dynamoContainer.getRegion)). + credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create(dynamoContainer.getAccessKey, dynamoContainer.getSecretKey))).build() + + private val imageCollectionsTable = "test-image-collections-table-" + UUID.randomUUID().toString + private val store = new ImageCollectionsStore(imageCollectionsTable, dynamoClient) + + override def beforeAll(): Unit = { + val attributeDefinitions = List( + AttributeDefinition.builder.attributeName("id").attributeType(ScalarAttributeType.S).build() + ) + val keySchema = List( + KeySchemaElement.builder.attributeName("id").keyType(KeyType.HASH).build() + ) + val provisionedThroughput = ProvisionedThroughput.builder.readCapacityUnits(1L).writeCapacityUnits(1L).build() + val request = CreateTableRequest.builder + .tableName(imageCollectionsTable) + .attributeDefinitions(attributeDefinitions.asJava) + .keySchema(keySchema.asJava) + .provisionedThroughput(provisionedThroughput) + .build() + dynamoClient.createTable(request).get() + } + + override def afterAll(): Unit = { + super.afterAll() + dynamoContainer.stop() + } + + describe("ImageCollectionsStore") { + val imageId = "test-image-id" + val collection1 = Collection(List("a", "b"), ActionData("author", DateTime.now()), "description 1") + val collection2 = Collection(List("c", "d"), ActionData("author", DateTime.now()), "description 2") + + it("should be able to add image to a collection") { + val eventualAddedResponse = store.add(imageId, collection1) + whenReady(eventualAddedResponse) { response => + response.size should be(1) + response.head.pathId should be("a/b") + } + + // Read back + val eventuallyReloaded = store.get(imageId) + whenReady(eventuallyReloaded) { collections => + collections.size should be(1) + collections.head.pathId should be("a/b") + } + + // Add another collection + whenReady(store.add(imageId, collection2)) { collections => + collections.size should be(2) + collections.map(_.pathId) should contain allOf("a/b", "c/d") + } + + whenReady(store.get(imageId)) { collections => + collections.size should be(2) + collections.head.pathId should be("a/b") + collections.last.pathId should be("c/d") + } + + // Update to replace all for this image + val newCollection = Collection(List("e", "f"), ActionData("new-author", DateTime.now()), "new description") + val eventuallyUpdated = store.update(imageId, List(newCollection)) + whenReady(eventuallyUpdated) { collections => + collections.size should be(1) + collections.head.pathId should be("e/f") + } + + whenReady(store.get(imageId)) { collections => + collections.size should be(1) + collections.head.pathId should be("e/f") + } + } + } +} From cbed798a5fa95772d212123754d5d221289bc537 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 12:11:49 +0100 Subject: [PATCH 13/58] For simpler test setup DynamoDB base class takes specific Dynamo dependencies rather than all of CommonConfig. Change the constructor parameters of EditsStore, SyndicationStore and UsageTable to take Dynamo client(s) and table name only. # Conflicts: # common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala --- .../gu/mediaservice/lib/aws/DynamoDB.scala | 6 ++-- .../app/MetadataEditorComponents.scala | 5 +-- metadata-editor/app/lib/EditsStore.scala | 3 +- .../app/lib/SyndicationStore.scala | 3 +- usage/app/UsageComponents.scala | 6 +++- usage/app/model/UsageTable.scala | 35 +++++++++---------- 6 files changed, 31 insertions(+), 27 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala index 11de358f687..d501b54e5e8 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala @@ -1,7 +1,6 @@ package com.gu.mediaservice.lib.aws import com.gu.mediaservice.lib.aws.DynamoDB.{deleteExpr, jsonWithNullAsEmptyString, setExpr} -import com.gu.mediaservice.lib.config.CommonConfig import com.gu.mediaservice.lib.logging.GridLogging import org.joda.time.DateTime import play.api.libs.json._ @@ -18,13 +17,12 @@ object NoItemFound extends Throwable("item not found") /** * A lightweight wrapper around AWS dynamo SDK for undertaking various operations - * @param config Common grid config including AWS credentials + * @param client DynamoDbClient client * @param tableName the table name for this instance of the dynamoDB wrapper * @param lastModifiedKey if set to a string the wrapper will maintain a last modified with that name on any update * @tparam T The type of this table */ -class DynamoDB[T](config: CommonConfig, tableName: String, lastModifiedKey: Option[String] = None) extends GridLogging { - lazy val client: DynamoDbClient = config.withAWSCredentials(DynamoDbClient.builder()).build() +class DynamoDB[T](client: DynamoDbClient, tableName: String, lastModifiedKey: Option[String] = None) extends GridLogging { lazy val dynamo: DynamoDbEnhancedClient = DynamoDbEnhancedClient.builder().dynamoDbClient(client).build() lazy val tableSchema = TableSchema.documentSchemaBuilder() .addIndexPartitionKey(TableMetadata.primaryIndexName(), IdKey, AttributeValueType.S) diff --git a/metadata-editor/app/MetadataEditorComponents.scala b/metadata-editor/app/MetadataEditorComponents.scala index 1b2c439ebcb..f3882cf39fa 100644 --- a/metadata-editor/app/MetadataEditorComponents.scala +++ b/metadata-editor/app/MetadataEditorComponents.scala @@ -4,12 +4,13 @@ import controllers.{EditsApi, EditsController, SyndicationController} import lib._ import play.api.ApplicationLoader.Context import router.Routes +import software.amazon.awssdk.services.dynamodb.DynamoDbClient class MetadataEditorComponents(context: Context) extends GridComponents(context, new EditsConfig(_)) { final override val buildInfo = utils.buildinfo.BuildInfo - val editsStore = new EditsStore(config) - val syndicationStore = new SyndicationStore(config) + val editsStore = new EditsStore(config.withAWSCredentials(DynamoDbClient.builder()).build(), config.editsTable) + val syndicationStore = new SyndicationStore(config.withAWSCredentials(DynamoDbClient.builder()).build(), config.syndicationTable) val notifications = new Notifications(config) val metrics = new MetadataEditorMetrics(config, actorSystem, applicationLifecycle) diff --git a/metadata-editor/app/lib/EditsStore.scala b/metadata-editor/app/lib/EditsStore.scala index dfeffbfe991..a1700830209 100644 --- a/metadata-editor/app/lib/EditsStore.scala +++ b/metadata-editor/app/lib/EditsStore.scala @@ -2,5 +2,6 @@ package lib import com.gu.mediaservice.lib.aws.DynamoDB import com.gu.mediaservice.model.Edits +import software.amazon.awssdk.services.dynamodb.DynamoDbClient -class EditsStore(config: EditsConfig) extends DynamoDB[Edits](config, config.editsTable, Some(Edits.LastModified)) +class EditsStore(client: DynamoDbClient, tableName: String) extends DynamoDB[Edits](client, tableName, Some(Edits.LastModified)) diff --git a/metadata-editor/app/lib/SyndicationStore.scala b/metadata-editor/app/lib/SyndicationStore.scala index 0c2f9d9b57f..e4d16596011 100644 --- a/metadata-editor/app/lib/SyndicationStore.scala +++ b/metadata-editor/app/lib/SyndicationStore.scala @@ -2,5 +2,6 @@ package lib import com.gu.mediaservice.lib.aws.DynamoDB import com.gu.mediaservice.model.SyndicationRights +import software.amazon.awssdk.services.dynamodb.DynamoDbClient -class SyndicationStore(config: EditsConfig) extends DynamoDB[SyndicationRights](config, config.syndicationTable) +class SyndicationStore(client: DynamoDbClient, tableName: String) extends DynamoDB[SyndicationRights](client, tableName) diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index 692e10502e9..08394033de3 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -6,6 +6,7 @@ import lib._ import model._ import play.api.ApplicationLoader.Context import router.Routes +import software.amazon.awssdk.services.dynamodb.DynamoDbClient import scala.concurrent.Future @@ -17,7 +18,10 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag val mediaWrapper = new MediaWrapperOps(usageMetadataBuilder) val liveContentApi = new LiveContentApi(config)(ScheduledExecutor()) val usageGroupOps = new UsageGroupOps(config, mediaWrapper) - val usageTable = new UsageTable(config) + val usageTable = new UsageTable( + config.withAWSCredentials(DynamoDbClient.builder()).build(), + config.usageRecordTable + ) val usageMetrics = new UsageMetrics(config, actorSystem, applicationLifecycle) val usageNotifier = new UsageNotifier(config, usageTable) val usageRecorder = new UsageRecorder(usageMetrics, usageTable, usageNotifier, usageNotifier) diff --git a/usage/app/model/UsageTable.scala b/usage/app/model/UsageTable.scala index 720faa7e73c..806eb52a303 100644 --- a/usage/app/model/UsageTable.scala +++ b/usage/app/model/UsageTable.scala @@ -5,27 +5,26 @@ import com.gu.mediaservice.lib.aws.DynamoDB.jsonWithNullAsEmptyString import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.lib.usage.ItemToMediaUsage import com.gu.mediaservice.model.usage.{MediaUsage, PendingUsageStatus, PublishedUsageStatus, UsageTableFullKey} -import lib.{BadInputException, UsageConfig, WithLogMarker} +import lib.{BadInputException, WithLogMarker} import play.api.libs.json._ import rx.lang.scala.Observable import software.amazon.awssdk.enhanced.dynamodb.document.EnhancedDocument -import software.amazon.awssdk.enhanced.dynamodb.model.{DeleteItemEnhancedRequest, QueryConditional, QueryEnhancedRequest, UpdateItemEnhancedRequest} -import software.amazon.awssdk.enhanced.dynamodb.{AttributeConverterProvider, AttributeValueType, DynamoDbEnhancedClient, Key, TableMetadata, TableSchema} +import software.amazon.awssdk.enhanced.dynamodb.model.{DeleteItemEnhancedRequest, QueryConditional, QueryEnhancedRequest} +import software.amazon.awssdk.enhanced.dynamodb._ import software.amazon.awssdk.services.dynamodb.DynamoDbClient import software.amazon.awssdk.services.dynamodb.model.{AttributeValue, ReturnValue, UpdateItemRequest} import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future -import scala.jdk.CollectionConverters.{IterableHasAsScala, IteratorHasAsScala, MapHasAsJava, MapHasAsScala} +import scala.jdk.CollectionConverters.{IterableHasAsScala, IteratorHasAsScala, MapHasAsJava} -class UsageTable(config: UsageConfig) extends GridLogging { +class UsageTable(client: DynamoDbClient, tableName: String) extends GridLogging { val hashKeyName = "grouping" val rangeKeyName = "usage_id" val imageIndexName = "media_id" - lazy val client: DynamoDbClient = config.withAWSCredentials(DynamoDbClient.builder()).build() lazy val dynamo: DynamoDbEnhancedClient = DynamoDbEnhancedClient.builder().dynamoDbClient(client).build() lazy val tableSchema = TableSchema.documentSchemaBuilder() .addIndexPartitionKey(TableMetadata.primaryIndexName(), hashKeyName, AttributeValueType.S) @@ -37,7 +36,7 @@ class UsageTable(config: UsageConfig) extends GridLogging { ) .attributeConverterProviders(AttributeValueConverterProvider, AttributeConverterProvider.defaultProvider()) .build() - lazy val table = dynamo.table(config.usageRecordTable, tableSchema) + lazy val table = dynamo.table(tableName, tableSchema) def queryByUsageId(id: String): Future[Option[MediaUsage]] = Future { UsageTableFullKey.build(id).flatMap((tableFullKey: UsageTableFullKey) => { @@ -96,9 +95,9 @@ class UsageTable(config: UsageConfig) extends GridLogging { }) if (publishedUsage.isEmpty) { - groupedUsages.headOption + groupedUsages.headOption } else { - publishedUsage + publishedUsage } }.toList @@ -169,13 +168,13 @@ class UsageTable(config: UsageConfig) extends GridLogging { client.updateItem(request) - }) - .onErrorResumeNext(e => { - logger.error(logMarker, s"Dynamo update fail for $record!", e) - Observable.error(e) - }) - .map(updateResponse => { - val doc = EnhancedDocument.fromAttributeValueMap(updateResponse.attributes()) - jsonWithNullAsEmptyString(Json.parse(doc.toJson)).as[JsObject] - }) + }) + .onErrorResumeNext(e => { + logger.error(logMarker, s"Dynamo update fail for $record!", e) + Observable.error(e) + }) + .map(updateResponse => { + val doc = EnhancedDocument.fromAttributeValueMap(updateResponse.attributes()) + jsonWithNullAsEmptyString(Json.parse(doc.toJson)).as[JsObject] + }) } From b74738167a5949937955aac8367f7efb3439e122 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 27 Aug 2026 16:56:23 +0100 Subject: [PATCH 14/58] EditStores tests --- metadata-editor/test/lib/EditsStoreTest.scala | 322 ++++++++++++++++++ 1 file changed, 322 insertions(+) create mode 100644 metadata-editor/test/lib/EditsStoreTest.scala diff --git a/metadata-editor/test/lib/EditsStoreTest.scala b/metadata-editor/test/lib/EditsStoreTest.scala new file mode 100644 index 00000000000..f519697d5d7 --- /dev/null +++ b/metadata-editor/test/lib/EditsStoreTest.scala @@ -0,0 +1,322 @@ +package lib + +import com.gu.mediaservice.model.{Edits, ImageMetadata} +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.time.{Millis, Seconds, Span} +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB +import org.testcontainers.utility.DockerImageName +import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.dynamodb.DynamoDbClient +import software.amazon.awssdk.services.dynamodb.model._ + +import java.util.UUID +import scala.concurrent.ExecutionContext.Implicits.global +import scala.jdk.CollectionConverters._ + +class EditsStoreTest extends AnyFunSpec with Matchers with ScalaFutures with BeforeAndAfterAll { + + implicit val defaultPatience: PatienceConfig = PatienceConfig(timeout = Span(5, Seconds), interval = Span(100, Millis)) + + private val dynamoContainer = new LocalStackContainer(DockerImageName.parse("localstack/localstack:1.4.0")).withServices(DYNAMODB) + dynamoContainer.start() + + val testTableName: String = "test-edits-table-" + UUID.randomUUID().toString + + private val dynamoClient2: DynamoDbClient = DynamoDbClient.builder(). + endpointOverride(dynamoContainer.getEndpointOverride(DYNAMODB)). + region(Region.of(dynamoContainer.getRegion)). + credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create(dynamoContainer.getAccessKey, dynamoContainer.getSecretKey))).build() + + private val store = new EditsStore(dynamoClient2, testTableName) { + } + + override def beforeAll(): Unit = { + def createTableRequestFor(tableName: String): CreateTableRequest = { + val attributeDefinitions = List( + AttributeDefinition.builder.attributeName("id").attributeType(ScalarAttributeType.S).build() + ) + val keySchema = List( + KeySchemaElement.builder.attributeName("id").keyType(KeyType.HASH).build() + ) + val provisionedThroughput = ProvisionedThroughput.builder.readCapacityUnits(1L).writeCapacityUnits(1L).build() + val request = CreateTableRequest.builder + .tableName(tableName) + .attributeDefinitions(attributeDefinitions.asJava) + .keySchema(keySchema.asJava) + .provisionedThroughput(provisionedThroughput) + .build() + request + } + + dynamoClient2.createTable(createTableRequestFor(testTableName)) + } + + override def afterAll(): Unit = { + super.afterAll() + dynamoContainer.stop() + } + + describe("EditsStore") { + + it("should fail with NoItemFound for a non-existent item in get") { + val imageId = "non-existent-image" + + val eventualResult = store.get(imageId) + + whenReady(eventualResult.failed) { exception => + exception shouldBe com.gu.mediaservice.lib.aws.NoItemFound + } + } + + it("should persist a boolean property using ") { + val imageId = "test-image-for-boolean-get-" + + val eventualResult = store.booleanSet(imageId, Edits.Archived, value = true).flatMap { _ => + store.booleanGet(imageId, Edits.Archived) + } + + whenReady(eventualResult) { result => + result should be(true) + } + } + + it("should fail with NoItemFound for a non-existent attribute in booleanGet") { + val imageId = "test-image-for-boolean-get--non-existent-attribute" + + // First, create the item so it exists + store.booleanSet(imageId, Edits.Archived, value = true).futureValue + + val eventualResult = store.booleanGet(imageId, "another-existent-attribute") + + whenReady(eventualResult.failed) { exception => + exception shouldBe com.gu.mediaservice.lib.aws.NoItemFound + } + } + + it("should get a previously persisted property using ") { + val imageId = "test-image-for-set-get-" + val labels = List("label1", "label2") + val eventualResult = store.setAdd(imageId, Edits.Labels, labels).flatMap { _ => + store.setGet(imageId, Edits.Labels) + } + + whenReady(eventualResult) { result => + result should be(labels.toSet) + } + } + + describe("setAdd") { + it("should add a value to a non-existent set, creating it") { + val imageId = "test-image-for-set-add-non-existent" + val label = "label1" + + val eventualResult = store.setAdd(imageId, Edits.Labels, List(label)).flatMap { _ => + store.setGet(imageId, Edits.Labels) + } + + whenReady(eventualResult) { result => + result should be(Set(label)) + } + } + + it("should append a value to an existing set") { + val imageId = "test-image-for-set-add-existing" + val labels = List("label1", "label2") + val newLabel = "label3" + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.setAdd(imageId, Edits.Labels, List(newLabel)) + result <- store.setGet(imageId, Edits.Labels) + } yield result + + whenReady(eventualResult) { result => + result should be(Set("label1", "label2", "label3")) + } + } + + it("should not add a duplicate value to a set") { + val imageId = "test-image-for-set-add-duplicate" + val label = "label1" + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, List(label)) + _ <- store.setAdd(imageId, Edits.Labels, List(label)) + result <- store.setGet(imageId, Edits.Labels) + } yield result + + whenReady(eventualResult) { result => + result should be(Set(label)) + } + } + } + + describe("booleanSetOrRemove") { + it("should set a boolean property to true using ") { + val imageId = "test-image-for-boolean-set-or-remove--set" + val key = "testBoolean" + + val eventualResult = for { + _ <- store.booleanSetOrRemove(imageId, key, value = true) + result <- store.booleanGet(imageId, key) + } yield result + + whenReady(eventualResult) { result => + result should be(true) + } + } + + it("should remove a boolean property when setting to false using ") { + val imageId = "test-image-for-boolean-set-or-remove--remove" + val key = "testBoolean" + + val setup = for { + _ <- store.booleanSetOrRemove(imageId, key, value = true) + _ <- store.booleanSetOrRemove(imageId, key, value = false) + result <- store.booleanGet(imageId, key).failed + } yield result + + whenReady(setup) { exception => + exception shouldBe com.gu.mediaservice.lib.aws.NoItemFound + } + } + } + + describe("removeKey") { + it("should remove a key from an existing item") { + val imageId = "test-image-for-remove-key" + val labels = List("label1", "label2") + val archived = true + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.booleanSet(imageId, Edits.Archived, archived) + _ <- store.removeKey(imageId, Edits.Labels) + result <- store.get(imageId) + } yield result + + whenReady(eventualResult) { result => + val edits = result.as[Edits] + edits.archived should be(archived) + edits.labels should be(empty) + } + } + + it("should not fail when removing a non-existent key from an existing item") { + val imageId = "test-image-for-remove-non-existent-key" + val labels = List("label1", "label2") + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.removeKey(imageId, Edits.Archived) + result <- store.get(imageId) + } yield result + + whenReady(eventualResult) { result => + val edits = result.as[Edits] + edits.labels.toSet should be(labels.toSet) + } + } + + it("will create an item when removing a key from a non-existent item") { + // TODO should this really be happening? + val imageId = "non-existent-image-for-remove-key" + + val eventualResult = for { + _ <- store.removeKey(imageId, Edits.Labels) + result <- store.get(imageId) + } yield result + + whenReady(eventualResult) { result => + val edits = result.as[Edits] + edits.labels should be(empty) + edits.metadata should be(ImageMetadata.empty) + } + } + } + + describe("setDelete") { + it("should delete an item from a set") { + val imageId = "test-image-for-set-delete" + val labels = List("label1", "label2", "label3") + val labelToDelete = "label2" + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.setDelete(imageId, Edits.Labels, labelToDelete) + result <- store.setGet(imageId, Edits.Labels) + } yield result + + whenReady(eventualResult) { result => + result should be(Set("label1", "label3")) + } + } + + it("should leave an empty set when deleting the last item from a set") { + val imageId = "test-image-for-set-delete-last-item" + val label = "label1" + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, List(label)) + _ <- store.setDelete(imageId, Edits.Labels, label) + result <- store.get(imageId) + } yield result + + whenReady(eventualResult) { result => + // After deleting the last item, the key (labels) should be removed. + // leaving an empty set rather than a missing key + val edits = result.as[Edits] + edits.labels should be(empty) + } + } + + it("should not fail when deleting a non-existent item from a set") { + val imageId = "test-image-for-set-delete-non-existent-item" + val labels = List("label1", "label2") + val labelToDelete = "label3" + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.setDelete(imageId, Edits.Labels, labelToDelete) + result <- store.setGet(imageId, Edits.Labels) + } yield result + + whenReady(eventualResult) { result => + result should be(Set("label1", "label2")) + } + } + } + + describe("deleteItem") { + it("should delete an existing item") { + val imageId = "test-image-for-delete-item-" + val labels = List("label1", "label2") + + val eventualResult = for { + _ <- store.setAdd(imageId, Edits.Labels, labels) + _ <- store.deleteItem(imageId) + result <- store.get(imageId).failed + } yield result + + whenReady(eventualResult) { exception => + exception shouldBe com.gu.mediaservice.lib.aws.NoItemFound + } + } + + it("should not fail when deleting a non-existent item") { + val imageId = "non-existent-image-for-delete-item-" + + val eventualResult = store.deleteItem(imageId) + + whenReady(eventualResult) { result => + result should be(()) + } + } + } + } +} From 3f21cc2e3f1c3526919bc4eeba6c84126ab9464f Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 16 Mar 2026 23:18:31 +0000 Subject: [PATCH 15/58] For simpler test setup LeaseStore takes specific Dynamo dependencies rather than all of CommonConfig. --- leases/app/LeasesComponents.scala | 3 ++- leases/app/lib/LeaseStore.scala | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/leases/app/LeasesComponents.scala b/leases/app/LeasesComponents.scala index e236f6f3139..1a1a005f058 100644 --- a/leases/app/LeasesComponents.scala +++ b/leases/app/LeasesComponents.scala @@ -4,11 +4,12 @@ import controllers.MediaLeaseController import lib.{LeaseNotifier, LeaseStore, LeasesConfig} import play.api.ApplicationLoader.Context import router.Routes +import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient class LeasesComponents(context: Context) extends GridComponents(context, new LeasesConfig(_)) { final override val buildInfo = utils.buildinfo.BuildInfo - val store = new LeaseStore(config) + val store = new LeaseStore(config.leasesTable,config.withAWSCredentials(DynamoDbAsyncClient.builder()).build()) val notifications = new LeaseNotifier(config, store) val controller = new MediaLeaseController(auth, store, config, notifications, controllerComponents) diff --git a/leases/app/lib/LeaseStore.scala b/leases/app/lib/LeaseStore.scala index 6e64fd4cbcb..f700724c0f5 100644 --- a/leases/app/lib/LeaseStore.scala +++ b/leases/app/lib/LeaseStore.scala @@ -10,15 +10,13 @@ import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient import scala.concurrent.{ExecutionContext, Future} -class LeaseStore(config: LeasesConfig) { - val client = config.withAWSCredentials(DynamoDbAsyncClient.builder()).build() - +class LeaseStore(tableName: String, client: DynamoDbAsyncClient) { implicit val dateTimeFormat: Typeclass[DateTime] = DynamoFormat.coercedXmap[DateTime, String, IllegalArgumentException](DateTime.parse, _.toString) implicit val enumFormat: Typeclass[MediaLeaseType] = DynamoFormat.coercedXmap[MediaLeaseType, String, IllegalArgumentException](MediaLeaseType(_), _.toString) - private val leasesTable = Table[MediaLease](config.leasesTable) + private val leasesTable = Table[MediaLease](tableName) def get(id: String)(implicit ec: ExecutionContext): Future[Option[MediaLease]] = { ScanamoAsync(client).exec(leasesTable.get("id" === id)).map(_.flatMap(_.toOption)) From d6b88a219a04ee8d169887261e62250c9d175d20 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 16 Mar 2026 23:18:52 +0000 Subject: [PATCH 16/58] LeaseStore tests --- leases/test/lib/LeaseStoreSpec.scala | 138 +++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 leases/test/lib/LeaseStoreSpec.scala diff --git a/leases/test/lib/LeaseStoreSpec.scala b/leases/test/lib/LeaseStoreSpec.scala new file mode 100644 index 00000000000..174dd9faf5e --- /dev/null +++ b/leases/test/lib/LeaseStoreSpec.scala @@ -0,0 +1,138 @@ +package lib + +import com.gu.mediaservice.model.leases.{AllowUseLease, MediaLease} +import org.joda.time.{DateTime, DateTimeZone} +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.time.{Millis, Seconds, Span} +import org.scalatestplus.mockito.MockitoSugar +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB +import org.testcontainers.utility.DockerImageName +import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient +import software.amazon.awssdk.services.dynamodb.model._ + +import java.util.UUID +import scala.concurrent.ExecutionContext.Implicits.global +import scala.jdk.CollectionConverters._ + +class LeaseStoreSpec extends AnyFunSpec with Matchers with ScalaFutures with BeforeAndAfterAll with MockitoSugar { + + implicit val defaultPatience: PatienceConfig = PatienceConfig(timeout = Span(2, Seconds), interval = Span(100, Millis)) + + private val dynamoContainer = new LocalStackContainer(DockerImageName.parse("localstack/localstack:1.4.0")).withServices(DYNAMODB) + dynamoContainer.start() + + private val dynamoClient = DynamoDbAsyncClient.builder(). + endpointOverride(dynamoContainer.getEndpointOverride(DYNAMODB)). + region(Region.of(dynamoContainer.getRegion)). + credentialsProvider(StaticCredentialsProvider.create(AwsBasicCredentials.create(dynamoContainer.getAccessKey, dynamoContainer.getSecretKey))).build() + + private val leasesTable = "test-leases-table-" + UUID.randomUUID().toString + + private val store = new LeaseStore(leasesTable, dynamoClient) + + override def beforeAll(): Unit = { + def createTableRequestFor(tableName: String): CreateTableRequest = { + val attributeDefinitions = List( + AttributeDefinition.builder.attributeName("id").attributeType(ScalarAttributeType.S).build(), + AttributeDefinition.builder.attributeName("mediaId").attributeType(ScalarAttributeType.S).build() + ) + val keySchema = List( + KeySchemaElement.builder.attributeName("id").keyType(KeyType.HASH).build() + ) + val globalSecondaryIndexes = List( + GlobalSecondaryIndex.builder() + .indexName("mediaId") + .keySchema(KeySchemaElement.builder().attributeName("mediaId").keyType(KeyType.HASH).build()) + .projection(Projection.builder().projectionType(ProjectionType.ALL).build()) + .provisionedThroughput(ProvisionedThroughput.builder().readCapacityUnits(1L).writeCapacityUnits(1L).build()) + .build() + ) + CreateTableRequest.builder + .tableName(tableName) + .attributeDefinitions(attributeDefinitions.asJava) + .keySchema(keySchema.asJava) + .globalSecondaryIndexes(globalSecondaryIndexes.asJava) + .provisionedThroughput(ProvisionedThroughput.builder.readCapacityUnits(1L).writeCapacityUnits(1L).build()) + .build() + } + + dynamoClient.createTable(createTableRequestFor(leasesTable)).get() + } + + override def afterAll(): Unit = { + super.afterAll() + dynamoContainer.stop() + } + + describe("LeaseStore") { + val now = DateTime.now.withZone(DateTimeZone.UTC) + val lease = MediaLease( + id = Some(UUID.randomUUID().toString), + mediaId = "media-id-1", + leasedBy = Some("test"), + notes = Some("test notes"), + access = AllowUseLease, + createdAt = now + ) + + it("should be able to add a lease") { + val eventualResult = store.put(lease) + whenReady(eventualResult) { _ => + val readBack = store.get(lease.id.get) + whenReady(readBack) { result => + result should equal(Some(lease)) + } + } + } + + it("should be able to get a lease for a media id") { + val lease2 = lease.copy(id = Some(UUID.randomUUID().toString), mediaId = "media-id-2") + val eventualResult = store.put(lease2) + whenReady(eventualResult) { _ => + val readBack = store.getForMedia("media-id-2") + whenReady(readBack) { result => + result should be(List(lease2)) + } + } + } + + it("should be able to get all leases") { + val lease3 = lease.copy(id = Some(UUID.randomUUID().toString)) + val lease4 = lease.copy(id = Some(UUID.randomUUID().toString)) + val eventualResult = store.putAll(List(lease3, lease4)) + + whenReady(eventualResult) { _ => + val readBack = store.forEach(identity) + whenReady(readBack) { result => + result should contain allOf(lease3, lease4) + } + } + } + + it("should be able to delete a lease") { + val lease5 = lease.copy(id = Some(UUID.randomUUID().toString)) + val eventualResult = store.put(lease5) + + whenReady(eventualResult) { _ => + val readBack = store.get(lease5.id.get) + whenReady(readBack) { result => + result should be(Some(lease5)) + } + + val deleteResult = store.delete(lease5.id.get) + whenReady(deleteResult) { _ => + val readBackAfterDelete = store.get(lease5.id.get) + whenReady(readBackAfterDelete) { result => + result should be(None) + } + } + } + } + } +} From 4d440b351b3a57a80aeca3e01d9f145fe99493bd Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 15 Mar 2026 10:34:48 +0000 Subject: [PATCH 17/58] UsageTable tests for matchUsageGroup, update, markAsRemoved, and deleteRecord --- usage/test/model/UsageTableTest.scala | 329 ++++++++++++++++++++++++++ 1 file changed, 329 insertions(+) create mode 100644 usage/test/model/UsageTableTest.scala diff --git a/usage/test/model/UsageTableTest.scala b/usage/test/model/UsageTableTest.scala new file mode 100644 index 00000000000..cb5a447047a --- /dev/null +++ b/usage/test/model/UsageTableTest.scala @@ -0,0 +1,329 @@ +package model + +import com.gu.mediaservice.lib.logging.{GridLogging, MarkerMap} +import com.gu.mediaservice.model.usage._ +import lib.WithLogMarker +import org.joda.time.DateTime +import org.scalatest.BeforeAndAfterAll +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.time.{Millis, Seconds, Span} +import org.testcontainers.containers.localstack.LocalStackContainer +import org.testcontainers.containers.localstack.LocalStackContainer.Service.DYNAMODB +import org.testcontainers.utility.DockerImageName +import software.amazon.awssdk.auth.credentials.{AwsBasicCredentials, StaticCredentialsProvider} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.dynamodb.DynamoDbClient +import software.amazon.awssdk.services.dynamodb.model._ + +import java.net.URI +import java.util.UUID +import scala.concurrent.Await +import scala.concurrent.duration._ +import scala.jdk.CollectionConverters._ + +class UsageTableTest extends AnyFunSpec with Matchers with GridLogging with ScalaFutures with BeforeAndAfterAll { + + implicit val defaultPatience: PatienceConfig = PatienceConfig(timeout = Span(5, Seconds), interval = Span(500, Millis)) + private val tenSeconds = 10.seconds + + private val dynamoContainer = new LocalStackContainer( + DockerImageName.parse("localstack/localstack:1.4.0") + ).withServices(DYNAMODB) + dynamoContainer.start() + + private val dynamoClientV2: DynamoDbClient = DynamoDbClient.builder() + .endpointOverride(dynamoContainer.getEndpointOverride(DYNAMODB)) + .region(Region.of(dynamoContainer.getRegion)) + .credentialsProvider(StaticCredentialsProvider.create( + AwsBasicCredentials.create(dynamoContainer.getAccessKey, dynamoContainer.getSecretKey) + )).build() + + private val usageTable = "test-usage-table-" + UUID.randomUUID().toString + private val store = new UsageTable(dynamoClientV2, usageTable) + + override def beforeAll(): Unit = { + val attributeDefinitions = List( + AttributeDefinition.builder.attributeName("grouping").attributeType(ScalarAttributeType.S).build(), + AttributeDefinition.builder.attributeName("usage_id").attributeType(ScalarAttributeType.S).build(), + AttributeDefinition.builder.attributeName("media_id").attributeType(ScalarAttributeType.S).build() + ) + val keySchema = List( + KeySchemaElement.builder.attributeName("grouping").keyType(KeyType.HASH).build(), + KeySchemaElement.builder.attributeName("usage_id").keyType(KeyType.RANGE).build() + ) + val provisionedThroughput = ProvisionedThroughput.builder.readCapacityUnits(1L).writeCapacityUnits(1L).build() + + val imageIndex = GlobalSecondaryIndex.builder() + .indexName("media_id") + .keySchema(KeySchemaElement.builder() + .attributeName("media_id") + .keyType(KeyType.HASH).build()) + .projection(Projection.builder().projectionType(ProjectionType.ALL).build()) + .provisionedThroughput(provisionedThroughput) + .build() + + val request = CreateTableRequest.builder + .tableName(usageTable) + .attributeDefinitions(attributeDefinitions.asJava) + .keySchema(keySchema.asJava) + .globalSecondaryIndexes(imageIndex) + .provisionedThroughput(provisionedThroughput) + .build() + dynamoClientV2.createTable(request) + } + + override def afterAll(): Unit = { + super.afterAll() + dynamoContainer.stop() + } + + describe("UsageTable") { + it("should be able to query by image id") { + val imageId1 = "test-image-id-1" + val imageId2 = "test-image-id-2" + + val usageId1 = UsageId(UUID.randomUUID().toString) + val usageId2 = UsageId(UUID.randomUUID().toString) + + val usage1 = MediaUsage( + usageId1, + s"grouping-${usageId1.toString}", + imageId1, + DigitalUsage, + "image", + PendingUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + val usage2 = MediaUsage( + usageId2, + s"grouping-${usageId2.toString}", + imageId2, + DigitalUsage, + "image", + PublishedUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + + val eventualUsage1Created = store.create(usage1)(MarkerMap()).toList.toBlocking.toFuture + val eventualUsage2Created = store.create(usage2)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsage1Created, tenSeconds) + Await.result(eventualUsage2Created, tenSeconds) + + val eventualResult = store.queryByImageId(imageId1)(MarkerMap()) + + whenReady(eventualResult) { result => + result.size should be(1) + result.head.mediaId should be(imageId1) + } + } + + it("should be able to query by usage id") { + val imageId = "test-image-id-for-by-usage-id-test" + val usageId = UsageId(UUID.randomUUID().toString) + val grouping = "some-grouping" + + val usage = MediaUsage( + usageId, + grouping, + imageId, + DigitalUsage, + "image", + PendingUsageStatus, + Some(PrintUsageMetadata( + sectionCode = "a-section", + sectionName = "A section", + pageNumber = 7, + issueDate = DateTime.now, + storyName = "a-story", + publicationCode = "tst", + publicationName = "Test publication", + edition = Some(1) + ) + ), + Some(DigitalUsageMetadata( + webUrl = new URI("http://localhost/test"), + webTitle = "A page", + sectionId = "a-section" + )), + Some(SyndicationUsageMetadata( + partnerName = "Test Partner", + syndicatedBy = Some("test-syndicator") + )), + Some(FrontUsageMetadata( + addedBy = "test-user", + front = "uk/culture" + )), + Some(DownloadUsageMetadata( + downloadedBy = "test-downloader" + )), + Some(ChildUsageMetadata( + addedBy = "Some One", + childMediaId = "some-other-media")), + DateTime.now() + ) + + val eventualUsage1Created = store.create(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsage1Created, tenSeconds) + + val eventualResult = store.queryByUsageId(s"${grouping}_${usageId.id}") + + whenReady(eventualResult) { result => + result.get.usageId should be(usageId) + result.get should be(usage) + } + } + + it("should be able to delete a record") { + val imageId = "test-image-id-for-delete-test" + val usageId = UsageId(UUID.randomUUID().toString) + val grouping = "some-grouping-for-delete" + + val usage = MediaUsage( + usageId, + grouping, + imageId, + DigitalUsage, + "image", + PendingUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + val eventualUsageCreated = store.create(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageCreated, tenSeconds) + val eventualReadbackResult = store.queryByUsageId(s"${grouping}_${usageId.id}") + whenReady(eventualReadbackResult) { result => + result should be(Some(usage)) + } + + store.deleteRecord(usage)(MarkerMap()) + + val eventualReadbackAfterDelete = store.queryByUsageId(s"${grouping}_${usageId.id}") + whenReady(eventualReadbackAfterDelete) { result => + result should be(None) + } + } + + it("should be able to update a record") { + val imageId = "test-image-id-for-update-test" + val usageId = UsageId(UUID.randomUUID().toString) + val grouping = "some-grouping-for-update" + + val usage = MediaUsage( + usageId, + grouping, + imageId, + DigitalUsage, + "image", + PendingUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + val eventualUsageCreated = store.create(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageCreated, tenSeconds) + val updatedUsage = usage.copy(status = PublishedUsageStatus) + + val eventualUsageUpdated = store.update(updatedUsage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageUpdated, tenSeconds) + + val eventualReadbackResult = store.queryByUsageId(s"${grouping}_${usageId.id}") + whenReady(eventualReadbackResult) { result => + result.get.status should be(PublishedUsageStatus) + } + } + + it("should be able to mark a record as removed") { + val imageId = "test-image-id-for-mark-as-removed-test" + val usageId = UsageId(UUID.randomUUID().toString) + val grouping = "some-grouping-for-mark-as-removed" + + val usage = MediaUsage( + usageId, + grouping, + imageId, + DigitalUsage, + "image", + PendingUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + val eventualUsageCreated = store.create(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageCreated, tenSeconds) + + val eventualUsageMarkedAsRemoved = store.markAsRemoved(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageMarkedAsRemoved, tenSeconds) + + val eventualReadbackResult = store.queryByUsageId(s"${grouping}_${usageId.id}") + whenReady(eventualReadbackResult) { result => + result.get.isRemoved should be(true) + } + } + + it("should be able to match a usage group") { + // TODO unclear what this means + val imageId = "test-image-id-for-match-usage-group-test" + val usageId = UsageId(UUID.randomUUID().toString) + val grouping = "some-grouping-for-match-usage-group" + + val usage = MediaUsage( + usageId, + grouping, + imageId, + DigitalUsage, + "image", + PendingUsageStatus, + None, + None, + None, + None, + None, + None, + DateTime.now() + ) + + val eventualUsageCreated = store.create(usage)(MarkerMap()).toList.toBlocking.toFuture + Await.result(eventualUsageCreated, tenSeconds) + + val usageGroup = UsageGroup( + usages = Set(usage), + grouping = grouping, + lastModified = DateTime.now + ) + + implicit val logMarker: MarkerMap = MarkerMap() + val eventualResult = store.matchUsageGroup(WithLogMarker(usageGroup)).toList.toBlocking.toFuture + + whenReady(eventualResult) { result => + result.head.value should be(Set(usage)) + } + } + } +} From 2ea2b0fc6ddd4296303efe43182641814d2e1af2 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 1 Jan 2026 14:09:07 +0000 Subject: [PATCH 18/58] [containerised] Latest sbt 1.11.7 --- project/build.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/build.properties b/project/build.properties index bc7390601f4..01a16ed1465 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1 @@ -sbt.version=1.10.3 +sbt.version=1.11.7 From 7e7e4600d5b2dbf2b5a40c68b6a069eee8028848 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 20 May 2025 18:28:52 +0100 Subject: [PATCH 19/58] [containered] Clean up; CollectionManager private and types. --- .../mediaservice/lib/collections/CollectionsManager.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/collections/CollectionsManager.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/collections/CollectionsManager.scala index 0e3c6bfc2c3..aad2db048ad 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/collections/CollectionsManager.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/collections/CollectionsManager.scala @@ -45,7 +45,7 @@ object CollectionsManager { def isValidPathBit(s: String) = if (s.contains(delimiter) || s.contains(doublequotes)) false else true // These use Source swatches - val collectionColours = Map( + private val collectionColours = Map( "australia" -> "#185E36", "culture" -> "#BB3B80", "film & music" -> "#6B5840", @@ -56,7 +56,7 @@ object CollectionsManager { "travel" -> "#041F4A" ) - def getCollectionColour(s: String) = collectionColours.get(s) + private def getCollectionColour(s: String) = collectionColours.get(s) - def getCssColour(path: List[String]) = path.headOption.map(_.toLowerCase).flatMap(getCollectionColour) + def getCssColour(path: List[String]): Option[String] = path.headOption.map(_.toLowerCase).flatMap(getCollectionColour) } From 54ece51ee84c912c6565767b0dbf4d802b876f3b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 15:45:28 +0100 Subject: [PATCH 20/58] Neuter CloudWatchMetrics; TODO push to config. --- .../com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala index 6a5af78c4f6..6d99a3703d7 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/metrics/CloudWatchMetrics.scala @@ -107,11 +107,13 @@ private class MetricsActor(namespace: String, client: CloudWatchClient) extends .toSeq aggregatedMetrics.grouped(maxGroupSize).foreach(chunkedMetrics => { //can only send max 20 metrics to CW at a time + /* Yeah nah client.putMetricData(PutMetricDataRequest.builder() .namespace(namespace) .metricData(chunkedMetrics.asJava) .build() ) + */ }) logger.info(s"Put ${data.size} metric data points (aggregated to ${aggregatedMetrics.size} points) to namespace $namespace") From 9ddaa266769070d46ae872ede82cdd0dcb1293dd Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 15:59:23 +0100 Subject: [PATCH 21/58] Want to disable Kinises client's noisey CloudWatch emissions. --- thrall/app/lib/ThrallConfig.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index cd331ad9835..38df04e45f6 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -27,7 +27,7 @@ case class KinesisReceiverConfig( override val isDev: Boolean, streamName: String, rewindFrom: Option[DateTime], - metricsLevel: MetricsLevel = MetricsLevel.DETAILED + metricsLevel: MetricsLevel = MetricsLevel.NONE ) extends AwsClientBuilderUtils { lazy val kinesisClient: KinesisAsyncClient = { val clientBuilder = withAWSCredentials(KinesisAsyncClient.builder()) From 73bf5f92cb11fad74ae25e1263ade7372c545663 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 29 May 2024 12:27:59 +0100 Subject: [PATCH 22/58] Play secret from ENV; need to explicitly resolve placeholders. com.typesafe.config.ConfigException$NotResolved: need to Config#resolve() each config before using it, see the API docs for Config#resolve() --- .../com/gu/mediaservice/lib/config/GridConfigLoader.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala index 7484ccf8103..1e027209b95 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/GridConfigLoader.scala @@ -57,7 +57,10 @@ object GridConfigLoader extends StrictLogging { if (file.getPath.endsWith(".properties")) { logger.warn(s"Configuring the Grid with Java properties files is deprecated as of #3011, please switch to .conf files. See #3037 for a conversion utility.") } - Configuration(ConfigFactory.parseFile(file)) + val parsed = ConfigFactory.parseFile(file) + logger.info(s"Resolving config parsed from file: $file") + val resolved = parsed.resolve() + Configuration(resolved) } else { logger.info(s"Skipping config file $file as it doesn't exist") Configuration.empty From b6d98a8e8317475ec9b277d5db1aa1f465bee48d Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 4 May 2024 18:49:21 +0100 Subject: [PATCH 23/58] Initial docker image builds sbt universal, then straight to sbt docker may be the correct path now. Play framework assembly docs do not help with Caused by: java.lang.ClassNotFoundException: play.core.server.ProdServerStart Attempt to build thrall as a fat jar using assembly. Reminder of what Thrall does. --- build.sbt | 35 ++++++++++++++++++++++------------- project/plugins.sbt | 2 -- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/build.sbt b/build.sbt index 8e3f2506865..e2b5e569385 100644 --- a/build.sbt +++ b/build.sbt @@ -4,8 +4,7 @@ import sbt.Package.FixedTimestamp import scala.sys.process._ import scala.util.control.NonFatal import scala.collection.JavaConverters._ - -import com.typesafe.sbt.packager.debian.JDebPackaging +import com.typesafe.sbt.packager.docker._ // We need to keep the timestamps to allow caching headers to work as expected on assets. // The below should work, but some problem in one of the plugins (possible the play plugin? or sbt-web?) causes @@ -243,10 +242,21 @@ val buildInfo = Seq( ) def playProject(projectName: String, port: Int, path: Option[String] = None): Project = { - val commonProject = project(projectName, path) - .enablePlugins(PlayScala, JDebPackaging, SystemdPlugin, BuildInfoPlugin) + project(projectName, path) + .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( + dockerBaseImage := "openjdk:11-jre", + dockerExposedPorts in Docker := Seq(port), + // TODO image-loader specific + dockerCommands ++= Seq( + Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), + Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), + Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick"), + Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick-imagemagick-compat"), + Cmd("RUN", "apt-get", "install", "-y", "pngquant"), + Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") + ), playDefaultPort := port, debianPackageDependencies := Seq("java11-runtime-headless"), Linux / maintainer := "Guardian Developers ", @@ -266,16 +276,15 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", - file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" + file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", + // TODO image-loader specific + file("image-loader/facebook-TINYsRGB_c2.icc") -> "facebook-TINYsRGB_c2.icc", + file("image-loader/grayscale.icc") -> "grayscale.icc", + file("image-loader/srgb.icc") -> "srgb.icc" ), Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", - s"-Dconfig.file=/usr/share/$projectName/conf/application.conf", - s"-Dlogger.file=/usr/share/$projectName/conf/logback.xml", - "-J-Xlog:gc*", - s"-J-Xlog:gc:/var/log/$projectName/gc.log" - ) - )) - //Add the BBC library dependency if defined - maybeBBCLib.fold(commonProject){commonProject.dependsOn(_)} + s"-Dconfig.file=/opt/docker/conf/application.conf", + s"-Dlogger.file=/opt/docker/conf/logback.xml" + ))) } diff --git a/project/plugins.sbt b/project/plugins.sbt index 841571ad763..fc22587b303 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,5 +1,3 @@ -libraryDependencies += "org.vafer" % "jdeb" % "1.3" artifacts (Artifact("jdeb", "jar", "jar")) - addSbtPlugin("org.playframework" % "sbt-plugin" % "3.0.10") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.9.0") From 4a3678fddf94a0ce284bfb10c45141b7d832be60 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 7 May 2024 14:55:02 +0100 Subject: [PATCH 24/58] Fork image loader specific play project. cmyk.icc is required for ingesting CMKY colour space JPEGs. --- build.sbt | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index e2b5e569385..2424b66e9cd 100644 --- a/build.sbt +++ b/build.sbt @@ -144,7 +144,7 @@ lazy val collections = playProject("collections", 9010) lazy val cropper = playProject("cropper", 9006) -lazy val imageLoader = playProject("image-loader", 9003).settings { +lazy val imageLoader = playImageLoaderProject("image-loader", 9003).settings { libraryDependencies ++= Seq( "org.apache.tika" % "tika-core" % "3.2.3", "com.drewnoakes" % "metadata-extractor" % "2.19.0" @@ -248,7 +248,41 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr .settings(commonSettings ++ buildInfo ++ Seq( dockerBaseImage := "openjdk:11-jre", dockerExposedPorts in Docker := Seq(port), - // TODO image-loader specific + playDefaultPort := port, + debianPackageDependencies := Seq("java11-runtime-headless"), + Linux / maintainer := "Guardian Developers ", + Linux / packageSummary := description.value, + packageDescription := description.value, + + bashScriptEnvConfigLocation := Some("/etc/environment"), + Debian / makeEtcDefault := None, + Debian / packageBin := { + val originalFileName = (Debian / packageBin).value + val (base, ext) = originalFileName.baseAndExt + val newBase = base.replace(s"_${version.value}_all","") + val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" + IO.move(originalFileName, newFileName) + println(s"Renamed $originalFileName to $newFileName") + newFileName + }, + Universal / mappings ++= Seq( + file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", + file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" + ), + Universal / javaOptions ++= Seq( + "-Dpidfile.path=/dev/null", + s"-Dconfig.file=/opt/docker/conf/application.conf", + s"-Dlogger.file=/opt/docker/conf/logback.xml" + ))) +} + +def playImageLoaderProject(projectName: String, port: Int, path: Option[String] = None): Project = { + project(projectName, path) + .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) + .dependsOn(restLib) + .settings(commonSettings ++ buildInfo ++ Seq( + dockerBaseImage := "openjdk:11-jre", + dockerExposedPorts in Docker := Seq(port), dockerCommands ++= Seq( Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), @@ -258,17 +292,16 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") ), playDefaultPort := port, - debianPackageDependencies := Seq("java11-runtime-headless"), + debianPackageDependencies := Seq("openjdk-8-jre-headless"), Linux / maintainer := "Guardian Developers ", Linux / packageSummary := description.value, packageDescription := description.value, - bashScriptEnvConfigLocation := Some("/etc/environment"), Debian / makeEtcDefault := None, Debian / packageBin := { val originalFileName = (Debian / packageBin).value val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all","") + val newBase = base.replace(s"_${version.value}_all", "") val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" IO.move(originalFileName, newFileName) println(s"Renamed $originalFileName to $newFileName") @@ -277,7 +310,7 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", - // TODO image-loader specific + file("image-loader/cmyk.icc") -> "cmyk.icc", file("image-loader/facebook-TINYsRGB_c2.icc") -> "facebook-TINYsRGB_c2.icc", file("image-loader/grayscale.icc") -> "grayscale.icc", file("image-loader/srgb.icc") -> "srgb.icc" From 3d537687739a9b6964bb7bd80df54f99753d6cf3 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 9 May 2024 15:25:13 +0100 Subject: [PATCH 25/58] Cropper asks for 'gm' so give it the same Debian packages as image-uploader. --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index 2424b66e9cd..a8dd6c386fb 100644 --- a/build.sbt +++ b/build.sbt @@ -142,7 +142,7 @@ lazy val auth = playProject("auth", 9011) lazy val collections = playProject("collections", 9010) -lazy val cropper = playProject("cropper", 9006) +lazy val cropper = playImageLoaderProject("cropper", 9006) lazy val imageLoader = playImageLoaderProject("image-loader", 9003).settings { libraryDependencies ++= Seq( From 0757d59a3c2b28db995b4bd92f4959d31304aaa5 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 3 May 2025 10:27:53 +0100 Subject: [PATCH 26/58] build.sbt drop Debian package related config which is not needed for image builds. Remove redundant debianPackageDependencies options. --- build.sbt | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/build.sbt b/build.sbt index a8dd6c386fb..f3e49c329ee 100644 --- a/build.sbt +++ b/build.sbt @@ -246,25 +246,11 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( - dockerBaseImage := "openjdk:11-jre", + dockerBaseImage := "eclipse-temurin:11", dockerExposedPorts in Docker := Seq(port), playDefaultPort := port, - debianPackageDependencies := Seq("java11-runtime-headless"), - Linux / maintainer := "Guardian Developers ", - Linux / packageSummary := description.value, - packageDescription := description.value, bashScriptEnvConfigLocation := Some("/etc/environment"), - Debian / makeEtcDefault := None, - Debian / packageBin := { - val originalFileName = (Debian / packageBin).value - val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all","") - val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" - IO.move(originalFileName, newFileName) - println(s"Renamed $originalFileName to $newFileName") - newFileName - }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml" @@ -281,7 +267,7 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( - dockerBaseImage := "openjdk:11-jre", + dockerBaseImage := "eclipse-temurin:11", dockerExposedPorts in Docker := Seq(port), dockerCommands ++= Seq( Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), @@ -292,21 +278,8 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") ), playDefaultPort := port, - debianPackageDependencies := Seq("openjdk-8-jre-headless"), - Linux / maintainer := "Guardian Developers ", - Linux / packageSummary := description.value, - packageDescription := description.value, + bashScriptEnvConfigLocation := Some("/etc/environment"), - Debian / makeEtcDefault := None, - Debian / packageBin := { - val originalFileName = (Debian / packageBin).value - val (base, ext) = originalFileName.baseAndExt - val newBase = base.replace(s"_${version.value}_all", "") - val newFileName = file(originalFileName.getParent) / s"$newBase.$ext" - IO.move(originalFileName, newFileName) - println(s"Renamed $originalFileName to $newFileName") - newFileName - }, Universal / mappings ++= Seq( file("common-lib/src/main/resources/application.conf") -> "conf/application.conf", file("common-lib/src/main/resources/logback.xml") -> "conf/logback.xml", From 3df4bcc2e382e98411e14e0232a56615053c8e6b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 31 Dec 2025 10:24:21 +0000 Subject: [PATCH 27/58] Deprecation: `in` is deprecated; migrate to slash syntax --- build.sbt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/build.sbt b/build.sbt index f3e49c329ee..d2c2281d9b8 100644 --- a/build.sbt +++ b/build.sbt @@ -1,10 +1,10 @@ -import play.sbt.PlayImport.PlayKeys._ +import com.typesafe.sbt.packager.docker.* +import play.sbt.PlayImport.PlayKeys.* import sbt.Package.FixedTimestamp -import scala.sys.process._ +import scala.collection.JavaConverters.* +import scala.sys.process.* import scala.util.control.NonFatal -import scala.collection.JavaConverters._ -import com.typesafe.sbt.packager.docker._ // We need to keep the timestamps to allow caching headers to work as expected on assets. // The below should work, but some problem in one of the plugins (possible the play plugin? or sbt-web?) causes @@ -247,7 +247,7 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( dockerBaseImage := "eclipse-temurin:11", - dockerExposedPorts in Docker := Seq(port), + dockerExposedPorts := Seq(port), playDefaultPort := port, bashScriptEnvConfigLocation := Some("/etc/environment"), @@ -268,7 +268,7 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( dockerBaseImage := "eclipse-temurin:11", - dockerExposedPorts in Docker := Seq(port), + dockerExposedPorts := Seq(port), dockerCommands ++= Seq( Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), From 6f4971524b5bc8ee41109b26d51954be2028a2e2 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 May 2024 11:48:38 +0100 Subject: [PATCH 28/58] Only log to stdout in the containerised world. --- common-lib/src/main/resources/logback.xml | 31 +---------------------- 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/common-lib/src/main/resources/logback.xml b/common-lib/src/main/resources/logback.xml index 733a850c110..47e666377ff 100644 --- a/common-lib/src/main/resources/logback.xml +++ b/common-lib/src/main/resources/logback.xml @@ -2,30 +2,6 @@ - - - - - - - - - - - ${LOGS_LOCATION}/application.log - - - ${LOGS_LOCATION}/application.log.%d{yyyy-MM-dd}.%i.gz - 100MB - 7 - 500MB - - - - %date - [%level] - from %logger in %thread markers=%marker %n%message%n%xException%n - - - @@ -39,12 +15,7 @@ - - - - - - + From d42c29a92761f8aec4f7805452c1b27f00ba2c31 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:06:11 +0100 Subject: [PATCH 29/58] Alter Syndication access check to use URIs not raw host name; allows all api host name to be internalised. Interface only talks about base URIs. --- .../mediaservice/lib/auth/ApiAccessor.scala | 5 +++- .../gu/mediaservice/lib/config/Services.scala | 25 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala index 3e1924451d5..14bd411fa7b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/auth/ApiAccessor.scala @@ -29,6 +29,9 @@ object ApiAccessor extends ArgoHelpers { def hasAccess(apiKey: ApiAccessor, request: RequestHeader, services: Services): Boolean = apiKey.tier match { case Internal => true case ReadOnly => request.method == "GET" - case Syndication => request.method == "GET" && request.host == services.apiHost && request.path.startsWith("/images") + case Syndication => { + val isMediaApiRequest = request.uri.startsWith(services.apiBaseUri) // TODO check this! + request.method == "GET" && isMediaApiRequest && request.path.startsWith("/images") + } } } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 896acf8549f..bf8df4cc66b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -39,19 +39,18 @@ object ServiceHosts { } class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) { - val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" - val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val authHost: String = s"${hosts.authPrefix}$domainRoot" - val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - + private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" + private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" + private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val authHost: String = s"${hosts.authPrefix}$domainRoot" + private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" val kahunaBaseUri = baseUri(kahunaHost) val apiBaseUri = baseUri(apiHost) From accab927f59c01818724c1dd43bcf8bcf6340178 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:20:02 +0100 Subject: [PATCH 30/58] Introduce an interface to document the exposed service uris. Split url Services into a trait and a Guardian specific implementation; exposes a few 4th wall breaking direct init's in services. --- .../lib/config/CommonConfig.scala | 2 +- .../gu/mediaservice/lib/config/Services.scala | 116 ++++++++++++------ image-loader/app/ImageLoaderComponents.scala | 5 +- media-api/app/controllers/MediaApi.scala | 3 +- .../app/controllers/EditsController.scala | 4 +- thrall/app/ThrallComponents.scala | 4 +- 6 files changed, 89 insertions(+), 45 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 60ebefabcf8..a9fed7b7a86 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -89,7 +89,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientBui val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new Services(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) + val services = new GuardianUrlSchemeServices(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index bf8df4cc66b..d5ea11f8ccc 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -1,19 +1,59 @@ package com.gu.mediaservice.lib.config +trait Services { + + def kahunaBaseUri: String + + def apiBaseUri: String + + def loaderBaseUri: String + + def projectionBaseUri: String + + def cropperBaseUri: String + + def metadataBaseUri: String + + def imgopsBaseUri: String + + def usageBaseUri: String + + def collectionsBaseUri: String + + def leasesBaseUri: String + + def authBaseUri: String + + def thrallBaseUri: String + + def allInternalUris: Seq[String] + + def guardianWitnessBaseUri: String + + def corsAllowedDomains: Set[String] + + def redirectUriParam: String + + def redirectUriPlaceholder: String + + def loginUriTemplate: String + +} + case class ServiceHosts( - kahunaPrefix: String, - apiPrefix: String, - loaderPrefix: String, - projectionPrefix: String, - cropperPrefix: String, - metadataPrefix: String, - imgopsPrefix: String, - usagePrefix: String, - collectionsPrefix: String, - leasesPrefix: String, - authPrefix: String, - thrallPrefix: String -) + kahunaPrefix: String, + apiPrefix: String, + loaderPrefix: String, + projectionPrefix: String, + cropperPrefix: String, + metadataPrefix: String, + imgopsPrefix: String, + usagePrefix: String, + collectionsPrefix: String, + leasesPrefix: String, + authPrefix: String, + thrallPrefix: String + ) object ServiceHosts { // this is tightly coupled to the Guardian's deployment. @@ -38,32 +78,32 @@ object ServiceHosts { } } -class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) { - private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" +class GuardianUrlSchemeServices(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { + private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" + private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" + private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val authHost: String = s"${hosts.authPrefix}$domainRoot" - private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - - val kahunaBaseUri = baseUri(kahunaHost) - val apiBaseUri = baseUri(apiHost) - val loaderBaseUri = baseUri(loaderHost) - val projectionBaseUri = baseUri(projectionHost) - val cropperBaseUri = baseUri(cropperHost) - val metadataBaseUri = baseUri(metadataHost) - val imgopsBaseUri = baseUri(imgopsHost) - val usageBaseUri = baseUri(usageHost) + private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val authHost: String = s"${hosts.authPrefix}$domainRoot" + private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" + private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" + + val kahunaBaseUri = baseUri(kahunaHost) + val apiBaseUri = baseUri(apiHost) + val loaderBaseUri = baseUri(loaderHost) + val projectionBaseUri = baseUri(projectionHost) + val cropperBaseUri = baseUri(cropperHost) + val metadataBaseUri = baseUri(metadataHost) + val imgopsBaseUri = baseUri(imgopsHost) + val usageBaseUri = baseUri(usageHost) val collectionsBaseUri = baseUri(collectionsHost) - val leasesBaseUri = baseUri(leasesHost) - val authBaseUri = baseUri(authHost) - val thrallBaseUri = baseUri(thrallHost) + val leasesBaseUri = baseUri(leasesHost) + val authBaseUri = baseUri(authHost) + val thrallBaseUri = baseUri(thrallHost) val allInternalUris = Seq( kahunaBaseUri, @@ -86,5 +126,5 @@ class Services(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: val redirectUriPlaceholder = s"{?$redirectUriParam}" val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - def baseUri(host: String) = s"https://$host" + private def baseUri(host: String) = s"https://$host" } diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index be7748d9525..10c4589e00a 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -1,6 +1,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{Bedrock, S3Vectors, SimpleSqsMessageConsumer, Embedder} -import com.gu.mediaservice.lib.config.Services +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController @@ -21,7 +21,8 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne logger.info(s" $index -> ${processor.description}") } - private val gridClient = GridClient(config.services, config.services.loaderBaseUri)(wsClient) + val services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) + private val gridClient = GridClient(services, config.services.loaderBaseUri)(wsClient) val store = new ImageLoaderStore(config) val maybeIngestQueue = config.maybeIngestSqsQueueUrl.map(queueUrl => new SimpleSqsMessageConsumer(queueUrl, config)) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 1a3017e9495..5f16dd18b96 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -9,6 +9,7 @@ import com.gu.mediaservice.lib.auth.Permissions.{ArchiveImages, DeleteCropsOrUsa import com.gu.mediaservice.lib.auth._ import com.gu.mediaservice.lib.aws._ import com.gu.mediaservice.lib.config.Services +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.formatting.printDateTime import com.gu.mediaservice.lib.logging.{LogMarker, MarkerMap} import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -50,7 +51,7 @@ class MediaApi( embedder: Embedder, )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition { - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services, services.apiBaseUri)(ws) // Process-local cache keyed on normalised query text. Stores the Bedrock Future so that diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 1eb2bdb1c9a..3de8da064da 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -11,7 +11,7 @@ import com.gu.mediaservice.lib.auth.Authentication.Principal import com.gu.mediaservice.lib.auth.Permissions.EditMetadata import com.gu.mediaservice.lib.auth.{Authentication, Authorisation} import com.gu.mediaservice.lib.aws.NoItemFound -import com.gu.mediaservice.lib.config.{ServiceHosts, Services} +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.model._ import com.gu.mediaservice.syntax.MessageSubjects import lib._ @@ -57,7 +57,7 @@ class EditsController( import com.gu.mediaservice.lib.metadata.UsageRightsMetadataMapper.usageRightsToMetadata - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services, services.metadataBaseUri)(ws) val metadataBaseUri = config.services.metadataBaseUri diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index cf9c5e75add..dea47a1273c 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -4,6 +4,8 @@ import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoS import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.config.Services import com.gu.mediaservice.lib.aws.{S3Ops, S3Vectors, ThrallMessageSender} +import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} +import com.gu.mediaservice.lib.aws.{S3Ops, ThrallMessageSender} import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents @@ -30,7 +32,7 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val es = new ElasticSearch(config.esConfig, Some(thrallMetrics), actorSystem.scheduler) es.ensureIndexExistsAndAliasAssigned() - val services: Services = new Services(config.domainRoot, config.serviceHosts, Set.empty) + val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) val gridClient: GridClient = GridClient(services, services.thrallBaseUri)(wsClient) // before firing up anything to consume streams or say we are OK let's do the critical good to go check From 6efa54ea00cc74d1da82786c3aff5b03568f27d5 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 11:38:56 +0100 Subject: [PATCH 31/58] Everyone using GuardianUrlSchemeServices directly should take Services supplied by common config Service trait; no need to trouble yourselves with the details of how those URLs are defined. --- image-loader/app/ImageLoaderComponents.scala | 4 +--- media-api/app/controllers/MediaApi.scala | 6 +----- .../app/controllers/EditsController.scala | 14 ++++---------- thrall/app/ThrallComponents.scala | 10 +++------- 4 files changed, 9 insertions(+), 25 deletions(-) diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index 10c4589e00a..44ddca89c21 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -1,6 +1,5 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{Bedrock, S3Vectors, SimpleSqsMessageConsumer, Embedder} -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController @@ -21,8 +20,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne logger.info(s" $index -> ${processor.description}") } - val services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - private val gridClient = GridClient(services, config.services.loaderBaseUri)(wsClient) + private val gridClient = GridClient(config.services, config.services.loaderBaseUri)(wsClient) val store = new ImageLoaderStore(config) val maybeIngestQueue = config.maybeIngestSqsQueueUrl.map(queueUrl => new SimpleSqsMessageConsumer(queueUrl, config)) diff --git a/media-api/app/controllers/MediaApi.scala b/media-api/app/controllers/MediaApi.scala index 5f16dd18b96..79ee3559f02 100644 --- a/media-api/app/controllers/MediaApi.scala +++ b/media-api/app/controllers/MediaApi.scala @@ -8,8 +8,6 @@ import com.gu.mediaservice.lib.auth.Authentication._ import com.gu.mediaservice.lib.auth.Permissions.{ArchiveImages, DeleteCropsOrUsages, EditMetadata, UploadImages, DeleteImage => DeleteImagePermission} import com.gu.mediaservice.lib.auth._ import com.gu.mediaservice.lib.aws._ -import com.gu.mediaservice.lib.config.Services -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} import com.gu.mediaservice.lib.formatting.printDateTime import com.gu.mediaservice.lib.logging.{LogMarker, MarkerMap} import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable @@ -32,7 +30,6 @@ import play.api.mvc.Security.AuthenticatedRequest import play.api.mvc._ import java.net.URI -import scala.concurrent.duration.DurationInt import scala.concurrent.{ExecutionContext, Future} import scala.util.Try @@ -51,8 +48,7 @@ class MediaApi( embedder: Embedder, )(implicit val ec: ExecutionContext) extends BaseController with MessageSubjects with ArgoHelpers with ContentDisposition { - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services, services.apiBaseUri)(ws) + private val gridClient: GridClient = GridClient(config.services, config.services.apiBaseUri)(ws) // Process-local cache keyed on normalised query text. Stores the Bedrock Future so that // concurrent requests for the same query share a single in-flight Bedrock call, and diff --git a/metadata-editor/app/controllers/EditsController.scala b/metadata-editor/app/controllers/EditsController.scala index 3de8da064da..3d549519d8b 100644 --- a/metadata-editor/app/controllers/EditsController.scala +++ b/metadata-editor/app/controllers/EditsController.scala @@ -1,29 +1,24 @@ package controllers -import java.net.URI -import java.net.URLDecoder.decode import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.argo.ArgoHelpers import com.gu.mediaservice.lib.argo.model._ -import com.gu.mediaservice.lib.aws.DynamoDB import com.gu.mediaservice.lib.auth.Authentication.Principal import com.gu.mediaservice.lib.auth.Permissions.EditMetadata import com.gu.mediaservice.lib.auth.{Authentication, Authorisation} -import com.gu.mediaservice.lib.aws.NoItemFound -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} +import com.gu.mediaservice.lib.aws.{DynamoDB, NoItemFound} import com.gu.mediaservice.model._ import com.gu.mediaservice.syntax.MessageSubjects import lib._ -import lib.Edit -import org.joda.time.DateTime import play.api.libs.json._ import play.api.libs.ws.WSClient import play.api.mvc.{BaseController, ControllerComponents} import software.amazon.awssdk.awscore.exception.AwsServiceException +import java.net.URI +import java.net.URLDecoder.decode import scala.concurrent.{ExecutionContext, Future} -import scala.collection.compat._ // FIXME: the argoHelpers are all returning `Ok`s (200) @@ -57,8 +52,7 @@ class EditsController( import com.gu.mediaservice.lib.metadata.UsageRightsMetadataMapper.usageRightsToMetadata - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services, services.metadataBaseUri)(ws) + private val gridClient: GridClient = GridClient(config.services, config.services.metadataBaseUri)(ws) val metadataBaseUri = config.services.metadataBaseUri private val AuthenticatedAndAuthorised = auth andThen authorisation.CommonActionFilters.authorisedForArchive diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index dea47a1273c..634fbbeb94b 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -1,11 +1,6 @@ -import org.apache.pekko.Done -import org.apache.pekko.stream.scaladsl.Source import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoStreamConfig} import com.gu.mediaservice.GridClient -import com.gu.mediaservice.lib.config.Services import com.gu.mediaservice.lib.aws.{S3Ops, S3Vectors, ThrallMessageSender} -import com.gu.mediaservice.lib.config.{GuardianUrlSchemeServices, Services} -import com.gu.mediaservice.lib.aws.{S3Ops, ThrallMessageSender} import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents @@ -14,6 +9,8 @@ import controllers.{AssetsComponents, HealthCheck, ReaperController, ThrallContr import lib._ import lib.elasticsearch._ import lib.kinesis.{KinesisConfig, ThrallEventConsumer} +import org.apache.pekko.Done +import org.apache.pekko.stream.scaladsl.Source import play.api.ApplicationLoader.Context import router.Routes @@ -32,8 +29,7 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val es = new ElasticSearch(config.esConfig, Some(thrallMetrics), actorSystem.scheduler) es.ensureIndexExistsAndAliasAssigned() - val services: Services = new GuardianUrlSchemeServices(config.domainRoot, config.serviceHosts, Set.empty) - val gridClient: GridClient = GridClient(services, services.thrallBaseUri)(wsClient) + val gridClient: GridClient = GridClient(config.services, config.services.thrallBaseUri)(wsClient) // before firing up anything to consume streams or say we are OK let's do the critical good to go check private val goodToGoCheckResult = Await.ready(GoodToGoCheck.run(es), 30 seconds) From f0073a72e905fb6aba6ece5634ca37273308bd7a Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 12:14:25 +0100 Subject: [PATCH 32/58] Drop GuardianUrlSchemeServices val constructor fields; these allow undocumented access to the private url building concerns. --- .../main/scala/com/gu/mediaservice/lib/config/Services.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index d5ea11f8ccc..776a237d4c8 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -78,7 +78,7 @@ object ServiceHosts { } } -class GuardianUrlSchemeServices(val domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { +class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" From 0daf5af7b15bfece58ef9ec35cadeba73456a651 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 8 May 2024 12:02:16 +0100 Subject: [PATCH 33/58] Add a Service URL implementation which map services to port numbers on the single hostname. Will work because HTTPS auth is not active. CORS for single host urls. Projection end points are on the image-loader service but have seperate config to permit reingession workloads to be on different instances. --- .../lib/config/CommonConfig.scala | 2 +- .../gu/mediaservice/lib/config/Services.scala | 53 ++++++++++++++++++- .../lib/play/GridComponents.scala | 4 +- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index a9fed7b7a86..1b5af04cbd2 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -89,7 +89,7 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientBui val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new GuardianUrlSchemeServices(domainRoot, serviceHosts, corsAllowedOrigins, domainRootOverride) + val services = new SingleHostServices("york.local", 32400) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 776a237d4c8..f54ee7cc881 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -78,7 +78,58 @@ object ServiceHosts { } } -class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { +protected class SingleHostServices(val hostname: String, baseport: Int) extends Services { + val kahunaBaseUri: String = baseUri(hostname, baseport + 20) + + val apiBaseUri: String = baseUri(hostname, baseport + 1) + + val loaderBaseUri: String = baseUri(hostname, baseport + 3) + + val projectionBaseUri: String = loaderBaseUri + + val cropperBaseUri: String = baseUri(hostname, baseport + 6) + + val metadataBaseUri: String = baseUri(hostname, baseport + 7) + + val imgopsBaseUri: String = baseUri(hostname, baseport + 8) + + val usageBaseUri: String = baseUri(hostname, baseport + 9) + + val collectionsBaseUri: String = baseUri(hostname, baseport + 10) + + val leasesBaseUri: String = baseUri(hostname, baseport + 12) + + val authBaseUri: String = baseUri(hostname, baseport + 11) + + val thrallBaseUri: String = baseUri(hostname, baseport + 200) + + val allInternalUris: Seq[String] = Seq( + kahunaBaseUri, + apiBaseUri, + loaderBaseUri, + cropperBaseUri, + metadataBaseUri, + usageBaseUri, + collectionsBaseUri, + leasesBaseUri, + authBaseUri, + thrallBaseUri + ) + + val guardianWitnessBaseUri: String = "https://n0ticeapis.com" + + val corsAllowedDomains: Set[String] = Set(kahunaBaseUri, apiBaseUri, thrallBaseUri) + + val redirectUriParam = "redirectUri" + val redirectUriPlaceholder = s"{?$redirectUriParam}" + val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" + + + private def baseUri(host: String, port: Int) = s"http://$host:$port" + +} + +protected class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala index 354613905d5..fd9aca26911 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala @@ -39,8 +39,8 @@ abstract class GridComponents[Config <: CommonConfig](context: Context, val load ) final override lazy val corsConfig: CORSConfig = CORSConfig.fromConfiguration(context.initialConfiguration).copy( - allowedOrigins = Origins.Matching(config.services.corsAllowedDomains) - ) + allowedOrigins = Origins.Matching(config.services.corsAllowedDomains) + ) lazy val management = new Management(controllerComponents, buildInfo) From ce45f577ef66b4cdf6205039fb5f34081245f890 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Mon, 3 Jun 2024 14:50:15 +0100 Subject: [PATCH 34/58] Drop Guardian services URL scheme. We have used it to shape the Service interface. It can be dropped now. --- .../lib/config/CommonConfig.scala | 14 --- .../gu/mediaservice/lib/config/Services.scala | 90 +------------------ 2 files changed, 1 insertion(+), 103 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index 1b5af04cbd2..f6ee30b676b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -72,20 +72,6 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientBui val domainRoot: String = string("domain.root") val domainRootOverride: Option[String] = stringOpt("domain.root-override") val rootAppName: String = stringDefault("app.name.root", "media") - val serviceHosts = ServiceHosts( - stringDefault("hosts.kahunaPrefix", s"$rootAppName."), - stringDefault("hosts.apiPrefix", s"api.$rootAppName."), - stringDefault("hosts.loaderPrefix", s"loader.$rootAppName."), - stringDefault("hosts.projectionPrefix", s"loader-projection.$rootAppName."), - stringDefault("hosts.cropperPrefix", s"cropper.$rootAppName."), - stringDefault("hosts.metadataPrefix", s"$rootAppName-metadata."), - stringDefault("hosts.imgopsPrefix", s"$rootAppName-imgops."), - stringDefault("hosts.usagePrefix", s"$rootAppName-usage."), - stringDefault("hosts.collectionsPrefix", s"$rootAppName-collections."), - stringDefault("hosts.leasesPrefix", s"$rootAppName-leases."), - stringDefault("hosts.authPrefix", s"$rootAppName-auth."), - stringDefault("hosts.thrallPrefix", s"thrall.$rootAppName.") - ) val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index f54ee7cc881..5dec784da91 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -40,45 +40,7 @@ trait Services { } -case class ServiceHosts( - kahunaPrefix: String, - apiPrefix: String, - loaderPrefix: String, - projectionPrefix: String, - cropperPrefix: String, - metadataPrefix: String, - imgopsPrefix: String, - usagePrefix: String, - collectionsPrefix: String, - leasesPrefix: String, - authPrefix: String, - thrallPrefix: String - ) - -object ServiceHosts { - // this is tightly coupled to the Guardian's deployment. - // TODO make more generic but w/out relying on Play config - def guardianPrefixes: ServiceHosts = { - val rootAppName: String = "media" - - ServiceHosts( - kahunaPrefix = s"$rootAppName.", - apiPrefix = s"api.$rootAppName.", - loaderPrefix = s"loader.$rootAppName.", - projectionPrefix = s"loader-projection.$rootAppName", - cropperPrefix = s"cropper.$rootAppName.", - metadataPrefix = s"$rootAppName-metadata.", - imgopsPrefix = s"$rootAppName-imgops.", - usagePrefix = s"$rootAppName-usage.", - collectionsPrefix = s"$rootAppName-collections.", - leasesPrefix = s"$rootAppName-leases.", - authPrefix = s"$rootAppName-auth.", - thrallPrefix = s"thrall.$rootAppName." - ) - } -} - -protected class SingleHostServices(val hostname: String, baseport: Int) extends Services { +protected class SingleHostServices(val hostname: String, val baseport: Int) extends Services { val kahunaBaseUri: String = baseUri(hostname, baseport + 20) val apiBaseUri: String = baseUri(hostname, baseport + 1) @@ -129,53 +91,3 @@ protected class SingleHostServices(val hostname: String, baseport: Int) extends } -protected class GuardianUrlSchemeServices(domainRoot: String, hosts: ServiceHosts, corsAllowedOrigins: Set[String], domainRootOverride: Option[String] = None) extends Services { - private val kahunaHost: String = s"${hosts.kahunaPrefix}$domainRoot" - private val apiHost: String = s"${hosts.apiPrefix}$domainRoot" - private val loaderHost: String = s"${hosts.loaderPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val cropperHost: String = s"${hosts.cropperPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val metadataHost: String = s"${hosts.metadataPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val imgopsHost: String = s"${hosts.imgopsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val usageHost: String = s"${hosts.usagePrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val collectionsHost: String = s"${hosts.collectionsPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val leasesHost: String = s"${hosts.leasesPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val authHost: String = s"${hosts.authPrefix}$domainRoot" - private val projectionHost: String = s"${hosts.projectionPrefix}${domainRootOverride.getOrElse(domainRoot)}" - private val thrallHost: String = s"${hosts.thrallPrefix}${domainRootOverride.getOrElse(domainRoot)}" - - val kahunaBaseUri = baseUri(kahunaHost) - val apiBaseUri = baseUri(apiHost) - val loaderBaseUri = baseUri(loaderHost) - val projectionBaseUri = baseUri(projectionHost) - val cropperBaseUri = baseUri(cropperHost) - val metadataBaseUri = baseUri(metadataHost) - val imgopsBaseUri = baseUri(imgopsHost) - val usageBaseUri = baseUri(usageHost) - val collectionsBaseUri = baseUri(collectionsHost) - val leasesBaseUri = baseUri(leasesHost) - val authBaseUri = baseUri(authHost) - val thrallBaseUri = baseUri(thrallHost) - - val allInternalUris = Seq( - kahunaBaseUri, - apiBaseUri, - loaderBaseUri, - cropperBaseUri, - metadataBaseUri, - usageBaseUri, - collectionsBaseUri, - leasesBaseUri, - authBaseUri, - thrallBaseUri - ) - - val guardianWitnessBaseUri: String = "https://n0ticeapis.com" - - val corsAllowedDomains: Set[String] = corsAllowedOrigins.map(baseUri) + kahunaBaseUri + apiBaseUri + thrallBaseUri - - val redirectUriParam = "redirectUri" - val redirectUriPlaceholder = s"{?$redirectUriParam}" - val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - - private def baseUri(host: String) = s"https://$host" -} From 43eba605c8723fcc622db05bb0a5defaf9d76129 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Fri, 10 May 2024 16:07:30 +0100 Subject: [PATCH 35/58] Move all public facing service urls to sub paths under single hostname. Config single.host.url is exclusively for our single host setup. --- .../lib/config/CommonConfig.scala | 3 +- .../gu/mediaservice/lib/config/Services.scala | 28 +++++++++---------- .../src/test/resources/application.conf | 1 + .../test/lib/elasticsearch/Fixtures.scala | 1 + 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala index f6ee30b676b..eca1152ad26 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/CommonConfig.scala @@ -75,7 +75,8 @@ abstract class CommonConfig(resources: GridConfigResources) extends AwsClientBui val corsAllowedOrigins: Set[String] = getStringSet("security.cors.allowedOrigins") - val services = new SingleHostServices("york.local", 32400) + private val singleHostUrl: String = string("single.host.url") + val services = new SingleHostServices(singleHostUrl) /** * Load in a list of domain metadata specifications from configuration. For example: diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 5dec784da91..4b38737e787 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -40,30 +40,30 @@ trait Services { } -protected class SingleHostServices(val hostname: String, val baseport: Int) extends Services { - val kahunaBaseUri: String = baseUri(hostname, baseport + 20) +protected class SingleHostServices(val rootUrl: String) extends Services { + val kahunaBaseUri: String = rootUrl - val apiBaseUri: String = baseUri(hostname, baseport + 1) + val apiBaseUri: String = subpathedServiceBaseUri("media-api") - val loaderBaseUri: String = baseUri(hostname, baseport + 3) + val loaderBaseUri: String = subpathedServiceBaseUri("image-loader") val projectionBaseUri: String = loaderBaseUri - val cropperBaseUri: String = baseUri(hostname, baseport + 6) + val cropperBaseUri: String = subpathedServiceBaseUri("cropper") - val metadataBaseUri: String = baseUri(hostname, baseport + 7) + val metadataBaseUri: String = subpathedServiceBaseUri("metadata-editor") - val imgopsBaseUri: String = baseUri(hostname, baseport + 8) + val imgopsBaseUri: String = subpathedServiceBaseUri("imgops") - val usageBaseUri: String = baseUri(hostname, baseport + 9) + val usageBaseUri: String =subpathedServiceBaseUri("usage") - val collectionsBaseUri: String = baseUri(hostname, baseport + 10) + val collectionsBaseUri: String = subpathedServiceBaseUri("collections") - val leasesBaseUri: String = baseUri(hostname, baseport + 12) + val leasesBaseUri: String = subpathedServiceBaseUri("leases") - val authBaseUri: String = baseUri(hostname, baseport + 11) + val authBaseUri: String = subpathedServiceBaseUri("auth") - val thrallBaseUri: String = baseUri(hostname, baseport + 200) + val thrallBaseUri: String = subpathedServiceBaseUri("thrall") val allInternalUris: Seq[String] = Seq( kahunaBaseUri, @@ -86,8 +86,6 @@ protected class SingleHostServices(val hostname: String, val baseport: Int) exte val redirectUriPlaceholder = s"{?$redirectUriParam}" val loginUriTemplate = s"$authBaseUri/login$redirectUriPlaceholder" - - private def baseUri(host: String, port: Int) = s"http://$host:$port" - + private def subpathedServiceBaseUri(serviceName: String): String = s"$rootUrl/$serviceName" } diff --git a/common-lib/src/test/resources/application.conf b/common-lib/src/test/resources/application.conf index 962a7678896..fa78ccd9211 100644 --- a/common-lib/src/test/resources/application.conf +++ b/common-lib/src/test/resources/application.conf @@ -3,6 +3,7 @@ grid.appName: "test" thrall.kinesis.stream.name: "not-used" thrall.kinesis.lowPriorityStream.name: "not-used" domain.root: "notused.example.com" +single.host.url: "notused.example.com" image.processors = [ "com.gu.mediaservice.lib.cleanup.GuardianMetadataCleaners", diff --git a/media-api/test/lib/elasticsearch/Fixtures.scala b/media-api/test/lib/elasticsearch/Fixtures.scala index 57fe3c005de..c484421c3be 100644 --- a/media-api/test/lib/elasticsearch/Fixtures.scala +++ b/media-api/test/lib/elasticsearch/Fixtures.scala @@ -30,6 +30,7 @@ trait Fixtures { "thrall.kinesis.stream.name", "thrall.kinesis.lowPriorityStream.name", "domain.root", + "single.host.url", "s3.config.bucket", "s3.usagemail.bucket", "quota.store.key", From bff26a89c32c80a7221bc2912cc20b876eb00c70 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 14 May 2024 08:26:35 +0100 Subject: [PATCH 36/58] Disable CSRF with is no longer bypassed on a single origin CORS check. No longer gets bypassed thanks to preceding CORS check; CORS filter does not appear to tag the request if it passes for same origin. --- .../scala/com/gu/mediaservice/lib/play/GridComponents.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala index fd9aca26911..e272581a208 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/play/GridComponents.scala @@ -30,7 +30,7 @@ abstract class GridComponents[Config <: CommonConfig](context: Context, val load final override def httpFilters: Seq[EssentialFilter] = Seq( corsFilter, - csrfFilter, + //csrfFilter TODO no longer gets bypassed thanks to preceding CORS check; CORS filter does not appear to tag the request if it passes for same origin. securityHeadersFilter, gzipFilter, new RequestLoggingFilter(materializer), From dc0a3454ffd52c8f848bb6b4af9185b145ca2787 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 11 Feb 2026 21:38:36 +0000 Subject: [PATCH 37/58] [containerised] Download crop asset links click through correctly when media-api is no longer on a different host. --- .../public/js/components/gr-display-crops/gr-display-crops.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kahuna/public/js/components/gr-display-crops/gr-display-crops.html b/kahuna/public/js/components/gr-display-crops/gr-display-crops.html index 80db07d7aab..3bcd3ac4212 100644 --- a/kahuna/public/js/components/gr-display-crops/gr-display-crops.html +++ b/kahuna/public/js/components/gr-display-crops/gr-display-crops.html @@ -19,7 +19,7 @@ From 02900f9ad2be61f165d19f4b702fc64fff01c269 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 12:20:52 +0100 Subject: [PATCH 38/58] Delete InnerServiceStatusCheckController "checks connectivity to all other internal services..." which sounds like something we can let the container orchestrator handle. InnerServiceStatusCheckController was the only user of Services.allInternalUris # Conflicts: # metadata-editor/app/MetadataEditorComponents.scala --- auth/app/auth/AuthComponents.scala | 5 +- auth/conf/routes | 1 - collections/app/CollectionsComponents.scala | 4 +- collections/conf/routes | 1 - .../gu/mediaservice/lib/config/Services.scala | 15 ----- cropper/app/CropperComponents.scala | 5 +- cropper/conf/routes | 1 - image-loader/app/ImageLoaderComponents.scala | 4 +- image-loader/conf/routes | 1 - kahuna/app/KahunaComponents.scala | 4 +- kahuna/conf/routes | 1 - leases/app/LeasesComponents.scala | 4 +- leases/conf/routes | 1 - media-api/app/MediaApiComponents.scala | 8 +-- media-api/conf/routes | 1 - .../app/MetadataEditorComponents.scala | 4 +- metadata-editor/conf/routes | 1 - .../InnerServiceStatusCheckController.scala | 67 ------------------- thrall/app/ThrallComponents.scala | 4 +- thrall/conf/routes | 2 - usage/app/UsageComponents.scala | 4 +- usage/conf/routes | 1 - 22 files changed, 14 insertions(+), 125 deletions(-) diff --git a/auth/app/auth/AuthComponents.scala b/auth/app/auth/AuthComponents.scala index 1077bb81186..0882a1086ac 100644 --- a/auth/app/auth/AuthComponents.scala +++ b/auth/app/auth/AuthComponents.scala @@ -1,6 +1,6 @@ package auth -import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.management.Management import com.gu.mediaservice.lib.play.GridComponents import play.api.ApplicationLoader.Context import play.api.{Configuration, Environment} @@ -14,10 +14,9 @@ class AuthComponents(context: Context) extends GridComponents(context, new AuthC val controller = new AuthController(auth, providers, config, controllerComponents, authorisation) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement) } object AuthHttpConfig { diff --git a/auth/conf/routes b/auth/conf/routes index 9a58260935c..4423cc15e99 100644 --- a/auth/conf/routes +++ b/auth/conf/routes @@ -16,7 +16,6 @@ GET /cookieMonster auth.AuthController.cookieMonster # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/collections/app/CollectionsComponents.scala b/collections/app/CollectionsComponents.scala index 2393aca70e2..116fc2e5c47 100644 --- a/collections/app/CollectionsComponents.scala +++ b/collections/app/CollectionsComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{CollectionsController, ImageCollectionsController} import lib.{CollectionsConfig, CollectionsMetrics, Notifications} @@ -17,8 +16,7 @@ class CollectionsComponents(context: Context) extends GridComponents(context, ne val collections = new CollectionsController(auth, config, collectionsStore, controllerComponents) val imageCollections = new ImageCollectionsController(auth, config, notifications, imageCollectionsStore, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, collections, imageCollections, management, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, collections, imageCollections, management) } diff --git a/collections/conf/routes b/collections/conf/routes index 21acc8e1243..8fd9d4424dc 100644 --- a/collections/conf/routes +++ b/collections/conf/routes @@ -16,7 +16,6 @@ POST /corrected-collections controllers.CollectionsC # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index 4b38737e787..b65c4bfee32 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -26,8 +26,6 @@ trait Services { def thrallBaseUri: String - def allInternalUris: Seq[String] - def guardianWitnessBaseUri: String def corsAllowedDomains: Set[String] @@ -65,19 +63,6 @@ protected class SingleHostServices(val rootUrl: String) extends Services { val thrallBaseUri: String = subpathedServiceBaseUri("thrall") - val allInternalUris: Seq[String] = Seq( - kahunaBaseUri, - apiBaseUri, - loaderBaseUri, - cropperBaseUri, - metadataBaseUri, - usageBaseUri, - collectionsBaseUri, - leasesBaseUri, - authBaseUri, - thrallBaseUri - ) - val guardianWitnessBaseUri: String = "https://n0ticeapis.com" val corsAllowedDomains: Set[String] = Set(kahunaBaseUri, apiBaseUri, thrallBaseUri) diff --git a/cropper/app/CropperComponents.scala b/cropper/app/CropperComponents.scala index 5a9d790ae2f..291049feeaa 100644 --- a/cropper/app/CropperComponents.scala +++ b/cropper/app/CropperComponents.scala @@ -1,6 +1,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.imaging.ImageOperations -import com.gu.mediaservice.lib.management.{InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.management.Management import com.gu.mediaservice.lib.play.GridComponents import controllers.CropperController import lib.{CropStore, CropperConfig, Crops, Notifications} @@ -20,8 +20,7 @@ class CropperComponents(context: Context) extends GridComponents(context, new Cr val controller = new CropperController(auth, crops, store, notifications, config, controllerComponents, authorisation, gridClient) val permissionsAwareManagement = new Management(controllerComponents, buildInfo) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, permissionsAwareManagement) } diff --git a/cropper/conf/routes b/cropper/conf/routes index c4aad4e4ee0..207c5df4b17 100644 --- a/cropper/conf/routes +++ b/cropper/conf/routes @@ -8,7 +8,6 @@ DELETE /crops/:id controllers.CropperControl # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/image-loader/app/ImageLoaderComponents.scala b/image-loader/app/ImageLoaderComponents.scala index 44ddca89c21..c5d088a8309 100644 --- a/image-loader/app/ImageLoaderComponents.scala +++ b/image-loader/app/ImageLoaderComponents.scala @@ -2,7 +2,6 @@ import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{Bedrock, S3Vectors, SimpleSqsMessageConsumer, Embedder} import com.gu.mediaservice.lib.imaging.ImageOperations import com.gu.mediaservice.lib.logging.GridLogging -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{ImageLoaderController, ImageLoaderManagement, UploadStatusController} import lib._ @@ -46,8 +45,7 @@ class ImageLoaderComponents(context: Context) extends GridComponents(context, ne val controller = new ImageLoaderController( auth, downloader, store, maybeIngestQueue, uploadStatusTable, notifications, config, uploader, quarantineUploader, projector, controllerComponents, gridClient, authorisation, metrics, applicationLifecycle) val uploadStatusController = new UploadStatusController(auth, uploadStatusTable, config, controllerComponents, authorisation) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) val imageLoaderManagement = new ImageLoaderManagement(controllerComponents, buildInfo, controller.maybeIngestQueueAndProcessor) - override lazy val router = new Routes(httpErrorHandler, controller, uploadStatusController, imageLoaderManagement, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, uploadStatusController, imageLoaderManagement) } diff --git a/image-loader/conf/routes b/image-loader/conf/routes index 83a4684e970..5dc429f8e7b 100644 --- a/image-loader/conf/routes +++ b/image-loader/conf/routes @@ -15,7 +15,6 @@ GET /uploadStatuses/:userId controllers.UploadStatusCo # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/kahuna/app/KahunaComponents.scala b/kahuna/app/KahunaComponents.scala index 8a10d6e6674..9c179e485c5 100644 --- a/kahuna/app/KahunaComponents.scala +++ b/kahuna/app/KahunaComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.net.URI import com.gu.mediaservice.lib.play.GridComponents import controllers.{AssetsComponents, KahunaController} @@ -14,9 +13,8 @@ class KahunaComponents(context: Context) extends GridComponents(context, new Kah final override val buildInfo = utils.buildinfo.BuildInfo val controller = new KahunaController(auth, config, controllerComponents, authorisation) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - final override val router = new Routes(httpErrorHandler, controller, assets, management, InnerServiceStatusCheckController) + final override val router = new Routes(httpErrorHandler, controller, assets, management) } diff --git a/kahuna/conf/routes b/kahuna/conf/routes index b8ad420d3c4..3003723ff03 100644 --- a/kahuna/conf/routes +++ b/kahuna/conf/routes @@ -18,7 +18,6 @@ GET /assets/*file controllers.Assets.version # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/leases/app/LeasesComponents.scala b/leases/app/LeasesComponents.scala index 1a1a005f058..6d09e4076c3 100644 --- a/leases/app/LeasesComponents.scala +++ b/leases/app/LeasesComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.MediaLeaseController import lib.{LeaseNotifier, LeaseStore, LeasesConfig} @@ -13,8 +12,7 @@ class LeasesComponents(context: Context) extends GridComponents(context, new Lea val notifications = new LeaseNotifier(config, store) val controller = new MediaLeaseController(auth, store, config, notifications, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, management, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, management) } diff --git a/leases/conf/routes b/leases/conf/routes index f5849a960db..2402837822a 100644 --- a/leases/conf/routes +++ b/leases/conf/routes @@ -15,7 +15,6 @@ POST /leases controllers.MediaLeaseCont # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away diff --git a/media-api/app/MediaApiComponents.scala b/media-api/app/MediaApiComponents.scala index dd7a1e99618..d17930c4fa7 100644 --- a/media-api/app/MediaApiComponents.scala +++ b/media-api/app/MediaApiComponents.scala @@ -1,5 +1,5 @@ -import com.gu.mediaservice.lib.aws.{Bedrock, Embedder, S3, S3Vectors, SimpleSqsMessageConsumer, ThrallMessageSender} -import com.gu.mediaservice.lib.management.{ElasticSearchHealthCheck, InnerServiceStatusCheckController, Management} +import com.gu.mediaservice.lib.aws._ +import com.gu.mediaservice.lib.management.{ElasticSearchHealthCheck, Management} import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents import controllers._ @@ -38,7 +38,6 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M val elasticSearchHealthCheck = new ElasticSearchHealthCheck(controllerComponents, elasticSearch) val healthcheckController = new Management(controllerComponents, buildInfo) val configurationController = new ConfigurationController(controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) override val router = new Routes( httpErrorHandler, @@ -48,7 +47,6 @@ class MediaApiComponents(context: Context) extends GridComponents(context, new M usageController, configurationController, elasticSearchHealthCheck, - healthcheckController, - InnerServiceStatusCheckController + healthcheckController ) } diff --git a/media-api/conf/routes b/media-api/conf/routes index 30f5196932f..9e84a5d8422 100644 --- a/media-api/conf/routes +++ b/media-api/conf/routes @@ -46,7 +46,6 @@ GET /configuration/crop-variations controllers. GET /management/healthcheck com.gu.mediaservice.lib.management.ElasticSearchHealthCheck.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest GET /management/imageCounts com.gu.mediaservice.lib.management.ElasticSearchHealthCheck.imageCounts -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/metadata-editor/app/MetadataEditorComponents.scala b/metadata-editor/app/MetadataEditorComponents.scala index f3882cf39fa..fe655df194b 100644 --- a/metadata-editor/app/MetadataEditorComponents.scala +++ b/metadata-editor/app/MetadataEditorComponents.scala @@ -1,4 +1,3 @@ -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.{EditsApi, EditsController, SyndicationController} import lib._ @@ -24,10 +23,9 @@ class MetadataEditorComponents(context: Context) extends GridComponents(context, val editsController = new EditsController(auth, editsStore, notifications, config, wsClient, authorisation, controllerComponents) val syndicationController = new SyndicationController(auth, editsStore, syndicationStore, notifications, config, controllerComponents) val controller = new EditsApi(auth, config, authorisation, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override val router = new Routes(httpErrorHandler, controller, editsController, syndicationController, management, InnerServiceStatusCheckController) + override val router = new Routes(httpErrorHandler, controller, editsController, syndicationController, management) } diff --git a/metadata-editor/conf/routes b/metadata-editor/conf/routes index 0b12d2f0e02..5aab6e425f8 100644 --- a/metadata-editor/conf/routes +++ b/metadata-editor/conf/routes @@ -33,7 +33,6 @@ DELETE /metadata/:id/syndication controllers.SyndicationC # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots diff --git a/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala b/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala index e313e238ba6..e69de29bb2d 100644 --- a/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala +++ b/rest-lib/src/main/scala/com/gu/mediaservice/lib/management/InnerServiceStatusCheckController.scala @@ -1,67 +0,0 @@ -package com.gu.mediaservice.lib.management - -import com.gu.mediaservice.lib.argo.ArgoHelpers -import com.gu.mediaservice.lib.auth.Authentication -import com.gu.mediaservice.lib.auth.Authentication.InnerServicePrincipal -import com.gu.mediaservice.lib.config.Services -import play.api.libs.json.{JsString, JsValue, Json, Writes} -import play.api.libs.ws.{WSClient, WSRequest} -import play.api.mvc.{BaseController, ControllerComponents} - -import scala.concurrent.{ExecutionContext, Future} -import scala.util.Try - -case class WhoAmIResponse (baseUri: String, status: Int, body: JsValue) -object WhoAmIResponse { implicit val writes: Writes[WhoAmIResponse] = Json.writes[WhoAmIResponse] } - -class InnerServiceStatusCheckController( - auth: Authentication, - override val controllerComponents: ControllerComponents, - services: Services, - ws: WSClient -)(implicit ec: ExecutionContext) - extends BaseController with ArgoHelpers { - - private def safeJsonParse(maybeJsonStr: String) = Try(Json.parse(maybeJsonStr)).getOrElse(JsString(maybeJsonStr)) - - private def callAllInternalServices(depth: Int, authenticator: WSRequest => WSRequest) = { - val nextDepth = depth - 1 - val whoAmIFutures = services.allInternalUris.map { baseUri => - authenticator(ws.url(s"$baseUri/management/whoAmI").addQueryStringParameters("depth" -> nextDepth.toString)).get() - .map(resp => WhoAmIResponse(baseUri, resp.status, safeJsonParse(resp.body))) - .recover{ - case throwable: Throwable => WhoAmIResponse(baseUri, SERVICE_UNAVAILABLE, Json.obj( - "errorMessage" -> throwable.getMessage, - "stackTrace" -> throwable.getStackTrace.map(_.toString) - ))} - } - - Future.sequence(whoAmIFutures).map { whoAmIResponses => - val overallStatus = whoAmIResponses.map(_.status).max - new Status(overallStatus)(Json.toJson(whoAmIResponses.map(resp => resp.baseUri -> resp).toMap)) - } - } - - def whoAmI(depth: Int) = auth.async { request => - if (depth < 0 || depth > 2) { Future.successful(BadRequest("'depth' query param must be at least 0 and no more than 2"))} - else request.user match { - case principal: InnerServicePrincipal if depth > 0 => - callAllInternalServices( - depth, - authenticator = auth.getOnBehalfOfPrincipal(principal) - ) - case _ => - Future.successful( - Ok(Json.toJson(request.user.toString)) - ) - } - } - - def statusCheck(depth: Int) = Action.async { - if (depth < 1 || depth > 3) { Future.successful(BadRequest("'depth' query param must be at least 1 and no more than 3"))} - else callAllInternalServices( - depth, - authenticator = auth.innerServiceCall - ) - } -} diff --git a/thrall/app/ThrallComponents.scala b/thrall/app/ThrallComponents.scala index 634fbbeb94b..01fcbee1038 100644 --- a/thrall/app/ThrallComponents.scala +++ b/thrall/app/ThrallComponents.scala @@ -1,7 +1,6 @@ import com.gu.kinesis.{KinesisRecord, KinesisSource, ConsumerConfig => KclPekkoStreamConfig} import com.gu.mediaservice.GridClient import com.gu.mediaservice.lib.aws.{S3Ops, S3Vectors, ThrallMessageSender} -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable import com.gu.mediaservice.lib.play.GridComponents import com.typesafe.scalalogging.StrictLogging @@ -87,7 +86,6 @@ class ThrallComponents(context: Context) extends GridComponents(context, new Thr val thrallController = new ThrallController(es, store, migrationSourceWithSender.send, messageSender, actorSystem, auth, config.services, controllerComponents, gridClient) val reaperController = new ReaperController(es, store, s3Vectors, authorisation, config, actorSystem.scheduler, maybeCustomReapableEligibility, softDeletedMetadataTable, thrallMetrics, auth, config.services, controllerComponents) val healthCheckController = new HealthCheck(es, streamRunning.isCompleted, config, controllerComponents) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, thrallController, reaperController, healthCheckController, management, InnerServiceStatusCheckController, assets) + override lazy val router = new Routes(httpErrorHandler, thrallController, reaperController, healthCheckController, management, assets) } diff --git a/thrall/conf/routes b/thrall/conf/routes index 977f323ab57..acb0c32de56 100644 --- a/thrall/conf/routes +++ b/thrall/conf/routes @@ -29,8 +29,6 @@ POST /resumeReaper controllers.ReaperControll # Management GET /management/healthcheck controllers.HealthCheck.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/innerServiceStatusCheck com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.statusCheck(depth: Int) -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) GET /assets/*file controllers.Assets.versioned(path="/public", file: Asset) diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index 08394033de3..427976b5c69 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -1,5 +1,4 @@ import com.gu.contentapi.client.ScheduledExecutor -import com.gu.mediaservice.lib.management.InnerServiceStatusCheckController import com.gu.mediaservice.lib.play.GridComponents import controllers.UsageApi import lib._ @@ -39,8 +38,7 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag }) val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, liveContentApi, controllerComponents, playBodyParsers) - val InnerServiceStatusCheckController = new InnerServiceStatusCheckController(auth, controllerComponents, config.services, wsClient) - override lazy val router = new Routes(httpErrorHandler, controller, management, InnerServiceStatusCheckController) + override lazy val router = new Routes(httpErrorHandler, controller, management) } diff --git a/usage/conf/routes b/usage/conf/routes index ebed0d15332..c8fee135ca4 100644 --- a/usage/conf/routes +++ b/usage/conf/routes @@ -15,7 +15,6 @@ GET /usages/digital/content/*contentId/reindex controllers.UsageApi.rei # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck GET /management/manifest com.gu.mediaservice.lib.management.Management.manifest -GET /management/whoAmI com.gu.mediaservice.lib.management.InnerServiceStatusCheckController.whoAmI(depth: Int) # Shoo robots away GET /robots.txt com.gu.mediaservice.lib.management.Management.disallowRobots From f860b8d9b03e96389e632776eebfaa2c749a7525 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 25 Jun 2024 13:17:55 +0100 Subject: [PATCH 39/58] Simplify reaper paused control to set by config only. Removes a bucket dependency. --- thrall/app/controllers/ReaperController.scala | 35 +++++-------------- thrall/app/lib/ThrallConfig.scala | 1 + thrall/app/views/reaper.scala.html | 6 ---- thrall/conf/routes | 2 -- 4 files changed, 10 insertions(+), 34 deletions(-) diff --git a/thrall/app/controllers/ReaperController.scala b/thrall/app/controllers/ReaperController.scala index b5a62a64cc2..ee38347459c 100644 --- a/thrall/app/controllers/ReaperController.scala +++ b/thrall/app/controllers/ReaperController.scala @@ -1,7 +1,5 @@ package controllers -import org.apache.pekko.actor.Scheduler -import com.gu.mediaservice.lib.{DateTimeUtils, ImageIngestOperations} import com.gu.mediaservice.lib.auth.Permissions.DeleteImage import com.gu.mediaservice.lib.auth.{Authentication, Authorisation, BaseControllerWithLoginRedirects} import com.gu.mediaservice.lib.aws.S3Vectors @@ -9,15 +7,16 @@ import com.gu.mediaservice.lib.config.Services import com.gu.mediaservice.lib.elasticsearch.ReapableEligibility import com.gu.mediaservice.lib.logging.{GridLogging, MarkerMap} import com.gu.mediaservice.lib.metadata.SoftDeletedMetadataTable +import com.gu.mediaservice.lib.{DateTimeUtils, ImageIngestOperations} import com.gu.mediaservice.model.{ImageStatusRecord, SoftDeletedMetadata} -import lib.{BatchDeletionIds, ThrallConfig, ThrallMetrics, ThrallStore} import lib.elasticsearch.ElasticSearch +import lib.{BatchDeletionIds, ThrallConfig, ThrallMetrics, ThrallStore} +import org.apache.pekko.actor.Scheduler import org.joda.time.{DateTime, DateTimeZone} import play.api.libs.json.{JsValue, Json} import play.api.mvc.{Action, AnyContent, ControllerComponents} import scalaz.NonEmptyList -import software.amazon.awssdk.core.sync.RequestBody -import software.amazon.awssdk.services.s3.model.{ListObjectsV2Request, PutObjectRequest, DeleteObjectRequest} +import software.amazon.awssdk.services.s3.model.ListObjectsV2Request import scala.concurrent.duration.DurationInt import scala.concurrent.{ExecutionContext, Future} @@ -41,9 +40,8 @@ class ReaperController( override val controllerComponents: ControllerComponents, )(implicit val ec: ExecutionContext) extends BaseControllerWithLoginRedirects with GridLogging { - private val CONTROL_FILE_NAME = "PAUSED" - private val INTERVAL = config.reaperInterval //default 15 minutes, based on max of 1000 per reap, this interval will max out at 96,000 images per day + private val isPaused = config.reaperPaused implicit val logMarker: MarkerMap = MarkerMap() @@ -54,8 +52,8 @@ class ReaperController( } } - (config.maybeReaperBucket, config.maybeReaperCountPerRun) match { - case (Some(reaperBucket), Some(countOfImagesToReap)) => + config.maybeReaperCountPerRun match { + case Some(countOfImagesToReap) => // We always want the reaps to occur at predictable times (e.g. on the hour, then 15, 30, 45 minutes past) // However, if the first reap is imminent, then skip it, to avoid double reaps during deployment when both // instances are up and running simultaneously @@ -68,7 +66,7 @@ class ReaperController( interval = INTERVAL, ){ () => try { - if (store.doesObjectExist(reaperBucket, CONTROL_FILE_NAME)) { + if (isPaused) { logger.info("Reaper is paused") es.countTotalSoftReapable(isReapable).map(metrics.softReapable.increment(Nil, _)) es.countTotalHardReapable(isReapable, config.hardReapImagesAge).map(metrics.hardReapable.increment(Nil, _)) @@ -86,7 +84,7 @@ class ReaperController( case NonFatal(e) => logger.error("Reap failed", e) } } - case _ => logger.info("scheduled reaper will not run since 's3.reaper.bucket' and 'reaper.countPerRun' need to be configured in thrall.conf") + case _ => logger.info("scheduled reaper will not run because 'reaper.countPerRun' needs to be configured in thrall.conf") } private def batchDeleteWrapper(count: Int)(func: (Int, String) => Future[JsValue]) = auth.async { request => @@ -188,7 +186,6 @@ class ReaperController( case (None, _) => NotImplemented("'s3.reaper.bucket' not configured in thrall.conf") case (_, None) => NotImplemented("'reaper.countPerRun' not configured in thrall.conf") case (Some(reaperBucket), Some(countOfImagesToReap)) => - val isPaused = store.doesObjectExist(reaperBucket, CONTROL_FILE_NAME) val recentRecords = List(now, now.minusDays(1), now.minusDays(2)).flatMap { day => val s3DirName = s3DirNameFromDate(day) val softDeletes = store.client.listObjectsV2( @@ -219,18 +216,4 @@ class ReaperController( } }} - def pauseReaper = auth { config.maybeReaperBucket match { - case None => NotImplemented("Reaper bucket not configured") - case Some(reaperBucket) => - store.putString(reaperBucket, CONTROL_FILE_NAME, "") - Redirect(routes.ReaperController.index) - }} - - def resumeReaper = auth { config.maybeReaperBucket match { - case None => NotImplemented("Reaper bucket not configured") - case Some(reaperBucket) => - store.client.deleteObject(DeleteObjectRequest.builder().bucket(reaperBucket).key(CONTROL_FILE_NAME).build()) - Redirect(routes.ReaperController.index) - }} - } diff --git a/thrall/app/lib/ThrallConfig.scala b/thrall/app/lib/ThrallConfig.scala index 38df04e45f6..45376fc5a43 100644 --- a/thrall/app/lib/ThrallConfig.scala +++ b/thrall/app/lib/ThrallConfig.scala @@ -71,6 +71,7 @@ class ThrallConfig(resources: GridConfigResources) extends CommonConfigWithElast val projectionParallelism: Int = intDefault("thrall.projection.parallelism", 1) val reaperInterval: FiniteDuration = intDefault("reaper.interval", 15) minutes + val reaperPaused: Boolean = false val hardReapImagesAge: Int = intDefault("reaper.hard.daysInSoftDelete", 14) // soft deleted images age to be hard deleted by Reaper Controller def kinesisConfig: KinesisReceiverConfig = KinesisReceiverConfig(thrallKinesisStream, rewindFrom, this) diff --git a/thrall/app/views/reaper.scala.html b/thrall/app/views/reaper.scala.html index 45967f43876..797d5ae2a91 100644 --- a/thrall/app/views/reaper.scala.html +++ b/thrall/app/views/reaper.scala.html @@ -27,14 +27,8 @@

Reaper

@if(isPaused) {

Reaper is currently paused.

-
- -
} else {

Reaper is currently running (up to @count images every @interval)

-
- -
}

Records from last 48 hours (UTC timestamps)

diff --git a/thrall/conf/routes b/thrall/conf/routes index acb0c32de56..a3397a0c5ab 100644 --- a/thrall/conf/routes +++ b/thrall/conf/routes @@ -23,8 +23,6 @@ GET /reaper controllers.ReaperControll GET /reaper/:key controllers.ReaperController.reaperRecord(key: String) DELETE /doBatchSoftReap controllers.ReaperController.doBatchSoftReap(count: Int) DELETE /doBatchHardReap controllers.ReaperController.doBatchHardReap(count: Int) -POST /pauseReaper controllers.ReaperController.pauseReaper -POST /resumeReaper controllers.ReaperController.resumeReaper # Management GET /management/healthcheck controllers.HealthCheck.healthCheck From 0ab4af773b95bea44dad1909e2513c2ef6e5dec5 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Feb 2026 12:55:06 +0000 Subject: [PATCH 40/58] [containered] End point to set a digital media usage. Use .metadata as the name of the usage metadata field. Shorter and consistent with Download and Front. Prefer digital to digital media in the public interface. More verbose builder method names because of type erasure clashes. Usage advertises the digital-usage end point. # Conflicts: # usage/app/controllers/UsageApi.scala --- .../model/usage/DigitalUsageMetadata.scala | 2 +- usage/app/controllers/UsageApi.scala | 27 ++++++++++++++++++- usage/app/lib/MediaUsageBuilder.scala | 26 ++++++++++++++---- .../app/model/DigitalMediaUsageRequest.scala | 26 ++++++++++++++++++ usage/app/model/UsageGroup.scala | 21 ++++++++++++++- usage/app/model/UsageIdBuilder.scala | 5 ++++ usage/conf/routes | 1 + 7 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 usage/app/model/DigitalMediaUsageRequest.scala diff --git a/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala b/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala index 17dc30d31c0..c7f88a248c1 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala @@ -4,7 +4,7 @@ import com.gu.mediaservice.lib.dynamo.{DbString, DynamoElement} import java.net.URI import play.api.libs.json._ -import com.gu.mediaservice.syntax._ +import org.joda.time.DateTime case class DigitalUsageMetadata ( webUrl: URI, diff --git a/usage/app/controllers/UsageApi.scala b/usage/app/controllers/UsageApi.scala index 6c035d6fced..48811b1301f 100644 --- a/usage/app/controllers/UsageApi.scala +++ b/usage/app/controllers/UsageApi.scala @@ -57,9 +57,11 @@ class UsageApi( Link("usages-by-id", s"${config.usageUri}/usages/{id}") ) + val digitalPostUri = URI.create(s"${config.usageUri}/usages/digital") val printPostUri = URI.create(s"${config.usageUri}/usages/print") val syndicationPostUri = URI.create(s"${config.usageUri}/usages/syndication") val actions = List( + ArgoAction("digital-usage", digitalPostUri, "POST"), ArgoAction("print-usage", printPostUri, "POST"), ArgoAction("syndication-usage", syndicationPostUri, "POST"), ) @@ -165,6 +167,29 @@ class UsageApi( } } + val maxDigitalRequestLength: Int = 1024 * config.maxPrintRequestLengthInKb + val setDigitalRequestBodyParser: BodyParser[JsValue] = playBodyParsers.json(maxLength = maxDigitalRequestLength) + + def setDigitalUsages = auth(setDigitalRequestBodyParser) { req => { + + val digitalMediaUsageRequestResult = req.body.validate[DigitalMediaUsageRequest] + digitalMediaUsageRequestResult.fold( + e => { + respondError(BadRequest, "digital-media-usage-request-parse-failed", JsError.toJson(e).toString) + }, + digitalMediaUsageRequest => { + implicit val logMarker: LogMarker = MarkerMap( + "requestType" -> "set-digital-media-usages", + "requestId" -> RequestLoggingFilter.getRequestId(req), + ) + val usageGroups = usageGroupOps.buildFromDigitalMediaUsageRecords(digitalMediaUsageRequest.digitalMediaUsageRecords) + usageGroups.map(ug => WithLogMarker.includeUsageGroup(ug)).foreach(usageApiSubject.onNext) + + Accepted + } + ) + }} + val maxPrintRequestLength: Int = 1024 * config.maxPrintRequestLengthInKb val setPrintRequestBodyParser: BodyParser[JsValue] = playBodyParsers.json(maxLength = maxPrintRequestLength) @@ -180,7 +205,7 @@ class UsageApi( "requestType" -> "set-print-usages", "requestId" -> RequestLoggingFilter.getRequestId(req), ) - val usageGroups = usageGroupOps.build(printUsageRequest.printUsageRecords) + val usageGroups = usageGroupOps.buildFromPrintUsageRecords(printUsageRequest.printUsageRecords) usageGroups.map(WithLogMarker.includeUsageGroup).foreach(usageApiSubject.onNext) Accepted diff --git a/usage/app/lib/MediaUsageBuilder.scala b/usage/app/lib/MediaUsageBuilder.scala index b6420bab140..c15c3e4c143 100644 --- a/usage/app/lib/MediaUsageBuilder.scala +++ b/usage/app/lib/MediaUsageBuilder.scala @@ -6,20 +6,36 @@ import model._ object MediaUsageBuilder { - def build(printUsage: PrintUsageRecord, usageId: UsageId, grouping: String) = MediaUsage( + def build(printUsageRecord: PrintUsageRecord, usageId: UsageId, grouping: String) = MediaUsage( usageId, grouping, - printUsage.mediaId, + printUsageRecord.mediaId, PrintUsage, "image", - printUsage.usageStatus, - Some(printUsage.printUsageMetadata), + printUsageRecord.usageStatus, + Some(printUsageRecord.printUsageMetadata), None, None, None, None, childUsageMetadata = None, - printUsage.dateAdded + printUsageRecord.dateAdded + ) + + def build(digitalMediaUsageRecord: DigitalMediaUsageRecord, usageId: UsageId, grouping: String): MediaUsage = MediaUsage( + usageId, + grouping, + digitalMediaUsageRecord.mediaId, + DigitalUsage, + "image", + PublishedUsageStatus, + None, + Some(digitalMediaUsageRecord.metadata), + None, + None, + None, + childUsageMetadata = None, + digitalMediaUsageRecord.dateAdded, ) def build(mediaWrapper: MediaWrapper): MediaUsage = { diff --git a/usage/app/model/DigitalMediaUsageRequest.scala b/usage/app/model/DigitalMediaUsageRequest.scala new file mode 100644 index 00000000000..97474c6019d --- /dev/null +++ b/usage/app/model/DigitalMediaUsageRequest.scala @@ -0,0 +1,26 @@ +package model + +import com.gu.mediaservice.model.usage.DigitalUsageMetadata +import org.joda.time.DateTime +import play.api.libs.json.{JodaReads, JodaWrites, Json, Reads, Writes} + +case class DigitalMediaUsageRequest(digitalMediaUsageRecords: List[DigitalMediaUsageRecord]) + +object DigitalMediaUsageRequest { + implicit val reads: Reads[DigitalMediaUsageRequest] = Json.reads[DigitalMediaUsageRequest] + implicit val writes: Writes[DigitalMediaUsageRequest] = Json.writes[DigitalMediaUsageRequest] +} + +case class DigitalMediaUsageRecord( + dateAdded: DateTime, + mediaId: String, + metadata: DigitalUsageMetadata, + ) + +object DigitalMediaUsageRecord { + import JodaWrites._ + import JodaReads._ + + implicit val reads: Reads[DigitalMediaUsageRecord] = Json.reads[DigitalMediaUsageRecord] + implicit val writes: Writes[DigitalMediaUsageRecord] = Json.writes[DigitalMediaUsageRecord] +} diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index c115fd9a04e..88cbdb2773c 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -21,6 +21,14 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) extends GridLogging { def buildId(contentWrapper: ContentWrapper) = contentWrapper.id + + def buildId(digitalMediaUsageRecord: DigitalMediaUsageRecord): String = + MD5.hash(List( + digitalMediaUsageRecord.mediaId, + digitalMediaUsageRecord.metadata.webUrl, + digitalMediaUsageRecord.dateAdded.getMillis.toString + ).mkString("_")) + def buildId(printUsage: PrintUsageRecord) = s"print/${MD5.hash(List( Some(printUsage.mediaId), Some(printUsage.printUsageMetadata.pageNumber), @@ -68,7 +76,7 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) UsageGroup(usages.toSet, contentWrapper.id, lastModified, isReindex, maybeStatus = Some(status)) }) - def build(printUsageRecords: List[PrintUsageRecord]) = + def buildFromPrintUsageRecords(printUsageRecords: List[PrintUsageRecord]): Seq[UsageGroup] = printUsageRecords.map(printUsageRecord => { val usageId = UsageIdBuilder.build(printUsageRecord) @@ -79,6 +87,17 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) ) }) + def buildFromDigitalMediaUsageRecords(digitalMediaUsageRecords: List[DigitalMediaUsageRecord]): Seq[UsageGroup] = + digitalMediaUsageRecords.map((digitalMediaUsageRecord: DigitalMediaUsageRecord) => { + val usageId = UsageIdBuilder.build(digitalMediaUsageRecord) + UsageGroup( + Set(MediaUsageBuilder.build(digitalMediaUsageRecord, usageId, buildId(digitalMediaUsageRecord))), + usageId.toString, + digitalMediaUsageRecord.dateAdded + ) + }) + + def build(syndicationUsageRequest: SyndicationUsageRequest): UsageGroup = { val usageGroupId = buildId(syndicationUsageRequest) UsageGroup( diff --git a/usage/app/model/UsageIdBuilder.scala b/usage/app/model/UsageIdBuilder.scala index b1ada62820e..60a0575adcb 100644 --- a/usage/app/model/UsageIdBuilder.scala +++ b/usage/app/model/UsageIdBuilder.scala @@ -15,6 +15,11 @@ object UsageIdBuilder { Some(printUsageRecord.usageStatus) )) + def build(digitalMediaUsageRecord: DigitalMediaUsageRecord) = buildId(List( + Some(digitalMediaUsageRecord.mediaId), + Some(digitalMediaUsageRecord.metadata.webUrl) + )) + def build(mediaWrapper: MediaWrapper) = buildId(List( Some(mediaWrapper.mediaId), Some(mediaWrapper.contentStatus) diff --git a/usage/conf/routes b/usage/conf/routes index c8fee135ca4..9f0b86c28a0 100644 --- a/usage/conf/routes +++ b/usage/conf/routes @@ -4,6 +4,7 @@ GET /usages/:id controllers.UsageApi.for GET /usages/media/:mediaId controllers.UsageApi.forMedia(mediaId: String) DELETE /usages/media/:mediaId controllers.UsageApi.deleteUsages(mediaId: String) DELETE /usages/media/:mediaId/*usageId controllers.UsageApi.deleteSingleUsage(mediaId: String, usageId: String) +POST /usages/digital controllers.UsageApi.setDigitalUsages() POST /usages/print controllers.UsageApi.setPrintUsages() POST /usages/syndication controllers.UsageApi.setSyndicationUsages() POST /usages/front controllers.UsageApi.setFrontUsages() From 2f673a8f5113a60d8dc7b68946e4347c9ba8157b Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 29 Mar 2026 12:14:39 +0100 Subject: [PATCH 41/58] Drop Guardian specific stream ingestion of usages. Drop more usages composer references; drops composer url config requirement. Remove usages UsageGroups from Guardian Content methods. Remove usages CAPI client and it's config. Delete usages streaming mode Crier steam listening and CAPI specific reindexForContent end point. Container consumers are never going to be Guardian internal. External users would integrate their streams by having a microapp or Lambda ping the instance specific usages API. We can drop this Guardian specific code and config. # Conflicts: # usage/app/UsageComponents.scala --- usage/app/UsageComponents.scala | 13 +- usage/app/controllers/UsageApi.scala | 31 ---- usage/app/lib/ContentApis.scala | 87 ---------- usage/app/lib/CrierEventProcessor.scala | 193 ----------------------- usage/app/lib/CrierStreamReader.scala | 132 ---------------- usage/app/lib/UsageConfig.scala | 66 -------- usage/app/lib/UsageMetadataBuilder.scala | 34 ---- usage/app/lib/UsageRecorder.scala | 2 +- usage/app/model/ContentWrapper.scala | 22 --- usage/app/model/UsageGroup.scala | 93 +---------- usage/conf/routes | 1 - 11 files changed, 7 insertions(+), 667 deletions(-) delete mode 100644 usage/app/lib/CrierEventProcessor.scala delete mode 100644 usage/app/lib/CrierStreamReader.scala delete mode 100644 usage/app/lib/UsageMetadataBuilder.scala delete mode 100644 usage/app/model/ContentWrapper.scala diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index 427976b5c69..d6705a5a59e 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -1,4 +1,5 @@ import com.gu.contentapi.client.ScheduledExecutor +import com.amazonaws.services.dynamodbv2.AmazonDynamoDBAsyncClientBuilder import com.gu.mediaservice.lib.play.GridComponents import controllers.UsageApi import lib._ @@ -13,10 +14,7 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag final override val buildInfo = utils.buildinfo.BuildInfo - val usageMetadataBuilder = new UsageMetadataBuilder(config) - val mediaWrapper = new MediaWrapperOps(usageMetadataBuilder) - val liveContentApi = new LiveContentApi(config)(ScheduledExecutor()) - val usageGroupOps = new UsageGroupOps(config, mediaWrapper) + val usageGroupOps = new UsageGroupOps(config) val usageTable = new UsageTable( config.withAWSCredentials(DynamoDbClient.builder()).build(), config.usageRecordTable @@ -26,18 +24,13 @@ class UsageComponents(context: Context) extends GridComponents(context, new Usag val usageRecorder = new UsageRecorder(usageMetrics, usageTable, usageNotifier, usageNotifier) val notifications = new Notifications(config) - if(!config.apiOnly) { - val crierReader = new CrierStreamReader(config, usageGroupOps, executionContext) - crierReader.start() - } - usageRecorder.start() context.lifecycle.addStopHook(() => { usageRecorder.stop() Future.successful(()) }) - val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, liveContentApi, controllerComponents, playBodyParsers) + val controller = new UsageApi(auth, authorisation, usageTable, usageGroupOps, notifications, config, usageRecorder.usageApiSubject, controllerComponents, playBodyParsers) override lazy val router = new Routes(httpErrorHandler, controller, management) diff --git a/usage/app/controllers/UsageApi.scala b/usage/app/controllers/UsageApi.scala index 48811b1301f..54c2926f2d5 100644 --- a/usage/app/controllers/UsageApi.scala +++ b/usage/app/controllers/UsageApi.scala @@ -29,7 +29,6 @@ class UsageApi( notifications: Notifications, config: UsageConfig, usageApiSubject: Subject[WithLogMarker[UsageGroup]], - liveContentApi: LiveContentApi, override val controllerComponents: ControllerComponents, playBodyParsers: PlayBodyParsers )( @@ -101,36 +100,6 @@ class UsageApi( } - def reindexForContent(contentId: String) = auth.async { req => - implicit val logMarker: LogMarker = MarkerMap( - "requestType" -> "reindex-for-content", - "requestId" -> RequestLoggingFilter.getRequestId(req), - "contentId" -> contentId, - ) - - val query = liveContentApi.usageQuery(contentId) - - liveContentApi.getResponse(query).map{response => - response.content match { - case Some(content) => - ContentHelpers - .getContentFirstPublished(content) - .map(LiveContentItem(content, _)) - .map(_.copy(isReindex = true)) - .foreach(_.emitAsUsageGroup( - usageApiSubject, - usageGroupOps - )) - Accepted - case _ => - NotFound - } - }.recover { case error: Exception => - logger.error(logMarker, s"UsageApi reindex for content ($contentId) failed!", error) - InternalServerError - } - } - def forMedia(mediaId: String) = auth.async { req => implicit val logMarker: LogMarker = MarkerMap( "requestType" -> "usages-for-media-id", diff --git a/usage/app/lib/ContentApis.scala b/usage/app/lib/ContentApis.scala index 3f5eaa12296..e69de29bb2d 100644 --- a/usage/app/lib/ContentApis.scala +++ b/usage/app/lib/ContentApis.scala @@ -1,87 +0,0 @@ -package lib - -import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider -import software.amazon.awssdk.regions.Region -import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider -import software.amazon.awssdk.services.sts.model.AssumeRoleRequest -import software.amazon.awssdk.services.sts.StsClient -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider - -import com.gu.contentapi.client._ -import com.gu.contentapi.client.model.{HttpResponse, ItemQuery} - -import java.net.URI -import scala.concurrent.duration.DurationInt -import scala.concurrent.{ExecutionContext, Future} - -abstract class UsageContentApiClient(config: UsageConfig)(implicit val executor: ScheduledExecutor) - extends GuardianContentClient(apiKey = config.capiApiKey) { - - def usageQuery(contentId: String): ItemQuery = { - ItemQuery(contentId) - .showFields("firstPublicationDate,isLive,internalComposerCode") - .showElements("image,cartoon") - .showAtoms("media") - } -} - -class LiveContentApi(config: UsageConfig)(implicit val ex: ScheduledExecutor) - extends UsageContentApiClient(config) with RetryableContentApiClient { - - override val targetUrl: String = config.capiLiveUrl - override val backoffStrategy: BackoffStrategy = BackoffStrategy.doublingStrategy(2.seconds, config.capiMaxRetries) -} - -class PreviewContentApi(protected val config: UsageConfig)(implicit val ex: ScheduledExecutor) - // ensure IAMAuthContentApiClient is the first trait in this list! - extends UsageContentApiClient(config) with IAMAuthContentApiClient with RetryableContentApiClient { - - override val targetUrl: String = config.capiPreviewUrl - override val backoffStrategy: BackoffStrategy = BackoffStrategy.doublingStrategy(2.seconds, config.capiMaxRetries) -} - -// order of mixing is important. Some client traits (notably RetryableContentApiClient!) -// also override get, adding header(s) (and could potentially edit the uri too) before calling super.get(). Those -// traits must be executed BEFORE this trait, so that the get override in this trait -// receives the headers that will actually be sent over the wire. -// so any class mixing this in should have it first in the list of traits, eg. -// class MyCapiClient extends GuardianContentApiClient(apiKey) -// with IAMAuthContentApiClient with RetryableContentApiClient with MyOtherClientTraits -// ie. the super calls will travel "from right to left" along the trait list, and this trait can sign the accumulated headers -trait IAMAuthContentApiClient extends ContentApiClient { - protected val config: UsageConfig - - lazy val sts: StsClient = StsClient.builder() - .region(Region.of(config.awsRegionName)) - .build() - - private lazy val sessionId: String = "session-" + Math.random() - lazy val capiCredentials: AwsCredentialsProvider = - config.capiPreviewRole.map(arn => { - - val assumeRoleRequest = AssumeRoleRequest.builder().roleArn(arn).roleSessionName(sessionId).build() - - StsAssumeRoleCredentialsProvider.builder() - .refreshRequest(assumeRoleRequest) - .stsClient(sts) - .build() - }).getOrElse(ProfileCredentialsProvider.create("capi")) // will be used if stream is ever run locally (unusual) - - abstract override def get( - url: String, - headers: Map[String, String] - )(implicit context: ExecutionContext): Future[HttpResponse] = { - - val uri = new URI(url) - val encodedQuery = IAMEncoder.encodeParams(uri.getQuery) - - // no mutation of uris, and no easy way to create from a given one - val encodedUri = new URI(uri.getScheme, uri.getAuthority, uri.getPath, encodedQuery, uri.getFragment) - - val signer = new IAMSigner(capiCredentials, config.awsRegionName) - - val withIamHeaders = signer.addIAMHeaders(headers, encodedUri) - - super.get(encodedUri.toString, withIamHeaders) - } -} diff --git a/usage/app/lib/CrierEventProcessor.scala b/usage/app/lib/CrierEventProcessor.scala deleted file mode 100644 index abf46e069ea..00000000000 --- a/usage/app/lib/CrierEventProcessor.scala +++ /dev/null @@ -1,193 +0,0 @@ -package lib - -import com.gu.contentapi.client.ScheduledExecutor -import com.gu.contentapi.client.model.ContentApiError -import com.gu.contentapi.client.model.v1.Content -import com.gu.crier.model.event.v1.{Event, EventPayload, EventType} -import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker, MarkerMap} -import com.gu.mediaservice.model.usage.{PendingUsageStatus, PublishedUsageStatus} -import com.gu.thrift.serializer.ThriftDeserializer -import com.twitter.scrooge.ThriftStructCodec -import model.{UsageGroup, UsageGroupOps} -import org.joda.time.DateTime -import rx.lang.scala.Subject -import rx.lang.scala.subjects.PublishSubject -import software.amazon.kinesis.exceptions.ShutdownException -import software.amazon.kinesis.leases.exceptions.InvalidStateException -import software.amazon.kinesis.lifecycle.events._ -import software.amazon.kinesis.processor.ShardRecordProcessor - -import java.util.UUID -import scala.concurrent.ExecutionContext.Implicits.global -import scala.jdk.CollectionConverters._ -import scala.util.Try - -trait ContentContainer extends GridLogging { - val content: Content - val lastModified: DateTime - val isReindex: Boolean - - private lazy val isEntirePieceTakenDown = - content.fields.exists(fields => fields.firstPublicationDate.isDefined && fields.isLive.contains(false)) - - def emitAsUsageGroup( - publishSubject: Subject[WithLogMarker[UsageGroup]], usageGroupOps: UsageGroupOps - )(implicit logMarker: LogMarker): Unit = { - usageGroupOps.build( - content, - status = this match { - case PreviewContentItem(_,_,_) => PendingUsageStatus - case LiveContentItem(_,_,_) => PublishedUsageStatus - }, - lastModified, - isReindex - ) match { - case None => logger.debug(logMarker, s"No fields in content of crier update for payload with content ID ${content.id}") - case Some(usageGroup) => - val groupingLogMarker = logMarker ++ Map("usageGroup" -> usageGroup.grouping) - - publishSubject.onNext(WithLogMarker(groupingLogMarker, usageGroup)) - - if (this.isInstanceOf[PreviewContentItem] && isEntirePieceTakenDown) { - logger.info(groupingLogMarker, s"${usageGroup.grouping} is taken down so producing empty UsageGroup to ensure any 'published' DB records are marked as removed") - publishSubject.onNext(WithLogMarker(groupingLogMarker, usageGroup.copy( - usages = Set.empty, - maybeStatus = Some(PublishedUsageStatus) - ))) - } - } - } -} - -object CrierUsageStream { - val observable: Subject[WithLogMarker[UsageGroup]] = PublishSubject[WithLogMarker[UsageGroup]]() -} - -case class LiveContentItem(content: Content, lastModified: DateTime, isReindex: Boolean = false) extends ContentContainer -case class PreviewContentItem(content: Content, lastModified: DateTime, isReindex: Boolean = false) extends ContentContainer - -abstract class CrierEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends ShardRecordProcessor with GridLogging { - - implicit val codec: ThriftStructCodec[Event] = Event - - val contentApiClient: UsageContentApiClient - - override def initialize(initializationInput: InitializationInput): Unit = { - logger.debug(s"Initialized an event processor for shard ${initializationInput.shardId}") - } - - override def processRecords(processRecordsInput: ProcessRecordsInput): Unit = { - val records = processRecordsInput.records - records.asScala.foreach { record => - val deserialization: Try[Event] = ThriftDeserializer.deserialize(record.data) - deserialization.foreach(processEvent) - deserialization.failed.foreach { e: Throwable => - logger.error("Failed to deserialize crier event", e) - } - } - - val lastRecord = records.asScala.last - - processRecordsInput.checkpointer.checkpoint(lastRecord.sequenceNumber(), lastRecord.subSequenceNumber()) - } - - override def leaseLost(leaseLostInput: LeaseLostInput): Unit = { - // nothing to do? - logger.debug("Lost lease, so stopping processing Crier") - } - - override def shardEnded(shardEndedInput: ShardEndedInput): Unit = { - try { - shardEndedInput.checkpointer.checkpoint() - logger.debug("Shard ended, so stopping processing Crier") - } catch { - case _: ShutdownException | _: InvalidStateException => - () - } - } - - override def shutdownRequested(shutdownRequestedInput: ShutdownRequestedInput): Unit = { - try { - shutdownRequestedInput.checkpointer.checkpoint() - logger.debug("Shutdown requested, so stopping processing Crier") - } catch { - case _: ShutdownException | _: InvalidStateException => - () - } - } - - def getContentItem(content: Content, time: DateTime): ContentContainer - - - private def processEvent(event: Event): Unit = { - implicit val logMarker: LogMarker = MarkerMap( - "payloadId" -> event.payloadId, - "requestId" -> UUID.randomUUID().toString - ) - - Try { - val dateTime: DateTime = new DateTime(event.dateTime) - - event.eventType match { - case EventType.Update => - - event.payload match { - case Some(content: EventPayload.Content) => - getContentItem(content.content, dateTime) - .emitAsUsageGroup(CrierUsageStream.observable, usageGroupOps) - case _ => - logger.warn(logMarker, s"Received crier update for ${event.payloadId} without payload") - } - case EventType.Delete => - //TODO: how do we deal with a piece of content that has been deleted? - case EventType.RetrievableUpdate => - - event.payload match { - case Some(retrievableContent: EventPayload.RetrievableContent) => - val capiUrl = retrievableContent.retrievableContent.capiUrl - - val query = contentApiClient.usageQuery(retrievableContent.retrievableContent.id) - - logger.info(logMarker, s"retrieving content event at $capiUrl parsed to id ${query.toString}") - - contentApiClient.getResponse(query).map(response => { - response.content match { - case Some(content) => - getContentItem(content, dateTime) - .emitAsUsageGroup(CrierUsageStream.observable, usageGroupOps) - case _ => - logger.debug( - logMarker, - s"Received retrievable update for ${retrievableContent.retrievableContent.id} without content" - ) - } - }).recover { - case e: ContentApiError => - logger.error(logMarker, s"CAPI error when fetching content update for ${event.payloadId}: ${e.httpStatus} ${e.httpMessage} ${e.errorResponse}", e) - case e => - logger.error(logMarker, s"Failed to fetch or process content update for ${event.payloadId}", e) - } - case _ => logger.warn(logMarker, s"Received crier update for ${event.payloadId} without payload") - } - - case _ => logger.warn(logMarker, s"Unsupported event type $EventType") - } - }.recover { - case e => logger.error(logMarker, s"Failed to process event ${event.payloadId}", e) - } - } -} - -private class CrierLiveEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends CrierEventProcessor(config, usageGroupOps) { - - def getContentItem(content: Content, date: DateTime): ContentContainer = LiveContentItem(content, date) - - override val contentApiClient: LiveContentApi = new LiveContentApi(config)(ScheduledExecutor()) -} - -private class CrierPreviewEventProcessor(config: UsageConfig, usageGroupOps: UsageGroupOps) extends CrierEventProcessor(config, usageGroupOps) { - - def getContentItem(content: Content, date: DateTime): ContentContainer = PreviewContentItem(content, date) - - override val contentApiClient: PreviewContentApi = new PreviewContentApi(config)(ScheduledExecutor()) -} diff --git a/usage/app/lib/CrierStreamReader.scala b/usage/app/lib/CrierStreamReader.scala deleted file mode 100644 index 2c4b42c4865..00000000000 --- a/usage/app/lib/CrierStreamReader.scala +++ /dev/null @@ -1,132 +0,0 @@ -package lib - -import com.gu.mediaservice.lib.logging.GridLogging -import model.UsageGroupOps -import software.amazon.awssdk.auth.credentials.{AwsCredentialsProviderChain, DefaultCredentialsProvider, ProfileCredentialsProvider} -import software.amazon.awssdk.regions.Region -import software.amazon.awssdk.services.cloudwatch.CloudWatchAsyncClient -import software.amazon.awssdk.services.dynamodb.DynamoDbAsyncClient -import software.amazon.awssdk.services.kinesis.KinesisAsyncClient -import software.amazon.awssdk.services.sts.StsClient -import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider -import software.amazon.awssdk.services.sts.model.AssumeRoleRequest -import software.amazon.kinesis.common.{ConfigsBuilder, InitialPositionInStream, InitialPositionInStreamExtended, KinesisClientUtil} -import software.amazon.kinesis.coordinator.CoordinatorConfig.ClientVersionConfig -import software.amazon.kinesis.coordinator.Scheduler -import software.amazon.kinesis.processor.{ShardRecordProcessor, ShardRecordProcessorFactory} -import software.amazon.kinesis.retrieval.polling.PollingConfig - -import java.net.InetAddress -import java.util.UUID -import scala.annotation.nowarn -import scala.concurrent.ExecutionContext - -// it's annoyingly hard to get the streamName out of the configsBuilder once built, so pass them around together -private case class ConfigsBuilderWithStreamName(configsBuilder: ConfigsBuilder, streamName: String) - -class CrierStreamReader( - config: UsageConfig, - usageGroupOps: UsageGroupOps, - executionContext: ExecutionContext -) extends GridLogging { - - private val region = Region.of(config.awsRegionName) - - private lazy val workerId: String = InetAddress.getLocalHost.getCanonicalHostName + ":" + UUID.randomUUID() - - private lazy val awsCredentialsProvider = DefaultCredentialsProvider.builder().profileName("media-service").build() - private lazy val stsClient = StsClient.builder().region(region).credentialsProvider(awsCredentialsProvider).build() - - private lazy val sessionId: String = "session-" + Math.random() - private val initialPosition = InitialPositionInStreamExtended.newInitialPosition(InitialPositionInStream.TRIM_HORIZON) - - private def kinesisCredentialsProvider(arn: String): AwsCredentialsProviderChain = { - val assumeRoleRequest = AssumeRoleRequest.builder().roleArn(arn).roleSessionName(sessionId).build() - - AwsCredentialsProviderChain.of( - ProfileCredentialsProvider.create("capi"), - StsAssumeRoleCredentialsProvider.builder().refreshRequest(assumeRoleRequest).stsClient(stsClient).build() - ) - } - - private def kinesisClientLibConfig(processorFactory: ShardRecordProcessorFactory) - (kinesisReaderConfig: KinesisReaderConfig): ConfigsBuilderWithStreamName = { - - val kinesisClient = KinesisClientUtil.createKinesisAsyncClient(KinesisAsyncClient.builder() - .region(region) - .credentialsProvider(kinesisCredentialsProvider(kinesisReaderConfig.arn))) - val dynamoClient = DynamoDbAsyncClient.builder() - .region(region) - .credentialsProvider(awsCredentialsProvider) - .build() - val cloudwatchClient = CloudWatchAsyncClient.builder() - .region(region) - .credentialsProvider(awsCredentialsProvider) - .build() - - ConfigsBuilderWithStreamName( - new ConfigsBuilder( - kinesisReaderConfig.streamName, - kinesisReaderConfig.appName, - kinesisClient, - dynamoClient, - cloudwatchClient, - workerId, - processorFactory - ), - kinesisReaderConfig.streamName - ) - } - - @nowarn("cat=deprecation") // initialPositionInStreamExtended is deprecated, but the upgrade path is unclear - private def kinesisClientLibScheduler(configsBuilderAndStreamName: ConfigsBuilderWithStreamName): Scheduler = { - val ConfigsBuilderWithStreamName(configsBuilder, streamName) = configsBuilderAndStreamName - new Scheduler( - configsBuilder.checkpointConfig(), - configsBuilder.coordinatorConfig() - .clientVersionConfig(ClientVersionConfig.CLIENT_VERSION_CONFIG_COMPATIBLE_WITH_2X), - configsBuilder.leaseManagementConfig(), - configsBuilder.lifecycleConfig(), - configsBuilder.metricsConfig(), - configsBuilder.processorConfig(), - configsBuilder.retrievalConfig() - .initialPositionInStreamExtended(initialPosition) - .retrievalSpecificConfig(new PollingConfig(streamName, configsBuilder.kinesisClient())), - ) - } - - private val LiveEventProcessorFactory = new ShardRecordProcessorFactory { - override def shardRecordProcessor(): ShardRecordProcessor = - new CrierLiveEventProcessor(config, usageGroupOps) - } - - private val PreviewEventProcessorFactory = new ShardRecordProcessorFactory { - override def shardRecordProcessor(): ShardRecordProcessor = - new CrierPreviewEventProcessor(config, usageGroupOps) - } - - private lazy val liveConfig = config.liveKinesisReaderConfig - .map(kinesisClientLibConfig(LiveEventProcessorFactory)) - private lazy val previewConfig = config.previewKinesisReaderConfig - .map(kinesisClientLibConfig(PreviewEventProcessorFactory)) - - private lazy val liveScheduler = liveConfig.map(kinesisClientLibScheduler) - private lazy val previewScheduler = previewConfig.map(kinesisClientLibScheduler) - - def start(): Unit = { - logger.info("Trying to start Crier Stream Readers") - - liveScheduler - .map(executionContext.execute) - .fold( - e => logger.error("No 'Crier Live Stream reader' thread to start", e), - _ => logger.info("Starting Crier Live Stream reader") - ) - previewScheduler - .map(executionContext.execute) - .fold( - e => logger.error("No 'Crier Preview Stream reader' thread to start", e), - _ => logger.info("Starting Crier Preview Stream reader") - ) - } -} diff --git a/usage/app/lib/UsageConfig.scala b/usage/app/lib/UsageConfig.scala index e2b7caa28ed..cfbfbdcad9d 100644 --- a/usage/app/lib/UsageConfig.scala +++ b/usage/app/lib/UsageConfig.scala @@ -3,83 +3,17 @@ package lib import com.gu.mediaservice.lib.config.{CommonConfig, GridConfigResources} import com.gu.mediaservice.lib.logging.GridLogging import com.gu.mediaservice.lib.net.URI.ensureSecure -import software.amazon.awssdk.core.exception.SdkClientException -import software.amazon.awssdk.services.iam.IamClient - -import scala.util.Try - - -case class KinesisReaderConfig(streamName: String, arn: String, appName: String) class UsageConfig(resources: GridConfigResources) extends CommonConfig(resources) with GridLogging { val usageUri: String = services.usageBaseUri val apiUri: String = services.apiBaseUri - val defaultMaxRetries = 4 val defaultMaxPrintRequestSizeInKb = 500 val defaultDateLimit = "2016-01-01T00:00:00+00:00" val maxPrintRequestLengthInKb: Int = intDefault("api.setPrint.maxLength", defaultMaxPrintRequestSizeInKb) - val capiLiveUrl = string("capi.live.url") - val capiPreviewUrl = string("capi.preview.url") - val capiPreviewRole = stringOpt("capi.preview.role") - val capiApiKey = string("capi.apiKey") - val capiMaxRetries: Int = intDefault("capi.maxRetries", defaultMaxRetries) - val usageDateLimit: String = stringDefault("usage.dateLimit", defaultDateLimit) - private val composerBaseUrlProperty: String = string("composer.baseUrl") - private val composerBaseUrl = ensureSecure(composerBaseUrlProperty) - - val composerContentBaseUrl: String = s"$composerBaseUrl/content" - val usageRecordTable = string("dynamo.tablename.usageRecordTable") - - val awsRegionName = string("aws.region") - - private val iamClient: IamClient = withAWSCredentials(IamClient.builder()).build() - - val postfix: String = if (isDev) { - try { - iamClient.getUser.user().userName() - } catch { - case e: SdkClientException => - logger.warn("Unable to determine current IAM user, probably because you're using temp credentials. Usage may not be able to determine the live/preview app names", e) - "tempcredentials" - } - } else { - stage - } - - val liveAppName = s"media-service-livex-$postfix" - val previewAppName = s"media-service-previewx-$postfix" - - val crierLiveKinesisStream = Try { string("crier.live.name") } - val crierPreviewKinesisStream = Try { string("crier.preview.name") } - - val crierLiveArn = Try { string("crier.live.arn") } - val crierPreviewArn = Try { string("crier.preview.arn") } - - val liveKinesisReaderConfig: Try[KinesisReaderConfig] = for { - liveStream <- crierLiveKinesisStream - liveArn <- crierLiveArn - } yield KinesisReaderConfig(liveStream, liveArn, liveAppName) - - val previewKinesisReaderConfig: Try[KinesisReaderConfig] = for { - previewStream <- crierPreviewKinesisStream - previewArn <- crierPreviewArn - } yield KinesisReaderConfig(previewStream, previewArn, previewAppName) - - val apiOnly: Boolean = stringOpt("app.name") match { - case Some("usage-stream") => - logger.info(s"Starting as Stream Reader Usage.") - false - case Some("usage") => - logger.info(s"Starting as API only Usage.") - true - case name => - logger.error(s"App name is invalid: $name") - sys.exit(1) - } } diff --git a/usage/app/lib/UsageMetadataBuilder.scala b/usage/app/lib/UsageMetadataBuilder.scala deleted file mode 100644 index 4f342f78376..00000000000 --- a/usage/app/lib/UsageMetadataBuilder.scala +++ /dev/null @@ -1,34 +0,0 @@ -package lib - -import java.net.URI - -import com.gu.contentapi.client.model.v1.Content -import com.gu.mediaservice.model.usage._ - -import scala.util.Try - -class UsageMetadataBuilder(config: UsageConfig) { - - def composerUrl(content: Content): Option[URI] = content.fields - .flatMap(_.internalComposerCode) - .flatMap(composerId => { - Try(URI.create(s"${config.composerContentBaseUrl}/$composerId")).toOption - }) - - def buildDownload(metadataMap: Map[String, Any]): Option[DownloadUsageMetadata] = { - Try { - DownloadUsageMetadata( - metadataMap("downloadedBy").asInstanceOf[String] - ) - }.toOption - } - - def build(content: Content): DigitalUsageMetadata = { - DigitalUsageMetadata( - URI.create(content.webUrl), - content.webTitle, - content.sectionId.getOrElse("none"), - composerUrl(content) - ) - } -} diff --git a/usage/app/lib/UsageRecorder.scala b/usage/app/lib/UsageRecorder.scala index 5136fe10a92..1a9ed699541 100644 --- a/usage/app/lib/UsageRecorder.scala +++ b/usage/app/lib/UsageRecorder.scala @@ -19,7 +19,7 @@ class UsageRecorder( ) extends GridLogging { val usageApiSubject: Subject[WithLogMarker[UsageGroup]] = PublishSubject[WithLogMarker[UsageGroup]]() - val combinedObservable: Observable[WithLogMarker[UsageGroup]] = CrierUsageStream.observable.merge(usageApiSubject) + val combinedObservable: Observable[WithLogMarker[UsageGroup]] = usageApiSubject val subscriber: Subscriber[LogMarker] = Subscriber((markers: LogMarker) => logger.debug(markers, s"Sent Usage Notification")) var maybeSubscription: Option[Subscription] = None diff --git a/usage/app/model/ContentWrapper.scala b/usage/app/model/ContentWrapper.scala deleted file mode 100644 index b5a074971f7..00000000000 --- a/usage/app/model/ContentWrapper.scala +++ /dev/null @@ -1,22 +0,0 @@ -package model - -import com.gu.contentapi.client.model.v1.Content -import com.gu.mediaservice.model.usage.UsageStatus - -import org.joda.time.DateTime - - -case class ContentWrapper( - id: String, - status: UsageStatus, - lastModified: DateTime, - content: Content -) -object ContentWrapper { - def build(content: Content, status: UsageStatus, lastModified: DateTime): Option[ContentWrapper] = { - extractId(content).map(ContentWrapper(_, status, lastModified, content)) - } - - def extractId(content: Content): Option[String] = - content.fields.flatMap(_.internalComposerCode).map(composerId => s"composer/$composerId") -} diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index 88cbdb2773c..b539f450287 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -5,11 +5,9 @@ import com.gu.contentapi.client.model.v1.{Content, Element, ElementType} import com.gu.contentatom.thrift.{Atom, AtomData} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} import com.gu.mediaservice.model.usage.{DigitalUsageMetadata, MediaUsage, PublishedUsageStatus, UsageStatus} -import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig, UsageMetadataBuilder} +import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig} import org.joda.time.DateTime -import scala.collection.compat._ - case class UsageGroup( usages: Set[MediaUsage], grouping: String, @@ -17,11 +15,9 @@ case class UsageGroup( isReindex: Boolean = false, maybeStatus: Option[UsageStatus] = None ) -class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) +class UsageGroupOps(config: UsageConfig) extends GridLogging { - def buildId(contentWrapper: ContentWrapper) = contentWrapper.id - def buildId(digitalMediaUsageRecord: DigitalMediaUsageRecord): String = MD5.hash(List( digitalMediaUsageRecord.mediaId, @@ -69,14 +65,7 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) ).mkString("_")) }" - def build(content: Content, status: UsageStatus, lastModified: DateTime, isReindex: Boolean)(implicit logMarker: LogMarker) = - ContentWrapper.build(content, status, lastModified).map(contentWrapper => { - val usages = createUsages(contentWrapper, isReindex) - logger.info(logMarker, s"Built UsageGroup: ${contentWrapper.id}") - UsageGroup(usages.toSet, contentWrapper.id, lastModified, isReindex, maybeStatus = Some(status)) - }) - - def buildFromPrintUsageRecords(printUsageRecords: List[PrintUsageRecord]): Seq[UsageGroup] = + def buildFromPrintUsageRecords(printUsageRecords: List[PrintUsageRecord]) = printUsageRecords.map(printUsageRecord => { val usageId = UsageIdBuilder.build(printUsageRecord) @@ -134,41 +123,6 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) ) } - def createUsages(contentWrapper: ContentWrapper, isReindex: Boolean)(implicit logMarker: LogMarker) = { - // Generate unique UUID to track extract job - val uuid = java.util.UUID.randomUUID.toString - implicit val extractJobLogMarkers: LogMarker = logMarker ++ Map("extract-job-id" -> uuid) - - val content = contentWrapper.content - val usageStatus = contentWrapper.status - - logger.info(extractJobLogMarkers, s"Extracting images from ${content.id}") - - val mediaAtomsUsages = extractMediaAtoms(content, usageStatus, isReindex)(extractJobLogMarkers).flatMap { atom => - getImageId(atom) match { - case Some(id) => - val mediaWrapper = mediaWrapperOps.build(mediaId = id, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - Seq(createUsagesLogging(usage)(logMarker)) - case None => Seq.empty - } - } - val imageElementUsages = extractImageElements(content, usageStatus, isReindex)(extractJobLogMarkers).map { element => - val mediaWrapper = mediaWrapperOps.build(mediaId = element.id, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - createUsagesLogging(usage)(logMarker) - } - val cartoonElementUsages = extractCartoonUniqueMediaIds(content).map { mediaId => - val mediaWrapper = mediaWrapperOps.build(mediaId, contentWrapper = contentWrapper, usageGroupId = buildId(contentWrapper)) - val usage = MediaUsageBuilder.build(mediaWrapper) - createUsagesLogging(usage)(logMarker) - } - - // TODO capture images from interactive embeds - - mediaAtomsUsages ++ imageElementUsages ++ cartoonElementUsages - } - private def createUsagesLogging(usage: MediaUsage)(implicit logMarker: LogMarker) = { logger.info(logMarker, s"Built MediaUsage for ${usage.mediaId}") @@ -191,40 +145,6 @@ class UsageGroupOps(config: UsageConfig, mediaWrapperOps: MediaWrapperOps) } } - private def extractMediaAtoms(content: Content, usageStatus: UsageStatus, isReindex: Boolean)(implicit logMarker: LogMarker) = { - val isNew = isNewContent(content, usageStatus) - val shouldRecordUsages = isNew || isReindex - - if (shouldRecordUsages) { - logger.info(logMarker, s"Passed shouldRecordUsages for media atom") - val groupedMediaAtoms = groupMediaAtoms(content) - - if (groupedMediaAtoms.isEmpty) { - logger.info(logMarker, s"No Matching media atoms found") - } else { - logger.info(logMarker, s"${groupedMediaAtoms.length} media atoms found") - groupedMediaAtoms.foreach(atom => logger.info(logMarker, s"Matching media atom ${atom.id} found")) - } - - groupedMediaAtoms - } else { - logger.info(logMarker, s"Failed shouldRecordUsages for media atoms: isNew-$isNew isReindex-$isReindex") - Seq.empty - } - } - - private def groupMediaAtoms(content: Content) = { - val mediaAtoms = content.atoms match { - case Some(atoms) => - atoms.media match { - case Some(mediaAtoms) => filterOutAtomsWithNoImage(mediaAtoms.toSeq) - case _ => Seq.empty - } - case _ => Seq.empty - } - mediaAtoms - } - private def filterOutAtomsWithNoImage(atoms: Seq[Atom]): Seq[Atom] = { for { atom <- atoms @@ -298,10 +218,3 @@ case class MediaWrapper( contentStatus: UsageStatus, usageMetadata: DigitalUsageMetadata, lastModified: DateTime) - -class MediaWrapperOps(usageMetadataBuilder: UsageMetadataBuilder) { - def build(mediaId: String, contentWrapper: ContentWrapper, usageGroupId: String): MediaWrapper = { - val usageMetadata = usageMetadataBuilder.build(contentWrapper.content) - MediaWrapper(mediaId, usageGroupId, contentWrapper.status, usageMetadata, contentWrapper.lastModified) - } -} diff --git a/usage/conf/routes b/usage/conf/routes index 9f0b86c28a0..07e387bbbee 100644 --- a/usage/conf/routes +++ b/usage/conf/routes @@ -11,7 +11,6 @@ POST /usages/front controllers.UsageApi.set POST /usages/download controllers.UsageApi.setDownloadUsages() POST /usages/child controllers.UsageApi.setChildUsages() PUT /usages/status/update/:mediaId/*usageId controllers.UsageApi.updateUsageStatus(mediaId: String, usageId: String) -GET /usages/digital/content/*contentId/reindex controllers.UsageApi.reindexForContent(contentId: String) # Management GET /management/healthcheck com.gu.mediaservice.lib.management.Management.healthCheck From 2d62cbd8a307e28b89c6fe529a7ab4fa8dcfaf00 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 Apr 2026 18:57:02 +0100 Subject: [PATCH 42/58] [containerised] Digital usage ids are prefixed with digital for consistency with print. --- usage/app/model/UsageGroup.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index b539f450287..8e037e1a3d6 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -18,12 +18,13 @@ case class UsageGroup( class UsageGroupOps(config: UsageConfig) extends GridLogging { - def buildId(digitalMediaUsageRecord: DigitalMediaUsageRecord): String = + def buildId(digitalMediaUsageRecord: DigitalMediaUsageRecord): String = s"digital/${ MD5.hash(List( digitalMediaUsageRecord.mediaId, digitalMediaUsageRecord.metadata.webUrl, digitalMediaUsageRecord.dateAdded.getMillis.toString ).mkString("_")) + }" def buildId(printUsage: PrintUsageRecord) = s"print/${MD5.hash(List( Some(printUsage.mediaId), From f40f1f39f200cd56ecc0a95e9745c63e54684711 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Feb 2026 18:45:55 +0000 Subject: [PATCH 43/58] [digital-usage] Digital usage webTitle and sectionId are optional. Read back optional webTitle and sectionId correctly from dynamoDB item. # Conflicts: # common-lib/src/main/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsage.scala # common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala # Conflicts: # common-lib/src/test/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsageTest.scala # usage/test/model/UsageRecordTest.scala --- .../lib/elasticsearch/MappingTest.scala | 4 ++-- .../lib/usage/ItemToMediaUsage.scala | 8 ++++---- .../mediaservice/lib/usage/UsageBuilder.scala | 2 +- .../model/usage/DigitalUsageMetadata.scala | 17 +++++++---------- .../lib/usage/ItemToMediaUsageTest.scala | 6 +++--- usage/app/UsageComponents.scala | 2 -- usage/test/model/UsageRecordTest.scala | 12 ++++++------ usage/test/model/UsageTableTest.scala | 4 ++-- 8 files changed, 25 insertions(+), 30 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/MappingTest.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/MappingTest.scala index f4935c04c53..b52577ab2d5 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/MappingTest.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/elasticsearch/MappingTest.scala @@ -183,8 +183,8 @@ object MappingTest { ), digitalUsageMetadata = Some(DigitalUsageMetadata( webUrl = new URI("https://gu.com/12345"), - webTitle = "Article title", - sectionId = "uk/news", + webTitle = Some("Article title"), + sectionId = Some("uk/news"), composerUrl = Some(new URI("https://composer/api/2345678987654321345678")) )), syndicationUsageMetadata = Some(SyndicationUsageMetadata( diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsage.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsage.scala index 882030a4907..4d142666720 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsage.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsage.scala @@ -71,10 +71,10 @@ object ItemToMediaUsage { private def buildDigital(metadataMap: Map[String, String]): Option[DigitalUsageMetadata] = { Try { DigitalUsageMetadata( - URI.create(metadataMap("webUrl")), - metadataMap("webTitle"), - metadataMap("sectionId"), - metadataMap.get("composerUrl").map(x => URI.create(x)) + URI.create(metadataMap("webUrl").asInstanceOf[String]), + metadataMap.get("webTitle").map(x => x.asInstanceOf[String]), + metadataMap.get("sectionId").map(x => x.asInstanceOf[String]), + metadataMap.get("composerUrl").map(x => URI.create(x.asInstanceOf[String])) ) }.toOption } diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/UsageBuilder.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/UsageBuilder.scala index eae0587c2d6..06386d6575d 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/UsageBuilder.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/usage/UsageBuilder.scala @@ -57,7 +57,7 @@ object UsageBuilder { private def buildDigitalUsageReference(usage: MediaUsage): List[UsageReference] = { (usage.digitalUsageMetadata, usage.frontUsageMetadata) match { case (Some(metadata), None) => List( - UsageReference(FrontendUsageReference, Some(metadata.webUrl), Some(metadata.webTitle)) + UsageReference(FrontendUsageReference, Some(metadata.webUrl), metadata.webTitle) ) ++ metadata.composerUrl.map(url => UsageReference(ComposerUsageReference, Some(url))) case (None, Some(metadata)) => List( UsageReference(FrontUsageReference, None, name = Some(metadata.front)) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala b/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala index c7f88a248c1..d2ea8efb54b 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/model/usage/DigitalUsageMetadata.scala @@ -1,31 +1,28 @@ package com.gu.mediaservice.model.usage import com.gu.mediaservice.lib.dynamo.{DbString, DynamoElement} +import play.api.libs.json._ import java.net.URI -import play.api.libs.json._ -import org.joda.time.DateTime case class DigitalUsageMetadata ( webUrl: URI, - webTitle: String, - sectionId: String, + webTitle: Option[String], + sectionId: Option[String], composerUrl: Option[URI] = None ) extends UsageMetadata { private val placeholderWebTitle = "No title given" - private val dynamoSafeWebTitle = if(webTitle.isEmpty) placeholderWebTitle else webTitle + private val dynamoSafeWebTitle = webTitle.find(_.nonEmpty).getOrElse(placeholderWebTitle) override def toMap: Map[String, String] = Map( "webUrl" -> webUrl.toString, - "webTitle" -> dynamoSafeWebTitle, - "sectionId" -> sectionId - ) ++ composerUrl.map("composerUrl" -> _.toString) + "webTitle" -> dynamoSafeWebTitle + ) ++ sectionId.filter(_.nonEmpty).map("sectionId" -> _) ++ composerUrl.map("composerUrl" -> _.toString) override def toDynamoMap: Map[String, DynamoElement] = Map( "webUrl" -> DbString(webUrl.toString), "webTitle" -> DbString(dynamoSafeWebTitle), - "sectionId" -> DbString(sectionId) - ) ++ composerUrl.map(c => "composerUrl" -> DbString(c.toString)) + ) ++ sectionId.filter(_.nonEmpty).map("sectionId" -> DbString(_)) ++ composerUrl.map(c => "composerUrl" -> DbString(c.toString)) } object DigitalUsageMetadata { diff --git a/common-lib/src/test/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsageTest.scala b/common-lib/src/test/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsageTest.scala index e109364e7b6..2200daeecc5 100644 --- a/common-lib/src/test/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsageTest.scala +++ b/common-lib/src/test/scala/com/gu/mediaservice/lib/usage/ItemToMediaUsageTest.scala @@ -82,7 +82,7 @@ class ItemToMediaUsageTest extends AnyFunSuiteLike { "addedBy" -> "parent_editor", "childMediaId" -> "child-media-999" ).asJava - + test("testTransform dynamo v2 with simple fields") { val enchancedDoc = EnhancedDocument.builder() .attributeConverterProviders(DefaultAttributeConverterProvider.create()) @@ -167,8 +167,8 @@ class ItemToMediaUsageTest extends AnyFunSuiteLike { mediaUsage.digitalUsageMetadata shouldEqual Some( DigitalUsageMetadata( URI.create("https://www.theguardian.com/world/2026/jul/20/article"), - "Breaking News", - "world", + Some("Breaking News"), + Some("world"), Some(URI.create("https://composer.gutools.co.uk/content/123")) ) ) diff --git a/usage/app/UsageComponents.scala b/usage/app/UsageComponents.scala index d6705a5a59e..8058986e6ce 100644 --- a/usage/app/UsageComponents.scala +++ b/usage/app/UsageComponents.scala @@ -1,5 +1,3 @@ -import com.gu.contentapi.client.ScheduledExecutor -import com.amazonaws.services.dynamodbv2.AmazonDynamoDBAsyncClientBuilder import com.gu.mediaservice.lib.play.GridComponents import controllers.UsageApi import lib._ diff --git a/usage/test/model/UsageRecordTest.scala b/usage/test/model/UsageRecordTest.scala index 83aac5bd477..89c7ad82d87 100644 --- a/usage/test/model/UsageRecordTest.scala +++ b/usage/test/model/UsageRecordTest.scala @@ -201,8 +201,8 @@ class UsageRecordTest extends AnyFunSpec with Matchers { it("should correctly compile DigitalUsageMetadata with all optional fields present") { val metadata = DigitalUsageMetadata( webUrl = new URI("https://www.theguardian.com/tech"), - webTitle = "Scala 3 adoption grows", - sectionId = "technology", + webTitle = Some("Scala 3 adoption grows"), + sectionId = Some("technology"), composerUrl = Some(new URI("https://composer.internal/123")) ) @@ -228,8 +228,8 @@ class UsageRecordTest extends AnyFunSpec with Matchers { it("should fall back to placeholder when webTitle is empty and omit composerUrl when None") { val metadata = DigitalUsageMetadata( webUrl = new URI("https://www.theguardian.com/media"), - webTitle = "", - sectionId = "media", + webTitle = Some(""), + sectionId = Some("media"), composerUrl = None ) @@ -268,8 +268,8 @@ class UsageRecordTest extends AnyFunSpec with Matchers { val digitalMetadata = DigitalUsageMetadata( webUrl = new URI("https://www.theguardian.com/tech"), - webTitle = "Scala adoption grows", - sectionId = "technology", + webTitle = Some("Scala adoption grows"), + sectionId = Some("technology"), composerUrl = Some(new URI("https://composer.internal/123")) ) diff --git a/usage/test/model/UsageTableTest.scala b/usage/test/model/UsageTableTest.scala index cb5a447047a..a26d12454c0 100644 --- a/usage/test/model/UsageTableTest.scala +++ b/usage/test/model/UsageTableTest.scala @@ -156,8 +156,8 @@ class UsageTableTest extends AnyFunSpec with Matchers with GridLogging with Scal ), Some(DigitalUsageMetadata( webUrl = new URI("http://localhost/test"), - webTitle = "A page", - sectionId = "a-section" + webTitle = Some("A page"), + sectionId = Some("a-section") )), Some(SyndicationUsageMetadata( partnerName = "Test Partner", From 055cb122e98bfec6fc92b78c5eabf76a58a0c589 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 5 Mar 2026 08:20:02 +0000 Subject: [PATCH 44/58] Unused MediaWrapper class and helpers. --- usage/app/lib/MediaUsageBuilder.scala | 20 -------------------- usage/app/model/UsageGroup.scala | 11 ++--------- usage/app/model/UsageIdBuilder.scala | 5 ----- 3 files changed, 2 insertions(+), 34 deletions(-) diff --git a/usage/app/lib/MediaUsageBuilder.scala b/usage/app/lib/MediaUsageBuilder.scala index c15c3e4c143..247f34140f0 100644 --- a/usage/app/lib/MediaUsageBuilder.scala +++ b/usage/app/lib/MediaUsageBuilder.scala @@ -38,26 +38,6 @@ object MediaUsageBuilder { digitalMediaUsageRecord.dateAdded, ) - def build(mediaWrapper: MediaWrapper): MediaUsage = { - val usageId = UsageIdBuilder.build(mediaWrapper) - - MediaUsage( - usageId = usageId, - grouping = mediaWrapper.usageGroupId, - mediaId = mediaWrapper.mediaId, - DigitalUsage, - mediaType = "image", - status = mediaWrapper.contentStatus, - printUsageMetadata = None, - digitalUsageMetadata = Some(mediaWrapper.usageMetadata), - None, - None, - None, - childUsageMetadata = None, - lastModified = mediaWrapper.lastModified - ) - } - def build(syndicationUsageRequest: SyndicationUsageRequest, groupId: String): MediaUsage = { val usageId = UsageIdBuilder.build(syndicationUsageRequest) MediaUsage( diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index 8e037e1a3d6..22145c93a1d 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -1,12 +1,12 @@ package model -import play.api.libs.json._ import com.gu.contentapi.client.model.v1.{Content, Element, ElementType} import com.gu.contentatom.thrift.{Atom, AtomData} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} -import com.gu.mediaservice.model.usage.{DigitalUsageMetadata, MediaUsage, PublishedUsageStatus, UsageStatus} +import com.gu.mediaservice.model.usage.{MediaUsage, PublishedUsageStatus, UsageStatus} import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig} import org.joda.time.DateTime +import play.api.libs.json._ case class UsageGroup( usages: Set[MediaUsage], @@ -212,10 +212,3 @@ class UsageGroupOps(config: UsageConfig) }) } } - -case class MediaWrapper( - mediaId: String, - usageGroupId: String, - contentStatus: UsageStatus, - usageMetadata: DigitalUsageMetadata, - lastModified: DateTime) diff --git a/usage/app/model/UsageIdBuilder.scala b/usage/app/model/UsageIdBuilder.scala index 60a0575adcb..cef7da7e9bb 100644 --- a/usage/app/model/UsageIdBuilder.scala +++ b/usage/app/model/UsageIdBuilder.scala @@ -20,11 +20,6 @@ object UsageIdBuilder { Some(digitalMediaUsageRecord.metadata.webUrl) )) - def build(mediaWrapper: MediaWrapper) = buildId(List( - Some(mediaWrapper.mediaId), - Some(mediaWrapper.contentStatus) - )) - def build(syndicationUsageRequest: SyndicationUsageRequest) = buildId(List( Some(syndicationUsageRequest.mediaId), Some(syndicationUsageRequest.metadata.partnerName), From 31b28bc14348774f7f2c8dad7db1bb508a0e4aa3 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 5 Mar 2026 08:33:35 +0000 Subject: [PATCH 45/58] Drop Guardian Content references. --- build.sbt | 2 - usage/app/controllers/UsageApi.scala | 5 +- usage/app/lib/ContentHelpers.scala | 12 ----- usage/app/model/UsageGroup.scala | 80 +--------------------------- 4 files changed, 4 insertions(+), 95 deletions(-) delete mode 100644 usage/app/lib/ContentHelpers.scala diff --git a/build.sbt b/build.sbt index d2c2281d9b8..8444174ea2f 100644 --- a/build.sbt +++ b/build.sbt @@ -191,8 +191,6 @@ lazy val thrall = playProject("thrall", 9002) lazy val usage = playProject("usage", 9009).settings( libraryDependencies ++= Seq( - "com.gu" %% "content-api-client-default" % "32.0.0", - "com.gu" %% "content-api-client-aws" % "1.0.1", "io.reactivex" %% "rxscala" % "0.27.0", "software.amazon.kinesis" % "amazon-kinesis-client" % awsKclVersion, // explicit dependencies on kinesis and dynamodb to upgrade the versions used by kcl diff --git a/usage/app/controllers/UsageApi.scala b/usage/app/controllers/UsageApi.scala index 54c2926f2d5..db3d72dea2d 100644 --- a/usage/app/controllers/UsageApi.scala +++ b/usage/app/controllers/UsageApi.scala @@ -1,7 +1,5 @@ package controllers -import java.net.URI -import com.gu.contentapi.client.model.ItemQuery import com.gu.mediaservice.lib.argo.ArgoHelpers import com.gu.mediaservice.lib.argo.model.{EntityResponse, Link, Action => ArgoAction} import com.gu.mediaservice.lib.auth.{Authentication, Authorisation} @@ -9,7 +7,7 @@ import com.gu.mediaservice.lib.aws.UpdateMessage import com.gu.mediaservice.lib.logging.{LogMarker, MarkerMap} import com.gu.mediaservice.lib.play.RequestLoggingFilter import com.gu.mediaservice.lib.usage.UsageBuilder -import com.gu.mediaservice.model.usage.{MediaUsage, SyndicatedUsageStatus, Usage, UsageNotice, UsageStatus} +import com.gu.mediaservice.model.usage.{MediaUsage, Usage, UsageNotice, UsageStatus} import com.gu.mediaservice.syntax.MessageSubjects import lib._ import model._ @@ -18,6 +16,7 @@ import play.api.mvc._ import play.utils.UriEncoding import rx.lang.scala.Subject +import java.net.URI import scala.concurrent.{ExecutionContext, Future} import scala.util.Try diff --git a/usage/app/lib/ContentHelpers.scala b/usage/app/lib/ContentHelpers.scala deleted file mode 100644 index e7e0504aabf..00000000000 --- a/usage/app/lib/ContentHelpers.scala +++ /dev/null @@ -1,12 +0,0 @@ -package lib - -import com.gu.contentapi.client.model.v1.Content -import org.joda.time.{DateTime, DateTimeZone} - -object ContentHelpers { - def getContentFirstPublished(content: Content): Option[DateTime] = for { - fields <- content.fields - firstPublicationDate <- fields.firstPublicationDate - date = new DateTime(firstPublicationDate.iso8601, DateTimeZone.UTC) - } yield date -} diff --git a/usage/app/model/UsageGroup.scala b/usage/app/model/UsageGroup.scala index 22145c93a1d..5fd050ddad5 100644 --- a/usage/app/model/UsageGroup.scala +++ b/usage/app/model/UsageGroup.scala @@ -1,10 +1,8 @@ package model -import com.gu.contentapi.client.model.v1.{Content, Element, ElementType} -import com.gu.contentatom.thrift.{Atom, AtomData} import com.gu.mediaservice.lib.logging.{GridLogging, LogMarker} -import com.gu.mediaservice.model.usage.{MediaUsage, PublishedUsageStatus, UsageStatus} -import lib.{ContentHelpers, MD5, MediaUsageBuilder, UsageConfig} +import com.gu.mediaservice.model.usage.{MediaUsage, UsageStatus} +import lib.{MD5, MediaUsageBuilder, UsageConfig} import org.joda.time.DateTime import play.api.libs.json._ @@ -137,78 +135,4 @@ class UsageGroupOps(config: UsageConfig) usage } - private def isNewContent(content: Content, usageStatus: UsageStatus): Boolean = { - val dateLimit = new DateTime(config.usageDateLimit) - val contentFirstPublished = ContentHelpers.getContentFirstPublished(content) - usageStatus match { - case PublishedUsageStatus => contentFirstPublished.exists(_.isAfter(dateLimit)) - case _ => true - } - } - - private def filterOutAtomsWithNoImage(atoms: Seq[Atom]): Seq[Atom] = { - for { - atom <- atoms - atomId = getImageId(atom) - if atomId.isDefined - } yield atom - } - - private def getImageId(atom: Atom): Option[String] = { - try { - val posterImage = atom.data.asInstanceOf[AtomData.Media].media.posterImage - posterImage match { - case Some(image) => Some(image.mediaId.replace(s"${config.apiUri}/images/", "")) - case _ => None - } - } catch { - case e: ClassCastException => None - } - } - - private def extractCartoonUniqueMediaIds(content: Content): Set[String] = - (for { - elements <- content.elements.toSeq - cartoonElement <- elements.filter(_.`type` == ElementType.Cartoon) - asset <- cartoonElement.assets.toSeq - data <- asset.typeData.toSeq - cartoonVariants <- data.cartoonVariants.toSeq - cartoonVariant <- cartoonVariants - image <- cartoonVariant.images - mediaId <- image.mediaId - } yield mediaId).toSet - - private def extractImageElements( - content: Content, usageStatus: UsageStatus, isReindex: Boolean - )(implicit logMarker: LogMarker): Seq[Element] = { - val isNew = isNewContent(content, usageStatus) - val shouldRecordUsages = isNew || isReindex - - if (shouldRecordUsages) { - logger.info(logMarker, s"Passed shouldRecordUsages") - val groupedElements = groupImageElements(content) - - if (groupedElements.isEmpty) { - logger.info(logMarker, s"No Matching elements found") - } else { - groupedElements.foreach(elements => { - logger.info(logMarker, s"${elements.length} elements found") - elements.foreach(element => logger.info(logMarker, s"Matching element ${element.id} found")) - }) - } - - groupedElements.getOrElse(Seq.empty) - } else { - logger.info(logMarker, s"Failed shouldRecordUsages: isNew-$isNew isReindex-$isReindex") - Seq.empty - } - } - - private def groupImageElements(content: Content): Option[Seq[Element]] = { - content.elements.map(elements => { - elements.filter(_.`type` == ElementType.Image) - .groupBy(_.id) - .map(_._2.head).to(collection.immutable.Seq) - }) - } } From b7d80520a324c18f61183323dca7f9b9bbc90041 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 5 Mar 2026 08:20:15 +0000 Subject: [PATCH 46/58] Unused ResetException. --- usage/app/lib/UsageRecorder.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/usage/app/lib/UsageRecorder.scala b/usage/app/lib/UsageRecorder.scala index 1a9ed699541..da0a46f41e7 100644 --- a/usage/app/lib/UsageRecorder.scala +++ b/usage/app/lib/UsageRecorder.scala @@ -9,8 +9,6 @@ import rx.lang.scala.{Observable, Subject, Subscriber, Subscription} import scala.concurrent.duration.DurationInt -case class ResetException() extends Exception - class UsageRecorder( usageMetrics: UsageMetrics, usageTable: UsageTable, From 5a058d7f7a66dca6cf70aca7646e927862a1a591 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 26 Jun 2024 12:53:20 +0100 Subject: [PATCH 47/58] Use imgproxy rather than nginx imgops for optimised image previews. Faster and lighter. Better colour profile support. Generate an imgproxy style preview URL. Move service name from imgops to imgproxy. Disable EXIF autorotation and explicitly correct rotation. imgproxy does not accept negative rotations. Be explicit about stripping colour profile to force sRGB. --- .../gu/mediaservice/lib/config/Services.scala | 2 +- media-api/app/lib/ImageResponse.scala | 31 +++++++++++-------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala index b65c4bfee32..8bf7b282a5a 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/config/Services.scala @@ -51,7 +51,7 @@ protected class SingleHostServices(val rootUrl: String) extends Services { val metadataBaseUri: String = subpathedServiceBaseUri("metadata-editor") - val imgopsBaseUri: String = subpathedServiceBaseUri("imgops") + val imgopsBaseUri: String = subpathedServiceBaseUri("imgproxy") val usageBaseUri: String =subpathedServiceBaseUri("usage") diff --git a/media-api/app/lib/ImageResponse.scala b/media-api/app/lib/ImageResponse.scala index fb50aa33511..4721e0e8b70 100644 --- a/media-api/app/lib/ImageResponse.scala +++ b/media-api/app/lib/ImageResponse.scala @@ -11,6 +11,7 @@ import com.gu.mediaservice.model.usage._ import lib.ImageResponse.extractAliasFieldValues import lib.elasticsearch.SourceWrapper import lib.usagerights.CostCalculator +import org.apache.commons.codec.binary.Base64 import org.joda.time.DateTime import play.api.libs.functional.syntax._ import play.api.libs.json._ @@ -146,9 +147,9 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota import BoolImplicitMagic.BoolToOption val cropLinkMaybe = valid.toOption(Link("crops", s"${config.cropperUri}/crops/$id")) val editLinkMaybe = withWritePermission.toOption(Link("edits", s"${config.metadataUri}/metadata/$id")) - val optimisedPngLinkMaybe = securePngUrl map { case secureUrl => Link("optimisedPng", makeImgopsUri(new URI(secureUrl), orientationMetadata)) } + val optimisedPngLinkMaybe = securePngUrl map { case secureUrl => Link("optimisedPng", makeImgProxyUri(new URI(secureUrl), orientationMetadata)) } - val optimisedLink = Link("optimised", makeImgopsUri(new URI(secureUrl), orientationMetadata)) + val optimisedLink = Link("optimised", makeImgProxyUri(new URI(secureUrl), orientationMetadata)) val imageLink = Link("ui:image", s"${config.kahunaUri}/images/$id") val usageLink = Link("usages", s"${config.usageUri}/usages/media/$id") val leasesLink = Link("leases", s"${config.leasesUri}/leases/media/$id") @@ -255,18 +256,22 @@ class ImageResponse(config: MediaApiConfig, s3Client: S3, usageQuota: UsageQuota "aliases" -> JsObject(aliases) )) - def makeImgopsUri(uri: URI, orientationMetadata: Option[OrientationMetadata]): String = { - val resizing = config.imgopsUri + List(uri.getPath, uri.getRawQuery).mkString("?") + "{&w,h,q}" - // imgops rotates counter-clockwise - val orientationCorrectionRotation = -orientationMetadata.map(_.orientationCorrection()).getOrElse(0) - // and ignores negative values - val normalised = if (orientationCorrectionRotation < 0) { - orientationCorrectionRotation + 360 - } else { - orientationCorrectionRotation + private def makeImgProxyUri(uri: URI, orientationMetadata: Option[OrientationMetadata]): String = { + def normaliseRotation(rotation: Int) = { + // imgproxy does not accept negative rotations + if (rotation < 0) { + rotation + 360 + } else { + rotation + } } - val orientationCorrection = s"&r=" + URLEncoder.encode(normalised.toString, "UTF-8") - resizing + orientationCorrection + val base64EncodedSourceURL = new String(Base64.encodeBase64URLSafe(uri.toURL.toExternalForm.getBytes), "UTF-8") + val resizing = Seq(config.imgopsUri, "no-signature", + "auto_rotate:false", "strip_metadata:true", "strip_color_profile:true", + "resize:fit:{w}:{h}", "quality:{q}") + val orientationCorrection = orientationMetadata.map(o => Seq("rotate:" + normaliseRotation(o.orientationCorrection()))).getOrElse(Seq.empty) + val pathComponents = resizing ++ orientationCorrection :+ base64EncodedSourceURL + pathComponents.mkString("/") } private def updateCustomSpecialInstructions(source: JsValue): Reads[JsObject] = { From 53b0165ce1315a3a094ac0afbb7982ab5f0860dc Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 28 May 2024 16:57:40 +0100 Subject: [PATCH 48/58] Play base project heap size set to 40%; want more heap for given container size than default ergonomic but mindful of OOM errors. 60% was running close to the OOM kill limit on Java 11 and Java 22 has nudged it into OOM kill. 40% after media-api and collections OOM after Java 25. --- build.sbt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 8444174ea2f..48b079fb581 100644 --- a/build.sbt +++ b/build.sbt @@ -256,8 +256,10 @@ def playProject(projectName: String, port: Int, path: Option[String] = None): Pr Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", s"-Dconfig.file=/opt/docker/conf/application.conf", - s"-Dlogger.file=/opt/docker/conf/logback.xml" - ))) + s"-Dlogger.file=/opt/docker/conf/logback.xml", + "-XX:+PrintCommandLineFlags", "-XX:MaxRAMPercentage=40" + )) + ) } def playImageLoaderProject(projectName: String, port: Int, path: Option[String] = None): Project = { From 3c9b17f0c6a276bbbac8af48b6d18db535efdb4c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Wed, 14 May 2025 21:13:48 +0100 Subject: [PATCH 49/58] image loader prints java memory settings. --- build.sbt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 48b079fb581..8dcb1aa3ba4 100644 --- a/build.sbt +++ b/build.sbt @@ -291,6 +291,7 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] Universal / javaOptions ++= Seq( "-Dpidfile.path=/dev/null", s"-Dconfig.file=/opt/docker/conf/application.conf", - s"-Dlogger.file=/opt/docker/conf/logback.xml" - ))) + s"-Dlogger.file=/opt/docker/conf/logback.xml", + "-XX:+PrintCommandLineFlags" + ))) } From 2cb57affa351854be1ac2b61699221181779ec1a Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 8 Sep 2024 18:02:37 +0100 Subject: [PATCH 50/58] Cloudbuild all artifacts as 1 build. x86 images only. Multi arch is too slow (14 mins) as buildx repeats the apt-get steps. Cloudbuild uses node 24 for Kahuna build. Explicit jdk 11 build in Cloudbuild build. --- cloudbuild.yaml | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 cloudbuild.yaml diff --git a/cloudbuild.yaml b/cloudbuild.yaml new file mode 100644 index 00000000000..c6785559acc --- /dev/null +++ b/cloudbuild.yaml @@ -0,0 +1,64 @@ +options: + machineType: 'N1_HIGHCPU_8' +steps: + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'install' ] + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'run', 'dist' ] + + - name: 'gcr.io/$PROJECT_ID/scala-sbt:1.6.2-jdk-11' + args: ['docker:publishLocal'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'auth:0.1', 'eu.gcr.io/$PROJECT_ID/auth:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/auth:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'cropper:0.1', 'eu.gcr.io/$PROJECT_ID/cropper:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/cropper:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'collections:0.1', 'eu.gcr.io/$PROJECT_ID/collections:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/collections:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'image-loader:0.1', 'eu.gcr.io/$PROJECT_ID/image-loader:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/image-loader:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'kahuna:0.1', 'eu.gcr.io/$PROJECT_ID/kahuna:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/kahuna:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'leases:0.1', 'eu.gcr.io/$PROJECT_ID/leases:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/leases:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'media-api:0.1', 'eu.gcr.io/$PROJECT_ID/media-api:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/media-api:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'metadata-editor:0.1', 'eu.gcr.io/$PROJECT_ID/metadata-editor:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/metadata-editor:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'thrall:0.1', 'eu.gcr.io/$PROJECT_ID/thrall:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/thrall:$BRANCH_NAME'] + + - name: 'gcr.io/cloud-builders/docker' + args: ['tag', 'usage:0.1', 'eu.gcr.io/$PROJECT_ID/usage:$BRANCH_NAME'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/usage:$BRANCH_NAME'] From 8f1485b64713b183c0a36245e5ce1a7462e617f8 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Tue, 20 May 2025 18:30:20 +0100 Subject: [PATCH 51/58] Cloudbuild runs Kahuna tests. --- cloudbuild.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cloudbuild.yaml b/cloudbuild.yaml index c6785559acc..596bb770d99 100644 --- a/cloudbuild.yaml +++ b/cloudbuild.yaml @@ -5,6 +5,10 @@ steps: entrypoint: 'npm' dir: 'kahuna' args: [ 'install' ] + - name: 'node:24-alpine' + entrypoint: 'npm' + dir: 'kahuna' + args: [ 'run', 'test' ] - name: 'node:24-alpine' entrypoint: 'npm' dir: 'kahuna' From 3ed614fd57ef3eb5aa7e713a86d25df5d92197eb Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Mar 2026 17:57:01 +0000 Subject: [PATCH 52/58] UsageTable methods which can be made private. # Conflicts: # usage/app/model/UsageTable.scala --- usage/app/model/UsageTable.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/usage/app/model/UsageTable.scala b/usage/app/model/UsageTable.scala index 806eb52a303..8a41948f0b5 100644 --- a/usage/app/model/UsageTable.scala +++ b/usage/app/model/UsageTable.scala @@ -80,14 +80,14 @@ class UsageTable(client: DynamoDbClient, tableName: String) extends GridLogging ) } - def hidePendingIfRemoved(usages: List[MediaUsage]): List[MediaUsage] = usages.filterNot((mediaUsage: MediaUsage) => { + private def hidePendingIfRemoved(usages: List[MediaUsage]): List[MediaUsage] = usages.filterNot((mediaUsage: MediaUsage) => { mediaUsage.status match { case PendingUsageStatus => mediaUsage.isRemoved case _ => false } }) - def hidePendingIfPublished(usages: List[MediaUsage]): List[MediaUsage] = usages.groupBy(_.grouping).flatMap { + private def hidePendingIfPublished(usages: List[MediaUsage]): List[MediaUsage] = usages.groupBy(_.grouping).flatMap { case (_, groupedUsages) => val publishedUsage = groupedUsages.find(_.status match { case PublishedUsageStatus => true @@ -151,7 +151,7 @@ class UsageTable(client: DynamoDbClient, tableName: String) extends GridLogging table.deleteItem(DeleteItemEnhancedRequest.builder().key(key).build()) } - def upsertFromRecord(record: UsageRecord)(implicit logMarker: LogMarker): Observable[JsObject] = Observable.from(Future { + private def upsertFromRecord(record: UsageRecord)(implicit logMarker: LogMarker): Observable[JsObject] = Observable.from(Future { val key = Map( hashKeyName -> AttributeValue.builder().s(record.hashKey).build(), rangeKeyName -> AttributeValue.builder().s(record.rangeKey).build() From 0229ff374ca6a36da62419b3b6e98ced2b9b9168 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Mar 2026 18:34:38 +0000 Subject: [PATCH 53/58] DynamoDB private methods. --- .../src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala index d501b54e5e8..0815e6fb3ed 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/DynamoDB.scala @@ -224,7 +224,7 @@ class DynamoDB[T](client: DynamoDbClient, tableName: String, lastModifiedKey: Op object DynamoDB { - def jsonToAttributeValue(json: JsValue): AttributeValueV2 = { + private def jsonToAttributeValue(json: JsValue): AttributeValueV2 = { json match { case JsString(v) => AttributeValueV2.fromS(v) case JsBoolean(b) => AttributeValueV2.fromBool(b) @@ -266,7 +266,7 @@ object DynamoDB { // fenced in this Dynamo play area. `null` is continual and big annoyance with AWS libs. // see: https://forums.aws.amazon.com/message.jspa?messageID=389032 // see: http://docs.aws.amazon.com/amazondynamodb/latest/developerguide/DataModel.html - def mapJsValue(jsValue: JsValue)(f: JsValue => JsValue): JsValue = jsValue match { + private def mapJsValue(jsValue: JsValue)(f: JsValue => JsValue): JsValue = jsValue match { case JsObject(items) => JsObject(items.map{ case (k, v) => k -> mapJsValue(v)(f) }) case JsArray(items) => JsArray(items.map(f)) case value => f(value) From b71b075e5bc46e49fafe7d6921d6a2956befef91 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sat, 21 Mar 2026 18:43:04 +0000 Subject: [PATCH 54/58] MetadataSqsMessageConsumer.processDeletedImage is private. --- metadata-editor/app/lib/MetadataSqsMessageConsumer.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata-editor/app/lib/MetadataSqsMessageConsumer.scala b/metadata-editor/app/lib/MetadataSqsMessageConsumer.scala index 3dc414102fb..4ab13396c9b 100644 --- a/metadata-editor/app/lib/MetadataSqsMessageConsumer.scala +++ b/metadata-editor/app/lib/MetadataSqsMessageConsumer.scala @@ -14,7 +14,7 @@ class MetadataSqsMessageConsumer(config: EditsConfig, metadataEditorMetrics: Met case "image-deleted" => processDeletedImage } - def processDeletedImage(message: JsValue) = Future { + private def processDeletedImage(message: JsValue) = Future { withImageId(message)(id => store.deleteItem(id)) } } From 6713c94fcd45c91d2b3eee452460bbc9465b62a6 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 Apr 2026 12:53:16 +0100 Subject: [PATCH 55/58] Build vips-8.18.4 / JDK25 base image. --- container-images/jdk-vips/Dockerfile | 45 +++++++++++++++++++++++ container-images/jdk-vips/cloudbuild.yaml | 6 +++ 2 files changed, 51 insertions(+) create mode 100644 container-images/jdk-vips/Dockerfile create mode 100644 container-images/jdk-vips/cloudbuild.yaml diff --git a/container-images/jdk-vips/Dockerfile b/container-images/jdk-vips/Dockerfile new file mode 100644 index 00000000000..75497d31765 --- /dev/null +++ b/container-images/jdk-vips/Dockerfile @@ -0,0 +1,45 @@ +FROM eclipse-temurin:25-noble +RUN apt-get update +RUN apt-get -y --no-install-suggests install -f build-essential ninja-build python3-pip bc wget +RUN apt-get -y --no-install-suggests install meson +RUN apt -y --no-install-suggests install \ + libfftw3-dev \ + libopenexr-dev \ + libgsf-1-dev \ + libglib2.0-dev \ + liborc-dev \ + libopenslide-dev \ + libmatio-dev \ + libwebp-dev \ + libjpeg-turbo8-dev \ + libexpat1-dev \ + libexif-dev \ + libtiff5-dev \ + libcfitsio-dev \ + libpoppler-glib-dev \ + librsvg2-dev \ + libpango1.0-dev \ + libopenjp2-7-dev \ + liblcms2-dev \ + libimagequant-dev + +WORKDIR /tmp +RUN wget https://github.com/libvips/libvips/releases/download/v8.18.4/vips-8.18.4.tar.xz +RUN tar xf vips-8.18.4.tar.xz +WORKDIR /tmp/vips-8.18.4 +RUN meson setup build +WORKDIR /tmp/vips-8.18.4/build +RUN meson compile +RUN meson test +RUN meson install +RUN ldconfig + +RUN rm /tmp/vips-8.18.4.tar.xz +RUN rm -r /tmp/vips-8.18.4/ + +RUN apt -y --no-install-suggests install \ + pngquant \ + libimage-exiftool-perl \ + libjemalloc-dev \ + graphicsmagick \ + graphicsmagick-imagemagick-compat diff --git a/container-images/jdk-vips/cloudbuild.yaml b/container-images/jdk-vips/cloudbuild.yaml new file mode 100644 index 00000000000..37a0861ac97 --- /dev/null +++ b/container-images/jdk-vips/cloudbuild.yaml @@ -0,0 +1,6 @@ +steps: + - name: 'gcr.io/cloud-builders/docker' + args: ['build', '-t', 'eu.gcr.io/$PROJECT_ID/jdk-vips:25-8.18.4', '.'] + - name: 'gcr.io/cloud-builders/docker' + args: ['push', 'eu.gcr.io/$PROJECT_ID/jdk-vips:25-8.18.4'] + From 5c236e33023be324a3fce2b5e70497b6195d6523 Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Sun, 5 Apr 2026 12:53:46 +0100 Subject: [PATCH 56/58] Use vips-8.18.4 base image for playImageLoaderProject. --- build.sbt | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/build.sbt b/build.sbt index 8dcb1aa3ba4..3169c231363 100644 --- a/build.sbt +++ b/build.sbt @@ -267,15 +267,10 @@ def playImageLoaderProject(projectName: String, port: Int, path: Option[String] .enablePlugins(PlayScala, BuildInfoPlugin, DockerPlugin) .dependsOn(restLib) .settings(commonSettings ++ buildInfo ++ Seq( - dockerBaseImage := "eclipse-temurin:11", + dockerBaseImage := "eu.gcr.io/grid-301122/jdk-vips:25-8.18.4", dockerExposedPorts := Seq(port), dockerCommands ++= Seq( - Cmd("USER", "root"), Cmd("RUN", "apt-get", "update"), - Cmd("RUN", "apt-get", "install", "-y", "apt-utils"), - Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick"), - Cmd("RUN", "apt-get", "install", "-y", "graphicsmagick-imagemagick-compat"), - Cmd("RUN", "apt-get", "install", "-y", "pngquant"), - Cmd("RUN", "apt-get", "install", "-y", "libimage-exiftool-perl") + Cmd("ENV", "LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libjemalloc.so") ), playDefaultPort := port, From f8e6e30212fbe87508aca7605dbb77aad225039c Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 21 Nov 2024 21:53:47 +0000 Subject: [PATCH 57/58] [containerised] We only need Environment credentials; may speed up first hit. --- .../com/gu/mediaservice/lib/aws/AwsClientBuilderUtils.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientBuilderUtils.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientBuilderUtils.scala index 14040d0c33b..34cc2672ae3 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientBuilderUtils.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/AwsClientBuilderUtils.scala @@ -1,7 +1,7 @@ package com.gu.mediaservice.lib.aws import com.gu.mediaservice.lib.logging.GridLogging -import software.amazon.awssdk.auth.credentials.{AwsCredentialsProvider, DefaultCredentialsProvider} +import software.amazon.awssdk.auth.credentials.{AwsCredentialsProvider, DefaultCredentialsProvider, EnvironmentVariableCredentialsProvider} import software.amazon.awssdk.awscore.client.builder.AwsClientBuilder import software.amazon.awssdk.regions.Region From 8c9f198adac2f1fec73a4f2f41659cb7c4a04c1f Mon Sep 17 00:00:00 2001 From: Tony McCrae Date: Thu, 27 Aug 2026 19:37:53 +0100 Subject: [PATCH 58/58] Nerf EC2MetadataUtils.getInstanceId which has no meaning in a containerised world. --- .../scala/com/gu/mediaservice/lib/aws/EC2MetadataUtils.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/EC2MetadataUtils.scala b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/EC2MetadataUtils.scala index 1249441b240..31d33a65b59 100644 --- a/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/EC2MetadataUtils.scala +++ b/common-lib/src/main/scala/com/gu/mediaservice/lib/aws/EC2MetadataUtils.scala @@ -6,8 +6,6 @@ import scala.util.Using object EC2MetadataUtils { - def getInstanceId: Option[String] = Using(Ec2MetadataClient.create()) { client => - client.get("/latest/meta-data/instance-id").asString() - }.toOption + def getInstanceId: Option[String] = None }