Skip to content

Commit 203eff3

Browse files
authored
Merge pull request #156 from blehnen/fix/155-npgsql-enum-coercion
fix(postgresql): cast QueueStatusAdmin enum to int for Npgsql 10.x (#155)
2 parents 7bdc7e8 + 28b0c8f commit 203eff3

9 files changed

Lines changed: 126 additions & 10 deletions

File tree

Source/DotNetWorkQueue.IntegrationTests.Shared/Admin/AdminSharedConsumer.cs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public void RunConsumer<TTransportInit>(QueueConnection queueConnection, bool ad
1414
int runTime, int messageCount,
1515
int workerCount, int timeOut,
1616
TimeSpan heartBeatTime, TimeSpan heartBeatMonitorTime, string updateTime, bool enableChaos,
17-
ICreationScope scope)
17+
ICreationScope scope, bool enableStatus = false)
1818
where TTransportInit : ITransportInit, new()
1919
{
2020

@@ -50,22 +50,33 @@ public void RunConsumer<TTransportInit>(QueueConnection queueConnection, bool ad
5050
var count = admin.Count(null);
5151
Assert.AreEqual(messageCount, count);
5252

53+
//Deterministic status-filtered count check. Reproduces issue #155: under Npgsql 10.x
54+
//the QueueStatusAdmin enum was bound to an integer parameter without a cast, throwing
55+
//InvalidCastException before the fix. All messages are produced and waiting before the
56+
//consumer starts, so these counts are timing-independent. Only valid when the status
57+
//table is enabled (otherwise the count query ignores the filter and returns the total).
58+
if (enableStatus)
59+
{
60+
Assert.AreEqual(messageCount, admin.Count(QueueStatusAdmin.Waiting));
61+
Assert.AreEqual(0, admin.Count(QueueStatusAdmin.Processing));
62+
}
63+
5364
//start looking for work
5465
queue.Start<TMessage>((message, notifications) =>
5566
{
5667
MessageHandlingShared.HandleFakeMessages(message, runTime, processedCount, messageCount,
5768
waitForFinish);
5869
}, CreateNotifications.Create(logProvider));
5970

60-
if (messageCount <= workerCount && runTime > 10)
71+
if (enableStatus && messageCount <= workerCount && runTime > 10)
6172
{
6273
Thread.Sleep(runTime / 2);
6374
var working = admin.Count(QueueStatusAdmin.Processing);
6475
var waiting = admin.Count(QueueStatusAdmin.Waiting);
6576
Assert.AreEqual(0, waiting);
6677
Assert.AreEqual(messageCount, working);
6778
}
68-
else if (runTime > 10)
79+
else if (enableStatus && runTime > 10)
6980
{
7081
Thread.Sleep(runTime / 2);
7182
var working = admin.Count(QueueStatusAdmin.Processing);

Source/DotNetWorkQueue.IntegrationTests.Shared/Admin/Implementation/SimpleConsumerAdmin.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ public void Run<TTransportInit, TMessage, TTransportCreate>(
1919
Action<TTransportCreate> setOptions,
2020
Func<QueueProducerConfiguration, AdditionalMessageData> generateData,
2121
Action<QueueConnection, QueueProducerConfiguration, long, ICreationScope> verify,
22-
Action<QueueConnection, IBaseTransportOptions, ICreationScope, int, bool, bool> verifyQueueCount)
22+
Action<QueueConnection, IBaseTransportOptions, ICreationScope, int, bool, bool> verifyQueueCount,
23+
bool enableStatus = false)
2324
where TTransportInit : ITransportInit, new()
2425
where TMessage : class
2526
where TTransportCreate : class, IQueueCreation
@@ -63,7 +64,8 @@ public void Run<TTransportInit, TMessage, TTransportCreate>(
6364
logProvider,
6465
runtime, messageCount,
6566
workerCount, timeOut,
66-
TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(35), "*/10 * * * * *", enableChaos, scope);
67+
TimeSpan.FromSeconds(30), TimeSpan.FromSeconds(35), "*/10 * * * * *", enableChaos, scope,
68+
enableStatus);
6769

6870
verifyQueueCount(queueConnection, oCreation.BaseTransportOptions, scope, 0, false,
6971
false);

Source/DotNetWorkQueue.Transport.PostgreSQL.Integration.Tests/Admin/SimpleConsumer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ public void Run(int messageCount, int runtime, int timeOut, int workerCount, boo
2222
messageCount, runtime, timeOut, workerCount, false, x => Helpers.SetOptions(x,
2323
true, !useTransactions, useTransactions, false,
2424
false, !useTransactions, true, false),
25-
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount);
25+
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount,
26+
enableStatus: !useTransactions);
2627
}
2728
}
2829
}

