Skip to content

Commit

Permalink
Support pagination even for queries that can not specify order-by
Browse files Browse the repository at this point in the history
Get rid of Aux[Q,R]

Allow specifying direction to get next search query

We're trying to support LightBox functionality that was introduced here: guardian/frontend#20462

Co-authored-by: Roberto Tyley <[email protected]>

Support direction for all paginatable queries, and remove unused PaginatedApiResponse classes
  • Loading branch information
rtyley committed Jun 10, 2022
1 parent e93dbb9 commit 9e2de5a
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 143 deletions.
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package com.gu.contentapi.client

import java.time.Instant

import com.gu.contentapi.client.model.v1.{ContentType, ErrorResponse, SearchResponse}
import com.gu.contentapi.client.model.{ContentApiError, ItemQuery, SearchQuery}
import com.gu.contentapi.client.model.Direction.{Next, Previous}
import com.gu.contentapi.client.model.v1.{ContentType, ErrorResponse, SearchResponse, TagsResponse}
import com.gu.contentapi.client.model.{ContentApiError, FollowingSearchQuery, ItemQuery, SearchQuery}
import com.gu.contentatom.thrift.{AtomData, AtomType}
import org.scalatest._
import org.scalatest.concurrent.{IntegrationPatience, ScalaFutures}
import org.scalatest.time.{Seconds, Span}

import java.time.Instant
import scala.concurrent.ExecutionContext.Implicits.global

object GuardianContentClientTest {
Expand Down Expand Up @@ -60,6 +60,13 @@ class GuardianContentClientTest extends FlatSpec with Matchers with ScalaFutures
content.futureValue.id should be (TestItemPath)
}

it should "perform a given sections query" in {
val query = ContentApiClient.sections.q("business")
val results = for (response <- api.getResponse(query)) yield response.results
val fResults = results.futureValue
fResults.size should be >= 3 // UK business, US business, all da business
}

it should "perform a given atoms query" in {
val query = ContentApiClient.atoms.types("explainer")
val results = for (response <- api.getResponse(query)) yield response.results
Expand Down Expand Up @@ -88,6 +95,24 @@ class GuardianContentClientTest extends FlatSpec with Matchers with ScalaFutures
}
}

it should "be able to find previous and next articles in a series" in {
// This kind of functionality is used eg. for the DotCom image lightbox forward-backward buttons:
// https://github.com/guardian/frontend/pull/20462
val queryForNext = FollowingSearchQuery(
ContentApiClient.search.tag("commentisfree/series/guardian-comment-cartoon").orderBy("oldest").pageSize(1),
"commentisfree/picture/2022/may/15/nicola-jennings-boris-johnson-northern-ireland-cartoon",
Next
)

api.getResponse(queryForNext).futureValue.results.head.id shouldBe
"commentisfree/picture/2022/may/16/steve-bell-on-the-queens-platinum-jubilee-cartoon"

val queryForPrevious = queryForNext.copy(direction = Previous)

api.getResponse(queryForPrevious).futureValue.results.head.id shouldBe
"commentisfree/picture/2022/may/13/martin-rowson-whats-left-in-the-tories-box-of-tricks-cartoon"
}

it should "paginate through all results" in {
val query = ContentApiClient.search
.q("brexit")
Expand Down Expand Up @@ -116,6 +141,19 @@ class GuardianContentClientTest extends FlatSpec with Matchers with ScalaFutures
result.futureValue should be (42)
}

it should "be able to paginate any query that supports paging, eg Tags!" in {
val query = ContentApiClient.tags
.tagType("newspaper-book")
.section("music")
.pageSize(5)
// https://content.guardianapis.com/tags?type=newspaper-book&section=music&page-size=5
// has 19 results as of May 2022

val result = api.paginateAccum(query)({ r: TagsResponse => r.results.length }, { (a: Int, b: Int) => a + b })

result.futureValue should be > 10
}

