Umbraco-CMS/docs/plans/2025-12-23-contentservice-refactor-phase3-implementation.md

ContentService Version Operations Extraction - Phase 3 Implementation Plan

For Claude: REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

Goal: Extract version operations (GetVersion, GetVersions, GetVersionsSlim, GetVersionIds, Rollback, DeleteVersions, DeleteVersion) from ContentService into a dedicated IContentVersionOperationService interface and ContentVersionOperationService implementation.

Architecture: ContentVersionOperationService implements IContentVersionOperationService and inherits from ContentServiceBase. The existing ContentService facade will delegate to ContentVersionOperationService for version operations. The new service handles synchronous version operations extracted from IContentService, separate from the existing async IContentVersionService which provides API-layer functionality.

Tech Stack: .NET 10.0, NUnit, Microsoft.Extensions.DependencyInjection

---

Plan Version History

Version	Date	Author	Changes
1.0	2025-12-23	Claude	Initial plan creation
1.1	2025-12-23	Claude	Applied critical review fixes (Issues 2.1-2.5, 3.2-3.4)
1.2	2025-12-23	Claude	Applied critical review v2 fixes (Issues 2.1-2.2, 3.1-3.4)
1.3	2025-12-23	Claude	Applied critical review v3 fixes (Issues 3.1-3.4, 3.6)

---

Changes in Version 1.1

Summary: Applied fixes from critical implementation review (2025-12-23-contentservice-refactor-phase3-implementation-critical-review-1.md)

Critical Fixes Applied:

1. Issue 2.1 - Rollback TOCTOU Race Condition: Consolidated Rollback into a single scope to eliminate race condition between read and write operations. Removed PerformRollback helper method; all operations now occur within one scope.

2. Issue 2.2 - Rollback Error Handling: Added try-catch around save operation with proper error logging ("was unable to rollback") and conditional notification (only fires ContentRolledBackNotification on success).

3. Issue 2.3 - GetVersionIds Missing ReadLock: Added scope.ReadLock(Constants.Locks.ContentTree) for consistency with other read operations. Added code comment explaining this is a bug fix.

4. Issue 2.4 - DeleteVersion Nested Scope: Changed deletePriorVersions branch to use DocumentRepository.GetVersion() and DocumentRepository.DeleteVersions() directly instead of calling the service methods that create nested scopes.

5. Issue 2.5 - Flaky Test Pattern: Replaced Thread.Sleep(100) with deterministic date comparison using actual version update dates.

Minor Improvements Applied:

6. Issue 3.2 - Cancellation Test: Added Rollback_WhenNotificationCancelled_ReturnsCancelledResult test.

7. Issue 3.3 - Published Version Protection Test: Added DeleteVersion_PublishedVersion_DoesNotDelete test.

8. Issue 3.4 - Interface Documentation: Enhanced GetVersionIds XML documentation to specify behavior for non-existent content and maxRows edge cases.

---

Changes in Version 1.2

Summary: Applied fixes from critical implementation review v2 (2025-12-23-contentservice-refactor-phase3-implementation-critical-review-2.md)

Critical Fixes Applied:

1. Issue 2.1 - Rollback Notification Preservation: Changed Rollback to use CrudService.Save instead of DocumentRepository.Save to preserve ContentSavingNotification and ContentSavedNotification firing. This maintains backward compatibility with existing notification handlers and ensures proper validation and audit trail.

2. Issue 2.2 - DeleteVersion Double-Notification Preservation: When deletePriorVersions=true, the method now fires notifications for both the prior versions deletion AND the specific version deletion, matching original behavior.

Minor Improvements Applied:

3. Issue 3.1 - Simplified Lock Acquisition: Restructured DeleteVersion to acquire WriteLock once at the start instead of multiple times, avoiding lock upgrade pattern.

4. Issue 3.2 - Test Notification Registration: Fixed the cancellation test to use the correct integration test pattern with CustomTestSetup and notification handler registration.

5. Issue 3.3 - CrudService Dependency: Added IContentCrudService as a constructor dependency to support proper save operation in Rollback.

6. Issue 3.4 - Publish Method Signature: Updated test to use correct SaveAndPublish method signature.

---

Changes in Version 1.3

Summary: Applied fixes from critical implementation review v3 (2025-12-23-contentservice-refactor-phase3-implementation-critical-review-3.md)

Must Fix Applied:

1. Issue 3.4 - Rollback Return Type: Fixed OperationResult<OperationResultType> to OperationResult to match IContentCrudService.Save signature and avoid compilation error.

Should Fix Applied:

2. Issue 3.1 - GetVersionIds Input Validation: Added ArgumentOutOfRangeException for maxRows <= 0 to match interface documentation contract.

3. Issue 3.3 - Audit Gap in DeleteVersion: Added audit entry for prior versions deletion when deletePriorVersions=true to preserve original two-audit-entry behavior.

Consider Applied:

4. Issue 3.2 - Redundant WriteLock in Rollback: Removed explicit WriteLock before CrudService.Save since CrudService handles its own locking internally.

5. Issue 3.6 - Prior Versions Cancellation Comment: Added clarifying comment documenting that specific version deletion proceeds even if prior versions notification is cancelled (matching original behavior).

---

Naming Decision

Conflict Resolution: An IContentVersionService already exists in the codebase (src/Umbraco.Core/Services/ContentVersionService.cs). This is an async API-layer service that provides version cleanup, paged version listing, and rollback via IContentService.

Decision: Name the new interface IContentVersionOperationService to avoid collision, following the pattern established for IContentPublishOperationService. The existing IContentVersionService remains unchanged as the higher-level async API orchestrator.

---

Phase 3 Overview

Phase 3 extracts version-related operations following patterns established in Phases 1-2:

1. IContentVersionOperationService - Interface for version operations (7 methods)
2. ContentVersionOperationService - Implementation inheriting from ContentServiceBase
3. DI Registration - Register new service alongside existing services
4. ContentService Delegation - Update facade to delegate to new service

The extraction reduces ContentService complexity by ~150 lines.

---

Methods to Extract

Method	Signature	Notifications
GetVersion	IContent? GetVersion(int versionId)	None (read-only)
GetVersions	IEnumerable<IContent> GetVersions(int id)	None (read-only)
GetVersionsSlim	IEnumerable<IContent> GetVersionsSlim(int id, int skip, int take)	None (read-only)
GetVersionIds	IEnumerable<int> GetVersionIds(int id, int maxRows)	None (read-only)
Rollback	OperationResult Rollback(int id, int versionId, string culture, int userId)	ContentRollingBackNotification, ContentRolledBackNotification
DeleteVersions	void DeleteVersions(int id, DateTime versionDate, int userId)	ContentDeletingVersionsNotification, ContentDeletedVersionsNotification
DeleteVersion	void DeleteVersion(int id, int versionId, bool deletePriorVersions, int userId)	ContentDeletingVersionsNotification, ContentDeletedVersionsNotification

---

Task 1: Create IContentVersionOperationService Interface

Files:

• Create: src/Umbraco.Core/Services/IContentVersionOperationService.cs

Step 1: Create the interface file

// src/Umbraco.Core/Services/IContentVersionOperationService.cs
using Umbraco.Cms.Core.Models;

namespace Umbraco.Cms.Core.Services;

/// <summary>
/// Service for content version operations (retrieving versions, rollback, deleting versions).
/// </summary>
/// <remarks>
/// <para>
/// <strong>Implementation Note:</strong> Do not implement this interface directly.
/// Instead, inherit from <see cref="ContentServiceBase"/> which provides required
/// infrastructure (scoping, repository access, auditing). Direct implementation
/// without this base class will result in missing functionality.
/// </para>
/// <para>
/// This interface is part of the ContentService refactoring initiative (Phase 3).
/// It extracts version operations into a focused, testable service.
/// </para>
/// <para>
/// <strong>Note:</strong> This interface provides synchronous version operations
/// extracted from <see cref="IContentService"/>. For async API-layer version operations,
/// see <see cref="IContentVersionService"/> which orchestrates via this service.
/// </para>
/// <para>
/// <strong>Versioning Policy:</strong> This interface follows additive-only changes.
/// New methods may be added with default implementations. Existing methods will not
/// be removed or have signatures changed without a 2 major version deprecation period.
/// </para>
/// <para>
/// <strong>Version History:</strong>
/// <list type="bullet">
///   <item><description>v1.0 (Phase 3): Initial interface with GetVersion, GetVersions, Rollback, DeleteVersions operations</description></item>
/// </list>
/// </para>
/// </remarks>
/// <since>1.0</since>
public interface IContentVersionOperationService : IService
{
    #region Version Retrieval

    /// <summary>
    /// Gets a specific version of content by version id.
    /// </summary>
    /// <param name="versionId">The version id to retrieve.</param>
    /// <returns>The content version, or null if not found.</returns>
    IContent? GetVersion(int versionId);

    /// <summary>
    /// Gets all versions of a content item.
    /// </summary>
    /// <param name="id">The content id.</param>
    /// <returns>All versions of the content, ordered by version date descending.</returns>
    IEnumerable<IContent> GetVersions(int id);

    /// <summary>
    /// Gets a paged subset of versions for a content item.
    /// </summary>
    /// <param name="id">The content id.</param>
    /// <param name="skip">Number of versions to skip.</param>
    /// <param name="take">Number of versions to take.</param>
    /// <returns>Paged versions of the content, ordered by version date descending.</returns>
    IEnumerable<IContent> GetVersionsSlim(int id, int skip, int take);

    /// <summary>
    /// Gets version ids for a content item, ordered with latest first.
    /// </summary>
    /// <param name="id">The content id.</param>
    /// <param name="maxRows">Maximum number of version ids to return. Must be positive.</param>
    /// <returns>Version ids ordered with latest first. Empty if content not found.</returns>
    /// <exception cref="ArgumentOutOfRangeException">Thrown if maxRows is less than or equal to zero.</exception>
    /// <remarks>
    /// This method acquires a read lock on the content tree for consistency with other
    /// version retrieval methods. If content with the specified id does not exist,
    /// an empty enumerable is returned rather than throwing an exception.
    /// </remarks>
    IEnumerable<int> GetVersionIds(int id, int maxRows);

    #endregion

    #region Rollback

    /// <summary>
    /// Rolls back content to a previous version.
    /// </summary>
    /// <param name="id">The content id to rollback.</param>
    /// <param name="versionId">The version id to rollback to.</param>
    /// <param name="culture">The culture to rollback, or "*" for all cultures.</param>
    /// <param name="userId">The user performing the rollback.</param>
    /// <returns>The operation result indicating success or failure.</returns>
    /// <remarks>
    /// Fires <see cref="Notifications.ContentRollingBackNotification"/> (cancellable) before rollback
    /// and <see cref="Notifications.ContentRolledBackNotification"/> after successful rollback.
    /// The rollback copies property values from the target version to the current content
    /// and saves it, creating a new version.
    /// </remarks>
    OperationResult Rollback(int id, int versionId, string culture = "*", int userId = Constants.Security.SuperUserId);

    #endregion

    #region Version Deletion

    /// <summary>
    /// Permanently deletes versions of content prior to a specific date.
    /// </summary>
    /// <param name="id">The content id.</param>
    /// <param name="versionDate">Delete versions older than this date.</param>
    /// <param name="userId">The user performing the deletion.</param>
    /// <remarks>
    /// This method will never delete the latest version of a content item.
    /// Fires <see cref="Notifications.ContentDeletingVersionsNotification"/> (cancellable) before deletion
    /// and <see cref="Notifications.ContentDeletedVersionsNotification"/> after deletion.
    /// </remarks>
    void DeleteVersions(int id, DateTime versionDate, int userId = Constants.Security.SuperUserId);

    /// <summary>
    /// Permanently deletes a specific version of content.
    /// </summary>
    /// <param name="id">The content id.</param>
    /// <param name="versionId">The version id to delete.</param>
    /// <param name="deletePriorVersions">If true, also deletes all versions prior to the specified version.</param>
    /// <param name="userId">The user performing the deletion.</param>
    /// <remarks>
    /// This method will never delete the current version or published version of a content item.
    /// Fires <see cref="Notifications.ContentDeletingVersionsNotification"/> (cancellable) before deletion
    /// and <see cref="Notifications.ContentDeletedVersionsNotification"/> after deletion.
    /// If deletePriorVersions is true, it first deletes all versions prior to the specified version's date,
    /// then deletes the specified version.
    /// </remarks>
    void DeleteVersion(int id, int versionId, bool deletePriorVersions, int userId = Constants.Security.SuperUserId);

