diff --git a/.editorconfig b/.editorconfig index d0561928..ed0419c3 100644 --- a/.editorconfig +++ b/.editorconfig @@ -401,4 +401,11 @@ dotnet_naming_style.camelcase.capitalization = camel_case dotnet_naming_style.s_camelcase.required_prefix = s_ dotnet_naming_style.s_camelcase.required_suffix = dotnet_naming_style.s_camelcase.word_separator = -dotnet_naming_style.s_camelcase.capitalization = camel_case \ No newline at end of file +dotnet_naming_style.s_camelcase.capitalization = camel_case + +[tests/**/*.cs] +# Tests intentionally use xUnit-style method names with underscores. +dotnet_diagnostic.CA1707.severity = none +# Test code deliberately exercises sync validators and uses xUnit-created fixtures. +dotnet_diagnostic.CA1849.severity = none +dotnet_diagnostic.CA1515.severity = none \ No newline at end of file diff --git a/docs/build-log.txt b/docs/build-log.txt index 4f7bee55..0970bf19 100644 --- a/docs/build-log.txt +++ b/docs/build-log.txt @@ -212,6 +212,93 @@ REMAINING BLOCKER None found for the SharpCompress / NU1902 issue after the package pin was applied and verified. +ADDENDUM: ISSUE #280 RELEASE ANALYZER WARNING CLEANUP +---------------------------------------------------- +Generated: 2026-05-10 +Branch: squad/280-cleanup-release-build-warnings +Scope: DevOps/infra-side warning cleanup after refreshing the preserved + issue branch with `dev`. + +BASELINE +-------- +- Preserved worktree branch was 4 commits ahead and 1 commit behind `dev`. + Merged `dev` into the worktree first so the baseline included PR #279 and + the SharpCompress transitive pin. +- Initial deduplicated Release analyzer baseline after the merge: + - 89 warnings + - 0 source-code errors + - 1 local worktree environment error: Tailwind CLI was not resolvable because + the sibling worktree lacked `node_modules` +- Top warning IDs before cleanup: + - CA2007 = 37 + - CA1707 = 34 + - CA1849 = 8 + - CA1515 = 4 + - CA2225 = 2 + - CA1062 = 2 + - CA1724 = 1 + - CA1865 = 1 +- Highest-leverage files before cleanup: + - tests/Domain.Tests/Behaviors/ValidationBehaviorTests.cs + - tests/Domain.Tests/Entities/BlogPostTests.cs + - tests/Domain.Tests/Abstractions/ResultTests.cs + - src/Web/Data/MongoDbBlogPostRepository.cs + - src/Web/Infrastructure/Caching/BlogPostCacheService.cs + - src/Web/Components/Theme/ThemeProvider.razor.cs + +FIXES APPLIED +------------- +- .editorconfig + - Added centralized `[tests/**/*.cs]` suppressions for CA1707, CA1849, and + CA1515 so test-only analyzer noise stays managed in one repo-wide location. + These warnings are xUnit naming / focused-sync-validator patterns in test + code, not production defects. +- src/ServiceDefaults/Extensions.cs + - Renamed the extension container type to `ServiceDefaultsExtensions` to + remove CA1724. + - Added `ArgumentNullException.ThrowIfNull(app)` to remove CA1062. +- src/Domain/Behaviors/ValidationBehavior.cs + - Added `ConfigureAwait(false)` to both async handler awaits. +- src/Web/Data/MongoDbBlogPostRepository.cs + - Added `ConfigureAwait(false)` to repository async calls. +- src/Web/Infrastructure/Caching/BlogPostCacheService.cs + - Added `ConfigureAwait(false)` to distributed-cache and fetch awaits. +- src/Web/Data/BlogDbContext.cs + - Added `ArgumentNullException.ThrowIfNull(modelBuilder)` for CA1062. + - Added a justified CA1515 suppression because the public DbContext type is + part of the composition root and shared test infrastructure. +- Local worktree-only environment fix + - Added an untracked local `node_modules` symlink back to the primary checkout + so Tailwind could build inside the sibling worktree. No tracked files + changed for this environment repair. + +RESULT +------ +- `dotnet build MyBlog.slnx --configuration Release --no-restore` + - Status: ✅ SUCCESS + - Final baseline: 0 warnings, 0 errors + +VERIFICATION +------------ +- `dotnet test tests/Architecture.Tests/Architecture.Tests.csproj --configuration Release --no-build` + - ✅ Passed 16/16 +- `dotnet test tests/Domain.Tests/Domain.Tests.csproj --configuration Release --no-build` + - ✅ Passed 42/42 +- `dotnet test tests/Web.Tests/Web.Tests.csproj --configuration Release --no-build` + - ✅ Passed 153/153 +- `dotnet test tests/Web.Tests.Bunit/Web.Tests.Bunit.csproj --configuration Release --no-build` + - ✅ Passed 69/69 +- `dotnet test tests/Web.Tests.Integration/Web.Tests.Integration.csproj --configuration Release --no-build` + - ✅ Passed 12/12 +- `dotnet test tests/AppHost.Tests/AppHost.Tests.csproj --configuration Release --no-build` + - ✅ Passed 48/49, Skipped 1 documented AppHost theme skip + +REMAINING WARNINGS / HANDOFF +--------------------------- +- None in the final Release build baseline. +- No natural handoff to Legolas is required from this pass because the build is + warning-clean after the infra/backend and test-project cleanup. + ================================================================================ END OF BUILD LOG ================================================================================ diff --git a/src/AppHost/AppHost.csproj b/src/AppHost/AppHost.csproj index eb49f970..cc06c988 100644 --- a/src/AppHost/AppHost.csproj +++ b/src/AppHost/AppHost.csproj @@ -11,6 +11,12 @@ + + + <_Parameter1>AppHost.Tests + + + Exe net10.0 diff --git a/src/AppHost/MongoDbResourceBuilderExtensions.cs b/src/AppHost/MongoDbResourceBuilderExtensions.cs index a8b08d2e..2adf5d0e 100644 --- a/src/AppHost/MongoDbResourceBuilderExtensions.cs +++ b/src/AppHost/MongoDbResourceBuilderExtensions.cs @@ -22,6 +22,12 @@ internal static class MongoDbResourceBuilderExtensions // Shared semaphore — guards all three dev commands (Clear, Seed, Stats) so only one runs at a time. private static readonly SemaphoreSlim _dbMutex = new(1, 1); +/// +/// Test-only hook used by AppHost.Tests to hold the seed command inside the shared mutex +/// so overlapping invocations can be asserted deterministically. +/// +internal static Func? SeedCommandAfterMutexAcquiredAsync { get; set; } + public static IResourceBuilder WithMongoDbDevCommands( this IResourceBuilder builder, string databaseName) @@ -180,6 +186,10 @@ private static void WithSeedDataCommand( try { + var afterMutexAcquired = SeedCommandAfterMutexAcquiredAsync; + if (afterMutexAcquired is not null) + await afterMutexAcquired(context.CancellationToken); + context.Logger.LogInformation( "Seed MyBlog data invoked on {ResourceName} — inserting seed data into '{Database}'.", context.ResourceName, databaseName); diff --git a/src/Domain/Abstractions/Result.cs b/src/Domain/Abstractions/Result.cs index c9bb72b8..324242cb 100644 --- a/src/Domain/Abstractions/Result.cs +++ b/src/Domain/Abstractions/Result.cs @@ -16,6 +16,8 @@ // Project Name : Domain // ======================================================= +using System.Diagnostics.CodeAnalysis; + namespace MyBlog.Domain.Abstractions; public enum ResultErrorCode @@ -140,6 +142,9 @@ public static Result FromValue(T? value) } #pragma warning restore CA1000 // Do not declare static members on generic types + // CA2225 does not recognize Result.ToValue()/FromValue() as valid alternates for + // these generic implicit conversions, so suppress the warning only on the operators. + [SuppressMessage("Usage", "CA2225:Operator overloads have named alternates", Justification = "Result already exposes ToValue()/FromValue() named conversion APIs; the implicit conversions are kept intentionally for application ergonomics.")] public static implicit operator T?(Result? result) { if (result is null) @@ -152,6 +157,7 @@ public static Result FromValue(T? value) return result.Value; } + [SuppressMessage("Usage", "CA2225:Operator overloads have named alternates", Justification = "Result already exposes ToValue()/FromValue() named conversion APIs; the implicit conversions are kept intentionally for application ergonomics.")] public static implicit operator Result(T? value) { return Ok(value); diff --git a/src/Domain/Behaviors/ValidationBehavior.cs b/src/Domain/Behaviors/ValidationBehavior.cs index 6c62077c..878f947a 100644 --- a/src/Domain/Behaviors/ValidationBehavior.cs +++ b/src/Domain/Behaviors/ValidationBehavior.cs @@ -28,7 +28,7 @@ public async Task Handle( ArgumentNullException.ThrowIfNull(next); if (!validators.Any()) - return await next(cancellationToken); + return await next(cancellationToken).ConfigureAwait(false); var context = new ValidationContext(request); var failures = validators @@ -43,7 +43,7 @@ public async Task Handle( return (TResponse)CreateFailResult(typeof(TResponse), errorMessage); } - return await next(cancellationToken); + return await next(cancellationToken).ConfigureAwait(false); } private static object CreateFailResult(Type resultType, string errorMessage) diff --git a/src/ServiceDefaults/Extensions.cs b/src/ServiceDefaults/Extensions.cs index e8b6dbd3..6636392d 100644 --- a/src/ServiceDefaults/Extensions.cs +++ b/src/ServiceDefaults/Extensions.cs @@ -26,7 +26,7 @@ namespace MyBlog.ServiceDefaults; // This project should be referenced by each service project in your solution. // To learn more about using this project, see https://aka.ms/dotnet/aspire/service-defaults [ExcludeFromCodeCoverage(Justification = "Aspire infrastructure bootstrap — not business logic")] -public static class Extensions +public static class ServiceDefaultsExtensions { private const string HealthEndpointPath = "/health"; private const string AlivenessEndpointPath = "/alive"; @@ -121,6 +121,8 @@ public static TBuilder AddDefaultHealthChecks(this TBuilder builder) w public static WebApplication MapDefaultEndpoints(this WebApplication app) { + ArgumentNullException.ThrowIfNull(app); + // Adding health checks endpoints to applications in non-development environments has security implications. // See https://aka.ms/dotnet/aspire/healthchecks for details before enabling these endpoints in non-development environments. if (app.Environment.IsDevelopment() || app.Environment.IsEnvironment("Testing")) diff --git a/src/Web/Data/BlogDbContext.cs b/src/Web/Data/BlogDbContext.cs index a49ccba9..e9c7232b 100644 --- a/src/Web/Data/BlogDbContext.cs +++ b/src/Web/Data/BlogDbContext.cs @@ -7,16 +7,21 @@ //Project Name : Web //======================================================= +using System.Diagnostics.CodeAnalysis; + using MongoDB.EntityFrameworkCore.Extensions; namespace MyBlog.Web.Data; +[SuppressMessage("Design", "CA1515:Consider making public types internal", Justification = "The DbContext type is part of the web composition root and shared test infrastructure.")] public sealed class BlogDbContext(DbContextOptions options) : DbContext(options) { public DbSet BlogPosts => Set(); protected override void OnModelCreating(ModelBuilder modelBuilder) { + ArgumentNullException.ThrowIfNull(modelBuilder); + var entity = modelBuilder.Entity(); entity.ToCollection("blogposts"); entity.HasKey(p => p.Id); diff --git a/src/Web/Data/MongoDbBlogPostRepository.cs b/src/Web/Data/MongoDbBlogPostRepository.cs index e7bc4c79..0ffffb2d 100644 --- a/src/Web/Data/MongoDbBlogPostRepository.cs +++ b/src/Web/Data/MongoDbBlogPostRepository.cs @@ -14,45 +14,45 @@ internal sealed class MongoDbBlogPostRepository(IDbContextFactory { public async Task GetByIdAsync(Guid id, CancellationToken ct = default) { - await using var ctx = await contextFactory.CreateDbContextAsync(ct); + await using var ctx = await contextFactory.CreateDbContextAsync(ct).ConfigureAwait(false); return await ctx.BlogPosts.AsNoTracking() - .FirstOrDefaultAsync(p => p.Id == id, ct); + .FirstOrDefaultAsync(p => p.Id == id, ct).ConfigureAwait(false); } public async Task> GetAllAsync(CancellationToken ct = default) { - await using var ctx = await contextFactory.CreateDbContextAsync(ct); + await using var ctx = await contextFactory.CreateDbContextAsync(ct).ConfigureAwait(false); return await ctx.BlogPosts.AsNoTracking() .OrderByDescending(p => p.CreatedAt) - .ToListAsync(ct); + .ToListAsync(ct).ConfigureAwait(false); } public async Task AddAsync(BlogPost post, CancellationToken ct = default) { - await using var ctx = await contextFactory.CreateDbContextAsync(ct); - await ctx.BlogPosts.AddAsync(post, ct); - await ctx.SaveChangesAsync(ct); + await using var ctx = await contextFactory.CreateDbContextAsync(ct).ConfigureAwait(false); + await ctx.BlogPosts.AddAsync(post, ct).ConfigureAwait(false); + await ctx.SaveChangesAsync(ct).ConfigureAwait(false); } public async Task UpdateAsync(BlogPost post, CancellationToken ct = default) { - await using var ctx = await contextFactory.CreateDbContextAsync(ct); + await using var ctx = await contextFactory.CreateDbContextAsync(ct).ConfigureAwait(false); var entry = ctx.Attach(post); // Version was incremented by post.Update(); the original value in the DB is Version - 1. // EF Core uses OriginalValue in the WHERE filter to detect concurrent modifications. entry.Property(p => p.Version).OriginalValue = post.Version - 1; entry.State = EntityState.Modified; - await ctx.SaveChangesAsync(ct); + await ctx.SaveChangesAsync(ct).ConfigureAwait(false); } public async Task DeleteAsync(Guid id, CancellationToken ct = default) { - await using var ctx = await contextFactory.CreateDbContextAsync(ct); - var post = await ctx.BlogPosts.FindAsync([id], ct); + await using var ctx = await contextFactory.CreateDbContextAsync(ct).ConfigureAwait(false); + var post = await ctx.BlogPosts.FindAsync([id], ct).ConfigureAwait(false); if (post is not null) { ctx.BlogPosts.Remove(post); - await ctx.SaveChangesAsync(ct); + await ctx.SaveChangesAsync(ct).ConfigureAwait(false); } } } diff --git a/src/Web/Infrastructure/Caching/BlogPostCacheService.cs b/src/Web/Infrastructure/Caching/BlogPostCacheService.cs index 897dd9c5..ec677684 100644 --- a/src/Web/Infrastructure/Caching/BlogPostCacheService.cs +++ b/src/Web/Infrastructure/Caching/BlogPostCacheService.cs @@ -34,7 +34,7 @@ public async ValueTask> GetOrFetchAllAsync( return cached; // L2 hit - var bytes = await distributedCache.GetAsync(BlogPostCacheKeys.All, ct); + var bytes = await distributedCache.GetAsync(BlogPostCacheKeys.All, ct).ConfigureAwait(false); if (bytes is not null) { try @@ -49,19 +49,19 @@ public async ValueTask> GetOrFetchAllAsync( catch (JsonException) { // Stale or corrupt bytes — remove and fall through to the DB - await distributedCache.RemoveAsync(BlogPostCacheKeys.All, CancellationToken.None); + await distributedCache.RemoveAsync(BlogPostCacheKeys.All, CancellationToken.None).ConfigureAwait(false); } } // DB via caller-supplied fetch - var result = await fetch(); + var result = await fetch().ConfigureAwait(false); var list = result as List ?? result.ToList(); localCache.Set(BlogPostCacheKeys.All, list, LocalOpts); await distributedCache.SetAsync( BlogPostCacheKeys.All, JsonSerializer.SerializeToUtf8Bytes(list, JsonOpts), RedisOpts, - ct); + ct).ConfigureAwait(false); return result; } @@ -77,7 +77,7 @@ await distributedCache.SetAsync( return cached; // L2 hit - var bytes = await distributedCache.GetAsync(key, ct); + var bytes = await distributedCache.GetAsync(key, ct).ConfigureAwait(false); if (bytes is not null) { try @@ -92,12 +92,12 @@ await distributedCache.SetAsync( catch (JsonException) { // Stale or corrupt bytes — remove and fall through to the DB - await distributedCache.RemoveAsync(key, CancellationToken.None); + await distributedCache.RemoveAsync(key, CancellationToken.None).ConfigureAwait(false); } } // DB via caller-supplied fetch - var result = await fetch(); + var result = await fetch().ConfigureAwait(false); if (result is null) return null; @@ -106,7 +106,7 @@ await distributedCache.SetAsync( key, JsonSerializer.SerializeToUtf8Bytes(result, JsonOpts), RedisOpts, - ct); + ct).ConfigureAwait(false); return result; } @@ -114,7 +114,7 @@ public async Task InvalidateAllAsync(CancellationToken ct = default) { localCache.Remove(BlogPostCacheKeys.All); // CancellationToken.None: the DB write already committed — must not be cancelled - await distributedCache.RemoveAsync(BlogPostCacheKeys.All, CancellationToken.None); + await distributedCache.RemoveAsync(BlogPostCacheKeys.All, CancellationToken.None).ConfigureAwait(false); } public async Task InvalidateByIdAsync(Guid id, CancellationToken ct = default) @@ -122,6 +122,6 @@ public async Task InvalidateByIdAsync(Guid id, CancellationToken ct = default) var key = BlogPostCacheKeys.ById(id); localCache.Remove(key); // CancellationToken.None: the DB write already committed — must not be cancelled - await distributedCache.RemoveAsync(key, CancellationToken.None); + await distributedCache.RemoveAsync(key, CancellationToken.None).ConfigureAwait(false); } } diff --git a/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs b/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs index c1bbcc3c..748502ce 100644 --- a/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs +++ b/tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs @@ -9,6 +9,8 @@ using AppHost.Tests.Infrastructure; +using Aspire.Hosting; + using FluentAssertions; using Microsoft.Extensions.Logging.Abstractions; @@ -80,36 +82,43 @@ public async Task SeedMyBlogData_Concurrent_Invocations_Allow_Only_One_Run() await db.CreateCollectionAsync("blogposts", cancellationToken: TestContext.Current.CancellationToken); var annotation = GetAnnotation(); - - // Act — dispatch both calls to thread-pool workers and open the gate at the same - // moment so they race to acquire _dbMutex. Without this the async lambda may run - // entirely synchronously (fast local MongoDB) and release the semaphore before the - // second call even starts, causing both to succeed (flake). var ct = TestContext.Current.CancellationToken; - using var startGate = new SemaphoreSlim(0, 2); + var enteredCriticalSection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseCriticalSection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - var firstTask = Task.Run(async () => + MongoDbResourceBuilderExtensions.SeedCommandAfterMutexAcquiredAsync = async cancellationToken => { - await startGate.WaitAsync(ct); - return await annotation.ExecuteCommand(MakeContext()); - }, ct); + enteredCriticalSection.TrySetResult(true); + await releaseCriticalSection.Task.WaitAsync(cancellationToken); + }; - var secondTask = Task.Run(async () => + try { - await startGate.WaitAsync(ct); - return await annotation.ExecuteCommand(MakeContext()); - }, ct); - - startGate.Release(2); // open the gate — both workers race for _dbMutex - var results = await Task.WhenAll(firstTask, secondTask); - - // Assert - results.Count(static r => r.Success).Should().Be(1, - "the semaphore should allow only one seed operation to run at a time"); - results.Count(static r => !r.Success).Should().Be(1, - "the overlapping seed attempt should fail fast instead of queueing"); - results.Single(static r => !r.Success).Message.Should().Contain("already in progress", - "the operator needs immediate feedback when another database operation is in flight"); + // Act — hold the first seed invocation inside the shared mutex, then trigger the + // second invocation while the first is still in flight. This proves true overlap + // deterministically instead of relying on Task.Run scheduler timing. + var firstTask = annotation.ExecuteCommand(MakeContext()); + + await enteredCriticalSection.Task.WaitAsync(ct); + var secondResult = await annotation.ExecuteCommand(MakeContext()); + + releaseCriticalSection.TrySetResult(true); + var firstResult = await firstTask; + var results = new[] { firstResult, secondResult }; + + // Assert + results.Count(static r => r.Success).Should().Be(1, + "the semaphore should allow only one seed operation to run at a time"); + results.Count(static r => !r.Success).Should().Be(1, + "the overlapping seed attempt should fail fast instead of queueing"); + results.Single(static r => !r.Success).Message.Should().Contain("already in progress", + "the operator needs immediate feedback when another database operation is in flight"); + } + finally + { + releaseCriticalSection.TrySetResult(true); + MongoDbResourceBuilderExtensions.SeedCommandAfterMutexAcquiredAsync = null; + } } ///