Media: Fix files not deleted from disk when recycle bin protection is enabled #21309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

AndyButland merged 3 commits into release/17.1 from v17/bugfix/delete-files-from-disk-when-recycle-bin-media-protection-is-enabled

Jan 7, 2026

src/Umbraco.Cms.Api.Management/Factories/MediaUrlFactory.cs

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,16 +1,10 @@
  
    ﻿using Microsoft.Extensions.Logging;

    using Microsoft.Extensions.Options;

    using Umbraco.Cms.Api.Management.Routing;

    using Umbraco.Cms.Api.Management.ViewModels.Content;

    using Umbraco.Cms.Api.Management.ViewModels.Document;

    using Umbraco.Cms.Api.Management.ViewModels.Media;

    using Umbraco.Cms.Core;

    using Umbraco.Cms.Core.Configuration.Models;

    using Umbraco.Cms.Core.Models;

    using Umbraco.Cms.Core.Models.PublishedContent;

    using Umbraco.Cms.Core.PropertyEditors;

    using Umbraco.Cms.Core.Routing;

    using Umbraco.Cms.Core.Services;

    using Umbraco.Cms.Core.Web;

    using Umbraco.Extensions;

    namespace Umbraco.Cms.Api.Management.Factories;

    @@ -21,7 +15,6 @@ public class MediaUrlFactory : IMediaUrlFactory
  
        private readonly MediaUrlGeneratorCollection _mediaUrlGenerators;

        private readonly IAbsoluteUrlBuilder _absoluteUrlBuilder;

        public MediaUrlFactory(

            IOptions<ContentSettings> contentSettings,

            MediaUrlGeneratorCollection mediaUrlGenerators,

    @@ -39,10 +32,26 @@ public IEnumerable<MediaUrlInfo> CreateUrls(IMedia media) =>
  
                .Select(mediaUrl => new MediaUrlInfo

                {

                    Culture = null,

                    Url = _absoluteUrlBuilder.ToAbsoluteUrl(mediaUrl).ToString(),

                    Url = CreateMediaUrl(mediaUrl),

                })

                .ToArray();

        private string CreateMediaUrl(string mediaUrl)

        {

            var url = _absoluteUrlBuilder.ToAbsoluteUrl(mediaUrl).ToString();

            if (_contentSettings.EnableMediaRecycleBinProtection is false)

            {

                return url;

            }

            return _contentSettings.EnableMediaRecycleBinProtection

                ? AddProtectedSuffixToMediaUrl(url)

                : url;

        }

        private static string AddProtectedSuffixToMediaUrl(string url) => Path.ChangeExtension(url, Constants.Conventions.Media.TrashedMediaSuffix + Path.GetExtension(url));

        public IEnumerable<MediaUrlInfoResponseModel> CreateUrlSets(IEnumerable<IMedia> mediaItems) =>

            mediaItems.Select(media => new MediaUrlInfoResponseModel(media.Key, CreateUrls(media))).ToArray();

    }

src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs

-Original file line number
+Diff line change
@@ Expand Up / @@ -130,7 +130,19 @@ public void Handle(ContentCopiedNotification notification) @@
         public void Handle(ContentDeletedNotification notification) => DeleteContainedFiles(notification.DeletedEntities);
         /// <inheritdoc/>
-        public void Handle(MediaDeletedNotification notification) => DeleteContainedFiles(notification.DeletedEntities);
+        public void Handle(MediaDeletedNotification notification)
+        {
+            if (_contentSettings.EnableMediaRecycleBinProtection)
+            {
+                RecycleBinMediaProtectionHelper.DeleteContainedFilesWithProtection(
+                    notification.DeletedEntities,
+                    ContainedFilePaths,
+                    _mediaFileManager);
+                return;
+            }
+            DeleteContainedFiles(notification.DeletedEntities);
+        }
         /// <inheritdoc/>
         public void Handle(MediaSavingNotification notification)
@@ Expand Down @@

