Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 52 additions & 9 deletions src/SharpFM.Model/Scripting/FmScript.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,14 @@
private List<string> ApplyAdd(ScriptStepOperation op)
{
if (op.StepName is null) return ["StepName is required for add operations."];
if (!Registry.StepRegistry.ByName.ContainsKey(op.StepName))
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 labeled HR tokens. POCOs that take
// unlabeled positional params parse them in order.
var hrParams = SynthesizeHrParams(op.Params);
// 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);

Check warning on line 191 in src/SharpFM.Model/Scripting/FmScript.cs

View workflow job for this annotation

GitHub Actions / CI Build (linux)

Argument of type 'Dictionary<string, string?>' cannot be used for parameter 'map' of type 'IReadOnlyDictionary<string, string>' in 'string[] FmScript.SynthesizeHrParams(IReadOnlyDictionary<string, string>? map, StepMetadata metadata)' due to differences in the nullability of reference types.

Check warning on line 191 in src/SharpFM.Model/Scripting/FmScript.cs

View workflow job for this annotation

GitHub Actions / CI Build (linux)

Argument of type 'Dictionary<string, string?>' cannot be used for parameter 'map' of type 'IReadOnlyDictionary<string, string>' in 'string[] FmScript.SynthesizeHrParams(IReadOnlyDictionary<string, string>? map, StepMetadata metadata)' due to differences in the nullability of reference types.
var step = StepDisplayFactory.TryCreate(op.StepName, op.Enabled ?? true, hrParams);
if (step is null)
return [$"No typed POCO factory registered for '{op.StepName}'."];
Expand All @@ -214,7 +215,7 @@
if (metadata is null)
return [$"Apply/update is not supported for step kind '{step.GetType().Name}'."];

var hrParams = SynthesizeHrParams(op.Params);
var hrParams = SynthesizeHrParams(op.Params, metadata);

Check warning on line 218 in src/SharpFM.Model/Scripting/FmScript.cs

View workflow job for this annotation

GitHub Actions / CI Build (linux)

Argument of type 'Dictionary<string, string?>' cannot be used for parameter 'map' of type 'IReadOnlyDictionary<string, string>' in 'string[] FmScript.SynthesizeHrParams(IReadOnlyDictionary<string, string>? map, StepMetadata metadata)' due to differences in the nullability of reference types.

Check warning on line 218 in src/SharpFM.Model/Scripting/FmScript.cs

View workflow job for this annotation

GitHub Actions / CI Build (linux)

Argument of type 'Dictionary<string, string?>' cannot be used for parameter 'map' of type 'IReadOnlyDictionary<string, string>' in 'string[] FmScript.SynthesizeHrParams(IReadOnlyDictionary<string, string>? map, StepMetadata metadata)' due to differences in the nullability of reference types.
var updated = StepDisplayFactory.TryCreate(metadata.Name, step.Enabled, hrParams);
if (updated is null)
return [$"No typed POCO factory registered for '{metadata.Name}'."];
Expand All @@ -223,20 +224,62 @@
return [];
}

private static string[] SynthesizeHrParams(IReadOnlyDictionary<string, string>? map)
/// <summary>
/// Convert a caller's param map into the ordered HR-token form each step's
/// FromDisplay factory expects. Iterates the step's metadata in declared
/// order, formatting each match as <c>"HrLabel: value"</c> when the param
/// has a label and as a raw positional value when it doesn't (e.g.
/// <c>SetVariableStep.Name</c>, <c>IfStep.Condition</c> — these go straight
/// into <c>&lt;Name&gt;</c> / <c>&lt;Calculation&gt;</c> without prefix).
/// </summary>
/// <remarks>
/// The previous implementation labeled every non-empty key, which caused
/// positional params to receive a <c>"Name: $foo"</c> string verbatim into
/// XML — structurally valid but semantically broken under FileMaker.
/// </remarks>
private static string[] SynthesizeHrParams(
IReadOnlyDictionary<string, string>? map,
Registry.StepMetadata metadata)
{
if (map is null || map.Count == 0) return [];

var consumedKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var result = new List<string>(map.Count);

foreach (var paramMeta in metadata.Params)
{
var matchedKey = FindMatchingKey(map, paramMeta.Name);
if (matchedKey is null) continue;

consumedKeys.Add(matchedKey);
var value = map[matchedKey];
result.Add(paramMeta.HrLabel is not null
? $"{paramMeta.HrLabel}: {value}"
: value);
}

// 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)
{
// Bare values (key is empty) are positional tokens; labeled
// entries become "Label: value" tokens which each POCO's
// FromDisplay parser recognizes.
if (consumedKeys.Contains(k)) continue;
result.Add(string.IsNullOrEmpty(k) ? v : $"{k}: {v}");
}

return result.ToArray();
}

