Fix `facia-tool` upgrade: tweak `SearchQueryBase`

rtyley commented 2 years ago

What's the problem?

facia-tool uses the CAPI client, and in particular has a custom CapiQueryGenerator query class that extends CAPI client's SearchQueryBase class. The changes in PR https://github.com/guardian/content-api-scala-client/pull/359 mean that all classes extending SearchQueryBase are also extending PaginatedApiQuery.

Consequently CapiQueryGenerator unexpectedly needed to implement two obscure methods from PaginatedApiQuery (setPaginationConsistentWith & followingQueryGivenFull) when upgrading to CAPI client v19 - otherwise compilation fails in facia-tool.

It's not fair to block upgrades by asking end users to implement these unfamiliar methods - especially if they aren't using the auto-pagination that needs them. When I was working on https://github.com/guardian/content-api-scala-client/pull/359 I wasn't really aware of how many classes would be extending SearchQueryBase or why!

What's the fix?

Rather than implementing those obscure methods in facia-tool's CapiQueryGenerator class, I'm making the smallest possible change to the CAPI client so that no change to facia-tool is necessary - removing PaginatedApiQuery from SearchQueryBase and placing it directly on the concrete SearchQuery class.

Before

classDiagram
    PaginatedApiQuery <|-- SearchQueryBase
    SearchQueryBase <|-- SearchQuery
    SearchQueryBase <|-- CapiQueryGenerator

After

classDiagram
    PaginatedApiQuery <|-- SearchQuery
    SearchQueryBase <|-- SearchQuery
    SearchQueryBase <|-- CapiQueryGenerator

I've released a beta of this change (at commit 89a4ffd515dfc0653ca2b5b5272e7b6a3295af97) as 19.0.1-beta.0, and you can see that using this fixed the compilation error when upgrading facia-tool.

An alternative approach might be to support auto-pagination much more extensively, ie allow it everywhere that a query implements PaginationParameters, but there's a fair bit of change involved in getting there, so I'm leaving it for now.

What is `facia-tool`'s `CapiQueryGenerator` for?

CapiQueryGenerator is used for Kindle & Editions support - it queries the /content/print-sent endpoint (introduced in https://github.com/guardian/content-api/pull/1131), which can access preview as well as live content, and filters to just content that is eligible for Kindle / Editions - that is, content which has all the metadata that indicates where it will go in the print newspaper - a page number, a print date, etc. The endpoint does not support prev or next endpoints like standard /search would, so far as I know?

Thanks to @Fweddi for lots of context about the endpoint! See this chat in P&E/Editorial Tools/Fronts.

rtyley commented 2 years ago

Ah, I may have merged this a little quick! Happy to revert if necessary?

rtyley commented 2 years ago

BTW: we're working on deprecating the /print-sent service so in time this feature will be removed from here too.

Yeah, I hear CAPI Channels will get rid of it which will be cool!

This is released as v19.0.3 (v19.0.2 got lost in sonatype, failed release).

guardian / content-api-scala-client