fix(sites): stop the integration suites creating a fake default site by swicken · Pull Request #38246 · dotCMS/core

swicken · 2026-10-26T15:28:03Z

Proposed Changes

This PR stops the integration suites from creating a fake default site partway through a run. That fake site is behind the ShortyServletAndTitleImageTest flake, which fails its first attempt in about 8 of 10 MainSuite 2b runs (#38209). It is very likely behind the TagAPITest flake as well (#38215). This PR also fixes a product defect in how the default site is found (#38208). Part of epic #34645.

The cause

Task211101AddContentletAsJsonColumnTest and PopulateContentletAsJSONUtilTest each drop and re-add, or empty, the contentlet_as_json column for the whole shared test database. That wipes the starter default site's data. The next findDefaultHost then finds no default and creates an unpublished noDefault-<millis> site, and the site cache keeps serving it. Shorty then sees an unpublished current host and answers 503. CI logs show exactly two "NO DEFAULT HOST, creating a fake one" errors in every recent MainSuite 2b run.

Test fixes (#38209)

a7d6ed6 and ed09993: both tests back up the column's JSON before they touch it and restore it afterwards, even when the test fails. Task211101 uses try-with-resources, and PopulateContentletAsJSONUtilTest uses @After.
b0c7510: the shared ContentletAsJsonBackup helper also removes any leftover noDefault- site and flushes the site and contentlet caches.
c75e8a7: the restore is a single update ... from <backup> join instead of a subquery per row. On a throwaway Postgres 18, the old form took 8.74 s at 20,000 rows and the join took 63 ms.
b800f99: if a test dies after dropping the column, the restore re-adds the column first and keeps the test's own failure, so a "column does not exist" error can no longer hide it.

Product fix (#38208)

c0604fe: HostFactoryImpl.findDefaultHost only ran its JSON-column lookup when persist.contentlet.as.json was on. With the flag off, a default site stored only in JSON could not be found, and a fake one was created. The lookup now runs on every database that supports it (Postgres and MSSQL, the only supported databases), and the legacy-column fallback is unchanged.
- With the flag on, which is the default, behaviour is identical.
- The extra query only runs on a site-cache miss.
- There is no schema, index or data change, so the change is rollback-safe.
14a5797, c1fd9c3: the regression test HostFactoryImplDefaultSiteTest is added at the end of MainSuite2a, the shortest suite. If it fails, it puts the real default site back, so one red test can't cascade through the rest of the suite.

Verification

Regression test: it failed 4 of 4 times without the fix and passes with it.
Shorty probe suites: before the fix, the default site's JSON rows went from 1 to 0 and Shorty failed 4 of 4. After it, the rows stayed at 1 and Shorty passed 11 of 11.
TagAPITest: 45 of 45 in a full local MainSuite2b with the JSON fixes, on the pre-rebase base.
Partial run after the rebase: a full MainSuite2b on this branch got through 121 classes with zero "NO DEFAULT HOST" errors. Task211101 and StoryBlockMapTest (12 of 12) passed. The run was stopped before reaching Populate, Shorty and TagAPITest, so this PR's CI run is the check for those.
Final commits: HostFactoryImplDefaultSiteTest 1/1, Task211101 1/1 and PopulateContentletAsJSONUtilTest 3/3, each on a fresh stack. The column guard and the default-site cleanup were checked with temporary local failure injection.

What to look for in CI:

No "NO DEFAULT HOST" lines in MainSuite 2b.
No ShortyServletAndTitleImageTest or TagAPITest in the Flakes block.

#38216 (the StoryBlockMapTest grid test) may share this cause. It has never failed locally, so it stays open until CI shows otherwise.

Fixes #38208
Fixes #38209
Refs #38215
Refs #38216

This PR fixes: #38209

…nColumnTest drops the column The test drops contentlet_as_json and lets the upgrade task add it back empty, which empties the JSON of every contentlet in the shared test database. In MainSuite2b that includes the starter default site, so the next call to findDefaultHost finds no default site and creates an unpublished one named noDefault-<millis>. ShortyServletAndTitleImageTest then resolves the request to that site, gets a 503 and fails its first attempt in every run. The test now copies the JSON into a backup table before dropping the column and writes it back afterwards, through a small ContentletAsJsonBackup helper that PopulateContentletAsJSONUtilTest uses too. With the default site as the probe, run after the upgrade test: its JSON rows go 1 -> 0 and the Shorty test fails 4 attempts of 4 before the change; they stay 1 -> 1 and the Shorty class passes 11 of 11 after. Refs #38209

…Test drops the column Each of its three tests drops contentlet_as_json and adds it back, then fills it for only some of the content. After the class the default site lookup finds no default site, so the next findDefaultHost creates an unpublished noDefault-<millis> site. The full MainSuite2b run logged that from Test_populate_host, and ShortyServletAndTitleImageTest failed its first attempt again. The class now copies the JSON before each test and writes it back after it. With the default site as the probe, run around the class: before the change the default becomes noDefault-<millis> and the Shorty test fails 4 attempts of 4; after it the default stays 'default' with its JSON intact and the Shorty class passes 11 of 11. Refs #38209

… is set to HostFactoryImpl.findDefaultHost only searched the contentlet_as_json column when persist.contentlet.as.json was true. That flag decides how new content is written, but a site saved while it was on lives only in the JSON column. With the flag off and an empty site cache the lookup found no default site, so HostAPIImpl created a second one named noDefault-<millis> and cached it, and later callers got that unpublished site instead of the real one. The JSON lookup now depends on the database supporting JSON, not on the flag, and the legacy column is still the fallback. In MainSuite2b this removes the fake site that Task05380ChangeContainerPathToAbsoluteTest left behind. It is not what made ShortyServletAndTitleImageTest fail; that is #38209. Refs #38208

Switches persist.contentlet.as.json off, empties the site cache and checks that findDefaultHost returns the existing default site and does not create a noDefault-<millis> one. It fails four times out of four before the change to HostFactoryImpl and passes after it. Registered at the end of MainSuite2a, which had the shortest median runtime of the MainSuite jobs across three recent PR runs (#38185, #38176, #38149): 2a 25.4 min, 1a 37.5, 1b 40.8, 2b 42.5, 3a 43.6. Refs #38208

…let JSON Inside PopulateContentletAsJSONUtilTest.Test_populate_host the populate util rebuilds the default site's JSON from legacy columns it does not use, so a default site lookup during the test finds none and creates an unpublished noDefault-<millis> site. Restoring the JSON afterwards brought the real default site back but left the fake one next to it and in the site cache, so ShortyServletAndTitleImageTest still failed its first attempt and the default site seen by TagAPITest could change under it. ContentletAsJsonBackup.restore now also archives and deletes any noDefault-<millis> site and empties the site and contentlet caches. Refs #38209

…ery per row ContentletAsJsonBackup.restore filled the column with a correlated scalar subquery against the backup table, which has no index. Postgres runs that subquery once per contentlet and scans the whole backup each time, so the restore grows with the square of the number of contentlets with JSON. It runs four times per suite, and in a late-suite database it could take minutes. The restore now uses UPDATE ... FROM, a single join. It still only touches contentlets that had JSON when the backup was taken. Refs #38209

…iled test If a test failed after dropping contentlet_as_json and before adding it again, ContentletAsJsonBackup.restore threw "column does not exist". In Task211101AddContentletAsJsonColumnTest that exception came from the finally block, replaced the real failure, and left the JSON unrestored for the rest of the suite. restore now adds the column back with Task211101AddContentletAsJsonColumn, which gives it the same type the test re-adds, before copying the JSON in. The backup is AutoCloseable and the upgrade task test uses it in try-with-resources, so a restore that still fails is attached to the test's own failure as a suppressed exception instead of hiding it. Refs #38209

…t fails When HostFactoryImplDefaultSiteTest fails, the lookup has saved a noDefault-<millis> site as the default, and saving it took the default flag off the real default site. The finally block deleted the fake site but left the real one unmarked, so one red run would break the default site for every test after it in the suite. The finally block now marks the site that was the default before the test as the default again if it lost the flag, then removes the fake site and empties the site cache. Also adds Javadoc to prepare(). Refs #38208

dotCMS-Machine-User

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

_{approved automatically by dotbot}

claude · 2026-10-26T15:37:01Z

Claude finished @swicken's task in 1m 52s —— View job

Code Review — PR #38246

Read the diff against origin/main
Review HostFactoryImpl product fix
Review test helper + test changes
Post review

The product fix is sound. Switching the JSON-column lookup gate from isPersistContentAsJson() to isJsonSupportedDatabase() (HostFactoryImpl.java:827) is strictly broader: with the flag on behavior is unchanged, and with it off a default site that lives only in contentlet_as_json is now found instead of triggering a fake one. The removed IllegalStateException is safe because isJsonSupportedDatabase() is exactly isPostgres() || isMsSql(), so the new else branch can only be the MSSQL query. JSON-lookup failures are still swallowed via Try.of(...).onFailure(log) and fall through to the legacy-column query, so the error path is preserved.

New Issues

🟡 Medium: dotcms-integration/src/test/java/com/dotcms/MainSuite2a.java:39 — HostFactoryImplDefaultSiteTest is registered at position 3 of 82 classes (right after the two ChatCompletions* tests), not at the end. This contradicts both this PR's own stated rationale ("added at the end … so one red test can't cascade through the rest of the suite") and the CLAUDE.md convention to append new registrations at the end of @SuiteClasses. The test deliberately creates the fake-default-site condition and relies on its finally block to restore the real default and delete the second site; if any cleanup step throws (e.g. makeDefault/archive/delete fails), a non-default or noDefault- site leaks into the ~79 tests that follow — exactly the cascade the end-placement was meant to avoid. The last commit (fdcbfa84) moved it here on purpose, so if that placement is intended, please update the PR description, which still says "at the end." Fix this →

Resolved

✅ dotcms-integration/src/test/java/com/dotcms/util/ContentletAsJsonBackup.java:119 — prior P3 (MSSQL portability of the ->/->> fake-site query) is addressed: the integration suite is Postgres-only, and the class Javadoc (lines 26-28) now documents the SQL as PostgreSQL-specific. No code change warranted.

Notes (non-blocking)

The helper's save()/restore() only round-trip rows whose contentlet_as_json was non-null at backup time (ContentletAsJsonBackup.java:58-59, 89-90). Rows a test newly populates stay populated and rows that were null stay null — harmless for the starter default site, which is the thing being protected. Worth keeping in mind if a future test asserts an exact null/non-null count across all contentlets.

Everything else — the try-with-resources wiring in Task211101AddContentletAsJsonColumnTest, the @Before/@After pair in PopulateContentletAsJSONUtilTest, the column-re-add guard, and the cache flushes — looks correct and matches the described verification.

• branch issue-38209-default-site-flake

github-actions · 2026-10-26T15:46:14Z

⚠️ AI review failed

Claude did not produce a review — the backend call errored before generating any output (provider: anthropic-bedrock, model: global.anthropic.claude-sonnet-5). This usually means the model has no Bedrock access grant in the target account, or the model ID is invalid — not a problem with this PR.

_{Run: #40349602295}

github-actions · 2026-10-26T16:10:57Z

⚠️ AI review failed

Claude did not produce a review — the backend call errored before generating any output (provider: anthropic-bedrock, model: global.anthropic.claude-sonnet-5). This usually means the model has no Bedrock access grant in the target account, or the model ID is invalid — not a problem with this PR.

_{Run: #40353737142}

github-actions · 2026-10-26T16:20:03Z

⚠️ AI review failed

Claude did not produce a review — the backend call errored before generating any output (provider: anthropic-bedrock, model: global.anthropic.claude-sonnet-5). This usually means the model has no Bedrock access grant in the target account, or the model ID is invalid — not a problem with this PR.

_{Run: #40353737842}

…ite-flake

…only The helper uses UPDATE ... FROM and the -> / ->> JSON operators. The integration suite only runs on PostgreSQL (docker.image.postgres, PostgreSQL driver in dotcms-integration/pom.xml), so no MSSQL variant is added; the Javadoc now says so.

swicken · 2026-10-27T15:27:33Z

On the Postgres-specific SQL in ContentletAsJsonBackup: agreed that the helper is narrower than the product code, but the integration suite is Postgres-only (parent/pom.xml:337, dotcms-integration/pom.xml:461, docs/testing/INTEGRATION_TESTS.md:13), so I documented that in the class Javadoc instead of adding MSSQL branches that could not be run (282bcd5).

…ecause dotCMS is

dotCMS-Machine-User

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

_{approved automatically by dotbot}

# Conflicts: # dotcms-integration/src/test/java/com/dotcms/MainSuite2a.java

dotCMS-Machine-User

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

_{approved automatically by dotbot}

…e2a instead of at the end Every new test was appended to the end of the suite list, so PRs adding tests to MainSuite2a conflicted with each other in the merge queue. Insert it at its name-derived position instead (see #38318). Refs #38209

github-actions · 2026-10-28T19:00:12Z

dotbot code review:

Reviewer: meta/muse-spark-1.3 (medium)
Overall: patch is correct
New findings this run: 0
Prior unresolved dotbot findings still relevant: 0
Active findings total: 0

Product fix correctly gates JSON lookup on DB capability with legacy fallback preserved, and test helpers restore column state and clean up fake sites. No P0/P1 defect introduced by this patch was found.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · meta/muse-spark-1.3 · medium}

github-actions · 2026-10-28T19:00:30Z

dotbot code review:

Reviewer: ~z-ai/glm-latest (medium)
Overall: patch is correct
New findings this run: 0
Prior unresolved dotbot findings still relevant: 0
Active findings total: 0

The product change correctly gates the JSON lookup in HostFactoryImpl.findDefaultHost on isJsonSupportedDatabase() (Postgres-or-MSSQL), making the removed IllegalStateException unreachable and the new if/else exhaustive, while preserving the legacy-column fallback when no JSON row is found. The test helpers snapshot and restore contentlet_as_json around each column drop, re-add the column if a test fails mid-flight, remove leftover fake noDefault- sites, and flush the host/contentlet caches; the Postgres-only SQL is documented and matches the Postgres-only integration suite. No qualifying defects were introduced by this patch.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

_{reviewed by dotbot · ~z-ai/glm-latest · medium}

dotCMS-Machine-User

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

_{approved automatically by dotbot}

swicken added 8 commits October 26, 2026 09:08

swicken added the PR : dotbot review Trigger dotbot AI code review and the post-merge QA test plan label Oct 26, 2026

github-project-automation Bot added this to dotCMS - Product Planning Oct 26, 2026

github-actions Bot mentioned this pull request Oct 26, 2026

[DEFECT] findDefaultHost ignores the JSON column when persist.contentlet.as.json is off and a fake default site is created #38208

Open

3 tasks

dotCMS-Machine-User approved these changes Oct 26, 2026

View reviewed changes

github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Oct 26, 2026

Merge branch 'main' into issue-38209-default-site-flake

e275eba

Merge remote-tracking branch 'origin/main' into issue-38209-default-s…

04fe04c

…ite-flake

github-actions Bot reviewed Oct 27, 2026

View reviewed changes

Comment thread dotcms-integration/src/test/java/com/dotcms/util/ContentletAsJsonBackup.java

test(sites): say the contentlet JSON backup helper is Postgres-only b…

0c05cf0

…ecause dotCMS is

dotCMS-Machine-User approved these changes Oct 27, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into HEAD

4a9658d

# Conflicts: # dotcms-integration/src/test/java/com/dotcms/MainSuite2a.java

claude Bot added the AI: Safe To Rollback label Oct 28, 2026

dotCMS-Machine-User approved these changes Oct 28, 2026

View reviewed changes

swicken added 2 commits October 28, 2026 14:56

Merge branch 'main' into issue-38209-default-site-flake

4158e07

github-actions Bot removed the AI: Safe To Rollback label Oct 28, 2026

dotCMS-Machine-User approved these changes Oct 28, 2026

View reviewed changes

claude Bot added the AI: Safe To Rollback label Oct 28, 2026

swicken added this pull request to the merge queue Oct 28, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 28, 2026

swicken added this pull request to the merge queue Oct 28, 2026

Merged via the queue into main with commit 50bb2ab Oct 29, 2026
73 checks passed

swicken deleted the issue-38209-default-site-flake branch October 29, 2026 01:33

claude Bot mentioned this pull request Oct 29, 2026

[TASK] Flaky test: ShortyServletAndTitleImageTest fails its first attempt in MainSuite 2b because two tests empty the JSON column and leave no default site #38209

Open

3 tasks

Navigation Menu

Repository navigation

fix(sites): stop the integration suites creating a fake default site - #38246

fix(sites): stop the integration suites creating a fake default site#38246
swicken merged 15 commits into
mainfrom
issue-38209-default-site-flake

swicken commented Oct 26, 2026 •

edited by github-actions Bot

Loading

Uh oh!

dotCMS-Machine-User left a comment

Uh oh!

claude Bot commented Oct 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 26, 2026

Uh oh!

github-actions Bot commented Oct 26, 2026

Uh oh!

github-actions Bot commented Oct 26, 2026

Uh oh!

Uh oh!

swicken commented Oct 27, 2026

Uh oh!

dotCMS-Machine-User left a comment

Uh oh!

dotCMS-Machine-User left a comment

Uh oh!

github-actions Bot commented Oct 28, 2026

Uh oh!

github-actions Bot commented Oct 28, 2026

Uh oh!

dotCMS-Machine-User left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

swicken commented Oct 26, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

The cause

Test fixes (#38209)

Product fix (#38208)

Verification

Uh oh!

dotCMS-Machine-User left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review — PR #38246

New Issues

Resolved

Notes (non-blocking)

Uh oh!

github-actions Bot commented Oct 26, 2026

⚠️ AI review failed

Uh oh!

github-actions Bot commented Oct 26, 2026

⚠️ AI review failed

Uh oh!

github-actions Bot commented Oct 26, 2026

⚠️ AI review failed

Uh oh!

Uh oh!

swicken commented Oct 27, 2026

Uh oh!

dotCMS-Machine-User left a comment

Choose a reason for hiding this comment

Uh oh!

dotCMS-Machine-User left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 28, 2026

Uh oh!

github-actions Bot commented Oct 28, 2026

Uh oh!

dotCMS-Machine-User left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

swicken commented Oct 26, 2026 •

edited by github-actions Bot

Loading

claude Bot commented Oct 26, 2026 •

edited

Loading