diff --git a/docs/requirements/Component-TemplateEngine.md b/docs/requirements/Component-TemplateEngine.md index eb6cbc51..de279904 100644 --- a/docs/requirements/Component-TemplateEngine.md +++ b/docs/requirements/Component-TemplateEngine.md @@ -93,6 +93,23 @@ When a template composes a feature module, members from that module are addresse - The composing template's own members (not from a module) have no prefix — they are top-level names. - Naming collision detection operates on canonical names, so two modules can define the same member name as long as their module instance names differ. +### Runtime actor-name rules (review 2026-10-21 01/SR-C1) + +Some names become Akka actor names at the site: the instance **unique name** (the +Instance Actor), and every **script**, **alarm** and **native alarm source** +canonical name (`script-…`, `alarm-…`, `native-alarm-…` children of the Instance +Actor) — which also embeds every **composition slot** name on its path. Akka accepts +only ASCII letters, digits and `-_.*$+:@&=,!~';` (plus `%HH` escapes) and no leading +`$`; anything else (a space, `#`, `/`, non-ASCII letters) makes `ActorOf` throw at +deploy — for an instance name, inside the site's Deployment Manager singleton. The +Template Engine therefore rejects such names **at save** with a readable error, using +the shared `Commons.Validation.ActorNameRules`: `InstanceService.CreateInstanceAsync` +(unique name), `TemplateService.AddScriptAsync` / `AddAlarmAsync` (member names are +immutable after creation, so updates need no check) and `AddCompositionAsync` / +`RenameCompositionAsync` (slot names). Attribute names and template names are not +actor names and are unrestricted. Native alarm sources are created outside +`TemplateService` (Management Service / Transport import); those paths must apply the same rule. + ### Derived template naming A composition slot is materialized as its own *derived* template. A derived diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs index 0d4a497e..8534f381 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Services/InstanceService.cs @@ -4,6 +4,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Validation; using ZB.MOM.WW.ScadaBridge.TemplateEngine; namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Services; @@ -59,6 +60,13 @@ public class InstanceService if (string.IsNullOrWhiteSpace(uniqueName)) return Result.Failure("Instance unique name is required."); + // The unique name becomes the site's Instance Actor name; a character Akka + // rejects would make ActorOf throw inside the Deployment Manager singleton on + // deploy (review 2026-10-21 01/SR-C1). Reject it here with a readable error. + var nameError = ActorNameRules.Describe(uniqueName); + if (nameError != null) + return Result.Failure($"Invalid instance unique name: {nameError}"); + // Verify template exists var template = await _repository.GetTemplateByIdAsync(templateId, cancellationToken); if (template == null) diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs index 736cbea0..4eae7d2b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/TemplateService.cs @@ -4,6 +4,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; +using ZB.MOM.WW.ScadaBridge.Commons.Validation; namespace ZB.MOM.WW.ScadaBridge.TemplateEngine; @@ -518,6 +519,10 @@ public class TemplateService string user, CancellationToken cancellationToken = default) { + var nameError = DescribeRuntimeNameError("Alarm", alarm.Name); + if (nameError != null) + return Result.Failure(nameError); + var template = await _repository.GetTemplateByIdAsync(templateId, cancellationToken); if (template == null) return Result.Failure($"Template with ID {templateId} not found."); @@ -693,6 +698,10 @@ public class TemplateService string user, CancellationToken cancellationToken = default) { + var nameError = DescribeRuntimeNameError("Script", script.Name); + if (nameError != null) + return Result.Failure(nameError); + var template = await _repository.GetTemplateByIdAsync(templateId, cancellationToken); if (template == null) return Result.Failure($"Template with ID {templateId} not found."); @@ -869,6 +878,10 @@ public class TemplateService if (string.IsNullOrWhiteSpace(instanceName)) return Result.Failure("Instance name is required for composition."); + var nameError = DescribeRuntimeNameError("Composition slot", instanceName); + if (nameError != null) + return Result.Failure(nameError); + var template = await _repository.GetTemplateByIdAsync(templateId, cancellationToken); if (template == null) return Result.Failure($"Template with ID {templateId} not found."); @@ -987,6 +1000,10 @@ public class TemplateService if (string.IsNullOrWhiteSpace(newInstanceName)) return Result.Failure("Slot name is required."); + var nameError = DescribeRuntimeNameError("Composition slot", newInstanceName); + if (nameError != null) + return Result.Failure(nameError); + var composition = await _repository.GetTemplateCompositionByIdAsync(compositionId, cancellationToken); if (composition == null) return Result.Failure($"Composition with ID {compositionId} not found."); @@ -1788,4 +1805,21 @@ public class TemplateService clone.Compositions.Add(newComp); return clone; } + + /// + /// Script, alarm and composition-slot names end up inside site actor names — + /// script-{canonical}, alarm-{canonical}, native-alarm-{canonical}, + /// where the canonical name is the dotted slot path plus the member name. A + /// character Akka rejects makes the Instance Actor's ActorOf throw at + /// deploy, so reject it at save with a readable error (review 2026-10-21 01/SR-C1). + /// Attributes are not actors and are not checked. + /// + /// The member kind, for the message. + /// The proposed name. + /// An error message, or null when the name is usable. + private static string? DescribeRuntimeNameError(string kind, string? name) + { + var error = ActorNameRules.Describe(name); + return error == null ? null : $"{kind} name is invalid: {error}"; + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/ActorNameValidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/ActorNameValidationTests.cs new file mode 100644 index 00000000..5814e9c0 --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/ActorNameValidationTests.cs @@ -0,0 +1,90 @@ +using Moq; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; +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.TemplateEngine.Services; + +namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests; + +/// +/// Review 2026-10-21 01/SR-C1 (central half): names that become Akka actor names at +/// the site — the instance unique name (the Instance Actor), and the script / alarm / +/// composition-slot names that make up the script-/alarm-/ +/// native-alarm- child canonical names — are rejected at save with a readable +/// error, instead of crashing the site's Deployment Manager / failing the deploy when +/// ActorOf throws InvalidActorNameException. +/// +public class ActorNameValidationTests +{ + private readonly Mock _repoMock = new(); + private readonly Mock _auditMock = new(); + + private TemplateService TemplateSut => new(_repoMock.Object, _auditMock.Object); + + /// Verifies that an instance unique name with invalid actor-name characters is rejected. + /// An instance name Akka cannot use as an actor name. + /// A task that represents the asynchronous operation. + [Theory] + [InlineData("Furnace #3")] + [InlineData("Line 1")] + [InlineData("Ofen Süd")] + [InlineData("a/b")] + [InlineData("$system")] + public async Task CreateInstance_InvalidActorName_FailsBeforeAnyWrite(string name) + { + var sut = new InstanceService(_repoMock.Object, _auditMock.Object); + + var result = await sut.CreateInstanceAsync(name, 1, 1, null, "admin"); + + Assert.True(result.IsFailure); + Assert.Contains("not allowed", result.Error); + _repoMock.Verify(r => r.AddInstanceAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + /// Verifies that a script name with invalid actor-name characters is rejected. + /// A task that represents the asynchronous operation. + [Fact] + public async Task AddScript_InvalidActorName_Fails() + { + var result = await TemplateSut.AddScriptAsync(1, new TemplateScript("Calc #1", "return 1;"), "admin"); + + Assert.True(result.IsFailure); + Assert.Contains("Calc #1", result.Error); + _repoMock.Verify(r => r.AddTemplateScriptAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + /// Verifies that an alarm name with invalid actor-name characters is rejected. + /// A task that represents the asynchronous operation. + [Fact] + public async Task AddAlarm_InvalidActorName_Fails() + { + var result = await TemplateSut.AddAlarmAsync(1, new TemplateAlarm("High Temp"), "admin"); + + Assert.True(result.IsFailure); + Assert.Contains("High Temp", result.Error); + _repoMock.Verify(r => r.AddTemplateAlarmAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + /// Verifies that a composition slot name with invalid actor-name characters is rejected. + /// A task that represents the asynchronous operation. + [Fact] + public async Task AddComposition_InvalidActorName_Fails() + { + var result = await TemplateSut.AddCompositionAsync(1, 2, "Left Sensor", "admin"); + + Assert.True(result.IsFailure); + Assert.Contains("Left Sensor", result.Error); + } + + /// Verifies that renaming a composition slot to an invalid actor name is rejected. + /// A task that represents the asynchronous operation. + [Fact] + public async Task RenameComposition_InvalidActorName_Fails() + { + var result = await TemplateSut.RenameCompositionAsync(50, "new slot", "admin"); + + Assert.True(result.IsFailure); + Assert.Contains("new slot", result.Error); + } +}