Repository navigation
feat: add Minimal API & Razor Pages support, update to .NET 10 - #5
Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class Minimal API and Razor Pages integration for NoBrute while upgrading the library and test suite to .NET 10 and simplifying ASP.NET Core dependencies.
Changes:
- Target framework bump to
net10.0, dependency cleanup viaFrameworkReferencetoMicrosoft.AspNetCore.App, and version bump to2.2.0. - Added Minimal API support (
NoBruteEndpointFilter+WithNoBrute()endpoint extension) and Razor Pages support (NoBrutePageFilter). - Added new unit tests for the new filters; updated CI workflows to use .NET 10 on Ubuntu runners.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/NoBruteTesting/NoBruteTesting.csproj | Update test TFM to net10.0, move to Microsoft.AspNetCore.App, bump test SDK/runner. |
| tests/NoBruteTesting/NoBrutePageFilterTests.cs | Adds Razor Pages filter test coverage. |
| tests/NoBruteTesting/NoBruteEndpointFilterTests.cs | Adds Minimal API endpoint filter test coverage. |
| tests/NoBruteTesting/Abstracts/NoBrutePageFilterTestCasesAbstract.cs | Adds Razor Pages test harness helpers/mocks. |
| tests/NoBruteTesting/Abstracts/NoBruteEndpointFilterTestCasesAbstract.cs | Adds endpoint filter test harness helpers/mocks. |
| src/NoBrutePageFilter.cs | Introduces Razor Pages filter implementation. |
| src/NoBruteEndpointFilter.cs | Introduces Minimal API endpoint filter implementation. |
| src/NoBrute.csproj | Updates to net10.0, adds version/tag/notes, replaces package refs with framework ref. |
| src/Extensions.cs | Adds DI registration for page filter and adds WithNoBrute() endpoint extension. |
| README.MD | Documents Minimal API and Razor Pages usage + .NET 10 update. |
| CHANGELOG.md | Adds 2.2.0 entry describing new features and .NET 10 update. |
| .github/workflows/pr.yml | Updates CI runner and .NET version. |
| .github/workflows/deploy.yml | Updates deploy pipeline for Ubuntu, .NET 10, and release creation changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nfMalde/NoBrute/sessions/a96abe68-3762-45ad-8d50-ce94939b4be9 Co-authored-by: nfMalde <5366984+nfMalde@users.noreply.github.com>
…into feat/version-jump
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <param name="services">The services.</param> | ||
| /// <param name="configure">Action to configure registration options.</param> | ||
| public static void AddNoBrute(this IServiceCollection services, Action<NoBrute.Models.NoBruteRegistrationOptions> configure) | ||
| { |
There was a problem hiding this comment.
configure(options) is called without a null check. Since this overload is a public extension method, passing null will cause a NullReferenceException at runtime. Consider validating configure (e.g., throw ArgumentNullException) or treating null as a no-op configuration to keep the API robust.
| { | |
| { | |
| if (configure == null) | |
| { | |
| throw new ArgumentNullException(nameof(configure)); | |
| } |
| Stopwatch stopwatch = Stopwatch.StartNew(); | ||
| this.RegisterNoBruteServiceMock(false, increaseMS, "127.0.1"); | ||
|
|
||
| Stopwatch sw = Stopwatch.StartNew(); | ||
| await filter.InvokeAsync(this.GetEndpointFilterInvocationContext(), this.GetEndpointFilterDelegate()); | ||
| sw.Stop(); | ||
|
|
||
| stopwatch.Stop(); |
There was a problem hiding this comment.
The timing assertion is based on stopwatch, which is started before RegisterNoBruteServiceMock and duplicates sw (which is not used in the assertion). This makes the measurement include setup time and leaves a redundant stopwatch variable. Consider timing only the filter invocation (use a single Stopwatch started immediately before calling InvokeAsync, and assert on that).
| Stopwatch stopwatch = Stopwatch.StartNew(); | |
| this.RegisterNoBruteServiceMock(false, increaseMS, "127.0.1"); | |
| Stopwatch sw = Stopwatch.StartNew(); | |
| await filter.InvokeAsync(this.GetEndpointFilterInvocationContext(), this.GetEndpointFilterDelegate()); | |
| sw.Stop(); | |
| stopwatch.Stop(); | |
| this.RegisterNoBruteServiceMock(false, increaseMS, "127.0.1"); | |
| Stopwatch stopwatch = Stopwatch.StartNew(); | |
| await filter.InvokeAsync(this.GetEndpointFilterInvocationContext(), this.GetEndpointFilterDelegate()); | |
| stopwatch.Stop(); |
| Stopwatch stopwatch = Stopwatch.StartNew(); | ||
| this.RegisterNoBruteServiceMock(false, increaseMS, "127.0.1"); | ||
|
|
||
| Stopwatch sw = Stopwatch.StartNew(); | ||
| await filter.OnPageHandlerExecutionAsync(this.GetPageHandlerExecutingContextMock(), this.GetPageHandlerExecutionDelegate()); | ||
| sw.Stop(); | ||
|
|
||
| stopwatch.Stop(); | ||
| stopwatch.ElapsedMilliseconds.ShouldBeGreaterThanOrEqualTo(increaseMS - timingToleranceMS); |
There was a problem hiding this comment.
Same issue in the async timing test: stopwatch starts before the mock/service setup and a second sw is created but not used in the assertion. This makes the measured duration include non-filter overhead and adds dead code. Consider using a single stopwatch around the OnPageHandlerExecutionAsync call and asserting on that value.
| var result = await next(context); | ||
|
|
||
| if (autoProcess) | ||
| { | ||
| service?.AutoProcessRequestRelease(context.HttpContext.Response.StatusCode, requestName); | ||
| } |
There was a problem hiding this comment.
There is no test covering the common Minimal API case where the endpoint returns an IResult (e.g., Results.BadRequest(...)) and the status code is only applied during result execution. Adding a test for that scenario would prevent regressions in auto-processing behavior once status code handling is corrected.
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/nfMalde/NoBrute/sessions/0237c079-8eac-4f7a-8c4e-961a797dee08 Co-authored-by: nfMalde <5366984+nfMalde@users.noreply.github.com>
Agent-Logs-Url: https://github.com/nfMalde/NoBrute/sessions/0237c079-8eac-4f7a-8c4e-961a797dee08 Co-authored-by: nfMalde <5366984+nfMalde@users.noreply.github.com>
Implemented in 8d61fa0 (and ae81db7). I applied the requested thread changes: added null-check for |
No description provided.