fix(m9/T32b): resolve $ref in InboundAPI runtime validators (no deploy-passes/runtime-400); diamond test; ref-annotation message

This commit is contained in:
Joseph Doherty
2026-06-18 12:16:39 -04:00
parent 26e2cdef23
commit 71d5722692
9 changed files with 424 additions and 25 deletions
@@ -104,10 +104,10 @@ public sealed class InboundApiSchema
public static InboundApiSchema? Parse(string? json, Func<string, string?>? resolveRef)
{
var result = ParseWithRefs(json, resolveRef);
if (result.UnresolvedRefs.Count > 0)
if (result.UnresolvedReferences.Count > 0)
{
throw new JsonException(
$"Schema contains unresolved $ref(s): {string.Join(", ", result.UnresolvedRefs)}.");
$"Schema contains unresolved $ref(s): {string.Join(", ", result.UnresolvedReferences.Select(r => r.Describe()))}.");
}
return result.Schema;
@@ -141,7 +141,7 @@ public sealed class InboundApiSchema
return new SchemaParseResult(null, []);
}
var unresolved = new List<string>();
var unresolved = new List<UnresolvedSchemaRef>();
// The active-ref set tracks the refs being resolved on the CURRENT path so a
// cycle (A→B→A) is detected and reported instead of recursing forever.
var ctx = new RefResolutionContext(resolveRef, unresolved, new HashSet<string>(StringComparer.Ordinal));
@@ -166,7 +166,7 @@ public sealed class InboundApiSchema
/// </summary>
private sealed record RefResolutionContext(
Func<string, string?>? Resolver,
List<string> Unresolved,
List<UnresolvedSchemaRef> Unresolved,
HashSet<string> ActiveRefs);
private static InboundApiSchema ParseSchema(JsonElement el, int depth, RefResolutionContext ctx)
@@ -242,6 +242,22 @@ public sealed class InboundApiSchema
/// <summary>The placeholder type for an unresolvable <c>$ref</c> node (dangling, cyclic, or over-depth).</summary>
private const string UnresolvedRefType = "ref";
/// <summary>
/// M9-T32b — cheap pre-flight check: does this definition JSON contain ANY
/// <c>$ref</c> token at all? Lets a caller (e.g. the InboundAPI runtime path) skip the
/// shared-schema library pre-load entirely when a schema uses no references — so a
/// <c>$ref</c>-free method pays NO extra cost beyond today. The check is intentionally
/// conservative (a substring scan, not a parse): it may return <c>true</c> for a
/// schema whose only <c>$ref</c> is not a <c>lib:</c> reference, in which case the
/// subsequent <see cref="ParseWithRefs"/> simply never consults the resolver — correct,
/// just not maximally lazy. It never returns <c>false</c> when a real <c>lib:</c> ref
/// is present, so it is always safe to gate the pre-load on.
/// </summary>
/// <param name="json">The definition JSON to scan; <c>null</c>/empty yields <c>false</c>.</param>
/// <returns><c>true</c> when the JSON contains a <c>$ref</c> token and a resolver may be needed.</returns>
public static bool MightContainRef(string? json) =>
!string.IsNullOrEmpty(json) && json.Contains("$ref", StringComparison.Ordinal);
/// <summary>
/// Recognizes a <c>{"$ref":"lib:Name"}</c> reference node and extracts its target
/// name (the part after the <c>lib:</c> scheme prefix). Returns <c>false</c> for any
@@ -294,14 +310,14 @@ public sealed class InboundApiSchema
// ParseWithRefs path rather than aborting the whole parse with a throw.
if (depth >= MaxDepth)
{
ctx.Unresolved.Add($"{refName} (ref nesting exceeds depth {MaxDepth})");
ctx.Unresolved.Add(new UnresolvedSchemaRef(refName, $"ref nesting exceeds depth {MaxDepth}"));
return new InboundApiSchema { Type = UnresolvedRefType };
}
// Cycle guard: this name is already being resolved on the current path.
if (ctx.ActiveRefs.Contains(refName))
{
ctx.Unresolved.Add($"{refName} (cyclic reference)");
ctx.Unresolved.Add(new UnresolvedSchemaRef(refName, "cyclic reference"));
return new InboundApiSchema { Type = UnresolvedRefType };
}
@@ -309,7 +325,7 @@ public sealed class InboundApiSchema
if (string.IsNullOrWhiteSpace(referenced))
{
// Dangling: the seam can't resolve it (or no seam was supplied).
ctx.Unresolved.Add(refName);
ctx.Unresolved.Add(new UnresolvedSchemaRef(refName, Reason: null));
return new InboundApiSchema { Type = UnresolvedRefType };
}
@@ -591,19 +607,64 @@ public sealed class InboundApiSchema
/// <param name="Schema">The recursive type schema the field's value must satisfy.</param>
public sealed record InboundApiSchemaField(string Name, bool Required, InboundApiSchema Schema);
/// <summary>
/// A single <c>{"$ref":"lib:Name"}</c> reference that could NOT be resolved during
/// <see cref="InboundApiSchema.ParseWithRefs"/> (M9-T32b). The reference <see cref="Name"/>
/// is kept SEPARATE from the diagnostic <see cref="Reason"/> so a message can render the
/// pointer cleanly (e.g. <c>schema 'lib:Foo' could not be resolved (cyclic reference)</c>)
/// rather than embedding the annotation inside the <c>lib:</c>-looking string.
/// </summary>
/// <param name="Name">
/// The library entry name (the part after the <c>lib:</c> scheme prefix) that could not be
/// resolved — never carries an annotation.
/// </param>
/// <param name="Reason">
/// The diagnostic reason for cyclic/over-depth cases (e.g. <c>"cyclic reference"</c> or
/// <c>"ref nesting exceeds depth 64"</c>), or <c>null</c> for a plain dangling reference
/// (the seam returned <c>null</c>, or no seam was supplied).
/// </param>
public sealed record UnresolvedSchemaRef(string Name, string? Reason)
{
/// <summary>
/// Renders this reference as the legacy single-string form (<c>Name</c> for a plain
/// dangling ref, <c>"Name (Reason)"</c> when annotated) — used to project the
/// backward-compatible <see cref="SchemaParseResult.UnresolvedRefs"/> list.
/// </summary>
/// <returns>The reference name, with the reason appended in parentheses when present.</returns>
public string ToLegacyString() => Reason is null ? Name : $"{Name} ({Reason})";
/// <summary>
/// Renders this reference for an end-user error message, keeping the <c>lib:</c>-qualified
/// pointer name separate from the parenthesised reason
/// (e.g. <c>lib:Foo (cyclic reference)</c> or just <c>lib:Foo</c>).
/// </summary>
/// <returns>The <c>lib:</c>-qualified reference, with the reason appended in parentheses when present.</returns>
public string Describe() => Reason is null ? $"lib:{Name}" : $"lib:{Name} ({Reason})";
}
/// <summary>
/// The outcome of <see cref="InboundApiSchema.ParseWithRefs"/> (M9-T32b): the parsed
/// schema (with <c>{"$ref":"lib:Name"}</c> references resolved where possible) plus the
/// names of any references that could NOT be resolved — dangling (the seam returned
/// <c>null</c> or no seam was supplied), cyclic, or over-depth. A non-empty
/// <see cref="UnresolvedRefs"/> is the deploy-blocking signal the validation layer acts on.
/// references that could NOT be resolved — dangling (the seam returned <c>null</c> or no
/// seam was supplied), cyclic, or over-depth. A non-empty <see cref="UnresolvedReferences"/>
/// is the deploy-/runtime-blocking signal the validation layer acts on.
/// </summary>
/// <param name="Schema">The parsed schema, or <c>null</c> when the input was empty.</param>
/// <param name="UnresolvedRefs">
/// The reference targets that could not be resolved, each annotated with the reason for
/// cyclic/over-depth cases (e.g. <c>"Foo (cyclic reference)"</c>). Empty when every
/// reference resolved.
/// <param name="UnresolvedReferences">
/// The structured reference targets that could not be resolved each carrying the bare
/// <see cref="UnresolvedSchemaRef.Name"/> separate from an optional
/// <see cref="UnresolvedSchemaRef.Reason"/>. Empty when every reference resolved.
/// </param>
public sealed record SchemaParseResult(
InboundApiSchema? Schema,
IReadOnlyList<string> UnresolvedRefs);
IReadOnlyList<UnresolvedSchemaRef> UnresolvedReferences)
{
/// <summary>
/// Backward-compatible flat view of <see cref="UnresolvedReferences"/>: each entry is the
/// reference name, with the reason appended in parentheses for cyclic/over-depth cases
/// (e.g. <c>"Foo (cyclic reference)"</c>). Empty when every reference resolved. Prefer
/// <see cref="UnresolvedReferences"/> for new code that needs the name and reason apart.
/// </summary>
public IReadOnlyList<string> UnresolvedRefs =>
UnresolvedReferences.Select(r => r.ToLegacyString()).ToList();
}
@@ -193,7 +193,20 @@ public static class EndpointExtensions
statusCode: 400);
}
var paramResult = ParameterValidator.Validate(body, method.ParameterDefinitions);
// M9-T32b: thread the JSON-Schema $ref resolution seam into parameter
// validation so a method whose ParameterDefinitions use a {"$ref":"lib:Name"}
// resolves the reference at RUNTIME (not just at deploy time). The shared-schema
// library is pre-loaded ONCE per request into an in-memory map (the seam the
// validator consumes is synchronous), and ONLY when the definition actually uses
// a $ref — a $ref-free method skips the repository round-trip entirely. A dangling
// ref surfaces as a clear, descriptive 400 naming the missing reference rather
// than an opaque parse-error 400 (the deploy-passes/runtime-breaks defect).
var sharedSchemaRepo =
httpContext.RequestServices.GetService<ISharedSchemaRepository>();
var resolveRef = await SchemaRefResolver.BuildAsync(
sharedSchemaRepo, [method.ParameterDefinitions], httpContext.RequestAborted);
var paramResult = ParameterValidator.Validate(body, method.ParameterDefinitions, resolveRef);
if (!paramResult.IsValid)
{
return Results.Json(
@@ -5,6 +5,7 @@ using Microsoft.CodeAnalysis.Scripting;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Messages.InboundApi;
using ZB.MOM.WW.ScadaBridge.Commons.Types;
@@ -307,11 +308,24 @@ public class InboundScriptExecutor
? JsonSerializer.Serialize(result)
: null;
// M9-T32b: thread the JSON-Schema $ref resolution seam into return
// validation so a method whose ReturnDefinition uses a {"$ref":"lib:Name"}
// resolves the reference at RUNTIME (not just at deploy time). The
// shared-schema library is pre-loaded ONCE from the per-execution DI scope
// (the seam the validator consumes is synchronous), and ONLY when the
// definition actually uses a $ref — a $ref-free return definition skips the
// repository round-trip entirely. A dangling ref surfaces as a descriptive
// validation failure naming the missing reference, not an opaque parse error.
var sharedSchemaRepo =
(scope?.ServiceProvider ?? _serviceProvider).GetService<ISharedSchemaRepository>();
var resolveRef = await SchemaRefResolver.BuildAsync(
sharedSchemaRepo, [method.ReturnDefinition], cts.Token);
// InboundAPI-014: validate the script's return value against the
// method's declared ReturnDefinition. A method whose script returns a
// shape inconsistent with its definition must not silently emit a
// malformed 200 — surface it as a script failure (500) and log.
var returnValidation = ReturnValueValidator.Validate(resultJson, method.ReturnDefinition);
var returnValidation = ReturnValueValidator.Validate(resultJson, method.ReturnDefinition, resolveRef);
if (!returnValidation.IsValid)
{
_logger.LogWarning(
@@ -30,21 +30,45 @@ public static class ParameterValidator
/// </summary>
/// <param name="body">The parsed JSON request body; null or undefined if no body was supplied.</param>
/// <param name="parameterDefinitions">JSON Schema describing the method's parameters (an object schema), or null/empty when no parameters are defined. The legacy flat-array form is also accepted.</param>
/// <param name="resolveRef">
/// M9-T32b: optional JSON-Schema <c>$ref</c> resolution seam mapping a
/// <c>{"$ref":"lib:Name"}</c> reference target's name to the referenced schema JSON
/// (or <c>null</c> when the library entry does not exist). The endpoint pre-loads the
/// shared-schema library (backed by <c>ISharedSchemaRepository</c>) and supplies it
/// ONLY when the definition actually uses a <c>$ref</c>. <c>null</c> means no resolver:
/// schemas with no <c>$ref</c> behave exactly as before; a <c>$ref</c> with no resolver
/// (or a dangling one) is surfaced as a CLEAR invalid result naming the missing
/// reference — NOT an opaque parse-error 400 from a thrown <see cref="JsonException"/>.
/// </param>
/// <returns>A <see cref="ParameterValidationResult"/> with coerced parameter values on success, or an error message on failure.</returns>
public static ParameterValidationResult Validate(
JsonElement? body,
string? parameterDefinitions)
string? parameterDefinitions,
Func<string, string?>? resolveRef = null)
{
InboundApiSchema? schema;
// M9-T32b: parse through the ref-COLLECTING path. A {"$ref":"lib:Name"} that the
// resolver can satisfy is resolved inline; a dangling/cyclic/over-depth ref is
// collected (not thrown) so the runtime returns a descriptive "could not be
// resolved" message instead of an opaque "Invalid parameter definitions" — the
// deploy-passes/runtime-400 defect this fix closes.
SchemaParseResult parsed;
try
{
schema = InboundApiSchema.Parse(parameterDefinitions);
parsed = InboundApiSchema.ParseWithRefs(parameterDefinitions, resolveRef);
}
catch (JsonException)
{
return ParameterValidationResult.Invalid("Invalid parameter definitions in method configuration");
}
if (parsed.UnresolvedReferences.Count > 0)
{
return ParameterValidationResult.Invalid(
$"Parameter definitions reference {DescribeUnresolved(parsed.UnresolvedReferences)} which could not be resolved");
}
InboundApiSchema? schema = parsed.Schema;
// No parameters defined (or an object schema with no declared fields) —
// the body is unconstrained and yields an empty parameter set.
if (schema is null || schema.Type != "object" || schema.Fields.Count == 0)
@@ -94,6 +118,18 @@ public static class ParameterValidator
return ParameterValidationResult.Valid(result);
}
/// <summary>
/// M9-T32b: renders the unresolved <c>{"$ref":"lib:Name"}</c> references for a clear,
/// descriptive runtime error — the bare <c>lib:</c>-qualified pointer name stays
/// separate from any parenthesised reason (cyclic/over-depth), so the message reads
/// e.g. <c>schema(s) 'lib:Foo' (cyclic reference)</c> rather than embedding the
/// annotation inside the <c>lib:</c>-looking string.
/// </summary>
/// <param name="unresolved">The references that could not be resolved.</param>
/// <returns>A human-readable description of the unresolved schema reference(s).</returns>
internal static string DescribeUnresolved(IReadOnlyList<UnresolvedSchemaRef> unresolved) =>
$"schema(s) {string.Join(", ", unresolved.Select(r => $"'{r.Describe()}'"))}";
/// <summary>
/// Converts a validated JSON element to the CLR value handed to the script.
/// Validation has already passed, so this only shapes the value: scalars to
@@ -36,8 +36,21 @@ public static class ReturnValueValidator
/// </summary>
/// <param name="resultJson">The JSON-serialized script return value to validate.</param>
/// <param name="returnDefinition">JSON Schema describing the method's return value, or null/empty to skip validation. The legacy flat-array form is also accepted.</param>
/// <param name="resolveRef">
/// M9-T32b: optional JSON-Schema <c>$ref</c> resolution seam mapping a
/// <c>{"$ref":"lib:Name"}</c> reference target's name to the referenced schema JSON
/// (or <c>null</c> when the library entry does not exist). The executor pre-loads the
/// shared-schema library (backed by <c>ISharedSchemaRepository</c>) and supplies it
/// ONLY when the definition actually uses a <c>$ref</c>. <c>null</c> means no resolver:
/// schemas with no <c>$ref</c> behave exactly as before; a <c>$ref</c> with no resolver
/// (or a dangling one) is surfaced as a CLEAR invalid result naming the missing
/// reference — NOT an opaque parse-error from a thrown <see cref="JsonException"/>.
/// </param>
/// <returns>A <see cref="ReturnValidationResult"/> indicating success or describing the validation failures.</returns>
public static ReturnValidationResult Validate(string? resultJson, string? returnDefinition)
public static ReturnValidationResult Validate(
string? resultJson,
string? returnDefinition,
Func<string, string?>? resolveRef = null)
{
if (string.IsNullOrWhiteSpace(returnDefinition))
{
@@ -45,10 +58,14 @@ public static class ReturnValueValidator
return ReturnValidationResult.Valid();
}
InboundApiSchema? schema;
// M9-T32b: parse through the ref-COLLECTING path so a {"$ref":"lib:Name"} the
// resolver can satisfy is resolved inline, and a dangling/cyclic/over-depth ref is
// surfaced as a descriptive "could not be resolved" message rather than an opaque
// "Invalid return definition" from a swallowed JsonException.
SchemaParseResult parsed;
try
{
schema = InboundApiSchema.Parse(returnDefinition);
parsed = InboundApiSchema.ParseWithRefs(returnDefinition, resolveRef);
}
catch (JsonException)
{
@@ -56,6 +73,14 @@ public static class ReturnValueValidator
"Invalid return definition in method configuration");
}
if (parsed.UnresolvedReferences.Count > 0)
{
return ReturnValidationResult.Invalid(
$"Return definition references {ParameterValidator.DescribeUnresolved(parsed.UnresolvedReferences)} which could not be resolved");
}
InboundApiSchema? schema = parsed.Schema;
// A schema that declares no constraints (e.g. an object schema with no
// fields) leaves the return value unconstrained.
if (schema is null || (schema.Type == "object" && schema.Fields.Count == 0))
@@ -0,0 +1,62 @@
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Types.InboundApi;
namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
/// <summary>
/// M9-T32b: builds the synchronous JSON-Schema <c>$ref</c> resolution seam the
/// InboundAPI RUNTIME validators (<see cref="ParameterValidator"/> /
/// <see cref="ReturnValueValidator"/>) consume, backed by the central shared-schema
/// library (<see cref="ISharedSchemaRepository"/>).
///
/// <para>
/// Before this fix the runtime validators called the single-arg
/// <c>InboundApiSchema.Parse(json)</c> with NO resolver, so a method whose
/// parameter/return definition used a <c>{"$ref":"lib:Name"}</c> deployed fine
/// (deploy-time validation resolves it) but FAILED at every runtime invocation with
/// an opaque HTTP 400 — a deploy-passes/runtime-breaks defect. This helper mirrors how
/// the deploy path (<c>FlatteningPipeline</c>) pre-loads the library ONCE into an
/// in-memory name→JSON map and exposes a pure synchronous lookup as the seam, avoiding
/// sync-over-async inside the validators.
/// </para>
///
/// <para>
/// The pre-load is gated on <see cref="InboundApiSchema.MightContainRef"/>: a definition
/// that uses no <c>$ref</c> at all skips the repository round-trip entirely and returns
/// <c>null</c> (no resolver), so a <c>$ref</c>-free method pays NO extra cost beyond
/// today's behavior.
/// </para>
/// </summary>
internal static class SchemaRefResolver
{
/// <summary>
/// Builds a <c>lib:Name</c> → schema-JSON resolution seam for the supplied
/// definitions, pre-loading the shared-schema library only when at least one of the
/// definitions actually contains a <c>$ref</c>.
/// </summary>
/// <param name="repository">
/// The shared-schema repository, or <c>null</c> when none is registered (e.g. a bare
/// test-double provider) — then no resolver is produced and any <c>$ref</c> dangles.
/// </param>
/// <param name="definitions">The schema definition JSON strings to be validated (parameter and/or return).</param>
/// <param name="ct">Cancellation token for the library load.</param>
/// <returns>
/// A synchronous <c>name → schema JSON?</c> resolver, or <c>null</c> when no definition
/// uses a <c>$ref</c> (so the validators take their unchanged no-resolver path).
/// </returns>
public static async Task<Func<string, string?>?> BuildAsync(
ISharedSchemaRepository? repository,
IEnumerable<string?> definitions,
CancellationToken ct = default)
{
// Skip the library round-trip entirely unless some definition uses a $ref.
if (repository is null || !definitions.Any(InboundApiSchema.MightContainRef))
{
return null;
}
var library = await repository.ListAsync(ct);
var map = library.ToDictionary(s => s.Name, s => s.SchemaJson, StringComparer.Ordinal);
return name => map.GetValueOrDefault(name);
}
}
@@ -186,10 +186,15 @@ public class ValidationService
return;
}
foreach (var missing in parsed.UnresolvedRefs)
foreach (var missing in parsed.UnresolvedReferences)
{
// Keep the lib:-qualified pointer name SEPARATE from the diagnostic
// reason so the message reads "schema 'lib:Foo' could not be resolved
// (cyclic reference)" rather than embedding the annotation inside the
// lib:-looking string (M9-T32b code-review MINOR).
var reasonSuffix = missing.Reason is null ? string.Empty : $" ({missing.Reason})";
errors.Add(ValidationEntry.Error(ValidationCategory.SchemaReference,
$"Script '{scriptName}' {schemaLabel} references schema 'lib:{missing}' which could not be resolved.",
$"Script '{scriptName}' {schemaLabel} references schema 'lib:{missing.Name}' which could not be resolved{reasonSuffix}.",
scriptName));
}
}