Skip to content

Commit 7a55d67

Browse files
Refactor URL parsing and GetMediaLinkForSrcSet to single-return style - PR Review - 09/15
- Extract ParseAbsoluteUriParams and ParseRelativeUriParams; make ParseUrlParams a single-return wrapper. - Convert ParseRelativeUriParams and GetSitecoreMediaUriWithPreservation to single-return style. - Preserve existing parsing behavior for absolute and relative URIs. - Improve debuggability and reduce branching/early return
2 parents 9abddc0 + 3184370 commit 7a55d67

1 file changed

Lines changed: 64 additions & 36 deletions

File tree

src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs

Lines changed: 64 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@ public static partial class SitecoreFieldExtensions
4545
ArgumentNullException.ThrowIfNull(imageField);
4646
string? urlStr = imageField.Value.Src;
4747

48-
if (urlStr == null)
48+
string? result = null;
49+
if (urlStr != null)
4950
{
50-
return null;
51+
Dictionary<string, object?> mergedParams = MergeParameters(imageParams, srcSetParams);
52+
result = GetSitecoreMediaUriWithPreservation(urlStr, mergedParams);
5153
}
5254

53-
Dictionary<string, object?> mergedParams = MergeParameters(imageParams, srcSetParams);
54-
return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams);
55+
return result;
5556
}
5657

5758
/// <summary>
@@ -137,36 +138,43 @@ private static string GetSitecoreMediaUri(string url, object? imageParams)
137138
/// <param name="urlStr">The URL string.</param>
138139
/// <param name="parameters">Parameters to merge.</param>
139140
/// <returns>Modified URL string.</returns>
140-
private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? parameters)
141+
private static string? GetSitecoreMediaUriWithPreservation(string? urlStr, object? parameters)
141142
{
143+
string? url;
144+
142145
if (string.IsNullOrEmpty(urlStr))
143146
{
144-
return urlStr;
147+
url = urlStr;
145148
}
146-
147-
// Parse existing query parameters and build merged parameters dictionary
148-
Dictionary<string, object?> mergedParams = new(StringComparer.OrdinalIgnoreCase);
149-
Uri? uri = null;
150-
if (!string.IsNullOrEmpty(urlStr))
149+
else
151150
{
152-
Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out uri);
153-
}
151+
// Parse existing query parameters and build merged parameters dictionary
152+
Dictionary<string, object?> mergedParams = new(StringComparer.OrdinalIgnoreCase);
154153

155-
string url = ParseUrlParams(uri, mergedParams);
154+
if (!Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out Uri? uri))
155+
{
156+
url = urlStr;
157+
}
158+
else
159+
{
160+
url = ParseUrlParams(uri, mergedParams);
156161

157-
// Add new parameters (these will override existing ones)
158-
AddParametersToResult(mergedParams, parameters, skipNullValues: true);
162+
// Add new parameters (these will override existing ones)
163+
AddParametersToResult(mergedParams, parameters, skipNullValues: true);
159164

160-
// Add query parameters
161-
foreach (KeyValuePair<string, object?> kvp in mergedParams)
162-
{
163-
if (kvp.Value != null)
164-
{
165-
url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty);
165+
// Add query parameters
166+
foreach (KeyValuePair<string, object?> kvp in mergedParams)
167+
{
168+
if (kvp.Value != null)
169+
{
170+
url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty);
171+
}
172+
}
166173
}
167174
}
168175

169-
return ApplyJssMediaUrlPrefix(url);
176+
string? result = url == null ? null : ApplyJssMediaUrlPrefix(url);
177+
return result;
170178
}
171179

172180
/// <summary>
@@ -177,29 +185,45 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object?
177185
/// <returns>The URL without query parameters.</returns>
178186
private static string ParseUrlParams(Uri? uri, Dictionary<string, object?> parameters)
179187
{
188+
string result;
189+
180190
if (uri == null)
181191
{
182-
return string.Empty;
192+
result = string.Empty;
193+
}
194+
else if (uri.IsAbsoluteUri)
195+
{
196+
result = ParseAbsoluteUriParams(uri, parameters);
197+
}
198+
else
199+
{
200+
result = ParseRelativeUriParams(uri, parameters);
183201
}
184202

185-
if (uri.IsAbsoluteUri)
203+
return result;
204+
}
205+
206+
private static string ParseAbsoluteUriParams(Uri uri, Dictionary<string, object?> parameters)
207+
{
208+
string url = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}";
209+
NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query);
210+
foreach (string? param in queryParams.AllKeys)
186211
{
187-
string url = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}";
188-
NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query);
189-
foreach (string? param in queryParams.AllKeys)
212+
if (!string.IsNullOrEmpty(param))
190213
{
191-
if (!string.IsNullOrEmpty(param))
192-
{
193-
parameters[param] = queryParams[param];
194-
}
214+
parameters[param] = queryParams[param];
195215
}
196-
197-
return url;
198216
}
199217

218+
return url;
219+
}
220+
221+
private static string ParseRelativeUriParams(Uri uri, Dictionary<string, object?> parameters)
222+
{
200223
// For relative URIs, accessing Uri.Query throws InvalidOperationException, so we use string manipulation
201224
string original = uri.OriginalString;
202225
int queryIndex = original.IndexOf('?');
226+
string result;
203227

204228
if (queryIndex >= 0)
205229
{
@@ -210,10 +234,14 @@ private static string ParseUrlParams(Uri? uri, Dictionary<string, object?> param
210234
parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null;
211235
}
212236

213-
return original[..queryIndex];
237+
result = original[..queryIndex];
238+
}
239+
else
240+
{
241+
result = original;
214242
}
215243

216-
return original;
244+
return result;
217245
}
218246

219247
/// <summary>

0 commit comments

Comments
 (0)