    #endregion
}

Step 2: Run build to verify interface compiles

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 3: Commit

git add src/Umbraco.Core/Services/IContentVersionOperationService.cs
git commit -m "$(cat <<'EOF'
refactor(core): add IContentVersionOperationService interface

Part of ContentService refactoring Phase 3.
Defines version operations to be extracted from ContentService.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 2: Create ContentVersionOperationService Implementation

Files:

• Create: src/Umbraco.Core/Services/ContentVersionOperationService.cs

Step 1: Create the implementation file

v1.1 Changes Applied:

• Issue 2.1: Consolidated Rollback into single scope (eliminated TOCTOU race condition)
• Issue 2.2: Added try-catch error handling with proper logging
• Issue 2.3: Added ReadLock to GetVersionIds for consistency
• Issue 2.4: Fixed DeleteVersion to use repository directly instead of nested service calls

v1.2 Changes Applied:

• Issue 2.1: Use CrudService.Save instead of DocumentRepository.Save to preserve ContentSaving/ContentSaved notifications
• Issue 2.2: Preserve double-notification behavior for DeleteVersion with deletePriorVersions=true
• Issue 3.1: Simplified lock acquisition in DeleteVersion (single WriteLock at start)
• Issue 3.3: Added IContentCrudService constructor dependency

v1.3 Changes Applied:

• Issue 3.1: Added input validation to GetVersionIds (ArgumentOutOfRangeException for maxRows <= 0)
• Issue 3.2: Removed redundant WriteLock in Rollback (CrudService.Save handles locking)
• Issue 3.3: Added audit entry for prior versions in DeleteVersion with deletePriorVersions=true
• Issue 3.4: Fixed Rollback return type from OperationResult to OperationResult
• Issue 3.6: Added clarifying comment for prior versions cancellation behavior

// src/Umbraco.Core/Services/ContentVersionOperationService.cs
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;

namespace Umbraco.Cms.Core.Services;

/// <summary>
/// Implements content version operations (retrieving versions, rollback, deleting versions).
/// </summary>
public class ContentVersionOperationService : ContentServiceBase, IContentVersionOperationService
{
    private readonly ILogger<ContentVersionOperationService> _logger;
    // v1.2 Fix (Issue 3.3): Added IContentCrudService for proper save with notifications
    private readonly IContentCrudService _crudService;

    public ContentVersionOperationService(
        ICoreScopeProvider provider,
        ILoggerFactory loggerFactory,
        IEventMessagesFactory eventMessagesFactory,
        IDocumentRepository documentRepository,
        IAuditService auditService,
        IUserIdKeyResolver userIdKeyResolver,
        IContentCrudService crudService)  // v1.2: Added for Rollback save operation
        : base(provider, loggerFactory, eventMessagesFactory, documentRepository, auditService, userIdKeyResolver)
    {
        _logger = loggerFactory.CreateLogger<ContentVersionOperationService>();
        _crudService = crudService;
    }

    #region Version Retrieval

    /// <inheritdoc />
    public IContent? GetVersion(int versionId)
    {
        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
        scope.ReadLock(Constants.Locks.ContentTree);
        return DocumentRepository.GetVersion(versionId);
    }

    /// <inheritdoc />
    public IEnumerable<IContent> GetVersions(int id)
    {
        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
        scope.ReadLock(Constants.Locks.ContentTree);
        return DocumentRepository.GetAllVersions(id);
    }

    /// <inheritdoc />
    public IEnumerable<IContent> GetVersionsSlim(int id, int skip, int take)
    {
        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
        scope.ReadLock(Constants.Locks.ContentTree);
        return DocumentRepository.GetAllVersionsSlim(id, skip, take);
    }

    /// <inheritdoc />
    public IEnumerable<int> GetVersionIds(int id, int maxRows)
    {
        // v1.3 Fix (Issue 3.1): Added input validation to match interface documentation.
        // The interface documents ArgumentOutOfRangeException for maxRows <= 0.
        if (maxRows <= 0)
        {
            throw new ArgumentOutOfRangeException(nameof(maxRows), maxRows, "Value must be greater than zero.");
        }

        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
        // v1.1 Fix (Issue 2.3): Added ReadLock for consistency with other read operations.
        // The original ContentService.GetVersionIds did not acquire a ReadLock, which was
        // inconsistent with GetVersion, GetVersions, and GetVersionsSlim.
        scope.ReadLock(Constants.Locks.ContentTree);
        return DocumentRepository.GetVersionIds(id, maxRows);
    }

    #endregion

    #region Rollback

    /// <inheritdoc />
    public OperationResult Rollback(int id, int versionId, string culture = "*", int userId = Constants.Security.SuperUserId)
    {
        EventMessages evtMsgs = EventMessagesFactory.Get();

        // v1.1 Fix (Issue 2.1): Use a single scope for the entire operation to eliminate
        // TOCTOU race condition. Previously used separate read and write scopes which
        // could allow concurrent modification between reading content and writing changes.
        using ICoreScope scope = ScopeProvider.CreateCoreScope();

        // Read operations - acquire read lock first
        scope.ReadLock(Constants.Locks.ContentTree);
        IContent? content = DocumentRepository.Get(id);
        // v1.1 Fix: Use DocumentRepository.GetVersion directly instead of calling
        // this.GetVersion() which would create a nested scope
        IContent? version = DocumentRepository.GetVersion(versionId);

        // Null checks - cannot rollback if content or version is missing, or if trashed
        if (content == null || version == null || content.Trashed)
        {
            scope.Complete();
            return new OperationResult(OperationResultType.FailedCannot, evtMsgs);
        }

        var rollingBackNotification = new ContentRollingBackNotification(content, evtMsgs);
        if (scope.Notifications.PublishCancelable(rollingBackNotification))
        {
            scope.Complete();
            return OperationResult.Cancel(evtMsgs);
        }

        // Copy the changes from the version
        content.CopyFrom(version, culture);

        // v1.2 Fix (Issue 2.1): Use CrudService.Save to preserve ContentSaving/ContentSaved notifications.
        // The original ContentService.Rollback called Save(content, userId) which fires these notifications.
        // Using DocumentRepository.Save directly would bypass validation, audit trail, and cache invalidation.
        // v1.3 Fix (Issue 3.2): Removed explicit WriteLock - CrudService.Save handles its own locking internally.
        // v1.3 Fix (Issue 3.4): Fixed return type from OperationResult<OperationResultType> to OperationResult.
        OperationResult saveResult = _crudService.Save(content, userId);
        if (!saveResult.Success)
        {
            _logger.LogError("User '{UserId}' was unable to rollback content '{ContentId}' to version '{VersionId}'", userId, id, versionId);
            scope.Complete();
            return new OperationResult(OperationResultType.Failed, evtMsgs);
        }

        // Only publish success notification if save succeeded
        scope.Notifications.Publish(
            new ContentRolledBackNotification(content, evtMsgs).WithStateFrom(rollingBackNotification));

        // Logging & Audit
        _logger.LogInformation("User '{UserId}' rolled back content '{ContentId}' to version '{VersionId}'", userId, content.Id, version.VersionId);
        Audit(AuditType.RollBack, userId, content.Id, $"Content '{content.Name}' was rolled back to version '{version.VersionId}'");

        scope.Complete();

        return OperationResult.Succeed(evtMsgs);
    }