Source/DotNetWorkQueue.Transport.RelationalDatabase.Tests/Basic/QueryPrepareHandler/FakeCommandStringCache.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ protected override void BuildCommands()
1515
CommandCache[CommandStringTypes.GetDashboardStatusCounts] = "SELECT status counts";
1616
CommandCache[CommandStringTypes.GetDashboardMessages] = "SELECT messages{0} ORDER BY QueuedDateTime DESC";
1717
CommandCache[CommandStringTypes.GetDashboardMessageCount] = "SELECT COUNT(*) FROM meta";
18+
CommandCache[CommandStringTypes.GetQueueCountAll] = "SELECT COUNT(*) FROM meta";
19+
CommandCache[CommandStringTypes.GetQueueCountStatus] = "select count(*) from meta where status = @status";
1820
CommandCache[CommandStringTypes.GetDashboardMessageDetail] = "SELECT detail{0} WHERE QueueID = @QueueId";
1921
CommandCache[CommandStringTypes.GetDashboardStaleMessages] = "SELECT stale{0}";
2022
CommandCache[CommandStringTypes.GetDashboardErrorMessages] = "SELECT errors";
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// ---------------------------------------------------------------------
2+
//This file is part of DotNetWorkQueue
3+
//Copyright © 2015-2026 Brian Lehnen
4+
//
5+
//This library is free software; you can redistribute it and/or
6+
//modify it under the terms of the GNU Lesser General Public
7+
//License as published by the Free Software Foundation; either
8+
//version 2.1 of the License, or (at your option) any later version.
9+
//
10+
//This library is distributed in the hope that it will be useful,
11+
//but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
//MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
13+
//Lesser General Public License for more details.
14+
//
15+
//You should have received a copy of the GNU Lesser General Public
16+
//License along with this library; if not, write to the Free Software
17+
//Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
18+
// ---------------------------------------------------------------------
19+
using System.Data;
20+
using System.Linq;
21+
using DotNetWorkQueue.Transport.RelationalDatabase;
22+
using DotNetWorkQueue.Transport.RelationalDatabase.Basic;
23+
using DotNetWorkQueue.Transport.RelationalDatabase.Basic.Query;
24+
using DotNetWorkQueue.Transport.RelationalDatabase.Basic.QueryPrepareHandler;
25+
using NSubstitute;
26+
using Microsoft.VisualStudio.TestTools.UnitTesting;
27+
28+
namespace DotNetWorkQueue.Transport.RelationalDatabase.Tests.Basic.QueryPrepareHandler
29+
{
30+
[TestClass]
31+
public class GetQueueCountQueryPrepareHandlerTests
32+
{
33+
[TestMethod]
34+
public void Handle_With_Status_And_StatusEnabled_Binds_Int_Not_Enum()
35+
{
36+
// Regression guard for issue #155: the QueueStatusAdmin enum must be cast to its underlying
37+
// numeric type before being assigned to the @Status parameter. Npgsql 10.x throws
38+
// InvalidCastException when a raw CLR enum is bound to an integer parameter.
39+
var handler = new GetQueueCountQueryPrepareHandler(new FakeCommandStringCache(), CreateOptions(enableStatus: true));
40+
var command = CreateDbCommand();
41+
42+
handler.Handle(new GetQueueCountQuery("conn", QueueStatusAdmin.Waiting), command, CommandStringTypes.GetQueueCountStatus);
43+
44+
var parameters = (DataParameterCollection)command.Parameters;
45+
// Parameter name must match the SQL placeholder casing exactly. System.Data.SQLite binds
46+
// parameters case-sensitively, so "@Status" against a "@status" placeholder throws
47+
// "Insufficient parameters supplied to the command" (issue #155 follow-up).
48+
Assert.AreEqual(1, parameters.Count);
49+
var status = parameters.First();
50+
Assert.AreEqual("@status", status.ParameterName);
51+
Assert.AreEqual(DbType.Int32, status.DbType);
52+
Assert.IsInstanceOfType(status.Value, typeof(int), "@status must be bound as an int, not the QueueStatusAdmin enum");
53+
Assert.AreEqual((int)QueueStatusAdmin.Waiting, status.Value);
54+
}
55+
56+
[TestMethod]
57+
public void Handle_With_Status_Uses_StatusFiltered_Command()
58+
{
59+
var handler = new GetQueueCountQueryPrepareHandler(new FakeCommandStringCache(), CreateOptions(enableStatus: true));
60+
var command = CreateDbCommand();
61+
62+
handler.Handle(new GetQueueCountQuery("conn", QueueStatusAdmin.Processing), command, CommandStringTypes.GetQueueCountStatus);
63+
64+
StringAssert.Contains(command.CommandText, "@status");
65+
}
66+
67+
[TestMethod]
68+
public void Handle_When_StatusDisabled_Adds_No_Parameter()
69+
{
70+
// When the status table is disabled the handler ignores the filter and counts all rows.
71+
var handler = new GetQueueCountQueryPrepareHandler(new FakeCommandStringCache(), CreateOptions(enableStatus: false));
72+
var command = CreateDbCommand();
73+
74+
handler.Handle(new GetQueueCountQuery("conn", QueueStatusAdmin.Waiting), command, CommandStringTypes.GetQueueCountStatus);
75+
76+
Assert.IsEmpty((DataParameterCollection)command.Parameters);
77+
}
78+
79+
private static ITransportOptionsFactory CreateOptions(bool enableStatus)
80+
{
81+
var options = Substitute.For<ITransportOptions>();
82+
options.EnableStatus.Returns(enableStatus);
83+
var factory = Substitute.For<ITransportOptionsFactory>();
84+
factory.Create().Returns(options);
85+
return factory;
86+
}
87+
88+
private static IDbCommand CreateDbCommand()
89+
{
90+
var command = Substitute.For<IDbCommand>();
91+
var parameters = new DataParameterCollection();
92+
command.Parameters.Returns(parameters);
93+
command.CreateParameter().Returns(_ => Substitute.For<IDbDataParameter>());
94+
return command;
95+
}
96+
}
97+
}

