From 39025b28faddbba96668a65a61c3ec9c1c2a1375 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 09:29:54 -0400 Subject: [PATCH] fix(transport): import rejects malformed site gRPC endpoints and non-actor-safe names as blockers, not mid-apply crashes (review 2026-10-21 03/R-1, lane-F ActorNameRules follow-up) --- docs/requirements/Component-Transport.md | 3 + .../Import/BundleImporter.cs | 177 +++++++++- .../CreateMissingSiteRelationalTests.cs | 4 +- .../Import/ImportNameRuleTests.cs | 317 ++++++++++++++++++ .../Import/SiteInstanceImportTests.cs | 10 +- .../RoundTripEquivalenceTests.cs | 4 +- .../SemanticValidatorImportTests.cs | 4 +- 7 files changed, 506 insertions(+), 13 deletions(-) create mode 100644 tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/ImportNameRuleTests.cs diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index f86f9513..9345cdf9 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -328,7 +328,10 @@ As of M8 the import result enumerates affected instances for real (closing the p | Map | `CreateNew` selected but the bundle does not carry that site/connection's config | Blocker — Create-new requires the full config to be present in the bundle payload | | Preview | ApiMethod script references shared script / external system not in bundle and not in target DB | Listed as a **blocker** row in the Diff step; cannot Apply until resolved (no downstream deploy gate for ApiMethods) | | Preview | Template script references shared script / external system not in bundle and not in target DB | Listed as an advisory **warning** row; import proceeds — the design-time deploy gate re-validates the script authoritatively | +| Preview | A bundled site's `GrpcNodeAAddress`/`GrpcNodeBAddress` is not empty and not an absolute `http(s)://host[:port]` URL (`GrpcEndpointRules`; review 2026-10-21 03/R-1) | **Blocker** row (`Site`) when the target has no such site (the import would create it); advisory **warning** when it does (only an Overwrite writes the addresses) | +| Preview | A name that becomes a site actor name fails `ActorNameRules` — an instance's unique name, or a template's script / alarm / composition-slot / native-alarm-source name | **Blocker** row per name (`Instance` / `Template` `{template}.{member}`), with the same readable message the Central UI save path gives | | Apply | Semantic validation fails (call target type mismatch, etc.) | Modal: "Validation failed — N errors", per-error list, no DB writes | +| Apply | Either rule above is violated by an entity the apply would write — skip-resolved entities are exempt; an instance's Rename target is checked; a site's endpoints are checked when it is created or Overwritten | Same `SemanticValidationException` path, raised before any row is staged — never a mid-apply exception, a central channel-build throw, or an `ActorOf` throw at deploy | | Apply | DB transaction fails | Rollback; full transactional guarantee — nothing partial lands; `BundleImportFailed` audit row written outside the rolled-back transaction | | Session | TTL expired | Diff step onward, refresh prompts re-upload | diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index dd56c664..126f08b7 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -22,6 +22,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Messages.Scripting; using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.Commons.Types.Flattening; +using ZB.MOM.WW.ScadaBridge.Commons.Validation; using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; using ZB.MOM.WW.ScadaBridge.TemplateEngine; @@ -583,6 +584,26 @@ public sealed class BundleImporter : IBundleImporter { var existing = await ResolveTargetSiteAsync(siteDto.SiteIdentifier).ConfigureAwait(false); items.Add(_diff.CompareSite(siteDto, existing)); + + // Review 2026-10-21 03/R-1: a malformed gRPC endpoint makes central's + // channel build throw for that site. A site the import must CREATE would + // certainly write it → Blocker; an existing site only receives the + // addresses on Overwrite → Warning (apply re-checks against the chosen + // resolution and rejects an Overwrite). + var endpointErrors = DescribeSiteEndpointErrors(siteDto); + if (endpointErrors.Count > 0) + { + items.Add(new ImportPreviewItem( + EntityType: "Site", + Name: siteDto.SiteIdentifier, + ExistingVersion: null, + IncomingVersion: null, + Kind: existing is null ? ConflictKind.Blocker : ConflictKind.Warning, + FieldDiffJson: null, + BlockerReason: string.Join(" ", endpointErrors) + (existing is null + ? string.Empty + : " The import will reject an Overwrite of this site; Skip leaves the target's endpoints unchanged."))); + } } // ---- DataConnections (site-scoped; matched by name within the auto-matched target site) ---- @@ -985,9 +1006,152 @@ public sealed class BundleImporter : IBundleImporter BlockerReason: message)); } + // Lane-F follow-up (review 2026-10-21): names that become site actor names. + foreach (var (entityType, name, message) in CollectActorNameViolations(content, resolutionMap: null)) + { + blockers.Add(new ImportPreviewItem( + EntityType: entityType, + Name: name, + ExistingVersion: null, + IncomingVersion: null, + Kind: ConflictKind.Blocker, + FieldDiffJson: null, + BlockerReason: message)); + } + return blockers; } + /// + /// Lane-F follow-up to review 2026-10-21 01/SR-C1: every bundled name that the + /// site runtime turns into an Akka actor name — an instance's unique name (or its + /// Rename target), and a template's script, alarm, composition-slot and + /// native-alarm-source names (they form the script-/alarm-/ + /// native-alarm- actor names under the Instance Actor) — must satisfy + /// . The Central UI / CLI save paths already enforce + /// this; an import that skipped it would persist a name that makes + /// ActorOf throw at deploy. Skip-resolved entities are not written, so they + /// are not checked ( is null at preview: check all). + /// + /// The bundle content. + /// The apply-time resolutions, or null at preview. + /// One (entity type, item name, readable message) tuple per violation. + private static IReadOnlyList<(string EntityType, string Name, string Message)> CollectActorNameViolations( + BundleContentDto content, + Dictionary<(string, string), ImportResolution>? resolutionMap) + { + var violations = new List<(string, string, string)>(); + + void CheckMember(TemplateDto t, string kind, string memberName) + { + var error = ActorNameRules.Describe(memberName); + if (error is not null) + { + violations.Add(("Template", $"{t.Name}.{memberName}", + $"Template '{t.Name}' {kind} name is invalid: {error}")); + } + } + + foreach (var t in content.Templates) + { + if (IsSkipResolution(resolutionMap, "Template", t.Name)) continue; + foreach (var s in t.Scripts) CheckMember(t, "Script", s.Name); + foreach (var a in t.Alarms) CheckMember(t, "Alarm", a.Name); + foreach (var c in t.Compositions) CheckMember(t, "Composition slot", c.InstanceName); + foreach (var n in t.NativeAlarmSources) CheckMember(t, "Native alarm source", n.Name); + } + + foreach (var inst in content.Instances) + { + if (IsSkipResolution(resolutionMap, "Instance", inst.UniqueName)) continue; + var effectiveName = inst.UniqueName; + if (resolutionMap is not null + && resolutionMap.TryGetValue(("Instance", inst.UniqueName), out var r) + && r.Action == ResolutionAction.Rename + && !string.IsNullOrEmpty(r.RenameTo)) + { + effectiveName = r.RenameTo; + } + + var error = ActorNameRules.Describe(effectiveName); + if (error is not null) + { + violations.Add(("Instance", inst.UniqueName, + $"Instance '{effectiveName}' unique name is invalid: {error}")); + } + } + + return violations; + } + + /// + /// Review 2026-10-21 03/R-1: validates a bundled site's gRPC endpoints with + /// (empty = "not configured", allowed). Central's + /// per-site channel build throws on a value that is not an absolute http(s) URL. + /// + /// The bundled site. + /// Readable errors; empty when both endpoints are acceptable. + private static IReadOnlyList DescribeSiteEndpointErrors(SiteDto dto) + { + var errors = new List(2); + if (!GrpcEndpointRules.TryValidate(dto.GrpcNodeAAddress, allowEmpty: true, out var a)) + { + errors.Add($"Site '{dto.SiteIdentifier}' GrpcNodeAAddress: {a}"); + } + if (!GrpcEndpointRules.TryValidate(dto.GrpcNodeBAddress, allowEmpty: true, out var b)) + { + errors.Add($"Site '{dto.SiteIdentifier}' GrpcNodeBAddress: {b}"); + } + return errors; + } + + /// + /// Review 2026-10-21 03/R-1, apply side: endpoint errors for every bundled site + /// whose addresses would actually write — a site it + /// creates (CreateNew, or no resolvable target) or an existing target it + /// Overwrites. Mirrors that method's mapping decision so a malformed address on a + /// Skip-resolved existing site (never written) does not block the import. + /// + private async Task> CollectSiteEndpointErrorsAsync( + BundleContentDto content, + Dictionary<(string, string), ImportResolution> resolutionMap, + BundleNameMap nameMap, + CancellationToken ct) + { + var errors = new List(); + var siteMappingByIdentifier = nameMap.Sites + .GroupBy(m => m.SourceSiteIdentifier, StringComparer.Ordinal) + .ToDictionary(g => g.Key, g => g.First(), StringComparer.Ordinal); + + foreach (var siteDto in content.Sites) + { + var endpointErrors = DescribeSiteEndpointErrors(siteDto); + if (endpointErrors.Count == 0) continue; + + siteMappingByIdentifier.TryGetValue(siteDto.SiteIdentifier, out var mapping); + var existing = await _siteRepo.GetSiteByIdentifierAsync(siteDto.SiteIdentifier, ct).ConfigureAwait(false); + var action = mapping?.Action + ?? (existing is not null ? MappingAction.MapToExisting : MappingAction.CreateNew); + var target = existing; + if (action == MappingAction.MapToExisting + && mapping?.TargetSiteIdentifier is { Length: > 0 } targetId + && !string.Equals(targetId, siteDto.SiteIdentifier, StringComparison.Ordinal)) + { + target = await _siteRepo.GetSiteByIdentifierAsync(targetId, ct).ConfigureAwait(false); + } + + var writes = action == MappingAction.CreateNew + || target is null + || ResolveOrDefault(resolutionMap, "Site", siteDto.SiteIdentifier).Action == ResolutionAction.Overwrite; + if (writes) + { + errors.AddRange(endpointErrors); + } + } + + return errors; + } + private static void CollectCallIdentifiers(string? body, HashSet sink) { if (string.IsNullOrEmpty(body)) return; @@ -1572,9 +1736,18 @@ public sealed class BundleImporter : IBundleImporter validationErrors = validationErrors.Count > 0 ? validationErrors : await ValidateSiteInstanceReferencesAsync(content, resolutionMap, nameMap, ct).ConfigureAwait(false); - if (validationErrors.Count > 0) + + // Review 2026-10-21 03/R-1 + lane-F follow-up: values the runtime later + // trusts blindly — site gRPC endpoints and names that become actor names — + // are rejected here, with the change tracker still empty, rather than + // surfacing as a channel-build throw on central or an ActorOf throw at deploy. + var ruleErrors = CollectActorNameViolations(content, resolutionMap) + .Select(v => v.Message) + .Concat(await CollectSiteEndpointErrorsAsync(content, resolutionMap, nameMap, ct).ConfigureAwait(false)) + .ToList(); + if (ruleErrors.Count > 0 || validationErrors.Count > 0) { - throw new SemanticValidationException(validationErrors); + throw new SemanticValidationException(ruleErrors.Concat(validationErrors).ToList()); } // Advisory template-script reference findings never block the import; diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs index f83f4ee7..969b1d78 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/CreateMissingSiteRelationalTests.cs @@ -91,8 +91,8 @@ public sealed class CreateMissingSiteRelationalTests : IDisposable Description = "source plant", NodeAAddress = "akka://site@10.0.0.1:2552", NodeBAddress = "akka://site@10.0.0.2:2552", - GrpcNodeAAddress = "10.0.0.1:8083", - GrpcNodeBAddress = "10.0.0.2:8083", + GrpcNodeAAddress = "http://10.0.0.1:8083", + GrpcNodeBAddress = "http://10.0.0.2:8083", }; ctx.Sites.Add(site); await ctx.SaveChangesAsync(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/ImportNameRuleTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/ImportNameRuleTests.cs new file mode 100644 index 00000000..31c79ac1 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/ImportNameRuleTests.cs @@ -0,0 +1,317 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates; +using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories; +using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Services; +using ZB.MOM.WW.ScadaBridge.Transport.Import; +using ZB.MOM.WW.ScadaBridge.Transport.Serialization; + +namespace ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests.Import; + +/// +/// Review 2026-10-21 03/R-1 + lane-F follow-up: the importer validates the values it +/// would write that the runtime later trusts blindly — a site's gRPC endpoints +/// (GrpcEndpointRules) and every name that becomes a site actor name +/// (ActorNameRules: instance unique names, template script / alarm / +/// composition-slot / native-alarm-source names). A violation is a Blocker row in +/// the preview and a at apply, raised +/// before any row is staged — never an exception mid-apply or a crash at deploy. +/// +public sealed class ImportNameRuleTests : IDisposable +{ + private readonly ServiceProvider _provider; + + /// Initializes the test class with a service provider backed by an in-memory database. + public ImportNameRuleTests() + { + var services = new ServiceCollection(); + services.AddSingleton( + new Microsoft.AspNetCore.DataProtection.EphemeralDataProtectionProvider()); + services.AddSingleton(new ConfigurationBuilder().AddInMemoryCollection().Build()); + + var dbName = $"ImportNameRuleTests_{Guid.NewGuid()}"; + services.AddDbContext(opts => opts + .UseInMemoryDatabase(dbName) + .ConfigureWarnings(w => w.Ignore(InMemoryEventId.TransactionIgnoredWarning))); + + services.AddSingleton(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + services.AddTransport(); + + _provider = services.BuildServiceProvider(); + } + + /// Releases the resources used by this instance. + public void Dispose() => _provider.Dispose(); + + private static BundleContentDto Empty() => new( + TemplateFolders: Array.Empty(), + Templates: Array.Empty(), + SharedScripts: Array.Empty(), + ExternalSystems: Array.Empty(), + DatabaseConnections: Array.Empty(), + NotificationLists: Array.Empty(), + SmtpConfigs: Array.Empty(), + ApiMethods: Array.Empty()); + + private static SiteDto SiteWithGrpc(string? a, string? b) => + new("plant-1", "Plant 1", null, null, null, a, b); + + private static TemplateDto TemplateWithMembers(string script, string alarm, string slot, string nativeSource) => + new( + Name: "Pump", + FolderName: null, + BaseTemplateName: null, + Description: null, + Attributes: Array.Empty(), + Alarms: new[] + { + new TemplateAlarmDto(alarm, null, 1, AlarmTriggerType.ValueMatch, null, false, null), + }, + Scripts: new[] + { + new TemplateScriptDto(script, "return 1;", null, null, null, null, false, null), + }, + Compositions: new[] { new TemplateCompositionDto(slot, "Other") }) + { + NativeAlarmSources = new[] + { + new TemplateNativeAlarmSourceDto(nativeSource, null, "Opc", "ns=1;s=A", null, false, false, false), + }, + }; + + private async Task PackAndLoadAsync(BundleContentDto content) + { + await using var scope = _provider.CreateAsyncScope(); + var manifestBuilder = scope.ServiceProvider.GetRequiredService(); + var serializer = scope.ServiceProvider.GetRequiredService(); + var importer = scope.ServiceProvider.GetRequiredService(); + + var summary = new BundleSummary( + Templates: content.Templates.Count, + TemplateFolders: 0, SharedScripts: 0, ExternalSystems: 0, DbConnections: 0, + NotificationLists: 0, SmtpConfigs: 0, ApiMethods: 0, + Sites: content.Sites.Count, DataConnections: 0, Instances: content.Instances.Count); + var manifest = manifestBuilder.Build( + sourceEnvironment: "dev", exportedBy: "alice", scadaBridgeVersion: "1.0.0", + encryption: null, summary: summary, contents: Array.Empty(), + contentBytes: serializer.SerializeContentBytes(content)); + + await using var packed = serializer.Pack(content, manifest, passphrase: null, encryptor: null); + using var ms = new MemoryStream(); + await packed.CopyToAsync(ms); + ms.Position = 0; + return (await importer.LoadAsync(ms, passphrase: null)).SessionId; + } + + private async Task PreviewAsync(Guid sessionId) + { + await using var scope = _provider.CreateAsyncScope(); + return await scope.ServiceProvider.GetRequiredService().PreviewAsync(sessionId); + } + + private async Task ApplyAsync(Guid sessionId, params ImportResolution[] resolutions) + { + await using var scope = _provider.CreateAsyncScope(); + return await scope.ServiceProvider.GetRequiredService() + .ApplyAsync(sessionId, resolutions, user: "bob", ct: CancellationToken.None); + } + + // ────────────────────────────────────────────────────────────────────── + // 03/R-1 — site gRPC endpoints + // ────────────────────────────────────────────────────────────────────── + + /// Verifies that a malformed gRPC endpoint on a site the import would create is a preview Blocker. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Preview_malformed_grpc_address_on_a_new_site_is_a_blocker() + { + var sessionId = await PackAndLoadAsync(Empty() with + { + Sites = new[] { SiteWithGrpc("site-a-node-a:8083", "http://site-a-node-b:8083") }, + }); + + var preview = await PreviewAsync(sessionId); + + var blocker = Assert.Single(preview.Items, i => i.Kind == ConflictKind.Blocker); + Assert.Equal("Site", blocker.EntityType); + Assert.Equal("plant-1", blocker.Name); + Assert.Contains("GrpcNodeAAddress", blocker.BlockerReason); + Assert.Contains("site-a-node-a:8083", blocker.BlockerReason); + } + + /// Verifies that applying a malformed gRPC endpoint is rejected before anything is written. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_malformed_grpc_address_on_a_new_site_is_rejected_before_any_write() + { + var sessionId = await PackAndLoadAsync(Empty() with + { + Sites = new[] { SiteWithGrpc("http://site-a-node-a:8083", "scadabridge-site-a-node-b") }, + }); + + var ex = await Assert.ThrowsAsync(() => + ApplyAsync(sessionId, new ImportResolution("Site", "plant-1", ResolutionAction.Add, null))); + + Assert.Contains(ex.Errors, e => e.Contains("GrpcNodeBAddress") && e.Contains("plant-1")); + await using var scope = _provider.CreateAsyncScope(); + var ctx = scope.ServiceProvider.GetRequiredService(); + Assert.Empty(await ctx.Sites.ToListAsync()); + } + + /// + /// Verifies that a malformed endpoint on a bundled site that maps to an existing + /// target site and is Skip-resolved does not block — the address is never written. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_malformed_grpc_address_is_allowed_when_the_existing_site_is_skipped() + { + await using (var seed = _provider.CreateAsyncScope()) + { + var ctx = seed.ServiceProvider.GetRequiredService(); + ctx.Sites.Add(new Site("Plant 1", "plant-1") { GrpcNodeAAddress = "http://good:8083" }); + await ctx.SaveChangesAsync(); + } + var sessionId = await PackAndLoadAsync(Empty() with + { + Sites = new[] { SiteWithGrpc("bad-address", null) }, + }); + + var preview = await PreviewAsync(sessionId); + Assert.DoesNotContain(preview.Items, i => i.Kind == ConflictKind.Blocker); + Assert.Contains(preview.Items, i => i.Kind == ConflictKind.Warning && i.EntityType == "Site"); + + await ApplyAsync(sessionId, new ImportResolution("Site", "plant-1", ResolutionAction.Skip, null)); + + await using var scope = _provider.CreateAsyncScope(); + var site = await scope.ServiceProvider.GetRequiredService().Sites.SingleAsync(); + Assert.Equal("http://good:8083", site.GrpcNodeAAddress); + } + + /// Verifies that Overwrite onto an existing site with a malformed endpoint is rejected. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_malformed_grpc_address_is_rejected_on_overwrite_of_an_existing_site() + { + await using (var seed = _provider.CreateAsyncScope()) + { + var ctx = seed.ServiceProvider.GetRequiredService(); + ctx.Sites.Add(new Site("Plant 1", "plant-1") { GrpcNodeAAddress = "http://good:8083" }); + await ctx.SaveChangesAsync(); + } + var sessionId = await PackAndLoadAsync(Empty() with + { + Sites = new[] { SiteWithGrpc("bad-address", null) }, + }); + + var ex = await Assert.ThrowsAsync(() => + ApplyAsync(sessionId, new ImportResolution("Site", "plant-1", ResolutionAction.Overwrite, null))); + Assert.Contains(ex.Errors, e => e.Contains("GrpcNodeAAddress")); + } + + // ────────────────────────────────────────────────────────────────────── + // ActorNameRules — names that become site actor names + // ────────────────────────────────────────────────────────────────────── + + /// Verifies that template member names Akka would reject are preview Blockers, one per member. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Preview_template_member_names_that_are_not_actor_safe_are_blockers() + { + var sessionId = await PackAndLoadAsync(Empty() with + { + Templates = new[] { TemplateWithMembers("bad/script", "bad alarm", "slot#1", "$native") }, + }); + + var preview = await PreviewAsync(sessionId); + + var blockers = preview.Items.Where(i => i.Kind == ConflictKind.Blocker).ToList(); + Assert.Contains(blockers, b => b.Name == "Pump.bad/script" && b.BlockerReason!.Contains("Script")); + Assert.Contains(blockers, b => b.Name == "Pump.bad alarm" && b.BlockerReason!.Contains("Alarm")); + Assert.Contains(blockers, b => b.Name == "Pump.slot#1" && b.BlockerReason!.Contains("Composition")); + Assert.Contains(blockers, b => b.Name == "Pump.$native" && b.BlockerReason!.Contains("Native alarm source")); + } + + /// Verifies that applying a template with a non-actor-safe member name is rejected before any write. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_template_member_name_that_is_not_actor_safe_is_rejected_before_any_write() + { + var sessionId = await PackAndLoadAsync(Empty() with + { + Templates = new[] { TemplateWithMembers("bad/script", "Hi", "Slot", "Native") }, + }); + + var ex = await Assert.ThrowsAsync(() => + ApplyAsync(sessionId, new ImportResolution("Template", "Pump", ResolutionAction.Add, null))); + + Assert.Contains(ex.Errors, e => e.Contains("bad/script")); + await using var scope = _provider.CreateAsyncScope(); + Assert.Empty(await scope.ServiceProvider.GetRequiredService().Templates.ToListAsync()); + } + + /// Verifies that a Skip-resolved template's member names are not checked at apply. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Apply_skipped_template_is_not_name_checked() + { + var sessionId = await PackAndLoadAsync(Empty() with + { + Templates = new[] { TemplateWithMembers("bad/script", "Hi", "Slot", "Native") }, + }); + + var result = await ApplyAsync(sessionId, new ImportResolution("Template", "Pump", ResolutionAction.Skip, null)); + + Assert.Equal(1, result.Skipped); + } + + /// Verifies that an instance unique name (or its rename target) Akka would reject is caught. + /// A task that represents the asynchronous operation. + [Fact] + public async Task Instance_unique_name_and_rename_target_are_actor_name_checked() + { + await using (var seed = _provider.CreateAsyncScope()) + { + var ctx = seed.ServiceProvider.GetRequiredService(); + ctx.Templates.Add(new Template("Pump")); + ctx.Sites.Add(new Site("Plant 1", "plant-1")); + await ctx.SaveChangesAsync(); + } + var instance = new InstanceDto( + "Pump 01", "Pump", "plant-1", null, InstanceState.NotDeployed, + Array.Empty(), Array.Empty(), + Array.Empty(), Array.Empty()); + var sessionId = await PackAndLoadAsync(Empty() with { Instances = new[] { instance } }); + + var preview = await PreviewAsync(sessionId); + Assert.Contains(preview.Items, i => + i.Kind == ConflictKind.Blocker && i.EntityType == "Instance" && i.Name == "Pump 01"); + + // Renaming to a valid name clears the instance-name error… + var renamed = await ApplyAsync(sessionId, new ImportResolution("Instance", "Pump 01", ResolutionAction.Rename, "Pump-01")); + Assert.Equal(1, renamed.Renamed); + + // …and renaming to an invalid one is rejected. + var second = await PackAndLoadAsync(Empty() with { Instances = new[] { instance with { UniqueName = "Pump-02" } } }); + var ex = await Assert.ThrowsAsync(() => + ApplyAsync(second, new ImportResolution("Instance", "Pump-02", ResolutionAction.Rename, "Pump/02"))); + Assert.Contains(ex.Errors, e => e.Contains("Pump/02")); + } +} diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs index 24b0bc28..89b4fc49 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/SiteInstanceImportTests.cs @@ -103,8 +103,8 @@ public sealed class SiteInstanceImportTests : IDisposable Description = "primary plant", NodeAAddress = "akka://site@10.0.0.1:2552", NodeBAddress = "akka://site@10.0.0.2:2552", - GrpcNodeAAddress = "10.0.0.1:8083", - GrpcNodeBAddress = "10.0.0.2:8083", + GrpcNodeAAddress = "http://10.0.0.1:8083", + GrpcNodeBAddress = "http://10.0.0.2:8083", }; ctx.Sites.Add(site); await ctx.SaveChangesAsync(); @@ -290,7 +290,7 @@ public sealed class SiteInstanceImportTests : IDisposable var site = await ctx.Sites.SingleAsync(s => s.SiteIdentifier == "plant-1"); Assert.Equal("Plant 1", site.Name); // Full config carried (D3 "carry full config"). - Assert.Equal("10.0.0.1:8083", site.GrpcNodeAAddress); + Assert.Equal("http://10.0.0.1:8083", site.GrpcNodeAAddress); Assert.Equal("akka://site@10.0.0.2:2552", site.NodeBAddress); var conn = await ctx.DataConnections.SingleAsync(c => c.Name == "OpcUaPrimary"); @@ -1439,8 +1439,8 @@ public sealed class SiteInstanceImportTests : IDisposable { NodeAAddress = "akka://site@10.0.0.1:2552", NodeBAddress = "akka://site@10.0.0.2:2552", - GrpcNodeAAddress = "10.0.0.1:8083", - GrpcNodeBAddress = "10.0.0.2:8083", + GrpcNodeAAddress = "http://10.0.0.1:8083", + GrpcNodeBAddress = "http://10.0.0.2:8083", }; ctx.Sites.Add(site); await ctx.SaveChangesAsync(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs index 862b4234..bcf842a7 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/RoundTripEquivalenceTests.cs @@ -805,8 +805,8 @@ public sealed class RoundTripEquivalenceTests : IDisposable Description = "primary plant", NodeAAddress = "akka://site@10.0.0.1:2552", NodeBAddress = "akka://site@10.0.0.2:2552", - GrpcNodeAAddress = "10.0.0.1:8083", - GrpcNodeBAddress = "10.0.0.2:8083", + GrpcNodeAAddress = "http://10.0.0.1:8083", + GrpcNodeBAddress = "http://10.0.0.2:8083", // Non-default deploy state, so the imported site is provably reset rather than // accidentally equal to an unset source. ArtifactRevisionHash = "artifact-hash-1", diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs index e6b456f7..0d38b14d 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/SemanticValidatorImportTests.cs @@ -146,8 +146,8 @@ public sealed class SemanticValidatorImportTests : IDisposable { NodeAAddress = "akka://site@10.0.0.1:2552", NodeBAddress = "akka://site@10.0.0.2:2552", - GrpcNodeAAddress = "10.0.0.1:8083", - GrpcNodeBAddress = "10.0.0.2:8083", + GrpcNodeAAddress = "http://10.0.0.1:8083", + GrpcNodeBAddress = "http://10.0.0.2:8083", }; ctx.Sites.Add(site); await ctx.SaveChangesAsync();