    #endregion

    #region Version Deletion

    /// <inheritdoc />
    public void DeleteVersions(int id, DateTime versionDate, int userId = Constants.Security.SuperUserId)
    {
        EventMessages evtMsgs = EventMessagesFactory.Get();

        using ICoreScope scope = ScopeProvider.CreateCoreScope();

        var deletingVersionsNotification = new ContentDeletingVersionsNotification(id, evtMsgs, dateToRetain: versionDate);
        if (scope.Notifications.PublishCancelable(deletingVersionsNotification))
        {
            scope.Complete();
            return;
        }

        scope.WriteLock(Constants.Locks.ContentTree);
        DocumentRepository.DeleteVersions(id, versionDate);

        scope.Notifications.Publish(
            new ContentDeletedVersionsNotification(id, evtMsgs, dateToRetain: versionDate).WithStateFrom(deletingVersionsNotification));
        Audit(AuditType.Delete, userId, Constants.System.Root, "Delete (by version date)");

        scope.Complete();
    }

    /// <inheritdoc />
    public void DeleteVersion(int id, int versionId, bool deletePriorVersions, int userId = Constants.Security.SuperUserId)
    {
        EventMessages evtMsgs = EventMessagesFactory.Get();

        using ICoreScope scope = ScopeProvider.CreateCoreScope();

        // v1.2 Fix (Issue 3.1): Acquire WriteLock once at the start instead of multiple times.
        // This simplifies the code and avoids the read→write lock upgrade pattern.
        scope.WriteLock(Constants.Locks.ContentTree);

        var deletingVersionsNotification = new ContentDeletingVersionsNotification(id, evtMsgs, versionId);
        if (scope.Notifications.PublishCancelable(deletingVersionsNotification))
        {
            scope.Complete();
            return;
        }

        // v1.2 Fix (Issue 2.2): Preserve original double-notification behavior for deletePriorVersions.
        // The original implementation called DeleteVersions() which fired its own notifications.
        // We inline the notification firing to maintain backward compatibility.
        // v1.3 Fix (Issue 3.6): Clarification - if prior versions deletion is cancelled, we still
        // proceed with deleting the specific version. This matches original ContentService behavior.
        if (deletePriorVersions)
        {
            IContent? versionContent = DocumentRepository.GetVersion(versionId);
            DateTime cutoffDate = versionContent?.UpdateDate ?? DateTime.UtcNow;

            // Publish notifications for prior versions (matching original behavior)
            var priorVersionsNotification = new ContentDeletingVersionsNotification(id, evtMsgs, dateToRetain: cutoffDate);
            if (!scope.Notifications.PublishCancelable(priorVersionsNotification))
            {
                DocumentRepository.DeleteVersions(id, cutoffDate);
                scope.Notifications.Publish(
                    new ContentDeletedVersionsNotification(id, evtMsgs, dateToRetain: cutoffDate)
                        .WithStateFrom(priorVersionsNotification));

                // v1.3 Fix (Issue 3.3): Add audit entry for prior versions deletion.
                // The original DeleteVersions() method created its own audit entry.
                Audit(AuditType.Delete, userId, Constants.System.Root, "Delete (by version date)");
            }
        }

        IContent? c = DocumentRepository.Get(id);

        // Don't delete the current or published version
        if (c?.VersionId != versionId && c?.PublishedVersionId != versionId)
        {
            DocumentRepository.DeleteVersion(versionId);
        }

        scope.Notifications.Publish(
            new ContentDeletedVersionsNotification(id, evtMsgs, versionId).WithStateFrom(deletingVersionsNotification));
        Audit(AuditType.Delete, userId, Constants.System.Root, "Delete (by version)");

        scope.Complete();
    }

    #endregion
}

Step 2: Run build to verify implementation compiles

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 3: Commit

git add src/Umbraco.Core/Services/ContentVersionOperationService.cs
git commit -m "$(cat <<'EOF'
refactor(core): add ContentVersionOperationService implementation

Part of ContentService refactoring Phase 3.
Implements version retrieval, rollback, and version deletion operations.

v1.1 fixes applied:
- Consolidated Rollback into single scope (Issue 2.1)
- Added error handling to Rollback (Issue 2.2)
- Added ReadLock to GetVersionIds (Issue 2.3)
- Fixed DeleteVersion nested scope (Issue 2.4)

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 3: Register Service in DI Container

Files:

• Modify: src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs

Step 1: Add service registration

Locate the line:

Services.AddUnique<IContentQueryOperationService, ContentQueryOperationService>();

Add after it:

Services.AddUnique<IContentVersionOperationService, ContentVersionOperationService>();

Step 2: Update ContentService factory registration

