Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use JsValue Not String For DCAR #27641

Merged
merged 4 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion applications/app/controllers/CrosswordsController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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 @@ -124,7 +125,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
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,9 +2,10 @@ 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 implicits.Dates.CapiRichDateTime
import model.dotcomrendering.DotcomRenderingUtils
import play.api.libs.json.{JsObject, Json, JsValue}

case class EditionsCrosswordRenderingDataModel(
crosswords: Iterable[Crossword],
Expand All @@ -25,9 +26,10 @@ object EditionsCrosswordRenderingDataModel {
}
}))

def toJson(model: EditionsCrosswordRenderingDataModel): String = {
JsonObject(
"crosswords" -> model.crosswords.asJson.deepDropNullValues,
).asJson.noSpaces
}
def toJson(model: EditionsCrosswordRenderingDataModel): JsValue =
DotcomRenderingUtils.withoutNull(
Json.obj(
"crosswords" -> Json.parse(model.crosswords.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
3 changes: 2 additions & 1 deletion common/test/renderers/DotcomRenderingServiceTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover, PrivateMethodTester}
import org.scalatest.matchers.should.Matchers
import test.{ConfiguredTestSuite, TestRequest, WithMaterializer, WithTestWsClient}
import play.api.libs.ws.{WSClient, WSRequest, WSResponse}
import play.api.libs.json.{JsString}
import play.api.mvc.{Result}
import conf.Configuration
import org.mockito.Matchers.any
Expand Down Expand Up @@ -35,7 +36,7 @@ import org.scalatest.flatspec.AnyFlatSpec
private val wsRequestMock = mock[WSRequest]

"post" should "return a 404 for DCR 415 errors" in {
val payload = "payload"
val payload = JsString("payload")
when(wsMock.url(any[String])).thenReturn(wsRequestMock)
when(wsRequestMock.withRequestTimeout(any())).thenReturn(wsRequestMock)
when(wsRequestMock.addHttpHeaders(any())).thenReturn(wsRequestMock)
Expand Down
Loading