...cture/PropertyEditors/NotificationHandlers/FileUploadContentDeletedNotificationHandler.cs

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -56,7 +56,19 @@ public FileUploadContentDeletedNotificationHandler(
  
        public void Handle(ContentDeletedBlueprintNotification notification) => DeleteContainedFiles(notification.DeletedBlueprints);

        /// <inheritdoc/>

        public void Handle(MediaDeletedNotification notification) => DeleteContainedFiles(notification.DeletedEntities);

        public void Handle(MediaDeletedNotification notification)

        {

            if (_contentSettings.EnableMediaRecycleBinProtection)

            {

                RecycleBinMediaProtectionHelper.DeleteContainedFilesWithProtection(

                    notification.DeletedEntities,

                    ContainedFilePaths,

                    MediaFileManager);

                return;

            }

            DeleteContainedFiles(notification.DeletedEntities);

        }

        /// <inheritdoc/>

        public void Handle(MediaMovedToRecycleBinNotification notification)

    @@ -115,7 +127,7 @@ private void SuffixContainedFiles(IEnumerable<IMedia> trashedMedia)
  
        private void RemoveSuffixFromContainedFiles(IEnumerable<IMedia> restoredMedia)

        {

            IEnumerable<string> filePathsToRename = ContainedFilePaths(restoredMedia);

            MediaFileManager.RemoveSuffixFromMediaFiles(filePathsToRename, Constants.Conventions.Media.TrashedMediaSuffix);

            RecycleBinMediaProtectionHelper.RemoveSuffixFromContainedFiles(filePathsToRename, MediaFileManager);

        }

        /// <summary>

...co.Infrastructure/PropertyEditors/NotificationHandlers/RecycleBinMediaProtectionHelper.cs

-Original file line number
+Diff line change
@@ -1,5 +1,6 @@
     using Umbraco.Cms.Core;
     using Umbraco.Cms.Core.IO;
+    using Umbraco.Cms.Core.Models;
     namespace Umbraco.Cms.Infrastructure.PropertyEditors.NotificationHandlers;
@@ Expand Down Expand Up @@
                 .Select(x => Path.ChangeExtension(x, Constants.Conventions.Media.TrashedMediaSuffix + Path.GetExtension(x)));
             mediaFileManager.RemoveSuffixFromMediaFiles(filePathsToRename, Constants.Conventions.Media.TrashedMediaSuffix);
         }
+        /// <summary>
+        /// Deletes all media files, accounting for recycle bin protection (trashed files have .deleted suffix on disk).
+        /// </summary>
+        /// <param name="deletedMedia">Deleted media entities.</param>
+        /// <param name="containedFilePaths">Function to extract file paths from media entities.</param>
+        /// <param name="mediaFileManager">The media file manager.</param>
+        public static void DeleteContainedFilesWithProtection(
+            IEnumerable<IMedia> deletedMedia,
+            Func<IEnumerable<IMedia>, IEnumerable<string>> containedFilePaths,
+            MediaFileManager mediaFileManager)
+        {
+            // Typically all deleted media will have Trashed == true since they come from the recycle bin.
+            // However, media can be force-deleted programmatically bypassing the recycle bin, in which
+            // case the files won't have the .deleted suffix on disk. We handle both cases here.
+            var trashedMedia = deletedMedia.Where(m => m.Trashed).ToList();
+            var nonTrashedMedia = deletedMedia.Where(m => !m.Trashed).ToList();
+            // Delete trashed media files (with .deleted suffix on disk).
+            if (trashedMedia.Count > 0)
+            {
+                IEnumerable<string> trashedPaths = containedFilePaths(trashedMedia)
+                    .Select(x => Path.ChangeExtension(x, Constants.Conventions.Media.TrashedMediaSuffix + Path.GetExtension(x)));
+                mediaFileManager.DeleteMediaFiles(trashedPaths);
+            }
+            // Delete non-trashed media files (original paths).
+            if (nonTrashedMedia.Count > 0)
+            {
+                mediaFileManager.DeleteMediaFiles(containedFilePaths(nonTrashedMedia));
+            }
+        }
     }

tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Factories/MediaUrlFactoryTests.cs

