From 0f74e296c765317e56c9c84fcb5f897ae09516ec Mon Sep 17 00:00:00 2001 From: yv01p Date: Wed, 24 Dec 2025 19:47:57 +0000 Subject: [PATCH] refactor(core): expose DeleteLocked on IContentCrudService and unify implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make existing private DeleteLocked method public and add to interface. Update ContentService.DeleteOfTypes to delegate to the service. Remove duplicate DeleteLocked from ContentService. Unify implementations by having ContentMoveOperationService.EmptyRecycleBin call IContentCrudService.DeleteLocked instead of its own local method. This eliminates the duplicate implementation and reduces maintenance burden. The ContentCrudService implementation includes iteration bounds and proper logging for edge cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../Services/ContentCrudService.cs | 2 +- .../Services/ContentMoveOperationService.cs | 60 +------------------ src/Umbraco.Core/Services/ContentService.cs | 27 +-------- .../Services/IContentCrudService.cs | 11 ++++ 4 files changed, 14 insertions(+), 86 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentCrudService.cs b/src/Umbraco.Core/Services/ContentCrudService.cs index 79e9f6ecd4..20a89bac50 100644 --- a/src/Umbraco.Core/Services/ContentCrudService.cs +++ b/src/Umbraco.Core/Services/ContentCrudService.cs @@ -634,7 +634,7 @@ public class ContentCrudService : ContentServiceBase, IContentCrudService /// Uses paging to handle large trees without loading everything into memory. /// Iteration bound prevents infinite loops if database is in inconsistent state. /// - private void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs) + public void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs) { void DoDelete(IContent c) { diff --git a/src/Umbraco.Core/Services/ContentMoveOperationService.cs b/src/Umbraco.Core/Services/ContentMoveOperationService.cs index 894a3ed38a..04c97ee7db 100644 --- a/src/Umbraco.Core/Services/ContentMoveOperationService.cs +++ b/src/Umbraco.Core/Services/ContentMoveOperationService.cs @@ -251,7 +251,7 @@ public class ContentMoveOperationService : ContentServiceBase, IContentMoveOpera continue; } - DeleteLocked(scope, content, eventMessages); + _crudService.DeleteLocked(scope, content, eventMessages); deleted.Add(content); } } @@ -298,64 +298,6 @@ public class ContentMoveOperationService : ContentServiceBase, IContentMoveOpera } } - /// - /// Deletes content and all descendants within an existing scope. - /// - private void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs) - { - void DoDelete(IContent c) - { - DocumentRepository.Delete(c); - scope.Notifications.Publish(new ContentDeletedNotification(c, evtMsgs)); - } - - // v1.1: Using class-level constants - var iteration = 0; - var total = long.MaxValue; - - while (total > 0 && iteration < MaxDeleteIterations) - { - IEnumerable descendants = GetPagedDescendantsLocked( - content.Id, - 0, - DefaultPageSize, - out total, - ordering: Ordering.By("Path", Direction.Descending)); - - var batch = descendants.ToList(); - - // v1.1: Break immediately when batch is empty (fix from critical review 2.5) - if (batch.Count == 0) - { - if (total > 0) - { - _logger.LogWarning( - "GetPagedDescendants reported {Total} total descendants but returned empty batch for content {ContentId}. Breaking loop.", - total, - content.Id); - } - break; // Break immediately, don't continue iterating - } - - foreach (IContent c in batch) - { - DoDelete(c); - } - - iteration++; - } - - if (iteration >= MaxDeleteIterations) - { - _logger.LogError( - "DeleteLocked exceeded maximum iteration limit ({MaxIterations}) for content {ContentId}. Tree may be incompletely deleted.", - MaxDeleteIterations, - content.Id); - } - - DoDelete(content); - } - #endregion #region Copy Operations diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index cb59a8463e..9ccf5a31c5 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -555,31 +555,6 @@ public class ContentService : RepositoryService, IContentService public OperationResult Delete(IContent content, int userId = Constants.Security.SuperUserId) => CrudService.Delete(content, userId); - private void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs) - { - void DoDelete(IContent c) - { - _documentRepository.Delete(c); - scope.Notifications.Publish(new ContentDeletedNotification(c, evtMsgs)); - - // media files deleted by QueuingEventDispatcher - } - - const int pageSize = 500; - var total = long.MaxValue; - while (total > 0) - { - // get descendants - ordered from deepest to shallowest - IEnumerable descendants = GetPagedDescendants(content.Id, 0, pageSize, out total, ordering: Ordering.By("Path", Direction.Descending)); - foreach (IContent c in descendants) - { - DoDelete(c); - } - } - - DoDelete(content); - } - // TODO: both DeleteVersions methods below have an issue. Sort of. They do NOT take care of files the way // Delete does - for a good reason: the file may be referenced by other, non-deleted, versions. BUT, // if that's not the case, then the file will never be deleted, because when we delete the content, @@ -881,7 +856,7 @@ public class ContentService : RepositoryService, IContentService // delete content // triggers the deleted event (and handles the files) - DeleteLocked(scope, content, eventMessages); + CrudService.DeleteLocked(scope, content, eventMessages); changes.Add(new TreeChange(content, TreeChangeTypes.Remove)); } diff --git a/src/Umbraco.Core/Services/IContentCrudService.cs b/src/Umbraco.Core/Services/IContentCrudService.cs index 86d993a2da..b7f8d06330 100644 --- a/src/Umbraco.Core/Services/IContentCrudService.cs +++ b/src/Umbraco.Core/Services/IContentCrudService.cs @@ -1,6 +1,8 @@ // src/Umbraco.Core/Services/IContentCrudService.cs +using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Scoping; namespace Umbraco.Cms.Core.Services; @@ -247,5 +249,14 @@ public interface IContentCrudService : IService /// The operation result. OperationResult Delete(IContent content, int userId = Constants.Security.SuperUserId); + /// + /// Performs the locked delete operation including descendants. + /// Used internally by DeleteOfTypes orchestration and EmptyRecycleBin. + /// + /// The active scope with write lock. + /// The document to delete. + /// Event messages collection. + void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs); + #endregion }