Skip to content

Sync: Fix SyncBootStateAccessor to use ILastSyncedManager to prevent unnecessary cold boots#21109

Merged
AndyButland merged 2 commits intomainfrom
v17/bugfix/sync-boot-state-use-ilastsyncedmanager
Dec 10, 2025
Merged

Sync: Fix SyncBootStateAccessor to use ILastSyncedManager to prevent unnecessary cold boots#21109
AndyButland merged 2 commits intomainfrom
v17/bugfix/sync-boot-state-use-ilastsyncedmanager

Conversation

@nikolajlauridsen
Copy link
Contributor

@nikolajlauridsen nikolajlauridsen commented Dec 10, 2025

Summary

  • Fix SyncBootStateAccessor to use the new ILastSyncedManager interface instead of the deprecated LastSyncedFileManager
  • This resolves an issue where cold boots were being triggered every time because the old LastSyncedFileManager was not being updated
  • Add backwards-compatible obsolete constructors using StaticServiceProvider to avoid breaking changes

Test plan

  • Run the new integration tests: dotnet test --filter "FullyQualifiedName~SyncBootStateAccessorTest"
  • Verify cold boot is triggered on fresh install (no last synced ID in database)
  • Verify warm boot is triggered when a valid last synced external ID exists with a corresponding cache instruction
  • Verify existing code using the old constructor signature still compiles (with obsolete warning)

🤖 Generated with Claude Code

nikolajlauridsen and others added 2 commits December 10, 2025 12:43
Update SyncBootStateAccessor to use the new ILastSyncedManager interface
instead of the deprecated LastSyncedFileManager, which was causing cold
boots to be triggered every time.

- Replace LastSyncedFileManager field with ILastSyncedManager
- Add new primary constructor accepting ILastSyncedManager
- Add obsolete constructors for backwards compatibility using StaticServiceProvider
- Update GetSyncBootState to use GetLastSyncedExternalAsync

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add integration tests to verify SyncBootStateAccessor correctly
determines cold boot vs warm boot state based on ILastSyncedManager.

- Test cold boot when no last synced ID exists
- Test warm boot when last synced external ID matches a cache instruction

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copy link
Contributor

Copilot AI left a comment

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 critical bug in SyncBootStateAccessor where it was using the deprecated file-based LastSyncedFileManager instead of the new database-backed ILastSyncedManager. This caused the system to incorrectly trigger cold boots every time because the last synced ID was never persisted to the database.

Key changes:

  • Updated SyncBootStateAccessor to use ILastSyncedManager interface for database-backed persistence
  • Added backward-compatible obsolete constructors to avoid breaking existing code
  • Implemented comprehensive integration tests to verify cold boot and warm boot behavior

Reviewed changes

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

File Description
src/Umbraco.Infrastructure/Sync/SyncBootStateAccessor.cs Replaced LastSyncedFileManager field with ILastSyncedManager, updated constructor to accept new dependency, added two obsolete constructors for backward compatibility, modified GetSyncBootState() to call async method synchronously using established pattern
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Sync/SyncBootStateAccessorTest.cs Added new integration test file with two test cases: one verifying cold boot when no last synced ID exists, and one verifying warm boot when valid last synced ID exists with corresponding cache instruction

Copy link
Contributor

@AndyButland AndyButland left a comment

Choose a reason for hiding this comment

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

Looks good and tests out as expected - I'm no longer seeing the repeated cold boots on startup.

@AndyButland AndyButland enabled auto-merge (squash) December 10, 2025 11:57
@AndyButland AndyButland merged commit 1a4ca0b into main Dec 10, 2025
32 of 33 checks passed
@AndyButland AndyButland deleted the v17/bugfix/sync-boot-state-use-ilastsyncedmanager branch December 10, 2025 13:02
AndyButland pushed a commit that referenced this pull request Dec 10, 2025
…unnecessary cold boots (#21109)

* Sync: Fix SyncBootStateAccessor to use ILastSyncedManager

Update SyncBootStateAccessor to use the new ILastSyncedManager interface
instead of the deprecated LastSyncedFileManager, which was causing cold
boots to be triggered every time.

- Replace LastSyncedFileManager field with ILastSyncedManager
- Add new primary constructor accepting ILastSyncedManager
- Add obsolete constructors for backwards compatibility using StaticServiceProvider
- Update GetSyncBootState to use GetLastSyncedExternalAsync

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

* Sync: Add integration tests for SyncBootStateAccessor

Add integration tests to verify SyncBootStateAccessor correctly
determines cold boot vs warm boot state based on ILastSyncedManager.

- Test cold boot when no last synced ID exists
- Test warm boot when last synced external ID matches a cache instruction

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>

---------

Co-authored-by: Claude Opus 4.5 <[email protected]>
@umbracocommunity
Copy link

This pull request has been mentioned on Umbraco community forum. There might be relevant details there:

https://forum.umbraco.com/t/cold-starts-after-upgrading-to-umbraco-17/6961/6

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.

4 participants