Skip to content

Commit fab5360

Browse files
BenjaminMichaelisBenjaminMichaelis
committed
PR Feedback
1 parent 04faf47 commit fab5360

4 files changed

Lines changed: 57 additions & 98 deletions

File tree

‎EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs‎

Lines changed: 54 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,13 @@ public void GenerateSitemapXml_DoesNotIncludeIdentityRoutes()
7676
var baseUrl = "https://test.example.com/";
7777

7878
// Act & Assert
79-
var nodes = _Factory.InServiceScope(serviceProvider =>
80-
{
81-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
82-
SitemapXmlHelpers.GenerateSitemapXml(
83-
tempDir,
84-
siteMappings,
85-
routeConfigurationService,
86-
baseUrl,
87-
out var nodes);
88-
return nodes;
89-
});
79+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
80+
SitemapXmlHelpers.GenerateSitemapXml(
81+
tempDir,
82+
siteMappings,
83+
routeConfigurationService,
84+
baseUrl,
85+
out var nodes);
9086

9187
var allUrls = nodes.Select(n => n.Url).ToList();
9288

@@ -108,17 +104,13 @@ public void GenerateSitemapXml_IncludesBaseUrl()
108104
var baseUrl = "https://test.example.com/";
109105

110106
// Act & Assert
111-
var nodes = _Factory.InServiceScope(serviceProvider =>
112-
{
113-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
114-
SitemapXmlHelpers.GenerateSitemapXml(
115-
tempDir,
116-
siteMappings,
117-
routeConfigurationService,
118-
baseUrl,
119-
out var nodes);
120-
return nodes;
121-
});
107+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
108+
SitemapXmlHelpers.GenerateSitemapXml(
109+
tempDir,
110+
siteMappings,
111+
routeConfigurationService,
112+
baseUrl,
113+
out var nodes);
122114

123115
Assert.Contains(nodes, node => node.Url == baseUrl);
124116

@@ -143,22 +135,19 @@ public void GenerateSitemapXml_IncludesSiteMappingsMarkedForXml()
143135
};
144136

145137
// Act & Assert
146-
_Factory.InServiceScope(serviceProvider =>
147-
{
148-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
149-
SitemapXmlHelpers.GenerateSitemapXml(
150-
tempDir,
151-
siteMappings,
152-
routeConfigurationService,
153-
baseUrl,
154-
out var nodes);
138+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
139+
SitemapXmlHelpers.GenerateSitemapXml(
140+
tempDir,
141+
siteMappings,
142+
routeConfigurationService,
143+
baseUrl,
144+
out var nodes);
155145

156-
var allUrls = nodes.Select(n => n.Url).ToList();
146+
var allUrls = nodes.Select(n => n.Url).ToList();
157147

158-
Assert.Contains(allUrls, url => url.Contains("test-page-1"));
159-
Assert.DoesNotContain(allUrls, url => url.Contains("test-page-2")); // Not marked for XML
160-
Assert.Contains(allUrls, url => url.Contains("test-page-3"));
161-
});
148+
Assert.Contains(allUrls, url => url.Contains("test-page-1"));
149+
Assert.DoesNotContain(allUrls, url => url.Contains("test-page-2")); // Not marked for XML
150+
Assert.Contains(allUrls, url => url.Contains("test-page-3"));
162151
}
163152

164153
[Fact]
@@ -170,21 +159,18 @@ public void GenerateSitemapXml_DoesNotIncludeIndexRoutes()
170159
var baseUrl = "https://test.example.com/";
171160

172161
// Act & Assert
173-
_Factory.InServiceScope(serviceProvider =>
174-
{
175-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
176-
SitemapXmlHelpers.GenerateSitemapXml(
177-
tempDir,
178-
siteMappings,
179-
routeConfigurationService,
180-
baseUrl,
181-
out var nodes);
162+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
163+
SitemapXmlHelpers.GenerateSitemapXml(
164+
tempDir,
165+
siteMappings,
166+
routeConfigurationService,
167+
baseUrl,
168+
out var nodes);
182169

183-
var allUrls = nodes.Select(n => n.Url).ToList();
170+
var allUrls = nodes.Select(n => n.Url).ToList();
184171

185-
// Should not include Index action routes (they're the default)
186-
Assert.DoesNotContain(allUrls, url => url.Contains("/Index", StringComparison.OrdinalIgnoreCase));
187-
});
172+
// Should not include Index action routes (they're the default)
173+
Assert.DoesNotContain(allUrls, url => url.Contains("/Index", StringComparison.OrdinalIgnoreCase));
188174
}
189175

190176
[Fact]
@@ -196,21 +182,18 @@ public void GenerateSitemapXml_DoesNotIncludeErrorRoutes()
196182
var baseUrl = "https://test.example.com/";
197183

