From df3f0a0167603d3f345de58676a3441112a0b566 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 2 Feb 2026 02:05:30 +0000 Subject: [PATCH 1/2] Move fragment_packet to fragment module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the layering violation where `fragment_packet` was defined in `app::fragment_utils` but used by the connection actor. The function now resides in its canonical location at `fragment::packet` alongside other fragmentation primitives, while preserving backward compatibility through a re-export from the original `app::fragment_utils` path. This follows the modularisation pattern established in PR #424. Closes #431 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/app/fragment_utils.rs | 44 ++++------------------------------ src/app/fragmentation_state.rs | 2 +- src/fragment/mod.rs | 2 ++ src/fragment/packet.rs | 43 +++++++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 40 deletions(-) create mode 100644 src/fragment/packet.rs diff --git a/src/app/fragment_utils.rs b/src/app/fragment_utils.rs index 4e298378..08a9445b 100644 --- a/src/app/fragment_utils.rs +++ b/src/app/fragment_utils.rs @@ -1,41 +1,7 @@ //! Shared helpers for applying transport-level fragmentation to packets. +//! +//! This module re-exports [`fragment_packet`] from the canonical location in +//! [`crate::fragment::packet`] to preserve backward compatibility. -use crate::{ - app::{Packet, PacketParts}, - fragment::{FragmentationError, Fragmenter, encode_fragment_payload}, -}; - -/// Fragment a packet using the provided fragmenter, returning one or more frames. -/// -/// Small payloads that fit within the fragment cap are returned unchanged. -/// -/// # Errors -/// -/// Returns [`FragmentationError`] if fragmenting the payload fails or if -/// encoding the fragment header and payload into an on-wire frame fails. -pub fn fragment_packet( - fragmenter: &Fragmenter, - packet: E, -) -> Result, FragmentationError> { - let parts = packet.into_parts(); - let id = parts.id(); - let correlation = parts.correlation_id(); - let payload = parts.payload(); - - let batch = fragmenter.fragment_bytes(&payload)?; - if !batch.is_fragmented() { - return Ok(vec![E::from_parts(PacketParts::new( - id, - correlation, - payload, - ))]); - } - - let mut frames = Vec::with_capacity(batch.len()); - for fragment in batch { - let (header, payload) = fragment.into_parts(); - let encoded = encode_fragment_payload(header, &payload)?; - frames.push(E::from_parts(PacketParts::new(id, correlation, encoded))); - } - Ok(frames) -} +// Re-export from canonical location for backward compatibility. +pub use crate::fragment::fragment_packet; diff --git a/src/app/fragmentation_state.rs b/src/app/fragmentation_state.rs index b4460379..e4afc980 100644 --- a/src/app/fragmentation_state.rs +++ b/src/app/fragmentation_state.rs @@ -40,7 +40,7 @@ impl FragmentationState { } pub(crate) fn fragment(&self, packet: E) -> Result, FragmentationError> { - crate::app::fragment_utils::fragment_packet(&self.fragmenter, packet) + crate::fragment::fragment_packet(&self.fragmenter, packet) } pub(crate) fn reassemble( diff --git a/src/fragment/mod.rs b/src/fragment/mod.rs index 5bb4e14d..a2e1bdb4 100644 --- a/src/fragment/mod.rs +++ b/src/fragment/mod.rs @@ -11,6 +11,7 @@ pub mod fragmenter; pub mod header; pub mod id; pub mod index; +pub mod packet; pub mod payload; pub mod reassembler; pub mod series; @@ -21,6 +22,7 @@ pub use fragmenter::{FragmentBatch, FragmentFrame, Fragmenter}; pub use header::FragmentHeader; pub use id::MessageId; pub use index::FragmentIndex; +pub use packet::fragment_packet; pub use payload::{ FRAGMENT_MAGIC, decode_fragment_payload, diff --git a/src/fragment/packet.rs b/src/fragment/packet.rs new file mode 100644 index 00000000..2c5e1747 --- /dev/null +++ b/src/fragment/packet.rs @@ -0,0 +1,43 @@ +//! Shared helpers for applying transport-level fragmentation to packets. +//! +//! This module provides the [`fragment_packet`] function used by both the +//! connection actor and the application layer to split oversized payloads +//! into transport-safe fragment frames. + +use super::{FragmentationError, Fragmenter, encode_fragment_payload}; +use crate::app::{Packet, PacketParts}; + +/// Fragment a packet using the provided fragmenter, returning one or more frames. +/// +/// Small payloads that fit within the fragment cap are returned unchanged. +/// +/// # Errors +/// +/// Returns [`FragmentationError`] if fragmenting the payload fails or if +/// encoding the fragment header and payload into an on-wire frame fails. +pub fn fragment_packet( + fragmenter: &Fragmenter, + packet: E, +) -> Result, FragmentationError> { + let parts = packet.into_parts(); + let id = parts.id(); + let correlation = parts.correlation_id(); + let payload = parts.payload(); + + let batch = fragmenter.fragment_bytes(&payload)?; + if !batch.is_fragmented() { + return Ok(vec![E::from_parts(PacketParts::new( + id, + correlation, + payload, + ))]); + } + + let mut frames = Vec::with_capacity(batch.len()); + for fragment in batch { + let (header, payload) = fragment.into_parts(); + let encoded = encode_fragment_payload(header, &payload)?; + frames.push(E::from_parts(PacketParts::new(id, correlation, encoded))); + } + Ok(frames) +} From 5f8ad0f34b8d723d9a65b470da8c91fae5a536a1 Mon Sep 17 00:00:00 2001 From: Leynos Date: Mon, 2 Feb 2026 13:55:22 +0000 Subject: [PATCH 2/2] Introduce Fragmentable trait to remove app layer dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses code review feedback on the fragment_packet refactoring: 1. Introduces `Fragmentable` trait and `FragmentParts` type in the fragment module, removing the dependency on `crate::app::{Packet, PacketParts}`. The fragmentation layer now operates on its own abstractions. 2. Adds a blanket implementation in the app module so all `Packet` types automatically implement `Fragmentable`, preserving backward compatibility. 3. Marks `app::fragment_utils::fragment_packet` as deprecated with guidance to migrate to `crate::fragment::fragment_packet`. This completes the layering fix: the fragment module no longer depends on app-layer types, and the transition path for callers is explicit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/app/envelope.rs | 22 ++++++++++- src/app/fragment_utils.rs | 9 ++++- src/app/fragmentation_state.rs | 6 ++- src/fragment/mod.rs | 2 +- src/fragment/packet.rs | 72 ++++++++++++++++++++++++++++++---- 5 files changed, 98 insertions(+), 13 deletions(-) diff --git a/src/app/envelope.rs b/src/app/envelope.rs index d11fca49..85b843c6 100644 --- a/src/app/envelope.rs +++ b/src/app/envelope.rs @@ -6,7 +6,11 @@ //! [`crate::app::builder::WireframeApp`] for how envelopes are used when //! registering routes. -use crate::{correlation::CorrelatableFrame, message::Message}; +use crate::{ + correlation::CorrelatableFrame, + fragment::{FragmentParts, Fragmentable}, + message::Message, +}; /// Envelope-like type used to wrap incoming and outgoing messages. /// @@ -243,3 +247,19 @@ impl From for Envelope { Envelope::new(id, correlation_id, payload) } } + +// Blanket implementation: any Packet is automatically Fragmentable. +impl Fragmentable for T { + fn into_fragment_parts(self) -> FragmentParts { + let parts = self.into_parts(); + FragmentParts::new(parts.id(), parts.correlation_id(), parts.payload()) + } + + fn from_fragment_parts(parts: FragmentParts) -> Self { + T::from_parts(PacketParts::new( + parts.id(), + parts.correlation_id(), + parts.payload(), + )) + } +} diff --git a/src/app/fragment_utils.rs b/src/app/fragment_utils.rs index 08a9445b..70e5fd77 100644 --- a/src/app/fragment_utils.rs +++ b/src/app/fragment_utils.rs @@ -1,7 +1,12 @@ //! Shared helpers for applying transport-level fragmentation to packets. //! -//! This module re-exports [`fragment_packet`] from the canonical location in -//! [`crate::fragment::packet`] to preserve backward compatibility. +//! **Deprecated:** This module re-exports [`fragment_packet`] from its +//! canonical location in [`crate::fragment`]. New code should import directly +//! from [`crate::fragment::fragment_packet`] instead. // Re-export from canonical location for backward compatibility. +#[deprecated( + since = "0.3.0", + note = "use `crate::fragment::fragment_packet` instead" +)] pub use crate::fragment::fragment_packet; diff --git a/src/app/fragmentation_state.rs b/src/app/fragmentation_state.rs index e4afc980..80b7b2ba 100644 --- a/src/app/fragmentation_state.rs +++ b/src/app/fragmentation_state.rs @@ -8,6 +8,7 @@ use thiserror::Error; use super::{Packet, PacketParts}; use crate::fragment::{ + Fragmentable, FragmentationError, Fragmenter, MessageId, @@ -39,7 +40,10 @@ impl FragmentationState { } } - pub(crate) fn fragment(&self, packet: E) -> Result, FragmentationError> { + pub(crate) fn fragment( + &self, + packet: E, + ) -> Result, FragmentationError> { crate::fragment::fragment_packet(&self.fragmenter, packet) } diff --git a/src/fragment/mod.rs b/src/fragment/mod.rs index a2e1bdb4..881f3061 100644 --- a/src/fragment/mod.rs +++ b/src/fragment/mod.rs @@ -22,7 +22,7 @@ pub use fragmenter::{FragmentBatch, FragmentFrame, Fragmenter}; pub use header::FragmentHeader; pub use id::MessageId; pub use index::FragmentIndex; -pub use packet::fragment_packet; +pub use packet::{FragmentParts, Fragmentable, fragment_packet}; pub use payload::{ FRAGMENT_MAGIC, decode_fragment_payload, diff --git a/src/fragment/packet.rs b/src/fragment/packet.rs index 2c5e1747..e8c21068 100644 --- a/src/fragment/packet.rs +++ b/src/fragment/packet.rs @@ -1,11 +1,63 @@ //! Shared helpers for applying transport-level fragmentation to packets. //! -//! This module provides the [`fragment_packet`] function used by both the -//! connection actor and the application layer to split oversized payloads -//! into transport-safe fragment frames. +//! This module provides the [`Fragmentable`] trait and [`fragment_packet`] +//! function used by both the connection actor and the application layer to +//! split oversized payloads into transport-safe fragment frames. use super::{FragmentationError, Fragmenter, encode_fragment_payload}; -use crate::app::{Packet, PacketParts}; + +/// Component values extracted from or used to build a [`Fragmentable`] packet. +/// +/// This type mirrors [`crate::app::PacketParts`] but lives in the fragment +/// module to avoid a layering violation where fragmentation depends on app +/// types. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FragmentParts { + id: u32, + correlation_id: Option, + payload: Vec, +} + +impl FragmentParts { + /// Construct a new set of fragment parts. + #[must_use] + pub fn new(id: u32, correlation_id: Option, payload: Vec) -> Self { + Self { + id, + correlation_id, + payload, + } + } + + /// Return the message identifier. + #[must_use] + pub const fn id(&self) -> u32 { self.id } + + /// Retrieve the correlation identifier, if present. + #[must_use] + pub const fn correlation_id(&self) -> Option { self.correlation_id } + + /// Consume the parts and return the raw payload bytes. + #[must_use] + pub fn payload(self) -> Vec { self.payload } +} + +/// A packet that can be decomposed into parts and reconstructed for fragmentation. +/// +/// This trait captures the minimal interface required by the fragmentation layer +/// to split oversized packets into smaller frames and reassemble them. It +/// intentionally avoids depending on application-layer types like +/// [`crate::app::Packet`]. +/// +/// Types implementing [`crate::app::Packet`] automatically implement this trait +/// via a blanket implementation in the `app` module. +pub trait Fragmentable: Send + Sync + 'static + Sized { + /// Consume the packet and return its identifier, correlation id and payload bytes. + fn into_fragment_parts(self) -> FragmentParts; + + /// Construct a new packet from raw parts. + fn from_fragment_parts(parts: FragmentParts) -> Self; +} /// Fragment a packet using the provided fragmenter, returning one or more frames. /// @@ -15,18 +67,18 @@ use crate::app::{Packet, PacketParts}; /// /// Returns [`FragmentationError`] if fragmenting the payload fails or if /// encoding the fragment header and payload into an on-wire frame fails. -pub fn fragment_packet( +pub fn fragment_packet( fragmenter: &Fragmenter, packet: E, ) -> Result, FragmentationError> { - let parts = packet.into_parts(); + let parts = packet.into_fragment_parts(); let id = parts.id(); let correlation = parts.correlation_id(); let payload = parts.payload(); let batch = fragmenter.fragment_bytes(&payload)?; if !batch.is_fragmented() { - return Ok(vec![E::from_parts(PacketParts::new( + return Ok(vec![E::from_fragment_parts(FragmentParts::new( id, correlation, payload, @@ -37,7 +89,11 @@ pub fn fragment_packet( for fragment in batch { let (header, payload) = fragment.into_parts(); let encoded = encode_fragment_payload(header, &payload)?; - frames.push(E::from_parts(PacketParts::new(id, correlation, encoded))); + frames.push(E::from_fragment_parts(FragmentParts::new( + id, + correlation, + encoded, + ))); } Ok(frames) }