Redirects: Fixes self referencing redirects (closes #20139) (#20767)

* Adding fix for self-referncing redirects for 17

* Using umbraco context on failing tests

* Tests to see if self referencing redirects gets deleted

* Refactoring and adding correct tests.

* Expanding tests for RedirectTrackerTests.cs

* Optimize by only retrieving th list of existing URLs for a content item if we have a valid route to create a redirect for.

* Extract method refactoring, added explanatory comment, fixed warnings and formatting.

* Resolved warnings in RedirectService.

* Minor naming and formatting refactor in tests.

---------

Co-authored-by: Andy Butland <[email protected]>

Author: NillasKA

src/Umbraco.Core/Services/RedirectUrlService.cs

@@ -1,4 +1,3 @@
-using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Umbraco.Cms.Core.Events;
 using Umbraco.Cms.Core.Models;
@@ -7,10 +6,16 @@
 
 namespace Umbraco.Cms.Core.Services;
 
+/// <summary>
+/// Provides services for managing redirect URLs.
+/// </summary>
 internal sealed class RedirectUrlService : RepositoryService, IRedirectUrlService
 {
     private readonly IRedirectUrlRepository _redirectUrlRepository;
 
+    /// <summary>
+    /// Initializes a new instance of the <see cref="RedirectUrlService"/> class.
+    /// </summary>
     public RedirectUrlService(
         ICoreScopeProvider provider,
         ILoggerFactory loggerFactory,
@@ -19,132 +24,119 @@ public RedirectUrlService(
         : base(provider, loggerFactory, eventMessagesFactory) =>
         _redirectUrlRepository = redirectUrlRepository;
 
+    /// <inheritdoc/>
     public void Register(string url, Guid contentKey, string? culture = null)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
+        using ICoreScope scope = ScopeProvider.CreateCoreScope();
+        IRedirectUrl? redir = _redirectUrlRepository.Get(url, contentKey, culture);
+        if (redir != null)
         {
-            IRedirectUrl? redir = _redirectUrlRepository.Get(url, contentKey, culture);
-            if (redir != null)
-            {
-                redir.CreateDateUtc = DateTime.UtcNow;
-            }
-            else
-            {
-                redir = new RedirectUrl { Key = Guid.NewGuid(), Url = url, ContentKey = contentKey, Culture = culture };
-            }
-
-            _redirectUrlRepository.Save(redir);
-            scope.Complete();
+            redir.CreateDateUtc = DateTime.UtcNow;
         }
+        else
+        {
+            redir = new RedirectUrl { Key = Guid.NewGuid(), Url = url, ContentKey = contentKey, Culture = culture };
+        }
+
+        _redirectUrlRepository.Save(redir);
+        scope.Complete();
     }
 
+    /// <inheritdoc/>
     public void Delete(IRedirectUrl redirectUrl)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
-        {
-            _redirectUrlRepository.Delete(redirectUrl);
-            scope.Complete();
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope();
+        _redirectUrlRepository.Delete(redirectUrl);
+        scope.Complete();
     }
 
+    /// <inheritdoc/>
     public void Delete(Guid id)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
-        {
-            _redirectUrlRepository.Delete(id);
-            scope.Complete();
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope();
+        _redirectUrlRepository.Delete(id);
+        scope.Complete();
     }
 
+    /// <inheritdoc/>
     public void DeleteContentRedirectUrls(Guid contentKey)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
-        {
-            _redirectUrlRepository.DeleteContentUrls(contentKey);
-            scope.Complete();
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope();
+        _redirectUrlRepository.DeleteContentUrls(contentKey);
+        scope.Complete();
     }
 
+    /// <inheritdoc/>
     public void DeleteAll()
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope())
-        {
-            _redirectUrlRepository.DeleteAll();
-            scope.Complete();
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope();
+        _redirectUrlRepository.DeleteAll();
+        scope.Complete();
     }
 
+    /// <inheritdoc/>
     public IRedirectUrl? GetMostRecentRedirectUrl(string url)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.GetMostRecentUrl(url);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.GetMostRecentUrl(url);
     }
 
+    /// <inheritdoc/>
     public async Task<IRedirectUrl?> GetMostRecentRedirectUrlAsync(string url)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return await _redirectUrlRepository.GetMostRecentUrlAsync(url);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return await _redirectUrlRepository.GetMostRecentUrlAsync(url);
     }
 
+    /// <inheritdoc/>
     public IEnumerable<IRedirectUrl> GetContentRedirectUrls(Guid contentKey)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.GetContentUrls(contentKey);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.GetContentUrls(contentKey);
     }
 
+    /// <inheritdoc/>
     public IEnumerable<IRedirectUrl> GetAllRedirectUrls(long pageIndex, int pageSize, out long total)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.GetAllUrls(pageIndex, pageSize, out total);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.GetAllUrls(pageIndex, pageSize, out total);
     }
 
+    /// <inheritdoc/>
     public IEnumerable<IRedirectUrl> GetAllRedirectUrls(int rootContentId, long pageIndex, int pageSize, out long total)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.GetAllUrls(rootContentId, pageIndex, pageSize, out total);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.GetAllUrls(rootContentId, pageIndex, pageSize, out total);
     }
 
+    /// <inheritdoc/>
     public IEnumerable<IRedirectUrl> SearchRedirectUrls(string searchTerm, long pageIndex, int pageSize, out long total)
     {
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.SearchUrls(searchTerm, pageIndex, pageSize, out total);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.SearchUrls(searchTerm, pageIndex, pageSize, out total);
     }
 
+    /// <inheritdoc/>
     public IRedirectUrl? GetMostRecentRedirectUrl(string url, string? culture)
     {
         if (string.IsNullOrWhiteSpace(culture))
         {
             return GetMostRecentRedirectUrl(url);
         }
 
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return _redirectUrlRepository.GetMostRecentUrl(url, culture);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return _redirectUrlRepository.GetMostRecentUrl(url, culture);
     }
 
+    /// <inheritdoc/>
     public async Task<IRedirectUrl?> GetMostRecentRedirectUrlAsync(string url, string? culture)
     {
         if (string.IsNullOrWhiteSpace(culture))
         {
             return await GetMostRecentRedirectUrlAsync(url);
         }
 
-        using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true))
-        {
-            return await _redirectUrlRepository.GetMostRecentUrlAsync(url, culture);
-        }
+        using ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true);
+        return await _redirectUrlRepository.GetMostRecentUrlAsync(url, culture);
     }
 }