From 7bc152a744e2b049bf14bf7424b0029d641249b1 Mon Sep 17 00:00:00 2001 From: yun-zhi-ztl <15071461069@163.com> Date: Fri, 13 Mar 2026 10:01:01 +0800 Subject: [PATCH] fix(publish): keep latest pointer aligned with published versions - stop publish submission from advancing skill.latestVersionId or overwriting published-facing skill metadata while a version is still pending review\n- move latest pointer and display metadata promotion into review approval so the public skill record changes only when a version becomes PUBLISHED\n- keep SkillPublishedEvent emission on review approval only, preserving search rebuild semantics for published versions\n- add regression coverage for pending review submissions retaining published metadata and for approval promoting latest pointer plus display fields --- .../skillhub/domain/review/ReviewService.java | 24 ++++- .../skill/service/SkillPublishService.java | 14 +-- .../domain/review/ReviewServiceTest.java | 15 ++- .../service/SkillPublishServiceTest.java | 94 ++++++++++++++++++- 4 files changed, 128 insertions(+), 19 deletions(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index 8e2f6308..205a2054 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.review; +import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; @@ -12,6 +13,7 @@ import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; import org.springframework.context.ApplicationEventPublisher; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; @@ -31,19 +33,22 @@ public class ReviewService { private final NamespaceRepository namespaceRepository; private final ReviewPermissionChecker permissionChecker; private final ApplicationEventPublisher eventPublisher; + private final ObjectMapper objectMapper; public ReviewService(ReviewTaskRepository reviewTaskRepository, SkillVersionRepository skillVersionRepository, SkillRepository skillRepository, NamespaceRepository namespaceRepository, ReviewPermissionChecker permissionChecker, - ApplicationEventPublisher eventPublisher) { + ApplicationEventPublisher eventPublisher, + ObjectMapper objectMapper) { this.reviewTaskRepository = reviewTaskRepository; this.skillVersionRepository = skillVersionRepository; this.skillRepository = skillRepository; this.namespaceRepository = namespaceRepository; this.permissionChecker = permissionChecker; this.eventPublisher = eventPublisher; + this.objectMapper = objectMapper; } @Transactional @@ -109,6 +114,8 @@ public class ReviewService { Skill skill = skillRepository.findById(skillVersion.getSkillId()) .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillVersion.getSkillId())); skill.setLatestVersionId(skillVersion.getId()); + applyPublishedMetadata(skill, skillVersion); + skill.setUpdatedBy(reviewerId); skillRepository.save(skill); eventPublisher.publishEvent(new SkillPublishedEvent( @@ -168,4 +175,19 @@ public class ReviewService { skillVersion.setStatus(SkillVersionStatus.DRAFT); skillVersionRepository.save(skillVersion); } + + private void applyPublishedMetadata(Skill skill, SkillVersion skillVersion) { + String metadataJson = skillVersion.getParsedMetadataJson(); + if (metadataJson == null || metadataJson.isBlank()) { + return; + } + + try { + SkillMetadata metadata = objectMapper.readValue(metadataJson, SkillMetadata.class); + skill.setDisplayName(metadata.name()); + skill.setSummary(metadata.description()); + } catch (Exception e) { + throw new IllegalStateException("Failed to deserialize skill metadata", e); + } + } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java index 2d6fa0bb..ec693df4 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.domain.skill.service; import com.fasterxml.jackson.databind.ObjectMapper; -import com.iflytek.skillhub.domain.event.SkillPublishedEvent; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; @@ -214,17 +213,8 @@ public class SkillPublishService { version.setTotalSize(totalSize); skillVersionRepository.save(version); - // 12. Update skill - skill.setLatestVersionId(version.getId()); - skill.setDisplayName(metadata.name()); - skill.setSummary(metadata.description()); - skill.setUpdatedBy(publisherId); - skillRepository.save(skill); - - // 13. Publish SkillPublishedEvent - eventPublisher.publishEvent(new SkillPublishedEvent(skill.getId(), version.getId(), publisherId)); - - // 14. Return published identifiers + // 12. Return published identifiers. Published-facing skill metadata is + // advanced only when review approval promotes this version to PUBLISHED. return new PublishResult(skill.getId(), skill.getSlug(), version); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java index c9325329..fd6815ae 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/review/ReviewServiceTest.java @@ -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.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; @@ -13,6 +14,7 @@ import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.metadata.SkillMetadata; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -50,12 +52,14 @@ class ReviewServiceTest { private static final String REVIEWER_ID = "user-200"; private static final Long REVIEW_TASK_ID = 1L; private static final Long SKILL_ID = 30L; + private ObjectMapper objectMapper; @BeforeEach void setUp() { + objectMapper = new ObjectMapper(); reviewService = new ReviewService( reviewTaskRepository, skillVersionRepository, skillRepository, - namespaceRepository, permissionChecker, eventPublisher); + namespaceRepository, permissionChecker, eventPublisher, objectMapper); } private SkillVersion createDraftSkillVersion() { @@ -186,6 +190,12 @@ class ReviewServiceTest { Namespace ns = createTeamNamespace(); SkillVersion sv = createPendingReviewSkillVersion(); Skill skill = createSkill(); + skill.setDisplayName("Published Name"); + skill.setSummary("Published Summary"); + skill.setUpdatedBy("previous-reviewer"); + assertDoesNotThrow(() -> sv.setParsedMetadataJson(objectMapper.writeValueAsString( + new SkillMetadata("Approved Name", "Approved Summary", "1.0.0", "Body", Map.of()) + ))); when(reviewTaskRepository.findById(REVIEW_TASK_ID)).thenReturn(Optional.of(task)); when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(ns)); @@ -206,6 +216,9 @@ class ReviewServiceTest { assertEquals(SkillVersionStatus.PUBLISHED, sv.getStatus()); assertNotNull(sv.getPublishedAt()); assertEquals(SKILL_VERSION_ID, skill.getLatestVersionId()); + assertEquals("Approved Name", skill.getDisplayName()); + assertEquals("Approved Summary", skill.getSummary()); + assertEquals(REVIEWER_ID, skill.getUpdatedBy()); verify(eventPublisher).publishEvent(any(SkillPublishedEvent.class)); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java index 416d441e..f0881441 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java @@ -1,7 +1,6 @@ package com.iflytek.skillhub.domain.skill.service; import com.fasterxml.jackson.databind.ObjectMapper; -import com.iflytek.skillhub.domain.event.SkillPublishedEvent; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceMember; import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; @@ -19,7 +18,6 @@ import com.iflytek.skillhub.storage.ObjectStorageService; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; @@ -101,6 +99,7 @@ class SkillPublishServiceTest { setId(skill, 1L); SkillVersion version = new SkillVersion(1L, "1.0.0", publisherId); setId(version, 10L); + version.setStatus(SkillVersionStatus.PENDING_REVIEW); when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); when(namespaceMemberRepository.findByNamespaceIdAndUserId(any(), eq(publisherId))).thenReturn(Optional.of(member)); @@ -110,7 +109,6 @@ class SkillPublishServiceTest { when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(Optional.of(skill)); when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("1.0.0"))).thenReturn(Optional.empty()); when(skillVersionRepository.save(any())).thenReturn(version); - when(skillRepository.save(any())).thenReturn(skill); // Act SkillPublishService.PublishResult result = service.publishFromEntries( @@ -125,11 +123,98 @@ class SkillPublishServiceTest { assertEquals(1L, result.skillId()); assertEquals("test-skill", result.slug()); assertEquals("1.0.0", result.version().getVersion()); - verify(eventPublisher).publishEvent(any(SkillPublishedEvent.class)); + assertEquals(SkillVersionStatus.PENDING_REVIEW, result.version().getStatus()); + assertNull(skill.getLatestVersionId()); + verify(eventPublisher, never()).publishEvent(any()); verify(skillFileRepository).saveAll(anyList()); verify(objectStorageService, atLeastOnce()).putObject(anyString(), any(), anyLong(), anyString()); } + @Test + void testPublishFromEntries_ShouldKeepLatestVersionPointingToPublishedVersion() throws Exception { + String namespaceSlug = "test-ns"; + String publisherId = "user-100"; + String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.1.0\n---\nBody"; + + PackageEntry skillMd = new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"); + List entries = List.of(skillMd); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + NamespaceMember member = mock(NamespaceMember.class); + SkillMetadata metadata = new SkillMetadata("test-skill", "Test", "1.1.0", "Body", Map.of()); + + Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setLatestVersionId(5L); + SkillVersion version = new SkillVersion(1L, "1.1.0", publisherId); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(any(), eq(publisherId))).thenReturn(Optional.of(member)); + when(skillPackageValidator.validate(entries)).thenReturn(ValidationResult.pass()); + when(skillMetadataParser.parse(skillMdContent)).thenReturn(metadata); + when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass()); + when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("1.1.0"))).thenReturn(Optional.empty()); + when(skillVersionRepository.save(any())).thenReturn(version); + + SkillPublishService.PublishResult result = service.publishFromEntries( + namespaceSlug, + entries, + publisherId, + SkillVisibility.PUBLIC + ); + + assertEquals(11L, result.version().getId()); + assertEquals(5L, skill.getLatestVersionId()); + verify(eventPublisher, never()).publishEvent(any()); + } + + @Test + void testPublishFromEntries_ShouldNotOverwritePublishedSkillMetadataBeforeApproval() throws Exception { + String namespaceSlug = "test-ns"; + String publisherId = "user-100"; + String skillMdContent = "---\nname: pending-name\ndescription: Pending Summary\nversion: 1.1.0\n---\nBody"; + + PackageEntry skillMd = new PackageEntry("SKILL.md", skillMdContent.getBytes(), skillMdContent.length(), "text/markdown"); + List entries = List.of(skillMd); + + Namespace namespace = new Namespace(namespaceSlug, "Test NS", "user-1"); + setId(namespace, 1L); + NamespaceMember member = mock(NamespaceMember.class); + SkillMetadata metadata = new SkillMetadata("pending-name", "Pending Summary", "1.1.0", "Body", Map.of()); + + Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); + setId(skill, 1L); + skill.setDisplayName("Published Name"); + skill.setSummary("Published Summary"); + skill.setUpdatedBy("previous-reviewer"); + skill.setLatestVersionId(5L); + + SkillVersion version = new SkillVersion(1L, "1.1.0", publisherId); + setId(version, 11L); + version.setStatus(SkillVersionStatus.PENDING_REVIEW); + + when(namespaceRepository.findBySlug(namespaceSlug)).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(any(), eq(publisherId))).thenReturn(Optional.of(member)); + when(skillPackageValidator.validate(entries)).thenReturn(ValidationResult.pass()); + when(skillMetadataParser.parse(skillMdContent)).thenReturn(metadata); + when(prePublishValidator.validate(any())).thenReturn(ValidationResult.pass()); + when(skillRepository.findByNamespaceIdAndSlug(any(), eq("pending-name"))).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("1.1.0"))).thenReturn(Optional.empty()); + when(skillVersionRepository.save(any())).thenReturn(version); + + service.publishFromEntries(namespaceSlug, entries, publisherId, SkillVisibility.PUBLIC); + + assertEquals("Published Name", skill.getDisplayName()); + assertEquals("Published Summary", skill.getSummary()); + assertEquals("previous-reviewer", skill.getUpdatedBy()); + assertEquals(5L, skill.getLatestVersionId()); + verify(skillRepository, never()).save(skill); + } + @Test void testPublishFromEntries_ShouldSlugifyNameBeforeLookupAndResponse() throws Exception { String namespaceSlug = "test-ns"; @@ -157,7 +242,6 @@ class SkillPublishServiceTest { when(skillRepository.findByNamespaceIdAndSlug(any(), eq("smoke-skill-two"))).thenReturn(Optional.of(skill)); when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("0.2.0"))).thenReturn(Optional.empty()); when(skillVersionRepository.save(any())).thenReturn(version); - when(skillRepository.save(any())).thenReturn(skill); SkillPublishService.PublishResult result = service.publishFromEntries( namespaceSlug,