Fix tracked reference queries for SqlServer (#16020)

* Fixed issue with SqlServer and optimized queries to not do the actual paging if total count is 0

* Cleanup

---------

Co-authored-by: Elitsa <elm@umbraco.dk>

src/Umbraco.Core/Persistence/Repositories/ITrackedReferencesRepository.cs

@@ -68,8 +68,7 @@ public interface ITrackedReferencesRepository
         long skip,
         long take,
         bool filterMustBeIsDependency,
-        out long totalRecords) =>
-        throw new NotImplementedException();
+        out long totalRecords);
 
     [Obsolete("Use overload that takes key instead of id. This will be removed in Umbraco 15.")]
     IEnumerable<RelationItemModel> GetPagedRelationsForItem(
@@ -77,8 +76,7 @@ public interface ITrackedReferencesRepository
         long skip,
         long take,
         bool filterMustBeIsDependency,
-        out long totalRecords) =>
-        throw new NotImplementedException();
+        out long totalRecords);
 
     /// <summary>
     ///     Gets a page of items used in any kind of relation from selected integer ids.

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

@@ -92,7 +92,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
             }
 
             var innerUnionSqlChild = _scopeAccessor.AmbientScope.Database.SqlContext.Sql().Select(
-                    "[cn].uniqueId as key", "[pn].uniqueId as otherKey, [cr].childId as id", "[cr].parentId as otherId", "[rt].[alias]", "[rt].[name]",
+                    "[cn].uniqueId as [key]", "[pn].uniqueId as otherKey, [cr].childId as id", "[cr].parentId as otherId", "[rt].[alias]", "[rt].[name]",
                     "[rt].[isDependency]", "[rt].[dual]")
                 .From<RelationDto>("cr")
                 .InnerJoin<RelationTypeDto>("rt")
@@ -103,7 +103,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 .On<RelationDto, NodeDto>((cr, pn) => cr.ParentId == pn.NodeId, "cr", "pn");
 
             var innerUnionSqlDualParent = _scopeAccessor.AmbientScope.Database.SqlContext.Sql().Select(
-                    "[pn].uniqueId as key", "[cn].uniqueId as otherKey, [dpr].parentId as id", "[dpr].childId as otherId", "[dprt].[alias]", "[dprt].[name]",
+                    "[pn].uniqueId as [key]", "[cn].uniqueId as otherKey, [dpr].parentId as id", "[dpr].childId as otherId", "[dprt].[alias]", "[dprt].[name]",
                     "[dprt].[isDependency]", "[dprt].[dual]")
                 .From<RelationDto>("dpr")
                 .InnerJoin<RelationTypeDto>("dprt")
@@ -115,7 +115,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 .On<RelationDto, NodeDto>((dpr, pn) => dpr.ParentId == pn.NodeId, "dpr", "pn");
 
             var innerUnionSql3 = _scopeAccessor.AmbientScope.Database.SqlContext.Sql().Select(
-                    "[cn].uniqueId as key", "[pn].uniqueId as otherKey, [dcr].childId as id", "[dcr].parentId as otherId", "[dcrt].[alias]", "[dcrt].[name]",
+                    "[cn].uniqueId as [key]", "[pn].uniqueId as otherKey, [dcr].childId as id", "[dcr].parentId as otherId", "[dcrt].[alias]", "[dcrt].[name]",
                     "[dcrt].[isDependency]", "[dcrt].[dual]")
                 .From<RelationDto>("dcr")
                 .InnerJoin<RelationTypeDto>("dcrt")
@@ -313,19 +313,32 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                     aliasRight: "d")
                 .Where<UnionHelperDto>(x => x.Key == key, "x");
 
+
             if (filterMustBeIsDependency)
             {
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            RelationItemDto[] pagedResult =
-                _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
-                Array.Empty<RelationItemDto>();
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
             return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
         }
 
@@ -368,14 +381,26 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            RelationItemDto[] pagedResult =
-                _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
-                Array.Empty<RelationItemDto>();
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
             return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
         }
 
@@ -420,15 +445,26 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            RelationItemDto[] pagedResult =
-                _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
-                Array.Empty<RelationItemDto>();
-
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
             return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
         }
 
@@ -487,14 +523,27 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            List<RelationItemDto>? pagedResult = _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql);
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
-            return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult ??
-                new List<RelationItemDto>());
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
+            return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
          }
 
          [Obsolete("Use overload that takes keys instead of ids. This will be removed in Umbraco 15.")]
@@ -539,14 +588,26 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            RelationItemDto[] pagedResult =
-                _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
-                Array.Empty<RelationItemDto>();
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
             return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
         }
 
