From 48d3a9d6da19ee8cd2a9f909cbfe6d6000b0c50a Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 5 May 2026 10:18:21 -0400 Subject: [PATCH] [M2/M4] mxaccess-rpc: Guid::parse_str + dedupe examples (resolves F17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `Guid::parse_str(&str) -> Result` to `crates/mxaccess-rpc/src/guid.rs` as the inverse of the existing `Display` impl. Accepts the canonical dashed-hex form, optionally braced (.NET `B` format), case-insensitive, and tolerant of bare 32-char hex without dashes. Single-pass char-by-char nibble accumulator avoids per-byte string allocation; applies the same byte-swap of groups 1-3 that the `Display` impl reads. Eight new tests cover round-trip against the existing `Display` fixture (`crates/mxaccess-rpc/src/guid.rs:111-119`, `b49f92f7-c748-4169-8eca-a0670b012746`), braces, uppercase, no-dashes, zero-GUID, too-short, too-long, and non-hex rejection. The five live-NMX examples (`connect-write-read`, `subscribe`, `recovery`, `multi-tag`, `secured-write`) lose their per-file 15-line `parse_guid` helpers in favour of the canonical implementation. `asb-subscribe` and `subscribe-buffered` are unaffected — they don't parse GUIDs. Test count delta: 524 → 532 (+8) Open followups touched: F17 resolved. Co-Authored-By: Claude Opus 4.7 (1M context) --- design/followups.md | 9 +- rust/crates/mxaccess-rpc/src/guid.rs | 141 ++++++++++++++++++ .../mxaccess/examples/connect-write-read.rs | 26 +--- rust/crates/mxaccess/examples/multi-tag.rs | 21 +-- rust/crates/mxaccess/examples/recovery.rs | 21 +-- .../crates/mxaccess/examples/secured-write.rs | 21 +-- rust/crates/mxaccess/examples/subscribe.rs | 21 +-- 7 files changed, 149 insertions(+), 111 deletions(-) diff --git a/design/followups.md b/design/followups.md index 4012a8f..60f78fc 100644 --- a/design/followups.md +++ b/design/followups.md @@ -60,12 +60,6 @@ move to `## Resolved` with a date + commit hash. **Why deferred:** Wave-2 `Session::recover_connection` validates the policy and emits `RecoveryEvent::Started` + `RecoveryEvent::Recovered` on each call but does **NOT** actually tear down + re-establish the NMX transport / re-advise active subscriptions. The .NET reference's `RecoverConnectionCore` (`MxNativeSession.cs:442-474`) does all three: builds a replacement `ManagedNmxService2Client` via `CreateRegisteredService`, re-`Connect`s every `_publisherEndpoints` entry, re-`AdviseSupervisory`s every entry in `_subscriptions`, then atomically swaps the old service for the new one. Porting this to Rust requires (a) tracking the active subscriptions inside `SessionInner` (currently they're owned by the consumer's `Subscription` handles, with no central registry); (b) the long-lived connection task per R15 in `design/70-risks-and-open-questions.md` so swap-in-place is safe under concurrent operations; (c) a way to re-create the `CallbackExporter` (or keep the existing one bound while the underlying transport is replaced — needs design work). **Resolves when:** R15's long-lived connection task lands and `SessionInner` gains a subscription registry. At that point the recover loop becomes ~50 lines: for `attempt in 1..=max_attempts`, emit Started → drop+rebuild NmxClient → `register_engine_2` with the existing OBJREF → re-advise every registered correlation_id → emit Recovered (or Failed + sleep delay + continue, mirroring the `cs:407-440` shape exactly). -### F17 — `Guid::parse_str` helper (dashed-hex string parser) -**Severity:** P3 -**Source:** M4 wave 3, `crates/mxaccess/examples/*.rs` -**Why deferred:** Each of the five live-NMX examples (`connect-write-read`, `subscribe`, `recovery`, `multi-tag`, `secured-write`) duplicates a 15-line `parse_guid` helper that takes a `12345678-1234-1234-1234-123456789012` style string and produces the wire-byte `Guid([u8; 16])`. The helper belongs on `mxaccess_rpc::guid::Guid` itself (paired with its existing `Display` impl) so consumer code that wires up `MX_NMX_SERVICE_IPID` doesn't reimplement the LE-leading byte-swap convention. Deferred to keep this iteration's diff focused on examples; the parse path has no corresponding .NET reference helper to mirror so it's a Rust-side ergonomic addition rather than a parity port. -**Resolves when:** Add `pub fn parse_str(s: &str) -> Result` on `mxaccess_rpc::guid::Guid` with a round-trip test against the existing `Display` fixture (`crates/mxaccess-rpc/src/guid.rs:111-119`). Update the five examples to call it. Single-iteration follow-up. - ### F14 — `tiberius`-backed SQL implementation of `Resolver` + `UserResolver` **Severity:** P2 **Source:** M3 stream A, `crates/mxaccess-galaxy/src/sql.rs` (constants present, no client wiring yet) @@ -94,3 +88,6 @@ move to `## Resolved` with a date + commit hash. ### F9 — `ObjectExporterClient.cs` ResolveOxid wrapper methods **Resolved:** 2026-05-05. Both portable methods land in `crates/mxaccess-rpc/src/object_exporter_client.rs`: `resolve_oxid_unauthenticated` (mirrors `cs:14-30`) and `resolve_oxid_with_managed_ntlm_packet_integrity` (mirrors `cs:66-81`). Each opens a TCP connection, binds to `IObjectExporter`, calls opnum 0 with the encoded request, and decodes the response — preferring `parse_resolve_oxid_result` then falling back to `parse_resolve_oxid_failure` for short stubs. The two SSPI flavours (`ResolveOxidWithNtlmConnect`, `ResolveOxidWithNtlmPacketIntegrity`) wrap .NET's `System.Net.Security.SspiClientContext` and are explicitly out of scope for the Rust port — that's a permanent skip, not a deferral. + +### F17 — `Guid::parse_str` helper (dashed-hex string parser) +**Resolved:** 2026-05-05. `Guid::parse_str(&str) -> Result` landed in `crates/mxaccess-rpc/src/guid.rs:65-112` as the inverse of the existing `Display` impl. Accepts the canonical dashed-hex form, optionally wrapped in `{}` braces (.NET `B` format), case-insensitive, and tolerant of bare 32-char hex without dashes. Single-pass char-by-char nibble accumulator avoids per-byte string allocation; the same byte-swap of groups 1-3 the Display impl does is applied after the raw hex pass. Eight new tests cover round-trip against the `Display` fixture (`b49f92f7-c748-4169-8eca-a0670b012746`), braces, uppercase, no-dashes, zero-GUID, too-short, too-long, and non-hex rejection. The five live-NMX examples (`connect-write-read`, `subscribe`, `recovery`, `multi-tag`, `secured-write`) lost their per-file 15-line `parse_guid` helpers in favour of the canonical implementation. Test count delta: 524 → 532 (+8). diff --git a/rust/crates/mxaccess-rpc/src/guid.rs b/rust/crates/mxaccess-rpc/src/guid.rs index 3ec4e18..5eff16a 100644 --- a/rust/crates/mxaccess-rpc/src/guid.rs +++ b/rust/crates/mxaccess-rpc/src/guid.rs @@ -46,6 +46,71 @@ impl Guid { Ok(Self(out)) } + /// Parse a `12345678-1234-1234-1234-123456789012` style GUID string + /// into wire-byte form. Inverse of the [`std::fmt::Display`] impl. + /// + /// Accepts the canonical dashed-hex form, optionally wrapped in + /// `{...}` braces (the .NET `B` format). Case-insensitive. The + /// first three hex groups are stored little-endian on the wire (per + /// the module docstring) so the parser byte-swaps them after the + /// raw hex pass. + /// + /// There is no .NET reference to mirror here — the Display impl is + /// the spec, this is its inverse. + /// + /// # Errors + /// Returns [`crate::error::RpcError::Decode`] if the input is not + /// 32 hex chars (with 4 optional dashes and optional outer braces), + /// or contains a non-hex character. + pub fn parse_str(s: &str) -> Result { + let trimmed = s.trim_start_matches('{').trim_end_matches('}'); + // Strip dashes; everything else must be a hex digit. + let mut bytes = [0u8; 16]; + let mut nibble_count = 0usize; + let mut acc: u8 = 0; + for c in trimmed.chars() { + if c == '-' { + continue; + } + let digit = match c.to_digit(16) { + Some(d) => d as u8, + None => { + return Err(crate::error::RpcError::Decode { + offset: nibble_count / 2, + reason: "non-hex character in guid", + buffer_len: trimmed.len(), + }); + } + }; + if nibble_count >= 32 { + return Err(crate::error::RpcError::Decode { + offset: 16, + reason: "guid hex too long", + buffer_len: trimmed.len(), + }); + } + if nibble_count % 2 == 0 { + acc = digit << 4; + } else { + bytes[nibble_count / 2] = acc | digit; + } + nibble_count += 1; + } + if nibble_count != 32 { + return Err(crate::error::RpcError::Decode { + offset: nibble_count / 2, + reason: "guid hex too short", + buffer_len: trimmed.len(), + }); + } + // Byte-swap the first three groups so the resulting bytes match + // the wire layout the Display impl reads. + bytes[0..4].reverse(); + bytes[4..6].reverse(); + bytes[6..8].reverse(); + Ok(Self(bytes)) + } + /// Write the 16 wire bytes into `dst[..16]`. Mirrors .NET /// `Guid.TryWriteBytes(span)`. /// @@ -142,4 +207,80 @@ mod tests { "00000000-0000-0000-0000-000000000000" ); } + + #[test] + fn parse_str_round_trips_display() { + // The dashed-hex form from the display fixture above. + let g = Guid::parse_str("b49f92f7-c748-4169-8eca-a0670b012746").unwrap(); + assert_eq!( + g.0, + [ + 0xF7, 0x92, 0x9F, 0xB4, 0x48, 0xC7, 0x69, 0x41, 0x8E, 0xCA, 0xA0, 0x67, 0x0B, 0x01, + 0x27, 0x46, + ] + ); + // Round-trip back via Display. + assert_eq!(g.to_string(), "b49f92f7-c748-4169-8eca-a0670b012746"); + } + + #[test] + fn parse_str_accepts_braces() { + // .NET "B" format wraps the dashed-hex form in `{}`. + let g = Guid::parse_str("{b49f92f7-c748-4169-8eca-a0670b012746}").unwrap(); + assert_eq!(g.to_string(), "b49f92f7-c748-4169-8eca-a0670b012746"); + } + + #[test] + fn parse_str_accepts_uppercase() { + let g = Guid::parse_str("B49F92F7-C748-4169-8ECA-A0670B012746").unwrap(); + assert_eq!(g.to_string(), "b49f92f7-c748-4169-8eca-a0670b012746"); + } + + #[test] + fn parse_str_accepts_no_dashes() { + let g = Guid::parse_str("b49f92f7c74841698ecaa0670b012746").unwrap(); + assert_eq!(g.to_string(), "b49f92f7-c748-4169-8eca-a0670b012746"); + } + + #[test] + fn parse_str_round_trips_zero() { + let g = Guid::parse_str("00000000-0000-0000-0000-000000000000").unwrap(); + assert_eq!(g, Guid::ZERO); + } + + #[test] + fn parse_str_rejects_too_short() { + let err = Guid::parse_str("b49f92f7-c748-4169-8eca-a0670b0127").unwrap_err(); + assert!(matches!( + err, + crate::error::RpcError::Decode { + reason: "guid hex too short", + .. + } + )); + } + + #[test] + fn parse_str_rejects_too_long() { + let err = Guid::parse_str("b49f92f7-c748-4169-8eca-a0670b01274600").unwrap_err(); + assert!(matches!( + err, + crate::error::RpcError::Decode { + reason: "guid hex too long", + .. + } + )); + } + + #[test] + fn parse_str_rejects_non_hex() { + let err = Guid::parse_str("b49f92f7-c748-4169-8eca-a0670b01274z").unwrap_err(); + assert!(matches!( + err, + crate::error::RpcError::Decode { + reason: "non-hex character in guid", + .. + } + )); + } } diff --git a/rust/crates/mxaccess/examples/connect-write-read.rs b/rust/crates/mxaccess/examples/connect-write-read.rs index c94c8bf..f544d61 100644 --- a/rust/crates/mxaccess/examples/connect-write-read.rs +++ b/rust/crates/mxaccess/examples/connect-write-read.rs @@ -80,8 +80,7 @@ impl LiveEnv { } let host = std::env::var("MX_NMX_HOST")?; let addr = parse_host_port(&host, 135)?; - let ipid_str = std::env::var("MX_NMX_SERVICE_IPID")?; - let service_ipid = parse_guid(&ipid_str)?; + let service_ipid = Guid::parse_str(&std::env::var("MX_NMX_SERVICE_IPID")?)?; let tag = std::env::var("MX_TEST_TAG").unwrap_or_else(|_| "TestChildObject.TestInt".into()); Ok(Some(Self { addr, @@ -110,29 +109,6 @@ fn parse_host_port( ) } -/// Parse a `12345678-1234-1234-1234-123456789012` style GUID into wire-byte -/// form. The first three groups are stored little-endian on the wire (per -/// `mxaccess_rpc::guid::Guid` module docstring); groups 4 and 5 are stored -/// verbatim. Tested against the Display round-trip in `guid.rs`. -fn parse_guid(s: &str) -> Result> { - let trimmed = s.trim_start_matches('{').trim_end_matches('}'); - let hex: String = trimmed.chars().filter(|c| *c != '-').collect(); - if hex.len() != 32 { - return Err(format!("invalid GUID format: {s}").into()); - } - let mut bytes = [0u8; 16]; - for (i, chunk) in bytes.iter_mut().enumerate() { - let pair = hex - .get(i * 2..i * 2 + 2) - .ok_or("guid hex slice out of range")?; - *chunk = u8::from_str_radix(pair, 16)?; - } - bytes[0..4].reverse(); - bytes[4..6].reverse(); - bytes[6..8].reverse(); - Ok(Guid(bytes)) -} - // ---- canned in-memory resolver ---------------------------------------------- struct StaticResolver { diff --git a/rust/crates/mxaccess/examples/multi-tag.rs b/rust/crates/mxaccess/examples/multi-tag.rs index c4aad86..8421c02 100644 --- a/rust/crates/mxaccess/examples/multi-tag.rs +++ b/rust/crates/mxaccess/examples/multi-tag.rs @@ -112,7 +112,7 @@ impl LiveEnv { } let host = std::env::var("MX_NMX_HOST")?; let addr = parse_host_port(&host, 135)?; - let service_ipid = parse_guid(&std::env::var("MX_NMX_SERVICE_IPID")?)?; + let service_ipid = Guid::parse_str(&std::env::var("MX_NMX_SERVICE_IPID")?)?; let tags = std::env::var("MX_TEST_TAGS") .unwrap_or_else(|_| "TestChildObject.TestInt,TestChildObject.TestBool".into()) .split(',') @@ -146,25 +146,6 @@ fn parse_host_port( ) } -fn parse_guid(s: &str) -> Result> { - let trimmed = s.trim_start_matches('{').trim_end_matches('}'); - let hex: String = trimmed.chars().filter(|c| *c != '-').collect(); - if hex.len() != 32 { - return Err(format!("invalid GUID format: {s}").into()); - } - let mut bytes = [0u8; 16]; - for (i, chunk) in bytes.iter_mut().enumerate() { - let pair = hex - .get(i * 2..i * 2 + 2) - .ok_or("guid hex slice out of range")?; - *chunk = u8::from_str_radix(pair, 16)?; - } - bytes[0..4].reverse(); - bytes[4..6].reverse(); - bytes[6..8].reverse(); - Ok(Guid(bytes)) -} - /// Multi-tag canned resolver. Returns Int32 metadata for any tag whose /// `Object.Attribute` form matches the configured allow-list, with a /// fresh `attribute_id` per tag so AdviseSupervisory frames don't diff --git a/rust/crates/mxaccess/examples/recovery.rs b/rust/crates/mxaccess/examples/recovery.rs index 3f1521b..12e03de 100644 --- a/rust/crates/mxaccess/examples/recovery.rs +++ b/rust/crates/mxaccess/examples/recovery.rs @@ -103,7 +103,7 @@ impl LiveEnv { } let host = std::env::var("MX_NMX_HOST")?; let addr = parse_host_port(&host, 135)?; - let service_ipid = parse_guid(&std::env::var("MX_NMX_SERVICE_IPID")?)?; + let service_ipid = Guid::parse_str(&std::env::var("MX_NMX_SERVICE_IPID")?)?; let tag = std::env::var("MX_TEST_TAG").unwrap_or_else(|_| "TestChildObject.TestInt".into()); Ok(Some(Self { addr, @@ -132,25 +132,6 @@ fn parse_host_port( ) } -fn parse_guid(s: &str) -> Result> { - let trimmed = s.trim_start_matches('{').trim_end_matches('}'); - let hex: String = trimmed.chars().filter(|c| *c != '-').collect(); - if hex.len() != 32 { - return Err(format!("invalid GUID format: {s}").into()); - } - let mut bytes = [0u8; 16]; - for (i, chunk) in bytes.iter_mut().enumerate() { - let pair = hex - .get(i * 2..i * 2 + 2) - .ok_or("guid hex slice out of range")?; - *chunk = u8::from_str_radix(pair, 16)?; - } - bytes[0..4].reverse(); - bytes[4..6].reverse(); - bytes[6..8].reverse(); - Ok(Guid(bytes)) -} - struct StaticResolver { tag_reference: String, metadata: GalaxyTagMetadata, diff --git a/rust/crates/mxaccess/examples/secured-write.rs b/rust/crates/mxaccess/examples/secured-write.rs index 5322222..ae3eb91 100644 --- a/rust/crates/mxaccess/examples/secured-write.rs +++ b/rust/crates/mxaccess/examples/secured-write.rs @@ -115,7 +115,7 @@ impl LiveEnv { } let host = std::env::var("MX_NMX_HOST")?; let addr = parse_host_port(&host, 135)?; - let service_ipid = parse_guid(&std::env::var("MX_NMX_SERVICE_IPID")?)?; + let service_ipid = Guid::parse_str(&std::env::var("MX_NMX_SERVICE_IPID")?)?; let tag = std::env::var("MX_TEST_TAG") .unwrap_or_else(|_| "TestChildObject.TestSecuredInt".into()); let user_id: i32 = std::env::var("MX_TEST_USER_ID")?.parse()?; @@ -152,25 +152,6 @@ fn parse_host_port( ) } -fn parse_guid(s: &str) -> Result> { - let trimmed = s.trim_start_matches('{').trim_end_matches('}'); - let hex: String = trimmed.chars().filter(|c| *c != '-').collect(); - if hex.len() != 32 { - return Err(format!("invalid GUID format: {s}").into()); - } - let mut bytes = [0u8; 16]; - for (i, chunk) in bytes.iter_mut().enumerate() { - let pair = hex - .get(i * 2..i * 2 + 2) - .ok_or("guid hex slice out of range")?; - *chunk = u8::from_str_radix(pair, 16)?; - } - bytes[0..4].reverse(); - bytes[4..6].reverse(); - bytes[6..8].reverse(); - Ok(Guid(bytes)) -} - struct StaticResolver { tag_reference: String, metadata: GalaxyTagMetadata, diff --git a/rust/crates/mxaccess/examples/subscribe.rs b/rust/crates/mxaccess/examples/subscribe.rs index 0c90fe0..eb07d43 100644 --- a/rust/crates/mxaccess/examples/subscribe.rs +++ b/rust/crates/mxaccess/examples/subscribe.rs @@ -83,7 +83,7 @@ impl LiveEnv { } let host = std::env::var("MX_NMX_HOST")?; let addr = parse_host_port(&host, 135)?; - let service_ipid = parse_guid(&std::env::var("MX_NMX_SERVICE_IPID")?)?; + let service_ipid = Guid::parse_str(&std::env::var("MX_NMX_SERVICE_IPID")?)?; let tag = std::env::var("MX_TEST_TAG").unwrap_or_else(|_| "TestChildObject.TestInt".into()); Ok(Some(Self { addr, @@ -112,25 +112,6 @@ fn parse_host_port( ) } -fn parse_guid(s: &str) -> Result> { - let trimmed = s.trim_start_matches('{').trim_end_matches('}'); - let hex: String = trimmed.chars().filter(|c| *c != '-').collect(); - if hex.len() != 32 { - return Err(format!("invalid GUID format: {s}").into()); - } - let mut bytes = [0u8; 16]; - for (i, chunk) in bytes.iter_mut().enumerate() { - let pair = hex - .get(i * 2..i * 2 + 2) - .ok_or("guid hex slice out of range")?; - *chunk = u8::from_str_radix(pair, 16)?; - } - bytes[0..4].reverse(); - bytes[4..6].reverse(); - bytes[6..8].reverse(); - Ok(Guid(bytes)) -} - struct StaticResolver { tag_reference: String, metadata: GalaxyTagMetadata,