Skip to content

Commit a7b4481

Browse files
NillasKAAndyButland
NillasKA
and
AndyButland
authored
Redirects: Fix self referencing redirects for 13 (#20908)
* Ported fix to 13 * Mocking context and cache * Removing unused parameter in constructor * Removed a couple of unused variables in the tests. --------- Co-authored-by: Andy Butland <[email protected]>
1 parent 9baaad4 commit a7b4481

File tree

3 files changed

+161
-3
lines changed

3 files changed

+161
-3
lines changed

‎src/Umbraco.Core/Services/RedirectUrlService.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
using System.Threading.Tasks;
2-
using Microsoft.Extensions.Logging;
1+
using Microsoft.Extensions.Logging;
32
using Umbraco.Cms.Core.Events;
43
using Umbraco.Cms.Core.Models;
54
using Umbraco.Cms.Core.Persistence.Repositories;
@@ -146,5 +145,5 @@ public IEnumerable<IRedirectUrl> SearchRedirectUrls(string searchTerm, long page
146145
{
147146
return await _redirectUrlRepository.GetMostRecentUrlAsync(url, culture);
148147
}
149-
}
148+
}
150149
}

‎src/Umbraco.Infrastructure/Routing/RedirectTracker.cs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public void CreateRedirects(IDictionary<(int ContentId, string Culture), (Guid C
9595

9696
using UmbracoContextReference reference = _umbracoContextFactory.EnsureUmbracoContext();
9797
IPublishedContentCache? contentCache = reference.UmbracoContext.Content;
98+
9899
if (contentCache == null)
99100
{
100101
_logger.LogWarning("Could not track redirects because there is no published content cache available on the current published snapshot.");
@@ -106,11 +107,16 @@ public void CreateRedirects(IDictionary<(int ContentId, string Culture), (Guid C
106107
try
107108
{
108109
var newRoute = contentCache.GetRouteById(contentId, culture);
110+
109111
if (!IsValidRoute(newRoute) || oldRoute == newRoute)
110112
{
111113
continue;
112114
}
113115

116+
// Ensure we don't create a self-referencing redirect. This can occur if a document is renamed and then the name is reverted back
117+
// to the original. We resolve this by removing any existing redirect that points to the new route.
118+
RemoveSelfReferencingRedirect(contentKey, newRoute);
119+
114120
_redirectUrlService.Register(oldRoute, contentKey, culture);
115121
}
116122
catch (Exception ex)
@@ -121,5 +127,17 @@ public void CreateRedirects(IDictionary<(int ContentId, string Culture), (Guid C
121127
}
122128

123129
private static bool IsValidRoute([NotNullWhen(true)] string? route) => route is not null && !route.StartsWith("err/");
130+
131+
private void RemoveSelfReferencingRedirect(Guid contentKey, string route)
132+
{
133+
IEnumerable<IRedirectUrl> allRedirectUrls = _redirectUrlService.GetContentRedirectUrls(contentKey);
134+
foreach (IRedirectUrl redirectUrl in allRedirectUrls)
135+
{
136+
if (redirectUrl.Url == route)
137+
{
138+
_redirectUrlService.Delete(redirectUrl.Key);
139+
}
140+
}
141+
}
124142
}
125143
}

‎tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Routing/RedirectTrackerTests.cs‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
using Microsoft.Extensions.Logging;
2+
using Microsoft.Extensions.Logging.Abstractions;
3+
using Moq;
4+
using NUnit.Framework;
5+
using Umbraco.Cms.Core;
6+
using Umbraco.Cms.Core.Cache;
7+
using Umbraco.Cms.Core.Models;
8+
using Umbraco.Cms.Core.Models.PublishedContent;
9+
using Umbraco.Cms.Core.PublishedCache;
10+
using Umbraco.Cms.Core.Routing;
11+
using Umbraco.Cms.Core.Services;
12+
using Umbraco.Cms.Core.Web;
13+
using Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement;
14+
using Umbraco.Cms.Infrastructure.Routing;
15+
using Umbraco.Cms.Infrastructure.Scoping;
16+
using Umbraco.Cms.Tests.Common.Testing;
17+
using Umbraco.Cms.Tests.Integration.Testing;
18+
19+
namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Routing;
20+
21+
[TestFixture]
22+
[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
23+
public class RedirectTrackerTests : UmbracoIntegrationTestWithContent
24+
{
25+
private IRedirectUrlService RedirectUrlService => GetRequiredService<IRedirectUrlService>();
26+
27+
private IContent _rootPage;
28+
private IContent _testPage;
29+
30+
public override void CreateTestData()
31+
{
32+
base.CreateTestData();
33+
34+
var rootContent = ContentService.GetRootContent().First();
35+
_rootPage = rootContent;
36+
var subPages = ContentService.GetPagedChildren(rootContent.Id, 0, 3, out _).ToList();
37+
_testPage = subPages[0];
38+
}
39+
40+
[Test]
41+
public void Can_Create_Redirects()
42+
{
43+
IDictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> dict =
44+
new Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)>
45+
{
46+
[(_testPage.Id, "en")] = (_testPage.Key, "/old-route"),
47+
};
48+
var redirectTracker = CreateRedirectTracker();
49+
50+
redirectTracker.CreateRedirects(dict);
51+
52+
var redirects = RedirectUrlService.GetContentRedirectUrls(_testPage.Key);
53+
Assert.AreEqual(1, redirects.Count());
54+
var redirect = redirects.First();
55+
Assert.AreEqual("/old-route", redirect.Url);
56+
}
57+
58+
[Test]
59+
public void Will_Remove_Self_Referencing_Redirects()
60+
{
61+
CreateExistingRedirect();
62+
63+
var redirects = RedirectUrlService.GetContentRedirectUrls(_testPage.Key);
64+
Assert.IsTrue(redirects.Any(x => x.Url == "/new-route")); // Ensure self referencing redirect exists.
65+
66+
IDictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)> dict =
67+
new Dictionary<(int ContentId, string Culture), (Guid ContentKey, string OldRoute)>
68+
{
69+
[(_testPage.Id, "en")] = (_testPage.Key, "/old-route"),
70+
};
71+
72+
var redirectTracker = CreateRedirectTracker();
73+
redirectTracker.CreateRedirects(dict);
74+
75+
redirects = RedirectUrlService.GetContentRedirectUrls(_testPage.Key);
76+
Assert.AreEqual(1, redirects.Count());
77+
var redirect = redirects.First();
78+
Assert.AreEqual("/old-route", redirect.Url);
79+
}
80+
81+
private RedirectUrlRepository CreateRedirectUrlRepository() =>
82+
new(
83+
(IScopeAccessor)ScopeProvider,
84+
AppCaches.Disabled,
85+
new NullLogger<RedirectUrlRepository>());
86+
87+
private IRedirectTracker CreateRedirectTracker()
88+
{
89+
var contentType = new Mock<IPublishedContentType>();
90+
contentType.SetupGet(c => c.Variations).Returns(ContentVariation.Nothing);
91+
92+
var cultures = new Dictionary<string, PublishedCultureInfo>
93+
{
94+
{ "en", new PublishedCultureInfo("en", "english", "/en/", DateTime.UtcNow) },
95+
};
96+
97+
var rootContent = new Mock<IPublishedContent>();
98+
rootContent.SetupGet(c => c.Id).Returns(_rootPage.Id);
99+
rootContent.SetupGet(c => c.Key).Returns(_rootPage.Key);
100+
rootContent.SetupGet(c => c.Name).Returns(_rootPage.Name);
101+
rootContent.SetupGet(c => c.Path).Returns(_rootPage.Path);
102+
103+
var content = new Mock<IPublishedContent>();
104+
content.SetupGet(c => c.Id).Returns(_testPage.Id);
105+
content.SetupGet(c => c.Key).Returns(_testPage.Key);
106+
content.SetupGet(c => c.Name).Returns(_testPage.Name);
107+
content.SetupGet(c => c.Path).Returns(_testPage.Path);
108+
content.SetupGet(c => c.ContentType).Returns(contentType.Object);
109+
content.SetupGet(c => c.Cultures).Returns(cultures);
110+
111+
IPublishedContentCache contentCache = Mock.Of<IPublishedContentCache>();
112+
Mock.Get(contentCache)
113+
.Setup(x => x.GetRouteById(_testPage.Id, "en"))
114+
.Returns("/new-route");
115+
116+
UmbracoContextReference contextReference = new UmbracoContextReference(Mock.Of<IUmbracoContext>(), false, Mock.Of<IUmbracoContextAccessor>());
117+
Mock.Get(contextReference.UmbracoContext)
118+
.Setup(x => x.Content)
119+
.Returns(contentCache);
120+
121+
IUmbracoContextFactory contextFactory = Mock.Of<IUmbracoContextFactory>();
122+
Mock.Get(contextFactory)
123+
.Setup(x => x.EnsureUmbracoContext())
124+
.Returns(contextReference);
125+
126+
return new RedirectTracker(
127+
contextFactory,
128+
GetRequiredService<IVariationContextAccessor>(),
129+
GetRequiredService<ILocalizationService>(),
130+
RedirectUrlService,
131+
GetRequiredService<ILogger<RedirectTracker>>());
132+
}
133+
134+
private void CreateExistingRedirect()
135+
{
136+
using var scope = ScopeProvider.CreateScope();
137+
var repository = CreateRedirectUrlRepository();
138+
repository.Save(new RedirectUrl { ContentKey = _testPage.Key, Url = "/new-route", Culture = "en" });
139+
scope.Complete();
140+
}
141+
}

0 commit comments

Comments
 (0)