feat(deploy): reject Sql Query tags naming undeclared queries; warn on Writable/writable disagreement (lmxopcua#781)
v2-ci / unit-tests (push) Has been cancelled
v2-ci / unit-tests (push) Has been cancelled
Item 5: DraftValidator now emits SqlQueryNotDeclared (always blocking) for a Query tag whose query is absent from the merged driver config's queries map, via the driver's own tag parse. Item 3: StartDeployment appends a non-blocking warning to the accepted message when the Tag entity's AccessLevel and the Sql blob's writable flag disagree. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
@@ -41,6 +41,7 @@ public static class DraftValidator
|
||||
ValidateEquipReferenceResolution(draft, errors);
|
||||
ValidateCalculationTags(draft, errors);
|
||||
ValidateSqlConnectionStringNotPersisted(draft, errors);
|
||||
errors.AddRange(SqlTagDeployChecks.UndeclaredQueryErrors(draft));
|
||||
ValidateMTConnectSingleDevice(draft, errors);
|
||||
ValidateNodeAclScopes(draft, errors);
|
||||
return errors;
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
using System.Text.Json;
|
||||
using ZB.MOM.WW.OtOpcUa.Commons.Types;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
||||
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
||||
using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Configuration.Validation;
|
||||
|
||||
/// <summary>
|
||||
/// Deploy-gate checks for Sql tags that need a fact outside the tag's own blob (lmxopcua#781 items 3 and 5).
|
||||
/// <para>Both go through <see cref="SqlEquipmentTagParser.TryParse"/> — the driver's own runtime parse — so a
|
||||
/// blob the driver would refuse anyway (already reported by the per-tag inspector) is never double-reported
|
||||
/// here, and nothing here can be stricter than the runtime.</para>
|
||||
/// </summary>
|
||||
public static class SqlTagDeployChecks
|
||||
{
|
||||
/// <summary>Error code for a Query tag naming a query its driver instance does not declare.</summary>
|
||||
public const string UndeclaredQueryCode = "SqlQueryNotDeclared";
|
||||
|
||||
/// <summary>
|
||||
/// Item 5. A <c>Query</c>-model tag whose <c>query</c> is not a key of the driver instance's
|
||||
/// <c>queries</c> map is skipped by the driver at Initialize (BadNodeIdUnknown, reason on /hosts). The
|
||||
/// <c>queries</c> map is read from the same merged config the driver binds
|
||||
/// (<see cref="DriverDeviceConfigMerger"/>: DriverConfig plus, for a single-device instance, the
|
||||
/// DeviceConfig), keys matched ordinally as the driver does.
|
||||
/// </summary>
|
||||
/// <param name="draft">The snapshot being validated.</param>
|
||||
/// <returns>One error per offending tag; empty when every Query tag names a declared query.</returns>
|
||||
public static IReadOnlyList<ValidationError> UndeclaredQueryErrors(DraftSnapshot draft)
|
||||
{
|
||||
var errors = new List<ValidationError>();
|
||||
foreach (var (driver, devices) in SqlInstances(draft))
|
||||
{
|
||||
HashSet<string>? declared = null;
|
||||
foreach (var tag in draft.Tags)
|
||||
{
|
||||
if (!devices.Any(d => string.Equals(d.DeviceId, tag.DeviceId, StringComparison.Ordinal))) continue;
|
||||
if (!SqlEquipmentTagParser.TryParse(tag.TagConfig, tag.Name, out var def)
|
||||
|| def.Model != SqlTagModel.Query) continue;
|
||||
declared ??= DeclaredQueries(driver, devices);
|
||||
if (declared.Contains(def.QueryName!)) continue;
|
||||
errors.Add(new(UndeclaredQueryCode,
|
||||
$"Sql tag '{tag.TagId}' reads named query '{def.QueryName}', which Sql driver instance " +
|
||||
$"'{driver.DriverInstanceId}' does not declare under 'queries' — the driver would skip the tag " +
|
||||
"and it would read BadNodeIdUnknown. Declare the query in the driver config or fix the tag's 'query'.",
|
||||
tag.TagId));
|
||||
}
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Item 3. The node's write permission needs BOTH the Tag entity's <c>AccessLevel</c> (ReadWrite) and the
|
||||
/// Sql blob's <c>"writable": true</c>; when they disagree the tag is silently read-only. A warning, never
|
||||
/// an error: both read-only combinations are legal states, just usually not the intended one.
|
||||
/// </summary>
|
||||
/// <param name="draft">The snapshot being validated.</param>
|
||||
/// <returns>One human-readable warning per disagreeing tag.</returns>
|
||||
public static IReadOnlyList<string> WritableMismatchWarnings(DraftSnapshot draft)
|
||||
{
|
||||
var warnings = new List<string>();
|
||||
foreach (var (_, devices) in SqlInstances(draft))
|
||||
{
|
||||
foreach (var tag in draft.Tags)
|
||||
{
|
||||
if (!devices.Any(d => string.Equals(d.DeviceId, tag.DeviceId, StringComparison.Ordinal))) continue;
|
||||
if (!SqlEquipmentTagParser.TryParse(tag.TagConfig, tag.Name, out var def)) continue;
|
||||
var entityWritable = tag.AccessLevel == TagAccessLevel.ReadWrite;
|
||||
if (entityWritable == def.Writable) continue;
|
||||
warnings.Add(entityWritable
|
||||
? $"tag '{tag.TagId}': Writable flag is on but the Sql TagConfig lacks \"writable\": true — the driver will refuse writes to it."
|
||||
: $"tag '{tag.TagId}': Sql TagConfig says \"writable\": true but the tag's Writable flag is off — the node will be read-only.");
|
||||
}
|
||||
}
|
||||
|
||||
return warnings;
|
||||
}
|
||||
|
||||
private static IEnumerable<(DriverInstance Driver, List<Device> Devices)> SqlInstances(DraftSnapshot draft)
|
||||
{
|
||||
foreach (var driver in draft.DriverInstances)
|
||||
{
|
||||
if (!string.Equals(driver.DriverType, DriverTypeNames.Sql, StringComparison.Ordinal)) continue;
|
||||
yield return (driver, draft.Devices
|
||||
.Where(d => string.Equals(d.DriverInstanceId, driver.DriverInstanceId, StringComparison.Ordinal))
|
||||
.ToList());
|
||||
}
|
||||
}
|
||||
|
||||
private static HashSet<string> DeclaredQueries(DriverInstance driver, List<Device> devices)
|
||||
{
|
||||
var set = new HashSet<string>(StringComparer.Ordinal);
|
||||
var merged = DriverDeviceConfigMerger.Merge(
|
||||
driver.DriverConfig, devices.Select(d => new DriverDeviceConfigMerger.DeviceRow(d.Name, d.DeviceConfig)).ToList(), []);
|
||||
using var doc = JsonDocument.Parse(merged);
|
||||
foreach (var p in doc.RootElement.EnumerateObject())
|
||||
{
|
||||
if (!string.Equals(p.Name, "queries", StringComparison.OrdinalIgnoreCase)
|
||||
|| p.Value.ValueKind != JsonValueKind.Object) continue;
|
||||
foreach (var q in p.Value.EnumerateObject()) set.Add(q.Name);
|
||||
}
|
||||
|
||||
return set;
|
||||
}
|
||||
}
|
||||
@@ -44,6 +44,9 @@
|
||||
<!-- WP7: the deploy-gate cycle check reuses DependencyGraph (Tarjan SCC) for the calc→calc
|
||||
dependency-cycle rule. Cycle-safe — Core.VirtualTags does not reference Configuration. -->
|
||||
<ProjectReference Include="..\ZB.MOM.WW.OtOpcUa.Core.VirtualTags\ZB.MOM.WW.OtOpcUa.Core.VirtualTags.csproj"/>
|
||||
<!-- lmxopcua#781: the Sql deploy checks reuse the driver's own tag parse. Cycle-safe — Sql.Contracts
|
||||
references only Core.Abstractions. -->
|
||||
<ProjectReference Include="..\..\Drivers\ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts\ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts.csproj"/>
|
||||
</ItemGroup>
|
||||
|
||||
</Project>
|
||||
|
||||
@@ -465,6 +465,12 @@ public sealed class AdminOperationsActor : ReceiveActor
|
||||
_log.Warning("StartDeployment: TagConfig warning (Warn mode, non-blocking): {Warning}", x.Text);
|
||||
}
|
||||
|
||||
// lmxopcua#781 item 3: always non-blocking (independent of TagConfigValidationMode) — the Tag
|
||||
// entity's Writable flag and the Sql blob's "writable" disagree.
|
||||
var writableWarnings = SqlTagDeployChecks.WritableMismatchWarnings(draft);
|
||||
foreach (var w in writableWarnings)
|
||||
_log.Warning("StartDeployment: Sql writable mismatch (non-blocking): {Warning}", w);
|
||||
|
||||
if (errors.Count > 0)
|
||||
{
|
||||
var summary = string.Join("; ", errors.Select(e => $"[{e.Code}] {e.Message}"));
|
||||
@@ -547,6 +553,7 @@ public sealed class AdminOperationsActor : ReceiveActor
|
||||
var messageParts = new List<string>();
|
||||
if (advisory is not null) messageParts.Add(advisory);
|
||||
messageParts.AddRange(tagConfigWarnings.Select(x => x.Text));
|
||||
messageParts.AddRange(writableWarnings);
|
||||
var acceptedMessage = messageParts.Count > 0 ? string.Join("; ", messageParts) : null;
|
||||
|
||||
return new ComposeOutcome(
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Entities;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
||||
using ZB.MOM.WW.OtOpcUa.Configuration.Validation;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Configuration.Tests;
|
||||
|
||||
/// <summary>lmxopcua#781 items 5 and 3: Sql deploy checks that need the driver config / Tag entity.</summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class DraftValidatorSqlQueryTests
|
||||
{
|
||||
private const string Code = SqlTagDeployChecks.UndeclaredQueryCode;
|
||||
private const string Declared =
|
||||
"""{"queries":{"line":{"sql":"SELECT 1 AS v","keyColumns":[]}}}""";
|
||||
|
||||
private static string QueryTag(string query, bool? writable = null) =>
|
||||
$$"""{"model":"Query","query":"{{query}}","column":"v"{{(writable is null ? "" : $",\"writable\":{writable.ToString()!.ToLowerInvariant()}")}}}""";
|
||||
|
||||
private static DraftSnapshot Draft(string driverConfig, string tagConfig,
|
||||
TagAccessLevel access = TagAccessLevel.Read, string deviceConfig = "{}") => new()
|
||||
{
|
||||
GenerationId = 1, ClusterId = "c",
|
||||
DriverInstances = [new DriverInstance
|
||||
{ DriverInstanceId = "di", ClusterId = "c", Name = "sql", DriverType = "Sql", DriverConfig = driverConfig }],
|
||||
Devices = [new Device { DeviceId = "dev", DriverInstanceId = "di", Name = "d", DeviceConfig = deviceConfig }],
|
||||
Tags = [new Tag { TagId = "t1", DeviceId = "dev", Name = "t", DataType = "Int32",
|
||||
AccessLevel = access, TagConfig = tagConfig }],
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public void Query_tag_naming_an_undeclared_query_is_a_deploy_error()
|
||||
{
|
||||
var errors = DraftValidator.Validate(Draft(Declared, QueryTag("nope")));
|
||||
errors.ShouldContain(e => e.Code == Code && e.Context == "t1" && e.Message.Contains("'nope'"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Query_tag_with_no_queries_map_at_all_is_a_deploy_error()
|
||||
=> DraftValidator.Validate(Draft("{}", QueryTag("line"))).ShouldContain(e => e.Code == Code);
|
||||
|
||||
[Fact]
|
||||
public void Positive_control_declared_query_is_quiet()
|
||||
=> DraftValidator.Validate(Draft(Declared, QueryTag("line"))).ShouldNotContain(e => e.Code == Code);
|
||||
|
||||
[Fact]
|
||||
public void Query_declared_on_the_single_device_config_counts_as_declared()
|
||||
=> DraftValidator.Validate(Draft("{}", QueryTag("line"), deviceConfig: Declared))
|
||||
.ShouldNotContain(e => e.Code == Code);
|
||||
|
||||
[Fact]
|
||||
public void Query_names_are_ordinal()
|
||||
=> DraftValidator.Validate(Draft(Declared, QueryTag("LINE"))).ShouldContain(e => e.Code == Code);
|
||||
|
||||
[Fact]
|
||||
public void Non_query_models_are_not_checked()
|
||||
=> DraftValidator.Validate(Draft("{}",
|
||||
"""{"model":"KeyValue","table":"t","keyColumn":"k","keyValue":"a","valueColumn":"v"}"""))
|
||||
.ShouldNotContain(e => e.Code == Code);
|
||||
|
||||
[Theory]
|
||||
[InlineData(TagAccessLevel.ReadWrite, false, true)]
|
||||
[InlineData(TagAccessLevel.Read, true, true)]
|
||||
[InlineData(TagAccessLevel.ReadWrite, true, false)]
|
||||
[InlineData(TagAccessLevel.Read, false, false)]
|
||||
public void Writable_flag_disagreement_warns_and_agreement_is_quiet(TagAccessLevel access, bool writable, bool warns)
|
||||
{
|
||||
var warnings = SqlTagDeployChecks.WritableMismatchWarnings(Draft(Declared, QueryTag("line", writable), access));
|
||||
(warnings.Count == 1).ShouldBe(warns);
|
||||
if (warns) warnings[0].ShouldContain("t1");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Writable_mismatch_is_never_a_validation_error()
|
||||
=> DraftValidator.Validate(Draft(Declared, QueryTag("line", true), TagAccessLevel.Read)).ShouldBeEmpty();
|
||||
}
|
||||
+45
@@ -124,4 +124,49 @@ public sealed class AdminOperationsActorTagConfigGateTests : ControlPlaneActorTe
|
||||
coordinator.ExpectMsg<DispatchDeployment>(PresenceBudget);
|
||||
ExpectMsg<StartDeploymentResult>(PresenceBudget).Outcome.ShouldBe(StartDeploymentOutcome.Accepted);
|
||||
}
|
||||
|
||||
// lmxopcua#781 item 3 — through the actor, in Error mode too: the writable mismatch is a warning, never a reject.
|
||||
private const string SqlQueries = "{\"queries\":{\"line\":{\"sql\":\"SELECT 1 AS v\",\"keyColumns\":[]}}}";
|
||||
|
||||
private StartDeploymentResult DeploySql(bool writableInBlob, TagAccessLevel access)
|
||||
{
|
||||
var dbFactory = NewInMemoryDbFactory();
|
||||
using (var db = dbFactory.CreateDbContext())
|
||||
{
|
||||
db.DriverInstances.Add(new DriverInstance
|
||||
{ DriverInstanceId = "d-1", ClusterId = "", Name = "drv", DriverType = "Sql", DriverConfig = SqlQueries });
|
||||
db.Devices.Add(new Device { DeviceId = "dev-1", DriverInstanceId = "d-1", Name = "dev", DeviceConfig = "{}" });
|
||||
db.Tags.Add(new Tag
|
||||
{
|
||||
TagId = "tag-1", DeviceId = "dev-1", Name = "flow", DataType = "Int32", AccessLevel = access,
|
||||
TagConfig = "{\"model\":\"Query\",\"query\":\"line\",\"column\":\"v\",\"writable\":"
|
||||
+ (writableInBlob ? "true" : "false") + "}",
|
||||
});
|
||||
db.SaveChanges();
|
||||
}
|
||||
var coordinator = CreateTestProbe("coord");
|
||||
var actor = Sys.ActorOf(AdminOperationsActor.Props(
|
||||
dbFactory, coordinator.Ref, Enumerable.Empty<IDriverProbe>(), TagConfigValidationMode.Error));
|
||||
actor.Tell(new StartDeployment("joe", CorrelationId.NewId()));
|
||||
coordinator.ExpectMsg<DispatchDeployment>(PresenceBudget);
|
||||
return ExpectMsg<StartDeploymentResult>(PresenceBudget);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Sql_writable_mismatch_is_accepted_with_a_warning_even_in_error_mode()
|
||||
{
|
||||
var reply = DeploySql(writableInBlob: true, TagAccessLevel.Read);
|
||||
reply.Outcome.ShouldBe(StartDeploymentOutcome.Accepted);
|
||||
reply.Message.ShouldNotBeNull();
|
||||
reply.Message.ShouldContain("tag-1");
|
||||
reply.Message.ShouldContain("writable");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Sql_writable_agreement_adds_no_warning()
|
||||
{
|
||||
var reply = DeploySql(writableInBlob: true, TagAccessLevel.ReadWrite);
|
||||
reply.Outcome.ShouldBe(StartDeploymentOutcome.Accepted);
|
||||
reply.Message.ShouldBeNull();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user