diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java index c7b9dd3a..257d33d2 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/LabelSearchSyncIntegrationTest.java @@ -17,6 +17,7 @@ import com.iflytek.skillhub.domain.skill.SkillVisibility; import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentEntity; import com.iflytek.skillhub.infra.jpa.SkillSearchDocumentJpaRepository; import com.iflytek.skillhub.search.SearchEmbeddingService; +import com.iflytek.skillhub.search.SearchRebuildService; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -30,6 +31,9 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.context.annotation.Import; import org.springframework.test.context.ActiveProfiles; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; +import org.springframework.transaction.support.TransactionTemplate; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.anyString; @@ -69,6 +73,12 @@ class LabelSearchSyncIntegrationTest { @Autowired private SkillSearchDocumentJpaRepository skillSearchDocumentJpaRepository; + @Autowired + private SearchRebuildService searchRebuildService; + + @Autowired + private TransactionTemplate transactionTemplate; + @MockBean private SearchEmbeddingService searchEmbeddingService; @@ -124,6 +134,65 @@ class LabelSearchSyncIntegrationTest { .contains(labelDisplayName); } + @Test + void detachingLabel_removesKeywordFromSearchIndex() throws Exception { + Fixture f = createFixture(); + + skillLabelAppService.attachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + SkillSearchDocumentEntity afterAttach = awaitIndexedDocument(f.skillId); + assertThat(afterAttach.getKeywords()) + .as("precondition: label keyword indexed after attach") + .contains(f.labelDisplayName); + + // Act: detach the same label. + skillLabelAppService.detachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + + // Assert: the rebuilt document must no longer contain the label keyword. + awaitKeywordAbsent(f.skillId, f.labelDisplayName); + } + + /** + * Guards against the {@code CallerRunsPolicy} regression: when the executor is + * saturated, {@code rebuildSkill} runs synchronously on the request thread inside + * the {@code afterCommit} phase — the exact context where the index write used to be + * dropped. This exercises that path directly (no async hop) and asserts the document + * is still persisted, proving the fix relies on {@code REQUIRES_NEW}, not on the + * executor having spare capacity. + */ + @Test + void syncRebuildInAfterCommitPhase_persistsIndex() throws Exception { + Fixture f = createFixture(); + + // Establish the skill-label association and a baseline index via the normal path. + skillLabelAppService.attachLabel( + f.namespaceSlug, f.skillSlug, f.labelSlug, f.ownerId, f.ownerRoles, auditContext()); + awaitIndexedDocument(f.skillId); + + // Clear the index so we can observe the synchronous rebuild in isolation. + transactionTemplate.executeWithoutResult( + status -> skillSearchDocumentJpaRepository.deleteBySkillId(f.skillId)); + assertThat(skillSearchDocumentJpaRepository.findBySkillId(f.skillId)).isEmpty(); + + // Rebuild synchronously on the caller thread, inside a post-commit synchronization + // (mirrors the CallerRuns fallback from afterCommit(() -> rebuildSkill(...))). + transactionTemplate.executeWithoutResult(status -> + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + searchRebuildService.rebuildBySkill(f.skillId); + } + })); + + SkillSearchDocumentEntity indexed = skillSearchDocumentJpaRepository.findBySkillId(f.skillId) + .orElseThrow(() -> new AssertionError( + "synchronous rebuild in afterCommit phase must persist the index document")); + assertThat(indexed.getKeywords()) + .as("label keyword must be indexed even on the synchronous caller-runs path") + .contains(f.labelDisplayName); + } + private SkillSearchDocumentEntity awaitIndexedDocument(Long skillId) throws InterruptedException { Instant deadline = Instant.now().plus(Duration.ofSeconds(15)); Optional indexed = skillSearchDocumentJpaRepository.findBySkillId(skillId); @@ -134,4 +203,67 @@ class LabelSearchSyncIntegrationTest { return indexed.orElseThrow( () -> new AssertionError("Expected search document for skill " + skillId)); } + + private void awaitKeywordAbsent(Long skillId, String keyword) throws InterruptedException { + Instant deadline = Instant.now().plus(Duration.ofSeconds(15)); + while (Instant.now().isBefore(deadline)) { + Optional indexed = + skillSearchDocumentJpaRepository.findBySkillId(skillId); + if (indexed.isPresent() && !indexed.get().getKeywords().contains(keyword)) { + return; + } + Thread.sleep(100L); + } + String keywords = skillSearchDocumentJpaRepository.findBySkillId(skillId) + .map(SkillSearchDocumentEntity::getKeywords) + .orElse(""); + throw new AssertionError( + "Expected keyword '" + keyword + "' to be removed from index for skill " + + skillId + " but keywords were: " + keywords); + } + + private AuditRequestContext auditContext() { + return new AuditRequestContext("127.0.0.1", "junit"); + } + + private Fixture createFixture() { + String suffix = UUID.randomUUID().toString().substring(0, 8); + String ownerId = "owner-" + suffix; + // ASCII display name so the tokenizer keeps it as a single searchable token. + String labelDisplayName = "MachineLearning" + suffix; + String labelSlug = "ml-" + suffix; + + Namespace namespace = new Namespace("ns-" + suffix, "NS " + suffix, ownerId); + namespace.setType(NamespaceType.GLOBAL); + namespace = namespaceRepository.save(namespace); + + Skill skill = new Skill(namespace.getId(), "skill-" + suffix, ownerId, SkillVisibility.PUBLIC); + skill.setDisplayName("Skill " + suffix); + skill.setSummary("A skill used to reproduce the label search sync bug."); + skill.setCreatedBy(ownerId); + skill.setUpdatedBy(ownerId); + skill = skillRepository.save(skill); + skillRepository.flush(); + + LabelDefinition label = labelDefinitionRepository.save( + new LabelDefinition(labelSlug, LabelType.RECOMMENDED, true, 0, ownerId)); + labelTranslationRepository.saveAll(List.of( + new LabelTranslation(label.getId(), "en", labelDisplayName))); + labelTranslationRepository.flush(); + + return new Fixture( + namespace.getSlug(), skill.getSlug(), skill.getId(), + labelSlug, labelDisplayName, ownerId, + Map.of(namespace.getId(), NamespaceRole.OWNER)); + } + + private record Fixture( + String namespaceSlug, + String skillSlug, + Long skillId, + String labelSlug, + String labelDisplayName, + String ownerId, + Map ownerRoles) { + } } diff --git a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java index bac3cce1..304b5ac0 100644 --- a/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java +++ b/server/skillhub-search/src/main/java/com/iflytek/skillhub/search/postgres/PostgresFullTextIndexService.java @@ -6,6 +6,7 @@ import com.iflytek.skillhub.search.SearchEmbeddingService; import com.iflytek.skillhub.search.SearchIndexService; import com.iflytek.skillhub.search.SkillSearchDocument; import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import java.util.List; @@ -32,7 +33,7 @@ public class PostgresFullTextIndexService implements SearchIndexService { } @Override - @Transactional + @Transactional(propagation = Propagation.REQUIRES_NEW) public void index(SkillSearchDocument document) { SkillSearchDocument normalizedDocument = normalize(document); Optional existing = repository.findBySkillId(document.skillId());