Add a max-page-size config option and paging for search results (#1485)

Adds a `max-page-size` option to configure the maximum size when returning lists from the api. For the search endpoint, enable paging parameters `page` and `pageSize`. The `pageSize` will be capped to the `max-page-size` config value. Refs: #1294
eikek · Sep 16, 2024 · 4b7701a · 4b7701a
1 parent acb8135
commit 4b7701a
Show file tree

Hide file tree

Showing 13 changed files with 115 additions and 20 deletions.
diff --git a/modules/backend/src/main/scala/sharry/backend/share/OShare.scala b/modules/backend/src/main/scala/sharry/backend/share/OShare.scala
@@ -84,7 +84,7 @@ trait OShare[F[_]] {
     *
     * The query is applied to the name, id and alias name.
     */
-  def findShares(q: String, accId: AccountId): Stream[F, ShareItem]
+  def findShares(q: String, accId: AccountId, page: Page): Stream[F, ShareItem]
 
   /** Get all details about a share. */
   def shareDetails(
@@ -336,8 +336,8 @@ object OShare {
       def getFileData(fileId: Ident, accId: AccountId): OptionT[F, FileData] =
         OptionT(store.transact(Queries.fileData(fileId)))
 
-      def findShares(q: String, accId: AccountId): Stream[F, ShareItem] =
-        store.transact(Queries.findShares(q, accId))
+      def findShares(q: String, accId: AccountId, page: Page): Stream[F, ShareItem] =
+        store.transact(Queries.findShares(q, accId, page))
 
       def shareDetails(
           shareId: ShareId,

diff --git a/modules/backend/src/main/scala/sharry/backend/share/Queries.scala b/modules/backend/src/main/scala/sharry/backend/share/Queries.scala
@@ -190,7 +190,11 @@ object Queries {
   private def aliasMemberOf(accId: Ident) =
     RAliasMember.aliasMemberOf(accId)
 
-  def findShares(q: String, accId: AccountId): Stream[ConnectionIO, ShareItem] = {
+  def findShares(
+      q: String,
+      accId: AccountId,
+      page: Page
+  ): Stream[ConnectionIO, ShareItem] = {
     val nfiles = Column("files")
     val nsize = Column("size")
     val shareId = "s" :: RShare.Columns.id
@@ -225,8 +229,10 @@ object Queries {
         Sql.or(account.is(accId.id), shareAlias.in(aliasMemberOf(accId.id))),
         Sql.or(name.like(qs), sid.like(qs), aliasName.like(qs), description.like(qs))
       )
-    ) ++ fr"ORDER BY" ++ created.f ++ fr"DESC"
-    logger.stream.trace(s"$frag").drain ++
+    ) ++ fr"ORDER BY" ++ created.f ++ fr"DESC" ++ fr"OFFSET ${page.offset} LIMIT ${page.limit}"
+    logger.stream
+      .trace(s"$frag (page.limit=${page.limit}  page.offset=${page.offset})")
+      .drain ++
       frag.query[ShareItem].stream
   }
 

diff --git a/modules/common/src/main/scala/sharry/common/Page.scala b/modules/common/src/main/scala/sharry/common/Page.scala
@@ -0,0 +1,14 @@
+package sharry.common
+
+final case class Page(limit: Int, offset: Int):
+  def next: Page = Page(limit, offset + limit)
+  def prev: Page = Page(limit, math.max(0, offset - limit))
+  def capped(max: Int): Page = copy(limit = math.min(max, limit))
+
+object Page:
+
+  def one(size: Int): Page = Page(size, 0)
+
+  def page(pageNum: Int, pageSize: Int): Page =
+    if (pageNum <= 1) one(pageSize)
+    else Page(pageSize, (pageNum - 1) * pageSize)
diff --git a/modules/restapi/src/main/resources/sharry-openapi.yml b/modules/restapi/src/main/resources/sharry-openapi.yml
@@ -662,6 +662,8 @@ paths:
         Returns a list of all shares of the current user.
       parameters:
         - $ref: "#/components/parameters/q"
+        - $ref: "#/components/parameters/page"
+        - $ref: "#/components/parameters/pageSize"
       responses:
         422:
           description: BadRequest
@@ -2195,6 +2197,22 @@ components:
       required: false
       schema:
         type: string
+    page:
+      name: page
+      in: query
+      description: The page number
+      required: false
+      schema:
+        type: integer
+        format: int32
+    pageSize:
+      name: pageSize
+      in: query
+      description: The page size
+      required: false
+      schema:
+        type: integer
+        format: int32
     SharryFileName:
       name: Sharry-File-Name
       in: header

diff --git a/modules/restserver/src/main/resources/reference.conf b/modules/restserver/src/main/resources/reference.conf
@@ -7,6 +7,8 @@ sharry.restserver {
   # should not end in a slash.
   base-url = "http://localhost:9090"
 
+  # Maximum size when returning result lists.
+  max-page-size = 100
 
   # Where the server binds to.
   bind {

diff --git a/modules/restserver/src/main/scala/sharry/restserver/RestServer.scala b/modules/restserver/src/main/scala/sharry/restserver/RestServer.scala
@@ -111,9 +111,9 @@ object RestServer {
       "auth" -> LoginRoutes.session(restApp.backend.login, cfg),
       "settings" -> SettingRoutes(restApp.backend, token),
       "alias-member" ->
-        (if (cfg.aliasMemberEnabled) AliasMemberRoutes(restApp.backend, token)
+        (if (cfg.aliasMemberEnabled) AliasMemberRoutes(restApp.backend, token, cfg)
          else notFound[F](token)),
-      "alias" -> AliasRoutes(restApp.backend, token),
+      "alias" -> AliasRoutes(restApp.backend, token, cfg),
       "share" -> ShareRoutes(restApp.backend, token, cfg),
       "upload" -> ShareUploadRoutes(
         restApp.backend,
@@ -130,7 +130,7 @@ object RestServer {
   ): HttpRoutes[F] =
     Router(
       "signup" -> RegisterRoutes(restApp.backend, cfg).genInvite,
-      "account" -> AccountRoutes(restApp.backend)
+      "account" -> AccountRoutes(restApp.backend, cfg)
     )
 
   def openRoutes[F[_]: Async](

diff --git a/modules/restserver/src/main/scala/sharry/restserver/config/Config.scala b/modules/restserver/src/main/scala/sharry/restserver/config/Config.scala
@@ -12,6 +12,7 @@ import com.comcast.ip4s.{Host, Port}
 case class Config(
     baseUrl: LenientUri,
     aliasMemberEnabled: Boolean,
+    maxPageSize: Int,
     bind: Config.Bind,
     fileDownload: Config.FileDownload,
     logging: LogConfig,
@@ -86,6 +87,10 @@ object Config {
       customHead: String
   )
 
+  final case class Api(
+      maxPageSize: Int
+  )
+
   private def validateTheme(str: String): String =
     if (str.equalsIgnoreCase("light") || str.equalsIgnoreCase("dark")) ""
     else s"Invalid theme: $str (use either 'light' or 'dark')"

diff --git a/modules/restserver/src/main/scala/sharry/restserver/config/ConfigValues.scala b/modules/restserver/src/main/scala/sharry/restserver/config/ConfigValues.scala
@@ -71,6 +71,12 @@ object ConfigValues extends ConfigDecoders:
 
   val baseUrl = key("base-url", "BASE_URL").as[LenientUri]
 
+  val maxPageSize = key("max-page-size", "MAX_PAGE_SIZE").as[Int].flatMap { n =>
+    if (n <= 0)
+      ConfigValue.failed(ConfigError(s"max-page-size must be greater than 0, got $n"))
+    else ConfigValue.loaded(ConfigKey("max-page-size"), n)
+  }
+
   val bind = {
     val address = key("bind.address", "BIND_ADDRESS").as[Host]
     val port = key("bind.port", "BIND_PORT").as[Port]
@@ -413,7 +419,16 @@ object ConfigValues extends ConfigDecoders:
     )
 
   val fullConfig =
-    (baseUrl, aliasMemberEnabled, bind, fileDownload, logConfig, webapp, backendConfig)
+    (
+      baseUrl,
+      aliasMemberEnabled,
+      maxPageSize,
+      bind,
+      fileDownload,
+      logConfig,
+      webapp,
+      backendConfig
+    )
       .mapN(
         Config.apply
       )

diff --git a/modules/restserver/src/main/scala/sharry/restserver/routes/AccountRoutes.scala b/modules/restserver/src/main/scala/sharry/restserver/routes/AccountRoutes.scala
@@ -8,6 +8,7 @@ import sharry.backend.BackendApp
 import sharry.backend.account.{AccountItem, NewAccount}
 import sharry.common.*
 import sharry.restapi.model.*
+import sharry.restserver.config.Config
 import sharry.store.records.ModAccount
 
 import org.http4s.HttpRoutes
@@ -16,7 +17,7 @@ import org.http4s.circe.CirceEntityEncoder.*
 import org.http4s.dsl.Http4sDsl
 
 object AccountRoutes {
-  def apply[F[_]: Async](backend: BackendApp[F]): HttpRoutes[F] = {
+  def apply[F[_]: Async](backend: BackendApp[F], cfg: Config): HttpRoutes[F] = {
     val dsl = new Http4sDsl[F] {}
     import dsl._
 
@@ -59,7 +60,7 @@ object AccountRoutes {
         val q = req.params.getOrElse("q", "")
         for {
           _ <- logger.trace(s"Listing accounts: $q")
-          all <- backend.account.findAccounts(q).take(100).compile.toVector
+          all <- backend.account.findAccounts(q).take(cfg.maxPageSize).compile.toVector
           list = AccountList(all.map(accountDetail).toList)
           resp <- Ok(list)
         } yield resp

diff --git a/modules/restserver/src/main/scala/sharry/restserver/routes/AliasMemberRoutes.scala b/modules/restserver/src/main/scala/sharry/restserver/routes/AliasMemberRoutes.scala
@@ -7,6 +7,7 @@ import sharry.backend.BackendApp
 import sharry.backend.account.AccountItem
 import sharry.backend.auth.AuthToken
 import sharry.restapi.model.*
+import sharry.restserver.config.Config
 
 import org.http4s.HttpRoutes
 import org.http4s.circe.CirceEntityEncoder.*
@@ -15,7 +16,8 @@ import org.http4s.dsl.Http4sDsl
 object AliasMemberRoutes {
   def apply[F[_]: Async](
       backend: BackendApp[F],
-      token: AuthToken
+      token: AuthToken,
+      cfg: Config
   ): HttpRoutes[F] = {
     val logger = sharry.logging.getLogger[F]
     val dsl = new Http4sDsl[F] {}
@@ -28,7 +30,7 @@ object AliasMemberRoutes {
         list <- backend.account
           .findAccounts(q)
           .filter(a => a.acc.id != token.account.id)
-          .take(100)
+          .take(cfg.maxPageSize)
           .compile
           .toVector
         resp <- Ok(AccountLightList(list.map(convert).toList))

diff --git a/modules/restserver/src/main/scala/sharry/restserver/routes/AliasRoutes.scala b/modules/restserver/src/main/scala/sharry/restserver/routes/AliasRoutes.scala
@@ -9,6 +9,7 @@ import sharry.backend.alias.OAlias
 import sharry.backend.auth.AuthToken
 import sharry.common.*
 import sharry.restapi.model.*
+import sharry.restserver.config.Config
 import sharry.store.records.RAlias
 
 import org.http4s.HttpRoutes
@@ -19,7 +20,8 @@ import org.http4s.dsl.Http4sDsl
 object AliasRoutes {
   def apply[F[_]: Async](
       backend: BackendApp[F],
-      token: AuthToken
+      token: AuthToken,
+      cfg: Config
   ): HttpRoutes[F] = {
     val logger = sharry.logging.getLogger[F]
     val dsl = new Http4sDsl[F] {}
@@ -40,7 +42,11 @@ object AliasRoutes {
         val q = req.params.getOrElse("q", "")
         for {
           _ <- logger.trace(s"Listing aliases for ${token.account}")
-          list <- backend.alias.findAll(token.account.id, q).take(100).compile.toVector
+          list <- backend.alias
+            .findAll(token.account.id, q)
+            .take(cfg.maxPageSize)
+            .compile
+            .toVector
           resp <- Ok(AliasList(list.map(convert).toList))
         } yield resp
 

diff --git a/modules/restserver/src/main/scala/sharry/restserver/routes/Params.scala b/modules/restserver/src/main/scala/sharry/restserver/routes/Params.scala
@@ -0,0 +1,23 @@
+package sharry.restserver.routes
+
+import sharry.common.Page as CPage
+
+import org.http4s.dsl.impl.OptionalQueryParamDecoderMatcher
+
+object Params:
+
+  object Query extends OptionalQueryParamDecoderMatcher[String]("q")
+
+  object Page {
+    def unapply(params: Map[String, collection.Seq[String]]): Option[CPage] =
+      val pageNum = params.get("page").flatMap(_.headOption).flatMap(_.toIntOption) match
+        case Some(n) if n >= 1 => n
+        case _                 => 1
+
+      val pageSize =
+        params.get("pageSize").flatMap(_.headOption).flatMap(_.toIntOption) match
+          case Some(n) if n >= 1 => n
+          case _                 => 100
+
+      Some(CPage.page(pageNum, pageSize))
+  }
diff --git a/modules/restserver/src/main/scala/sharry/restserver/routes/ShareRoutes.scala b/modules/restserver/src/main/scala/sharry/restserver/routes/ShareRoutes.scala
@@ -30,12 +30,15 @@ object ShareRoutes {
     import dsl._
 
     HttpRoutes.of[F] {
-      case req @ GET -> Root / "search" =>
-        val q = req.params.getOrElse("q", "")
+      case req @ GET -> Root / "search" :? Params.Query(optQ) +& Params.Page(p) =>
+        val q = optQ.getOrElse("")
         for {
-          _ <- logger.trace(s"Listing shares: $q")
+          _ <- logger.trace(s"Listing shares: $q $p")
           now <- Timestamp.current[F]
-          all <- backend.share.findShares(q, token.account).take(100).compile.toVector
+          all <- backend.share
+            .findShares(q, token.account, p.capped(cfg.maxPageSize))
+            .compile
+            .toVector
           list = ShareList(all.map(shareListItem(now)).toList)
           resp <- Ok(list)
         } yield resp