private static string? FindMatchingKey(IReadOnlyDictionary<string, string> map, string paramName)
{
foreach (var (k, _) in map)
{
if (string.Equals(k, paramName, StringComparison.OrdinalIgnoreCase))
return k;
}
return null;
}

private List<string> ApplyRemove(ScriptStepOperation op)
{
if (ValidateStepIndex(op.Index) is { } err) return [err];
Expand Down
125 changes: 125 additions & 0 deletions tests/SharpFM.Tests/Scripting/FmScriptApplyTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using System.Collections.Generic;
using System.Linq;
using SharpFM.Model.Scripting;
using SharpFM.Model.Scripting.Steps;
using Xunit;

namespace SharpFM.Tests.Scripting;

/// <summary>
/// Regression coverage for <see cref="FmScript.Apply"/>'s param-map handling.
/// MCP / external callers send structured param dictionaries
/// (<c>{"Name": "$i"}</c>), which the apply path must translate into the
/// HR-token form each step's FromDisplay factory expects. Positional params
/// (no <c>HrLabel</c> in metadata) must arrive unprefixed, otherwise the
/// label leaks into the XML and breaks under FileMaker at runtime.
/// </summary>
public class FmScriptApplyTests
{
private static FmScript EmptyScript() => new(new List<ScriptStep>());

[Fact]
public void ApplyAdd_SetVariable_PositionalNameDoesNotReceiveLabelPrefix()
{
var script = EmptyScript();

var op = new ScriptStepOperation(
Action: "add",
StepName: "Set Variable",
Params: new Dictionary<string, string?> { ["Name"] = "$i", ["Value"] = "1" });

Assert.Empty(script.Apply(op));

var step = Assert.IsType<SetVariableStep>(script.Steps.Single());
Assert.Equal("$i", step.Name);
Assert.Equal("1", step.Value.Text);
}

[Fact]
public void ApplyAdd_ExitLoopIf_PositionalConditionDoesNotReceiveLabelPrefix()
{
var script = EmptyScript();

var op = new ScriptStepOperation(
Action: "add",
StepName: "Exit Loop If",
Params: new Dictionary<string, string?> { ["condition"] = "$i > $limit" });

Assert.Empty(script.Apply(op));

var step = Assert.IsType<ExitLoopIfStep>(script.Steps.Single());
Assert.Equal("$i > $limit", step.Condition.Text);
}

[Fact]
public void ApplyAdd_If_PositionalConditionDoesNotReceiveLabelPrefix()
{
var script = EmptyScript();

var op = new ScriptStepOperation(
Action: "add",
StepName: "If",
Params: new Dictionary<string, string?> { ["condition"] = "$x = 1" });

Assert.Empty(script.Apply(op));

var step = Assert.IsType<IfStep>(script.Steps.Single());
Assert.Equal("$x = 1", step.Condition.Text);
}

[Fact]
public void ApplyAdd_SetVariable_OutOfOrderParams_ProduceCorrectXml()
{
// Agent provides params in arbitrary order; metadata-driven ordering
// ensures the HR token sequence matches what FromDisplay expects.
var script = EmptyScript();

var op = new ScriptStepOperation(
Action: "add",
StepName: "Set Variable",
Params: new Dictionary<string, string?> { ["Value"] = "100", ["Name"] = "$count" });

Assert.Empty(script.Apply(op));

var step = Assert.IsType<SetVariableStep>(script.Steps.Single());
Assert.Equal("$count", step.Name);
Assert.Equal("100", step.Value.Text);
}

[Fact]
public void ApplyAdd_SetVariable_KeyMatchIsCaseInsensitive()
{
var script = EmptyScript();

var op = new ScriptStepOperation(
Action: "add",
StepName: "Set Variable",
Params: new Dictionary<string, string?> { ["name"] = "$lc", ["value"] = "5" });

Assert.Empty(script.Apply(op));

var step = Assert.IsType<SetVariableStep>(script.Steps.Single());
Assert.Equal("$lc", step.Name);
Assert.Equal("5", step.Value.Text);
}

[Fact]
public void ApplyUpdate_SetVariable_PositionalNameDoesNotReceiveLabelPrefix()
{
var script = EmptyScript();
script.Apply(new ScriptStepOperation(
Action: "add",
StepName: "Set Variable",
Params: new Dictionary<string, string?> { ["Name"] = "$old", ["Value"] = "1" }));

var update = new ScriptStepOperation(
Action: "update",
Index: 0,
Params: new Dictionary<string, string?> { ["Name"] = "$new" });

Assert.Empty(script.Apply(update));

var step = Assert.IsType<SetVariableStep>(script.Steps[0]);
Assert.Equal("$new", step.Name);
}
}
Loading