Skip to content

feat(web): polish skill discovery cards and harden namespace lookups #450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
paradoxgacsd wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(web): polish skill discovery cards and harden namespace lookups #450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1c7737
Improve skill discovery cards and labels
paradoxgacsd May 16, 2026
c7bd0a1
Prevent missing namespace slug from causing server errors
paradoxgacsd May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.release.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ POSTGRES_PASSWORD=change-this-postgres-password

REDIS_BIND_ADDRESS=127.0.0.1
REDIS_PORT=6379
API_PORT=8080
API_PORT=8082
WEB_PORT=80
SESSION_COOKIE_SECURE=false

Expand Down
56 changes: 47 additions & 9 deletions ...lhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillPublishController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,25 @@
import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
import com.iflytek.skillhub.controller.BaseApiController;
import com.iflytek.skillhub.controller.support.SkillPackageArchiveExtractor;
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.skill.SkillVisibility;
import com.iflytek.skillhub.domain.skill.service.SkillPublishService;
import com.iflytek.skillhub.domain.skill.validation.PackageEntry;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.dto.PublishResponse;
import com.iflytek.skillhub.metrics.SkillHubMetrics;
import com.iflytek.skillhub.ratelimit.RateLimit;
import com.iflytek.skillhub.service.SkillPublishAppService;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile;

import java.io.IOException;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;

