diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java index e7f86d45..3e2119d9 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java @@ -98,7 +98,7 @@ public class SkillLifecycleAppService { Map userNamespaceRoles, AuditRequestContext auditContext) { Skill skill = findSkill(namespace, slug, userId); - SkillVersion skillVersion = findVersion(skill.getId(), version); + SkillVersion skillVersion = findVersionForUpdate(skill.getId(), version); skillGovernanceService.deleteVersion( skill, skillVersion, @@ -261,6 +261,13 @@ public class SkillLifecycleAppService { .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); } + private SkillVersion findVersionForUpdate(Long skillId, String version) { + return skillVersionRepository.findBySkillIdForUpdate(skillId).stream() + .filter(candidate -> candidate.getVersion().equals(version)) + .findFirst() + .orElseThrow(() -> new DomainBadRequestException("error.skill.version.notFound", version)); + } + private Map normalizeRoles(Map userNamespaceRoles) { return userNamespaceRoles != null ? userNamespaceRoles : Map.of(); } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java index 09751065..bef8c274 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleControllerTest.java @@ -144,7 +144,8 @@ class SkillLifecycleControllerTest { given(namespaceRepository.findBySlug("global")).willReturn(java.util.Optional.of(namespace)); given(skillSlugResolutionService.resolve(1L, "demo-skill", "usr_1", SkillSlugResolutionService.Preference.CURRENT_USER)) .willReturn(skill); - given(skillVersionRepository.findBySkillIdAndVersion(1L, "1.0.0")).willReturn(java.util.Optional.of(version)); + given(skillVersionRepository.findBySkillIdForUpdate(1L)) + .willReturn(java.util.List.of(version)); mockMvc.perform(delete("/api/web/skills/global/demo-skill/versions/1.0.0") .requestAttr("userId", "usr_1") diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java new file mode 100644 index 00000000..25d585e5 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillVersionDeleteFlowIntegrationTest.java @@ -0,0 +1,144 @@ +package com.iflytek.skillhub.controller.portal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.verify; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.review.ReviewTask; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.review.ReviewTaskStatus; +import com.iflytek.skillhub.domain.skill.Skill; +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.SkillVisibility; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class SkillVersionDeleteFlowIntegrationTest { + + @Autowired + private MockMvc mockMvc; + + @Autowired + private NamespaceRepository namespaceRepository; + + @Autowired + private SkillRepository skillRepository; + + @Autowired + private SkillVersionRepository skillVersionRepository; + + @Autowired + private ReviewTaskRepository reviewTaskRepository; + + @MockBean + private ObjectStorageService objectStorageService; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @Test + void deleteRejectedVersion_removesOnlyItsReviewHistory() throws Exception { + String ownerId = "owner-1"; + String suffix = UUID.randomUUID().toString().substring(0, 8); + Namespace namespace = namespaceRepository.save( + new Namespace("version-delete-" + suffix, "Version Delete " + suffix, ownerId) + ); + + Skill skill = new Skill(namespace.getId(), "demo-skill-" + suffix, ownerId, SkillVisibility.PUBLIC); + skill.setCreatedBy(ownerId); + skill.setUpdatedBy(ownerId); + skill = skillRepository.save(skill); + + SkillVersion rejectedVersion = new SkillVersion(skill.getId(), "1.0.0", ownerId); + rejectedVersion.setStatus(SkillVersionStatus.REJECTED); + rejectedVersion = skillVersionRepository.save(rejectedVersion); + + SkillVersion retainedVersion = new SkillVersion(skill.getId(), "2.0.0", ownerId); + retainedVersion.setStatus(SkillVersionStatus.REJECTED); + retainedVersion = skillVersionRepository.save(retainedVersion); + + ReviewTask rejectedTask = new ReviewTask(rejectedVersion.getId(), namespace.getId(), ownerId); + rejectedTask.setStatus(ReviewTaskStatus.REJECTED); + rejectedTask = reviewTaskRepository.save(rejectedTask); + + ReviewTask approvedTask = new ReviewTask(rejectedVersion.getId(), namespace.getId(), ownerId); + approvedTask.setStatus(ReviewTaskStatus.APPROVED); + approvedTask = reviewTaskRepository.save(approvedTask); + + ReviewTask retainedTask = new ReviewTask(retainedVersion.getId(), namespace.getId(), ownerId); + retainedTask.setStatus(ReviewTaskStatus.REJECTED); + retainedTask = reviewTaskRepository.save(retainedTask); + + Long skillId = skill.getId(); + Long rejectedVersionId = rejectedVersion.getId(); + + mockMvc.perform(delete("/api/web/skills/{namespace}/{slug}/versions/{version}", + namespace.getSlug(), skill.getSlug(), rejectedVersion.getVersion()) + .with(authentication(portalAuth(ownerId, "USER"))) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.skillId").value(skillId)) + .andExpect(jsonPath("$.data.versionId").value(rejectedVersionId)) + .andExpect(jsonPath("$.data.action").value("DELETE_VERSION")) + .andExpect(jsonPath("$.data.status").value("1.0.0")); + + assertThat(skillVersionRepository.findById(rejectedVersion.getId())).isEmpty(); + assertThat(skillVersionRepository.findById(retainedVersion.getId())).isPresent(); + assertThat(reviewTaskRepository.findById(rejectedTask.getId())).isEmpty(); + assertThat(reviewTaskRepository.findById(approvedTask.getId())).isEmpty(); + assertThat(reviewTaskRepository.findById(retainedTask.getId())).isPresent(); + verify(objectStorageService).deleteObjects(argThat(keys -> + keys.equals(List.of("packages/" + skillId + "/" + rejectedVersionId + "/bundle.zip")) + )); + } + + private UsernamePasswordAuthenticationToken portalAuth(String userId, String... roles) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of(roles) + ); + List authorities = Arrays.stream(roles) + .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) + .toList(); + return new UsernamePasswordAuthenticationToken(principal, null, authorities); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java index e2c10146..b007621f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillLifecycleAppServiceTest.java @@ -14,7 +14,9 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.review.ReviewService; import com.iflytek.skillhub.domain.skill.Skill; +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.service.SkillGovernanceService; import com.iflytek.skillhub.domain.skill.service.SkillPublishService; @@ -76,4 +78,50 @@ class SkillLifecycleAppServiceTest { assertThat(response.status()).isEqualTo("ARCHIVED"); verify(skillGovernanceService).archiveSkill(11L, "owner-1", Map.of(7L, NamespaceRole.OWNER), "127.0.0.1", "JUnit", "cleanup"); } + + @Test + void deleteVersion_locksAllSkillVersionsBeforeDelegatingLifecycleMutation() { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + ReflectionTestUtils.setField(namespace, "id", 7L); + Skill skill = new Skill(7L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + ReflectionTestUtils.setField(skill, "id", 11L); + SkillVersion version = new SkillVersion(11L, "1.0.0", "owner-1"); + ReflectionTestUtils.setField(version, "id", 13L); + version.setStatus(SkillVersionStatus.REJECTED); + SkillVersion retainedVersion = new SkillVersion(11L, "2.0.0", "owner-1"); + ReflectionTestUtils.setField(retainedVersion, "id", 14L); + retainedVersion.setStatus(SkillVersionStatus.UPLOADED); + + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillSlugResolutionService.resolve( + 7L, + "demo-skill", + "owner-1", + SkillSlugResolutionService.Preference.CURRENT_USER + )).thenReturn(skill); + when(skillVersionRepository.findBySkillIdForUpdate(11L)) + .thenReturn(java.util.List.of(version, retainedVersion)); + + var response = service.deleteVersion( + "global", + "demo-skill", + "1.0.0", + "owner-1", + Map.of(7L, NamespaceRole.OWNER), + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(response.versionId()).isEqualTo(13L); + assertThat(response.action()).isEqualTo("DELETE_VERSION"); + verify(skillVersionRepository).findBySkillIdForUpdate(11L); + verify(skillGovernanceService).deleteVersion( + skill, + version, + "owner-1", + Map.of(7L, NamespaceRole.OWNER), + "127.0.0.1", + "JUnit", + "global" + ); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java index 3175c83f..293a3dd7 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerLoggingTest.java @@ -249,6 +249,11 @@ class ScanTaskConsumerLoggingTest { throw new UnsupportedOperationException(); } + @Override + public List findBySkillIdForUpdate(Long skillId) { + throw new UnsupportedOperationException(); + } + @Override public List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status) { throw new UnsupportedOperationException(); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java index 8e9a8ff6..c8c55f0f 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/stream/ScanTaskConsumerTest.java @@ -415,6 +415,11 @@ class ScanTaskConsumerTest { throw unsupported(); } + @Override + public List findBySkillIdForUpdate(Long skillId) { + throw unsupported(); + } + @Override public List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status) { throw unsupported(); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java index 6565436f..b99cc50f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java @@ -12,6 +12,7 @@ public interface SkillVersionRepository { List findBySkillIdIn(List skillIds); List findBySkillIdInAndStatus(List skillIds, SkillVersionStatus status); List findBySkillId(Long skillId); + List findBySkillIdForUpdate(Long skillId); Optional findBySkillIdAndVersion(Long skillId, String version); List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status); SkillVersion save(SkillVersion version); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index bbb748f8..3cad927f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -3,6 +3,7 @@ package com.iflytek.skillhub.domain.skill.service; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent; import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; @@ -41,6 +42,7 @@ public class SkillGovernanceService { private final SkillRepository skillRepository; private final SkillVersionRepository skillVersionRepository; private final SkillFileRepository skillFileRepository; + private final ReviewTaskRepository reviewTaskRepository; private final ObjectStorageService objectStorageService; private final AuditLogService auditLogService; private final ApplicationEventPublisher eventPublisher; @@ -51,6 +53,7 @@ public class SkillGovernanceService { public SkillGovernanceService(SkillRepository skillRepository, SkillVersionRepository skillVersionRepository, SkillFileRepository skillFileRepository, + ReviewTaskRepository reviewTaskRepository, ObjectStorageService objectStorageService, AuditLogService auditLogService, ApplicationEventPublisher eventPublisher, @@ -60,6 +63,7 @@ public class SkillGovernanceService { this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.skillFileRepository = skillFileRepository; + this.reviewTaskRepository = reviewTaskRepository; this.objectStorageService = objectStorageService; this.auditLogService = auditLogService; this.eventPublisher = eventPublisher; @@ -172,6 +176,8 @@ public class SkillGovernanceService { throw new DomainBadRequestException("error.skill.version.delete.lastVersion", version.getVersion()); } + // Rejected versions retain terminal review history whose FK must not outlive the version. + reviewTaskRepository.deleteBySkillVersionIdIn(List.of(version.getId())); List files = skillFileRepository.findByVersionId(version.getId()); List storageKeys = new ArrayList<>(); files.stream() diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java index b6f3faff..50aa3e4b 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceServiceTest.java @@ -5,40 +5,43 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.doThrow; -import static org.mockito.BDDMockito.given; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent; import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; import com.iflytek.skillhub.domain.security.SecurityScanService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillFile; import com.iflytek.skillhub.domain.skill.SkillFileRepository; -import com.iflytek.skillhub.domain.skill.SkillStatus; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillStatus; 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.storage.ObjectStorageService; import java.time.Clock; import java.time.Instant; -import java.util.Optional; -import java.util.Map; import java.time.ZoneOffset; -import org.springframework.context.ApplicationEventPublisher; -import org.springframework.transaction.support.TransactionSynchronization; -import org.springframework.transaction.support.TransactionSynchronizationManager; +import java.util.Map; +import java.util.Optional; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; @ExtendWith(MockitoExtension.class) class SkillGovernanceServiceTest { @@ -52,6 +55,8 @@ class SkillGovernanceServiceTest { @Mock private SkillFileRepository skillFileRepository; @Mock + private ReviewTaskRepository reviewTaskRepository; + @Mock private ObjectStorageService objectStorageService; @Mock private AuditLogService auditLogService; @@ -70,6 +75,7 @@ class SkillGovernanceServiceTest { skillRepository, skillVersionRepository, skillFileRepository, + reviewTaskRepository, objectStorageService, auditLogService, eventPublisher, @@ -229,6 +235,35 @@ class SkillGovernanceServiceTest { verify(auditLogService).record("owner", "DELETE_SKILL_VERSION", "SKILL_VERSION", 2L, null, "127.0.0.1", "JUnit", "{\"version\":\"1.0.0\"}"); } + @Test + void deleteVersion_removesReviewTasksBeforeRejectedVersion() { + Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); + setField(skill, "id", 1L); + SkillVersion rejectedVersion = new SkillVersion(1L, "1.0.0", "owner"); + setField(rejectedVersion, "id", 2L); + rejectedVersion.setStatus(SkillVersionStatus.REJECTED); + SkillVersion otherVersion = new SkillVersion(1L, "2.0.0", "owner"); + setField(otherVersion, "id", 3L); + otherVersion.setStatus(SkillVersionStatus.DRAFT); + given(skillVersionRepository.findBySkillId(1L)) + .willReturn(java.util.List.of(rejectedVersion, otherVersion)); + given(skillFileRepository.findByVersionId(2L)).willReturn(java.util.List.of()); + + service.deleteVersion( + skill, + rejectedVersion, + "owner", + Map.of(), + "127.0.0.1", + "JUnit", + "test-ns" + ); + + InOrder deletionOrder = inOrder(reviewTaskRepository, skillVersionRepository); + deletionOrder.verify(reviewTaskRepository).deleteBySkillVersionIdIn(java.util.List.of(2L)); + deletionOrder.verify(skillVersionRepository).delete(rejectedVersion); + } + @Test void deleteVersion_deletesStorageAfterCommitWhenSynchronizationIsActive() { Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); @@ -309,10 +344,36 @@ class SkillGovernanceServiceTest { assertThrows(DomainBadRequestException.class, () -> service.deleteVersion(skill, version, "owner", Map.of(), "127.0.0.1", "JUnit", "test-ns")); + verify(reviewTaskRepository, never()).deleteBySkillVersionIdIn(anyList()); verify(skillVersionRepository, never()).delete(any()); verify(objectStorageService, never()).deleteObject(any()); } + @Test + void deleteVersion_rejectsUnauthorizedUserWithoutDeletingReviewTasks() { + Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); + setField(skill, "id", 1L); + SkillVersion version = new SkillVersion(1L, "1.0.0", "owner"); + setField(version, "id", 2L); + version.setStatus(SkillVersionStatus.REJECTED); + + assertThrows( + DomainForbiddenException.class, + () -> service.deleteVersion( + skill, + version, + "member", + Map.of(1L, NamespaceRole.MEMBER), + "127.0.0.1", + "JUnit", + "test-ns" + ) + ); + + verify(reviewTaskRepository, never()).deleteBySkillVersionIdIn(anyList()); + verify(skillVersionRepository, never()).delete(any()); + } + @Test void deleteVersion_rejectsLastRemainingVersion() { Skill skill = new Skill(1L, "demo", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); @@ -326,6 +387,7 @@ class SkillGovernanceServiceTest { () -> service.deleteVersion(skill, version, "owner", Map.of(), "127.0.0.1", "JUnit", "test-ns")); assertThat(ex.messageCode()).isEqualTo("error.skill.version.delete.lastVersion"); + verify(reviewTaskRepository, never()).deleteBySkillVersionIdIn(anyList()); verify(skillVersionRepository, never()).delete(any()); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java index eabb01e4..e7c85e1c 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java @@ -3,16 +3,22 @@ package com.iflytek.skillhub.infra.jpa; import com.iflytek.skillhub.domain.skill.SkillVersion; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.skill.SkillVersionStatus; +import java.util.List; +import java.util.Optional; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; -import java.util.List; -import java.util.Optional; - /** * JPA-backed repository for skill version history and status-oriented version queries. + * + *

The deletion lock uses explicit {@code FOR UPDATE} SQL because Hibernate's PostgreSQL dialect + * emits {@code FOR NO KEY UPDATE}, which H2's PostgreSQL compatibility mode cannot execute. It + * locks every version in stable ID order so concurrent deletions cannot both remove the last + * versions of one skill. */ @Repository public interface SkillVersionJpaRepository extends JpaRepository, SkillVersionRepository { @@ -22,6 +28,16 @@ public interface SkillVersionJpaRepository extends JpaRepository findBySkillIdInAndStatusOrderByCreatedAtDesc(List skillIds, SkillVersionStatus status); Optional findBySkillIdAndVersion(Long skillId, String version); + @Override + @Query(value = """ + SELECT skill_version.* + FROM skill_version + WHERE skill_version.skill_id = :skillId + ORDER BY skill_version.id + FOR UPDATE + """, nativeQuery = true) + List findBySkillIdForUpdate(@Param("skillId") Long skillId); + @Override default List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status) { return findBySkillIdAndStatusOrderByCreatedAtDesc(skillId, status);