330 lines
19 KiB
Markdown
330 lines
19 KiB
Markdown
# Array Write Ergonomics & Default-Fill Partial Writes — Implementation Plan
|
|
|
|
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans to implement this plan task-by-task.
|
|
|
|
**Goal:** Let clients write array attributes by their bare name (gateway auto-appends `[]` at AddItem), and write a sparse, default-filled array (only the indices they care about + a total length) instead of marshalling the whole array.
|
|
|
|
**Architecture:** All new behavior lives in the **contract** and the **gateway**; the worker is untouched and keeps doing an honest whole-array COM write. The gateway intercepts outbound commands at the single choke point `GatewaySession.InvokeAsync(WorkerCommand)`: it (a) normalizes `AddItem`/`AddItem2` `item_definition` to the `[]` form when Galaxy metadata says the attribute is an array, and (b) expands an `MxSparseArray` write value into a full default-filled `MxArray` before it leaves the gateway. Partial writes are **stateless default-fill** — unmentioned indices are the type default (reset), never preserved.
|
|
|
|
**Tech Stack:** .NET 10 (gateway) / .NET Framework 4.8 x86 (worker, unchanged), protobuf + Grpc.Tools, xUnit + FakeWorkerHarness; clients in C#, Go, Python, Rust, Java.
|
|
|
|
**Design doc:** `docs/plans/2026-06-18-array-write-ergonomics-design.md`
|
|
|
|
**Key references for the implementer:**
|
|
- Choke point for all outbound commands: `src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs:947-955` (`InvokeAsync(WorkerCommand command, ...)`). `command.Command` is the `MxCommand`.
|
|
- Handle→address tracking: `GatewaySession.TrackCommandReply` (lines 975-1014, AddItem at 989, AddItem2 at 992) → `TrackItem` (1826-1837) → `SessionItemRegistration` record (`Sessions/SessionItemRegistration.cs`). Tracking reads the **same** `MxCommand` instance that passed through `InvokeAsync`, so mutating `item_definition` there flows through automatically.
|
|
- Galaxy metadata lookup: `IGalaxyHierarchyCache.Current.Index.TagsByAddress.TryGetValue(addr, out GalaxyTagLookup)`, then `lookup.Attribute?.IsArray`. The index is keyed by `FullTagReference`, which **already contains** `[]` for arrays — look up `addr + "[]"`. See `Security/Authorization/ConstraintEnforcer.cs:15-17,197-204` for the injection + lookup pattern.
|
|
- Proto: `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto` — `MxValue` (1026-1044), `MxArray` (1046-1063), `WriteCommand` (244-249), `Write2Command` (251-257), `WriteSecuredCommand`/`WriteSecured2Command`, `WriteBulk*`, `AddItemCommand` (192-195), `AddItem2Command` (197-201). Generated into `Contracts/Generated/MxaccessGateway.cs` (Compile-Removed + regenerated by Grpc.Tools).
|
|
- Gateway proto regen + commit rule (memory `project_proto_codegen_regen`): after a `.proto` edit, delete `Generated/*.cs`, rebuild contracts to regenerate, and **commit** `Generated/` or the net48 worker build fails CS0246.
|
|
- Java client gotcha (memory `project_java_generated_churn`): gradle regenerates a tracked 64k-line file with spurious protobuf-version churn — revert that churn; build/test Java on **windev** (memory `project_java_build_host`), Mac has no JRE.
|
|
|
|
---
|
|
|
|
## Task 0: Contract — add `MxSparseArray` and regenerate
|
|
|
|
**Classification:** high-risk
|
|
**Estimated implement time:** ~4 min
|
|
**Parallelizable with:** none (blocks all other tasks)
|
|
|
|
**Files:**
|
|
- Modify: `src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto` (MxValue oneof ~line 1043; new messages after MxArray ~line 1063)
|
|
- Regenerate + commit: `src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs`
|
|
|
|
**Step 1: Add the messages to the proto.** After the `MxArray` message (line 1063), add:
|
|
|
|
```proto
|
|
// Write-only sparse array value. The gateway expands this into a full,
|
|
// default-filled MxArray before forwarding to the worker; the worker never
|
|
// receives or produces it. Unmentioned indices take the element type's
|
|
// default (reset, NOT preserved).
|
|
message MxSparseArray {
|
|
MxDataType element_data_type = 1;
|
|
uint32 total_length = 2;
|
|
repeated MxSparseElement elements = 3;
|
|
}
|
|
|
|
message MxSparseElement {
|
|
uint32 index = 1;
|
|
MxValue value = 2; // scalar
|
|
}
|
|
```
|
|
|
|
**Step 2: Add the oneof arm to `MxValue`.** Inside the `oneof kind { ... }` block, after `bytes raw_value = 18;`:
|
|
|
|
```proto
|
|
MxSparseArray sparse_array_value = 19;
|
|
```
|
|
|
|
**Step 3: Regenerate generated code.**
|
|
|
|
Run (PowerShell on windev, or locally on Mac — .NET builds fine):
|
|
```
|
|
del src/ZB.MOM.WW.MxGateway.Contracts/Generated/*.cs
|
|
dotnet build src/ZB.MOM.WW.MxGateway.Contracts/ZB.MOM.WW.MxGateway.Contracts.csproj
|
|
```
|
|
Expected: build succeeds; `Generated/MxaccessGateway.cs` now contains `MxSparseArray`, `MxSparseElement`, and `MxValue.SparseArrayValue`.
|
|
|
|
**Step 4: Verify net10 + net48 both compile** (the worker consumes these types via net48):
|
|
```
|
|
dotnet build src/ZB.MOM.WW.MxGateway.slnx
|
|
```
|
|
Expected: PASS (no CS0246 on the new types).
|
|
|
|
**Step 5: Commit** (include regenerated `Generated/`):
|
|
```bash
|
|
git add src/ZB.MOM.WW.MxGateway.Contracts/Protos/mxaccess_gateway.proto \
|
|
src/ZB.MOM.WW.MxGateway.Contracts/Generated/MxaccessGateway.cs
|
|
git commit -m "feat(contracts): add MxSparseArray write-only value for default-fill partial writes"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 1: Gateway — `SparseArrayExpander` (pure expansion + validation)
|
|
|
|
**Classification:** standard
|
|
**Estimated implement time:** ~5 min
|
|
**Parallelizable with:** Task 2, Tasks 4-9
|
|
|
|
**Files:**
|
|
- Create: `src/ZB.MOM.WW.MxGateway.Server/Sessions/SparseArrayExpander.cs`
|
|
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SparseArrayExpanderTests.cs`
|
|
|
|
**Step 1: Write failing tests.** Cover: default-fill sizing + placement (one per element type is enough for two types here, rest in step 4); `total_length == 0` → `InvalidArgument`; index `>= total_length` → `InvalidArgument`; duplicate index → `InvalidArgument`; `Raw`/`Unspecified` element type → `InvalidArgument`; empty `elements` → all-defaults array of length N; timestamp default == Unix epoch.
|
|
|
|
```csharp
|
|
using Grpc.Core;
|
|
using Mxaccess.Gateway.V1; // adjust to the generated namespace
|
|
using ZB.MOM.WW.MxGateway.Server.Sessions;
|
|
using Xunit;
|
|
|
|
public sealed class SparseArrayExpanderTests
|
|
{
|
|
private static MxValue Sparse(MxDataType type, uint length, params (uint Index, MxValue Value)[] els)
|
|
{
|
|
MxSparseArray sparse = new() { ElementDataType = type, TotalLength = length };
|
|
foreach ((uint index, MxValue value) in els)
|
|
sparse.Elements.Add(new MxSparseElement { Index = index, Value = value });
|
|
return new MxValue { SparseArrayValue = sparse };
|
|
}
|
|
|
|
[Fact]
|
|
public void Expand_Int32_FillsDefaultsAndPlacesValues()
|
|
{
|
|
MxValue v = Sparse(MxDataType.Integer, 4, (1, new MxValue { Int32Value = 7 }));
|
|
SparseArrayExpander.Expand(v);
|
|
Assert.Equal(MxValue.KindOneofCase.ArrayValue, v.KindCase);
|
|
Assert.Equal(new[] { 0, 7, 0, 0 }, v.ArrayValue.Int32Values.Values);
|
|
Assert.Equal((uint)4, v.ArrayValue.Dimensions[0]);
|
|
}
|
|
|
|
[Fact]
|
|
public void Expand_EmptyElements_ProducesAllDefaults()
|
|
{
|
|
MxValue v = Sparse(MxDataType.Boolean, 3);
|
|
SparseArrayExpander.Expand(v);
|
|
Assert.Equal(new[] { false, false, false }, v.ArrayValue.BoolValues.Values);
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData(0u, 0u)] // total_length == 0
|
|
[InlineData(2u, 5u)] // index >= total_length
|
|
public void Expand_InvalidShape_Throws(uint length, uint badIndex)
|
|
{
|
|
MxValue v = Sparse(MxDataType.Integer, length, (badIndex, new MxValue { Int32Value = 1 }));
|
|
RpcException ex = Assert.Throws<RpcException>(() => SparseArrayExpander.Expand(v));
|
|
Assert.Equal(StatusCode.InvalidArgument, ex.StatusCode);
|
|
}
|
|
|
|
[Fact]
|
|
public void Expand_DuplicateIndex_Throws()
|
|
{
|
|
MxValue v = Sparse(MxDataType.Integer, 4,
|
|
(1, new MxValue { Int32Value = 1 }), (1, new MxValue { Int32Value = 2 }));
|
|
Assert.Throws<RpcException>(() => SparseArrayExpander.Expand(v));
|
|
}
|
|
}
|
|
```
|
|
|
|
**Step 2: Run, confirm fail.** `dotnet test src/ZB.MOM.WW.MxGateway.Tests/... --filter FullyQualifiedName~SparseArrayExpanderTests` → FAIL (type not defined).
|
|
|
|
**Step 3: Implement `SparseArrayExpander`.** Mutates the passed `MxValue` in place, replacing `SparseArrayValue` with `ArrayValue`. Throw `RpcException(new Status(StatusCode.InvalidArgument, msg))` on any validation failure. Element-type switch must cover the supported scalar element types (`Boolean`, `Integer` → int32 or int64, `Float`, `Double`, `String`, `Time`); default/timestamp default = Unix epoch (`new Timestamp { Seconds = 0, Nanos = 0 }`); reject `Raw`/`Unknown`/`Unspecified`. Set `MxArray.Dimensions = { total_length }` and `ElementDataType`. Validate: `total_length > 0`, every index `< total_length`, no duplicate indices, each element `value` scalar kind matches `element_data_type`.
|
|
|
|
(Mirror the typed sub-array shapes from `VariantConverter.ConvertToComArray` in the worker so the worker's existing read path is satisfied: `Int32Values`/`Int64Values`/`BoolValues`/`FloatValues`/`DoubleValues`/`StringValues`/`TimestampValues` with their `Values` repeated fields.)
|
|
|
|
**Step 4: Add remaining element-type tests** (int64, float, double, string, time/epoch, type-mismatch element → throws). Run filter → PASS.
|
|
|
|
**Step 5: Commit.**
|
|
```bash
|
|
git add src/ZB.MOM.WW.MxGateway.Server/Sessions/SparseArrayExpander.cs \
|
|
src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/SparseArrayExpanderTests.cs
|
|
git commit -m "feat(gateway): add SparseArrayExpander for default-fill partial array writes"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 2: Gateway — `ArrayAddressNormalizer` (suffix normalization)
|
|
|
|
**Classification:** standard
|
|
**Estimated implement time:** ~5 min
|
|
**Parallelizable with:** Task 1, Tasks 4-9
|
|
|
|
**Files:**
|
|
- Create: `src/ZB.MOM.WW.MxGateway.Server/Sessions/ArrayAddressNormalizer.cs`
|
|
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/ArrayAddressNormalizerTests.cs`
|
|
|
|
**Step 1: Write failing tests** using a fake/in-memory `IGalaxyHierarchyCache` whose `Current.Index.TagsByAddress` contains `"Obj.Arr[]"` (array) and `"Obj.Scalar"` (non-array). Cases:
|
|
- `"Obj.Arr"` (bare, is array) → `"Obj.Arr[]"`.
|
|
- `"Obj.Arr[]"` (already suffixed) → unchanged.
|
|
- `"Obj.Scalar"` (non-array) → unchanged.
|
|
- `"Obj.Unknown"` (not in cache / metadata cold) → unchanged (pass-through fallback).
|
|
|
|
```csharp
|
|
[Fact]
|
|
public void Normalize_BareArrayName_AppendsSuffix()
|
|
{
|
|
ArrayAddressNormalizer normalizer = new(FakeCacheWith("Obj.Arr[]", isArray: true));
|
|
Assert.Equal("Obj.Arr[]", normalizer.Normalize("Obj.Arr"));
|
|
}
|
|
|
|
[Theory]
|
|
[InlineData("Obj.Arr[]")] // already suffixed
|
|
[InlineData("Obj.Scalar")] // non-array
|
|
[InlineData("Obj.Unknown")] // not in cache → fallback pass-through
|
|
public void Normalize_LeavesUnchanged(string address) =>
|
|
Assert.Equal(address, new ArrayAddressNormalizer(FakeCacheWith("Obj.Arr[]", true)).Normalize(address));
|
|
```
|
|
|
|
**Step 2: Run, confirm fail.**
|
|
|
|
**Step 3: Implement.** Constructor injects `IGalaxyHierarchyCache cache`. `Normalize(string)`:
|
|
1. If `string.IsNullOrWhiteSpace(address)` or `address.EndsWith("[]", StringComparison.Ordinal)` → return unchanged.
|
|
2. Look up `address + "[]"` in `cache.Current.Index.TagsByAddress`. If found and `lookup.Attribute?.IsArray == true` → return `address + "[]"`.
|
|
3. Otherwise return `address` unchanged. Never throw (best-effort convenience).
|
|
|
|
**Step 4: Run filter → PASS.**
|
|
|
|
**Step 5: Commit.**
|
|
```bash
|
|
git add src/ZB.MOM.WW.MxGateway.Server/Sessions/ArrayAddressNormalizer.cs \
|
|
src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/ArrayAddressNormalizerTests.cs
|
|
git commit -m "feat(gateway): add ArrayAddressNormalizer for bare-name array AddItem"
|
|
```
|
|
|
|
---
|
|
|
|
## Task 3: Gateway — wire normalization + expansion into the outbound path
|
|
|
|
**Classification:** high-risk
|
|
**Estimated implement time:** ~5 min
|
|
**Parallelizable with:** none (depends on Tasks 1, 2)
|
|
|
|
**Files:**
|
|
- Modify: `src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs` (constructor — inject `ArrayAddressNormalizer`; `InvokeAsync` at 947-955)
|
|
- Modify: DI registration wherever `ArrayAddressNormalizer`/`GatewaySession` deps are registered (search `Security/Authorization/ConstraintEnforcer` registration for the pattern; register `ArrayAddressNormalizer` as scoped/singleton consistent with `IGalaxyHierarchyCache`)
|
|
- Test: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs`
|
|
|
|
**Step 1: Write a failing integration test** with `FakeWorkerHarness` (pattern: `src/ZB.MOM.WW.MxGateway.Tests/Gateway/Workers/FakeWorkerHarnessTests.cs:51-69` — `CreateConnectedPairAsync`, `ReadCommandAsync`, `ReplyToCommandAsync`). Two assertions:
|
|
1. Client sends `AddItemCommand{ item_definition = "Obj.Arr" }` (array per the test's Galaxy cache) → the `WorkerEnvelope` the fake worker reads has `item_definition == "Obj.Arr[]"`.
|
|
2. Client sends `WriteCommand{ value = MxSparseArray(Integer, 4, {1:7}) }` → the worker receives `value.ArrayValue.Int32Values.Values == [0,7,0,0]` (no `SparseArrayValue` reaches the worker).
|
|
|
|
**Step 2: Run, confirm fail.**
|
|
|
|
**Step 3: Implement.** At the top of `InvokeAsync(WorkerCommand command, ...)`, before forwarding, transform `command.Command` (the `MxCommand`) by `PayloadCase`:
|
|
- `AddItem` → `command.Command.AddItem.ItemDefinition = _addressNormalizer.Normalize(command.Command.AddItem.ItemDefinition);`
|
|
- `AddItem2` → same on `AddItem2.ItemDefinition`.
|
|
- `Write`/`WriteSecured` → if `cmd.Value?.KindCase == SparseArrayValue` call `SparseArrayExpander.Expand(cmd.Value)`.
|
|
- `Write2`/`WriteSecured2` → expand `Value` only (not `TimestampValue`).
|
|
- `WriteBulk`/`Write2Bulk`/`WriteSecuredBulk`/`WriteSecured2Bulk` → expand each `entry.Value`.
|
|
|
|
Keep it a single private helper `NormalizeOutbound(MxCommand)` called once at the choke point. Because `TrackCommandReply` later reads the **same** `MxCommand` instance, the normalized `item_definition` flows into `SessionItemRegistration` with no extra change — add an assertion in the test that `TryGetItemRegistration(...).TagAddress == "Obj.Arr[]"` to lock that in.
|
|
|
|
**Step 4: Run the wiring test + Tasks 1/2 filters → PASS.** Then run the AddItem/Write fake-worker regression group once:
|
|
```
|
|
dotnet test src/ZB.MOM.WW.MxGateway.Tests/ZB.MOM.WW.MxGateway.Tests.csproj --filter "FullyQualifiedName~ArrayWrite|FullyQualifiedName~SparseArray|FullyQualifiedName~ArrayAddressNormalizer"
|
|
```
|
|
|
|
**Step 5: Commit.**
|
|
```bash
|
|
git add src/ZB.MOM.WW.MxGateway.Server/Sessions/GatewaySession.cs \
|
|
src/ZB.MOM.WW.MxGateway.Tests/Gateway/Sessions/GatewayArrayWriteWiringTests.cs
|
|
git commit -m "feat(gateway): normalize array AddItem suffix and expand sparse writes at the worker boundary"
|
|
```
|
|
|
|
---
|
|
|
|
## Tasks 4-8: Client helpers + READMEs (one task per client, parallelizable)
|
|
|
|
Each client task does the same four things; only paths/idioms differ. **Depends on Task 0** (needs regenerated proto types). All five are parallelizable with each other and with Tasks 1-3, 9.
|
|
|
|
Common helper contract: `WriteArrayElements(serverHandle, itemHandle, elementDataType, totalLength, elements /* index→scalar MxValue */, userId)` builds an `MxValue { SparseArrayValue = MxSparseArray{...} }` and calls the existing raw write. Add a unit test that the built command carries `sparse_array_value` with the right `total_length`/indices (serialization round-trip; no live gateway). Update the **"Array writes replace the whole array"** README section to document: default-fill semantics (unmentioned = reset to default, not preserved), the required `total_length`, and that bare-name array writes now auto-normalize.
|
|
|
|
### Task 4: .NET client
|
|
**Classification:** standard · **~4 min** · **Parallelizable with:** Tasks 5-9, 1-3
|
|
- Regenerate types: `dotnet build clients/dotnet/ZB.MOM.WW.MxGateway.Client.slnx`.
|
|
- Add helper next to `WriteAsync` in `clients/dotnet/ZB.MOM.WW.MxGateway.Client/MxGatewaySession.cs:678-688`.
|
|
- Test alongside existing client tests; README `clients/dotnet/README.md:162-170`.
|
|
- Verify: build slnx + `dotnet test` the client test project.
|
|
|
|
### Task 5: Go client
|
|
**Classification:** standard · **~4 min** · **Parallelizable with:** Tasks 4,6-9, 1-3
|
|
- Regenerate per `clients/go` README; helper next to `Write`/`WriteRaw` in `clients/go/mxgateway/session.go:559-581`.
|
|
- README `clients/go/README.md:139-147`.
|
|
- Verify: `gofmt`, `go build ./...`, `go test ./...` from `clients/go`.
|
|
|
|
### Task 6: Python client
|
|
**Classification:** standard · **~4 min** · **Parallelizable with:** Tasks 4-5,7-9, 1-3
|
|
- Regenerate per `clients/python` README; helper next to `write` in `clients/python/src/zb_mom_ww_mxgateway/session.py:469-490`.
|
|
- README `clients/python/README.md:142-150`.
|
|
- Verify: `python -m pytest` from `clients/python`.
|
|
|
|
### Task 7: Rust client
|
|
**Classification:** standard · **~4 min** · **Parallelizable with:** Tasks 4-6,8-9, 1-3
|
|
- Helper next to `write` in `clients/rust/src/session.rs:530-548`; conversion helpers in `clients/rust/src/value.rs`.
|
|
- README `clients/rust/README.md:162-170`.
|
|
- Verify: `cargo fmt`, `cargo check --workspace`, `cargo test --workspace`, `cargo clippy --all-targets -- -D warnings` from `clients/rust`.
|
|
|
|
### Task 8: Java client
|
|
**Classification:** standard · **~5 min** · **Parallelizable with:** Tasks 4-7,9, 1-3
|
|
- Helper next to `write`/`writeRaw` in `clients/java/.../client/MxGatewaySession.java:581-604`.
|
|
- README `clients/java/README.md:118-126`.
|
|
- **Build/test on windev (JDK 21) via an isolated `origin/<branch>` worktree — Mac has no JRE** (memory `project_java_build_host`). After gradle regen, **revert the spurious protobuf-version churn** in `clients/java/src/main/generated/.../MxaccessGateway.java` if no proto semantics beyond the new messages changed (memory `project_java_generated_churn`); keep only the real `MxSparseArray` additions.
|
|
- Verify: `gradle test` on windev.
|
|
|
|
(Each client task ends with its own commit, e.g. `feat(client-<lang>): add WriteArrayElements default-fill helper and document semantics`.)
|
|
|
|
---
|
|
|
|
## Task 9: Docs — gateway.md + value conversion
|
|
|
|
**Classification:** small
|
|
**Estimated implement time:** ~3 min
|
|
**Parallelizable with:** Tasks 1-8 (depends on Task 0 only)
|
|
|
|
**Files:**
|
|
- Modify: `gateway.md` (command/value surface — document `MxSparseArray` as a write-only value and bare-name AddItem normalization)
|
|
- Modify: the value-conversion doc under `docs/` (search for where `MxArray`/value conversion is described) — add the default-fill + epoch-default note and the parity statement (worker still does a whole-array COM write)
|
|
|
|
**Step 1:** Add a subsection describing: `sparse_array_value` is write-only and gateway-expanded; default-fill semantics (epoch for time); `total_length` required; bare-name array writes auto-normalize to `[]` at AddItem with metadata pass-through fallback; non-goal: no preserve-unchanged merge, no element-wise COM write.
|
|
|
|
**Step 2: Commit.**
|
|
```bash
|
|
git add gateway.md docs/
|
|
git commit -m "docs: document MxSparseArray default-fill writes and bare-name array AddItem"
|
|
```
|
|
|
|
---
|
|
|
|
## Dependency summary
|
|
|
|
- **Task 0** blocks everything.
|
|
- **Tasks 1, 2** depend on 0; parallel with each other.
|
|
- **Task 3** depends on 1 and 2.
|
|
- **Tasks 4-8** depend on 0; parallel with each other and with 1-3, 9.
|
|
- **Task 9** depends on 0; parallel with all.
|
|
|
|
## Verification gates (per CLAUDE.md targeted-tests rule)
|
|
|
|
- Run only the `--filter` for the task you touched; run the array-write fake-worker group once after Task 3.
|
|
- Java verified on windev only. .NET/Go/Rust/Python verified locally.
|
|
- Live MXAccess (opt-in, windev): after merge, one default-fill write and one bare-name array write against real COM (`MXGATEWAY_RUN_LIVE_MXACCESS_TESTS=1`).
|