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

fix(suite): close final review gaps

Browse source 
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-09 20:36:49 +08:00
parent a062c8f34a
commit 0dd694859a
7 changed files with 116 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

89
server/skillhub-app/src/test/java/com/iflytek/skillhub/repository/SkillSuiteVersionOptimisticLockingTest.java
View file

@ -5,22 +5,33 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy;
import com.iflytek.skillhub.domain.namespace.Namespace;
import com.iflytek.skillhub.domain.suite.SkillSuite;
import com.iflytek.skillhub.domain.suite.SkillSuiteMemberSelection;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersion;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersionMember;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersionMemberRepository;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersionRepository;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersionStatus;
import com.iflytek.skillhub.domain.skill.SkillVisibility;
import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.OptimisticLockException;
import java.time.Instant;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.jdbc.AutoConfigureTestDatabase;
import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
import org.springframework.orm.ObjectOptimisticLockingFailureException;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.annotation.Propagation;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
@ -47,6 +58,15 @@ class SkillSuiteVersionOptimisticLockingTest {
@Autowired
private EntityManagerFactory entityManagerFactory;
@Autowired
private SkillSuiteVersionRepository versionRepository;
@Autowired
private SkillSuiteVersionMemberRepository memberRepository;
@Autowired
private PlatformTransactionManager transactionManager;
@Test
@Transactional(propagation = Propagation.NOT_SUPPORTED)
void publishingRejectsAConcurrentStaleDraftUpdate() {
@ -85,6 +105,52 @@ class SkillSuiteVersionOptimisticLockingTest {
}
}
@Test
@Transactional(propagation = Propagation.NOT_SUPPORTED)
void publishingRejectsAConcurrentMemberOnlyDraftUpdate() throws Exception {
PersistedSuite persisted = persistDraft();
CountDownLatch editorLoaded = new CountDownLatch(1);
CountDownLatch publisherCommitted = new CountDownLatch(1);
TransactionTemplate transactions = new TransactionTemplate(transactionManager);
try (var executor = Executors.newSingleThreadExecutor()) {
var staleEdit = executor.submit(() -> transactions.executeWithoutResult(status -> {
SkillSuiteVersion version = versionRepository.findByIdForDefinitionUpdate(persisted.versionId())
.orElseThrow();
editorLoaded.countDown();
await(publisherCommitted);
memberRepository.deleteBySuiteVersionId(version.getId());
memberRepository.saveAll(List.of(member(version.getId(), "replacement-after-publish")));
}));
assertThat(editorLoaded.await(10, TimeUnit.SECONDS)).isTrue();
transactions.executeWithoutResult(status -> {
SkillSuiteVersion version = versionRepository.findById(persisted.versionId()).orElseThrow();
version.setStatus(SkillSuiteVersionStatus.PUBLISHED);
version.setPublishedAt(Instant.parse("2026-09-09T08:00:00Z"));
versionRepository.save(version);
});
publisherCommitted.countDown();
assertThatThrownBy(() -> staleEdit.get(10, TimeUnit.SECONDS))
.satisfies(error -> assertThat(
hasCause(error, ObjectOptimisticLockingFailureException.class)).isTrue());
transactions.executeWithoutResult(status -> {
SkillSuiteVersion saved = versionRepository.findById(persisted.versionId()).orElseThrow();
List<SkillSuiteVersionMember> members =
memberRepository.findBySuiteVersionIdOrderByPosition(persisted.versionId());
assertThat(saved.getStatus()).isEqualTo(SkillSuiteVersionStatus.PUBLISHED);
assertThat(members).singleElement()
.extracting(SkillSuiteVersionMember::getSkillSlugSnapshot)
.isEqualTo("original-member");
});
} finally {
publisherCommitted.countDown();
deleteSuite(persisted);
}
}
private PersistedSuite persistDraft() {
EntityManager entityManager = entityManagerFactory.createEntityManager();
try {
@ -96,6 +162,7 @@ class SkillSuiteVersionOptimisticLockingTest {
SkillSuiteVersion version = new SkillSuiteVersion(
suite.getId(), "1.0.0", "Original draft", "Summary", SkillVisibility.PUBLIC, "owner");
entityManager.persist(version);
entityManager.persist(member(version.getId(), "original-member"));
entityManager.getTransaction().commit();
return new PersistedSuite(namespace.getId(), suite.getId(), version.getId());
} finally {
@ -108,6 +175,9 @@ class SkillSuiteVersionOptimisticLockingTest {
EntityManager entityManager = entityManagerFactory.createEntityManager();
try {
entityManager.getTransaction().begin();
entityManager.createQuery("DELETE FROM SkillSuiteVersionMember member WHERE member.suiteVersionId = :id")
.setParameter("id", persisted.versionId())
.executeUpdate();
entityManager.createQuery("DELETE FROM SkillSuiteVersion version WHERE version.id = :id")
.setParameter("id", persisted.versionId())
.executeUpdate();
@ -141,6 +211,25 @@ class SkillSuiteVersionOptimisticLockingTest {
return false;
}
private SkillSuiteVersionMember member(Long suiteVersionId, String slug) {
return new SkillSuiteVersionMember(
suiteVersionId,
new SkillSuiteMemberSelection(null, null, "global", slug, "1.0.0", "sha256:" + slug),
0,
true);
}
private void await(CountDownLatch latch) {
try {
if (!latch.await(10, TimeUnit.SECONDS)) {
throw new AssertionError("Timed out waiting for concurrent transaction");
}
} catch (InterruptedException error) {
Thread.currentThread().interrupt();
throw new AssertionError("Interrupted while waiting for concurrent transaction", error);
}
}
private record PersistedSuite(Long namespaceId, Long suiteId, Long versionId) {
}
}

2
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftService.java
View file

@ -128,7 +128,7 @@ public class SkillSuiteDraftService {
SkillSuite suite = suiteRepository.findById(suiteId)
.orElseThrow(() -> new DomainNotFoundException("error.suite.notFound", suiteId));
requireWritableNamespace(suite.getNamespaceId());
SkillSuiteVersion version = versionRepository.findById(versionId)
SkillSuiteVersion version = versionRepository.findByIdForDefinitionUpdate(versionId)
.orElseThrow(() -> new DomainNotFoundException("error.suite.version.notFound", versionId));
if (!suiteId.equals(version.getSuiteId())) {
throw new DomainBadRequestException("error.suite.version.mismatch");

1
server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/suite/SkillSuiteVersionRepository.java
View file

@ -6,6 +6,7 @@ import java.util.Optional;
/** Persistence contract for immutable Suite version snapshots. */
public interface SkillSuiteVersionRepository {
Optional<SkillSuiteVersion> findById(Long id);
Optional<SkillSuiteVersion> findByIdForDefinitionUpdate(Long id);
Optional<SkillSuiteVersion> findBySuiteIdAndVersion(Long suiteId, String version);
List<SkillSuiteVersion> findByIdIn(List<Long> ids);
List<SkillSuiteVersion> findBySuiteId(Long suiteId);

2
server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftServiceTest.java
View file

@ -97,7 +97,7 @@ class SkillSuiteDraftServiceTest {
setId(version, 20L);
when(namespaceRepository.findById(1L)).thenReturn(Optional.of(namespace));
when(suiteRepository.findById(10L)).thenReturn(Optional.of(suite));
when(versionRepository.findById(20L)).thenReturn(Optional.of(version));
when(versionRepository.findByIdForDefinitionUpdate(20L)).thenReturn(Optional.of(version));
when(versionRepository.save(any())).thenAnswer(invocation -> invocation.getArgument(0));
when(memberRepository.saveAll(any())).thenAnswer(invocation -> invocation.getArgument(0));

12
server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillSuiteVersionJpaRepository.java
View file

@ -2,11 +2,23 @@ package com.iflytek.skillhub.infra.jpa;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersion;
import com.iflytek.skillhub.domain.suite.SkillSuiteVersionRepository;
import jakarta.persistence.LockModeType;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Lock;
import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;
import java.util.Optional;
/** JPA adapter for Suite version snapshots. */
@Repository
public interface SkillSuiteVersionJpaRepository
extends JpaRepository<SkillSuiteVersion, Long>, SkillSuiteVersionRepository {
/** Ensure member-only draft edits still conflict with a concurrent lifecycle transition. */
@Override
@Lock(LockModeType.OPTIMISTIC_FORCE_INCREMENT)
@Query("select version from SkillSuiteVersion version where version.id = :id")
Optional<SkillSuiteVersion> findByIdForDefinitionUpdate(@Param("id") Long id);
}

7
web/src/pages/suite-detail.test.tsx
View file

@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => ({
detail: { data: undefined as SkillSuite | undefined, isLoading: false, error: null as Error | null },
submit: { mutateAsync: vi.fn(), isPending: false },
}))
const originalRuntimeConfig = window.__SKILLHUB_RUNTIME_CONFIG__
vi.mock('@tanstack/react-router', () => ({
useNavigate: () => mocks.navigate,
@ -91,6 +92,7 @@ describe('SuiteDetailPage', () => {
afterEach(() => {
cleanup()
vi.clearAllMocks()
window.__SKILLHUB_RUNTIME_CONFIG__ = originalRuntimeConfig
})
it('adds entry skill guidance to the overview and keeps the full member grid separate', () => {
@ -172,6 +174,9 @@ describe('SuiteDetailPage', () => {
})
it('places Suite metadata and installation in the detail sidebar', () => {
window.__SKILLHUB_RUNTIME_CONFIG__ = {
appBaseUrl: 'https://registry.internal.example/skillhub',
}
mocks.detail = { data: suite(), isLoading: false, error: null }
render(<SuiteDetailPage />)
@ -180,7 +185,7 @@ describe('SuiteDetailPage', () => {
expect(within(sidebar).getByText('v1.0.0')).not.toBeNull()
expect(within(sidebar).getByText('suite.installCommand')).not.toBeNull()
expect(within(sidebar).getByText(
'skillhub suite install @global/care-workflow --version 1.0.0',
'skillhub suite install @global/care-workflow --version 1.0.0 --registry https://registry.internal.example/skillhub',
)).not.toBeNull()
expect(within(sidebar).getByLabelText('suite.copyInstallCommand')).not.toBeNull()
})

8
web/src/pages/suite-detail.tsx
View file

@ -6,6 +6,7 @@ import { useSuiteDetail, useSuiteVersions, useSubmitSuite } from '@/shared/hooks
import { suiteBlockingReasonLabel, suiteStatusLabel, suiteVisibilityLabel } from '@/features/suite/suite-labels'
import { SuiteManagementActions } from '@/features/suite/suite-management-actions'
import { MarkdownRenderer } from '@/features/skill/markdown-renderer'
import { getBaseUrl } from '@/features/skill/install-command'
import { Card } from '@/shared/ui/card'
import { Button, buttonVariants } from '@/shared/ui/button'
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/shared/ui/tabs'
@ -23,9 +24,12 @@ export function SuiteDetailPage() {
const { data: suite, isLoading, error } = useSuiteDetail(namespace, slug, search.version)
const { data: versions } = useSuiteVersions(namespace, slug)
const submitMutation = useSubmitSuite()
const registryUrl = useMemo(() => getBaseUrl(), [])
const command = useMemo(
() => suite ? `skillhub suite install @${suite.namespace}/${suite.slug} --version ${suite.version}` : '',
[suite],
() => suite
? `skillhub suite install @${suite.namespace}/${suite.slug} --version ${suite.version} --registry ${registryUrl}`
: '',
[registryUrl, suite],
)
const suitePath = `/suite/${encodeURIComponent(namespace)}/${encodeURIComponent(slug)}`
const returnTo = search.version