-
Notifications
You must be signed in to change notification settings - Fork 85
Add batch versions latest API endpoint #965
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| package controllers | ||
|
|
||
| import cats.data.Validated.{Invalid, Valid} | ||
| import io.apibuilder.api.v0.models.BatchVersionsLatestForm | ||
| import io.apibuilder.api.v0.models.json.* | ||
| import lib.Validation | ||
| import play.api.libs.json.Json | ||
| import play.api.mvc.Action | ||
| import services.BatchVersionsLatestService | ||
|
|
||
| import javax.inject.{Inject, Singleton} | ||
|
|
||
| @Singleton | ||
| class BatchVersionsLatest @Inject() ( | ||
| override val apiBuilderControllerComponents: ApiBuilderControllerComponents, | ||
| service: BatchVersionsLatestService, | ||
| ) extends ApiBuilderController { | ||
|
|
||
| def post(orgKey: String): Action[BatchVersionsLatestForm] = Anonymous(parse.json[BatchVersionsLatestForm]) { request => | ||
| service.process(orgKey, request.body) match { | ||
| case Valid(result) => Ok(Json.toJson(result)) | ||
| case Invalid(errors) => Conflict(Json.toJson(Validation.errors(errors))) | ||
| } | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -209,6 +209,44 @@ class InternalVersionsDao @Inject()( | |
| }).map(InternalVersion(_)) | ||
| } | ||
|
|
||
| /** | ||
| * Efficient single-query lookup of the latest version string for multiple applications | ||
| * within an organization. Returns a map of application_key -> latest_version. | ||
| */ | ||
| def findLatestVersions( | ||
| orgKey: String, | ||
| applicationKeys: Seq[String], | ||
| ): Map[String, String] = { | ||
| if (applicationKeys.isEmpty) { | ||
| Map.empty | ||
| } else { | ||
| val keyParams = applicationKeys.zipWithIndex.map { case (_, i) => s"{app_key_$i}" }.mkString(", ") | ||
| val namedParams = applicationKeys.zipWithIndex.map { case (key, i) => | ||
| NamedParameter(s"app_key_$i", key) | ||
| } | ||
|
|
||
| dao.db.withConnection { implicit c => | ||
| SQL( | ||
| s"""select distinct on (a.key) a.key as app_key, versions.version | ||
| | from versions | ||
| | join applications a on a.guid = versions.application_guid | ||
| | join organizations o on o.guid = a.organization_guid | ||
| | where o.key = {org_key} | ||
| | and a.key in ($keyParams) | ||
| | and versions.deleted_at is null | ||
|
Comment on lines
+223
to
+236
|
||
| | and a.deleted_at is null | ||
| | and o.deleted_at is null | ||
| | and $HasServiceJsonClause | ||
| | order by a.key, versions.version_sort_key desc""".stripMargin | ||
| ).on( | ||
| (Seq(NamedParameter("org_key", orgKey)) ++ namedParams)* | ||
| ).as( | ||
| (SqlParser.str("app_key") ~ SqlParser.str("version")).map { case key ~ version => key -> version }.* | ||
| ).toMap | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Efficient query to fetch all versions of a given application | ||
| def findAllVersions( | ||
| authorization: Authorization, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package services | ||
|
|
||
| import cats.data.ValidatedNec | ||
| import cats.implicits.* | ||
| import db.InternalVersionsDao | ||
| import io.apibuilder.api.v0.models.{BatchVersionLatest, BatchVersionsLatest, BatchVersionsLatestForm} | ||
|
|
||
| import javax.inject.Inject | ||
|
|
||
| class BatchVersionsLatestService @Inject() ( | ||
| versionsDao: InternalVersionsDao, | ||
| ) { | ||
|
|
||
| private val MaxApplicationKeys = 500 | ||
|
|
||
| def process( | ||
| orgKey: String, | ||
| form: BatchVersionsLatestForm, | ||
| ): ValidatedNec[String, BatchVersionsLatest] = { | ||
| if (form.applicationKeys.length > MaxApplicationKeys) { | ||
| s"Maximum of $MaxApplicationKeys application keys allowed, but ${form.applicationKeys.length} were provided".invalidNec | ||
| } else { | ||
| val latestVersions = versionsDao.findLatestVersions(orgKey, form.applicationKeys) | ||
|
|
||
| BatchVersionsLatest( | ||
| applications = form.applicationKeys.map { key => | ||
| BatchVersionLatest( | ||
| applicationKey = key, | ||
| latestVersion = latestVersions.get(key), | ||
| ) | ||
| }, | ||
| ).validNec | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||||||||||||||||||||||||||||
| package controllers | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import org.scalatestplus.play.guice.GuiceOneServerPerSuite | ||||||||||||||||||||||||||||||||||
| import org.scalatestplus.play.PlaySpec | ||||||||||||||||||||||||||||||||||
| import io.apibuilder.api.v0.models.BatchVersionsLatestForm | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| class BatchVersionsLatestSpec extends PlaySpec with MockClient with GuiceOneServerPerSuite { | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| private lazy val org = createOrganization() | ||||||||||||||||||||||||||||||||||
| private lazy val version = createVersion(createApplication(org)) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| "post" must { | ||||||||||||||||||||||||||||||||||
| "returns latest version for existing application" in { | ||||||||||||||||||||||||||||||||||
| val result = await { | ||||||||||||||||||||||||||||||||||
| client.batchVersionsLatest.post( | ||||||||||||||||||||||||||||||||||
| org.key, | ||||||||||||||||||||||||||||||||||
| BatchVersionsLatestForm(applicationKeys = Seq(version.application.key)) | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| result.applications.size must equal(1) | ||||||||||||||||||||||||||||||||||
| result.applications.head.applicationKey must equal(version.application.key) | ||||||||||||||||||||||||||||||||||
| result.applications.head.latestVersion must equal(Some(version.version)) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| "returns no version for non-existent application" in { | ||||||||||||||||||||||||||||||||||
| val result = await { | ||||||||||||||||||||||||||||||||||
| client.batchVersionsLatest.post( | ||||||||||||||||||||||||||||||||||
| org.key, | ||||||||||||||||||||||||||||||||||
| BatchVersionsLatestForm(applicationKeys = Seq(randomString())) | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| result.applications.size must equal(1) | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+28
to
+34
|
||||||||||||||||||||||||||||||||||
| val result = await { | |
| client.batchVersionsLatest.post( | |
| org.key, | |
| BatchVersionsLatestForm(applicationKeys = Seq(randomString())) | |
| ) | |
| } | |
| result.applications.size must equal(1) | |
| val nonExistent = randomString() | |
| val result = await { | |
| client.batchVersionsLatest.post( | |
| org.key, | |
| BatchVersionsLatestForm(applicationKeys = Seq(nonExistent)) | |
| ) | |
| } | |
| result.applications.size must equal(1) | |
| result.applications.head.applicationKey must equal(nonExistent) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -546,6 +546,29 @@ | |||||
| { "name": "application_key", "type": "string" }, | ||||||
| { "name": "version", "type": "string", "default": "latest", "required": false } | ||||||
| ] | ||||||
| }, | ||||||
|
|
||||||
| "batch_versions_latest_form": { | ||||||
| "description": "Form to request the latest version for multiple applications in a single API call.", | ||||||
| "fields": [ | ||||||
| { "name": "application_keys", "type": "[string]", "description": "List of application keys to look up" } | ||||||
|
||||||
| { "name": "application_keys", "type": "[string]", "description": "List of application keys to look up" } | |
| { "name": "application_keys", "type": "[string]", "description": "List of application keys to look up (maximum 500 keys per request)" } |
Copilot
AI
Mar 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec declares only a 200 response, but the controller can return a non-200 on validation failure (currently 409). This is an API-contract mismatch for generated clients. Either (a) add the error response code(s) and type(s) to the spec (e.g., 400/409 with the standard error model), or (b) change the controller to only return documented status codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning
409 Conflictfor input validation failures (e.g., too many keys) is atypical;400 Bad Requestis usually the correct status for invalid request bodies. Consider switching toBadRequest(...)here (and documenting that code in the API spec) so clients can handle validation errors consistently.