diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs index 6d282d97..90404451 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/Commands/TemplateCommands.cs @@ -241,6 +241,28 @@ public static class TemplateCommands internal const string ElementTypeOptionDescription = "Element scalar type for a List attribute (String, Int32, Float, Double, Boolean, DateTime). Required when --data-type is List."; + /// Shared description for the --min-time-between-runs option on script add/update. + internal const string MinTimeBetweenRunsOptionDescription = + "Minimum time between script runs (a throttle for triggered scripts; the re-fire interval for a WhileTrue trigger). " + + "Accepts a value with a unit suffix: ms, s/sec, or m/min (e.g. 500ms, 5s, 2min); a bare number is seconds. " + + "Omit, or pass 0 / a blank value, to leave it unset (no throttle)."; + + /// Shared description for the --execution-timeout-seconds option on script add/update. + internal const string ExecutionTimeoutOptionDescription = + "Per-script execution timeout in seconds. Omit (or pass a non-positive value) to use the site's global " + + "default (SiteRuntimeOptions.ScriptExecutionTimeoutSeconds)."; + + /// Shared description for the script --trigger-kind option on script add/update. + internal const string ScriptTriggerKindOptionDescription = + "Expression trigger analysis kind: advisory (default) or strict. " + + "Strict escalates a blank expression from a warning to a deploy-blocking error. " + + "Requires --trigger-config: with no config the kind is ignored (a warning is printed to stderr)."; + + /// Shared description for the alarm --trigger-kind option on alarm update. + internal const string AlarmTriggerKindOptionDescription = + "Expression trigger analysis kind: advisory (default) or strict. " + + "Requires --trigger-config: with no config the kind is ignored (a warning is printed to stderr)."; + /// /// The element scalar types permitted for a List attribute — derived from the /// single source of truth, , @@ -321,6 +343,94 @@ public static class TemplateCommands private static string? NormalizeElementType(string? elementType) => string.IsNullOrWhiteSpace(elementType) ? null : elementType.Trim(); + /// + /// Parses the raw --min-time-between-runs value into a . + /// A blank/absent value means "unset" ( null → null, no error). + /// Otherwise the value must be a positive integer with an optional unit suffix: + /// ms, s/sec, or m/min; a bare number is interpreted as + /// seconds (matching the UI duration input's default unit). A value of 0 resolves to + /// null (unset), mirroring DurationInput.Compose. The unit set and "bare = seconds" + /// default deliberately mirror the Central UI duration input so the two authoring surfaces agree. + /// + /// The raw flag value, or null when the flag was omitted. + /// The parsed duration, or null for an absent/unset value. + /// A descriptive error message when parsing fails; otherwise null. + /// true when the value is absent or parses cleanly; false on a malformed value. + internal static bool TryParseMinTimeBetweenRuns(string? value, out TimeSpan? duration, out string? error) + { + duration = null; + error = null; + + var trimmed = value?.Trim(); + if (string.IsNullOrEmpty(trimmed)) + return true; // omitted → unset + + // Split a trailing alphabetic unit suffix from the leading numeric part. + var splitIndex = trimmed.Length; + while (splitIndex > 0 && char.IsLetter(trimmed[splitIndex - 1])) + splitIndex--; + + var numberPart = trimmed[..splitIndex]; + var unitPart = trimmed[splitIndex..].ToLowerInvariant(); + + if (!long.TryParse(numberPart, System.Globalization.NumberStyles.Integer, System.Globalization.CultureInfo.InvariantCulture, out var amount) + || amount < 0) + { + error = $"Invalid --min-time-between-runs '{value}'. Expected a non-negative number with an optional " + + "unit suffix (ms, s/sec, m/min); a bare number is seconds (e.g. 500ms, 5s, 2min)."; + return false; + } + + var factorMs = unitPart switch + { + "ms" => 1L, + "" or "s" or "sec" => 1000L, + "m" or "min" => 60000L, + _ => -1L, + }; + + if (factorMs < 0) + { + error = $"Invalid --min-time-between-runs unit in '{value}'. Valid units are: ms, s/sec, m/min."; + return false; + } + + // 0 (with or without a unit) → unset, mirroring DurationInput.Compose's non-positive handling. + duration = amount == 0 ? null : TimeSpan.FromMilliseconds(amount * factorMs); + return true; + } + + /// + /// Determines whether --trigger-kind was supplied without a --trigger-config. + /// In that case has no JSON object to inject + /// into and silently drops the kind (#257) — the caller should warn the user. + /// + /// The raw --trigger-config JSON value, or null when absent. + /// The raw --trigger-kind value, or null when absent. + /// true when a kind was given but no config — i.e. the kind will be ignored. + internal static bool TriggerKindWillBeIgnored(string? triggerConfig, string? triggerKind) + => !string.IsNullOrWhiteSpace(triggerKind) && string.IsNullOrWhiteSpace(triggerConfig); + + /// + /// Warns (to stderr) when --trigger-kind is supplied without a --trigger-config + /// (see ). The command still proceeds — the kind is + /// advisory metadata, not a required field — but the user is told it had no effect so they are + /// not surprised that a strict request did not take. Warn-and-continue (rather than a hard + /// error) keeps the flag combination non-fatal: a script/alarm is still created, just without the + /// requested analysis kind. + /// + /// The raw --trigger-config JSON value, or null when absent. + /// The raw --trigger-kind value, or null when absent. + private static void WarnIfTriggerKindIgnored(string? triggerConfig, string? triggerKind) + { + if (TriggerKindWillBeIgnored(triggerConfig, triggerKind)) + { + Console.Error.WriteLine( + $"warning: --trigger-kind '{triggerKind!.Trim()}' was ignored because no --trigger-config was supplied. " + + "The analysis kind is written into the trigger-config JSON, so it has no effect without one."); + } + } + private static Command BuildAlarm(Option urlOption, Option formatOption, Option usernameOption, Option passwordOption) { var group = new Command("alarm") { Description = "Manage template alarms" }; @@ -417,7 +527,7 @@ public static class TemplateCommands // M9-T28b: --trigger-kind for update (same semantics as add) var updateTriggerKindOption = new Option("--trigger-kind") { - Description = "Expression trigger analysis kind: advisory (default) or strict." + Description = AlarmTriggerKindOptionDescription }; var updateCmd = new Command("update") { Description = "Update a template alarm" }; @@ -431,8 +541,10 @@ public static class TemplateCommands updateCmd.Add(updateTriggerKindOption); updateCmd.SetAction(async (ParseResult result) => { + var rawConfig = result.GetValue(updateTriggerConfigOption); + WarnIfTriggerKindIgnored(rawConfig, result.GetValue(updateTriggerKindOption)); var triggerConfig = TriggerConfigJson.InjectAnalysisKind( - result.GetValue(updateTriggerConfigOption), + rawConfig, result.GetValue(updateTriggerKindOption)); return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, @@ -542,12 +654,12 @@ public static class TemplateCommands var paramsOption = new Option("--parameters") { Description = "Parameter definitions JSON" }; var returnOption = new Option("--return-def") { Description = "Return definition JSON" }; + var minTimeOption = new Option("--min-time-between-runs") { Description = MinTimeBetweenRunsOptionDescription }; + var execTimeoutOption = new Option("--execution-timeout-seconds") { Description = ExecutionTimeoutOptionDescription }; // M9-T28b: analysis kind for Expression triggers (advisory|strict; default advisory). var scriptTriggerKindOption = new Option("--trigger-kind") { - Description = "Expression trigger analysis kind: advisory (default) or strict. " + - "Strict escalates a blank expression from a warning to a deploy-blocking error. " + - "Used with --trigger-config or appended to a built config when --trigger-type is Expression." + Description = ScriptTriggerKindOptionDescription }; var addCmd = new Command("add") { Description = "Add a script to a template" }; @@ -559,11 +671,21 @@ public static class TemplateCommands addCmd.Add(lockedOption); addCmd.Add(paramsOption); addCmd.Add(returnOption); + addCmd.Add(minTimeOption); + addCmd.Add(execTimeoutOption); addCmd.Add(scriptTriggerKindOption); addCmd.SetAction(async (ParseResult result) => { + if (!TryParseMinTimeBetweenRuns(result.GetValue(minTimeOption), out var minTime, out var minTimeError)) + { + OutputFormatter.WriteError(minTimeError!, "INVALID_ARGUMENT"); + return 1; + } + + var rawConfig = result.GetValue(triggerConfigOption); + WarnIfTriggerKindIgnored(rawConfig, result.GetValue(scriptTriggerKindOption)); var triggerConfig = TriggerConfigJson.InjectAnalysisKind( - result.GetValue(triggerConfigOption), + rawConfig, result.GetValue(scriptTriggerKindOption)); return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, @@ -575,7 +697,9 @@ public static class TemplateCommands triggerConfig, result.GetValue(lockedOption), result.GetValue(paramsOption), - result.GetValue(returnOption))); + result.GetValue(returnOption), + minTime, + result.GetValue(execTimeoutOption))); }); group.Add(addCmd); @@ -589,10 +713,12 @@ public static class TemplateCommands var updateParamsOption = new Option("--parameters") { Description = "Parameter definitions JSON" }; var updateReturnOption = new Option("--return-def") { Description = "Return definition JSON" }; + var updateMinTimeOption = new Option("--min-time-between-runs") { Description = MinTimeBetweenRunsOptionDescription }; + var updateExecTimeoutOption = new Option("--execution-timeout-seconds") { Description = ExecutionTimeoutOptionDescription }; // M9-T28b: --trigger-kind for update (same semantics as add) var updateScriptTriggerKindOption = new Option("--trigger-kind") { - Description = "Expression trigger analysis kind: advisory (default) or strict." + Description = ScriptTriggerKindOptionDescription }; var updateCmd = new Command("update") { Description = "Update a template script" }; @@ -604,11 +730,21 @@ public static class TemplateCommands updateCmd.Add(updateLockedOption); updateCmd.Add(updateParamsOption); updateCmd.Add(updateReturnOption); + updateCmd.Add(updateMinTimeOption); + updateCmd.Add(updateExecTimeoutOption); updateCmd.Add(updateScriptTriggerKindOption); updateCmd.SetAction(async (ParseResult result) => { + if (!TryParseMinTimeBetweenRuns(result.GetValue(updateMinTimeOption), out var minTime, out var minTimeError)) + { + OutputFormatter.WriteError(minTimeError!, "INVALID_ARGUMENT"); + return 1; + } + + var rawConfig = result.GetValue(updateTriggerConfigOption); + WarnIfTriggerKindIgnored(rawConfig, result.GetValue(updateScriptTriggerKindOption)); var triggerConfig = TriggerConfigJson.InjectAnalysisKind( - result.GetValue(updateTriggerConfigOption), + rawConfig, result.GetValue(updateScriptTriggerKindOption)); return await CommandHelpers.ExecuteCommandAsync( result, urlOption, formatOption, usernameOption, passwordOption, @@ -620,7 +756,9 @@ public static class TemplateCommands triggerConfig, result.GetValue(updateLockedOption), result.GetValue(updateParamsOption), - result.GetValue(updateReturnOption))); + result.GetValue(updateReturnOption), + minTime, + result.GetValue(updateExecTimeoutOption))); }); group.Add(updateCmd); diff --git a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md index 06a78296..a3f48fb1 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CLI/README.md +++ b/src/ZB.MOM.WW.ScadaBridge.CLI/README.md @@ -271,7 +271,7 @@ scadabridge --url template alarm update --id --name --trigg | `--description` | no | Description | | `--trigger-config` | no | Trigger configuration JSON | | `--locked` | no | Lock the alarm in derived templates | -| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Ignored for non-`Expression` trigger types. | +| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Ignored for non-`Expression` trigger types. **Requires `--trigger-config`** — the kind is written into the trigger-config JSON, so on `update` it has no effect without one (the CLI prints a warning to stderr and continues). | #### `template alarm delete` @@ -325,7 +325,7 @@ scadabridge --url template native-alarm-source remove --id Add a script to a template. ```sh -scadabridge --url template script add --template-id --name --trigger-type --code [--trigger-config ] [--locked] [--parameters ] [--return-def ] [--trigger-kind ] +scadabridge --url template script add --template-id --name --trigger-type --code [--trigger-config ] [--locked] [--parameters ] [--return-def ] [--min-time-between-runs ] [--execution-timeout-seconds ] [--trigger-kind ] ``` | Option | Required | Description | @@ -338,7 +338,9 @@ scadabridge --url template script add --template-id --name | `--locked` | no | Lock the script in derived templates | | `--parameters` | no | Parameter definitions JSON | | `--return-def` | no | Return definition JSON | -| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Only meaningful when `--trigger-type` is `Expression`. | +| `--min-time-between-runs` | no | Minimum time between runs — a throttle for triggered scripts, or the re-fire interval for a `WhileTrue` trigger. Accepts a unit suffix `ms`, `s`/`sec`, or `m`/`min` (e.g. `500ms`, `5s`, `2min`); a bare number is seconds. Omit, or pass `0`, to leave it unset (no throttle). | +| `--execution-timeout-seconds` | no | Per-script execution timeout in seconds. Omit (or pass a non-positive value) to use the site's global default (`SiteRuntimeOptions.ScriptExecutionTimeoutSeconds`). | +| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Only meaningful when `--trigger-type` is `Expression`. **Requires `--trigger-config`** — the kind is written into the trigger-config JSON, so passing `--trigger-kind` without `--trigger-config` has no effect (the CLI prints a warning to stderr and continues). | #### `template script update` @@ -346,7 +348,7 @@ Update a script on a template. An update **replaces** the whole entity — every field below must be supplied with its post-update value, even if unchanged. ```sh -scadabridge --url template script update --id --name --trigger-type --code [--trigger-config ] [--locked] [--parameters ] [--return-def ] [--trigger-kind ] +scadabridge --url template script update --id --name --trigger-type --code [--trigger-config ] [--locked] [--parameters ] [--return-def ] [--min-time-between-runs ] [--execution-timeout-seconds ] [--trigger-kind ] ``` | Option | Required | Description | @@ -359,7 +361,9 @@ scadabridge --url template script update --id --name --trig | `--locked` | no | Lock the script in derived templates | | `--parameters` | no | Parameter definitions JSON | | `--return-def` | no | Return definition JSON | -| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Only meaningful when `--trigger-type` is `Expression`. | +| `--min-time-between-runs` | no | Minimum time between runs (throttle / `WhileTrue` re-fire interval). Unit suffix `ms`, `s`/`sec`, or `m`/`min` (e.g. `500ms`, `5s`, `2min`); a bare number is seconds. Omit or `0` leaves it unset. As an update replaces the whole entity, omitting this **clears** any existing value. | +| `--execution-timeout-seconds` | no | Per-script execution timeout in seconds; omit (or non-positive) uses the site default. As an update replaces the whole entity, omitting this **clears** any existing override. | +| `--trigger-kind` | no | Expression-trigger analysis kind: `advisory` (default) or `strict`. Only meaningful when `--trigger-type` is `Expression`. **Requires `--trigger-config`** — without it the kind is ignored (the CLI warns to stderr and continues). | #### `template script delete` diff --git a/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateTriggerKindIgnoredTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateTriggerKindIgnoredTests.cs new file mode 100644 index 00000000..e2bfd6ca --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.CLI.Tests/Commands/TemplateTriggerKindIgnoredTests.cs @@ -0,0 +1,48 @@ +using ZB.MOM.WW.ScadaBridge.CLI.Commands; + +namespace ZB.MOM.WW.ScadaBridge.CLI.Tests.Commands; + +/// +/// #257: --trigger-kind is written into the trigger-config JSON by +/// , which +/// returns null (dropping the kind) when no --trigger-config is supplied. Passing +/// --trigger-kind without --trigger-config was therefore a silent no-op. +/// The CLI now detects that combination () +/// and warns the user (warn-and-continue, not a hard error). These tests pin the predicate. +/// +public class TemplateTriggerKindIgnoredTests +{ + [Fact] + public void KindWithoutConfig_WillBeIgnored() + { + Assert.True(TemplateCommands.TriggerKindWillBeIgnored(triggerConfig: null, triggerKind: "strict")); + } + + [Fact] + public void KindWithBlankConfig_WillBeIgnored() + { + Assert.True(TemplateCommands.TriggerKindWillBeIgnored(triggerConfig: " ", triggerKind: "strict")); + } + + [Fact] + public void KindWithConfig_NotIgnored() + { + Assert.False(TemplateCommands.TriggerKindWillBeIgnored( + triggerConfig: "{\"expression\":\"x > 0\"}", triggerKind: "strict")); + } + + [Fact] + public void NoKind_NotIgnored() + { + // No --trigger-kind at all → nothing to warn about, even without a config. + Assert.False(TemplateCommands.TriggerKindWillBeIgnored(triggerConfig: null, triggerKind: null)); + Assert.False(TemplateCommands.TriggerKindWillBeIgnored(triggerConfig: null, triggerKind: " ")); + } + + [Fact] + public void NoKindWithConfig_NotIgnored() + { + Assert.False(TemplateCommands.TriggerKindWillBeIgnored( + triggerConfig: "{\"expression\":\"x > 0\"}", triggerKind: null)); + } +}