Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ private void MigrateSqlServer()
// Convert existing invariant records to use NULL languageId and remove duplicates.
ConvertInvariantDocumentUrlRecords();
ConvertInvariantDocumentUrlAliasRecords();

// Trigger rebuild to update the in-memory cache with new structure.
TriggerRebuild();
}

private void MigrateSqlite()
Expand All @@ -73,7 +70,7 @@ private void MigrateSqlite()
Create.Table<DocumentUrlDto>().Do();
Create.Table<DocumentUrlAliasDto>().Do();

// Trigger rebuild on startup to repopulate the tables
// Trigger rebuild on startup to repopulate the tables.
TriggerRebuild();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@
// do the deletes and inserts
if (toDelete.Count > 0)
{
Database.DeleteMany<DocumentUrlAliasDto>().Where(x => toDelete.Contains(x.Id)).Execute();
foreach (IEnumerable<int> group in toDelete.InGroupsOf(Constants.Sql.MaxParameterCount))
{
Database.DeleteMany<DocumentUrlAliasDto>().Where(x => group.Contains(x.Id)).Execute();
}

Check warning on line 79 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentUrlAliasRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Bumpy Road Ahead

Save has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

Database.InsertBulk(toInsert.Values);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@
// do the deletes, updates and inserts
if (toDelete.Count > 0)
{
Database.DeleteMany<DocumentUrlDto>().Where(x => toDelete.Contains(x.NodeId)).Execute();
foreach (IEnumerable<int> group in toDelete.InGroupsOf(Constants.Sql.MaxParameterCount))
{
Database.DeleteMany<DocumentUrlDto>().Where(x => group.Contains(x.NodeId)).Execute();
}

Check warning on line 87 in src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentUrlRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Bumpy Road Ahead

Save has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is 2 blocks per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}

Database.InsertBulk(toInsert.Values);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using NUnit.Framework;

Check warning on line 1 in tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentUrlServiceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Low Cohesion

This module has at least 19 different responsibilities amongst its 34 functions, threshold = 3. Cohesion is calculated using the LCOM4 metric. Low cohesion means that the module/class has multiple unrelated responsibilities, doing too many things and breaking the Single Responsibility Principle.
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Models;
Expand All @@ -9,6 +9,7 @@
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Core.Sync;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Cms.Tests.Common.Builders;
using Umbraco.Cms.Tests.Common.Builders.Extensions;
using Umbraco.Cms.Tests.Common.Testing;
Expand Down Expand Up @@ -725,4 +726,121 @@
}

#endregion

#region Parameter Count Batching Tests

[Test]
[Explicit("Slow test that requires LocalDb to reproduce the SQL Server 2100 parameter limit. Run manually to verify the batching fix.")]
public async Task Save_With_Many_Stale_Rows_Does_Not_Exceed_Sql_Parameter_Limit()
{
// Arrange
// This test simulates the upgrade scenario where invariant documents previously had
// URL rows stored per-language (non-null languageId). After the v17.3 optimization,
// invariant documents store with NULL languageId. On rebuild, all old per-language rows
// become deletes. If the delete is not batched with InGroupsOf, sites with many documents
// and languages exceed SQL Server's 2100 parameter limit (SqlException error 8003).
//
// NOTE: SQLite does not enforce a parameter limit, so this test verifies functional
// correctness on SQLite and will catch the SQL Server regression when run with LocalDb.

// Create languages via the service (simulating a typical multi-language site).
string[] cultureCodes = ["da-DK", "de-DE", "fr-FR", "es-ES", "it-IT", "nl-NL", "pt-PT", "sv-SE", "nb-NO", "fi-FI"];
var languageIds = new List<int>();
foreach (var cultureCode in cultureCodes)
{
var language = new LanguageBuilder().WithCultureInfo(cultureCode).Build();
var result = await LanguageService.CreateAsync(language, Constants.Security.SuperUserKey);
Assert.IsTrue(result.Success, $"Failed to create language {cultureCode}");
languageIds.Add(result.Result!.Id);
}

// Each document produces (languageCount × 2) stale rows (draft + published per language).
// Compute the required document count dynamically to exceed SQL Server's hard limit of 2100
// parameters (not just Constants.Sql.MaxParameterCount which is 2000).
const int draftPublishedMultiplier = 2;
const int sqlServerParameterLimit = 2100;
var staleRowsPerDocument = languageIds.Count * draftPublishedMultiplier;
var requiredDocumentCount = (sqlServerParameterLimit / staleRowsPerDocument) + 1;

// Start with the documents already created by the base class.
var documentKeys = new List<Guid> { Textpage.Key, Subpage.Key, Subpage2.Key, Subpage3.Key };

// Create additional content nodes to reach the required count.
for (var i = documentKeys.Count; i < requiredDocumentCount; i++)
{
var content = ContentBuilder.CreateSimpleContent(ContentType, $"Bulk Page {i}", Textpage.Id);
ContentService.Save(content, -1);
documentKeys.Add(content.Key);
}

using ICoreScope scope = CoreScopeProvider.CreateCoreScope();
scope.WriteLock(Constants.Locks.DocumentUrls);

var database = ScopeAccessor.AmbientScope!.Database;

// Delete any existing URL rows to start clean.
database.Execute("DELETE FROM umbracoDocumentUrl");

// Insert stale rows: one per (document × language × draft/published).
// These simulate pre-v17.3 data where invariant documents had per-language rows.
var staleRowCount = 0;
foreach (var documentKey in documentKeys)
{
foreach (var languageId in languageIds)
{
foreach (var isDraft in new[] { true, false })
{
database.Execute(
"INSERT INTO umbracoDocumentUrl (uniqueId, languageId, isDraft, urlSegment, isPrimary) VALUES (@0, @1, @2, @3, @4)",
documentKey,
languageId,
isDraft,
"test-segment",
true);
staleRowCount++;
}
}
}

Assert.That(
staleRowCount,
Is.GreaterThan(sqlServerParameterLimit),
$"Test setup should create more than {sqlServerParameterLimit} stale rows to exceed SQL Server's parameter limit");

// Act - Save new-format data with NULL languageId (invariant).
// Every stale row should be deleted because the keys won't match (null vs non-null languageId).
var newSegments = documentKeys.SelectMany(key => new[]
{
new PublishedDocumentUrlSegment
{
DocumentKey = key,
NullableLanguageId = null,
IsDraft = true,
UrlSegment = "test-segment",
IsPrimary = true,
},
new PublishedDocumentUrlSegment
{
DocumentKey = key,
NullableLanguageId = null,
IsDraft = false,
UrlSegment = "test-segment",
IsPrimary = true,
},
}).ToList();

// This should not throw SqlException "too many parameters".
Assert.DoesNotThrow(() => DocumentUrlRepository.Save(newSegments));

// Verify: old rows deleted, new rows inserted.
var remainingRows = database.ExecuteScalar<int>("SELECT COUNT(*) FROM umbracoDocumentUrl");
Assert.That(
remainingRows,
Is.EqualTo(newSegments.Count),
"Should have exactly the new invariant rows after save");

scope.Complete();
}

Check warning on line 843 in tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentUrlServiceTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (main)

❌ New issue: Large Method

Save_With_Many_Stale_Rows_Does_Not_Exceed_Sql_Parameter_Limit has 76 lines, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.

#endregion
}
Loading