Find the IContentService factory registration (starts with Services.AddUnique<IContentService>(sp =>).

Add the new parameter to the constructor call:

sp.GetRequiredService<IContentVersionOperationService>()

The updated registration should look like:

Services.AddUnique<IContentService>(sp =>
    new ContentService(
        sp.GetRequiredService<ICoreScopeProvider>(),
        sp.GetRequiredService<ILoggerFactory>(),
        sp.GetRequiredService<IEventMessagesFactory>(),
        sp.GetRequiredService<IDocumentRepository>(),
        sp.GetRequiredService<IEntityRepository>(),
        sp.GetRequiredService<IAuditService>(),
        sp.GetRequiredService<IContentTypeRepository>(),
        sp.GetRequiredService<IDocumentBlueprintRepository>(),
        sp.GetRequiredService<ILanguageRepository>(),
        sp.GetRequiredService<Lazy<IPropertyValidationService>>(),
        sp.GetRequiredService<IShortStringHelper>(),
        sp.GetRequiredService<ICultureImpactFactory>(),
        sp.GetRequiredService<IUserIdKeyResolver>(),
        sp.GetRequiredService<PropertyEditorCollection>(),
        sp.GetRequiredService<IIdKeyMap>(),
        sp.GetRequiredService<IOptionsMonitor<ContentSettings>>(),
        sp.GetRequiredService<IRelationService>(),
        sp.GetRequiredService<IContentCrudService>(),
        sp.GetRequiredService<IContentQueryOperationService>(),
        sp.GetRequiredService<IContentVersionOperationService>()));  // NEW

Step 3: Run build to verify registration

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build fails because ContentService doesn't have the new constructor parameter yet (expected)

Step 4: Commit

git add src/Umbraco.Core/DependencyInjection/UmbracoBuilder.cs
git commit -m "$(cat <<'EOF'
refactor(core): register IContentVersionOperationService in DI

Part of ContentService refactoring Phase 3.
Adds service registration and updates ContentService factory.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 4: Add VersionOperationService Property to ContentService

Files:

• Modify: src/Umbraco.Core/Services/ContentService.cs

Step 1: Add private fields for the service

After the existing _queryOperationServiceLazy field declaration, add:

// Version operation service fields (for Phase 3 extracted version operations)
private readonly IContentVersionOperationService? _versionOperationService;
private readonly Lazy<IContentVersionOperationService>? _versionOperationServiceLazy;

Step 2: Add property accessor

After the QueryOperationService property, add:

/// <summary>
/// Gets the version operation service.
/// </summary>
/// <exception cref="InvalidOperationException">Thrown if the service was not properly initialized.</exception>
private IContentVersionOperationService VersionOperationService =>
    _versionOperationService ?? _versionOperationServiceLazy?.Value
    ?? throw new InvalidOperationException("VersionOperationService not initialized. Ensure the service is properly injected via constructor.");

Step 3: Update the primary constructor

Find the constructor marked with [ActivatorUtilitiesConstructor].

Add the new parameter:

IContentVersionOperationService versionOperationService)  // NEW PARAMETER - Phase 3 version operations

In the constructor body, add:

// Phase 3: Version operation service (direct injection)
ArgumentNullException.ThrowIfNull(versionOperationService);
_versionOperationService = versionOperationService;
_versionOperationServiceLazy = null;  // Not needed when directly injected

Step 4: Update the obsolete constructors

Find the obsolete constructors (marked with [Obsolete]). Add lazy resolution for the new service:

// Phase 3: Lazy resolution of IContentVersionOperationService
_versionOperationServiceLazy = new Lazy<IContentVersionOperationService>(() =>
    StaticServiceProvider.Instance.GetRequiredService<IContentVersionOperationService>(),
    LazyThreadSafetyMode.ExecutionAndPublication);

Step 5: Run build to verify

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 6: Commit

git add src/Umbraco.Core/Services/ContentService.cs
git commit -m "$(cat <<'EOF'
refactor(core): add VersionOperationService property to ContentService

Part of ContentService refactoring Phase 3.
Adds constructor parameter and property for version operations delegation.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 5: Delegate Version Retrieval Methods

Files:

• Modify: src/Umbraco.Core/Services/ContentService.cs

Step 1: Delegate GetVersion

Find the GetVersion method (around line 572). Replace the implementation with:

public IContent? GetVersion(int versionId)
    => VersionOperationService.GetVersion(versionId);

Step 2: Delegate GetVersions

Find the GetVersions method (around line 586). Replace the implementation with:

public IEnumerable<IContent> GetVersions(int id)
    => VersionOperationService.GetVersions(id);

Step 3: Delegate GetVersionsSlim

Find the GetVersionsSlim method (around line 599). Replace the implementation with:

public IEnumerable<IContent> GetVersionsSlim(int id, int skip, int take)
    => VersionOperationService.GetVersionsSlim(id, skip, take);

Step 4: Delegate GetVersionIds

Find the GetVersionIds method (around line 614). Replace the implementation with:

public IEnumerable<int> GetVersionIds(int id, int maxRows)
    => VersionOperationService.GetVersionIds(id, maxRows);

Step 5: Run build to verify

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 6: Run tests to verify behavior unchanged

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentService" --no-restore Expected: All tests pass

Step 7: Commit

git add src/Umbraco.Core/Services/ContentService.cs
git commit -m "$(cat <<'EOF'
refactor(core): delegate version retrieval methods to VersionOperationService

Part of ContentService refactoring Phase 3.
Delegates GetVersion, GetVersions, GetVersionsSlim, GetVersionIds.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 6: Delegate Rollback Method

Files:

• Modify: src/Umbraco.Core/Services/ContentService.cs

Step 1: Update Rollback to delegate to VersionOperationService

Find the Rollback method in the #region Rollback section (around line 243).

Note: The current Rollback implementation calls GetById and Save from the current ContentService. The VersionOperationService implementation must work independently. Review the implementation in Task 2 which directly uses DocumentRepository.

Replace the entire Rollback method with:

public OperationResult Rollback(int id, int versionId, string culture = "*", int userId = Constants.Security.SuperUserId)
    => VersionOperationService.Rollback(id, versionId, culture, userId);

Step 2: Run build to verify

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 3: Run tests to verify rollback behavior

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentService" --no-restore Expected: All tests pass

Step 4: Commit

git add src/Umbraco.Core/Services/ContentService.cs
git commit -m "$(cat <<'EOF'
refactor(core): delegate Rollback to VersionOperationService

Part of ContentService refactoring Phase 3.
Notification ordering preserved: RollingBack -> RolledBack.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 7: Delegate Version Deletion Methods

Files:

• Modify: src/Umbraco.Core/Services/ContentService.cs

Step 1: Delegate DeleteVersions

Find the DeleteVersions method (around line 1978). Replace the implementation with:

public void DeleteVersions(int id, DateTime versionDate, int userId = Constants.Security.SuperUserId)
    => VersionOperationService.DeleteVersions(id, versionDate, userId);

