scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-09-08 22:21:09 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Merge pull request #784 from iflytek/fix/concurrent-publish-coordinate-race

Browse source 
fix(publish): return deterministic conflict on concurrent coordinate race
This commit is contained in:
XiaoSeS 2026-08-31 18:47:19 +08:00 • committed by GitHub
commit a73997c672
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 101 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
server/skillhub-app/src/main/resources/messages.properties
View file

@ -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

1
server/skillhub-app/src/main/resources/messages_zh.properties
View file

@ -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 状态，无法确认发布

24
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillPublishService.java
View file

@ -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<SkillFile> skillFiles = new ArrayList<>();

78
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillPublishServiceTest.java
View file

@ -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<ReviewTask> 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<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.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<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.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);