Implement TASK-010: pull-request and issue read-only API contracts
Frozen domain/data contracts for issues, pull requests, reviews, and comments so TASK-008's UI can be built without touching shared layers. - Separate immutable IssueSummary/Detail and PullRequestSummary/Detail models; WorkItemAuthor.Unavailable instead of inventing an identity for deleted/null authors. - PullRequestState derived from Gitea's merged flag + open/closed state (no single field already encodes "merged"). - listIssues drops entries with a non-null pull_request field so Gitea's issue-list endpoint (which also returns PRs) never duplicates listPullRequests, done client-side rather than trusting an unverified query parameter. - Reuses existing Page<T> and ApiFailure rather than new parallel types. Read-only: no put/delete/postForm call anywhere in the file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
525a0aa2ff
commit
7d122d4388
@ -0,0 +1,307 @@
|
||||
package com.ubisam.ubgitea.data.workitems
|
||||
|
||||
import com.google.gson.Gson
|
||||
import com.google.gson.JsonParseException
|
||||
import com.google.gson.annotations.SerializedName
|
||||
import com.google.gson.reflect.TypeToken
|
||||
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.model.Page
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.workitems.CommentSummary
|
||||
import com.ubisam.ubgitea.domain.workitems.IssueDetail
|
||||
import com.ubisam.ubgitea.domain.workitems.IssueState
|
||||
import com.ubisam.ubgitea.domain.workitems.IssueSummary
|
||||
import com.ubisam.ubgitea.domain.workitems.PullRequestDetail
|
||||
import com.ubisam.ubgitea.domain.workitems.PullRequestState
|
||||
import com.ubisam.ubgitea.domain.workitems.PullRequestSummary
|
||||
import com.ubisam.ubgitea.domain.workitems.ReviewState
|
||||
import com.ubisam.ubgitea.domain.workitems.ReviewSummary
|
||||
import com.ubisam.ubgitea.domain.workitems.WorkItemAuthor
|
||||
import com.ubisam.ubgitea.domain.workitems.WorkItemsRepository
|
||||
import java.io.IOException
|
||||
import java.time.Instant
|
||||
import java.time.format.DateTimeParseException
|
||||
import okhttp3.HttpUrl
|
||||
import okhttp3.HttpUrl.Companion.toHttpUrl
|
||||
|
||||
/**
|
||||
* Read-only pull-request and issue data access. Gitea's issue-list endpoint can also return
|
||||
* pull requests (a PR is internally an issue with a non-null `pull_request` field); entries with
|
||||
* that field set are dropped here so [listIssues] never exposes a duplicate of a PR. Because that
|
||||
* filtering happens client-side after fetching a page, a filtered page can contain fewer than
|
||||
* [pageSize] genuine issues even when more exist — an accepted, documented tradeoff rather than a
|
||||
* guessed, unverified server-side "issues only" query parameter.
|
||||
*/
|
||||
class GiteaWorkItemsApi(
|
||||
private val config: OAuthClientConfig,
|
||||
private val transport: HttpTransport,
|
||||
private val tokenProvider: BearerTokenProvider,
|
||||
private val gson: Gson = Gson(),
|
||||
) : WorkItemsRepository {
|
||||
override suspend fun listIssues(repository: RepositoryCoordinates, page: Int, pageSize: Int): Page<IssueSummary> {
|
||||
validatePage(page, pageSize)
|
||||
val response = get(repoUrl(repository, "issues").withPage(page, pageSize))
|
||||
val items = parseList<IssueDto>(response.body)
|
||||
.filter { it.pullRequest == null }
|
||||
.map(::mapIssue)
|
||||
return response.toPage(items, page, pageSize)
|
||||
}
|
||||
|
||||
override suspend fun getIssue(repository: RepositoryCoordinates, number: Long): IssueDetail {
|
||||
val dto = parseObject<IssueDto>(get(repoUrl(repository, "issues", number.toString())).body)
|
||||
return mapIssueDetail(dto)
|
||||
}
|
||||
|
||||
override suspend fun listIssueComments(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommentSummary> = listComments(repository, number, page, pageSize)
|
||||
|
||||
override suspend fun listPullRequests(
|
||||
repository: RepositoryCoordinates,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<PullRequestSummary> {
|
||||
validatePage(page, pageSize)
|
||||
val response = get(repoUrl(repository, "pulls").withPage(page, pageSize))
|
||||
val items = parseList<PullRequestDto>(response.body).map(::mapPullRequest)
|
||||
return response.toPage(items, page, pageSize)
|
||||
}
|
||||
|
||||
override suspend fun getPullRequest(repository: RepositoryCoordinates, number: Long): PullRequestDetail {
|
||||
val dto = parseObject<PullRequestDto>(get(repoUrl(repository, "pulls", number.toString())).body)
|
||||
return mapPullRequestDetail(dto)
|
||||
}
|
||||
|
||||
override suspend fun listPullRequestReviews(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<ReviewSummary> {
|
||||
validatePage(page, pageSize)
|
||||
val response = get(repoUrl(repository, "pulls", number.toString(), "reviews").withPage(page, pageSize))
|
||||
val items = parseList<ReviewDto>(response.body).map(::mapReview)
|
||||
return response.toPage(items, page, pageSize)
|
||||
}
|
||||
|
||||
override suspend fun listPullRequestComments(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommentSummary> = listComments(repository, number, page, pageSize)
|
||||
|
||||
// Gitea pull requests are issues internally, so PR-level discussion comments live under the
|
||||
// same /issues/{number}/comments endpoint as plain issue comments.
|
||||
private suspend fun listComments(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommentSummary> {
|
||||
validatePage(page, pageSize)
|
||||
val response = get(repoUrl(repository, "issues", number.toString(), "comments").withPage(page, pageSize))
|
||||
val items = parseList<CommentDto>(response.body).map(::mapComment)
|
||||
return response.toPage(items, page, pageSize)
|
||||
}
|
||||
|
||||
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 fun mapAuthor(dto: UserDto?): WorkItemAuthor {
|
||||
val login = dto?.login?.takeIf(String::isNotBlank) ?: return WorkItemAuthor.Unavailable
|
||||
return WorkItemAuthor.Known(login, dto.fullName?.takeIf(String::isNotBlank))
|
||||
}
|
||||
|
||||
private fun mapIssue(dto: IssueDto): IssueSummary = IssueSummary(
|
||||
number = dto.number ?: throw ApiFailure.InvalidResponse(),
|
||||
title = dto.title.required(),
|
||||
state = dto.state.toIssueState(),
|
||||
author = mapAuthor(dto.user),
|
||||
updatedAtEpochMillis = dto.updatedAt.required().toEpochMillisOrInvalid(),
|
||||
commentCount = dto.comments ?: 0,
|
||||
)
|
||||
|
||||
private fun mapIssueDetail(dto: IssueDto): IssueDetail = IssueDetail(
|
||||
number = dto.number ?: throw ApiFailure.InvalidResponse(),
|
||||
title = dto.title.required(),
|
||||
state = dto.state.toIssueState(),
|
||||
author = mapAuthor(dto.user),
|
||||
body = dto.body.orEmpty(),
|
||||
labels = dto.labels.orEmpty().mapNotNull { it.name },
|
||||
assignees = dto.assignees.orEmpty().mapNotNull { it.login },
|
||||
createdAtEpochMillis = dto.createdAt.required().toEpochMillisOrInvalid(),
|
||||
updatedAtEpochMillis = dto.updatedAt.required().toEpochMillisOrInvalid(),
|
||||
)
|
||||
|
||||
private fun mapPullRequest(dto: PullRequestDto): PullRequestSummary = PullRequestSummary(
|
||||
number = dto.number ?: throw ApiFailure.InvalidResponse(),
|
||||
title = dto.title.required(),
|
||||
state = dto.toPullRequestState(),
|
||||
author = mapAuthor(dto.user),
|
||||
updatedAtEpochMillis = dto.updatedAt.required().toEpochMillisOrInvalid(),
|
||||
commentCount = dto.comments ?: 0,
|
||||
)
|
||||
|
||||
private fun mapPullRequestDetail(dto: PullRequestDto): PullRequestDetail = PullRequestDetail(
|
||||
number = dto.number ?: throw ApiFailure.InvalidResponse(),
|
||||
title = dto.title.required(),
|
||||
state = dto.toPullRequestState(),
|
||||
author = mapAuthor(dto.user),
|
||||
body = dto.body.orEmpty(),
|
||||
baseRef = dto.base?.ref.required(),
|
||||
headRef = dto.head?.ref.required(),
|
||||
isMergeable = dto.mergeable ?: false,
|
||||
createdAtEpochMillis = dto.createdAt.required().toEpochMillisOrInvalid(),
|
||||
updatedAtEpochMillis = dto.updatedAt.required().toEpochMillisOrInvalid(),
|
||||
)
|
||||
|
||||
private fun mapReview(dto: ReviewDto): ReviewSummary = ReviewSummary(
|
||||
id = dto.id ?: throw ApiFailure.InvalidResponse(),
|
||||
author = mapAuthor(dto.user),
|
||||
state = dto.state?.toReviewState() ?: ReviewState.OTHER,
|
||||
body = dto.body.orEmpty(),
|
||||
submittedAtEpochMillis = dto.submittedAt?.let { it.toEpochMillisOrInvalid() },
|
||||
)
|
||||
|
||||
private fun mapComment(dto: CommentDto): CommentSummary = CommentSummary(
|
||||
id = dto.id ?: throw ApiFailure.InvalidResponse(),
|
||||
author = mapAuthor(dto.user),
|
||||
body = dto.body.orEmpty(),
|
||||
createdAtEpochMillis = dto.createdAt.required().toEpochMillisOrInvalid(),
|
||||
updatedAtEpochMillis = dto.updatedAt.required().toEpochMillisOrInvalid(),
|
||||
)
|
||||
|
||||
private fun String?.toIssueState(): IssueState = when (this) {
|
||||
"open" -> IssueState.OPEN
|
||||
"closed" -> IssueState.CLOSED
|
||||
else -> throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private fun PullRequestDto.toPullRequestState(): PullRequestState = when {
|
||||
merged == true -> PullRequestState.MERGED
|
||||
state == "closed" -> PullRequestState.CLOSED
|
||||
state == "open" -> PullRequestState.OPEN
|
||||
else -> throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private fun String.toReviewState(): ReviewState = when (uppercase()) {
|
||||
"PENDING" -> ReviewState.PENDING
|
||||
"APPROVED" -> ReviewState.APPROVED
|
||||
"REQUEST_CHANGES" -> ReviewState.REQUEST_CHANGES
|
||||
"COMMENT" -> ReviewState.COMMENT
|
||||
"REQUEST_REVIEW" -> ReviewState.REQUEST_REVIEW
|
||||
else -> ReviewState.OTHER
|
||||
}
|
||||
|
||||
private fun String.toEpochMillisOrInvalid(): Long = try {
|
||||
Instant.parse(this).toEpochMilli()
|
||||
} catch (_: DateTimeParseException) {
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private inline fun <reified T> parseList(body: String): List<T> = try {
|
||||
val type = object : TypeToken<List<T>>() {}.type
|
||||
gson.fromJson<List<T>>(body, type) ?: throw ApiFailure.InvalidResponse()
|
||||
} catch (_: JsonParseException) {
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
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)
|
||||
|
||||
private fun HttpUrl.withPage(page: Int, pageSize: Int): HttpUrl = newBuilder()
|
||||
.addQueryParameter("page", page.toString())
|
||||
.addQueryParameter("limit", pageSize.toString())
|
||||
.build()
|
||||
|
||||
private fun <T> HttpResponse.toPage(items: List<T>, page: Int, pageSize: Int): Page<T> {
|
||||
val hasMore = headers.entries.firstOrNull { it.key.equals("X-HasMore", true) }
|
||||
?.value?.equals("true", true)
|
||||
val needsNext = hasMore == true || hasMore == null && items.size == pageSize
|
||||
return Page.of(items, if (needsNext) page.safeNextPage() else null)
|
||||
}
|
||||
|
||||
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 UserDto(val login: String?, @SerializedName("full_name") val fullName: String?)
|
||||
private data class LabelDto(val name: String?)
|
||||
private data class PullRequestMetaDto(val merged: Boolean?)
|
||||
private data class BranchInfoDto(val ref: String?)
|
||||
private data class IssueDto(
|
||||
val number: Long?,
|
||||
val title: String?,
|
||||
val state: String?,
|
||||
val user: UserDto?,
|
||||
val body: String?,
|
||||
val labels: List<LabelDto>?,
|
||||
val assignees: List<UserDto>?,
|
||||
val comments: Int?,
|
||||
@SerializedName("created_at") val createdAt: String?,
|
||||
@SerializedName("updated_at") val updatedAt: String?,
|
||||
@SerializedName("pull_request") val pullRequest: PullRequestMetaDto?,
|
||||
)
|
||||
private data class PullRequestDto(
|
||||
val number: Long?,
|
||||
val title: String?,
|
||||
val state: String?,
|
||||
val merged: Boolean?,
|
||||
val mergeable: Boolean?,
|
||||
val user: UserDto?,
|
||||
val body: String?,
|
||||
val comments: Int?,
|
||||
val base: BranchInfoDto?,
|
||||
val head: BranchInfoDto?,
|
||||
@SerializedName("created_at") val createdAt: String?,
|
||||
@SerializedName("updated_at") val updatedAt: String?,
|
||||
)
|
||||
private data class ReviewDto(
|
||||
val id: Long?,
|
||||
val user: UserDto?,
|
||||
val state: String?,
|
||||
val body: String?,
|
||||
@SerializedName("submitted_at") val submittedAt: String?,
|
||||
)
|
||||
private data class CommentDto(
|
||||
val id: Long?,
|
||||
val user: UserDto?,
|
||||
val body: String?,
|
||||
@SerializedName("created_at") val createdAt: String?,
|
||||
@SerializedName("updated_at") val updatedAt: String?,
|
||||
)
|
||||
}
|
||||
@ -0,0 +1,76 @@
|
||||
package com.ubisam.ubgitea.domain.workitems
|
||||
|
||||
sealed interface WorkItemAuthor {
|
||||
data class Known(val login: String, val displayName: String?) : WorkItemAuthor
|
||||
data object Unavailable : WorkItemAuthor
|
||||
}
|
||||
|
||||
enum class IssueState { OPEN, CLOSED }
|
||||
|
||||
enum class PullRequestState { OPEN, CLOSED, MERGED }
|
||||
|
||||
/**
|
||||
* REQUEST_REVIEW is a review-assignment request, not a submitted review outcome. OTHER covers
|
||||
* any future state value this client does not yet recognize; reviews are decorative status, so
|
||||
* an unrecognized state falls back to OTHER instead of failing the whole review list closed.
|
||||
*/
|
||||
enum class ReviewState { PENDING, APPROVED, REQUEST_CHANGES, COMMENT, REQUEST_REVIEW, OTHER }
|
||||
|
||||
data class IssueSummary(
|
||||
val number: Long,
|
||||
val title: String,
|
||||
val state: IssueState,
|
||||
val author: WorkItemAuthor,
|
||||
val updatedAtEpochMillis: Long,
|
||||
val commentCount: Int,
|
||||
)
|
||||
|
||||
data class IssueDetail(
|
||||
val number: Long,
|
||||
val title: String,
|
||||
val state: IssueState,
|
||||
val author: WorkItemAuthor,
|
||||
val body: String,
|
||||
val labels: List<String>,
|
||||
val assignees: List<String>,
|
||||
val createdAtEpochMillis: Long,
|
||||
val updatedAtEpochMillis: Long,
|
||||
)
|
||||
|
||||
data class PullRequestSummary(
|
||||
val number: Long,
|
||||
val title: String,
|
||||
val state: PullRequestState,
|
||||
val author: WorkItemAuthor,
|
||||
val updatedAtEpochMillis: Long,
|
||||
val commentCount: Int,
|
||||
)
|
||||
|
||||
data class PullRequestDetail(
|
||||
val number: Long,
|
||||
val title: String,
|
||||
val state: PullRequestState,
|
||||
val author: WorkItemAuthor,
|
||||
val body: String,
|
||||
val baseRef: String,
|
||||
val headRef: String,
|
||||
val isMergeable: Boolean,
|
||||
val createdAtEpochMillis: Long,
|
||||
val updatedAtEpochMillis: Long,
|
||||
)
|
||||
|
||||
data class ReviewSummary(
|
||||
val id: Long,
|
||||
val author: WorkItemAuthor,
|
||||
val state: ReviewState,
|
||||
val body: String,
|
||||
val submittedAtEpochMillis: Long?,
|
||||
)
|
||||
|
||||
data class CommentSummary(
|
||||
val id: Long,
|
||||
val author: WorkItemAuthor,
|
||||
val body: String,
|
||||
val createdAtEpochMillis: Long,
|
||||
val updatedAtEpochMillis: Long,
|
||||
)
|
||||
@ -0,0 +1,40 @@
|
||||
package com.ubisam.ubgitea.domain.workitems
|
||||
|
||||
import com.ubisam.ubgitea.domain.model.Page
|
||||
import com.ubisam.ubgitea.domain.model.RepositoryCoordinates
|
||||
|
||||
/**
|
||||
* Read-only pull-request and issue access. No create/edit/merge/close/comment/notification-write
|
||||
* method exists on this interface, and none may be added without a separate acceptance-criteria
|
||||
* task and human product-owner approval (see AGENTS.md and TASK-010).
|
||||
*/
|
||||
interface WorkItemsRepository {
|
||||
suspend fun listIssues(repository: RepositoryCoordinates, page: Int, pageSize: Int): Page<IssueSummary>
|
||||
|
||||
suspend fun getIssue(repository: RepositoryCoordinates, number: Long): IssueDetail
|
||||
|
||||
suspend fun listIssueComments(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommentSummary>
|
||||
|
||||
suspend fun listPullRequests(repository: RepositoryCoordinates, page: Int, pageSize: Int): Page<PullRequestSummary>
|
||||
|
||||
suspend fun getPullRequest(repository: RepositoryCoordinates, number: Long): PullRequestDetail
|
||||
|
||||
suspend fun listPullRequestReviews(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<ReviewSummary>
|
||||
|
||||
suspend fun listPullRequestComments(
|
||||
repository: RepositoryCoordinates,
|
||||
number: Long,
|
||||
page: Int,
|
||||
pageSize: Int,
|
||||
): Page<CommentSummary>
|
||||
}
|
||||
@ -0,0 +1,217 @@
|
||||
package com.ubisam.ubgitea.data.workitems
|
||||
|
||||
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.model.RepositoryCoordinates
|
||||
import com.ubisam.ubgitea.domain.workitems.IssueState
|
||||
import com.ubisam.ubgitea.domain.workitems.PullRequestState
|
||||
import com.ubisam.ubgitea.domain.workitems.ReviewState
|
||||
import com.ubisam.ubgitea.domain.workitems.WorkItemAuthor
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import okhttp3.HttpUrl.Companion.toHttpUrl
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
|
||||
class GiteaWorkItemsApiTest {
|
||||
private val repository = RepositoryCoordinates.of("team name", "mobile+#")
|
||||
|
||||
@Test
|
||||
fun `issue list excludes pull requests and maps state author and comment count`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""[
|
||||
{"number":1,"title":"Bug","state":"open","user":{"login":"dev","full_name":"Dev Name"},"comments":3,"updated_at":"2026-08-25T10:20:30Z"},
|
||||
{"number":2,"title":"A PR","state":"open","user":{"login":"dev"},"updated_at":"2026-08-25T10:20:30Z","pull_request":{"merged":false}}
|
||||
]""",
|
||||
mapOf("X-HasMore" to "true"),
|
||||
),
|
||||
)
|
||||
|
||||
val page = api(transport).listIssues(repository, 1, 20)
|
||||
|
||||
assertEquals(1, page.items.size)
|
||||
val issue = page.items.single()
|
||||
assertEquals(1L, issue.number)
|
||||
assertEquals(IssueState.OPEN, issue.state)
|
||||
assertEquals(WorkItemAuthor.Known("dev", "Dev Name"), issue.author)
|
||||
assertEquals(3, issue.commentCount)
|
||||
assertEquals(2, page.nextPage)
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(listOf("api", "v1", "repos", "team name", "mobile+#", "issues"), url.pathSegments)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `issue detail maps labels assignees and body`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""{"number":5,"title":"Feature","state":"closed","user":{"login":"dev"},"body":"Please add X",
|
||||
"labels":[{"name":"bug"},{"name":"P1"}],"assignees":[{"login":"a"},{"login":"b"}],
|
||||
"created_at":"2026-08-25T10:20:30Z","updated_at":"2026-08-25T11:00:00Z"}""",
|
||||
),
|
||||
)
|
||||
|
||||
val issue = api(transport).getIssue(repository, 5)
|
||||
|
||||
assertEquals(IssueState.CLOSED, issue.state)
|
||||
assertEquals("Please add X", issue.body)
|
||||
assertEquals(listOf("bug", "P1"), issue.labels)
|
||||
assertEquals(listOf("a", "b"), issue.assignees)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a comment with no user maps to an unavailable author instead of crashing or inventing identity`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""[{"id":9,"user":null,"body":"deleted user's comment","created_at":"2026-08-25T10:20:30Z","updated_at":"2026-08-25T10:20:30Z"}]""",
|
||||
),
|
||||
)
|
||||
|
||||
val comments = api(transport).listIssueComments(repository, 5, 1, 20)
|
||||
|
||||
assertEquals(WorkItemAuthor.Unavailable, comments.items.single().author)
|
||||
assertEquals("deleted user's comment", comments.items.single().body)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pull request state is derived from merged flag and open closed state`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""[
|
||||
{"number":10,"title":"Open PR","state":"open","merged":false,"user":{"login":"dev"},"updated_at":"2026-08-25T10:20:30Z"},
|
||||
{"number":11,"title":"Merged PR","state":"closed","merged":true,"user":{"login":"dev"},"updated_at":"2026-08-25T10:20:30Z"},
|
||||
{"number":12,"title":"Closed PR","state":"closed","merged":false,"user":{"login":"dev"},"updated_at":"2026-08-25T10:20:30Z"}
|
||||
]""",
|
||||
),
|
||||
)
|
||||
|
||||
val page = api(transport).listPullRequests(repository, 1, 20)
|
||||
|
||||
assertEquals(
|
||||
listOf(PullRequestState.OPEN, PullRequestState.MERGED, PullRequestState.CLOSED),
|
||||
page.items.map { it.state },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pull request detail maps base head and mergeable independently`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""{"number":20,"title":"Feature","state":"open","merged":false,"mergeable":true,"user":{"login":"dev"},
|
||||
"body":"desc","base":{"ref":"main"},"head":{"ref":"feature/a+b"},
|
||||
"created_at":"2026-08-25T10:20:30Z","updated_at":"2026-08-25T11:00:00Z"}""",
|
||||
),
|
||||
)
|
||||
|
||||
val pr = api(transport).getPullRequest(repository, 20)
|
||||
|
||||
assertEquals("main", pr.baseRef)
|
||||
assertEquals("feature/a+b", pr.headRef)
|
||||
assertTrue(pr.isMergeable)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `review state maps known values and falls back to OTHER for anything unrecognized`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""[
|
||||
{"id":1,"user":{"login":"r1"},"state":"APPROVED","body":"lgtm","submitted_at":"2026-08-25T10:20:30Z"},
|
||||
{"id":2,"user":{"login":"r2"},"state":"SOME_FUTURE_STATE","body":"","submitted_at":"2026-08-25T10:20:30Z"}
|
||||
]""",
|
||||
),
|
||||
)
|
||||
|
||||
val reviews = api(transport).listPullRequestReviews(repository, 20, 1, 20)
|
||||
|
||||
assertEquals(ReviewState.APPROVED, reviews.items[0].state)
|
||||
assertEquals(ReviewState.OTHER, reviews.items[1].state)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pull request comments reuse the issue comments endpoint`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response("""[{"id":1,"user":{"login":"dev"},"body":"hi","created_at":"2026-08-25T10:20:30Z","updated_at":"2026-08-25T10:20:30Z"}]"""),
|
||||
)
|
||||
|
||||
api(transport).listPullRequestComments(repository, 20, 1, 20)
|
||||
|
||||
val url = transport.urls.single().toHttpUrl()
|
||||
assertEquals(listOf("api", "v1", "repos", "team name", "mobile+#", "issues", "20", "comments"), url.pathSegments)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `malformed required fields fail closed`() = runTest {
|
||||
val transport = QueueTransport(response("""[{"number":1,"title":null,"state":"open","updated_at":"2026-08-25T10:20:30Z"}]"""))
|
||||
|
||||
try {
|
||||
api(transport).listIssues(repository, 1, 20)
|
||||
fail("Expected InvalidResponse")
|
||||
} catch (_: ApiFailure.InvalidResponse) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `missing token fails closed with authentication expired`() = runTest {
|
||||
val missingToken = GiteaWorkItemsApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
QueueTransport(response("[]")),
|
||||
BearerTokenProvider { "" },
|
||||
)
|
||||
|
||||
try {
|
||||
missingToken.listIssues(repository, 1, 20)
|
||||
fail("Expected AuthenticationExpired")
|
||||
} catch (_: ApiFailure.AuthenticationExpired) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `http failures map to typed results`() = runTest {
|
||||
val transport = QueueTransport(HttpResponse(404, "", emptyMap()))
|
||||
try {
|
||||
api(transport).getIssue(repository, 1)
|
||||
fail("Expected NotFound")
|
||||
} catch (_: ApiFailure.NotFound) {
|
||||
// Expected.
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `no page returns a page without a next page cursor when fewer than the page size`() = runTest {
|
||||
val transport = QueueTransport(response("""[{"number":1,"title":"a","state":"open","user":{"login":"d"},"updated_at":"2026-08-25T10:20:30Z"}]"""))
|
||||
|
||||
val page = api(transport).listIssues(repository, 1, 20)
|
||||
|
||||
assertNull(page.nextPage)
|
||||
}
|
||||
|
||||
private fun api(transport: HttpTransport) = GiteaWorkItemsApi(
|
||||
UBGiteaOAuthDefaults.config,
|
||||
transport,
|
||||
BearerTokenProvider { "access" },
|
||||
)
|
||||
|
||||
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")
|
||||
}
|
||||
}
|
||||
@ -190,3 +190,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Known limitations: No physical device was available — the new-scope login path is unverified. Existing signed-in users need one fresh login before Favorites works. Favorite state is never cached locally (always read fresh from Gitea).
|
||||
- Next recommended task: Codex reviews TASK-017 and TASK-018 together (same branch) and verifies the `write:user` login on a physical device before this reaches users; TASK-007 (history UI) remains next in the original roadmap once TASK-009 lands.
|
||||
- Next owner: Codex, for review and device verification.
|
||||
|
||||
## 2026-08-26 — TASK-010: pull-request and issue API contracts
|
||||
|
||||
- Agent and role: Claude, feature implementer (ownership transferred by human product owner in `handoff/claude-master-handoff.md`), continuing work per the human product owner's "진행해줘" (proceed) instruction while TASK-009 remained uncommitted on Codex's side.
|
||||
- Commit/diff: branch `claude/task-010-work-items-contract` from `525a0aa`; new `domain/workitems/` and `data/workitems/` packages.
|
||||
- Features implemented: Frozen, read-only domain and data contracts for pull requests, issues, reviews, and comments — `listIssues`/`getIssue`/`listIssueComments`, `listPullRequests`/`getPullRequest`/`listPullRequestReviews`/`listPullRequestComments`.
|
||||
- How it was implemented: `IssueSummary`/`IssueDetail` and `PullRequestSummary`/`PullRequestDetail` are fully separate models. `WorkItemAuthor` is `Known`/`Unavailable` so a deleted or null author never becomes an invented name. `PullRequestState` is derived from Gitea's `merged` flag combined with its open/closed state (there is no single field that already encodes "merged"). `listIssues` drops entries whose `pull_request` field is non-null so Gitea's issue-list endpoint (which also returns PRs) never duplicates what `listPullRequests` already exposes — done client-side rather than trusting a guessed, unverified query parameter. PR/issue discussion comments both reuse `/issues/{number}/comments`, matching how Gitea treats PRs as issues internally. Reused `domain.model.Page<T>` and `core.network.ApiFailure` rather than introducing parallel types.
|
||||
- Files changed: `domain/workitems/WorkItemModels.kt`, `domain/workitems/WorkItemsRepository.kt`, `data/workitems/GiteaWorkItemsApi.kt`, `data/workitems/GiteaWorkItemsApiTest.kt`, `tasks/010-work-items-api-contract.md`.
|
||||
- Checks and results: `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` BUILD SUCCESSFUL — 128 unit tests across 24 suites, 0 failures/errors, 0 lint errors; `git diff --check` clean; sensitive-pattern scan clean; grep-confirmed no `put`/`delete`/`postForm` call anywhere in the new API class (read-only, as required).
|
||||
- Review findings and resolutions: None — self-review found no P0-P3. Independent Codex review and contract-freeze recording are still required before TASK-008 (the UI that consumes this contract) may begin.
|
||||
- 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.
|
||||
|
||||
@ -1,8 +1,8 @@
|
||||
# TASK-010: Implement pull-request and issue 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 (ownership transferred by human product owner)
|
||||
Reviewer: Codex
|
||||
|
||||
## Outcome
|
||||
|
||||
@ -18,14 +18,14 @@ Provide frozen read-only domain and data contracts for pull requests, issues, re
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [ ] Pull requests and issues use separate immutable domain models.
|
||||
- [ ] Lists, reviews, and comments have explicit immutable pagination contracts.
|
||||
- [ ] Nullable or deleted authors map without crashes or invented identity.
|
||||
- [ ] 401, 403, 404, 422, rate-limit, network, parse, and server failures map to typed results.
|
||||
- [ ] Endpoint DTOs remain outside domain and UI packages.
|
||||
- [ ] Representative JSON and fake-server tests cover lists, reviews, comments, deleted users, pagination, and failures.
|
||||
- [ ] No create, edit, merge, close, comment, or notification-write endpoint is exposed.
|
||||
- [ ] Evidence records the reviewed contract-freeze commit for TASK-008.
|
||||
- [x] Pull requests and issues use separate immutable domain models (`IssueSummary`/`IssueDetail` vs `PullRequestSummary`/`PullRequestDetail`; no shared base type, no shared mutable state).
|
||||
- [x] Lists, reviews, and comments have explicit immutable pagination contracts — reuses the existing `domain.model.Page<T>` (unmodifiable list, explicit `nextPage`) already used by every other paged endpoint in the app, rather than a new pagination type.
|
||||
- [x] Nullable or deleted authors map without crashes or invented identity — `WorkItemAuthor` is `Known(login, displayName)` or `Unavailable`; a null/blank `user` field maps to `Unavailable`, never a fabricated name.
|
||||
- [x] 401, 403, 404, 422, rate-limit, network, parse, and server failures map to typed results — reuses the existing `ApiFailure` sealed class (`AuthenticationExpired`, `PermissionDenied`, `NotFound`, `Unprocessable`, `RateLimited`, `Network`, `InvalidResponse`, `Server`) rather than a parallel failure hierarchy.
|
||||
- [x] Endpoint DTOs remain outside domain and UI packages — all `*Dto` types are `private` nested classes of `data.workitems.GiteaWorkItemsApi`; nothing in `domain.workitems` references Gson or JSON.
|
||||
- [x] Representative JSON and fake-server tests cover lists, reviews, comments, deleted users, pagination, and failures (see Evidence).
|
||||
- [x] No create, edit, merge, close, comment, or notification-write endpoint is exposed — `GiteaWorkItemsApi` only calls `HttpTransport.get`; confirmed by grep (no `put`/`delete`/`postForm` call anywhere in the file).
|
||||
- [ ] Evidence records the reviewed contract-freeze commit for TASK-008 — 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.
|
||||
|
||||
## Required checks
|
||||
|
||||
@ -34,3 +34,32 @@ Provide frozen read-only domain and data contracts for pull requests, issues, re
|
||||
./gradlew testDebugUnitTest
|
||||
./gradlew lintDebug
|
||||
```
|
||||
|
||||
## Evidence
|
||||
|
||||
Implemented on branch `claude/task-010-work-items-contract` from commit `525a0aa` (latest reviewed Claude work at the time; base contracts unchanged from `0929282`).
|
||||
|
||||
- New files (writable scope only): `domain/workitems/WorkItemModels.kt`, `domain/workitems/WorkItemsRepository.kt`, `data/workitems/GiteaWorkItemsApi.kt`, `data/workitems/GiteaWorkItemsApiTest.kt`.
|
||||
- Design: `IssueSummary`/`IssueDetail` and `PullRequestSummary`/`PullRequestDetail` are separate data classes. `PullRequestState` is derived from Gitea's `merged` boolean plus the issue-level `open`/`closed` state (`merged=true` → `MERGED`; else `state` maps directly), matching Gitea's actual struct shape (there is no single `state` enum value that already means "merged"). `ReviewState` maps five known Gitea values case-insensitively and falls back to `OTHER` for anything unrecognized instead of failing the whole review list closed — reviews are decorative status, not identity- or safety-critical, so this is intentionally looser than the fail-closed convention used elsewhere (documented in a code comment).
|
||||
- Gitea quirk handled: `GET /repos/{owner}/{repo}/issues` can also return pull requests (a PR is internally an issue with a non-null `pull_request` field). `listIssues` drops any entry with that field set, confirmed by a test with a mixed issue+PR response. This is done client-side rather than via a guessed/unverified `type=` query parameter, so correctness does not depend on an assumption about Gitea's exact filter semantics; the accepted tradeoff (documented in a class-level comment) is that a filtered page can return fewer than `pageSize` genuine issues.
|
||||
- PR/issue discussion comments both use `/issues/{number}/comments` (Gitea treats PRs as issues internally for this endpoint), confirmed by a test asserting the exact URL path segments from `listPullRequestComments`.
|
||||
- Reused rather than duplicated: `domain.model.Page<T>` for pagination, `core.network.ApiFailure`/`requireSuccess()` for typed failures, `data.repositories.BearerTokenProvider` for the token-supply contract — no parallel types were introduced for things the app already has.
|
||||
|
||||
Test coverage (12 cases in `GiteaWorkItemsApiTest`): issue list excludes PRs and maps state/author/comments (1), issue detail maps labels/assignees/body (1), deleted-author comment maps to `Unavailable` without crashing (1), PR state derivation across open/merged/closed (1), PR detail maps base/head/mergeable independently (1), review state known-values-plus-`OTHER`-fallback (1), PR comments reuse the issue-comments URL (1), malformed required field fails closed (1), missing token fails closed as `AuthenticationExpired` (1), HTTP status mapping (1), no-next-page-when-under-page-size (1), plus URL/component-encoding assertions folded into the list/comments tests using an owner/repo containing a space and `+`/`#`.
|
||||
|
||||
Checks run from the new worktree (`local.properties` created locally, not committed):
|
||||
|
||||
```text
|
||||
./gradlew testDebugUnitTest --tests "com.ubisam.ubgitea.data.workitems.GiteaWorkItemsApiTest" → BUILD SUCCESSFUL, 12/12 passed
|
||||
./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks → BUILD SUCCESSFUL
|
||||
- 128 unit tests across 24 suites, 0 failures/errors
|
||||
- 0 lint errors
|
||||
git diff --check → clean
|
||||
sensitive-pattern scan of changed files → no matches
|
||||
```
|
||||
|
||||
Known limitations:
|
||||
- Gitea's exact query-parameter behavior for filtering pull requests out of the issue-list endpoint was not confirmed against live documentation (uncertain across versions); the client-side `pull_request`-field filter used instead is correct regardless of that behavior, at the cost of potentially-short pages after filtering.
|
||||
- No physical device or live Gitea instance was used; all coverage is fake-transport/unit-level, consistent with how TASK-004/009 were verified before their own device/contract-freeze review.
|
||||
|
||||
No open P0-P3 findings from self-review against the acceptance criteria. Requesting Codex read-only review and contract-freeze recording so TASK-008 can begin.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user