diff --git a/src/SharpFM.Model/Scripting/FmScript.cs b/src/SharpFM.Model/Scripting/FmScript.cs index 9c07e48..b079db5 100644 --- a/src/SharpFM.Model/Scripting/FmScript.cs +++ b/src/SharpFM.Model/Scripting/FmScript.cs @@ -192,15 +192,15 @@ private List ApplyAdd(ScriptStepOperation op) if (!Registry.StepRegistry.ByName.TryGetValue(op.StepName, out var metadata)) return [$"Unknown step name '{op.StepName}'."]; - // Route through each POCO's FromDisplay factory with the caller's - // param map synthesized into HR tokens. The synthesizer consults the - // step's metadata so positional params (no HrLabel) pass as raw - // values and labeled params get the canonical "Label: value" form. - var hrParams = SynthesizeHrParams(op.Params, metadata); - var step = StepDisplayFactory.TryCreate(op.StepName, op.Enabled ?? true, hrParams); + // Construct a blank instance through the registry factory, then set + // each caller param directly onto the POCO's shape-bound properties. + var step = StepDisplayFactory.TryCreate(metadata.Name, op.Enabled ?? true, []); if (step is null) return [$"No typed POCO factory registered for '{op.StepName}'."]; + var errors = ApplyParams(step, op.Params); + if (errors.Count > 0) return errors; + var index = op.Index < 0 || op.Index >= Steps.Count ? Steps.Count : op.Index; Steps.Insert(index, step); return []; @@ -211,74 +211,42 @@ private List ApplyUpdate(ScriptStepOperation op) if (ValidateStepIndex(op.Index) is { } err) return [err]; var step = Steps[op.Index]; - if (op.Enabled is not null) step.Enabled = op.Enabled.Value; - - if (op.Params is null) return []; - - // Param updates are rebuilt by re-parsing the display form with the - // new param map overlaid onto the old. This uses the POCO's own - // FromDisplay factory — the same path ApplyAdd takes — so the - // update never leaves the typed-POCO world. - var metadata = Registry.StepRegistry.MetadataFor(step); - if (metadata is null) + if (op.Params is not null && op.Params.Count > 0 + && Registry.StepRegistry.MetadataFor(step) is null) + { return [$"Apply/update is not supported for step kind '{step.GetType().Name}'."]; + } - var hrParams = SynthesizeHrParams(op.Params, metadata); - var updated = StepDisplayFactory.TryCreate(metadata.Name, step.Enabled, hrParams); - if (updated is null) - return [$"No typed POCO factory registered for '{metadata.Name}'."]; + if (op.Enabled is not null) step.Enabled = op.Enabled.Value; - Steps[op.Index] = updated; - return []; + return ApplyParams(step, op.Params); } /// - /// Convert a caller's param map into the ordered HR-token form each step's - /// FromDisplay factory expects. Iterates the shape's display slots in - /// display order, formatting each match as "HrLabel: value" when the - /// slot has a label and as a raw positional value when it doesn't (e.g. - /// SetVariableStep.Name, IfStep.Condition — these go straight - /// into <Name> / <Calculation> without prefix). - /// Keys may address a slot by its bound property, its XML element, or its - /// display label, so pre-cutover param names keep working. + /// Sets each param onto the step's shape-bound public properties in + /// place, through 's virtual dispatch. + /// Properties the map does not name keep their current values. Params + /// apply in map order and application is not atomic: on error, params + /// earlier in the map have already been set. /// - /// - /// An earlier implementation labeled every non-empty key, which caused - /// positional params to receive a "Name: $foo" string verbatim into - /// XML — structurally valid but semantically broken under FileMaker. - /// - private static string[] SynthesizeHrParams( - IReadOnlyDictionary? map, - Registry.StepMetadata metadata) + private static List ApplyParams(ScriptStep step, IReadOnlyDictionary? map) { if (map is null || map.Count == 0) return []; - var consumedKeys = new HashSet(StringComparer.OrdinalIgnoreCase); - var result = new List(map.Count); - - foreach (var node in Shapes.ShapeHrView.HrNodes(metadata.Shape)) + var errors = new List(); + foreach (var (name, value) in map) { - var matchedKey = map.Keys.FirstOrDefault(k => - !consumedKeys.Contains(k) && Shapes.ShapeHrView.MatchesName(node, k)); - if (matchedKey is null) continue; - - consumedKeys.Add(matchedKey); - var value = map[matchedKey]; - result.Add(node.HrLabel is not null - ? $"{node.HrLabel}: {value}" - : value); - } + if (value is null) + { + errors.Add($"Param '{name}' has no value."); + continue; + } - // Forward-compat: any keys we don't recognise pass through with the - // old formatting so newly-introduced params keep working until their - // metadata catches up. - foreach (var (k, v) in map) - { - if (consumedKeys.Contains(k)) continue; - result.Add(string.IsNullOrEmpty(k) ? v : $"{k}: {v}"); + if (step.ApplyParam(name, value) is { } error) + errors.Add(error); } - return result.ToArray(); + return errors; } private List ApplyRemove(ScriptStepOperation op) diff --git a/src/SharpFM.Model/Scripting/ScriptStep.cs b/src/SharpFM.Model/Scripting/ScriptStep.cs index fc4a226..e7f46b6 100644 --- a/src/SharpFM.Model/Scripting/ScriptStep.cs +++ b/src/SharpFM.Model/Scripting/ScriptStep.cs @@ -42,6 +42,17 @@ protected ScriptStep(bool enabled) /// protected internal abstract void PopulateFromDisplay(string[] hrParams); + /// + /// Sets one shape-bound public property from an HR-friendly text value, + /// mutating this instance in place; properties not named are untouched. + /// Returns null on success, or a human-readable error message. Steps + /// whose slots need hand grammar (button blocks, variant targets) + /// override this and fall back to base for everything else. The base + /// rejects everything — RawStep has no param surface. + /// + protected internal virtual string? ApplyParam(string name, string value) => + $"Step '{GetType().Name}' does not support param updates."; + public virtual List Validate(int lineIndex) => new(); /// diff --git a/src/SharpFM.Model/Scripting/ScriptStepOfT.cs b/src/SharpFM.Model/Scripting/ScriptStepOfT.cs index 89fd1a5..9209005 100644 --- a/src/SharpFM.Model/Scripting/ScriptStepOfT.cs +++ b/src/SharpFM.Model/Scripting/ScriptStepOfT.cs @@ -30,6 +30,9 @@ protected internal override void PopulateFromXml(XElement step) => protected internal override void PopulateFromDisplay(string[] hrParams) => StepDisplayParser.Populate(this, hrParams, TSelf.Metadata); + protected internal override string? ApplyParam(string name, string value) => + StepParamApplier.Apply(this, TSelf.Metadata, name, value); + /// Typed counterpart of the dispatch /// for callers that already know the step kind. public static TSelf Parse(XElement step) diff --git a/src/SharpFM.Model/Scripting/Serialization/ShapeReflection.cs b/src/SharpFM.Model/Scripting/Serialization/ShapeReflection.cs index dfa36e9..0bb7386 100644 --- a/src/SharpFM.Model/Scripting/Serialization/ShapeReflection.cs +++ b/src/SharpFM.Model/Scripting/Serialization/ShapeReflection.cs @@ -38,4 +38,12 @@ public static void Set(object target, string name, object? value) /// Declared type of a shape-bound property (for typed list parsing). public static Type PropertyType(object source, string name) => Prop(source.GetType(), name).PropertyType; + + /// + /// True when the shape-bound property has a setter. False marks an + /// emit-only projection (e.g. a wire-order alias of another property), + /// which callers that need the write to actually land must route around. + /// + public static bool CanWrite(object target, string name) => + Prop(target.GetType(), name).CanWrite; } diff --git a/src/SharpFM.Model/Scripting/Serialization/StepDisplayParser.cs b/src/SharpFM.Model/Scripting/Serialization/StepDisplayParser.cs index 6f1f450..8f57d94 100644 --- a/src/SharpFM.Model/Scripting/Serialization/StepDisplayParser.cs +++ b/src/SharpFM.Model/Scripting/Serialization/StepDisplayParser.cs @@ -131,7 +131,7 @@ private static void Assign(object target, ShapeNode node, string value) } /// Inverse of the renderer's wire→display translation. - private static string ToWireValue(ShapeNode node, string display) + internal static string ToWireValue(ShapeNode node, string display) { if (node.DisplayValues is null || node.ValidValues is null) return display; var i = node.DisplayValues.ToList().FindIndex(v => v.Equals(display, StringComparison.OrdinalIgnoreCase)); diff --git a/src/SharpFM.Model/Scripting/Serialization/StepParamApplier.cs b/src/SharpFM.Model/Scripting/Serialization/StepParamApplier.cs new file mode 100644 index 0000000..5da0b36 --- /dev/null +++ b/src/SharpFM.Model/Scripting/Serialization/StepParamApplier.cs @@ -0,0 +1,286 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Xml.Linq; +using SharpFM.Model.Scripting.Registry; +using SharpFM.Model.Scripting.Shapes; +using SharpFM.Model.Scripting.Values; + +namespace SharpFM.Model.Scripting.Serialization; + +/// +/// Applies one named HR-friendly param value onto a step POCO's shape-bound +/// public property, mutating the instance in place — the structured-caller +/// counterpart of . Where display parsing is +/// tolerant by contract (display text is user-edited), param application is +/// validating: unknown names, unrecognized values, and out-of-range values +/// return an error instead of being silently dropped. +/// +/// +/// Slots the typed conversion cannot set — , variant / +/// value-type / list slots, and slots bound to emit-only projection +/// properties — route through the step's own display-token grammar via +/// , so any value a step can parse from +/// its display line is also settable as a param, with no per-step wiring. +/// +/// +internal static class StepParamApplier +{ + /// + /// Resolves to a display slot (by bound property, + /// XML element, or display label — see ) + /// and assigns the converted value. Returns null on success, or a + /// human-readable error message. + /// + public static string? Apply(ScriptStep target, StepMetadata meta, string name, string value) + { + // Several shapes declare an HrOnly display alias next to the slot + // that actually binds the property (e.g. Insert Audio/Video's + // UniversalPathList), so prefer a typed-assignable match over the + // first name match. + var matches = ShapeHrView.HrNodes(meta.Shape).Where(n => ShapeHrView.MatchesName(n, name)).ToList(); + if (matches.Count == 0) + return $"Unknown param '{name}' for step '{meta.Name}'."; + + var slot = matches.FirstOrDefault(n => IsTypedAssignable(target, n)) ?? matches[0]; + return Assign(target, meta, slot, name, value); + } + + /// + /// True when the typed conversion in can set this + /// slot directly: a convertible node kind whose bound property is + /// writable. Emit-only projections (e.g. Perform Script's + /// ParameterBeforeScript) and enums whose display values have no paired + /// wire values are excluded — they route through the display grammar, + /// which reaches the real storage. + /// + private static bool IsTypedAssignable(ScriptStep target, ShapeNode node) => node switch + { + BoolStateChild b => ShapeReflection.CanWrite(target, b.PocoProperty ?? b.Element), + FlagChild f => ShapeReflection.CanWrite(target, f.PocoProperty ?? f.Element), + // Display→wire translation pairs DisplayValues with ValidValues by + // index; DisplayValues alone leaves the wire form unknowable. + EnumValueChild e => ShapeReflection.CanWrite(target, e.PocoProperty ?? e.Element) + && !(e.DisplayValues is { Count: > 0 } && e.ValidValues is not { Count: > 0 }), + BareCalcChild => ShapeReflection.CanWrite(target, node.PocoProperty ?? "Calculation"), + NamedCalcChild nc => ShapeReflection.CanWrite(target, nc.PocoProperty ?? nc.Element), + NamedTextChild nt => ShapeReflection.CanWrite(target, nt.PocoProperty ?? nt.Element), + FieldChild f => ShapeReflection.CanWrite(target, f.PocoProperty ?? f.Element), + NamedRefChild nr => ShapeReflection.CanWrite(target, nr.PocoProperty ?? nr.Element), + _ => false, + }; + + private static string? Assign(ScriptStep target, StepMetadata meta, ShapeNode node, string name, string value) + { + if (!IsTypedAssignable(target, node)) + { + return value.TrimStart().StartsWith('<') + ? ApplyXmlFragment(target, meta, node, name, value) + : ApplyViaDisplayGrammar(target, meta, node, name, value); + } + + switch (node) + { + case BoolStateChild b: + if (!TryParseOnOff(value, out var state)) + return OnOffError(meta, name, value); + ShapeReflection.Set(target, b.PocoProperty ?? b.Element, state != b.DisplayInverted); + return null; + + case FlagChild f: + if (!TryParseOnOff(value, out var present)) + return OnOffError(meta, name, value); + ShapeReflection.Set(target, f.PocoProperty ?? f.Element, present != f.DisplayInverted); + return null; + + case EnumValueChild e: + var wire = StepDisplayParser.ToWireValue(e, value); + if (e.ValidValues is { Count: > 0 } valid && !valid.Contains(wire, StringComparer.OrdinalIgnoreCase)) + return $"Param '{name}' of step '{meta.Name}' must be one of: " + + $"{string.Join(", ", ShapeHrView.DisplayValuesOf(e))}. Got '{value}'."; + ShapeReflection.Set(target, e.PocoProperty ?? e.Element, wire); + return null; + + case BareCalcChild: + ShapeReflection.Set(target, node.PocoProperty ?? "Calculation", new Calculation(value)); + return null; + + case NamedCalcChild nc: + ShapeReflection.Set(target, nc.PocoProperty ?? nc.Element, new Calculation(value)); + return null; + + case NamedTextChild nt: + ShapeReflection.Set(target, nt.PocoProperty ?? nt.Element, value); + return null; + + case FieldChild f: + if (value.Length == 0) + return $"Param '{name}' of step '{meta.Name}' requires a field reference " + + "(e.g. \"Table::Field\" or \"$variable\")."; + ShapeReflection.Set(target, f.PocoProperty ?? f.Element, FieldRef.FromDisplayToken(value)); + return null; + + case NamedRefChild nr: + // The text form carries the name only; id 0 is the unknown sentinel. + ShapeReflection.Set(target, nr.PocoProperty ?? nr.Element, new NamedRef(0, value)); + return null; + + default: + return ApplyViaDisplayGrammar(target, meta, node, name, value); + } + } + + /// + /// Routes a value through the step's own display-token grammar. The + /// synthesized token is validated against a blank probe first: it must + /// produce a wire-level change there, otherwise the grammar did not + /// recognize it and re-parsing could silently reset state. The accepted + /// token is then re-parsed onto the live instance together with the + /// step's current display tokens — those re-assert the display-visible + /// state, and properties display text does not carry stay untouched. + /// Current tokens carrying the same label are dropped so the new token + /// wins under both first-match and last-match parser styles; raw tokens + /// are appended, which the raw-form parsers resolve last-wins. + /// + private static string? ApplyViaDisplayGrammar(ScriptStep target, StepMetadata meta, ShapeNode node, string name, string value) + { + var blank = StepDisplayFactory.TryCreate(meta.Name, true, []); + if (blank is null) + return $"No typed POCO factory registered for '{meta.Name}'."; + var blankXml = blank.ToXml().ToString(); + + foreach (var (token, labelPrefix) in CandidateTokens(target, node, value)) + { + var probe = StepDisplayFactory.TryCreate(meta.Name, true, [token]); + if (probe is null || probe.ToXml().ToString() == blankXml) continue; + + IEnumerable retained = ScriptLineParser.ParseLine(target.ToDisplayLine()).Params; + if (labelPrefix is not null) + retained = retained.Where(t => !t.TrimStart().StartsWith(labelPrefix, StringComparison.OrdinalIgnoreCase)); + var current = retained.ToList(); + + // The value is already in the display line — nothing to change. + if (current.Any(t => t.Trim().Equals(token, StringComparison.OrdinalIgnoreCase))) + return null; + + // Parsers differ in whether the first or the last matching token + // wins, so try the new token in both positions and require a + // wire-level change. A no-change re-parse only re-assigns the + // display-visible state to itself, so the failed order is + // harmless. + var before = target.ToXml().ToString(); + + target.PopulateFromDisplay([.. current, token]); + if (target.ToXml().ToString() != before) return null; + + target.PopulateFromDisplay([token, .. current]); + if (target.ToXml().ToString() != before) return null; + } + + var elements = WireElementsOf(node); + var hint = elements.Count > 0 + ? $" This param also accepts a <{elements[0]}> XML fragment." + : " Edit the step XML for exact control."; + return $"Param '{name}' of step '{meta.Name}' did not accept value '{value}'; " + + $"it may match the step's default.{hint}"; + } + + /// + /// Grafts an XML fragment into the step's wire form: the fragment + /// replaces the step element's existing children of the same name and + /// the merged element is re-read in place through + /// . The fragment's root must be + /// one of the slot's wire elements (an slot names + /// its wire element), and the re-emitted step must retain it — a reader + /// that drops the element would silently discard the caller's data, so + /// the original state is restored and an error returned instead. + /// + private static string? ApplyXmlFragment(ScriptStep target, StepMetadata meta, ShapeNode node, string name, string value) + { + XElement fragment; + try + { + fragment = XElement.Parse(value); + } + catch (System.Xml.XmlException ex) + { + return $"Param '{name}' of step '{meta.Name}' looks like an XML fragment but does not parse: {ex.Message}"; + } + + var elements = WireElementsOf(node); + if (elements.Count == 0) + return $"Param '{name}' of step '{meta.Name}' has no wire element to set from an XML fragment; " + + "edit the full step XML instead."; + if (!elements.Contains(fragment.Name.LocalName, StringComparer.OrdinalIgnoreCase)) + return $"Param '{name}' of step '{meta.Name}' takes " + + $"{string.Join(" or ", elements.Select(e => $"<{e}>"))} as an XML fragment; got <{fragment.Name.LocalName}>."; + + var merged = target.ToXml(); + var before = merged.ToString(); + merged.Elements() + .Where(e => string.Equals(e.Name.LocalName, fragment.Name.LocalName, StringComparison.OrdinalIgnoreCase)) + .Remove(); + merged.Add(fragment); + target.PopulateFromXml(merged); + + var after = target.ToXml(); + if (after.Elements().Any(e => string.Equals(e.Name.LocalName, fragment.Name.LocalName, StringComparison.OrdinalIgnoreCase))) + return null; + + target.PopulateFromXml(XElement.Parse(before)); + return $"Step '{meta.Name}' did not retain the <{fragment.Name.LocalName}> element; " + + "edit the full step XML instead."; + } + + private static List WireElementsOf(ShapeNode node) + { + var elements = StepXmlValidator.ElementNamesOf(node).ToList(); + if (node is HrOnly h) elements.Add(h.Name); + return elements; + } + + // A raw (unlabeled) token is only meaningful to a hand-written display + // parser, which recognizes tokens by form; the shape-driven parser would + // bind it to the first unused positional slot — the wrong one. For a + // slot with no HrLabel the raw form is canonical and goes first (a + // free-text grammar would swallow a name-prefixed token verbatim); the + // name-prefixed form is the rescue for grammars keyed on the slot name + // (e.g. Show Custom Dialog's "Buttons:"). + private static IEnumerable<(string Token, string? LabelPrefix)> CandidateTokens(ScriptStep target, ShapeNode node, string value) + { + var handParser = HasHandWrittenDisplayParser(target.GetType()); + var label = node.HrLabel ?? ShapeHrView.NameOf(node); + + if (node.HrLabel is null && handParser) + yield return (value, null); + + if (label.Length > 0) + yield return ($"{label}: {value}", $"{label}:"); + + if (node.HrLabel is not null && handParser) + yield return (value, null); + } + + private static readonly ConcurrentDictionary _handDisplayParser = new(); + + /// + /// True when the step overrides + /// itself rather than inheriting the shape-driven default declared on the + /// generic ScriptStep<TSelf> base. + /// + private static bool HasHandWrittenDisplayParser(Type stepType) => + _handDisplayParser.GetOrAdd(stepType, t => + t.GetMethod(nameof(ScriptStep.PopulateFromDisplay), BindingFlags.NonPublic | BindingFlags.Instance) + ?.DeclaringType?.IsGenericType == false); + + private static string OnOffError(StepMetadata meta, string name, string value) => + $"Param '{name}' of step '{meta.Name}' must be 'On' or 'Off' (got '{value}')."; + + private static bool TryParseOnOff(string value, out bool on) + { + on = value.Equals("On", StringComparison.OrdinalIgnoreCase); + return on || value.Equals("Off", StringComparison.OrdinalIgnoreCase); + } +} diff --git a/src/SharpFM.Model/Scripting/Steps/ShowCustomDialogStep.cs b/src/SharpFM.Model/Scripting/Steps/ShowCustomDialogStep.cs index 9ad1d03..130084b 100644 --- a/src/SharpFM.Model/Scripting/Steps/ShowCustomDialogStep.cs +++ b/src/SharpFM.Model/Scripting/Steps/ShowCustomDialogStep.cs @@ -388,7 +388,10 @@ private static (string CalcText, string Keyword) SplitTrailingKeyword(string slo new NamedCalcChild("DistanceFromLeft") { Optional = true, Display = DisplayMode.Hidden }, new Passthrough { PocoProperty = "ButtonsAndInputsWire" }, new HrOnly("Buttons"), - new HrOnly("InputFields"), + // HrLabel matches the display grammar's "Inputs:" prefix so + // label-addressed lookups resolve to the token form the parser + // actually recognizes. + new HrOnly("InputFields") { HrLabel = "Inputs" }, ], }; } diff --git a/tests/SharpFM.Tests/Scripting/FmScriptApplyTests.cs b/tests/SharpFM.Tests/Scripting/FmScriptApplyTests.cs index 6c634a2..81f1a0f 100644 --- a/tests/SharpFM.Tests/Scripting/FmScriptApplyTests.cs +++ b/tests/SharpFM.Tests/Scripting/FmScriptApplyTests.cs @@ -2,6 +2,7 @@ using System.Linq; using SharpFM.Model.Scripting; using SharpFM.Model.Scripting.Steps; +using SharpFM.Model.Scripting.Values; using Xunit; namespace SharpFM.Tests.Scripting; @@ -122,4 +123,479 @@ public void ApplyUpdate_SetVariable_PositionalNameDoesNotReceiveLabelPrefix() var step = Assert.IsType(script.Steps[0]); Assert.Equal("$new", step.Name); } + + [Fact] + public void ApplyUpdate_PreservesParamsNotInMap() + { + var script = EmptyScript(); + script.Apply(new ScriptStepOperation( + Action: "add", + StepName: "Set Variable", + Params: new Dictionary { ["Name"] = "$x", ["Value"] = "1" })); + + var update = new ScriptStepOperation( + Action: "update", + Index: 0, + Params: new Dictionary { ["Value"] = "2" }); + + Assert.Empty(script.Apply(update)); + + var step = Assert.IsType(script.Steps[0]); + Assert.Equal("$x", step.Name); + Assert.Equal("2", step.Value.Text); + } + + [Fact] + public void ApplyUpdate_MutatesStepInstanceInPlace() + { + var script = EmptyScript(); + script.Apply(new ScriptStepOperation( + Action: "add", + StepName: "Set Variable", + Params: new Dictionary { ["Name"] = "$x", ["Value"] = "1" })); + var before = script.Steps[0]; + + var update = new ScriptStepOperation( + Action: "update", + Index: 0, + Params: new Dictionary { ["Value"] = "2" }); + + Assert.Empty(script.Apply(update)); + + Assert.Same(before, script.Steps[0]); + } + + [Fact] + public void ApplyUpdate_PreservesStateDisplayTextCannotCarry() + { + // Custom dialog buttons only exist in the XML; a Title-only update + // must not reset them (or the Message) to defaults. + var script = FmScript.FromXml(""" + + + <Calculation><![CDATA["Hi"]]></Calculation> + + + + +