From 4b7701aef95b5cc9f92eec05b38d44987e78bbd8 Mon Sep 17 00:00:00 2001 From: eikek <701128+eikek@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:49:54 +0200 Subject: [PATCH] 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 --- .../scala/sharry/backend/share/OShare.scala | 6 ++--- .../scala/sharry/backend/share/Queries.scala | 12 +++++++--- .../src/main/scala/sharry/common/Page.scala | 14 +++++++++++ .../src/main/resources/sharry-openapi.yml | 18 +++++++++++++++ .../src/main/resources/reference.conf | 2 ++ .../scala/sharry/restserver/RestServer.scala | 6 ++--- .../sharry/restserver/config/Config.scala | 5 ++++ .../restserver/config/ConfigValues.scala | 17 +++++++++++++- .../restserver/routes/AccountRoutes.scala | 5 ++-- .../restserver/routes/AliasMemberRoutes.scala | 6 +++-- .../restserver/routes/AliasRoutes.scala | 10 ++++++-- .../sharry/restserver/routes/Params.scala | 23 +++++++++++++++++++ .../restserver/routes/ShareRoutes.scala | 11 +++++---- 13 files changed, 115 insertions(+), 20 deletions(-) create mode 100644 modules/common/src/main/scala/sharry/common/Page.scala create mode 100644 modules/restserver/src/main/scala/sharry/restserver/routes/Params.scala diff --git a/modules/backend/src/main/scala/sharry/backend/share/OShare.scala b/modules/backend/src/main/scala/sharry/backend/share/OShare.scala index df9e63f1..4fa4b95d 100644 --- 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 index 48d90b56..617e8e01 100644 --- 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 new file mode 100644 index 00000000..8576e737 --- /dev/null +++ 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 index df3e6cbd..159e9396 100644 --- 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 index 93211f57..2bf522d8 100644 --- 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 index 83d909db..4afa93c5 100644 --- 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 index 8f2d5492..c05c74d7 100644 --- 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 index 2e08f1c0..45e68bbe 100644 --- 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 index f8922712..7ceba4bd 100644 --- 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 index 10c8a778..7c245ea6 100644 --- 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 index db2b1735..2b941a83 100644 --- 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 new file mode 100644 index 00000000..b48842e0 --- /dev/null +++ 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 index 8ad1dda1..8df50dc5 100644 --- 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