From 453507de6d57cf57b03d16dbb1b955e99f56edc1 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Wed, 23 Sep 2026 17:20:31 +0200 Subject: [PATCH 1/9] feat(ingestion): server-side pagination and faceted search for artifacts --- .gitignore | 4 + .../controller/ArtifactController.kt | 85 +++- .../model/dto/ArtifactFilterCriteria.kt | 28 ++ .../dto/response/ArtifactFacetsResponse.kt | 19 + .../ingestion/model/entity/Artifact.kt | 6 + .../repository/ArtifactFacetRepository.kt | 42 ++ .../repository/ArtifactFacetRepositoryImpl.kt | 424 ++++++++++++++++++ .../repository/ArtifactRepository.kt | 16 +- .../ingestion/service/ArtifactQueryService.kt | 87 +++- .../V20__add_artifact_project_index.sql | 13 + .../controller/ArtifactControllerTest.kt | 127 ++++++ .../service/ArtifactFacetServiceTest.kt | 100 +++++ .../service/ArtifactQueryServiceTest.kt | 94 ++++ 13 files changed, 1027 insertions(+), 18 deletions(-) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt create mode 100644 src/main/resources/db/migration/V20__add_artifact_project_index.sql create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt diff --git a/.gitignore b/.gitignore index b6b09312e..3f0883ac6 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,7 @@ CLAUDE.md # Tooling residue: Python helper scripts used while editing this repo. __pycache__/ *.pyc + +# Hermes agent working files (per-developer) +.worktrees/ +.worktreeinclude diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index e5aaba7be..fca6f630e 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -1,8 +1,14 @@ package com.sprintstart.sprintstartbackend.ingestion.controller +import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentRedirectResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactPageResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse +import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactQueryService import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactService import io.swagger.v3.oas.annotations.Operation @@ -89,12 +95,13 @@ class ArtifactController( summary = "Get project artifacts", description = "Returns a paginated artifact list limited to one project visible to the " + - "authenticated user. When a filter is provided, the search is performed " + + "authenticated user. When a filter or criteria are provided, the search is performed " + "case-insensitively across the configured searchable fields.", ) @ApiResponses( value = [ ApiResponse(responseCode = "200", description = "Project artifact page returned successfully"), + ApiResponse(responseCode = "400", description = "Invalid query or pagination parameters"), ApiResponse(responseCode = "403", description = "Caller has no access to the project"), ], ) @@ -102,13 +109,85 @@ class ArtifactController( @RequestParam(defaultValue = DEFAULT_PAGE) @Min(1) page: Int, @RequestParam(defaultValue = DEFAULT_SIZE) @Min(1) @Max(MAX_PAGE_SIZE) size: Int, @RequestParam(defaultValue = "") filter: String, + @RequestParam(required = false) search: String?, + @RequestParam(required = false) types: Set?, + @RequestParam(required = false) sources: Set?, + @RequestParam(required = false) repositories: Set?, + @RequestParam(required = false) format: UploadFormat?, @Parameter( description = "UUID of the project whose artifacts should be returned", ) @PathVariable projectId: UUID, @Parameter(hidden = true) @AuthenticationPrincipal jwt: Jwt, - ): ResponseEntity = + ): ResponseEntity { + val effectiveSearch = search ?: (if (filter.isNotBlank()) filter else null) + val criteria = ArtifactFilterCriteria( + search = effectiveSearch, + types = types, + sources = sources, + repositories = repositories, + format = format, + ) + return ResponseEntity.ok( + artifactQueryService.getProjectArtifacts(page, size, criteria, projectId, jwt.subject), + ) + } + + @GetMapping("projects/{projectId}/artifacts/facets") + @PreAuthorize("hasRole('USER')") + @Operation( + summary = "Get project artifact facets", + description = "Returns aggregated counts for artifact facets scoped to a project.", + ) + @ApiResponses( + value = [ + ApiResponse(responseCode = "200", description = "Facet counts returned successfully"), + ApiResponse(responseCode = "400", description = "Invalid facet query parameters"), + ApiResponse(responseCode = "403", description = "Caller has no access to the project"), + ], + ) + fun getProjectArtifactFacets( + @RequestParam(required = false) search: String?, + @RequestParam(required = false) types: Set?, + @RequestParam(required = false) sources: Set?, + @RequestParam(required = false) repositories: Set?, + @RequestParam(required = false) format: UploadFormat?, + @Parameter( + description = "UUID of the project whose artifact facets should be calculated", + ) @PathVariable projectId: UUID, + @Parameter(hidden = true) @AuthenticationPrincipal jwt: Jwt, + ): ResponseEntity { + val criteria = ArtifactFilterCriteria( + search = search, + types = types, + sources = sources, + repositories = repositories, + format = format, + ) + return ResponseEntity.ok( + artifactQueryService.getProjectArtifactFacets(projectId, criteria, jwt.subject), + ) + } + + @GetMapping("projects/{projectId}/artifacts/{artifactId}") + @PreAuthorize("hasRole('USER')") + @Operation( + summary = "Get single artifact", + description = "Returns metadata for one artifact when the caller has access to the requested project.", + ) + @ApiResponses( + value = [ + ApiResponse(responseCode = "200", description = "Artifact returned successfully"), + ApiResponse(responseCode = "403", description = "Caller has no access to the project"), + ApiResponse(responseCode = "404", description = "Artifact not found in project"), + ], + ) + fun getArtifact( + @Parameter(description = "UUID of the project that scopes artifact access") @PathVariable projectId: UUID, + @Parameter(description = "UUID of the artifact to return") @PathVariable artifactId: UUID, + @Parameter(hidden = true) @AuthenticationPrincipal jwt: Jwt, + ): ResponseEntity = ResponseEntity.ok( - artifactQueryService.getProjectArtifacts(page, size, filter, projectId, jwt.subject), + artifactQueryService.getArtifact(projectId, artifactId, jwt.subject), ) /** diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt new file mode 100644 index 000000000..1d32e86af --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt @@ -0,0 +1,28 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto + +import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType + +/** + * File formats recognized for UPLOAD-sourced artifacts. + */ +enum class UploadFormat { + PDF, + MARKDOWN, + IMAGE, + OTHER, +} + +/** + * Filter criteria for project-scoped artifact searches and facet calculations. + * + * Encapsulates full-text search, type filtering, source filtering, repository selection, + * and upload format selection. + */ +data class ArtifactFilterCriteria( + val search: String? = null, + val types: Set? = null, + val sources: Set? = null, + val repositories: Set? = null, + val format: UploadFormat? = null, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt new file mode 100644 index 000000000..265067ecc --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt @@ -0,0 +1,19 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto.response + +/** + * A single facet option with its corresponding artifact count. + */ +data class FacetCountResponse( + val value: String, + val count: Long, +) + +/** + * Aggregated facet counts for artifact types, sources, upload formats, and repositories. + */ +data class ArtifactFacetsResponse( + val types: List, + val sources: List, + val formats: List, + val repositories: List, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt index 1aef89f67..ee3c5c256 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt @@ -48,6 +48,12 @@ class Artifact( @CollectionTable( name = "artifact_projects", joinColumns = [JoinColumn(name = "artifact_id")], + indexes = [ + jakarta.persistence.Index( + name = "idx_artifact_projects_project", + columnList = "project_id, artifact_id", + ), + ], ) @Column(name = "project_id", nullable = false) // Add companion obj to Artifact to have Artifact.create diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt new file mode 100644 index 000000000..bb799680a --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt @@ -0,0 +1,42 @@ +package com.sprintstart.sprintstartbackend.ingestion.repository + +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse +import org.springframework.data.domain.Page +import org.springframework.data.domain.Pageable +import java.util.UUID + +/** + * Custom repository fragment providing multi-criteria paginated projection queries + * and aggregated facet calculations for project artifacts. + */ +interface ArtifactFacetRepository { + /** + * Resolves a paginated list of artifact projections using dynamic criteria without hydrating + * the heavyweight `content` column. + * + * @param projectId Scopes artifacts to the target project. + * @param criteria Filter criteria containing search text, types, sources, repos, and formats. + * @param pageable Requested pagination and sorting. + * @return Paginated page of artifact response projections. + */ + fun findProjectArtifactsWithCriteria( + projectId: UUID, + criteria: ArtifactFilterCriteria, + pageable: Pageable, + ): Page + + /** + * Calculates aggregated counts for types, sources, upload formats, and repositories + * using the "count each would add" model (own-facet-excluded, other-facets-applied). + * + * @param projectId Scopes facet calculations to the target project. + * @param criteria The currently active filter criteria. + * @return Aggregated facet counts. + */ + fun findFacets( + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): ArtifactFacetsResponse +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt new file mode 100644 index 000000000..68d39c488 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -0,0 +1,424 @@ +package com.sprintstart.sprintstartbackend.ingestion.repository + +import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse +import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact +import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import jakarta.persistence.EntityManager +import jakarta.persistence.PersistenceContext +import jakarta.persistence.criteria.CriteriaBuilder +import jakarta.persistence.criteria.Join +import jakarta.persistence.criteria.Predicate +import jakarta.persistence.criteria.Root +import org.springframework.data.domain.Page +import org.springframework.data.domain.PageImpl +import org.springframework.data.domain.Pageable +import org.springframework.stereotype.Repository +import org.springframework.transaction.annotation.Transactional +import java.time.Instant +import java.util.UUID + +private enum class FacetKind { + TYPES, + SOURCES, + FORMATS, + REPOSITORIES, +} + +private val SOURCES_EXCLUDED_FACETS = setOf(FacetKind.SOURCES, FacetKind.FORMATS, FacetKind.REPOSITORIES) + +@Repository +@Transactional(readOnly = true) +class ArtifactFacetRepositoryImpl( + @PersistenceContext private val entityManager: EntityManager, +) : ArtifactFacetRepository { + override fun findProjectArtifactsWithCriteria( + projectId: UUID, + criteria: ArtifactFilterCriteria, + pageable: Pageable, + ): Page { + val cb = entityManager.criteriaBuilder + + // 1. Data Query (Projection - D3: does not hydrate content TEXT column) + val query = cb.createQuery(ArtifactResponse::class.java) + val root = query.from(Artifact::class.java) + val projectJoin = root.join("projectIdsInternal") + + query.select( + cb.construct( + ArtifactResponse::class.java, + root.get("id"), + root.get("title"), + root.get("sourceSystem"), + root.get("sourceId"), + root.get("sourceUrl"), + root.get("artifactType"), + root.get("ingestedAt"), + root.get("lastChangedAt"), + root.get("metadata"), + root.get("sourceVersion"), + ), + ) + + val predicates = buildPredicates(cb, root, projectJoin, projectId, criteria, null) + query.where(*predicates.toTypedArray()) + + // D2: Deterministic sort (ingestedAt DESC, id ASC) + query.orderBy( + cb.desc(root.get("ingestedAt")), + cb.asc(root.get("id")), + ) + + val typedQuery = entityManager.createQuery(query) + typedQuery.firstResult = pageable.offset.toInt() + typedQuery.maxResults = pageable.pageSize + val items = typedQuery.resultList + + // 2. Count Query + val countQuery = cb.createQuery(Long::class.java) + val countRoot = countQuery.from(Artifact::class.java) + val countJoin = countRoot.join("projectIdsInternal") + + countQuery.select(cb.countDistinct(countRoot.get("id"))) + val countPredicates = buildPredicates(cb, countRoot, countJoin, projectId, criteria, null) + countQuery.where(*countPredicates.toTypedArray()) + + val totalElements = entityManager.createQuery(countQuery).singleResult ?: 0L + + return PageImpl(items, pageable, totalElements) + } + + override fun findFacets( + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): ArtifactFacetsResponse { + val cb = entityManager.criteriaBuilder + return ArtifactFacetsResponse( + types = computeTypeFacets(cb, projectId, criteria), + sources = computeSourceFacets(cb, projectId, criteria), + formats = computeFormatFacets(cb, projectId, criteria), + repositories = computeRepositoryFacets(cb, projectId, criteria), + ) + } + + private fun computeTypeFacets( + cb: CriteriaBuilder, + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): List { + val typeCountsMap = mutableMapOf() + val query = cb.createQuery(Array::class.java) + val root = query.from(Artifact::class.java) + val join = root.join("projectIdsInternal") + query.multiselect( + root.get("artifactType"), + cb.countDistinct(root.get("id")), + ) + val preds = buildPredicates(cb, root, join, projectId, criteria, FacetKind.TYPES) + query.where(*preds.toTypedArray()) + query.groupBy(root.get("artifactType")) + + for (row in entityManager.createQuery(query).resultList) { + val type = row[0] as ArtifactType + val count = (row[1] as Number).toLong() + typeCountsMap[type] = count + } + criteria.types?.forEach { type -> + typeCountsMap.putIfAbsent(type, 0L) + } + return typeCountsMap.map { (type, count) -> + FacetCountResponse(type.name, count) + } + } + + private fun computeSourceFacets( + cb: CriteriaBuilder, + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): List { + val sourceCountsMap = mutableMapOf() + val query = cb.createQuery(Array::class.java) + val root = query.from(Artifact::class.java) + val join = root.join("projectIdsInternal") + query.multiselect( + root.get("sourceSystem"), + cb.countDistinct(root.get("id")), + ) + val preds = buildPredicates(cb, root, join, projectId, criteria, FacetKind.SOURCES) + query.where(*preds.toTypedArray()) + query.groupBy(root.get("sourceSystem")) + + for (row in entityManager.createQuery(query).resultList) { + val source = row[0] as SourceSystem + val count = (row[1] as Number).toLong() + sourceCountsMap[source] = count + } + criteria.sources?.forEach { source -> + sourceCountsMap.putIfAbsent(source, 0L) + } + return sourceCountsMap.map { (source, count) -> + FacetCountResponse(source.name, count) + } + } + + private fun computeFormatFacets( + cb: CriteriaBuilder, + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): List { + val formatCountsMap = mutableMapOf( + UploadFormat.PDF to 0L, + UploadFormat.MARKDOWN to 0L, + UploadFormat.IMAGE to 0L, + UploadFormat.OTHER to 0L, + ) + val query = cb.createQuery(Array::class.java) + val root = query.from(Artifact::class.java) + val join = root.join("projectIdsInternal") + query.multiselect( + root.get("title"), + root.get("sourceUrl"), + root.get("sourceId"), + root.get("mime"), + root.get("language"), + ) + val preds = buildPredicates(cb, root, join, projectId, criteria, FacetKind.FORMATS).toMutableList() + preds.add(cb.equal(root.get("sourceSystem"), SourceSystem.UPLOAD)) + query.where(*preds.toTypedArray()) + + for (row in entityManager.createQuery(query).resultList) { + val title = row[0] as? String + val sourceUrl = row[1] as? String + val sourceId = row[2] as String + val mime = row[3] as? String + val language = row[4] as? String + val format = classifyUploadFormat(title, sourceUrl, sourceId, mime, language) + formatCountsMap[format] = (formatCountsMap[format] ?: 0L) + 1L + } + + return formatCountsMap + .filter { (fmt, count) -> count > 0L || criteria.format == fmt } + .map { (fmt, count) -> FacetCountResponse(fmt.name, count) } + } + + private fun computeRepositoryFacets( + cb: CriteriaBuilder, + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): List { + val repoCountsMap = mutableMapOf() + val orgCountsMap = mutableMapOf() + val query = cb.createQuery(Array::class.java) + val root = query.from(Artifact::class.java) + val join = root.join("projectIdsInternal") + query.multiselect( + root.get("sourceId"), + root.get("artifactType"), + ) + val preds = buildPredicates(cb, root, join, projectId, criteria, FacetKind.REPOSITORIES).toMutableList() + preds.add(cb.equal(root.get("sourceSystem"), SourceSystem.GITHUB)) + query.where(*preds.toTypedArray()) + + for (row in entityManager.createQuery(query).resultList) { + val sourceId = row[0] as String + val artifactType = row[1] as ArtifactType + + if (artifactType == ArtifactType.ORG_METADATA) { + val orgLogin = sourceId.trim().lowercase() + orgCountsMap[orgLogin] = (orgCountsMap[orgLogin] ?: 0L) + 1L + } else { + val repo = extractRepositoryFromSourceId(sourceId) + if (repo != null) { + repoCountsMap[repo] = (repoCountsMap[repo] ?: 0L) + 1L + } + } + } + for (repo in repoCountsMap.keys.toList()) { + val owner = repo.substringBefore('/').trim().lowercase() + val orgCount = orgCountsMap[owner] ?: 0L + if (orgCount > 0L) { + repoCountsMap[repo] = (repoCountsMap[repo] ?: 0L) + orgCount + } + } + criteria.repositories?.forEach { repo -> + repoCountsMap.putIfAbsent(repo, 0L) + } + return repoCountsMap + .filter { (repo, count) -> count > 0L || criteria.repositories?.contains(repo) == true } + .entries + .sortedBy { it.key } + .map { (repo, count) -> FacetCountResponse(repo, count) } + } + + private fun buildPredicates( + cb: CriteriaBuilder, + root: Root, + projectJoin: Join, + projectId: UUID, + criteria: ArtifactFilterCriteria, + exclude: FacetKind?, + ): List { + val predicates = mutableListOf() + predicates.add(cb.equal(projectJoin, projectId)) + + if (!criteria.search.isNullOrBlank()) { + val pattern = "%${criteria.search.trim().lowercase()}%" + val titleMatch = cb.like(cb.lower(root.get("title")), pattern) + val sourceIdMatch = cb.like(cb.lower(root.get("sourceId")), pattern) + val sourceUrlMatch = cb.like(cb.lower(root.get("sourceUrl")), pattern) + predicates.add(cb.or(titleMatch, sourceIdMatch, sourceUrlMatch)) + } + + if (exclude != FacetKind.TYPES && !criteria.types.isNullOrEmpty()) { + predicates.add(root.get("artifactType").`in`(criteria.types)) + } + + if (exclude !in SOURCES_EXCLUDED_FACETS && !criteria.sources.isNullOrEmpty()) { + predicates.add(root.get("sourceSystem").`in`(criteria.sources)) + } + + if (exclude != FacetKind.FORMATS && criteria.format != null) { + val notUpload = cb.notEqual(root.get("sourceSystem"), SourceSystem.UPLOAD) + val uploadMatchesFormat = buildUploadFormatPredicate(cb, root, criteria.format) + predicates.add(cb.or(notUpload, uploadMatchesFormat)) + } + + if (exclude != FacetKind.REPOSITORIES && !criteria.repositories.isNullOrEmpty()) { + val notGithub = cb.notEqual(root.get("sourceSystem"), SourceSystem.GITHUB) + val githubMatchesRepo = buildGithubRepoPredicate(cb, root, criteria.repositories) + predicates.add(cb.or(notGithub, githubMatchesRepo)) + } + + return predicates + } + + private fun buildUploadFormatPredicate( + cb: CriteriaBuilder, + root: Root, + format: UploadFormat, + ): Predicate { + val titleLower = cb.lower(root.get("title")) + val sourceUrlLower = cb.lower(root.get("sourceUrl")) + val sourceIdLower = cb.lower(root.get("sourceId")) + val mimeLower = cb.lower(root.get("mime")) + val languageLower = cb.lower(root.get("language")) + + val isPdf = cb.or( + cb.equal(mimeLower, "application/pdf"), + cb.like(titleLower, "%.pdf"), + cb.like(sourceUrlLower, "%.pdf"), + cb.like(sourceIdLower, "%.pdf"), + ) + + val isMarkdown = cb.or( + languageLower.`in`("markdown", "md"), + cb.like(mimeLower, "%markdown%"), + cb.like(titleLower, "%.md"), + cb.like(titleLower, "%.markdown"), + cb.like(sourceUrlLower, "%.md"), + cb.like(sourceUrlLower, "%.markdown"), + cb.like(sourceIdLower, "%.md"), + cb.like(sourceIdLower, "%.markdown"), + ) + + val imageExtPredicates = IMAGE_EXTENSIONS.flatMap { ext -> + listOf(cb.like(titleLower, "%$ext"), cb.like(sourceUrlLower, "%$ext")) + } + val isImage = cb.or( + cb.like(mimeLower, "image/%"), + *imageExtPredicates.toTypedArray(), + ) + + return when (format) { + UploadFormat.PDF -> isPdf + UploadFormat.MARKDOWN -> isMarkdown + UploadFormat.IMAGE -> isImage + UploadFormat.OTHER -> cb.not(cb.or(isPdf, isMarkdown, isImage)) + } + } + + private fun buildGithubRepoPredicate( + cb: CriteriaBuilder, + root: Root, + repositories: Set, + ): Predicate { + val sourceId = root.get("sourceId") + val artifactType = root.get("artifactType") + + val repoPrefixPredicates = repositories.map { repo -> + cb.like(sourceId, "github:$repo:%") + } + val isNonOrgRepoMatch = cb.and( + cb.notEqual(artifactType, ArtifactType.ORG_METADATA), + cb.or(*repoPrefixPredicates.toTypedArray()), + ) + + val owners = repositories.map { it.substringBefore('/').trim().lowercase() }.toSet() + val isOrgMatch = cb.and( + cb.equal(artifactType, ArtifactType.ORG_METADATA), + cb.lower(sourceId).`in`(owners), + ) + + return cb.or(isNonOrgRepoMatch, isOrgMatch) + } + + companion object { + private val IMAGE_EXTENSIONS = listOf( + ".png", + ".jpg", + ".jpeg", + ".gif", + ".webp", + ".svg", + ".bmp", + ".avif", + ) + + fun extractRepositoryFromSourceId(sourceId: String): String? { + if (!sourceId.startsWith("github:")) return null + val parts = sourceId.split(':') + return if (parts.size >= 3) parts[1] else null + } + + private fun isPdf(title: String, url: String, id: String, mime: String): Boolean = + mime == "application/pdf" || title.endsWith(".pdf") || url.endsWith(".pdf") || id.endsWith(".pdf") + + private fun isMarkdown(title: String, url: String, id: String, mime: String, lang: String): Boolean = + lang in listOf("markdown", "md") || + mime.contains("markdown") || + title.endsWith(".md") || + title.endsWith(".markdown") || + url.endsWith(".md") || + url.endsWith(".markdown") || + id.endsWith(".md") || + id.endsWith(".markdown") + + private fun isImage(title: String, url: String, mime: String): Boolean = + mime.startsWith("image/") || IMAGE_EXTENSIONS.any { title.endsWith(it) || url.endsWith(it) } + + fun classifyUploadFormat( + title: String?, + sourceUrl: String?, + sourceId: String, + mime: String?, + language: String?, + ): UploadFormat { + val titleLower = title?.lowercase() ?: "" + val sourceUrlLower = sourceUrl?.lowercase() ?: "" + val sourceIdLower = sourceId.lowercase() + val mimeLower = mime?.lowercase() ?: "" + val languageLower = language?.lowercase() ?: "" + + return when { + isPdf(titleLower, sourceUrlLower, sourceIdLower, mimeLower) -> UploadFormat.PDF + isMarkdown(titleLower, sourceUrlLower, sourceIdLower, mimeLower, languageLower) -> UploadFormat.MARKDOWN + isImage(titleLower, sourceUrlLower, mimeLower) -> UploadFormat.IMAGE + else -> UploadFormat.OTHER + } + } + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt index 959c238b9..cf6ab8f10 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt @@ -6,6 +6,7 @@ import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import org.springframework.data.domain.Page import org.springframework.data.domain.Pageable import org.springframework.data.jpa.repository.JpaRepository +import org.springframework.data.jpa.repository.JpaSpecificationExecutor import org.springframework.data.jpa.repository.Query import org.springframework.data.repository.query.Param import java.time.Instant @@ -18,9 +19,22 @@ import java.util.UUID * are asked of artifacts, not a repository doing too many things. */ @Suppress("TooManyFunctions") -interface ArtifactRepository : JpaRepository { +interface ArtifactRepository : + JpaRepository, + JpaSpecificationExecutor, + ArtifactFacetRepository { fun findBySourceId(sourceId: String): Artifact? + @Query( + """ + SELECT a + FROM Artifact a + JOIN a.projectIdsInternal p + WHERE a.id = :artifactId AND p = :projectId + """, + ) + fun findByIdAndProjectId(@Param("artifactId") artifactId: UUID, @Param("projectId") projectId: UUID): Artifact? + /** * Batch variant of [findBySourceId]. Source ids with no artifact are simply absent, so a * caller comparing a set of rows against the corpus learns which of them it no longer holds. diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt index b06602daf..91a595715 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt @@ -1,6 +1,9 @@ package com.sprintstart.sprintstartbackend.ingestion.service +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactPageResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.PageMetadata import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact import com.sprintstart.sprintstartbackend.ingestion.model.mapper.ArtifactMapper @@ -69,14 +72,11 @@ class ArtifactQueryService( } /** - * Returns one paginated artifact list limited to a single project visible to the caller. - * - * The method first validates project access and then delegates to project-scoped repository - * queries using the same filter semantics as the global artifact search. + * Returns one paginated artifact list limited to a single project visible to the caller using criteria. * * @param page The 1-based page number to return. * @param size The maximum number of artifacts to include in one page. - * @param filter Optional case-insensitive text used to narrow the result set. + * @param criteria Filter criteria containing search string, types, sources, repos, and formats. * @param projectId The SprintStart project that scopes the artifact listing. * @param authId The authenticated caller subject from the JWT. * @return One project-scoped artifact page together with pagination metadata. @@ -88,7 +88,7 @@ class ArtifactQueryService( fun getProjectArtifacts( page: Int, size: Int, - filter: String?, + criteria: ArtifactFilterCriteria, projectId: UUID, authId: String, ): ArtifactPageResponse { @@ -96,17 +96,13 @@ class ArtifactQueryService( val pageable = PageRequest.of( page - 1, size, - Sort.by("ingestedAt").descending(), + Sort.by("ingestedAt").descending().and(Sort.by("id").ascending()), ) - val result: Page = - if (filter.isNullOrBlank()) { - artifactRepository.findAllByProjectId(projectId, pageable) - } else { - artifactRepository.searchByProjectId(projectId, filter.trim(), pageable) - } + val result: Page = + artifactRepository.findProjectArtifactsWithCriteria(projectId, criteria, pageable) return ArtifactPageResponse( - items = result.content.map { artifactMapper.toResponse(it) }, + items = result.content, page = PageMetadata( number = page.toLong(), size = size.toLong(), @@ -118,6 +114,69 @@ class ArtifactQueryService( ) } + /** + * Legacy overload for project artifact queries specifying only a filter string. + */ + @Transactional(readOnly = true) + @Tracked("Retrieving list of artifacts for project") + fun getProjectArtifacts( + page: Int, + size: Int, + filter: String?, + projectId: UUID, + authId: String, + ): ArtifactPageResponse = getProjectArtifacts( + page = page, + size = size, + criteria = ArtifactFilterCriteria(search = filter), + projectId = projectId, + authId = authId, + ) + + /** + * Returns aggregated facet counts for a project based on the supplied criteria. + * + * @param projectId The SprintStart project that scopes the artifact listing. + * @param criteria Active filter criteria. + * @param authId The authenticated caller subject from the JWT. + * @return Aggregated facet counts. + */ + @Transactional(readOnly = true) + @Tracked("Retrieving artifact facets for project") + fun getProjectArtifactFacets( + projectId: UUID, + criteria: ArtifactFilterCriteria, + authId: String, + ): ArtifactFacetsResponse { + ensureAccessToProject(authId, projectId) + return artifactRepository.findFacets(projectId, criteria) + } + + /** + * Retrieves a single artifact by its ID within the project scope. + * + * @param projectId The SprintStart project that scopes the artifact. + * @param artifactId The ID of the artifact to retrieve. + * @param authId The authenticated caller subject from the JWT. + * @return The artifact response DTO. + * @throws ResponseStatusException `403` if access is denied, `404` if not found in project. + */ + @Transactional(readOnly = true) + @Tracked("Retrieving single artifact for project") + fun getArtifact( + projectId: UUID, + artifactId: UUID, + authId: String, + ): ArtifactResponse { + ensureAccessToProject(authId, projectId) + val artifact = artifactRepository.findByIdAndProjectId(artifactId, projectId) + ?: throw ResponseStatusException( + HttpStatus.NOT_FOUND, + "Artifact $artifactId not found in project $projectId", + ) + return artifactMapper.toResponse(artifact) + } + /** * Verifies that the authenticated caller may read artifacts for the requested project. * diff --git a/src/main/resources/db/migration/V20__add_artifact_project_index.sql b/src/main/resources/db/migration/V20__add_artifact_project_index.sql new file mode 100644 index 000000000..8c9e255e0 --- /dev/null +++ b/src/main/resources/db/migration/V20__add_artifact_project_index.sql @@ -0,0 +1,13 @@ +-- Reference-only migration script for the project-to-artifact join table index. +-- +-- In local development and automated test environments, database schema updates +-- are automatically applied by Hibernate (spring.jpa.hibernate.ddl-auto: update) +-- from the @Index annotation on Artifact.projectIdsInternal. +-- +-- In production PostgreSQL environments with high table volume, this index should +-- be created concurrently to prevent locking: +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_artifact_projects_project +-- ON artifact_projects(project_id, artifact_id); + +CREATE INDEX IF NOT EXISTS idx_artifact_projects_project + ON artifact_projects(project_id, artifact_id); diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index fcf12a6a7..e905803e6 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -2,10 +2,14 @@ package com.sprintstart.sprintstartbackend.ingestion.controller import com.ninjasquad.springmockk.MockkBean import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentRedirectResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactPageResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.PageMetadata import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactQueryService @@ -126,6 +130,129 @@ class ArtifactControllerTest( } } + @Test + fun `getProjectArtifacts forwards criteria with repeatable params and pagination`() { + val projectId = UUID.randomUUID() + val criteria = com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria( + search = "test", + types = setOf(ArtifactType.FILE, ArtifactType.ISSUE), + sources = setOf(SourceSystem.GITHUB), + repositories = setOf("owner/repo"), + format = UploadFormat.PDF, + ) + every { + artifactQueryService.getProjectArtifacts(1, 20, criteria, projectId, "auth-user") + } returns response() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("page", "1") + .param("size", "20") + .param("search", "test") + .param("types", "FILE", "ISSUE") + .param("sources", "GITHUB") + .param("repositories", "owner/repo") + .param("format", "PDF") + .with( + jwt() + .jwt { it.subject("auth-user") } + .authorities(SimpleGrantedAuthority("ROLE_USER")), + ), + ).andExpect(status().isOk) + .andExpect(jsonPath("$.items[0].title").value("README.md")) + + verify(exactly = 1) { + artifactQueryService.getProjectArtifacts(1, 20, criteria, projectId, "auth-user") + } + } + + @Test + fun `getProjectArtifactFacets returns facet counts and never binds to single artifact route`() { + val projectId = UUID.randomUUID() + val facets = ArtifactFacetsResponse( + types = listOf(FacetCountResponse("FILE", 10)), + sources = listOf(FacetCountResponse("GITHUB", 10)), + formats = listOf(FacetCountResponse("PDF", 2)), + repositories = listOf(FacetCountResponse("owner/repo", 8)), + ) + every { + artifactQueryService.getProjectArtifactFacets(projectId, any(), "auth-user") + } returns facets + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts/facets") + .with( + jwt() + .jwt { it.subject("auth-user") } + .authorities(SimpleGrantedAuthority("ROLE_USER")), + ), + ).andExpect(status().isOk) + .andExpect(jsonPath("$.types[0].value").value("FILE")) + .andExpect(jsonPath("$.types[0].count").value(10)) + .andExpect(jsonPath("$.sources[0].value").value("GITHUB")) + .andExpect(jsonPath("$.formats[0].value").value("PDF")) + .andExpect(jsonPath("$.repositories[0].value").value("owner/repo")) + + verify(exactly = 1) { + artifactQueryService.getProjectArtifactFacets(projectId, any(), "auth-user") + } + verify(exactly = 0) { + artifactQueryService.getArtifact(any(), any(), any()) + } + } + + @Test + fun `getArtifact returns single artifact when found`() { + val projectId = UUID.randomUUID() + val artifactId = UUID.randomUUID() + val artifactResponse = response().items.single().copy(id = artifactId) + every { + artifactQueryService.getArtifact(projectId, artifactId, "auth-user") + } returns artifactResponse + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts/$artifactId") + .with( + jwt() + .jwt { it.subject("auth-user") } + .authorities(SimpleGrantedAuthority("ROLE_USER")), + ), + ).andExpect(status().isOk) + .andExpect(jsonPath("$.id").value(artifactId.toString())) + .andExpect(jsonPath("$.title").value("README.md")) + + verify(exactly = 1) { + artifactQueryService.getArtifact(projectId, artifactId, "auth-user") + } + } + + @Test + fun `getArtifact returns 404 when artifact not found in project`() { + val projectId = UUID.randomUUID() + val artifactId = UUID.randomUUID() + every { + artifactQueryService.getArtifact(projectId, artifactId, "auth-user") + } throws org.springframework.web.server + .ResponseStatusException(org.springframework.http.HttpStatus.NOT_FOUND) + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts/$artifactId") + .with( + jwt() + .jwt { it.subject("auth-user") } + .authorities(SimpleGrantedAuthority("ROLE_USER")), + ), + ).andExpect(status().isNotFound) + + verify(exactly = 1) { + artifactQueryService.getArtifact(projectId, artifactId, "auth-user") + } + } + private fun response() = ArtifactPageResponse( items = listOf( ArtifactResponse( diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt new file mode 100644 index 000000000..f8f119d7c --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt @@ -0,0 +1,100 @@ +package com.sprintstart.sprintstartbackend.ingestion.service + +import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat +import com.sprintstart.sprintstartbackend.ingestion.repository.ArtifactFacetRepositoryImpl +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class ArtifactFacetServiceTest { + @Test + fun `extractRepositoryFromSourceId extracts owner and repo correctly`() { + val standard = "github:SprintStartProject/sprintstart-backend:FILE:README.md" + assertThat(ArtifactFacetRepositoryImpl.extractRepositoryFromSourceId(standard)) + .isEqualTo("SprintStartProject/sprintstart-backend") + + val issue = "github:owner/repo:ISSUE:42" + assertThat(ArtifactFacetRepositoryImpl.extractRepositoryFromSourceId(issue)) + .isEqualTo("owner/repo") + + val invalid = "jira:INSTANCE:ISSUE:101" + assertThat(ArtifactFacetRepositoryImpl.extractRepositoryFromSourceId(invalid)).isNull() + + val empty = "" + assertThat(ArtifactFacetRepositoryImpl.extractRepositoryFromSourceId(empty)).isNull() + } + + @Test + fun `classifyUploadFormat correctly classifies PDF`() { + val pdfMime = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "document", + sourceUrl = null, + sourceId = "uuid", + mime = "application/pdf", + language = null, + ) + assertThat(pdfMime).isEqualTo(UploadFormat.PDF) + + val pdfExt = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "guide.pdf", + sourceUrl = null, + sourceId = "uuid", + mime = null, + language = null, + ) + assertThat(pdfExt).isEqualTo(UploadFormat.PDF) + } + + @Test + fun `classifyUploadFormat correctly classifies Markdown`() { + val mdLang = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "README", + sourceUrl = null, + sourceId = "uuid", + mime = null, + language = "markdown", + ) + assertThat(mdLang).isEqualTo(UploadFormat.MARKDOWN) + + val mdExt = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "notes.md", + sourceUrl = null, + sourceId = "uuid", + mime = "text/plain", + language = null, + ) + assertThat(mdExt).isEqualTo(UploadFormat.MARKDOWN) + } + + @Test + fun `classifyUploadFormat correctly classifies Images`() { + val imgMime = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "diagram", + sourceUrl = null, + sourceId = "uuid", + mime = "image/png", + language = null, + ) + assertThat(imgMime).isEqualTo(UploadFormat.IMAGE) + + val imgExt = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "architecture.svg", + sourceUrl = null, + sourceId = "uuid", + mime = null, + language = null, + ) + assertThat(imgExt).isEqualTo(UploadFormat.IMAGE) + } + + @Test + fun `classifyUploadFormat falls back to Other`() { + val other = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "archive.zip", + sourceUrl = null, + sourceId = "uuid", + mime = "application/zip", + language = null, + ) + assertThat(other).isEqualTo(UploadFormat.OTHER) + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt index 6dcb527e6..20323a7fd 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt @@ -1,6 +1,9 @@ package com.sprintstart.sprintstartbackend.ingestion.service import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRun @@ -77,6 +80,97 @@ class ArtifactQueryServiceTest { verify(exactly = 0) { artifactRepository.findAll(any()) } } + @Test + fun `getProjectArtifacts forwards criteria and enforces access`() { + val projectId = UUID.randomUUID() + val authId = "auth-1" + val criteria = com.sprintstart.sprintstartbackend.ingestion.model.dto + .ArtifactFilterCriteria(search = "doc") + val pageable = slot() + val responseItem = com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse( + id = UUID.randomUUID(), + title = "doc.md", + sourceSystem = SourceSystem.GITHUB, + sourceId = "github:owner/repo:FILE:doc.md", + sourceUrl = null, + artifactType = ArtifactType.FILE, + ingestedAt = Instant.now(), + lastChangedAt = null, + metadata = "{}", + ) + every { userApi.userHasAccessToProject(authId, projectId) } returns true + every { + artifactRepository.findProjectArtifactsWithCriteria(projectId, criteria, capture(pageable)) + } returns PageImpl(listOf(responseItem), PageRequest.of(0, 20), 1) + + val result = service.getProjectArtifacts(1, 20, criteria, projectId, authId) + + assertThat(result.items).hasSize(1) + assertThat(result.items.single().title).isEqualTo("doc.md") + assertThat( + pageable.captured.sort + .getOrderFor("ingestedAt") + ?.isDescending, + ).isTrue() + assertThat( + pageable.captured.sort + .getOrderFor("id") + ?.isAscending, + ).isTrue() + } + + @Test + fun `getProjectArtifactFacets returns facets from repository`() { + val projectId = UUID.randomUUID() + val authId = "auth-1" + val criteria = ArtifactFilterCriteria() + val facets = ArtifactFacetsResponse( + types = listOf(FacetCountResponse("FILE", 5)), + sources = listOf(FacetCountResponse("GITHUB", 5)), + formats = emptyList(), + repositories = listOf(FacetCountResponse("owner/repo", 5)), + ) + every { userApi.userHasAccessToProject(authId, projectId) } returns true + every { artifactRepository.findFacets(projectId, criteria) } returns facets + + val result = service.getProjectArtifactFacets(projectId, criteria, authId) + + assertThat(result.types.single().value).isEqualTo("FILE") + assertThat(result.repositories.single().value).isEqualTo("owner/repo") + } + + @Test + fun `getArtifact returns mapped artifact when found`() { + val projectId = UUID.randomUUID() + val artifactId = UUID.randomUUID() + val authId = "auth-1" + val entity = artifact().apply { + val idField = Artifact::class.java.getDeclaredField("id") + idField.isAccessible = true + idField.set(this, artifactId) + } + every { userApi.userHasAccessToProject(authId, projectId) } returns true + every { artifactRepository.findByIdAndProjectId(artifactId, projectId) } returns entity + + val result = service.getArtifact(projectId, artifactId, authId) + + assertThat(result.id).isEqualTo(artifactId) + assertThat(result.title).isEqualTo("README.md") + } + + @Test + fun `getArtifact throws 404 when not found in project`() { + val projectId = UUID.randomUUID() + val artifactId = UUID.randomUUID() + val authId = "auth-1" + every { userApi.userHasAccessToProject(authId, projectId) } returns true + every { artifactRepository.findByIdAndProjectId(artifactId, projectId) } returns null + + org.junit.jupiter.api.assertThrows { + service.getArtifact(projectId, artifactId, authId) + } + } + private fun artifact() = Artifact( id = UUID.fromString("3fa85f64-5717-4562-b3fc-2c963f66afa6"), sourceSystem = SourceSystem.GITHUB, From 46476fcfec0244cf316f18bcb5fd77878b1ccc48 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Wed, 23 Sep 2026 20:38:09 +0200 Subject: [PATCH 2/9] upgrade(ingestion): finish the artifact pagination upgrade MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the server-side pagination and faceted search work with the findings from a full review of the change. Correctness: * the upload-format filter disagreed with the facet counts it sits next to. The predicate compared nullable columns directly, so `NOT(OR(...))` — the OTHER bucket — evaluated to NULL instead of TRUE: an upload with no mime and a .txt title was counted as OTHER but returned nothing when that filter was picked. The nullable columns now fold to "" exactly as `classifyUploadFormat` does, so the counts and the filter can no longer describe different rows. * `GET .../artifacts/{artifactId}` hydrated the entity, dragging the eagerly fetched `content` TEXT column along to answer a metadata question. Both reads now share one projection, so the deep-link path cannot drift from the list. Cleanup: * removed the legacy `getProjectArtifacts(page, size, filter, ...)` overload and `ArtifactRepository.findByIdAndProjectId`; neither had a caller left. * `filter` is documented as what it now is — an alias of `search` — in OpenAPI and in code, instead of quietly changing meaning under its old description. * `Index` is imported rather than written out fully qualified; the reference-only V20 script no longer documents CONCURRENTLY and then ships the blocking statement; the `.gitignore` note no longer names the tool that wrote it. * `ArtifactFacetServiceTest` tested `ArtifactFacetRepositoryImpl` — renamed and moved to the repository package, plus a classifier case for the row that exposed the OTHER bug above. Gates: ./gradlew build (ktlint, detekt, tests). --- .gitignore | 2 +- .../controller/ArtifactController.kt | 14 +++- .../ingestion/model/entity/Artifact.kt | 3 +- .../repository/ArtifactFacetRepository.kt | 15 ++++ .../repository/ArtifactFacetRepositoryImpl.kt | 76 ++++++++++++++----- .../repository/ArtifactRepository.kt | 10 --- .../ingestion/service/ArtifactQueryService.kt | 25 +----- .../V20__add_artifact_project_index.sql | 19 +++-- .../controller/ArtifactControllerTest.kt | 2 +- .../ArtifactFacetRepositoryImplTest.kt} | 20 ++++- .../service/ArtifactQueryServiceTest.kt | 24 ++++-- 11 files changed, 134 insertions(+), 76 deletions(-) rename src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/{service/ArtifactFacetServiceTest.kt => repository/ArtifactFacetRepositoryImplTest.kt} (80%) diff --git a/.gitignore b/.gitignore index 3f0883ac6..31df2b31d 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,6 @@ CLAUDE.md __pycache__/ *.pyc -# Hermes agent working files (per-developer) +# Local parallel-work checkouts (per-developer, never committed) .worktrees/ .worktreeinclude diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index fca6f630e..bfa0c2567 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -108,8 +108,18 @@ class ArtifactController( fun getProjectArtifacts( @RequestParam(defaultValue = DEFAULT_PAGE) @Min(1) page: Int, @RequestParam(defaultValue = DEFAULT_SIZE) @Min(1) @Max(MAX_PAGE_SIZE) size: Int, - @RequestParam(defaultValue = "") filter: String, - @RequestParam(required = false) search: String?, + // `filter` predates `search` and kept its own contract (a fragment matched against title, + // type, source system and metadata) until the Knowledge Base moved server-side. It is now + // an alias of `search`: nothing in this repo sends it, and it stays only so an outside + // client that does is not broken by a query whose meaning it cannot see changing. + @Parameter(description = "Deprecated alias of `search`; send `search` instead") + @RequestParam(defaultValue = "") + filter: String, + @Parameter( + description = "Case-insensitive match against the artifact's title, source id and source url", + ) + @RequestParam(required = false) + search: String?, @RequestParam(required = false) types: Set?, @RequestParam(required = false) sources: Set?, @RequestParam(required = false) repositories: Set?, diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt index ee3c5c256..11005bc02 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/entity/Artifact.kt @@ -9,6 +9,7 @@ import jakarta.persistence.EnumType import jakarta.persistence.Enumerated import jakarta.persistence.FetchType import jakarta.persistence.Id +import jakarta.persistence.Index import jakarta.persistence.JoinColumn import jakarta.persistence.ManyToOne import java.time.Instant @@ -49,7 +50,7 @@ class Artifact( name = "artifact_projects", joinColumns = [JoinColumn(name = "artifact_id")], indexes = [ - jakarta.persistence.Index( + Index( name = "idx_artifact_projects_project", columnList = "project_id, artifact_id", ), diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt index bb799680a..1d1a6513a 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt @@ -27,6 +27,21 @@ interface ArtifactFacetRepository { pageable: Pageable, ): Page + /** + * Resolves one artifact's metadata projection, scoped to a project the caller can see. + * + * Used to open a deep-linked artifact that is not on the page currently loaded, so it must + * not hydrate `content` either — see [findProjectArtifactsWithCriteria]. + * + * @param projectId Scopes the artifact to the target project. + * @param artifactId The artifact to resolve. + * @return The artifact projection, or null when it is not linked to that project. + */ + fun findProjectArtifactById( + projectId: UUID, + artifactId: UUID, + ): ArtifactResponse? + /** * Calculates aggregated counts for types, sources, upload formats, and repositories * using the "count each would add" model (own-facet-excluded, other-facets-applied). diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt index 68d39c488..2d1d33b3e 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -10,6 +10,7 @@ import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import jakarta.persistence.EntityManager import jakarta.persistence.PersistenceContext +import jakarta.persistence.criteria.CompoundSelection import jakarta.persistence.criteria.CriteriaBuilder import jakarta.persistence.criteria.Join import jakarta.persistence.criteria.Predicate @@ -48,21 +49,7 @@ class ArtifactFacetRepositoryImpl( val root = query.from(Artifact::class.java) val projectJoin = root.join("projectIdsInternal") - query.select( - cb.construct( - ArtifactResponse::class.java, - root.get("id"), - root.get("title"), - root.get("sourceSystem"), - root.get("sourceId"), - root.get("sourceUrl"), - root.get("artifactType"), - root.get("ingestedAt"), - root.get("lastChangedAt"), - root.get("metadata"), - root.get("sourceVersion"), - ), - ) + query.select(artifactProjection(cb, root)) val predicates = buildPredicates(cb, root, projectJoin, projectId, criteria, null) query.where(*predicates.toTypedArray()) @@ -92,6 +79,52 @@ class ArtifactFacetRepositoryImpl( return PageImpl(items, pageable, totalElements) } + override fun findProjectArtifactById( + projectId: UUID, + artifactId: UUID, + ): ArtifactResponse? { + val cb = entityManager.criteriaBuilder + val query = cb.createQuery(ArtifactResponse::class.java) + val root = query.from(Artifact::class.java) + val projectJoin = root.join("projectIdsInternal") + + query.select(artifactProjection(cb, root)) + query.where( + cb.equal(root.get("id"), artifactId), + cb.equal(projectJoin, projectId), + ) + + return entityManager + .createQuery(query) + .setMaxResults(1) + .resultList + .firstOrNull() + } + + /** + * The response projection shared by every artifact read. + * + * Deliberately not the entity: `Artifact` carries the eagerly fetched `content` TEXT column, + * so hydrating it to answer a metadata question drags whole file bodies across JDBC. Keep the + * field list here and nowhere else — [ArtifactResponse] is the only shape either query builds. + */ + private fun artifactProjection( + cb: CriteriaBuilder, + root: Root, + ): CompoundSelection = cb.construct( + ArtifactResponse::class.java, + root.get("id"), + root.get("title"), + root.get("sourceSystem"), + root.get("sourceId"), + root.get("sourceUrl"), + root.get("artifactType"), + root.get("ingestedAt"), + root.get("lastChangedAt"), + root.get("metadata"), + root.get("sourceVersion"), + ) + override fun findFacets( projectId: UUID, criteria: ArtifactFilterCriteria, @@ -301,11 +334,16 @@ class ArtifactFacetRepositoryImpl( root: Root, format: UploadFormat, ): Predicate { - val titleLower = cb.lower(root.get("title")) - val sourceUrlLower = cb.lower(root.get("sourceUrl")) + // Nullable columns fold to "" exactly as `classifyUploadFormat` does, so this predicate + // and the Kotlin classifier that produces the facet counts can never disagree. Without + // the coalesce, `NOT(OR(...))` — the OTHER bucket — evaluates to NULL rather than TRUE + // for a row whose mime, language and title are unset, and an upload the facet counts as + // OTHER would come back from the filter as nothing at all. + val titleLower = cb.coalesce(cb.lower(root.get("title")), "") + val sourceUrlLower = cb.coalesce(cb.lower(root.get("sourceUrl")), "") val sourceIdLower = cb.lower(root.get("sourceId")) - val mimeLower = cb.lower(root.get("mime")) - val languageLower = cb.lower(root.get("language")) + val mimeLower = cb.coalesce(cb.lower(root.get("mime")), "") + val languageLower = cb.coalesce(cb.lower(root.get("language")), "") val isPdf = cb.or( cb.equal(mimeLower, "application/pdf"), diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt index cf6ab8f10..f3bf79b0f 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt @@ -25,16 +25,6 @@ interface ArtifactRepository : ArtifactFacetRepository { fun findBySourceId(sourceId: String): Artifact? - @Query( - """ - SELECT a - FROM Artifact a - JOIN a.projectIdsInternal p - WHERE a.id = :artifactId AND p = :projectId - """, - ) - fun findByIdAndProjectId(@Param("artifactId") artifactId: UUID, @Param("projectId") projectId: UUID): Artifact? - /** * Batch variant of [findBySourceId]. Source ids with no artifact are simply absent, so a * caller comparing a set of rows against the corpus learns which of them it no longer holds. diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt index 91a595715..a7e721def 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt @@ -114,25 +114,6 @@ class ArtifactQueryService( ) } - /** - * Legacy overload for project artifact queries specifying only a filter string. - */ - @Transactional(readOnly = true) - @Tracked("Retrieving list of artifacts for project") - fun getProjectArtifacts( - page: Int, - size: Int, - filter: String?, - projectId: UUID, - authId: String, - ): ArtifactPageResponse = getProjectArtifacts( - page = page, - size = size, - criteria = ArtifactFilterCriteria(search = filter), - projectId = projectId, - authId = authId, - ) - /** * Returns aggregated facet counts for a project based on the supplied criteria. * @@ -169,12 +150,14 @@ class ArtifactQueryService( authId: String, ): ArtifactResponse { ensureAccessToProject(authId, projectId) - val artifact = artifactRepository.findByIdAndProjectId(artifactId, projectId) + // A projection, not the entity: opening a deep link needs metadata only, and + // `Artifact.content` is an eagerly fetched TEXT column (see + // ArtifactFacetRepositoryImpl.artifactProjection). + return artifactRepository.findProjectArtifactById(projectId, artifactId) ?: throw ResponseStatusException( HttpStatus.NOT_FOUND, "Artifact $artifactId not found in project $projectId", ) - return artifactMapper.toResponse(artifact) } /** diff --git a/src/main/resources/db/migration/V20__add_artifact_project_index.sql b/src/main/resources/db/migration/V20__add_artifact_project_index.sql index 8c9e255e0..bc5c784c2 100644 --- a/src/main/resources/db/migration/V20__add_artifact_project_index.sql +++ b/src/main/resources/db/migration/V20__add_artifact_project_index.sql @@ -1,13 +1,12 @@ --- Reference-only migration script for the project-to-artifact join table index. +-- Reference-only script for the project-to-artifact join-table index. -- --- In local development and automated test environments, database schema updates --- are automatically applied by Hibernate (spring.jpa.hibernate.ddl-auto: update) --- from the @Index annotation on Artifact.projectIdsInternal. +-- Nothing executes this file. The backend has no Flyway; schema changes reach a +-- database through Hibernate (`spring.jpa.hibernate.ddl-auto: update`), which +-- creates the index declared on `Artifact.projectIdsInternal` on the next boot. +-- It is kept so the index stays reviewable, and so a production database can be +-- brought in line by hand. -- --- In production PostgreSQL environments with high table volume, this index should --- be created concurrently to prevent locking: --- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_artifact_projects_project --- ON artifact_projects(project_id, artifact_id); - -CREATE INDEX IF NOT EXISTS idx_artifact_projects_project +-- Run it OUTSIDE a transaction — CONCURRENTLY is rejected inside one — during a +-- quiet window: it takes a SHARE UPDATE EXCLUSIVE lock and never blocks writes. +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_artifact_projects_project ON artifact_projects(project_id, artifact_id); diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index e905803e6..752a44dab 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -133,7 +133,7 @@ class ArtifactControllerTest( @Test fun `getProjectArtifacts forwards criteria with repeatable params and pagination`() { val projectId = UUID.randomUUID() - val criteria = com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria( + val criteria = ArtifactFilterCriteria( search = "test", types = setOf(ArtifactType.FILE, ArtifactType.ISSUE), sources = setOf(SourceSystem.GITHUB), diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt similarity index 80% rename from src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt rename to src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt index f8f119d7c..d6cf8e350 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactFacetServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt @@ -1,11 +1,10 @@ -package com.sprintstart.sprintstartbackend.ingestion.service +package com.sprintstart.sprintstartbackend.ingestion.repository import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat -import com.sprintstart.sprintstartbackend.ingestion.repository.ArtifactFacetRepositoryImpl import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -class ArtifactFacetServiceTest { +class ArtifactFacetRepositoryImplTest { @Test fun `extractRepositoryFromSourceId extracts owner and repo correctly`() { val standard = "github:SprintStartProject/sprintstart-backend:FILE:README.md" @@ -44,6 +43,21 @@ class ArtifactFacetServiceTest { assertThat(pdfExt).isEqualTo(UploadFormat.PDF) } + @Test + fun `classifyUploadFormat buckets a row with no mime and an unknown extension as OTHER`() { + // The facet counts come from this classifier while the filter runs as SQL. The predicate + // folds null columns to "" for exactly this row, so a plain .txt upload stays reachable + // through the OTHER filter instead of being counted but unfilterable. + val other = ArtifactFacetRepositoryImpl.classifyUploadFormat( + title = "notes.txt", + sourceUrl = null, + sourceId = "6f1e2f2c-0000-4000-8000-000000000000", + mime = null, + language = null, + ) + assertThat(other).isEqualTo(UploadFormat.OTHER) + } + @Test fun `classifyUploadFormat correctly classifies Markdown`() { val mdLang = ArtifactFacetRepositoryImpl.classifyUploadFormat( diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt index 20323a7fd..388a5327a 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt @@ -3,6 +3,7 @@ package com.sprintstart.sprintstartbackend.ingestion.service import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType @@ -140,17 +141,24 @@ class ArtifactQueryServiceTest { } @Test - fun `getArtifact returns mapped artifact when found`() { + fun `getArtifact returns the projected artifact when found`() { val projectId = UUID.randomUUID() val artifactId = UUID.randomUUID() val authId = "auth-1" - val entity = artifact().apply { - val idField = Artifact::class.java.getDeclaredField("id") - idField.isAccessible = true - idField.set(this, artifactId) - } every { userApi.userHasAccessToProject(authId, projectId) } returns true - every { artifactRepository.findByIdAndProjectId(artifactId, projectId) } returns entity + every { + artifactRepository.findProjectArtifactById(projectId, artifactId) + } returns ArtifactResponse( + id = artifactId, + title = "README.md", + sourceSystem = SourceSystem.GITHUB, + sourceId = "github:owner/repo:FILE:README.md", + sourceUrl = "https://github.com/owner/repo/blob/main/README.md", + artifactType = ArtifactType.FILE, + ingestedAt = Instant.parse("2026-06-19T09:16:30Z"), + lastChangedAt = null, + metadata = """{"repositoryFullName":"owner/repo"}""", + ) val result = service.getArtifact(projectId, artifactId, authId) @@ -164,7 +172,7 @@ class ArtifactQueryServiceTest { val artifactId = UUID.randomUUID() val authId = "auth-1" every { userApi.userHasAccessToProject(authId, projectId) } returns true - every { artifactRepository.findByIdAndProjectId(artifactId, projectId) } returns null + every { artifactRepository.findProjectArtifactById(projectId, artifactId) } returns null org.junit.jupiter.api.assertThrows { service.getArtifact(projectId, artifactId, authId) From 69bf7b296f7a053c7000894b8a59cfe5dbaedb56 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 10:01:41 +0200 Subject: [PATCH 3/9] feat(knowledge-base): sort the project artifact list by added, changed or title The project artifact list could only be read in one order (newest import first). Knowledge-base users asked for "recently changed" and an alphabetical view, so GET /api/v1/projects/{projectId}/artifacts now takes a `sort` query parameter: ADDED_DESC (default) ingestedAt DESC, id ASC -- exactly today's order CHANGED_DESC coalesce(lastChangedAt, ingestedAt) DESC, id ASC TITLE_ASC lower(title) ASC NULLS LAST, id ASC The values and their SQL come from the pinned cross-repo contract (kb-contract.md) that the frontend and the AI service are built against in parallel, so the names are uppercase enum constants and are not negotiable here. Why a new enum (ArtifactSort) instead of Spring's `sort=field,dir`: - The contract offers three named orderings, not arbitrary columns. Accepting free-form Pageable sort strings would let a client order by any entity property (including the heavyweight `content` column, or properties the projection does not even select) and would make the set of supported orderings an accident of the entity shape. - Binding to an enum gives the "unknown value -> 400" rule for free: Spring's String->Enum conversion fails with a type mismatch, which the default handler resolver answers with 400 before the service runs. No hand-written validation, no chance of a silent fallback. - `defaultValue = "ADDED_DESC"` keeps every existing caller (which sends no sort) on the exact order it had before, so this is backwards compatible for the current frontend. Why the order is carried as its own argument and not through Pageable: - ArtifactQueryService built a PageRequest with Sort(ingestedAt DESC, id ASC), but ArtifactFacetRepositoryImpl never read pageable.sort; it hard-coded the same ORDER BY itself. The Sort was dead code that only looked like it controlled ordering. It is removed; the service now passes an unsorted PageRequest (page window only) plus the ArtifactSort, and the repository KDoc says so. - It is a separate parameter rather than a field on ArtifactFilterCriteria because the criteria object is also what the facets endpoint receives and what the list/facet parity guarantee is defined over. Facets ignore sort (contract), so putting sort into the criteria would make two criteria that select the same rows compare unequal and would invite a future facet query to depend on it. Why every ordering ends in `id ASC`: - The list is offset-paginated. Without a unique tie-break, rows that share the leading key (same ingest instant from a bulk sync, same title, or no title at all) can be returned in a different order by each query, so a row can appear on two pages or on none. id is the primary key, so the full ORDER BY is a total order. Why TITLE_ASC uses lower(title) and the JPA 3.2 Nulls API: - lower() makes "alpha" and "Alpha" sort together, which is what a person scanning an alphabetical list expects; the id tie-break then keeps the two deterministic. - Untitled artifacts must not float to the top. PostgreSQL sorts NULLs last for ASC but H2 and other databases differ, so the null placement is stated explicitly with cb.asc(expr, jakarta.persistence.criteria .Nulls.LAST) -- the portable JPA 3.2 API that Hibernate 7 renders as NULLS LAST -- instead of Hibernate's NullPrecedence extension, which would tie the repository to a provider-specific API for no gain. Why ArtifactFacetRepositoryImpl now carries @Suppress("TooManyFunctions"): - The ordering builder is its 12th function and detekt caps classes at 11. The class is one function per facet dimension plus the shared predicate/order builders; splitting it would scatter the own-dimension exclusion rule that has to stay identical between list and facets. The suppression follows the 30+ existing ones in the codebase (ArtifactRepository included) rather than inventing a new split. Why CHANGED_DESC coalesces with ingestedAt: - lastChangedAt is null until ingestion sees the content change for the first time. Treating "never changed" as "changed when it was imported" gives those rows a real position instead of lumping them at one end, and ingestedAt is non-null so the expression never is. Tests: - New ArtifactFacetRepositoryQueryTest, a @DataJpaTest against H2 (same setup as ArtifactProjectRepositoryTest). The existing ArtifactFacetRepositoryImplTest only unit-tests helpers and never executes a query, so nothing pinned the SQL the criteria API renders. Covers: each ordering, the id tie-break for equal keys, NULLS LAST and case folding for titles, the coalesce fallback, stable page boundaries for rows sharing a key, and that sorting does not change totalElements. Ids are UUID(0, n) so their order is the same on every database. - ArtifactControllerTest: explicit sort binds to the enum and reaches the service; the default is ADDED_DESC; an unknown value is rejected with 400 and never reaches the service. - ArtifactQueryServiceTest: the sort is forwarded to the repository and the Pageable is unsorted (page window only), replacing the assertions on the dead Sort. Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco) green: 3426 tests, 0 failures (baseline before this change: 3419). --- .../controller/ArtifactController.kt | 10 +- .../ingestion/model/dto/ArtifactSort.kt | 18 ++ .../repository/ArtifactFacetRepository.kt | 6 +- .../repository/ArtifactFacetRepositoryImpl.kt | 36 +++- .../ingestion/service/ArtifactQueryService.kt | 13 +- .../controller/ArtifactControllerTest.kt | 58 +++++- .../ArtifactFacetRepositoryQueryTest.kt | 183 ++++++++++++++++++ .../service/ArtifactQueryServiceTest.kt | 24 +-- 8 files changed, 321 insertions(+), 27 deletions(-) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactSort.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index bfa0c2567..f13b773ca 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -2,6 +2,7 @@ package com.sprintstart.sprintstartbackend.ingestion.controller import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentRedirectResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentResponse @@ -124,6 +125,13 @@ class ArtifactController( @RequestParam(required = false) sources: Set?, @RequestParam(required = false) repositories: Set?, @RequestParam(required = false) format: UploadFormat?, + @Parameter( + description = "Row order: ADDED_DESC (newest import first, the default), " + + "CHANGED_DESC (latest content change first, falling back to the import time) or " + + "TITLE_ASC (case-insensitive, untitled last). Any other value is rejected with 400.", + ) + @RequestParam(defaultValue = "ADDED_DESC") + sort: ArtifactSort, @Parameter( description = "UUID of the project whose artifacts should be returned", ) @PathVariable projectId: UUID, @@ -138,7 +146,7 @@ class ArtifactController( format = format, ) return ResponseEntity.ok( - artifactQueryService.getProjectArtifacts(page, size, criteria, projectId, jwt.subject), + artifactQueryService.getProjectArtifacts(page, size, criteria, sort, projectId, jwt.subject), ) } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactSort.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactSort.kt new file mode 100644 index 000000000..94e8d5813 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactSort.kt @@ -0,0 +1,18 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto + +/** + * Orderings offered by the project artifact list. + * + * Every ordering ends with `id ASC` as a tie-break, so a page boundary never splits or repeats + * rows that share the leading sort key. Facet counts are order independent and ignore this. + */ +enum class ArtifactSort { + /** Newest first import: `ingestedAt DESC, id ASC`. The default. */ + ADDED_DESC, + + /** Most recently changed: `coalesce(lastChangedAt, ingestedAt) DESC, id ASC`. */ + CHANGED_DESC, + + /** Alphabetical: `lower(title) ASC NULLS LAST, id ASC`. */ + TITLE_ASC, +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt index 1d1a6513a..b1cb52cb7 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt @@ -1,6 +1,7 @@ package com.sprintstart.sprintstartbackend.ingestion.repository import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse import org.springframework.data.domain.Page @@ -18,12 +19,15 @@ interface ArtifactFacetRepository { * * @param projectId Scopes artifacts to the target project. * @param criteria Filter criteria containing search text, types, sources, repos, and formats. - * @param pageable Requested pagination and sorting. + * @param sort Row order. Applied here rather than through [pageable], whose sort is ignored: + * the criteria query owns ordering so every [ArtifactSort] keeps its `id ASC` tie-break. + * @param pageable Requested page number and size. * @return Paginated page of artifact response projections. */ fun findProjectArtifactsWithCriteria( projectId: UUID, criteria: ArtifactFilterCriteria, + sort: ArtifactSort, pageable: Pageable, ): Page diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt index 2d1d33b3e..cc817d2bb 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -2,6 +2,7 @@ package com.sprintstart.sprintstartbackend.ingestion.repository import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse @@ -13,6 +14,8 @@ import jakarta.persistence.PersistenceContext import jakarta.persistence.criteria.CompoundSelection import jakarta.persistence.criteria.CriteriaBuilder import jakarta.persistence.criteria.Join +import jakarta.persistence.criteria.Nulls +import jakarta.persistence.criteria.Order import jakarta.persistence.criteria.Predicate import jakarta.persistence.criteria.Root import org.springframework.data.domain.Page @@ -34,12 +37,16 @@ private val SOURCES_EXCLUDED_FACETS = setOf(FacetKind.SOURCES, FacetKind.FORMATS @Repository @Transactional(readOnly = true) +// One function per facet dimension plus the shared predicate/order builders; splitting them would +// scatter the own-dimension exclusion rule that must stay identical across list and facets. +@Suppress("TooManyFunctions") class ArtifactFacetRepositoryImpl( @PersistenceContext private val entityManager: EntityManager, ) : ArtifactFacetRepository { override fun findProjectArtifactsWithCriteria( projectId: UUID, criteria: ArtifactFilterCriteria, + sort: ArtifactSort, pageable: Pageable, ): Page { val cb = entityManager.criteriaBuilder @@ -54,11 +61,8 @@ class ArtifactFacetRepositoryImpl( val predicates = buildPredicates(cb, root, projectJoin, projectId, criteria, null) query.where(*predicates.toTypedArray()) - // D2: Deterministic sort (ingestedAt DESC, id ASC) - query.orderBy( - cb.desc(root.get("ingestedAt")), - cb.asc(root.get("id")), - ) + // D2: Deterministic sort -- the requested key, then id ASC as the tie-break. + query.orderBy(orderFor(cb, root, sort)) val typedQuery = entityManager.createQuery(query) typedQuery.firstResult = pageable.offset.toInt() @@ -79,6 +83,28 @@ class ArtifactFacetRepositoryImpl( return PageImpl(items, pageable, totalElements) } + /** + * Builds the ORDER BY clause for [sort], always ending in `id ASC`. + * + * The id tie-break keeps offset pagination stable: rows sharing the leading key (same + * import instant, same title, or no title at all) keep one fixed order across pages. + */ + private fun orderFor( + cb: CriteriaBuilder, + root: Root, + sort: ArtifactSort, + ): List { + val leading = when (sort) { + ArtifactSort.ADDED_DESC -> cb.desc(root.get("ingestedAt")) + ArtifactSort.CHANGED_DESC -> cb.desc( + cb.coalesce(root.get("lastChangedAt"), root.get("ingestedAt")), + ) + // Nulls.LAST is spelled out: databases disagree on where NULL sorts by default. + ArtifactSort.TITLE_ASC -> cb.asc(cb.lower(root.get("title")), Nulls.LAST) + } + return listOf(leading, cb.asc(root.get("id"))) + } + override fun findProjectArtifactById( projectId: UUID, artifactId: UUID, diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt index a7e721def..267248f84 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt @@ -1,6 +1,7 @@ package com.sprintstart.sprintstartbackend.ingestion.service import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactPageResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse @@ -77,6 +78,7 @@ class ArtifactQueryService( * @param page The 1-based page number to return. * @param size The maximum number of artifacts to include in one page. * @param criteria Filter criteria containing search string, types, sources, repos, and formats. + * @param sort Row order; the repository applies it together with the `id ASC` tie-break. * @param projectId The SprintStart project that scopes the artifact listing. * @param authId The authenticated caller subject from the JWT. * @return One project-scoped artifact page together with pagination metadata. @@ -89,18 +91,17 @@ class ArtifactQueryService( page: Int, size: Int, criteria: ArtifactFilterCriteria, + sort: ArtifactSort, projectId: UUID, authId: String, ): ArtifactPageResponse { ensureAccessToProject(authId, projectId) - val pageable = PageRequest.of( - page - 1, - size, - Sort.by("ingestedAt").descending().and(Sort.by("id").ascending()), - ) + // Unsorted on purpose: the criteria repository owns ORDER BY (see ArtifactSort) and + // would ignore a Pageable sort, so passing one here would only suggest otherwise. + val pageable = PageRequest.of(page - 1, size) val result: Page = - artifactRepository.findProjectArtifactsWithCriteria(projectId, criteria, pageable) + artifactRepository.findProjectArtifactsWithCriteria(projectId, criteria, sort, pageable) return ArtifactPageResponse( items = result.content, page = PageMetadata( diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index 752a44dab..eff22cbb7 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -3,6 +3,7 @@ package com.sprintstart.sprintstartbackend.ingestion.controller import com.ninjasquad.springmockk.MockkBean import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentRedirectResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentResponse @@ -141,7 +142,7 @@ class ArtifactControllerTest( format = UploadFormat.PDF, ) every { - artifactQueryService.getProjectArtifacts(1, 20, criteria, projectId, "auth-user") + artifactQueryService.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, projectId, "auth-user") } returns response() mockMvc @@ -163,7 +164,56 @@ class ArtifactControllerTest( .andExpect(jsonPath("$.items[0].title").value("README.md")) verify(exactly = 1) { - artifactQueryService.getProjectArtifacts(1, 20, criteria, projectId, "auth-user") + artifactQueryService.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, projectId, "auth-user") + } + } + + @Test + fun `getProjectArtifacts binds an explicit sort`() { + val projectId = UUID.randomUUID() + every { + artifactQueryService.getProjectArtifacts( + 1, + 20, + ArtifactFilterCriteria(), + ArtifactSort.CHANGED_DESC, + projectId, + "auth-user", + ) + } returns response() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("sort", "CHANGED_DESC") + .with(userJwt()), + ).andExpect(status().isOk) + + verify(exactly = 1) { + artifactQueryService.getProjectArtifacts( + 1, + 20, + ArtifactFilterCriteria(), + ArtifactSort.CHANGED_DESC, + projectId, + "auth-user", + ) + } + } + + @Test + fun `getProjectArtifacts rejects an unknown sort with 400`() { + val projectId = UUID.randomUUID() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("sort", "RANDOM") + .with(userJwt()), + ).andExpect(status().isBadRequest) + + verify(exactly = 0) { + artifactQueryService.getProjectArtifacts(any(), any(), any(), any(), any(), any()) } } @@ -276,4 +326,8 @@ class ArtifactControllerTest( hasPrevious = false, ), ) + + private fun userJwt() = jwt() + .jwt { it.subject("auth-user") } + .authorities(SimpleGrantedAuthority("ROLE_USER")) } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt new file mode 100644 index 000000000..f7e313755 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt @@ -0,0 +1,183 @@ +package com.sprintstart.sprintstartbackend.ingestion.repository + +import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort +import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact +import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRun +import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRunStatus +import com.sprintstart.sprintstartbackend.shared.crypto.CryptoConfiguration +import jakarta.persistence.EntityManager +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest +import org.springframework.context.annotation.Import +import org.springframework.data.domain.PageRequest +import org.springframework.test.context.ActiveProfiles +import java.time.Instant +import java.util.UUID + +/** + * Runs the criteria queries behind the project artifact list and its facets against H2. + * + * Ordering, date bounds and case folding are decided by the SQL the criteria API renders, which a + * mocked repository cannot see; these tests pin that SQL's behaviour instead of the builder calls. + */ +@ActiveProfiles("test") +@DataJpaTest +// A JPA slice loads no @Configuration of its own, but the entity graph reaches an +// AttributeConverter that needs the encryptor. +@Import(CryptoConfiguration::class) +class ArtifactFacetRepositoryQueryTest { + @Autowired + private lateinit var repository: ArtifactRepository + + @Autowired + private lateinit var entityManager: EntityManager + + private lateinit var run: IngestionRun + + private val projectId = UUID.randomUUID() + + @BeforeEach + fun setUp() { + run = IngestionRun( + id = UUID.randomUUID(), + sourceSystem = SourceSystem.GITHUB, + status = IngestionRunStatus.COMPLETED, + ) + entityManager.persist(run) + } + + // ========================== sort ========================== + + @Test + fun `ADDED_DESC lists the newest import first and breaks ties by id`() { + val old = store(id = id(1), ingestedAt = BASE.minusSeconds(60)) + val newerSecond = store(id = id(3), ingestedAt = BASE) + val newerFirst = store(id = id(2), ingestedAt = BASE) + flush() + + assertThat(listIds(sort = ArtifactSort.ADDED_DESC)) + .containsExactly(newerFirst.id, newerSecond.id, old.id) + } + + @Test + fun `CHANGED_DESC orders by the latest change and falls back to the import time`() { + // Imported first but changed last: the change, not the import, decides its place. + val changedLate = store(id = id(1), ingestedAt = BASE, lastChangedAt = BASE.plusSeconds(500)) + val neverChanged = store(id = id(4), ingestedAt = BASE.plusSeconds(300)) + val neverChangedTie = store(id = id(2), ingestedAt = BASE.plusSeconds(300)) + val changedEarly = store(id = id(3), ingestedAt = BASE, lastChangedAt = BASE.plusSeconds(100)) + flush() + + assertThat(listIds(sort = ArtifactSort.CHANGED_DESC)) + .containsExactly(changedLate.id, neverChangedTie.id, neverChanged.id, changedEarly.id) + } + + @Test + fun `TITLE_ASC ignores case, puts untitled artifacts last and breaks ties by id`() { + val untitledSecond = store(id = id(6), title = null) + val beta = store(id = id(1), title = "beta") + val untitledFirst = store(id = id(2), title = null) + val lowerAlpha = store(id = id(5), title = "alpha") + val upperAlpha = store(id = id(3), title = "Alpha") + flush() + + // "Alpha" and "alpha" fold to one key, so only the id decides between them. + assertThat(listIds(sort = ArtifactSort.TITLE_ASC)).containsExactly( + upperAlpha.id, + lowerAlpha.id, + beta.id, + untitledFirst.id, + untitledSecond.id, + ) + } + + @Test + fun `the id tie-break keeps rows sharing a sort key stable across pages`() { + val first = store(id = id(1), title = null) + val second = store(id = id(2), title = null) + val third = store(id = id(3), title = null) + flush() + + val pages = (0..2).flatMap { page -> + list(sort = ArtifactSort.TITLE_ASC, page = page, size = 1).content.map { it.id } + } + + assertThat(pages).containsExactly(first.id, second.id, third.id) + } + + @Test + fun `sorting leaves the total count untouched`() { + repeat(3) { store() } + store(project = UUID.randomUUID()) + flush() + + ArtifactSort.entries.forEach { sort -> + assertThat(list(sort = sort).totalElements).isEqualTo(3) + } + } + + // ========================== helpers ========================== + + private fun list( + criteria: ArtifactFilterCriteria = ArtifactFilterCriteria(), + sort: ArtifactSort = ArtifactSort.ADDED_DESC, + page: Int = 0, + size: Int = 50, + ) = repository.findProjectArtifactsWithCriteria(projectId, criteria, sort, PageRequest.of(page, size)) + + private fun listIds( + criteria: ArtifactFilterCriteria = ArtifactFilterCriteria(), + sort: ArtifactSort = ArtifactSort.ADDED_DESC, + ): List = list(criteria, sort).content.map { it.id } + + /** Ids whose natural order is unambiguous on every database: `...0001` sorts before `...0002`. */ + private fun id(n: Long): UUID = UUID(0L, n) + + @Suppress("LongParameterList") + private fun store( + id: UUID = UUID.randomUUID(), + title: String? = "file-$id", + ingestedAt: Instant = BASE, + lastChangedAt: Instant? = null, + language: String? = null, + sourceSystem: SourceSystem = SourceSystem.GITHUB, + type: ArtifactType = ArtifactType.FILE, + project: UUID = projectId, + ): Artifact { + val artifact = Artifact( + id = id, + sourceSystem = sourceSystem, + sourceId = "github:acme/repo:$type:$id", + sourceUrl = "https://github.com/acme/repo", + artifactType = type, + title = title, + content = "content", + mime = null, + language = language, + createdAtSource = null, + updatedAtSource = null, + ingestedAt = ingestedAt, + lastChangedAt = lastChangedAt, + ingestionRun = run, + hash = null, + ) + artifact.addProjectId(project) + entityManager.persist(artifact) + return artifact + } + + private fun flush() { + entityManager.flush() + entityManager.clear() + } + + private companion object { + val BASE: Instant = Instant.parse("2026-03-10T12:00:00Z") + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt index 388a5327a..790b271e7 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt @@ -2,6 +2,7 @@ package com.sprintstart.sprintstartbackend.ingestion.service import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse @@ -101,23 +102,22 @@ class ArtifactQueryServiceTest { ) every { userApi.userHasAccessToProject(authId, projectId) } returns true every { - artifactRepository.findProjectArtifactsWithCriteria(projectId, criteria, capture(pageable)) + artifactRepository.findProjectArtifactsWithCriteria( + projectId, + criteria, + ArtifactSort.TITLE_ASC, + capture(pageable), + ) } returns PageImpl(listOf(responseItem), PageRequest.of(0, 20), 1) - val result = service.getProjectArtifacts(1, 20, criteria, projectId, authId) + val result = service.getProjectArtifacts(1, 20, criteria, ArtifactSort.TITLE_ASC, projectId, authId) assertThat(result.items).hasSize(1) assertThat(result.items.single().title).isEqualTo("doc.md") - assertThat( - pageable.captured.sort - .getOrderFor("ingestedAt") - ?.isDescending, - ).isTrue() - assertThat( - pageable.captured.sort - .getOrderFor("id") - ?.isAscending, - ).isTrue() + // The repository owns ORDER BY, so the Pageable carries only the page window. + assertThat(pageable.captured.pageNumber).isEqualTo(0) + assertThat(pageable.captured.pageSize).isEqualTo(20) + assertThat(pageable.captured.sort.isUnsorted).isTrue() } @Test From 464f18b63b6928c45e25b1391382ffe48647e9b2 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 10:13:06 +0200 Subject: [PATCH 4/9] feat(knowledge-base): filter artifacts and facets by an import-date window Adds optional `from` / `to` query parameters (ISO yyyy-MM-dd) to GET /api/v1/projects/{projectId}/artifacts and to its /facets sibling, so the Knowledge Base can answer "what was imported last week" without paging through everything. Contract (pinned with the frontend and AI-service work on this branch): - from, to = ISO calendar dates, both optional, both inclusive. - They bound `ingestedAt` (first import), read as UTC calendar days: from -> ingestedAt >= from 00:00Z; to -> ingestedAt < (to + 1) 00:00Z. - from > to -> 400. A malformed date -> 400 (Spring's type conversion). - The facets endpoint applies the exact same predicate (parity). Why the window is on ingestedAt and not coalesce(lastChangedAt, ingestedAt): - The original plan suggested filtering on the "last changed" instant; the contract agreed afterwards pins ingestedAt. It is the column the default ADDED_DESC order uses, so "added from 1 March" and "sorted by added" talk about the same instant. It is also NOT NULL, so no row silently escapes an open-ended window. Why UTC calendar days with a half-open upper bound: - The server has no user time zone to work with, and ingestedAt is an Instant; pinning UTC makes the same URL return the same rows for every caller and on every server, whatever its default zone. - `to` becomes `< start of the next day` rather than `<= 23:59:59.999`, so the last microsecond of that day still matches regardless of the precision the timestamp column keeps (H2 and PostgreSQL both keep microseconds today; the bound does not depend on it). Why the predicate lives in buildPredicates and is never "excluded": - Every list, count and facet query already goes through buildPredicates; no facet counts the import date, so no FacetKind ever skips the window. That single code path is what guarantees facet counts equal the list's totalElements under the same filter -- the parity the contract requires. It sits in its own buildIngestedWindowPredicates helper so the already long buildPredicates stays under detekt's complexity threshold. Why from > to is a 400 and why the check is in ArtifactQueryService: - An inverted window can never match; answering it with an empty page would disguise a client bug (swapped bounds) as "no results". - AGENTS.md puts validation in services, not controllers. One private requireValidDateWindow is called by both getProjectArtifacts and getProjectArtifactFacets, so the two endpoints cannot disagree on what a valid window is. It runs before the access check and the repository, like Spring's own binding errors, which also precede the access check. - from == to is valid and means "that one day". Tests: - ArtifactFacetRepositoryQueryTest (real H2 via @DataJpaTest): boundary instants -- 23:59:59.999999 the day before (out), 00:00:00 on `from` (in), 23:59:59.999999 on `to` (in), 00:00:00 the day after (out); each open bound on its own; and facet parity: types and sources facet counts sum to the list's totalElements under the same window. - ArtifactQueryServiceTest: from > to is a 400 from both list and facets and reaches neither the access check nor the repository; from == to is passed through. - ArtifactControllerTest: from/to bind as ISO dates on list and facets into the criteria; a non-ISO date is a 400 that never reaches the service. Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco) green: 3435 tests, 0 failures (previous commit: 3426). --- .../controller/ArtifactController.kt | 37 ++++++++++- .../model/dto/ArtifactFilterCriteria.kt | 10 ++- .../repository/ArtifactFacetRepositoryImpl.kt | 25 +++++++ .../ingestion/service/ArtifactQueryService.kt | 25 ++++++- .../controller/ArtifactControllerTest.kt | 66 +++++++++++++++++++ .../ArtifactFacetRepositoryQueryTest.kt | 43 ++++++++++++ .../service/ArtifactQueryServiceTest.kt | 43 ++++++++++++ 7 files changed, 245 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index f13b773ca..c3dea658f 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -19,6 +19,7 @@ import io.swagger.v3.oas.annotations.responses.ApiResponses import io.swagger.v3.oas.annotations.tags.Tag import jakarta.validation.constraints.Max import jakarta.validation.constraints.Min +import org.springframework.format.annotation.DateTimeFormat import org.springframework.http.HttpHeaders import org.springframework.http.HttpStatus import org.springframework.http.MediaType @@ -33,11 +34,17 @@ import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.RestController import java.net.URI +import java.time.LocalDate import java.util.UUID private const val DEFAULT_PAGE = "1" private const val DEFAULT_SIZE = "20" private const val MAX_PAGE_SIZE = 100L +private const val FROM_DESCRIPTION = + "First import day to include, ISO yyyy-MM-dd, read as a UTC calendar day (inclusive). " + + "Must not be after `to`, else 400." +private const val TO_DESCRIPTION = + "Last import day to include, ISO yyyy-MM-dd, read as a UTC calendar day (inclusive)." /** * Read-only HTTP entry point for opening one artifact. @@ -102,7 +109,10 @@ class ArtifactController( @ApiResponses( value = [ ApiResponse(responseCode = "200", description = "Project artifact page returned successfully"), - ApiResponse(responseCode = "400", description = "Invalid query or pagination parameters"), + ApiResponse( + responseCode = "400", + description = "Invalid query or pagination parameters, unknown sort, malformed date, or from after to", + ), ApiResponse(responseCode = "403", description = "Caller has no access to the project"), ], ) @@ -132,6 +142,14 @@ class ArtifactController( ) @RequestParam(defaultValue = "ADDED_DESC") sort: ArtifactSort, + @Parameter(description = FROM_DESCRIPTION) + @RequestParam(required = false) + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) + from: LocalDate?, + @Parameter(description = TO_DESCRIPTION) + @RequestParam(required = false) + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) + to: LocalDate?, @Parameter( description = "UUID of the project whose artifacts should be returned", ) @PathVariable projectId: UUID, @@ -144,6 +162,8 @@ class ArtifactController( sources = sources, repositories = repositories, format = format, + from = from, + to = to, ) return ResponseEntity.ok( artifactQueryService.getProjectArtifacts(page, size, criteria, sort, projectId, jwt.subject), @@ -159,7 +179,10 @@ class ArtifactController( @ApiResponses( value = [ ApiResponse(responseCode = "200", description = "Facet counts returned successfully"), - ApiResponse(responseCode = "400", description = "Invalid facet query parameters"), + ApiResponse( + responseCode = "400", + description = "Invalid facet query parameters, malformed date, or from after to", + ), ApiResponse(responseCode = "403", description = "Caller has no access to the project"), ], ) @@ -169,6 +192,14 @@ class ArtifactController( @RequestParam(required = false) sources: Set?, @RequestParam(required = false) repositories: Set?, @RequestParam(required = false) format: UploadFormat?, + @Parameter(description = FROM_DESCRIPTION) + @RequestParam(required = false) + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) + from: LocalDate?, + @Parameter(description = TO_DESCRIPTION) + @RequestParam(required = false) + @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) + to: LocalDate?, @Parameter( description = "UUID of the project whose artifact facets should be calculated", ) @PathVariable projectId: UUID, @@ -180,6 +211,8 @@ class ArtifactController( sources = sources, repositories = repositories, format = format, + from = from, + to = to, ) return ResponseEntity.ok( artifactQueryService.getProjectArtifactFacets(projectId, criteria, jwt.subject), diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt index 1d32e86af..4a3d4deca 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt @@ -2,6 +2,7 @@ package com.sprintstart.sprintstartbackend.ingestion.model.dto import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import java.time.LocalDate /** * File formats recognized for UPLOAD-sourced artifacts. @@ -17,7 +18,12 @@ enum class UploadFormat { * Filter criteria for project-scoped artifact searches and facet calculations. * * Encapsulates full-text search, type filtering, source filtering, repository selection, - * and upload format selection. + * upload format selection, and an import-date window. + * + * @property from First day (inclusive) of the import window, read as a UTC calendar day: rows with + * `ingestedAt >= from 00:00Z` match. Null leaves the window open at the start. + * @property to Last day (inclusive) of the import window, read as a UTC calendar day: rows with + * `ingestedAt < (to + 1 day) 00:00Z` match. Null leaves the window open at the end. */ data class ArtifactFilterCriteria( val search: String? = null, @@ -25,4 +31,6 @@ data class ArtifactFilterCriteria( val sources: Set? = null, val repositories: Set? = null, val format: UploadFormat? = null, + val from: LocalDate? = null, + val to: LocalDate? = null, ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt index cc817d2bb..902e3eb5a 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -24,6 +24,8 @@ import org.springframework.data.domain.Pageable import org.springframework.stereotype.Repository import org.springframework.transaction.annotation.Transactional import java.time.Instant +import java.time.LocalDate +import java.time.ZoneOffset import java.util.UUID private enum class FacetKind { @@ -352,9 +354,32 @@ class ArtifactFacetRepositoryImpl( predicates.add(cb.or(notGithub, githubMatchesRepo)) } + // No facet counts the import date, so the window applies to every query alike -- which is + // what keeps facet counts equal to the list's totalElements under the same filter. + predicates.addAll(buildIngestedWindowPredicates(cb, root, criteria.from, criteria.to)) + return predicates } + /** + * Restricts `ingestedAt` to the inclusive UTC calendar-day window `[from, to]`. + * + * The end bound is `< start of the day after [to]` rather than `<= end of [to]`, so an import + * in the last microsecond of that day still matches, whatever precision the column keeps. + */ + private fun buildIngestedWindowPredicates( + cb: CriteriaBuilder, + root: Root, + from: LocalDate?, + to: LocalDate?, + ): List { + val ingestedAt = root.get("ingestedAt") + return listOfNotNull( + from?.let { cb.greaterThanOrEqualTo(ingestedAt, it.atStartOfDay(ZoneOffset.UTC).toInstant()) }, + to?.let { cb.lessThan(ingestedAt, it.plusDays(1).atStartOfDay(ZoneOffset.UTC).toInstant()) }, + ) + } + private fun buildUploadFormatPredicate( cb: CriteriaBuilder, root: Root, diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt index 267248f84..ba3e25cc8 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt @@ -82,7 +82,8 @@ class ArtifactQueryService( * @param projectId The SprintStart project that scopes the artifact listing. * @param authId The authenticated caller subject from the JWT. * @return One project-scoped artifact page together with pagination metadata. - * @throws ResponseStatusException `403` when the caller has no access to the project. + * @throws ResponseStatusException `400` when `from` is after `to`, `403` when the caller has no + * access to the project. * @throws IllegalArgumentException when Spring Data rejects the requested page or page size. */ @Transactional(readOnly = true) @@ -95,6 +96,7 @@ class ArtifactQueryService( projectId: UUID, authId: String, ): ArtifactPageResponse { + requireValidDateWindow(criteria) ensureAccessToProject(authId, projectId) // Unsorted on purpose: the criteria repository owns ORDER BY (see ArtifactSort) and // would ignore a Pageable sort, so passing one here would only suggest otherwise. @@ -122,6 +124,8 @@ class ArtifactQueryService( * @param criteria Active filter criteria. * @param authId The authenticated caller subject from the JWT. * @return Aggregated facet counts. + * @throws ResponseStatusException `400` when `from` is after `to`, `403` when the caller has no + * access to the project. */ @Transactional(readOnly = true) @Tracked("Retrieving artifact facets for project") @@ -130,6 +134,7 @@ class ArtifactQueryService( criteria: ArtifactFilterCriteria, authId: String, ): ArtifactFacetsResponse { + requireValidDateWindow(criteria) ensureAccessToProject(authId, projectId) return artifactRepository.findFacets(projectId, criteria) } @@ -161,6 +166,24 @@ class ArtifactQueryService( ) } + /** + * Rejects an import-date window whose start lies after its end. + * + * Such a window can match nothing, so answering it with an empty page would hide a client bug + * (typically swapped bounds) behind a plausible "no results". List and facets both call this, + * so the two endpoints can never disagree on whether a window is valid. + * + * @param criteria The filter whose `from`/`to` bounds are checked; open bounds always pass. + * @throws ResponseStatusException `400` when `from` is after `to`. + */ + private fun requireValidDateWindow(criteria: ArtifactFilterCriteria) { + val from = criteria.from ?: return + val to = criteria.to ?: return + if (from.isAfter(to)) { + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "`from` ($from) must not be after `to` ($to)") + } + } + /** * Verifies that the authenticated caller may read artifacts for the requested project. * diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index eff22cbb7..7e988ff2c 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -32,6 +32,7 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPat import org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status import java.time.Instant +import java.time.LocalDate import java.util.UUID @WebMvcTest(controllers = [ArtifactController::class]) @@ -217,6 +218,64 @@ class ArtifactControllerTest( } } + @Test + fun `getProjectArtifacts binds from and to as ISO calendar days`() { + val projectId = UUID.randomUUID() + val criteria = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 1), to = LocalDate.of(2026, 3, 31)) + every { + artifactQueryService.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, projectId, "auth-user") + } returns response() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("from", "2026-03-01") + .param("to", "2026-03-31") + .with(userJwt()), + ).andExpect(status().isOk) + + verify(exactly = 1) { + artifactQueryService.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, projectId, "auth-user") + } + } + + @Test + fun `getProjectArtifacts rejects a malformed date with 400`() { + val projectId = UUID.randomUUID() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("from", "01.03.2026") + .with(userJwt()), + ).andExpect(status().isBadRequest) + + verify(exactly = 0) { + artifactQueryService.getProjectArtifacts(any(), any(), any(), any(), any(), any()) + } + } + + @Test + fun `getProjectArtifactFacets binds the same date window as the list`() { + val projectId = UUID.randomUUID() + val criteria = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 1), to = LocalDate.of(2026, 3, 1)) + every { + artifactQueryService.getProjectArtifactFacets(projectId, criteria, "auth-user") + } returns facets() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts/facets") + .param("from", "2026-03-01") + .param("to", "2026-03-01") + .with(userJwt()), + ).andExpect(status().isOk) + + verify(exactly = 1) { + artifactQueryService.getProjectArtifactFacets(projectId, criteria, "auth-user") + } + } + @Test fun `getProjectArtifactFacets returns facet counts and never binds to single artifact route`() { val projectId = UUID.randomUUID() @@ -330,4 +389,11 @@ class ArtifactControllerTest( private fun userJwt() = jwt() .jwt { it.subject("auth-user") } .authorities(SimpleGrantedAuthority("ROLE_USER")) + + private fun facets() = ArtifactFacetsResponse( + types = emptyList(), + sources = emptyList(), + formats = emptyList(), + repositories = emptyList(), + ) } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt index f7e313755..00ce32116 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt @@ -18,6 +18,7 @@ import org.springframework.context.annotation.Import import org.springframework.data.domain.PageRequest import org.springframework.test.context.ActiveProfiles import java.time.Instant +import java.time.LocalDate import java.util.UUID /** @@ -122,6 +123,48 @@ class ArtifactFacetRepositoryQueryTest { } } + // ========================== date window ========================== + + @Test + fun `the date window includes both boundary days in full and nothing beyond`() { + val justBefore = store(ingestedAt = Instant.parse("2026-03-09T23:59:59.999999Z")) + val firstInstant = store(ingestedAt = Instant.parse("2026-03-10T00:00:00Z")) + val lastInstant = store(ingestedAt = Instant.parse("2026-03-12T23:59:59.999999Z")) + val justAfter = store(ingestedAt = Instant.parse("2026-03-13T00:00:00Z")) + flush() + + val window = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 10), to = LocalDate.of(2026, 3, 12)) + + assertThat(listIds(window)).containsExactlyInAnyOrder(firstInstant.id, lastInstant.id) + assertThat(listIds(window)).doesNotContain(justBefore.id, justAfter.id) + } + + @Test + fun `an open bound leaves that side of the window unrestricted`() { + val early = store(ingestedAt = Instant.parse("2020-01-01T00:00:00Z")) + val onDay = store(ingestedAt = Instant.parse("2026-03-10T08:00:00Z")) + val late = store(ingestedAt = Instant.parse("2030-01-01T00:00:00Z")) + flush() + val day = LocalDate.of(2026, 3, 10) + + assertThat(listIds(ArtifactFilterCriteria(from = day))).containsExactlyInAnyOrder(onDay.id, late.id) + assertThat(listIds(ArtifactFilterCriteria(to = day))).containsExactlyInAnyOrder(early.id, onDay.id) + } + + @Test + fun `facets count under the same date window as the list`() { + store(ingestedAt = Instant.parse("2026-03-10T08:00:00Z")) + store(ingestedAt = Instant.parse("2026-03-10T09:00:00Z"), type = ArtifactType.ISSUE) + store(ingestedAt = Instant.parse("2026-03-11T08:00:00Z")) + flush() + val window = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 10), to = LocalDate.of(2026, 3, 10)) + + val facets = repository.findFacets(projectId, window) + + assertThat(facets.types.sumOf { it.count }).isEqualTo(list(window).totalElements).isEqualTo(2) + assertThat(facets.sources.sumOf { it.count }).isEqualTo(list(window).totalElements) + } + // ========================== helpers ========================== private fun list( diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt index 790b271e7..83ed1d74b 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt @@ -19,10 +19,14 @@ import io.mockk.slot import io.mockk.verify import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.springframework.data.domain.PageImpl import org.springframework.data.domain.PageRequest import org.springframework.data.domain.Pageable +import org.springframework.http.HttpStatus +import org.springframework.web.server.ResponseStatusException import java.time.Instant +import java.time.LocalDate import java.util.UUID class ArtifactQueryServiceTest { @@ -140,6 +144,31 @@ class ArtifactQueryServiceTest { assertThat(result.repositories.single().value).isEqualTo("owner/repo") } + @Test + fun `getProjectArtifacts rejects from after to with 400 before any lookup`() { + val criteria = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 2), to = LocalDate.of(2026, 3, 1)) + + val error = assertThrows { + service.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, UUID.randomUUID(), "auth-1") + } + + assertThat(error.statusCode).isEqualTo(HttpStatus.BAD_REQUEST) + verify(exactly = 0) { userApi.userHasAccessToProject(any(), any()) } + verify(exactly = 0) { artifactRepository.findProjectArtifactsWithCriteria(any(), any(), any(), any()) } + } + + @Test + fun `getProjectArtifactFacets rejects from after to with 400 like the list`() { + val criteria = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 2), to = LocalDate.of(2026, 3, 1)) + + val error = assertThrows { + service.getProjectArtifactFacets(UUID.randomUUID(), criteria, "auth-1") + } + + assertThat(error.statusCode).isEqualTo(HttpStatus.BAD_REQUEST) + verify(exactly = 0) { artifactRepository.findFacets(any(), any()) } + } + @Test fun `getArtifact returns the projected artifact when found`() { val projectId = UUID.randomUUID() @@ -166,6 +195,20 @@ class ArtifactQueryServiceTest { assertThat(result.title).isEqualTo("README.md") } + @Test + fun `a one-day window where from equals to is accepted`() { + val projectId = UUID.randomUUID() + val day = LocalDate.of(2026, 3, 1) + val criteria = ArtifactFilterCriteria(from = day, to = day) + every { userApi.userHasAccessToProject("auth-1", projectId) } returns true + every { artifactRepository.findFacets(projectId, criteria) } returns + ArtifactFacetsResponse(emptyList(), emptyList(), emptyList(), emptyList()) + + service.getProjectArtifactFacets(projectId, criteria, "auth-1") + + verify(exactly = 1) { artifactRepository.findFacets(projectId, criteria) } + } + @Test fun `getArtifact throws 404 when not found in project`() { val projectId = UUID.randomUUID() From dde464e4357e01d4e322b936e5180acc3fd3417f Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 10:27:34 +0200 Subject: [PATCH 5/9] feat(knowledge-base): filter artifacts by language and facet the languages Artifacts already carry a `language` column, filled at ingestion by FileMetaDataResolver's extension map ("kt" -> "Kotlin", "md" -> "Markdown", "txt" -> "Plain Text", ...), but the API neither exposed it nor let users narrow by it. This commit adds, per the pinned contract: - `languages` query parameter (repeatable, case-insensitive) on GET /api/v1/projects/{projectId}/artifacts and on /artifacts/facets. - `language: String?` on every ArtifactResponse (list and detail). - `languages: [{value, count}]` on ArtifactFacetsResponse. Why the match is lower(language) IN (lowercased selection): - The stored value is a display name ("Kotlin", "YAML"), and a client may send it from a chip, a URL someone typed, or an older build. Case must not decide whether "kotlin" finds Kotlin files. Folding both sides with lower() keeps the predicate a plain IN list, which every database plans well, instead of per-row equalsIgnoreCase logic. - Selected values are trimmed, blanks dropped and deduplicated ignoring case before use (selectedLanguages()), so `languages=` or `languages=Kotlin&languages=kotlin` behave like the obvious intent. Why the language filter narrows every source (unlike format/repository): - The existing format and repository filters are written as "not UPLOAD or matches format" / "not GITHUB or matches repo", because they are sub-filters of one source. Language is a property any artifact may have, so it is a plain AND like `types`: while it is set, artifacts without a language (issues, PRs, pages) drop out. That is what a user choosing "Kotlin" expects, and it is stated in the KDoc and the Swagger parameter description. Why the language facet counts the way it does: - Contract part 2 says: copy whatever own-dimension rule the existing facets use. In buildPredicates each facet drops only its own filter (types drops types; sources additionally drops format/repository because those are sub-filters of a source). Language is an independent dimension like types, so the language facet passes FacetKind.LANGUAGES and drops only the language filter; every other facet keeps applying it. Result: each chip shows how many rows the list would have if that language were (also) picked. - Grouping is by lower(language), the same folding as the filter, so a stray "kotlin" next to "Kotlin" is one group whose count equals the list total for that filter (list/facet parity). least(language) picks one stored spelling to display; which one is collation-dependent, and the test deliberately only checks it case-insensitively. - Null languages are excluded (isNotNull), and "Markdown"/"Plain Text" are hidden because they describe documents, which the format facet already covers; offering both would duplicate the same choice. Exception: if the client did select one of them, it is still shown with its real count so a selected chip never vanishes. - A selected language with no match is returned with count 0, the same way the types and repositories facets echo their selection, so the UI can render the active chip and let the user remove it. - Order is count descending then value ascending, matching the other facets and the contract, and it is deterministic across databases. Why the shaping lives in a top-level internal languageFacetOptions(): - Hiding documents, echoing selections and ordering are pure list operations. Keeping them out of the JPA query makes them unit-testable without a database and keeps the SQL a simple GROUP BY; the class already carries @Suppress("TooManyFunctions") and this does not add to it. Why `language` is appended last to the ArtifactResponse projection: - The list query builds DTOs with cb.construct(), which binds by constructor position. Appending the new nullable, defaulted property at the end keeps every existing positional call and named call site valid and makes the projection change a one-line addition. Tests: - ArtifactFacetRepositoryQueryTest (real H2 via @DataJpaTest): filter ignores case and drops null-language rows; the language facet drops its own filter but applies types; documents and nulls are hidden and an unmatched selection comes back at 0; "Kotlin"+"kotlin" is one group whose count equals the list totalElements for that filter. - ArtifactFacetRepositoryImplTest: a selected document language is kept with its real count. - ArtifactControllerTest: repeated `languages` bind into the criteria on list and facets; `language` and `languages` appear in the JSON. - Existing facet fixtures gained the new required `languages` list. Gate: ./gradlew build (compile, detekt, ktlint, full test suite, jacoco) green: 3441 tests, 0 failures (previous commit: 3435). --- .../controller/ArtifactController.kt | 11 +++ .../model/dto/ArtifactFilterCriteria.kt | 3 + .../dto/response/ArtifactFacetsResponse.kt | 7 +- .../model/dto/response/ArtifactResponse.kt | 6 ++ .../ingestion/model/mapper/ArtifactMapper.kt | 1 + .../repository/ArtifactFacetRepository.kt | 2 +- .../repository/ArtifactFacetRepositoryImpl.kt | 67 ++++++++++++++++++ .../controller/ArtifactControllerTest.kt | 28 +++++++- .../ArtifactFacetRepositoryImplTest.kt | 13 ++++ .../ArtifactFacetRepositoryQueryTest.kt | 69 +++++++++++++++++++ .../service/ArtifactQueryServiceTest.kt | 3 +- 11 files changed, 205 insertions(+), 5 deletions(-) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index c3dea658f..e98f91883 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -45,6 +45,9 @@ private const val FROM_DESCRIPTION = "Must not be after `to`, else 400." private const val TO_DESCRIPTION = "Last import day to include, ISO yyyy-MM-dd, read as a UTC calendar day (inclusive)." +private const val LANGUAGES_DESCRIPTION = + "Language display names to keep (repeatable, case-insensitive), e.g. Kotlin. " + + "Artifacts without a language are excluded while set." /** * Read-only HTTP entry point for opening one artifact. @@ -150,6 +153,9 @@ class ArtifactController( @RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) to: LocalDate?, + @Parameter(description = LANGUAGES_DESCRIPTION) + @RequestParam(required = false) + languages: Set?, @Parameter( description = "UUID of the project whose artifacts should be returned", ) @PathVariable projectId: UUID, @@ -164,6 +170,7 @@ class ArtifactController( format = format, from = from, to = to, + languages = languages, ) return ResponseEntity.ok( artifactQueryService.getProjectArtifacts(page, size, criteria, sort, projectId, jwt.subject), @@ -200,6 +207,9 @@ class ArtifactController( @RequestParam(required = false) @DateTimeFormat(iso = DateTimeFormat.ISO.DATE) to: LocalDate?, + @Parameter(description = LANGUAGES_DESCRIPTION) + @RequestParam(required = false) + languages: Set?, @Parameter( description = "UUID of the project whose artifact facets should be calculated", ) @PathVariable projectId: UUID, @@ -213,6 +223,7 @@ class ArtifactController( format = format, from = from, to = to, + languages = languages, ) return ResponseEntity.ok( artifactQueryService.getProjectArtifactFacets(projectId, criteria, jwt.subject), diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt index 4a3d4deca..0171d3b75 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt @@ -24,6 +24,8 @@ enum class UploadFormat { * `ingestedAt >= from 00:00Z` match. Null leaves the window open at the start. * @property to Last day (inclusive) of the import window, read as a UTC calendar day: rows with * `ingestedAt < (to + 1 day) 00:00Z` match. Null leaves the window open at the end. + * @property languages Language display names to keep, matched case-insensitively against the + * stored name. Narrows every source: artifacts without a language drop out while it is set. */ data class ArtifactFilterCriteria( val search: String? = null, @@ -33,4 +35,5 @@ data class ArtifactFilterCriteria( val format: UploadFormat? = null, val from: LocalDate? = null, val to: LocalDate? = null, + val languages: Set? = null, ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt index 265067ecc..a4f5e5da5 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactFacetsResponse.kt @@ -9,11 +9,16 @@ data class FacetCountResponse( ) /** - * Aggregated facet counts for artifact types, sources, upload formats, and repositories. + * Aggregated facet counts for artifact types, sources, upload formats, repositories, and languages. + * + * @property languages Language display names ("Kotlin", "YAML", ...), count descending then value + * ascending. Null, "Markdown" and "Plain Text" are left out; the format facet covers documents. + * Selected languages always appear, with count 0 when nothing matches. */ data class ArtifactFacetsResponse( val types: List, val sources: List, val formats: List, val repositories: List, + val languages: List, ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactResponse.kt index 2896fb1bd..58972cfd9 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactResponse.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactResponse.kt @@ -20,4 +20,10 @@ data class ArtifactResponse( val lastChangedAt: Instant?, val metadata: String, val sourceVersion: String? = null, + /** + * Display name of the artifact's programming or document language (for example "Kotlin", + * "Markdown", "Plain Text"), derived from the file extension at ingestion; null when it has no + * file extension to go by, as with issues, pull requests and pages. + */ + val language: String? = null, ) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/mapper/ArtifactMapper.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/mapper/ArtifactMapper.kt index aa5859e2c..9fb82f33c 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/mapper/ArtifactMapper.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/mapper/ArtifactMapper.kt @@ -18,6 +18,7 @@ class ArtifactMapper { ingestedAt = artifact.ingestedAt, lastChangedAt = artifact.lastChangedAt, sourceVersion = artifact.sourceVersion, + language = artifact.language, ) } } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt index b1cb52cb7..5615969b1 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepository.kt @@ -47,7 +47,7 @@ interface ArtifactFacetRepository { ): ArtifactResponse? /** - * Calculates aggregated counts for types, sources, upload formats, and repositories + * Calculates aggregated counts for types, sources, upload formats, repositories, and languages * using the "count each would add" model (own-facet-excluded, other-facets-applied). * * @param projectId Scopes facet calculations to the target project. diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt index 902e3eb5a..9fb159134 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -33,10 +33,43 @@ private enum class FacetKind { SOURCES, FORMATS, REPOSITORIES, + LANGUAGES, } private val SOURCES_EXCLUDED_FACETS = setOf(FacetKind.SOURCES, FacetKind.FORMATS, FacetKind.REPOSITORIES) +/** Lower-cased language names the facet never offers: the format facet covers documents. */ +private val DOCUMENT_LANGUAGES = setOf("markdown", "plain text") + +/** Selected languages, trimmed, blanks dropped, deduplicated ignoring case. */ +private fun ArtifactFilterCriteria.selectedLanguages(): List = + languages + .orEmpty() + .map { it.trim() } + .filter { it.isNotEmpty() } + .distinctBy { it.lowercase() } + +/** + * Shapes counted `(display name, count)` language groups into facet options. + * + * Document languages are dropped unless selected; a selected language with no match is added at + * count 0, so its chip stays visible. Ordered by count descending, then value ascending. + */ +internal fun languageFacetOptions( + counted: List>, + selected: List, +): List { + val selectedLower = selected.map { it.lowercase() }.toSet() + val countedLower = counted.map { it.first.lowercase() }.toSet() + val shown = counted.filter { (value, _) -> + value.lowercase() !in DOCUMENT_LANGUAGES || value.lowercase() in selectedLower + } + val unmatched = selected.filter { it.lowercase() !in countedLower }.map { it to 0L } + return (shown + unmatched) + .map { (value, count) -> FacetCountResponse(value, count) } + .sortedWith(compareByDescending { it.count }.thenBy { it.value }) +} + @Repository @Transactional(readOnly = true) // One function per facet dimension plus the shared predicate/order builders; splitting them would @@ -151,6 +184,7 @@ class ArtifactFacetRepositoryImpl( root.get("lastChangedAt"), root.get("metadata"), root.get("sourceVersion"), + root.get("language"), ) override fun findFacets( @@ -163,6 +197,7 @@ class ArtifactFacetRepositoryImpl( sources = computeSourceFacets(cb, projectId, criteria), formats = computeFormatFacets(cb, projectId, criteria), repositories = computeRepositoryFacets(cb, projectId, criteria), + languages = computeLanguageFacets(cb, projectId, criteria), ) } @@ -315,6 +350,32 @@ class ArtifactFacetRepositoryImpl( .map { (repo, count) -> FacetCountResponse(repo, count) } } + /** + * Counts artifacts per language under every active filter except the language one. + * + * Groups by lower(language) so the counts use the same case folding as the filter predicate; + * min(language) then picks one stored spelling to display for the group. + */ + private fun computeLanguageFacets( + cb: CriteriaBuilder, + projectId: UUID, + criteria: ArtifactFilterCriteria, + ): List { + val query = cb.createQuery(Array::class.java) + val root = query.from(Artifact::class.java) + val join = root.join("projectIdsInternal") + val language = root.get("language") + query.multiselect(cb.least(language), cb.countDistinct(root.get("id"))) + val preds = buildPredicates(cb, root, join, projectId, criteria, FacetKind.LANGUAGES) + query.where(*(preds + cb.isNotNull(language)).toTypedArray()) + query.groupBy(cb.lower(language)) + + val counted = entityManager.createQuery(query).resultList.map { row -> + (row[0] as String) to (row[1] as Number).toLong() + } + return languageFacetOptions(counted, criteria.selectedLanguages()) + } + private fun buildPredicates( cb: CriteriaBuilder, root: Root, @@ -354,6 +415,12 @@ class ArtifactFacetRepositoryImpl( predicates.add(cb.or(notGithub, githubMatchesRepo)) } + val languages = criteria.selectedLanguages() + if (exclude != FacetKind.LANGUAGES && languages.isNotEmpty()) { + val languageLower = cb.lower(root.get("language")) + predicates.add(languageLower.`in`(languages.map { it.lowercase() })) + } + // No facet counts the import date, so the window applies to every query alike -- which is // what keeps facet counts equal to the list's totalElements under the same filter. predicates.addAll(buildIngestedWindowPredicates(cb, root, criteria.from, criteria.to)) diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index 7e988ff2c..1f2f88e4c 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -239,6 +239,23 @@ class ArtifactControllerTest( } } + @Test + fun `getProjectArtifacts binds repeated languages and returns each language`() { + val projectId = UUID.randomUUID() + val criteria = ArtifactFilterCriteria(languages = setOf("Kotlin", "yaml")) + every { + artifactQueryService.getProjectArtifacts(1, 20, criteria, ArtifactSort.ADDED_DESC, projectId, "auth-user") + } returns response() + + mockMvc + .perform( + get("/api/v1/projects/$projectId/artifacts") + .param("languages", "Kotlin", "yaml") + .with(userJwt()), + ).andExpect(status().isOk) + .andExpect(jsonPath("$.items[0].language").value("Markdown")) + } + @Test fun `getProjectArtifacts rejects a malformed date with 400`() { val projectId = UUID.randomUUID() @@ -256,9 +273,10 @@ class ArtifactControllerTest( } @Test - fun `getProjectArtifactFacets binds the same date window as the list`() { + fun `getProjectArtifactFacets binds the same date window and languages as the list`() { val projectId = UUID.randomUUID() - val criteria = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 1), to = LocalDate.of(2026, 3, 1)) + val day = LocalDate.of(2026, 3, 1) + val criteria = ArtifactFilterCriteria(from = day, to = day, languages = setOf("Kotlin")) every { artifactQueryService.getProjectArtifactFacets(projectId, criteria, "auth-user") } returns facets() @@ -268,6 +286,7 @@ class ArtifactControllerTest( get("/api/v1/projects/$projectId/artifacts/facets") .param("from", "2026-03-01") .param("to", "2026-03-01") + .param("languages", "Kotlin") .with(userJwt()), ).andExpect(status().isOk) @@ -284,6 +303,7 @@ class ArtifactControllerTest( sources = listOf(FacetCountResponse("GITHUB", 10)), formats = listOf(FacetCountResponse("PDF", 2)), repositories = listOf(FacetCountResponse("owner/repo", 8)), + languages = listOf(FacetCountResponse("Kotlin", 6)), ) every { artifactQueryService.getProjectArtifactFacets(projectId, any(), "auth-user") @@ -303,6 +323,8 @@ class ArtifactControllerTest( .andExpect(jsonPath("$.sources[0].value").value("GITHUB")) .andExpect(jsonPath("$.formats[0].value").value("PDF")) .andExpect(jsonPath("$.repositories[0].value").value("owner/repo")) + .andExpect(jsonPath("$.languages[0].value").value("Kotlin")) + .andExpect(jsonPath("$.languages[0].count").value(6)) verify(exactly = 1) { artifactQueryService.getProjectArtifactFacets(projectId, any(), "auth-user") @@ -374,6 +396,7 @@ class ArtifactControllerTest( ingestedAt = Instant.parse("2026-01-02T03:04:05Z"), lastChangedAt = Instant.parse("2026-01-09T03:04:05Z"), metadata = """{"repositoryFullName":"owner/repo"}""", + language = "Markdown", ), ), page = PageMetadata( @@ -395,5 +418,6 @@ class ArtifactControllerTest( sources = emptyList(), formats = emptyList(), repositories = emptyList(), + languages = emptyList(), ) } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt index d6cf8e350..7460a0ccd 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImplTest.kt @@ -1,6 +1,7 @@ package com.sprintstart.sprintstartbackend.ingestion.repository import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test @@ -111,4 +112,16 @@ class ArtifactFacetRepositoryImplTest { ) assertThat(other).isEqualTo(UploadFormat.OTHER) } + + @Test + fun `languageFacetOptions keeps a selected document language with its real count`() { + val counted = listOf("Markdown" to 3L, "Kotlin" to 3L, "Plain Text" to 9L) + + val options = languageFacetOptions(counted, selected = listOf("markdown")) + + assertThat(options).containsExactly( + FacetCountResponse("Kotlin", 3), + FacetCountResponse("Markdown", 3), + ) + } } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt index 00ce32116..cf80f22ee 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt @@ -3,6 +3,7 @@ package com.sprintstart.sprintstartbackend.ingestion.repository import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRun @@ -165,6 +166,74 @@ class ArtifactFacetRepositoryQueryTest { assertThat(facets.sources.sumOf { it.count }).isEqualTo(list(window).totalElements) } + // ========================== languages ========================== + + @Test + fun `the language filter ignores case and drops artifacts without a language`() { + val kotlin = store(language = "Kotlin") + store(language = "YAML") + store(language = null, type = ArtifactType.ISSUE) + flush() + + val found = list(ArtifactFilterCriteria(languages = setOf("KOTLIN"))) + + assertThat(found.content.map { it.id }).containsExactly(kotlin.id) + assertThat(found.content.single().language).isEqualTo("Kotlin") + } + + @Test + fun `the language facet skips its own filter but applies the others`() { + store(language = "Kotlin") + store(language = "Kotlin") + store(language = "YAML") + store(language = "Shell", type = ArtifactType.PULL_REQUEST) + flush() + val criteria = ArtifactFilterCriteria(types = setOf(ArtifactType.FILE), languages = setOf("YAML")) + + val languages = repository.findFacets(projectId, criteria).languages + + // Kotlin still counts although only YAML is selected; Shell is outside the FILE type. + assertThat(languages).containsExactly( + FacetCountResponse("Kotlin", 2), + FacetCountResponse("YAML", 1), + ) + } + + @Test + fun `the language facet hides documents and keeps a selected language without matches`() { + store(language = "Kotlin") + store(language = "Markdown") + store(language = "Plain Text") + store(language = null, type = ArtifactType.ISSUE) + flush() + + val languages = repository + .findFacets(projectId, ArtifactFilterCriteria(languages = setOf("Rust"))) + .languages + + assertThat(languages).containsExactly( + FacetCountResponse("Kotlin", 1), + FacetCountResponse("Rust", 0), + ) + } + + @Test + fun `spellings differing only in case count as one language, matching the list total`() { + store(language = "Kotlin") + store(language = "kotlin") + store(language = "YAML") + flush() + val criteria = ArtifactFilterCriteria(languages = setOf("KOTLIN")) + + val kotlin = repository + .findFacets(projectId, criteria) + .languages + .filter { it.value.equals("kotlin", ignoreCase = true) } + + assertThat(kotlin).hasSize(1) + assertThat(kotlin.single().count).isEqualTo(list(criteria).totalElements).isEqualTo(2) + } + // ========================== helpers ========================== private fun list( diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt index 83ed1d74b..f80683a57 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryServiceTest.kt @@ -134,6 +134,7 @@ class ArtifactQueryServiceTest { sources = listOf(FacetCountResponse("GITHUB", 5)), formats = emptyList(), repositories = listOf(FacetCountResponse("owner/repo", 5)), + languages = emptyList(), ) every { userApi.userHasAccessToProject(authId, projectId) } returns true every { artifactRepository.findFacets(projectId, criteria) } returns facets @@ -202,7 +203,7 @@ class ArtifactQueryServiceTest { val criteria = ArtifactFilterCriteria(from = day, to = day) every { userApi.userHasAccessToProject("auth-1", projectId) } returns true every { artifactRepository.findFacets(projectId, criteria) } returns - ArtifactFacetsResponse(emptyList(), emptyList(), emptyList(), emptyList()) + ArtifactFacetsResponse(emptyList(), emptyList(), emptyList(), emptyList(), emptyList()) service.getProjectArtifactFacets(projectId, criteria, "auth-1") From 455f1e212148fb545e3ed1e3a84e550cae88cfb7 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 11:04:03 +0200 Subject: [PATCH 6/9] feat(knowledge-base): report per-id outcomes for a bulk artifact delete DELETE /api/v1/uploads used to answer 204 whatever happened: ids that were missing, belonged to another project or failed in storage were swallowed, so the Knowledge Base could only assume every row was gone and silently drifted from the server. It now answers 200 with { "deletedIds": [uuid], "failed": [{ "artifactId": uuid, "error": str }] } Contract (pinned with the frontend work on this branch): - deletedIds keeps request order, so the client can drop exactly those rows and leave the rest selected. - Every requested id lands in exactly one of the two lists; a partial failure never fails the whole request (still 200). - The frontend also accepts an empty 204 from an older backend and then treats every requested id as deleted, so rollout order is free. Why the client-facing error is never the raw exception message: - Storage exceptions can carry file-system paths, bucket names or driver internals. Those belong in operator logs, not in a JSON body any PM can read. Not-found keeps "Artifact with id not found." (it only echoes the caller's own id); every storage failure reports the fixed "Artifact could not be deleted.". - The raw message is not lost: it stays in the UploadBatchDeletionFinishedEvent outcome, exactly as before, and is written to a warn log with the artifact id, so ingestion bookkeeping and debugging see the real cause. - The response failures are therefore collected in their own list next to the event outcomes instead of being mapped from them afterwards; mapping would have leaked the raw text or needed a second lookup. Why the event and the per-item behaviour are otherwise unchanged: - Listeners of UploadStartedEvent / UploadBatchDeletionFinishedEvent (the upload ingestion-run lifecycle) keep receiving the same payload, raw error text included, so this is purely an HTTP-surface change. - Foreign-project ids are still reported as "not found" rather than "forbidden", so the endpoint does not confirm that an id exists in a project the caller cannot see. Tests: - UploadServiceTest: deleted ids in request order, not-found reason, and a storage failure that carries a path-like message: the response shows only the generic reason while the event outcome keeps the raw message. - UploadControllerTest: 200 with the deletedIds/failed body for PM and admin; the existing role / validation guards are unchanged. Gate: ./gradlew build green (3442 tests, 0 failures; detekt 0 issues; ktlint clean). --- .../upload/controller/UploadController.kt | 26 +++++----- .../dto/response/DeleteUploadsResponse.kt | 27 +++++++++++ .../upload/service/UploadService.kt | 30 +++++++++++- .../upload/controller/UploadControllerTest.kt | 23 ++++++--- .../upload/service/UploadServiceTest.kt | 48 +++++++++++++++++-- 5 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/upload/model/dto/response/DeleteUploadsResponse.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadController.kt index 8442e1cfb..e811e9168 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadController.kt @@ -2,6 +2,7 @@ package com.sprintstart.sprintstartbackend.upload.controller import com.sprintstart.sprintstartbackend.upload.model.dto.request.DeleteArtifactsRequest import com.sprintstart.sprintstartbackend.upload.model.dto.request.UploadArtifactsRequest +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadsResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadArtifactResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadListItemResponse import com.sprintstart.sprintstartbackend.upload.service.UploadService @@ -113,12 +114,16 @@ class UploadController( * * @param jwt Authenticated JWT used to resolve the current user. * @param request Deletion metadata containing artifact ids and the target project. - * @return No content when the deletion batch has been processed. + * @return 200 with the deleted ids and a failure entry per id that was not deleted. Missing, + * foreign-project and storage-failed ids land in `failed`; they never fail the whole request. */ @Operation(summary = "Delete project uploads") @ApiResponses( value = [ - ApiResponse(responseCode = "204", description = "Deletion batch processed"), + ApiResponse( + responseCode = "200", + description = "Deletion batch processed; body lists deletedIds and per-id failures", + ), ApiResponse(responseCode = "401", description = "Authentication required"), ApiResponse(responseCode = "403", description = "Insufficient role or project access"), ApiResponse(responseCode = "404", description = "Authenticated user not found"), @@ -135,15 +140,12 @@ class UploadController( @Valid @RequestPart("request") request: DeleteArtifactsRequest, - ): ResponseEntity { - uploadService.deleteUpload( - authId = jwt.subject, - artifactIds = request.artifactIds, - projectId = request.projectId, + ): ResponseEntity = + ResponseEntity.ok( + uploadService.deleteUpload( + authId = jwt.subject, + artifactIds = request.artifactIds, + projectId = request.projectId, + ), ) - - return ResponseEntity - .noContent() - .build() - } } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/model/dto/response/DeleteUploadsResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/model/dto/response/DeleteUploadsResponse.kt new file mode 100644 index 000000000..6cd67bb77 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/model/dto/response/DeleteUploadsResponse.kt @@ -0,0 +1,27 @@ +package com.sprintstart.sprintstartbackend.upload.model.dto.response + +import java.util.UUID + +/** + * Result of a bulk upload deletion: what was removed and what was not. + * + * @property deletedIds Artifact ids that were deleted, in request order. + * @property failed One entry per requested id that was not deleted, with the reason. + */ +data class DeleteUploadsResponse( + val deletedIds: List, + val failed: List, +) + +/** + * One artifact the deletion batch skipped. + * + * @property artifactId The requested artifact id. + * @property error Client-safe reason: "Artifact with id not found." for missing or + * foreign ids, the generic "Artifact could not be deleted." for storage failures. Never a raw + * exception message, which could leak storage paths. + */ +data class DeleteUploadFailure( + val artifactId: UUID, + val error: String, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadService.kt index c006a9673..b4969d701 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadService.kt @@ -8,6 +8,8 @@ import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.Uploa import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.UploadBatchFinishedEvent import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.UploadFileDeletedEvent import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.UploadStartedEvent +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadFailure +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadsResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadArtifactResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadListItemResponse import com.sprintstart.sprintstartbackend.upload.model.entity.UploadedArtifact @@ -15,6 +17,7 @@ import com.sprintstart.sprintstartbackend.upload.repository.LinkedImageRepositor import com.sprintstart.sprintstartbackend.upload.repository.UploadedArtifactRepository import com.sprintstart.sprintstartbackend.upload.service.storage.ArtifactStorageService import com.sprintstart.sprintstartbackend.user.external.UserApi +import org.slf4j.LoggerFactory import org.springframework.context.ApplicationEventPublisher import org.springframework.http.HttpStatus import org.springframework.stereotype.Service @@ -24,6 +27,14 @@ import org.springframework.web.server.ResponseStatusException import java.security.MessageDigest import java.util.UUID +/** + * Client-facing reason for a storage failure during deletion. + * + * Exception messages can leak storage paths or driver internals, so the HTTP response always + * uses this text; the raw message stays in the deletion event outcome and the warn log. + */ +private const val DELETE_FAILED_REASON = "Artifact could not be deleted." + /** * Coordinates project upload storage, upload metadata persistence, and ingestion events. * @@ -41,6 +52,8 @@ class UploadService( private val artifactLinkingService: ArtifactLinkingService, private val publisher: ApplicationEventPublisher, ) { + private val logger = LoggerFactory.getLogger(javaClass) + /** * Uploads artifacts into a project as the authenticated PM or admin. * @@ -177,6 +190,10 @@ class UploadService( * @param authId The authenticated user's external auth id. * @param artifactIds The uploaded artifact ids requested for deletion. * @param projectId The project that owns the artifacts being deleted. + * @return The ids deleted, in request order, and one failure entry (id and reason) for every + * id that was skipped. Not-found ids report which id was missing; storage failures always + * report the generic [DELETE_FAILED_REASON], while the raw exception message goes only to + * the batch-finished event outcome and a warn log. * @throws ResponseStatusException `403` when the authenticated user cannot access the project. * @throws ResponseStatusException `404` when the authenticated user has no local projection. */ @@ -186,11 +203,13 @@ class UploadService( authId: String, artifactIds: Set, projectId: UUID, - ) { + ): DeleteUploadsResponse { val removerId = resolveCurrentUserId(userApi, authId) requireProjectAccess(userApi, authId, projectId) val deleteArtifactOutcomes = mutableSetOf() + val deletedIds = mutableListOf() + val failed = mutableListOf() val transactionId = UUID.randomUUID() publisher.publishEvent(UploadStartedEvent(transactionId = transactionId, projectId = projectId)) @@ -198,14 +217,16 @@ class UploadService( artifactIds.forEach { artifactId -> val artifact = uploadedArtifactRepository.findByIdAndProjectId(artifactId, projectId) if (artifact == null) { + val reason = "Artifact with id $artifactId not found." deleteArtifactOutcomes.add( UploadArtifactOperationOutcome( id = artifactId, filename = "unknown", status = UploadArtifactStatus.FAILED, - error = "Artifact with id $artifactId not found.", + error = reason, ), ) + failed.add(DeleteUploadFailure(artifactId = artifactId, error = reason)) return@forEach } @@ -223,6 +244,8 @@ class UploadService( error = e.message, ), ) + logger.warn("Storage delete failed for artifact {}: {}", artifactId, e.message) + failed.add(DeleteUploadFailure(artifactId = artifactId, error = DELETE_FAILED_REASON)) return@forEach } @@ -233,6 +256,7 @@ class UploadService( ), ) uploadedArtifactRepository.delete(artifact) + deletedIds.add(artifactId) } publisher.publishEvent( @@ -242,6 +266,8 @@ class UploadService( deleteArtifactOutcomes = deleteArtifactOutcomes, ), ) + + return DeleteUploadsResponse(deletedIds = deletedIds, failed = failed) } /** diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadControllerTest.kt index 6454a2060..2c5129594 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/controller/UploadControllerTest.kt @@ -2,6 +2,8 @@ package com.sprintstart.sprintstartbackend.upload.controller import com.ninjasquad.springmockk.MockkBean import com.sprintstart.sprintstartbackend.config.SecurityConfig +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadFailure +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadsResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadArtifactResponse import com.sprintstart.sprintstartbackend.upload.model.dto.response.UploadListItemResponse import com.sprintstart.sprintstartbackend.upload.service.UploadService @@ -229,33 +231,42 @@ class UploadControllerTest { // ========================== deleteUpload ========================== @Test - fun `deleteUpload returns 204 for PM`() { + fun `deleteUpload returns 200 with deleted ids and failures for PM`() { + val missingId = UUID.randomUUID() every { uploadService.deleteUpload(authId, setOf(artifactId), projectId) - } returns Unit + } returns DeleteUploadsResponse( + deletedIds = listOf(artifactId), + failed = listOf(DeleteUploadFailure(missingId, "Artifact with id $missingId not found.")), + ) mockMvc .perform( multipart(HttpMethod.DELETE, "/api/v1/uploads") .file(deleteRequest) .with(pmJwt), - ).andExpect(status().isNoContent) + ).andExpect(status().isOk) + .andExpect(jsonPath("$.deletedIds[0]").value(artifactId.toString())) + .andExpect(jsonPath("$.failed[0].artifactId").value(missingId.toString())) + .andExpect(jsonPath("$.failed[0].error").value("Artifact with id $missingId not found.")) verify { uploadService.deleteUpload(authId, setOf(artifactId), projectId) } } @Test - fun `deleteUpload returns 204 for admin`() { + fun `deleteUpload returns 200 for admin`() { every { uploadService.deleteUpload(authId, setOf(artifactId), projectId) - } returns Unit + } returns DeleteUploadsResponse(deletedIds = listOf(artifactId), failed = emptyList()) mockMvc .perform( multipart(HttpMethod.DELETE, "/api/v1/uploads") .file(deleteRequest) .with(adminJwt), - ).andExpect(status().isNoContent) + ).andExpect(status().isOk) + .andExpect(jsonPath("$.deletedIds[0]").value(artifactId.toString())) + .andExpect(jsonPath("$.failed").isEmpty) verify { uploadService.deleteUpload(authId, setOf(artifactId), projectId) } } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadServiceTest.kt index 3e5c5e329..1b5664961 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadServiceTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/upload/service/UploadServiceTest.kt @@ -1,6 +1,9 @@ package com.sprintstart.sprintstartbackend.upload.service +import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.UploadBatchDeletionFinishedEvent import com.sprintstart.sprintstartbackend.upload.external.events.ingestion.UploadStartedEvent +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadFailure +import com.sprintstart.sprintstartbackend.upload.model.dto.response.DeleteUploadsResponse import com.sprintstart.sprintstartbackend.upload.model.entity.UploadedArtifact import com.sprintstart.sprintstartbackend.upload.repository.LinkedImageRepository import com.sprintstart.sprintstartbackend.upload.repository.UploadedArtifactRepository @@ -146,8 +149,9 @@ class UploadServiceTest { every { storageService.delete("uploads/guide.md") } returns Unit every { uploadedArtifactRepository.delete(artifact) } returns Unit - service.deleteUpload(authId, setOf(artifactId), projectId) + val result = service.deleteUpload(authId, setOf(artifactId), projectId) + assertEquals(DeleteUploadsResponse(deletedIds = listOf(artifactId), failed = emptyList()), result) verify(exactly = 1) { uploadedArtifactRepository.findByIdAndProjectId(artifactId, projectId) } verify(exactly = 1) { storageService.delete("uploads/guide.md") } verify(exactly = 1) { uploadedArtifactRepository.delete(artifact) } @@ -159,21 +163,55 @@ class UploadServiceTest { every { userApi.userHasAccessToProject(authId, projectId) } returns true every { uploadedArtifactRepository.findByIdAndProjectId(artifactId, projectId) } returns null - service.deleteUpload(authId, setOf(artifactId), projectId) + val result = service.deleteUpload(authId, setOf(artifactId), projectId) + assertEquals(emptyList(), result.deletedIds) + assertEquals( + listOf(DeleteUploadFailure(artifactId, "Artifact with id $artifactId not found.")), + result.failed, + ) verify(exactly = 1) { uploadedArtifactRepository.findByIdAndProjectId(artifactId, projectId) } verify(exactly = 0) { storageService.delete(any()) } verify(exactly = 0) { uploadedArtifactRepository.delete(any()) } } - private fun artifact(): UploadedArtifact = + @Test + fun `deleteUpload reports a storage failure with the generic reason and keeps the raw one in the event`() { + val brokenId = UUID.randomUUID() + val broken = artifact(id = brokenId, storagePath = "uploads/broken.md") + every { userApi.getUserIdByAuthId(authId) } returns Optional.of(userId) + every { userApi.userHasAccessToProject(authId, projectId) } returns true + every { uploadedArtifactRepository.findByIdAndProjectId(brokenId, projectId) } returns broken + every { uploadedArtifactRepository.findByIdAndProjectId(artifactId, projectId) } returns artifact() + every { storageService.delete("uploads/broken.md") } throws IllegalStateException("disk gone: /srv/uploads") + every { storageService.delete("uploads/guide.md") } returns Unit + every { uploadedArtifactRepository.delete(any()) } returns Unit + + val result = service.deleteUpload(authId, linkedSetOf(brokenId, artifactId), projectId) + + assertEquals(listOf(artifactId), result.deletedIds) + assertEquals(listOf(DeleteUploadFailure(brokenId, "Artifact could not be deleted.")), result.failed) + verify(exactly = 1) { + publisher.publishEvent( + match { event -> + event.deleteArtifactOutcomes.map { it.id to it.error } == + listOf(brokenId to "disk gone: /srv/uploads") + }, + ) + } + } + + private fun artifact( + id: UUID = artifactId, + storagePath: String = "uploads/guide.md", + ): UploadedArtifact = UploadedArtifact( - id = artifactId, + id = id, filename = "guide.md", hash = "hash", uploadedAt = Instant.now(), mime = "text/markdown", - storagePath = "uploads/guide.md", + storagePath = storagePath, uploaderId = userId, projectId = projectId, ) From 1a0e84374a09c7e7cb5cef1fda9162b5fd40eca9 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 11:32:25 +0200 Subject: [PATCH 7/9] feat(knowledge-base): proxy the AI index status of project artifacts Adds GET /api/v1/projects/{projectId}/artifacts/ai-status?ids=a&ids=b so the Knowledge Base can show, per row, whether the AI assistant can actually find an artifact (Indexed / Indexing / Failed / Not indexed). Contract (kb-contract part 3, pinned with the frontend on this branch): 200 { "aiAvailable": bool, "items": [{ "artifactId", "status", "updatedAt", "chunkCount" }] } - status = INDEXED | PROCESSING | FAILED | DEINDEXED | UNKNOWN. - aiAvailable=true: the AI answered; UNKNOWN means it holds no record. - aiAvailable=false: AI unreachable, timed out, non-2xx or unreadable body; every visible id is UNKNOWN with null fields; warn log. - Ids outside the project are omitted; more than 100 ids -> 400; no ids -> 200 { aiAvailable: true, items: [] } without an AI call. - Same USER role and project-access check as the artifact list. Why a separate endpoint instead of a field on the list: - The list is served from our own database and must stay fast and available. Folding an AI call into it would make every page wait for, or fail with, the AI service. The frontend asks for chips after the page rendered, for exactly the ids it shows. Why aiAvailable exists and why AI trouble is never a 5xx: - "The AI has no record" (show "Not indexed") and "we could not ask" (show nothing) look the same as bare UNKNOWN items. The flag lets the frontend hide chips during an outage instead of claiming that every artifact is missing from the index. - Status chips are decoration; a broken AI must not turn a working Knowledge Base page into an error. Every failure, including an unexpected one, becomes aiAvailable=false plus a one-line warn log (no stack trace: an outage would otherwise flood the log on every page view). Coroutine cancellation is still propagated. Why foreign ids are dropped before the AI is asked: - ArtifactRepository.findIdsInProject selects ids only (no content column is loaded) through the same project join the list uses. Only ids linked to the project reach the AI, so the endpoint can neither leak another project's index state nor confirm that an id exists elsewhere; unknown ids are omitted the same way. Duplicates collapse and the answer keeps request order. If nothing visible remains, the AI is not called at all. Why the 100-id cap is checked in the controller: - It is request-shape validation, like the page-size @Max, and it matches the AI endpoint's own limit, so an oversized request fails with our 400 before any database or AI work instead of an AI 422. It counts raw ids, the same way the AI counts them. Why RequestBuilder gains an optional per-request timeout: - The shared HttpClient only has a 10 s connect timeout and no request timeout, so a hung AI (accepting connections, never answering) would hold a Knowledge Base request forever. A coroutine withTimeout cannot help: SyncExecution runs the blocking HttpClient.send on Dispatchers.IO, and withContext waits for it to return. - RequestBuilder.timeout(Duration) sets HttpRequest.timeout for that one request and is carried through copy(). It is additive: unset (every existing caller) builds exactly the request it built before. The extra method tips detekt's TooManyFunctions on this fluent builder; it is suppressed there with the reason in the KDoc. - fetchIngestStatus uses 3 s: the AI side is a keyed metadata read, and the frontend polls this per page, so a stuck AI costs a short wait and then degrades to aiAvailable=false. Why the backend owns the status enum and maps it leniently: - ArtifactIngestStatusAiItem keeps the AI wire format as raw strings; ArtifactAiIndexStatus.fromAi maps case-insensitively and turns null, "unknown" and anything unrecognised into UNKNOWN. A status the AI adds later therefore degrades to "Not indexed" instead of failing deserialisation and hiding every chip. - UNKNOWN always carries null updatedAt/chunkCount, whatever the AI sent, so it has one meaning downstream. An id the AI omits is UNKNOWN. - updatedAt is passed through as the AI's ISO string rather than parsed into an Instant: the backend is a proxy here, and re-parsing would turn an offset-less but valid timestamp into a failure. Why the endpoint is a suspend function in its own service: - ArtifactIngestionClient is suspend-based. ArtifactAiStatusService is deliberately not @Transactional (the annotation does not apply to suspend functions, and the one id lookup needs none), the same choice ArtifactProjectService documents. Tests: - ArtifactAiStatusControllerTest (imports SecurityConfig so the @PreAuthorize guard is really exercised): contract JSON shape incl. explicit nulls, missing ids -> empty, 101 ids -> 400 without calling the service, exactly 100 accepted, service 403 passed through, no USER role -> 403 without calling the service, malformed id -> 400. - ArtifactAiStatusServiceTest: access denied before any lookup, empty request and all-foreign request make no AI call, foreign ids dropped and duplicates collapsed in request order, connect error / timeout / non-2xx / bad body each give aiAvailable=false with UNKNOWN nulls, and status mapping incl. uppercase, "unknown", unrecognised and omitted ids. - ArtifactIngestionClientTest (MockWebServer): GET path with repeated artifact_ids, snake_case parsing with unknown fields ignored, 503 -> IngestionResponseException, wrong body -> SerializationException, closed port -> IOException, hung server -> HttpTimeoutException. - RequestBuilderTest: no timeout by default; timeout() survives chaining. ArtifactRepositoryIdsInProjectTest: the project join on H2. Gate: ./gradlew build green (3463 tests, 0 failures, 0 skipped; detekt 0 issues; ktlint clean; jacoco verification passed). --- .../ingestion/ArtifactIngestionClient.kt | 35 ++++ .../controller/ArtifactController.kt | 45 ++++++ .../model/dto/ArtifactAiIndexStatus.kt | 35 ++++ .../dto/response/ArtifactAiStatusResponse.kt | 31 ++++ .../ArtifactIngestStatusAiResponse.kt | 33 ++++ .../repository/ArtifactRepository.kt | 22 +++ .../service/ArtifactAiStatusService.kt | 109 +++++++++++++ .../shared/web/RequestBuilder.kt | 20 +++ .../ingestion/ArtifactIngestionClientTest.kt | 106 +++++++++++++ .../ArtifactAiStatusControllerTest.kt | 148 +++++++++++++++++ .../controller/ArtifactControllerTest.kt | 5 + .../ArtifactRepositoryIdsInProjectTest.kt | 74 +++++++++ .../service/ArtifactAiStatusServiceTest.kt | 150 ++++++++++++++++++ .../shared/web/RequestBuilderTest.kt | 22 +++ 14 files changed, 835 insertions(+) create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactAiIndexStatus.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactAiStatusResponse.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactIngestStatusAiResponse.kt create mode 100644 src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusService.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClientTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactAiStatusControllerTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepositoryIdsInProjectTest.kt create mode 100644 src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusServiceTest.kt diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt index 43d3e838f..68b10993c 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt @@ -5,6 +5,7 @@ import com.sprintstart.sprintstartbackend.ingestion.model.dto.request.AiArtifact import com.sprintstart.sprintstartbackend.ingestion.model.dto.request.ArtifactProjectsAiSyncRequest import com.sprintstart.sprintstartbackend.ingestion.model.dto.request.RunArtifactsAiSyncRequest import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.AiArtifactSummaryStreamMessage +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactIngestStatusAiResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactProjectsAiSyncResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ProjectMembershipsDeletedAiResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.RunArtifactsIngestResponse @@ -19,6 +20,7 @@ import org.springframework.http.HttpStatus import org.springframework.stereotype.Component import org.springframework.web.server.ResponseStatusException import java.net.URI +import java.time.Duration import java.util.UUID /** @@ -104,6 +106,34 @@ class ArtifactIngestionClient( ) } + /** + * Reads the AI index state of artifacts. Read-only on the AI side. + * + * Bounded by [INGEST_STATUS_TIMEOUT]: the Knowledge Base asks on every page view, so a hung AI + * service must cost the user a short wait, not a request that never returns. The shared + * client only has a connect timeout. + * + * @param artifactIds Ids to look up (the AI accepts at most 100), sent as repeated + * `artifact_ids` query parameters. + * @return One item per distinct requested id, in request order; `unknown` for ids without record. + * @throws IngestionResponseException when the AI service returns a non-successful HTTP response. + * @throws java.io.IOException when the AI service is unreachable or does not answer in time. + * @throws kotlinx.serialization.SerializationException when the body has an unexpected shape. + */ + suspend fun fetchIngestStatus(artifactIds: Collection): ArtifactIngestStatusAiResponse { + val query = artifactIds.joinToString("&") { "artifact_ids=$it" } + return try { + webClient + .get() + .uri(uri("/api/v1/ingest/status?$query")) + .timeout(INGEST_STATUS_TIMEOUT) + .sync() + .perform() + } catch (@Suppress("SwallowedException") e: WebClientException) { + throw IngestionResponseException("Failed to read ingest status (HTTP ${e.statusCode}): ${e.body}") + } + } + /** * Opens an SSE stream for a summary of [artifactId]. * @@ -143,4 +173,9 @@ class ArtifactIngestionClient( } private fun uri(path: String): URI = URI.create("${applicationConfig.ai.baseUrl}$path") + + companion object { + /** Upper bound for one AI status lookup; the metadata read behind it is a keyed select. */ + val INGEST_STATUS_TIMEOUT: Duration = Duration.ofSeconds(3) + } } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index e98f91883..0ccfc2a56 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -4,14 +4,17 @@ import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactFilterCriteria import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactSort import com.sprintstart.sprintstartbackend.ingestion.model.dto.UploadFormat +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactAiStatusResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentRedirectResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactContentResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactFacetsResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactPageResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactResponse import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactAiStatusService import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactQueryService import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactService +import com.sprintstart.sprintstartbackend.ingestion.service.MAX_AI_STATUS_IDS import io.swagger.v3.oas.annotations.Operation import io.swagger.v3.oas.annotations.Parameter import io.swagger.v3.oas.annotations.responses.ApiResponse @@ -33,6 +36,7 @@ import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.RestController +import org.springframework.web.server.ResponseStatusException import java.net.URI import java.time.LocalDate import java.util.UUID @@ -65,6 +69,7 @@ private const val LANGUAGES_DESCRIPTION = class ArtifactController( private val artifactService: ArtifactService, private val artifactQueryService: ArtifactQueryService, + private val artifactAiStatusService: ArtifactAiStatusService, ) { /** * Returns a paginated artifact list across all projects for administrative callers. @@ -230,6 +235,46 @@ class ArtifactController( ) } + /** + * Returns the AI index state of the artifacts shown on one Knowledge Base page. + * + * Separate from the list on purpose: the list never waits for, or fails because of, the AI + * service; the frontend asks for chips after the page rendered. The id cap is checked here, + * at the HTTP edge, before any project lookup. AI trouble is never a 5xx: it comes back as + * `aiAvailable = false`. + */ + @GetMapping("projects/{projectId}/artifacts/ai-status") + @PreAuthorize("hasRole('USER')") + @Operation( + summary = "Get AI index status of project artifacts", + description = "Per-id AI index state; ids outside the project are omitted.", + ) + @ApiResponses( + value = [ + ApiResponse( + responseCode = "200", + description = "Status per visible id; aiAvailable=false when the AI could not be asked", + ), + ApiResponse(responseCode = "400", description = "More than 100 ids, or a malformed id"), + ApiResponse(responseCode = "403", description = "Caller has no access to the project"), + ], + ) + suspend fun getProjectArtifactAiStatus( + @Parameter(description = "Artifact ids to check (repeatable, at most 100)") + @RequestParam(required = false) + ids: List?, + @Parameter(description = "UUID of the project the artifacts belong to") + @PathVariable + projectId: UUID, + @Parameter(hidden = true) @AuthenticationPrincipal jwt: Jwt, + ): ResponseEntity { + val requested = ids.orEmpty() + if (requested.size > MAX_AI_STATUS_IDS) { + throw ResponseStatusException(HttpStatus.BAD_REQUEST, "At most $MAX_AI_STATUS_IDS ids per request") + } + return ResponseEntity.ok(artifactAiStatusService.getAiStatus(jwt.subject, projectId, requested)) + } + @GetMapping("projects/{projectId}/artifacts/{artifactId}") @PreAuthorize("hasRole('USER')") @Operation( diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactAiIndexStatus.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactAiIndexStatus.kt new file mode 100644 index 000000000..1d386810a --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactAiIndexStatus.kt @@ -0,0 +1,35 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto + +/** + * AI index state of one artifact, as shown by the Knowledge Base status chip. + * + * The backend owns this enum rather than forwarding the AI's lowercase strings, so the frontend + * gets a closed set and a new or misspelt AI value can never reach it: [fromAi] maps anything + * unrecognised to [UNKNOWN]. + */ +enum class ArtifactAiIndexStatus { + /** Embedded and searchable by the assistant (AI `indexed`, recorded as `completed`). */ + INDEXED, + + /** Ingestion is still running. */ + PROCESSING, + + /** The last ingestion attempt failed. */ + FAILED, + + /** Removed from the index on purpose. */ + DEINDEXED, + + /** The AI holds no record, reports a value we do not know, or could not be asked. */ + UNKNOWN, + ; + + companion object { + /** + * Maps the AI's status string, case-insensitively; null or unknown values become [UNKNOWN]. + */ + fun fromAi(value: String?): ArtifactAiIndexStatus = + entries.firstOrNull { it != UNKNOWN && it.name.equals(value?.trim(), ignoreCase = true) } + ?: UNKNOWN + } +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactAiStatusResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactAiStatusResponse.kt new file mode 100644 index 000000000..dcae226b1 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactAiStatusResponse.kt @@ -0,0 +1,31 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto.response + +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactAiIndexStatus +import java.util.UUID + +/** + * AI index state of one visible artifact. + * + * @property updatedAt The AI's ISO timestamp of the last recorded change, passed through verbatim; + * null when the AI holds no record or was unreachable. + * @property chunkCount Chunks the AI recorded; null when unknown. + */ +data class ArtifactAiStatusItemResponse( + val artifactId: UUID, + val status: ArtifactAiIndexStatus, + val updatedAt: String?, + val chunkCount: Int?, +) + +/** + * Answer of `GET /projects/{projectId}/artifacts/ai-status`. + * + * @property aiAvailable False when the AI service could not be asked (unreachable, timeout, + * non-2xx, unreadable body). Every item is then UNKNOWN with nulls, and the frontend hides the + * chip instead of claiming "Not indexed". + * @property items One entry per requested id that belongs to the project, in request order. + */ +data class ArtifactAiStatusResponse( + val aiAvailable: Boolean, + val items: List, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactIngestStatusAiResponse.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactIngestStatusAiResponse.kt new file mode 100644 index 000000000..7eb1a4374 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/response/ArtifactIngestStatusAiResponse.kt @@ -0,0 +1,33 @@ +package com.sprintstart.sprintstartbackend.ingestion.model.dto.response + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * One artifact's index state as the AI service reports it on `GET /api/v1/ingest/status`. + * + * Kept as raw strings on purpose: this is the AI wire format, and mapping onto the backend's own + * [com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactAiIndexStatus] happens in the + * service, so a status the AI adds later degrades to UNKNOWN instead of failing deserialization. + * + * @property artifactId The requested id, echoed verbatim. + * @property status Lowercase AI status (indexed, processing, failed, deindexed, unknown). + * @property updatedAt ISO timestamp of the last recorded change; null when the AI holds no record. + * @property chunkCount Chunks recorded for the artifact; null when the AI holds no record. + */ +@Serializable +data class ArtifactIngestStatusAiItem( + @SerialName("artifact_id") + val artifactId: String, + val status: String, + @SerialName("updated_at") + val updatedAt: String? = null, + @SerialName("chunk_count") + val chunkCount: Int? = null, +) + +/** AI response body of `GET /api/v1/ingest/status`: one item per distinct requested id. */ +@Serializable +data class ArtifactIngestStatusAiResponse( + val items: List, +) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt index f3bf79b0f..a041aea67 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepository.kt @@ -167,6 +167,28 @@ interface ArtifactRepository : ) fun findProjectIdsByArtifactIdIn(@Param("artifactIds") artifactIds: Collection): Set + /** + * Returns which of [artifactIds] belong to the project. + * + * Selects ids only, so a status lookup never loads artifact content. Callers use it to drop ids + * from other projects before asking the AI service about them. + * + * @param artifactIds The ids to check; callers must not pass an empty collection. + */ + @Query( + """ + SELECT DISTINCT a.id + FROM Artifact a + JOIN a.projectIdsInternal p + WHERE p = :projectId + AND a.id IN :artifactIds + """, + ) + fun findIdsInProject( + @Param("projectId") projectId: UUID, + @Param("artifactIds") artifactIds: Collection, + ): Set + /** * Returns one artifact page limited to artifacts linked to the given project. */ diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusService.kt new file mode 100644 index 000000000..603a9a407 --- /dev/null +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusService.kt @@ -0,0 +1,109 @@ +package com.sprintstart.sprintstartbackend.ingestion.service + +import com.sprintstart.sprintstartbackend.ingestion.ArtifactIngestionClient +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactAiIndexStatus +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactAiStatusItemResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactAiStatusResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactIngestStatusAiItem +import com.sprintstart.sprintstartbackend.ingestion.repository.ArtifactRepository +import com.sprintstart.sprintstartbackend.user.external.UserApi +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import org.slf4j.LoggerFactory +import org.springframework.http.HttpStatus +import org.springframework.stereotype.Service +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +/** Most ids one status request may ask about: one Knowledge Base page, and the AI's own cap. */ +const val MAX_AI_STATUS_IDS = 100 + +/** + * Tells the Knowledge Base whether each visible artifact is indexed for the AI assistant. + * + * A thin proxy over the AI service's read-only status endpoint. It never fails the page: an AI + * outage turns into `aiAvailable = false` with UNKNOWN items, so the list still renders and the + * frontend just hides the chips. Not `@Transactional`: the only database work is one id-only + * lookup, and that annotation does not apply to suspend functions. + */ +@Service +class ArtifactAiStatusService( + private val artifactRepository: ArtifactRepository, + private val artifactIngestionClient: ArtifactIngestionClient, + private val userApi: UserApi, +) { + private val logger = LoggerFactory.getLogger(javaClass) + + /** + * Returns the AI index state of the requested artifacts that belong to the project. + * + * @param authId JWT subject; must have access to the project (same check as the list). + * @param projectId The project whose artifacts are asked about. + * @param artifactIds Requested ids; duplicates collapse, foreign or unknown ids are omitted + * silently so the endpoint never confirms that an id exists elsewhere. + * @return Items in request order. Empty (and no AI call) when nothing visible was asked. + * @throws ResponseStatusException `403` when the user has no access to the project. + */ + suspend fun getAiStatus(authId: String, projectId: UUID, artifactIds: List): ArtifactAiStatusResponse { + if (!userApi.userHasAccessToProject(authId, projectId)) { + throw ResponseStatusException(HttpStatus.FORBIDDEN, "No access to project with id $projectId") + } + val visibleIds = visibleIds(projectId, artifactIds) + if (visibleIds.isEmpty()) { + return ArtifactAiStatusResponse(aiAvailable = true, items = emptyList()) + } + return fetchStatuses(projectId, visibleIds) + } + + private fun visibleIds(projectId: UUID, artifactIds: List): List { + val requested = artifactIds.distinct() + if (requested.isEmpty()) return emptyList() + val inProject = artifactRepository.findIdsInProject(projectId, requested) + return requested.filter { it in inProject } + } + + private suspend fun fetchStatuses(projectId: UUID, visibleIds: List): ArtifactAiStatusResponse { + val aiItems = try { + artifactIngestionClient.fetchIngestStatus(visibleIds).items + } catch (e: Exception) { + // A cancelled request must stay cancelled; anything else means "AI not answering". + currentCoroutineContext().ensureActive() + logger.warn( + "AI status lookup for {} artifact(s) of project {} failed, reporting UNKNOWN: {}: {}", + visibleIds.size, + projectId, + e.javaClass.simpleName, + e.message, + ) + null + } + if (aiItems == null) { + return ArtifactAiStatusResponse(aiAvailable = false, items = visibleIds.map(::unknownItem)) + } + val byId = aiItems.associateBy { it.artifactId.lowercase() } + return ArtifactAiStatusResponse( + aiAvailable = true, + items = visibleIds.map { id -> byId[id.toString()]?.let { toItem(id, it) } ?: unknownItem(id) }, + ) + } + + private fun toItem(id: UUID, aiItem: ArtifactIngestStatusAiItem): ArtifactAiStatusItemResponse { + val status = ArtifactAiIndexStatus.fromAi(aiItem.status) + // UNKNOWN always carries nulls, whatever the AI sent, so it has a single meaning downstream. + if (status == ArtifactAiIndexStatus.UNKNOWN) return unknownItem(id) + return ArtifactAiStatusItemResponse( + artifactId = id, + status = status, + updatedAt = aiItem.updatedAt, + chunkCount = aiItem.chunkCount, + ) + } + + private fun unknownItem(id: UUID) = + ArtifactAiStatusItemResponse( + artifactId = id, + status = ArtifactAiIndexStatus.UNKNOWN, + updatedAt = null, + chunkCount = null, + ) +} diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt index 44d517809..0a9feb42c 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt @@ -3,6 +3,7 @@ package com.sprintstart.sprintstartbackend.shared.web import java.net.URI import java.net.http.HttpRequest.BodyPublishers.noBody import java.net.http.HttpRequest.BodyPublishers.ofString +import java.time.Duration /** * Immutable accumulator for HTTP request parameters, constructed via [WebClient]. @@ -24,7 +25,11 @@ import java.net.http.HttpRequest.BodyPublishers.ofString * .sync() * .perform() * ``` + * + * `TooManyFunctions` is suppressed: a fluent builder is one small method per request option, and + * splitting it would only scatter them. */ +@Suppress("TooManyFunctions") class RequestBuilder( val method: String, val httpClient: java.net.http.HttpClient, @@ -32,6 +37,7 @@ class RequestBuilder( val uri: URI? = null, val headers: Map = emptyMap(), val rawBody: String? = null, + val timeout: Duration? = null, ) { // ── URI ────────────────────────────────────────────────────────────────── @@ -70,6 +76,16 @@ class RequestBuilder( headers = headers + ("Content-Type" to "application/json"), ) + /** + * Bounds this single request (response headers must arrive within [timeout]). + * + * The shared [java.net.http.HttpClient] only has a connect timeout, and a coroutine + * `withTimeout` cannot interrupt the blocking `send` on [kotlinx.coroutines.Dispatchers.IO], + * so a caller that must answer fast even when the peer hangs sets it here. On expiry the + * send throws [java.net.http.HttpTimeoutException]. + */ + fun timeout(timeout: Duration): RequestBuilder = copy(timeout = timeout) + // ── Execution context selection ─────────────────────────────────────────── /** @@ -92,6 +108,7 @@ class RequestBuilder( uri: URI? = this.uri, headers: Map = this.headers, rawBody: String? = this.rawBody, + timeout: Duration? = this.timeout, ): RequestBuilder = RequestBuilder( method = method, httpClient = this.httpClient, @@ -99,6 +116,7 @@ class RequestBuilder( uri = uri, headers = headers, rawBody = rawBody, + timeout = timeout, ) @PublishedApi @@ -111,11 +129,13 @@ class RequestBuilder( noBody() } + val requestTimeout = timeout return java.net.http.HttpRequest .newBuilder() .uri(uri) .method(method.uppercase(), bodyPublisher) .apply { headers.forEach { (k, v) -> header(k, v) } } + .apply { if (requestTimeout != null) timeout(requestTimeout) } .build() } } diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClientTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClientTest.kt new file mode 100644 index 000000000..6a4ec5102 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClientTest.kt @@ -0,0 +1,106 @@ +package com.sprintstart.sprintstartbackend.ingestion + +import com.sprintstart.sprintstartbackend.AiConfig +import com.sprintstart.sprintstartbackend.ApplicationConfig +import com.sprintstart.sprintstartbackend.CryptoConfig +import com.sprintstart.sprintstartbackend.GithubConfig +import com.sprintstart.sprintstartbackend.UploadConfig +import com.sprintstart.sprintstartbackend.shared.web.WebClient +import com.sprintstart.sprintstartbackend.upload.model.exceptions.IngestionResponseException +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.SocketPolicy +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.io.IOException +import java.net.http.HttpClient +import java.net.http.HttpTimeoutException +import java.util.UUID +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertNull + +class ArtifactIngestionClientTest { + private val mockWebServer = MockWebServer() + private lateinit var client: ArtifactIngestionClient + + @BeforeEach + fun setUp() { + mockWebServer.start() + val webClient = WebClient(HttpClient.newBuilder().build(), Json { ignoreUnknownKeys = true }) + val applicationConfig = ApplicationConfig( + ai = AiConfig(baseUrl = mockWebServer.url("/").toString().removeSuffix("/")), + github = GithubConfig(baseUrl = "https://github.example.com"), + crypto = CryptoConfig(masterKey = "test-master-key", salt = "test-salt"), + upload = UploadConfig(directory = "/tmp/uploads", maxFileSizeBytes = 100), + ) + client = ArtifactIngestionClient(webClient, applicationConfig) + } + + @AfterEach + fun tearDown() { + mockWebServer.shutdown() + } + + @Test + fun `fetchIngestStatus sends a GET with repeated artifact_ids and parses the snake_case body`() = runTest { + val first = UUID.randomUUID() + val second = UUID.randomUUID() + mockWebServer.enqueue( + MockResponse().setResponseCode(200).setBody( + """{"items":[""" + + """{"artifact_id":"$first","status":"indexed","updated_at":"2026-09-20""" + + """T10:00:00+00:00","chunk_count":3,"extra":1},""" + + """{"artifact_id":"$second","status":"unknown","updated_at":null,"chunk_count":null}]}""", + ), + ) + + val response = client.fetchIngestStatus(listOf(first, second)) + + val request = mockWebServer.takeRequest() + assertEquals("GET", request.method) + assertEquals("/api/v1/ingest/status?artifact_ids=$first&artifact_ids=$second", request.path) + assertEquals(listOf(first.toString(), second.toString()), response.items.map { it.artifactId }) + assertEquals("indexed", response.items[0].status) + assertEquals("2026-09-20T10:00:00+00:00", response.items[0].updatedAt) + assertEquals(3, response.items[0].chunkCount) + assertNull(response.items[1].updatedAt) + assertNull(response.items[1].chunkCount) + } + + @Test + fun `fetchIngestStatus turns a non-2xx answer into IngestionResponseException`() = runTest { + mockWebServer.enqueue(MockResponse().setResponseCode(503).setBody("down")) + + val error = assertFailsWith { + client.fetchIngestStatus(listOf(UUID.randomUUID())) + } + + assertEquals("Failed to read ingest status (HTTP 503): down", error.message) + } + + @Test + fun `fetchIngestStatus rejects a body that is not the status shape`() = runTest { + mockWebServer.enqueue(MockResponse().setResponseCode(200).setBody("""{"artifacts":[]}""")) + + assertFailsWith { client.fetchIngestStatus(listOf(UUID.randomUUID())) } + } + + @Test + fun `fetchIngestStatus surfaces an unreachable AI service as IOException`() = runTest { + mockWebServer.shutdown() + + assertFailsWith { client.fetchIngestStatus(listOf(UUID.randomUUID())) } + } + + @Test + fun `fetchIngestStatus gives up on a hung AI service after the status timeout`() = runTest { + mockWebServer.enqueue(MockResponse().setSocketPolicy(SocketPolicy.NO_RESPONSE)) + + assertFailsWith { client.fetchIngestStatus(listOf(UUID.randomUUID())) } + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactAiStatusControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactAiStatusControllerTest.kt new file mode 100644 index 000000000..26621d041 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactAiStatusControllerTest.kt @@ -0,0 +1,148 @@ +package com.sprintstart.sprintstartbackend.ingestion.controller + +import com.ninjasquad.springmockk.MockkBean +import com.sprintstart.sprintstartbackend.config.SecurityConfig +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactAiIndexStatus +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactAiStatusItemResponse +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactAiStatusResponse +import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactAiStatusService +import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactQueryService +import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactService +import io.mockk.coEvery +import io.mockk.coVerify +import org.hamcrest.Matchers.hasKey +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc +import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest +import org.springframework.context.annotation.Import +import org.springframework.http.HttpStatus +import org.springframework.security.core.authority.SimpleGrantedAuthority +import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.jwt +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.ResultActions +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.request +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status +import org.springframework.web.server.ResponseStatusException +import java.util.UUID + +/** + * Web-layer contract of `GET /projects/{projectId}/artifacts/ai-status`: role guard, id cap and + * the JSON shape the Knowledge Base chip reads. Service behaviour has its own test. + * [SecurityConfig] is imported so `@PreAuthorize` is live; without it the slice has no method + * security and the role guard would go untested. + */ +@WebMvcTest(controllers = [ArtifactController::class]) +@Import(SecurityConfig::class) +@AutoConfigureMockMvc +class ArtifactAiStatusControllerTest( + @Autowired private val mockMvc: MockMvc, +) { + @MockkBean + private lateinit var artifactAiStatusService: ArtifactAiStatusService + + @MockkBean + private lateinit var artifactQueryService: ArtifactQueryService + + @MockkBean + private lateinit var artifactService: ArtifactService + + private val projectId = UUID.randomUUID() + private val path = "/api/v1/projects/$projectId/artifacts/ai-status" + private val userJwt = jwt().jwt { it.subject("user-1") }.authorities(SimpleGrantedAuthority("ROLE_USER")) + + @Test + fun `returns aiAvailable and one item per visible id in the contract shape`() { + val indexed = UUID.randomUUID() + val unknown = UUID.randomUUID() + coEvery { artifactAiStatusService.getAiStatus("user-1", projectId, listOf(indexed, unknown)) } returns + ArtifactAiStatusResponse( + aiAvailable = true, + items = listOf( + ArtifactAiStatusItemResponse(indexed, ArtifactAiIndexStatus.INDEXED, "2026-09-20T10:00:00Z", 7), + ArtifactAiStatusItemResponse(unknown, ArtifactAiIndexStatus.UNKNOWN, null, null), + ), + ) + + performAsync(get("$path?ids=$indexed&ids=$unknown").with(userJwt)) + .andExpect(status().isOk) + .andExpect(jsonPath("$.aiAvailable").value(true)) + .andExpect(jsonPath("$.items.length()").value(2)) + .andExpect(jsonPath("$.items[0].artifactId").value(indexed.toString())) + .andExpect(jsonPath("$.items[0].status").value("INDEXED")) + .andExpect(jsonPath("$.items[0].updatedAt").value("2026-09-20T10:00:00Z")) + .andExpect(jsonPath("$.items[0].chunkCount").value(7)) + .andExpect(jsonPath("$.items[1].status").value("UNKNOWN")) + .andExpect(jsonPath("$.items[1]", hasKey("updatedAt"))) + .andExpect(jsonPath("$.items[1].updatedAt").isEmpty) + .andExpect(jsonPath("$.items[1].chunkCount").isEmpty) + } + + @Test + fun `treats a missing ids parameter as an empty request`() { + coEvery { artifactAiStatusService.getAiStatus("user-1", projectId, emptyList()) } returns + ArtifactAiStatusResponse(aiAvailable = true, items = emptyList()) + + performAsync(get(path).with(userJwt)) + .andExpect(status().isOk) + .andExpect(jsonPath("$.aiAvailable").value(true)) + .andExpect(jsonPath("$.items").isEmpty) + } + + @Test + fun `rejects more than 100 ids with 400 before asking the service`() { + val query = (1..101).joinToString("&") { "ids=${UUID.randomUUID()}" } + + performAsync(get("$path?$query").with(userJwt)) + .andExpect(status().isBadRequest) + + coVerify(exactly = 0) { artifactAiStatusService.getAiStatus(any(), any(), any()) } + } + + @Test + fun `accepts exactly 100 ids`() { + val ids = List(100) { UUID.randomUUID() } + coEvery { artifactAiStatusService.getAiStatus("user-1", projectId, ids) } returns + ArtifactAiStatusResponse(aiAvailable = false, items = emptyList()) + + performAsync(get("$path?${ids.joinToString("&") { "ids=$it" }}").with(userJwt)) + .andExpect(status().isOk) + .andExpect(jsonPath("$.aiAvailable").value(false)) + } + + @Test + fun `passes the project access denial through as 403`() { + val id = UUID.randomUUID() + coEvery { artifactAiStatusService.getAiStatus("user-1", projectId, listOf(id)) } throws + ResponseStatusException(HttpStatus.FORBIDDEN, "No access to project with id $projectId") + + performAsync(get("$path?ids=$id").with(userJwt)) + .andExpect(status().isForbidden) + } + + @Test + fun `rejects a caller without the USER role`() { + // Method security on a suspend handler is evaluated inside the coroutine, hence async. + performAsync(get("$path?ids=${UUID.randomUUID()}").with(jwt().jwt { it.subject("user-1") })) + .andExpect(status().isForbidden) + + coVerify(exactly = 0) { artifactAiStatusService.getAiStatus(any(), any(), any()) } + } + + @Test + fun `rejects a malformed id with 400`() { + mockMvc + .perform(get("$path?ids=not-a-uuid").with(userJwt)) + .andExpect(status().isBadRequest) + } + + /** Suspend handlers answer through an async dispatch; this runs both legs. */ + private fun performAsync(builder: MockHttpServletRequestBuilder): ResultActions { + val started = mockMvc.perform(builder).andExpect(request().asyncStarted()).andReturn() + return mockMvc.perform(asyncDispatch(started)) + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt index 1f2f88e4c..f0bce41f8 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactControllerTest.kt @@ -13,6 +13,7 @@ import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactR import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.FacetCountResponse import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.PageMetadata import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactAiStatusService import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactQueryService import com.sprintstart.sprintstartbackend.ingestion.service.ArtifactService import io.mockk.every @@ -46,6 +47,10 @@ class ArtifactControllerTest( @MockkBean private lateinit var artifactService: ArtifactService + // Only a constructor dependency here; the ai-status endpoint is covered by ArtifactAiStatusControllerTest. + @MockkBean + private lateinit var artifactAiStatusService: ArtifactAiStatusService + @Test fun `getAllArtifacts uses default pagination and empty filter`() { every { artifactQueryService.getAllArtifacts(1, 20, "") } returns response() diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepositoryIdsInProjectTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepositoryIdsInProjectTest.kt new file mode 100644 index 000000000..d0eb574d6 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactRepositoryIdsInProjectTest.kt @@ -0,0 +1,74 @@ +package com.sprintstart.sprintstartbackend.ingestion.repository + +import com.sprintstart.sprintstartbackend.ingestion.external.model.SourceSystem +import com.sprintstart.sprintstartbackend.ingestion.model.entity.Artifact +import com.sprintstart.sprintstartbackend.ingestion.model.entity.ArtifactType +import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRun +import com.sprintstart.sprintstartbackend.ingestion.model.entity.IngestionRunStatus +import com.sprintstart.sprintstartbackend.shared.crypto.CryptoConfiguration +import jakarta.persistence.EntityManager +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.data.jpa.test.autoconfigure.DataJpaTest +import org.springframework.context.annotation.Import +import org.springframework.test.context.ActiveProfiles +import java.util.UUID + +/** + * The ai-status endpoint trusts this query to hide foreign artifacts, so it meets a real database: + * a mocked repository could not show that the project join actually filters. + */ +@ActiveProfiles("test") +@DataJpaTest +@Import(CryptoConfiguration::class) +class ArtifactRepositoryIdsInProjectTest { + @Autowired + private lateinit var repository: ArtifactRepository + + @Autowired + private lateinit var entityManager: EntityManager + + @Test + fun `returns only the requested ids that are linked to the project`() { + val projectId = UUID.randomUUID() + val otherProject = UUID.randomUUID() + val mine = store("mine").apply { addProjectId(projectId) } + val shared = store("shared").apply { addProjectIds(setOf(projectId, otherProject)) } + val foreign = store("foreign").apply { addProjectId(otherProject) } + store("unrequested").addProjectId(projectId) + entityManager.flush() + + val found = repository.findIdsInProject( + projectId, + listOf(mine.id, shared.id, foreign.id, UUID.randomUUID()), + ) + + assertThat(found).containsExactlyInAnyOrder(mine.id, shared.id) + } + + private fun store(name: String): Artifact { + val run = IngestionRun( + id = UUID.randomUUID(), + sourceSystem = SourceSystem.UPLOAD, + status = IngestionRunStatus.COMPLETED, + ) + entityManager.persist(run) + val artifact = Artifact( + sourceSystem = SourceSystem.UPLOAD, + sourceId = "upload:$name", + sourceUrl = null, + artifactType = ArtifactType.FILE, + title = name, + content = "content", + mime = null, + language = null, + createdAtSource = null, + updatedAtSource = null, + ingestionRun = run, + hash = null, + ) + entityManager.persist(artifact) + return artifact + } +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusServiceTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusServiceTest.kt new file mode 100644 index 000000000..b4e728600 --- /dev/null +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactAiStatusServiceTest.kt @@ -0,0 +1,150 @@ +package com.sprintstart.sprintstartbackend.ingestion.service + +import com.sprintstart.sprintstartbackend.ingestion.ArtifactIngestionClient +import com.sprintstart.sprintstartbackend.ingestion.model.dto.ArtifactAiIndexStatus +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactIngestStatusAiItem +import com.sprintstart.sprintstartbackend.ingestion.model.dto.response.ArtifactIngestStatusAiResponse +import com.sprintstart.sprintstartbackend.ingestion.repository.ArtifactRepository +import com.sprintstart.sprintstartbackend.upload.model.exceptions.IngestionResponseException +import com.sprintstart.sprintstartbackend.user.external.UserApi +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.SerializationException +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.http.HttpStatus +import org.springframework.web.server.ResponseStatusException +import java.net.ConnectException +import java.net.http.HttpTimeoutException +import java.util.UUID +import kotlin.test.assertFailsWith + +class ArtifactAiStatusServiceTest { + private val artifactRepository = mockk() + private val artifactIngestionClient = mockk() + private val userApi = mockk() + private val service = ArtifactAiStatusService(artifactRepository, artifactIngestionClient, userApi) + + private val authId = "user-1" + private val projectId = UUID.randomUUID() + + init { + every { userApi.userHasAccessToProject(authId, projectId) } returns true + } + + @Test + fun `rejects a user without project access before any lookup`() = runTest { + every { userApi.userHasAccessToProject(authId, projectId) } returns false + + val error = assertFailsWith { + service.getAiStatus(authId, projectId, listOf(UUID.randomUUID())) + } + + assertThat(error.statusCode).isEqualTo(HttpStatus.FORBIDDEN) + verify(exactly = 0) { artifactRepository.findIdsInProject(any(), any()) } + coVerify(exactly = 0) { artifactIngestionClient.fetchIngestStatus(any()) } + } + + @Test + fun `answers an empty request as available and empty without any lookup`() = runTest { + val result = service.getAiStatus(authId, projectId, emptyList()) + + assertThat(result.aiAvailable).isTrue() + assertThat(result.items).isEmpty() + verify(exactly = 0) { artifactRepository.findIdsInProject(any(), any()) } + coVerify(exactly = 0) { artifactIngestionClient.fetchIngestStatus(any()) } + } + + @Test + fun `does not ask the AI when every requested id belongs to another project`() = runTest { + val foreign = UUID.randomUUID() + every { artifactRepository.findIdsInProject(projectId, listOf(foreign)) } returns emptySet() + + val result = service.getAiStatus(authId, projectId, listOf(foreign)) + + assertThat(result.aiAvailable).isTrue() + assertThat(result.items).isEmpty() + coVerify(exactly = 0) { artifactIngestionClient.fetchIngestStatus(any()) } + } + + @Test + fun `omits foreign ids, collapses duplicates and asks the AI only about visible ones`() = runTest { + val first = UUID.randomUUID() + val second = UUID.randomUUID() + val foreign = UUID.randomUUID() + every { artifactRepository.findIdsInProject(projectId, listOf(first, foreign, second)) } returns + setOf(second, first) + coEvery { artifactIngestionClient.fetchIngestStatus(listOf(first, second)) } returns + aiResponse(aiItem(first, "indexed"), aiItem(second, "processing")) + + val result = service.getAiStatus(authId, projectId, listOf(first, foreign, second, first)) + + assertThat(result.aiAvailable).isTrue() + assertThat(result.items.map { it.artifactId }).containsExactly(first, second) + assertThat(result.items.map { it.status }) + .containsExactly(ArtifactAiIndexStatus.INDEXED, ArtifactAiIndexStatus.PROCESSING) + } + + @Test + fun `reports aiAvailable false with UNKNOWN items when the AI cannot be asked`() = runTest { + val id = UUID.randomUUID() + every { artifactRepository.findIdsInProject(projectId, listOf(id)) } returns setOf(id) + val failures = listOf( + ConnectException("Connection refused"), + HttpTimeoutException("request timed out"), + IngestionResponseException("Failed to read ingest status (HTTP 503): down"), + SerializationException("Unexpected JSON token"), + ) + + failures.forEach { failure -> + coEvery { artifactIngestionClient.fetchIngestStatus(listOf(id)) } throws failure + + val result = service.getAiStatus(authId, projectId, listOf(id)) + + assertThat(result.aiAvailable).`as`(failure.javaClass.simpleName).isFalse() + val item = result.items.single() + assertThat(item.artifactId).isEqualTo(id) + assertThat(item.status).isEqualTo(ArtifactAiIndexStatus.UNKNOWN) + assertThat(item.updatedAt).isNull() + assertThat(item.chunkCount).isNull() + } + } + + @Test + fun `maps AI statuses and degrades unknown, unrecognised and missing ones to UNKNOWN`() = runTest { + val ids = List(7) { UUID.randomUUID() } + every { artifactRepository.findIdsInProject(projectId, ids) } returns ids.toSet() + val aiStatuses = listOf("indexed", "PROCESSING", "failed", "deindexed", "unknown", "reindexing") + coEvery { artifactIngestionClient.fetchIngestStatus(ids) } returns + aiResponse(*ids.zip(aiStatuses).map { (id, status) -> aiItem(id, status) }.toTypedArray()) + + val result = service.getAiStatus(authId, projectId, ids) + + assertThat(result.aiAvailable).isTrue() + assertThat(result.items.map { it.status }).containsExactly( + ArtifactAiIndexStatus.INDEXED, + ArtifactAiIndexStatus.PROCESSING, + ArtifactAiIndexStatus.FAILED, + ArtifactAiIndexStatus.DEINDEXED, + ArtifactAiIndexStatus.UNKNOWN, + ArtifactAiIndexStatus.UNKNOWN, + ArtifactAiIndexStatus.UNKNOWN, + ) + assertThat(result.items[0].updatedAt).isEqualTo("2026-09-20T10:00:00+00:00") + assertThat(result.items[0].chunkCount).isEqualTo(4) + assertThat(result.items.drop(4).map { it.updatedAt to it.chunkCount }).containsOnly(null to null) + } + + private fun aiItem(id: UUID, status: String) = ArtifactIngestStatusAiItem( + artifactId = id.toString(), + status = status, + updatedAt = "2026-09-20T10:00:00+00:00", + chunkCount = 4, + ) + + private fun aiResponse(vararg items: ArtifactIngestStatusAiItem) = ArtifactIngestStatusAiResponse(items.toList()) +} diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt index 21ebaf67c..83c95d347 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt @@ -8,6 +8,7 @@ import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import java.net.http.HttpClient +import java.time.Duration import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -139,4 +140,25 @@ class RequestBuilderTest { assertEquals("application/json", recorded.getHeader("Content-Type")) assertEquals("""{"key":"value"}""", recorded.body.readUtf8()) } + + // ── Timeout ─────────────────────────────────────────────────────────────── + + @Test + fun `no request timeout is set unless asked for`() { + val request = webClient.get().uri("https://ai.test/x").buildHttpRequest() + + assertEquals(false, request.timeout().isPresent) + } + + @Test + fun `timeout() applies to the built request and survives further chaining`() { + val request = webClient + .get() + .timeout(Duration.ofSeconds(2)) + .uri("https://ai.test/x") + .header("X-Trace", "1") + .buildHttpRequest() + + assertEquals(Duration.ofSeconds(2), request.timeout().get()) + } } From 04e68596ad9e615c93cf1cd6b373865c04fc4eab Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 12:27:20 +0200 Subject: [PATCH 8/9] feat(knowledge-base): filter the date window on last activity `from` / `to` now match an artifact's last activity, COALESCE(lastChangedAt, ingestedAt), instead of its first import. Why: the original plan decided on activity semantics ("added or changed in this window"). The pinned API contract narrowed it to ingestedAt by mistake. For onboarding, "what changed recently that I should re-read" is the more useful question: a README imported months ago but edited yesterday should show up under "Last 7 days". - The window uses the same key as the CHANGED_DESC sort, so "updated in the last 7 days" and "most recently changed first" always agree. - An artifact imported inside the window but changed after it no longer matches, because its latest activity lies outside the window. - The UTC whole-day bounds and the `from > to` 400 are unchanged; the window still applies to list and facets alike (count parity). - New repository test pins all three shapes: old but edited inside, imported inside but changed after, never changed and imported inside. - KDoc and OpenAPI descriptions say "activity" instead of "import". --- .../controller/ArtifactController.kt | 7 +++--- .../model/dto/ArtifactFilterCriteria.kt | 13 ++++++----- .../repository/ArtifactFacetRepositoryImpl.kt | 22 ++++++++++++------- .../ingestion/service/ArtifactQueryService.kt | 2 +- .../ArtifactFacetRepositoryQueryTest.kt | 20 +++++++++++++++++ 5 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt index 0ccfc2a56..91fbacab6 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/controller/ArtifactController.kt @@ -45,10 +45,11 @@ private const val DEFAULT_PAGE = "1" private const val DEFAULT_SIZE = "20" private const val MAX_PAGE_SIZE = 100L private const val FROM_DESCRIPTION = - "First import day to include, ISO yyyy-MM-dd, read as a UTC calendar day (inclusive). " + - "Must not be after `to`, else 400." + "First activity day to include (last content change, else import), ISO yyyy-MM-dd, " + + "read as a UTC calendar day (inclusive). Must not be after `to`, else 400." private const val TO_DESCRIPTION = - "Last import day to include, ISO yyyy-MM-dd, read as a UTC calendar day (inclusive)." + "Last activity day to include (last content change, else import), ISO yyyy-MM-dd, " + + "read as a UTC calendar day (inclusive)." private const val LANGUAGES_DESCRIPTION = "Language display names to keep (repeatable, case-insensitive), e.g. Kotlin. " + "Artifacts without a language are excluded while set." diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt index 0171d3b75..30ef235d8 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/model/dto/ArtifactFilterCriteria.kt @@ -18,12 +18,15 @@ enum class UploadFormat { * Filter criteria for project-scoped artifact searches and facet calculations. * * Encapsulates full-text search, type filtering, source filtering, repository selection, - * upload format selection, and an import-date window. + * upload format selection, and an activity-date window. * - * @property from First day (inclusive) of the import window, read as a UTC calendar day: rows with - * `ingestedAt >= from 00:00Z` match. Null leaves the window open at the start. - * @property to Last day (inclusive) of the import window, read as a UTC calendar day: rows with - * `ingestedAt < (to + 1 day) 00:00Z` match. Null leaves the window open at the end. + * Activity is `COALESCE(lastChangedAt, ingestedAt)`: the last content change, or the import when + * the artifact never changed (the same key the `CHANGED_DESC` sort uses). + * + * @property from First day (inclusive) of the activity window, read as a UTC calendar day: rows + * with `activity >= from 00:00Z` match. Null leaves the window open at the start. + * @property to Last day (inclusive) of the activity window, read as a UTC calendar day: rows with + * `activity < (to + 1 day) 00:00Z` match. Null leaves the window open at the end. * @property languages Language display names to keep, matched case-insensitively against the * stored name. Narrows every source: artifacts without a language drop out while it is set. */ diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt index 9fb159134..ceaa685d5 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryImpl.kt @@ -421,29 +421,35 @@ class ArtifactFacetRepositoryImpl( predicates.add(languageLower.`in`(languages.map { it.lowercase() })) } - // No facet counts the import date, so the window applies to every query alike -- which is + // No facet counts the activity date, so the window applies to every query alike -- which is // what keeps facet counts equal to the list's totalElements under the same filter. - predicates.addAll(buildIngestedWindowPredicates(cb, root, criteria.from, criteria.to)) + predicates.addAll(buildActivityWindowPredicates(cb, root, criteria.from, criteria.to)) return predicates } /** - * Restricts `ingestedAt` to the inclusive UTC calendar-day window `[from, to]`. + * Restricts an artifact's last activity to the inclusive UTC calendar-day window `[from, to]`. * - * The end bound is `< start of the day after [to]` rather than `<= end of [to]`, so an import + * Activity is `COALESCE(lastChangedAt, ingestedAt)`: the last content change, or the import for + * an artifact that never changed. It is the same key `CHANGED_DESC` sorts by, so "changed in + * the last 7 days" and "most recently changed first" always agree. An artifact imported long + * ago but edited inside the window matches; one imported inside the window and changed after + * it does not, because its latest activity lies outside. + * + * The end bound is `< start of the day after [to]` rather than `<= end of [to]`, so activity * in the last microsecond of that day still matches, whatever precision the column keeps. */ - private fun buildIngestedWindowPredicates( + private fun buildActivityWindowPredicates( cb: CriteriaBuilder, root: Root, from: LocalDate?, to: LocalDate?, ): List { - val ingestedAt = root.get("ingestedAt") + val activityAt = cb.coalesce(root.get("lastChangedAt"), root.get("ingestedAt")) return listOfNotNull( - from?.let { cb.greaterThanOrEqualTo(ingestedAt, it.atStartOfDay(ZoneOffset.UTC).toInstant()) }, - to?.let { cb.lessThan(ingestedAt, it.plusDays(1).atStartOfDay(ZoneOffset.UTC).toInstant()) }, + from?.let { cb.greaterThanOrEqualTo(activityAt, it.atStartOfDay(ZoneOffset.UTC).toInstant()) }, + to?.let { cb.lessThan(activityAt, it.plusDays(1).atStartOfDay(ZoneOffset.UTC).toInstant()) }, ) } diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt index ba3e25cc8..aaec78bdc 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/service/ArtifactQueryService.kt @@ -167,7 +167,7 @@ class ArtifactQueryService( } /** - * Rejects an import-date window whose start lies after its end. + * Rejects an activity-date window whose start lies after its end. * * Such a window can match nothing, so answering it with an empty page would hide a client bug * (typically swapped bounds) behind a plausible "no results". List and facets both call this, diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt index cf80f22ee..380b32bec 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/ingestion/repository/ArtifactFacetRepositoryQueryTest.kt @@ -152,6 +152,26 @@ class ArtifactFacetRepositoryQueryTest { assertThat(listIds(ArtifactFilterCriteria(to = day))).containsExactlyInAnyOrder(early.id, onDay.id) } + @Test + fun `the date window matches the last change, not the first import`() { + val window = ArtifactFilterCriteria(from = LocalDate.of(2026, 3, 10), to = LocalDate.of(2026, 3, 12)) + val oldButEditedInside = + store( + ingestedAt = Instant.parse("2025-01-01T00:00:00Z"), + lastChangedAt = Instant.parse("2026-03-11T10:00:00Z"), + ) + val importedInsideChangedAfter = + store( + ingestedAt = Instant.parse("2026-03-10T10:00:00Z"), + lastChangedAt = Instant.parse("2026-03-20T10:00:00Z"), + ) + val neverChangedInside = store(ingestedAt = Instant.parse("2026-03-12T10:00:00Z")) + flush() + + assertThat(listIds(window)).containsExactlyInAnyOrder(oldButEditedInside.id, neverChangedInside.id) + assertThat(listIds(window)).doesNotContain(importedInsideChangedAfter.id) + } + @Test fun `facets count under the same date window as the list`() { store(ingestedAt = Instant.parse("2026-03-10T08:00:00Z")) From 3512a0aec88edee81eb81af9bf3786b13893d742 Mon Sep 17 00:00:00 2001 From: Daniil Perkin Date: Thu, 24 Sep 2026 12:27:21 +0200 Subject: [PATCH 9/9] feat(knowledge-base): pass the AI status timeout to sync() instead of the builder Drops the `@Suppress("TooManyFunctions")` that the ai-status proxy added to the shared RequestBuilder. The 3 s bound on the AI status call stays, because a hung AI service must cost the Knowledge Base a short wait, not a request that never returns. It now travels as an optional parameter: `sync(timeout: Duration? = null)`, not a separate `timeout()` builder method. - That keeps RequestBuilder at detekt's function limit, so no new suppression lands (sprintstart-helper rule: don't commit suppressions). - It fits the design: the timeout says how to run the request, not what to send, and `sync()` is already the execution-context step. - Source-compatible: every existing `.sync()` call keeps its behaviour, with no timeout unless asked for. - RequestBuilderTest now covers `sync(timeout)` landing on the built request, and `sync()` leaving it unbounded. --- .../ingestion/ArtifactIngestionClient.kt | 3 +-- .../shared/web/RequestBuilder.kt | 24 +++++++------------ .../shared/web/RequestBuilderTest.kt | 20 +++++++++++++--- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt index 68b10993c..418fe90d9 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/ingestion/ArtifactIngestionClient.kt @@ -126,8 +126,7 @@ class ArtifactIngestionClient( webClient .get() .uri(uri("/api/v1/ingest/status?$query")) - .timeout(INGEST_STATUS_TIMEOUT) - .sync() + .sync(timeout = INGEST_STATUS_TIMEOUT) .perform() } catch (@Suppress("SwallowedException") e: WebClientException) { throw IngestionResponseException("Failed to read ingest status (HTTP ${e.statusCode}): ${e.body}") diff --git a/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt b/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt index 0a9feb42c..56d9a63f6 100644 --- a/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt +++ b/src/main/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilder.kt @@ -25,11 +25,7 @@ import java.time.Duration * .sync() * .perform() * ``` - * - * `TooManyFunctions` is suppressed: a fluent builder is one small method per request option, and - * splitting it would only scatter them. */ -@Suppress("TooManyFunctions") class RequestBuilder( val method: String, val httpClient: java.net.http.HttpClient, @@ -76,23 +72,21 @@ class RequestBuilder( headers = headers + ("Content-Type" to "application/json"), ) - /** - * Bounds this single request (response headers must arrive within [timeout]). - * - * The shared [java.net.http.HttpClient] only has a connect timeout, and a coroutine - * `withTimeout` cannot interrupt the blocking `send` on [kotlinx.coroutines.Dispatchers.IO], - * so a caller that must answer fast even when the peer hangs sets it here. On expiry the - * send throws [java.net.http.HttpTimeoutException]. - */ - fun timeout(timeout: Duration): RequestBuilder = copy(timeout = timeout) - // ── Execution context selection ─────────────────────────────────────────── /** * Returns a [SyncExecution] context for a standard request/response cycle. * Call `.perform()` on the result to fire the request. + * + * @param timeout Optional bound for this single request (response headers must arrive within + * it). The shared [java.net.http.HttpClient] only has a connect timeout, and a coroutine + * `withTimeout` cannot interrupt the blocking `send` on [kotlinx.coroutines.Dispatchers.IO], + * so a caller that must answer fast even when the peer hangs passes one here. On expiry the + * send throws [java.net.http.HttpTimeoutException]. It is a parameter of the execution step + * rather than a builder method because it says how to run the request, not what to send. */ - fun sync(): SyncExecution = SyncExecution(this) + fun sync(timeout: Duration? = null): SyncExecution = + SyncExecution(if (timeout == null) this else copy(timeout = timeout)) /** * Returns a [StreamExecution] context for SSE / chunked streaming responses. diff --git a/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt b/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt index 83c95d347..2b0c25702 100644 --- a/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt +++ b/src/test/kotlin/com/sprintstart/sprintstartbackend/shared/web/RequestBuilderTest.kt @@ -151,14 +151,28 @@ class RequestBuilderTest { } @Test - fun `timeout() applies to the built request and survives further chaining`() { - val request = webClient + fun `sync(timeout) applies the timeout to the built request`() { + val execution = webClient .get() - .timeout(Duration.ofSeconds(2)) .uri("https://ai.test/x") .header("X-Trace", "1") + .sync(timeout = Duration.ofSeconds(2)) + + val request = execution.builder .buildHttpRequest() assertEquals(Duration.ofSeconds(2), request.timeout().get()) } + + @Test + fun `sync() without a timeout leaves the request unbounded`() { + val request = webClient + .get() + .uri("https://ai.test/x") + .sync() + .builder + .buildHttpRequest() + + assertEquals(false, request.timeout().isPresent) + } }