Source/DotNetWorkQueue.Transport.RelationalDatabase/Basic/QueryPrepareHandler/GetQueueCountQueryPrepareHandler.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ public void Handle(GetQueueCountQuery query, IDbCommand dbCommand, CommandString
4848
{
4949
dbCommand.CommandText = _commandCache.GetCommand(CommandStringTypes.GetQueueCountStatus);
5050
var status = dbCommand.CreateParameter();
51-
status.ParameterName = "@Status";
51+
status.ParameterName = "@status";
5252
status.DbType = DbType.Int32;
53-
status.Value = query.Status.Value;
53+
status.Value = (int)query.Status.Value;
5454
dbCommand.Parameters.Add(status);
5555
}
5656
else

Source/DotNetWorkQueue.Transport.SQLite.Integration.Tests/Admin/SimpleConsumer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ public void Run(int messageCount, int runtime, int timeOut, int workerCount, boo
2121
messageCount, runtime, timeOut, workerCount, false, x => Helpers.SetOptions(x,
2222
true, true, false,
2323
false, true, true, false),
24-
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount);
24+
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount,
25+
enableStatus: true);
2526
}
2627
}
2728
}

Source/DotNetWorkQueue.Transport.SqlServer.IntegrationTests/Admin/SimpleConsumer.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ public void Run(int messageCount, int runtime, int timeOut, int workerCount, boo
2121
false, !useTransactions, useTransactions,
2222
false,
2323
false, !useTransactions, true, false),
24-
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount);
24+
Helpers.GenerateData, Helpers.Verify, Helpers.VerifyQueueCount,
25+
enableStatus: !useTransactions);
2526
}
2627
}
2728
}

