Skip to content

Commit f56253f

Browse files
iancooperclaude
andcommitted
fix: PR #4039 2026-05-20 review items 1 & 2 — doc/regex alignment + drop AsyncLocal table-name smuggle
Item 1 — IAmARelationalDatabaseConfiguration.SchemaName XML doc previously claimed Identifiers.AssertSafe enforced a 64-char limit and reserved-prefix collision check, but the regex enforces neither. Trim the doc to describe what is actually enforced (^[A-Za-z][A-Za-z0-9_]*$) and rephrase the leading-underscore note in terms of the first-character letter class, which is what the regex really requires. Item 2 — MySqlBoxMigrationRunner used AsyncLocal<string?> to smuggle the per-invocation tableName from LockResourceFor into CreateUnitOfWorkAsync (needed so the UoW could surface the raw name in its tri-state RELEASE_LOCK Warning, since MySqlMigrationLockName.For hash-truncates long composites). Change the base abstract signature to pass (schemaName, tableName) explicitly into CreateUnitOfWorkAsync; the MySQL override reads the parameter directly and the AsyncLocal field is gone. Removes an implicit cross-hook coupling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6c675b0 commit f56253f

24 files changed

Lines changed: 57 additions & 68 deletions

File tree

src/Paramore.Brighter.BoxProvisioning.MsSql/MsSqlBoxMigrationRunner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ protected override async Task<SqlConnection> OpenConnectionAsync(CancellationTok
101101
}
102102

103103
protected override Task<IAmAProvisioningUnitOfWork<SqlTransaction>> CreateUnitOfWorkAsync(
104-
SqlConnection connection, CancellationToken cancellationToken)
104+
SqlConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
105105
=> Task.FromResult<IAmAProvisioningUnitOfWork<SqlTransaction>>(
106106
new MsSqlProvisioningUnitOfWork(connection, _advisoryLock, Logger));
107107

src/Paramore.Brighter.BoxProvisioning.MySql/MySqlBoxMigrationRunner.cs

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,6 @@ public class MySqlBoxMigrationRunner : SqlBoxMigrationRunner<MySqlConnection, My
5454

5555
private readonly IMySqlAdvisoryLock _advisoryLock;
5656

57-
// Threaded through the base's LockResourceFor → CreateUnitOfWorkAsync sequence so the
58-
// per-invocation tableName reaches MySqlProvisioningUnitOfWork's ctor for NF1-faithful
59-
// tri-state RELEASE_LOCK Warning emission (MySqlMigrationLockName.For hash-truncates the
60-
// raw tableName for long composites, so lockResource alone cannot surface it). AsyncLocal
61-
// preserves correctness under concurrent MigrateAsync invocations on the same instance.
62-
private readonly AsyncLocal<string?> _activeTableName = new();
63-
6457
/// <summary>
6558
/// Initialises the runner with an explicit detection helper and optional UoW dependencies.
6659
/// </summary>
@@ -108,32 +101,20 @@ protected override async Task<MySqlConnection> OpenConnectionAsync(CancellationT
108101
}
109102

110103
protected override Task<IAmAProvisioningUnitOfWork<MySqlTransaction>> CreateUnitOfWorkAsync(
111-
MySqlConnection connection, CancellationToken cancellationToken)
104+
MySqlConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
112105
{
113-
// Capture _activeTableName.Value into the UoW ctor and immediately clear the
114-
// AsyncLocal so the value does not leak onto the ExecutionContext of any continuation
115-
// observed after MigrateAsync returns — per PR #4039 reviewer item M4-4. The base
116-
// runner's hook sequence guarantees LockResourceFor (which sets the value) runs
117-
// exactly once before CreateUnitOfWorkAsync (which reads it), so clearing here is
118-
// safe: no other hook reads _activeTableName, and the UoW has already captured the
119-
// string into its `_tableName` field by the time the AsyncLocal is reset.
120-
try
121-
{
122-
return Task.FromResult<IAmAProvisioningUnitOfWork<MySqlTransaction>>(
123-
new MySqlProvisioningUnitOfWork(connection, _advisoryLock, Logger, _activeTableName.Value));
124-
}
125-
finally
126-
{
127-
_activeTableName.Value = null;
128-
}
106+
_ = schemaName;
107+
// The raw tableName threads into the UoW ctor for NF1-faithful tri-state RELEASE_LOCK
108+
// Warning emission — MySqlMigrationLockName.For hash-truncates long composites for the
109+
// GET_LOCK 64-char limit, so the lockResource string alone cannot surface the original
110+
// name. The base runner passes it explicitly per the LockResourceFor → CreateUnitOfWorkAsync
111+
// sequence; no cross-hook state is needed.
112+
return Task.FromResult<IAmAProvisioningUnitOfWork<MySqlTransaction>>(
113+
new MySqlProvisioningUnitOfWork(connection, _advisoryLock, Logger, tableName));
129114
}
130115