-Original file line number
+Diff line change
@@ -0,0 +1,246 @@
+    using Microsoft.Extensions.Options;
+    using Moq;
+    using NUnit.Framework;
+    using Umbraco.Cms.Api.Management.Factories;
+    using Umbraco.Cms.Api.Management.Routing;
+    using Umbraco.Cms.Core;
+    using Umbraco.Cms.Core.Configuration.Models;
+    using Umbraco.Cms.Core.Models;
+    using Umbraco.Cms.Core.PropertyEditors;
+    namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Cms.Api.Management.Factories;
+    [TestFixture]
+    public class MediaUrlFactoryTests
+    {
+        private const string BaseUrl = "https://example.com";
+        private const string MediaPath = "/media/image.jpg";
+        private const string PropertyAlias = Constants.Conventions.Media.File;
+        [Test]
+        public void CreateUrls_WithRecycleBinProtectionDisabled_ReturnsUrlWithoutDeletedSuffix()
+        {
+            // Arrange
+            var factory = CreateFactory(enableMediaRecycleBinProtection: false);
+            var media = CreateMediaWithUrl(MediaPath);
+            // Act
+            var result = factory.CreateUrls(media).ToList();
+            // Assert
+            Assert.AreEqual(1, result.Count);
+            Assert.AreEqual($"{BaseUrl}{MediaPath}", result[0].Url);
+            Assert.IsNull(result[0].Culture);
+        }
+        [Test]
+        public void CreateUrls_WithRecycleBinProtectionEnabled_ReturnsUrlWithDeletedSuffix()
+        {
+            // Arrange
+            var factory = CreateFactory(enableMediaRecycleBinProtection: true);
+            var media = CreateMediaWithUrl(MediaPath);
+            // Act
+            var result = factory.CreateUrls(media).ToList();
+            // Assert
+            Assert.AreEqual(1, result.Count);
+            Assert.AreEqual($"{BaseUrl}/media/image{Constants.Conventions.Media.TrashedMediaSuffix}.jpg", result[0].Url);
+            Assert.IsNull(result[0].Culture);
+        }
+        [Test]
+        public void CreateUrls_WithNoUrls_ReturnsEmptyCollection()
+        {
+            // Arrange
+            var factory = CreateFactory(enableMediaRecycleBinProtection: false);
+            var media = CreateMediaWithUrl(null);
+            // Act
+            var result = factory.CreateUrls(media).ToList();
+            // Assert
+            Assert.IsEmpty(result);
+        }
+        [Test]
+        public void CreateUrls_WithMultipleUrls_ReturnsAllUrls()
+        {
+            // Arrange
+            const string secondPropertyAlias = "secondImage";
+            const string secondMediaPath = "/media/second.png";
+            var contentSettings = CreateContentSettings(false, PropertyAlias, secondPropertyAlias);
+            var mediaUrlGenerators = CreateMediaUrlGeneratorCollection();
+            var absoluteUrlBuilder = CreateAbsoluteUrlBuilder();
+            var factory = new MediaUrlFactory(
+                Options.Create(contentSettings),
+                mediaUrlGenerators,
+                absoluteUrlBuilder);
+            var media = CreateMediaWithMultipleUrls(
+                (PropertyAlias, MediaPath),
+                (secondPropertyAlias, secondMediaPath));
+            // Act
+            var result = factory.CreateUrls(media).ToList();
+            // Assert
+            Assert.AreEqual(2, result.Count);
+            Assert.AreEqual($"{BaseUrl}{MediaPath}", result[0].Url);
+            Assert.AreEqual($"{BaseUrl}{secondMediaPath}", result[1].Url);
+        }
+        [Test]
+        public void CreateUrlSets_WithSingleMedia_ReturnsCorrectResponseModel()
+        {
+            // Arrange
+            var factory = CreateFactory(enableMediaRecycleBinProtection: false);
+            var mediaKey = Guid.NewGuid();
+            var media = CreateMediaWithUrl(MediaPath, mediaKey);
+            // Act
+            var result = factory.CreateUrlSets([media]).ToList();
+            // Assert
+            Assert.AreEqual(1, result.Count);
+            Assert.AreEqual(mediaKey, result[0].Id);
+            Assert.AreEqual(1, result[0].UrlInfos.Count());
+            Assert.AreEqual($"{BaseUrl}{MediaPath}", result[0].UrlInfos.First().Url);
+        }
+        [Test]
+        public void CreateUrlSets_WithMultipleMedia_ReturnsAllMediaUrlInfos()
+        {
+            // Arrange
+            var factory = CreateFactory(enableMediaRecycleBinProtection: false);
+            var mediaKey1 = Guid.NewGuid();
+            var mediaKey2 = Guid.NewGuid();
+            const string mediaPath1 = "/media/image1.jpg";
+            const string mediaPath2 = "/media/image2.png";
+            var media1 = CreateMediaWithUrl(mediaPath1, mediaKey1);
+            var media2 = CreateMediaWithUrl(mediaPath2, mediaKey2);
+            // Act
+            var result = factory.CreateUrlSets([media1, media2]).ToList();
+            // Assert
+            Assert.AreEqual(2, result.Count);
+            Assert.AreEqual(mediaKey1, result[0].Id);
+            Assert.AreEqual($"{BaseUrl}{mediaPath1}", result[0].UrlInfos.First().Url);
+            Assert.AreEqual(mediaKey2, result[1].Id);
+            Assert.AreEqual($"{BaseUrl}{mediaPath2}", result[1].UrlInfos.First().Url);
+        }
+        private static MediaUrlFactory CreateFactory(bool enableMediaRecycleBinProtection)
+        {
+            var contentSettings = CreateContentSettings(enableMediaRecycleBinProtection, PropertyAlias);
+            var mediaUrlGenerators = CreateMediaUrlGeneratorCollection();
+            var absoluteUrlBuilder = CreateAbsoluteUrlBuilder();
+            return new MediaUrlFactory(
+                Options.Create(contentSettings),
+                mediaUrlGenerators,
+                absoluteUrlBuilder);
+        }
+        private static ContentSettings CreateContentSettings(bool enableMediaRecycleBinProtection, params string[] propertyAliases)
+        {
+            var autoFillProperties = propertyAliases
+                .Select(alias => new ImagingAutoFillUploadField { Alias = alias })
+                .ToHashSet();
+            return new ContentSettings
+            {
+                EnableMediaRecycleBinProtection = enableMediaRecycleBinProtection,
+                Imaging = new ContentImagingSettings
+                {
+                    AutoFillImageProperties = autoFillProperties,
+                },
+            };
+        }
+        private static MediaUrlGeneratorCollection CreateMediaUrlGeneratorCollection()
+        {
+            var generators = new List<IMediaUrlGenerator>
+            {
+                new StubMediaUrlGenerator(),
+            };
+            return new MediaUrlGeneratorCollection(() => generators);
+        }
+        private static IAbsoluteUrlBuilder CreateAbsoluteUrlBuilder()
+        {
+            var mock = new Mock<IAbsoluteUrlBuilder>();
+            mock
+                .Setup(x => x.ToAbsoluteUrl(It.IsAny<string>()))
+                .Returns<string>(url => new Uri($"{BaseUrl}{url}"));
+            return mock.Object;
+        }
+        private static IMedia CreateMediaWithUrl(string? mediaPath, Guid? key = null)
+        {
+            var urlMappings = mediaPath != null
+                ? [(PropertyAlias, mediaPath)]
+                : Array.Empty<(string, string)>();
+            return CreateMedia(key ?? Guid.NewGuid(), urlMappings);
+        }
+        private static IMedia CreateMediaWithMultipleUrls(params (string Alias, string Path)[] urlMappings)
+            => CreateMedia(Guid.NewGuid(), urlMappings);
+        private static IMedia CreateMedia(Guid key, (string Alias, string Path)[] urlMappings)
+        {
+            var mediaMock = new Mock<IMedia>();
+            mediaMock.SetupGet(m => m.Key).Returns(key);
+            var propertiesMock = new Mock<IPropertyCollection>();
+            foreach (var (alias, path) in urlMappings)
+            {
+                IProperty? outProperty = CreatePropertyMock(alias, path);
+                propertiesMock
+                    .Setup(p => p.TryGetValue(alias, out outProperty))
+                    .Returns(true);
+            }
+            mediaMock.SetupGet(m => m.Properties).Returns(propertiesMock.Object);
+            return mediaMock.Object;
+        }
+        private static IProperty CreatePropertyMock(string alias, string path)
+        {
+            var propertyTypeMock = new Mock<IPropertyType>();
+            propertyTypeMock.SetupGet(pt => pt.PropertyEditorAlias).Returns(Constants.PropertyEditors.Aliases.UploadField);
+            var propertyMock = new Mock<IProperty>();
+            propertyMock.SetupGet(p => p.Alias).Returns(alias);
+            propertyMock.SetupGet(p => p.PropertyType).Returns(propertyTypeMock.Object);
+            propertyMock.Setup(p => p.GetValue(It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<bool>())).Returns(path);
+            return propertyMock.Object;
+        }
+        private class StubMediaUrlGenerator : IMediaUrlGenerator
+        {
+            public bool TryGetMediaPath(string? propertyEditorAlias, object? value, out string? mediaPath)
+            {
+                if (value is string stringValue && !string.IsNullOrEmpty(stringValue))
+                {
+                    mediaPath = stringValue;
+                    return true;
+                }
+                mediaPath = null;
+                return false;
+            }
+        }
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Media: Fix files not deleted from disk when recycle bin protection is enabled #21309

Uh oh!

Diff view

Diff view

There are no files selected for viewing

CodeScene Delta Analysis / CodeScene Code Health Review (release/17.1)

Uh oh!

Uh oh!