Skip to content

Migrations: Ensure the umbracoContent.contentTypeId index is created when upgrading from 18.0 to 18.1 - #23466

Merged
AndyButland merged 2 commits into
release/18.1from
v18/bugfix/fix-migration-plan-for-18.1
Jul 23, 2026
Merged

Migrations: Ensure the umbracoContent.contentTypeId index is created when upgrading from 18.0 to 18.1#23466
AndyButland merged 2 commits into
release/18.1from
v18/bugfix/fix-migration-plan-for-18.1

Conversation

@AndyButland

@AndyButland AndyButland commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Description

PR #23328 added a migration (V_17_6_0.AddContentTypeIdIndexForContent) that creates the IX_umbracoContent_contentTypeId index on upgrade. It was merged up into the v18 line, but inserted into the migration chain before the 18.0 migrations.

As a result, sites already on an 18.0.x release never receive the index when upgrading to 18.1: their stored migration state ({6FE4656E-8B8D-452F-AE2A-438A615B61BC}, the final 18.0 state) sits after the 17.6 migration in the linear chain, so the upgrader — which only walks forward — never reaches it.

Fresh 18.1 installs are unaffected (the index is declared on ContentDto), as are upgrades from 17.x (they pass through the 17.6 step).

To fix I have added:

  • A new migration V_18_1_0.AddContentTypeIdIndexForContent is appended to the end of the plan with a new GUID ({AE533AF6-4611-4E25-AA4D-89AEFA468E79}), giving 18.0.x installs a forward path to it.
  • It inherits V_17_6_0.AddContentTypeIdIndexForContent, so both paths build the exact same DTO-derived index. The operation is idempotent (guarded by IndexExists), so sites that already gained the index via the 17.6 path are unaffected.
  • The migration lives in the V_18_1_0 namespace (rather than re-referencing the 17.6 type) so that RuntimeState.CurrentMigrationVersion — derived from the final migration type's namespace via GetVersionForState — correctly reports 18.1.0.

Testing

Verified by running the app against an 18.0 database: the migration runs, the stored migration state in the database is updated, and the IX_umbracoContent_contentTypeId index is created.

Copilot AI review requested due to automatic review settings July 23, 2026 11:44
@claude

claude Bot commented Jul 23, 2026

Copy link
Copy Markdown

Claude finished @AndyButland's task in 2m 19s —— View job


PR Review

  • Gather context and understand the request
  • Load review standards
  • Collect changed files and diff
  • Perform code review
  • Analyze impact and breaking changes
  • Post review findings

PR Review

Target: origin/release/18.1 · Based on commit: c3d564bc

Adds a new migration step (V_18_1_0.AddContentTypeIdIndexForContent) at the end of the 18.x plan so sites upgrading from 18.0.x receive the IX_umbracoContent_contentTypeId index that the 17.6 path delivers to 17.x upgrades — plugging the gap that existed because the 17.6 migration was inserted before the final 18.0 state in the linear chain.

  • Other changes: Sites upgrading from 18.0.x will now have the IX_umbracoContent_contentTypeId index created on first startup after upgrading to 18.1. The operation is idempotent (guarded by IndexExists), so all other upgrade paths are unaffected.

Suggestions

  • src/Umbraco.Infrastructure/Migrations/Upgrade/V_18_1_0/AddContentTypeIdIndexForContent.cs:1: No automated test covers the 18.0 → 18.1 upgrade path. The idempotency and the index creation logic are inherited from the 17.6 migration, but a test that verifies the migration is invoked when starting from an 18.0 state (i.e., with state {6FE4656E-8B8D-452F-AE2A-438A615B61BC}) would prevent a regression if the plan ordering were accidentally changed. The existing MigrationPlanTests.cs would be a natural home for this.

Approved

The fix is correct and well-reasoned. The inheritance approach is clean and DRY — the 18.1 wrapper class delegates all logic to the 17.6 migration while living in the correct namespace for version reporting. The GUID is unique in the plan, the comment in UmbracoPlan.cs is justified (explains a non-obvious ordering constraint), and no public API surface is touched. Please do a manual sanity check and testing before merging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a migration-plan gap for v18 upgrades by ensuring the IX_umbracoContent_contentTypeId index is created when upgrading from 18.0.x → 18.1, even though the original index migration was merged into the v18 branch before the final 18.0 migration state.

Changes:

  • Adds a new V_18_1_0 migration that reuses the existing V_17_6_0 index-creation logic via inheritance.
  • Appends a new terminal migration state in UmbracoPlan (18.1.0) so 18.0.x installs have a forward path to run the index migration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/Umbraco.Infrastructure/Migrations/Upgrade/V_18_1_0/AddContentTypeIdIndexForContent.cs Introduces an 18.1 migration that re-applies the existing 17.6 index migration logic for 18.0.x upgrade paths.
src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs Appends the new 18.1 migration/state so databases ending at the 18.0 final state will still execute the index creation when moving forward.

@claude claude Bot added the area/backend label Jul 23, 2026
@sonarqubecloud

Copy link
Copy Markdown

@lauraneto lauraneto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just gave this a test by updating from a 18.0.0 database (ran only new migration) and a 17.5.3 database (ran both migrations), and it worked as expected.
I do think this will be tricky to prevent though, maybe something to discuss with the group. 🤔

@AndyButland

Copy link
Copy Markdown
Contributor Author

Thanks Laura. Yes, I agree - it should settle down though, as we move on with the LTS and STS version, the latter will be getting less updates, and less migrations; and really just ones like this, for performance. So we shouldn't have to remember too often. But hopefully the memory file updates I included will help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants