mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-08 03:07:51 +00:00
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
This commit is contained in:
yun-zhi-ztl
parent
f99ffbd7ec
commit
7bc152a744
4 changed files with 128 additions and 19 deletions
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<PackageEntry> 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<PackageEntry> 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,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue