diff --git a/src/Umbraco.Core/Services/DocumentUrlService.cs b/src/Umbraco.Core/Services/DocumentUrlService.cs index ae35d89a97..6cee0e0f5c 100644 --- a/src/Umbraco.Core/Services/DocumentUrlService.cs +++ b/src/Umbraco.Core/Services/DocumentUrlService.cs @@ -432,7 +432,8 @@ public class DocumentUrlService : IDocumentUrlService if (draftUrlSegments.Any() is false) { - _logger.LogWarning("No draft URL segments found for document {DocumentKey} in culture {Culture}", document.Key, culture ?? "{null}"); + // Log at debug level because this is expected when a document is not published in a given language. + _logger.LogDebug("No draft URL segments found for document {DocumentKey} in culture {Culture}", document.Key, culture ?? "{null}"); } else { diff --git a/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs b/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs index fb0d2c9219..73078b5868 100644 --- a/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs +++ b/src/Umbraco.Infrastructure/Examine/Deferred/DeliveryApiContentIndexHandleContentChanges.cs @@ -59,7 +59,19 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC RemoveFromIndex(pendingRemovals, index); pendingRemovals.Clear(); - Reindex(content, index); + ReIndexResult reIndexResult = Reindex(content, index); + + + // When we get to this point, we are dealing with either + // a refresh node or a refresh branch (see reindex =...). + // A refresh branch can be many things, the Reindex function takes care of most scenarios. + // But it only reindexes descendants if the base node has any changed cultures (see comments in that function) + // So by checking what kind of operation it did when the initial indexrequest is for a refresh branch, + // we can support reindexing a branch while the base node was unchanged. + if (reIndexResult == ReIndexResult.Updated && changeTypes.HasType(TreeChangeTypes.RefreshBranch)) + { + ReindexDescendants(content, index); + } } } @@ -68,7 +80,7 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC return Task.CompletedTask; }); - private void Reindex(IContent content, IIndex index) + private ReIndexResult Reindex(IContent content, IIndex index) { // get the currently indexed cultures for the content CulturePublishStatus[] existingCultures = index @@ -95,16 +107,19 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC // we likely got here because a removal triggered a "refresh branch" notification, now we // need to delete every last culture of this content and all descendants RemoveFromIndex(content.Id, index); - return; + return ReIndexResult.Removed; } - // if the published state changed of any culture, chances are there are similar changes ot the content descendants + // if the published state changed of any culture, chances are there are similar changes at the content descendants // that need to be reflected in the index, so we'll reindex all descendants var changedCulturePublishStatus = indexedCultures.Intersect(existingCultures).Count() != existingCultures.Length; if (changedCulturePublishStatus) { ReindexDescendants(content, index); + return ReIndexResult.UpdatedWithDescendants; } + + return ReIndexResult.Updated; } private CulturePublishStatus[] UpdateIndex(IContent content, IIndex index) @@ -179,4 +194,11 @@ internal sealed class DeliveryApiContentIndexHandleContentChanges : DeliveryApiC public override int GetHashCode() => HashCode.Combine(Culture, Published); } + + private enum ReIndexResult + { + Updated, + UpdatedWithDescendants, + Removed, + } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 2cb5c0599e..28778c401a 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1155,25 +1155,38 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement IEnumerable propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishedVersionId, entity.Properties, LanguageRepository, out edited, out editedCultures); + var toUpdate = new List(); + var toInsert = new List(); foreach (PropertyDataDto propertyDataDto in propertyDataDtos) { // Check if this already exists and update, else insert a new one if (propertyTypeToPropertyData.TryGetValue((propertyDataDto.PropertyTypeId, propertyDataDto.VersionId, propertyDataDto.LanguageId, propertyDataDto.Segment), out PropertyDataDto? propData)) { propertyDataDto.Id = propData.Id; - Database.Update(propertyDataDto); + toUpdate.Add(propertyDataDto); } else { - // TODO: we can speed this up: Use BulkInsert and then do one SELECT to re-retrieve the property data inserted with assigned IDs. - // This is a perfect thing to benchmark with Benchmark.NET to compare perf between Nuget releases. - Database.Insert(propertyDataDto); + toInsert.Add(propertyDataDto); } // track which ones have been processed existingPropDataIds.Remove(propertyDataDto.Id); } + if (toUpdate.Count > 0) + { + var updateBatch = toUpdate + .Select(x => UpdateBatch.For(x)) + .ToList(); + Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 }); + } + + if (toInsert.Count > 0) + { + Database.InsertBulk(toInsert); + } + // For any remaining that haven't been processed they need to be deleted if (existingPropDataIds.Count > 0) { diff --git a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs index 26efa02f10..d4a9e492bd 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs @@ -54,6 +54,8 @@ public class MediaPicker3PropertyEditor : DataEditor /// internal sealed class MediaPicker3PropertyValueEditor : DataValueEditor, IDataValueReference { + private const string MediaCacheKeyFormat = nameof(MediaPicker3PropertyValueEditor) + "_Media_{0}"; + private readonly IDataTypeConfigurationCache _dataTypeReadCache; private readonly IJsonSerializer _jsonSerializer; private readonly IMediaImportService _mediaImportService; @@ -61,6 +63,7 @@ public class MediaPicker3PropertyEditor : DataEditor private readonly ITemporaryFileService _temporaryFileService; private readonly IScopeProvider _scopeProvider; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + private readonly AppCaches _appCaches; /// /// Initializes a new instance of the class. @@ -93,6 +96,8 @@ public class MediaPicker3PropertyEditor : DataEditor _scopeProvider = scopeProvider; _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _dataTypeReadCache = dataTypeReadCache; + _appCaches = appCaches; + var validators = new TypedJsonValidatorRunner, MediaPicker3Configuration>( jsonSerializer, new MinMaxValidator(localizedTextService), @@ -203,13 +208,31 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { - IMedia? media = _mediaService.GetById(mediaWithCropsDto.MediaKey); + IMedia? media = GetMediaById(mediaWithCropsDto.MediaKey); mediaWithCropsDto.MediaTypeAlias = media?.ContentType.Alias ?? unknownMediaType; } return mediaWithCropsDtos.Where(m => m.MediaTypeAlias != unknownMediaType).ToList(); } + private IMedia? GetMediaById(Guid key) + { + // Cache media lookups in case the same media is handled multiple times across a save operation, + // which is possible, particularly if we have multiple languages and blocks. + var cacheKey = string.Format(MediaCacheKeyFormat, key); + IMedia? media = _appCaches.RequestCache.GetCacheItem(cacheKey); + if (media is null) + { + media = _mediaService.GetById(key); + if (media is not null) + { + _appCaches.RequestCache.Set(cacheKey, media); + } + } + + return media; + } + private List HandleTemporaryMediaUploads(List mediaWithCropsDtos, MediaPicker3Configuration configuration) { var invalidDtos = new List(); @@ -217,7 +240,7 @@ public class MediaPicker3PropertyEditor : DataEditor foreach (MediaWithCropsDto mediaWithCropsDto in mediaWithCropsDtos) { // if the media already exist, don't bother with it - if (_mediaService.GetById(mediaWithCropsDto.MediaKey) != null) + if (GetMediaById(mediaWithCropsDto.MediaKey) != null) { continue; } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentPublishingServiceTests.Publish.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentPublishingServiceTests.Publish.cs index d843f37558..d58fcb2157 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentPublishingServiceTests.Publish.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/ContentPublishingServiceTests.Publish.cs @@ -3,6 +3,7 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models.ContentPublishing; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; using Umbraco.Cms.Tests.Integration.Testing; namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; @@ -388,4 +389,31 @@ public partial class ContentPublishingServiceTests : UmbracoIntegrationTestWithC ?? throw new InvalidOperationException("Expected a publish date for EN"); Assert.Greater(lastPublishDateEn, firstPublishDateEn); } + + [Test] + public async Task Publishing_Single_Culture_Persists_Expected_Property_Data() + { + var (langEn, langDa, langBe, contentType) = await SetupVariantDoctypeAsync(); + var content = await CreateVariantContentAsync(langEn, langDa, langBe, contentType); + + using (var scope = ScopeProvider.CreateScope()) + { + var dtos = scope.Database.Fetch(); + Assert.AreEqual(18, dtos.Count); // 3 properties * 3 cultures * 2 (published + edited). + scope.Complete(); + } + + var publishAttempt = await ContentPublishingService.PublishAsync( + content.Key, + [new() { Culture = langEn.IsoCode }], + Constants.Security.SuperUserKey); + Assert.IsTrue(publishAttempt.Success); + + using (var scope = ScopeProvider.CreateScope()) + { + var dtos = scope.Database.Fetch(); + Assert.AreEqual(19, dtos.Count); // + 3 for published populated title property, - 2 for existing published properties of other cultures. + scope.Complete(); + } + } } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoFetchTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoFetchTests.cs index b7996b779f..d7885bd27f 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoFetchTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoFetchTests.cs @@ -1,13 +1,10 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Collections.Generic; -using System.Linq; using NPoco; using NUnit.Framework; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; -using Umbraco.Extensions; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence.NPocoTests; diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoUpdateBatchTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoUpdateBatchTests.cs new file mode 100644 index 0000000000..ff35ea259c --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/NPocoTests/NPocoUpdateBatchTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using NPoco; +using NUnit.Framework; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence.NPocoTests; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +internal sealed class NPocoUpdateBatchTests : UmbracoIntegrationTest +{ + [Test] + public void Can_Update_Batch() + { + // Arrange + var servers = new List(); + for (var i = 0; i < 3; i++) + { + servers.Add(new ServerRegistrationDto + { + Id = i + 1, + ServerAddress = "address" + i, + ServerIdentity = "computer" + i, + DateRegistered = DateTime.Now, + IsActive = false, + DateAccessed = DateTime.Now, + }); + } + + using (var scope = ScopeProvider.CreateScope()) + { + scope.Database.BulkInsertRecords(servers); + scope.Complete(); + } + + // Act + for (var i = 0; i < 3; i++) + { + servers[i].ServerAddress = "newaddress" + i; + servers[i].IsActive = true; + } + + using (var scope = ScopeProvider.CreateScope()) + { + var updateBatch = servers + .Select(x => UpdateBatch.For(x)) + .ToList(); + var updated = scope.Database.UpdateBatch(updateBatch, new BatchOptions { BatchSize = 100 }); + Assert.AreEqual(3, updated); + scope.Complete(); + } + + // Assert + using (var scope = ScopeProvider.CreateScope()) + { + var dtos = scope.Database.Fetch(); + Assert.AreEqual(3, dtos.Count); + for (var i = 0; i < 3; i++) + { + Assert.AreEqual(servers[i].ServerAddress, dtos[i].ServerAddress); + Assert.AreEqual(servers[i].ServerIdentity, dtos[i].ServerIdentity); + Assert.AreEqual(servers[i].IsActive, dtos[i].IsActive); + } + } + } +}