131116
protected override string LockResourceFor(string? schemaName, string tableName)
132117
{
133-
// Capture the per-invocation table name so CreateUnitOfWorkAsync can thread it into
134-
// the UoW ctor for NF1-faithful tri-state Warning emission — MySqlMigrationLockName.For
135-
// hash-truncates long composites and would otherwise lose the raw tableName.
136-
_activeTableName.Value = tableName;
137118
// The schema is folded into the lock name so two same-named tables in different schemas
138119
// acquire distinct advisory locks. MySQL's GET_LOCK has a 64-char limit; the helper
139120
// hashes names exceeding it (long form) and otherwise preserves the simple form for

src/Paramore.Brighter.BoxProvisioning.PostgreSql/PostgreSqlBoxMigrationRunner.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ protected override async Task<NpgsqlConnection> OpenConnectionAsync(Cancellation
9999
}
100100

101101
protected override Task<IAmAProvisioningUnitOfWork<NpgsqlTransaction>> CreateUnitOfWorkAsync(
102-
NpgsqlConnection connection, CancellationToken cancellationToken)
102+
NpgsqlConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
103103
=> Task.FromResult<IAmAProvisioningUnitOfWork<NpgsqlTransaction>>(
104104
new PostgreSqlProvisioningUnitOfWork(connection, _advisoryLock, Logger));
105105

src/Paramore.Brighter.BoxProvisioning.Sqlite/SqliteBoxMigrationRunner.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,10 @@ protected override async Task<SqliteConnection> OpenConnectionAsync(Cancellation
151151
}
152152

153153
protected override Task<IAmAProvisioningUnitOfWork<SqliteTransaction>> CreateUnitOfWorkAsync(
154-
SqliteConnection connection, CancellationToken cancellationToken)
154+
SqliteConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
155155
{
156+
_ = schemaName;
157+
_ = tableName;
156158
_ = cancellationToken;
157159
return Task.FromResult<IAmAProvisioningUnitOfWork<SqliteTransaction>>(
158160
new SqliteProvisioningUnitOfWork(connection, Logger));

src/Paramore.Brighter.BoxProvisioning/SqlBoxMigrationRunner.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ public async Task MigrateAsync(
192192
{
193193
connection = await OpenConnectionAsync(cancellationToken);
194194
var lockResource = LockResourceFor(schemaName, tableName);
195-
uow = await CreateUnitOfWorkAsync(connection, cancellationToken);
195+
uow = await CreateUnitOfWorkAsync(connection, schemaName, tableName, cancellationToken);
196196
await uow.BeginAsync(lockResource, _lockTimeout, cancellationToken);
197197
}
198198
catch (Exception ex)
@@ -316,10 +316,14 @@ await RunNormalPathAsync(
316316
/// Constructs the per-backend <see cref="IAmAProvisioningUnitOfWork{TTransaction}"/> over
317317
/// <paramref name="connection"/>. The runner declares the result with <c>await using</c>
318318
/// and immediately calls <c>BeginAsync</c>; the implementation should NOT begin the
319-
/// lock+transaction here, only construct the UoW.
319+
/// lock+transaction here, only construct the UoW. The per-invocation
320+
/// <paramref name="schemaName"/> and <paramref name="tableName"/> are passed explicitly
321+
/// so backend UoWs that need them at construction (e.g. MySQL's tri-state RELEASE_LOCK
322+
/// warning emission, which surfaces the raw table name lost to MySqlMigrationLockName.For's
323+
/// hash-truncation) can capture them directly instead of via cross-hook state.
320324
/// </summary>
321325
protected abstract Task<IAmAProvisioningUnitOfWork<TTransaction>> CreateUnitOfWorkAsync(
322-
TConnection connection, CancellationToken cancellationToken);
326+
TConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken);
323327

324328
/// <summary>
325329
/// Computes the backend-specific advisory lock resource string for the given table.

src/Paramore.Brighter/IAmARelationalDatabaseConfiguration.cs

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,17 @@ public interface IAmARelationalDatabaseConfiguration
4646
/// </summary>
4747
/// <value>The schema name, or <c>null</c> for the backend's default schema.</value>
4848
/// <remarks>
49-
/// Box-provisioning enforces a strict regex on identifier inputs (table, column, and
50-
/// schema names) at the framework chokepoint via <c>Paramore.Brighter.BoxProvisioning.Identifiers.AssertSafe</c>:
51-
/// the accepted character class is <c>[A-Za-z][A-Za-z0-9_]*</c> with length ≤ 64, no
52-
/// leading underscore, and no reserved-prefix collisions. The rule is intentionally
53-
/// over-restrictive — it applies the strictest-backend rule (Spanner's reserved
54-
/// <c>_</c> prefix) uniformly so identifier validation is platform-portable. Schema
55-
/// names that fail the regex will surface as <see cref="ConfigurationException"/>
56-
/// at provisioning entry rather than as a downstream SQL error. Per PR #4039
57-
/// reviewer item M2-7.
49+
/// Box-provisioning enforces a regex on identifier inputs (table, column, and schema
50+
/// names) at the framework chokepoint via
51+
/// <c>Paramore.Brighter.BoxProvisioning.Identifiers.AssertSafe</c>: the accepted pattern
52+
/// is <c>^[A-Za-z][A-Za-z0-9_]*$</c> (must start with an ASCII letter; remaining
53+
/// characters are ASCII letters, digits, or underscore). The first-character letter
54+
/// class excludes leading underscores — Spanner reserves <c>_</c>-prefixed names while
55+
/// other backends accept them, so the regex picks the strictest backend's rule to keep
56+
/// portable configuration safe. Schema names that fail the regex surface as
57+
/// <see cref="ConfigurationException"/> at provisioning entry rather than as a
58+
/// downstream SQL error. Per PR #4039 reviewer items M2-7 and the multi-part 2026-05-20
59+
/// review item 1.
5860
/// </remarks>
5961
string? SchemaName { get; }
6062
}

tests/Paramore.Brighter.BoxProvisioning.Tests/When_relational_box_migration_runner_base_begin_async_throws_it_should_skip_commit_and_rollback_and_still_dispose.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ protected override Task<FakeDbConnection> OpenConnectionAsync(CancellationToken
118118
=> Task.FromResult(new FakeDbConnection());
119119

120120
protected override Task<IAmAProvisioningUnitOfWork<FakeDbTransaction>> CreateUnitOfWorkAsync(
121-
FakeDbConnection connection, CancellationToken cancellationToken)
121+
FakeDbConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
122122
=> Task.FromResult<IAmAProvisioningUnitOfWork<FakeDbTransaction>>(_unitOfWork);
123123

124124
protected override string LockResourceFor(string? schemaName, string tableName)

tests/Paramore.Brighter.BoxProvisioning.Tests/When_relational_box_migration_runner_base_hook_throws_it_should_rollback_with_cancellation_token_none_and_rethrow.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ protected override Task<FakeDbConnection> OpenConnectionAsync(CancellationToken
132132
=> Task.FromResult(new FakeDbConnection());
133133

134134
protected override Task<IAmAProvisioningUnitOfWork<FakeDbTransaction>> CreateUnitOfWorkAsync(
135-
FakeDbConnection connection, CancellationToken cancellationToken)
135+
FakeDbConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
136136
=> Task.FromResult<IAmAProvisioningUnitOfWork<FakeDbTransaction>>(_unitOfWork);
137137

138138
protected override string LockResourceFor(string? schemaName, string tableName)

tests/Paramore.Brighter.BoxProvisioning.Tests/When_relational_box_migration_runner_base_migrate_receives_non_monotonic_migrations_it_should_throw_before_opening_connection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ protected override Task<FakeDbConnection> OpenConnectionAsync(CancellationToken
108108
}
109109

110110
protected override Task<IAmAProvisioningUnitOfWork<FakeDbTransaction>> CreateUnitOfWorkAsync(
111-
FakeDbConnection connection, CancellationToken cancellationToken)
111+
FakeDbConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
112112
=> throw new NotSupportedException("CreateUnitOfWorkAsync must not be reached when validation throws.");
113113

114114
protected override string LockResourceFor(string? schemaName, string tableName)

tests/Paramore.Brighter.BoxProvisioning.Tests/When_relational_box_migration_runner_base_migrate_receives_null_migrations_it_should_throw_before_opening_connection.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ protected override Task<FakeDbConnection> OpenConnectionAsync(CancellationToken
105105
}
106106

107107
protected override Task<IAmAProvisioningUnitOfWork<FakeDbTransaction>> CreateUnitOfWorkAsync(
108-
FakeDbConnection connection, CancellationToken cancellationToken)
108+
FakeDbConnection connection, string? schemaName, string tableName, CancellationToken cancellationToken)
109109
=> throw new NotSupportedException("CreateUnitOfWorkAsync must not be reached when null-migrations validation throws.");
110110

111111
protected override string LockResourceFor(string? schemaName, string tableName)

0 commit comments

Comments
 (0)