guardian / content-api-scala-client

A Scala client library for the Guardian's Content API
Apache License 2.0
40 stars 16 forks source link

Make the ContentApiQuery type convariant in Response #429

Closed fredex42 closed 4 months ago

fredex42 commented 4 months ago

What does this change?

Changes the Response type parameter from invarant to covariant.

This is to make pattern-matching in tests to work better - used in integration tests introduced with https://github.com/guardian/crier/pull/169.

In a nutshell, we needed to be able to use Mockito to verify two different calls to the library. If the calls changed order, the behaviour would still be correct (ordering cannot be guaranteed in this case) but the test would then fail if it tried to cast the SearchQuery to the ItemQuery or vice-versa.

With this update in place, we can make a correctly scoped match statement to handle both of these cases gracefully:

      verify(mockLiveContentApi, times(2)).getResponse(argThat((arg:ContentApiQuery[ThriftStruct])=> arg match {
        case searchQuery: SearchQuery=>
          searchQuery.channelId.contains("all") &&
            searchQuery.parameters.get("ids").contains(s"internal-code/composer/${notifications.head.value.composerId}") &&
            searchQuery.parameters.get("show-channels").contains("all")
        case itemQuery: ItemQuery=>
          itemQuery.channelId.contains("all") &&
            itemQuery.id=="lifeandstyle/article/2024/jun/27/bzbcxvzxvzxv"
        case other@_ =>
          fail(s"Unexpected CAPI call: ${other.getClass.getTypeName}")
      }))(any, any)

Without this update, the compiler objects since the type signature of ContentApiQuery[Response] requires specific response type and not an ancestor of this type. When changed to ContentApiQuery[+Response] we are able to successfully match against any possible value.

How to test

Integration tests linked above work.

Tested locally.

How can we measure success?

Able to continue on Crier

Have we considered potential risks?

n/a

github-actions[bot] commented 4 months ago

Test Results

0 tests  ±0   0 :white_check_mark: ±0   0s :stopwatch: ±0s 0 suites ±0   0 :zzz: ±0  0 files   ±0   0 :x: ±0 

Results for commit 4f294296. ± Comparison against base commit c406c9e1.

gu-scala-library-release[bot] commented 4 months ago

@fredex42 has published a preview version of this PR with release workflow run #64, based on commit 4f294296ce3f0ee50193d93697353812f3326602:

31.0.3-PREVIEW.agcovariant-query-response.2024-06-28T1631.4f294296

Want to make another preview release? Click 'Run workflow' in the [GitHub UI](https://github.com/guardian/content-api-scala-client/actions/workflows/release.yml), specifying the ag/covariant-query-response branch, or use the [GitHub CLI](https://cli.github.com/) command: gh workflow run release.yml --ref ag/covariant-query-response
Want to make a full release after this PR is merged? Click 'Run workflow' in the [GitHub UI](https://github.com/guardian/content-api-scala-client/actions/workflows/release.yml), leaving the branch as the default, or use the [GitHub CLI](https://cli.github.com/) command: gh workflow run release.yml