From 91d0ae15046edb2042ab3ac201e8d6fa22d86f2a Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:49:24 +0800 Subject: [PATCH] feat(api): return skill labels from the skill listing endpoints on request (#730) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(api): return skill labels from the skill listing endpoints on request Skill labels were reachable only one skill at a time, through /api/{v1,web}/skills/{namespace}/{slug}/labels, so a client rendering a list had to issue a follow-up request per row. Add includeLabels=true to GET /api/v1/skills and GET /api/web/skills. The labels array is populated only when the parameter is set and left out of the payload otherwise, so existing responses are byte-identical. Labels for the whole page are resolved by SkillLabelProjectionService in three queries — assignments, definitions, translations — rather than three per skill. Closes #710 Signed-off-by: FenjuFu * fix(api): use include parameter for skill labels Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> * test(api): reject unsupported include before search Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --------- Signed-off-by: FenjuFu Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../compat/ClawHubCompatAppService.java | 31 +++++- .../compat/ClawHubCompatController.java | 8 +- .../compat/dto/ClawHubSkillListResponse.java | 23 ++++- .../portal/SkillSearchController.java | 25 ++++- .../controller/support/IncludeOptions.java | 42 +++++++++ .../skillhub/dto/SkillSummaryResponse.java | 47 +++++++++- .../service/SkillLabelProjectionService.java | 94 +++++++++++++++++++ .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../compat/ClawHubCompatAppServiceTest.java | 40 +++++++- .../compat/ClawHubCompatControllerTest.java | 63 +++++++++++++ .../controller/SkillSearchControllerTest.java | 62 ++++++++++++ .../support/IncludeOptionsTest.java | 28 ++++++ .../SkillLabelProjectionServiceTest.java | 81 ++++++++++++++++ .../domain/label/SkillLabelService.java | 7 ++ web/src/api/generated/schema.d.ts | 6 ++ 16 files changed, 548 insertions(+), 11 deletions(-) create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/IncludeOptions.java create mode 100644 server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelProjectionService.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/IncludeOptionsTest.java create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelProjectionServiceTest.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java index b9cfb13d..801b2ed2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java @@ -20,8 +20,10 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.domain.social.SkillStarService; +import com.iflytek.skillhub.dto.SkillLabelDto; import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.service.SkillLabelProjectionService; import com.iflytek.skillhub.service.SkillSearchAppService; import java.io.IOException; import java.util.HashMap; @@ -51,6 +53,7 @@ public class ClawHubCompatAppService { private final CompatSkillLookupService compatSkillLookupService; private final SkillStarService skillStarService; private final RequestIdAccessor requestIdAccessor; + private final SkillLabelProjectionService skillLabelProjectionService; public ClawHubCompatAppService(CanonicalSlugMapper mapper, SkillSearchAppService skillSearchAppService, @@ -61,7 +64,8 @@ public class ClawHubCompatAppService { AuditLogService auditLogService, CompatSkillLookupService compatSkillLookupService, SkillStarService skillStarService, - RequestIdAccessor requestIdAccessor) { + RequestIdAccessor requestIdAccessor, + SkillLabelProjectionService skillLabelProjectionService) { this.mapper = mapper; this.skillSearchAppService = skillSearchAppService; this.skillQueryService = skillQueryService; @@ -72,6 +76,7 @@ public class ClawHubCompatAppService { this.compatSkillLookupService = compatSkillLookupService; this.skillStarService = skillStarService; this.requestIdAccessor = requestIdAccessor; + this.skillLabelProjectionService = skillLabelProjectionService; } public ClawHubSearchResponse search(String q, @@ -195,6 +200,15 @@ public class ClawHubCompatAppService { String sort, String userId, Map userNsRoles) { + return listSkills(page, limit, sort, false, userId, userNsRoles); + } + + public ClawHubSkillListResponse listSkills(int page, + int limit, + String sort, + boolean includeLabels, + String userId, + Map userNsRoles) { String sortBy = sort != null ? sort : "newest"; SkillSearchAppService.SearchResponse response = skillSearchAppService.search( "", @@ -206,8 +220,15 @@ public class ClawHubCompatAppService { userNsRoles ); + Map> labelsBySkillId = includeLabels + ? skillLabelProjectionService.labelsBySkillIds( + response.items().stream().map(SkillSummaryResponse::id).toList()) + : Map.of(); + List items = response.items().stream() - .map(this::toSkillListItem) + .map(item -> toSkillListItem( + item, + includeLabels ? labelsBySkillId.getOrDefault(item.id(), List.of()) : null)) .toList(); String nextCursor = null; @@ -383,7 +404,8 @@ public class ClawHubCompatAppService { return new ClawHubResolveResponse(matchVersion, latestVersion); } - private ClawHubSkillListResponse.SkillListItem toSkillListItem(SkillSummaryResponse item) { + private ClawHubSkillListResponse.SkillListItem toSkillListItem(SkillSummaryResponse item, + List labels) { long createdAt = 0; long updatedAt = item.updatedAt() != null ? item.updatedAt().toEpochMilli() : 0; @@ -413,7 +435,8 @@ public class ClawHubCompatAppService { stats, createdAt, updatedAt, - latestVersion + latestVersion, + labels ); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java index 2d499c31..50013251 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatController.java @@ -10,10 +10,13 @@ import com.iflytek.skillhub.compat.dto.ClawHubSkillResponse; import com.iflytek.skillhub.compat.dto.ClawHubStarResponse; import com.iflytek.skillhub.compat.dto.ClawHubUnstarResponse; import com.iflytek.skillhub.compat.dto.ClawHubWhoamiResponse; +import com.iflytek.skillhub.controller.support.IncludeOptions; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.ratelimit.RateLimit; +import io.swagger.v3.oas.annotations.Parameter; import jakarta.servlet.http.HttpServletRequest; import java.io.IOException; +import java.util.List; import java.util.Map; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -93,9 +96,12 @@ public class ClawHubCompatController { public ClawHubSkillListResponse listSkills(@RequestParam(defaultValue = "0") int page, @RequestParam(defaultValue = "25") int limit, @RequestParam(required = false) String sort, + @Parameter(description = "Optional response expansions. Supported value: labels") + @RequestParam(name = "include", required = false) List include, @RequestAttribute(value = "userId", required = false) String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { - return clawHubCompatAppService.listSkills(page, limit, sort, userId, userNsRoles); + return clawHubCompatAppService.listSkills( + page, limit, sort, IncludeOptions.includesLabels(include), userId, userNsRoles); } @RateLimit(category = "skills", authenticated = 60, anonymous = 20) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/dto/ClawHubSkillListResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/dto/ClawHubSkillListResponse.java index d1f7f376..f2bdea7e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/dto/ClawHubSkillListResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/dto/ClawHubSkillListResponse.java @@ -1,5 +1,7 @@ package com.iflytek.skillhub.compat.dto; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.iflytek.skillhub.dto.SkillLabelDto; import java.util.List; public record ClawHubSkillListResponse( @@ -14,8 +16,27 @@ public record ClawHubSkillListResponse( Object stats, long createdAt, long updatedAt, - LatestVersion latestVersion + LatestVersion latestVersion, + /** + * Labels attached to the skill, present only when the caller passes + * {@code include=labels}. Omitted otherwise, so the legacy payload is unchanged. + */ + @JsonInclude(JsonInclude.Include.NON_NULL) + List labels ) { + + public SkillListItem( + String slug, + String displayName, + String summary, + Object tags, + Object stats, + long createdAt, + long updatedAt, + LatestVersion latestVersion) { + this(slug, displayName, summary, tags, stats, createdAt, updatedAt, latestVersion, null); + } + public record LatestVersion( String version, long createdAt, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillSearchController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillSearchController.java index 36227b31..403560aa 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillSearchController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillSearchController.java @@ -1,16 +1,20 @@ package com.iflytek.skillhub.controller.portal; import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.controller.support.IncludeOptions; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; +import com.iflytek.skillhub.dto.SkillLabelDto; +import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.ratelimit.RateLimit; +import com.iflytek.skillhub.service.SkillLabelProjectionService; import com.iflytek.skillhub.service.SkillSearchAppService; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.media.Schema; -import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.*; +import java.util.List; import java.util.Map; import java.util.regex.Pattern; @@ -27,11 +31,14 @@ public class SkillSearchController extends BaseApiController { private static final int DEFAULT_SIZE = 20; private final SkillSearchAppService skillSearchAppService; + private final SkillLabelProjectionService skillLabelProjectionService; public SkillSearchController(SkillSearchAppService skillSearchAppService, + SkillLabelProjectionService skillLabelProjectionService, ApiResponseFactory responseFactory) { super(responseFactory); this.skillSearchAppService = skillSearchAppService; + this.skillLabelProjectionService = skillLabelProjectionService; } @GetMapping @@ -40,6 +47,8 @@ public class SkillSearchController extends BaseApiController { @RequestParam(required = false) String q, @RequestParam(required = false) String namespace, @RequestParam(name = "label", required = false) java.util.List labels, + @Parameter(description = "Optional response expansions. Supported value: labels") + @RequestParam(name = "include", required = false) List include, @Parameter(schema = @Schema(defaultValue = DEFAULT_SORT)) @RequestParam(required = false) String sort, @Parameter(schema = @Schema(type = "integer", defaultValue = "0", minimum = "0")) @@ -49,6 +58,7 @@ public class SkillSearchController extends BaseApiController { @RequestAttribute(value = "userId", required = false) String userId, @RequestAttribute(value = "userNsRoles", required = false) Map userNsRoles) { + boolean includeLabels = IncludeOptions.includesLabels(include); SkillSearchAppService.SearchResponse response = skillSearchAppService.search( q, namespace, @@ -60,7 +70,18 @@ public class SkillSearchController extends BaseApiController { userNsRoles ); - return ok("response.success.read", response); + return ok("response.success.read", includeLabels ? withLabels(response) : response); + } + + private SkillSearchAppService.SearchResponse withLabels(SkillSearchAppService.SearchResponse response) { + Map> labelsBySkillId = skillLabelProjectionService.labelsBySkillIds( + response.items().stream().map(SkillSummaryResponse::id).toList()); + + List items = response.items().stream() + .map(item -> item.withLabels(labelsBySkillId.getOrDefault(item.id(), List.of()))) + .toList(); + + return new SkillSearchAppService.SearchResponse(items, response.total(), response.page(), response.size()); } private String normalizeSort(String sort) { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/IncludeOptions.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/IncludeOptions.java new file mode 100644 index 00000000..72af5407 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/support/IncludeOptions.java @@ -0,0 +1,42 @@ +package com.iflytek.skillhub.controller.support; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import java.util.List; +import java.util.Locale; +import java.util.Set; + +/** + * Parses optional response expansions from {@code include=...} query parameters. + */ +public final class IncludeOptions { + + private static final String LABELS = "labels"; + private static final Set SUPPORTED = Set.of(LABELS); + + private IncludeOptions() { + } + + public static boolean includesLabels(List include) { + if (include == null || include.isEmpty()) { + return false; + } + + boolean requested = false; + for (String rawValue : include) { + if (rawValue == null || rawValue.isBlank()) { + continue; + } + for (String rawOption : rawValue.split(",")) { + String option = rawOption.trim().toLowerCase(Locale.ROOT); + if (option.isBlank()) { + continue; + } + if (!SUPPORTED.contains(option)) { + throw new DomainBadRequestException("error.request.include.unsupported", option); + } + requested = true; + } + } + return requested; + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillSummaryResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillSummaryResponse.java index 8011a12c..8717ba62 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillSummaryResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillSummaryResponse.java @@ -1,7 +1,9 @@ package com.iflytek.skillhub.dto; +import com.fasterxml.jackson.annotation.JsonInclude; import java.math.BigDecimal; import java.time.Instant; +import java.util.List; public record SkillSummaryResponse( Long id, @@ -21,5 +23,46 @@ public record SkillSummaryResponse( SkillLifecycleVersionResponse publishedVersion, SkillLifecycleVersionResponse ownerPreviewVersion, String resolutionMode, - ComplianceSnapshotResponse complianceSnapshot -) {} + ComplianceSnapshotResponse complianceSnapshot, + /** + * Labels attached to the skill, present only when the caller asked for them. + * Left out of the payload otherwise, so responses are unchanged for callers + * that do not opt in. + */ + @JsonInclude(JsonInclude.Include.NON_NULL) + List labels +) { + + /** + * Summary without label projection. + */ + public SkillSummaryResponse( + Long id, + String slug, + String displayName, + String summary, + String visibility, + String status, + Long downloadCount, + Integer starCount, + BigDecimal ratingAvg, + Integer ratingCount, + String namespace, + Instant updatedAt, + boolean canSubmitPromotion, + SkillLifecycleVersionResponse headlineVersion, + SkillLifecycleVersionResponse publishedVersion, + SkillLifecycleVersionResponse ownerPreviewVersion, + String resolutionMode, + ComplianceSnapshotResponse complianceSnapshot) { + this(id, slug, displayName, summary, visibility, status, downloadCount, starCount, ratingAvg, + ratingCount, namespace, updatedAt, canSubmitPromotion, headlineVersion, publishedVersion, + ownerPreviewVersion, resolutionMode, complianceSnapshot, null); + } + + public SkillSummaryResponse withLabels(List labels) { + return new SkillSummaryResponse(id, slug, displayName, summary, visibility, status, downloadCount, + starCount, ratingAvg, ratingCount, namespace, updatedAt, canSubmitPromotion, headlineVersion, + publishedVersion, ownerPreviewVersion, resolutionMode, complianceSnapshot, labels); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelProjectionService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelProjectionService.java new file mode 100644 index 00000000..a0292f34 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelProjectionService.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.domain.label.LabelDefinition; +import com.iflytek.skillhub.domain.label.LabelDefinitionService; +import com.iflytek.skillhub.domain.label.LabelTranslation; +import com.iflytek.skillhub.domain.label.SkillLabel; +import com.iflytek.skillhub.domain.label.SkillLabelService; +import com.iflytek.skillhub.dto.SkillLabelDto; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.springframework.stereotype.Service; + +/** + * Projects skill labels for a whole page of skills in a fixed number of queries. + * + *

Listing endpoints need labels for every item they return, so resolving them one + * skill at a time would issue three queries per row. This service batches the + * assignment, definition, and translation lookups instead.

+ */ +@Service +public class SkillLabelProjectionService { + + private final SkillLabelService skillLabelService; + private final LabelDefinitionService labelDefinitionService; + private final LabelLocalizationService labelLocalizationService; + + public SkillLabelProjectionService(SkillLabelService skillLabelService, + LabelDefinitionService labelDefinitionService, + LabelLocalizationService labelLocalizationService) { + this.skillLabelService = skillLabelService; + this.labelDefinitionService = labelDefinitionService; + this.labelLocalizationService = labelLocalizationService; + } + + /** + * Labels for each requested skill, keyed by skill id. Skills without labels are absent + * from the map rather than mapped to an empty list. + */ + public Map> labelsBySkillIds(List skillIds) { + if (skillIds == null || skillIds.isEmpty()) { + return Map.of(); + } + + List distinctSkillIds = skillIds.stream() + .filter(java.util.Objects::nonNull) + .distinct() + .toList(); + if (distinctSkillIds.isEmpty()) { + return Map.of(); + } + + List assignments = skillLabelService.listSkillLabelsBySkillIds(distinctSkillIds); + if (assignments.isEmpty()) { + return Map.of(); + } + + List labelIds = assignments.stream() + .map(SkillLabel::getLabelId) + .distinct() + .toList(); + Map definitionsById = labelDefinitionService.listByIds(labelIds).stream() + .collect(Collectors.toMap(LabelDefinition::getId, Function.identity())); + Map> translationsByLabelId = + labelDefinitionService.listTranslationsByLabelIds(labelIds); + + return assignments.stream() + .filter(assignment -> definitionsById.containsKey(assignment.getLabelId())) + .collect(Collectors.groupingBy( + SkillLabel::getSkillId, + Collectors.collectingAndThen( + Collectors.toList(), + skillAssignments -> skillAssignments.stream() + .map(assignment -> toDto( + definitionsById.get(assignment.getLabelId()), + translationsByLabelId)) + .sorted(Comparator.comparing(SkillLabelDto::type) + .thenComparing(SkillLabelDto::slug)) + .toList()))); + } + + private SkillLabelDto toDto(LabelDefinition definition, + Map> translationsByLabelId) { + return new SkillLabelDto( + definition.getSlug(), + definition.getType().name(), + labelLocalizationService.resolveDisplayName( + definition.getSlug(), + translationsByLabelId.getOrDefault(definition.getId(), List.of())) + ); + } +} diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 8791e6be..50493763 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -54,6 +54,7 @@ error.forbidden=Forbidden error.apiToken.scope.missing=API token is missing required scope: {0} error.apiToken.endpoint.unsupported=API token cannot access endpoint: {0} error.request.timeout=Request timed out +error.request.include.unsupported=Unsupported include option: {0} error.rateLimit.exceeded=Rate limit exceeded error.storage.unavailable=Object storage is temporarily unavailable. Please try again later. error.internal=An unexpected error occurred diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 0e1b3fc3..febc9c4e 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -54,6 +54,7 @@ error.forbidden=没有权限执行该操作 error.apiToken.scope.missing=API 令牌缺少所需权限范围：{0} error.apiToken.endpoint.unsupported=API 令牌无法访问接口：{0} error.request.timeout=请求超时 +error.request.include.unsupported=不支持的 include 参数：{0} error.rateLimit.exceeded=请求过于频繁，请稍后再试 error.storage.unavailable=对象存储暂时不可用，请稍后再试 error.internal=服务器内部错误 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java index 7c2a694f..b3a7f019 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatAppServiceTest.java @@ -15,8 +15,13 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; import com.iflytek.skillhub.domain.skill.service.SkillQueryService; import com.iflytek.skillhub.domain.social.SkillStarService; +import com.iflytek.skillhub.compat.dto.ClawHubSkillListResponse; +import com.iflytek.skillhub.dto.SkillLabelDto; +import com.iflytek.skillhub.dto.SkillSummaryResponse; import com.iflytek.skillhub.observability.RequestIdAccessor; +import com.iflytek.skillhub.service.SkillLabelProjectionService; import com.iflytek.skillhub.service.SkillSearchAppService; +import java.util.List; import java.util.Map; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -31,6 +36,7 @@ class ClawHubCompatAppServiceTest { private final AuditLogService auditLogService = mock(AuditLogService.class); private final CompatSkillLookupService compatSkillLookupService = mock(CompatSkillLookupService.class); private final SkillStarService skillStarService = mock(SkillStarService.class); + private final SkillLabelProjectionService skillLabelProjectionService = mock(SkillLabelProjectionService.class); private final ClawHubCompatAppService service = new ClawHubCompatAppService( new CanonicalSlugMapper(), @@ -42,7 +48,8 @@ class ClawHubCompatAppServiceTest { auditLogService, compatSkillLookupService, skillStarService, - new RequestIdAccessor() + new RequestIdAccessor(), + skillLabelProjectionService ); @Test @@ -120,4 +127,35 @@ class ClawHubCompatAppServiceTest { assertThat(location).isEqualTo("/api/v1/skills/team-a/my-skill/versions/20260707.025847/download"); } + + @Test + void listSkills_omitsLabelsByDefault() { + when(skillSearchAppService.search("", null, "newest", 0, 25, null, Map.of())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 25)); + + ClawHubSkillListResponse response = service.listSkills(0, 25, null, null, Map.of()); + + assertThat(response.items()).hasSize(1); + assertThat(response.items().get(0).labels()).isNull(); + } + + @Test + void listSkills_returnsLabelsWhenRequested() { + when(skillSearchAppService.search("", null, "newest", 0, 25, null, Map.of())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 25)); + when(skillLabelProjectionService.labelsBySkillIds(List.of(7L))) + .thenReturn(Map.of(7L, List.of(new SkillLabelDto("automation", "RECOMMENDED", "Automation")))); + + ClawHubSkillListResponse response = service.listSkills(0, 25, null, true, null, Map.of()); + + assertThat(response.items().get(0).labels()) + .extracting(SkillLabelDto::slug) + .containsExactly("automation"); + } + + private static SkillSummaryResponse summary(Long id) { + return new SkillSummaryResponse( + id, "demo-skill", "Demo Skill", "A demo", "PUBLIC", "PUBLISHED", + 0L, 0, null, 0, "global", null, false, null, null, null, null, null); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java index b07914a4..ac1abaff 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/compat/ClawHubCompatControllerTest.java @@ -19,7 +19,9 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.domain.user.UserAccount; import com.iflytek.skillhub.domain.user.UserAccountRepository; import com.iflytek.skillhub.dto.SkillLifecycleVersionResponse; +import com.iflytek.skillhub.dto.SkillLabelDto; import com.iflytek.skillhub.dto.SkillSummaryResponse; +import com.iflytek.skillhub.service.SkillLabelProjectionService; import com.iflytek.skillhub.service.SkillSearchAppService; import java.math.BigDecimal; import java.nio.charset.StandardCharsets; @@ -48,6 +50,7 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; @@ -72,6 +75,9 @@ class ClawHubCompatControllerTest { @MockBean private SkillSearchAppService skillSearchAppService; + @MockBean + private SkillLabelProjectionService skillLabelProjectionService; + @MockBean private SkillQueryService skillQueryService; @@ -154,6 +160,41 @@ class ClawHubCompatControllerTest { verify(apiTokenService).touchLastUsed(same(token)); } + @Test + void listSkills_shouldOmitLabelsByDefault() throws Exception { + when(skillSearchAppService.search(eq(""), isNull(), eq("newest"), eq(0), eq(25), isNull(), isNull())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 25)); + + mockMvc.perform(get("/api/v1/skills")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items[0].slug").value("demo-skill")) + .andExpect(jsonPath("$.items[0].labels").doesNotExist()); + } + + @Test + void listSkills_shouldReturnLabelsWhenIncluded() throws Exception { + when(skillSearchAppService.search(eq(""), isNull(), eq("newest"), eq(0), eq(25), isNull(), isNull())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 25)); + when(skillLabelProjectionService.labelsBySkillIds(List.of(7L))) + .thenReturn(java.util.Map.of( + 7L, + List.of(new SkillLabelDto("automation", "RECOMMENDED", "Automation")))); + + mockMvc.perform(get("/api/v1/skills").param("include", "labels")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.items[0].labels[0].slug").value("automation")) + .andExpect(jsonPath("$.items[0].labels[0].type").value("RECOMMENDED")) + .andExpect(jsonPath("$.items[0].labels[0].displayName").value("Automation")); + } + + @Test + void listSkills_shouldRejectUnsupportedIncludeOptions() throws Exception { + mockMvc.perform(get("/api/v1/skills").param("include", "labels,stats")) + .andExpect(status().isBadRequest()); + + verifyNoInteractions(skillSearchAppService); + } + @Test void downloadQuery_withBearerToken_shouldProjectNamespaceRolesIntoRequestContext() throws Exception { ApiToken token = new ApiToken("user-7", "cli", "sk_test", "hash", "[]"); @@ -401,6 +442,28 @@ class ClawHubCompatControllerTest { return version; } + private SkillSummaryResponse summary(Long id) { + return new SkillSummaryResponse( + id, + "demo-skill", + "Demo Skill", + "A demo", + "PUBLIC", + "PUBLISHED", + 0L, + 0, + null, + 0, + "global", + null, + false, + null, + null, + null, + null, + null); + } + private UsernamePasswordAuthenticationToken superAdminAuth() { PlatformPrincipal principal = new PlatformPrincipal( "user-42", diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillSearchControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillSearchControllerTest.java index bed761ad..2674fc2e 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillSearchControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillSearchControllerTest.java @@ -1,6 +1,9 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.dto.SkillLabelDto; +import com.iflytek.skillhub.dto.SkillSummaryResponse; +import com.iflytek.skillhub.service.SkillLabelProjectionService; import com.iflytek.skillhub.service.SkillSearchAppService; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -15,6 +18,7 @@ import java.util.Map; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; @@ -34,6 +38,9 @@ class SkillSearchControllerTest { @MockBean private SkillSearchAppService skillSearchAppService; + @MockBean + private SkillLabelProjectionService skillLabelProjectionService; + @Test void searchShouldUseUnifiedEnvelopeAndItemsField() throws Exception { when(skillSearchAppService.search( @@ -143,4 +150,59 @@ class SkillSearchControllerTest { .andExpect(jsonPath("$.data.page").value(0)) .andExpect(jsonPath("$.data.size").value(20)); } + + @Test + void searchShouldOmitLabelsUnlessRequested() throws Exception { + when(skillSearchAppService.search( + eq(null), eq(null), eq("newest"), eq(0), eq(20), eq(null), any(), any())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 20)); + + mockMvc.perform(get("/api/web/skills")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items[0].slug").value("demo-skill")) + .andExpect(jsonPath("$.data.items[0].labels").doesNotExist()); + } + + @Test + void searchShouldReturnLabelsWhenRequested() throws Exception { + when(skillSearchAppService.search( + eq(null), eq(null), eq("newest"), eq(0), eq(20), eq(null), any(), any())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 20)); + when(skillLabelProjectionService.labelsBySkillIds(List.of(7L))) + .thenReturn(Map.of(7L, List.of(new SkillLabelDto("automation", "TOPIC", "Automation")))); + + mockMvc.perform(get("/api/web/skills").param("include", "labels")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items[0].labels[0].slug").value("automation")) + .andExpect(jsonPath("$.data.items[0].labels[0].type").value("TOPIC")) + .andExpect(jsonPath("$.data.items[0].labels[0].displayName").value("Automation")); + } + + @Test + void searchShouldReturnEmptyLabelArrayForSkillsWithoutLabels() throws Exception { + when(skillSearchAppService.search( + eq(null), eq(null), eq("newest"), eq(0), eq(20), eq(null), any(), any())) + .thenReturn(new SkillSearchAppService.SearchResponse(List.of(summary(7L)), 1, 0, 20)); + when(skillLabelProjectionService.labelsBySkillIds(List.of(7L))).thenReturn(Map.of()); + + mockMvc.perform(get("/api/web/skills").param("include", "labels")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.data.items[0].labels").isArray()) + .andExpect(jsonPath("$.data.items[0].labels").isEmpty()); + } + + @Test + void searchShouldRejectUnsupportedIncludeOptions() throws Exception { + mockMvc.perform(get("/api/web/skills").param("include", "labels,stats")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value(400)); + + verifyNoInteractions(skillSearchAppService); + } + + private static SkillSummaryResponse summary(Long id) { + return new SkillSummaryResponse( + id, "demo-skill", "Demo Skill", "A demo", "PUBLIC", "PUBLISHED", + 0L, 0, null, 0, "global", null, false, null, null, null, null, null); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/IncludeOptionsTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/IncludeOptionsTest.java new file mode 100644 index 00000000..7379f7c0 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/support/IncludeOptionsTest.java @@ -0,0 +1,28 @@ +package com.iflytek.skillhub.controller.support; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import java.util.List; +import org.junit.jupiter.api.Test; + +class IncludeOptionsTest { + + @Test + void includesLabels_acceptsAbsentBlankRepeatedAndCommaSeparatedInputs() { + assertThat(IncludeOptions.includesLabels(null)).isFalse(); + assertThat(IncludeOptions.includesLabels(List.of("", " "))).isFalse(); + assertThat(IncludeOptions.includesLabels(List.of("labels"))).isTrue(); + assertThat(IncludeOptions.includesLabels(List.of(" LABELS "))).isTrue(); + assertThat(IncludeOptions.includesLabels(List.of("labels,"))).isTrue(); + assertThat(IncludeOptions.includesLabels(List.of("", "labels"))).isTrue(); + } + + @Test + void includesLabels_rejectsUnsupportedOptions() { + assertThatThrownBy(() -> IncludeOptions.includesLabels(List.of("labels,stats"))) + .isInstanceOf(DomainBadRequestException.class) + .hasMessage("error.request.include.unsupported"); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelProjectionServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelProjectionServiceTest.java new file mode 100644 index 00000000..55fa32dc --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLabelProjectionServiceTest.java @@ -0,0 +1,81 @@ +package com.iflytek.skillhub.service; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.label.LabelDefinition; +import com.iflytek.skillhub.domain.label.LabelDefinitionService; +import com.iflytek.skillhub.domain.label.LabelType; +import com.iflytek.skillhub.domain.label.SkillLabel; +import com.iflytek.skillhub.domain.label.SkillLabelService; +import com.iflytek.skillhub.dto.SkillLabelDto; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; + +class SkillLabelProjectionServiceTest { + + private final SkillLabelService skillLabelService = mock(SkillLabelService.class); + private final LabelDefinitionService labelDefinitionService = mock(LabelDefinitionService.class); + private final LabelLocalizationService labelLocalizationService = new LabelLocalizationService(); + + private final SkillLabelProjectionService service = new SkillLabelProjectionService( + skillLabelService, labelDefinitionService, labelLocalizationService); + + @Test + void labelsBySkillIds_groupsLabelsPerSkillInOneBatch() { + LabelDefinition automation = definition(10L, "automation", LabelType.RECOMMENDED); + LabelDefinition audited = definition(11L, "audited", LabelType.PRIVILEGED); + + when(skillLabelService.listSkillLabelsBySkillIds(List.of(1L, 2L))).thenReturn(List.of( + new SkillLabel(1L, 10L, "owner-1"), + new SkillLabel(1L, 11L, "owner-1"), + new SkillLabel(2L, 10L, "owner-2") + )); + when(labelDefinitionService.listByIds(anyList())).thenReturn(List.of(automation, audited)); + when(labelDefinitionService.listTranslationsByLabelIds(anyList())).thenReturn(Map.of()); + + Map> labels = service.labelsBySkillIds(List.of(1L, 2L)); + + // sorted by label type, then slug: PRIVILEGED before RECOMMENDED + assertEquals(List.of("audited", "automation"), labels.get(1L).stream().map(SkillLabelDto::slug).toList()); + assertEquals(List.of("automation"), labels.get(2L).stream().map(SkillLabelDto::slug).toList()); + + // One query per lookup for the whole page, not per skill. + verify(skillLabelService, times(1)).listSkillLabelsBySkillIds(anyList()); + verify(labelDefinitionService, times(1)).listByIds(anyList()); + verify(labelDefinitionService, times(1)).listTranslationsByLabelIds(anyList()); + } + + @Test + void labelsBySkillIds_skipsAssignmentsWithoutADefinition() { + when(skillLabelService.listSkillLabelsBySkillIds(anyList())) + .thenReturn(List.of(new SkillLabel(1L, 99L, "owner-1"))); + when(labelDefinitionService.listByIds(anyList())).thenReturn(List.of()); + when(labelDefinitionService.listTranslationsByLabelIds(anyList())).thenReturn(Map.of()); + + assertTrue(service.labelsBySkillIds(List.of(1L)).isEmpty()); + } + + @Test + void labelsBySkillIds_touchesNoRepositoryForAnEmptyPage() { + assertTrue(service.labelsBySkillIds(List.of()).isEmpty()); + assertTrue(service.labelsBySkillIds(null).isEmpty()); + + verify(skillLabelService, never()).listSkillLabelsBySkillIds(any()); + } + + private static LabelDefinition definition(Long id, String slug, LabelType type) { + LabelDefinition definition = new LabelDefinition(slug, type, true, 0, "admin"); + ReflectionTestUtils.setField(definition, "id", id); + return definition; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/label/SkillLabelService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/label/SkillLabelService.java index 9c699cb5..b0b66095 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/label/SkillLabelService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/label/SkillLabelService.java @@ -38,6 +38,13 @@ public class SkillLabelService { return skillLabelRepository.findBySkillId(skillId); } + public List listSkillLabelsBySkillIds(List skillIds) { + if (skillIds == null || skillIds.isEmpty()) { + return List.of(); + } + return skillLabelRepository.findBySkillIdIn(skillIds); + } + public List listByLabelId(Long labelId) { return skillLabelRepository.findByLabelId(labelId); } diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 3c4f5b8e..423ee239 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -4353,6 +4353,7 @@ export interface components { ownerPreviewVersion?: components["schemas"]["SkillLifecycleVersionResponse"]; resolutionMode?: string; complianceSnapshot?: components["schemas"]["ComplianceSnapshotResponse"]; + labels?: components["schemas"]["SkillLabelDto"][]; }; ApiResponseBoolean: { /** Format: int32 */ @@ -4980,6 +4981,7 @@ export interface components { /** Format: int64 */ updatedAt?: number; latestVersion?: components["schemas"]["LatestVersion"]; + labels?: components["schemas"]["SkillLabelDto"][]; }; ApiResponseListSecurityAuditResponse: { /** Format: int32 */ @@ -8002,6 +8004,8 @@ export interface operations { page?: number; limit?: number; sort?: string; + /** @description Optional response expansions. Supported value: labels */ + include?: string[]; }; header?: never; path?: never; @@ -9039,6 +9043,8 @@ export interface operations { q?: string; namespace?: string; label?: string[]; + /** @description Optional response expansions. Supported value: labels */ + include?: string[]; sort?: string; page?: number; size?: number;