Step 2: Delegate DeleteVersion

Find the DeleteVersion method (around line 2012). Replace the implementation with:

public void DeleteVersion(int id, int versionId, bool deletePriorVersions, int userId = Constants.Security.SuperUserId)
    => VersionOperationService.DeleteVersion(id, versionId, deletePriorVersions, userId);

Step 3: Run build to verify

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds

Step 4: Run full ContentService tests

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentService" --no-restore Expected: All tests pass

Step 5: Commit

git add src/Umbraco.Core/Services/ContentService.cs
git commit -m "$(cat <<'EOF'
refactor(core): delegate DeleteVersions and DeleteVersion to VersionOperationService

Part of ContentService refactoring Phase 3.
Completes version operations extraction.

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 8: Create Integration Tests for ContentVersionOperationService

Files:

• Create: tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentVersionOperationServiceTests.cs

Step 1: Create test file with comprehensive tests

v1.1 Changes Applied:

• Issue 2.5: Replaced Thread.Sleep with deterministic date comparison
• Issue 3.2: Added Rollback_WhenNotificationCancelled_ReturnsCancelledResult test
• Issue 3.3: Added DeleteVersion_PublishedVersion_DoesNotDelete test

// tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentVersionOperationServiceTests.cs
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentPublishing;
using Umbraco.Cms.Core.Notifications;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Testing;
using Umbraco.Cms.Tests.Integration.Testing;

namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services;

[TestFixture]
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
public class ContentVersionOperationServiceTests : UmbracoIntegrationTest
{
    private IContentVersionOperationService VersionOperationService => GetRequiredService<IContentVersionOperationService>();
    private IContentService ContentService => GetRequiredService<IContentService>();
    private IContentTypeService ContentTypeService => GetRequiredService<IContentTypeService>();
    private IContentPublishingService ContentPublishingService => GetRequiredService<IContentPublishingService>();

    // v1.2 Fix (Issue 3.2): Use CustomTestSetup to register notification handlers
    protected override void CustomTestSetup(IUmbracoBuilder builder)
        => builder.AddNotificationHandler<ContentRollingBackNotification, VersionNotificationHandler>();

    private IContentType CreateContentType()
    {
        var contentType = new ContentTypeBuilder()
            .WithAlias("testContentType")
            .WithName("Test Content Type")
            .AddPropertyType()
                .WithAlias("title")
                .WithName("Title")
                .WithDataTypeId(Constants.DataTypes.Textbox)
                .Done()
            .Build();
        ContentTypeService.Save(contentType);
        return contentType;
    }

    private IContent CreateAndSaveContent(IContentType contentType, string name = "Test Content")
    {
        var content = new ContentBuilder()
            .WithContentType(contentType)
            .WithName(name)
            .Build();
        ContentService.Save(content);
        return content;
    }

    #region GetVersion Tests

    [Test]
    public void GetVersion_ExistingVersion_ReturnsContent()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var versionId = content.VersionId;

        // Act
        var result = VersionOperationService.GetVersion(versionId);

