Remove wildcard caching logic and simplify URL lookup#186
Remove wildcard caching logic and simplify URL lookup#186Alexandre-P26 wants to merge 1 commit intoGeta:masterfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the CustomRedirectCollection by removing complex wildcard and sub-segment matching logic in favor of a direct dictionary lookup. While this streamlines the code, it results in several private methods becoming dead code and is expected to cause failures in the existing test suite which relies on the removed functionality.
| private CustomRedirect FindInternal(string url) | ||
| { | ||
| url = HttpUtility.UrlDecode(url) ?? string.Empty; | ||
| if (_quickLookupTable.TryGetValue(url, out var redirect)) | ||
| { | ||
| return redirect; | ||
| } | ||
|
|
||
| // working with local copy to avoid multi-threading issues | ||
| var redirectsZA = _redirectsZACache; | ||
| if (redirectsZA == null) | ||
| { | ||
| redirectsZA = _quickLookupTable.OrderByDescending(x => x.Key, StringComparer.OrdinalIgnoreCase).ToArray(); | ||
| _redirectsZACache = redirectsZA; | ||
| } | ||
|
|
||
| var path = url.AsPathSpan(); | ||
| var query = url.AsQuerySpan(); | ||
|
|
||
| // No exact match could be done, so we'll check if the 404 url | ||
| // starts with one of the urls we're matching against. This | ||
| // will be kind of a wild card match (even though we only check | ||
| // for the start of the url | ||
| // Example: http://www.mysite.com/news/mynews.html is not found | ||
| // We have defined an "<old>/news</old>" entry in the config | ||
| // file. We will get a match on the /news part of /news/myne... | ||
| // Depending on the skip wild card append setting, we will either | ||
| // redirect using the <new> url as is, or we'll append the 404 | ||
| // url to the <new> url. | ||
| foreach (var redirectPair in redirectsZA) | ||
| { | ||
| var oldUrl = redirectPair.Key; | ||
| if (string.IsNullOrWhiteSpace(oldUrl)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var oldPath = oldUrl.AsPathSpan(); | ||
| var oldQuery = oldUrl.AsQuerySpan(); | ||
|
|
||
| // See if this "old" url (the one that cannot be found) starts with one | ||
| if (path.UrlPathMatch(oldPath) && query.StartsWith(oldQuery, StringComparison.InvariantCultureIgnoreCase)) | ||
| { | ||
| var cr = redirectPair.Value; | ||
| if (cr.State == (int)RedirectState.Ignored) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| if (cr.WildCardSkipAppend) | ||
| { | ||
| // Remove path from original url but keep query string. | ||
| return CreateSubSegmentRedirect(path, query, cr, oldPath, true); | ||
| } | ||
|
|
||
| if (UrlIsOldUrlsSubSegment(path, oldUrl)) | ||
| { | ||
| return CreateSubSegmentRedirect(path, query, cr, oldPath); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
|
|
||
| return _quickLookupTable.GetValueOrDefault(url); | ||
| } |
There was a problem hiding this comment.
The removal of the wildcard matching logic in FindInternal renders several private methods in this class (CreateSubSegmentRedirect, BuildPath, BuildQuery, UrlIsOldUrlsSubSegment) as dead code. Additionally, many unit tests in CustomRedirectCollectionTests.cs (such as Find_handles_path_and_query_parameters_when_matching_sub_segment and Find_finds_redirect_and_appends_relative_path_when_not_found_url_starts_with_stored_url) will now fail because they rely on the removed wildcard/sub-segment matching functionality. Please ensure that the unused methods are removed and the test suite is updated to reflect the new simplified redirect logic.
2 participants
Removed wildcare redirect. If the redirect is not in the redirect table it should ne be redirecting. This created issue with redirect loop that was hard to diagnosed since the redirect rule is not explicite or easy to find.