Skip to content

Commit

Permalink
Use JsValue not String for DCAR
Browse files Browse the repository at this point in the history
Frontend posts the data for articles, fronts, tags pages and so on to DCAR as JSON, using `WSClient`. At the moment the following steps are taken to transform from an instance of a case class in frontend to the format that `WSClient` uses (`BodyWritable`):

```
case class instance -> JsValue -> String -> BodyWritable
```

The final transformation here, `String -> BodyWritable`, is handled by `WSClient`, as it defines an implicit way to get from one type to the other. However, it also defines a way to do this for a `play-json` `JsValue`, so we do not need to convert to a `String` beforehand. We should be able to pass a `JsValue` to `WSClient` directly, removing one of the transformation steps:

```
case class instance -> JsValue -> BodyWritable
```

The way to achieve this is by modifying our methods that use `WSClient.post` to take a `JsValue` payload instead of a `String`. We are then able to update the `toJson` methods on each of our models to remove the `Json.stringify` step. However, aside from `RemoteRender` requests, these `toJson` methods are also used to provide for `JsonFormat` requests via the `renderJson` method, which at the moment requires a `String`. This method, though, makes use of Play's `Writeable`, which also includes an implicit transformation to get from a `play-json` `JsValue`. So we can update it to take a `JsValue` instead, and save the additional `String` transformation step here too.

Note that there is one exception where we're using `circe` instead of `play-json`, for crosswords, so in that case we're transforming from one to the other via `String`.
  • Loading branch information