        // Assert
        Assert.That(result, Is.Not.Null);
        Assert.That(result!.Id, Is.EqualTo(content.Id));
    }

    [Test]
    public void GetVersion_NonExistentVersion_ReturnsNull()
    {
        // Act
        var result = VersionOperationService.GetVersion(999999);

        // Assert
        Assert.That(result, Is.Null);
    }

    #endregion

    #region GetVersions Tests

    [Test]
    public void GetVersions_ContentWithMultipleVersions_ReturnsAllVersions()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);

        // Create additional versions
        content.SetValue("title", "Version 2");
        ContentService.Save(content);
        content.SetValue("title", "Version 3");
        ContentService.Save(content);

        // Act
        var versions = VersionOperationService.GetVersions(content.Id).ToList();

        // Assert
        Assert.That(versions.Count, Is.GreaterThanOrEqualTo(3));
    }

    [Test]
    public void GetVersions_NonExistentContent_ReturnsEmpty()
    {
        // Act
        var versions = VersionOperationService.GetVersions(999999).ToList();

        // Assert
        Assert.That(versions, Is.Empty);
    }

    #endregion

    #region GetVersionsSlim Tests

    [Test]
    public void GetVersionsSlim_ReturnsPagedVersions()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);

        // Create additional versions
        for (int i = 2; i <= 5; i++)
        {
            content.SetValue("title", $"Version {i}");
            ContentService.Save(content);
        }

        // Act
        var versions = VersionOperationService.GetVersionsSlim(content.Id, skip: 1, take: 2).ToList();

        // Assert
        Assert.That(versions.Count, Is.EqualTo(2));
    }

    #endregion

    #region GetVersionIds Tests

    [Test]
    public void GetVersionIds_ReturnsVersionIdsOrderedByLatestFirst()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var firstVersionId = content.VersionId;

        content.SetValue("title", "Version 2");
        ContentService.Save(content);
        var secondVersionId = content.VersionId;

        // Act
        var versionIds = VersionOperationService.GetVersionIds(content.Id, maxRows: 10).ToList();

        // Assert
        Assert.That(versionIds.Count, Is.GreaterThanOrEqualTo(2));
        Assert.That(versionIds[0], Is.EqualTo(secondVersionId)); // Latest first
    }

    #endregion

    #region Rollback Tests

    [Test]
    public void Rollback_ToEarlierVersion_RestoresPropertyValues()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        content.SetValue("title", "Original Value");
        ContentService.Save(content);
        var originalVersionId = content.VersionId;

        content.SetValue("title", "Changed Value");
        ContentService.Save(content);

        // Act
        var result = VersionOperationService.Rollback(content.Id, originalVersionId);

        // Assert
        Assert.That(result.Success, Is.True);
        var rolledBackContent = ContentService.GetById(content.Id);
        Assert.That(rolledBackContent!.GetValue<string>("title"), Is.EqualTo("Original Value"));
    }

    [Test]
    public void Rollback_NonExistentContent_Fails()
    {
        // Act
        var result = VersionOperationService.Rollback(999999, 1);

        // Assert
        Assert.That(result.Success, Is.False);
        Assert.That(result.Result, Is.EqualTo(OperationResultType.FailedCannot));
    }

    [Test]
    public void Rollback_TrashedContent_Fails()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var versionId = content.VersionId;
        ContentService.MoveToRecycleBin(content);

        // Act
        var result = VersionOperationService.Rollback(content.Id, versionId);

        // Assert
        Assert.That(result.Success, Is.False);
        Assert.That(result.Result, Is.EqualTo(OperationResultType.FailedCannot));
    }

    /// <summary>
    /// v1.2 Fix (Issue 3.2): Test that cancellation notification works correctly.
    /// Uses the correct integration test pattern with CustomTestSetup and static action.
    /// </summary>
    [Test]
    public void Rollback_WhenNotificationCancelled_ReturnsCancelledResult()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        content.SetValue("title", "Original Value");
        ContentService.Save(content);
        var originalVersionId = content.VersionId;

        content.SetValue("title", "Changed Value");
        ContentService.Save(content);

        // Set up the notification handler to cancel the rollback
        VersionNotificationHandler.RollingBackContent = notification => notification.Cancel = true;

        try
        {
            // Act
            var result = VersionOperationService.Rollback(content.Id, originalVersionId);

            // Assert
            Assert.That(result.Success, Is.False);
            Assert.That(result.Result, Is.EqualTo(OperationResultType.Cancelled));

            // Verify content was not modified
            var unchangedContent = ContentService.GetById(content.Id);
            Assert.That(unchangedContent!.GetValue<string>("title"), Is.EqualTo("Changed Value"));
        }
        finally
        {
            // Clean up the static action
            VersionNotificationHandler.RollingBackContent = null;
        }
    }

    #endregion

    #region DeleteVersions Tests

    /// <summary>
    /// v1.1 Fix (Issue 2.5): Use deterministic date comparison instead of Thread.Sleep.
    /// </summary>
    [Test]
    public void DeleteVersions_ByDate_DeletesOlderVersions()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var firstVersionId = content.VersionId;

        content.SetValue("title", "Version 2");
        ContentService.Save(content);

        // Get the actual update date of version 2 for deterministic comparison
        var version2 = VersionOperationService.GetVersion(content.VersionId);
        var cutoffDate = version2!.UpdateDate.AddMilliseconds(1);

        content.SetValue("title", "Version 3");
        ContentService.Save(content);
        var version3Id = content.VersionId;

        var versionCountBefore = VersionOperationService.GetVersions(content.Id).Count();

        // Act
        VersionOperationService.DeleteVersions(content.Id, cutoffDate);

        // Assert
        var remainingVersions = VersionOperationService.GetVersions(content.Id).ToList();
        Assert.That(remainingVersions.Any(v => v.VersionId == version3Id), Is.True, "Current version should remain");
        Assert.That(remainingVersions.Count, Is.LessThan(versionCountBefore));
    }

    #endregion

    #region DeleteVersion Tests

    [Test]
    public void DeleteVersion_SpecificVersion_DeletesOnlyThatVersion()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);

        content.SetValue("title", "Version 2");
        ContentService.Save(content);
        var versionToDelete = content.VersionId;

        content.SetValue("title", "Version 3");
        ContentService.Save(content);
        var currentVersionId = content.VersionId;

        // Act
        VersionOperationService.DeleteVersion(content.Id, versionToDelete, deletePriorVersions: false);

        // Assert
        var deletedVersion = VersionOperationService.GetVersion(versionToDelete);
        Assert.That(deletedVersion, Is.Null);
        var currentVersion = VersionOperationService.GetVersion(currentVersionId);
        Assert.That(currentVersion, Is.Not.Null);
    }

    [Test]
    public void DeleteVersion_CurrentVersion_DoesNotDelete()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var currentVersionId = content.VersionId;

        // Act
        VersionOperationService.DeleteVersion(content.Id, currentVersionId, deletePriorVersions: false);

        // Assert
        var version = VersionOperationService.GetVersion(currentVersionId);
        Assert.That(version, Is.Not.Null); // Should not be deleted
    }

    /// <summary>
    /// v1.2 Fix (Issue 3.3, 3.4): Test that published version is protected from deletion.
    /// Uses the correct async ContentPublishingService.PublishAsync method.
    /// </summary>
    [Test]
    public async Task DeleteVersion_PublishedVersion_DoesNotDelete()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);

        // v1.2 Fix (Issue 3.4): Use ContentPublishingService.PublishAsync with correct signature
        var publishResult = await ContentPublishingService.PublishAsync(
            content.Key,
            new[] { new CulturePublishScheduleModel() },
            Constants.Security.SuperUserKey);
        Assert.That(publishResult.Success, Is.True, "Publish should succeed");

        // Refresh content to get the published version id
        content = ContentService.GetById(content.Id)!;
        var publishedVersionId = content.PublishedVersionId;
        Assert.That(publishedVersionId, Is.Not.Null, "Content should have a published version");

        // Create a newer draft version
        content.SetValue("title", "Draft");
        ContentService.Save(content);

        // Act
        VersionOperationService.DeleteVersion(content.Id, publishedVersionId!.Value, deletePriorVersions: false);

        // Assert
        var version = VersionOperationService.GetVersion(publishedVersionId!.Value);
        Assert.That(version, Is.Not.Null, "Published version should not be deleted");
    }

    #endregion

    #region Behavioral Equivalence Tests

    [Test]
    public void GetVersion_ViaService_MatchesContentService()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        var versionId = content.VersionId;

        // Act
        var viaService = VersionOperationService.GetVersion(versionId);
        var viaContentService = ContentService.GetVersion(versionId);

        // Assert
        Assert.That(viaService?.Id, Is.EqualTo(viaContentService?.Id));
        Assert.That(viaService?.VersionId, Is.EqualTo(viaContentService?.VersionId));
    }

    [Test]
    public void GetVersions_ViaService_MatchesContentService()
    {
        // Arrange
        var contentType = CreateContentType();
        var content = CreateAndSaveContent(contentType);
        content.SetValue("title", "Version 2");
        ContentService.Save(content);

        // Act
        var viaService = VersionOperationService.GetVersions(content.Id).ToList();
        var viaContentService = ContentService.GetVersions(content.Id).ToList();

        // Assert
        Assert.That(viaService.Count, Is.EqualTo(viaContentService.Count));
    }

    #endregion

    #region Notification Handler

    /// <summary>
    /// v1.2 Fix (Issue 3.2): Notification handler for testing using the correct integration test pattern.
    /// Uses static actions that can be set in individual tests.
    /// </summary>
    private class VersionNotificationHandler : INotificationHandler<ContentRollingBackNotification>
    {
        public static Action<ContentRollingBackNotification>? RollingBackContent { get; set; }

        public void Handle(ContentRollingBackNotification notification)
            => RollingBackContent?.Invoke(notification);
    }

    #endregion
}

