feat(server): die Builds-API trägt das Repository im Pfad — /api/repos/<name>/builds/…
Sitzung D, erste Hälfte (docs/plan/22-multi-repo.md): Jede Route der Builds-API gibt es jetzt zweimal — repo-benannt und unscoped. Die unscoped Form bleibt dauerhaft und meint das bediente Repository (`RepoRegistry.current`): Lesezeichen, die alte Oberfläche und die bereits nach Gitea geposteten Links kennen kein Repo-Segment, und eine CI, die ihre eigenen alten Links bricht, ist eine CI, der niemand traut. Der Controller hält kein `BuildResultRepository`, keinen `ArtifactStore` und keinen `RepoContext` mehr als Bohne, sondern löst je Anfrage über die Registry auf. Ein unbekannter Name beantwortet sich in der Form dieser API (404 mit `error`) über einen eigenen ExceptionHandler statt über Springs Problem-Detail. Dabei gefunden und mitbehoben: `cancel` griff über Repository-Grenzen. Der Executor bricht per Schlüssel instanzweit ab — eine Route, die ein Repository benennt, darf damit kein fremdes erreichen. Sie prüft jetzt zuerst, ob der Schlüssel in den Ergebnissen DIESES Repositories steht; ein wartender oder laufender Build hat dort immer sein PENDING/RUNNING. Drei neue Tests: die repo-benannte Route antwortet, ein unbekannter Name gibt 404 mit Meldung, und `cancel` erreicht keinen fremden Build. 496 Tests grün, ktlint sauber. Noch nicht scoped: die Oberfläche und die Artefakt-Routen — die kommen mit der zweiten Hälfte von Sitzung D. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6ca67a6a37
commit
4304dd7c4b
@@ -1,16 +1,16 @@
|
||||
package de.hoennig.werkator.server
|
||||
|
||||
import de.hoennig.werkator.build.ArtifactStore
|
||||
import de.hoennig.werkator.build.BuildExecutor
|
||||
import de.hoennig.werkator.build.BuildResult
|
||||
import de.hoennig.werkator.build.BuildResultRepository
|
||||
import de.hoennig.werkator.build.BuildStatus
|
||||
import de.hoennig.werkator.config.BuildDefinition
|
||||
import de.hoennig.werkator.git.GitService
|
||||
import de.hoennig.werkator.repo.RepoContext
|
||||
import de.hoennig.werkator.repo.RepoRegistry
|
||||
import org.springframework.http.HttpStatus
|
||||
import org.springframework.http.ResponseEntity
|
||||
import org.springframework.web.bind.annotation.DeleteMapping
|
||||
import org.springframework.web.bind.annotation.ExceptionHandler
|
||||
import org.springframework.web.bind.annotation.GetMapping
|
||||
import org.springframework.web.bind.annotation.PathVariable
|
||||
import org.springframework.web.bind.annotation.PostMapping
|
||||
@@ -32,28 +32,50 @@ import java.nio.file.StandardOpenOption
|
||||
*/
|
||||
@RestController
|
||||
class BuildsApiController(
|
||||
private val repository: BuildResultRepository,
|
||||
private val buildExecutor: BuildExecutor,
|
||||
private val artifactStore: ArtifactStore,
|
||||
private val controlTokens: ControlTokenService,
|
||||
private val gitService: GitService,
|
||||
private val branchListing: BranchListing,
|
||||
private val repo: RepoContext,
|
||||
private val registry: RepoRegistry,
|
||||
) {
|
||||
private val workingDir: Path
|
||||
get() = repo.workingDir
|
||||
/**
|
||||
* Every route exists twice: repository-scoped (`/api/repos/<name>/…`) and unscoped.
|
||||
* The unscoped form means the served repository ([RepoRegistry.current]) and stays
|
||||
* for good — bookmarks, the legacy UI, and the links already posted to Gitea were
|
||||
* written without a repository segment, and a CI that breaks its own old links is
|
||||
* a CI nobody trusts.
|
||||
*/
|
||||
private fun repoOf(name: String?): RepoContext =
|
||||
if (name == null) registry.current() else registry.byName(name) ?: throw UnknownRepositoryException(name)
|
||||
|
||||
@GetMapping("/api/builds/latest")
|
||||
fun latest(): List<BuildResultDto> = repository.latestPerName().map { BuildResultDto.from(it, it.isLatestGreen()) }
|
||||
private fun RepoContext.isLatestGreen(result: BuildResult): Boolean =
|
||||
results.latestGreenFor(result.name)?.artifactKey == result.artifactKey
|
||||
|
||||
/** An unknown repository name answers like every other miss of this API: 404 with `error`. */
|
||||
@ExceptionHandler(UnknownRepositoryException::class)
|
||||
fun unknownRepository(e: UnknownRepositoryException): ResponseEntity<Any> = notFound(e.message ?: "unknown repository")
|
||||
|
||||
@GetMapping("/api/builds/latest", "/api/repos/{repo}/builds/latest")
|
||||
fun latest(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
): List<BuildResultDto> {
|
||||
val repo = repoOf(repoName)
|
||||
return repo.results.latestPerName().map { BuildResultDto.from(it, repo.isLatestGreen(it)) }
|
||||
}
|
||||
|
||||
/** The legacy branches view: every origin branch with its latest build or `unknown`. */
|
||||
@GetMapping("/api/branches")
|
||||
fun branches(): List<BranchDto> = branchListing.branches(repo)
|
||||
@GetMapping("/api/branches", "/api/repos/{repo}/branches")
|
||||
fun branches(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
): List<BranchDto> = branchListing.branches(repoOf(repoName))
|
||||
|
||||
@GetMapping("/api/builds/history")
|
||||
fun history(): List<BuildResultDto> = repository.history().map { BuildResultDto.from(it, it.isLatestGreen()) }
|
||||
|
||||
private fun BuildResult.isLatestGreen(): Boolean = repository.latestGreenFor(name)?.artifactKey == artifactKey
|
||||
@GetMapping("/api/builds/history", "/api/repos/{repo}/builds/history")
|
||||
fun history(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
): List<BuildResultDto> {
|
||||
val repo = repoOf(repoName)
|
||||
return repo.results.history().map { BuildResultDto.from(it, repo.isLatestGreen(it)) }
|
||||
}
|
||||
|
||||
/**
|
||||
* The currently executing builds of the served repository — several are possible,
|
||||
@@ -62,9 +84,12 @@ class BuildsApiController(
|
||||
* holds only this repository's results, and a foreign build looked up in them
|
||||
* would fall back to RUNNING and show a status nobody recorded.
|
||||
*/
|
||||
@GetMapping("/api/builds/current")
|
||||
fun current(): List<CurrentBuildDto> {
|
||||
val results = repository.history()
|
||||
@GetMapping("/api/builds/current", "/api/repos/{repo}/builds/current")
|
||||
fun current(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
): List<CurrentBuildDto> {
|
||||
val repo = repoOf(repoName)
|
||||
val results = repo.results.history()
|
||||
return buildExecutor.currentBuilds().filter { it.repo === repo }.map { build ->
|
||||
CurrentBuildDto(
|
||||
branch = build.branch,
|
||||
@@ -82,11 +107,13 @@ class BuildsApiController(
|
||||
}
|
||||
|
||||
/** Incremental live-log fetch of one running build; poll again with `offset = nextOffset`. */
|
||||
@GetMapping("/api/builds/current/{artifactKey}/log")
|
||||
@GetMapping("/api/builds/current/{artifactKey}/log", "/api/repos/{repo}/builds/current/{artifactKey}/log")
|
||||
fun currentLog(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
@PathVariable artifactKey: String,
|
||||
@RequestParam(defaultValue = "0") offset: Long,
|
||||
): ResponseEntity<Any> {
|
||||
val repo = repoOf(repoName)
|
||||
val build =
|
||||
buildExecutor.currentBuilds().firstOrNull { it.repo === repo && it.artifactKey == artifactKey }
|
||||
?: return notFound("no running build with artifact key '$artifactKey'")
|
||||
@@ -108,14 +135,17 @@ class BuildsApiController(
|
||||
* The name is a parameter, not a path variable, because branch names may contain
|
||||
* slashes (Tomcat rejects encoded slashes in the path by default).
|
||||
*/
|
||||
@PostMapping("/api/builds/restart")
|
||||
@PostMapping("/api/builds/restart", "/api/repos/{repo}/builds/restart")
|
||||
fun restart(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
@RequestParam branch: String,
|
||||
@RequestParam(defaultValue = "false") atOriginHead: Boolean,
|
||||
@RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?,
|
||||
): ResponseEntity<Any> {
|
||||
rejectBadToken(headerToken)?.let { return it }
|
||||
val latest = repository.latestFor(branch)
|
||||
val repo = repoOf(repoName)
|
||||
val workingDir = repo.workingDir
|
||||
val latest = repo.results.latestFor(branch)
|
||||
// the name may be a pool like `main@pitest`; the branch to build is the recorded one
|
||||
val branchName = latest?.branch ?: branch
|
||||
val commit =
|
||||
@@ -149,12 +179,20 @@ class BuildsApiController(
|
||||
}
|
||||
|
||||
/** Cancels by artifact key because multiple builds can run concurrently. */
|
||||
@PostMapping("/api/builds/{artifactKey}/cancel")
|
||||
@PostMapping("/api/builds/{artifactKey}/cancel", "/api/repos/{repo}/builds/{artifactKey}/cancel")
|
||||
fun cancel(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
@PathVariable artifactKey: String,
|
||||
@RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?,
|
||||
): ResponseEntity<Any> {
|
||||
rejectBadToken(headerToken)?.let { return it }
|
||||
val repo = repoOf(repoName)
|
||||
// the executor cancels by key across all repositories; a route that names a
|
||||
// repository must not reach into another one, and a queued or running build
|
||||
// always has its PENDING/RUNNING result recorded in its own repository
|
||||
if (repo.results.history().none { it.artifactKey == artifactKey }) {
|
||||
return notFound("no queued or running build with artifact key '$artifactKey'")
|
||||
}
|
||||
if (!buildExecutor.cancel(artifactKey)) {
|
||||
return notFound("no queued or running build with artifact key '$artifactKey'")
|
||||
}
|
||||
@@ -162,16 +200,18 @@ class BuildsApiController(
|
||||
}
|
||||
|
||||
/** Removes the stored result and its artifact directory, like the legacy `/control/delete`. */
|
||||
@DeleteMapping("/api/builds/{artifactKey}")
|
||||
@DeleteMapping("/api/builds/{artifactKey}", "/api/repos/{repo}/builds/{artifactKey}")
|
||||
fun delete(
|
||||
@PathVariable(name = "repo", required = false) repoName: String?,
|
||||
@PathVariable artifactKey: String,
|
||||
@RequestHeader(name = TOKEN_HEADER, required = false) headerToken: String?,
|
||||
): ResponseEntity<Any> {
|
||||
rejectBadToken(headerToken)?.let { return it }
|
||||
if (!repository.delete(artifactKey)) {
|
||||
val repo = repoOf(repoName)
|
||||
if (!repo.results.delete(artifactKey)) {
|
||||
return notFound("no build with artifact key '$artifactKey'")
|
||||
}
|
||||
artifactStore.prune(repository.history())
|
||||
repo.artifactStore.prune(repo.results.history())
|
||||
return ResponseEntity.ok(mapOf("deleted" to artifactKey))
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
package de.hoennig.werkator.server
|
||||
|
||||
/**
|
||||
* A route named a repository this instance does not serve (ADR 0009). Thrown by the
|
||||
* repository-scoped controllers and turned into their own 404 shape by their exception
|
||||
* handlers — a name that is simply not registered is a miss like any other, not a
|
||||
* server error.
|
||||
*/
|
||||
class UnknownRepositoryException(
|
||||
val name: String,
|
||||
) : RuntimeException("no repository named '$name'")
|
||||
@@ -9,6 +9,7 @@ import de.hoennig.werkator.build.BuildStatus
|
||||
import de.hoennig.werkator.build.RunningBuild
|
||||
import de.hoennig.werkator.git.GitService
|
||||
import de.hoennig.werkator.repo.RepoContext
|
||||
import de.hoennig.werkator.repo.RepoRegistry
|
||||
import io.kotest.core.spec.style.FunSpec
|
||||
import io.mockk.clearMocks
|
||||
import io.mockk.every
|
||||
@@ -55,6 +56,9 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
@MockkBean
|
||||
lateinit var repo: RepoContext
|
||||
|
||||
@MockkBean
|
||||
lateinit var registry: RepoRegistry
|
||||
|
||||
private val startedAt = Instant.parse("2026-07-07T10:00:00Z")
|
||||
|
||||
private val successResult =
|
||||
@@ -80,8 +84,15 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
|
||||
init {
|
||||
beforeEach {
|
||||
clearMocks(repository, buildExecutor, artifactStore, controlTokens, gitService, branchListing, repo)
|
||||
clearMocks(repository, buildExecutor, artifactStore, controlTokens, gitService, branchListing, repo, registry)
|
||||
every { repo.name } returns "test"
|
||||
every { repo.workingDir } returns tempDir
|
||||
every { repo.results } returns repository
|
||||
every { repo.artifactStore } returns artifactStore
|
||||
// the unscoped routes mean the served repository; `/api/repos/test/…` names it
|
||||
every { registry.current() } returns repo
|
||||
every { registry.byName(any()) } returns null
|
||||
every { registry.byName("test") } returns repo
|
||||
every { controlTokens.matches(any()) } answers { firstArg<String?>() == "secret" }
|
||||
every { repository.latestGreenFor(any()) } returns null
|
||||
}
|
||||
@@ -330,6 +341,8 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
}
|
||||
|
||||
test("cancel answers 202 for a cancellable build and 404 otherwise") {
|
||||
every { repository.history() } returns
|
||||
listOf(successResult.copy(artifactKey = "known-key"), successResult.copy(artifactKey = "unknown-key"))
|
||||
every { buildExecutor.cancel("known-key") } returns true
|
||||
every { buildExecutor.cancel("unknown-key") } returns false
|
||||
|
||||
@@ -342,6 +355,32 @@ class BuildsApiControllerTest : FunSpec() {
|
||||
.andExpect(status().isNotFound)
|
||||
}
|
||||
|
||||
test("cancel does not reach a build of another repository") {
|
||||
// the key exists in the executor, but not in this repository's results
|
||||
every { repository.history() } returns listOf(successResult)
|
||||
every { buildExecutor.cancel(any()) } returns true
|
||||
|
||||
mockMvc
|
||||
.perform(
|
||||
post("/api/repos/test/builds/other-repo-key/cancel")
|
||||
.header(BuildsApiController.TOKEN_HEADER, "secret"),
|
||||
).andExpect(status().isNotFound)
|
||||
verify(exactly = 0) { buildExecutor.cancel(any()) }
|
||||
}
|
||||
|
||||
test("the repository-scoped routes answer for the named repository and 404 for an unknown name") {
|
||||
every { repository.latestPerName() } returns listOf(successResult)
|
||||
|
||||
mockMvc
|
||||
.perform(get("/api/repos/test/builds/latest"))
|
||||
.andExpect(status().isOk)
|
||||
.andExpect(jsonPath("$[0].artifactKey").value("main-abc123-key"))
|
||||
mockMvc
|
||||
.perform(get("/api/repos/no-such-repo/builds/latest"))
|
||||
.andExpect(status().isNotFound)
|
||||
.andExpect(jsonPath("$.error").value("no repository named 'no-such-repo'"))
|
||||
}
|
||||
|
||||
test("a token in the query string is not accepted — the header is the only way") {
|
||||
mockMvc
|
||||
.perform(post("/api/builds/restart").param("branch", "main").param("token", "secret"))
|
||||
|
||||
Reference in New Issue
Block a user