From c6f85322afdc366eb3956b2eee283e3d892aac4f Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Sun, 2 Aug 2026 16:00:11 -0500 Subject: [PATCH] fix(sqlite,mysql): implement the non-generic Weasel.Core.ICommandBuilder (weasel#423) Weasel.Sqlite.CommandBuilder and Weasel.MySql.CommandBuilder derived from CommandBuilderBase<,,> but never declared the non-generic Weasel.Core.ICommandBuilder, so nothing targeting the neutral contract could be handed one. That contract is what every Weasel.Storage closed-shape operation configures itself against: void ConfigureCommand(ICommandBuilder builder, IStorageSession session); so the practical effect was that no Weasel.Storage document or event operation could execute against SQLite at all -- there was no way to construct the builder argument. Found while building Fisher, the SQLite event store, on Weasel.Storage. Postgresql, SqlServer and Oracle already carry the interface; Sqlite and MySql were the two outliers left behind by #327. Four members were missing on each: TenantId, AppendParameters(params object[]), a DbParameter-returning AppendParameter(object) (the inherited overloads all return void, so none satisfied the interface), and CreateGroupedParameterBuilder. Everything else the interface needs was already inherited. AppendParameter and AppendParameters are implemented explicitly, following Weasel.Oracle rather than Weasel.SqlServer: the base class already exposes void-returning AppendParameter overloads, and a public member here would hide them and silently change which overload existing call sites bind to. StartNewCommand is deliberately not overridden. The base is already a no-op, which is correct for both providers, so an override would be pure noise that implies a difference that does not exist. Adds a contract test to each provider's suite so the set cannot drift again. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DLKYzKRqfvk2vS88kfcvqc --- .../CommandBuilderNeutralContractTests.cs | 69 +++++++++++++++++++ src/Weasel.MySql/CommandBuilder.cs | 43 +++++++++++- .../CommandBuilderTests.cs | 64 +++++++++++++++++ src/Weasel.Sqlite/CommandBuilder.cs | 47 ++++++++++++- 4 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 src/Weasel.MySql.Tests/CommandBuilderNeutralContractTests.cs diff --git a/src/Weasel.MySql.Tests/CommandBuilderNeutralContractTests.cs b/src/Weasel.MySql.Tests/CommandBuilderNeutralContractTests.cs new file mode 100644 index 00000000..1ed0da86 --- /dev/null +++ b/src/Weasel.MySql.Tests/CommandBuilderNeutralContractTests.cs @@ -0,0 +1,69 @@ +using Shouldly; +using Xunit; + +namespace Weasel.MySql.Tests; + +/// +/// weasel#423: Weasel.MySql.CommandBuilder shipped without the non-generic +/// Weasel.Core.ICommandBuilder, which is the surface every Weasel.Storage closed-shape operation +/// configures itself against. SQLite was the provider where this actually blocked a consumer, but +/// MySql had the identical gap and would have hit it the moment a Weasel.Storage consumer targeted +/// it. These guard the members the base class cannot supply on its own. +/// +public class CommandBuilderNeutralContractTests +{ + [Fact] + public void implements_the_dialect_neutral_command_builder() + { + typeof(Weasel.Core.ICommandBuilder).IsAssignableFrom(typeof(CommandBuilder)).ShouldBeTrue(); + } + + [Fact] + public void append_parameter_returns_the_created_parameter() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.Append("select * from foo where bar = "); + var parameter = builder.AppendParameter("baz"); + + parameter.ShouldNotBeNull(); + parameter.Value.ShouldBe("baz"); + builder.ToString().ShouldBe("select * from foo where bar = @p0"); + } + + [Fact] + public void append_parameters_writes_each_value_comma_separated() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.Append("select * from foo where bar in ("); + builder.AppendParameters("one", "two", "three"); + builder.Append(")"); + + builder.ToString().ShouldBe("select * from foo where bar in (@p0, @p1, @p2)"); + } + + [Fact] + public void append_parameters_rejects_an_empty_set() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + Should.Throw(() => builder.AppendParameters()); + } + + [Fact] + public void creates_a_grouped_parameter_builder() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.CreateGroupedParameterBuilder().ShouldNotBeNull(); + } + + [Fact] + public void carries_a_tenant_id() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder { TenantId = "tenant-a" }; + + builder.TenantId.ShouldBe("tenant-a"); + } +} diff --git a/src/Weasel.MySql/CommandBuilder.cs b/src/Weasel.MySql/CommandBuilder.cs index b12913d2..969712ae 100644 --- a/src/Weasel.MySql/CommandBuilder.cs +++ b/src/Weasel.MySql/CommandBuilder.cs @@ -4,7 +4,7 @@ namespace Weasel.MySql; -public class CommandBuilder: CommandBuilderBase +public class CommandBuilder: CommandBuilderBase, ICommandBuilder { public CommandBuilder(): this(new MySqlCommand()) { @@ -13,6 +13,47 @@ public CommandBuilder(): this(new MySqlCommand()) public CommandBuilder(MySqlCommand command): base(MySqlProvider.Instance, '@', command) { } + + /// + /// It became so common, that it's turned out to be convenient to place + /// this here + /// + public string TenantId { get; set; } = string.Empty; + + /// + /// Append a single parameter through the dialect-neutral value path, returning the newly created + /// parameter upcast to . + /// + /// Explicitly implemented, as in Weasel.Oracle: the base class already exposes void-returning + /// AppendParameter overloads, so a public member here would hide them and silently change + /// which one existing call sites bind to. + /// + /// + DbParameter ICommandBuilder.AppendParameter(object value) + { + base.AppendParameter(value); + return _command.Parameters[^1]; + } + + void ICommandBuilder.AppendParameters(params object[] parameters) + { + if (parameters.Length == 0) + throw new ArgumentOutOfRangeException(nameof(parameters), + "Must be at least one parameter value, but got " + parameters.Length); + + AppendParameter(parameters[0]); + + for (var i = 1; i < parameters.Length; i++) + { + Append(", "); + AppendParameter(parameters[i]); + } + } + + public IGroupedParameterBuilder CreateGroupedParameterBuilder(char? seperator = null) + { + return new GroupedParameterBuilder(this, seperator); + } } public static class CommandBuilderExtensions diff --git a/src/Weasel.Sqlite.Tests/CommandBuilderTests.cs b/src/Weasel.Sqlite.Tests/CommandBuilderTests.cs index 8e6d1030..6f394b79 100644 --- a/src/Weasel.Sqlite.Tests/CommandBuilderTests.cs +++ b/src/Weasel.Sqlite.Tests/CommandBuilderTests.cs @@ -116,3 +116,67 @@ public void append_multiple_times() builder.ToString().ShouldBe("SELECT * FROM users"); } } + +/// +/// weasel#423: Weasel.Sqlite.CommandBuilder shipped without the non-generic +/// Weasel.Core.ICommandBuilder, which is the surface every Weasel.Storage closed-shape operation +/// configures itself against — so no document or event operation could execute against SQLite at +/// all. These guard the members the base class cannot supply on its own. +/// +public class CommandBuilderNeutralContractTests +{ + [Fact] + public void implements_the_dialect_neutral_command_builder() + { + typeof(Weasel.Core.ICommandBuilder).IsAssignableFrom(typeof(CommandBuilder)).ShouldBeTrue(); + } + + [Fact] + public void append_parameter_returns_the_created_parameter() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.Append("select * from foo where bar = "); + var parameter = builder.AppendParameter("baz"); + + parameter.ShouldNotBeNull(); + parameter.Value.ShouldBe("baz"); + builder.ToString().ShouldBe("select * from foo where bar = @p0"); + } + + [Fact] + public void append_parameters_writes_each_value_comma_separated() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.Append("select * from foo where bar in ("); + builder.AppendParameters("one", "two", "three"); + builder.Append(")"); + + builder.ToString().ShouldBe("select * from foo where bar in (@p0, @p1, @p2)"); + } + + [Fact] + public void append_parameters_rejects_an_empty_set() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + Should.Throw(() => builder.AppendParameters()); + } + + [Fact] + public void creates_a_grouped_parameter_builder() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder(); + + builder.CreateGroupedParameterBuilder().ShouldNotBeNull(); + } + + [Fact] + public void carries_a_tenant_id() + { + Weasel.Core.ICommandBuilder builder = new CommandBuilder { TenantId = "tenant-a" }; + + builder.TenantId.ShouldBe("tenant-a"); + } +} diff --git a/src/Weasel.Sqlite/CommandBuilder.cs b/src/Weasel.Sqlite/CommandBuilder.cs index 90f91531..e389f558 100644 --- a/src/Weasel.Sqlite/CommandBuilder.cs +++ b/src/Weasel.Sqlite/CommandBuilder.cs @@ -4,7 +4,7 @@ namespace Weasel.Sqlite; -public class CommandBuilder: CommandBuilderBase +public class CommandBuilder: CommandBuilderBase, ICommandBuilder { public CommandBuilder(): this(new SqliteCommand()) { @@ -13,6 +13,51 @@ public CommandBuilder(): this(new SqliteCommand()) public CommandBuilder(SqliteCommand command): base(SqliteProvider.Instance, '@', command) { } + + /// + /// It became so common, that it's turned out to be convenient to place + /// this here + /// + public string TenantId { get; set; } = string.Empty; + + /// + /// Append a single parameter through the dialect-neutral value path, returning the newly created + /// parameter upcast to . + /// + /// Explicitly implemented, as in Weasel.Oracle: the base class already exposes void-returning + /// AppendParameter overloads, so a public member here would hide them and silently change + /// which one existing call sites bind to. + /// + /// + DbParameter ICommandBuilder.AppendParameter(object value) + { + base.AppendParameter(value); + return _command.Parameters[^1]; + } + + void ICommandBuilder.AppendParameters(params object[] parameters) + { + if (parameters.Length == 0) + throw new ArgumentOutOfRangeException(nameof(parameters), + "Must be at least one parameter value, but got " + parameters.Length); + + AppendParameter(parameters[0]); + + for (var i = 1; i < parameters.Length; i++) + { + Append(", "); + AppendParameter(parameters[i]); + } + } + + public IGroupedParameterBuilder CreateGroupedParameterBuilder(char? seperator = null) + { + return new GroupedParameterBuilder(this, seperator); + } + + // StartNewCommand is deliberately not overridden: the base is already a no-op, which is correct + // here because Microsoft.Data.Sqlite executes several semicolon-separated statements from one + // command. } public static class CommandBuilderExtensions