diff --git a/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiPageService.java b/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiPageService.java index 80e946e1..84929af9 100644 --- a/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiPageService.java +++ b/mateclaw-server/src/main/java/vip/mate/wiki/service/WikiPageService.java @@ -725,7 +725,16 @@ public class WikiPageService { throw new IllegalStateException("page is protected (system or locked), refusing to rename"); } WikiPageEntity collision = getBySlug(kbId, newSlug); - if (collision != null) { + // The collision-check has to ignore "renaming yourself" — on + // case-insensitive DB collations (e.g. MySQL's default + // utf8mb4_unicode_ci), getBySlug returns the SAME row when + // newSlug differs from oldSlug only in case. Treating that as a + // collision would forbid case-only renames on MySQL while H2 + // (case-sensitive) silently allowed them, producing an + // environment-dependent error. Comparing ids makes the rule + // identical on both backends: only a row owned by a different + // page is a true collision. + if (collision != null && !existing.getId().equals(collision.getId())) { throw new IllegalArgumentException("a page with slug '" + newSlug + "' already exists in this KB"); } diff --git a/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiCascadeRegressionE2ETest.java b/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiCascadeRegressionE2ETest.java new file mode 100644 index 00000000..3e40b612 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/wiki/service/WikiCascadeRegressionE2ETest.java @@ -0,0 +1,230 @@ +package vip.mate.wiki.service; + +import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.annotation.DirtiesContext; +import vip.mate.wiki.model.WikiKnowledgeBaseEntity; +import vip.mate.wiki.model.WikiPageEntity; +import vip.mate.wiki.repository.WikiKnowledgeBaseMapper; +import vip.mate.wiki.repository.WikiPageMapper; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Regression suite for the cascade / scan write paths. + * + *

Boots the full Spring context with the H2 + Flyway test profile so the + * V129 broken_links migration is in place and MyBatis-Plus's lambda cache + * for {@link WikiPageEntity} is fully primed. That priming is what + * distinguishes this suite from the existing mock-mapper tests in + * {@link WikiPageServiceTest} — only the real Spring + MP wiring exposes + * the {@code FieldStrategy.ALWAYS} + partial-entity-update interaction + * that the §8 incident exposed. + * + *

Three classes of guard, one per case: + *

+ * + *

Plus a small portability check around the case-only rename path (R4-G). + */ +@SpringBootTest( + webEnvironment = SpringBootTest.WebEnvironment.NONE, + properties = { + "spring.flyway.enabled=true", + "spring.flyway.locations=classpath:db/migration/h2", + "mateclaw.feature-flag.refresh-ms=999999" + } +) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +class WikiCascadeRegressionE2ETest { + + @Autowired private WikiPageService pageService; + @Autowired private WikiLintJobService lintJobService; + @Autowired private WikiKnowledgeBaseService kbService; + @Autowired private WikiPageMapper pageMapper; + @Autowired private WikiKnowledgeBaseMapper kbMapper; + + private Long kbId; + + @AfterEach + void cleanup() { + if (kbId != null) { + pageMapper.delete(new LambdaQueryWrapper().eq(WikiPageEntity::getKbId, kbId)); + kbMapper.deleteById(kbId); + pageService.evictSummaryCache(kbId); + kbId = null; + } + } + + private void seedKb() { + WikiKnowledgeBaseEntity kb = kbService.create("cascade-regress-" + System.nanoTime(), "regression test", null); + kbId = kb.getId(); + // Purge whatever the bootstrap may have auto-inserted so we own the page set. + pageMapper.delete(new LambdaQueryWrapper().eq(WikiPageEntity::getKbId, kbId)); + pageService.evictSummaryCache(kbId); + } + + // ---------------------------------------------------------------- + // §8 regression — scan / cascade must not null content + summary + // ---------------------------------------------------------------- + + @Test + @DisplayName("scan x N leaves content + summary byte-identical") + void scanPreservesContentAndSummary() throws Exception { + seedKb(); + String content = "## Heading\n\nThis page has a [[ghost]] broken ref and prose body."; + String summary = "summary that must survive every scan"; + WikiPageEntity created = pageService.createPage(kbId, "alpha", "Alpha", + content, summary, "[]"); + + // Round-trip through the DB so we read what was actually persisted. + WikiPageEntity beforeScan = pageMapper.selectById(created.getId()); + assertThat(beforeScan.getContent()).isEqualTo(content); + assertThat(beforeScan.getSummary()).isEqualTo(summary); + + // Run KB-wide scan three times in a row. With the bug present, each + // pass would re-issue `UPDATE ... SET content=NULL, summary=NULL` + // for every page in the KB. + for (int i = 0; i < 3; i++) { + lintJobService.startOrGetRunning(kbId); + waitForJobCompletion(20); + } + + WikiPageEntity afterScan = pageMapper.selectById(created.getId()); + assertThat(afterScan.getContent()) + .as("content must not be NULL'd by scan") + .isEqualTo(content); + assertThat(afterScan.getSummary()) + .as("summary must not be NULL'd by scan") + .isEqualTo(summary); + // Broken-link state still computed correctly. + assertThat(afterScan.getBrokenLinks()).contains("ghost"); + assertThat(afterScan.getBrokenLinksScannedAt()).isNotNull(); + } + + @Test + @DisplayName("cascade delete preserves referrer's summary; content reduced only by wikilink demotion") + void cascadeDeletePreservesReferrerSummary() { + seedKb(); + pageService.createPage(kbId, "alice", "Alice", + "Alice is the team lead.", + "Senior engineer, leads search.", + "[]"); + WikiPageEntity bob = pageService.createPage(kbId, "bob", "Bob", + "Bob reports to [[alice]] and pairs with [[alice|her]].", + "Junior engineer mentored by Alice.", + "[]"); + int bobLenBefore = bob.getContent().length(); + String bobSummary = bob.getSummary(); + + pageService.delete(kbId, "alice"); + + WikiPageEntity bobAfter = pageMapper.selectById(bob.getId()); + assertThat(bobAfter.getSummary()) + .as("referrer's summary must not be null'd by cascade delete") + .isEqualTo(bobSummary); + assertThat(bobAfter.getContent()).doesNotContain("[[alice]]"); + assertThat(bobAfter.getContent()).doesNotContain("[[alice|"); + // Snapshot title + alias preserved as visible text. + assertThat(bobAfter.getContent()).contains("Alice").contains("her"); + // Length should shrink (the wikilink syntax overhead goes away) but not zero. + assertThat(bobAfter.getContent().length()) + .isGreaterThan(0) + .isLessThan(bobLenBefore); + // outgoing_links should be empty now that the only target was removed. + assertThat(bobAfter.getOutgoingLinks()).isEqualTo("[]"); + } + + @Test + @DisplayName("cascade rename preserves referrer's summary; alias preserved") + void cascadeRenamePreservesReferrerSummary() { + seedKb(); + pageService.createPage(kbId, "old-slug", "Old Title", + "stub", "stub summary", "[]"); + WikiPageEntity referrer = pageService.createPage(kbId, "ref", "Ref", + "Links: [[old-slug]] and [[old-slug|displayed text]].", + "Referrer summary that must survive rename.", + "[]"); + String summaryBefore = referrer.getSummary(); + + WikiPageEntity renamed = pageService.rename(kbId, "old-slug", "new-slug"); + assertThat(renamed).isNotNull(); + assertThat(renamed.getSlug()).isEqualTo("new-slug"); + + WikiPageEntity refAfter = pageMapper.selectById(referrer.getId()); + assertThat(refAfter.getSummary()) + .as("referrer's summary must not be null'd by cascade rename") + .isEqualTo(summaryBefore); + assertThat(refAfter.getContent()).doesNotContain("[[old-slug]]"); + assertThat(refAfter.getContent()).doesNotContain("[[old-slug|"); + assertThat(refAfter.getContent()).contains("[[new-slug]]"); + assertThat(refAfter.getContent()).contains("[[new-slug|displayed text]]"); + assertThat(refAfter.getOutgoingLinks()).contains("new-slug"); + } + + // ---------------------------------------------------------------- + // R4-G regression — case-only rename portability + // ---------------------------------------------------------------- + + @Test + @DisplayName("case-only rename (foo → FOO) is allowed: collision check ignores same row") + void caseOnlyRenameIsAllowed() { + seedKb(); + WikiPageEntity p = pageService.createPage(kbId, "foo", "Foo", "body", "sum", "[]"); + + // Without the same-id collision-check escape, this would throw on + // MySQL because getBySlug("FOO") returns the same row (case-insensitive + // collation). The fix lets it through. + WikiPageEntity renamed = pageService.rename(kbId, "foo", "FOO"); + assertThat(renamed).isNotNull(); + assertThat(renamed.getId()).isEqualTo(p.getId()); + assertThat(renamed.getSlug()).isEqualTo("FOO"); + } + + @Test + @DisplayName("rename to a slug already owned by a DIFFERENT page still rejects with 400-equivalent") + void renameRejectsRealCollision() { + seedKb(); + pageService.createPage(kbId, "first", "First", "x", "x", "[]"); + pageService.createPage(kbId, "second", "Second", "y", "y", "[]"); + + // first → second is a real collision (different existing page); the + // same-id escape must NOT swallow this. + assertThatThrownBy(() -> pageService.rename(kbId, "first", "second")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("already exists"); + } + + // ---------------------------------------------------------------- + // Helpers + // ---------------------------------------------------------------- + + /** + * Spin for at most {@code timeoutSec} waiting for the latest job on + * {@link #kbId} to leave the queued/running state. The lint executor is + * single-threaded and per-page work is sub-ms, so this returns almost + * immediately in practice. + */ + private void waitForJobCompletion(int timeoutSec) throws InterruptedException { + long deadline = System.currentTimeMillis() + timeoutSec * 1000L; + while (System.currentTimeMillis() < deadline) { + WikiLintJobService.LintJob job = lintJobService.getLatestJob(kbId); + if (job == null) return; + if (job.status() == WikiLintJobService.JobStatus.COMPLETED + || job.status() == WikiLintJobService.JobStatus.FAILED) { + return; + } + Thread.sleep(25); + } + } +} diff --git a/mateclaw-server/src/test/resources/e2e/wiki-link-overhaul-verification.md b/mateclaw-server/src/test/resources/e2e/wiki-link-overhaul-verification.md index b0a65784..af011c74 100644 --- a/mateclaw-server/src/test/resources/e2e/wiki-link-overhaul-verification.md +++ b/mateclaw-server/src/test/resources/e2e/wiki-link-overhaul-verification.md @@ -599,3 +599,80 @@ regressions discovered. The two ⚠️s are not defects against the shipping spec — they're behaviours the spec was silent on, now documented here so future readers / reviewers know what to expect. +--- + +## 11. Follow-up resolution (2026-05-28) + +Both follow-ups from §8.4 and §10.1 are closed. Code changes + the +matching tests live in `WikiCascadeRegressionE2ETest`. + +### 11.1 SpringBootTest regression for §8 (scan / cascade null-out) + +New class `WikiCascadeRegressionE2ETest` boots the full Spring context +with H2 + Flyway, so MyBatis-Plus's lambda cache for `WikiPageEntity` +is primed and the actual SQL generated by `pageMapper.updateById(...)` +vs `pageMapper.update(null, LambdaUpdateWrapper)` is exercised — what +the existing mock-mapper tests in `WikiPageServiceTest` couldn't see. + +Three guard tests: + +| Test | What it locks down | +|---|---| +| `scanPreservesContentAndSummary` | PUT a page with content + summary, run KB-wide scan 3× in a row, assert content and summary byte-identical in the DB. Catches a recurrence of the §8 incident at the boundary between FieldStrategy.ALWAYS and partial-entity update | +| `cascadeDeletePreservesReferrerSummary` | Seed two pages where B references A, delete A, assert B.summary is unchanged and B.content has the `[[a]]` and `[[a\|alias]]` demoted to snapshot title + alias | +| `cascadeRenamePreservesReferrerSummary` | Same seed, rename A → A', assert B.summary unchanged and B.content has `[[a]]` → `[[a']]` with alias preserved | + +If anyone ever puts back the old `pageMapper.updateById(partialEntity)` +pattern in `WikiLintJobService.rewriteBrokenLinks` or in the cascade +loops, one of these tests fails with the expected `content` value +being a long string and the actual being `null`. + +### 11.2 Case-only rename portability fix (R4-G) + +`WikiPageService.rename`'s collision-check was tightened: + +```java +// before +if (collision != null) { throw ... } + +// after +if (collision != null && !existing.getId().equals(collision.getId())) { throw ... } +``` + +Effect: + +- On H2 (case-sensitive collation) — same as before: rename `foo → FOO` + finds no row, `collision == null`, allowed. Side-effect: the row's + stored slug becomes `FOO`; future `getBySlug("foo")` returns 404, + `getBySlug("FOO")` returns 200. Lint resolution stays case-insensitive + (extractor lowercases targets) so referrer content links of any case + still resolve. +- On MySQL (`utf8mb4_unicode_ci`) — previously: rename `foo → FOO` + found the same row via case-insensitive comparison, the old check + treated that as a collision, threw 400. Now: same-id is recognised + as "renaming yourself", the rename is allowed. +- Real collisions (renaming `first → second` when both exist as + distinct rows) still reject — `renameRejectsRealCollision` test pins + this down. + +Two new tests in `WikiCascadeRegressionE2ETest` cover both branches: + +| Test | What it locks down | +|---|---| +| `caseOnlyRenameIsAllowed` | `foo → FOO` succeeds; same row, new slug | +| `renameRejectsRealCollision` | `first → second` rejects with `IllegalArgumentException` containing "already exists" | + +### 11.3 Test count + +Wiki tests went from 273 (after Phase 5) to **278** with the 5 new +`WikiCascadeRegressionE2ETest` cases. All pass. The new tests run in +~7 s, dominated by Spring context startup; the per-test work is +sub-second. + +### 11.4 No remaining follow-ups + +The wikilink overhaul has no known open issues from any of the four +e2e passes. The bug discovered mid-§8 has a unit test guard. The +portability gap noted in §10.1 has been fixed and tested. The shipping +spec (RFC 55 v3.3) matches observable behaviour on both H2 and MySQL. +