From b238228d8b6106e54b61fe9b0406ed174dc9ad9b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 15:10:32 -0400 Subject: [PATCH] docs: design for structured multi-value (List) attributes Add a first-class DataType.List + ElementDataType companion so object attributes can store homogeneous scalar lists (e.g. MoveInWorkOrderNumbers, MoveInPartNumbers) across all four lifecycle paths: script write/read, static authored default, OPC UA array read, OPC UA array write. Canonical JSON value codec; whole-list override; element type fixed by base; idempotent migration widening Value to nvarchar(max) + adding ElementDataType. Approved via brainstorming. --- .../2026-06-16-multivalue-attribute-design.md | 216 ++++++++++++++++++ 1 file changed, 216 insertions(+) create mode 100644 docs/plans/2026-06-16-multivalue-attribute-design.md diff --git a/docs/plans/2026-06-16-multivalue-attribute-design.md b/docs/plans/2026-06-16-multivalue-attribute-design.md new file mode 100644 index 00000000..a12e8712 --- /dev/null +++ b/docs/plans/2026-06-16-multivalue-attribute-design.md @@ -0,0 +1,216 @@ +# Structured Multi-Value Attribute — Design + +**Date:** 2026-06-16 +**Status:** Approved (brainstorming) — ready for implementation plan +**Branch:** `feature/multivalue-attribute` + +## Problem + +ScadaBridge's `DataType` enum (`Boolean·Int32·Float·Double·String·DateTime·Binary`) has no +collection type. An attribute is persisted as a single `string? Value` plus a `DataType` +discriminator. So a Galaxy/object attribute that is conceptually a list of values — e.g. +`MoveInWorkOrderNumbers`, `MoveInPartNumbers` (string arrays arriving via the +`IpsenMESMoveIn` inbound-API method) — can only be stored by collapsing it to a single +`DataType.String` blob. We want these stored as a **first-class structured multi-value +attribute** that round-trips through every path. + +## Scope (decided) + +- **Element types:** a *homogeneous* list of any existing **scalar** `DataType` — + `String, Int32, Float, Double, Boolean, DateTime`. **Excluded:** `Binary` elements and + nested lists (no lists-of-lists, no lists-of-objects). +- **Lifecycle paths — all four in scope:** + 1. Script writes the attribute at runtime and reads it back (the `IpsenMESMoveIn` case); + persists as a static attribute (site SQLite, survives failover). + 2. Statically authored default in a template, optionally overridden per instance, via + Central UI and CLI. + 3. Read in from an OPC UA array node (DCL subscription → live attribute value). + 4. Written out to an OPC UA array node via the DCL write path. + +## Decision summary (defaults locked in) + +| Decision | Choice | +|---|---| +| Type modeling | One `DataType.List` member + nullable `ElementDataType` companion | +| Value encoding | JSON array, invariant culture (round-trippable) | +| Display encoding | `ValueFormatter.FormatDisplayValue` stays comma-joined, display-only | +| Element types | 6 scalars; no `Binary`, no nesting | +| Override granularity | Whole-list replacement (no per-element override) | +| Element type mutability | Fixed by base attribute; not overridable on derived/instance | +| Bad OPC UA element | Coerce/validate; mismatch → Bad quality + log (non-fatal) | +| Empty vs unset | `null` = unset/cleared; `"[]"` = explicit empty list | +| gRPC wire | Existing `string value` field carries the canonical JSON (additive) | + +## Architecture + +The key insight from the blast-radius survey: most of the runtime is already +collection-capable. `ScriptParameters` already converts to typed `List`/`T[]`; the OPC UA +write path wraps values in a `Variant` (which supports arrays natively); +`AttributeValueChanged` already carries `object?`. The real work concentrates in **(a)** the +type model, **(b)** a canonical encode/decode codec, **(c)** the InstanceActor / script-accessor +boundary, **(d)** validation, and **(e)** the UI/CLI editors. + +### 1. Type model + +- Add **one** enum member: `DataType.List` + (`Commons/Types/Enums/DataType.cs`). +- Add nullable `ElementDataType` (`DataType?`) to: + - `Commons/Entities/Templates/TemplateAttribute.cs` + - `Commons/Entities/Instances/InstanceAttributeOverride.cs` + - `Commons/Types/Flattening/FlattenedConfiguration.cs` → `ResolvedAttribute` +- **Invariant:** `ElementDataType` required when `DataType == List`, null otherwise; element + type must be one of the 6 allowed scalars. + +### 2. Canonical value codec — new `AttributeValueCodec` + +One round-trippable codec used everywhere a value is **stored or transmitted**: + +- `Encode(object? value)`: + - scalar → invariant-culture string (**identical to today's behavior**; existing scalars + unchanged) + - non-string `IEnumerable` → JSON array, invariant culture + (`["WO-1","WO-2"]`, `[1,2,3]`, `["2026-06-16T00:00:00Z"]`) +- `Decode(string? value, DataType dataType, DataType? elementType)`: + - `List` → typed `List` (reuse the existing `ScriptParameters` element-conversion + machinery where practical) + - scalar → string (unchanged) +- `null` value = unset/cleared; `"[]"` = explicit empty list. + +`ValueFormatter.FormatDisplayValue` (comma-joined) remains **display-only** — diff text, +log lines, KPI strings — and is *not* the persisted/wire form. + +### 3. Persistence & migration + +- One EF Core migration, **written idempotent** (per the open follow-up #70 lesson — + guard inserts/alters so re-running against a partially-migrated DB is safe): + - add `ElementDataType nvarchar(50) NULL` to `TemplateAttributes` and + `InstanceAttributeOverrides` + - widen `Value` / `OverrideValue` from `nvarchar(4000)` → `nvarchar(max)` (lists can + exceed 4000) +- Site **SQLite** static-attribute store: no schema change (already a string column) — it + simply stores the canonical JSON. +- EF config: `TemplateConfiguration.cs`, `InstanceConfiguration.cs` map the new column + (`HasConversion().HasMaxLength(50)` for `ElementDataType`). + +### 4. Flatten / resolve + +- `TemplateEngine/Flattening/FlatteningService.cs` carries `ElementDataType` into + `ResolvedAttribute` next to `DataType`. +- Inheritance / compose / override shape unchanged. +- **Override replaces the whole list** (no per-element merge). +- A derived template or instance override **cannot change `ElementDataType`** — it is fixed + by the base attribute, enforced like the existing `LockedInDerived` path. + +### 5. Site runtime (the core change) + +- **`SiteRuntime/Scripts/ScopeAccessors.cs:56,73`** — `AttributeAccessor` set / `SetAsync`: + replace `value?.ToString()` with `AttributeValueCodec.Encode(value)`. (Today a + `List` would `.ToString()` to `"System.Collections.Generic.List`1[...]"` — the + central bug this fixes.) Scalars produce the same string as before. +- **`SiteRuntime/Actors/InstanceActor.cs:116-121`** — on load, for `List` attributes + `Decode` the JSON `Value` into a typed `List` and store *that* in `_attributes` + (scalars unchanged). +- **`HandleSetStaticAttribute`** — for `List` attributes: decode the canonical string → + typed list for `_attributes`, validate element types, persist the canonical JSON to + SQLite, stream the canonical JSON. +- **`HandleGetAttribute`** — unchanged; returns the live `_attributes` object, now a real + `List` for list attributes. Scripts read a first-class list. + +### 6. Data Connection Layer (OPC UA read + write) + +- **Read:** an OPC UA array node yields a CLR array → attribute value object. Coerce / + validate element type against `ElementDataType`; on mismatch → **Bad quality** + log, + never crash the actor. +- **Write:** `DataConnectionLayer/Adapters/RealOpcUaClient.cs:619-628` already wraps values + in `new Variant(value)`, which supports arrays natively; a `List`/`T[]` writes as an + OPC UA array. Minimal change beyond passing the typed list through. + +### 7. Validation (pre-deploy semantic) + +`TemplateEngine/Validation/SemanticValidator.cs` + `ValidationService.cs`: + +- `ElementDataType` present and a valid scalar when `DataType == List`; absent otherwise. +- Authored default `Value` (if present) must parse as a JSON array whose elements match + `ElementDataType`. +- List attributes **rejected as operands** in numeric alarm triggers (HiLo / + RangeViolation) and binary triggers — extends the existing `NumericDataTypes` guard. + +### 8. Streaming / DebugView + +- `Communication/Actors/StreamRelayActor.cs:48` — encode via `AttributeValueCodec` (JSON for + lists) instead of `FormatDisplayValue`. Additive: lists are a new type, so no existing wire + consumer breaks; the proto `string value` field (`sitestream.proto:57`) is unchanged. +- Central DebugView renders the canonical string; for lists it shows the JSON array. + *Optional polish:* chip rendering of list elements. + +### 9. Central UI + +- **`CentralUI/Components/Pages/Design/TemplateEdit.razor`** — attribute form: when + type = `List`, reveal an `ElementDataType` dropdown and a **list editor** (repeatable + add/remove rows) bound to the JSON value; inline per-element validation. +- **`CentralUI/Components/Pages/Deployment/InstanceConfigure.razor`** — override panel: the + same list editor for overriding a list attribute. + +### 10. CLI + +- **`CLI/Commands/TemplateCommands.cs`** (and the instance-override command): add + `--element-type `; `--value` accepts a JSON array (`'["a","b"]'`). Optional + ergonomic alternative: repeatable `--value`. Validate element type before submit. +- `ManagementService/ManagementActor.cs` add/update attribute handlers: parse + validate + `ElementDataType`, validate `Value` against it before persisting. + +### 11. Transport (import/export) + +- `Transport/Serialization/EntityDtos.cs` `TemplateAttributeDto` already carries the + `DataType` enum + `string? Value`; add `ElementDataType`. Enum/value round-trip is + otherwise automatic. `BundleImporter` carries the new field through. + +## Error handling + +- Malformed JSON in a persisted `Value`: caught at deploy-time validation; if it ever + reaches runtime decode, log + treat the attribute as Bad quality — never crash the + Instance Actor (audit/best-effort principle). +- OPC UA element-type mismatch on read: Bad quality + log (non-fatal). +- Script writes a non-list to a list attribute (or vice versa): rejected at + `HandleSetStaticAttribute` validation, surfaced to the script as an error. + +## Testing strategy + +- **Codec round-trip:** every element type; empty list; embedded commas/quotes/escaping; + `DateTime`; culture-invariance. +- **Flatten:** override replaces whole list; element-type lock enforced. +- **Validation:** good/bad authored defaults; trigger-operand rejection; missing/extra + `ElementDataType`. +- **InstanceActor:** set/get typed list; SQLite persistence survives simulated failover. +- **DCL:** OPC UA array read coercion; Bad quality on element mismatch; `Variant` array + write. +- **Streaming:** `StreamRelayActor` emits canonical JSON for a list attribute. +- **CLI:** `--element-type` + JSON `--value` parse/validate. +- **UI:** list-editor component behavior (if harness present). + +## Out of scope (deferred) + +- Nested objects / lists-of-records; heterogeneous lists. +- `Binary` element lists. +- Per-element overrides; element-level alarm triggers. + +## Blast-radius reference (file:area) + +| Area | Key locations | Change size | +|---|---|---| +| Enum + entities | `DataType.cs`, `TemplateAttribute.cs`, `InstanceAttributeOverride.cs`, `FlattenedConfiguration.cs` | small | +| Codec (new) | `Commons/Types/AttributeValueCodec.cs` (new) | small–med | +| EF + migration | `TemplateConfiguration.cs`, `InstanceConfiguration.cs`, new migration | small | +| Flatten | `FlatteningService.cs:177` | small | +| Runtime boundary | `ScopeAccessors.cs:56,73`, `InstanceActor.cs:116-121,246-315` | **med (core)** | +| DCL | `RealOpcUaClient.cs` read coercion (write already array-capable) | small–med | +| Validation | `SemanticValidator.cs:18-21,130-193`, `ValidationService.cs` | small–med | +| Streaming | `StreamRelayActor.cs:48` | small | +| UI | `TemplateEdit.razor`, `InstanceConfigure.razor` | **med (largest)** | +| CLI + mgmt | `TemplateCommands.cs`, `ManagementActor.cs:1441-1461` | small–med | +| Transport | `EntityDtos.cs:77-83`, `BundleImporter.cs:2302` | small | + +**Note:** the Inbound API type system (`ParameterDefinition` with `Object`/`List`, +`InboundApiSchema`) is genuinely separate from attribute `DataType` and is **not** modified; +its `ScriptParameters.ConvertToList/ConvertToArray` conversion helpers are reusable by the +codec.