diff --git a/src/Umbraco.Core/Services/DocumentUrlService.cs b/src/Umbraco.Core/Services/DocumentUrlService.cs index 5d27f6d634..12f8823652 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/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 82fc8f9119..4426374af2 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -1132,25 +1132,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); + } + } + } +}