refactor: SonarCloud Bucket A — mechanical Roslyn CA/SYSLIB fixes (158)#186
Merged
Conversation
- CA1834: StringBuilder.Append(char) instead of Append(string) for single chars (SQL transports) - CA1510: ArgumentNullException.ThrowIfNull instead of explicit if/throw - CA2016: forward cancellationToken to ReadAsStringAsync (Dashboard.Client) 39 sites. Hand-applied (net10 has no working bulk CA fixer; dotnet format won't apply editorconfig-elevated rules, Roslynator can't load Roslyn 5.0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the ThrowIfDisposed guard idiom to ObjectDisposedException.ThrowIf(cond, this) across 30 files; drop the now-unused [CallerMemberName] name parameter and orphaned System.Runtime.CompilerServices usings (29). ObjectName moves from member name to type name (conventional; no test depends on it). Disposed-guard tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add GC.SuppressFinalize(this) to 26 Dispose()/DisposeAsync() methods that lacked it (completes the standard dispose pattern); add missing using System where needed. Full solution builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…1868/CA1825 - CA1872: Convert.ToHexString over BitConverter.ToString+Replace (7 GenerateQueueName + ReceiveMessage) - CA1850: static MD5.HashData over MD5.Create().ComputeHash (ReceiveMessage) - CA2249: string.Contains over IndexOf(...) >= 0 (3) - CA1860: Count==0 over !Any() (LocalSourceHostedService) - CA1864/CA1854: TryAdd over ContainsKey+Add; TryGetValue (RedisErrorTracking) - CA1868: drop redundant HashSet.Contains guard before Add (ContainerWrapper) - CA1825: Array.Empty over new T[0] (TagsHelper) Redis ErrorTracking tests pass. Full solution builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace ContainsKey followed by indexer with a single TryGetValue lookup across Messages get-or-add (AdditionalMessageData/CustomHeaders/Message/ MessageContext), Metrics get-or-create, integration Helpers, and the SQL transport ConfigurationExtensions read paths (unique out-vars; write paths left untouched). Message + PostgreSQL unit tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the 6 transports' ValidQueueNamePattern from a compiled Regex field to a source-generated [GeneratedRegex] partial method (partial class, usage gets ()). Faster startup, AOT-friendly. Connection-info validation tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onfig.Slugify Convert the two Regex.Replace slug calls to source-generated partial methods. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CA1861: extract constant test-data arrays to static readonly fields (Cors/AssemblyPaths) - SYSLIB1045: GeneratedRegex partial methods in E2E test classes + DashboardSubprocess Completes Bucket A (158/158). Full solution builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR modernizes disposal, dictionary access, regex generation, hash formatting, null checks, and SQL string assembly across Dashboard, transport, queue, messaging, and test code. ChangesModernization sweep across Dashboard, transports, queues, and shared helpers
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Source/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.cs (1)
79-107: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDispose pattern still not thread-safe despite
GC.SuppressFinalizeaddition.This class retains the legacy
_disposedValuebool-field dispose-guard pattern (Lines 80, 88-96) instead of theInterlocked-based pattern adopted elsewhere in this same PR (e.g.,MessageMethodHandling.cs,TaskScheduling/Scheduler.cs). ConcurrentDispose()calls here are not safe against races on_disposedValue. Since this file was touched to addGC.SuppressFinalize(this), consider aligning it with the codebase-wide thread-safe disposal convention.As per coding guidelines: "Use
Interlocked-based disposal patterns for thread-safe disposal throughout the codebase."♻️ Proposed fix aligning with Interlocked-based dispose pattern
- `#region` IDisposable Support - private bool _disposedValue; // To detect redundant calls + `#region` IDisposable Support + private int _disposeCount; /// <summary> /// Releases unmanaged and - optionally - managed resources. /// </summary> /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param> protected virtual void Dispose(bool disposing) { - if (!_disposedValue) - { - if (disposing) - { - _queueCreation.Dispose(); - } - _disposedValue = true; - } + if (!disposing) return; + if (System.Threading.Interlocked.Increment(ref _disposeCount) != 1) return; + _queueCreation.Dispose(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.cs` around lines 79 - 107, The dispose implementation in SqlServerJobQueueCreation still uses the non-thread-safe _disposedValue guard. Update Dispose(bool) and Dispose() to follow the same Interlocked-based disposal pattern used elsewhere in the PR, so concurrent Dispose() calls only run cleanup once. Keep the cleanup centered on _queueCreation.Dispose() and preserve GC.SuppressFinalize(this) in the public Dispose method.Source: Coding guidelines
Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.cs (1)
151-159: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale XML doc references removed
nameparameter.
ThrowIfDisposed()no longer takes anameparameter, but the<param name="name">doc comment above it wasn't updated.📝 Proposed doc fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> private void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.cs` around lines 151 - 159, The XML documentation for ThrowIfDisposed is stale because it still references a removed name parameter. Update the doc comment above ConnectionHolder.ThrowIfDisposed to remove the <param name="name"> entry and keep the summary/exception tags aligned with the current zero-argument method signature.Source/DotNetWorkQueue/Queue/BaseQueue.cs (1)
155-164: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winStale XML doc references removed
nameparameter.Same as the analogous change in
ConnectionHolder.cs:ThrowIfDisposed()dropped thenameparameter but the<param name="name">doc line remains.📝 Proposed doc fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> protected void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue/Queue/BaseQueue.cs` around lines 155 - 164, The XML documentation for ThrowIfDisposed no longer matches its signature because the obsolete name parameter was removed but the <param name="name"> entry still remains. Update the doc comment above ThrowIfDisposed in BaseQueue to remove the stale parameter reference and keep the summary/exception tags aligned with the current method signature, following the same cleanup done in ConnectionHolder.Source/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.cs (1)
57-80: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse an Interlocked disposal guard here.
_disposedValueis still a plain bool, so concurrentDispose()calls can both enterDispose(bool)and double-dispose_creation. AddingGC.SuppressFinalize(this)doesn't fix that race. As per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase."Proposed fix
- private bool _disposedValue; // To detect redundant calls + private int _disposeCount; ... protected virtual void Dispose(bool disposing) { - if (!_disposedValue) - { - if (disposing) - { - _creation.Dispose(); - } - _disposedValue = true; - } + if (!disposing) return; + if (System.Threading.Interlocked.Increment(ref _disposeCount) != 1) return; + + _creation.Dispose(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.cs` around lines 57 - 80, The Dispose pattern in RedisJobQueueCreation is not thread-safe because _disposedValue is a plain bool, so concurrent Dispose calls can race and double-dispose _creation. Update the Dispose(bool) guard to use an Interlocked-based state transition and ensure only one caller can enter the cleanup path, while keeping Dispose() and GC.SuppressFinalize(this) behavior unchanged.Source: Coding guidelines
🧹 Nitpick comments (14)
Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.cs (1)
104-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale
nameparam doc after signature change.Same as
BaseContainer.ThrowIfDisposed— the parameter was dropped but the doc comment fornameremains.✏️ Proposed fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> protected void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.cs` around lines 104 - 112, The ThrowIfDisposed signature in RedisQueueMonitor no longer takes a name parameter, so the XML doc is stale. Remove the leftover <param name="name"> documentation from ThrowIfDisposed and keep the summary/exception docs aligned with the current signature, matching the cleanup done in BaseContainer.ThrowIfDisposed.Source/DotNetWorkQueue/BaseContainer.cs (1)
48-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale
nameparam doc after signature change.
ThrowIfDisposedno longer takes anameparameter, but the XML doc still documents<param name="name">.✏️ Proposed fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> protected void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue/BaseContainer.cs` around lines 48 - 56, The XML documentation for ThrowIfDisposed is stale because the method no longer accepts a name parameter. Update the summary/comments in BaseContainer.ThrowIfDisposed to remove the <param name="name"> documentation and keep the XML doc aligned with the current signature. If needed, also verify the exception doc still matches the method’s actual behavior.Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.cs (1)
82-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDisposal pattern isn't Interlocked-based, unlike sibling files in this PR.
_disposedValueis a plainboolchecked/set without atomicity, whileBaseContainerandRedisQueueMonitorin this same PR useInterlocked.CompareExchange/Interlocked.Incrementon a_disposeCountfield for thread-safe disposal. This class is inconsistent with that pattern.As per coding guidelines: "Use
Interlocked-based disposal patterns for thread-safe disposal throughout the codebase."♻️ Proposed fix
- private bool _disposedValue; // To detect redundant calls + private int _disposeCount; protected virtual void Dispose(bool disposing) { - if (!_disposedValue) - { - if (disposing) - { - _queueCreation.Dispose(); - } - _disposedValue = true; - } + if (!disposing) return; + if (Interlocked.Increment(ref _disposeCount) != 1) return; + _queueCreation.Dispose(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.cs` around lines 82 - 108, The Dispose pattern in SqliteJobQueueCreation is using a non-atomic _disposedValue flag, which is inconsistent with the Interlocked-based approach used by sibling types like BaseContainer and RedisQueueMonitor. Update Dispose(bool) and Dispose() to use a thread-safe Interlocked-based disposal guard, ideally with a dispose counter or CompareExchange pattern, and keep _queueCreation.Dispose() inside the one-time disposal path so the class follows the same lifecycle pattern as the rest of the PR.Source: Coding guidelines
Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs (1)
76-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDispose guard isn't Interlocked-based.
_disposedValueis a plain bool guardingDispose(bool), which isn't thread-safe under concurrentDispose()calls. As per coding guidelines: "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase." Other files touched in this PR (e.g.,DashboardApi.cs) already useInterlocked.Increment/ObjectDisposedException.ThrowIffor this.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs` around lines 76 - 102, The Dispose guard in PostgreSQLJobQueueCreation is using a plain _disposedValue bool, which is not thread-safe for concurrent Dispose calls. Update the Dispose(bool) and Dispose() pattern to use an Interlocked-based guard consistent with the rest of the codebase, similar to DashboardApi, and make sure _queueCreation.Dispose() is only invoked once under concurrency.Source: Coding guidelines
Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.cs (1)
84-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDisposal guard doesn't use
Interlocked, per repo coding guideline.
_disposedValueis a plainboolguarded by_createLockerfor writes but read unguarded inGetDatabase()(Line 86). This diff doesn't touch the guard itself, but as per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase," which this class doesn't follow. Worth a follow-up refactor (e.g.,Interlocked.CompareExchangeon an int flag) outside the scope of this mechanical CA-fix PR.Also applies to: 106-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.cs` around lines 84 - 104, The disposal state in LiteDbConnectionManager still relies on a plain _disposedValue bool and unguarded reads in GetDatabase, which violates the repo’s Interlocked-based disposal pattern. Update the disposal tracking in LiteDbConnectionManager (including GetDatabase and the dispose path) to use an Interlocked-managed integer flag instead of direct bool checks, and keep the existing behavior for shared vs. owned LiteDbConnection creation while making the disposed check thread-safe.Source: Coding guidelines
Source/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.cs (1)
161-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale XML doc references removed
nameparameter.Same as the sibling
SQLServerMessageQueueCreation.cschange:ThrowIfDisposed()is now parameterless but the<param name="name">doc line (Line 164) remains.📝 Proposed doc fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> private void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.cs` around lines 161 - 169, The XML documentation for ThrowIfDisposed is stale because the method is now parameterless but still documents a nonexistent name parameter. Update the comment on ThrowIfDisposed in ConnectionHolder to remove the <param name="name"> entry and keep the summary/exception docs aligned with the current signature, matching the sibling SQLServerMessageQueueCreation change.Source/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.cs (1)
167-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale XML doc references removed
nameparameter.
ThrowIfDisposed()is now parameterless but the doc comment still has<param name="name">The name.</param>at Line 170.📝 Proposed doc fix
/// <summary> /// Throws an exception if this instance has been disposed. /// </summary> - /// <param name="name">The name.</param> /// <exception cref="System.ObjectDisposedException"></exception> protected void ThrowIfDisposed()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.cs` around lines 167 - 175, The XML documentation for ThrowIfDisposed is stale because the method is now parameterless but still declares a <param name="name"> entry. Remove the outdated parameter doc from the ThrowIfDisposed member comment so the summary and exception documentation match the actual signature and no invalid XML doc reference remains.Source/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.cs (1)
267-294: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSibling setters still use
ContainsKey+indexer, unlike the newly-converted getters above.
SetUserParametersAndClause(Lines 223-242),SetUserParameters(Lines 267-277), andSetUserWhereClause(Lines 284-294) remain on the oldContainsKey+indexer pattern whileGetUserParameters,GetUserClause, andAddUserParameterwere converted toTryGetValue. Not a bug, but leaves inconsistent style within the same class after this refactor pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.cs` around lines 267 - 294, The remaining setters in QueueConsumerConfiguration still use the old ContainsKey plus indexer pattern, which is inconsistent with the TryGetValue refactor used in the related getters and AddUserParameter. Update SetUserParametersAndClause, SetUserParameters, and SetUserWhereClause to follow the same TryGetValue-based lookup/add flow as the other methods, using the same AdditionalSettings keys and preserving the existing behavior.Source/DotNetWorkQueue.Dashboard.Client/DashboardApiClient.cs (2)
63-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent modernization: this constructor still uses the old null-check pattern.
The other two constructors were updated to
ArgumentNullException.ThrowIfNull(...), but thisHttpClientoverload still useshttpClient ?? throw new ArgumentNullException(nameof(httpClient)).♻️ Proposed fix for consistency
public DashboardApiClient(HttpClient httpClient) { - _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); + ArgumentNullException.ThrowIfNull(httpClient); + _httpClient = httpClient; _ownsHttpClient = false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Dashboard.Client/DashboardApiClient.cs` around lines 63 - 67, The DashboardApiClient(HttpClient) constructor still uses the older null-check pattern, unlike the other constructors in DashboardApiClient. Update this overload to use ArgumentNullException.ThrowIfNull for the httpClient parameter, then keep the existing assignment to _httpClient and _ownsHttpClient so the constructor matches the modernized style used elsewhere.
310-316: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose lacks Interlocked-based guard against re-entrant/concurrent disposal.
Dispose()conditionally disposes_httpClientwithout an atomic disposed flag; concurrent callers could race on_ownsHttpClient/_httpClientstate. Repo guideline requires Interlocked-based disposal patterns.As per coding guidelines: "Use
Interlocked-based disposal patterns for thread-safe disposal throughout the codebase."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Dashboard.Client/DashboardApiClient.cs` around lines 310 - 316, The DashboardApiClient.Dispose method needs a thread-safe, Interlocked-based disposal guard to prevent re-entrant or concurrent disposal races. Update the Dispose pattern in DashboardApiClient so it atomically marks the instance as disposed before touching _ownsHttpClient or _httpClient, then only disposes the HttpClient once based on that guard and keeps GC.SuppressFinalize(this) in the successful disposal path. Use the existing Dispose() method and the _ownsHttpClient/_httpClient fields as the locating symbols when applying the thread-safe pattern.Source: Coding guidelines
Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/Fixtures/DashboardSubprocess.cs (1)
109-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose doesn't use Interlocked-based guard.
Dispose()uses a plain best-effort pattern without an atomic disposed-flag, so concurrentDispose()calls could race on_process. Repo guideline mandates Interlocked-based disposal patterns for thread-safe disposal.As per coding guidelines: "Use
Interlocked-based disposal patterns for thread-safe disposal throughout the codebase."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/Fixtures/DashboardSubprocess.cs` around lines 109 - 118, Dispose in DashboardSubprocess is not thread-safe because it relies on a plain best-effort check and can race on _process when called concurrently. Update the Dispose method to use an Interlocked-based disposed guard, and make the kill/dispose path run only once by protecting the _process access in DashboardSubprocess with that atomic flag.Source: Coding guidelines
Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/LoginFlowE2ETests.cs (1)
34-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
NotLoginUrl()regex across test classes.The
NotLoginUrl()[GeneratedRegex](pattern^(?!.*/login).*$) is duplicated verbatim inAuthDisabledE2ETests.cs. Consider extracting shared login-flow regex helpers into a common base/helper class to avoid divergence if the pattern needs to change later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/LoginFlowE2ETests.cs` around lines 34 - 38, The NotLoginUrl() GeneratedRegex is duplicated in multiple test classes, so consolidate the shared login-flow regex helpers into one common place. Move the shared login URL regex definitions (including NotLoginUrl() and any related login-flow helpers) into a reusable base class or helper used by LoginFlowE2ETests and AuthDisabledE2ETests, then update both tests to reference the shared symbol instead of keeping separate copies.Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.cs (1)
63-63: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake
Dispose()idempotent with the repo’sInterlockeddisposal pattern.Adding
GC.SuppressFinalize(this)here still leavesDispose()able to rerun the delete/retry block on repeated calls. Please guard the method with anInterlockeddispose flag like the rest of the codebase.As per coding guidelines, "Use
Interlocked-based disposal patterns for thread-safe disposal throughout the codebase."Proposed fix
public class IntegrationConnectionInfo : IDisposable { private string _fileName; + private int _disposeCount; @@ public void Dispose() { + if (Interlocked.Increment(ref _disposeCount) != 1) + return; + if (!string.IsNullOrWhiteSpace(_fileName)) { try { File.Delete(_fileName); @@ } } GC.SuppressFinalize(this); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.cs` at line 63, `Dispose()` in `ConnectionString` is not idempotent because `GC.SuppressFinalize(this)` alone does not prevent the delete/retry logic from running again on repeated calls. Add an `Interlocked`-based disposed flag to the `Dispose()` method, following the same disposal pattern used elsewhere in the repository, and gate the cleanup block so it only executes once before calling `GC.SuppressFinalize(this)`.Source: Coding guidelines
Source/DotNetWorkQueue.Dashboard.Ui/Services/DashboardApiSourceConfig.cs (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this configuration type to use the repository suffix.
DashboardApiSourceConfigstill bakes in aConfigsuffix on a touched configuration type. Please align it to*Configurationbefore this rename propagates further. As per coding guidelines, "UseConfigurationas the config suffix."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Dashboard.Ui/Services/DashboardApiSourceConfig.cs` at line 27, Rename the touched configuration type from DashboardApiSourceConfig to use the Configuration suffix, and update its partial class declaration accordingly. Also search for any references to DashboardApiSourceConfig across the dashboard UI code and rename them to the new type so the repository-wide naming stays consistent with the Configuration convention.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Source/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.cs`:
- Line 1: The file is missing the required LGPL-2.1 license header because it
now begins with a preprocessor directive. Add the repository-standard LGPL-2.1
header at the top of GenerateQueueName.cs before the existing `#region` Using
block, following the same source-file header pattern used in other C# files.
In
`@Source/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.cs`:
- Line 1: Add the missing LGPL-2.1 license header at the top of
GenerateQueueName.cs before the `#region` Using prologue. Update the file header
so it matches the repo’s standard license block required for all source files,
keeping the rest of the GenerateQueueName file unchanged.
In
`@Source/DotNetWorkQueue.Transport.Redis.IntegrationTests/GenerateQueueName.cs`:
- Line 1: The GenerateQueueName.cs file is missing the repository-required
LGPL-2.1 license header because it currently starts directly with a using
statement. Add the standard LGPL-2.1 header comment at the very top of the file
before any code, matching the format used by other C# files in the project.
In
`@Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/GenerateQueueName.cs`:
- Line 1: The GenerateQueueName.cs file is missing the repository-required
LGPL-2.1 license header because it currently begins with a preprocessor
directive. Add the standard LGPL-2.1 header at the very top of the file, before
the `#region` Using directive, so the file matches the required source-file
licensing convention.
In `@Source/DotNetWorkQueue.Transport.SQLite/Basic/CreationScope.cs`:
- Around line 95-98: CreationScope.Dispose currently relies on a plain bool
guard, so concurrent callers can dispose the scoped resources twice. Update
CreationScope to use the repo’s Interlocked-based disposal counter pattern,
matching the thread-safe disposal approach used elsewhere, and apply the check
inside Dispose/Dispose(bool) so only one thread can run the cleanup path and
GC.SuppressFinalize(this) remains paired with that single successful disposal.
In
`@Source/DotNetWorkQueue.Transport.SqlServer.IntegrationTests/GenerateQueueName.cs`:
- Line 1: The file prologue is missing the required LGPL-2.1 license header
before the existing `#region Using` block. Add the standard LGPL-2.1 header at
the very top of `GenerateQueueName.cs`, keeping the current `#region Using`
content intact and ensuring the file matches the repo-wide licensing
requirement.
---
Outside diff comments:
In `@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.cs`:
- Around line 151-159: The XML documentation for ThrowIfDisposed is stale
because it still references a removed name parameter. Update the doc comment
above ConnectionHolder.ThrowIfDisposed to remove the <param name="name"> entry
and keep the summary/exception tags aligned with the current zero-argument
method signature.
In `@Source/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.cs`:
- Around line 57-80: The Dispose pattern in RedisJobQueueCreation is not
thread-safe because _disposedValue is a plain bool, so concurrent Dispose calls
can race and double-dispose _creation. Update the Dispose(bool) guard to use an
Interlocked-based state transition and ensure only one caller can enter the
cleanup path, while keeping Dispose() and GC.SuppressFinalize(this) behavior
unchanged.
In
`@Source/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.cs`:
- Around line 79-107: The dispose implementation in SqlServerJobQueueCreation
still uses the non-thread-safe _disposedValue guard. Update Dispose(bool) and
Dispose() to follow the same Interlocked-based disposal pattern used elsewhere
in the PR, so concurrent Dispose() calls only run cleanup once. Keep the cleanup
centered on _queueCreation.Dispose() and preserve GC.SuppressFinalize(this) in
the public Dispose method.
In `@Source/DotNetWorkQueue/Queue/BaseQueue.cs`:
- Around line 155-164: The XML documentation for ThrowIfDisposed no longer
matches its signature because the obsolete name parameter was removed but the
<param name="name"> entry still remains. Update the doc comment above
ThrowIfDisposed in BaseQueue to remove the stale parameter reference and keep
the summary/exception tags aligned with the current method signature, following
the same cleanup done in ConnectionHolder.
---
Nitpick comments:
In `@Source/DotNetWorkQueue.Dashboard.Client/DashboardApiClient.cs`:
- Around line 63-67: The DashboardApiClient(HttpClient) constructor still uses
the older null-check pattern, unlike the other constructors in
DashboardApiClient. Update this overload to use
ArgumentNullException.ThrowIfNull for the httpClient parameter, then keep the
existing assignment to _httpClient and _ownsHttpClient so the constructor
matches the modernized style used elsewhere.
- Around line 310-316: The DashboardApiClient.Dispose method needs a
thread-safe, Interlocked-based disposal guard to prevent re-entrant or
concurrent disposal races. Update the Dispose pattern in DashboardApiClient so
it atomically marks the instance as disposed before touching _ownsHttpClient or
_httpClient, then only disposes the HttpClient once based on that guard and
keeps GC.SuppressFinalize(this) in the successful disposal path. Use the
existing Dispose() method and the _ownsHttpClient/_httpClient fields as the
locating symbols when applying the thread-safe pattern.
In
`@Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/Fixtures/DashboardSubprocess.cs`:
- Around line 109-118: Dispose in DashboardSubprocess is not thread-safe because
it relies on a plain best-effort check and can race on _process when called
concurrently. Update the Dispose method to use an Interlocked-based disposed
guard, and make the kill/dispose path run only once by protecting the _process
access in DashboardSubprocess with that atomic flag.
In `@Source/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/LoginFlowE2ETests.cs`:
- Around line 34-38: The NotLoginUrl() GeneratedRegex is duplicated in multiple
test classes, so consolidate the shared login-flow regex helpers into one common
place. Move the shared login URL regex definitions (including NotLoginUrl() and
any related login-flow helpers) into a reusable base class or helper used by
LoginFlowE2ETests and AuthDisabledE2ETests, then update both tests to reference
the shared symbol instead of keeping separate copies.
In `@Source/DotNetWorkQueue.Dashboard.Ui/Services/DashboardApiSourceConfig.cs`:
- Line 27: Rename the touched configuration type from DashboardApiSourceConfig
to use the Configuration suffix, and update its partial class declaration
accordingly. Also search for any references to DashboardApiSourceConfig across
the dashboard UI code and rename them to the new type so the repository-wide
naming stays consistent with the Configuration convention.
In
`@Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.cs`:
- Line 63: `Dispose()` in `ConnectionString` is not idempotent because
`GC.SuppressFinalize(this)` alone does not prevent the delete/retry logic from
running again on repeated calls. Add an `Interlocked`-based disposed flag to the
`Dispose()` method, following the same disposal pattern used elsewhere in the
repository, and gate the cleanup block so it only executes once before calling
`GC.SuppressFinalize(this)`.
In `@Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.cs`:
- Around line 84-104: The disposal state in LiteDbConnectionManager still relies
on a plain _disposedValue bool and unguarded reads in GetDatabase, which
violates the repo’s Interlocked-based disposal pattern. Update the disposal
tracking in LiteDbConnectionManager (including GetDatabase and the dispose path)
to use an Interlocked-managed integer flag instead of direct bool checks, and
keep the existing behavior for shared vs. owned LiteDbConnection creation while
making the disposed check thread-safe.
In
`@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs`:
- Around line 76-102: The Dispose guard in PostgreSQLJobQueueCreation is using a
plain _disposedValue bool, which is not thread-safe for concurrent Dispose
calls. Update the Dispose(bool) and Dispose() pattern to use an
Interlocked-based guard consistent with the rest of the codebase, similar to
DashboardApi, and make sure _queueCreation.Dispose() is only invoked once under
concurrency.
In `@Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.cs`:
- Around line 104-112: The ThrowIfDisposed signature in RedisQueueMonitor no
longer takes a name parameter, so the XML doc is stale. Remove the leftover
<param name="name"> documentation from ThrowIfDisposed and keep the
summary/exception docs aligned with the current signature, matching the cleanup
done in BaseContainer.ThrowIfDisposed.
In `@Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.cs`:
- Around line 82-108: The Dispose pattern in SqliteJobQueueCreation is using a
non-atomic _disposedValue flag, which is inconsistent with the Interlocked-based
approach used by sibling types like BaseContainer and RedisQueueMonitor. Update
Dispose(bool) and Dispose() to use a thread-safe Interlocked-based disposal
guard, ideally with a dispose counter or CompareExchange pattern, and keep
_queueCreation.Dispose() inside the one-time disposal path so the class follows
the same lifecycle pattern as the rest of the PR.
In `@Source/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.cs`:
- Around line 161-169: The XML documentation for ThrowIfDisposed is stale
because the method is now parameterless but still documents a nonexistent name
parameter. Update the comment on ThrowIfDisposed in ConnectionHolder to remove
the <param name="name"> entry and keep the summary/exception docs aligned with
the current signature, matching the sibling SQLServerMessageQueueCreation
change.
In
`@Source/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.cs`:
- Around line 167-175: The XML documentation for ThrowIfDisposed is stale
because the method is now parameterless but still declares a <param name="name">
entry. Remove the outdated parameter doc from the ThrowIfDisposed member comment
so the summary and exception documentation match the actual signature and no
invalid XML doc reference remains.
In `@Source/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.cs`:
- Around line 267-294: The remaining setters in QueueConsumerConfiguration still
use the old ContainsKey plus indexer pattern, which is inconsistent with the
TryGetValue refactor used in the related getters and AddUserParameter. Update
SetUserParametersAndClause, SetUserParameters, and SetUserWhereClause to follow
the same TryGetValue-based lookup/add flow as the other methods, using the same
AdditionalSettings keys and preserving the existing behavior.
In `@Source/DotNetWorkQueue/BaseContainer.cs`:
- Around line 48-56: The XML documentation for ThrowIfDisposed is stale because
the method no longer accepts a name parameter. Update the summary/comments in
BaseContainer.ThrowIfDisposed to remove the <param name="name"> documentation
and keep the XML doc aligned with the current signature. If needed, also verify
the exception doc still matches the method’s actual behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b8e8b0d-7c38-4b80-8712-9b14f6ff6153
📒 Files selected for processing (99)
Source/DotNetWorkQueue.Dashboard.Api.Integration.Tests/Helpers/ConsumerStateHelper.csSource/DotNetWorkQueue.Dashboard.Api.Integration.Tests/Helpers/DashboardTestServer.csSource/DotNetWorkQueue.Dashboard.Api.Integration.Tests/Helpers/TransportFixture.csSource/DotNetWorkQueue.Dashboard.Api.Integration.Tests/Tests/CorsIntegrationTests.csSource/DotNetWorkQueue.Dashboard.Api.Tests/Extensions/DashboardExtensionsCorsAndAuthTests.csSource/DotNetWorkQueue.Dashboard.Api.Tests/Extensions/DashboardExtensionsTests.csSource/DotNetWorkQueue.Dashboard.Api/Configuration/DashboardOptions.csSource/DotNetWorkQueue.Dashboard.Api/DashboardApi.csSource/DotNetWorkQueue.Dashboard.Client/DashboardApiClient.csSource/DotNetWorkQueue.Dashboard.Client/DashboardConsumerClient.csSource/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/AuthDisabledE2ETests.csSource/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/Fixtures/DashboardSubprocess.csSource/DotNetWorkQueue.Dashboard.Ui.E2E.Tests/LoginFlowE2ETests.csSource/DotNetWorkQueue.Dashboard.Ui/Services/DashboardApiSourceConfig.csSource/DotNetWorkQueue.Dashboard.Ui/Services/LocalSourceHostedService.csSource/DotNetWorkQueue.IntegrationTests.Shared/Helpers.csSource/DotNetWorkQueue.IntegrationTests.Shared/MessageHandlingShared.csSource/DotNetWorkQueue.IntegrationTests.Shared/Metrics/Metrics.csSource/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.csSource/DotNetWorkQueue.Tests.Shared/AssertHelper.csSource/DotNetWorkQueue.Tests/Messages/MessageContextTests.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnection.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbJobQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbMessageQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/LiteDbConnectionInformation.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLMessageQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.PostgreSQL/SQLConnectionInformation.csSource/DotNetWorkQueue.Transport.Redis.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Redis.IntegrationTests/VerifyQueueData.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisErrorTracking.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueWorkSub.csSource/DotNetWorkQueue.Transport.Redis/RedisConnection.csSource/DotNetWorkQueue.Transport.Redis/RedisConnectionInfo.csSource/DotNetWorkQueue.Transport.RelationalDatabase.Tests/Basic/WriteMessageHistoryHandlerTests.csSource/DotNetWorkQueue.Transport.RelationalDatabase/Basic/BatchPartitionExtensions.csSource/DotNetWorkQueue.Transport.RelationalDatabase/Basic/ExternalTransactionValidator.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/ConnectionString.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SQLite/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.SQLite/Basic/QueryHandler/ReceiveMessage.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindExpiredRecordsToDeleteQueryHandlerErrorDecorator.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindRecordsToResetByHeartBeatDecorator.csSource/DotNetWorkQueue.Transport.SQLite/SqliteConnectionInformation.csSource/DotNetWorkQueue.Transport.SqlServer.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SqlServer/SQLConnectionInformation.csSource/DotNetWorkQueue/Admin/AdminApi.csSource/DotNetWorkQueue/BaseContainer.csSource/DotNetWorkQueue/IoC/ContainerWrapper.csSource/DotNetWorkQueue/Messages/AdditionalMessageData.csSource/DotNetWorkQueue/Messages/CustomHeaders.csSource/DotNetWorkQueue/Messages/Message.csSource/DotNetWorkQueue/Messages/MessageContext.csSource/DotNetWorkQueue/Messages/MessageMethodHandling.csSource/DotNetWorkQueue/Metrics/Net/MetricsNet.csSource/DotNetWorkQueue/Metrics/Net/TagsHelper.csSource/DotNetWorkQueue/Queue/BaseMonitor.csSource/DotNetWorkQueue/Queue/BaseQueue.csSource/DotNetWorkQueue/Queue/HeartBeatWorker.csSource/DotNetWorkQueue/Queue/ProducerQueue.csSource/DotNetWorkQueue/Queue/QueueCancelWork.csSource/DotNetWorkQueue/Queue/QueueMaintenanceService.csSource/DotNetWorkQueue/Queue/QueueMonitor.csSource/DotNetWorkQueue/Queue/WaitForEventOrCancel.csSource/DotNetWorkQueue/Queue/WorkerBase.csSource/DotNetWorkQueue/Queue/WorkerWaitForEventOrCancel.csSource/DotNetWorkQueue/TaskScheduling/Scheduler.csSource/DotNetWorkQueue/TaskScheduling/TaskScheduler.csSource/DotNetWorkQueue/TaskScheduling/WaitForEventOrCancelThreadPool.csSource/DotNetWorkQueue/Trace/Decorator/MessageMethodHandlingDecorator.csSource/DotNetWorkQueue/Trace/Decorator/ProducerMethodJobQueueDecorator.csSource/DotNetWorkQueue/Transport/Memory/Basic/MessageQueueCreation.csSource/DotNetWorkQueue/Transport/Memory/ConnectionInformation.cs
The scripted Bucket A edits wrote utf-8 (no BOM), dropping the byte-order mark on 67 files. Restore it to match the repo convention (CodeRabbit flag). No code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs (1)
83-100: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNon-atomic disposal guard alongside
GC.SuppressFinalize.
_disposedValueis a plain bool, so concurrent callers can both pass the guard inDispose(bool)and dispose_queueCreationtwice. The same pattern was flagged as a stability issue inCreationScope.cs. As per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase."🔒️ Proposed fix using an Interlocked counter
- private bool _disposedValue; + private int _disposeCount; protected virtual void Dispose(bool disposing) { - if (!_disposedValue) + if (disposing && Interlocked.Increment(ref _disposeCount) == 1) { - if (disposing) - { - _queueCreation.Dispose(); - } - _disposedValue = true; + _queueCreation.Dispose(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs` around lines 83 - 100, The disposal guard in PostgreSQLJobQueueCreation.Dispose(bool) is non-atomic, so concurrent Dispose calls can both enter the cleanup path and double-dispose _queueCreation. Replace the plain _disposedValue check with an Interlocked-based thread-safe disposal pattern, mirroring the approach used in CreationScope, and keep GC.SuppressFinalize in Dispose() after the atomic guard ensures cleanup runs only once.Source: Coding guidelines
🧹 Nitpick comments (1)
Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs (1)
212-222: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo disposal guard on
ActivitySourceWrapper.Dispose().Unlike other disposal classes in this PR, this method has no guard against repeated calls; a second
Dispose()would re-invoke_provider.ForceFlush(2000)/Dispose(). As per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase."🔒️ Proposed fix using an Interlocked guard
+ private int _disposeCount; + public void Dispose() { - _listener?.Dispose(); - Source?.Dispose(); - if (_provider != null) - { - _provider.ForceFlush(2000); - _provider.Dispose(); - } + if (Interlocked.Increment(ref _disposeCount) == 1) + { + _listener?.Dispose(); + Source?.Dispose(); + if (_provider != null) + { + _provider.ForceFlush(2000); + _provider.Dispose(); + } + } GC.SuppressFinalize(this); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs` around lines 212 - 222, The ActivitySourceWrapper.Dispose() method currently lacks a disposal guard, so repeated calls can re-run _provider.ForceFlush(2000) and _provider.Dispose(). Update Dispose() to use the same Interlocked-based, thread-safe disposal pattern used elsewhere in the codebase, and gate the cleanup so the body runs only once while still disposing _listener, Source, and _provider safely.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.cs`:
- Line 1: The GenerateQueueName.cs file is missing the required LGPL-2.1 license
header, so add the standard license comment block at the very top of the file
before the existing `#region` Using section. Follow the same header format used in
the other GenerateQueueName.cs files in the project to keep the file consistent
with the licensing guideline.
In
`@Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.cs`:
- Line 1: The file starts with `#region` Using but is missing the required
LGPL-2.1 license header. Add the standard LGPL-2.1 header at the top of
GenerateQueueName.cs before any regions or using directives, matching the
project's source file licensing convention.
---
Outside diff comments:
In
`@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs`:
- Around line 83-100: The disposal guard in
PostgreSQLJobQueueCreation.Dispose(bool) is non-atomic, so concurrent Dispose
calls can both enter the cleanup path and double-dispose _queueCreation. Replace
the plain _disposedValue check with an Interlocked-based thread-safe disposal
pattern, mirroring the approach used in CreationScope, and keep
GC.SuppressFinalize in Dispose() after the atomic guard ensures cleanup runs
only once.
---
Nitpick comments:
In `@Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs`:
- Around line 212-222: The ActivitySourceWrapper.Dispose() method currently
lacks a disposal guard, so repeated calls can re-run _provider.ForceFlush(2000)
and _provider.Dispose(). Update Dispose() to use the same Interlocked-based,
thread-safe disposal pattern used elsewhere in the codebase, and gate the
cleanup so the body runs only once while still disposing _listener, Source, and
_provider safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45ca590e-ebb5-4fb0-971c-d1a50693b999
📒 Files selected for processing (67)
Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnection.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbJobQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbMessageQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/LiteDbConnectionInformation.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLMessageQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.PostgreSQL/SQLConnectionInformation.csSource/DotNetWorkQueue.Transport.Redis.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueWorkSub.csSource/DotNetWorkQueue.Transport.Redis/RedisConnection.csSource/DotNetWorkQueue.Transport.Redis/RedisConnectionInfo.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/ConnectionString.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SQLite/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindExpiredRecordsToDeleteQueryHandlerErrorDecorator.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindRecordsToResetByHeartBeatDecorator.csSource/DotNetWorkQueue.Transport.SQLite/SqliteConnectionInformation.csSource/DotNetWorkQueue.Transport.SqlServer.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SqlServer/SQLConnectionInformation.csSource/DotNetWorkQueue/Admin/AdminApi.csSource/DotNetWorkQueue/BaseContainer.csSource/DotNetWorkQueue/IoC/ContainerWrapper.csSource/DotNetWorkQueue/Messages/MessageContext.csSource/DotNetWorkQueue/Messages/MessageMethodHandling.csSource/DotNetWorkQueue/Queue/BaseMonitor.csSource/DotNetWorkQueue/Queue/BaseQueue.csSource/DotNetWorkQueue/Queue/HeartBeatWorker.csSource/DotNetWorkQueue/Queue/ProducerQueue.csSource/DotNetWorkQueue/Queue/QueueCancelWork.csSource/DotNetWorkQueue/Queue/QueueMonitor.csSource/DotNetWorkQueue/Queue/WaitForEventOrCancel.csSource/DotNetWorkQueue/Queue/WorkerBase.csSource/DotNetWorkQueue/Queue/WorkerWaitForEventOrCancel.csSource/DotNetWorkQueue/TaskScheduling/Scheduler.csSource/DotNetWorkQueue/TaskScheduling/TaskScheduler.csSource/DotNetWorkQueue/TaskScheduling/WaitForEventOrCancelThreadPool.csSource/DotNetWorkQueue/Trace/Decorator/MessageMethodHandlingDecorator.csSource/DotNetWorkQueue/Trace/Decorator/ProducerMethodJobQueueDecorator.csSource/DotNetWorkQueue/Transport/Memory/Basic/MessageQueueCreation.csSource/DotNetWorkQueue/Transport/Memory/ConnectionInformation.cs
✅ Files skipped from review due to trivial changes (5)
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SQLite/Decorator/FindExpiredRecordsToDeleteQueryHandlerErrorDecorator.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/CommandHandler/SendMessage.cs
🚧 Files skipped from review as they are similar to previous changes (50)
- Source/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/IntegrationConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/ConnectionString.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnection.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/CreationScope.cs
- Source/DotNetWorkQueue/Trace/Decorator/MessageMethodHandlingDecorator.cs
- Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/IntegrationConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.Redis/RedisConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/SQLConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.cs
- Source/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/CommandHandler/SendMessage.cs
- Source/DotNetWorkQueue/Transport/Memory/ConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/ConfigurationExtensions.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueCreation.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.cs
- Source/DotNetWorkQueue/Queue/QueueCancelWork.cs
- Source/DotNetWorkQueue.Transport.Redis/RedisConnection.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueWorkSub.cs
- Source/DotNetWorkQueue/Queue/BaseMonitor.cs
- Source/DotNetWorkQueue/Queue/WaitForEventOrCancel.cs
- Source/DotNetWorkQueue/Messages/MessageMethodHandling.cs
- Source/DotNetWorkQueue/IoC/ContainerWrapper.cs
- Source/DotNetWorkQueue/Queue/BaseQueue.cs
- Source/DotNetWorkQueue/Queue/ProducerQueue.cs
- Source/DotNetWorkQueue/Messages/MessageContext.cs
- Source/DotNetWorkQueue.Transport.SQLite/ConfigurationExtensions.cs
- Source/DotNetWorkQueue/TaskScheduling/WaitForEventOrCancelThreadPool.cs
- Source/DotNetWorkQueue/Transport/Memory/Basic/MessageQueueCreation.cs
- Source/DotNetWorkQueue/TaskScheduling/TaskScheduler.cs
- Source/DotNetWorkQueue/Queue/WorkerWaitForEventOrCancel.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.cs
- Source/DotNetWorkQueue.Transport.SQLite/Decorator/FindRecordsToResetByHeartBeatDecorator.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.cs
- Source/DotNetWorkQueue.Transport.SqlServer/SQLConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/LiteDbConnectionInformation.cs
- Source/DotNetWorkQueue/BaseContainer.cs
- Source/DotNetWorkQueue/Admin/AdminApi.cs
- Source/DotNetWorkQueue/TaskScheduling/Scheduler.cs
- Source/DotNetWorkQueue/Queue/HeartBeatWorker.cs
- Source/DotNetWorkQueue.Transport.SQLite/SqliteConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbMessageQueueCreation.cs
- Source/DotNetWorkQueue/Queue/QueueMonitor.cs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs (1)
83-100: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNon-atomic disposal guard alongside
GC.SuppressFinalize.
_disposedValueis a plain bool, so concurrent callers can both pass the guard inDispose(bool)and dispose_queueCreationtwice. The same pattern was flagged as a stability issue inCreationScope.cs. As per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase."🔒️ Proposed fix using an Interlocked counter
- private bool _disposedValue; + private int _disposeCount; protected virtual void Dispose(bool disposing) { - if (!_disposedValue) + if (disposing && Interlocked.Increment(ref _disposeCount) == 1) { - if (disposing) - { - _queueCreation.Dispose(); - } - _disposedValue = true; + _queueCreation.Dispose(); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs` around lines 83 - 100, The disposal guard in PostgreSQLJobQueueCreation.Dispose(bool) is non-atomic, so concurrent Dispose calls can both enter the cleanup path and double-dispose _queueCreation. Replace the plain _disposedValue check with an Interlocked-based thread-safe disposal pattern, mirroring the approach used in CreationScope, and keep GC.SuppressFinalize in Dispose() after the atomic guard ensures cleanup runs only once.Source: Coding guidelines
🧹 Nitpick comments (1)
Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs (1)
212-222: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo disposal guard on
ActivitySourceWrapper.Dispose().Unlike other disposal classes in this PR, this method has no guard against repeated calls; a second
Dispose()would re-invoke_provider.ForceFlush(2000)/Dispose(). As per coding guidelines, "UseInterlocked-based disposal patterns for thread-safe disposal throughout the codebase."🔒️ Proposed fix using an Interlocked guard
+ private int _disposeCount; + public void Dispose() { - _listener?.Dispose(); - Source?.Dispose(); - if (_provider != null) - { - _provider.ForceFlush(2000); - _provider.Dispose(); - } + if (Interlocked.Increment(ref _disposeCount) == 1) + { + _listener?.Dispose(); + Source?.Dispose(); + if (_provider != null) + { + _provider.ForceFlush(2000); + _provider.Dispose(); + } + } GC.SuppressFinalize(this); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs` around lines 212 - 222, The ActivitySourceWrapper.Dispose() method currently lacks a disposal guard, so repeated calls can re-run _provider.ForceFlush(2000) and _provider.Dispose(). Update Dispose() to use the same Interlocked-based, thread-safe disposal pattern used elsewhere in the codebase, and gate the cleanup so the body runs only once while still disposing _listener, Source, and _provider safely.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.cs`:
- Line 1: The GenerateQueueName.cs file is missing the required LGPL-2.1 license
header, so add the standard license comment block at the very top of the file
before the existing `#region` Using section. Follow the same header format used in
the other GenerateQueueName.cs files in the project to keep the file consistent
with the licensing guideline.
In
`@Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.cs`:
- Line 1: The file starts with `#region` Using but is missing the required
LGPL-2.1 license header. Add the standard LGPL-2.1 header at the top of
GenerateQueueName.cs before any regions or using directives, matching the
project's source file licensing convention.
---
Outside diff comments:
In
`@Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.cs`:
- Around line 83-100: The disposal guard in
PostgreSQLJobQueueCreation.Dispose(bool) is non-atomic, so concurrent Dispose
calls can both enter the cleanup path and double-dispose _queueCreation. Replace
the plain _disposedValue check with an Interlocked-based thread-safe disposal
pattern, mirroring the approach used in CreationScope, and keep
GC.SuppressFinalize in Dispose() after the atomic guard ensures cleanup runs
only once.
---
Nitpick comments:
In `@Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.cs`:
- Around line 212-222: The ActivitySourceWrapper.Dispose() method currently
lacks a disposal guard, so repeated calls can re-run _provider.ForceFlush(2000)
and _provider.Dispose(). Update Dispose() to use the same Interlocked-based,
thread-safe disposal pattern used elsewhere in the codebase, and gate the
cleanup so the body runs only once while still disposing _listener, Source, and
_provider safely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45ca590e-ebb5-4fb0-971c-d1a50693b999
📒 Files selected for processing (67)
Source/DotNetWorkQueue.IntegrationTests.Shared/SharedSetup.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.csSource/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnection.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbJobQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbMessageQueueCreation.csSource/DotNetWorkQueue.Transport.LiteDB/LiteDbConnectionInformation.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/IntegrationConnectionInfo.csSource/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLJobQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/Basic/PostgreSQLMessageQueueCreation.csSource/DotNetWorkQueue.Transport.PostgreSQL/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.PostgreSQL/SQLConnectionInformation.csSource/DotNetWorkQueue.Transport.Redis.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.csSource/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueWorkSub.csSource/DotNetWorkQueue.Transport.Redis/RedisConnection.csSource/DotNetWorkQueue.Transport.Redis/RedisConnectionInfo.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/ConnectionString.csSource/DotNetWorkQueue.Transport.SQLite.Integration.Tests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SQLite/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SQLite/Basic/CreationScope.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.csSource/DotNetWorkQueue.Transport.SQLite/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindExpiredRecordsToDeleteQueryHandlerErrorDecorator.csSource/DotNetWorkQueue.Transport.SQLite/Decorator/FindRecordsToResetByHeartBeatDecorator.csSource/DotNetWorkQueue.Transport.SQLite/SqliteConnectionInformation.csSource/DotNetWorkQueue.Transport.SqlServer.IntegrationTests/GenerateQueueName.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/BuildMoveToErrorQueueSql.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/CommandHandler/SendMessage.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.csSource/DotNetWorkQueue.Transport.SqlServer/ConfigurationExtensions.csSource/DotNetWorkQueue.Transport.SqlServer/SQLConnectionInformation.csSource/DotNetWorkQueue/Admin/AdminApi.csSource/DotNetWorkQueue/BaseContainer.csSource/DotNetWorkQueue/IoC/ContainerWrapper.csSource/DotNetWorkQueue/Messages/MessageContext.csSource/DotNetWorkQueue/Messages/MessageMethodHandling.csSource/DotNetWorkQueue/Queue/BaseMonitor.csSource/DotNetWorkQueue/Queue/BaseQueue.csSource/DotNetWorkQueue/Queue/HeartBeatWorker.csSource/DotNetWorkQueue/Queue/ProducerQueue.csSource/DotNetWorkQueue/Queue/QueueCancelWork.csSource/DotNetWorkQueue/Queue/QueueMonitor.csSource/DotNetWorkQueue/Queue/WaitForEventOrCancel.csSource/DotNetWorkQueue/Queue/WorkerBase.csSource/DotNetWorkQueue/Queue/WorkerWaitForEventOrCancel.csSource/DotNetWorkQueue/TaskScheduling/Scheduler.csSource/DotNetWorkQueue/TaskScheduling/TaskScheduler.csSource/DotNetWorkQueue/TaskScheduling/WaitForEventOrCancelThreadPool.csSource/DotNetWorkQueue/Trace/Decorator/MessageMethodHandlingDecorator.csSource/DotNetWorkQueue/Trace/Decorator/ProducerMethodJobQueueDecorator.csSource/DotNetWorkQueue/Transport/Memory/Basic/MessageQueueCreation.csSource/DotNetWorkQueue/Transport/Memory/ConnectionInformation.cs
✅ Files skipped from review due to trivial changes (5)
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SQLite/Decorator/FindExpiredRecordsToDeleteQueryHandlerErrorDecorator.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/CommandHandler/SendMessage.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/CommandHandler/SendMessage.cs
🚧 Files skipped from review as they are similar to previous changes (50)
- Source/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/IntegrationConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/ConnectionString.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnection.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/SqlServerJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/GenerateQueueName.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/SqliteJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/CreationScope.cs
- Source/DotNetWorkQueue/Trace/Decorator/MessageMethodHandlingDecorator.cs
- Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/IntegrationConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.Redis/RedisConnectionInfo.cs
- Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/ConnectionString.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/SQLConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbConnectionManager.cs
- Source/DotNetWorkQueue.Transport.Memory.Linq.Integration.Tests/GenerateQueueName.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisJobQueueCreation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/BuildMoveToErrorQueueSql.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/CommandHandler/SendMessage.cs
- Source/DotNetWorkQueue/Transport/Memory/ConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/ConfigurationExtensions.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/SQLServerMessageQueueCreation.cs
- Source/DotNetWorkQueue.Transport.SQLite/Basic/SqLiteMessageQueueCreation.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueMonitor.cs
- Source/DotNetWorkQueue/Queue/QueueCancelWork.cs
- Source/DotNetWorkQueue.Transport.Redis/RedisConnection.cs
- Source/DotNetWorkQueue.Transport.Redis/Basic/RedisQueueWorkSub.cs
- Source/DotNetWorkQueue/Queue/BaseMonitor.cs
- Source/DotNetWorkQueue/Queue/WaitForEventOrCancel.cs
- Source/DotNetWorkQueue/Messages/MessageMethodHandling.cs
- Source/DotNetWorkQueue/IoC/ContainerWrapper.cs
- Source/DotNetWorkQueue/Queue/BaseQueue.cs
- Source/DotNetWorkQueue/Queue/ProducerQueue.cs
- Source/DotNetWorkQueue/Messages/MessageContext.cs
- Source/DotNetWorkQueue.Transport.SQLite/ConfigurationExtensions.cs
- Source/DotNetWorkQueue/TaskScheduling/WaitForEventOrCancelThreadPool.cs
- Source/DotNetWorkQueue/Transport/Memory/Basic/MessageQueueCreation.cs
- Source/DotNetWorkQueue/TaskScheduling/TaskScheduler.cs
- Source/DotNetWorkQueue/Queue/WorkerWaitForEventOrCancel.cs
- Source/DotNetWorkQueue.Transport.SqlServer/Basic/ConnectionHolder.cs
- Source/DotNetWorkQueue.Transport.SQLite/Decorator/FindRecordsToResetByHeartBeatDecorator.cs
- Source/DotNetWorkQueue.Transport.PostgreSQL/Basic/ConnectionHolder.cs
- Source/DotNetWorkQueue.Transport.SqlServer/SQLConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/LiteDbConnectionInformation.cs
- Source/DotNetWorkQueue/BaseContainer.cs
- Source/DotNetWorkQueue/Admin/AdminApi.cs
- Source/DotNetWorkQueue/TaskScheduling/Scheduler.cs
- Source/DotNetWorkQueue/Queue/HeartBeatWorker.cs
- Source/DotNetWorkQueue.Transport.SQLite/SqliteConnectionInformation.cs
- Source/DotNetWorkQueue.Transport.LiteDB/Basic/LiteDbMessageQueueCreation.cs
- Source/DotNetWorkQueue/Queue/QueueMonitor.cs
🛑 Comments failed to post (2)
Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.cs (1)
1-1: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing required LGPL-2.1 license header.
The file starts directly at
#region Usingwith no license header. The same issue was flagged in the siblingGenerateQueueName.csfiles. As per coding guidelines, "All source files must include LGPL-2.1 license headers."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.LiteDB.IntegrationTests/GenerateQueueName.cs` at line 1, The GenerateQueueName.cs file is missing the required LGPL-2.1 license header, so add the standard license comment block at the very top of the file before the existing `#region` Using section. Follow the same header format used in the other GenerateQueueName.cs files in the project to keep the file consistent with the licensing guideline.Source: Coding guidelines
Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.cs (1)
1-1: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Missing LGPL-2.1 license header.
File begins directly with
#region Using, with no license header. As per coding guidelines,**/*.cs: All source files must include LGPL-2.1 license headers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Source/DotNetWorkQueue.Transport.Memory.Integration.Tests/GenerateQueueName.cs` at line 1, The file starts with `#region` Using but is missing the required LGPL-2.1 license header. Add the standard LGPL-2.1 header at the top of GenerateQueueName.cs before any regions or using directives, matching the project's source file licensing convention.Source: Coding guidelines
…tionScope CodeRabbit: the CA1816-touched classes still used the legacy _disposedValue bool guard (races on concurrent Dispose). Align 7 of them with the codebase's Interlocked disposal convention: _disposeCount + Interlocked.Increment guard. LiteDbConnectionManager intentionally NOT converted — its _disposedValue is an ownership-conditional connection-state flag (set only when it owns the connection, read in GetDatabase, lock-protected), not a naive dispose-once guard; forcing Interlocked would break connection sharing. Transport unit tests pass; production builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #186 +/- ##
==========================================
+ Coverage 87.48% 87.57% +0.09%
==========================================
Files 1023 1023
Lines 33787 33709 -78
Branches 2841 2803 -38
==========================================
- Hits 29557 29522 -35
+ Misses 3350 3322 -28
+ Partials 880 865 -15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Hand-fixes the 158 mechanical Roslyn (
CA*/SYSLIB*) code smells SonarCloud flags — the "Bucket A" tranche of the code-smell badge reduction. These are behavior-preserving modernizations of production code (plus some test/integration-shared code). Applied by hand because no bulk auto-fixer works on this net10 codebase:dotnet formatwon't apply editorconfig-elevated default-off CA rules, and Roslynator 0.12 can't load the net10 SDK analyzers (Roslyn 5.0 vs its 4.14) — and the CA code-fixers aren't packaged standalone anyway. (dotnet formatdid work for the earlier enabled-by-default MSTEST0037 sweep, PR #184.)Rules fixed (per-commit)
StringBuilder.Append(char)for single-char stringsObjectDisposedException.ThrowIf— converted theThrowIfDisposed([CallerMemberName])idiom, dropped the now-unused param + 29 orphanedusing System.Runtime.CompilerServicesGC.SuppressFinalize(this)inDispose/DisposeAsyncTryGetValueoverContainsKey+indexer (get-or-add, Metrics, SQL config read paths only; write paths untouched)ArgumentNullException.ThrowIfNull[GeneratedRegex]source generation (6 prodValidQueueNamePattern, Slugify, E2E)Convert.ToHexString/ staticMD5.HashDatacancellationToken,string.Contains, extract const arrays,TryAdd,Array.Empty, etc.Notable decisions
ObjectDisposedException.ObjectNamefrom the accessed member name to the type name. Verified no test depends on the ObjectName (all assert only the exception type); disposed-guard tests pass. This is the only intentional behavior change in the PR.Validation
IntegrationTests.Sharedhelpers on the integration hot path)..gitattributes).Behavior-preserving except CA1513's ObjectName; no version bump (no public API or functional change).
🤖 Generated with Claude Code
Summary by CodeRabbit