/**
* Upload endpoints for skill packages.
Expand All @@ -29,18 +33,18 @@
@RequestMapping({"/api/v1/skills", "/api/web/skills"})
public class SkillPublishController extends BaseApiController {

private final SkillPublishService skillPublishService;
private final SkillPackageArchiveExtractor skillPackageArchiveExtractor;
private final SkillHubMetrics skillHubMetrics;
private final SkillPublishAppService skillPublishAppService;

public SkillPublishController(SkillPublishService skillPublishService,
SkillPackageArchiveExtractor skillPackageArchiveExtractor,
public SkillPublishController(SkillPackageArchiveExtractor skillPackageArchiveExtractor,
ApiResponseFactory responseFactory,
SkillHubMetrics skillHubMetrics) {
SkillHubMetrics skillHubMetrics,
SkillPublishAppService skillPublishAppService) {
super(responseFactory);
this.skillPublishService = skillPublishService;
this.skillPackageArchiveExtractor = skillPackageArchiveExtractor;
this.skillHubMetrics = skillHubMetrics;
this.skillPublishAppService = skillPublishAppService;
}

/**
Expand All @@ -54,8 +58,14 @@ public ApiResponse<PublishResponse> publish(
@RequestParam("file") MultipartFile file,
@RequestParam("visibility") String visibility,
@RequestParam(value = "confirmWarnings", defaultValue = "false") boolean confirmWarnings,
@RequestParam(value = "labels", required = false) List<String> labels,
@RequestParam(value = "summary", required = false) String summary,
@RequestParam(value = "description", required = false) String description,
@RequestAttribute(value = "userNsRoles", required = false) Map<Long, NamespaceRole> userNsRoles,
@AuthenticationPrincipal PlatformPrincipal principal) throws IOException {

List<String> normalizedLabels = normalizeLabels(labels);
String summaryOverride = resolveSummaryOverride(summary, description);
SkillVisibility skillVisibility = SkillVisibility.valueOf(visibility.toUpperCase());

List<PackageEntry> entries;
Expand All @@ -75,14 +85,18 @@ public ApiResponse<PublishResponse> publish(
String.join("\n", extractionWarnings));
}

SkillPublishService.PublishResult publishResult = skillPublishService.publishFromEntries(
SkillPublishAppService.PublishOutcome publishOutcome = skillPublishAppService.publishFromEntries(
namespace,
entries,
principal.userId(),
skillVisibility,
principal.platformRoles(),
confirmWarnings
confirmWarnings,
summaryOverride,
normalizedLabels,
userNsRoles == null ? Map.of() : userNsRoles
);
var publishResult = publishOutcome.publishResult();

PublishResponse response = new PublishResponse(
publishResult.skillId(),
Expand All @@ -91,10 +105,34 @@ public ApiResponse<PublishResponse> publish(
publishResult.version().getVersion(),
publishResult.version().getStatus().name(),
publishResult.version().getFileCount(),
publishResult.version().getTotalSize()
publishResult.version().getTotalSize(),
publishOutcome.labels()
);
skillHubMetrics.incrementSkillPublish(namespace, publishResult.version().getStatus().name());

return ok("response.success.published", response);
}

private List<String> normalizeLabels(List<String> labels) {
if (labels == null) {
return Collections.emptyList();
}
return labels.stream()
.map(String::trim)
.filter(label -> !label.isBlank())
.collect(java.util.stream.Collectors.collectingAndThen(
java.util.stream.Collectors.toCollection(LinkedHashSet::new),
List::copyOf
));
}

private String resolveSummaryOverride(String summary, String description) {
if (summary != null && !summary.isBlank()) {
return summary.trim();
}
if (description != null && !description.isBlank()) {
return description.trim();
}
return null;
}
}
5 changes: 4 additions & 1 deletion server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/PublishResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.iflytek.skillhub.dto;

import java.util.List;

public record PublishResponse(
Long skillId,
String namespace,
String slug,
String version,
String status,
int fileCount,
long totalSize
long totalSize,
List<String> labels
) {}
4 changes: 3 additions & 1 deletion server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillSummaryResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.math.BigDecimal;
import java.time.Instant;
import java.util.List;

public record SkillSummaryResponse(
Long id,
Expand All @@ -20,5 +21,6 @@ public record SkillSummaryResponse(
SkillLifecycleVersionResponse headlineVersion,
SkillLifecycleVersionResponse publishedVersion,
SkillLifecycleVersionResponse ownerPreviewVersion,
String resolutionMode
String resolutionMode,
List<SkillLabelDto> labels
) {}
3 changes: 2 additions & 1 deletion ...skillhub-app/src/main/java/com/iflytek/skillhub/repository/JpaMySkillQueryRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,8 @@ private SkillSummaryResponse toSummaryResponse(Skill skill,
toLifecycleVersion(headlineVersion),
toLifecycleVersion(publishedVersion),
toLifecycleVersion(ownerPreviewVersion),
projection.resolutionMode().name()
projection.resolutionMode().name(),
List.of()
);
}

Expand Down
23 changes: 16 additions & 7 deletions ...lhub-app/src/main/java/com/iflytek/skillhub/service/NamespacePortalCommandAppService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.iflytek.skillhub.domain.namespace.NamespaceMemberService;
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
import com.iflytek.skillhub.domain.namespace.NamespaceService;
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.user.UserAccount;
import com.iflytek.skillhub.domain.user.UserAccountRepository;
import com.iflytek.skillhub.dto.BatchMemberRequest;
Expand Down Expand Up @@ -71,7 +72,7 @@ public NamespaceResponse createNamespace(NamespaceRequest request, PlatformPrinc

@Transactional
public NamespaceResponse updateNamespace(String slug, NamespaceRequest request, String userId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
Namespace updated = namespaceService.updateNamespace(
namespace.getId(),
request.displayName(),
Expand All @@ -84,7 +85,7 @@ public NamespaceResponse updateNamespace(String slug, NamespaceRequest request,

@Transactional
public MessageResponse deleteNamespace(String slug, String userId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
namespaceService.deleteNamespace(namespace.getId(), userId);
return new MessageResponse("Namespace deleted successfully");
}
Expand Down Expand Up @@ -147,7 +148,7 @@ public NamespaceResponse restoreNamespace(String slug, String userId, AuditReque

@Transactional
public MemberResponse addMember(String slug, String memberUserId, com.iflytek.skillhub.domain.namespace.NamespaceRole role, String operatorUserId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
NamespaceMember member = namespaceMemberService.addMember(
namespace.getId(),
memberUserId,
Expand All @@ -161,7 +162,7 @@ public MemberResponse addMember(String slug, String memberUserId, com.iflytek.sk
// Intentionally not @Transactional: each addMember runs in its own transaction
// so partial success is possible (some members added even if others fail).
public BatchMemberResponse batchAddMembers(String slug, List<MemberRequest> members, String operatorUserId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
Long namespaceId = namespace.getId();

List<BatchMemberResult> results = new ArrayList<>();
Expand Down Expand Up @@ -195,7 +196,7 @@ private String mapBatchError(Exception e) {

@Transactional
public MessageResponse removeMember(String slug, String memberUserId, String operatorUserId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
namespaceMemberService.removeMember(namespace.getId(), memberUserId, operatorUserId);
return new MessageResponse("Member removed successfully");
}
Expand All @@ -205,7 +206,7 @@ public MemberResponse updateMemberRole(String slug,
String userId,
UpdateMemberRoleRequest request,
String operatorUserId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
NamespaceMember member = namespaceMemberService.updateMemberRole(
namespace.getId(),
userId,
Expand All @@ -217,11 +218,19 @@ public MemberResponse updateMemberRole(String slug,

@Transactional
public MessageResponse transferOwnership(String slug, String newOwnerId, String currentOwnerId) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
Namespace namespace = requireNamespace(slug);
namespaceMemberService.transferOwnership(namespace.getId(), currentOwnerId, newOwnerId);
return new MessageResponse("Ownership transferred successfully");
}

private Namespace requireNamespace(String slug) {
Namespace namespace = namespaceService.getNamespaceBySlug(slug);
if (namespace == null) {
throw new DomainBadRequestException("error.namespace.slug.notFound", slug);
}
return namespace;
}

private boolean canCreateNamespace(PlatformPrincipal principal) {
return principal.platformRoles().contains("SKILL_ADMIN")
|| principal.platformRoles().contains("SUPER_ADMIN");
Expand Down
78 changes: 78 additions & 0 deletions server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ public List<SkillLabelDto> listSkillLabelsBySkillId(Long skillId) {
return toDtos(skillLabelService.listSkillLabels(skillId));
}

public Map<Long, List<SkillLabelDto>> listSkillLabelsBySkillIds(List<Long> skillIds) {
if (skillIds == null || skillIds.isEmpty()) {
return Map.of();
}
List<Long> normalizedSkillIds = skillIds.stream()
.filter(java.util.Objects::nonNull)
.distinct()
.toList();
if (normalizedSkillIds.isEmpty()) {
return Map.of();
}

Map<Long, List<SkillLabel>> labelsBySkillId = skillLabelService.listSkillLabelsBySkillIds(normalizedSkillIds)
.stream()
.collect(Collectors.groupingBy(SkillLabel::getSkillId));
return toDtosBySkillId(labelsBySkillId);
}

@Transactional
public SkillLabelDto attachLabel(String namespaceSlug,
String skillSlug,
Expand All @@ -97,6 +115,31 @@ public SkillLabelDto attachLabel(String namespaceSlug,
return toDtos(List.of(attached)).getFirst();
}

@Transactional
public List<SkillLabelDto> attachLabels(String namespaceSlug,
String skillSlug,
List<String> labelSlugs,
String userId,
Map<Long, NamespaceRole> userNsRoles,
AuditRequestContext auditContext) {
if (labelSlugs == null || labelSlugs.isEmpty()) {
return List.of();
}
Skill skill = resolveSkill(namespaceSlug, skillSlug, userId);
List<SkillLabel> attachedLabels = skillLabelService.attachLabels(
skill.getId(),
labelSlugs,
userId,
normalizeRoles(userNsRoles),
platformRoles(userId)
);
afterCommit(() -> labelSearchSyncService.rebuildSkill(skill.getId()));
for (String labelSlug : labelSlugs) {
recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}");
}
return toDtos(attachedLabels);
Comment on lines +137 to +140
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

此处审计日志记录逻辑存在以下问题：

  1. 记录了未经验证的输入：循环遍历的是原始输入的 labelSlugs，这可能包含重复项、无效项或最终因权限等原因未实际绑定的标签。建议在调用 skillLabelService.attachLabels 成功后，遍历返回的已绑定标签列表进行记录，以确保审计日志的准确性。
  2. JSON 构造不安全：手动拼接 JSON 字符串（"{"labelSlug":"" + labelSlug + ""}"）在处理包含特殊字符（如引号、反斜杠）的输入时会生成非法的 JSON。建议使用可靠的 JSON 序列化方式或确保输入已通过严格校验。
        List<SkillLabelDto> attachedDtos = toDtos(attachedLabels);
        for (SkillLabelDto dto : attachedDtos) {
            recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + dto.slug() + "\"}");
        }
        return attachedDtos;

}

@Transactional
public MessageResponse detachLabel(String namespaceSlug,
String skillSlug,
Expand Down Expand Up @@ -144,6 +187,41 @@ private List<SkillLabelDto> toDtos(List<SkillLabel> skillLabels) {
.toList();
}

private Map<Long, List<SkillLabelDto>> toDtosBySkillId(Map<Long, List<SkillLabel>> labelsBySkillId) {
if (labelsBySkillId.isEmpty()) {
return Map.of();
}
List<SkillLabel> allSkillLabels = labelsBySkillId.values().stream()
.flatMap(List::stream)
.toList();
List<Long> labelIds = allSkillLabels.stream()
.map(SkillLabel::getLabelId)
.distinct()
.toList();
Map<Long, LabelDefinition> definitionsById = labelDefinitionService.listByIds(labelIds).stream()
.collect(Collectors.toMap(LabelDefinition::getId, Function.identity()));
Map<Long, List<LabelTranslation>> translationsByLabelId = labelDefinitionService.listTranslationsByLabelIds(labelIds);

return labelsBySkillId.entrySet().stream()
.collect(Collectors.toMap(
Map.Entry::getKey,
entry -> entry.getValue().stream()
.filter(skillLabel -> definitionsById.containsKey(skillLabel.getLabelId()))
.map(skillLabel -> {
LabelDefinition definition = definitionsById.get(skillLabel.getLabelId());
return new SkillLabelDto(
definition.getSlug(),
definition.getType().name(),
labelLocalizationService.resolveDisplayName(
definition.getSlug(),
translationsByLabelId.getOrDefault(definition.getId(), List.of()))
);
})
.sorted(java.util.Comparator.comparing(SkillLabelDto::type).thenComparing(SkillLabelDto::slug))
.toList()
));
}

private Skill resolveSkill(String namespaceSlug, String skillSlug, String userId) {
Namespace namespace = namespaceRepository.findBySlug(namespaceSlug)
.orElseThrow(() -> new DomainBadRequestException("error.namespace.slug.notFound", namespaceSlug));
Expand Down
Loading