docs/lessons-learned.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,4 @@ Historical engineering notes from DotNetWorkQueue development. Moved here from `
5959
- **Phase scope reframing — RESEARCH should validate the phase isn't already done.** ROADMAP Phase 7 framed as "add XML doc comments to phases 2-4 public types." RESEARCH §1 surfaced that the builders had already added docs as they went. Phase 7 reframed to a VERIFICATION pass + csproj gate fixes (net8 `<DocumentationFile>` gap + ISSUE-032 NU1902 closure on Transport.SQLite). Pattern: when a phase title implies authoring, researcher should explicitly confirm the work was not already incidentally done before architect plans new authoring work.
6060
- **`<WarningsNotAsErrors>` pattern for accepted advisory carry-forward.** ISSUE-032 (OpenTelemetry NU1902 advisory) blocked the strictest `dotnet build -c Release -p:CI=true` build path on `Transport.SQLite`. Phase 7 PLAN-1.1 / commit `88ff8996` added `<WarningsNotAsErrors>NU1902</WarningsNotAsErrors>` to all 3 Release PropertyGroup blocks (`Release|net10.0`, `Release|net8.0`, `Release|AnyCPU`). The advisory still surfaces as a visible warning on every Release build (long-term remediation isn't forgotten) but the build is no longer blocked. Reusable pattern when a ship-blocking advisory is out of scope for the current milestone.
6161
- **SQLite + hold-transaction inbox pattern: structurally incompatible.** Inbox milestone (PR #143, issue #149) attempted to add `IRelationalWorkerNotification` to the SQLite transport alongside SqlServer + PostgreSQL. Failed on Jenkins across 14 CI runs in two recurring shapes: (1) `Sync_Commit`/`Async_Commit` NRE on `relational.Transaction` despite cast succeeding, (2) `Sync_Rollback`/`Async_Rollback`/`BusinessRow_*Visibility` handler-never-invoked 30s timeouts. Root cause is NOT a code bug — SQLite uses `BEGIN EXCLUSIVE`-on-write semantics. Holding the dequeue transaction for the duration of the user handler blocks ALL other writers on the queue table, including the worker thread's own next dequeue attempt → deadlock. `EnableHoldTransactionUntilMessageCommitted` has always been a no-op on SQLite for this reason. **Inbox pattern is a permanent non-goal for SQLite; outbox is viable with a documented concurrency caveat.** Treat the existing options-property as obsolete on SQLite (issue #149 tracks the cleanup). Don't re-attempt the four debugging directions tried in PR #143: lazy-options removal, property-injection, static `AsyncLocal<>`, factory-delegate wiring — none address the structural lock.
62+
- **System.Data.SQLite binds parameter names CASE-SENSITIVELY; Npgsql and Microsoft.Data.SqlClient do not.** Issue #155 (PR #156) — the shared `GetQueueCountQueryPrepareHandler` bound the status filter as `"@Status"` while all three transport SQL texts use lowercase `"@status"` (`... where status = @status`). PostgreSQL and SQL Server worked because their drivers match parameter names case-insensitively; System.Data.SQLite threw `SQLiteException: unknown error — Insufficient parameters supplied to the command` because it could not find a parameter named `@status`. The mismatch had existed indefinitely but was dormant: the status-filtered admin `Count` path was dead (no test ever enabled status on it), so it only surfaced once the #155 regression test exercised it. Fix (commit `3b9f3cb1`): align the `ParameterName` to the SQL placeholder's exact casing. Pattern: any handler that creates a parameter MUST match the SQL placeholder's case verbatim — assert the literal parameter name in unit tests (the `FakeCommandStringCache` mirrors the real lowercase SQL so the guard is meaningful), and never rely on driver-level case folding. Sibling lesson to the enum→int cast (issue #155, commit `c8093b65`): both bugs lived in the same dead code path, which is why repairing the test guard was as important as the fixes.

0 commit comments

Comments
 (0)