Step 2: Run the new tests

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentVersionOperationServiceTests" --no-restore Expected: All tests pass

Step 3: Commit

git add tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentVersionOperationServiceTests.cs
git commit -m "$(cat <<'EOF'
test(integration): add ContentVersionOperationServiceTests

Part of ContentService refactoring Phase 3.
Covers version retrieval, rollback, and version deletion.

v1.1 fixes applied:
- Deterministic date comparison instead of Thread.Sleep (Issue 2.5)
- Added Rollback cancellation test (Issue 3.2)
- Added published version protection test (Issue 3.3)

v1.2 fixes applied:
- Fixed notification handler registration pattern (Issue 3.2)
- Fixed Publish method signature using ContentPublishingService (Issue 3.4)

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

---

Task 9: Run Phase Gate Tests

Step 1: Run ContentService refactoring tests

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentServiceRefactoringTests" --no-restore Expected: All tests pass

Step 2: Run all ContentService tests

Run: dotnet test tests/Umbraco.Tests.Integration --filter "FullyQualifiedName~ContentService" --no-restore Expected: All tests pass (or same count as baseline)

Step 3: Run full integration test suite

Run: dotnet test tests/Umbraco.Tests.Integration --no-restore Expected: All tests pass (or same count as baseline)

Step 4: Verify build with no errors

Run: dotnet build src/Umbraco.Core --no-restore Expected: Build succeeds with no errors

---

Task 10: Update Design Document and Create Git Tag

Files:

• Modify: docs/plans/2025-12-19-contentservice-refactor-design.md

Step 1: Update Phase 3 status in design document

Find the Implementation Order table and update Phase 3 row:

Change:

| 3 | Version Service | All ContentService*Tests | All pass | Pending |

To:

| 3 | Version Service | All ContentService*Tests | All pass | ✅ Complete |

Step 2: Add Phase 3 completion notes

Add after the Phase 2 notes in the Phase Details section:

4. **Phase 3: Version Service** ✅ - Complete! Created:
   - `IContentVersionOperationService.cs` - Interface (7 methods)
   - `ContentVersionOperationService.cs` - Implementation
   - Updated `ContentService.cs` to delegate version operations
   - Git tag: `phase-3-version-extraction`

Step 3: Commit design document update

git add docs/plans/2025-12-19-contentservice-refactor-design.md
git commit -m "$(cat <<'EOF'
docs: mark Phase 3 complete in design document

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

Co-Authored-By: Claude <noreply@anthropic.com>
EOF
)"

Step 4: Create git tag

git tag -a phase-3-version-extraction -m "Phase 3: Version operations extracted to ContentVersionOperationService"

---

Task Summary

Task	Description	Files
1	Create IContentVersionOperationService interface	IContentVersionOperationService.cs
2	Create ContentVersionOperationService implementation	ContentVersionOperationService.cs
3	Register service in DI container	UmbracoBuilder.cs
4	Add VersionOperationService property to ContentService	ContentService.cs
5	Delegate version retrieval methods	ContentService.cs
6	Delegate Rollback method	ContentService.cs
7	Delegate version deletion methods	ContentService.cs
8	Create integration tests	ContentVersionOperationServiceTests.cs
9	Run phase gate tests	N/A
10	Update design document and create tag	design.md

---

v1.1 Changes Summary

Issue	Severity	Description	Fix Applied
2.1	Critical	Rollback TOCTOU race condition	Consolidated into single scope
2.2	Critical	Rollback missing error handling	Added try-catch with logging
2.3	Important	GetVersionIds missing ReadLock	Added ReadLock
2.4	Critical	DeleteVersion nested scope	Use repository directly
2.5	Important	Thread.Sleep in tests	Deterministic date comparison
3.2	Minor	Missing cancellation test	Added test
3.3	Minor	Missing published version test	Added test
3.4	Minor	Interface docs incomplete	Enhanced GetVersionIds docs

---

v1.2 Changes Summary

Issue	Severity	Description	Fix Applied
2.1	Critical	Rollback bypasses ContentSaving/ContentSaved notifications	Use CrudService.Save to preserve notifications
2.2	Important	DeleteVersion with deletePriorVersions changes notification semantics	Inline notification firing to preserve double-notification
3.1	Minor	Redundant WriteLock acquisition in DeleteVersion	Single WriteLock at start
3.2	Minor	Test notification registration may not compile	Use CustomTestSetup with static action pattern
3.3	Minor	Missing IContentCrudService constructor dependency	Added to constructor
3.4	Minor	Publish method signature incorrect in test	Use ContentPublishingService.PublishAsync

---

v1.3 Changes Summary

Issue	Severity	Description	Fix Applied
3.1	Should Fix	GetVersionIds missing input validation	Added ArgumentOutOfRangeException for maxRows <= 0
3.2	Consider	Redundant WriteLock in Rollback	Removed explicit WriteLock (CrudService.Save handles locking)
3.3	Should Fix	Audit gap in DeleteVersion with deletePriorVersions	Added audit entry for prior versions deletion
3.4	Must Fix	Rollback return type mismatch	Fixed OperationResult to OperationResult
3.6	Consider	Prior versions cancellation behavior unclear	Added clarifying comment

---

Rollback Procedure

If tests fail during Phase 3:

1. Revert all Phase 3 commits:

   git revert HEAD~N..HEAD  # Where N is number of Phase 3 commits
   

2. Or reset to Phase 2 tag:

   git reset --hard phase-2-query-extraction
   

3. Document the failure:

   • Which tests failed
   • What behavior changed
   • Root cause analysis

4. Create fix and retry:

   • Fix the identified issue
   • Re-run all tasks from that point

---

Success Criteria

• All 7 version methods delegated from ContentService
• IContentVersionOperationService interface created with documentation
• ContentVersionOperationService implementation with proper scoping
• DI registration complete
• All ContentService tests pass
• Integration tests for new service pass
• Design document updated with Phase 3 completion
• Git tag phase-3-version-extraction created