diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index 6fe44613..a90a8e34 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -207,6 +207,11 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa the OpcUaClient history passthrough are deleted — nothing ever dispatched HistoryRead to a driver; it is served only by the server-wide `IHistorianDataSource`. `isHistorized` on an OpcUaClient tag means OtOpcUa records it, not that upstream history is proxied. A stored `ResilienceConfig` still carrying a `HistoryRead` key parses (diagnostic only). +- **Sql (#753):** five providers through ONE map (`SqlDialects` — factory, probe and browser all read it; they drifted + when each had a switch). `Odbc` is the ANSI profile only (INFORMATION_SCHEMA + FETCH FIRST + positional `?` — callers + must add parameters in marker order). Named `Query` tags run inside an always-rolled-back transaction behind the + `SqlReadOnlyStatement` guard; writes are `allowWrites` + tag `"writable": true` + a single-row model, as a + parameterised UPDATE over gate-canonical identifiers that commits only on exactly one row. `docs/drivers/Sql.md`. - **FOCAS / AbLegacy:** background loops are joined (≤ 5 s) on shutdown. - **FOCAS (#257/#266):** `cnc_statinfo` flags are a `Status/` fixed-tree subtree (`FIXED:Status/Run`…, capability-probed); adjacent PMC tags in one poll batch coalesce into one `pmc_rdpmcrng` (`PmcCoalesce` config, default on) — a refused range diff --git a/docs/drivers/Sql.md b/docs/drivers/Sql.md index 78bc92f6..59ae77cf 100644 --- a/docs/drivers/Sql.md +++ b/docs/drivers/Sql.md @@ -3,8 +3,9 @@ > **Status lives in Gitea.** Open follow-ups and known gaps noted in this doc are tracked as issues on > gitea.dohertylan.com/dohertj2/lmxopcua (label `migrated`, 2026-11-06). This doc describes the system — do not track status here; file new work as an issue. -Getting-started guide for the `Sql` driver — a **read-only** Equipment-kind driver that polls rows -out of a relational database and publishes them as OPC UA variables. For the design rationale read +Getting-started guide for the `Sql` driver — an Equipment-kind driver that polls rows out of a +relational database and publishes them as OPC UA variables. It is **read-only unless writes are +explicitly enabled** at both the driver and the tag (see [Writes](#writes-opt-in-753)). For the design rationale read [`docs/plans/2026-07-15-sql-poll-driver-design.md`](../plans/2026-07-15-sql-poll-driver-design.md); for the build-vs-plan record read [`docs/plans/2026-07-24-sql-poll-driver.md`](../plans/2026-07-24-sql-poll-driver.md). @@ -54,10 +55,8 @@ Two provider-specific things are worth knowing before you author against them: type map, so an integer-shaped `NUMBER` still infers double. Use the per-tag `type` override (see [Optional per-tag fields](#optional-per-tag-fields)) when the OPC UA type matters. -**Read-only, structurally.** `SqlDriver` does not implement `IWritable`, so no configuration can -produce a write. The `allowWrites` config field exists and is **inert**; the factory *warns* when it -is authored `true` rather than failing, on the grounds that the flag cannot do harm but an operator -who believes writes are enabled can. → #753 +**Read-only by default.** `allowWrites` is off unless authored `true`, and even then only a tag whose +own TagConfig says `"writable": true` can be written — see [Writes](#writes-opt-in-753). ## Tag models @@ -186,7 +185,8 @@ row: | Field | Effect | |---|---| | `timestampColumn` | Source timestamp for the value. Absent ⇒ the driver stamps the poll time. | -| `type` | Explicit OPC UA type override. Absent ⇒ inferred from the result set's column metadata. | +| `type` | Explicit OPC UA type override. Absent ⇒ inferred from the result set's column metadata. A write is coerced to it. | +| `writable` | The tag's write opt-in — a JSON **boolean**; absent ⇒ read-only. Anything else (`"true"`, `1`) refuses the tag. Effective only with the driver's `allowWrites`. | There is **no per-tag poll-interval field**. The interval is the subscriber's, floored by `PollGroupEngine.DefaultMinInterval`, falling back to the instance's `defaultPollInterval` when the @@ -216,7 +216,7 @@ existed in the parser — nothing reads it.) | `commandTimeout` | `00:00:10` | ADO.NET server-side backstop. **Must be less than `operationTimeout`** — it is the backstop, not the deadline. | | `maxConcurrentGroups` | `4` | Connection-pool guard: cap on group queries in flight. | | `nullIsBad` | `false` | `false` ⇒ a NULL cell publishes **Uncertain**; `true` ⇒ **Bad**. | -| `allowWrites` | — | **Inert.** v1 has no write path; authoring `true` logs a warning. | +| `allowWrites` | `false` | Driver-level write switch. Off ⇒ every write is `BadNotWritable`. See [Writes](#writes-opt-in-753). | | `queries` | — | Named read-only queries for `Query` tags — see [`Query`](#query--a-named-read-only-query-753). Edited as JSON in the driver form, which applies the factory's own rules. | ### Credentials never enter the deployment artifact @@ -329,6 +329,43 @@ catalog can supply it. `RawBrowseCommitMapper.DescribeUncommittableLeaf` refuses names the table, and points at the tag editor; no selector is invented, and the mapper's blob stays selector-less rather than dropping to a generic `{"address": …}` the typed editor cannot read. +## Writes (opt-in, #753) + +A write reaches the database only when **every** gate holds, outermost first: + +1. the OPC UA node is Writable (the Tag's own flag) and the caller holds `WriteOperate` — the server; +2. the driver config says `"allowWrites": true` — otherwise `BadNotWritable`; +3. the tag's TagConfig says `"writable": true` — otherwise `BadNotWritable`; +4. the model names exactly one row: **KeyValue**, or **WideRow with a `whereColumn`/`whereValue` + pair**. `topByTimestamp` ("the newest row" moves under a concurrent insert) and `Query` (a projection + names no table) are `BadNotWritable`. + +The write is the **only** statement shape the driver can produce — there is no authored write SQL, +INSERT, UPSERT or procedure call: + +```sql +UPDATE SET = @v WHERE = @k +``` + +- **Identifiers are the catalog gate's**: a write resolves against the gate-accepted definition, so + every name in that statement was read back out of the catalog and dialect-quoted. A gate-rejected tag + answers `BadNodeIdUnknown`. The value and the key are always **bound parameters**. +- **Exactly one row or nothing.** The UPDATE runs in a transaction that commits only when it touched + one row. Zero rows ⇒ `BadNoData` (the driver never inserts); more than one ⇒ rolled back, + `BadConfigurationError` (the selector is ambiguous — the read path's rule); a provider that cannot + report a row count ⇒ rolled back, `BadNotSupported`. +- The value is coerced to the tag's `type` when it has one (`BadTypeMismatch` when it will not + coerce), else bound as the scalar the client sent. A null writes SQL NULL. +- Bounded by `operationTimeout` (`BadTimeout`). A provider error after the connection opened — a + missing `UPDATE` grant, a constraint — is `BadCommunicationError`, nothing committed. Writes never + change driver health. +- Discovery advertises `Operate` only for a tag where all of 2–4 hold, `ViewOnly` otherwise. +- **Grant the login `UPDATE` on just the tables that carry writable tags.** The live gate + (`MySqlReadTests`) proves the server refuses a write to a SELECT-only table even with every driver + gate open. +- A source timestamp column is **not** touched by a write, so the next read keeps the row's old + timestamp unless the database maintains it (a trigger or `ON UPDATE CURRENT_TIMESTAMP`). + ## Authoring in the AdminUI - **Driver config** — `/raw` → driver node → *Configure*. Sql is a **single-connection** driver: the @@ -338,9 +375,10 @@ selector-less rather than dropping to a generic `{"address": …}` the typed edi ## Capabilities -`IDriver`, `ITagDiscovery`, `IReadable`, `ISubscribable`, `IHostConnectivityProbe`. +`IDriver`, `ITagDiscovery`, `IReadable`, `IWritable` (opt-in, above), `ISubscribable`, +`IHostConnectivityProbe`. -No `IWritable` (structural, above). No `IRediscoverable` — a SQL schema change is not signalled, and +No `IRediscoverable` — a SQL schema change is not signalled, and `DiscoverAsync` echoes authored config rather than enumerating the database, so this driver is deliberately excluded from any discovery-diff machinery. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs index 4f3f4b00..79e2372a 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlDriverConfigDto.cs @@ -44,10 +44,10 @@ public sealed class SqlDriverConfigDto public bool? NullIsBad { get; init; } /// - /// Master write kill-switch. v1 is read-only and this field is inert — the driver does not - /// implement IWritable, so no value here can produce a write. The factory warns when it - /// is authored rather than failing the driver, since the flag cannot do harm but - /// an operator who believes writes are enabled can. + /// Master write switch (design §3.4, lmxopcua#753). Absent or ⇒ every write is + /// refused. ⇒ a tag may be written when its own TagConfig also says + /// "writable": true and its model names exactly one row; the node additionally needs its Writable + /// flag and the caller the WriteOperate role. /// public bool? AllowWrites { get; init; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs index 1754c9af..bd3ef307 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlEquipmentTagParser.cs @@ -56,6 +56,16 @@ public static class SqlEquipmentTagParser var timestampColumn = ReadString(root, "timestampColumn"); + // The per-tag write opt-in (design §3.4). Strict like the enums: absent ⇒ read-only, a JSON boolean ⇒ + // that, anything else (a "true" string, a number) ⇒ the tag is refused rather than guessed at — a + // write permission is the last field to coerce leniently. + var writable = false; + if (root.TryGetProperty("writable", out var writableEl)) + { + if (writableEl.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) return false; + writable = writableEl.GetBoolean(); + } + if (model == SqlTagModel.Query) { // A named query reads no table of its own: the tag names the driver-config query, the result @@ -77,7 +87,7 @@ public static class SqlEquipmentTagParser Name: rawPath, Model: model, Table: string.Empty, TimestampColumn: timestampColumn, ColumnName: column, DeclaredType: declaredType, QueryName: queryName, - RowKeyColumn: rowKeyColumn, RowKeyValue: rowKeyValue); + RowKeyColumn: rowKeyColumn, RowKeyValue: rowKeyValue, Writable: writable); return true; } @@ -98,7 +108,7 @@ public static class SqlEquipmentTagParser def = new SqlTagDefinition( Name: rawPath, Model: model, Table: table!, KeyColumn: keyColumn, KeyValue: keyValue, ValueColumn: valueColumn, - TimestampColumn: timestampColumn, DeclaredType: declaredType); + TimestampColumn: timestampColumn, DeclaredType: declaredType, Writable: writable); return true; } @@ -122,7 +132,7 @@ public static class SqlEquipmentTagParser RowSelectorColumn: hasWherePair ? selectorColumn : null, RowSelectorValue: hasWherePair ? selectorValue : null, RowSelectorTopByTimestamp: hasWherePair ? null : topByTimestamp, - DeclaredType: declaredType); + DeclaredType: declaredType, Writable: writable); return true; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinition.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinition.cs index 8bf64cc9..7144cf7f 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinition.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinition.cs @@ -34,6 +34,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; /// return exactly one row. Matched against the result set, never quoted into SQL. /// /// : the value (matched as text). +/// +/// The tag's own write opt-in ("writable": true in its TagConfig, design §3.4). Necessary but not +/// sufficient: the driver also needs allowWrites, the model must name one row to update (KeyValue, or +/// a WideRow where-pair), and the node needs its Writable flag plus the caller's WriteOperate role. +/// /// /// Optional explicit type override from the blob's type field. ⇒ the /// driver infers the type from the result set's column metadata. @@ -53,4 +58,5 @@ public sealed record SqlTagDefinition( DriverDataType? DeclaredType = null, string? QueryName = null, string? RowKeyColumn = null, - string? RowKeyValue = null); + string? RowKeyValue = null, + bool Writable = false); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs index 6750f1a1..2eb8b9c5 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts/SqlTagDefinitionFactory.cs @@ -110,6 +110,12 @@ public static class SqlTagDefinitionFactory if (typeWarning is not null) warnings.Add(typeWarning); if (modelWarning is not null) return; // the model is unknown; the per-model checks cannot run + if (root.TryGetProperty("writable", out var writable) + && writable.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) + { + warnings.Add("Sql TagConfig 'writable' must be a JSON boolean (true/false) — the driver will skip this tag."); + } + var model = TagConfigJson.ReadEnumOrDefault(root, "model", SqlTagModel.KeyValue); if (model == SqlTagModel.Query) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs index f1f2e3b1..39f0fe3c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriver.cs @@ -9,15 +9,19 @@ using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// -/// The Sql driver instance: a read-only Equipment-kind driver that polls SQL tables/views -/// and publishes selected cells as OPC UA variables. Structurally the SQL analogue of +/// The Sql driver instance: an Equipment-kind driver that polls SQL tables/views (and named read-only +/// queries) and publishes selected cells as OPC UA variables — read-only unless writes are explicitly enabled. Structurally the SQL analogue of /// ModbusDriver — this type owns lifecycle, the authored RawPath table, health, host /// connectivity, and the subscription overlay, while /// owns the read path. -/// Read-only is structural, not configured. is deliberately not -/// implemented, so no amount of config (and no WriteOperate role) can produce a write; every -/// discovered variable is . A future write feature is a -/// new capability, not a flag flip. +/// Writes are opt-in at two levels and default off (design §3.4, lmxopcua#753). A write needs +/// the driver's allowWrites, the tag's own "writable": true, and a model that names one row +/// (KeyValue, or a WideRow where-pair) — on top of the node's Writable flag and the caller's WriteOperate +/// role, which the server checks first. It is a parameterised single-row UPDATE over the tag's +/// catalog-validated identifiers (), run in a transaction that commits only when +/// exactly one row was touched. With allowWrites off every write is +/// and every variable discovers as +/// . /// Health is classified here, because nothing below does it. The reader honours /// literally: it throws only when the database cannot be reached, and turns /// every other failure — including a frozen database — into Bad-coded snapshots. Those return @@ -28,7 +32,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; /// manufacture an exception to force backoff. /// public sealed class SqlDriver - : IDriver, ITagDiscovery, IReadable, IBatchSubscribable, IHostConnectivityProbe, ISkippedTagReporter, IAsyncDisposable + : IDriver, ITagDiscovery, IReadable, IWritable, IBatchSubscribable, IHostConnectivityProbe, ISkippedTagReporter, + IAsyncDisposable { /// Authored tags this driver could not build into its poll table. private readonly SkippedTagLedger _skippedTags = new(); @@ -532,8 +537,9 @@ public sealed class SqlDriver DriverDataType: tag.DeclaredType ?? DriverDataType.String, IsArray: false, ArrayDim: null, - // v1 is read-only: no tag, and no configuration, can widen this. - SecurityClass: SecurityClassification.ViewOnly, + // Operate only when a write could actually succeed — the driver switch, the tag's opt-in and a + // single-row model all hold; otherwise the node advertises what it is: read-only. + SecurityClass: IsWritePermitted(tag) ? SecurityClassification.Operate : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, WriteIdempotent: false)); @@ -612,6 +618,131 @@ public sealed class SqlDriver } } + // ---- IWritable (design §3.4, lmxopcua#753) ---- + + /// + /// Whether a write to is permitted by configuration: the driver's allowWrites, + /// the tag's own writable opt-in, and a model with a single-row target. Says nothing about the node's + /// Writable flag or the caller's role, which the server enforces before a write reaches the driver. + /// + private bool IsWritePermitted(SqlTagDefinition tag) + => _options.AllowWrites && tag.Writable + && (tag.Model == SqlTagModel.KeyValue + || (tag.Model == SqlTagModel.WideRow && tag.RowSelectorColumn is not null)); + + /// + // Each write is a parameterised single-row UPDATE (SqlWritePlanner) over the tag's CATALOG-VALIDATED + // definition — a tag the gate rejected misses here and answers BadNodeIdUnknown, so no unvalidated identifier + // can reach an UPDATE. It runs in a transaction that commits only when exactly one row was touched: zero rows + // is BadNoData (nothing to update — the driver never inserts), more than one is BadConfigurationError (the + // selector is ambiguous, the read-side rule), and both roll back. Bounded by operationTimeout like a poll. A + // write never changes driver health: its outcome is the write's answer, not a statement about the database. + public async Task> WriteAsync( + IReadOnlyList writes, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(writes); + var results = new WriteResult[writes.Count]; + for (var i = 0; i < writes.Count; i++) + { + var write = writes[i]; + results[i] = new WriteResult(await WriteOneAsync(write, cancellationToken).ConfigureAwait(false)); + } + + return results; + } + + private async Task WriteOneAsync(WriteRequest write, CancellationToken cancellationToken) + { + if (!_options.AllowWrites) return SqlStatusCodes.BadNotWritable; + if (write.FullReference is null || Lookup(write.FullReference) is not { } tag) + return SqlStatusCodes.BadNodeIdUnknown; + if (!IsWritePermitted(tag)) return SqlStatusCodes.BadNotWritable; + + var planned = SqlWritePlanner.TryPlan(tag, write.Value, _dialect, out var plan); + if (planned != SqlStatusCodes.Good) return planned; + + try + { + return await RunBoundedAsync( + token => ExecuteWriteAsync(tag, plan!, token), + _options.OperationTimeout, + "the write did not return", + cancellationToken).ConfigureAwait(false); + } + catch (TimeoutException) + { + _logger.LogWarning("Sql write to '{Tag}' timed out against {Endpoint}. Driver={DriverInstanceId}", + tag.Name, Endpoint, _driverInstanceId); + return SqlStatusCodes.BadTimeout; + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // The caller's write deadline fired; the uncommitted transaction dies with its connection. + return SqlStatusCodes.BadTimeout; + } + catch (Exception ex) + { + // Open or execute failed. Type only on any surface; the exception (never the connection string) to the + // log sink. Nothing was committed. + _logger.LogWarning(ex, "Sql write to '{Tag}' failed against {Endpoint}: {Type}. Driver={DriverInstanceId}", + tag.Name, Endpoint, ex.GetType().Name, _driverInstanceId); + return SqlStatusCodes.BadCommunicationError; + } + } + + /// Opens a connection and runs one planned UPDATE in a transaction that commits only on one row. + private async Task ExecuteWriteAsync(SqlTagDefinition tag, SqlWritePlan plan, CancellationToken cancellationToken) + { + var connection = _factory.CreateConnection() + ?? throw new InvalidOperationException( + $"the {_dialect.Provider} provider factory returned no connection."); + + await using (connection.ConfigureAwait(false)) + { + connection.ConnectionString = _connectionString; + await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + + var transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); + await using (transaction.ConfigureAwait(false)) + { + int rows; + var command = connection.CreateCommand(); + await using (command.ConfigureAwait(false)) + { + _dialect.ConfigureCommand(command); + command.Transaction = transaction; + command.CommandText = plan.SqlText; + command.CommandTimeout = Math.Max(1, (int)Math.Ceiling(_options.CommandTimeout.TotalSeconds)); + for (var i = 0; i < plan.ParameterBindNames.Count; i++) + { + var parameter = command.CreateParameter(); + parameter.ParameterName = plan.ParameterBindNames[i]; + parameter.Value = plan.Parameters[i] ?? DBNull.Value; + _dialect.ConfigureParameter(parameter, plan.Parameters[i]); + command.Parameters.Add(parameter); + } + + rows = await command.ExecuteNonQueryAsync(cancellationToken).ConfigureAwait(false); + } + + if (rows == 1) + { + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + return SqlStatusCodes.Good; + } + + await transaction.RollbackAsync(CancellationToken.None).ConfigureAwait(false); + if (rows == 0) return SqlStatusCodes.BadNoData; + + _logger.LogWarning( + "Sql write to '{Tag}' matched {Rows} row(s) on '{Table}' and was rolled back: a write must update " + + "exactly one row. Driver={DriverInstanceId}", + tag.Name, rows, tag.Table, _driverInstanceId); + return rows > 1 ? SqlStatusCodes.BadConfigurationError : SqlStatusCodes.BadNotSupported; + } + } + } + // ---- ISubscribable (polling overlay via the shared engine) ---- /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs index a0bfc63c..445b77fd 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverFactoryExtensions.cs @@ -148,18 +148,6 @@ public static class SqlDriverFactoryExtensions var logger = (ILogger?)loggerFactory?.CreateLogger(typeof(SqlDriverFactoryExtensions).FullName!) ?? NullLogger.Instance; - if (dto.AllowWrites is true) - { - // Inert by construction (SqlDriver does not implement IWritable), so failing the driver over it - // would brick a deployment for a flag that cannot do harm. The operator who believes writes are - // on, however, can — so this is loud. - logger.LogWarning( - "Sql driver {DriverInstanceId} authored allowWrites=true, which this build ignores: the Sql " + - "driver is read-only (it implements no write capability), so no configuration can enable a " + - "write. Remove the flag, or expect reads only.", - driverInstanceId); - } - var driver = new SqlDriver( options, driverInstanceId, @@ -176,9 +164,10 @@ public static class SqlDriverFactoryExtensions // permitted outside the provider call. logger.LogInformation( "Sql driver {DriverInstanceId} configured for {Provider} at {Endpoint} with {TagCount} " + - "authored tag(s), poll {PollInterval}, operation timeout {OperationTimeout}.", + "authored tag(s), poll {PollInterval}, operation timeout {OperationTimeout}, writes {Writes}.", driverInstanceId, dto.Provider, driver.Endpoint, options.RawTags.Count, - options.DefaultPollInterval, options.OperationTimeout); + options.DefaultPollInterval, options.OperationTimeout, + options.AllowWrites ? "allowed for opted-in tags" : "off"); return driver; } @@ -273,6 +262,7 @@ public static class SqlDriverFactoryExtensions CommandTimeout = commandTimeout, MaxConcurrentGroups = maxConcurrentGroups, NullIsBad = dto.NullIsBad ?? false, + AllowWrites = dto.AllowWrites ?? false, Queries = BuildQueries(dto, driverInstanceId, dialect), }; } @@ -372,8 +362,8 @@ public static class SqlDriverFactoryExtensions /// The typed driver options. /// The provider seam the provider key selects. /// The connection string resolved from connectionStringRef. Never logged. -/// The raw parsed DTO, so a caller needing a key the options do not carry (e.g. -/// allowWrites, which the driver ignores but warns about) need not re-deserialize. +/// The raw parsed DTO, so a caller needing a key the options do not carry need not +/// re-deserialize. public sealed record SqlDriverBinding( SqlDriverOptions Options, ISqlDialect Dialect, diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverOptions.cs index 45582b6f..5c84a896 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlDriverOptions.cs @@ -54,6 +54,14 @@ public sealed class SqlDriverOptions /// public bool NullIsBad { get; init; } + /// + /// The driver-level write switch (design §3.4, lmxopcua#753). (the default) refuses + /// every write with whatever a tag says; + /// permits writes to tags that ALSO opted in () and whose model names + /// one row (KeyValue, or a WideRow where-pair). + /// + public bool AllowWrites { get; init; } + /// /// The validated named queries (design §5.4) a tag reads by name, keyed /// ordinally. Empty when the config declares none. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs index 3c69bee0..6e98dfc3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlPollReader.cs @@ -1021,7 +1021,7 @@ public sealed class SqlPollReader : IDisposable /// Coerces a provider cell to the tag's effective , so the published CLR /// type matches the type the address space declared for the node. Never throws. /// - private static bool TryCoerce(object cell, DriverDataType target, out object? value) + internal static bool TryCoerce(object cell, DriverDataType target, out object? value) { try { @@ -1251,6 +1251,15 @@ public static class SqlStatusCodes /// A defect in the reader itself; never expected in the field. public const uint BadInternalError = 0x80020000u; + /// + /// A write the driver refuses by configuration: allowWrites is off, the tag did not opt in, or its + /// model names no single row to update (lmxopcua#753). + /// + public const uint BadNotWritable = 0x803B0000u; + + /// The provider could not report how many rows a write touched, so it was rolled back. + public const uint BadNotSupported = 0x803D0000u; + /// The quality-class mask — the top two bits of an OPC UA status code. private const uint SeverityMask = 0xC0000000u; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlWritePlanner.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlWritePlanner.cs new file mode 100644 index 00000000..f5388678 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql/SqlWritePlanner.cs @@ -0,0 +1,124 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql; + +/// +/// One compiled single-row write (lmxopcua#753): a parameterised UPDATE whose identifiers are the +/// tag's catalog-validated ones and whose values are all bound. +/// +/// The statement — dialect-quoted identifiers and parameter markers only. +/// The bind names, in marker order (positional providers bind in this order). +/// The bound values, aligned with . +public sealed record SqlWritePlan( + string SqlText, + IReadOnlyList ParameterBindNames, + IReadOnlyList Parameters); + +/// +/// Turns one OPC UA write into a parameterised single-row UPDATE (design §3.4, lmxopcua#753) — pure, no +/// I/O. The write surface is deliberately narrow: no authored SQL, no INSERT/UPSERT/procedure, only +/// UPDATE <table> SET <value column> = @v WHERE <key column> = @k built from the +/// tag's own identifiers. +/// +/// : SET valueColumn = @v WHERE keyColumn = @k. +/// with a where-pair: SET columnName = @v WHERE whereColumn = @w. +/// by topByTimestamp, and : +/// refused () — "the newest row" moves under a concurrent +/// insert, and a named query's projection names no table to update. +/// +/// Injection boundary. Every identifier is one the catalog gate resolved and substituted (the +/// driver plans writes only for polled, i.e. gate-accepted, definitions) and reaches the text through +/// ; the written value and the key are bound parameters. Nothing an +/// OPC UA client sends reaches the command text. +/// +public static class SqlWritePlanner +{ + private const string ValueBaseName = "v"; + private const string KeyBaseName = "k"; + + /// + /// Plans a write of to , or explains why it is refused. + /// + /// The catalog-validated definition. + /// The value the client wrote. + /// Quoting, markers and bind names. + /// The plan, when this returns . + /// , or the status the write publishes. + public static uint TryPlan(SqlTagDefinition tag, object? value, ISqlDialect dialect, out SqlWritePlan? plan) + { + ArgumentNullException.ThrowIfNull(tag); + ArgumentNullException.ThrowIfNull(dialect); + plan = null; + + string setColumn, keyColumn; + string? keyValue; + switch (tag.Model) + { + case SqlTagModel.KeyValue when !Blank(tag.ValueColumn) && !Blank(tag.KeyColumn) && tag.KeyValue is not null: + setColumn = tag.ValueColumn!; + keyColumn = tag.KeyColumn!; + keyValue = tag.KeyValue; + break; + case SqlTagModel.WideRow when !Blank(tag.ColumnName) && !Blank(tag.RowSelectorColumn) + && tag.RowSelectorValue is not null: + setColumn = tag.ColumnName!; + keyColumn = tag.RowSelectorColumn!; + keyValue = tag.RowSelectorValue; + break; + default: + return SqlStatusCodes.BadNotWritable; + } + + if (!TryConvert(value, tag.DeclaredType, out var bound)) return SqlStatusCodes.BadTypeMismatch; + + string sql; + try + { + sql = string.Concat( + "UPDATE ", QuoteQualifiedName(tag.Table, dialect), + " SET ", dialect.QuoteIdentifier(setColumn), " = ", dialect.ParameterMarker(ValueBaseName), + " WHERE ", dialect.QuoteIdentifier(keyColumn), " = ", dialect.ParameterMarker(KeyBaseName)); + } + catch (ArgumentException) + { + // Unreachable for a gate-accepted definition (its identifiers came from the catalog); refuse rather + // than build a statement around a name the dialect would not quote. + return SqlStatusCodes.BadConfigurationError; + } + + // Marker order: SET's value first, then WHERE's key — a positional provider (ODBC) binds in this order. + plan = new SqlWritePlan( + sql, + [dialect.ParameterBindName(ValueBaseName), dialect.ParameterBindName(KeyBaseName)], + [bound, keyValue]); + return SqlStatusCodes.Good; + } + + /// + /// Converts the client's value to what is bound: coerced to the tag's declared type when it has one + /// (the same coercion a read applies), otherwise passed through when it is a scalar a provider can bind. + /// A null writes SQL NULL. + /// + private static bool TryConvert(object? value, DriverDataType? declared, out object? bound) + { + bound = null; + if (value is null) return true; + if (declared is { } type) return SqlPollReader.TryCoerce(value, type, out bound); + + if (value is string or bool or byte or sbyte or short or ushort or int or uint or long or ulong + or float or double or decimal or DateTime or DateTimeOffset or Guid) + { + bound = value is DateTime dt && dt.Kind == DateTimeKind.Local ? dt.ToUniversalTime() : value; + return true; + } + + return false; + } + + /// Quotes each part of a (catalog-canonical) schema.table name. + private static string QuoteQualifiedName(string name, ISqlDialect dialect) + => string.Join('.', name.Split('.').Select(dialect.QuoteIdentifier)); + + private static bool Blank(string? value) => string.IsNullOrWhiteSpace(value); +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SqlDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SqlDriverForm.razor index 9151782f..14a16485 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SqlDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SqlDriverForm.razor @@ -1,4 +1,4 @@ -@* Embeddable read-only Sql driver (connection/dialect) config form body. There is NO per-device +@* Embeddable Sql driver (connection/dialect) config form body. There is NO per-device endpoint — the whole database connection is named indirectly by ConnectionStringRef (an env-var name, resolved at Initialize), so no connection string is ever authored into the config. Hosted by DriverConfigModal. Exposes GetConfigJson() for the parent to pull at save time and fires @@ -90,11 +90,13 @@
- +
- Read-only v1 does not implement writes — this flag is inert and is not authored. + Off = every write refused (default). On = only tags whose TagConfig says writable: true are + written, as a single-row parameterised UPDATE committed only when exactly one row matches. + Give the connecting login UPDATE on just those tables.
@@ -159,7 +161,7 @@ } /// Serializes the current connection/dialect config to camelCase JSON with enums as NAME strings. - /// rawTags is never emitted (the composer adds it at deploy); allowWrites is never emitted (v1 read-only). + /// rawTags is never emitted (the composer adds it at deploy); allowWrites only when switched on. public string GetConfigJson() => JsonSerializer.Serialize(_form.ToDto(), _jsonOpts); private async Task EmitAsync() @@ -182,8 +184,8 @@ } /// Flat mutable mirror of the driver-level fields of . TimeSpans are - /// edited as nullable whole-second ints (blank ⇒ null ⇒ the factory default). rawTags + allowWrites are not - /// mirrored — the composer owns rawTags and v1 is read-only. + /// edited as nullable whole-second ints (blank ⇒ null ⇒ the factory default). rawTags is not mirrored — the + /// composer owns it. public sealed class FormModel { public SqlProvider Provider { get; set; } = SqlProvider.SqlServer; @@ -194,6 +196,9 @@ public int? MaxConcurrentGroups { get; set; } public bool NullIsBad { get; set; } + /// The driver-level write switch; unchecked ⇒ omitted ⇒ read-only. + public bool AllowWrites { get; set; } + /// The named queries the form emits — the last text that parsed and validated. public Dictionary? Queries { get; set; } @@ -254,6 +259,7 @@ CommandTimeoutSeconds = ToSeconds(d.CommandTimeout), MaxConcurrentGroups = d.MaxConcurrentGroups, NullIsBad = d.NullIsBad ?? false, + AllowWrites = d.AllowWrites ?? false, Queries = d.Queries, QueriesJson = d.Queries is { Count: > 0 } q ? JsonSerializer.Serialize(q, QueryJsonOptions) : "", }; @@ -266,9 +272,10 @@ OperationTimeout = ToTimeSpan(OperationTimeoutSeconds), CommandTimeout = ToTimeSpan(CommandTimeoutSeconds), MaxConcurrentGroups = MaxConcurrentGroups, - // Emit nullIsBad only when true; unchecked ⇒ null ⇒ the factory default (Uncertain). allowWrites - // and rawTags are deliberately left null so they are omitted from the authored blob. + // Emit nullIsBad / allowWrites only when true; unchecked ⇒ null ⇒ omitted ⇒ the factory default + // (Uncertain / read-only). rawTags is deliberately left null — the composer owns it. NullIsBad = NullIsBad ? true : null, + AllowWrites = AllowWrites ? true : null, Queries = Queries is { Count: > 0 } ? Queries : null, }; diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/SqlTagConfigEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/SqlTagConfigEditor.razor index 82213acb..acd1a9a2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/SqlTagConfigEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/SqlTagConfigEditor.razor @@ -87,6 +87,21 @@ else } +
+
+
+ + +
+
+ Also needs the driver's Allow writes and the tag's Writable flag. Writes are a single-row + UPDATE of the value column — KeyValue, or WideRow by where-pair only. +
+
+
+
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs index dba43dc2..14f503da 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/SqlTagConfigModel.cs @@ -54,6 +54,11 @@ public sealed class SqlTagConfigModel /// exactly one row. public SqlRowKeyModel RowKey { get; set; } = new(); + /// The tag's own write opt-in (the blob's writable). null ⇒ absent ⇒ read-only. Takes effect + /// only when the driver's allowWrites is on and the model names one row (KeyValue, or a WideRow where-pair); + /// the Tag's Writable flag still gates the OPC UA node. + public bool? Writable { get; set; } + /// Optional explicit OPC UA data type override (the blob's type field). null ⇒ the /// driver infers the type from the result-set column metadata. public DriverDataType? Type { get; set; } @@ -65,6 +70,9 @@ public sealed class SqlTagConfigModel private string? _invalidModelRaw; private string? _invalidTypeRaw; + // True when "writable" is present but not a JSON boolean — the parser refuses that tag. + private bool _invalidWritable; + // The blob this model was LOADED FROM, or null when built by object initialiser — the input to the // parse consult (see TagConfigParseGate). private string? _source; @@ -91,6 +99,9 @@ public sealed class SqlTagConfigModel TimestampColumn = TagConfigJson.GetString(o, "timestampColumn"), ColumnName = TagConfigJson.GetString(o, "columnName"), RowSelector = SqlRowSelectorModel.FromJson(selectorObj), + Writable = o.TryGetPropertyValue("writable", out var wn) && wn is JsonValue wv && wv.TryGetValue(out var wb) ? wb : null, + _invalidWritable = o.TryGetPropertyValue("writable", out var wn2) + && !(wn2 is JsonValue wv2 && wv2.TryGetValue(out _)), QueryName = TagConfigJson.GetString(o, "query"), Column = TagConfigJson.GetString(o, "column"), RowKey = SqlRowKeyModel.FromJson(o.TryGetPropertyValue("rowKey", out var rk) ? rk : null), @@ -117,6 +128,8 @@ public sealed class SqlTagConfigModel TagConfigJson.Set(_bag, "columnName", ColumnName); TagConfigJson.Set(_bag, "type", Type); TagConfigJson.Set(_bag, "query", QueryName); + if (Writable is { } writable) { _bag["writable"] = writable; } + else if (!_invalidWritable) { _bag.Remove("writable"); } TagConfigJson.Set(_bag, "column", Column); var rowKey = RowKey.ToJsonNode(); @@ -138,6 +151,7 @@ public sealed class SqlTagConfigModel // Strict enums (mirrors the parser's TryReadEnumStrict): a present-but-invalid string rejects the tag. if (_invalidModelRaw is not null) { return $"model: '{_invalidModelRaw}' is not a valid Sql tag model."; } if (_invalidTypeRaw is not null) { return $"type: '{_invalidTypeRaw}' is not a valid data type."; } + if (_invalidWritable && Writable is null) { return "writable must be true or false (a JSON boolean)."; } if (Model == SqlTagModel.Query) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/mysql/seed-mysql.sql b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/mysql/seed-mysql.sql index c605ae97..fd13d6b7 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/mysql/seed-mysql.sql +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/Docker/mysql/seed-mysql.sql @@ -6,8 +6,8 @@ -- backends. Two things are load-bearing: -- -- 1. The suite connects as `otopcua`, which holds SELECT on `sqlpoll` and nothing else (plus EXECUTE on the --- side-effect function, below). A MySQL "schema" is a database, and information_schema shows a principal --- only the objects it holds a privilege on — so MySqlDialect.ListSchemasSql returning `sqlpoll` for this +-- side-effect function and UPDATE on the one write-target table, below). A MySQL "schema" is a database, +-- and information_schema shows a principal only the objects it holds a privilege on — so MySqlDialect.ListSchemasSql returning `sqlpoll` for this -- login is the real provisioning check, not a superuser's view. -- 2. Every identifier is lower case. The suite authors the table name in UPPER case and expects the catalog -- gate to resolve it through a unique case-insensitive match: on Linux, MySQL table names are case- @@ -65,9 +65,23 @@ BEGIN END// DELIMITER ; +-- Write target (lmxopcua#753): the ONLY table the suite login may UPDATE. `dup` deliberately holds two rows +-- for one key, so a write keyed on it matches two rows and must be rolled back. tag_values stays SELECT-only, +-- so a write tag pointed there proves the server's own grant refuses it. +CREATE TABLE setpoints ( + tag_name varchar(128) NOT NULL, + sp_value double NULL +) ENGINE = InnoDB; +INSERT INTO setpoints (tag_name, sp_value) VALUES + ('Line1.SpeedSp', 10.0), + ('Line1.TempSp', 20.0), + ('dup', 1.0), + ('dup', 2.0); + -- ── the read-only login ────────────────────────────────────────────────────────────────────── DROP USER IF EXISTS 'otopcua'@'%'; CREATE USER 'otopcua'@'%' IDENTIFIED BY '__APP_PASSWORD__'; GRANT SELECT ON sqlpoll.* TO 'otopcua'@'%'; GRANT EXECUTE ON FUNCTION sqlpoll.bump_counter TO 'otopcua'@'%'; +GRANT UPDATE ON sqlpoll.setpoints TO 'otopcua'@'%'; FLUSH PRIVILEGES; diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/MySqlReadTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/MySqlReadTests.cs index ee922f10..75c75d2b 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/MySqlReadTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.IntegrationTests/MySqlReadTests.cs @@ -265,6 +265,88 @@ public sealed class MySqlReadTests (await _my.ScalarAsync("SELECT COUNT(*) FROM sqlpoll.side_effects")).ShouldBe(0L); } + // ---- writes (design §3.4) against a real server ---- + + /// All gates open: the UPDATE lands on exactly the keyed row, and the driver reads it back. + [Fact] + public async Task Write_realMySql_updatesTheKeyedRowOnly() + { + SkipUnlessLive(); + + await using var driver = await StartAsync(null, allowWrites: true, + Setpoint("Sql/Sp/Speed", "Line1.SpeedSp", writable: true)); + var target = 10.0 + Random.Shared.Next(1, 1000); + + (await WriteAsync(driver, "Sql/Sp/Speed", target)).ShouldBe(SqlStatusCodes.Good); + + Convert.ToDouble(await _my.ScalarRawAsync("SELECT sp_value FROM sqlpoll.setpoints WHERE tag_name = 'Line1.SpeedSp'"), + CultureInfo.InvariantCulture).ShouldBe(target); + Convert.ToDouble(await _my.ScalarRawAsync("SELECT sp_value FROM sqlpoll.setpoints WHERE tag_name = 'Line1.TempSp'"), + CultureInfo.InvariantCulture).ShouldBe(20.0); + Convert.ToDouble((await driver.ReadAsync(["Sql/Sp/Speed"], TestContext.Current.CancellationToken))[0].Value, + CultureInfo.InvariantCulture).ShouldBe(target); + } + + /// A key matching two rows is rolled back on InnoDB — neither row changes. + [Fact] + public async Task Write_realMySql_aKeyMatchingTwoRowsIsRolledBack() + { + SkipUnlessLive(); + + await using var driver = await StartAsync(null, allowWrites: true, Setpoint("Sql/Sp/Dup", "dup", writable: true)); + + (await WriteAsync(driver, "Sql/Sp/Dup", 99.0)).ShouldBe(SqlStatusCodes.BadConfigurationError); + (await _my.ScalarAsync("SELECT COUNT(*) FROM sqlpoll.setpoints WHERE tag_name = 'dup' AND sp_value = 99")).ShouldBe(0L); + } + + /// + /// Defence in depth: with every driver gate open, a table the login holds no UPDATE on is refused by the + /// server itself (the provider error surfaces as BadCommunicationError) and the row is unchanged. + /// + [Fact] + public async Task Write_realMySql_aTableWithoutAnUpdateGrantIsRefusedByTheServer() + { + SkipUnlessLive(); + + await using var driver = await StartAsync(null, allowWrites: true, Tag("Sql/Kv/Speed", new JsonObject + { + ["model"] = "KeyValue", ["table"] = MySqlFixture.Qualified("tag_values"), ["keyColumn"] = "tag_name", + ["keyValue"] = MySqlFixture.PresentKey, ["valueColumn"] = "num_value", ["writable"] = true, + })); + + (await WriteAsync(driver, "Sql/Kv/Speed", 1.0)).ShouldBe(SqlStatusCodes.BadCommunicationError); + Convert.ToDouble(await _my.ScalarRawAsync("SELECT num_value FROM sqlpoll.tag_values WHERE tag_name = 'Line1.Speed'"), + CultureInfo.InvariantCulture).ShouldBe(MySqlFixture.PresentValue); + } + + /// The default: no allowWrites ⇒ refused before any connection, whatever the tag says. + [Fact] + public async Task Write_realMySql_withoutAllowWritesIsRefused() + { + SkipUnlessLive(); + + await using var driver = await StartAsync(null, allowWrites: false, Setpoint("Sql/Sp/Temp", "Line1.TempSp", writable: true)); + + (await WriteAsync(driver, "Sql/Sp/Temp", 77.0)).ShouldBe(SqlStatusCodes.BadNotWritable); + Convert.ToDouble(await _my.ScalarRawAsync("SELECT sp_value FROM sqlpoll.setpoints WHERE tag_name = 'Line1.TempSp'"), + CultureInfo.InvariantCulture).ShouldBe(20.0); + } + + private static RawTagEntry Setpoint(string rawPath, string key, bool writable) => Tag(rawPath, new JsonObject + { + ["model"] = "KeyValue", + ["table"] = MySqlFixture.Qualified("setpoints"), + ["keyColumn"] = "tag_name", + ["keyValue"] = key, + ["valueColumn"] = "sp_value", + ["type"] = "Float64", + ["writable"] = writable, + }); + + private static async Task WriteAsync(SqlDriver driver, string reference, object? value) + => (await ((IWritable)driver).WriteAsync([new WriteRequest(reference, value)], TestContext.Current.CancellationToken)) + .ShouldHaveSingleItem().StatusCode; + // ---- helpers ---- private void SkipUnlessLive() @@ -274,13 +356,16 @@ public sealed class MySqlReadTests private Task StartAsync(params RawTagEntry[] tags) => StartAsync(queriesJson: null, tags); + private Task StartAsync(string? queriesJson, params RawTagEntry[] tags) + => StartAsync(queriesJson, allowWrites: false, tags); + /// /// Builds and initializes a driver through the PRODUCTION factory (provider: MySql, the connection /// string resolved from the fixture's connectionStringRef environment variable), so the suite /// exercises MySqlConnectorFactory.Instance and the factory's own named-query validation exactly as a /// deployed config would. /// - private async Task StartAsync(string? queriesJson, params RawTagEntry[] tags) + private async Task StartAsync(string? queriesJson, bool allowWrites, params RawTagEntry[] tags) { var rawTags = new JsonArray(); foreach (var tag in tags) @@ -302,6 +387,7 @@ public sealed class MySqlReadTests ["rawTags"] = rawTags, }; if (queriesJson is not null) config["queries"] = JsonNode.Parse(queriesJson); + if (allowWrites) config["allowWrites"] = true; var json = config.ToJsonString(); var driver = SqlDriverFactoryExtensions.CreateInstance("sql-mysql-integration", json); @@ -433,6 +519,15 @@ public sealed class MySqlFixture : IAsyncLifetime /// The statement. /// The scalar, as a 64-bit integer when numeric. public async Task ScalarAsync(string sql) + { + var value = await ScalarRawAsync(sql).ConfigureAwait(false); + return value is IConvertible and not string ? Convert.ToInt64(value, CultureInfo.InvariantCulture) : value; + } + + /// Runs a scalar query as the fixture login and returns the provider's value unconverted. + /// The statement. + /// The scalar. + public async Task ScalarRawAsync(string sql) { var connection = new MySqlDialect().Factory.CreateConnection() ?? throw new InvalidOperationException("MySqlConnector returned no connection object."); @@ -444,8 +539,7 @@ public sealed class MySqlFixture : IAsyncLifetime await using (command.ConfigureAwait(false)) { command.CommandText = sql; - var value = await command.ExecuteScalarAsync().ConfigureAwait(false); - return value is IConvertible and not string ? Convert.ToInt64(value, CultureInfo.InvariantCulture) : value; + return await command.ExecuteScalarAsync().ConfigureAwait(false); } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs index 52cfdd36..03fd2c47 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverFactoryTests.cs @@ -337,34 +337,43 @@ public sealed class SqlDriverFactoryTests options.RawTags.ShouldBeEmpty(); } - // ---- v1 is read-only, structurally ---- + // ---- allowWrites (lmxopcua#753): honoured now, default off ---- + + [Theory] + [InlineData(",\"allowWrites\":true", true)] + [InlineData(",\"allowWrites\":false", false)] + [InlineData("", false)] // absent ⇒ read-only + public void ParseBinding_carriesAllowWritesOntoTheOptions_defaultOff(string fragment, bool expected) + { + var name = NewRef(); + using var _ = new EnvironmentVariableScope( + SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db"); + + SqlDriverFactoryExtensions.ParseBinding("d1", $$"""{"connectionStringRef":"{{name}}"{{fragment}}}""") + .Options.AllowWrites.ShouldBe(expected); + } [Fact] - public void CreateInstance_anAuthoredAllowWrites_warnsLoudly_andTheDriverStaysReadOnly() + public void CreateInstance_withAllowWrites_logsNoWarning_andSaysWritesAreOn() { - // allowWrites is inert by construction (IWritable is not implemented), so honouring it is impossible - // and rejecting it would brick a deployment over a flag that cannot do harm. The remaining failure - // mode is an operator who believes writes are enabled — which only a loud warning closes. + // allowWrites used to be inert and drew a "this build ignores it" warning; it is honoured now, so the + // flag is an ordinary setting — reported in the configured line, never as a warning. var name = NewRef(); using var _ = new EnvironmentVariableScope( SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db"); var loggerFactory = new RecordingLoggerFactory(); - var driver = SqlDriverFactoryExtensions.CreateInstance( - "d1", $$"""{"connectionStringRef":"{{name}}","allowWrites":true}""", loggerFactory); + SqlDriverFactoryExtensions.CreateInstance( + "d1", $$"""{"connectionStringRef":"{{name}}","allowWrites":true}""", loggerFactory) + .ShouldBeAssignableTo(); - driver.ShouldNotBeAssignableTo(); - var warning = loggerFactory.Entries - .Where(e => e.Level >= LogLevel.Warning) - .ShouldHaveSingleItem(); - warning.Message.ShouldContain("allowWrites"); - warning.Message.ShouldContain("read-only"); + loggerFactory.Entries.Where(e => e.Level >= LogLevel.Warning).ShouldBeEmpty(); + loggerFactory.Entries.ShouldContain(e => e.Message.Contains("allowed for opted-in tags")); } [Fact] public void CreateInstance_withoutAllowWrites_logsNoWarning() { - // The falsifiability control for the warning above — a warning that always fires proves nothing. var name = NewRef(); using var _ = new EnvironmentVariableScope( SqlConnectionStringResolver.EnvironmentVariableFor(name), "Server=srv;Database=db"); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs index 5bc66c12..15e9ab72 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlDriverTests.cs @@ -46,8 +46,9 @@ public sealed class SqlDriverTests capture.Variables.ShouldContain(v => v.Info.FullName == "Speed" && v.Info.SecurityClass == SecurityClassification.ViewOnly); - // v1 is read-only by construction, not by configuration. - driver.ShouldNotBeAssignableTo(); + // Writes are off by default (no allowWrites): the driver is IWritable, but refuses. + (await ((IWritable)driver).WriteAsync([new WriteRequest("Speed", 1.0)], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode.ShouldBe(SqlStatusCodes.BadNotWritable); } [Fact] diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlWriteTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlWriteTests.cs new file mode 100644 index 00000000..e3045a70 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests/SqlWriteTests.cs @@ -0,0 +1,307 @@ +using System.Globalization; +using System.Text.Json.Nodes; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Sql.Tests; + +/// +/// The opt-in write path (design §3.4, lmxopcua#753): allowWrites + the tag's "writable": true +/// + a single-row model, executed as a parameterised UPDATE over catalog-validated identifiers that +/// commits only when exactly one row matched. Over SQLite, end to end through . +/// +public sealed class SqlWriteTests +{ + private const string DriverInstanceId = "sql-write"; + + // ---- tag opt-in parse ---- + + [Fact] + public void TryParse_readsWritable_andDefaultsItOff() + { + SqlEquipmentTagParser.TryParse(Kv(writable: true), "a", out var on).ShouldBeTrue(); + on.Writable.ShouldBeTrue(); + SqlEquipmentTagParser.TryParse(Kv(writable: null), "b", out var absent).ShouldBeTrue(); + absent.Writable.ShouldBeFalse(); + } + + [Theory] + [InlineData("\"true\"")] + [InlineData("1")] + [InlineData("null")] + public void TryParse_aNonBooleanWritable_refusesTheTag_andTheGateSaysWhy(string writable) + { + var json = $$"""{"model":"KeyValue","table":"T","keyColumn":"k","keyValue":"x","valueColumn":"v","writable":{{writable}}}"""; + SqlEquipmentTagParser.TryParse(json, "a", out _).ShouldBeFalse(); + SqlTagDefinitionFactory.Inspect(json).ShouldContain(w => w.Contains("writable", StringComparison.Ordinal)); + } + + // ---- planner: the only statement a write can become ---- + + [Fact] + public void Plan_keyValue_isASingleRowUpdate_withTheValueAndKeyBound() + { + var tag = new SqlTagDefinition("A", SqlTagModel.KeyValue, "dbo.TagValues", + KeyColumn: "tag_name", KeyValue: "Line1.Speed", ValueColumn: "num_value", Writable: true); + + SqlWritePlanner.TryPlan(tag, 12.5, new SqlServerDialect(), out var plan).ShouldBe(SqlStatusCodes.Good); + + plan!.SqlText.ShouldBe("UPDATE [dbo].[TagValues] SET [num_value] = @v WHERE [tag_name] = @k"); + plan.ParameterBindNames.ShouldBe(["@v", "@k"]); + plan.Parameters.ShouldBe([12.5, "Line1.Speed"]); + } + + [Fact] + public void Plan_wideRowWherePair_updatesTheSelectedRowsColumn_valueBeforeKeyForPositionalProviders() + { + var tag = new SqlTagDefinition("A", SqlTagModel.WideRow, "plant.status", ColumnName: "setpoint", + RowSelectorColumn: "station_id", RowSelectorValue: "7", Writable: true); + + SqlWritePlanner.TryPlan(tag, 3, new OdbcDialect(), out var plan).ShouldBe(SqlStatusCodes.Good); + + plan!.SqlText.ShouldBe("UPDATE \"plant\".\"status\" SET \"setpoint\" = ? WHERE \"station_id\" = ?"); + plan.Parameters.ShouldBe([3, "7"]); // marker order: SET value, then WHERE key + } + + [Fact] + public void Plan_aHostileValueIsBound_neverConcatenated() + { + const string hostile = "x'; DROP TABLE TagValues; --"; + var tag = new SqlTagDefinition("A", SqlTagModel.KeyValue, "T", KeyColumn: "k", KeyValue: "x", ValueColumn: "v"); + + SqlWritePlanner.TryPlan(tag, hostile, new SqlServerDialect(), out var plan).ShouldBe(SqlStatusCodes.Good); + + plan!.SqlText.ShouldNotContain("DROP"); + plan.Parameters[0].ShouldBe(hostile); + } + + [Fact] + public void Plan_refusesModelsWithNoSingleRowTarget() + { + var newest = new SqlTagDefinition("A", SqlTagModel.WideRow, "T", ColumnName: "c", RowSelectorTopByTimestamp: "ts"); + var query = new SqlTagDefinition("B", SqlTagModel.Query, string.Empty, ColumnName: "c", QueryName: "q"); + + SqlWritePlanner.TryPlan(newest, 1, new SqlServerDialect(), out _).ShouldBe(SqlStatusCodes.BadNotWritable); + SqlWritePlanner.TryPlan(query, 1, new SqlServerDialect(), out _).ShouldBe(SqlStatusCodes.BadNotWritable); + } + + [Fact] + public void Plan_coercesToTheDeclaredType_andRefusesWhatWillNotCoerce() + { + var tag = new SqlTagDefinition("A", SqlTagModel.KeyValue, "T", KeyColumn: "k", KeyValue: "x", ValueColumn: "v", + DeclaredType: DriverDataType.Int32); + + SqlWritePlanner.TryPlan(tag, "41", new SqlServerDialect(), out var plan).ShouldBe(SqlStatusCodes.Good); + plan!.Parameters[0].ShouldBe(41); + SqlWritePlanner.TryPlan(tag, "not a number", new SqlServerDialect(), out _).ShouldBe(SqlStatusCodes.BadTypeMismatch); + SqlWritePlanner.TryPlan(tag with { DeclaredType = null }, new object(), new SqlServerDialect(), out _) + .ShouldBe(SqlStatusCodes.BadTypeMismatch); + } + + // ---- the gates, end to end ---- + + [Fact] + public async Task Write_withAllowWritesOff_isRefused_andTheTableIsUntouched() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: false, Entry("Speed", Kv(writable: true))); + + (await WriteAsync(driver, "Speed", 99.0)).ShouldBe(SqlStatusCodes.BadNotWritable); + ValueOf(fixture, SqlitePollFixture.PresentKey).ShouldBe(SqlitePollFixture.PresentValue); + } + + [Fact] + public async Task Write_toATagThatDidNotOptIn_isRefused_evenWithAllowWritesOn() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, Entry("Speed", Kv(writable: null))); + + (await WriteAsync(driver, "Speed", 99.0)).ShouldBe(SqlStatusCodes.BadNotWritable); + ValueOf(fixture, SqlitePollFixture.PresentKey).ShouldBe(SqlitePollFixture.PresentValue); + } + + [Fact] + public async Task Write_allGatesOpen_updatesTheRow_andAReadSeesIt() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, + Entry("Speed", Kv(writable: true)), Entry("Pressure", Kv(writable: true, key: SqlitePollFixture.SecondPresentKey))); + + (await WriteAsync(driver, "Speed", 99.5)).ShouldBe(SqlStatusCodes.Good); + + ValueOf(fixture, SqlitePollFixture.PresentKey).ShouldBe(99.5); + ValueOf(fixture, SqlitePollFixture.SecondPresentKey).ShouldBe(SqlitePollFixture.SecondPresentValue); // only one row + var read = (await driver.ReadAsync(["Speed"], CancellationToken.None))[0]; + Convert.ToDouble(read.Value, CultureInfo.InvariantCulture).ShouldBe(99.5); + } + + [Fact] + public async Task Write_aWideRowWherePair_updatesThatStationsColumnOnly() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, Entry("Oven7", $$""" + {"model":"WideRow","table":"{{SqlitePollFixture.WideRowTable}}","columnName":"oven_temp", + "rowSelector":{"whereColumn":"station_id","whereValue":7},"writable":true} + """)); + + (await WriteAsync(driver, "Oven7", 190.0)).ShouldBe(SqlStatusCodes.Good); + + Scalar(fixture, "SELECT oven_temp FROM LatestStatus WHERE station_id = 7").ShouldBe(190.0); + Scalar(fixture, "SELECT oven_temp FROM LatestStatus WHERE station_id = 8").ShouldBe(SqlitePollFixture.NewestStationOvenTemp); + } + + [Fact] + public async Task Write_toAnAbsentKey_isBadNoData_andInsertsNothing() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, + Entry("Missing", Kv(writable: true, key: SqlitePollFixture.AbsentKey))); + + (await WriteAsync(driver, "Missing", 1.0)).ShouldBe(SqlStatusCodes.BadNoData); + Scalar(fixture, "SELECT COUNT(*) FROM TagValues").ShouldBe(3.0); + } + + /// + /// A key that matches two rows would update both. The write is rolled back and refused, so neither row + /// changes — the write-side twin of the read path's duplicate-key refusal. + /// + [Fact] + public async Task Write_matchingMoreThanOneRow_isRolledBack_andRefused() + { + using var fixture = new SqlitePollFixture(); + fixture.Execute($"INSERT INTO TagValues (tag_name, num_value, sample_ts) VALUES ('{SqlitePollFixture.PresentKey}', 7.0, '2026-07-24T11:00:00Z')"); + await using var driver = await StartAsync(fixture, allowWrites: true, Entry("Speed", Kv(writable: true))); + + (await WriteAsync(driver, "Speed", 55.0)).ShouldBe(SqlStatusCodes.BadConfigurationError); + + Scalar(fixture, $"SELECT COUNT(*) FROM TagValues WHERE num_value = 55.0").ShouldBe(0.0); + } + + [Fact] + public async Task Write_toAnUnknownOrGateRejectedTag_isBadNodeIdUnknown() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, Entry("Ghost", """ + {"model":"KeyValue","table":"NoSuchTable","keyColumn":"k","keyValue":"x","valueColumn":"v","writable":true} + """)); + + (await WriteAsync(driver, "Ghost", 1.0)).ShouldBe(SqlStatusCodes.BadNodeIdUnknown); // the catalog gate rejected it + (await WriteAsync(driver, "Nobody", 1.0)).ShouldBe(SqlStatusCodes.BadNodeIdUnknown); + } + + [Fact] + public async Task Write_aHostileString_isStoredVerbatim() + { + using var fixture = new SqlitePollFixture(); + fixture.Execute("CREATE TABLE Notes (k TEXT NOT NULL, note TEXT NULL)"); + fixture.Execute("INSERT INTO Notes VALUES ('a', 'old')"); + const string hostile = "x'); DROP TABLE Notes; --"; + await using var driver = await StartAsync(fixture, allowWrites: true, Entry("Note", """ + {"model":"KeyValue","table":"Notes","keyColumn":"k","keyValue":"a","valueColumn":"note","writable":true,"type":"String"} + """)); + + (await WriteAsync(driver, "Note", hostile)).ShouldBe(SqlStatusCodes.Good); + + using var command = fixture.Connection.CreateCommand(); + command.CommandText = "SELECT note FROM Notes WHERE k = 'a'"; + command.ExecuteScalar().ShouldBe(hostile); + } + + [Fact] + public async Task Discover_advertisesOperate_onlyWhereAWriteCouldSucceed() + { + using var fixture = new SqlitePollFixture(); + await using var driver = await StartAsync(fixture, allowWrites: true, + Entry("Writable", Kv(writable: true)), + Entry("ReadOnly", Kv(writable: null)), + Entry("Newest", $$""" + {"model":"WideRow","table":"{{SqlitePollFixture.WideRowTable}}","columnName":"oven_temp", + "rowSelector":{"topByTimestamp":"sample_ts"},"writable":true} + """)); + + var builder = new ClassCapturingBuilder(); + await driver.DiscoverAsync(builder, CancellationToken.None); + + builder.Classes["Writable"].ShouldBe(SecurityClassification.Operate); + builder.Classes["ReadOnly"].ShouldBe(SecurityClassification.ViewOnly); + builder.Classes["Newest"].ShouldBe(SecurityClassification.ViewOnly); // no single-row target + } + + // ---- helpers ---- + + private static string Kv(bool? writable, string key = SqlitePollFixture.PresentKey) + { + var o = new JsonObject + { + ["model"] = "KeyValue", + ["table"] = SqlitePollFixture.KeyValueTable, + ["keyColumn"] = SqlitePollFixture.KeyColumn, + ["keyValue"] = key, + ["valueColumn"] = SqlitePollFixture.ValueColumn, + ["type"] = "Float64", + }; + if (writable is { } w) o["writable"] = w; + return o.ToJsonString(); + } + + private static RawTagEntry Entry(string rawPath, string json) => new(rawPath, json, WriteIdempotent: false); + + private static async Task WriteAsync(SqlDriver driver, string reference, object? value) + => (await ((IWritable)driver).WriteAsync([new WriteRequest(reference, value)], CancellationToken.None)) + .ShouldHaveSingleItem().StatusCode; + + private static double ValueOf(SqlitePollFixture fixture, string key) + => Scalar(fixture, $"SELECT num_value FROM TagValues WHERE tag_name = '{key}'"); + + private static double Scalar(SqlitePollFixture fixture, string sql) + { + using var command = fixture.Connection.CreateCommand(); + command.CommandText = sql; + return Convert.ToDouble(command.ExecuteScalar(), CultureInfo.InvariantCulture); + } + + private static async Task StartAsync(SqlitePollFixture fixture, bool allowWrites, params RawTagEntry[] tags) + { + var driver = new SqlDriver( + new SqlDriverOptions + { + RawTags = tags, + OperationTimeout = TimeSpan.FromSeconds(15), + CommandTimeout = TimeSpan.FromSeconds(10), + AllowWrites = allowWrites, + }, + DriverInstanceId, new SqliteDialect(), fixture.ConnectionString, fixture.Factory); + await driver.InitializeAsync("{}", CancellationToken.None); + return driver; + } + + /// Records each discovered variable's security classification. + private sealed class ClassCapturingBuilder : IAddressSpaceBuilder + { + public Dictionary Classes { get; } = []; + + public IAddressSpaceBuilder Folder(string browseName, string displayName) => this; + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo attributeInfo) + { + Classes[attributeInfo.FullName] = attributeInfo.SecurityClass; + return new Handle(attributeInfo.FullName); + } + + public void AddProperty(string browseName, DriverDataType dataType, object? value) { } + + private sealed class Handle(string fullReference) : IVariableHandle + { + public string FullReference => fullReference; + + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new Sink(); + + private sealed class Sink : IAlarmConditionSink + { + public void OnTransition(AlarmEventArgs args) { } + } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlDriverFormContractTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlDriverFormContractTests.cs index 69770dfd..aeda0d25 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlDriverFormContractTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlDriverFormContractTests.cs @@ -14,7 +14,7 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; /// (1) the exact JSON bytes the form emits for a minimal + fuller config are accepted by the authoritative /// reader, ; and (2) the /// serialization contract — provider as a NAME string (never an ordinal), camelCase keys, and no -/// composer-owned rawTags / inert allowWrites leaking into the authored blob. A numeric enum +/// composer-owned rawTags or an unrequested allowWrites leaking into the authored blob. A numeric enum /// or a stray connection string is the "authors fine, never reads / leaks a secret" defect class this guards. /// public sealed class SqlDriverFormContractTests @@ -74,7 +74,7 @@ public sealed class SqlDriverFormContractTests json.ShouldBe("""{"provider":"SqlServer","connectionStringRef":"MesStaging"}"""); json.ShouldNotContain("\"provider\":0"); // never an ordinal — that faults the string-typed factory read json.ShouldNotContain("rawTags"); // composer-owned; never authored by the form - json.ShouldNotContain("allowWrites"); // inert in v1 (read-only); never authored + json.ShouldNotContain("allowWrites"); // off ⇒ omitted ⇒ read-only json.ShouldNotContain("defaultPollInterval"); // null optional omitted ⇒ factory default applies } @@ -169,6 +169,32 @@ public sealed class SqlDriverFormContractTests } } + [Fact] + public void AllowWrites_is_emitted_only_when_switched_on_and_the_factory_honours_it() + { + var off = JsonSerializer.Serialize(new SqlDriverForm.FormModel { ConnectionStringRef = "R" }.ToDto(), FormOptions); + off.ShouldNotContain("allowWrites"); + + var form = new SqlDriverForm.FormModel { ConnectionStringRef = "AdminUiFormTest", AllowWrites = true }; + var on = JsonSerializer.Serialize(form.ToDto(), FormOptions); + on.ShouldContain("\"allowWrites\":true"); + SqlDriverForm.FormModel.FromDto(JsonSerializer.Deserialize(on, FormOptions)!) + .AllowWrites.ShouldBeTrue(); + + var envVar = SqlConnectionStringResolver.EnvironmentVariableFor("AdminUiFormTest"); + var previous = Environment.GetEnvironmentVariable(envVar); + Environment.SetEnvironmentVariable(envVar, "Server=localhost;Database=Test;Trusted_Connection=True;"); + try + { + SqlDriverFactoryExtensions.CreateInstance("adminui-form-test", on) + .ShouldBeAssignableTo(); + } + finally + { + Environment.SetEnvironmentVariable(envVar, previous); + } + } + [Fact] public void Queries_survive_the_form_model_round_trip_and_a_typo_keeps_the_last_valid_set() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs index 413e23de..6a5b655a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/SqlTagConfigModelTests.cs @@ -258,6 +258,27 @@ public sealed class SqlTagConfigModelTests def.TimestampColumn.ShouldBe("ts"); } + [Fact] + public void Writable_round_trips_as_a_json_boolean_and_absent_means_read_only() + { + var on = SqlTagConfigModel.FromJson("""{"model":"KeyValue","table":"t","keyColumn":"k","keyValue":"v","valueColumn":"c","writable":true}"""); + on.Writable.ShouldBe(true); + on.ToJson().ShouldContain("\"writable\":true"); + SqlEquipmentTagParser.TryParse(on.ToJson(), "raw", out var def).ShouldBeTrue(); + def.Writable.ShouldBeTrue(); + + on.Writable = null; + on.ToJson().ShouldNotContain("writable"); + } + + [Theory] + [InlineData("\"true\"")] + [InlineData("1")] + public void Validate_rejects_a_non_boolean_writable(string writable) + => SqlTagConfigModel.FromJson( + $$"""{"model":"KeyValue","table":"t","keyColumn":"k","keyValue":"v","valueColumn":"c","writable":{{writable}}}""") + .Validate().ShouldNotBeNull(); + [Fact] public void Clearing_the_row_key_omits_it_from_the_blob() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs index 8fa9689b..0fc879cb 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/TagConfigNonEnumParityTests.cs @@ -410,6 +410,12 @@ public sealed class TagConfigNonEnumParityTests (DriverTypeNames.Sql, """{"model":"Query","query":"lineStatus","column":"speed","rowKey":"line"}"""), (DriverTypeNames.Sql, """{"model":"Query","query":"lineStatus"}"""), (DriverTypeNames.Sql, """{"model":"Query","column":"speed","query":" "}"""), + // lmxopcua#753 — the per-tag write opt-in is a strict JSON boolean on both sides. + (DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","valueColumn":"v","keyValue":"x","writable":true}"""), + (DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","valueColumn":"v","keyValue":"x","writable":false}"""), + (DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","valueColumn":"v","keyValue":"x","writable":"true"}"""), + (DriverTypeNames.Sql, """{"model":"KeyValue","table":"dbo.T","keyColumn":"k","valueColumn":"v","keyValue":"x","writable":1}"""), + (DriverTypeNames.Sql, """{"model":"Query","query":"q","column":"c","writable":null}"""), (DriverTypeNames.Sql, """{"model":"WideRow","table":"dbo.T","columnName":"Speed","rowSelector":{"whereColumn":"LineId","whereValue":"L1"}}"""), (DriverTypeNames.Sql, """{"model":"WideRow","table":"dbo.T","columnName":"Speed"}"""),