feat(transport): wire full SemanticValidator at bundle import time
This commit is contained in:
Joseph Doherty
@@ -11,7 +11,10 @@ using ScadaLink.Commons.Interfaces.Repositories;
|
||||
using ScadaLink.Commons.Interfaces.Services;
|
||||
using ScadaLink.Commons.Interfaces.Transport;
|
||||
using ScadaLink.Commons.Types.Transport;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
using ScadaLink.Commons.Types.Flattening;
|
||||
using ScadaLink.ConfigurationDatabase;
|
||||
using ScadaLink.TemplateEngine.Validation;
|
||||
using ScadaLink.Transport.Encryption;
|
||||
using ScadaLink.Transport.Serialization;
|
||||
|
||||
@@ -63,6 +66,7 @@ public sealed class BundleImporter : IBundleImporter
|
||||
private readonly IBundleSessionStore _sessionStore;
|
||||
private readonly IOptions<TransportOptions> _options;
|
||||
private readonly TimeProvider _timeProvider;
|
||||
private readonly SemanticValidator _semanticValidator;
|
||||
|
||||
public BundleImporter(
|
||||
BundleSerializer bundleSerializer,
|
||||
@@ -78,7 +82,8 @@ public sealed class BundleImporter : IBundleImporter
|
||||
IInboundApiRepository inboundApiRepo,
|
||||
IAuditService auditService,
|
||||
IAuditCorrelationContext correlationContext,
|
||||
ScadaLinkDbContext dbContext)
|
||||
ScadaLinkDbContext dbContext,
|
||||
SemanticValidator semanticValidator)
|
||||
{
|
||||
_bundleSerializer = bundleSerializer ?? throw new ArgumentNullException(nameof(bundleSerializer));
|
||||
_manifestValidator = manifestValidator ?? throw new ArgumentNullException(nameof(manifestValidator));
|
||||
@@ -94,6 +99,7 @@ public sealed class BundleImporter : IBundleImporter
|
||||
_auditService = auditService ?? throw new ArgumentNullException(nameof(auditService));
|
||||
_correlationContext = correlationContext ?? throw new ArgumentNullException(nameof(correlationContext));
|
||||
_dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
|
||||
_semanticValidator = semanticValidator ?? throw new ArgumentNullException(nameof(semanticValidator));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -448,17 +454,20 @@ public sealed class BundleImporter : IBundleImporter
|
||||
/// later category can resolve name-keyed references to earlier ones.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Semantic validation is the minimal v1 variant: every script-callable
|
||||
/// identifier referenced by the merged target must resolve to either a
|
||||
/// pre-existing or in-bundle <c>SharedScript</c> / <c>ExternalSystem</c>.
|
||||
/// Wiring the full <see cref="TemplateEngine.Validation.SemanticValidator"/>
|
||||
/// requires running the flattening pipeline over the merged target, which
|
||||
/// isn't reachable from the import path without a fixture — deferred to a
|
||||
/// follow-up; today's check catches the same crash surface the operator
|
||||
/// would otherwise hit at deploy time. The minimal check is run AGAINST the
|
||||
/// merged target (incoming-bundle DTOs already in memory, target DB read
|
||||
/// Semantic validation is two-tier: a minimal name-resolution scan first
|
||||
/// (every script-callable identifier referenced by the merged target must
|
||||
/// resolve to either a pre-existing or in-bundle <c>SharedScript</c> /
|
||||
/// <c>ExternalSystem</c>), then — on Pass 1 success — the full
|
||||
/// <see cref="TemplateEngine.Validation.SemanticValidator"/> over each
|
||||
/// imported template scoped to its own single-template
|
||||
/// <c>FlattenedConfiguration</c>. The minimal pass is run AGAINST the
|
||||
/// merged target (incoming-bundle DTOs in memory plus the target DB read
|
||||
/// inside the transaction) so a Skip resolution can legitimately fail
|
||||
/// validation if it would have provided a missing dependency.
|
||||
/// validation if it would have provided a missing dependency. The full
|
||||
/// pass scopes to imported templates only — pre-existing untouched
|
||||
/// templates aren't revalidated so a latent issue elsewhere in the
|
||||
/// catalog doesn't block this import. See <see cref="RunSemanticValidationAsync"/>
|
||||
/// for the per-pass contract.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Audit-row contract: every per-entity write goes through
|
||||
@@ -1511,19 +1520,32 @@ public sealed class BundleImporter : IBundleImporter
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Minimal v1 semantic validation: scan every TemplateScript / ApiMethod
|
||||
/// body in the (post-merge) target for identifier-shaped references that
|
||||
/// cannot resolve to either a pre-existing or in-bundle SharedScript /
|
||||
/// ExternalSystem. Mirrors the algorithm used by <c>DetectBlockersAsync</c>
|
||||
/// in the preview path, but operates against the actual merge result —
|
||||
/// Skip-resolved DTOs are excluded from the in-bundle name set, so a Skip
|
||||
/// that would have provided a dependency surfaces here as an error.
|
||||
/// Two-tier semantic validation run before any rows are flushed:
|
||||
/// <list type="number">
|
||||
/// <item><b>Pass 1 — minimal name-resolution scan.</b> Catches the
|
||||
/// import-specific crash surface that the full <c>SemanticValidator</c>
|
||||
/// can't see: identifier-shaped call targets in
|
||||
/// <c>TemplateScript</c> / <c>ApiMethod</c> bodies that resolve to neither
|
||||
/// an in-bundle nor a pre-existing target <c>SharedScript</c> /
|
||||
/// <c>ExternalSystem</c>. Skip-resolved DTOs are excluded from the
|
||||
/// in-bundle name set so a Skip that would have provided a dependency
|
||||
/// surfaces here. Fails fast: if Pass 1 finds errors, Pass 2 is not run.</item>
|
||||
/// <item><b>Pass 2 — full <see cref="SemanticValidator"/>.</b> For each
|
||||
/// template being imported (Add / Overwrite / Rename — not Skip), build a
|
||||
/// per-template <see cref="FlattenedConfiguration"/> directly from the DTO
|
||||
/// (single-template scope — no inheritance / composition resolution, since
|
||||
/// the inheritance chain is reconstructed only at deploy time) and invoke
|
||||
/// the same validator the deployment pipeline uses. Errors from every
|
||||
/// template are aggregated into one list so the operator sees the full
|
||||
/// surface at once. SharedScripts are passed as <see cref="ResolvedScript"/>
|
||||
/// values combining bundle + target so call-target checks resolve in either
|
||||
/// direction.</item>
|
||||
/// </list>
|
||||
/// <para>
|
||||
/// The full <c>TemplateEngine.Validation.SemanticValidator</c> (which
|
||||
/// requires a <c>FlattenedConfiguration</c> built from the central template
|
||||
/// graph) is deferred to a follow-up — wiring it into the import path
|
||||
/// without a flattening fixture is non-trivial and the simpler check
|
||||
/// covers the same crash surface (unresolvable callsites at runtime).
|
||||
/// Per-template scoping is intentional: pre-existing target templates that
|
||||
/// haven't been touched by this bundle aren't run through the validator —
|
||||
/// otherwise a latent validation issue on an unrelated template (one the
|
||||
/// operator isn't trying to import) would block the import.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
private async Task<IReadOnlyList<string>> RunSemanticValidationAsync(
|
||||
@@ -1533,6 +1555,8 @@ public sealed class BundleImporter : IBundleImporter
|
||||
{
|
||||
var errors = new List<string>();
|
||||
|
||||
// ---- Pass 1: minimal name-resolution scan ----
|
||||
|
||||
// Build the known-resolvable set. For in-bundle entries, EXCLUDE the
|
||||
// Skip-resolved names — those aren't being written, so they can't
|
||||
// satisfy a downstream reference. Renamed entries register under both
|
||||
@@ -1562,7 +1586,8 @@ public sealed class BundleImporter : IBundleImporter
|
||||
}
|
||||
|
||||
// Pre-existing target entries always count as resolvable.
|
||||
foreach (var s in await _templateRepo.GetAllSharedScriptsAsync(ct).ConfigureAwait(false))
|
||||
var preExistingSharedScripts = await _templateRepo.GetAllSharedScriptsAsync(ct).ConfigureAwait(false);
|
||||
foreach (var s in preExistingSharedScripts)
|
||||
{
|
||||
sharedScriptNames.Add(s.Name);
|
||||
}
|
||||
@@ -1604,6 +1629,145 @@ public sealed class BundleImporter : IBundleImporter
|
||||
$"Script references SharedScript or ExternalSystem '{candidate}' not present in bundle or target.");
|
||||
}
|
||||
|
||||
// Fail fast — running the full validator over templates that already
|
||||
// failed name resolution would produce duplicate / lower-quality errors
|
||||
// (the missing identifier shows up there as "callee not found" too).
|
||||
if (errors.Count > 0) return errors;
|
||||
|
||||
// ---- Pass 2: full SemanticValidator over imported templates ----
|
||||
|
||||
// Build the shared-script catalog the validator uses to resolve
|
||||
// CallShared targets. Combine in-bundle (non-Skip) + pre-existing
|
||||
// target — same resolution model as Pass 1's name set.
|
||||
var sharedScripts = new List<ResolvedScript>();
|
||||
foreach (var s in content.SharedScripts)
|
||||
{
|
||||
var resolution = ResolveOrDefault(resolutionMap, "SharedScript", s.Name);
|
||||
if (resolution.Action == ResolutionAction.Skip) continue;
|
||||
var name = resolution.Action == ResolutionAction.Rename && !string.IsNullOrEmpty(resolution.RenameTo)
|
||||
? resolution.RenameTo
|
||||
: s.Name;
|
||||
sharedScripts.Add(new ResolvedScript
|
||||
{
|
||||
CanonicalName = name,
|
||||
Code = s.Code,
|
||||
ParameterDefinitions = s.ParameterDefinitions,
|
||||
ReturnDefinition = s.ReturnDefinition,
|
||||
});
|
||||
}
|
||||
foreach (var s in preExistingSharedScripts)
|
||||
{
|
||||
// Pre-existing target wins on duplicate name only when the bundle
|
||||
// didn't supply it; otherwise the bundle's version (the one about
|
||||
// to be written) is the right signature surface to validate against.
|
||||
if (sharedScripts.Any(rs => string.Equals(rs.CanonicalName, s.Name, StringComparison.Ordinal))) continue;
|
||||
sharedScripts.Add(new ResolvedScript
|
||||
{
|
||||
CanonicalName = s.Name,
|
||||
Code = s.Code,
|
||||
ParameterDefinitions = s.ParameterDefinitions,
|
||||
ReturnDefinition = s.ReturnDefinition,
|
||||
});
|
||||
}
|
||||
|
||||
foreach (var dto in content.Templates)
|
||||
{
|
||||
var resolution = ResolveOrDefault(resolutionMap, "Template", dto.Name);
|
||||
if (resolution.Action == ResolutionAction.Skip) continue;
|
||||
|
||||
var importedName = resolution.Action == ResolutionAction.Rename && !string.IsNullOrEmpty(resolution.RenameTo)
|
||||
? resolution.RenameTo
|
||||
: dto.Name;
|
||||
|
||||
var config = BuildFlattenedConfigForValidation(dto, importedName);
|
||||
var result = _semanticValidator.Validate(config, sharedScripts);
|
||||
foreach (var entry in result.Errors)
|
||||
{
|
||||
errors.Add($"Template '{importedName}': {entry.Message}");
|
||||
}
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds a <see cref="FlattenedConfiguration"/> for a single template DTO
|
||||
/// — the validator's input contract. The bundle DTO carries only the
|
||||
/// template's own attributes / alarms / scripts (no inheritance / no
|
||||
/// composition resolution at design time), so the flattening here is a
|
||||
/// straight 1:1 copy with the alarm on-trigger-script name carried through
|
||||
/// as the canonical name (the same script's bare name, since composed
|
||||
/// modules aren't expanded at import time). This is intentionally narrower
|
||||
/// than the production <c>FlatteningService</c> pipeline, which needs a
|
||||
/// concrete <c>Instance</c> plus site / connection context that doesn't
|
||||
/// exist yet at design time. The deployment-time flatten will revalidate
|
||||
/// against the full graph; this pass catches the same-template-scope
|
||||
/// errors that operators would otherwise only hit at deploy time.
|
||||
/// </summary>
|
||||
private static FlattenedConfiguration BuildFlattenedConfigForValidation(TemplateDto dto, string templateName)
|
||||
{
|
||||
var attributes = new List<ResolvedAttribute>(dto.Attributes.Count);
|
||||
foreach (var a in dto.Attributes)
|
||||
{
|
||||
attributes.Add(new ResolvedAttribute
|
||||
{
|
||||
CanonicalName = a.Name,
|
||||
Value = a.Value,
|
||||
DataType = a.DataType.ToString(),
|
||||
IsLocked = a.IsLocked,
|
||||
Description = a.Description,
|
||||
DataSourceReference = a.DataSourceReference,
|
||||
Source = "Template",
|
||||
});
|
||||
}
|
||||
|
||||
var alarms = new List<ResolvedAlarm>(dto.Alarms.Count);
|
||||
foreach (var al in dto.Alarms)
|
||||
{
|
||||
alarms.Add(new ResolvedAlarm
|
||||
{
|
||||
CanonicalName = al.Name,
|
||||
Description = al.Description,
|
||||
PriorityLevel = al.PriorityLevel,
|
||||
IsLocked = al.IsLocked,
|
||||
TriggerType = al.TriggerType.ToString(),
|
||||
TriggerConfiguration = al.TriggerConfiguration,
|
||||
// The bundle carries the on-trigger script by NAME (not id);
|
||||
// at this single-template-scope validation step the bare name
|
||||
// IS the canonical name, so just pass it through.
|
||||
OnTriggerScriptCanonicalName = string.IsNullOrEmpty(al.OnTriggerScriptName) ? null : al.OnTriggerScriptName,
|
||||
Source = "Template",
|
||||
});
|
||||
}
|
||||
|
||||
var scripts = new List<ResolvedScript>(dto.Scripts.Count);
|
||||
foreach (var s in dto.Scripts)
|
||||
{
|
||||
scripts.Add(new ResolvedScript
|
||||
{
|
||||
CanonicalName = s.Name,
|
||||
Code = s.Code,
|
||||
IsLocked = s.IsLocked,
|
||||
TriggerType = s.TriggerType,
|
||||
TriggerConfiguration = s.TriggerConfiguration,
|
||||
ParameterDefinitions = s.ParameterDefinitions,
|
||||
ReturnDefinition = s.ReturnDefinition,
|
||||
MinTimeBetweenRuns = s.MinTimeBetweenRuns,
|
||||
Source = "Template",
|
||||
});
|
||||
}
|
||||
|
||||
return new FlattenedConfiguration
|
||||
{
|
||||
InstanceUniqueName = templateName,
|
||||
TemplateId = 0,
|
||||
SiteId = 0,
|
||||
AreaId = null,
|
||||
Attributes = attributes,
|
||||
Alarms = alarms,
|
||||
Scripts = scripts,
|
||||
Connections = null,
|
||||
GeneratedAtUtc = DateTimeOffset.UtcNow,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.DependencyInjection.Extensions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ScadaLink.Commons.Interfaces.Transport;
|
||||
using ScadaLink.TemplateEngine.Validation;
|
||||
using ScadaLink.Transport.Encryption;
|
||||
using ScadaLink.Transport.Export;
|
||||
using ScadaLink.Transport.Import;
|
||||
@@ -30,6 +31,11 @@ public static class ServiceCollectionExtensions
|
||||
services.AddScoped<DependencyResolver>();
|
||||
services.AddScoped<IBundleExporter, BundleExporter>();
|
||||
services.AddSingleton<IBundleSessionStore, BundleSessionStore>();
|
||||
// SemanticValidator is a stateless utility used by ApplyAsync; use
|
||||
// TryAdd so a host that already calls AddTemplateEngine() (which
|
||||
// registers the same type as Transient) wins. Either registration
|
||||
// satisfies the BundleImporter constructor.
|
||||
services.TryAddTransient<SemanticValidator>();
|
||||
services.AddScoped<IBundleImporter, BundleImporter>();
|
||||
// Remaining concrete services added in later tasks.
|
||||
return services;
|
||||
|
||||
Reference in New Issue
Block a user