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

Make pagination more generic and not Franklin specific #413

Merged

Conversation

pomadchin
Copy link
Collaborator

@pomadchin pomadchin commented Oct 4, 2021

Overview

This PR makes pagination more generic and not Franklin specific.

Checklist

  • Changelog updated (please use chan)
// add "com.softwaremill.sttp.client3" %% "async-http-client-backend-cats-ce2" % "3.3.15" % Test
package com.azavea.stac4s.api.client

import com.azavea.stac4s.StacItem
import cats.effect.IO
import cats.syntax.either._
import io.circe.parser
import sttp.client3.asynchttpclient.cats.AsyncHttpClientCatsBackend
import sttp.client3.UriContext

import org.scalatest.BeforeAndAfterAll
import org.scalatest.funspec.AnyFunSpec
import org.scalatest.matchers.should.Matchers

import scala.concurrent.ExecutionContext

class ItSpec extends AnyFunSpec with Matchers with BeforeAndAfterAll {
  val filters = parser
    .parse("""{
          "collections": [
          "mcd43a4"
        ],
        "bbox": [
          34.74406064953656,
          -4.950752223991549,
          35.321110216900706,
          -4.573783231966013
        ],
        "datetime": "2017-01-01T00:00:00.757Z/2017-12-31T00:00:00.635Z"
      }""")
    .flatMap(_.as[SearchFilters])
    .valueOr(throw _)
  
  describe("should fetch items with pagination") {
    it("request with limit no merge") {
      implicit val cs = IO.contextShift(ExecutionContext.global)
      val realitems: List[StacItem] = AsyncHttpClientCatsBackend
        .resource[IO]()
        .use { backend =>
          SttpStacClient(backend, uri"https://eod-catalog-svc-prod.astraea.earth/")
            .search(filters)
            .compile
            .toList
        }
        .unsafeRunSync()

      realitems.length shouldBe 364
    }
  }
}

Closes #409
Closes #410
Closes #411

@pomadchin pomadchin requested a review from jisantuc October 4, 2021 22:10
@pomadchin pomadchin force-pushed the feature/pagination-improvements branch from 4b0df4c to 9074f4c Compare October 4, 2021 22:13
@pomadchin pomadchin force-pushed the feature/pagination-improvements branch 3 times, most recently from 9d9d405 to 37ea65f Compare October 4, 2021 22:20
@@ -151,20 +149,48 @@ case class SttpStacClientF[F[_]: MonadThrow, S: Lens[*, Option[PaginationToken]]
}

object SttpStacClientF {
// TODO: should be a newtype
type PaginationToken = NonEmptyString
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is probably not a part of this PR and could be a more broad discussion. Good for now!

Copy link
Contributor

Choose a reason for hiding this comment

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

Eh, I think this is fine -- we don't really know anything about "semantics" of a pagination token, it might not even be a non-empty string in some cases. We should revisit if this becomes insufficient but I think this is pretty much all we know about pagination tokens right now

@pomadchin pomadchin force-pushed the feature/pagination-improvements branch 8 times, most recently from 4c5613a to a05c99f Compare October 5, 2021 00:21
@pomadchin pomadchin force-pushed the feature/pagination-improvements branch from a05c99f to 0468782 Compare October 5, 2021 00:24
Copy link
Contributor

@jisantuc jisantuc left a comment

Choose a reason for hiding this comment

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

I'm good with this -- I think when we were talking about pagination originally I wanted stac4s to treat the token as completely opaque, but we went with copy-pasta instead to unblock ourselves, so I'm happy this is getting genericized

@pomadchin pomadchin merged commit 9e3c173 into stac-utils:master Oct 5, 2021
@pomadchin pomadchin deleted the feature/pagination-improvements branch October 5, 2021 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants