Merge feature/native-typed-json: native-typed JSON for List attribute values + data normalization

List values now encode as native-typed JSON ([10,20], [true,false], ISO dates; strings stay quoted) via AttributeValueCodec; Decode reads both native and the earlier array-of-strings form for every element type. Already-persisted old-form data is normalized on the fly: idempotent central startup normalizer (ListValueNormalizer), active site-SQLite normalization on InstanceActor override-load, and normalize-on-import in the bundle importer. Instance-override writes now stamp ElementDataType (#93/M3). Full solution 0/0; feature-targeted tests green. Plan: docs/plans/2026-06-16-native-typed-json.md.
2026-06-16 18:36:07 -04:00
parent d312dfb139 dc9f31537a
commit 33af948651
17 changed files with 1289 additions and 18 deletions
@@ -25,11 +25,10 @@ public static class AttributeValueCodec
            case string s: return s;                       // already canonical
            case IFormattable f: return f.ToString(null, CultureInfo.InvariantCulture);
            case IEnumerable e:
-                var items = e.Cast<object?>()
-                             .Select(x => x is IFormattable xf
-                                 ? xf.ToString(null, CultureInfo.InvariantCulture)
-                                 : x?.ToString());
-                return JsonSerializer.Serialize(items, JsonOpts);
+                // Native-typed JSON: serialize the runtime collection so System.Text.Json emits
+                // numbers/bools unquoted, strings quoted, and DateTime as ISO-8601. Boxed as object
+                // so STJ uses the runtime element type. STJ numbers/dates are culture-invariant.
+                return JsonSerializer.Serialize<object>(e, JsonOpts);
            default: return value.ToString();
        }
    }
@@ -46,18 +45,25 @@ public static class AttributeValueCodec
        if (elementType is null)
            throw new FormatException("List attribute requires an element type.");

-        string?[] raw;
-        try { raw = JsonSerializer.Deserialize<string?[]>(value) ?? []; }
+        JsonElement[] raw;
+        try { raw = JsonSerializer.Deserialize<JsonElement[]>(value) ?? []; }
        catch (JsonException ex) { throw new FormatException("Malformed list JSON.", ex); }

        var clrType = ElementClrType(elementType.Value);
        var listType = typeof(List<>).MakeGenericType(clrType);
        var result = (IList)Activator.CreateInstance(listType)!;
-        foreach (var item in raw)
-            result.Add(ParseScalar(item, elementType.Value));
+        foreach (var el in raw)
+            result.Add(ParseScalar(JsonElementToString(el), elementType.Value));
        return result;
    }

+    private static string? JsonElementToString(JsonElement el) => el.ValueKind switch
+    {
+        JsonValueKind.String => el.GetString(),   // old form, or string-typed lists
+        JsonValueKind.Null => null,               // ParseScalar throws "may not be null"
+        _ => el.GetRawText()                       // number/bool → "10" / "1.5" / "true"
+    };
+
    private static Type ElementClrType(DataType t) => t switch
    {
        DataType.String => typeof(string),
@@ -0,0 +1,118 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.ScadaBridge.Commons.Types;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+
+namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase;
+
+/// <summary>
+/// Idempotent central startup normalizer that rewrites already-persisted List attribute
+/// values from the old array-of-strings JSON form (<c>["10","20"]</c>) to the new
+/// native-typed form (<c>[10,20]</c>).
+///
+/// <para>
+/// This is a safety net: at the time of writing there is no deployed List attribute data,
+/// so in practice it finds nothing to rewrite. It runs on every central startup after
+/// migrations apply and MUST never abort startup for data reasons — every row's work is
+/// wrapped in a per-row try/catch that logs and skips on malformed data. A second run
+/// finds nothing to change (native → native re-encode is byte-identical).
+/// </para>
+/// </summary>
+public static class ListValueNormalizer
+{
+    /// <summary>
+    /// Rewrites old-form List attribute values to the native-typed JSON form across
+    /// <see cref="ScadaBridgeDbContext.TemplateAttributes"/> and
+    /// <see cref="ScadaBridgeDbContext.InstanceAttributeOverrides"/>. Idempotent and
+    /// best-effort: malformed rows are logged and skipped, never rethrown.
+    /// </summary>
+    /// <param name="db">The configuration database context.</param>
+    /// <param name="logger">Optional logger for diagnostics.</param>
+    /// <param name="ct">Cancellation token.</param>
+    /// <returns>A task that represents the asynchronous operation.</returns>
+    public static async Task NormalizeAsync(
+        ScadaBridgeDbContext db,
+        ILogger? logger = null,
+        CancellationToken ct = default)
+    {
+        var rewritten = 0;
+
+        // TemplateAttributes: List rows carry the element type on the row itself.
+        var templateRows = await db.TemplateAttributes
+            .Where(a => a.DataType == DataType.List)
+            .ToListAsync(ct);
+
+        foreach (var a in templateRows)
+        {
+            try
+            {
+                var native = AttributeValueCodec.Encode(
+                    AttributeValueCodec.Decode(a.Value, DataType.List, a.ElementDataType));
+                if (native != a.Value)
+                {
+                    a.Value = native;
+                    rewritten++;
+                }
+            }
+            catch (Exception ex)
+            {
+                // Never abort startup for a single bad row. FormatException from Decode is the
+                // expected case; the broad catch also covers an unexpected serialize failure
+                // (e.g. a JsonException on a non-finite value) so one poison row can't crash boot.
+                logger?.LogWarning(ex,
+                    "List value normalizer: skipping unprocessable list value for TemplateAttribute {Id}.",
+                    a.Id);
+            }
+        }
+
+        // InstanceAttributeOverrides: only rows that carry an element type are List rows.
+        // Rows with a null ElementDataType are scalar/legacy rows (no deployed List data
+        // exists, so none in practice) and are skipped.
+        var overrideRows = await db.InstanceAttributeOverrides
+            .Where(o => o.ElementDataType != null)
+            .ToListAsync(ct);
+
+        foreach (var o in overrideRows)
+        {
+            try
+            {
+                var native = AttributeValueCodec.Encode(
+                    AttributeValueCodec.Decode(o.OverrideValue, DataType.List, o.ElementDataType));
+                if (native != o.OverrideValue)
+                {
+                    o.OverrideValue = native;
+                    rewritten++;
+                }
+            }
+            catch (Exception ex)
+            {
+                logger?.LogWarning(ex,
+                    "List value normalizer: skipping unprocessable list value for InstanceAttributeOverride {Id}.",
+                    o.Id);
+            }
+        }
+
+        try
+        {
+            await db.SaveChangesAsync(ct);
+        }
+        catch (Exception ex)
+        {
+            // A catastrophic DB failure on SaveChanges may propagate, but log it first so
+            // startup diagnostics are not silent. Per-row data problems are already handled
+            // above and never reach here.
+            logger?.LogError(ex, "List value normalizer: SaveChanges failed.");
+            throw;
+        }
+
+        if (rewritten > 0)
+        {
+            logger?.LogInformation(
+                "List value normalizer: rewrote {n} attribute value(s) to native JSON.", rewritten);
+        }
+        else
+        {
+            logger?.LogDebug("List value normalizer: no attribute values required rewriting.");
+        }
+    }
+}
@@ -47,6 +47,12 @@ public static class MigrationHelper
                    "Apply migrations using 'dotnet ef database update' or the generated SQL scripts before starting in production mode.");
            }
        }
+
+        // Safety-net normalizer: rewrite any already-persisted List attribute values from the
+        // old array-of-strings JSON form to the new native-typed form. Idempotent and never
+        // aborts startup for data reasons (per-row skip + log). Safe even if both central
+        // nodes run it concurrently on startup.
+        await ListValueNormalizer.NormalizeAsync(dbContext, logger, cancellationToken);
    }

    private static async Task WaitForDatabaseReadyAsync(
@@ -948,10 +948,33 @@ public class InstanceActor : ReceiveActor
            if (_resolvedAttributeByName.TryGetValue(kvp.Key, out var resolved)
                && IsListAttribute(resolved))
            {
+                // NJ-4: decode the stored List override (both old array-of-strings
+                // and native-typed forms decode) and re-persist the native form if
+                // the stored value is still in the OLD form. Re-encoding the decoded
+                // list and comparing to the stored string detects old-form values
+                // (native → native is byte-identical, so a native value is a no-op).
+                // The re-persist is fire-and-forget and never throws into the actor.
                var decoded = DecodeAttributeValue(resolved, kvp.Value);
                _attributes[kvp.Key] = decoded;
                if (decoded is null && !string.IsNullOrEmpty(kvp.Value))
+                {
                    _attributeQualities[kvp.Key] = "Bad";
+                }
+                else if (decoded is not null)
+                {
+                    var native = AttributeValueCodec.Encode(decoded);
+                    if (native != kvp.Value)   // stored value was old-form → normalize on disk
+                    {
+                        var key = kvp.Key;
+                        var logger = _logger;
+                        var instanceName = _instanceUniqueName;
+                        _storage.SetStaticOverrideAsync(instanceName, key, native!)
+                            .ContinueWith(t => logger.LogWarning(t.Exception?.GetBaseException(),
+                                "Failed to normalize static override {Instance}.{Attr} to native JSON",
+                                instanceName, key),
+                                TaskContinuationOptions.OnlyOnFaulted);
+                    }
+                }
            }
            else
            {
@@ -172,6 +172,7 @@ public class InstanceService
        if (existingOverride != null)
        {
            existingOverride.OverrideValue = overrideValue;
+            existingOverride.ElementDataType = templateAttr.ElementDataType;
            await _repository.UpdateInstanceAttributeOverrideAsync(existingOverride, cancellationToken);
            await _repository.SaveChangesAsync(cancellationToken);

@@ -185,7 +186,8 @@ public class InstanceService
            var newOverride = new InstanceAttributeOverride(attributeName)
            {
                InstanceId = instanceId,
-                OverrideValue = overrideValue
+                OverrideValue = overrideValue,
+                ElementDataType = templateAttr.ElementDataType
            };

            await _repository.AddInstanceAttributeOverrideAsync(newOverride, cancellationToken);
@@ -1,6 +1,7 @@
 using System.IO.Compression;
 using System.Security.Cryptography;
 using System.Text.Json;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
@@ -70,6 +71,7 @@ public sealed class BundleImporter : IBundleImporter
    private readonly IOptions<TransportOptions> _options;
    private readonly TimeProvider _timeProvider;
    private readonly SemanticValidator _semanticValidator;
+    private readonly ILogger<BundleImporter>? _logger;

    /// <summary>
    /// Initializes a new <see cref="BundleImporter"/> with all required dependencies.
@@ -106,7 +108,8 @@ public sealed class BundleImporter : IBundleImporter
        IAuditService auditService,
        IAuditCorrelationContext correlationContext,
        ScadaBridgeDbContext dbContext,
-        SemanticValidator semanticValidator)
+        SemanticValidator semanticValidator,
+        ILogger<BundleImporter>? logger = null)
    {
        _bundleSerializer = bundleSerializer ?? throw new ArgumentNullException(nameof(bundleSerializer));
        _manifestValidator = manifestValidator ?? throw new ArgumentNullException(nameof(manifestValidator));
@@ -124,6 +127,7 @@ public sealed class BundleImporter : IBundleImporter
        _correlationContext = correlationContext ?? throw new ArgumentNullException(nameof(correlationContext));
        _dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
        _semanticValidator = semanticValidator ?? throw new ArgumentNullException(nameof(semanticValidator));
+        _logger = logger;
    }

    /// <inheritdoc />
@@ -1040,7 +1044,7 @@ public sealed class BundleImporter : IBundleImporter
        {
            t.Attributes.Add(new TemplateAttribute(a.Name)
            {
-                Value = a.Value,
+                Value = ImportValueNormalizer.NormalizeListValue(a.Value, a.DataType, a.ElementDataType),
                DataType = a.DataType,
                IsLocked = a.IsLocked,
                Description = a.Description,
@@ -1115,11 +1119,17 @@ public sealed class BundleImporter : IBundleImporter
        // Adds + Updates.
        foreach (var attrDto in dto.Attributes)
        {
+            // Normalise List values to the native-typed JSON form on import so the
+            // comparison (and the persisted value) match what the target already
+            // stores natively — otherwise an idempotent re-import of an old-form
+            // bundle would spuriously report a Value change.
+            var normalizedValue = ImportValueNormalizer.NormalizeListValue(
+                attrDto.Value, attrDto.DataType, attrDto.ElementDataType, _logger, attrDto.Name);
            if (existingByName.TryGetValue(attrDto.Name, out var current))
            {
                // Update only if any field actually changed.
                bool changed =
-                    !string.Equals(current.Value, attrDto.Value, StringComparison.Ordinal) ||
+                    !string.Equals(current.Value, normalizedValue, StringComparison.Ordinal) ||
                    current.DataType != attrDto.DataType ||
                    current.IsLocked != attrDto.IsLocked ||
                    !string.Equals(current.Description, attrDto.Description, StringComparison.Ordinal) ||
@@ -1127,7 +1137,7 @@ public sealed class BundleImporter : IBundleImporter
                    current.ElementDataType != attrDto.ElementDataType;
                if (!changed) continue;

-                current.Value = attrDto.Value;
+                current.Value = normalizedValue;
                current.DataType = attrDto.DataType;
                current.IsLocked = attrDto.IsLocked;
                current.Description = attrDto.Description;
@@ -1157,7 +1167,7 @@ public sealed class BundleImporter : IBundleImporter
            {
                var newAttr = new TemplateAttribute(attrDto.Name)
                {
-                    Value = attrDto.Value,
+                    Value = normalizedValue,
                    DataType = attrDto.DataType,
                    IsLocked = attrDto.IsLocked,
                    Description = attrDto.Description,
@@ -199,7 +199,7 @@ public sealed class EntitySerializer
                    t.Attributes.Add(new TemplateAttribute(a.Name)
                    {
                        TemplateId = t.Id,
-                        Value = a.Value,
+                        Value = ImportValueNormalizer.NormalizeListValue(a.Value, a.DataType, a.ElementDataType),
                        DataType = a.DataType,
                        IsLocked = a.IsLocked,
                        Description = a.Description,
@@ -0,0 +1,63 @@
+using Microsoft.Extensions.Logging;
+using ZB.MOM.WW.ScadaBridge.Commons.Types;
+using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
+
+namespace ZB.MOM.WW.ScadaBridge.Transport.Serialization;
+
+/// <summary>
+/// Import-time normalization of attribute values to the native-typed JSON form.
+/// <para>
+/// Bundles exported before the native-typed-JSON change carry List attribute
+/// values in the old quoted-element form (e.g. <c>["10","20"]</c> for an
+/// <see cref="DataType.Int32"/> list). Already-exported bundle files can't be
+/// rewritten, so List values are normalised on import: every DTO→entity write
+/// site routes the value through <see cref="NormalizeListValue"/> so imported
+/// data lands native (<c>[10,20]</c>). The central DB normalizer remains the
+/// backstop for anything that slips through.
+/// </para>
+/// <para>
+/// Non-List attributes and null/empty values pass through unchanged. A value
+/// that fails to decode (malformed JSON / un-parseable element) is left exactly
+/// as-is so the import still succeeds — the DB normalizer is the backstop.
+/// </para>
+/// </summary>
+internal static class ImportValueNormalizer
+{
+    /// <summary>
+    /// Returns the native-typed JSON form of a List attribute value, or the
+    /// value unchanged for non-List / null / empty / malformed inputs.
+    /// </summary>
+    /// <param name="value">The attribute value as carried by the bundle DTO.</param>
+    /// <param name="dataType">The attribute's declared data type.</param>
+    /// <param name="elementType">The List element type (null for scalars).</param>
+    /// <param name="logger">Optional logger; a warning is emitted when a malformed value is left as-is.</param>
+    /// <param name="attributeName">Optional attribute name for the diagnostic message.</param>
+    public static string? NormalizeListValue(
+        string? value,
+        DataType dataType,
+        DataType? elementType,
+        ILogger? logger = null,
+        string? attributeName = null)
+    {
+        if (dataType != DataType.List || string.IsNullOrEmpty(value))
+        {
+            return value;
+        }
+
+        try
+        {
+            return AttributeValueCodec.Encode(
+                AttributeValueCodec.Decode(value, DataType.List, elementType));
+        }
+        catch (FormatException ex)
+        {
+            // Leave malformed values exactly as imported; the DB normalizer is
+            // the backstop. Never abort the import for a single bad value.
+            logger?.LogWarning(ex,
+                "Bundle import: could not normalize List value for attribute {Attribute}; " +
+                "importing verbatim (the central DB normalizer is the backstop).",
+                attributeName ?? "(unknown)");
+            return value;
+        }
+    }
+}