Skip to content

Add new module for pruning old content items versions.#18983

Open
PUXVSE wants to merge 1 commit intoOrchardCMS:mainfrom
PUXVSE:PUXVSE/content-version-pruning
Open

Add new module for pruning old content items versions.#18983
PUXVSE wants to merge 1 commit intoOrchardCMS:mainfrom
PUXVSE:PUXVSE/content-version-pruning

Conversation

@PUXVSE
Copy link
Copy Markdown

@PUXVSE PUXVSE commented Mar 11, 2026

Fixes #12669

Similar to AuditTrail trimming or Workflow trimming settings.

@github-actions
Copy link
Copy Markdown
Contributor

Thank you for submitting your first pull request, awesome! 🚀 If you haven't already, please take a moment to review our contribution guide. This guide provides helpful information to ensure your contribution aligns with our standards. A core team member will review your pull request.

If you like Orchard Core, please star our repo and join our community channels.

@PUXVSE
Copy link
Copy Markdown
Author

PUXVSE commented Mar 11, 2026

@dotnet-policy-service agree company="PUXdesign"

Comment on lines +59 to +75
var query = _session
.Query<ContentItem, ContentItemIndex>(x =>
!x.Latest &&
!x.Published &&
(x.ModifiedUtc == null || x.ModifiedUtc < dateThreshold));

var filterByType = settings.ContentTypes?.Length > 0;
if (filterByType)
{
query = query.Where(x => x.ContentType.IsIn(settings.ContentTypes));
}

var candidates = await query.ListAsync();

var versionsToKeep = Math.Max(0, settings.VersionsToKeep);

return ContentVersionPruningSelector.SelectForDeletion(candidates, versionsToKeep);
Copy link
Copy Markdown
Author

@PUXVSE PUXVSE Mar 12, 2026

Choose a reason for hiding this comment

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

This ISession querying here and then filtering in ContentVersionPruningSelector.SelectForDeletion() is quite inefficient as it pulls all content items to memory for GroupBy and Skip.

However ISession doesn't provide API to do these operations in database query - following SQL command is needed (MSSQL version) and I haven't find a nice way to accomplish this (rather than writing raw SQL for each database provider):

SELECT 
    [ContentItemId],
    [ContentItemVersionId]
FROM (
    SELECT
        [ContentItemId],
        [ContentItemVersionId],
        [Latest],
        [Published],
        [ContentType],
        [ModifiedUtc],
        ROW_NUMBER() OVER (
            PARTITION BY [ContentItemId]
            ORDER BY [ModifiedUtc] DESC
        ) AS [RowNum]
    FROM [ContentItemIndex]
    WHERE [Latest] != 1 AND [Published] != 1
) AS [Ranked]
WHERE [RowNum] > 1  -- skip N=1 newest rows per group
ORDER BY [ContentItemId], [ModifiedUtc] DESC

var pruned = await _pruningService.PruneVersionsAsync(settings);

var container = await _siteService.LoadSiteSettingsAsync();
container.Alter<ContentVersionPruningSettings>(nameof(ContentVersionPruningSettings), settings =>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
container.Alter<ContentVersionPruningSettings>(nameof(ContentVersionPruningSettings), settings =>
container.Alter<ContentVersionPruningSettings>(settings =>

}

[HttpPost]
public async Task<IActionResult> Prune()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be a background task not controller.

public int VersionsToKeep { get; set; } = 1;

/// <summary>
/// The content types to prune. When empty, all content types are pruned.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
/// The content types to prune. When empty, all content types are pruned.
/// The content types to prune. When empty, no content types are pruned.

.Where(x =>
!x.Latest &&
!x.Published)
.OrderByDescending(x => x.ModifiedUtc, Comparer<DateTime?>.Create(Nullable.Compare))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.ThenBy(x => x.Id)

{
// Fetch all candidate archived versions (neither latest nor published)
// that are older than the retention threshold. Versions with a null ModifiedUtc
// are treated as oldest (they predate any known modification) and are always included.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If the content types are empty, not thing to prune or fetch

var filterByType = settings.ContentTypes?.Length > 0;
if (filterByType)
{
query = query.Where(x => x.ContentType.IsIn(settings.ContentTypes));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This condition should always be added.

query = query.Where(x => x.ContentType.IsIn(settings.ContentTypes));
}

var candidates = await query.ListAsync();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could pull too much data that may break the site. You should add batch and process like 100 at a time

@@ -0,0 +1,27 @@
<Project Sdk="Microsoft.NET.Sdk.Razor">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don’t think this needs to be added into a new module. Make it a feature in the contents module instead


namespace OrchardCore.Contents.VersionPruning;

public sealed class Permissions : IPermissionProvider
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Internal

/// <summary>
/// The last time the pruning task was run.
/// </summary>
public DateTime? LastRunUtc { get; set; }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should not be part of the settings and it should not be configurable. I actually don’t even think it’s needed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hard Delete of OC Documents

2 participants