Implement TASK-009: commit history and diff read-only API contracts
Implemented directly at human product owner's direction, independent of Codex's separate uncommitted attempt at the same task (not read or referenced here). - CommitDetail (reusing CommitSummary) with per-file added/modified/ deleted/renamed/other status; deleted-author and missing-rename-path cases handled without crashing or inventing data. - BoundedPatch for commit and comparison diffs, sourced from Gitea's .diff raw-text endpoints (JSON commit/compare objects only carry file status, not diff content); byte limit then line limit, both constructor-injected and validated positive. - Comparison uses one combined base...head path segment, built by concatenating both refs before a single addPathSegment call so each is escaped exactly once; pagination computed locally since Gitea's compare endpoint has no server-side paging. - Reuses Page<T> and ApiFailure; no put/delete/postForm call anywhere in the file (read-only, as required). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
7d122d4388
commit
3c17476e92
@ -0,0 +1,185 @@
|
||||
package com.ubisam.ubgitea.data.history
|
||||
|
||||
import com.google.gson.Gson
|
||||
import com.google.gson.JsonParseException
|
||||
import com.google.gson.annotations.SerializedName
|
||||
import com.ubisam.ubgitea.core.auth.protocol.OAuthClientConfig
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.HttpResponse
|
||||
import com.ubisam.ubgitea.core.network.HttpTransport
|
||||
import com.ubisam.ubgitea.core.network.requireSuccess
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.domain.history.BoundedPatch
|
||||
import com.ubisam.ubgitea.domain.history.CommitDetail
|
||||
import com.ubisam.ubgitea.domain.history.FileChange
|
||||
import com.ubisam.ubgitea.domain.history.FileChangeStatus
|
||||
import com.ubisam.ubgitea.domain.history.HistoryRepository
|
||||
import com.ubisam.ubgitea.domain.model.CommitSummary
|
||||
import com.ubisam.ubgitea.domain.model.Page
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryRef
|
||||
import java.io.IOException
|
||||
import java.time.Instant
|
||||
import java.time.format.DateTimeParseException
|
||||
import okhttp3.HttpUrl
|
||||
import okhttp3.HttpUrl.Companion.toHttpUrl
|
||||
|
||||
/**
|
||||
* Read-only commit detail, bounded diff/patch text, and ref comparison against Gitea. See
|
||||
* [HistoryRepository] for why commit *listing* is intentionally absent from this class.
|
||||
*/
|
||||
class GiteaHistoryApi(
|
||||
private val config: OAuthClientConfig,
|
||||
private val transport: HttpTransport,
|
||||
private val tokenProvider: BearerTokenProvider,
|
||||
private val maxPatchBytes: Int = 200_000,
|
||||
private val maxPatchLines: Int = 4_000,
|
||||
private val gson: Gson = Gson(),
|
||||
) : HistoryRepository {
|
||||
init {
|
||||
require(maxPatchBytes > 0) { "maxPatchBytes must be positive" }
|
||||
require(maxPatchLines > 0) { "maxPatchLines must be positive" }
|
||||
}
|
||||
|
||||
override suspend fun getCommitDetail(repository: RepositoryCoordinates, sha: String): CommitDetail {
|
||||
val dto = parseObject<CommitDto>(get(repoUrl(repository, "commits", sha.required())).body)
|
||||
return mapCommitDetail(dto)
|
||||
}
|
||||
|
||||
override suspend fun getCommitPatch(repository: RepositoryCoordinates, sha: String): BoundedPatch {
|
||||
val url = repoUrl(repository, "git", "commits", "${sha.required()}.diff")
|
||||
return bound(get(url).body)
|
||||
}
|
||||
|
||||
override suspend fun compare(
|
||||
repository: RepositoryCoordinates,
|
||||
baseRef: RepositoryRef,
|
||||
headRef: RepositoryRef,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommitDetail> {
|
||||
validatePage(page, pageSize)
|
||||
val dto = parseObject<CompareDto>(get(compareUrl(repository, baseRef, headRef)).body)
|
||||
val commits = dto.commits.orEmpty().map(::mapCommitDetail)
|
||||
val fromIndex = ((page - 1L) * pageSize).coerceAtMost(commits.size.toLong()).toInt()
|
||||
val toIndex = (fromIndex + pageSize).coerceAtMost(commits.size)
|
||||
val next = if (toIndex < commits.size) page.safeNextPage() else null
|
||||
return Page.of(commits.subList(fromIndex, toIndex), next)
|
||||
}
|
||||
|
||||
override suspend fun getComparePatch(
|
||||
repository: RepositoryCoordinates,
|
||||
baseRef: RepositoryRef,
|
||||
headRef: RepositoryRef,
|
||||
): BoundedPatch = bound(get(compareUrl(repository, baseRef, headRef, suffix = ".diff")).body)
|
||||
|
||||
private fun bound(text: String): BoundedPatch {
|
||||
val byteTruncated = text.toByteArray(Charsets.UTF_8).size > maxPatchBytes
|
||||
val bytesBounded = if (byteTruncated) {
|
||||
String(text.toByteArray(Charsets.UTF_8).copyOf(maxPatchBytes), Charsets.UTF_8)
|
||||
} else {
|
||||
text
|
||||
}
|
||||
val lines = bytesBounded.split("\n")
|
||||
val lineTruncated = lines.size > maxPatchLines
|
||||
val bounded = if (lineTruncated) lines.take(maxPatchLines).joinToString("\n") else bytesBounded
|
||||
return BoundedPatch(bounded, byteTruncated || lineTruncated)
|
||||
}
|
||||
|
||||
private fun mapCommitDetail(dto: CommitDto): CommitDetail {
|
||||
val id = dto.sha.required()
|
||||
val author = dto.commit?.author ?: throw ApiFailure.InvalidResponse()
|
||||
val summary = CommitSummary(
|
||||
id = id,
|
||||
shortId = id.take(7),
|
||||
message = dto.commit.message.required(),
|
||||
authorName = author.name.required(),
|
||||
authoredAtEpochMillis = author.date.required().toEpochMillisOrInvalid(),
|
||||
)
|
||||
val files = dto.files.orEmpty().map { file ->
|
||||
FileChange(
|
||||
path = file.filename.required(),
|
||||
previousPath = file.previousFilename?.takeIf(String::isNotBlank),
|
||||
status = file.status.toFileChangeStatus(),
|
||||
)
|
||||
}
|
||||
return CommitDetail(summary, files)
|
||||
}
|
||||
|
||||
private fun String?.toFileChangeStatus(): FileChangeStatus = when (this) {
|
||||
"added" -> FileChangeStatus.ADDED
|
||||
"modified", "changed" -> FileChangeStatus.MODIFIED
|
||||
"deleted", "removed" -> FileChangeStatus.DELETED
|
||||
"renamed" -> FileChangeStatus.RENAMED
|
||||
else -> FileChangeStatus.OTHER
|
||||
}
|
||||
|
||||
private fun String.toEpochMillisOrInvalid(): Long = try {
|
||||
Instant.parse(this).toEpochMilli()
|
||||
} catch (_: DateTimeParseException) {
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private suspend fun get(url: HttpUrl): HttpResponse {
|
||||
val token = tokenProvider.accessToken()
|
||||
if (token.isBlank()) throw ApiFailure.AuthenticationExpired()
|
||||
return try {
|
||||
transport.get(url.toString(), token).requireSuccess()
|
||||
} catch (failure: IOException) {
|
||||
throw ApiFailure.Network(failure)
|
||||
}
|
||||
}
|
||||
|
||||
private inline fun <reified T> parseObject(body: String): T = try {
|
||||
gson.fromJson(body, T::class.java) ?: throw ApiFailure.InvalidResponse()
|
||||
} catch (_: JsonParseException) {
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private fun apiUrl(vararg segments: String): HttpUrl = config.baseUri.toString().toHttpUrl()
|
||||
.newBuilder()
|
||||
.apply { segments.forEach(::addPathSegment) }
|
||||
.build()
|
||||
|
||||
private fun repoUrl(repository: RepositoryCoordinates, vararg tail: String): HttpUrl =
|
||||
apiUrl("api", "v1", "repos", repository.owner, repository.name, *tail)
|
||||
|
||||
// Gitea's compare endpoint takes one combined "base...head" path segment. Building that string
|
||||
// first and passing it through a single addPathSegment call encodes each ref's characters
|
||||
// (including any "/" inside a branch name) exactly once; the literal ".." separator is left
|
||||
// untouched because "." is an unreserved URL character.
|
||||
private fun compareUrl(
|
||||
repository: RepositoryCoordinates,
|
||||
baseRef: RepositoryRef,
|
||||
headRef: RepositoryRef,
|
||||
suffix: String = "",
|
||||
): HttpUrl = repoUrl(repository, "compare", "${baseRef.value}...${headRef.value}$suffix")
|
||||
|
||||
private fun String?.required(): String = takeIf { !it.isNullOrBlank() }
|
||||
?: throw ApiFailure.InvalidResponse()
|
||||
|
||||
private fun validatePage(page: Int, pageSize: Int) {
|
||||
require(page > 0) { "Page must be positive" }
|
||||
require(pageSize in 1..100) { "Page size must be between 1 and 100" }
|
||||
}
|
||||
|
||||
private fun Int.safeNextPage(): Int =
|
||||
if (this == Int.MAX_VALUE) throw ApiFailure.InvalidResponse() else this + 1
|
||||
|
||||
private data class CommitAuthorDto(val name: String?, val date: String?)
|
||||
private data class CommitDataDto(val message: String?, val author: CommitAuthorDto?)
|
||||
private data class CommitFileDto(
|
||||
val filename: String?,
|
||||
val status: String?,
|
||||
@SerializedName("previous_filename") val previousFilename: String?,
|
||||
)
|
||||
private data class CommitDto(
|
||||
val sha: String?,
|
||||
val commit: CommitDataDto?,
|
||||
val files: List<CommitFileDto>?,
|
||||
)
|
||||
private data class CompareDto(
|
||||
@SerializedName("total_commits") val totalCommits: Int?,
|
||||
val commits: List<CommitDto>?,
|
||||
)
|
||||
}
|
||||
@ -0,0 +1,27 @@
|
||||
package com.ubisam.ubgitea.domain.history
|
||||
|
||||
import com.ubisam.ubgitea.domain.model.CommitSummary
|
||||
|
||||
/**
|
||||
* OTHER covers Gitea file-change statuses beyond added/modified/deleted/renamed (e.g. copied,
|
||||
* typechange) so an unrecognized value degrades gracefully instead of failing a whole commit
|
||||
* closed. Gitea does not reliably supply [FileChange.previousPath] for a renamed file; treat it
|
||||
* as absent rather than assuming it is always present.
|
||||
*/
|
||||
enum class FileChangeStatus { ADDED, MODIFIED, DELETED, RENAMED, OTHER }
|
||||
|
||||
data class FileChange(
|
||||
val path: String,
|
||||
val previousPath: String?,
|
||||
val status: FileChangeStatus,
|
||||
)
|
||||
|
||||
data class CommitDetail(
|
||||
val summary: CommitSummary,
|
||||
val files: List<FileChange>,
|
||||
)
|
||||
|
||||
data class BoundedPatch(
|
||||
val text: String,
|
||||
val isTruncated: Boolean,
|
||||
)
|
||||
@ -0,0 +1,32 @@
|
||||
package com.ubisam.ubgitea.domain.history
|
||||
|
||||
import com.ubisam.ubgitea.domain.model.Page
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryRef
|
||||
|
||||
/**
|
||||
* Read-only commit detail, bounded diff/patch text, and ref comparison. Commit *history listing*
|
||||
* itself is already provided by [com.ubisam.ubgitea.domain.repository.GiteaRepositoryBrowser.listCommits]
|
||||
* (TASK-004); this interface adds only what that contract does not: per-commit changed files,
|
||||
* bounded patch text, and comparing two refs. No write method exists here or may be added without
|
||||
* a separate acceptance-criteria task.
|
||||
*/
|
||||
interface HistoryRepository {
|
||||
suspend fun getCommitDetail(repository: RepositoryCoordinates, sha: String): CommitDetail
|
||||
|
||||
suspend fun getCommitPatch(repository: RepositoryCoordinates, sha: String): BoundedPatch
|
||||
|
||||
suspend fun compare(
|
||||
repository: RepositoryCoordinates,
|
||||
baseRef: RepositoryRef,
|
||||
headRef: RepositoryRef,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommitDetail>
|
||||
|
||||
suspend fun getComparePatch(
|
||||
repository: RepositoryCoordinates,
|
||||
baseRef: RepositoryRef,
|
||||
headRef: RepositoryRef,
|
||||
): BoundedPatch
|
||||
}
|
||||
@ -0,0 +1,231 @@
|
||||
package com.ubisam.ubgitea.data.history
|
||||
|
||||
import com.ubisam.ubgitea.core.auth.protocol.UBGiteaOAuthDefaults
|
||||
import com.ubisam.ubgitea.core.network.ApiFailure
|
||||
import com.ubisam.ubgitea.core.network.HttpResponse
|
||||
import com.ubisam.ubgitea.core.network.HttpTransport
|
||||
import com.ubisam.ubgitea.data.repositories.BearerTokenProvider
|
||||
import com.ubisam.ubgitea.domain.history.FileChangeStatus
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryRef
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import okhttp3.HttpUrl.Companion.toHttpUrl
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
|
||||
class GiteaHistoryApiTest {
|
||||
private val repository = RepositoryCoordinates.of("team name", "mobile+#")
|
||||
|
||||
@Test
|
||||
fun `commit detail maps files with every recognized status plus a fallback for unknown ones`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""{"sha":"abcdef1234567890","commit":{"message":"Fix it","author":{"name":"Dev","date":"2026-08-25T10:20:30Z"}},
|
||||
"files":[
|
||||
{"filename":"a.kt","status":"added"},
|
||||
{"filename":"b.kt","status":"modified"},
|
||||
{"filename":"c.kt","status":"deleted"},
|
||||
{"filename":"d.kt","status":"renamed","previous_filename":"old_d.kt"},
|
||||
{"filename":"e.kt","status":"typechange"}
|
||||
]}""",
|
||||
),
|
||||
)
|
||||
|
||||
val detail = api(transport).getCommitDetail(repository, "abcdef1234567890")
|
||||
|
||||
assertEquals("abcdef1", detail.summary.shortId)
|
||||
assertEquals(
|
||||
listOf(
|
||||
FileChangeStatus.ADDED,
|
||||
FileChangeStatus.MODIFIED,
|
||||
FileChangeStatus.DELETED,
|
||||
FileChangeStatus.RENAMED,
|
||||
FileChangeStatus.OTHER,
|
||||
),
|
||||
detail.files.map { it.status },
|
||||
)
|
||||
assertEquals("old_d.kt", detail.files[3].previousPath)
|
||||
assertNull(detail.files[0].previousPath)
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(listOf("api", "v1", "repos", "team name", "mobile+#", "commits", "abcdef1234567890"), url.pathSegments)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a renamed file without a previous_filename maps previousPath to null instead of crashing`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""{"sha":"abc","commit":{"message":"m","author":{"name":"d","date":"2026-08-25T10:20:30Z"}},
|
||||
"files":[{"filename":"d.kt","status":"renamed"}]}""",
|
||||
),
|
||||
)
|
||||
|
||||
val detail = api(transport).getCommitDetail(repository, "abc")
|
||||
|
||||
assertEquals(FileChangeStatus.RENAMED, detail.files.single().status)
|
||||
assertNull(detail.files.single().previousPath)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `commit patch under both limits is returned whole and not truncated`() = runTest {
|
||||
val patchText = "diff --git a/x b/x\n+line\n"
|
||||
val transport = QueueTransport(response(patchText))
|
||||
|
||||
val patch = api(transport, maxPatchBytes = 10_000, maxPatchLines = 100).getCommitPatch(repository, "abc")
|
||||
|
||||
assertEquals(patchText, patch.text)
|
||||
assertFalse(patch.isTruncated)
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(listOf("api", "v1", "repos", "team name", "mobile+#", "git", "commits", "abc.diff"), url.pathSegments)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `commit patch over the byte limit is truncated and marked`() = runTest {
|
||||
val transport = QueueTransport(response("x".repeat(1_000)))
|
||||
|
||||
val patch = api(transport, maxPatchBytes = 100, maxPatchLines = 1_000).getCommitPatch(repository, "abc")
|
||||
|
||||
assertEquals(100, patch.text.toByteArray(Charsets.UTF_8).size)
|
||||
assertTrue(patch.isTruncated)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `commit patch over the line limit is truncated and marked`() = runTest {
|
||||
val transport = QueueTransport(response((1..50).joinToString("\n") { "line $it" }))
|
||||
|
||||
val patch = api(transport, maxPatchBytes = 100_000, maxPatchLines = 5).getCommitPatch(repository, "abc")
|
||||
|
||||
assertEquals(5, patch.text.split("\n").size)
|
||||
assertTrue(patch.isTruncated)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `compare encodes base and head refs into one basehead segment even when a ref contains a slash`() = runTest {
|
||||
val transport = QueueTransport(response("""{"total_commits":0,"commits":[]}"""))
|
||||
|
||||
api(transport).compare(repository, RepositoryRef.of("main"), RepositoryRef.of("refs/heads/feature/a+b"), 1, 20)
|
||||
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(
|
||||
listOf("api", "v1", "repos", "team name", "mobile+#", "compare", "main...refs/heads/feature/a+b"),
|
||||
url.pathSegments,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `compare paginates its single response locally`() = runTest {
|
||||
fun commitJson(sha: String) =
|
||||
"""{"sha":"$sha","commit":{"message":"m","author":{"name":"d","date":"2026-08-25T10:20:30Z"}},"files":[]}"""
|
||||
val transport = QueueTransport(
|
||||
response("""{"total_commits":3,"commits":[${commitJson("a")},${commitJson("b")},${commitJson("c")}]}"""),
|
||||
)
|
||||
|
||||
val firstPage = api(transport).compare(repository, RepositoryRef.of("main"), RepositoryRef.of("feature"), 1, 2)
|
||||
assertEquals(listOf("a", "b"), firstPage.items.map { it.summary.id })
|
||||
assertEquals(2, firstPage.nextPage)
|
||||
|
||||
val secondPageTransport = QueueTransport(
|
||||
response("""{"total_commits":3,"commits":[${commitJson("a")},${commitJson("b")},${commitJson("c")}]}"""),
|
||||
)
|
||||
val secondPage = api(secondPageTransport).compare(repository, RepositoryRef.of("main"), RepositoryRef.of("feature"), 2, 2)
|
||||
assertEquals(listOf("c"), secondPage.items.map { it.summary.id })
|
||||
assertNull(secondPage.nextPage)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `compare patch requests the diff-suffixed compare url`() = runTest {
|
||||
val transport = QueueTransport(response("diff text"))
|
||||
|
||||
val patch = api(transport).getComparePatch(repository, RepositoryRef.of("main"), RepositoryRef.of("feature"))
|
||||
|
||||
assertEquals("diff text", patch.text)
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(
|
||||
listOf("api", "v1", "repos", "team name", "mobile+#", "compare", "main...feature.diff"),
|
||||
url.pathSegments,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `missing token fails closed with authentication expired`() = runTest {
|
||||
val missingToken = GiteaHistoryApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
QueueTransport(response("{}")),
|
||||
BearerTokenProvider { "" },
|
||||
)
|
||||
try {
|
||||
missingToken.getCommitDetail(repository, "abc")
|
||||
fail("Expected AuthenticationExpired")
|
||||
} catch (_: ApiFailure.AuthenticationExpired) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `http status mapping distinguishes not found and conflict for missing ref and empty repository`() = runTest {
|
||||
val notFound = QueueTransport(HttpResponse(404, "", emptyMap()))
|
||||
try {
|
||||
api(notFound).getCommitDetail(repository, "missing")
|
||||
fail("Expected NotFound")
|
||||
} catch (_: ApiFailure.NotFound) {
|
||||
// Expected.
|
||||
}
|
||||
|
||||
val conflict = QueueTransport(HttpResponse(409, "", emptyMap()))
|
||||
try {
|
||||
api(conflict).getCommitDetail(repository, "abc")
|
||||
fail("Expected Conflict")
|
||||
} catch (_: ApiFailure.Conflict) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `malformed commit response fails closed`() = runTest {
|
||||
val transport = QueueTransport(response("""{"sha":"abc","commit":null,"files":[]}"""))
|
||||
try {
|
||||
api(transport).getCommitDetail(repository, "abc")
|
||||
fail("Expected InvalidResponse")
|
||||
} catch (_: ApiFailure.InvalidResponse) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test(expected = IllegalArgumentException::class)
|
||||
fun `rejects non-positive maxPatchBytes`() {
|
||||
GiteaHistoryApi(UBGiteaOAuthDefaults.config, QueueTransport(), BearerTokenProvider { "access" }, maxPatchBytes = 0)
|
||||
}
|
||||
|
||||
@Test(expected = IllegalArgumentException::class)
|
||||
fun `rejects non-positive maxPatchLines`() {
|
||||
GiteaHistoryApi(UBGiteaOAuthDefaults.config, QueueTransport(), BearerTokenProvider { "access" }, maxPatchLines = 0)
|
||||
}
|
||||
|
||||
private fun api(transport: HttpTransport, maxPatchBytes: Int = 200_000, maxPatchLines: Int = 4_000) = GiteaHistoryApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
transport,
|
||||
BearerTokenProvider { "access" },
|
||||
maxPatchBytes = maxPatchBytes,
|
||||
maxPatchLines = maxPatchLines,
|
||||
)
|
||||
|
||||
private fun response(body: String, headers: Map<String, String> = emptyMap()) =
|
||||
HttpResponse(200, body, headers)
|
||||
|
||||
private class QueueTransport(vararg responses: HttpResponse) : HttpTransport {
|
||||
private val queue = ArrayDeque(responses.toList())
|
||||
val urls = mutableListOf<String>()
|
||||
|
||||
override suspend fun postForm(url: String, fields: Map<String, String>) = error("Unexpected POST")
|
||||
override suspend fun get(url: String, bearerToken: String): HttpResponse {
|
||||
urls += url
|
||||
assertEquals("access", bearerToken)
|
||||
return queue.removeFirst()
|
||||
}
|
||||
override suspend fun put(url: String, bearerToken: String) = error("Unexpected PUT")
|
||||
override suspend fun delete(url: String, bearerToken: String) = error("Unexpected DELETE")
|
||||
}
|
||||
}
|
||||
@ -203,3 +203,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Known limitations: Gitea's exact issue/PR filtering query-parameter behavior was not confirmed against authoritative live documentation; the client-side `pull_request`-field filter is correct regardless, at the cost of potentially shorter-than-requested pages after filtering. No physical device or live Gitea instance was used — coverage is fake-transport/unit-level only, consistent with how TASK-004/009 were verified before their own freeze review.
|
||||
- Next recommended task: Codex reviews and freezes this contract; TASK-008 (PR/issue UI) can then begin. TASK-007 remains blocked on TASK-009 independently.
|
||||
- Next owner: Codex, for review and contract-freeze recording.
|
||||
|
||||
## 2026-08-26 — TASK-009: history and diff API contracts
|
||||
|
||||
- Agent and role: Claude, feature implementer, at the human product owner's explicit direction ("그냥 코덱스 말고 너가 다시 작업해줘" — implement it yourself instead of Codex) rather than continue waiting on Codex's separate, still-uncommitted attempt in the primary worktree.
|
||||
- Commit/diff: branch `claude/task-009-history-diff-contract` from `7d122d4`; new `domain/history/` and `data/history/` packages. Codex's parallel uncommitted attempt at the same task was not read or referenced, to keep this an independent implementation.
|
||||
- Features implemented: Commit detail with per-file added/modified/deleted/renamed/other status, bounded commit and comparison patch/diff text with an explicit truncated flag, and ref comparison with locally-computed pagination.
|
||||
- How it was implemented: Verified Gitea's actual response shapes against its own Go source (`modules/structs/repo_commit.go`, `compare.go` field names) rather than guessing. Deliberately did not duplicate commit-history listing, since TASK-004's `GiteaRepositoryBrowser.listCommits` already provides it. Patch text comes from Gitea's `.diff`-suffixed raw-text endpoints (not the JSON commit/compare objects, which only carry file status, not diff content) and is bounded by byte limit then line limit, both constructor-injected and validated positive. The compare endpoint's single combined `base...head` path segment is built by concatenating both ref values before one `addPathSegment` call, so each ref (including any `/` in a branch name) is escaped exactly once — verified by a test using a ref containing a slash. Comparison pagination is computed locally against Gitea's one-shot compare response, reusing the same slicing pattern TASK-004 already uses for tree listing.
|
||||
- Files changed: `domain/history/HistoryModels.kt`, `domain/history/HistoryRepository.kt`, `data/history/GiteaHistoryApi.kt`, `data/history/GiteaHistoryApiTest.kt`, `tasks/009-history-diff-api-contract.md`.
|
||||
- Checks and results: `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` BUILD SUCCESSFUL — 141 unit tests across 25 suites, 0 failures/errors, 0 lint errors; `git diff --check` clean; sensitive-pattern scan clean; grep-confirmed no `put`/`delete`/`postForm` call in the new API class.
|
||||
- Review findings and resolutions: None — self-review found no P0-P3. Two items flagged for reviewer attention rather than treated as settled: the exact Gitea file-status vocabulary beyond added/modified/deleted/renamed was not exhaustively confirmed, and the `.diff`-suffixed compare route was inferred from the documented single-commit convention rather than independently verified.
|
||||
- Known limitations: No physical device or live Gitea instance was used. Codex was independently attempting this same task in parallel (uncommitted); this entry and this implementation exist alongside that one and the two need to be reconciled by whoever reviews, not assumed to be the same.
|
||||
- Next recommended task: Codex (or whoever reviews) reconciles this implementation against its own parallel TASK-009 attempt, reviews, and records the contract-freeze commit so TASK-007 can begin.
|
||||
- Next owner: Codex, for reconciliation, review, and contract-freeze recording.
|
||||
|
||||
@ -1,8 +1,8 @@
|
||||
# TASK-009: Implement history and diff API contracts
|
||||
|
||||
Status: planned — depends on TASK-004
|
||||
Owner: Codex
|
||||
Reviewer: Claude
|
||||
Status: implemented; all required checks pass; pending Codex review and contract freeze
|
||||
Owner: Claude (human product owner directed Claude to implement this directly rather than wait for Codex, 2026-08-26)
|
||||
Reviewer: Codex
|
||||
|
||||
## Outcome
|
||||
|
||||
@ -18,14 +18,14 @@ Provide frozen read-only domain and data contracts for commit history, changed f
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] Domain contracts distinguish added, modified, renamed, and deleted files.
|
||||
- [ ] Commit history and comparison pagination is explicit and immutable.
|
||||
- [ ] Patch size and line limits produce an explicit truncated state.
|
||||
- [ ] Base and head refs are encoded independently exactly once.
|
||||
- [ ] Empty repository, missing ref, 401, 403, 404, 409, 422, rate-limit, network, parse, and server failures map to typed results.
|
||||
- [ ] Representative JSON and fake-server tests cover commit detail, rename/delete, comparison, pagination, and truncation.
|
||||
- [ ] No write endpoint is exposed.
|
||||
- [ ] Evidence records the reviewed contract-freeze commit for TASK-007.
|
||||
- [x] Domain contracts distinguish added, modified, renamed, and deleted files (`FileChangeStatus`; an `OTHER` fallback covers Gitea statuses beyond those four, e.g. `typechange`, so an unrecognized value degrades instead of failing a whole commit closed).
|
||||
- [x] Commit history and comparison pagination is explicit and immutable — commit *listing* reuses the existing `Page<CommitSummary>` from TASK-004's `GiteaRepositoryBrowser.listCommits`; comparison pagination reuses the same `Page<T>` type, computed locally (Gitea's compare endpoint returns the full commit list in one response with no server-side paging — confirmed against known Gitea API limitations, not assumed).
|
||||
- [x] Patch size and line limits produce an explicit truncated state — `BoundedPatch(text, isTruncated)`; byte limit applied before line limit, both independently tested.
|
||||
- [x] Base and head refs are encoded independently exactly once — Gitea's compare endpoint takes one combined `base...head` path segment; both ref values are concatenated first and passed through a single `addPathSegment` call so each character (including any `/` inside a branch name) is escaped exactly once, verified by a test using a ref containing `/`.
|
||||
- [x] Empty repository, missing ref, 401, 403, 404, 409, 422, rate-limit, network, parse, and server failures map to typed results — reuses the existing `ApiFailure` sealed class (no parallel hierarchy); 404/409 explicitly tested here (a missing ref and an empty-repository response are both plausible Gitea outcomes covered by those two codes).
|
||||
- [x] Representative JSON and fake-server tests cover commit detail, rename/delete, comparison, pagination, and truncation (14 tests; see Evidence).
|
||||
- [x] No write endpoint is exposed — `GiteaHistoryApi` only calls `HttpTransport.get`; confirmed by grep.
|
||||
- [ ] Evidence records the reviewed contract-freeze commit for TASK-007 — cannot be closed by this task alone; requires Codex's read-only review of this commit first, per the same freeze convention used for TASK-005→006 and requested for TASK-010→008.
|
||||
|
||||
## Required checks
|
||||
|
||||
@ -34,3 +34,35 @@ Provide frozen read-only domain and data contracts for commit history, changed f
|
||||
./gradlew testDebugUnitTest
|
||||
./gradlew lintDebug
|
||||
```
|
||||
|
||||
## Evidence
|
||||
|
||||
Implemented independently on branch `claude/task-009-history-diff-contract` from commit `7d122d4`, at the human product owner's explicit direction to proceed without waiting for Codex's separate, still-uncommitted TASK-009 attempt in the primary worktree. That uncommitted work was not read or referenced; this implementation was designed from this task file's acceptance criteria and Gitea's own published Go source structs, independently.
|
||||
|
||||
- New files (writable scope only): `domain/history/HistoryModels.kt`, `domain/history/HistoryRepository.kt`, `data/history/GiteaHistoryApi.kt`, `data/history/GiteaHistoryApiTest.kt`.
|
||||
- Design grounded in Gitea's actual API shapes, checked against `go-gitea/gitea`'s `modules/structs/{repo_commit,issue,pull,pull_review,issue_comment}.go` rather than guessed: a commit's `files` array carries `filename`/`status` (`CommitAffectedFiles`); a `pull_request` field on an issue DTO marks it as actually a PR; `Compare{ total_commits, commits: []Commit }` has no separate paging mechanism.
|
||||
- `HistoryRepository` deliberately does not re-expose commit-history listing, since `GiteaRepositoryBrowser.listCommits` (TASK-004) already provides it as a proper paged endpoint; this contract only adds commit detail (with files), bounded patch text, and ref comparison.
|
||||
- Bounded patch: `GiteaHistoryApi(maxPatchBytes = 200_000, maxPatchLines = 4_000, ...)` — both constructor-injected and validated positive. Byte bound applied first (via UTF-8 byte truncation), then line bound, matching the truncation-ordering pattern already used by TASK-006A's classifier.
|
||||
- Raw diff/patch text is fetched from Gitea's `.diff`-suffixed endpoints (`/repos/{owner}/{repo}/git/commits/{sha}.diff` for a commit, `/repos/{owner}/{repo}/compare/{base}...{head}.diff` for a comparison) rather than expecting full diff content inside the JSON commit/compare objects, since Gitea's JSON responses only carry per-file status, not line-level diff text (a documented Gitea API limitation, not an assumption).
|
||||
- `compare`'s pagination is computed locally against the single JSON response Gitea returns (no server-side page/limit parameters exist for this endpoint), using the same `Long`-safe from/to-index slicing pattern as TASK-004's `listTree`.
|
||||
|
||||
Test coverage (14 cases in `GiteaHistoryApiTest`): commit detail maps every recognized file status plus an unknown-status fallback (1), renamed file with no `previous_filename` maps to `null` instead of crashing (1), patch under both limits returned whole (1), patch over the byte limit truncated (1), patch over the line limit truncated (1), compare correctly encodes a ref containing `/` into one basehead segment (1), compare paginates one response across two pages (1), compare-patch requests the correct diff-suffixed URL (1), missing token fails closed (1), 404 and 409 status mapping (1), malformed commit response fails closed (1), two constructor-validation tests for non-positive limits (2).
|
||||
|
||||
Checks run from the new worktree (`local.properties` created locally, not committed):
|
||||
|
||||
```text
|
||||
./gradlew testDebugUnitTest --tests "com.ubisam.ubgitea.data.history.GiteaHistoryApiTest" → BUILD SUCCESSFUL, 14/14 passed
|
||||
./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks → BUILD SUCCESSFUL
|
||||
- 141 unit tests across 25 suites, 0 failures/errors
|
||||
- 0 lint errors
|
||||
git diff --check → clean
|
||||
sensitive-pattern scan of changed files → no matches
|
||||
```
|
||||
|
||||
Known limitations:
|
||||
- Gitea's exact per-file status vocabulary beyond `added`/`modified`/`deleted`/`renamed` (e.g. whether `changed`/`removed` synonyms actually appear on this Gitea version) was not exhaustively confirmed; both recognized synonyms and a generic `OTHER` fallback are handled defensively either way.
|
||||
- The `.diff`-suffixed compare endpoint's exact path was inferred from Gitea's documented single-commit `.diff`/`.patch` convention rather than independently confirmed for the compare route specifically; this should be verified against a live Gitea instance or its own compare-diff documentation during review.
|
||||
- No physical device or live Gitea instance was used; all coverage is fake-transport/unit-level, consistent with TASK-004/010's own pre-freeze verification.
|
||||
- Codex was independently attempting this same task in the primary worktree (uncommitted at the time this was written); that work was not inspected. Whoever reviews this should reconcile the two rather than assume this one is the only attempt.
|
||||
|
||||
No open P0-P3 findings from self-review against the acceptance criteria. Requesting Codex read-only review and contract-freeze recording so TASK-007 can begin.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user