JamieB-gu committed Dec 3, 2024
1 parent 46b6cf6 commit dcd3e5c
Show file tree
Hide file tree
Showing 13 changed files with 36 additions and 28 deletions.
3 changes: 2 additions & 1 deletion applications/app/controllers/CrosswordsController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import org.joda.time.{DateTime, LocalDate}
import pages.{CrosswordHtmlPage, IndexHtmlPage, PrintableCrosswordHtmlPage}
import play.api.data.Forms._
import play.api.data._
import play.api.libs.json.JsValue
import play.api.libs.ws.WSClient
import play.api.mvc._
import renderers.DotcomRenderingService
Expand Down Expand Up @@ -117,7 +118,7 @@ class CrosswordPageController(
}
private def getDCRJson(crosswordPage: CrosswordPageWithContent, pageType: PageType)(implicit
request: RequestHeader,
): String =
): JsValue =
DotcomRenderingDataModel.toJson(
DotcomRenderingDataModel.forCrossword(crosswordPage, request, pageType),
)
Expand Down
3 changes: 2 additions & 1 deletion applications/app/controllers/GalleryController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import contentapi.ContentApiClient
import model._
import model.dotcomrendering.{DotcomRenderingDataModel, PageType}
import pages.GalleryHtmlPage
import play.api.libs.json.JsValue
import play.api.libs.ws.WSClient
import play.api.mvc._
import play.twirl.api.Html
Expand Down Expand Up @@ -74,7 +75,7 @@ class GalleryController(

private def getDCRJson(galleryPage: GalleryPage, pageType: PageType, blocks: Blocks)(implicit
request: RequestHeader,
): String = {
): JsValue = {
DotcomRenderingDataModel.toJson(DotcomRenderingDataModel.forGallery(galleryPage, request, pageType, blocks))
}

Expand Down
2 changes: 1 addition & 1 deletion applications/app/controllers/ImageContentController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ImageContentController(

private def getDCRJson(content: ImageContentPage, pageType: PageType, mainBlock: Option[Block])(implicit
request: RequestHeader,
): String = {
): JsValue = {
DotcomRenderingDataModel.toJson(DotcomRenderingDataModel.forImageContent(content, request, pageType, mainBlock))
}

Expand Down
4 changes: 2 additions & 2 deletions applications/app/controllers/MediaController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import implicits.{AppsFormat, JsonFormat}
import model._
import model.dotcomrendering.{DotcomRenderingDataModel, PageType}
import pages.ContentHtmlPage
import play.api.libs.json.{Format, JsObject, Json}
import play.api.libs.json.{Format, JsObject, Json, JsValue}
import play.api.libs.ws.WSClient
import play.api.mvc._
import renderers.DotcomRenderingService
Expand Down Expand Up @@ -96,7 +96,7 @@ class MediaController(

private def getDCRJson(content: MediaPage, pageType: PageType, blocks: Blocks)(implicit
request: RequestHeader,
): String = {
): JsValue = {
DotcomRenderingDataModel.toJson(DotcomRenderingDataModel.forMedia(content, request, pageType, blocks))
}

Expand Down
4 changes: 2 additions & 2 deletions article/app/controllers/ArticleController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import model.Cached.{RevalidatableResult, WithoutRevalidationResult}
import model.dotcomrendering.{DotcomRenderingDataModel, PageType}
import model._
import pages.{ArticleEmailHtmlPage, ArticleHtmlPage}
import play.api.libs.json.Json
import play.api.libs.json.{Json, JsValue}
import play.api.libs.ws.WSClient
import play.api.mvc._
import renderers.DotcomRenderingService
Expand Down Expand Up @@ -91,7 +91,7 @@ class ArticleController(

/** Returns a JSON representation of the payload that's sent to DCR when rendering the Article.
*/
private def getDCRJson(article: ArticlePage, blocks: Blocks)(implicit request: RequestHeader): String = {
private def getDCRJson(article: ArticlePage, blocks: Blocks)(implicit request: RequestHeader): JsValue = {
val pageType: PageType = PageType(article, request, context)
val newsletter = newsletterService.getNewsletterForArticle(article)

Expand Down
7 changes: 5 additions & 2 deletions common/app/common/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import model.CacheTime.RecentlyUpdated
import model.Cached.RevalidatableResult
import model.{ApplicationContext, Cached, NoCache}
import play.api.Logger
import play.api.libs.json.{JsObject, JsString}
import play.api.libs.json.{JsObject, JsString, JsValue}
import play.api.mvc.{RequestHeader, Result}
import play.twirl.api.Html
import model.ApplicationContext
Expand Down Expand Up @@ -162,7 +162,10 @@ object `package`
JsonComponent(page, json)
}

def renderJson(json: String, page: model.Page)(implicit request: RequestHeader, context: ApplicationContext): Result =
def renderJson(json: JsValue, page: model.Page)(implicit
request: RequestHeader,
context: ApplicationContext,
): Result =
Cached(page) {
RevalidatableResult.Ok(json)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ object DotcomBlocksRenderingDataModel {
}
}

def toJson(model: DotcomBlocksRenderingDataModel): String = {
def toJson(model: DotcomBlocksRenderingDataModel): JsValue = {
val jsValue = Json.toJson(model)
Json.stringify(DotcomRenderingUtils.withoutNull(jsValue))
DotcomRenderingUtils.withoutNull(jsValue)
}

def apply(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ object DotcomFrontsRenderingDataModel {
)
}

def toJson(model: DotcomFrontsRenderingDataModel): String = {
def toJson(model: DotcomFrontsRenderingDataModel): JsValue = {
val jsValue = Json.toJson(model)
Json.stringify(DotcomRenderingUtils.withoutNull(jsValue))
DotcomRenderingUtils.withoutNull(jsValue)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ object DotcomNewslettersPageRenderingDataModel {
)
}

def toJson(model: DotcomNewslettersPageRenderingDataModel): String = {
def toJson(model: DotcomNewslettersPageRenderingDataModel): JsValue = {
val jsValue = Json.toJson(model)
Json.stringify(DotcomRenderingUtils.withoutNull(jsValue))
DotcomRenderingUtils.withoutNull(jsValue)
}

private def convertNewsletterResponseToData(response: NewsletterResponseV2): NewsletterData = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ object DotcomRenderingDataModel {
}
}

def toJson(model: DotcomRenderingDataModel): String = {
def toJson(model: DotcomRenderingDataModel): JsValue = {
val jsValue = Json.toJson(model)
Json.stringify(withoutNull(jsValue))
withoutNull(jsValue)
}

def forInteractive(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ object DotcomTagPagesRenderingDataModel {
)
}

def toJson(model: DotcomTagPagesRenderingDataModel): String = {
def toJson(model: DotcomTagPagesRenderingDataModel): JsValue = {
val jsValue = Json.toJson(model)
Json.stringify(DotcomRenderingUtils.withoutNull(jsValue))
DotcomRenderingUtils.withoutNull(jsValue)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,21 @@ package model.dotcomrendering.pageElements

import com.gu.contentapi.client.model.v1.Crossword
import com.gu.contentapi.json.CirceEncoders._
import io.circe.JsonObject
import io.circe.syntax._
import model.dotcomrendering.DotcomRenderingUtils
import play.api.libs.json.{JsObject, Json, JsValue}

case class EditionsCrosswordRenderingDataModel(
quick: Crossword,
cryptic: Crossword,
)

object EditionsCrosswordRenderingDataModel {
def toJson(model: EditionsCrosswordRenderingDataModel): String = {
JsonObject(
"quick" -> model.quick.asJson.dropNullValues,
"cryptic" -> model.cryptic.asJson.dropNullValues,
).asJson.dropNullValues.noSpaces
}
def toJson(model: EditionsCrosswordRenderingDataModel): JsValue =
DotcomRenderingUtils.withoutNull(
Json.obj(
"quick" -> Json.parse(model.quick.asJson.toString()),
"cryptic" -> Json.parse(model.cryptic.asJson.toString()),
),
)
}
5 changes: 3 additions & 2 deletions common/app/renderers/DotcomRenderingService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import model.{
RelatedContentItem,
SimplePage,
}
import play.api.libs.json.JsValue
import play.api.libs.ws.{WSClient, WSResponse}
import play.api.mvc.Results.{InternalServerError, NotFound}
import play.api.mvc.{RequestHeader, Result}
Expand Down Expand Up @@ -56,7 +57,7 @@ class DotcomRenderingService extends GuLogging with ResultWithPreconnectPreload

private[this] def postWithoutHandler(
ws: WSClient,
payload: String,
payload: JsValue,
endpoint: String,
timeout: Duration = Configuration.rendering.timeout,
)(implicit request: RequestHeader): Future[WSResponse] = {
Expand Down Expand Up @@ -93,7 +94,7 @@ class DotcomRenderingService extends GuLogging with ResultWithPreconnectPreload

private[this] def post(
ws: WSClient,
payload: String,
payload: JsValue,
endpoint: String,
cacheTime: CacheTime,
timeout: Duration = Configuration.rendering.timeout,
Expand Down

0 comments on commit dcd3e5c

Please sign in to comment.