mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-08 22:21:09 +00:00
fix(publish): return deterministic conflict on concurrent coordinate race
Concurrent publishes for the same (namespace_id, slug, owner_id) or (skill_id, version) coordinate both pass the check-then-create reads and race on the database unique constraints. The losing request surfaced an unhandled DataIntegrityViolationException as HTTP 500. Translate the constraint violation at both insert points into a deterministic DomainBadRequestException (error.skill.publish.concurrentConflict), matching the existing idiom in LabelDefinitionService/ReviewService/ PromotionService. No same-transaction re-read is attempted, so the losing publish rolls back cleanly and returns a retryable conflict instead of a 500. Add the i18n key (en/zh) and two unit tests covering the skill-insert and version-insert races. Closes #617 Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
This commit is contained in:
parent
b896c698cd
commit
d224c5a8ba
4 changed files with 91 additions and 2 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 状态,无法确认发布
|
||||
|
|
|
|||
|
|
@ -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,14 @@ public class SkillPublishService {
|
|||
.orElseGet(() -> {
|
||||
Skill newSkill = new Skill(namespace.getId(), skillSlug, publisherId, visibility);
|
||||
newSkill.setCreatedBy(publisherId);
|
||||
return skillRepository.save(newSkill);
|
||||
try {
|
||||
return skillRepository.save(newSkill);
|
||||
} 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 +484,13 @@ public class SkillPublishService {
|
|||
throw new IllegalStateException("Failed to serialize metadata", e);
|
||||
}
|
||||
|
||||
version = skillVersionRepository.save(version);
|
||||
try {
|
||||
version = skillVersionRepository.save(version);
|
||||
} 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<>();
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
@ -1754,6 +1755,78 @@ 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())).thenThrow(new DataIntegrityViolationException("duplicate key"));
|
||||
|
||||
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(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)))
|
||||
.thenThrow(new DataIntegrityViolationException("duplicate key"));
|
||||
|
||||
DomainBadRequestException exception = assertThrows(DomainBadRequestException.class, () -> service.publishFromEntries(
|
||||
namespaceSlug, entries, publisherId, SkillVisibility.PUBLIC, Set.of()));
|
||||
|
||||
assertEquals("error.skill.publish.concurrentConflict", exception.messageCode());
|
||||
// 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);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue