diff --git a/build.sbt b/build.sbt index 2f9690db..a85ca02e 100644 --- a/build.sbt +++ b/build.sbt @@ -81,6 +81,7 @@ def faciaJson(playJsonVersion: PlayJsonVersion) = playJsonSpecificProject("facia def fapiClient(playJsonVersion: PlayJsonVersion) = playJsonSpecificProject("fapi-client", playJsonVersion) .settings( libraryDependencies ++= Seq( + eTagCachingS3SdkV2, contentApi, contentApiDefault, commercialShared, diff --git a/facia-json/src/main/scala/com/gu/facia/client/AmazonSdkS3Client.scala b/facia-json/src/main/scala/com/gu/facia/client/AmazonSdkS3Client.scala index 73403fb9..6f67b033 100644 --- a/facia-json/src/main/scala/com/gu/facia/client/AmazonSdkS3Client.scala +++ b/facia-json/src/main/scala/com/gu/facia/client/AmazonSdkS3Client.scala @@ -9,7 +9,6 @@ import scala.concurrent.{ExecutionContext, Future, blocking} import scala.util.Try - case class AmazonSdkS3Client(client: AmazonS3)(implicit executionContext: ExecutionContext) extends S3Client { def get(bucket: String, path: String): Future[FaciaResult] = Future { blocking { diff --git a/facia-json/src/main/scala/com/gu/facia/client/ApiClient.scala b/facia-json/src/main/scala/com/gu/facia/client/ApiClient.scala index 7231f36a..e2bda2ee 100644 --- a/facia-json/src/main/scala/com/gu/facia/client/ApiClient.scala +++ b/facia-json/src/main/scala/com/gu/facia/client/ApiClient.scala @@ -18,6 +18,9 @@ trait ApiClient { object ApiClient { private val Encoding = "utf-8" + /** + * Legacy constructor for creating a client that does not support caching. Use `ApiClient.withCaching()` instead. + */ def apply( bucket: String, environment: String, // e.g., CODE, PROD, DEV ... @@ -51,8 +54,8 @@ object ApiClient { def withCaching( bucket: String, - s3Fetching: Fetching[ObjectId, Array[Byte]], // Eg S3ObjectFetching(s3AsyncClient, Bytes).mapResponse(_.asByteArray()) environment: Environment, + s3Fetching: Fetching[ObjectId, Array[Byte]], // Eg S3ObjectFetching(s3AsyncClient, Bytes).mapResponse(_.asByteArray()) configureCollectionCache: ConfigCache = _.maximumSize(10000) // at most 1GB RAM worst case )(implicit ec: ExecutionContext): ApiClient = new ApiClient { diff --git a/facia-json/src/test/scala/com/gu/facia/client/ApiClientSpec.scala b/facia-json/src/test/scala/com/gu/facia/client/ApiClientSpec.scala index 91f499a2..38ddcf48 100644 --- a/facia-json/src/test/scala/com/gu/facia/client/ApiClientSpec.scala +++ b/facia-json/src/test/scala/com/gu/facia/client/ApiClientSpec.scala @@ -1,33 +1,58 @@ package com.gu.facia.client +import com.gu.etagcaching.aws.s3.ObjectId +import com.gu.etagcaching.fetching.{ETaggedData, Fetching, Missing, MissingOrETagged} import com.gu.facia.client.lib.ResourcesHelper import org.scalatest.OptionValues import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures} import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.Future +import scala.concurrent.{ExecutionContext, Future} +import scala.util.hashing.MurmurHash3 + +object FakeS3Fetching extends Fetching[ObjectId, Array[Byte]] with ResourcesHelper { + private def pretendETagFor(bytes: Array[Byte]): String = MurmurHash3.bytesHash(bytes).toHexString + + override def fetch(objectId: ObjectId)(implicit ec: ExecutionContext): Future[MissingOrETagged[Array[Byte]]] = Future { + slurpBytes(objectId.key).fold(Missing: MissingOrETagged[Array[Byte]]) { bytes => + ETaggedData(pretendETagFor(bytes), bytes) + } + } + + override def fetchOnlyIfETagChanged(objectId: ObjectId, oldETag: String)(implicit ec: ExecutionContext): Future[Option[MissingOrETagged[Array[Byte]]]] = { + fetch(objectId).map { + case taggedData: ETaggedData[_] => + Option.unless(oldETag == taggedData.eTag)(taggedData) // simulate a Not-Modified response, if there's no change in ETag + case x => Some(x) + } + } +} class ApiClientSpec extends AnyFlatSpec with Matchers with OptionValues with ScalaFutures with IntegrationPatience { + import scala.concurrent.ExecutionContext.Implicits.global + object FakeS3Client extends S3Client with ResourcesHelper { override def get(bucket: String, path: String): Future[FaciaResult] = Future { slurpOrDie(path) } } - val client: ApiClient = ApiClient("not used", "DEV", FakeS3Client) + val legacyClient: ApiClient = ApiClient("not used", "DEV", FakeS3Client) + val cachingClient: ApiClient = ApiClient.withCaching("not used", Environment.Dev, FakeS3Fetching) - "ApiClient" should "fetch the config" in { - val config = client.config.futureValue + for ((name, client) <- Map("legacy" -> legacyClient, "caching" -> cachingClient)) { + s"$name ApiClient" should "fetch the config" in { + val config = client.config.futureValue - config.collections should have size 334 - config.fronts should have size 79 - } + config.collections should have size 334 + config.fronts should have size 79 + } - it should "fetch a collection" in { - val collectionOpt = client.collection("2409-31b3-83df0-de5a").futureValue + it should "fetch a collection" in { + val collectionOpt = cachingClient.collection("2409-31b3-83df0-de5a").futureValue - collectionOpt.value.live should have size 8 + collectionOpt.value.live should have size 8 + } } } \ No newline at end of file diff --git a/facia-json/src/test/scala/com/gu/facia/client/lib/ApiTestClient.scala b/facia-json/src/test/scala/com/gu/facia/client/lib/ApiTestClient.scala index 1f139d42..5d68c362 100644 --- a/facia-json/src/test/scala/com/gu/facia/client/lib/ApiTestClient.scala +++ b/facia-json/src/test/scala/com/gu/facia/client/lib/ApiTestClient.scala @@ -1,19 +1,12 @@ package com.gu.facia.client.lib -import com.amazonaws.auth.{AWSStaticCredentialsProvider, BasicAWSCredentials} -import com.amazonaws.regions.Regions -import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder} import com.gu.facia.client.models.{CollectionJson, ConfigJson} -import com.gu.facia.client.{AmazonSdkS3Client, ApiClient, Environment} +import com.gu.facia.client.{ApiClient, Environment} import play.api.libs.json.{Format, Json} import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future -object Amazon { - val amazonS3Client = AmazonS3ClientBuilder.standard().withRegion(Regions.EU_WEST_1).withCredentials(new AWSStaticCredentialsProvider(new BasicAWSCredentials("key", "pass"))).build() -} - class ApiTestClient extends ApiClient with ResourcesHelper { private val environment = Environment.Dev diff --git a/facia-json/src/test/scala/com/gu/facia/client/lib/ResourcesHelper.scala b/facia-json/src/test/scala/com/gu/facia/client/lib/ResourcesHelper.scala index 4a0f1e34..c862aed3 100644 --- a/facia-json/src/test/scala/com/gu/facia/client/lib/ResourcesHelper.scala +++ b/facia-json/src/test/scala/com/gu/facia/client/lib/ResourcesHelper.scala @@ -1,11 +1,17 @@ package com.gu.facia.client.lib import com.gu.facia.client.FaciaSuccess +import org.apache.commons.io.IOUtils + +import java.nio.charset.StandardCharsets.UTF_8 trait ResourcesHelper { + def slurpBytes(path: String): Option[Array[Byte]] = + Option(getClass.getClassLoader.getResource(path)).map(url => IOUtils.toByteArray(url.openStream())) + def slurp(path: String): Option[String] = - Option(getClass.getClassLoader.getResource(path)).map(scala.io.Source.fromURL(_).mkString) + slurpBytes(path).map(bytes => new String(bytes, UTF_8)) - def slurpOrDie(path: String) = slurp(path).map(_.getBytes).map(FaciaSuccess.apply).getOrElse { + def slurpOrDie(path: String) = slurpBytes(path).map(FaciaSuccess.apply).getOrElse { throw new RuntimeException(s"Required resource $path not on class path") } } \ No newline at end of file diff --git a/fapi-client-core/src/main/scala/com/gu/facia/client/S3Client.scala b/fapi-client-core/src/main/scala/com/gu/facia/client/S3Client.scala index e0537d52..ed741f1c 100644 --- a/fapi-client-core/src/main/scala/com/gu/facia/client/S3Client.scala +++ b/fapi-client-core/src/main/scala/com/gu/facia/client/S3Client.scala @@ -2,8 +2,14 @@ package com.gu.facia.client import scala.concurrent.Future -/** For mocking in tests, but also to allow someone to define a properly asynchronous S3 client. (The one in the AWS - * SDK is unfortunately synchronous only.) +/** + * Legacy class for mocking in tests, but also previously used to allow library users to define a properly + * asynchronous S3 client (back when the one in the AWS SDK was synchronous only). + * + * Note that use of `S3Client` is now discouraged, as `facia-scala-client` now supports caching using the + * `etag-caching` library, which provides its own more powerful abstraction for fetching & parsing data, and + * `com.gu.etagcaching.aws.sdkv2.s3.S3ObjectFetching`, a ready-made implementation of fetching that includes + * support for ETags & Conditional Requests, decoding S3 exceptions, etc. */ trait S3Client { def get(bucket: String, path: String): Future[FaciaResult] diff --git a/fapi-client/src/test/scala/lib/IntegrationTestConfig.scala b/fapi-client/src/test/scala/lib/IntegrationTestConfig.scala index 81697e67..444a7353 100644 --- a/fapi-client/src/test/scala/lib/IntegrationTestConfig.scala +++ b/fapi-client/src/test/scala/lib/IntegrationTestConfig.scala @@ -1,11 +1,16 @@ package lib -import com.amazonaws.auth._ -import com.amazonaws.auth.profile.ProfileCredentialsProvider -import com.amazonaws.services.s3.AmazonS3ClientBuilder +//import com.amazonaws.auth._ +import software.amazon.awssdk.auth.credentials.{EnvironmentVariableCredentialsProvider, ProfileCredentialsProvider} +// import com.amazonaws.auth.profile.ProfileCredentialsProvider +//import com.amazonaws.services.s3.AmazonS3ClientBuilder import com.gu.contentapi.client.GuardianContentClient -import com.gu.facia.client.{AmazonSdkS3Client, ApiClient} -import com.amazonaws.regions.Regions.EU_WEST_1 +import com.gu.etagcaching.aws.sdkv2.s3.S3ObjectFetching +import com.gu.etagcaching.aws.sdkv2.s3.response.Transformer.Bytes +import com.gu.facia.client.{ApiClient, Environment} +import software.amazon.awssdk.auth.credentials.{AwsCredentialsProvider, AwsCredentialsProviderChain} +import software.amazon.awssdk.regions.Region +import software.amazon.awssdk.services.s3.S3AsyncClient private case class TestContentApiClient(override val apiKey: String, override val targetUrl: String) extends GuardianContentClient(apiKey) @@ -22,16 +27,14 @@ trait IntegrationTestConfig extends ExecutionContext { } } - implicit val apiClient: ApiClient = { - val credentialsProvider = new AWSCredentialsProviderChain( - new EnvironmentVariableCredentialsProvider(), - new SystemPropertiesCredentialsProvider(), - new ProfileCredentialsProvider(awsProfileName) - ) - val amazonS3Client = AmazonS3ClientBuilder.standard() - .withRegion(EU_WEST_1) - .withCredentials(credentialsProvider) - .build() - ApiClient("facia-tool-store", "DEV", AmazonSdkS3Client(amazonS3Client)) - } + def credentialsForDevAndCI(devProfile: String, ciCreds: AwsCredentialsProvider): AwsCredentialsProviderChain = + AwsCredentialsProviderChain.of(ciCreds, ProfileCredentialsProvider.builder().profileName(devProfile).build()) + + implicit val apiClient: ApiClient = ApiClient.withCaching( + "facia-tool-store", + Environment.Dev, + S3ObjectFetching(S3AsyncClient.builder() + .region(Region.EU_WEST_1) + .credentialsProvider(credentialsForDevAndCI(awsProfileName, EnvironmentVariableCredentialsProvider.create())).build(), Bytes).mapResponse(_.asByteArray()) + ) } diff --git a/project/dependencies.scala b/project/dependencies.scala index 0e280507..d9266f1b 100644 --- a/project/dependencies.scala +++ b/project/dependencies.scala @@ -2,7 +2,7 @@ import sbt._ object Dependencies { val capiVersion = "31.0.2" - val eTagCachingVersion = "4.0.0-PREVIEW.suppress-caffeine-exceptions-by-allowing-fetch-results-to-indicate-missing-keys.2024-07-09T1701.cbc4304a" + val eTagCachingVersion = "4.0.0" val awsS3SdkV1 = "com.amazonaws" % "aws-java-sdk-s3" % "1.12.673" val eTagCachingS3Base = "com.gu.etag-caching" %% "aws-s3-base" % eTagCachingVersion