Skip to content

Commit

Permalink
Update CAPI & FAPI clients (also remove Lightbox hack)
Browse files Browse the repository at this point in the history
The latest version of the CAPI client (v19) includes
guardian/content-api-scala-client#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 #5199), currently blocking the
Scala 2.13 upgrade for Frontend (see #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:
#20462) can now be removed.

A similar PR updating FAPI & CAPI in Ophan is at
guardian/ophan#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:

* #25139 (comment)
* https://github.com/guardian/ophan/pull/4719/files#diff-ee7f97c92065084bba37d70d043ad0daa0d7745f235d0ad3206b59f073829529
  • Loading branch information
ioannakok authored and rtyley committed Jun 30, 2022
1 parent 555fc4c commit 1081f7e
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 16 deletions.
20 changes: 6 additions & 14 deletions applications/app/controllers/ImageContentController.scala
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 =>
Expand All @@ -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"
}
31 changes: 31 additions & 0 deletions facia-press/test/frontpress/FaciaClientTest.scala
Original file line number Diff line number Diff line change
@@ -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"),
),
),
)
}

}
4 changes: 2 additions & 2 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 1081f7e

Please sign in to comment.