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.

Sign up for GitHub

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

Editions crossword cryptic and quick data #27604

Merged
merged 5 commits into from
Nov 20, 2024
Merged

Editions crossword cryptic and quick data #27604

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb1900e
WIP
DanielCliftonGuardian Nov 14, 2024
d600162
Add to DCR service
DanielCliftonGuardian Nov 15, 2024
115f994
Send to dcr
DanielCliftonGuardian Nov 15, 2024
f6b8b9a
Update CrosswordsController.scala
DanielCliftonGuardian Nov 20, 2024
cc8dd30
Use CacheTime.Default
DanielCliftonGuardian Nov 20, 2024
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
40 changes: 30 additions & 10 deletions applications/app/controllers/CrosswordsController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package controllers

import com.gu.contentapi.client.model.v1.Content.unsafeEmpty.crossword
DanielCliftonGuardian marked this conversation as resolved.
Show resolved Hide resolved
import com.gu.contentapi.client.model.v1.CrosswordType.{Cryptic, Quick}
import com.gu.contentapi.client.model.v1.{Crossword, ItemResponse, Content => ApiContent, Section => ApiSection}
import common.{Edition, GuLogging, ImplicitControllerExecutionContext}
import conf.Static
Expand All @@ -22,6 +24,8 @@ import play.api.data._
import play.api.mvc.{Action, RequestHeader, Result, _}
import services.{IndexPage, IndexPageItem}
import html.HtmlPageHelpers.ContentCSSFile
import model.dotcomrendering.pageElements.EditionsCrosswordRenderingDataModel
import model.dotcomrendering.pageElements.EditionsCrosswordRenderingDataModel.toJson
import model.dotcomrendering.{DotcomRenderingDataModel, PageType}
import play.api.libs.ws.WSClient
import renderers.DotcomRenderingService
Expand Down Expand Up @@ -290,17 +294,33 @@ class CrosswordSearchController(
case class CrosswordLookup(crosswordType: String, id: Int)
}

class CrosswordEditionsController(val controllerComponents: ControllerComponents)
extends BaseController
class CrosswordEditionsController(
val contentApiClient: ContentApiClient,
val controllerComponents: ControllerComponents,
val remoteRenderer: DotcomRenderingService = DotcomRenderingService(),
val wsClient: WSClient,
) extends BaseController
with GuLogging
with ImplicitControllerExecutionContext {

def digitalEdition: Action[AnyContent] =
Action.async { implicit request =>
Future.successful(
Cached(CacheTime.Default)(
RevalidatableResult.Ok("Digital Edition Crossword Entry Point"),
),
)
}
def digitalEdition: Action[AnyContent] = Action.async { implicit request =>
getCrosswords
.map(parseCrosswords)
.flatMap {
case Some(crosswordPage) =>
remoteRenderer.getEditionsCrossword(wsClient, crosswordPage)
case None => Future.successful(NotFound)
}
}

private lazy val crosswordsQuery = contentApiClient.item("crosswords")

private def getCrosswords: Future[ItemResponse] = contentApiClient.getResponse(crosswordsQuery)

private def parseCrosswords(response: ItemResponse): Option[EditionsCrosswordRenderingDataModel] =
for {
results <- response.results
quick <- results.find(_.crossword.exists(_.`type` == Quick)).flatMap(_.crossword)
cryptic <- results.find(_.crossword.exists(_.`type` == Cryptic)).flatMap(_.crossword)
} yield EditionsCrosswordRenderingDataModel(quick, cryptic)
}
1 change: 1 addition & 0 deletions build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ val common = library("common")
eTagCachingS3,
nettyCodecHttp2,
contentApiClient,
contentApiModelsJson,
enumeratumPlayJson,
filters,
commonsLang,
Expand Down
20 changes: 20 additions & 0 deletions common/app/model/dotcomrendering/pageElements/EditionsCrosswordRenderingDataModel.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
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._

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
}
}
11 changes: 10 additions & 1 deletion common/app/renderers/DotcomRenderingService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package renderers

import org.apache.pekko.actor.{ActorSystem => PekkoActorSystem}
import com.gu.contentapi.client.model.v1.{Block, Blocks, Content}
import com.gu.contentapi.client.model.v1.{Block, Blocks, Content, Crossword}
import common.{DCRMetrics, GuLogging}
import concurrent.CircuitBreakerRegistry
import conf.Configuration
Expand All @@ -10,6 +10,7 @@ import crosswords.CrosswordPageWithContent
import http.{HttpPreconnections, ResultWithPreconnectPreload}
import model.Cached.{RevalidatableResult, WithoutRevalidationResult}
import model.dotcomrendering._
import model.dotcomrendering.pageElements.EditionsCrosswordRenderingDataModel
import model.{
CacheTime,
Cached,
Expand Down Expand Up @@ -417,6 +418,14 @@ class DotcomRenderingService extends GuLogging with ResultWithPreconnectPreload
val json = DotcomRenderingDataModel.toJson(dataModel)
post(ws, json, Configuration.rendering.articleBaseURL + "/Article", CacheTime.Facia)
}

def getEditionsCrossword(
ws: WSClient,
crosswords: EditionsCrosswordRenderingDataModel,
)(implicit request: RequestHeader): Future[Result] = {
val json = EditionsCrosswordRenderingDataModel.toJson(crosswords)
post(ws, json, Configuration.rendering.articleBaseURL + "/EditionsCrossword", CacheTime.Facia)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why CacheTime.Facia?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the other post requests in the DotcomRenderingService are using this cache time including for articles. Its confusingly named, maybe we can look at reviewing this separately.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think generally post requests for articles are using a different cache time based on their metadata:

frontend/common/app/renderers/DotcomRenderingService.scala

Line 215 in c7b403b

post(ws, json, Configuration.rendering.articleBaseURL + path, page.metadata.cacheTime)

frontend/common/app/renderers/DotcomRenderingService.scala

Line 311 in c7b403b

post(ws, json, Configuration.rendering.interactiveBaseURL + "/Interactive", page.metadata.cacheTime, 4.seconds)

with CacheTime.Default as the fallback:

frontend/common/app/model/meta.scala

Line 326 in c7b403b

cacheTime: CacheTime = CacheTime.Default,

?

Post requests for fronts and tag pages are using CacheTime.Facia, but that's likely correct because they're examples of "facia" content. I agree that probably we should review getMedia, getImageContent and getGallery's usage of CacheTime.Facia; have captured this in #27628.

Given this, I don't think it's what we want to use here. Perhaps CacheTime.Default is the correct choice if we don't have a specific, different cache setting for these pages? @SiAdcock @arelra any thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I didn't think about this much and just copied what was being done for similar use cases. What about 50 minutes surrogate and 5 minutes for client cache arbitrarily. This is a simple optimisation unlikely to effect users too much unless that are hitting crosswords in press reader at midnight

}
}

object DotcomRenderingService {
Expand Down
1 change: 1 addition & 0 deletions project/Dependencies.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ object Dependencies {
val commonsIo = "commons-io" % "commons-io" % "2.16.1"
val cssParser = "net.sourceforge.cssparser" % "cssparser" % "0.9.30"
val contentApiClient = "com.gu" %% "content-api-client" % capiVersion
val contentApiModelsJson = "com.gu" %% "content-api-models-json" % "25.1.0"
val dfpAxis = "com.google.api-ads" % "dfp-axis" % "5.6.0"
val faciaFapiScalaClient = "com.gu" %% "fapi-client-play30" % faciaVersion
val identityCookie = "com.gu.identity" %% "identity-cookie" % identityLibVersion
Expand Down