yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(core): expose DeleteLocked on IContentCrudService and unify implementations

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
yv01p
2025-12-24 19:47:57 +00:00
parent d09b726f9d
commit 0f74e296c7
4 changed files with 14 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Umbraco.Core/Services/ContentCrudService.cs
View File

@@ -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.
/// </remarks>
private void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs)
public void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs)
{
void DoDelete(IContent c)
{

60
src/Umbraco.Core/Services/ContentMoveOperationService.cs
View File

@@ -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
}
}
/// <summary>
/// Deletes content and all descendants within an existing scope.
/// </summary>
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<IContent> 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

27
src/Umbraco.Core/Services/ContentService.cs
View File

@@ -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<IContent> 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<IContent>(content, TreeChangeTypes.Remove));
}

11
src/Umbraco.Core/Services/IContentCrudService.cs
View File

@@ -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
/// <returns>The operation result.</returns>
OperationResult Delete(IContent content, int userId = Constants.Security.SuperUserId);
/// <summary>
/// Performs the locked delete operation including descendants.
/// Used internally by DeleteOfTypes orchestration and EmptyRecycleBin.
/// </summary>
/// <param name="scope">The active scope with write lock.</param>
/// <param name="content">The document to delete.</param>
/// <param name="evtMsgs">Event messages collection.</param>
void DeleteLocked(ICoreScope scope, IContent content, EventMessages evtMsgs);
#endregion
}