diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 82b255e5..c350aff7 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -148,6 +148,7 @@ error.admin.user.status.invalid=Invalid user status: {0} error.admin.user.status.unsupported=Only ACTIVE or DISABLED status can be managed here error.skill.publish.nameConflict=A published skill with name ''{0}'' already exists in this namespace error.skill.publish.nameConflict.private=A private skill with name ''{0}'' has already been published in this namespace +error.skill.publish.concurrentConflict=A concurrent publish for skill ''{0}'' is already being processed; please retry. error.skill.approve.nameConflict=Cannot approve: a published skill with name ''{0}'' already exists in this namespace error.skill.version.submit.notUploaded=Version ''{0}'' is not in UPLOADED status and cannot be submitted for review error.skill.version.confirm.notUploaded=Version ''{0}'' is not in UPLOADED status and cannot be confirmed diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 0f2e1808..25b879ef 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -148,6 +148,7 @@ error.admin.user.status.invalid=无效的用户状态：{0} error.admin.user.status.unsupported=这里只允许管理 ACTIVE 或 DISABLED 状态的用户 error.skill.publish.nameConflict=该命名空间下已存在名为"{0}"的已发布技能，无法提交 error.skill.publish.nameConflict.private=该命名空间下已存在名为"{0}"的已发布私有技能，无法提交 +error.skill.publish.concurrentConflict=技能"{0}"存在并发的发布请求正在处理，请稍后重试。 error.skill.approve.nameConflict=无法通过审核：该命名空间下已存在名为"{0}"的已发布技能 error.skill.version.submit.notUploaded=版本"{0}"不在 UPLOADED 状态，无法提交审核 error.skill.version.confirm.notUploaded=版本"{0}"不在 UPLOADED 状态，无法确认发布 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 06eb39c3..bc6e0b0a 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 @@ -28,6 +28,7 @@ import com.iflytek.skillhub.storage.ObjectStorageService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.yaml.snakeyaml.Yaml; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.context.ApplicationEventPublisher; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -422,7 +423,18 @@ public class SkillPublishService { .orElseGet(() -> { Skill newSkill = new Skill(namespace.getId(), skillSlug, publisherId, visibility); newSkill.setCreatedBy(publisherId); - return skillRepository.save(newSkill); + try { + Skill savedSkill = skillRepository.save(newSkill); + // save() may defer the unique-constraint check until transaction commit. + // Flush here so this boundary can translate the coordinate race. + skillRepository.flush(); + return savedSkill; + } catch (DataIntegrityViolationException ex) { + // A concurrent publish for the same (namespace, slug, owner) coordinate inserted + // the skill first and won the unique-constraint race. Surface a deterministic + // business conflict instead of leaking the violation as an HTTP 500. + throw new DomainBadRequestException("error.skill.publish.concurrentConflict", skillSlug); + } }); if (skill.getStatus() == SkillStatus.ARCHIVED) { @@ -476,7 +488,15 @@ public class SkillPublishService { throw new IllegalStateException("Failed to serialize metadata", e); } - version = skillVersionRepository.save(version); + try { + version = skillVersionRepository.save(version); + // Detect the version-coordinate race before object storage writes begin. + skillVersionRepository.flush(); + } catch (DataIntegrityViolationException ex) { + // A concurrent publish inserted the same (skillId, version) coordinate first and won the + // unique-constraint race. Surface a deterministic business conflict rather than an HTTP 500. + throw new DomainBadRequestException("error.skill.publish.concurrentConflict", skillSlug); + } // 9. Upload each file to storage and compute SHA-256 List skillFiles = new ArrayList<>(); 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 c084c2f8..a680e320 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 @@ -22,6 +22,7 @@ import com.iflytek.skillhub.domain.skill.validation.SkillPackageValidator; import com.iflytek.skillhub.domain.skill.validation.ValidationResult; import com.iflytek.skillhub.storage.ObjectStorageService; import org.springframework.context.ApplicationEventPublisher; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; import org.junit.jupiter.api.AfterEach; @@ -475,7 +476,7 @@ class SkillPublishServiceTest { verify(reviewTaskRepository).deleteBySkillVersionIdIn(List.of(8L)); verify(skillFileRepository).deleteByVersionId(8L); verify(skillVersionRepository).delete(rejectedVersion); - verify(skillVersionRepository).flush(); + verify(skillVersionRepository, times(2)).flush(); verify(objectStorageService).deleteObjects(List.of("skills/1/8/SKILL.md", "packages/1/8/bundle.zip")); ArgumentCaptor reviewTaskCaptor = ArgumentCaptor.forClass(ReviewTask.class); @@ -1754,6 +1755,81 @@ class SkillPublishServiceTest { return List.of(skillMd, readme); } + @Test + void testPublishFromEntries_concurrentSkillInsertReturnsBusinessConflict() throws Exception { + String namespaceSlug = "test-ns"; + String publisherId = "user-100"; + String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.0.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.0.0", "Body", Map.of()); + + 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()); + // No other owner's skill blocks the name, and this owner has none yet either. + when(skillRepository.findByNamespaceIdAndSlug(any(), eq("test-skill"))).thenReturn(List.of()); + when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("test-skill"), eq(publisherId))) + .thenReturn(Optional.empty()); + // A concurrent publish inserts the same (namespace, slug, owner) coordinate first and wins the race. + when(skillRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0)); + doThrow(new DataIntegrityViolationException("duplicate key")).when(skillRepository).flush(); + + DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> service.publishFromEntries( + namespaceSlug, entries, publisherId, SkillVisibility.PUBLIC, Set.of())); + + assertEquals("error.skill.publish.concurrentConflict", exception.messageCode()); + assertEquals("test-skill", String.valueOf(exception.messageArgs()[0])); + verify(skillRepository).flush(); + verify(skillVersionRepository, never()).save(any(SkillVersion.class)); + } + + @Test + void testPublishFromEntries_concurrentVersionInsertReturnsBusinessConflict() throws Exception { + String namespaceSlug = "test-ns"; + String publisherId = "user-100"; + String skillMdContent = "---\nname: test-skill\ndescription: Test\nversion: 1.0.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.0.0", "Body", Map.of()); + + Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); + setId(skill, 1L); + + 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(List.of(skill)); + when(skillRepository.findByNamespaceIdAndSlugAndOwnerId(any(), eq("test-skill"), eq(publisherId))) + .thenReturn(Optional.of(skill)); + when(skillVersionRepository.findBySkillIdAndVersion(any(), eq("1.0.0"))).thenReturn(Optional.empty()); + // A concurrent publish inserts the same (skillId, version) coordinate first and wins the race. + when(skillVersionRepository.save(any(SkillVersion.class))).thenAnswer(invocation -> invocation.getArgument(0)); + doThrow(new DataIntegrityViolationException("duplicate key")).when(skillVersionRepository).flush(); + + DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> service.publishFromEntries( + namespaceSlug, entries, publisherId, SkillVisibility.PUBLIC, Set.of())); + + assertEquals("error.skill.publish.concurrentConflict", exception.messageCode()); + verify(skillVersionRepository).flush(); + // Nothing should be uploaded to object storage once the coordinate race is lost. + verify(objectStorageService, never()).putObject(anyString(), any(), anyLong(), anyString()); + } + private void setId(Object entity, Long id) throws Exception { Field idField = entity.getClass().getDeclaredField("id"); idField.setAccessible(true);