DraftValidator/DraftSnapshot is dead code (no src/ caller) — possible publish-time enforcement gap #422

New Issue

2026-06-01T10:19:51-04:00

dohertj2 commented

2026-06-01 10:19:51 -04:00

Summary

DraftValidator (the managed "pre-publish validator per decision #91") has no live caller anywhere in src/. A repo-wide search finds its only references are its own source files and its test:

src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs
src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs
tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs (15 tests)

Nothing constructs a DraftSnapshot or calls DraftValidator.Validate(...) / ValidateClusterTopology(...) outside that test. It is not registered in DI and is not invoked from the publish path.

$ grep -rn --include='*.cs' -e 'DraftValidator\.' -e 'new DraftSnapshot' src \
    | grep -v 'Validation/DraftValidator.cs\|Validation/DraftSnapshot.cs'
# (no results)

The enforced pre-publish draft validation actually runs DB-side in sp_ValidateDraft (src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Migrations/20260417215224_StoredProcedures.cs, Procs.ValidateDraft ~:157), as part of the Status='Draft' → sp_PublishGeneration generation lifecycle. So the managed DraftValidator is currently dormant complement code — and there are passing tests exercising a validator that production never calls.

Why this may be more than dead code (potential enforcement gap)

DraftValidator's own doc-comment (:7–13) says it "complements the structural checks in sp_ValidateDraft — this layer owns schema validation for JSON columns, UNS segment regex, EquipmentId derivation, cross-cluster checks, and anything else uncomfortable to express in T-SQL." But because it is not wired in, the rules it uniquely owns may be enforced nowhere at publish time.

Comparing its private rule methods against the sp_ValidateDraft RAISERROR checks:

`DraftValidator` rule	In `sp_ValidateDraft`?
`ValidateUnsSegments` (UNS segment regex)	No
`ValidatePathLength` (≤ 200)	No
`ValidateEquipmentIdDerivation`	No
`ValidateClusterTopology` (topology vs `RedundancyMode`)	No
`ValidateEquipmentUuidImmutability`	Yes (`:233`)
`ValidateSameClusterNamespaceBinding`	Yes (`:218`, `BadCrossClusterNamespaceBinding`)
`ValidateReservationPreflight` (ZTag/SAPID)	Yes (`:245`/`:257`)
`ValidateDriverNamespaceCompatibility`	Partial (driver-type→namespace-kind, #111)

So UNS-segment regex, path-length, EquipmentId derivation, and cluster-topology-vs-RedundancyMode appear to be guarded only by the uncalled C# validator. (I have not read the full stored-proc body or checked for DB constraints/triggers that might cover them — worth confirming before concluding they're truly unenforced.)

Suggested resolution — pick one

Re-wire DraftValidator as intended — invoke it in the publish path (alongside / before sp_ValidateDraft) so the JSON-schema / UNS-regex / EquipmentId-derivation / cluster-topology rules are actually enforced. Confirms the 15 existing tests guard live behavior.
Remove it as dead code — delete DraftValidator.cs, DraftSnapshot.cs, and DraftValidatorTests.cs, and confirm sp_ValidateDraft (plus any DB constraints) is the intended sole enforcement. Only safe if the four "No" rules above are genuinely covered elsewhere or deliberately dropped.

Option 1 is preferable if those rules matter at publish time; option 2 only after verifying nothing is lost.

Context

Surfaced while normalizing config-validation across the ZB.MOM.WW.* family (scadaproj → ZB.MOM.WW.Configuration). This is purely an OtOpcUa housekeeping/correctness question — unrelated to that shared options-validation library (draft/generation-content validation is correctly out of its scope either way). Filing here so it isn't lost.

Verified against main on 2026-06-01.

## Summary `DraftValidator` (the managed "pre-publish validator per decision #91") has **no live caller** anywhere in `src/`. A repo-wide search finds its only references are its own source files and its test: - `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftValidator.cs` - `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Validation/DraftSnapshot.cs` - `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/DraftValidatorTests.cs` (15 tests) Nothing constructs a `DraftSnapshot` or calls `DraftValidator.Validate(...)` / `ValidateClusterTopology(...)` outside that test. It is not registered in DI and is not invoked from the publish path. ``` $ grep -rn --include='*.cs' -e 'DraftValidator\.' -e 'new DraftSnapshot' src \ | grep -v 'Validation/DraftValidator.cs\|Validation/DraftSnapshot.cs' # (no results) ``` The enforced pre-publish draft validation actually runs **DB-side** in `sp_ValidateDraft` (`src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Migrations/20260417215224_StoredProcedures.cs`, `Procs.ValidateDraft` ~`:157`), as part of the `Status='Draft' → sp_PublishGeneration` generation lifecycle. So the managed `DraftValidator` is currently **dormant complement code** — and there are passing tests exercising a validator that production never calls. ## Why this may be more than dead code (potential enforcement gap) `DraftValidator`'s own doc-comment (`:7–13`) says it "complements the structural checks in `sp_ValidateDraft` — this layer owns schema validation for JSON columns, UNS segment regex, EquipmentId derivation, cross-cluster checks, and anything else uncomfortable to express in T-SQL." But because it is not wired in, the rules it **uniquely** owns may be enforced **nowhere** at publish time. Comparing its private rule methods against the `sp_ValidateDraft` RAISERROR checks: | `DraftValidator` rule | In `sp_ValidateDraft`? | |---|---| | `ValidateUnsSegments` (UNS segment regex) | **No** | | `ValidatePathLength` (≤ 200) | **No** | | `ValidateEquipmentIdDerivation` | **No** | | `ValidateClusterTopology` (topology vs `RedundancyMode`) | **No** | | `ValidateEquipmentUuidImmutability` | Yes (`:233`) | | `ValidateSameClusterNamespaceBinding` | Yes (`:218`, `BadCrossClusterNamespaceBinding`) | | `ValidateReservationPreflight` (ZTag/SAPID) | Yes (`:245`/`:257`) | | `ValidateDriverNamespaceCompatibility` | Partial (driver-type→namespace-kind, #111) | So **UNS-segment regex, path-length, EquipmentId derivation, and cluster-topology-vs-RedundancyMode** appear to be guarded only by the uncalled C# validator. (I have not read the full stored-proc body or checked for DB constraints/triggers that might cover them — worth confirming before concluding they're truly unenforced.) ## Suggested resolution — pick one 1. **Re-wire `DraftValidator` as intended** — invoke it in the publish path (alongside / before `sp_ValidateDraft`) so the JSON-schema / UNS-regex / EquipmentId-derivation / cluster-topology rules are actually enforced. Confirms the 15 existing tests guard live behavior. 2. **Remove it as dead code** — delete `DraftValidator.cs`, `DraftSnapshot.cs`, and `DraftValidatorTests.cs`, and confirm `sp_ValidateDraft` (plus any DB constraints) is the intended sole enforcement. Only safe if the four "No" rules above are genuinely covered elsewhere or deliberately dropped. Option 1 is preferable if those rules matter at publish time; option 2 only after verifying nothing is lost. ## Context Surfaced while normalizing config-validation across the `ZB.MOM.WW.*` family (`scadaproj` → `ZB.MOM.WW.Configuration`). This is purely an OtOpcUa housekeeping/correctness question — unrelated to that shared **options**-validation library (draft/generation-content validation is correctly out of its scope either way). Filing here so it isn't lost. _Verified against `main` on 2026-06-01._

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#422