From f35f91616a4e4f75de3aa14c0b56f980860423ff Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Wed, 10 Jun 2026 16:51:35 +0800 Subject: [PATCH] fix(publish): preserve latest version reference cleanup order Signed-off-by: dongmucat <1127093059@qq.com> --- .../bootstrap/BuiltinSkillInitializer.java | 75 +++++++++++++------ .../BuiltinSkillInitializerTest.java | 10 ++- .../skill/service/SkillPublishService.java | 11 ++- .../service/SkillPublishServiceTest.java | 7 +- 4 files changed, 72 insertions(+), 31 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java index 34379db7..20b9a363 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializer.java @@ -126,10 +126,21 @@ public class BuiltinSkillInitializer { return; } + int published = 0; + int idempotentSkipped = 0; + int conflictSkipped = 0; + int failed = 0; for (ManifestItem item : items) { try { - syncItem(namespace.get(), item); + SyncOutcome outcome = syncItem(namespace.get(), item); + switch (outcome) { + case PUBLISHED -> published++; + case IDEMPOTENT_SKIPPED -> idempotentSkipped++; + case CONFLICT_SKIPPED -> conflictSkipped++; + case FAILED -> failed++; + } } catch (Exception exception) { + failed++; log.error( "Failed to synchronize built-in skill slug={} version={}: {}", item.slug(), @@ -139,6 +150,14 @@ public class BuiltinSkillInitializer { ); } } + log.info( + "Built-in skill synchronization finished: total={}, published={}, idempotentSkipped={}, conflictSkipped={}, failed={}", + items.size(), + published, + idempotentSkipped, + conflictSkipped, + failed + ); } private boolean ensureSystemPublisher(Namespace namespace) { @@ -171,21 +190,22 @@ public class BuiltinSkillInitializer { return true; } - private void syncItem(Namespace namespace, ManifestItem item) throws Exception { - if (shouldSkipBeforeDownload(namespace.getId(), item)) { - return; + private SyncOutcome syncItem(Namespace namespace, ManifestItem item) throws Exception { + Optional skipBeforeDownload = shouldSkipBeforeDownload(namespace.getId(), item); + if (skipBeforeDownload.isPresent()) { + return skipBeforeDownload.get(); } Optional packageUri = parsePackageUri(item); if (packageUri.isEmpty()) { - return; + return SyncOutcome.FAILED; } Optional packageBytes = downloader.download(packageUri.get()); if (packageBytes.isEmpty()) { log.warn("Skipping built-in skill slug={} version={} because package download failed", item.slug(), item.version()); - return; + return SyncOutcome.FAILED; } SkillPackageArchiveExtractor.ExtractionResult extractionResult = extractor.extract(packageBytes.get()); @@ -199,7 +219,7 @@ public class BuiltinSkillInitializer { item.version(), packageSlug ); - return; + return SyncOutcome.FAILED; } if (!item.version().equals(metadata.version())) { log.warn( @@ -208,11 +228,12 @@ public class BuiltinSkillInitializer { item.version(), metadata.version() ); - return; + return SyncOutcome.FAILED; } - if (shouldSkipExisting(namespace.getId(), item, entries)) { - return; + Optional skipExisting = shouldSkipExisting(namespace.getId(), item, entries); + if (skipExisting.isPresent()) { + return skipExisting.get(); } try { @@ -226,14 +247,16 @@ public class BuiltinSkillInitializer { ); log.info("Published built-in skill slug={} version={} to @{}", item.slug(), item.version(), GLOBAL_NAMESPACE); + return SyncOutcome.PUBLISHED; } catch (RuntimeException exception) { if (isAlreadyPublishedWithSameFingerprint(namespace.getId(), item, entries)) { log.info("Built-in skill slug={} version={} was published concurrently, skipping", item.slug(), item.version()); - return; + return SyncOutcome.IDEMPOTENT_SKIPPED; } log.error("Failed to publish built-in skill slug={} version={}: {}", item.slug(), item.version(), exception.getMessage(), exception); + return SyncOutcome.FAILED; } } @@ -256,25 +279,25 @@ public class BuiltinSkillInitializer { return metadataParser.parse(new String(skillMd.content(), StandardCharsets.UTF_8)); } - private boolean shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { + private Optional shouldSkipBeforeDownload(Long namespaceId, ManifestItem item) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); if (hasOtherOwnerConflict(existingSkills)) { log.warn("Skipping built-in skill slug={} before download because the slug already belongs to another user", item.slug()); - return true; + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } Optional builtinSkill = existingSkills.stream() .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) .findFirst(); if (builtinSkill.isEmpty()) { - return false; + return Optional.empty(); } Optional existingVersion = skillVersionRepository .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); if (existingVersion.isEmpty()) { - return false; + return Optional.empty(); } SkillVersion version = existingVersion.get(); @@ -285,35 +308,35 @@ public class BuiltinSkillInitializer { log.info("Skipping built-in skill slug={} version={} before download because existing version status is {}", item.slug(), item.version(), version.getStatus()); } - return true; + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } - private boolean shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { + private Optional shouldSkipExisting(Long namespaceId, ManifestItem item, List entries) { List existingSkills = skillRepository.findByNamespaceIdAndSlug(namespaceId, item.slug()); if (hasOtherOwnerConflict(existingSkills)) { log.warn("Skipping built-in skill slug={} because the slug already belongs to another user", item.slug()); - return true; + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } Optional builtinSkill = existingSkills.stream() .filter(skill -> SYSTEM_PUBLISHER_ID.equals(skill.getOwnerId())) .findFirst(); if (builtinSkill.isEmpty()) { - return false; + return Optional.empty(); } Optional existingVersion = skillVersionRepository .findBySkillIdAndVersion(builtinSkill.get().getId(), item.version()); if (existingVersion.isEmpty()) { - return false; + return Optional.empty(); } SkillVersion version = existingVersion.get(); if (version.getStatus() != SkillVersionStatus.PUBLISHED) { log.info("Skipping built-in skill slug={} version={} because existing version status is {}", item.slug(), item.version(), version.getStatus()); - return true; + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } String packageFingerprint = computeFingerprint(entries); @@ -321,6 +344,7 @@ public class BuiltinSkillInitializer { if (packageFingerprint.equals(existingFingerprint)) { log.info("Skipping built-in skill slug={} version={} because it is already published", item.slug(), item.version()); + return Optional.of(SyncOutcome.IDEMPOTENT_SKIPPED); } else { log.warn( "Skipping built-in skill slug={} version={} because published fingerprint differs: existing={}, package={}", @@ -329,8 +353,8 @@ public class BuiltinSkillInitializer { existingFingerprint, packageFingerprint ); + return Optional.of(SyncOutcome.CONFLICT_SKIPPED); } - return true; } private boolean isAlreadyPublishedWithSameFingerprint(Long namespaceId, ManifestItem item, List entries) { @@ -404,4 +428,11 @@ public class BuiltinSkillInitializer { private record FileDigest(String path, String sha256) { } + + private enum SyncOutcome { + PUBLISHED, + IDEMPOTENT_SKIPPED, + CONFLICT_SKIPPED, + FAILED + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java index c2b5a836..296f547a 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/bootstrap/BuiltinSkillInitializerTest.java @@ -196,14 +196,16 @@ class BuiltinSkillInitializerTest { } @Test - void skipsUnpublishedSkillOwnedByAnotherUserBeforeDownloadingPackage() { + void skipsSkillOwnedByAnotherUserAfterDownloadingPackage() throws Exception { Skill otherSkill = skill(100L, "skillhub-hello", "someone-else"); - givenManifestAndSystemPublisher(); - when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")).thenReturn(List.of(otherSkill)); + givenExtractedPackage(packageEntries("skillhub-hello", "1.0.0", "same")); + when(skillRepository.findByNamespaceIdAndSlug(1L, "skillhub-hello")) + .thenReturn(List.of()) + .thenReturn(List.of(otherSkill)); runInitializer(); - verify(downloader, never()).download(any()); + verify(downloader).download(URI.create(ITEM.url())); verify(skillPublishService, never()).publishFromEntries(any(), any(), any(), any(), any(), eq(false)); } 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 fed90cfc..698a0fdd 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 @@ -564,6 +564,13 @@ public class SkillPublishService { throw new DomainBadRequestException("error.skill.version.exists", version.getVersion()); } + // PostgreSQL prevents deleting a skill_version while skill.latest_version_id still references it. + if (version.getId().equals(skill.getLatestVersionId())) { + skill.setLatestVersionId(null); + skillRepository.save(skill); + skillRepository.flush(); + } + reviewTaskRepository.findBySkillVersionIdAndStatus(version.getId(), ReviewTaskStatus.PENDING) .ifPresent(reviewTaskRepository::delete); @@ -579,10 +586,6 @@ public class SkillPublishService { securityScanService.softDeleteByVersionId(version.getId()); skillVersionRepository.delete(version); skillVersionRepository.flush(); - - if (version.getId().equals(skill.getLatestVersionId())) { - skill.setLatestVersionId(null); - } } private String resolveNamespaceSlug(Long namespaceId) { 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 77264b23..e40fa545 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 @@ -455,6 +455,7 @@ class SkillPublishServiceTest { Skill skill = new Skill(1L, "test-skill", publisherId, SkillVisibility.PUBLIC); setId(skill, 1L); + skill.setLatestVersionId(8L); SkillVersion draftVersion = new SkillVersion(1L, "1.0.0-beta", publisherId); draftVersion.setStatus(SkillVersionStatus.DRAFT); setId(draftVersion, 8L); @@ -485,10 +486,14 @@ class SkillPublishServiceTest { Set.of() ); - InOrder inOrder = inOrder(skillVersionRepository); + assertNull(skill.getLatestVersionId()); + InOrder inOrder = inOrder(skillRepository, skillVersionRepository); + inOrder.verify(skillRepository).save(skill); + inOrder.verify(skillRepository).flush(); inOrder.verify(skillVersionRepository).delete(draftVersion); inOrder.verify(skillVersionRepository).flush(); inOrder.verify(skillVersionRepository, times(2)).save(any(SkillVersion.class)); + inOrder.verify(skillRepository).save(skill); } @Test