scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-10-16 04:28:39 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(publish): keep version display metadata consistent across lifecycle

Browse source 
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-10-15 13:52:17 +08:00
parent 3b61d2365f
commit 6b85e96326
10 changed files with 167 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
docs/07-skill-protocol.md
View file

@ -41,6 +41,7 @@ description: When to use # 必需，1-2 句话
- `name` 和 `description` 为必需字段，缺失则校验失败
- `name` 映射为 `skill.slug`（首次发布时），后续版本不可变更
- 可选的 `metadata.display-name` 映射为 `skill.display_name`，不参与 slug 生成。必须是字符串，去除首尾空白后最多 200 个字符；缺失、null 或空白时回退到 `name`。展示名沿用版本发布/审核流程，待审核更新不覆盖已发布标题；撤下最新版本并回退到旧版本时，标题和摘要同步恢复到该旧版本。
- 上述类型和长度限制在新上传解析时检查。读取历史版本快照时，非字符串或超长的旧 `display-name` 回退到该版本的 `name`，原始 frontmatter 保留，避免阻塞审核、提升或版本撤回。
- `description` 映射为 `skill.summary`
- frontmatter 完整解析结果存入 `skill_version.parsed_metadata_json`

15
server/skillhub-app/src/test/java/com/iflytek/skillhub/service/bundle/SkillSuiteBundlePackageAnalyzerTest.java
View file

@ -61,6 +61,21 @@ class SkillSuiteBundlePackageAnalyzerTest {
});
}
@Test
void rejectsInvalidDisplayNamesInNewBundlePackages() throws Exception {
for (String invalidValue : new String[] {"123", "中".repeat(201)}) {
String skill = "---\nname: first\ndescription: Test\nversion: 1.0.0\nmetadata:\n display-name: "
+ invalidValue + "\n---\nBody";
SkillSuiteBundlePackageAnalyzer.BundleAnalysis result = analyzer.analyze(List.of(
entry("SUITE.yaml", manifest(packageMember("first"), "@global/first")),
entry("skills/first/SKILL.md", skill)
));
assertThat(result.packageMembers()).singleElement().satisfies(member ->
assertThat(member.validation().passed()).isFalse());
}
}
@Test
void reportsMissingRootSkillMdAndDoesNotPromoteNestedOne() throws Exception {
SkillSuiteBundlePackageAnalyzer.BundleAnalysis result = analyzer.analyze(List.of(

29
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java
View file

@ -1,5 +1,6 @@
package com.iflytek.skillhub.domain.review;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.domain.audit.AuditDetail;
import com.iflytek.skillhub.domain.event.PromotionApprovedEvent;
import com.iflytek.skillhub.domain.event.PromotionRejectedEvent;
@ -17,6 +18,7 @@ import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException;
import com.iflytek.skillhub.domain.skill.*;
import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata;
import jakarta.persistence.EntityManager;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.dao.DataIntegrityViolationException;
@ -51,6 +53,7 @@ public class PromotionService {
private final GovernanceNotificationService governanceNotificationService;
private final EntityManager entityManager;
private final Clock clock;
private final ObjectMapper objectMapper;
public PromotionService(PromotionRequestRepository promotionRequestRepository,
SkillRepository skillRepository,
@ -62,7 +65,8 @@ public class PromotionService {
ApplicationEventPublisher eventPublisher,
GovernanceNotificationService governanceNotificationService,
EntityManager entityManager,
Clock clock) {
Clock clock,
ObjectMapper objectMapper) {
this.promotionRequestRepository = promotionRequestRepository;
this.skillRepository = skillRepository;
this.skillVersionRepository = skillVersionRepository;
@ -74,6 +78,7 @@ public class PromotionService {
this.governanceNotificationService = governanceNotificationService;
this.entityManager = entityManager;
this.clock = clock;
this.objectMapper = objectMapper;
}
/**
@ -217,8 +222,7 @@ public class PromotionService {
// Serialize approval with ordinary uploads that lock target Skill before publishing.
entityManager.lock(targetSkill, jakarta.persistence.LockModeType.PESSIMISTIC_WRITE);
assertTargetVersionAvailable(targetSkill.getId(), sourceVersion.getVersion());
targetSkill.setDisplayName(sourceSkill.getDisplayName());
targetSkill.setSummary(sourceSkill.getSummary());
applySourceVersionMetadata(targetSkill, sourceSkill, sourceVersion);
} else {
if (promotionRequestRepository.findActiveInitialBySourceSkillId(sourceSkill.getId()).isPresent()) {
throw new DomainBadRequestException("promotion.already_promoted", sourceSkill.getId());
@ -226,8 +230,7 @@ public class PromotionService {
assertTargetSkillNotExists(approvedRequest, sourceSkill);
targetSkill = new Skill(approvedRequest.getTargetNamespaceId(), sourceSkill.getSlug(),
sourceSkill.getOwnerId(), SkillVisibility.PUBLIC);
targetSkill.setDisplayName(sourceSkill.getDisplayName());
targetSkill.setSummary(sourceSkill.getSummary());
applySourceVersionMetadata(targetSkill, sourceSkill, sourceVersion);
targetSkill.setSourceSkillId(sourceSkill.getId());
targetSkill.setCreatedBy(reviewerId);
targetSkill.setUpdatedBy(reviewerId);
@ -293,6 +296,22 @@ public class PromotionService {
return savedRequest;
}
private void applySourceVersionMetadata(Skill target, Skill source, SkillVersion sourceVersion) {
String metadataJson = sourceVersion.getParsedMetadataJson();
if (metadataJson == null || metadataJson.isBlank()) {
target.setDisplayName(source.getDisplayName());
target.setSummary(source.getSummary());
return;
}
try {
SkillMetadata metadata = objectMapper.readValue(metadataJson, SkillMetadata.class);
target.setDisplayName(metadata.displayName());
target.setSummary(metadata.description());
} catch (Exception e) {
throw new IllegalStateException("Failed to deserialize skill metadata", e);
}
}
private void assertTargetSkillNotExists(PromotionRequest approvedRequest, Skill sourceSkill) {
for (Skill existing : skillRepository.findByNamespaceIdAndSlug(
approvedRequest.getTargetNamespaceId(), sourceSkill.getSlug())) {

11
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillMetadata.java
View file

@ -1,6 +1,5 @@
package com.iflytek.skillhub.domain.skill.metadata;
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
import java.util.Map;
public record SkillMetadata(
@ -10,11 +9,7 @@ public record SkillMetadata(
String body,
Map<String, Object> frontmatter
) {
public SkillMetadata {
resolveDisplayName(name, frontmatter);
}
/** Human-readable label; the package name remains the source of the skill slug. */
/** Human-readable label; unsupported legacy snapshot values fall back to the package name. */
public String displayName() {
return resolveDisplayName(name, frontmatter);
}
@ -28,14 +23,14 @@ public record SkillMetadata(
return name;
}
if (!(value instanceof String text)) {
throw new DomainBadRequestException("error.skill.metadata.displayName.invalid");
return name;
}
String displayName = text.strip();
if (displayName.isEmpty()) {
return name;
}
if (displayName.length() > 200) {
throw new DomainBadRequestException("error.skill.metadata.displayName.tooLong", 200);
return name;
}
return displayName;
}

17
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/metadata/SkillMetadataParser.java
View file

@ -59,9 +59,26 @@ public class SkillMetadataParser {
version = extractNestedOptionalField(frontmatter, "metadata", "version");
}
validateDisplayName(frontmatter);
return new SkillMetadata(name, description, version, body, frontmatter);
}
private void validateDisplayName(Map<String, Object> frontmatter) {
if (!(frontmatter.get("metadata") instanceof Map<?, ?> metadata)) {
return;
}
Object value = metadata.get("display-name");
if (value == null) {
return;
}
if (!(value instanceof String text)) {
throw new DomainBadRequestException("error.skill.metadata.displayName.invalid");
}
if (text.strip().length() > 200) {
throw new DomainBadRequestException("error.skill.metadata.displayName.tooLong", 200);
}
}
private Map<String, Object> parseFrontmatter(String yamlContent) {
try {
LoaderOptions loaderOptions = new LoaderOptions();

41
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/PromotionServiceTest.java
View file

@ -1,5 +1,6 @@
package com.iflytek.skillhub.domain.review;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.iflytek.skillhub.domain.event.SkillPublishedEvent;
import com.iflytek.skillhub.domain.governance.GovernanceNotificationService;
import com.iflytek.skillhub.domain.namespace.Namespace;
@ -16,6 +17,8 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
@ -64,7 +67,7 @@ class PromotionServiceTest {
promotionService = new PromotionService(
promotionRequestRepository, skillRepository, skillVersionRepository,
skillFileRepository, namespaceRepository, namespaceMemberRepository, permissionChecker,
eventPublisher, governanceNotificationService, entityManager, CLOCK);
eventPublisher, governanceNotificationService, entityManager, CLOCK, new ObjectMapper());
lenient().when(namespaceRepository.findById(5L)).thenReturn(Optional.of(createSourceNamespace()));
}
@ -100,6 +103,13 @@ class PromotionServiceTest {
return sv;
}
private String selectedVersionMetadata(boolean hasDisplayName) {
return """
{"name":"old-package-name","description":"Old version summary",
"frontmatter":{"metadata":{"display-name":"%s"}}}
""".formatted(hasDisplayName ? " 旧版标题 " : " ");
}
private Namespace createGlobalNamespace() {
Namespace ns = new Namespace("global", "Global", "user-1");
setField(ns, "id", TARGET_NAMESPACE_ID);
@ -655,12 +665,15 @@ class PromotionServiceTest {
assertEquals("promotion.target_skill_conflict", ex.messageCode());
}
@Test
void shouldCopyDisplayNameAndSummaryToNewSkill() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void shouldCopySelectedVersionMetadataToNewSkill(boolean hasDisplayName) {
PromotionRequest pr = createPendingPromotion();
PromotionRequest approvedRequest = approvedPromotion(pr, "ok");
Skill sourceSkill = createSourceSkill();
SkillVersion sourceVersion = createPublishedVersion();
sourceSkill.setLatestVersionId(999L);
sourceVersion.setParsedMetadataJson(selectedVersionMetadata(hasDisplayName));
when(promotionRequestRepository.findById(PROMOTION_ID))
.thenReturn(Optional.of(pr), Optional.of(approvedRequest));
@ -686,8 +699,9 @@ class PromotionServiceTest {
ArgumentCaptor<Skill> skillCaptor = ArgumentCaptor.forClass(Skill.class);
verify(skillRepository, times(2)).save(skillCaptor.capture());
Skill newSkill = skillCaptor.getAllValues().get(0);
assertEquals("My Skill", newSkill.getDisplayName());
assertEquals("A test skill", newSkill.getSummary());
assertEquals(hasDisplayName ? "旧版标题" : "old-package-name", newSkill.getDisplayName());
assertEquals("Old version summary", newSkill.getSummary());
assertEquals(NEW_VERSION_ID, newSkill.getLatestVersionId());
}
}
@ -784,8 +798,9 @@ class PromotionServiceTest {
assertEquals("2.0.0", state.targetCurrentVersion());
}
@Test
void approvesUpdateWithoutCreatingAnotherGlobalSkill() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
void approvesUpdateUsingSelectedVersionMetadata(boolean hasDisplayName) {
PromotionRequest pending = createPendingPromotion();
pending.setRequestKind(PromotionRequestKind.UPDATE);
pending.setTargetSkillId(NEW_SKILL_ID);
@ -808,8 +823,12 @@ class PromotionServiceTest {
PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "approved",
NEW_SKILL_ID, pending.getVersion()))
.thenReturn(1);
when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(createSourceSkill()));
when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(createPublishedVersion()));
Skill source = createSourceSkill();
source.setLatestVersionId(998L);
SkillVersion sourceVersion = createPublishedVersion();
sourceVersion.setParsedMetadataJson(selectedVersionMetadata(hasDisplayName));
when(skillRepository.findById(SOURCE_SKILL_ID)).thenReturn(Optional.of(source));
when(skillVersionRepository.findById(SOURCE_VERSION_ID)).thenReturn(Optional.of(sourceVersion));
when(promotionRequestRepository.findActiveInitialBySourceSkillId(SOURCE_SKILL_ID))
.thenReturn(Optional.of(initialLink()));
when(skillRepository.findById(NEW_SKILL_ID)).thenReturn(Optional.of(target));
@ -823,8 +842,8 @@ class PromotionServiceTest {
assertEquals(NEW_SKILL_ID, result.getTargetSkillId());
assertEquals(NEW_VERSION_ID, result.getTargetVersionId());
assertEquals(NEW_VERSION_ID, target.getLatestVersionId());
assertEquals("My Skill", target.getDisplayName());
assertEquals("A test skill", target.getSummary());
assertEquals(hasDisplayName ? "旧版标题" : "old-package-name", target.getDisplayName());
assertEquals("Old version summary", target.getSummary());
verify(promotionRequestRepository).updateStatusWithVersion(
PROMOTION_ID, ReviewTaskStatus.APPROVED, REVIEWER_ID, "approved",
NEW_SKILL_ID, pending.getVersion());

19
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/metadata/SkillMetadataParserTest.java
View file

@ -42,6 +42,25 @@ class SkillMetadataParserTest {
assertEquals("old-skill", new SkillMetadata("old-skill", "Test", "1.0.0", "Body", null).displayName());
}
@Test
void readsLegacyDisplayNameSnapshotsWithoutDiscardingRawValues() throws Exception {
var mapper = new com.fasterxml.jackson.databind.ObjectMapper();
for (Object legacyValue : new Object[] {123, true, java.util.List.of("one", "two"),
Map.of("label", "test"), "中".repeat(201)}) {
String snapshot = mapper.writeValueAsString(Map.of(
"name", "old-skill", "description", "Historical summary",
"frontmatter", Map.of("metadata", Map.of("display-name", legacyValue))));
SkillMetadata metadata = mapper.readValue(snapshot, SkillMetadata.class);
assertEquals("old-skill", metadata.displayName());
assertEquals(legacyValue,
((Map<?, ?>) metadata.frontmatter().get("metadata")).get("display-name"));
assertEquals(mapper.readTree(snapshot).get("frontmatter"),
mapper.readTree(mapper.writeValueAsString(metadata)).get("frontmatter"));
}
}
@Test
void rejectsNonStringDisplayNames() {
for (String value : new String[] {"123", "true", "[one, two]", "{label: test}"}) {

15
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java
View file

@ -255,8 +255,8 @@ class SkillGovernanceServiceTest {
}
@ParameterizedTest
@ValueSource(booleans = {false, true})
void yankVersion_restoresLatestPublishedMetadata(boolean hasDisplayName) throws Exception {
@ValueSource(strings = {"missing", "custom", "number", "tooLong"})
void yankVersion_restoresLatestPublishedMetadata(String displayNameCase) throws Exception {
SkillVersion yanked = new SkillVersion(2L, "2.0.0", "owner");
setField(yanked, "id", 22L);
yanked.setStatus(SkillVersionStatus.PUBLISHED);
@ -269,9 +269,12 @@ class SkillGovernanceServiceTest {
fallback.setParsedMetadataJson(new ObjectMapper().writeValueAsString(Map.of(
"name", "example-skill",
"description", "Published summary from v1",
"frontmatter", hasDisplayName
? Map.of("metadata", Map.of("display-name", "示例技能一"))
: Map.of())));
"frontmatter", switch (displayNameCase) {
case "custom" -> Map.of("metadata", Map.of("display-name", "示例技能一"));
case "number" -> Map.of("metadata", Map.of("display-name", 123));
case "tooLong" -> Map.of("metadata", Map.of("display-name", "中".repeat(201)));
default -> Map.of();
})));
Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC);
setField(skill, "id", 2L);
@ -288,7 +291,7 @@ class SkillGovernanceServiceTest {
service.yankVersion(22L, "admin", "127.0.0.1", "JUnit", "broken");
assertThat(skill.getLatestVersionId()).isEqualTo(11L);
assertThat(skill.getDisplayName()).isEqualTo(hasDisplayName ? "示例技能一" : "example-skill");
assertThat(skill.getDisplayName()).isEqualTo(displayNameCase.equals("custom") ? "示例技能一" : "example-skill");
assertThat(skill.getSummary()).isEqualTo("Published summary from v1");
verify(skillRepository).save(skill);
}

7
server/skillhub-search/src/main/java/com/iflytek/skillhub/search/event/SearchIndexEventListener.java
View file

@ -2,6 +2,7 @@ package com.iflytek.skillhub.search.event;
import com.iflytek.skillhub.domain.event.SkillPublishedEvent;
import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent;
import com.iflytek.skillhub.domain.event.SkillVersionYankedEvent;
import com.iflytek.skillhub.domain.skill.SkillStatus;
import com.iflytek.skillhub.search.SearchIndexService;
import com.iflytek.skillhub.search.SearchRebuildService;
@ -32,6 +33,12 @@ public class SearchIndexEventListener {
searchRebuildService.rebuildBySkill(event.skillId());
}
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
@Async("skillhubEventExecutor")
public void onSkillVersionYanked(SkillVersionYankedEvent event) {
searchRebuildService.rebuildBySkill(event.skillId());
}
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
@Async("skillhubEventExecutor")
public void onSkillStatusChanged(SkillStatusChangedEvent event) {

42
server/skillhub-search/src/test/java/com/iflytek/skillhub/search/event/SearchIndexEventListenerTest.java
View file

@ -1,14 +1,21 @@
package com.iflytek.skillhub.search.event;
import com.iflytek.skillhub.domain.event.SkillPublishedEvent;
import com.iflytek.skillhub.domain.event.SkillVersionYankedEvent;
import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent;
import com.iflytek.skillhub.domain.skill.SkillStatus;
import com.iflytek.skillhub.search.SearchIndexService;
import com.iflytek.skillhub.search.SearchRebuildService;
import org.junit.jupiter.api.Test;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.transaction.event.TransactionalEventListenerFactory;
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.transaction.support.TransactionSynchronizationUtils;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
class SearchIndexEventListenerTest {
@ -23,6 +30,41 @@ class SearchIndexEventListenerTest {
verify(searchRebuildService).rebuildBySkill(42L);
}
@Test
void yankedVersionShouldRebuildOnlyAfterCommit() {
assertYankTransactionOutcome(TransactionSynchronization.STATUS_COMMITTED);
}
@Test
void rolledBackYankShouldNotRebuildSearchDocument() {
assertYankTransactionOutcome(TransactionSynchronization.STATUS_ROLLED_BACK);
}
private void assertYankTransactionOutcome(int completionStatus) {
SearchRebuildService rebuildService = mock(SearchRebuildService.class);
try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext()) {
context.registerBean(TransactionalEventListenerFactory.class);
context.registerBean(SearchIndexEventListener.class,
() -> new SearchIndexEventListener(rebuildService, mock(SearchIndexService.class)));
context.refresh();
TransactionSynchronizationManager.setActualTransactionActive(true);
TransactionSynchronizationManager.initSynchronization();
try {
context.publishEvent(new SkillVersionYankedEvent(42L, 100L, "owner-1", true));
verifyNoInteractions(rebuildService);
TransactionSynchronizationUtils.triggerAfterCompletion(completionStatus);
if (completionStatus == TransactionSynchronization.STATUS_COMMITTED) {
verify(rebuildService).rebuildBySkill(42L);
} else {
verifyNoInteractions(rebuildService);
}
} finally {
TransactionSynchronizationManager.clearSynchronization();
TransactionSynchronizationManager.setActualTransactionActive(false);
}
}
}
@Test
void archivedStatusShouldRemoveSearchDocument() {
SearchRebuildService searchRebuildService = mock(SearchRebuildService.class);