198184
// Act & Assert
199-
_Factory.InServiceScope(serviceProvider =>
200-
{
201-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
202-
SitemapXmlHelpers.GenerateSitemapXml(
203-
tempDir,
204-
siteMappings,
205-
routeConfigurationService,
206-
baseUrl,
207-
out var nodes);
185+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
186+
SitemapXmlHelpers.GenerateSitemapXml(
187+
tempDir,
188+
siteMappings,
189+
routeConfigurationService,
190+
baseUrl,
191+
out var nodes);
208192

209-
var allUrls = nodes.Select(n => n.Url).ToList();
193+
var allUrls = nodes.Select(n => n.Url).ToList();
210194

211-
// Should not include Error action routes
212-
Assert.DoesNotContain(allUrls, url => url.Contains("/Error", StringComparison.OrdinalIgnoreCase));
213-
});
195+
// Should not include Error action routes
196+
Assert.DoesNotContain(allUrls, url => url.Contains("/Error", StringComparison.OrdinalIgnoreCase));
214197
}
215198

216199
[Fact]
@@ -224,24 +207,18 @@ public void GenerateAndSerializeSitemapXml_CreatesFileSuccessfully()
224207

225208
// Clean up any existing file
226209
var expectedXmlPath = Path.Combine(tempDir.FullName, "sitemap.xml");
227-
if (File.Exists(expectedXmlPath))
228-
{
229-
File.Delete(expectedXmlPath);
230-
}
210+
File.Delete(expectedXmlPath);
231211

232212
try
233213
{
234214
// Act
235-
_Factory.InServiceScope(serviceProvider =>
236-
{
237-
var routeConfigurationService = serviceProvider.GetRequiredService<IRouteConfigurationService>();
238-
SitemapXmlHelpers.GenerateAndSerializeSitemapXml(
239-
tempDir,
240-
siteMappings,
241-
logger,
242-
routeConfigurationService,
243-
baseUrl);
244-
});
215+
var routeConfigurationService = _Factory.Services.GetRequiredService<IRouteConfigurationService>();
216+
SitemapXmlHelpers.GenerateAndSerializeSitemapXml(
217+
tempDir,
218+
siteMappings,
219+
logger,
220+
routeConfigurationService,
221+
baseUrl);
245222

246223
// Assert
247224
Assert.True(File.Exists(expectedXmlPath));
@@ -254,10 +231,7 @@ public void GenerateAndSerializeSitemapXml_CreatesFileSuccessfully()
254231
finally
255232
{
256233
// Clean up
257-
if (File.Exists(expectedXmlPath))
258-
{
259-
File.Delete(expectedXmlPath);
260-
}
234+
File.Delete(expectedXmlPath);
261235
}
262236
}
263237

‎EssentialCSharp.Web/Helpers/SitemapXmlHelpers.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,3 @@ private static decimal GetPriorityForRoute(string route)
9898
};
9999
}
100100
}
101-
102-
public class InvalidItemException : Exception
103-
{
104-
public InvalidItemException(string? message) : base(message)
105-
{
106-
}
107-
public InvalidItemException(string? message, Exception exception) : base(message, exception)
108-
{
109-
}
110-
}

‎EssentialCSharp.Web/Program.cs‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
using Microsoft.AspNetCore.HttpOverrides;
1212
using Microsoft.AspNetCore.Identity;
1313
using Microsoft.AspNetCore.Identity.UI.Services;
14-
using Microsoft.AspNetCore.Mvc.Infrastructure;
1514
using Microsoft.EntityFrameworkCore;
1615

1716
namespace EssentialCSharp.Web;
@@ -148,7 +147,7 @@ private static void Main(string[] args)
148147
builder.Services.AddRazorPages();
149148
builder.Services.AddCaptchaService(builder.Configuration.GetSection(CaptchaOptions.CaptchaSender));
150149
builder.Services.AddSingleton<ISiteMappingService, SiteMappingService>();
151-
builder.Services.AddScoped<IRouteConfigurationService, RouteConfigurationService>();
150+
builder.Services.AddSingleton<IRouteConfigurationService, RouteConfigurationService>();
152151
builder.Services.AddHostedService<DatabaseMigrationService>();
153152
builder.Services.AddScoped<IReferralService, ReferralService>();
154153

@@ -228,9 +227,8 @@ private static void Main(string[] args)
228227
try
229228
{
230229
// Create a scope to resolve scoped services
231-
using var scope = app.Services.CreateScope();
232-
var routeConfigurationService = scope.ServiceProvider.GetRequiredService<IRouteConfigurationService>();
233-
230+
var routeConfigurationService = app.Services.GetRequiredService<IRouteConfigurationService>();
231+
234232
SitemapXmlHelpers.EnsureSitemapHealthy(siteMappingService.SiteMappings.ToList());
235233
SitemapXmlHelpers.GenerateAndSerializeSitemapXml(wwwrootDirectory, siteMappingService.SiteMappings.ToList(), logger, routeConfigurationService, baseUrl);
236234
logger.LogInformation("Sitemap.xml generation completed successfully during application startup");

‎EssentialCSharp.Web/Services/IRouteConfigurationService.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
using Microsoft.AspNetCore.Mvc;
2-
using System.Reflection;
3-
41
namespace EssentialCSharp.Web.Services;
52

63
public interface IRouteConfigurationService

0 commit comments

Comments
 (0)