@@ -603,15 +664,27 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement
                 sql = sql?.Where<RelationTypeDto>(rt => rt.IsDependency, "x");
             }
 
-            // Ordering is required for paging
-            sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
-
-            List<RelationItemDto>? pagedResult =
-                _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql);
+            // find the count before ordering
             totalRecords = _scopeAccessor.AmbientScope?.Database.Count(sql!) ?? 0;
 
-            return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult ??
-                new List<RelationItemDto>());
+            RelationItemDto[] pagedResult;
+
+            //Only to all this, if there is items
+            if (totalRecords > 0)
+            {
+                // Ordering is required for paging
+                sql = sql?.OrderBy<RelationTypeDto>(x => x.Alias, "x");
+
+                pagedResult =
+                    _scopeAccessor.AmbientScope?.Database.SkipTake<RelationItemDto>(skip, take, sql).ToArray() ??
+                    Array.Empty<RelationItemDto>();
+            }
+            else
+            {
+                pagedResult = Array.Empty<RelationItemDto>();
+            }
+
+            return _umbracoMapper.MapEnumerable<RelationItemDto, RelationItemModel>(pagedResult);
         }
 
         private class UnionHelperDto

src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseExtensions.cs

@@ -77,7 +77,8 @@ internal static class UmbracoDatabaseExtensions
 
     public static long Count(this IUmbracoDatabase database, Sql sql)
     {
-        var query = new Sql().Select("COUNT(*)").From().Append("(").Append(sql).Append(")");
+        // We need to copy the sql into a new object, to avoid this method from changing the sql.
+        var query = new Sql().Select("COUNT(*)").From().Append("(").Append(new Sql(sql.SQL, sql.Arguments)).Append(") as count_query");
 
         return database.ExecuteScalar<long>(query);
     }

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

@@ -0,0 +1,91 @@
+// Copyright (c) Umbraco.
+// See LICENSE for more details.
+using NUnit.Framework;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Services;
+using Umbraco.Cms.Tests.Common.Builders;
+using Umbraco.Cms.Tests.Common.Builders.Extensions;
+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)]
+public class TrackedReferencesServiceTests : UmbracoIntegrationTest
+{
+    private IContentTypeService ContentTypeService => GetRequiredService<IContentTypeService>();
+
+    private IContentService ContentService => GetRequiredService<IContentService>();
+
+    private Content Root1 { get; set; }
+
+    private Content Root2 { get; set; }
+
+    private IContentType ContentType { get; set; }
+
+    [SetUp]
+    public void Setup() => CreateTestData();
+
+    protected virtual void CreateTestData()
+    {
+        ContentType = new ContentTypeBuilder()
+            .WithName("Page")
+            .AddPropertyType()
+                .WithAlias("ContentPicker")
+                .WithName("contentPicker")
+                .WithDataTypeId(1046)
+                .WithPropertyEditorAlias(Constants.PropertyEditors.Aliases.ContentPicker)
+            .Done()
+            .Build();
+
+        ContentTypeService.Save(ContentType);
+
+        Root1 = new ContentBuilder()
+            .WithContentType(ContentType)
+            .WithName("Root 1")
+            .Build();
+
+        ContentService.Save(Root1);
+        ContentService.Publish(Root1, ["*"]);
+
+        Root2 = new ContentBuilder()
+            .WithContentType(ContentType)
+            .WithName("Root 2")
+            .WithPropertyValues(new
+            {
+                contentPicker = Udi.Create(Constants.UdiEntityType.Document, Root1.Key) // contentPicker is the alias of the property type
+            })
+            .Build();
+
+        ContentService.Save(Root2);
+        ContentService.Publish(Root2, ["*"]);
+    }
+
+    [Test]
+    public async Task Get_Pages_That_Reference_This()
+    {
+        var sut = GetRequiredService<ITrackedReferencesService>();
+
+        var actual = await sut.GetPagedRelationsForItemAsync(Root1.Key, 0, 10, true);
+
+        Assert.Multiple(() =>
+        {
+            Assert.AreEqual(1, actual.Total);
+            var item = actual.Items.FirstOrDefault();
+            Assert.AreEqual(Root2.ContentType.Alias, item?.ContentTypeAlias);
+            Assert.AreEqual(Root2.Key, item?.NodeKey);
+        });
+    }
+
+    [Test]
+    public async Task Does_not_return_references_if_item_is_not_referenced()
+    {
+        var sut = GetRequiredService<ITrackedReferencesService>();
+
+        var actual = await sut.GetPagedRelationsForItemAsync(Root2.Key, 0, 10, true);
+
+        Assert.AreEqual(0, actual.Total);
+    }
+}