Explicitly flush isolated caches by key for content updates (#20519)

* Explicitly flush isolated caches by key for content updates

* Apply suggestions from code review

---------

Co-authored-by: Andy Butland <abutland73@gmail.com>

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs

@@ -1378,6 +1378,11 @@ public class DocumentRepository : ContentRepositoryBase<int, IContent, DocumentR
 
         entity.ResetDirtyProperties();
 
+        // We need to flush the isolated cache by key explicitly here.
+        // The ContentCacheRefresher does the same thing, but by the time it's invoked, custom notification handlers
+        // might have already consumed the cached version (which at this point is the previous version).
+        IsolatedCache.ClearByKey(RepositoryCacheKeys.GetKey<IContent, Guid>(entity.Key));
+
         // troubleshooting
         //if (Database.ExecuteScalar<int>($"SELECT COUNT(*) FROM {Constants.DatabaseSchema.Tables.DocumentVersion} JOIN {Constants.DatabaseSchema.Tables.ContentVersion} ON {Constants.DatabaseSchema.Tables.DocumentVersion}.id={Constants.DatabaseSchema.Tables.ContentVersion}.id WHERE published=1 AND nodeId=" + content.Id) > 1)
         //{

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs

@@ -541,6 +541,11 @@ public class MediaRepository : ContentRepositoryBase<int, IMedia, MediaRepositor
         OnUowRefreshedEntity(new MediaRefreshNotification(entity, new EventMessages()));
 
         entity.ResetDirtyProperties();
+
+        // We need to flush the isolated cache by key explicitly here.
+        // The MediaCacheRefresher does the same thing, but by the time it's invoked, custom notification handlers
+        // might have already consumed the cached version (which at this point is the previous version).
+        IsolatedCache.ClearByKey(RepositoryCacheKeys.GetKey<IMedia, Guid>(entity.Key));
     }
 
     protected override void PersistDeletedItem(IMedia entity)

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentServiceNotificationWithCacheTests.cs

@@ -0,0 +1,148 @@
+using Microsoft.Extensions.DependencyInjection;
+using Moq;
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Events;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.ContentEditing;
+using Umbraco.Cms.Core.Notifications;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
+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,
+    PublishedRepositoryEvents = true,
+    WithApplication = true,
+    Logger = UmbracoTestOptions.Logger.Console)]
+internal sealed class ContentServiceNotificationWithCacheTests : UmbracoIntegrationTest
+{
+    private IContentType _contentType;
+
+    private IContentTypeService ContentTypeService => GetRequiredService<IContentTypeService>();
+
+    private IContentService ContentService => GetRequiredService<IContentService>();
+
+    private IContentEditingService ContentEditingService => GetRequiredService<IContentEditingService>();
+
+    protected override void ConfigureTestServices(IServiceCollection services)
+        => services.AddSingleton(AppCaches.Create(Mock.Of<IRequestCache>()));
+
+    [SetUp]
+    public async Task SetupTest()
+    {
+        ContentRepositoryBase.ThrowOnWarning = true;
+
+        _contentType = ContentTypeBuilder.CreateBasicContentType();
+        _contentType.AllowedAsRoot = true;
+        await ContentTypeService.CreateAsync(_contentType, Constants.Security.SuperUserKey);
+    }
+
+    [TearDown]
+    public void Teardown() => ContentRepositoryBase.ThrowOnWarning = false;
+
+    protected override void CustomTestSetup(IUmbracoBuilder builder) => builder
+        .AddNotificationHandler<ContentSavingNotification, ContentNotificationHandler>()
+        .AddNotificationHandler<ContentSavedNotification, ContentNotificationHandler>();
+
+    [Test]
+    public async Task Saving_Saved_Get_Value()
+    {
+        var createAttempt = await ContentEditingService.CreateAsync(
+            new ContentCreateModel
+            {
+                ContentTypeKey = _contentType.Key,
+                Variants = [
+                    new() { Name = "Initial name" }
+                ],
+            },
+            Constants.Security.SuperUserKey);
+
+        Assert.Multiple(() =>
+        {
+            Assert.IsTrue(createAttempt.Success);
+            Assert.IsNotNull(createAttempt.Result.Content);
+        });
+
+        var savingWasCalled = false;
+        var savedWasCalled = false;
+
+        ContentNotificationHandler.SavingContent = notification =>
+        {
+            savingWasCalled = true;
+
+            var saved = notification.SavedEntities.First();
+            var documentById = ContentService.GetById(saved.Id)!;
+            var documentByKey = ContentService.GetById(saved.Key)!;
+
+            Assert.Multiple(() =>
+            {
+                Assert.AreEqual("Updated name", saved.Name);
+                Assert.AreEqual("Initial name", documentById.Name);
+                Assert.AreEqual("Initial name", documentByKey.Name);
+            });
+        };
+
+        ContentNotificationHandler.SavedContent = notification =>
+        {
+            savedWasCalled = true;
+
+            var saved = notification.SavedEntities.First();
+            var documentById = ContentService.GetById(saved.Id)!;
+            var documentByKey = ContentService.GetById(saved.Key)!;
+
+            Assert.Multiple(() =>
+            {
+                Assert.AreEqual("Updated name", saved.Name);
+                Assert.AreEqual("Updated name", documentById.Name);
+                Assert.AreEqual("Updated name", documentByKey.Name);
+            });
+        };
+
+        try
+        {
+            var updateAttempt = await ContentEditingService.UpdateAsync(
+                createAttempt.Result.Content!.Key,
+                new ContentUpdateModel
+                {
+                    Variants = [
+                        new() { Name = "Updated name" }
+                    ],
+                },
+                Constants.Security.SuperUserKey);
+
+            Assert.Multiple(() =>
+            {
+                Assert.IsTrue(updateAttempt.Success);
+                Assert.IsNotNull(updateAttempt.Result.Content);
+            });
+
+            Assert.IsTrue(savingWasCalled);
+            Assert.IsTrue(savedWasCalled);
+        }
+        finally
+        {
+            ContentNotificationHandler.SavingContent = null;
+            ContentNotificationHandler.SavedContent = null;
+        }
+    }
+
+    internal sealed class ContentNotificationHandler :
+        INotificationHandler<ContentSavingNotification>,
+        INotificationHandler<ContentSavedNotification>
+    {
+        public static Action<ContentSavingNotification>? SavingContent { get; set; }
+
+        public static Action<ContentSavedNotification>? SavedContent { get; set; }
+
+        public void Handle(ContentSavedNotification notification) => SavedContent?.Invoke(notification);
+
+        public void Handle(ContentSavingNotification notification) => SavingContent?.Invoke(notification);
+    }
+}

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaServiceNotificationWithCacheTests.cs

@@ -0,0 +1,146 @@
+using Microsoft.Extensions.DependencyInjection;
+using Moq;
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Cache;
+using Umbraco.Cms.Core.Events;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models.ContentEditing;
+using Umbraco.Cms.Core.Notifications;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
+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,
+    PublishedRepositoryEvents = true,
+    WithApplication = true,
+    Logger = UmbracoTestOptions.Logger.Console)]
+internal sealed class MediaServiceNotificationWithCacheTests : UmbracoIntegrationTest
+{
+    private IMediaType _mediaType;
+
+    private IMediaTypeService MediaTypeService => GetRequiredService<IMediaTypeService>();
+
+    private IMediaService MediaService => GetRequiredService<IMediaService>();
+
+    private IMediaEditingService MediaEditingService => GetRequiredService<IMediaEditingService>();
+
+    protected override void ConfigureTestServices(IServiceCollection services)
+        => services.AddSingleton(AppCaches.Create(Mock.Of<IRequestCache>()));
+
+    [SetUp]
+    public void SetupTest()
+    {
+        ContentRepositoryBase.ThrowOnWarning = true;
+
+        _mediaType = MediaTypeService.Get("folder")
+                     ?? throw new ApplicationException("Could not find the \"folder\" media type");
+    }
+
+    [TearDown]
+    public void Teardown() => ContentRepositoryBase.ThrowOnWarning = false;
+
+    protected override void CustomTestSetup(IUmbracoBuilder builder) => builder
+        .AddNotificationHandler<MediaSavingNotification, MediaNotificationHandler>()
+        .AddNotificationHandler<MediaSavedNotification, MediaNotificationHandler>();
+
+    [Test]
+    public async Task Saving_Saved_Get_Value()
+    {
+        var createAttempt = await MediaEditingService.CreateAsync(
+            new MediaCreateModel
+            {
+                ContentTypeKey = _mediaType.Key,
+                Variants = [
+                    new() { Name = "Initial name" }
+                ],
+            },
+            Constants.Security.SuperUserKey);
+
+        Assert.Multiple(() =>
+        {
+            Assert.IsTrue(createAttempt.Success);
+            Assert.IsNotNull(createAttempt.Result.Content);
+        });
+
+        var savingWasCalled = false;
+        var savedWasCalled = false;
+
+        MediaNotificationHandler.SavingMedia = notification =>
+        {
+            savingWasCalled = true;
+
+            var saved = notification.SavedEntities.First();
+            var documentById = MediaService.GetById(saved.Id)!;
+            var documentByKey = MediaService.GetById(saved.Key)!;
+
+            Assert.Multiple(() =>
+            {
+                Assert.AreEqual("Updated name", saved.Name);
+                Assert.AreEqual("Initial name", documentById.Name);
+                Assert.AreEqual("Initial name", documentByKey.Name);
+            });
+        };
+
+        MediaNotificationHandler.SavedMedia = notification =>
+        {
+            savedWasCalled = true;
+
+            var saved = notification.SavedEntities.First();
+            var documentById = MediaService.GetById(saved.Id)!;
+            var documentByKey = MediaService.GetById(saved.Key)!;
+
+            Assert.Multiple(() =>
+            {
+                Assert.AreEqual("Updated name", saved.Name);
+                Assert.AreEqual("Updated name", documentById.Name);
+                Assert.AreEqual("Updated name", documentByKey.Name);
+            });
+        };
+
+        try
+        {
+            var updateAttempt = await MediaEditingService.UpdateAsync(
+                createAttempt.Result.Content!.Key,
+                new MediaUpdateModel
+                {
+                    Variants = [
+                        new() { Name = "Updated name" }
+                    ],
+                },
+                Constants.Security.SuperUserKey);
+
+            Assert.Multiple(() =>
+            {
+                Assert.IsTrue(updateAttempt.Success);
+                Assert.IsNotNull(updateAttempt.Result.Content);
+            });
+
+            Assert.IsTrue(savingWasCalled);
+            Assert.IsTrue(savedWasCalled);
+        }
+        finally
+        {
+            MediaNotificationHandler.SavingMedia = null;
+            MediaNotificationHandler.SavedMedia = null;
+        }
+    }
+
+    internal sealed class MediaNotificationHandler :
+        INotificationHandler<MediaSavingNotification>,
+        INotificationHandler<MediaSavedNotification>
+    {
+        public static Action<MediaSavingNotification>? SavingMedia { get; set; }
+
+        public static Action<MediaSavedNotification>? SavedMedia { get; set; }
+
+        public void Handle(MediaSavedNotification notification) => SavedMedia?.Invoke(notification);
+
+        public void Handle(MediaSavingNotification notification) => SavingMedia?.Invoke(notification);
+    }
+}