diff --git a/docs/builder-pattern-conventions.md b/docs/builder-pattern-conventions.md new file mode 100644 index 00000000..d1bc579b --- /dev/null +++ b/docs/builder-pattern-conventions.md @@ -0,0 +1,27 @@ +# Builder pattern conventions + +This guide describes how to handle type-transitioning builder methods in +Wireframe. When a method changes a generic parameter, struct update syntax +(`..self`) cannot be used, so the builder must be reconstructed explicitly. + +## Choosing an approach + +Use a helper method when: + +- The builder has many fields (roughly ten or more). +- Type transitions update multiple related fields. +- The same reconstruction logic appears in more than one method. + +Use a macro when: + +- The builder has a small, stable field set (single digits). +- Each method updates a single field. +- The reconstruction is a straightforward field copy. + +## Current patterns + +- `WireframeApp::rebuild_with_params` centralizes reconstruction for the + 13-field server builder and keeps coordinated updates for serializer, codec, + protocol, and fragmentation together. +- `builder_field_update!` in `src/client/builder/mod.rs` covers the five-field + client builder, where each type change updates one field at a time. diff --git a/docs/contents.md b/docs/contents.md index ee94c1e4..76b2c787 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -57,6 +57,8 @@ the-road-to-wireframe-1-0-feature-set-philosophy-and-capability-maturity.md - [Refactoring guide](complexity-antipatterns-and-refactoring-strategies.md) Strategies for taming code complexity and refactoring. +- [Builder pattern conventions](builder-pattern-conventions.md) Guidance for + type-transitioning builders and reconstruction patterns. - [Documentation style guide](documentation-style-guide.md) Conventions for writing project documentation. - [Server configuration](server/configuration.md) Tuning accept loop backoff diff --git a/src/app/builder/codec.rs b/src/app/builder/codec.rs index 3dd07ec2..820fa799 100644 --- a/src/app/builder/codec.rs +++ b/src/app/builder/codec.rs @@ -1,6 +1,6 @@ //! Codec and serializer configuration for `WireframeApp`. -use super::WireframeApp; +use super::{WireframeApp, core::RebuildParams}; use crate::{ app::Packet, codec::{FrameCodec, LengthDelimitedFrameCodec, clamp_frame_length}, @@ -26,7 +26,13 @@ where { let serializer = std::mem::take(&mut self.serializer); let message_assembler = self.message_assembler.take(); - self.rebuild_with_params(serializer, codec, None, None, message_assembler) + self.rebuild_with_params(RebuildParams { + serializer, + codec, + protocol: None, + fragmentation: None, + message_assembler, + }) } /// Replace the serializer used for messages. @@ -40,13 +46,13 @@ where let protocol = self.protocol.take(); let fragmentation = self.fragmentation.take(); let message_assembler = self.message_assembler.take(); - self.rebuild_with_params( + self.rebuild_with_params(RebuildParams { serializer, codec, protocol, fragmentation, message_assembler, - ) + }) } } diff --git a/src/app/builder/core.rs b/src/app/builder/core.rs index 29e7eb3b..7dfc9ce9 100644 --- a/src/app/builder/core.rs +++ b/src/app/builder/core.rs @@ -1,10 +1,6 @@ //! Core builder types for `WireframeApp`. -use std::{ - any::{Any, TypeId}, - collections::HashMap, - sync::Arc, -}; +use std::{collections::HashMap, sync::Arc}; use tokio::sync::{OnceCell, mpsc}; @@ -16,6 +12,7 @@ use crate::{ lifecycle::{ConnectionSetup, ConnectionTeardown}, middleware_types::{Handler, Middleware}, }, + app_data_store::AppDataStore, codec::{FrameCodec, LengthDelimitedFrameCodec}, hooks::WireframeProtocol, message_assembler::MessageAssembler, @@ -38,7 +35,7 @@ pub struct WireframeApp< pub(in crate::app) routes: OnceCell>>>, pub(in crate::app) middleware: Vec>>, pub(in crate::app) serializer: S, - pub(in crate::app) app_data: HashMap>, + pub(in crate::app) app_data: AppDataStore, pub(in crate::app) on_connect: Option>>, pub(in crate::app) on_disconnect: Option>>, pub(in crate::app) protocol: @@ -66,7 +63,7 @@ where routes: OnceCell::new(), middleware: Vec::new(), serializer: S::default(), - app_data: HashMap::new(), + app_data: AppDataStore::default(), on_connect: None, on_disconnect: None, protocol: None, @@ -115,6 +112,18 @@ where } } +/// Groups the type-changing parameters for [`WireframeApp::rebuild_with_params`]. +/// +/// Consolidates serializer, codec, protocol, fragmentation, and message +/// assembler into a single value to keep the rebuild signature concise. +pub(super) struct RebuildParams { + pub(super) serializer: S2, + pub(super) codec: F2, + pub(super) protocol: Option>>, + pub(super) fragmentation: Option, + pub(super) message_assembler: Option>, +} + impl WireframeApp where S: Serializer + Send + Sync, @@ -124,19 +133,15 @@ where { /// Helper to rebuild the app when changing type parameters. /// - /// This centralises the field-by-field reconstruction required when - /// transforming between different serializer or codec types. - #[expect( - clippy::too_many_arguments, - reason = "internal helper grouping fields for type-transitioning builders" - )] + /// The `WireframeApp` builder carries 13 fields that must be moved together + /// when swapping serializer or codec types. Centralising the reconstruction + /// here keeps the transitions consistent and avoids repeating the same + /// field list across each type-changing method. For smaller builders with + /// only a handful of fields and single-field updates, prefer the macro-based + /// pattern used by `WireframeClientBuilder`. pub(super) fn rebuild_with_params( self, - serializer: S2, - codec: F2, - protocol: Option>>, - fragmentation: Option, - message_assembler: Option>, + params: RebuildParams, ) -> WireframeApp where S2: Serializer + Send + Sync, @@ -146,16 +151,16 @@ where handlers: self.handlers, routes: OnceCell::new(), middleware: self.middleware, - serializer, + serializer: params.serializer, app_data: self.app_data, on_connect: self.on_connect, on_disconnect: self.on_disconnect, - protocol, + protocol: params.protocol, push_dlq: self.push_dlq, - codec, + codec: params.codec, read_timeout_ms: self.read_timeout_ms, - fragmentation, - message_assembler, + fragmentation: params.fragmentation, + message_assembler: params.message_assembler, } } } diff --git a/src/app/builder/state.rs b/src/app/builder/state.rs index 904b31f8..835b2877 100644 --- a/src/app/builder/state.rs +++ b/src/app/builder/state.rs @@ -1,7 +1,5 @@ //! Shared state configuration for `WireframeApp`. -use std::{any::TypeId, sync::Arc}; - use super::WireframeApp; use crate::{app::Packet, codec::FrameCodec, serializer::Serializer}; @@ -17,14 +15,11 @@ where /// The value can later be retrieved using [`crate::extractor::SharedState`]. Registering /// another value of the same type overwrites the previous one. #[must_use] - pub fn app_data(mut self, state: T) -> Self + pub fn app_data(self, state: T) -> Self where T: Send + Sync + 'static, { - self.app_data.insert( - TypeId::of::(), - Arc::new(state) as Arc, - ); + self.app_data.insert(state); self } } diff --git a/src/app_data_store.rs b/src/app_data_store.rs new file mode 100644 index 00000000..4c62cdc8 --- /dev/null +++ b/src/app_data_store.rs @@ -0,0 +1,322 @@ +//! Concurrent type-erased application data store for shared state. +//! +//! `AppDataStore` stores one value per concrete type, keyed by `TypeId`. Values +//! are stored in `Arc` to allow cheap cloning and safe +//! sharing across threads. The underlying `DashMap` provides lock-free +//! concurrent reads and sharded writes, enabling multiple threads to insert +//! and retrieve state simultaneously without external synchronisation. +//! Typed accessors provide a small API surface while hiding the underlying +//! type-erasure details. + +use std::{ + any::{Any, TypeId}, + sync::Arc, +}; + +use dashmap::DashMap; + +/// Stores application-scoped state values keyed by concrete type. +/// +/// `AppDataStore` is used by `WireframeApp` and `MessageRequest` to share +/// connection-independent state with extractors without exposing the underlying +/// type-erasure map. +/// +/// # Examples +/// +/// ```rust +/// use wireframe::AppDataStore; +/// +/// let store = AppDataStore::default(); +/// store.insert(42u32); +/// let value = store.get::().expect("value should exist"); +/// assert_eq!(*value, 42); +/// ``` +#[derive(Clone, Default)] +pub struct AppDataStore { + values: DashMap>, +} + +impl AppDataStore { + /// Insert a value of type `T` into the store. + /// + /// Concurrent calls to `insert` from multiple threads are safe. Any + /// existing value of the same type is replaced. + /// + /// # Parameters + /// - `value`: The value to store. Any existing value of the same type is replaced. + /// + /// # Examples + /// + /// ```rust + /// use wireframe::AppDataStore; + /// + /// let store = AppDataStore::default(); + /// store.insert("hello".to_string()); + /// ``` + pub fn insert(&self, value: T) + where + T: Send + Sync + 'static, + { + self.values.insert( + TypeId::of::(), + Arc::new(value) as Arc, + ); + } + + /// Retrieve a shared value of type `T`, if present. + /// + /// # Returns + /// An `Arc` when the value is present, or `None` if no value of type `T` + /// has been registered. + /// + /// # Examples + /// + /// ```rust + /// use wireframe::AppDataStore; + /// + /// let store = AppDataStore::default(); + /// store.insert(5u32); + /// let value = store.get::().expect("value should be present"); + /// assert_eq!(*value, 5); + /// ``` + #[must_use] + pub fn get(&self) -> Option> + where + T: Send + Sync + 'static, + { + self.values + .get(&TypeId::of::()) + .and_then(|guard| Arc::clone(guard.value()).downcast::().ok()) + } + + /// Check whether a value of type `T` is present in the store. + /// + /// # Examples + /// + /// ```rust + /// use wireframe::AppDataStore; + /// + /// let store = AppDataStore::default(); + /// assert!(!store.contains::()); + /// store.insert(42u32); + /// assert!(store.contains::()); + /// ``` + #[must_use] + pub fn contains(&self) -> bool + where + T: 'static, + { + self.values.contains_key(&TypeId::of::()) + } + + /// Remove a value of type `T` from the store, returning it if present. + /// + /// Concurrent calls to `remove` from multiple threads are safe. + /// + /// # Examples + /// + /// ```rust + /// use wireframe::AppDataStore; + /// + /// let store = AppDataStore::default(); + /// store.insert(42u32); + /// let removed = store.remove::(); + /// assert_eq!(*removed.expect("value should have been present"), 42); + /// assert!(!store.contains::()); + /// ``` + #[must_use] + pub fn remove(&self) -> Option> + where + T: Send + Sync + 'static, + { + self.values + .remove(&TypeId::of::()) + .and_then(|(_, arc)| arc.downcast::().ok()) + } +} + +#[cfg(test)] +#[expect( + unused_braces, + reason = "rstest fixture proc-macro consumes item-level attributes before clippy sees them" +)] +mod tests { + //! Unit tests for [`AppDataStore`] covering insertion, retrieval, + //! removal, containment checks, and concurrent access. + + use std::{ + sync::{Arc, Barrier}, + thread, + }; + + use rstest::{fixture, rstest}; + + use super::AppDataStore; + + #[derive(Debug, PartialEq)] + struct CustomState { + label: &'static str, + value: u32, + } + + #[fixture] + fn empty_store() -> AppDataStore { AppDataStore::default() } + + fn assert_send_sync() {} + + #[rstest] + fn insert_and_get_multiple_types(empty_store: AppDataStore) { + empty_store.insert(12u32); + empty_store.insert("hello".to_string()); + empty_store.insert(CustomState { + label: "alpha", + value: 7, + }); + + let number = empty_store.get::().expect("u32 should be present"); + assert_eq!(*number, 12); + + let text = empty_store + .get::() + .expect("String should be present"); + assert_eq!(text.as_str(), "hello"); + + let custom = empty_store + .get::() + .expect("CustomState should be present"); + assert_eq!( + *custom, + CustomState { + label: "alpha", + value: 7, + } + ); + } + + #[rstest] + fn insert_overwrites_existing_value(empty_store: AppDataStore) { + empty_store.insert(10u32); + empty_store.insert(20u32); + + let number = empty_store.get::().expect("u32 should be present"); + assert_eq!(*number, 20); + } + + #[rstest] + fn missing_type_returns_none(empty_store: AppDataStore) { + assert!(empty_store.get::().is_none()); + } + + #[rstest] + fn contains_returns_true_for_present_type(empty_store: AppDataStore) { + assert!(!empty_store.contains::()); + empty_store.insert(42u32); + assert!(empty_store.contains::()); + } + + #[rstest] + fn remove_returns_and_deletes_value(empty_store: AppDataStore) { + empty_store.insert(42u32); + let removed = empty_store.remove::().expect("u32 should be present"); + assert_eq!(*removed, 42); + assert!(empty_store.get::().is_none()); + } + + #[rstest] + fn remove_returns_none_for_absent_type(empty_store: AppDataStore) { + assert!(empty_store.remove::().is_none()); + } + + #[test] + fn store_is_send_and_sync() { assert_send_sync::(); } + + #[rstest] + fn concurrent_insert_and_get(empty_store: AppDataStore) { + let store = Arc::new(empty_store); + let barrier = Arc::new(Barrier::new(3)); + + let handles: Vec<_> = vec![ + { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + store.insert(42u32); + }) + }, + { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + store.insert("hello".to_string()); + }) + }, + { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + store.insert(CustomState { + label: "concurrent", + value: 99, + }); + }) + }, + ]; + + for handle in handles { + handle.join().expect("thread should not panic"); + } + + let number = store.get::().expect("u32 should be present"); + assert_eq!(*number, 42); + let text = store.get::().expect("String should be present"); + assert_eq!(text.as_str(), "hello"); + let custom = store + .get::() + .expect("CustomState should be present"); + assert_eq!( + *custom, + CustomState { + label: "concurrent", + value: 99, + } + ); + } + + #[rstest] + fn concurrent_overwrite_converges(empty_store: AppDataStore) { + let store = Arc::new(empty_store); + let thread_count = 8; + let barrier = Arc::new(Barrier::new(thread_count)); + + let handles: Vec<_> = (0..thread_count) + .map(|i| { + let store = Arc::clone(&store); + let barrier = Arc::clone(&barrier); + thread::spawn(move || { + barrier.wait(); + #[expect( + clippy::cast_possible_truncation, + reason = "thread_count is well within u32 range" + )] + store.insert(i as u32); + }) + }) + .collect(); + + for handle in handles { + handle.join().expect("thread should not panic"); + } + + // One of the threads' values must have "won". + let value = store.get::().expect("u32 should be present"); + #[expect( + clippy::cast_possible_truncation, + reason = "thread_count is well within u32 range" + )] + let upper = thread_count as u32; + assert!(*value < upper); + } +} diff --git a/src/byte_order.rs b/src/byte_order.rs index f31e3e34..6dd5de3f 100644 --- a/src/byte_order.rs +++ b/src/byte_order.rs @@ -17,8 +17,7 @@ pub fn write_network_u16(value: u16) -> [u8; 2] { #[expect( clippy::big_endian_bytes, - reason = "Network byte order is mandated; big-endian bytes keep the wire contract \ - explicit." + reason = "Network byte order requires big-endian bytes." )] value.to_be_bytes() } @@ -36,8 +35,131 @@ pub fn write_network_u16(value: u16) -> [u8; 2] { pub fn read_network_u16(bytes: [u8; 2]) -> u16 { #[expect( clippy::big_endian_bytes, - reason = "Network byte order is mandated; big-endian bytes keep the wire contract \ - explicit." + reason = "Network byte order requires big-endian bytes." )] u16::from_be_bytes(bytes) } + +/// Serialise a `u32` in network byte order (big-endian). +/// +/// # Examples +/// +/// ``` +/// use wireframe::byte_order::write_network_u32; +/// +/// assert_eq!(write_network_u32(0x1234_5678), [0x12, 0x34, 0x56, 0x78]); +/// ``` +#[must_use] +pub fn write_network_u32(value: u32) -> [u8; 4] { + #[expect( + clippy::big_endian_bytes, + reason = "Network byte order requires big-endian bytes." + )] + value.to_be_bytes() +} + +/// Parse a network-order `u32` from its on-wire representation. +/// +/// # Examples +/// +/// ``` +/// use wireframe::byte_order::read_network_u32; +/// +/// assert_eq!(read_network_u32([0x12, 0x34, 0x56, 0x78]), 0x1234_5678); +/// ``` +#[must_use] +pub fn read_network_u32(bytes: [u8; 4]) -> u32 { + #[expect( + clippy::big_endian_bytes, + reason = "Network byte order requires big-endian bytes." + )] + u32::from_be_bytes(bytes) +} + +/// Serialise a `u64` in network byte order (big-endian). +/// +/// # Examples +/// +/// ``` +/// use wireframe::byte_order::write_network_u64; +/// +/// assert_eq!( +/// write_network_u64(0x1122_3344_5566_7788), +/// [0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88] +/// ); +/// ``` +#[must_use] +pub fn write_network_u64(value: u64) -> [u8; 8] { + #[expect( + clippy::big_endian_bytes, + reason = "Network byte order requires big-endian bytes." + )] + value.to_be_bytes() +} + +/// Parse a network-order `u64` from its on-wire representation. +/// +/// # Examples +/// +/// ``` +/// use wireframe::byte_order::read_network_u64; +/// +/// assert_eq!( +/// read_network_u64([0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88]), +/// 0x1122_3344_5566_7788 +/// ); +/// ``` +#[must_use] +pub fn read_network_u64(bytes: [u8; 8]) -> u64 { + #[expect( + clippy::big_endian_bytes, + reason = "Network byte order requires big-endian bytes." + )] + u64::from_be_bytes(bytes) +} + +#[cfg(test)] +mod tests { + //! Round-trip tests for network byte-order conversion helpers. + + use rstest::rstest; + + use super::{ + read_network_u16, + read_network_u32, + read_network_u64, + write_network_u16, + write_network_u32, + write_network_u64, + }; + + /// Verify that each network-order write/read pair round-trips correctly. + #[rstest] + #[case::u16( + 0x1234u64, + &write_network_u16(0x1234)[..], + &[0x12, 0x34], + u64::from(read_network_u16([0x12, 0x34])) + )] + #[case::u32( + 0x1234_5678u64, + &write_network_u32(0x1234_5678)[..], + &[0x12, 0x34, 0x56, 0x78], + u64::from(read_network_u32([0x12, 0x34, 0x56, 0x78])) + )] + #[case::u64( + 0x1122_3344_5566_7788u64, + &write_network_u64(0x1122_3344_5566_7788)[..], + &[0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88], + read_network_u64([0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88]) + )] + fn network_byte_order_round_trip( + #[case] value: u64, + #[case] written: &[u8], + #[case] expected_bytes: &[u8], + #[case] read_back: u64, + ) { + assert_eq!(written, expected_bytes); + assert_eq!(read_back, value); + } +} diff --git a/src/client/builder/mod.rs b/src/client/builder/mod.rs index 56ba8343..6752500e 100644 --- a/src/client/builder/mod.rs +++ b/src/client/builder/mod.rs @@ -7,6 +7,13 @@ /// parameter changes, struct update syntax (`..self`) cannot be used, so fields /// must be copied explicitly. /// +/// Use this macro for small builders with a limited number of fields (five in +/// this case) and single-field updates per method. For larger builders with +/// many coordinated updates, prefer a dedicated helper method to keep the +/// reconstruction logic centralised and easier to audit (see +/// `WireframeApp::rebuild_with_params` and +/// `docs/builder-pattern-conventions.md`). +/// /// The `lifecycle_hooks` field requires special handling because `LifecycleHooks` /// is parameterized by the connection state type. When changing `S` or `P`, the /// hooks can be moved directly since `C` is unchanged. When changing `C` via diff --git a/src/codec.rs b/src/codec.rs index 31b87e23..5eabfec9 100644 --- a/src/codec.rs +++ b/src/codec.rs @@ -23,6 +23,8 @@ use std::io; use bytes::{Bytes, BytesMut}; use tokio_util::codec::{Decoder, Encoder, LengthDelimitedCodec}; +use crate::byte_order::read_network_u32; + pub mod error; pub mod recovery; @@ -182,15 +184,6 @@ impl Decoder for LengthDelimitedDecoder { } } -/// Parse a u32 length prefix from a 4-byte big-endian array. -#[expect( - clippy::big_endian_bytes, - reason = "Wire endianness is explicit; length-delimited codec uses big-endian." -)] -fn parse_length_header(bytes: [u8; LENGTH_HEADER_SIZE]) -> usize { - u32::from_be_bytes(bytes) as usize -} - /// Build the appropriate EOF error based on remaining buffer state. /// /// Determines whether the connection closed mid-header or mid-frame: @@ -206,7 +199,7 @@ fn build_eof_error(src: &BytesMut) -> io::Error { let expected = src .get(..LENGTH_HEADER_SIZE) .and_then(|slice| <[u8; LENGTH_HEADER_SIZE]>::try_from(slice).ok()) - .map(parse_length_header); + .and_then(|bytes| usize::try_from(read_network_u32(bytes)).ok()); match expected { Some(expected) => { diff --git a/src/extractor/request.rs b/src/extractor/request.rs index 1b372595..876122a9 100644 --- a/src/extractor/request.rs +++ b/src/extractor/request.rs @@ -1,14 +1,9 @@ //! Request context and payload buffer types for extractors. -use std::{ - any::{Any, TypeId}, - collections::HashMap, - net::SocketAddr, - sync::{Arc, Mutex}, -}; +use std::{net::SocketAddr, sync::Mutex}; use super::SharedState; -use crate::request::RequestBodyStream; +use crate::{app_data_store::AppDataStore, request::RequestBodyStream}; /// Request context passed to extractors. /// @@ -20,9 +15,9 @@ pub struct MessageRequest { pub peer_addr: Option, /// Shared state values registered with the application. /// - /// Values are keyed by their [`TypeId`]. Registering additional + /// Values are keyed by their concrete type. Registering additional /// state of the same type will replace the previous entry. - pub(crate) app_data: HashMap>, + pub(crate) app_data: AppDataStore, /// Optional streaming body for handlers that opt into streaming consumption. /// /// When present, the [`StreamingBody`](crate::extractor::StreamingBody) @@ -75,7 +70,7 @@ impl MessageRequest { /// .expect("failed to initialize app") /// .app_data(5u32); /// // The framework populates the request with application data. - /// # let mut req = MessageRequest::default(); + /// # let req = MessageRequest::default(); /// # req.insert_state(5u32); /// let val: Option> = req.state(); /// assert_eq!(*val.expect("shared state missing"), 5); @@ -85,10 +80,7 @@ impl MessageRequest { where T: Send + Sync + 'static, { - self.app_data - .get(&TypeId::of::()) - .and_then(|data| data.clone().downcast::().ok()) - .map(SharedState::from) + self.app_data.get::().map(SharedState::from) } /// Insert shared state of type `T` into the request. @@ -100,19 +92,16 @@ impl MessageRequest { /// ```rust /// use wireframe::extractor::{MessageRequest, SharedState}; /// - /// let mut req = MessageRequest::default(); + /// let req = MessageRequest::default(); /// req.insert_state(5u32); /// let val: Option> = req.state(); /// assert_eq!(*val.expect("shared state missing"), 5); /// ``` - pub fn insert_state(&mut self, state: T) + pub fn insert_state(&self, state: T) where T: Send + Sync + 'static, { - self.app_data.insert( - TypeId::of::(), - Arc::new(state) as Arc, - ); + self.app_data.insert(state); } /// Set the streaming body for this request. diff --git a/src/frame/conversion.rs b/src/frame/conversion.rs index 19387288..a0c96467 100644 --- a/src/frame/conversion.rs +++ b/src/frame/conversion.rs @@ -2,20 +2,17 @@ use std::io; use super::format::Endianness; +use crate::byte_order::{ + read_network_u64, + write_network_u16, + write_network_u32, + write_network_u64, +}; pub(crate) const ERR_UNSUPPORTED_PREFIX: &str = "unsupported length prefix size"; pub(crate) const ERR_FRAME_TOO_LARGE: &str = "frame too large"; pub(crate) const ERR_INCOMPLETE_PREFIX: &str = "incomplete length prefix"; -#[inline] -fn u64_from_be_bytes(bytes: [u8; 8]) -> u64 { - #[expect( - clippy::big_endian_bytes, - reason = "Wire endianness is explicit; from_be_bytes keeps decoding host-independent." - )] - u64::from_be_bytes(bytes) -} - #[inline] fn u64_from_le_bytes(bytes: [u8; 8]) -> u64 { #[expect( @@ -88,7 +85,7 @@ pub fn bytes_to_u64(bytes: &[u8], size: usize, endianness: Endianness) -> io::Re // using explicit conversion helpers keeps decoding deterministic on any host // CPU. let val = match endianness { - Endianness::Big => u64_from_be_bytes(buf), + Endianness::Big => read_network_u64(buf), Endianness::Little => u64_from_le_bytes(buf), }; Ok(val) @@ -116,21 +113,42 @@ fn convert_len_to_value(len: usize, size: usize) -> io::Result { } /// Write a u64 into `prefix` according to the specified endianness. +/// +/// For big-endian, the function delegates to the typed network-order write +/// helpers mirroring how the read path delegates to `read_network_u64`. fn write_bytes_with_endianness(value: u64, endianness: Endianness, prefix: &mut [u8]) { let size = prefix.len(); match endianness { - Endianness::Big => { - for (i, byte) in prefix.iter_mut().enumerate() { - let shift = 8 * (size - 1 - i); - *byte = ((value >> shift) & 0xff) as u8; - } - } - Endianness::Little => { - for (i, byte) in prefix.iter_mut().enumerate() { - let shift = 8 * i; - *byte = ((value >> shift) & 0xff) as u8; - } - } + Endianness::Big => write_big_endian_prefix(value, size, prefix), + Endianness::Little => write_little_endian_prefix(value, prefix), + } +} + +/// Encode `value` into `prefix` in network (big-endian) byte order. +/// +/// Delegates to the typed `write_network_*` helpers so the encode and +/// decode paths share the same lint-suppressed conversion point. +/// Callers guarantee the value fits in the requested prefix size via +/// `checked_prefix_cast` in `convert_len_to_value`. +fn write_big_endian_prefix(value: u64, size: usize, prefix: &mut [u8]) { + #[expect( + clippy::cast_possible_truncation, + reason = "caller validates value fits in prefix via checked_prefix_cast" + )] + match size { + 1 => prefix.copy_from_slice(&[value as u8]), + 2 => prefix.copy_from_slice(&write_network_u16(value as u16)), + 4 => prefix.copy_from_slice(&write_network_u32(value as u32)), + 8 => prefix.copy_from_slice(&write_network_u64(value)), + _ => debug_assert!(false, "size validated upstream to be 1, 2, 4, or 8"), + } +} + +/// Encode `value` into `prefix` in little-endian byte order. +fn write_little_endian_prefix(value: u64, prefix: &mut [u8]) { + for (i, byte) in prefix.iter_mut().enumerate() { + let shift = 8 * i; + *byte = ((value >> shift) & 0xff) as u8; } } diff --git a/src/lib.rs b/src/lib.rs index 1b9321ac..7b902ff5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,11 +7,13 @@ extern crate self as wireframe; pub mod app; +pub mod app_data_store; pub mod byte_order; pub mod codec; /// Result type alias re-exported for convenience when working with the /// application builder. pub use app::error::Result; +pub use app_data_store::AppDataStore; #[cfg(not(loom))] pub mod client; pub mod serializer; diff --git a/tests/app_data.rs b/tests/app_data.rs index 0b85688b..ececd99e 100644 --- a/tests/app_data.rs +++ b/tests/app_data.rs @@ -26,7 +26,7 @@ fn empty_payload() -> Payload<'static> { #[rstest] fn shared_state_extractor_returns_data( - mut request: MessageRequest, + request: MessageRequest, mut empty_payload: Payload<'static>, ) { request.insert_state(5u32); diff --git a/tests/extractor.rs b/tests/extractor.rs index df64adeb..1708bb19 100644 --- a/tests/extractor.rs +++ b/tests/extractor.rs @@ -62,7 +62,7 @@ fn connection_info_reports_peer(mut request: MessageRequest, mut empty_payload: /// Inserts an `Arc` into the request's shared state, extracts it using the `SharedState` /// extractor, and asserts that the extracted value matches the original. #[rstest] -fn shared_state_extractor(mut request: MessageRequest, mut empty_payload: Payload<'static>) { +fn shared_state_extractor(request: MessageRequest, mut empty_payload: Payload<'static>) { request.insert_state(42u8); let state = diff --git a/tests/fixtures/codec_error/decoder_ops.rs b/tests/fixtures/codec_error/decoder_ops.rs index ea42da8f..b08fa646 100644 --- a/tests/fixtures/codec_error/decoder_ops.rs +++ b/tests/fixtures/codec_error/decoder_ops.rs @@ -7,6 +7,7 @@ use bytes::BytesMut; use tokio_util::codec::Decoder; use wireframe::{ FrameCodec, + byte_order::{read_network_u32, write_network_u32}, codec::{EofError, LENGTH_HEADER_SIZE, LengthDelimitedFrameCodec}, }; @@ -50,7 +51,7 @@ impl CodecErrorWorld { pub fn send_partial_frame_header_only(&mut self) { // Write a length prefix indicating 100 bytes, but don't write any payload // 4-byte big-endian length prefix - self.buffer.extend_from_slice(&[0x00, 0x00, 0x00, 0x64]); // 100 bytes expected + self.buffer.extend_from_slice(&write_network_u32(100)); // 100 bytes expected } /// Call `decode_eof` to simulate a clean close at frame boundary. @@ -87,15 +88,11 @@ impl CodecErrorWorld { /// Extract the expected payload length from the buffer's length header. /// /// Returns 0 if the buffer doesn't contain a complete length header. - #[expect( - clippy::big_endian_bytes, - reason = "Wire protocol uses big-endian length prefix; this matches the codec." - )] fn extract_expected_length(&self) -> usize { self.buffer .get(..LENGTH_HEADER_SIZE) .and_then(|slice| <[u8; LENGTH_HEADER_SIZE]>::try_from(slice).ok()) - .map_or(0, |bytes| u32::from_be_bytes(bytes) as usize) + .map_or(0, |bytes| read_network_u32(bytes) as usize) } /// Classify the EOF error type from the error message.