it should "fold over the results" in {
val query = ContentApiClient.search
.q("brexit")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ package com.gu.contentapi.client

import com.gu.contentapi.buildinfo.CapiBuildInfo
import com.gu.contentapi.client.HttpRetry.withRetry
import com.gu.contentapi.client.model.Direction.Next
import com.gu.contentapi.client.model.HttpResponse.isSuccessHttpResponse
import com.gu.contentapi.client.model._
import com.gu.contentapi.client.thrift.ThriftDeserializer
import com.gu.contentatom.thrift.AtomType
import com.twitter.scrooge.{ThriftStruct, ThriftStructCodec}

import scala.concurrent.{ExecutionContext, Future}

trait ContentApiClient {
import Decoder._

/** Your API key */
def apiKey: String
Expand Down Expand Up @@ -46,7 +48,7 @@ trait ContentApiClient {

/** Streamlines the handling of a valid CAPI response */

private def fetchResponse(contentApiQuery: ContentApiQuery)(implicit context: ExecutionContext): Future[Array[Byte]] = get(url(contentApiQuery), headers).flatMap {
private def fetchResponse(contentApiQuery: ContentApiQuery[_])(implicit ec: ExecutionContext): Future[Array[Byte]] = get(url(contentApiQuery), headers).flatMap {
case response if isSuccessHttpResponse(response) => Future.successful(response)
case response => Future.failed(ContentApiError(response))
}.map(_.body)
Expand All @@ -59,7 +61,7 @@ trait ContentApiClient {
}
}

def url(contentApiQuery: ContentApiQuery): String =
def url(contentApiQuery: ContentApiQuery[_]): String =
contentApiQuery.getUrl(targetUrl, parameters)

/** Runs the query against the Content API.
Expand All @@ -68,10 +70,10 @@ trait ContentApiClient {
* @param query the query
* @return a future resolving to an unmarshalled response
*/
def getResponse[Q <: ContentApiQuery](query: Q)(
implicit
decoder: Decoder[Q],
context: ExecutionContext): Future[decoder.Response] =
def getResponse[R <: ThriftStruct](query: ContentApiQuery[R])(
implicit
decoder: Decoder[R],
context: ExecutionContext): Future[R] =
fetchResponse(query) map decoder.decode

/** Unfolds a query to its results, page by page
Expand All @@ -82,15 +84,12 @@ trait ContentApiClient {
* @param f a result-processing function
* @return a future of a list of result-processed results
*/
def paginate[Q <: PaginatedApiQuery[Q], R, M](query: Q)(f: R => M)(
implicit
decoder: Decoder.Aux[Q, R],
pager: PaginatedApiResponse[R],
def paginate[R <: ThriftStruct: Decoder, E, M](query: PaginatedApiQuery[R, E])(f: R => M)(
implicit
context: ExecutionContext
): Future[List[M]] =
unfoldM { r: R =>
val nextQuery = pager.getNextId(r).map { id => getResponse(ContentApiClient.next(query, id)) }
(f(r), nextQuery)
(f(r), query.followingQueryGiven(r, Next).map(getResponse(_)))
}(getResponse(query))

/** Unfolds a query by accumulating its results
Expand All @@ -101,10 +100,8 @@ trait ContentApiClient {
* @param f a result-processing function
* @return a future of an accumulated value
*/
def paginateAccum[Q <: PaginatedApiQuery[Q], R, M](query: Q)(f: R => M, g: (M, M) => M)(
implicit
decoder: Decoder.Aux[Q, R],
pager: PaginatedApiResponse[R],
def paginateAccum[R <: ThriftStruct: Decoder, E, M](query: PaginatedApiQuery[R, E])(f: R => M, g: (M, M) => M)(
implicit
context: ExecutionContext
): Future[M] =
paginate(query)(f).map {
Expand All @@ -114,26 +111,22 @@ trait ContentApiClient {
}

/** Unfolds a query by accumulating its results
*
* @tparam Q the type of a Content API query with pagination parameters
*
* @tparam R the type of response expected for `Q`
* @param query the initial query
* @param f a result-processing function
* @return a future of an accumulated value
*/
def paginateFold[Q <: PaginatedApiQuery[Q], R, M](query: Q)(m: M)(f: (R, M) => M)(
implicit
decoder: Decoder.Aux[Q, R],
decoderNext: Decoder.Aux[NextQuery[Q], R],
pager: PaginatedApiResponse[R],
def paginateFold[R <: ThriftStruct: Decoder, E, M](query: PaginatedApiQuery[R, E])(m: M)(f: (R, M) => M)(
implicit
context: ExecutionContext
): Future[M] = {
def paginateFoldIn(nextQuery: Option[NextQuery[Q]])(m: M): Future[M] = {
val req = nextQuery.map(getResponse(_)).getOrElse(getResponse(query))
def paginateFoldIn(currentQuery: Option[PaginatedApiQuery[R, E]])(m: M): Future[M] = {
val req = currentQuery.map(getResponse(_)).getOrElse(getResponse(query))
req.flatMap { r: R =>
pager.getNextId(r) match {
query.followingQueryGiven(r, Next) match {
case None => Future.successful(f(r, m))
case Some(id) => paginateFoldIn(Some(ContentApiClient.next(query, id)))(f(r, m))
case Some(nextQuery) => paginateFoldIn(Some(nextQuery))(f(r, m))
}
}
}
Expand Down Expand Up @@ -169,19 +162,4 @@ trait ContentApiQueries {
val restaurantReviews = RestaurantReviewsQuery()
val filmReviews = FilmReviewsQuery()
val videoStats = VideoStatsQuery()
def next[Q <: PaginatedApiQuery[Q]](q: Q, id: String) = NextQuery(normalize(q), id)

private def normalize[Q <: PaginatedApiQuery[Q]]: Q => Q =
normalizePageSize andThen normalizeOrder

private def normalizePageSize[Q <: PaginatedApiQuery[Q]]: Q => Q =
q => if (q.has("page-size")) q else q.pageSize(10)

private def normalizeOrder[Q <: PaginatedApiQuery[Q]]: Q => Q =
q => if (q.has("order-by"))
q
else if (q.has("q"))
q.orderBy("relevance")
else
q.orderBy("newest")
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,13 @@ trait Parameters[Owner <: Parameters[Owner]] { self: Owner =>
case class StringParameter(name: String, value: Option[String] = None) extends OwnedParameter {
type Self = String
def withValue(newValue: Option[String]) = copy(value = newValue)
def setIfUndefined(str: String) = if (owner.has(name)) owner else apply(str)
}

case class IntParameter(name: String, value: Option[Int] = None) extends OwnedParameter {
type Self = Int
def withValue(newValue: Option[Int]) = copy(value = newValue)
def setIfUndefined(v: Int) = if (owner.has(name)) owner else apply(v)
}

case class DateParameter(name: String, value: Option[Instant] = None) extends OwnedParameter {
Expand Down
58 changes: 22 additions & 36 deletions client/src/main/scala/com.gu.contentapi.client/model/Decoder.scala
Original file line number Diff line number Diff line change
@@ -1,48 +1,34 @@
package com.gu.contentapi.client

import com.gu.contentapi.client.model._
import com.gu.contentapi.client.model.v1._
import com.gu.contentapi.client.thrift.ThriftDeserializer
import com.twitter.scrooge.{ThriftStruct, ThriftStructCodec}

/** Typeclass witnessing how to unmarshall a Thrift stream of bytes
* into a concrete data type
* @tparam Query the query type
*/
trait Decoder[Query] {
/** the response type corresponding to `Query` */
type Response <: ThriftStruct
/** the codec unmarshalling instances of `Response` */
def codec: ThriftStructCodec[Response]
/** performs the unmarshalling
* @return a function taking an array of bytes into a `Response`
*/
def decode: Array[Byte] => Response = ThriftDeserializer.deserialize(_, codec)
}

private[client] object Decoder {
class Decoder[Response <: ThriftStruct](codec: ThriftStructCodec[Response]) {
def decode(data: Array[Byte]): Response = ThriftDeserializer.deserialize(data, codec)
}

type Aux[Q, R] = Decoder[Q] { type Response = R }
trait PaginationDecoder[Response, Element] {
val pageSize: Response => Int
val elements: Response => collection.Seq[Element]
}

private def apply[Q, R <: ThriftStruct](c: ThriftStructCodec[R]) = new Decoder[Q] {
type Response = R
def codec = c
}
object Decoder {
type PageableResponseDecoder[Response <: ThriftStruct, Element] = Decoder[Response] with PaginationDecoder[Response, Element]

private def atomsDecoder[Query] = apply[Query, AtomsResponse](AtomsResponse)
def pageableResponseDecoder[R <: ThriftStruct, E](c: ThriftStructCodec[R])(ps: R => Int, el: R => collection.Seq[E]): PageableResponseDecoder[R, E] =
new Decoder[R](c) with PaginationDecoder[R, E] {
val pageSize: R => Int = ps
val elements: R => collection.Seq[E] = el
}

implicit val itemQuery = apply[ItemQuery, ItemResponse](ItemResponse)
implicit val tagsQuery = apply[TagsQuery, TagsResponse](TagsResponse)
implicit val sectionsQuery = apply[SectionsQuery, SectionsResponse](SectionsResponse)
implicit val editionsQuery = apply[EditionsQuery, EditionsResponse](EditionsResponse)
implicit val videoStatsQuery = apply[VideoStatsQuery, VideoStatsResponse](VideoStatsResponse)
implicit val atomsQuery = atomsDecoder[AtomsQuery]
implicit val recipesQuery = atomsDecoder[RecipesQuery]
implicit val reviewsQuery = atomsDecoder[ReviewsQuery]
implicit val gameReviewsQuery = atomsDecoder[GameReviewsQuery]
implicit val restaurantReviewsQuery = atomsDecoder[RestaurantReviewsQuery]
implicit val filmReviewsQuery = atomsDecoder[FilmReviewsQuery]
implicit def searchQueryBase[T <: SearchQueryBase[T]] = apply[T, SearchResponse](SearchResponse)
implicit def nextQuery[Q <: PaginatedApiQuery[Q]](implicit d: Decoder[Q]) = apply[NextQuery[Q], d.Response](d.codec)
implicit def atomsUsageQuery = apply[AtomUsageQuery, AtomUsageResponse](AtomUsageResponse)
implicit val itemDecoder: Decoder[ItemResponse] = new Decoder(ItemResponse)
implicit val tagsDecoder: PageableResponseDecoder[TagsResponse, Tag] = pageableResponseDecoder(TagsResponse)(_.pageSize, _.results)
implicit val sectionsQuery: Decoder[SectionsResponse] = new Decoder(SectionsResponse)
implicit val editionsDecoder: Decoder[EditionsResponse] = new Decoder(EditionsResponse)
implicit val videoStatsDecoder: Decoder[VideoStatsResponse] = new Decoder(VideoStatsResponse)
implicit val atomsDecoder: Decoder[AtomsResponse] = new Decoder(AtomsResponse)
implicit val searchDecoder: PageableResponseDecoder[SearchResponse, Content] = pageableResponseDecoder(SearchResponse)(_.pageSize, _.results)
implicit val atomUsageDecoder: Decoder[AtomUsageResponse] = new Decoder(AtomUsageResponse)
}

This file was deleted.

Loading

0 comments on commit 9e2de5a

Please sign in to comment.