fix(json-cppagent): omit empty name attribute from Probe DataItems#140
Draft
ottobolyos wants to merge 24 commits intoTrakHound:masterfrom
Draft
fix(json-cppagent): omit empty name attribute from Probe DataItems#140ottobolyos wants to merge 24 commits intoTrakHound:masterfrom
ottobolyos wants to merge 24 commits intoTrakHound:masterfrom
Conversation
ottobolyos
force-pushed
the
fix/issue-138
branch
from
2c5b715 to
025e8b2
Compare
ottobolyos
force-pushed
the
fix/issue-138
branch
from
1d71665 to
61afb16
Compare
ottobolyos
force-pushed
the
fix/issue-138
branch
4 times, most recently
from
dce987f to
10e37c5
Compare
The fixture previously called `device.AddComponent(heating)`, which delegates to `Organizers.GetOrganizerType(component.Type)` and, when the type is a `System` substitution-group member, auto-wraps the component under a `Systems` organizer (so the wire path becomes `Components.Systems[0].Components.Heating[0]` rather than `Components.Heating[0]`). Whether `Heating` is auto-wrapped depends on whether `HeatingComponent.TypeId` is listed in `Organizers._systems` — a list that evolves with the SysML model. The fixture pins the wire-shape `name`-omission contract on a `DataItem`; that contract is independent of where the Heating component lives in the organizer tree. Attach the component directly to `device.Components` to keep the fixture's JSON path (`Components.Heating[0]`) deterministic regardless of `Organizers._systems` membership.
The docs/testing/issue-138/ subtree carried phase-by-phase campaign writeups that referenced internal tooling (CONVENTIONS rule-book, internal section numbers, extra-files.user/ paths, internal tracker terminology). Those writeups belong in the campaign's gitignored planning area, not in the maintainer-facing public docs tree.
Switches JsonDataItemSourceGuardTests off the inline repo-root walker and onto the shared MTConnect.NET_JSON_cppagent_Tests.TestHelpers.RepoRootLocator helper landed on feat/issue-133. Future source-grep guards reuse the helper rather than re-implementing the walk.
Adds regression fixture asserting that JsonDataItem(IDataItem) routes each IAbstractDataItemRelationship into the matching container bucket on JsonRelationshipContainer (DataItemRelationships, SpecificationRelationships) and aggregates duplicates within the same bucket. Lets the F-P-H6 perf optimization swap the IsAssignableFrom chain for switch pattern matching without regressing the routing.
Replaces the four-branch typeof(IRelationship).IsAssignableFrom(relationship.GetType()) chain in the JsonDataItem(IDataItem) constructor with a switch-on-type pattern match in both libraries/MTConnect.NET-JSON-cppagent and libraries/MTConnect.NET-JSON. Each iteration now does a single is-check + cast rather than four reflection probes per element. Behavior is preserved per JsonDataItemRelationshipCategorizationTests (11/11 in the cppagent suite).
Adds RED file-content fixture asserting ClientAgentCommunicationTests.cs constructs HttpServerConfiguration with Server = "127.0.0.1" so the embedded HTTP server binds to loopback only. Drives the F-S-L3 hardening that prevents an in-process integration run from accidentally exposing the test agent on a non-loopback interface of the dev machine.
Sets HttpServerConfiguration.Server = "127.0.0.1" alongside the existing Port assignment in the ClientAgentCommunicationTests fixture so the embedded HTTP server binds to loopback only. Closes the F-S-L3 risk that an in-process integration run accidentally exposes the test agent on a non-loopback interface of the dev machine. Greens the F-S-L3 source-grep regression pin.
The pattern-matching switch in commit 56cb98a used `??=` (null-coalescing assignment) which is a C# 8.0 feature. MTConnect.NET-JSON multi-targets net47 / net472 / netstandard2.0 in Release where the default LangVersion is 7.3, so the Release pack failed with CS8370. Replace `relationships.X ??= new List<JsonRelationship>()` with the explicit `if (relationships.X == null) relationships.X = new List<...>()` on all four cases. Behaviour is identical. Surfaced via `dotnet pack -c Release` from the integration branch.
Three test files carried internal audit-finding codes in comments explaining loopback-binding rationale and a relationship-classifier perf optimisation. Rewrite each comment to make the same point as a self-contained sentence and drop the codes.
ottobolyos
force-pushed
the
fix/issue-138
branch
from
10e37c5 to
2417831
Compare
Same omit-when-null fix as the JsonDataItem path (already in this PR) applied to the sibling probe DTOs. The v2.7 Devices XSD declares Component/@name and Composition/@name use="optional"; cppagent's reference printer omits the attribute when the model-side value is null. JsonComponent and JsonComposition were emitting "name": "" on operator-authored Devices.xml entries that never set a Name. Tests pin the wire shape for null / empty / explicit Name sources across Component and Composition, matching the existing sibling JsonDataItemEmptyNameOmissionTests pattern. Closes the remaining surface of TrakHound#138 (the original PR scope covered JsonDataItem only; Component and Composition were left exposed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ottobolyos
force-pushed
the
fix/issue-138
branch
from
493d26e to
cc7a3e7
Compare
The constructor-level guard `if (!string.IsNullOrEmpty(name)) Name = name`
relies on `JsonFunctions.DefaultOptions.DefaultIgnoreCondition =
WhenWritingDefault` for the runtime-correct wire shape. External consumers
that probe the property metadata directly (dime-connector's symbol probe
is the immediate trigger) — or hand the DTO to a fresh JsonSerializer with
no project options — see only `[JsonPropertyName("name")]` and not the
ignore-when-null pin, so they treat the property as always-emit and flag
the DTO as non-compliant with the spec's `use="optional"` attribute.
Pin the contract on the type itself with
`[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]` on:
- JsonDataItem.Name
- JsonComposition.Name
- JsonComponent.Name
The property type stays `string` (non-nullable). Adding `string?` would
propagate through every consumer; the attribute is enough to satisfy the
metadata-level pin.
New TDD pins assert (a) the attribute is present with the correct
condition via reflection, and (b) raw `JsonSerializer.Serialize` with no
project-options omits the `name` key when null, on all three DTOs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #138 — JSON-cppagent Probe emitted
"name": ""for nested probe elements whose caller did not setName. The empty-string emission breaks strict cppagent JSON v2 consumers; the optionalnameattribute should be omitted entirely when unset.The PR closes the gap on every nested probe DTO that carries an optional
Nameslot:JsonDataItem,JsonComponent, andJsonComposition— not just the originalJsonDataItemsurface.Changes
Namecopy inJsonDataItem(IDataItem)ofMTConnect.NET-JSON-cppagent/Devices/JsonDataItem.cswithstring.IsNullOrEmpty, mirroring the already-correct behavior ofMTConnect.NET-XML/Devices/XmlDataItem.cs.MTConnect.NET-JSON/Devices/JsonDataItem.csso the base JSON formatter matches the cppagent variant.JsonComponent(IComponent)andJsonComposition(IComposition)— the dime-team's downstream symbol probe surfaced these as still emitting""even after the JsonDataItem fix landed.Nameproperty now carries[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]alongside its[JsonPropertyName("name")]. This makes the contract correct under rawJsonSerializer.Serialize(...)(without the agent'sJsonFunctions.DefaultOptions), so a downstream consumer reflecting on the property metadata sees the omit-when-null contract directly.netstandard2.0/net48).Tests
NUnit regression coverage for:
Namecases on each ofJsonDataItem,JsonComponent,JsonCompositionfor both JSON formatters.[JsonIgnore(WhenWritingNull)]attribute presence on each Name property.JsonSerializer.Serialize(not via the agent's options) omits thenamekey when the source is null.Backward compatibility
Consumers reading the optional
nameslot through System.Text.Json typed deserialisation seenullinstead of""when the source was unset — a strictly more correct shape per the XSD'suse="optional"semantic and the cppagent reference behaviour. Producers previously emitting""continue to work unchanged because the guards apply only at the model→wire boundary; the public POCO setters still accept any string.