From 1081f7e60ac2e3283a9e25120e5047f3c1845605 Mon Sep 17 00:00:00 2001
From: Ioanna Kokkini <ioannakok@hotmail.com>
Date: Thu, 9 Jun 2022 16:49:51 +0100
Subject: [PATCH] Update CAPI & FAPI clients (also remove Lightbox hack)

The latest version of the CAPI client (v19) includes
https://github.com/guardian/content-api-scala-client/pull/359,
which we can use to replace the pagination code based on `play-iteratees`
(https://github.com/playframework/play-iteratees - introduced in July 2014
with https://github.com/guardian/frontend/pull/5199), currently blocking the
Scala 2.13 upgrade for Frontend (see https://github.com/guardian/frontend/issues/24817).

The updated CAPI client is binary-incompatible with previous versions of
the CAPI client, meaning we also need to update the FAPI client as it also
uses CAPI.

The updates to the CAPI client also support `previous`/`next` search
functionality, as used by the Lightbox, meaning that the small
`ContentApiNavQuery` hack for the Lightbox (introduced here:
https://github.com/guardian/frontend/pull/20462) can now be removed.

A similar PR updating FAPI & CAPI in Ophan is at
https://github.com/guardian/ophan/pull/4719.

Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>

We're adding a test that will break at build time rather than runtime if
our versions of the FAPI & CAPI clients are incompatible.

See also:

* https://github.com/guardian/frontend/pull/25139#issuecomment-1163407402
* https://github.com/guardian/ophan/pull/4719/files#diff-ee7f97c92065084bba37d70d043ad0daa0d7745f235d0ad3206b59f073829529
---
 .../controllers/ImageContentController.scala  | 20 ++++--------
 .../test/frontpress/FaciaClientTest.scala     | 31 +++++++++++++++++++
 project/Dependencies.scala                    |  4 +--
 3 files changed, 39 insertions(+), 16 deletions(-)
 create mode 100644 facia-press/test/frontpress/FaciaClientTest.scala

diff --git a/applications/app/controllers/ImageContentController.scala b/applications/app/controllers/ImageContentController.scala
index a324a08b82d6..f061c3b4a876 100644
--- a/applications/app/controllers/ImageContentController.scala
+++ b/applications/app/controllers/ImageContentController.scala
@@ -1,7 +1,6 @@
 package controllers
 
-import com.gu.contentapi.client.Parameter
-import com.gu.contentapi.client.model.SearchQueryBase
+import com.gu.contentapi.client.model.{Direction, FollowingSearchQuery, SearchQuery, SearchQueryBase}
 import com.gu.contentapi.client.model.v1.{ItemResponse, Content => ApiContent}
 import common._
 import conf.switches.Switches
@@ -53,11 +52,11 @@ class ImageContentController(
 
   def getNextLightboxJson(path: String, tag: String, direction: String): Action[AnyContent] =
     Action.async { implicit request =>
-      val capiQuery: ContentApiNavQuery = ContentApiNavQuery(currentId = path, direction = direction)
-        .tag(tag)
-        .showTags("all")
-        .showElements("image")
-        .pageSize(contentApi.nextPreviousPageSize)
+      val capiQuery = FollowingSearchQuery(
+        SearchQuery().tag(tag).showTags("all").showElements("image").pageSize(contentApi.nextPreviousPageSize),
+        path,
+        Direction.forPathSegment(direction),
+      )
 
       contentApiClient.thriftClient.getResponse(capiQuery).map { response =>
         val lightboxJson = response.results.flatMap(result =>
@@ -70,10 +69,3 @@ class ImageContentController(
       }
     }
 }
-
-case class ContentApiNavQuery(parameterHolder: Map[String, Parameter] = Map.empty, currentId: String, direction: String)
-    extends SearchQueryBase[ContentApiNavQuery] {
-  def withParameters(parameterMap: Map[String, Parameter]): ContentApiNavQuery = copy(parameterMap)
-
-  override def pathSegment: String = s"content/$currentId/$direction"
-}
diff --git a/facia-press/test/frontpress/FaciaClientTest.scala b/facia-press/test/frontpress/FaciaClientTest.scala
new file mode 100644
index 000000000000..2a8391974a4e
--- /dev/null
+++ b/facia-press/test/frontpress/FaciaClientTest.scala
@@ -0,0 +1,31 @@
+package frontpress
+
+import com.gu.contentapi.client.ContentApiClient
+import com.gu.contentapi.client.model.SearchQuery
+import org.scalatest.flatspec._
+import org.scalatest.matchers.should.Matchers
+import org.scalatestplus.mockito.MockitoSugar
+
+class FaciaClientTest extends AsyncFlatSpec with Matchers with MockitoSugar {
+
+  "FAPI Client" should "send a CAPI client request without a runtime error like java.lang.NoSuchMethodError" in {
+    // The FAPI client uses the Content API client. If this test fails with a java.lang.NoSuchMethodError, the
+    // versions of FAPI client and CAPI client we are using are incompatible - the FAPI client will have been
+    // compiled against a different version of the CAPI client.
+
+    val mockContentApiClient = mock[ContentApiClient]
+
+    // This is only exercising the code to send the request, not recieve the result, but it's enough to trigger the
+    // java.lang.NoSuchMethodError seen in https://github.com/guardian/frontend/pull/25139#issuecomment-1163407402
+    noException shouldBe thrownBy(
+      com.gu.facia.api.contentapi.ContentApi.getHydrateResponse(
+        mockContentApiClient,
+        Seq(
+          SearchQuery().tag("profile/roberto-tyley"),
+          SearchQuery().tag("stage/comedy"),
+        ),
+      ),
+    )
+  }
+
+}
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index fc6d6b33f7b6..6a1be2aa3eca 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -5,8 +5,8 @@ import sbt._
 object Dependencies {
   val identityLibVersion = "3.254"
   val awsVersion = "1.12.205"
-  val capiVersion = "18.0.1"
-  val faciaVersion = "3.3.12"
+  val capiVersion = "19.0.0"
+  val faciaVersion = "4.0.0"
   val dispatchVersion = "0.13.1"
   val romeVersion = "1.0"
   val jerseyVersion = "1.19.4"