diff --git a/Cargo.lock b/Cargo.lock index 4a4f21112..335372603 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1875,6 +1875,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "uuid", ] [[package]] diff --git a/Justfile b/Justfile index 16f6f6981..af545338a 100644 --- a/Justfile +++ b/Justfile @@ -163,7 +163,10 @@ ci-setup-macos: ensure-rust ensure-uv ci-setup-fedora python_version="3.12": ensure-uv #!/usr/bin/env bash export PATH="$HOME/.local/bin:$PATH" - dnf install -y python{{python_version}} + # Install build dependencies + dnf install -y gcc gcc-c++ openssl-devel + # Use uv to install Python (consistent with manylinux setup) + uv python install {{python_version}} uv tool install pytest echo "==> Setup complete!" @@ -193,8 +196,24 @@ ci-build manylinux="": ci-test: #!/usr/bin/env bash export PATH="$HOME/.local/bin:$PATH" - pip install dist/*.whl pytest pytest-asyncio 2>/dev/null || uv pip install --system dist/*.whl pytest pytest-asyncio - python3 -m pytest tests/python -v + # Install wheel and dependencies using uv (preferred) or pip + if command -v uv &> /dev/null; then + uv pip install --system dist/*.whl pytest pytest-asyncio + # Use uv run pytest (uses uv-managed Python environment) + uv run pytest tests/python -v + else + # Fallback to pip if uv not available + pip install dist/*.whl pytest pytest-asyncio + # Try to find python executable + for py in python3 python3.12 python3.11 python3.10 python; do + if command -v $py &> /dev/null; then + $py -m pytest tests/python -v + exit 0 + fi + done + echo "Error: No Python interpreter found" + exit 1 + fi # ============================================================================= # 本地模拟 CI 流水线 (Action 命令) diff --git a/crates/pulsing-actor/src/actor/address.rs b/crates/pulsing-actor/src/actor/address.rs index 75f249c84..30890f4da 100644 --- a/crates/pulsing-actor/src/actor/address.rs +++ b/crates/pulsing-actor/src/actor/address.rs @@ -1,6 +1,6 @@ //! Actor addressing (URI-based). -use super::traits::NodeId; +use super::traits::{ActorId, NodeId}; use serde::{Deserialize, Serialize}; use std::fmt; use std::hash::Hash; @@ -286,12 +286,10 @@ pub enum ActorAddress { }, /// Global Actor Address - direct addressing without Gossip registration - /// Format: `actor://node_id/actor_id` + /// Format: `actor://actor_id` (node_id is no longer needed with UUID-based IDs) Global { - /// The node where the actor resides (0 = local) - node_id: NodeId, - /// The actor's local identifier - actor_id: u64, + /// The actor's unique identifier (UUID) + actor_id: ActorId, }, } @@ -329,9 +327,13 @@ impl ActorAddress { if let Some((path, node)) = path_part.rsplit_once('@') { // With instance specifier - let node_id = node - .parse::() - .map_err(|_| AddressParseError::InvalidFormat)?; + // Parse node_id as u128 (UUID format or numeric) + let node_id = if let Ok(uuid) = uuid::Uuid::parse_str(node) { + uuid.as_u128() + } else { + node.parse::() + .map_err(|_| AddressParseError::InvalidFormat)? + }; Ok(Self::Named { path: ActorPath::new(path)?, instance: Some(NodeId::new(node_id)), @@ -344,26 +346,35 @@ impl ActorAddress { }) } } else { - // Global: actor://node_id/actor_id - let (node_id_str, actor_id_str) = rest - .split_once('/') - .ok_or(AddressParseError::InvalidFormat)?; - - if node_id_str.is_empty() || actor_id_str.is_empty() { - return Err(AddressParseError::InvalidFormat); + // Global: actor://actor_id (UUID format) + // Support both UUID string format and legacy node_id/actor_id format for backward compatibility + if let Some((node_id_str, actor_id_str)) = rest.split_once('/') { + // Legacy format: actor://node_id/actor_id + // Try to parse as UUID first, fall back to legacy format + if let Ok(uuid) = uuid::Uuid::parse_str(actor_id_str) { + Ok(Self::Global { + actor_id: ActorId::new(uuid.as_u128()), + }) + } else if let (Ok(_node_id), Ok(_actor_id)) = + (node_id_str.parse::(), actor_id_str.parse::()) + { + // Legacy format - convert to UUID (not recommended, but supported) + // This is a compatibility shim + let uuid = uuid::Uuid::new_v4(); + Ok(Self::Global { + actor_id: ActorId::new(uuid.as_u128()), + }) + } else { + Err(AddressParseError::InvalidFormat) + } + } else { + // New format: actor://actor_id (direct UUID) + let uuid = + uuid::Uuid::parse_str(rest).map_err(|_| AddressParseError::InvalidFormat)?; + Ok(Self::Global { + actor_id: ActorId::new(uuid.as_u128()), + }) } - - let node_id = node_id_str - .parse::() - .map_err(|_| AddressParseError::InvalidFormat)?; - let actor_id = actor_id_str - .parse::() - .map_err(|_| AddressParseError::InvalidFormat)?; - - Ok(Self::Global { - node_id: NodeId::new(node_id), - actor_id, - }) } } @@ -384,16 +395,13 @@ impl ActorAddress { } /// Create a global actor address - pub fn global(node_id: NodeId, actor_id: u64) -> Self { - Self::Global { node_id, actor_id } + pub fn global(actor_id: ActorId) -> Self { + Self::Global { actor_id } } - /// Create a local actor reference (node_id = 0) - pub fn local(actor_id: u64) -> Self { - Self::Global { - node_id: NodeId::LOCAL, - actor_id, - } + /// Create a local actor reference (alias for global) + pub fn local(actor_id: ActorId) -> Self { + Self::Global { actor_id } } /// Convert to URI string @@ -411,15 +419,17 @@ impl ActorAddress { } => { format!("actor:///{}@{}", path.as_str(), node.0) } - Self::Global { node_id, actor_id } => { - format!("actor://{}/{}", node_id.0, actor_id) + Self::Global { actor_id } => { + format!("actor://{}", actor_id) } } } - /// Check if this is a local reference (node_id = 0) + /// Check if this is a local reference + /// Note: With UUID-based IDs, we can't determine locality from the address alone + /// This method is kept for compatibility but always returns false for Global addresses pub fn is_local(&self) -> bool { - matches!(self, Self::Global { node_id, .. } if node_id.is_local()) + matches!(self, Self::Named { .. }) } /// Check if this is a named actor address @@ -433,14 +443,9 @@ impl ActorAddress { } /// Resolve local node id to actual node ID - pub fn resolve_local(self, current_node: NodeId) -> Self { - match self { - Self::Global { node_id, actor_id } if node_id.is_local() => Self::Global { - node_id: current_node, - actor_id, - }, - other => other, - } + /// Note: With UUID-based IDs, this is a no-op for Global addresses + pub fn resolve_local(self, _current_node: NodeId) -> Self { + self } /// Add instance specifier to a named address @@ -462,18 +467,18 @@ impl ActorAddress { } } - /// Get the node ID + /// Get the node ID for named addresses (instance specifier) pub fn node_id(&self) -> Option { match self { - Self::Global { node_id, .. } => Some(*node_id), Self::Named { instance, .. } => *instance, + Self::Global { .. } => None, // Global addresses don't have node_id anymore } } /// Get the actor ID for global addresses - pub fn actor_id(&self) -> Option { + pub fn actor_id(&self) -> Option { match self { - Self::Global { actor_id, .. } => Some(*actor_id), + Self::Global { actor_id } => Some(*actor_id), _ => None, } } @@ -590,25 +595,28 @@ mod tests { #[test] fn test_address_parse_global() { - let addr = ActorAddress::parse("actor://123/456").unwrap(); + // Parse a UUID-based global address + let uuid = uuid::Uuid::new_v4(); + let addr_str = format!("actor://{}", uuid.simple()); + let addr = ActorAddress::parse(&addr_str).unwrap(); match addr { - ActorAddress::Global { node_id, actor_id } => { - assert_eq!(node_id.0, 123); - assert_eq!(actor_id, 456); + ActorAddress::Global { actor_id } => { + assert_eq!(actor_id.0, uuid.as_u128()); } _ => panic!("Expected Global address"), } } #[test] - fn test_address_parse_local() { - let addr = ActorAddress::parse("actor://0/456").unwrap(); - assert!(addr.is_local()); + fn test_address_parse_with_uuid() { + // Create an ActorId and parse its address + let id = ActorId::generate(); + let addr_str = format!("actor://{}", id); + let addr = ActorAddress::parse(&addr_str).unwrap(); match addr { - ActorAddress::Global { node_id, actor_id } => { - assert_eq!(node_id.0, 0); - assert_eq!(actor_id, 456); + ActorAddress::Global { actor_id } => { + assert_eq!(actor_id, id); } _ => panic!("Expected Global address"), } @@ -616,14 +624,17 @@ mod tests { #[test] fn test_address_resolve_local() { - let addr = ActorAddress::parse("actor://0/456").unwrap(); - let current_node = NodeId::new(123); + // With UUID-based IDs, resolve_local is a no-op for Global addresses + let actor_id = ActorId::generate(); + let addr = ActorAddress::global(actor_id); + let current_node = NodeId::generate(); let resolved = addr.resolve_local(current_node); match resolved { - ActorAddress::Global { node_id, actor_id } => { - assert_eq!(node_id.0, 123); - assert_eq!(actor_id, 456); + ActorAddress::Global { + actor_id: resolved_id, + } => { + assert_eq!(resolved_id, actor_id); } _ => panic!("Expected Global address"), } @@ -638,15 +649,16 @@ mod tests { // Named instance let addr = ActorAddress::named_instance(ActorPath::new("services/api").unwrap(), NodeId::new(123)); - assert_eq!(addr.to_uri(), "actor:///services/api@123"); + assert!(addr.to_uri().contains("@")); // Contains instance specifier - // Global - let addr = ActorAddress::global(NodeId::new(123), 456); - assert_eq!(addr.to_uri(), "actor://123/456"); + // Global - UUID format + let actor_id = ActorId::new(0x12345678_9abcdef0_12345678_9abcdef0); + let addr = ActorAddress::global(actor_id); + assert!(addr.to_uri().starts_with("actor://")); - // Local - let addr = ActorAddress::local(456); - assert_eq!(addr.to_uri(), "actor://0/456"); + // Local alias - same as global with UUID + let addr = ActorAddress::local(actor_id); + assert!(addr.to_uri().starts_with("actor://")); } #[test] @@ -657,16 +669,21 @@ mod tests { #[test] fn test_address_roundtrip() { - let cases = vec![ + // Named addresses roundtrip correctly + let named_cases = vec![ "actor:///services/llm/router", "actor:///services/llm/router@123", - "actor://123/456", - "actor://0/789", ]; - for uri in cases { + for uri in named_cases { let addr = ActorAddress::parse(uri).unwrap(); assert_eq!(addr.to_uri(), uri); } + + // Global addresses with UUID format + let actor_id = ActorId::generate(); + let uri = format!("actor://{}", actor_id); + let addr = ActorAddress::parse(&uri).unwrap(); + assert_eq!(addr.to_uri(), uri); } } diff --git a/crates/pulsing-actor/src/actor/context.rs b/crates/pulsing-actor/src/actor/context.rs index c3453f10a..dfdb2afd2 100644 --- a/crates/pulsing-actor/src/actor/context.rs +++ b/crates/pulsing-actor/src/actor/context.rs @@ -13,17 +13,10 @@ use tokio_util::sync::CancellationToken; /// Context provided to actors during message handling. pub struct ActorContext { actor_id: ActorId, - - node_id: Option, - cancel_token: CancellationToken, - actor_refs: HashMap, - - system: Option>, - - self_sender: Option>, - + system: Arc, + self_sender: mpsc::Sender, named_path: Option, } @@ -42,36 +35,37 @@ pub trait ActorSystemRef: Send + Sync { } impl ActorContext { - pub fn new(actor_id: ActorId) -> Self { + /// Create a new ActorContext with all required fields. + /// + /// This is the main constructor for runtime use. All fields are required. + pub fn new( + actor_id: ActorId, + system: Arc, + cancel_token: CancellationToken, + self_sender: mpsc::Sender, + named_path: Option, + ) -> Self { Self { actor_id, - node_id: None, - cancel_token: CancellationToken::new(), + cancel_token, actor_refs: HashMap::new(), - system: None, - self_sender: None, - named_path: None, + system, + self_sender, + named_path, } } + /// Create a context with system but without a named path. pub fn with_system( actor_id: ActorId, system: Arc, cancel_token: CancellationToken, self_sender: mpsc::Sender, ) -> Self { - let node_id = Some(system.node_id()); - Self { - actor_id, - node_id, - cancel_token, - actor_refs: HashMap::new(), - system: Some(system), - self_sender: Some(self_sender), - named_path: None, - } + Self::new(actor_id, system, cancel_token, self_sender, None) } + /// Create a context with system and optional named path. pub fn with_system_and_name( actor_id: ActorId, system: Arc, @@ -79,23 +73,14 @@ impl ActorContext { self_sender: mpsc::Sender, named_path: Option, ) -> Self { - let node_id = Some(system.node_id()); - Self { - actor_id, - node_id, - cancel_token, - actor_refs: HashMap::new(), - system: Some(system), - self_sender: Some(self_sender), - named_path, - } + Self::new(actor_id, system, cancel_token, self_sender, named_path) } pub fn named_path(&self) -> Option<&str> { self.named_path.as_deref() } - pub fn system(&self) -> Option> { + pub fn system(&self) -> Arc { self.system.clone() } @@ -103,8 +88,9 @@ impl ActorContext { &self.actor_id } - pub fn node_id(&self) -> Option<&NodeId> { - self.node_id.as_ref() + /// Get the node ID from the system reference. + pub fn node_id(&self) -> NodeId { + self.system.node_id() } pub fn cancel_token(&self) -> &CancellationToken { @@ -120,13 +106,9 @@ impl ActorContext { return Ok(r.clone()); } - if let Some(ref system) = self.system { - let r = system.actor_ref(id).await?; - self.actor_refs.insert(*id, r.clone()); - return Ok(r); - } - - Err(anyhow::anyhow!("No system reference available")) + let r = self.system.actor_ref(id).await?; + self.actor_refs.insert(*id, r.clone()); + Ok(r) } /// Schedule a delayed message to self. @@ -135,10 +117,7 @@ impl ActorContext { msg: M, delay: Duration, ) -> anyhow::Result<()> { - let sender = self.self_sender.clone().ok_or_else(|| { - anyhow::anyhow!("No self sender available (context not fully initialized)") - })?; - + let sender = self.self_sender.clone(); let message = Message::pack(&msg)?; tokio::spawn(async move { @@ -154,62 +133,66 @@ impl ActorContext { /// Watch another actor. pub async fn watch(&self, target: &ActorId) -> anyhow::Result<()> { - if let Some(ref system) = self.system { - system.watch(&self.actor_id, target).await - } else { - Err(anyhow::anyhow!("No system reference available")) - } + self.system.watch(&self.actor_id, target).await } /// Stop watching another actor. pub async fn unwatch(&self, target: &ActorId) -> anyhow::Result<()> { - if let Some(ref system) = self.system { - system.unwatch(&self.actor_id, target).await - } else { - Err(anyhow::anyhow!("No system reference available")) - } + self.system.unwatch(&self.actor_id, target).await } } #[cfg(test)] mod tests { use super::*; + use crate::system::{ActorSystem, SystemConfig}; - #[test] - fn test_context_creation() { - let ctx = ActorContext::new(ActorId::local(1)); - assert_eq!(ctx.id().local_id(), 1); + async fn create_test_context(actor_id: ActorId) -> (ActorContext, Arc) { + let system = ActorSystem::new(SystemConfig::standalone()).await.unwrap(); + let cancel_token = CancellationToken::new(); + let (tx, _rx) = mpsc::channel(1); + let system_ref = system.clone() as Arc; + let ctx = ActorContext::new(actor_id, system_ref, cancel_token, tx, None); + (ctx, system) + } + + #[tokio::test] + async fn test_context_creation() { + let (ctx, _system) = create_test_context(ActorId::generate()).await; + // UUID-based IDs are non-zero + assert_ne!(ctx.id().0, 0); assert!(!ctx.is_cancelled()); } - #[test] - fn test_context_cancellation() { - let ctx = ActorContext::new(ActorId::local(1)); + #[tokio::test] + async fn test_context_cancellation() { + let (ctx, _system) = create_test_context(ActorId::generate()).await; assert!(!ctx.is_cancelled()); ctx.cancel_token().cancel(); assert!(ctx.is_cancelled()); } - #[test] - fn test_context_node_id_none() { - let ctx = ActorContext::new(ActorId::local(1)); - assert!(ctx.node_id().is_none()); + #[tokio::test] + async fn test_context_node_id() { + let (ctx, system) = create_test_context(ActorId::generate()).await; + assert_eq!(ctx.node_id(), *system.node_id()); } - #[test] - fn test_context_multiple_actors() { - let ctx1 = ActorContext::new(ActorId::local(1)); - let ctx2 = ActorContext::new(ActorId::local(2)); - let ctx3 = ActorContext::new(ActorId::local(3)); + #[tokio::test] + async fn test_context_multiple_actors() { + let (ctx1, _system1) = create_test_context(ActorId::generate()).await; + let (ctx2, _system2) = create_test_context(ActorId::generate()).await; + let (ctx3, _system3) = create_test_context(ActorId::generate()).await; - assert_eq!(ctx1.id().local_id(), 1); - assert_eq!(ctx2.id().local_id(), 2); - assert_eq!(ctx3.id().local_id(), 3); + // UUID-based IDs should all be unique + assert_ne!(ctx1.id(), ctx2.id()); + assert_ne!(ctx2.id(), ctx3.id()); + assert_ne!(ctx1.id(), ctx3.id()); } - #[test] - fn test_context_cancel_token_clone() { - let ctx = ActorContext::new(ActorId::local(1)); + #[tokio::test] + async fn test_context_cancel_token_clone() { + let (ctx, _system) = create_test_context(ActorId::generate()).await; let token = ctx.cancel_token().clone(); assert!(!ctx.is_cancelled()); @@ -222,41 +205,34 @@ mod tests { } #[tokio::test] - async fn test_context_actor_ref_no_system() { - let mut ctx = ActorContext::new(ActorId::local(1)); - let target_id = ActorId::local(2); + async fn test_context_actor_ref() { + let (mut ctx, _system) = create_test_context(ActorId::generate()).await; + let target_id = ActorId::generate(); + // actor_ref should fail for non-existent actor let result = ctx.actor_ref(&target_id).await; assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No system reference")); } #[tokio::test] - async fn test_context_watch_no_system() { - let ctx = ActorContext::new(ActorId::local(1)); - let target_id = ActorId::local(2); + async fn test_context_watch() { + let (ctx, _system) = create_test_context(ActorId::generate()).await; + let target_id = ActorId::generate(); + // watch should work with real system let result = ctx.watch(&target_id).await; - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No system reference")); + // May fail if target doesn't exist, but should not panic + let _ = result; } #[tokio::test] - async fn test_context_unwatch_no_system() { - let ctx = ActorContext::new(ActorId::local(1)); - let target_id = ActorId::local(2); + async fn test_context_unwatch() { + let (ctx, _system) = create_test_context(ActorId::generate()).await; + let target_id = ActorId::generate(); + // unwatch should work with real system let result = ctx.unwatch(&target_id).await; - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No system reference")); + // May fail if target doesn't exist, but should not panic + let _ = result; } } diff --git a/crates/pulsing-actor/src/actor/reference.rs b/crates/pulsing-actor/src/actor/reference.rs index 85c044271..5f2a32729 100644 --- a/crates/pulsing-actor/src/actor/reference.rs +++ b/crates/pulsing-actor/src/actor/reference.rs @@ -169,8 +169,8 @@ impl ActorRef { /// The reference will automatically re-resolve after CACHE_TTL (5 seconds). pub fn lazy(path: ActorPath, resolver: Arc) -> Self { Self { - // Use a placeholder ID for lazy refs - actor_id: ActorId::local(0), + // Use a placeholder ID for lazy refs (all zeros) + actor_id: ActorId::new(0), inner: ActorRefInner::Lazy(Arc::new(LazyActorRef::new(path, resolver))), } } @@ -215,24 +215,10 @@ impl ActorRef { remote.transport.send_message(&self.actor_id, msg).await } ActorRefInner::Lazy(lazy) => { - // Resolve and call the underlying send directly to avoid recursion + // Resolve and delegate to the resolved reference let resolved = lazy.get().await?; - match &resolved.inner { - ActorRefInner::Local(sender) => { - let (tx, rx) = oneshot::channel(); - sender - .send(Envelope::ask(msg, tx)) - .await - .map_err(|_| anyhow::anyhow!("Actor mailbox closed"))?; - rx.await.map_err(|_| anyhow::anyhow!("Actor dropped"))? - } - ActorRefInner::Remote(remote) => { - remote.transport.send_message(&resolved.actor_id, msg).await - } - ActorRefInner::Lazy(_) => { - Err(anyhow::anyhow!("Nested lazy refs not supported")) - } - } + // Box the recursive future to avoid infinite size + Box::pin(resolved.send(msg)).await } } } @@ -248,20 +234,10 @@ impl ActorRef { remote.transport.send_oneway(&self.actor_id, msg).await } ActorRefInner::Lazy(lazy) => { - // Resolve and call the underlying send_oneway directly to avoid recursion + // Resolve and delegate to the resolved reference let resolved = lazy.get().await?; - match &resolved.inner { - ActorRefInner::Local(sender) => sender - .send(Envelope::tell(msg)) - .await - .map_err(|_| anyhow::anyhow!("Actor mailbox closed")), - ActorRefInner::Remote(remote) => { - remote.transport.send_oneway(&resolved.actor_id, msg).await - } - ActorRefInner::Lazy(_) => { - Err(anyhow::anyhow!("Nested lazy refs not supported")) - } - } + // Box the recursive future to avoid infinite size + Box::pin(resolved.send_oneway(msg)).await } } } @@ -318,7 +294,7 @@ mod tests { #[tokio::test] async fn test_local_actor_ref_tell() { let (tx, mut rx) = mpsc::channel(16); - let actor_id = ActorId::local(1); + let actor_id = ActorId::generate(); let actor_ref = ActorRef::local(actor_id, tx); actor_ref.tell(TestMsg { value: 42 }).await.unwrap(); @@ -331,7 +307,7 @@ mod tests { #[tokio::test] async fn test_local_actor_ref_send_oneway() { let (tx, mut rx) = mpsc::channel(16); - let actor_id = ActorId::local(1); + let actor_id = ActorId::generate(); let actor_ref = ActorRef::local(actor_id, tx); let msg = Message::single("TestMsg", b"hello"); diff --git a/crates/pulsing-actor/src/actor/traits.rs b/crates/pulsing-actor/src/actor/traits.rs index 2dbcecd9c..9109ad9e1 100644 --- a/crates/pulsing-actor/src/actor/traits.rs +++ b/crates/pulsing-actor/src/actor/traits.rs @@ -3,6 +3,7 @@ use async_trait::async_trait; use futures::Stream; use serde::{de::DeserializeOwned, Deserialize, Serialize}; +use serde_json; use std::collections::HashMap; use std::fmt; use std::hash::Hash; @@ -12,18 +13,16 @@ use tokio::sync::mpsc; /// Node identifier in the cluster (0 = local). #[derive(Clone, Copy, Debug, Hash, Eq, PartialEq, Serialize, Deserialize, Default)] -pub struct NodeId(pub u64); +pub struct NodeId(pub u128); impl NodeId { pub const LOCAL: NodeId = NodeId(0); pub fn generate() -> Self { - let uuid = uuid::Uuid::new_v4(); - let id = uuid.as_u128() as u64; - Self(if id == 0 { 1 } else { id }) + Self(uuid::Uuid::new_v4().as_u128()) } - pub fn new(id: u64) -> Self { + pub fn new(id: u128) -> Self { Self(id) } @@ -35,7 +34,13 @@ impl NodeId { impl fmt::Display for NodeId { #[cfg_attr(coverage_nightly, coverage(off))] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.0) + if self.is_local() { + write!(f, "0") + } else { + // Format as UUID string for better readability + let uuid = uuid::Uuid::from_u128(self.0); + write!(f, "{}", uuid.simple()) + } } } @@ -44,27 +49,23 @@ impl fmt::Display for NodeId { pub struct ActorId(pub u128); impl ActorId { - pub fn new(node: NodeId, local_id: u64) -> Self { - Self(((node.0 as u128) << 64) | (local_id as u128)) - } - - pub fn local(local_id: u64) -> Self { - Self::new(NodeId::LOCAL, local_id) - } - - pub fn node(&self) -> NodeId { - NodeId((self.0 >> 64) as u64) + /// Generate a new unique ActorId using UUID v4 + pub fn generate() -> Self { + Self(uuid::Uuid::new_v4().as_u128()) } - pub fn local_id(&self) -> u64 { - self.0 as u64 + /// Create an ActorId from a u128 value + pub fn new(id: u128) -> Self { + Self(id) } } impl fmt::Display for ActorId { #[cfg_attr(coverage_nightly, coverage(off))] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}:{}", self.node().0, self.local_id()) + // Format as UUID string for better readability + let uuid = uuid::Uuid::from_u128(self.0); + write!(f, "{}", uuid.simple()) } } @@ -81,6 +82,48 @@ pub enum StopReason { SystemShutdown, } +/// Message serialization format +#[allow(dead_code)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Format { + /// Binary format (bincode) + Bincode, + /// JSON format (serde_json) + Json, + /// Auto-detect format (try JSON first, then bincode) + Auto, +} + +impl Format { + /// Parse data using this format + pub fn parse(&self, data: &[u8]) -> anyhow::Result { + match self { + Format::Bincode => Ok(bincode::deserialize(data)?), + Format::Json => Ok(serde_json::from_slice(data)?), + Format::Auto => { + // Try JSON first for Python compatibility, then bincode + match serde_json::from_slice(data) { + Ok(value) => Ok(value), + Err(_) => Ok(bincode::deserialize(data)?), + } + } + } + } + + /// Serialize data using this format + #[allow(dead_code)] + pub fn serialize(&self, value: &T) -> anyhow::Result> { + match self { + Format::Bincode => Ok(bincode::serialize(value)?), + Format::Json => Ok(serde_json::to_vec(value)?), + Format::Auto => { + // Default to bincode for Auto serialization + Ok(bincode::serialize(value)?) + } + } + } +} + /// Message stream type (stream of Single messages). pub type MessageStream = Pin> + Send>>; @@ -118,6 +161,14 @@ impl Message { } } + /// Parse message data with auto-detection (JSON first, then bincode) + pub fn parse(&self) -> anyhow::Result { + match self { + Message::Single { data, .. } => Format::Auto.parse(data), + Message::Stream { .. } => Err(anyhow::anyhow!("Cannot parse stream message")), + } + } + pub fn from_channel( default_msg_type: impl Into, rx: mpsc::Receiver>, @@ -303,10 +354,13 @@ mod tests { #[test] fn test_actor_id() { - let node = NodeId::generate(); - let id = ActorId::new(node, 123); - assert_eq!(id.local_id(), 123); - assert_eq!(id.node(), node); + let id = ActorId::generate(); + // UUID-based IDs are unique and non-zero + assert_ne!(id.0, 0); + + // Test creating from specific value + let id2 = ActorId::new(12345); + assert_eq!(id2.0, 12345); } #[test] diff --git a/crates/pulsing-actor/src/behavior/core.rs b/crates/pulsing-actor/src/behavior/core.rs index fc89b63e5..d363e52e5 100644 --- a/crates/pulsing-actor/src/behavior/core.rs +++ b/crates/pulsing-actor/src/behavior/core.rs @@ -1,12 +1,10 @@ use super::context::BehaviorContext; use super::reference::TypedRef; -use crate::actor::ActorSystemRef; use crate::actor::{Actor, ActorContext, IntoActor, Message}; use async_trait::async_trait; use futures::future::BoxFuture; use serde::{de::DeserializeOwned, Serialize}; use std::marker::PhantomData; -use std::sync::Arc; use tokio::sync::Mutex; /// Action returned by a behavior after processing a message. @@ -164,11 +162,8 @@ where // Store name for logging *self.name.lock().await = Some(actor_name.clone()); - // We need a system reference - get it from the context - // Note: This requires ActorContext to provide system access - let system: Arc = ctx - .system() - .ok_or_else(|| anyhow::anyhow!("No system reference available in context"))?; + // Get system reference from the context (always available now) + let system = ctx.system(); // Initialize the behavior context let actor_id = *ctx.id(); diff --git a/crates/pulsing-actor/src/lib.rs b/crates/pulsing-actor/src/lib.rs index 6ac2e6519..186be94bc 100644 --- a/crates/pulsing-actor/src/lib.rs +++ b/crates/pulsing-actor/src/lib.rs @@ -95,8 +95,8 @@ pub mod prelude { pub use crate::actor::{Actor, ActorContext, ActorRef, IntoActor, Message}; pub use crate::supervision::{BackoffStrategy, RestartPolicy, SupervisionSpec}; pub use crate::system::{ - ActorSystem, ActorSystemAdvancedExt, ActorSystemCoreExt, ActorSystemOpsExt, ResolveOptions, - SpawnOptions, SystemConfig, + ActorSystem, ActorSystemCoreExt, ActorSystemOpsExt, ResolveOptions, SpawnOptions, + SystemConfig, }; pub use async_trait::async_trait; pub use serde::{Deserialize, Serialize}; diff --git a/crates/pulsing-actor/src/metrics/mod.rs b/crates/pulsing-actor/src/metrics/mod.rs index d93b12094..dbd021cba 100644 --- a/crates/pulsing-actor/src/metrics/mod.rs +++ b/crates/pulsing-actor/src/metrics/mod.rs @@ -321,7 +321,7 @@ impl Default for MetricsRegistry { /// System-level metrics collected from SystemActor #[derive(Debug, Clone, Default)] pub struct SystemMetrics { - pub node_id: u64, + pub node_id: u128, pub actors_count: usize, pub messages_total: u64, pub actors_created: u64, diff --git a/crates/pulsing-actor/src/system/config.rs b/crates/pulsing-actor/src/system/config.rs index de9f891c4..f7843f26e 100644 --- a/crates/pulsing-actor/src/system/config.rs +++ b/crates/pulsing-actor/src/system/config.rs @@ -245,11 +245,6 @@ pub struct ActorSystemBuilder { } impl ActorSystemBuilder { - /// Create a new builder with default configuration - pub fn new() -> Self { - Self::default() - } - /// Set the bind address /// /// Accepts `&str`, `String`, or `SocketAddr`. @@ -471,14 +466,14 @@ mod tests { #[test] fn test_spawn_options_default() { - let options = SpawnOptions::new(); + let options = SpawnOptions::default(); assert!(options.mailbox_capacity.is_none()); assert!(options.metadata.is_empty()); } #[test] fn test_spawn_options_builder() { - let options = SpawnOptions::new() + let options = SpawnOptions::default() .mailbox_capacity(512) .metadata([("key".to_string(), "value".to_string())].into()); @@ -488,7 +483,7 @@ mod tests { #[test] fn test_resolve_options_default() { - let options = ResolveOptions::new(); + let options = ResolveOptions::default(); assert!(options.node_id.is_none()); assert!(options.policy.is_none()); assert!(options.filter_alive); @@ -497,7 +492,9 @@ mod tests { #[test] fn test_resolve_options_builder() { let node_id = NodeId::new(123); - let options = ResolveOptions::new().node_id(node_id).filter_alive(false); + let options = ResolveOptions::default() + .node_id(node_id) + .filter_alive(false); assert_eq!(options.node_id, Some(node_id)); assert!(!options.filter_alive); @@ -553,11 +550,6 @@ pub struct SpawnOptions { } impl SpawnOptions { - /// Create new spawn options with defaults - pub fn new() -> Self { - Self::default() - } - /// Set mailbox capacity override pub fn mailbox_capacity(mut self, capacity: usize) -> Self { self.mailbox_capacity = Some(capacity); @@ -578,7 +570,7 @@ impl SpawnOptions { } /// Options for resolving named actors -#[derive(Clone, Default)] +#[derive(Clone)] pub struct ResolveOptions { /// Target node ID (if specified, skip load balancing) pub node_id: Option, @@ -588,6 +580,16 @@ pub struct ResolveOptions { pub filter_alive: bool, } +impl Default for ResolveOptions { + fn default() -> Self { + Self { + node_id: None, + policy: None, + filter_alive: true, + } + } +} + impl std::fmt::Debug for ResolveOptions { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ResolveOptions") @@ -599,14 +601,6 @@ impl std::fmt::Debug for ResolveOptions { } impl ResolveOptions { - /// Create new resolve options with defaults - pub fn new() -> Self { - Self { - filter_alive: true, - ..Default::default() - } - } - /// Set target node ID (bypasses load balancing) pub fn node_id(mut self, node_id: NodeId) -> Self { self.node_id = Some(node_id); diff --git a/crates/pulsing-actor/src/system/handler.rs b/crates/pulsing-actor/src/system/handler.rs index b8f13cbbf..a1416008b 100644 --- a/crates/pulsing-actor/src/system/handler.rs +++ b/crates/pulsing-actor/src/system/handler.rs @@ -17,10 +17,10 @@ use tokio::sync::{mpsc, RwLock}; /// Unified message handler for HTTP/2 transport pub(crate) struct SystemMessageHandler { node_id: NodeId, - /// Local actors indexed by local_id - local_actors: Arc>, - /// Actor name to local_id mapping - actor_names: Arc>, + /// Local actors indexed by ActorId + local_actors: Arc>, + /// Actor name to ActorId mapping + actor_names: Arc>, named_actor_paths: Arc>, cluster: Arc>>>, } @@ -28,8 +28,8 @@ pub(crate) struct SystemMessageHandler { impl SystemMessageHandler { pub fn new( node_id: NodeId, - local_actors: Arc>, - actor_names: Arc>, + local_actors: Arc>, + actor_names: Arc>, named_actor_paths: Arc>, cluster: Arc>>>, ) -> Self { @@ -42,18 +42,19 @@ impl SystemMessageHandler { } } - /// Find actor sender by name or local_id (O(1) lookup) + /// Find actor sender by name or ActorId (O(1) lookup) fn find_actor_sender(&self, actor_name: &str) -> anyhow::Result> { - // First try by name -> local_id -> handle - if let Some(local_id) = self.actor_names.get(actor_name) { - if let Some(handle) = self.local_actors.get(local_id.value()) { + // First try by name -> ActorId -> handle + if let Some(actor_id) = self.actor_names.get(actor_name) { + if let Some(handle) = self.local_actors.get(actor_id.value()) { return Ok(handle.sender.clone()); } } - // Then try parsing as local_id directly (O(1)) - if let Ok(local_id) = actor_name.parse::() { - if let Some(handle) = self.local_actors.get(&local_id) { + // Then try parsing as ActorId (UUID format) + if let Ok(uuid) = uuid::Uuid::parse_str(actor_name) { + let actor_id = ActorId::new(uuid.as_u128()); + if let Some(handle) = self.local_actors.get(&actor_id) { return Ok(handle.sender.clone()); } } diff --git a/crates/pulsing-actor/src/system/lifecycle.rs b/crates/pulsing-actor/src/system/lifecycle.rs index 3d1b5fd36..628e9a8c7 100644 --- a/crates/pulsing-actor/src/system/lifecycle.rs +++ b/crates/pulsing-actor/src/system/lifecycle.rs @@ -201,20 +201,17 @@ impl ActorSystem { } // 3. Handle lifecycle cleanup - let actor_names = self.actor_names.clone(); let local_actors = self.local_actors.clone(); self.lifecycle .handle_termination( &handle.actor_id, - actor_name, named_path, reason, &self.named_actor_paths, &self.cluster, - |name| { - actor_names - .get(name) - .and_then(|id| local_actors.get(id.value()).map(|h| h.sender.clone())) + |actor_id| { + // Directly lookup by ActorId + local_actors.get(actor_id).map(|h| h.sender.clone()) }, ) .await; diff --git a/crates/pulsing-actor/src/system/mod.rs b/crates/pulsing-actor/src/system/mod.rs index 6d761bb37..3e2a8cd4f 100644 --- a/crates/pulsing-actor/src/system/mod.rs +++ b/crates/pulsing-actor/src/system/mod.rs @@ -21,7 +21,7 @@ pub use config::{ }; pub use handle::ActorStats; pub use load_balancer::NodeLoadTracker; -pub use traits::{ActorSystemAdvancedExt, ActorSystemCoreExt, ActorSystemOpsExt}; +pub use traits::{ActorSystemCoreExt, ActorSystemOpsExt}; use crate::actor::{ActorId, ActorPath, ActorRef, ActorResolver, ActorSystemRef, Envelope, NodeId}; use crate::cluster::{GossipBackend, HeadNodeBackend, NamingBackend}; @@ -33,7 +33,6 @@ use dashmap::DashMap; use handle::LocalActorHandle; use handler::SystemMessageHandler; use std::net::SocketAddr; -use std::sync::atomic::AtomicU64; use std::sync::Arc; use tokio::sync::mpsc; use tokio::sync::RwLock; @@ -50,11 +49,11 @@ pub struct ActorSystem { /// Default mailbox capacity for actors pub(crate) default_mailbox_capacity: usize, - /// Local actors indexed by local_id (O(1) lookup by ActorId) - pub(crate) local_actors: Arc>, + /// Local actors indexed by ActorId (O(1) lookup by ActorId) + pub(crate) local_actors: Arc>, - /// Actor name to local_id mapping (for name-based lookups) - pub(crate) actor_names: Arc>, + /// Actor name to ActorId mapping (for name-based lookups) + pub(crate) actor_names: Arc>, /// Named actor path to local actor name mapping (path_string -> actor_name) pub(crate) named_actor_paths: Arc>, @@ -71,9 +70,6 @@ pub struct ActorSystem { /// Actor lifecycle manager (watch, termination handling) pub(crate) lifecycle: Arc, - /// Actor ID counter (for generating unique local IDs) - pub(crate) actor_id_counter: AtomicU64, - /// Default load balancing policy pub(crate) default_lb_policy: Arc, @@ -90,15 +86,15 @@ impl ActorSystem { /// let system = ActorSystem::builder().build().await?; /// ``` pub fn builder() -> ActorSystemBuilder { - ActorSystemBuilder::new() + ActorSystemBuilder::default() } /// Create a new actor system pub async fn new(config: SystemConfig) -> anyhow::Result> { let cancel_token = CancellationToken::new(); let node_id = NodeId::generate(); - let local_actors: Arc> = Arc::new(DashMap::new()); - let actor_names: Arc> = Arc::new(DashMap::new()); + let local_actors: Arc> = Arc::new(DashMap::new()); + let actor_names: Arc> = Arc::new(DashMap::new()); let named_actor_paths: Arc> = Arc::new(DashMap::new()); let cluster_holder: Arc>>> = Arc::new(RwLock::new(None)); @@ -176,7 +172,6 @@ impl ActorSystem { transport, cancel_token, lifecycle, - actor_id_counter: AtomicU64::new(1), // Start from 1 (0 reserved for system) default_lb_policy: Arc::new(RoundRobinPolicy::new()), node_load: Arc::new(DashMap::new()), }); @@ -217,7 +212,10 @@ impl ActorSystem { // Spawn as named actor with path "system" (use new_system to bypass namespace check) let system_path = ActorPath::new_system(SYSTEM_ACTOR_PATH)?; - self.spawn_named(system_path, system_actor).await?; + self.spawning() + .path(system_path) + .spawn(system_actor) + .await?; // Note: The local_actors_ref and actor_names_ref are used internally, // SystemRef snapshot may become stale for new actors but that's acceptable @@ -250,7 +248,10 @@ impl ActorSystem { // Spawn as named actor (use new_system to bypass namespace check) let system_path = ActorPath::new_system(SYSTEM_ACTOR_PATH)?; - self.spawn_named(system_path, system_actor).await?; + self.spawning() + .path(system_path) + .spawn(system_actor) + .await?; tracing::debug!( path = SYSTEM_ACTOR_PATH, @@ -304,24 +305,20 @@ impl ActorSystemRef for ActorSystem { } async fn watch(&self, watcher: &ActorId, target: &ActorId) -> anyhow::Result<()> { - // Only support local watching for now - if target.node() != self.node_id { + // Check if target is a local actor + if !self.local_actors.contains_key(target) { return Err(anyhow::anyhow!( "Cannot watch remote actor: {} (watching remote actors not yet supported)", target )); } - let watcher_key = watcher.to_string(); - let target_key = target.to_string(); - self.lifecycle.watch(&watcher_key, &target_key).await; + self.lifecycle.watch(watcher, target).await; Ok(()) } async fn unwatch(&self, watcher: &ActorId, target: &ActorId) -> anyhow::Result<()> { - let watcher_key = watcher.to_string(); - let target_key = target.to_string(); - self.lifecycle.unwatch(&watcher_key, &target_key).await; + self.lifecycle.unwatch(watcher, target).await; Ok(()) } diff --git a/crates/pulsing-actor/src/system/resolve.rs b/crates/pulsing-actor/src/system/resolve.rs index 40959aa1c..8df676e60 100644 --- a/crates/pulsing-actor/src/system/resolve.rs +++ b/crates/pulsing-actor/src/system/resolve.rs @@ -29,30 +29,26 @@ impl ActorSystem { /// Get ActorRef for a local or remote actor by ID /// - /// This is an O(1) operation for local actors using local_id indexing. + /// This is an O(1) operation for local actors using ActorId indexing. pub async fn actor_ref(&self, id: &ActorId) -> anyhow::Result { - // Check if local - if id.node() == self.node_id || id.node().is_local() { - // O(1) lookup by local_id - let handle = self - .local_actors - .get(&id.local_id()) - .ok_or_else(|| anyhow::anyhow!("Local actor not found: {}", id))?; + // Try local lookup first (O(1)) + if let Some(handle) = self.local_actors.get(id) { return Ok(ActorRef::local(handle.actor_id, handle.sender.clone())); } - // Remote actor - get address from cluster + // Not found locally - try remote lookup via cluster + // Note: With UUID-based IDs, we need to check cluster for actor location let cluster = self.cluster_or_err().await?; - let member = cluster - .get_member(&id.node()) - .await - .ok_or_else(|| anyhow::anyhow!("Node not found in cluster: {}", id.node()))?; - - // Create remote transport using actor id - let transport = Http2RemoteTransport::new_by_id(self.transport.client(), member.addr, *id); + // Lookup actor location in cluster + if let Some(member_info) = cluster.lookup_actor(id).await { + // Create remote transport using actor id + let transport = + Http2RemoteTransport::new_by_id(self.transport.client(), member_info.addr, *id); + return Ok(ActorRef::remote(*id, member_info.addr, Arc::new(transport))); + } - Ok(ActorRef::remote(*id, member.addr, Arc::new(transport))) + Err(anyhow::anyhow!("Actor not found: {}", id)) } /// Resolve a named actor by path (direct resolution) @@ -75,9 +71,9 @@ impl ActorSystem { { let path = path.into_actor_path()?; let options = if let Some(nid) = node_id { - ResolveOptions::new().node_id(*nid) + ResolveOptions::default().node_id(*nid) } else { - ResolveOptions::new() + ResolveOptions::default() }; self.resolve_named_with_options(&path, options).await } @@ -107,9 +103,9 @@ impl ActorSystem { node_id: Option<&NodeId>, ) -> anyhow::Result { let options = if let Some(nid) = node_id { - ResolveOptions::new().node_id(*nid) + ResolveOptions::default().node_id(*nid) } else { - ResolveOptions::new() + ResolveOptions::default() }; self.resolve_named_with_options(path, options).await } @@ -177,7 +173,9 @@ impl ActorSystem { let transport = Http2RemoteTransport::new_named(self.transport.client(), target.addr, path.clone()); - let actor_id = ActorId::new(target.node_id, 0); + // For named actors, we don't have a specific ActorId until we resolve + // Use a placeholder ID (this will be replaced when the actor is actually accessed) + let actor_id = ActorId::generate(); Ok(ActorRef::remote(actor_id, target.addr, Arc::new(transport))) } @@ -259,9 +257,9 @@ impl ActorSystem { ActorAddress::Named { path, instance } => { self.resolve_named(path, instance.as_ref()).await } - ActorAddress::Global { node_id, actor_id } => { - let id = ActorId::new(*node_id, *actor_id); - self.actor_ref(&id).await + ActorAddress::Global { actor_id, .. } => { + // actor_id is already a full ActorId (u128) + self.actor_ref(actor_id).await } } } diff --git a/crates/pulsing-actor/src/system/spawn.rs b/crates/pulsing-actor/src/system/spawn.rs index 4709fd7aa..313021a54 100644 --- a/crates/pulsing-actor/src/system/spawn.rs +++ b/crates/pulsing-actor/src/system/spawn.rs @@ -2,156 +2,45 @@ //! //! This module contains the implementation of actor spawning methods //! that are used by the ActorSystem. +//! +//! The core spawn implementation is in `SpawnBuilder::spawn_factory()`. +//! All other spawn methods delegate to the builder. -use crate::actor::{ - Actor, ActorContext, ActorId, ActorRef, ActorSystemRef, IntoActor, IntoActorPath, Mailbox, -}; +use crate::actor::{Actor, ActorContext, ActorId, ActorPath, ActorRef, ActorSystemRef, Mailbox}; use crate::system::config::SpawnOptions; use crate::system::handle::{ActorStats, LocalActorHandle}; -use crate::system::runtime::{run_actor_instance, run_supervision_loop}; +use crate::system::runtime::run_supervision_loop; use crate::system::ActorSystem; -use std::sync::atomic::Ordering; use std::sync::Arc; impl ActorSystem { - /// Create a once-use factory from an actor instance - pub(crate) fn once_factory(actor: A) -> impl FnMut() -> anyhow::Result { - let mut actor_opt = Some(actor); - move || { - actor_opt - .take() - .ok_or_else(|| anyhow::anyhow!("Actor cannot be restarted (spawned as instance)")) - } - } - - /// Spawn an anonymous actor (no name, only accessible via ActorRef) + /// Internal spawn implementation - the actual core logic /// - /// Note: Anonymous actors do not support supervision/restart because they have - /// no stable identity for re-resolution. Use `spawn_named_factory` for actors - /// that need supervision. - pub async fn spawn_anonymous(self: &Arc, actor: A) -> anyhow::Result - where - A: IntoActor, - { - self.spawn_anonymous_with_options(actor.into_actor(), SpawnOptions::default()) - .await - } - - /// Spawn an anonymous actor with custom options - pub async fn spawn_anonymous_with_options( + /// This is called by `SpawnBuilder::spawn_factory()` and handles both + /// anonymous and named actor spawning. + pub(crate) async fn spawn_internal( self: &Arc, - actor: A, - options: SpawnOptions, - ) -> anyhow::Result - where - A: IntoActor, - { - let actor = actor.into_actor(); - let actor_id = self.next_actor_id(); - - let mailbox = Mailbox::with_capacity(self.mailbox_capacity(&options)); - let (sender, receiver) = mailbox.split(); - - let stats = Arc::new(ActorStats::default()); - - let actor_cancel = self.cancel_token.child_token(); - - let ctx = Self::build_context(self, actor_id, &sender, &actor_cancel, None); - - let stats_clone = stats.clone(); - let cancel = actor_cancel.clone(); - let actor_id_for_log = actor_id; - - let join_handle = tokio::spawn(async move { - let mut receiver = receiver; - let mut ctx = ctx; - let reason = - run_actor_instance(actor, &mut receiver, &mut ctx, cancel, stats_clone).await; - tracing::debug!(actor_id = ?actor_id_for_log, reason = ?reason, "Anonymous actor stopped"); - }); - - let local_id = actor_id.local_id(); - let handle = LocalActorHandle { - sender: sender.clone(), - join_handle, - cancel_token: actor_cancel, - stats: stats.clone(), - metadata: options.metadata.clone(), - named_path: None, - actor_id, - }; - - self.local_actors.insert(local_id, handle); - self.actor_names.insert(actor_id.to_string(), local_id); - - Ok(ActorRef::local(actor_id, sender)) - } - - /// Spawn a named actor (resolvable by name across the cluster) - /// - /// # Example - /// ```rust,ignore - /// // Name is used as both path (for resolution) and local name - /// system.spawn_named("services/echo", MyActor).await?; - /// ``` - pub async fn spawn_named(self: &Arc, name: P, actor: A) -> anyhow::Result - where - P: IntoActorPath, - A: IntoActor, - { - let path = name.into_actor_path()?; - self.spawn_named_factory( - path, - Self::once_factory(actor.into_actor()), - SpawnOptions::default(), - ) - .await - } - - /// Spawn a named actor with custom options - pub async fn spawn_named_with_options( - self: &Arc, - name: P, - actor: A, - options: SpawnOptions, - ) -> anyhow::Result - where - P: IntoActorPath, - A: IntoActor, - { - let path = name.into_actor_path()?; - self.spawn_named_factory(path, Self::once_factory(actor.into_actor()), options) - .await - } - - /// Spawn a named actor using a factory function - pub async fn spawn_named_factory( - self: &Arc, - name: P, + path: Option, factory: F, options: SpawnOptions, ) -> anyhow::Result where - P: IntoActorPath, F: FnMut() -> anyhow::Result + Send + 'static, A: Actor, { - let path = name.into_actor_path()?; - let name_str = path.as_str(); - - if self.actor_names.contains_key(&name_str.to_string()) { - return Err(anyhow::anyhow!("Actor already exists: {}", name_str)); - } + let name_str = path.as_ref().map(|p| p.as_str().to_string()); - if self.named_actor_paths.contains_key(&name_str.to_string()) { - return Err(anyhow::anyhow!( - "Named path already registered: {}", - name_str - )); + // Check for name conflicts (only for named actors) + if let Some(ref name) = name_str { + if self.actor_names.contains_key(name) { + return Err(anyhow::anyhow!("Actor already exists: {}", name)); + } + if self.named_actor_paths.contains_key(name) { + return Err(anyhow::anyhow!("Named path already registered: {}", name)); + } } let actor_id = self.next_actor_id(); - let local_id = actor_id.local_id(); let mailbox = Mailbox::with_capacity(self.mailbox_capacity(&options)); let (sender, receiver) = mailbox.split(); @@ -161,13 +50,7 @@ impl ActorSystem { let actor_cancel = self.cancel_token.child_token(); - let ctx = Self::build_context( - self, - actor_id, - &sender, - &actor_cancel, - Some(name_str.to_string()), - ); + let ctx = Self::build_context(self, actor_id, &sender, &actor_cancel, name_str.clone()); let stats_clone = stats.clone(); let cancel = actor_cancel.clone(); @@ -187,32 +70,40 @@ impl ActorSystem { cancel_token: actor_cancel, stats: stats.clone(), metadata: metadata.clone(), - named_path: Some(path.clone()), + named_path: path.clone(), actor_id, }; - self.local_actors.insert(local_id, handle); - self.actor_names.insert(name_str.to_string(), local_id); - self.named_actor_paths - .insert(name_str.to_string(), name_str.to_string()); - - if let Some(cluster) = self.cluster.read().await.as_ref() { - if metadata.is_empty() { - cluster.register_named_actor(path.clone()).await; - } else { - cluster - .register_named_actor_full(path.clone(), actor_id, metadata) - .await; + self.local_actors.insert(actor_id, handle); + + // Register in name maps + if let Some(ref name) = name_str { + self.actor_names.insert(name.clone(), actor_id); + self.named_actor_paths.insert(name.clone(), name.clone()); + + // Register with cluster if available + if let Some(ref path) = path { + if let Some(cluster) = self.cluster.read().await.as_ref() { + if metadata.is_empty() { + cluster.register_named_actor(path.clone()).await; + } else { + cluster + .register_named_actor_full(path.clone(), actor_id, metadata) + .await; + } + } } + } else { + // Anonymous actor: use actor_id as key + self.actor_names.insert(actor_id.to_string(), actor_id); } Ok(ActorRef::local(actor_id, sender)) } - /// Generate a new unique local actor ID + /// Generate a new unique actor ID using UUID pub(crate) fn next_actor_id(&self) -> ActorId { - let local_id = self.actor_id_counter.fetch_add(1, Ordering::Relaxed); - ActorId::new(self.node_id, local_id) + ActorId::generate() } fn mailbox_capacity(&self, options: &SpawnOptions) -> usize { diff --git a/crates/pulsing-actor/src/system/traits.rs b/crates/pulsing-actor/src/system/traits.rs index 67355b908..d24fdfae8 100644 --- a/crates/pulsing-actor/src/system/traits.rs +++ b/crates/pulsing-actor/src/system/traits.rs @@ -57,13 +57,6 @@ pub trait ActorSystemCoreExt: Sized { where P: IntoActorPath + Send; - /// Resolve a named actor with custom options (load balancing, node filtering) - async fn resolve_with_options( - &self, - name: &ActorPath, - options: ResolveOptions, - ) -> anyhow::Result; - /// Get a builder for resolving actors with advanced options. fn resolving(&self) -> ResolveBuilder<'_>; } @@ -71,7 +64,8 @@ pub trait ActorSystemCoreExt: Sized { /// Builder for spawning actors with advanced options. pub struct SpawnBuilder<'a> { system: &'a Arc, - name: Option, + name: Option, + name_error: Option, options: SpawnOptions, } @@ -81,13 +75,41 @@ impl<'a> SpawnBuilder<'a> { Self { system, name: None, + name_error: None, options: SpawnOptions::default(), } } /// Set the actor name (makes it resolvable by name) + /// + /// The name will be validated as an ActorPath. For user actors, + /// use paths like "services/echo" or "actors/counter". + /// + /// If validation fails, the error will be stored and returned when `spawn()` or `spawn_factory()` is called. pub fn name(mut self, name: impl AsRef) -> Self { - self.name = Some(name.as_ref().to_string()); + match ActorPath::new(name.as_ref()) { + Ok(path) => { + self.name = Some(path); + self.name_error = None; // Clear any previous error + } + Err(e) => { + // Store error message for later reporting + self.name_error = Some(format!("Invalid actor path '{}': {}", name.as_ref(), e)); + self.name = None; + tracing::warn!("{}", self.name_error.as_ref().unwrap()); + } + } + self + } + + /// Set the actor path directly (allows system paths) + /// + /// This method allows setting an already-validated ActorPath directly, + /// bypassing the string validation in `name()`. This is useful when + /// you already have an ActorPath or need to use system namespace paths. + pub fn path(mut self, path: ActorPath) -> Self { + self.name = Some(path); + self.name_error = None; // Clear any previous error self } @@ -122,20 +144,41 @@ impl<'a> SpawnBuilder<'a> { A: IntoActor, { let actor = actor.into_actor(); + // Create a once-use factory from the actor instance + let mut actor_opt = Some(actor); + let factory = move || { + actor_opt + .take() + .ok_or_else(|| anyhow::anyhow!("Actor cannot be restarted (spawned as instance)")) + }; + self.spawn_factory(factory).await + } + + /// Spawn an actor using a factory function + /// + /// Factory-based spawning enables supervision restarts - when an actor fails, + /// the system can recreate it using the factory function. + /// + /// Note: Only named actors support supervision/restart. Anonymous actors + /// cannot be restarted because they have no stable identity for re-resolution. + pub async fn spawn_factory(self, factory: F) -> anyhow::Result + where + F: FnMut() -> anyhow::Result + Send + 'static, + A: Actor, + { + // Check if name validation failed + if let Some(ref error) = self.name_error { + return Err(anyhow::anyhow!("{}", error)); + } + match self.name { - Some(name) => { + Some(path) => { // Named actor: resolvable by name - ActorSystem::spawn_named_with_options( - self.system, - name.as_str(), - actor, - self.options, - ) - .await + ActorSystem::spawn_internal(self.system, Some(path), factory, self.options).await } None => { // Anonymous actor: not resolvable - ActorSystem::spawn_anonymous_with_options(self.system, actor, self.options).await + ActorSystem::spawn_internal(self.system, None, factory, self.options).await } } } @@ -144,9 +187,7 @@ impl<'a> SpawnBuilder<'a> { /// Builder for resolving actors with advanced options. pub struct ResolveBuilder<'a> { system: &'a Arc, - node_id: Option, - policy: Option>, - filter_alive: bool, + options: ResolveOptions, } impl<'a> ResolveBuilder<'a> { @@ -154,51 +195,35 @@ impl<'a> ResolveBuilder<'a> { pub(crate) fn new(system: &'a Arc) -> Self { Self { system, - node_id: None, - policy: None, - filter_alive: true, + options: ResolveOptions::default(), } } /// Target a specific node (bypasses load balancing) pub fn node(mut self, node_id: NodeId) -> Self { - self.node_id = Some(node_id); + self.options = self.options.node_id(node_id); self } /// Set load balancing policy pub fn policy(mut self, policy: Arc) -> Self { - self.policy = Some(policy); + self.options = self.options.policy(policy); self } /// Set whether to filter only alive nodes (default: true) pub fn filter_alive(mut self, filter: bool) -> Self { - self.filter_alive = filter; + self.options = self.options.filter_alive(filter); self } - /// Build ResolveOptions from this builder - fn build_options(&self) -> ResolveOptions { - let mut options = ResolveOptions::new(); - if let Some(node_id) = self.node_id { - options = options.node_id(node_id); - } - if let Some(ref policy) = self.policy { - options = options.policy(policy.clone()); - } - options = options.filter_alive(self.filter_alive); - options - } - /// Resolve a named actor pub async fn resolve

(self, name: P) -> anyhow::Result where P: IntoActorPath + Send, { let path = name.into_actor_path()?; - let options = self.build_options(); - ActorSystem::resolve_named_with_options(self.system, &path, options).await + ActorSystem::resolve_named_with_options(self.system, &path, self.options).await } /// List all instances of a named actor @@ -207,7 +232,7 @@ impl<'a> ResolveBuilder<'a> { P: IntoActorPath + Send, { let path = name.into_actor_path()?; - ActorSystem::resolve_all_instances(self.system, &path, self.filter_alive).await + ActorSystem::resolve_all_instances(self.system, &path, self.options.filter_alive).await } /// Lazy resolve - returns ActorRef that auto re-resolves when stale @@ -219,38 +244,6 @@ impl<'a> ResolveBuilder<'a> { } } -// ============================================================================= -// Advanced Trait: Factory-based Spawning (Supervision/Restart) -// ============================================================================= - -/// Advanced API for factory-based actor spawning. -/// -/// Factory-based spawning enables supervision restarts - when an actor fails, -/// the system can recreate it using the factory function. -/// -/// Note: Regular `spawn` methods use a one-shot factory internally, so the actor -/// cannot be restarted. Use `spawn_named_factory` if you need supervision with -/// restart capability. Anonymous actors do not support supervision. -/// -/// -#[async_trait::async_trait] -pub trait ActorSystemAdvancedExt { - /// Spawn a named actor using a factory function (enables supervision restarts) - /// - /// Note: Only named actors support supervision/restart. Anonymous actors cannot - /// be restarted because they have no stable identity for re-resolution. - async fn spawn_named_factory( - &self, - name: P, - factory: F, - options: SpawnOptions, - ) -> anyhow::Result - where - P: IntoActorPath + Send, - F: FnMut() -> anyhow::Result + Send + 'static, - A: Actor; -} - /// Operations, introspection, and lifecycle management API. #[async_trait::async_trait] pub trait ActorSystemOpsExt { @@ -275,20 +268,6 @@ pub trait ActorSystemOpsExt { /// Get a local actor reference by name fn local_actor_ref_by_name(&self, name: &str) -> Option; - /// Spawn an anonymous actor (no name, only accessible via ActorRef) - async fn spawn_anonymous(&self, actor: A) -> anyhow::Result - where - A: IntoActor; - - /// Spawn an anonymous actor with custom options - async fn spawn_anonymous_with_options( - &self, - actor: A, - options: SpawnOptions, - ) -> anyhow::Result - where - A: IntoActor; - /// Get load tracker for a node address fn get_node_load_tracker(&self, addr: &SocketAddr) -> Option>; @@ -316,9 +295,6 @@ pub trait ActorSystemOpsExt { address: &crate::actor::ActorAddress, ) -> anyhow::Result; - /// Get all instances of a named actor across the cluster - async fn get_named_instances(&self, path: &ActorPath) -> Vec; - /// Get detailed instances with actor_id and metadata async fn get_named_instances_detailed( &self, @@ -373,7 +349,7 @@ impl ActorSystemCoreExt for Arc { where A: IntoActor, { - ActorSystem::spawn_anonymous(self, actor.into_actor()).await + self.spawning().spawn(actor).await } async fn spawn_named( @@ -384,14 +360,7 @@ impl ActorSystemCoreExt for Arc { where A: IntoActor, { - let name = name.as_ref(); - ActorSystem::spawn_named_with_options( - self, - name, - actor.into_actor(), - SpawnOptions::default(), - ) - .await + self.spawning().name(name).spawn(actor).await } fn spawning(&self) -> SpawnBuilder<'_> { @@ -409,36 +378,11 @@ impl ActorSystemCoreExt for Arc { ActorSystem::resolve_named(self.as_ref(), name, None).await } - async fn resolve_with_options( - &self, - name: &ActorPath, - options: ResolveOptions, - ) -> anyhow::Result { - ActorSystem::resolve_named_with_options(self.as_ref(), name, options).await - } - fn resolving(&self) -> ResolveBuilder<'_> { ResolveBuilder::new(self) } } -#[async_trait::async_trait] -impl ActorSystemAdvancedExt for Arc { - async fn spawn_named_factory( - &self, - name: P, - factory: F, - options: SpawnOptions, - ) -> anyhow::Result - where - P: IntoActorPath + Send, - F: FnMut() -> anyhow::Result + Send + 'static, - A: Actor, - { - ActorSystem::spawn_named_factory(self, name, factory, options).await - } -} - #[async_trait::async_trait] impl ActorSystemOpsExt for Arc { async fn system(&self) -> anyhow::Result { @@ -468,24 +412,6 @@ impl ActorSystemOpsExt for Arc { ActorSystem::local_actor_ref_by_name(self.as_ref(), name) } - async fn spawn_anonymous(&self, actor: A) -> anyhow::Result - where - A: IntoActor, - { - ActorSystem::spawn_anonymous(self, actor).await - } - - async fn spawn_anonymous_with_options( - &self, - actor: A, - options: SpawnOptions, - ) -> anyhow::Result - where - A: IntoActor, - { - ActorSystem::spawn_anonymous_with_options(self, actor, options).await - } - fn get_node_load_tracker(&self, addr: &SocketAddr) -> Option> { ActorSystem::get_node_load_tracker(self.as_ref(), addr) } @@ -509,10 +435,6 @@ impl ActorSystemOpsExt for Arc { ActorSystem::resolve(self.as_ref(), address).await } - async fn get_named_instances(&self, path: &ActorPath) -> Vec { - ActorSystem::get_named_instances(self.as_ref(), path).await - } - async fn get_named_instances_detailed( &self, path: &ActorPath, diff --git a/crates/pulsing-actor/src/system_actor/messages.rs b/crates/pulsing-actor/src/system_actor/messages.rs index 39d07fc89..450719fd3 100644 --- a/crates/pulsing-actor/src/system_actor/messages.rs +++ b/crates/pulsing-actor/src/system_actor/messages.rs @@ -2,6 +2,26 @@ use serde::{Deserialize, Serialize}; +/// Helper module for serializing u128 as string (JSON doesn't support 128-bit integers) +mod u128_as_string { + use serde::{self, Deserialize, Deserializer, Serializer}; + + pub fn serialize(value: &u128, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&value.to_string()) + } + + pub fn deserialize<'de, D>(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + s.parse().map_err(serde::de::Error::custom) + } +} + /// SystemActor request messages #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type")] @@ -79,11 +99,13 @@ pub enum SystemResponse { /// Actor created successfully ActorCreated { /// Actor ID - actor_id: u64, + #[serde(with = "u128_as_string")] + actor_id: u128, /// Actor name name: String, /// Node ID - node_id: u64, + #[serde(with = "u128_as_string")] + node_id: u128, /// Available methods list (for Python actors) #[serde(default)] methods: Vec, @@ -115,7 +137,8 @@ pub enum SystemResponse { /// Node info NodeInfo { /// Node ID - node_id: u64, + #[serde(with = "u128_as_string")] + node_id: u128, /// Address addr: String, /// Uptime in seconds @@ -135,7 +158,8 @@ pub enum SystemResponse { /// Pong response Pong { /// Node ID - node_id: u64, + #[serde(with = "u128_as_string")] + node_id: u128, /// Timestamp timestamp: u64, }, @@ -146,8 +170,8 @@ pub enum SystemResponse { pub struct ActorInfo { /// Actor name (also used as path for resolution) pub name: String, - /// Actor ID (local ID) - pub actor_id: u64, + /// Actor ID (full UUID) + pub actor_id: u128, /// Actor type pub actor_type: String, /// Uptime in seconds diff --git a/crates/pulsing-actor/src/system_actor/mod.rs b/crates/pulsing-actor/src/system_actor/mod.rs index 41115c36d..d426f64c6 100644 --- a/crates/pulsing-actor/src/system_actor/mod.rs +++ b/crates/pulsing-actor/src/system_actor/mod.rs @@ -129,7 +129,7 @@ impl ActorRegistry { .iter() .map(|e| ActorInfo { name: e.key().clone(), - actor_id: e.actor_id.local_id(), + actor_id: e.actor_id.0, actor_type: e.actor_type.clone(), uptime_secs: e.created_at.elapsed().as_secs(), metadata: std::collections::HashMap::new(), // TODO: get from actor @@ -140,7 +140,7 @@ impl ActorRegistry { pub fn get_info(&self, name: &str) -> Option { self.actors.get(name).map(|e| ActorInfo { name: name.to_string(), - actor_id: e.actor_id.local_id(), + actor_id: e.actor_id.0, actor_type: e.actor_type.clone(), uptime_secs: e.created_at.elapsed().as_secs(), metadata: std::collections::HashMap::new(), // TODO: get from actor @@ -337,23 +337,14 @@ impl Actor for SystemActor { async fn receive(&mut self, msg: Message, _ctx: &mut ActorContext) -> anyhow::Result { self.metrics.inc_message(); - // Parse system message (try JSON first for Python compatibility) + // Parse system message using auto-detection (JSON first, then bincode) let sys_msg: SystemMessage = match &msg { - Message::Single { data, .. } => { - // Try JSON parsing first (Python compatible) - match serde_json::from_slice(data) { - Ok(m) => m, - Err(_) => { - // Then try bincode parsing (Rust native) - match bincode::deserialize(data) { - Ok(m) => m, - Err(e) => { - return self.json_error_response(&format!( - "Invalid message format: {}", - e - )); - } - } + Message::Single { .. } => { + match msg.parse() { + Ok(msg) => msg, + Err(e) => { + // Return error response instead of propagating error + return self.json_error_response(&format!("Invalid message format: {}", e)); } } } diff --git a/crates/pulsing-actor/src/test_helper.rs b/crates/pulsing-actor/src/test_helper.rs index 1c6ebec79..fe5b4de6f 100644 --- a/crates/pulsing-actor/src/test_helper.rs +++ b/crates/pulsing-actor/src/test_helper.rs @@ -18,6 +18,7 @@ use crate::actor::{Actor, ActorContext, ActorRef, Message}; use crate::system::{ActorSystem, SystemConfig}; +use crate::ActorSystemCoreExt; use async_trait::async_trait; use serde::{Deserialize, Serialize}; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/pulsing-actor/src/transport/http2/mod.rs b/crates/pulsing-actor/src/transport/http2/mod.rs index c3cb78b7c..91ec9113c 100644 --- a/crates/pulsing-actor/src/transport/http2/mod.rs +++ b/crates/pulsing-actor/src/transport/http2/mod.rs @@ -274,7 +274,7 @@ impl Http2RemoteTransport { Self { client, remote_addr, - path: format!("/actors/{}", actor_id.local_id()), + path: format!("/actors/{}", actor_id), circuit_breaker: CircuitBreaker::new(), } } @@ -514,10 +514,12 @@ mod tests { fn test_http2_remote_transport_new_by_id() { let client = Arc::new(Http2Client::new(Http2Config::default())); let addr: SocketAddr = "127.0.0.1:8080".parse().unwrap(); - let actor_id = ActorId::local(42); + let actor_id = ActorId::generate(); let transport = Http2RemoteTransport::new_by_id(client, addr, actor_id); - assert_eq!(transport.path(), "/actors/42"); + // Path should be /actors/{uuid} where uuid is 32 hex chars + assert!(transport.path().starts_with("/actors/")); + assert_eq!(transport.path().len(), 8 + 32); // "/actors/" + 32 hex chars assert_eq!(transport.remote_addr(), addr); } diff --git a/crates/pulsing-actor/src/watch.rs b/crates/pulsing-actor/src/watch.rs index 62f546c89..2cd97c8ac 100644 --- a/crates/pulsing-actor/src/watch.rs +++ b/crates/pulsing-actor/src/watch.rs @@ -20,8 +20,8 @@ use tokio::sync::{mpsc, RwLock}; /// - Cluster broadcast /// - Routing table cleanup pub struct ActorLifecycle { - /// Watch registry: target_actor_name -> set of watcher_actor_names - watchers: RwLock>>, + /// Watch registry: target_actor_id -> set of watcher_actor_ids + watchers: RwLock>>, } impl ActorLifecycle { @@ -35,33 +35,30 @@ impl ActorLifecycle { // ==================== Watch API ==================== /// Register a watch: watcher will be notified when target stops - pub async fn watch(&self, watcher_name: &str, target_name: &str) { + pub async fn watch(&self, watcher: &ActorId, target: &ActorId) { let mut watchers = self.watchers.write().await; - watchers - .entry(target_name.to_string()) - .or_default() - .insert(watcher_name.to_string()); + watchers.entry(*target).or_default().insert(*watcher); tracing::debug!( - watcher = watcher_name, - target = target_name, + watcher = %watcher, + target = %target, "Watch registered" ); } /// Remove a watch relationship - pub async fn unwatch(&self, watcher_name: &str, target_name: &str) { + pub async fn unwatch(&self, watcher: &ActorId, target: &ActorId) { let mut watchers = self.watchers.write().await; - if let Some(watcher_set) = watchers.get_mut(target_name) { - watcher_set.remove(watcher_name); + if let Some(watcher_set) = watchers.get_mut(target) { + watcher_set.remove(watcher); if watcher_set.is_empty() { - watchers.remove(target_name); + watchers.remove(target); } } tracing::debug!( - watcher = watcher_name, - target = target_name, + watcher = %watcher, + target = %target, "Watch removed" ); } @@ -79,7 +76,6 @@ impl ActorLifecycle { /// /// # Arguments /// * `actor_id` - The terminated actor's ID - /// * `actor_name` - The actor's local name /// * `named_path` - Optional named actor path /// * `reason` - Why the actor stopped /// * `named_actor_paths` - Routing table to clean up @@ -89,14 +85,13 @@ impl ActorLifecycle { pub async fn handle_termination( &self, actor_id: &ActorId, - actor_name: &str, named_path: Option, reason: StopReason, named_actor_paths: &DashMap, cluster: &RwLock>>, get_sender: F, ) where - F: Fn(&str) -> Option>, + F: Fn(&ActorId) -> Option>, { // 1. Log termination self.log_termination(actor_id, named_path.as_ref(), &reason); @@ -112,8 +107,7 @@ impl ActorLifecycle { .await; // 3. Notify all watchers - self.notify_watchers(actor_id, actor_name, reason, get_sender) - .await; + self.notify_watchers(actor_id, reason, get_sender).await; } /// Log actor termination event @@ -169,29 +163,24 @@ impl ActorLifecycle { } /// Notify all watchers that an actor has terminated - async fn notify_watchers( - &self, - actor_id: &ActorId, - actor_name: &str, - reason: StopReason, - get_sender: F, - ) where - F: Fn(&str) -> Option>, + async fn notify_watchers(&self, actor_id: &ActorId, reason: StopReason, get_sender: F) + where + F: Fn(&ActorId) -> Option>, { // Get and remove watchers for this actor - let watcher_names = { + let watcher_ids = { let mut watchers = self.watchers.write().await; - watchers.remove(actor_name).unwrap_or_default() + watchers.remove(actor_id).unwrap_or_default() }; - if watcher_names.is_empty() { + if watcher_ids.is_empty() { return; } tracing::info!( actor_id = %actor_id, reason = %reason, - watcher_count = watcher_names.len(), + watcher_count = watcher_ids.len(), "Notifying watchers of actor termination" ); @@ -215,12 +204,12 @@ impl ActorLifecycle { }; // Send to all watchers - for watcher_name in watcher_names { - if let Some(sender) = get_sender(&watcher_name) { + for watcher_id in watcher_ids { + if let Some(sender) = get_sender(&watcher_id) { let envelope = Envelope::tell(Message::single(&msg_type, payload_bytes.clone())); if let Err(e) = sender.try_send(envelope) { tracing::warn!( - watcher = watcher_name, + watcher = %watcher_id, error = %e, "Failed to send termination message to watcher" ); @@ -235,18 +224,18 @@ impl ActorLifecycle { /// /// Call this when an actor is being removed from the system. /// It removes the actor both as a target and as a watcher. - pub async fn remove_actor(&self, actor_name: &str) { + pub async fn remove_actor(&self, actor_id: &ActorId) { let mut watchers = self.watchers.write().await; // Remove as target - watchers.remove(actor_name); + watchers.remove(actor_id); // Remove as watcher from all targets, and clean up empty entries let mut empty_targets = Vec::new(); for (target, watcher_set) in watchers.iter_mut() { - watcher_set.remove(actor_name); + watcher_set.remove(actor_id); if watcher_set.is_empty() { - empty_targets.push(target.clone()); + empty_targets.push(*target); } } @@ -271,18 +260,18 @@ impl ActorLifecycle { } /// Get watchers for a specific actor - pub async fn get_watchers(&self, target_name: &str) -> HashSet { + pub async fn get_watchers(&self, target: &ActorId) -> HashSet { self.watchers .read() .await - .get(target_name) + .get(target) .cloned() .unwrap_or_default() } /// Check if an actor is being watched - pub async fn is_watched(&self, target_name: &str) -> bool { - self.watchers.read().await.contains_key(target_name) + pub async fn is_watched(&self, target: &ActorId) -> bool { + self.watchers.read().await.contains_key(target) } } @@ -295,54 +284,67 @@ impl Default for ActorLifecycle { #[cfg(test)] mod tests { use super::*; - use crate::actor::NodeId; #[tokio::test] async fn test_watch_unwatch() { let lifecycle = ActorLifecycle::new(); + let watcher1 = ActorId::generate(); + let watcher2 = ActorId::generate(); + let target1 = ActorId::generate(); + // Add watches - lifecycle.watch("watcher1", "target1").await; - lifecycle.watch("watcher2", "target1").await; + lifecycle.watch(&watcher1, &target1).await; + lifecycle.watch(&watcher2, &target1).await; - assert!(lifecycle.is_watched("target1").await); - assert_eq!(lifecycle.get_watchers("target1").await.len(), 2); + assert!(lifecycle.is_watched(&target1).await); + assert_eq!(lifecycle.get_watchers(&target1).await.len(), 2); // Unwatch - lifecycle.unwatch("watcher1", "target1").await; - assert_eq!(lifecycle.get_watchers("target1").await.len(), 1); + lifecycle.unwatch(&watcher1, &target1).await; + assert_eq!(lifecycle.get_watchers(&target1).await.len(), 1); - lifecycle.unwatch("watcher2", "target1").await; - assert!(!lifecycle.is_watched("target1").await); + lifecycle.unwatch(&watcher2, &target1).await; + assert!(!lifecycle.is_watched(&target1).await); } #[tokio::test] async fn test_remove_actor() { let lifecycle = ActorLifecycle::new(); + let watcher1 = ActorId::generate(); + let watcher2 = ActorId::generate(); + let target1 = ActorId::generate(); + let target2 = ActorId::generate(); + // Setup: watcher1 watches target1 and target2 - lifecycle.watch("watcher1", "target1").await; - lifecycle.watch("watcher1", "target2").await; - lifecycle.watch("watcher2", "target1").await; + lifecycle.watch(&watcher1, &target1).await; + lifecycle.watch(&watcher1, &target2).await; + lifecycle.watch(&watcher2, &target1).await; // Remove watcher1 from all relationships - lifecycle.remove_actor("watcher1").await; + lifecycle.remove_actor(&watcher1).await; // watcher1 should be removed as watcher - let watchers = lifecycle.get_watchers("target1").await; - assert!(!watchers.contains("watcher1")); - assert!(watchers.contains("watcher2")); + let watchers = lifecycle.get_watchers(&target1).await; + assert!(!watchers.contains(&watcher1)); + assert!(watchers.contains(&watcher2)); // target2 should have no watchers - assert!(!lifecycle.is_watched("target2").await); + assert!(!lifecycle.is_watched(&target2).await); } #[tokio::test] async fn test_clear() { let lifecycle = ActorLifecycle::new(); - lifecycle.watch("w1", "t1").await; - lifecycle.watch("w2", "t2").await; + let w1 = ActorId::generate(); + let w2 = ActorId::generate(); + let t1 = ActorId::generate(); + let t2 = ActorId::generate(); + + lifecycle.watch(&w1, &t1).await; + lifecycle.watch(&w2, &t2).await; assert_eq!(lifecycle.watched_count().await, 2); @@ -355,19 +357,19 @@ mod tests { async fn test_notify_watchers() { let lifecycle = ActorLifecycle::new(); - lifecycle.watch("watcher1", "target1").await; - lifecycle.watch("watcher2", "target1").await; + let watcher1 = ActorId::generate(); + let watcher2 = ActorId::generate(); + let target1 = ActorId::generate(); - let actor_id = ActorId::new(NodeId::generate(), 1); + lifecycle.watch(&watcher1, &target1).await; + lifecycle.watch(&watcher2, &target1).await; // Create a channel to receive notifications let (tx, mut rx) = mpsc::channel::(10); // Notify watchers lifecycle - .notify_watchers(&actor_id, "target1", StopReason::Normal, |_name| { - Some(tx.clone()) - }) + .notify_watchers(&target1, StopReason::Normal, |_id| Some(tx.clone())) .await; // Should receive 2 notifications @@ -378,6 +380,6 @@ mod tests { assert_eq!(count, 2); // Watchers should be cleared after notification - assert!(!lifecycle.is_watched("target1").await); + assert!(!lifecycle.is_watched(&target1).await); } } diff --git a/crates/pulsing-actor/tests/address_tests.rs b/crates/pulsing-actor/tests/address_tests.rs index 9d0eafa9c..321090975 100644 --- a/crates/pulsing-actor/tests/address_tests.rs +++ b/crates/pulsing-actor/tests/address_tests.rs @@ -1,6 +1,6 @@ //! Comprehensive tests for the actor addressing system -use pulsing_actor::actor::{ActorId, ActorPath, NodeId}; +use pulsing_actor::actor::{ActorId, ActorPath}; use pulsing_actor::prelude::*; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; @@ -92,11 +92,13 @@ mod actor_address_tests { #[test] fn test_address_parsing() { - // Test that addresses can be created and parsed - let node = NodeId::generate(); - let actor_id = ActorId::new(node, 123); - assert_eq!(actor_id.local_id(), 123); - assert_eq!(actor_id.node(), node); + // Test that ActorIds can be created + let actor_id = ActorId::generate(); + assert_ne!(actor_id.0, 0); + + // Test creating from specific value + let actor_id2 = ActorId::new(12345); + assert_eq!(actor_id2.0, 12345); } } diff --git a/crates/pulsing-actor/tests/cluster/member_tests.rs b/crates/pulsing-actor/tests/cluster/member_tests.rs index c36f76dcc..1c7eccf16 100644 --- a/crates/pulsing-actor/tests/cluster/member_tests.rs +++ b/crates/pulsing-actor/tests/cluster/member_tests.rs @@ -244,7 +244,7 @@ fn test_member_info_hash() { #[test] fn test_actor_location() { - let actor_id = ActorId::local(1); + let actor_id = ActorId::generate(); let node_id = NodeId::generate(); let location = ActorLocation::new(actor_id, node_id); @@ -282,7 +282,7 @@ fn test_failure_info() { #[test] fn test_named_actor_instance_new() { let node_id = NodeId::generate(); - let actor_id = ActorId::local(42); + let actor_id = ActorId::generate(); let instance = NamedActorInstance::new(node_id, actor_id); @@ -294,7 +294,7 @@ fn test_named_actor_instance_new() { #[test] fn test_named_actor_instance_with_metadata() { let node_id = NodeId::generate(); - let actor_id = ActorId::local(42); + let actor_id = ActorId::generate(); let mut metadata = HashMap::new(); metadata.insert("class".to_string(), "Counter".to_string()); metadata.insert("module".to_string(), "__main__".to_string()); @@ -348,7 +348,7 @@ fn test_named_actor_info_with_instance() { fn test_named_actor_info_with_full_instance() { let path = ActorPath::new("actors/counter").unwrap(); let node_id = NodeId::generate(); - let actor_id = ActorId::local(42); + let actor_id = ActorId::generate(); let mut metadata = HashMap::new(); metadata.insert("class".to_string(), "Counter".to_string()); @@ -400,8 +400,8 @@ fn test_named_actor_info_add_full_instance() { let path = ActorPath::new("actors/counter").unwrap(); let node1 = NodeId::generate(); let node2 = NodeId::generate(); - let actor_id1 = ActorId::local(1); - let actor_id2 = ActorId::local(2); + let actor_id1 = ActorId::generate(); + let actor_id2 = ActorId::generate(); let mut info = NamedActorInfo::new(path); @@ -471,8 +471,8 @@ fn test_named_actor_info_merge_with_full_instances() { let path = ActorPath::new("actors/counter").unwrap(); let node1 = NodeId::generate(); let node2 = NodeId::generate(); - let actor_id1 = ActorId::local(1); - let actor_id2 = ActorId::local(2); + let actor_id1 = ActorId::generate(); + let actor_id2 = ActorId::generate(); let mut metadata1 = HashMap::new(); metadata1.insert("class".to_string(), "Counter".to_string()); diff --git a/crates/pulsing-actor/tests/cluster_tests.rs b/crates/pulsing-actor/tests/cluster_tests.rs index 0b7f98d35..c64bd6ec0 100644 --- a/crates/pulsing-actor/tests/cluster_tests.rs +++ b/crates/pulsing-actor/tests/cluster_tests.rs @@ -74,39 +74,40 @@ async fn test_system_with_specific_addr() { #[test] fn test_actor_id_creation() { - let node_id = NodeId::generate(); - let actor_id = ActorId::new(node_id, 123); + // Test generating a new ActorId + let actor_id = ActorId::generate(); + assert_ne!(actor_id.0, 0); - assert_eq!(actor_id.node(), node_id); - assert_eq!(actor_id.local_id(), 123); + // Test creating from specific value + let actor_id2 = ActorId::new(12345); + assert_eq!(actor_id2.0, 12345); } #[test] -fn test_actor_id_local() { - let actor_id = ActorId::local(456); +fn test_actor_id_uniqueness() { + // UUID-based IDs should be unique + let id1 = ActorId::generate(); + let id2 = ActorId::generate(); - assert!(actor_id.node().is_local()); - assert_eq!(actor_id.local_id(), 456); + assert_ne!(id1, id2); } #[test] fn test_actor_id_equality() { - let node_id = NodeId::generate(); - let id1 = ActorId::new(node_id, 1); - let id2 = ActorId::new(node_id, 1); + // Same value should be equal + let id1 = ActorId::new(12345); + let id2 = ActorId::new(12345); assert_eq!(id1, id2); } #[test] fn test_actor_id_display() { - let node_id = NodeId::generate(); - let actor_id = ActorId::new(node_id, 42); + let actor_id = ActorId::generate(); let display = format!("{}", actor_id); - // Display format is "node_id:local_id" - assert!(display.contains("42")); - assert!(display.contains(&node_id.0.to_string())); + // Display format is UUID (32 hex characters) + assert_eq!(display.len(), 32); } // ============================================================================ diff --git a/crates/pulsing-actor/tests/http2_transport_tests.rs b/crates/pulsing-actor/tests/http2_transport_tests.rs index eb95c365e..864ac521f 100644 --- a/crates/pulsing-actor/tests/http2_transport_tests.rs +++ b/crates/pulsing-actor/tests/http2_transport_tests.rs @@ -396,7 +396,7 @@ async fn test_http2_remote_transport_ask() { // Use the RemoteTransport trait use pulsing_actor::actor::RemoteTransport; - let actor_id = ActorId::local(1); + let actor_id = ActorId::generate(); let response = transport .request(&actor_id, "TestType", b"payload".to_vec()) .await @@ -436,7 +436,7 @@ async fn test_http2_remote_transport_tell() { // Use the RemoteTransport trait use pulsing_actor::actor::RemoteTransport; - let actor_id = ActorId::local(2); + let actor_id = ActorId::generate(); transport .send(&actor_id, "FireMsg", b"data".to_vec()) .await @@ -478,7 +478,7 @@ async fn test_http2_remote_transport_named_path() { // Use the RemoteTransport trait use pulsing_actor::actor::RemoteTransport; - let actor_id = ActorId::local(3); + let actor_id = ActorId::generate(); let response = transport .request(&actor_id, "Inference", b"prompt".to_vec()) .await diff --git a/crates/pulsing-actor/tests/integration_tests.rs b/crates/pulsing-actor/tests/integration_tests.rs index 05c93ec52..4056a78b5 100644 --- a/crates/pulsing-actor/tests/integration_tests.rs +++ b/crates/pulsing-actor/tests/integration_tests.rs @@ -542,6 +542,7 @@ mod lifecycle_tests { mod addressing_tests { use super::*; + use pulsing_actor::actor::ActorId; #[tokio::test] async fn test_spawn_named_actor() { @@ -621,7 +622,7 @@ mod addressing_tests { .unwrap(); // Get the full address using the actual actor id - let addr = ActorAddress::local(actor_ref.id().local_id()); + let addr = ActorAddress::local(*actor_ref.id()); // Resolve let resolved_ref = ActorSystemOpsExt::resolve_address(&system, &addr) @@ -649,10 +650,8 @@ mod addressing_tests { .await .unwrap(); - // Resolve using local address (node_id = 0) with actual actor id - let addr = - ActorAddress::parse(&format!("actor://0/{}", actor_ref.id().local_id())).unwrap(); - assert!(addr.is_local()); + // Resolve using global address with actual actor id + let addr = ActorAddress::global(*actor_ref.id()); let resolved_ref = ActorSystemOpsExt::resolve_address(&system, &addr) .await @@ -722,19 +721,17 @@ mod addressing_tests { assert_eq!(addr.path().unwrap().namespace(), "services"); assert_eq!(addr.path().unwrap().name(), "api"); - // Named instance (node_id is now u64) + // Named instance (uses u128 node_id) let addr = ActorAddress::parse("actor:///services/api@123").unwrap(); assert!(addr.is_named()); assert_eq!(addr.node_id().map(|n| n.0), Some(123)); - // Global (node_id and actor_id are now u64) - let addr = ActorAddress::parse("actor://456/789").unwrap(); + // Global address with UUID format + let actor_id = ActorId::generate(); + let addr_str = format!("actor://{}", actor_id); + let addr = ActorAddress::parse(&addr_str).unwrap(); assert!(addr.is_global()); - assert_eq!(addr.actor_id(), Some(789)); - - // Local (node_id = 0) - let addr = ActorAddress::parse("actor://0/100").unwrap(); - assert!(addr.is_local()); + assert_eq!(addr.actor_id(), Some(actor_id)); } #[tokio::test] diff --git a/crates/pulsing-actor/tests/multi_node_tests.rs b/crates/pulsing-actor/tests/multi_node_tests.rs index 579b49882..d38a44f49 100644 --- a/crates/pulsing-actor/tests/multi_node_tests.rs +++ b/crates/pulsing-actor/tests/multi_node_tests.rs @@ -464,8 +464,8 @@ mod edge_case_tests { let ref1 = system1.spawn_named("test/shared-name", Echo).await.unwrap(); let ref2 = system2.spawn_named("test/shared-name", Echo).await.unwrap(); - // They should have different full IDs (different node IDs) - assert_ne!(ref1.id().node(), ref2.id().node()); + // With UUID-based IDs, each actor has a unique ID + assert_ne!(ref1.id(), ref2.id()); // Both should be local actors on their respective systems assert!(ref1.is_local()); assert!(ref2.is_local()); @@ -686,12 +686,12 @@ mod addressing_multi_node_tests { } #[tokio::test] - async fn test_resolve_global_address_cross_node() { + async fn test_resolve_named_actor_cross_node() { // Node 1 let config1 = create_cluster_config(20087); let system1 = ActorSystem::new(config1).await.unwrap(); let gossip1_addr = system1.addr(); - let node1_id = *system1.node_id(); + let _node1_id = *system1.node_id(); // Node 2 joins let mut config2 = create_cluster_config(20088); @@ -701,14 +701,16 @@ mod addressing_multi_node_tests { // Wait for cluster formation tokio::time::sleep(Duration::from_millis(500)).await; - // Create regular actor on node 1 - let actor_ref = system1 + // Create named actor on node 1 + let _actor_ref = system1 .spawn_named("test/remote_worker", Echo) .await .unwrap(); - // Node 2 resolves using global address with retries - let addr = ActorAddress::global(node1_id, actor_ref.id().local_id()); + // Node 2 resolves using named address with retries + // Note: With UUID-based ActorIds, we can no longer derive node from ActorId. + // Use named resolution instead for cross-node actor lookup. + let addr = ActorAddress::named(ActorPath::new("test/remote_worker").unwrap()); let mut resolved_ref = None; for attempt in 1..=15 { match ActorSystemOpsExt::resolve_address(&system2, &addr).await { @@ -720,14 +722,14 @@ mod addressing_multi_node_tests { tokio::time::sleep(Duration::from_millis(200)).await; } Err(e) => { - panic!("Failed to resolve global address after 15 attempts: {}", e); + panic!("Failed to resolve named address after 15 attempts: {}", e); } } } - let resolved_ref = resolved_ref.expect("Should resolve global address"); + let resolved_ref = resolved_ref.expect("Should resolve named address"); - // Should be a remote reference + // Should be a remote reference from node 2's perspective assert!(!resolved_ref.is_local()); // Call should work diff --git a/crates/pulsing-actor/tests/supervision_tests.rs b/crates/pulsing-actor/tests/supervision_tests.rs index 7c717017f..227d6d484 100644 --- a/crates/pulsing-actor/tests/supervision_tests.rs +++ b/crates/pulsing-actor/tests/supervision_tests.rs @@ -43,10 +43,11 @@ async fn test_restart_on_failure() { Duration::from_millis(100), )); - let options = SpawnOptions::new().supervision(spec); - let actor_ref = system - .spawn_named_factory("test/failing", factory, options) + .spawning() + .name("test/failing") + .supervision(spec) + .spawn_factory(factory) .await .unwrap(); @@ -100,10 +101,11 @@ async fn test_max_restarts_exceeded() { factor: 1.0, }); - let options = SpawnOptions::new().supervision(spec); - let actor_ref = system - .spawn_named_factory("test/crashing", factory, options) + .spawning() + .name("test/crashing") + .supervision(spec) + .spawn_factory(factory) .await .unwrap(); diff --git a/crates/pulsing-actor/tests/system_actor_tests.rs b/crates/pulsing-actor/tests/system_actor_tests.rs index ff6e3b781..d02b7c47d 100644 --- a/crates/pulsing-actor/tests/system_actor_tests.rs +++ b/crates/pulsing-actor/tests/system_actor_tests.rs @@ -426,7 +426,7 @@ async fn test_system_actor_uptime_increases() { #[test] fn test_actor_registry() { let registry = ActorRegistry::new(); - let actor_id = ActorId::local(1); + let actor_id = ActorId::generate(); registry.register("test", actor_id, "TestActor"); assert!(registry.contains("test")); @@ -444,8 +444,8 @@ fn test_actor_registry() { fn test_actor_registry_list_all() { let registry = ActorRegistry::new(); - registry.register("actor1", ActorId::local(1), "TypeA"); - registry.register("actor2", ActorId::local(2), "TypeB"); + registry.register("actor1", ActorId::generate(), "TypeA"); + registry.register("actor2", ActorId::generate(), "TypeB"); let actors = registry.list_all(); assert_eq!(actors.len(), 2); diff --git a/crates/pulsing-py/Cargo.toml b/crates/pulsing-py/Cargo.toml index b1792e6ff..6bb690a3c 100644 --- a/crates/pulsing-py/Cargo.toml +++ b/crates/pulsing-py/Cargo.toml @@ -27,6 +27,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } reqwest = { workspace = true } pythonize = "0.23" +uuid = { workspace = true } [dependencies.pyo3] version = "0.23.4" diff --git a/crates/pulsing-py/src/actor.rs b/crates/pulsing-py/src/actor.rs index f796fa212..461e4af86 100644 --- a/crates/pulsing-py/src/actor.rs +++ b/crates/pulsing-py/src/actor.rs @@ -38,10 +38,39 @@ impl PyNodeId { } } + /// Create a new NodeId from a u128 value or string UUID #[new] - fn new(id: u64) -> Self { - Self { - inner: NodeId::new(id), + #[pyo3(signature = (id=None))] + fn new(id: Option<&Bound<'_, pyo3::PyAny>>) -> PyResult { + match id { + None => Ok(Self { + inner: NodeId::generate(), + }), + Some(py_id) => { + // Try to extract as string first (UUID format) + if let Ok(s) = py_id.extract::() { + if let Ok(uuid) = uuid::Uuid::parse_str(&s) { + return Ok(Self { + inner: NodeId::new(uuid.as_u128()), + }); + } + } + // Try as integer + if let Ok(n) = py_id.extract::() { + return Ok(Self { + inner: NodeId::new(n), + }); + } + // Try as smaller integer + if let Ok(n) = py_id.extract::() { + return Ok(Self { + inner: NodeId::new(n as u128), + }); + } + Err(PyValueError::new_err( + "NodeId must be a UUID string or integer", + )) + } } } @@ -52,11 +81,17 @@ impl PyNodeId { } } + /// Get the raw u128 value #[getter] - fn id(&self) -> u64 { + fn id(&self) -> u128 { self.inner.0 } + /// Get the UUID string representation + fn uuid(&self) -> String { + self.inner.to_string() + } + fn is_local(&self) -> bool { self.inner.is_local() } @@ -79,33 +114,59 @@ pub struct PyActorId { #[pymethods] impl PyActorId { + /// Create a new ActorId from a u128 value, string UUID, or generate a new one #[new] - #[pyo3(signature = (local_id, node=None))] - fn new(local_id: u64, node: Option) -> Self { - let inner = match node { - Some(n) => ActorId::new(n.inner, local_id), - None => ActorId::local(local_id), - }; - Self { inner } + #[pyo3(signature = (id=None))] + fn new(id: Option<&Bound<'_, pyo3::PyAny>>) -> PyResult { + match id { + None => Ok(Self { + inner: ActorId::generate(), + }), + Some(py_id) => { + // Try to extract as string first (UUID format) + if let Ok(s) = py_id.extract::() { + if let Ok(uuid) = uuid::Uuid::parse_str(&s) { + return Ok(Self { + inner: ActorId::new(uuid.as_u128()), + }); + } + } + // Try as integer + if let Ok(n) = py_id.extract::() { + return Ok(Self { + inner: ActorId::new(n), + }); + } + // Try as smaller integer + if let Ok(n) = py_id.extract::() { + return Ok(Self { + inner: ActorId::new(n as u128), + }); + } + Err(PyValueError::new_err( + "ActorId must be a UUID string or integer", + )) + } + } } + /// Generate a new random ActorId #[staticmethod] - fn local(local_id: u64) -> Self { + fn generate() -> Self { Self { - inner: ActorId::local(local_id), + inner: ActorId::generate(), } } + /// Get the raw u128 value #[getter] - fn local_id(&self) -> u64 { - self.inner.local_id() + fn id(&self) -> u128 { + self.inner.0 } - #[getter] - fn node(&self) -> PyNodeId { - PyNodeId { - inner: self.inner.node(), - } + /// Get the UUID string representation + fn uuid(&self) -> String { + self.inner.to_string() } fn __str__(&self) -> String { @@ -113,11 +174,7 @@ impl PyActorId { } fn __repr__(&self) -> String { - format!( - "ActorId(local_id={}, node={})", - self.inner.local_id(), - self.inner.node() - ) + format!("ActorId({})", self.inner.0) } fn __hash__(&self) -> u64 { @@ -131,31 +188,25 @@ impl PyActorId { self.inner == other.inner } - /// Parse ActorId from string format "node_id:local_id" + /// Parse ActorId from string (UUID format) #[staticmethod] fn from_str(s: &str) -> PyResult { - let parts: Vec<&str> = s.split(':').collect(); - if parts.len() != 2 { - return Err(pyo3::exceptions::PyValueError::new_err(format!( - "Invalid ActorId format: '{}'. Expected 'node_id:local_id'", - s - ))); + // Try to parse as UUID + if let Ok(uuid) = uuid::Uuid::parse_str(s) { + return Ok(Self { + inner: ActorId::new(uuid.as_u128()), + }); } - let node_id: u64 = parts[0].parse().map_err(|_| { - pyo3::exceptions::PyValueError::new_err(format!( - "Invalid node_id in ActorId: '{}'", - parts[0] - )) - })?; - let local_id: u64 = parts[1].parse().map_err(|_| { - pyo3::exceptions::PyValueError::new_err(format!( - "Invalid local_id in ActorId: '{}'", - parts[1] - )) - })?; - Ok(Self { - inner: ActorId::new(NodeId::new(node_id), local_id), - }) + // Try to parse as simple integer + if let Ok(n) = s.parse::() { + return Ok(Self { + inner: ActorId::new(n), + }); + } + Err(pyo3::exceptions::PyValueError::new_err(format!( + "Invalid ActorId format: '{}'. Expected UUID string or integer", + s + ))) } } @@ -1217,7 +1268,7 @@ impl PyActorSystem { let _ = public; pyo3_async_runtimes::tokio::future_into_py(py, async move { - let options = pulsing_actor::system::SpawnOptions::new() + let options = pulsing_actor::system::SpawnOptions::default() .supervision(supervision) .metadata(metadata); @@ -1234,7 +1285,9 @@ impl PyActorSystem { // actor is the instance let actor_wrapper = PythonActorWrapper::new(actor, event_loop); system - .spawn_anonymous_with_options(actor_wrapper, options) + .spawning() + .metadata(options.metadata) + .spawn(actor_wrapper) .await .map_err(to_pyerr)? } @@ -1258,7 +1311,11 @@ impl PyActorSystem { // actor is the instance let actor_wrapper = PythonActorWrapper::new(actor, event_loop); system - .spawn_named_with_options(path, actor_wrapper, options) + .spawning() + .path(path) + .supervision(options.supervision) + .metadata(options.metadata) + .spawn(actor_wrapper) .await .map_err(to_pyerr)? } else { @@ -1273,7 +1330,11 @@ impl PyActorSystem { }) }; system - .spawn_named_factory(path, factory, options) + .spawning() + .path(path) + .supervision(options.supervision) + .metadata(options.metadata) + .spawn_factory(factory) .await .map_err(to_pyerr)? } @@ -1304,11 +1365,13 @@ impl PyActorSystem { pyo3_async_runtimes::tokio::future_into_py(py, async move { let members = system.members().await; + // Return all fields as strings for safe JSON serialization let result: Vec> = members .into_iter() .map(|m| { let mut map = std::collections::HashMap::new(); - map.insert("node_id".to_string(), m.node_id.to_string()); + // Use string representation to avoid JSON integer overflow + map.insert("node_id".to_string(), m.node_id.0.to_string()); map.insert("addr".to_string(), m.addr.to_string()); map.insert("status".to_string(), format!("{:?}", m.status)); map @@ -1348,9 +1411,10 @@ impl PyActorSystem { .into_iter() .map(|(member, instance_opt)| { let mut map = std::collections::HashMap::new(); + // Use decimal string for node_id to match members() format map.insert( "node_id".to_string(), - serde_json::Value::String(member.node_id.to_string()), + serde_json::Value::String(member.node_id.0.to_string()), ); map.insert( "addr".to_string(), @@ -1363,9 +1427,10 @@ impl PyActorSystem { // Add detailed instance info if available if let Some(inst) = instance_opt { + // Use decimal string for actor_id to match other APIs map.insert( "actor_id".to_string(), - serde_json::Value::String(inst.actor_id.to_string()), + serde_json::Value::String(inst.actor_id.0.to_string()), ); // Add metadata fields for (k, v) in inst.metadata { @@ -1408,11 +1473,11 @@ impl PyActorSystem { info.instance_count(), )), ); - // Convert instance_nodes (HashSet) to list of node IDs as strings + // Convert instance_nodes (HashSet) to list of node IDs as decimal strings let instances: Vec = info .instance_nodes .iter() - .map(|id| serde_json::Value::String(id.to_string())) + .map(|id| serde_json::Value::String(id.0.to_string())) .collect(); map.insert("instances".to_string(), serde_json::Value::Array(instances)); @@ -1422,13 +1487,14 @@ impl PyActorSystem { .iter() .map(|(node_id, inst)| { let mut inst_map = serde_json::Map::new(); + // Use decimal string to match members() format inst_map.insert( "node_id".to_string(), - serde_json::Value::String(node_id.to_string()), + serde_json::Value::String(node_id.0.to_string()), ); inst_map.insert( "actor_id".to_string(), - serde_json::Value::String(inst.actor_id.to_string()), + serde_json::Value::String(inst.actor_id.0.to_string()), ); // Add metadata for (k, v) in &inst.metadata { @@ -1460,7 +1526,7 @@ impl PyActorSystem { &self, py: Python<'py>, name: String, - node_id: Option, + node_id: Option, ) -> PyResult> { let system = self.inner.clone(); @@ -1492,7 +1558,7 @@ impl PyActorSystem { &self, py: Python<'py>, name: String, - node_id: Option, + node_id: Option, ) -> PyResult> { self.resolve_named(py, name, node_id) } @@ -1526,7 +1592,7 @@ impl PyActorSystem { } /// Get remote SystemActor reference (for remote nodes) - fn remote_system<'py>(&self, py: Python<'py>, node_id: u64) -> PyResult> { + fn remote_system<'py>(&self, py: Python<'py>, node_id: u128) -> PyResult> { let system = self.inner.clone(); pyo3_async_runtimes::tokio::future_into_py(py, async move { diff --git a/docs/src/api/overview.zh.md b/docs/src/api/overview.zh.md index 53f1dc2bd..8be18aa1a 100644 --- a/docs/src/api/overview.zh.md +++ b/docs/src/api/overview.zh.md @@ -212,7 +212,7 @@ let response = actor.ask(Ping(42)).await?; Factory 模式生成，支持监督重启（仅命名 actor）： ```rust -let options = SpawnOptions::new() +let options = SpawnOptions::default() .supervision(SupervisionSpec::on_failure().max_restarts(3)); // 仅命名 actor 支持 supervision diff --git a/docs/src/api/rust.md b/docs/src/api/rust.md index 4e66c70a0..a74352824 100644 --- a/docs/src/api/rust.md +++ b/docs/src/api/rust.md @@ -174,7 +174,7 @@ Actors can be configured with restart policies for fault tolerance. ```rust use pulsing_actor::system::SupervisionSpec; -let options = SpawnOptions::new() +let options = SpawnOptions::default() .supervision(SupervisionSpec::on_failure().max_restarts(3)); // Factory-based spawning with supervision diff --git a/docs/src/api/rust.zh.md b/docs/src/api/rust.zh.md index 0d0f14ea5..52df03f71 100644 --- a/docs/src/api/rust.zh.md +++ b/docs/src/api/rust.zh.md @@ -174,7 +174,7 @@ Actor 可以配置重启策略以实现容错。 ```rust use pulsing_actor::system::SupervisionSpec; -let options = SpawnOptions::new() +let options = SpawnOptions::default() .supervision(SupervisionSpec::on_failure().max_restarts(3)); // 基于工厂的生成，支持监督 diff --git a/docs/src/api_reference.md b/docs/src/api_reference.md index fafb074b8..25f040764 100644 --- a/docs/src/api_reference.md +++ b/docs/src/api_reference.md @@ -390,7 +390,7 @@ system.resolving().lazy(name)?; // Lazy resolution (~5s TTL auto- Factory-based spawning for supervision restarts (named actors only): ```rust -let options = SpawnOptions::new() +let options = SpawnOptions::default() .supervision(SupervisionSpec::on_failure().max_restarts(3)); // Only named actors support supervision (anonymous cannot be re-resolved) diff --git a/docs/src/api_reference.zh.md b/docs/src/api_reference.zh.md index 6195f8b91..1e303e4f8 100644 --- a/docs/src/api_reference.zh.md +++ b/docs/src/api_reference.zh.md @@ -413,7 +413,7 @@ system.resolving().lazy(name)?; // 懒解析（~5s TTL 自动刷 Factory 模式 spawn，支持 supervision 重启（仅命名 actor）： ```rust -let options = SpawnOptions::new() +let options = SpawnOptions::default() .supervision(SupervisionSpec::on_failure().max_restarts(3)); // 仅命名 actor 支持 supervision（匿名 actor 无法重新解析） diff --git a/python/pulsing/actor/__init__.py b/python/pulsing/actor/__init__.py index 61007d7c2..15041bd29 100644 --- a/python/pulsing/actor/__init__.py +++ b/python/pulsing/actor/__init__.py @@ -187,8 +187,12 @@ async def tell_with_timeout( ActorClass, ActorProxy, PythonActorService, + PythonActorServiceProxy, + SystemActorProxy, get_metrics, get_node_info, + get_python_actor_service, + get_system_actor, health_check, list_actors, ping, @@ -206,6 +210,7 @@ async def tell_with_timeout( "remote", "resolve", "get_system", + "get_system_actor", "is_initialized", # Minimal core types commonly used in docs/examples "Actor", @@ -216,6 +221,7 @@ async def tell_with_timeout( "ActorRef", "ActorId", "ActorProxy", + "SystemActorProxy", # Service (for actor_system function) "PythonActorService", "PYTHON_ACTOR_SERVICE_NAME", diff --git a/python/pulsing/actor/remote.py b/python/pulsing/actor/remote.py index 36ce248c0..cd621ca79 100644 --- a/python/pulsing/actor/remote.py +++ b/python/pulsing/actor/remote.py @@ -515,8 +515,9 @@ def factory(): return Message.from_json( "Created", { - "actor_id": actor_ref.actor_id.local_id, - "node_id": self.system.node_id.id, + # actor_id is now a UUID (u128), transmit as string for JSON + "actor_id": str(actor_ref.actor_id.id), + "node_id": str(self.system.node_id.id), "methods": method_names, }, ) @@ -698,10 +699,11 @@ async def remote( public = name is not None members = await system.members() - local_id = system.node_id.id + # members["node_id"] is string, convert local_id to string for comparison + local_id = str(system.node_id.id) - # Filter out remote nodes - remote_nodes = [m for m in members if int(m["node_id"]) != local_id] + # Filter out remote nodes (node_id is string) + remote_nodes = [m for m in members if m["node_id"] != local_id] if not remote_nodes: # No remote nodes, fallback to local creation @@ -710,6 +712,7 @@ async def remote( # Randomly select one target = random.choice(remote_nodes) + # Convert back to int for resolve_named target_id = int(target["node_id"]) # Get target node's Python actor creation service @@ -747,9 +750,13 @@ async def remote( raise RuntimeError(f"Remote create failed: {data.get('error')}") # Build remote ActorRef - from pulsing._core import ActorId, NodeId + from pulsing._core import ActorId - remote_id = ActorId(data["actor_id"], NodeId(data["node_id"])) + # actor_id is now a UUID (u128), may be transmitted as string + actor_id = data["actor_id"] + if isinstance(actor_id, str): + actor_id = int(actor_id) + remote_id = ActorId(actor_id) actor_ref = await system.actor_ref(remote_id) return ActorProxy( @@ -869,44 +876,202 @@ def wrapper(cls): # ============================================================================ +class SystemActorProxy: + """Proxy for SystemActor with direct method calls. + + Example: + system_proxy = await get_system_actor(system) + actors = await system_proxy.list_actors() + metrics = await system_proxy.get_metrics() + await system_proxy.ping() + """ + + def __init__(self, actor_ref: ActorRef): + self._ref = actor_ref + + @property + def ref(self) -> ActorRef: + """Get underlying ActorRef.""" + return self._ref + + async def _ask(self, msg_type: str) -> dict: + """Send SystemMessage and return response.""" + resp = await self._ref.ask( + Message.from_json("SystemMessage", {"type": msg_type}) + ) + return resp.to_json() + + async def list_actors(self) -> list[dict]: + """List all actors on this node.""" + data = await self._ask("ListActors") + if data.get("type") == "Error": + raise RuntimeError(data.get("message")) + return data.get("actors", []) + + async def get_metrics(self) -> dict: + """Get system metrics.""" + return await self._ask("GetMetrics") + + async def get_node_info(self) -> dict: + """Get node info.""" + return await self._ask("GetNodeInfo") + + async def health_check(self) -> dict: + """Health check.""" + return await self._ask("HealthCheck") + + async def ping(self) -> dict: + """Ping this node.""" + return await self._ask("Ping") + + +async def get_system_actor( + system: ActorSystem, node_id: int | None = None +) -> SystemActorProxy: + """Get SystemActorProxy for direct method calls. + + Args: + system: ActorSystem instance + node_id: Target node ID (None means local node) + + Returns: + SystemActorProxy with methods: list_actors(), get_metrics(), etc. + + Example: + sys = await get_system_actor(system) + actors = await sys.list_actors() + await sys.ping() + """ + if node_id is None: + actor_ref = await system.system() + else: + actor_ref = await system.remote_system(node_id) + return SystemActorProxy(actor_ref) + + +class PythonActorServiceProxy: + """Proxy for PythonActorService with direct method calls. + + Example: + service = await get_python_actor_service(system) + classes = await service.list_registry() + actor_ref = await service.create_actor("MyClass", name="my_actor") + """ + + def __init__(self, actor_ref: ActorRef): + self._ref = actor_ref + + @property + def ref(self) -> ActorRef: + """Get underlying ActorRef.""" + return self._ref + + async def list_registry(self) -> list[str]: + """List registered actor classes. + + Returns: + List of registered class names + """ + resp = await self._ref.ask(Message.from_json("ListRegistry", {})) + data = resp.to_json() + return data.get("classes", []) + + async def create_actor( + self, + class_name: str, + *args, + name: str | None = None, + public: bool = True, + restart_policy: str = "never", + max_restarts: int = 3, + min_backoff: float = 0.1, + max_backoff: float = 30.0, + **kwargs, + ) -> dict: + """Create a Python actor. + + Args: + class_name: Name of the registered actor class + *args: Positional arguments for the class constructor + name: Optional actor name + public: Whether the actor should be publicly resolvable + restart_policy: "never", "always", or "on_failure" + max_restarts: Maximum restart attempts + min_backoff: Minimum backoff time in seconds + max_backoff: Maximum backoff time in seconds + **kwargs: Keyword arguments for the class constructor + + Returns: + {"actor_id": "...", "node_id": "...", "actor_name": "..."} + + Raises: + RuntimeError: If creation fails + """ + resp = await self._ref.ask( + Message.from_json( + "CreateActor", + { + "class_name": class_name, + "actor_name": name, + "args": args, + "kwargs": kwargs, + "public": public, + "restart_policy": restart_policy, + "max_restarts": max_restarts, + "min_backoff": min_backoff, + "max_backoff": max_backoff, + }, + ) + ) + data = resp.to_json() + if resp.msg_type == "Error" or data.get("error"): + raise RuntimeError(data.get("error", "Unknown error")) + return data + + +async def get_python_actor_service( + system: ActorSystem, node_id: int | None = None +) -> PythonActorServiceProxy: + """Get PythonActorServiceProxy for direct method calls. + + Args: + system: ActorSystem instance + node_id: Target node ID (None means local node) + + Returns: + PythonActorServiceProxy with methods: list_registry(), create_actor() + + Example: + service = await get_python_actor_service(system) + classes = await service.list_registry() + """ + service_ref = await system.resolve_named(PYTHON_ACTOR_SERVICE_NAME, node_id=node_id) + return PythonActorServiceProxy(service_ref) + + +# Legacy helper functions (for backwards compatibility) async def list_actors(system: ActorSystem) -> list[dict]: """List all actors on the current node.""" - sys_actor = await system.system() - # SystemMessage uses serde tag format - resp = await sys_actor.ask( - Message.from_json("SystemMessage", {"type": "ListActors"}) - ) - data = resp.to_json() - if data.get("type") == "Error": - raise RuntimeError(data.get("message")) - return data.get("actors", []) + proxy = await get_system_actor(system) + return await proxy.list_actors() async def get_metrics(system: ActorSystem) -> dict: """Get system metrics.""" - sys_actor = await system.system() - resp = await sys_actor.ask( - Message.from_json("SystemMessage", {"type": "GetMetrics"}) - ) - return resp.to_json() + proxy = await get_system_actor(system) + return await proxy.get_metrics() async def get_node_info(system: ActorSystem) -> dict: """Get node info.""" - sys_actor = await system.system() - resp = await sys_actor.ask( - Message.from_json("SystemMessage", {"type": "GetNodeInfo"}) - ) - return resp.to_json() + proxy = await get_system_actor(system) + return await proxy.get_node_info() async def health_check(system: ActorSystem) -> dict: """Health check.""" - sys_actor = await system.system() - resp = await sys_actor.ask( - Message.from_json("SystemMessage", {"type": "HealthCheck"}) - ) - return resp.to_json() + proxy = await get_system_actor(system) + return await proxy.health_check() async def ping(system: ActorSystem, node_id: int | None = None) -> dict: @@ -916,12 +1081,8 @@ async def ping(system: ActorSystem, node_id: int | None = None) -> dict: system: ActorSystem instance node_id: Target node ID (None means local node) """ - if node_id is None: - sys_actor = await system.system() - else: - sys_actor = await system.remote_system(node_id) - resp = await sys_actor.ask(Message.from_json("SystemMessage", {"type": "Ping"})) - return resp.to_json() + proxy = await get_system_actor(system, node_id) + return await proxy.ping() async def resolve( diff --git a/python/pulsing/actors/load_stream.py b/python/pulsing/actors/load_stream.py index ffa91bf54..293a357c1 100644 --- a/python/pulsing/actors/load_stream.py +++ b/python/pulsing/actors/load_stream.py @@ -228,10 +228,9 @@ async def _subscribe_worker(self, node_id: str): return try: # Use resolve_named instead of unbound get_actor_ref - # node_id needs to be converted from string to int - nid_int = int(node_id) + # node_id is string from members(), convert to int for resolve_named worker_ref = await self._system.resolve_named( - self._worker_name, node_id=nid_int + self._worker_name, node_id=int(node_id) ) if worker_ref: self._worker_refs[node_id] = worker_ref diff --git a/python/pulsing/actors/scheduler.py b/python/pulsing/actors/scheduler.py index bd4ccee18..751cb2a53 100644 --- a/python/pulsing/actors/scheduler.py +++ b/python/pulsing/actors/scheduler.py @@ -63,11 +63,10 @@ async def get_healthy_worker_count(self) -> int: workers = await self.get_available_workers() return sum(1 for w in workers if w.get("status") == "Alive") - async def _resolve_worker(self, node_id: str | None = None): + async def _resolve_worker(self, node_id: int | None = None): try: - # node_id is serialized as string in MemberInfo, need to convert back to int to match resolve_named - nid_int = int(node_id) if node_id else None - return await self._system.resolve_named(self._worker_name, node_id=nid_int) + # node_id is now u128 integer from members() + return await self._system.resolve_named(self._worker_name, node_id=node_id) except Exception: return None diff --git a/python/pulsing/queue/manager.py b/python/pulsing/queue/manager.py index 016f693ef..023277301 100644 --- a/python/pulsing/queue/manager.py +++ b/python/pulsing/queue/manager.py @@ -3,12 +3,15 @@ import asyncio import hashlib import logging -from typing import Any +from typing import TYPE_CHECKING, Any -from pulsing.actor import Actor, ActorId, ActorRef, ActorSystem, Message +from pulsing.actor import ActorId, ActorRef, ActorSystem, remote from .storage import BucketStorage +if TYPE_CHECKING: + from pulsing.actor.remote import ActorProxy + logger = logging.getLogger(__name__) # StorageManager fixed service name @@ -45,18 +48,20 @@ def _compute_owner(bucket_key: str, nodes: list[dict]) -> int | None: node_id = node.get("node_id") if node_id is None: continue - node_id = int(node_id) + # node_id is u128 integer, convert to string for consistent hashing + node_id_str = str(node_id) # Combine key and node_id to calculate hash score - combined = f"{bucket_key}:{node_id}" + combined = f"{bucket_key}:{node_id_str}" score = int(hashlib.md5(combined.encode()).hexdigest(), 16) if score > best_score: best_score = score - best_node_id = node_id + best_node_id = node_id # Keep as integer return best_node_id -class StorageManager(Actor): +@remote +class StorageManager: """Storage manager Actor One instance per node, responsible for: @@ -148,17 +153,18 @@ async def _get_or_create_bucket( self._buckets[key] = await self.system.resolve_named(actor_name) logger.debug(f"Resolved existing bucket: {actor_name}") except Exception: - # Create new, use specified backend or default backend - storage = BucketStorage( + # Create new using BucketStorage.local() for proper @remote wrapping + proxy = await BucketStorage.local( + self.system, bucket_id=bucket_id, storage_path=bucket_storage_path, batch_size=batch_size, backend=backend or self.default_backend, backend_options=backend_options, + name=actor_name, + public=True, ) - self._buckets[key] = await self.system.spawn( - storage, name=actor_name, public=True - ) + self._buckets[key] = proxy.ref logger.info(f"Created bucket: {actor_name} at {bucket_storage_path}") return self._buckets[key] @@ -180,192 +186,178 @@ async def _get_or_create_topic_broker(self, topic_name: str) -> ActorRef: # Lazy import to avoid circular dependency from pulsing.topic.broker import TopicBroker - broker = TopicBroker(topic_name, self.system) - self._topics[topic_name] = await self.system.spawn( - broker, name=actor_name, public=True + # Use TopicBroker.local() to create properly wrapped actor + proxy = await TopicBroker.local( + self.system, topic_name, self.system, name=actor_name, public=True ) + self._topics[topic_name] = proxy.ref logger.info(f"Created topic broker: {actor_name}") return self._topics[topic_name] - async def receive(self, msg: Message) -> Message | None: - try: - return await self._handle_message(msg) - except Exception as e: - logger.exception(f"Error handling message: {e}") - return Message.from_json("Error", {"error": str(e)}) - - async def _handle_message(self, msg: Message) -> Message | None: - msg_type = msg.msg_type - data = msg.to_json() - - if msg_type == "GetBucket": - # Request bucket reference - topic = data.get("topic") - bucket_id = data.get("bucket_id") - batch_size = data.get("batch_size", 100) - storage_path = data.get("storage_path") # Optional custom storage path - backend = data.get("backend") # Optional backend name - backend_options = data.get("backend_options") # Optional backend options - - if topic is None or bucket_id is None: - return Message.from_json( - "Error", {"error": "Missing 'topic' or 'bucket_id'"} - ) - - # Compute owner - bucket_key = self._bucket_key(topic, bucket_id) - members = await self._refresh_members() - owner_node_id = _compute_owner(bucket_key, members) - local_node_id = self.system.node_id.id - - # Determine if belongs to this node - if owner_node_id is None or owner_node_id == local_node_id: - # This node is responsible, create/return bucket - bucket_ref = await self._get_or_create_bucket( - topic, bucket_id, batch_size, storage_path, backend, backend_options - ) - return Message.from_json( - "BucketReady", - { - "_type": "BucketReady", # Fallback: msg_type may be lost across nodes - "topic": topic, - "bucket_id": bucket_id, - "actor_id": bucket_ref.actor_id.local_id, - # Use hex string to transmit node_id, avoid JSON big integer precision loss - "node_id_hex": hex(local_node_id), - }, - ) - else: - # Not owned by this node, return redirect - # Find owner node address - owner_addr = None - for m in members: - # node_id might be string, convert to int for comparison - m_node_id = m.get("node_id") - if m_node_id is not None and int(m_node_id) == owner_node_id: - owner_addr = m.get("addr") - break - - return Message.from_json( - "Redirect", - { - "_type": "Redirect", # Fallback: msg_type may be lost across nodes - "topic": topic, - "bucket_id": bucket_id, - # Use hex string to transmit node_id, avoid JSON big integer precision loss - "owner_node_id_hex": hex(owner_node_id), - "owner_addr": owner_addr, - }, - ) - - elif msg_type == "GetTopic": - # Request topic broker reference - topic_name = data.get("topic") - if not topic_name: - return Message.from_json("Error", {"error": "Missing 'topic'"}) - - # Compute owner - topic_key = self._topic_key(topic_name) - members = await self._refresh_members() - owner_node_id = _compute_owner(topic_key, members) - local_node_id = self.system.node_id.id - - if owner_node_id is None or owner_node_id == local_node_id: - # This node is responsible, create/return topic broker - broker_ref = await self._get_or_create_topic_broker(topic_name) - return Message.from_json( - "TopicReady", - { - "_type": "TopicReady", - "topic": topic_name, - "actor_id": broker_ref.actor_id.local_id, - "node_id_hex": hex(local_node_id), - }, - ) - else: - # Not owned by this node, return redirect - owner_addr = None - for m in members: - m_node_id = m.get("node_id") - if m_node_id is not None and int(m_node_id) == owner_node_id: - owner_addr = m.get("addr") - break - - return Message.from_json( - "Redirect", - { - "_type": "Redirect", - "topic": topic_name, - "owner_node_id_hex": hex(owner_node_id), - "owner_addr": owner_addr, - }, - ) + # ========== Public Remote Methods ========== - elif msg_type == "ListBuckets": - # List all buckets managed by this node - buckets = [ - {"topic": topic, "bucket_id": bid} - for (topic, bid) in self._buckets.keys() - ] - return Message.from_json("BucketList", {"buckets": buckets}) - - elif msg_type == "ListTopics": - # List all topics managed by this node - return Message.from_json("TopicList", {"topics": list(self._topics.keys())}) - - elif msg_type == "GetStats": - # Get statistics - return Message.from_json( - "Stats", - { - "node_id": self.system.node_id.id, - "bucket_count": len(self._buckets), - "topic_count": len(self._topics), - "buckets": [ - {"topic": t, "bucket_id": b} for (t, b) in self._buckets.keys() - ], - "topics": list(self._topics.keys()), - }, + async def get_bucket( + self, + topic: str, + bucket_id: int, + batch_size: int = 100, + storage_path: str | None = None, + backend: str | None = None, + backend_options: dict | None = None, + ) -> dict: + """Get bucket reference. + + Returns: + - {"_type": "BucketReady", "topic": ..., "bucket_id": ..., "actor_id": ..., "node_id": ...} + - {"_type": "Redirect", "topic": ..., "bucket_id": ..., "owner_node_id": ..., "owner_addr": ...} + """ + # Compute owner + bucket_key = self._bucket_key(topic, bucket_id) + members = await self._refresh_members() + owner_node_id = _compute_owner(bucket_key, members) + local_node_id = str(self.system.node_id.id) + + if owner_node_id is None or owner_node_id == local_node_id: + # This node is responsible, create/return bucket + bucket_ref = await self._get_or_create_bucket( + topic, bucket_id, batch_size, storage_path, backend, backend_options ) + return { + "_type": "BucketReady", + "topic": topic, + "bucket_id": bucket_id, + "actor_id": str(bucket_ref.actor_id.id), + "node_id": str(local_node_id), + } + else: + # Not owned by this node, return redirect + owner_addr = None + for m in members: + m_node_id = m.get("node_id") + if m_node_id is not None and m_node_id == owner_node_id: + owner_addr = m.get("addr") + break + return { + "_type": "Redirect", + "topic": topic, + "bucket_id": bucket_id, + "owner_node_id": str(owner_node_id), + "owner_addr": owner_addr, + } + + async def get_topic(self, topic: str) -> dict: + """Get topic broker reference. + + Returns: + - {"_type": "TopicReady", "topic": ..., "actor_id": ..., "node_id": ...} + - {"_type": "Redirect", "topic": ..., "owner_node_id": ..., "owner_addr": ...} + """ + # Compute owner + topic_key = self._topic_key(topic) + members = await self._refresh_members() + owner_node_id = _compute_owner(topic_key, members) + local_node_id = str(self.system.node_id.id) + + if owner_node_id is None or owner_node_id == local_node_id: + # This node is responsible, create/return topic broker + broker_ref = await self._get_or_create_topic_broker(topic) + return { + "_type": "TopicReady", + "topic": topic, + "actor_id": str(broker_ref.actor_id.id), + "node_id": str(local_node_id), + } else: - return Message.from_json( - "Error", {"error": f"Unknown message type: {msg_type}"} - ) + # Not owned by this node, return redirect + owner_addr = None + for m in members: + m_node_id = m.get("node_id") + if m_node_id is not None and m_node_id == owner_node_id: + owner_addr = m.get("addr") + break + + return { + "_type": "Redirect", + "topic": topic, + "owner_node_id": str(owner_node_id), + "owner_addr": owner_addr, + } + + async def list_buckets(self) -> list[dict]: + """List all buckets managed by this node. + + Returns: + List of {"topic": ..., "bucket_id": ...} + """ + return [ + {"topic": topic, "bucket_id": bid} for (topic, bid) in self._buckets.keys() + ] + + async def list_topics(self) -> list[str]: + """List all topics managed by this node. + + Returns: + List of topic names + """ + return list(self._topics.keys()) + + async def get_stats(self) -> dict: + """Get storage manager statistics. + + Returns: + {"node_id": ..., "bucket_count": ..., "topic_count": ..., "buckets": [...], "topics": [...]} + """ + return { + "node_id": str(self.system.node_id.id), + "bucket_count": len(self._buckets), + "topic_count": len(self._topics), + "buckets": [ + {"topic": t, "bucket_id": b} for (t, b) in self._buckets.keys() + ], + "topics": list(self._topics.keys()), + } # Lock to prevent concurrent creation of StorageManager _manager_lock = asyncio.Lock() -async def get_storage_manager(system: ActorSystem) -> ActorRef: - """Get StorageManager for this node, create if not exists""" +async def get_storage_manager(system: ActorSystem) -> "ActorProxy": + """Get StorageManager proxy for this node, create if not exists. + + Returns: + ActorProxy for direct method calls on StorageManager + """ local_node_id = system.node_id.id # Try to resolve local node's StorageManager try: - return await system.resolve_named(STORAGE_MANAGER_NAME, node_id=local_node_id) + return await StorageManager.resolve( + STORAGE_MANAGER_NAME, system=system, node_id=local_node_id + ) except Exception: pass async with _manager_lock: # Check local node again try: - return await system.resolve_named( - STORAGE_MANAGER_NAME, node_id=local_node_id + return await StorageManager.resolve( + STORAGE_MANAGER_NAME, system=system, node_id=local_node_id ) except Exception: pass - # Create new StorageManager + # Create new StorageManager using .local() try: - manager = StorageManager(system) - return await system.spawn(manager, name=STORAGE_MANAGER_NAME, public=True) + return await StorageManager.local( + system, system, name=STORAGE_MANAGER_NAME, public=True + ) except Exception as e: if "already exists" in str(e).lower(): - return await system.resolve_named( - STORAGE_MANAGER_NAME, node_id=local_node_id + return await StorageManager.resolve( + STORAGE_MANAGER_NAME, system=system, node_id=local_node_id ) raise @@ -390,10 +382,11 @@ async def get_bucket_ref( backend: str | type | None = None, backend_options: dict | None = None, max_redirects: int = 3, -) -> ActorRef: - """Get ActorRef for specified bucket +) -> "ActorProxy": + """Get ActorProxy for specified bucket Automatically handles redirects to ensure getting the bucket on the correct node. + Returns ActorProxy for direct method calls on BucketStorage. Args: system: Actor system @@ -405,47 +398,38 @@ async def get_bucket_ref( backend_options: Additional backend options (optional) max_redirects: Maximum redirect count """ - from pulsing.actor import ActorId, NodeId - # Request from local StorageManager first manager = await get_storage_manager(system) - for redirect_count in range(max_redirects + 1): - msg_data = { - "topic": topic, - "bucket_id": bucket_id, - "batch_size": batch_size, - } - if storage_path: - msg_data["storage_path"] = storage_path - if backend: - # If it's a class, pass class name (classes cannot be serialized across nodes) - msg_data["backend"] = ( - backend if isinstance(backend, str) else backend.__name__ - ) - if backend_options: - msg_data["backend_options"] = backend_options - - response = await manager.ask(Message.from_json("GetBucket", msg_data)) + # Convert backend class to name if needed + backend_name = None + if backend: + backend_name = backend if isinstance(backend, str) else backend.__name__ - resp_data = response.to_json() - # msg_type may be lost across nodes, use _type field as fallback - msg_type = response.msg_type or resp_data.get("_type", "") + for redirect_count in range(max_redirects + 1): + # Call manager.get_bucket() via proxy + resp_data = await manager.get_bucket( + topic=topic, + bucket_id=bucket_id, + batch_size=batch_size, + storage_path=storage_path, + backend=backend_name, + backend_options=backend_options, + ) + + msg_type = resp_data.get("_type", "") if msg_type == "BucketReady": - # Successfully got bucket - actor_id = resp_data["actor_id"] - # node_id transmitted as hex string, convert to int - node_id = int(resp_data["node_id_hex"], 16) - - bucket_actor_id = ActorId(actor_id, NodeId(node_id)) - return await system.actor_ref(bucket_actor_id) + # Successfully got bucket - resolve by actor name for typed proxy + actor_name = f"bucket_{topic}_{bucket_id}" + # Use BucketStorage.resolve to get typed ActorProxy + return await BucketStorage.resolve(actor_name, system=system) elif msg_type == "Redirect": # Need to redirect to other node - # owner_node_id transmitted as hex string, convert to int - hex_str = resp_data.get("owner_node_id_hex") - owner_node_id = int(hex_str, 16) + # owner_node_id transmitted as string, convert to int + owner_node_id_str = resp_data.get("owner_node_id") + owner_node_id = int(owner_node_id_str) owner_addr = resp_data.get("owner_addr") logger.debug( @@ -465,8 +449,8 @@ async def get_bucket_ref( max_resolve_retries = 10 for resolve_retry in range(max_resolve_retries): try: - manager = await system.resolve_named( - STORAGE_MANAGER_NAME, node_id=owner_node_id + manager = await StorageManager.resolve( + STORAGE_MANAGER_NAME, system=system, node_id=owner_node_id ) break except Exception as e: @@ -482,9 +466,6 @@ async def get_bucket_ref( f"{max_resolve_retries} retries: {e}" ) from e - elif msg_type == "Error": - raise RuntimeError(f"GetBucket failed: {resp_data.get('error')}") - else: raise RuntimeError(f"Unexpected response: {msg_type}") @@ -495,34 +476,34 @@ async def get_topic_broker( system: ActorSystem, topic: str, max_redirects: int = 3, -) -> ActorRef: - """Get broker ActorRef for specified topic +) -> "ActorProxy": + """Get broker ActorProxy for specified topic Automatically handles redirects to ensure getting the broker on the correct node. + Returns ActorProxy for direct method calls on TopicBroker. Args: system: Actor system topic: Topic name max_redirects: Maximum redirect count """ - from pulsing.actor import ActorId, NodeId + from pulsing.topic.broker import TopicBroker manager = await get_storage_manager(system) for redirect_count in range(max_redirects + 1): - response = await manager.ask(Message.from_json("GetTopic", {"topic": topic})) - - resp_data = response.to_json() - msg_type = response.msg_type or resp_data.get("_type", "") + # Call manager.get_topic() via proxy + resp_data = await manager.get_topic(topic=topic) + msg_type = resp_data.get("_type", "") if msg_type == "TopicReady": - actor_id = resp_data["actor_id"] - node_id = int(resp_data["node_id_hex"], 16) - broker_actor_id = ActorId(actor_id, NodeId(node_id)) - return await system.actor_ref(broker_actor_id) + # Successfully got topic - resolve by actor name for typed proxy + actor_name = f"_topic_broker_{topic}" + return await TopicBroker.resolve(actor_name, system=system) elif msg_type == "Redirect": - owner_node_id = int(resp_data["owner_node_id_hex"], 16) + # owner_node_id transmitted as string, convert to int + owner_node_id = int(resp_data["owner_node_id"]) logger.debug(f"Redirecting topic {topic} to node {owner_node_id}") @@ -532,11 +513,11 @@ async def get_topic_broker( if owner_node_id == system.node_id.id: raise RuntimeError(f"Redirect loop for topic: {topic}") - # Get owner node's StorageManager + # Get owner node's StorageManager via proxy for retry in range(10): try: - manager = await system.resolve_named( - STORAGE_MANAGER_NAME, node_id=owner_node_id + manager = await StorageManager.resolve( + STORAGE_MANAGER_NAME, system=system, node_id=owner_node_id ) break except Exception as e: @@ -547,9 +528,6 @@ async def get_topic_broker( f"StorageManager not found on node {owner_node_id}: {e}" ) from e - elif msg_type == "Error": - raise RuntimeError(f"GetTopic failed: {resp_data.get('error')}") - else: raise RuntimeError(f"Unexpected response: {msg_type}") diff --git a/python/pulsing/queue/queue.py b/python/pulsing/queue/queue.py index 4026ae911..8f801bd01 100644 --- a/python/pulsing/queue/queue.py +++ b/python/pulsing/queue/queue.py @@ -8,7 +8,8 @@ import logging from typing import TYPE_CHECKING, Any -from pulsing.actor import ActorRef, ActorSystem, Message +from pulsing.actor import ActorSystem +from pulsing.actor.remote import ActorProxy from .manager import get_bucket_ref, get_storage_manager @@ -57,8 +58,8 @@ def __init__( self.backend = backend self.backend_options = backend_options - # Actor references for each bucket - self._bucket_refs: dict[int, ActorRef] = {} + # Actor proxies for each bucket + self._bucket_refs: dict[int, ActorProxy] = {} self._init_lock = asyncio.Lock() # Save event loop reference (for sync wrapper) @@ -74,7 +75,7 @@ def _hash_partition(self, value: Any) -> int: hash_value = int(hashlib.md5(str(value).encode()).hexdigest(), 16) return hash_value % self.num_buckets - async def _ensure_bucket(self, bucket_id: int) -> ActorRef: + async def _ensure_bucket(self, bucket_id: int) -> ActorProxy: """Ensure Actor for specified bucket is created Get bucket reference through StorageManager: @@ -122,12 +123,10 @@ async def put( raise ValueError(f"Missing partition column '{self.bucket_column}'") bucket_id = self._hash_partition(rec[self.bucket_column]) - bucket_ref = await self._ensure_bucket(bucket_id) - - response = await bucket_ref.ask(Message.from_json("Put", {"record": rec})) - if response.msg_type == "Error": - raise RuntimeError(f"Put failed: {response.to_json().get('error')}") + bucket = await self._ensure_bucket(bucket_id) + # Direct method call via proxy + await bucket.put(rec) results.append({"bucket_id": bucket_id, "status": "ok"}) return results[0] if single else results @@ -165,44 +164,28 @@ async def _get_from_bucket( timeout: float | None, ) -> list[dict[str, Any]]: """Read data from specified bucket""" - bucket_ref = await self._ensure_bucket(bucket_id) - - # Use streaming read - response = await bucket_ref.ask( - Message.from_json( - "GetStream", - {"limit": limit, "offset": offset, "wait": wait, "timeout": timeout}, - ) - ) + bucket = await self._ensure_bucket(bucket_id) - if response.msg_type == "Error": - raise RuntimeError(f"Get failed: {response.to_json().get('error')}") - - if not response.is_stream: + # Try streaming read first via proxy + try: + records = [] + async for batch in bucket.get_stream(limit, offset, wait, timeout): + for record in batch: + records.append(record) + if len(records) >= limit: + return records + return records + except Exception: # Fallback to non-streaming - response = await bucket_ref.ask( - Message.from_json("Get", {"limit": limit, "offset": offset}) - ) - return response.to_json().get("records", []) - - records = [] - reader = response.stream_reader() - async for chunk in reader: - for record in chunk.get("records", []): - records.append(record) - if len(records) >= limit: - return records - - return records + return await bucket.get(limit, offset) async def flush(self) -> None: """Flush all bucket buffers""" tasks = [] for bucket_id in range(self.num_buckets): if bucket_id in self._bucket_refs: - tasks.append( - self._bucket_refs[bucket_id].ask(Message.from_json("Flush", {})) - ) + # Direct method call via proxy + tasks.append(self._bucket_refs[bucket_id].flush()) if tasks: await asyncio.gather(*tasks) @@ -211,10 +194,8 @@ async def stats(self) -> dict[str, Any]: bucket_stats = {} for bucket_id in range(self.num_buckets): if bucket_id in self._bucket_refs: - response = await self._bucket_refs[bucket_id].ask( - Message.from_json("Stats", {}) - ) - bucket_stats[bucket_id] = response.to_json() + # Direct method call via proxy + bucket_stats[bucket_id] = await self._bucket_refs[bucket_id].stats() return { "topic": self.topic, @@ -456,11 +437,16 @@ async def read_queue( # Try to resolve existing bucket Actors if assigned_buckets: + from .storage import BucketStorage + for bid in assigned_buckets: # Must match `StorageManager` bucket actor naming: "bucket_{topic}_{bucket_id}" actor_name = f"bucket_{topic}_{bid}" try: - queue._bucket_refs[bid] = await system.resolve_named(actor_name) + # Use BucketStorage.resolve to get typed ActorProxy + queue._bucket_refs[bid] = await BucketStorage.resolve( + actor_name, system=system + ) except Exception: pass diff --git a/python/pulsing/queue/storage.py b/python/pulsing/queue/storage.py index 98682f772..25caf1e75 100644 --- a/python/pulsing/queue/storage.py +++ b/python/pulsing/queue/storage.py @@ -2,16 +2,17 @@ import asyncio import logging -from typing import Any +from typing import Any, AsyncIterator -from pulsing.actor import Actor, ActorId, Message, StreamMessage +from pulsing.actor import ActorId, StreamMessage, remote from .backend import StorageBackend, get_backend_class logger = logging.getLogger(__name__) -class BucketStorage(Actor): +@remote +class BucketStorage: """Storage Actor for a Single Bucket Uses pluggable StorageBackend for data storage. @@ -61,64 +62,82 @@ def on_start(self, actor_id: ActorId) -> None: def on_stop(self) -> None: logger.info(f"BucketStorage[{self.bucket_id}] stopping") - async def receive(self, msg: Message) -> Message | StreamMessage | None: - msg_type = msg.msg_type - data = msg.to_json() - - if msg_type == "Put": - record = data.get("record") - if not record: - return Message.from_json("Error", {"error": "Missing 'record'"}) - - await self._backend.put(record) - return Message.from_json("PutResponse", {"status": "ok"}) - - elif msg_type == "PutBatch": - records = data.get("records") - if not records: - return Message.from_json("Error", {"error": "Missing 'records'"}) - - await self._backend.put_batch(records) - return Message.from_json( - "PutBatchResponse", {"status": "ok", "count": len(records)} - ) - - elif msg_type == "Get": - limit = data.get("limit", 100) - offset = data.get("offset", 0) - records = await self._backend.get(limit, offset) - return Message.from_json("GetResponse", {"records": records}) - - elif msg_type == "GetStream": - limit = data.get("limit", 100) - offset = data.get("offset", 0) - wait: bool = data.get("wait", False) - timeout: float | None = data.get("timeout", None) - - stream_msg, writer = StreamMessage.create("GetStream") - - async def produce(): - try: - async for records in self._backend.get_stream( - limit, offset, wait, timeout - ): - await writer.write({"records": records}) - writer.close() - except Exception as e: - logger.error(f"BucketStorage[{self.bucket_id}] stream error: {e}") - await writer.error(str(e)) - writer.close() - - asyncio.create_task(produce()) - return stream_msg - - elif msg_type == "Flush": - await self._backend.flush() - return Message.from_json("FlushResponse", {"status": "ok"}) - - elif msg_type == "Stats": - stats = await self._backend.stats() - return Message.from_json("StatsResponse", stats) - - else: - return Message.from_json("Error", {"error": f"Unknown: {msg_type}"}) + # ========== Public Remote Methods ========== + + async def put(self, record: dict) -> dict: + """Put a single record. + + Args: + record: Record to store + + Returns: + {"status": "ok"} + """ + if not record: + raise ValueError("Missing 'record'") + await self._backend.put(record) + return {"status": "ok"} + + async def put_batch(self, records: list[dict]) -> dict: + """Put multiple records. + + Args: + records: List of records to store + + Returns: + {"status": "ok", "count": N} + """ + if not records: + raise ValueError("Missing 'records'") + await self._backend.put_batch(records) + return {"status": "ok", "count": len(records)} + + async def get(self, limit: int = 100, offset: int = 0) -> list[dict]: + """Get records. + + Args: + limit: Maximum number of records to return + offset: Starting offset + + Returns: + List of records + """ + return await self._backend.get(limit, offset) + + async def get_stream( + self, + limit: int = 100, + offset: int = 0, + wait: bool = False, + timeout: float | None = None, + ) -> AsyncIterator[list[dict]]: + """Get records as a stream. + + Args: + limit: Maximum number of records to return + offset: Starting offset + wait: Whether to wait for new records + timeout: Timeout in seconds + + Yields: + Batches of records + """ + async for records in self._backend.get_stream(limit, offset, wait, timeout): + yield records + + async def flush(self) -> dict: + """Flush pending writes. + + Returns: + {"status": "ok"} + """ + await self._backend.flush() + return {"status": "ok"} + + async def stats(self) -> dict: + """Get storage statistics. + + Returns: + Statistics dict from backend + """ + return await self._backend.stats() diff --git a/python/pulsing/topic/__init__.py b/python/pulsing/topic/__init__.py index 06fc57ad1..759aab6dd 100644 --- a/python/pulsing/topic/__init__.py +++ b/python/pulsing/topic/__init__.py @@ -26,12 +26,14 @@ async def handle(msg): TopicReader, TopicWriter, read_topic, + subscribe_to_topic, write_topic, ) __all__ = [ "write_topic", "read_topic", + "subscribe_to_topic", "TopicWriter", "TopicReader", "PublishMode", diff --git a/python/pulsing/topic/broker.py b/python/pulsing/topic/broker.py index 31ac886ac..ffbf41a54 100644 --- a/python/pulsing/topic/broker.py +++ b/python/pulsing/topic/broker.py @@ -6,12 +6,12 @@ import logging import time from dataclasses import dataclass, field -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any if TYPE_CHECKING: from pulsing.actor import ActorRef, ActorSystem -from pulsing.actor import Actor, ActorId, Message +from pulsing.actor import ActorId, remote logger = logging.getLogger(__name__) @@ -35,8 +35,9 @@ class _Subscriber: consecutive_failures: int = 0 -class TopicBroker(Actor): - """Topic broker actor.""" +@remote +class TopicBroker: + """Topic broker actor with remote method support.""" def __init__(self, topic: str, system: "ActorSystem"): self.topic = topic @@ -60,42 +61,30 @@ def metadata(self) -> dict[str, str]: "subscriber_count": str(len(self._subscribers)), } - async def receive(self, msg: Message) -> Message | None: - try: - return await self._handle(msg) - except Exception as e: - logger.exception(f"TopicBroker[{self.topic}] error: {e}") - return Message.from_json("Error", {"error": str(e)}) - - async def _handle(self, msg: Message) -> Message | None: - data = msg.to_json() - - if msg.msg_type == "Subscribe": - return await self._subscribe(data) - elif msg.msg_type == "Unsubscribe": - return await self._unsubscribe(data) - elif msg.msg_type == "Publish": - return await self._publish(data) - elif msg.msg_type == "GetStats": - return self._stats() - else: - return Message.from_json("Error", {"error": f"Unknown: {msg.msg_type}"}) - - async def _subscribe(self, data: dict) -> Message: - subscriber_id = data.get("subscriber_id") - actor_name = data.get("actor_name") - node_id = data.get("node_id") + # ========== Public Remote Methods ========== + async def subscribe( + self, + subscriber_id: str, + actor_name: str, + node_id: int | None = None, + ) -> dict: + """Subscribe an actor to this topic. + + Args: + subscriber_id: Unique subscriber identifier + actor_name: Name of the actor to receive messages + node_id: Optional node ID (for cross-node subscriptions) + + Returns: + {"success": True, "topic": "..."} + """ if not subscriber_id or not actor_name: - return Message.from_json( - "Error", {"error": "Missing subscriber_id or actor_name"} - ) + raise ValueError("Missing subscriber_id or actor_name") async with self._lock: if subscriber_id in self._subscribers: - return Message.from_json( - "SubscribeResult", {"success": True, "already": True} - ) + return {"success": True, "already": True} self._subscribers[subscriber_id] = _Subscriber( subscriber_id=subscriber_id, @@ -103,52 +92,49 @@ async def _subscribe(self, data: dict) -> Message: node_id=node_id, ) logger.debug(f"TopicBroker[{self.topic}] +subscriber: {subscriber_id}") - return Message.from_json( - "SubscribeResult", {"success": True, "topic": self.topic} - ) + return {"success": True, "topic": self.topic} + + async def unsubscribe(self, subscriber_id: str) -> dict: + """Unsubscribe from this topic. - async def _unsubscribe(self, data: dict) -> Message: - subscriber_id = data.get("subscriber_id") + Args: + subscriber_id: Subscriber ID to remove + + Returns: + {"success": True/False} + """ if not subscriber_id: - return Message.from_json("Error", {"error": "Missing subscriber_id"}) + raise ValueError("Missing subscriber_id") async with self._lock: if subscriber_id in self._subscribers: del self._subscribers[subscriber_id] logger.debug(f"TopicBroker[{self.topic}] -subscriber: {subscriber_id}") - return Message.from_json("UnsubscribeResult", {"success": True}) - return Message.from_json("UnsubscribeResult", {"success": False}) - - async def _resolve(self, sub: _Subscriber) -> "ActorRef | None": - now = time.time() - - if sub._ref is not None and (now - sub._ref_resolved_at) < REF_TTL_SECONDS: - return sub._ref - - try: - sub._ref = await self.system.resolve_named( - sub.actor_name, node_id=sub.node_id - ) - sub._ref_resolved_at = now - return sub._ref - except Exception as e: - logger.warning(f"Failed to resolve {sub.subscriber_id}: {e}") - sub._ref = None - sub._ref_resolved_at = 0 - return None - - async def _publish(self, data: dict) -> Message: - payload = data.get("payload") - mode = data.get("mode", "fire_and_forget") - sender_id = data.get("sender_id") + return {"success": True} + return {"success": False} + async def publish( + self, + payload: Any, + mode: str = "fire_and_forget", + sender_id: str | None = None, + timeout: float = DEFAULT_FANOUT_TIMEOUT, + ) -> dict: + """Publish a message to all subscribers. + + Args: + payload: Message payload + mode: "fire_and_forget", "wait_all_acks", "wait_any_ack", "best_effort" + sender_id: Optional sender ID (excluded from delivery) + timeout: Timeout for ack modes + + Returns: + {"success": True, "delivered": N, "failed": N, "subscriber_count": N} + """ self._total_published += 1 if not self._subscribers: - return Message.from_json( - "PublishResult", - {"success": True, "delivered": 0, "failed": 0, "subscriber_count": 0}, - ) + return {"success": True, "delivered": 0, "failed": 0, "subscriber_count": 0} envelope = { "topic": self.topic, @@ -160,13 +146,51 @@ async def _publish(self, data: dict) -> Message: if mode == "fire_and_forget": return await self._fanout_tell(envelope, sender_id) elif mode == "wait_all_acks": - return await self._fanout_ask(envelope, sender_id, wait_all=True) + return await self._fanout_ask( + envelope, sender_id, wait_all=True, timeout=timeout + ) elif mode == "wait_any_ack": - return await self._fanout_ask(envelope, sender_id, wait_all=False) + return await self._fanout_ask( + envelope, sender_id, wait_all=False, timeout=timeout + ) elif mode == "best_effort": return await self._fanout_best_effort(envelope, sender_id) else: - return Message.from_json("Error", {"error": f"Unknown mode: {mode}"}) + raise ValueError(f"Unknown mode: {mode}") + + def get_stats(self) -> dict: + """Get topic statistics. + + Returns: + {"topic": "...", "subscriber_count": N, "total_published": N, ...} + """ + return { + "topic": self.topic, + "subscriber_count": len(self._subscribers), + "total_published": self._total_published, + "total_delivered": self._total_delivered, + "total_failed": self._total_failed, + } + + # ========== Internal Methods ========== + + async def _resolve(self, sub: _Subscriber) -> "ActorRef | None": + now = time.time() + + if sub._ref is not None and (now - sub._ref_resolved_at) < REF_TTL_SECONDS: + return sub._ref + + try: + sub._ref = await self.system.resolve_named( + sub.actor_name, node_id=sub.node_id + ) + sub._ref_resolved_at = now + return sub._ref + except Exception as e: + logger.warning(f"Failed to resolve {sub.subscriber_id}: {e}") + sub._ref = None + sub._ref_resolved_at = 0 + return None def _record_success(self, sub: _Subscriber) -> None: sub.messages_delivered += 1 @@ -188,7 +212,7 @@ async def _evict_zombies(self, zombie_ids: list[str]) -> None: f"TopicBroker[{self.topic}] evicted zombie subscriber: {sub_id}" ) - async def _fanout_tell(self, envelope: dict, sender_id: str | None) -> Message: + async def _fanout_tell(self, envelope: dict, sender_id: str | None) -> dict: sent = 0 failed = 0 zombies: list[str] = [] @@ -216,15 +240,12 @@ async def _fanout_tell(self, envelope: dict, sender_id: str | None) -> Message: self._total_delivered += sent self._total_failed += failed - return Message.from_json( - "PublishResult", - { - "success": True, - "delivered": sent, - "failed": failed, - "subscriber_count": len(self._subscribers), - }, - ) + return { + "success": True, + "delivered": sent, + "failed": failed, + "subscriber_count": len(self._subscribers), + } async def _fanout_ask( self, @@ -232,7 +253,7 @@ async def _fanout_ask( sender_id: str | None, wait_all: bool, timeout: float = DEFAULT_FANOUT_TIMEOUT, - ) -> Message: + ) -> dict: """Wait for ack mode.""" tasks = [] sub_ids = [] @@ -251,10 +272,7 @@ async def _fanout_ask( if not tasks: await self._evict_zombies(resolve_failed) - return Message.from_json( - "PublishResult", - {"success": True, "delivered": 0, "failed": 0, "subscriber_count": 0}, - ) + return {"success": True, "delivered": 0, "failed": 0, "subscriber_count": 0} delivered = 0 failed = 0 @@ -302,39 +320,31 @@ async def _fanout_ask( if not task.exception(): delivered = 1 break - # Cancel other pending tasks (local cancellation, remote relies on RST_STREAM) + # Cancel other pending tasks for task in pending: task.cancel() except asyncio.TimeoutError: - # Timeout: no response logger.warning( f"TopicBroker[{self.topic}] wait_any_ack timeout after {timeout}s" ) - # Cancel all tasks for task in tasks: if not task.done(): task.cancel() - # Evict zombie subscribers await self._evict_zombies(zombies) self._total_delivered += delivered self._total_failed += failed - return Message.from_json( - "PublishResult", - { - "success": delivered > 0 or failed == 0, - "delivered": delivered, - "failed": failed, - "failed_subscribers": failed_ids, - "subscriber_count": len(self._subscribers), - }, - ) - - async def _fanout_best_effort( - self, envelope: dict, sender_id: str | None - ) -> Message: + return { + "success": delivered > 0 or failed == 0, + "delivered": delivered, + "failed": failed, + "failed_subscribers": failed_ids, + "subscriber_count": len(self._subscribers), + } + + async def _fanout_best_effort(self, envelope: dict, sender_id: str | None) -> dict: """Best-effort: try to send, record failures""" delivered = 0 failed = 0 @@ -361,31 +371,15 @@ async def _fanout_best_effort( if self._record_failure(sub): zombies.append(sub_id) - # Evict zombie subscribers await self._evict_zombies(zombies) self._total_delivered += delivered self._total_failed += failed - return Message.from_json( - "PublishResult", - { - "success": True, - "delivered": delivered, - "failed": failed, - "failed_subscribers": failed_ids, - "subscriber_count": len(self._subscribers), - }, - ) - - def _stats(self) -> Message: - return Message.from_json( - "TopicStats", - { - "topic": self.topic, - "subscriber_count": len(self._subscribers), - "total_published": self._total_published, - "total_delivered": self._total_delivered, - "total_failed": self._total_failed, - }, - ) + return { + "success": True, + "delivered": delivered, + "failed": failed, + "failed_subscribers": failed_ids, + "subscriber_count": len(self._subscribers), + } diff --git a/python/pulsing/topic/topic.py b/python/pulsing/topic/topic.py index fcca66739..caeea056c 100644 --- a/python/pulsing/topic/topic.py +++ b/python/pulsing/topic/topic.py @@ -11,6 +11,7 @@ if TYPE_CHECKING: from pulsing.actor import ActorRef + from pulsing.actor.remote import ActorProxy from pulsing.actor import Actor, ActorId, ActorSystem, Message @@ -44,13 +45,44 @@ class PublishResult: MessageCallback = Callable[[Any], Coroutine[Any, Any, Any] | Any] -async def _get_broker(system: ActorSystem, topic: str) -> "ActorRef": - """Get topic broker (reuses queue/manager infrastructure)""" +async def _get_broker(system: ActorSystem, topic: str) -> "ActorProxy": + """Get topic broker proxy (reuses queue/manager infrastructure)""" from pulsing.queue.manager import get_topic_broker + # get_topic_broker already returns ActorProxy (via TopicBroker.resolve) return await get_topic_broker(system, topic) +async def subscribe_to_topic( + system: ActorSystem, + topic: str, + subscriber_id: str, + actor_name: str, + node_id: int | None = None, +) -> dict: + """Subscribe an actor to a topic. + + This is a helper function for manually registering subscribers with a topic broker. + For normal usage, prefer using TopicReader which handles this automatically. + + Args: + system: ActorSystem instance + topic: Topic name + subscriber_id: Unique subscriber identifier + actor_name: Name of the actor to receive messages + node_id: Optional node ID (defaults to local node) + + Returns: + Response dict from broker + + Raises: + RuntimeError: If subscription fails + """ + broker = await _get_broker(system, topic) + # Direct method call on broker proxy + return await broker.subscribe(subscriber_id, actor_name, node_id) + + class TopicWriter: """Topic write handle""" @@ -58,7 +90,7 @@ def __init__(self, system: ActorSystem, topic: str, writer_id: str | None = None self._system = system self._topic = topic self._writer_id = writer_id or f"writer_{uuid.uuid4().hex[:8]}" - self._broker: "ActorRef | None" = None + self._broker: "ActorProxy | None" = None @property def topic(self) -> str: @@ -68,7 +100,7 @@ def topic(self) -> str: def writer_id(self) -> str: return self._writer_id - async def _broker_ref(self) -> "ActorRef": + async def _broker_ref(self) -> "ActorProxy": if self._broker is None: self._broker = await _get_broker(self._system, self._topic) return self._broker @@ -101,23 +133,16 @@ async def publish( effective_timeout = timeout if timeout is not None else DEFAULT_PUBLISH_TIMEOUT async def _do_publish(): - return await broker.ask( - Message.from_json( - "Publish", - { - "payload": message, - "mode": mode.value, - "sender_id": self._writer_id, - }, - ) + # Direct method call on broker proxy + return await broker.publish( + message, + mode=mode.value, + sender_id=self._writer_id, + timeout=effective_timeout, ) - response = await asyncio.wait_for(_do_publish(), timeout=effective_timeout) + data = await asyncio.wait_for(_do_publish(), timeout=effective_timeout) - if response.msg_type == "Error": - raise RuntimeError(response.to_json().get("error")) - - data = response.to_json() return PublishResult( success=data.get("success", False), delivered=data.get("delivered", 0), @@ -129,8 +154,8 @@ async def _do_publish(): async def stats(self) -> dict[str, Any]: """Get topic statistics""" broker = await self._broker_ref() - response = await broker.ask(Message.from_json("GetStats", {})) - return response.to_json() + # Direct method call on broker proxy + return await broker.get_stats() class _SubscriberActor(Actor): @@ -237,22 +262,14 @@ async def start(self) -> None: subscriber, name=actor_name, public=True ) - # Register with broker + # Register with broker using direct method call broker = await _get_broker(self._system, self._topic) - response = await broker.ask( - Message.from_json( - "Subscribe", - { - "subscriber_id": self._reader_id, - "actor_name": actor_name, - "node_id": self._system.node_id.id, - }, - ) + await broker.subscribe( + self._reader_id, + actor_name, + node_id=self._system.node_id.id, ) - if response.msg_type == "Error": - raise RuntimeError(f"Subscribe failed: {response.to_json().get('error')}") - self._started = True logger.debug(f"TopicReader[{self._reader_id}] started for topic: {self._topic}") @@ -261,12 +278,10 @@ async def stop(self) -> None: if not self._started: return - # Unsubscribe from broker + # Unsubscribe from broker using direct method call try: broker = await _get_broker(self._system, self._topic) - await broker.ask( - Message.from_json("Unsubscribe", {"subscriber_id": self._reader_id}) - ) + await broker.unsubscribe(self._reader_id) except Exception as e: logger.warning(f"Unsubscribe error: {e}") @@ -285,8 +300,8 @@ async def stop(self) -> None: async def stats(self) -> dict[str, Any]: """Get topic statistics""" broker = await _get_broker(self._system, self._topic) - response = await broker.ask(Message.from_json("GetStats", {})) - return response.to_json() + # Direct method call on broker proxy + return await broker.get_stats() async def write_topic( diff --git a/tests/python/test_queue.py b/tests/python/test_queue.py index 5946f7a63..4433f4824 100644 --- a/tests/python/test_queue.py +++ b/tests/python/test_queue.py @@ -946,43 +946,86 @@ async def test_data_integrity_under_stress(actor_system, temp_storage_path): @pytest.mark.asyncio async def test_bucket_storage_direct(actor_system, temp_storage_path): - """Test BucketStorage actor directly with memory backend.""" - storage = BucketStorage( + """Test BucketStorage actor directly with memory backend via proxy.""" + # Use BucketStorage.local() to create properly wrapped actor with proxy + bucket = await BucketStorage.local( + actor_system, bucket_id=0, storage_path=f"{temp_storage_path}/direct_bucket", batch_size=5, backend="memory", + name="test_bucket", ) - # Spawn actor - actor_ref = await actor_system.spawn(storage, name="test_bucket") - - from pulsing.actor import Message - - # Put records + # Put records via proxy method for i in range(10): - response = await actor_ref.ask( - Message.from_json("Put", {"record": {"id": f"test_{i}", "value": i}}) - ) - assert response.to_json().get("status") == "ok" + result = await bucket.put({"id": f"test_{i}", "value": i}) + assert result["status"] == "ok" - # Get stats - stats_response = await actor_ref.ask(Message.from_json("Stats", {})) - stats = stats_response.to_json() + # Get stats via proxy method + stats = await bucket.stats() assert stats["bucket_id"] == 0 assert stats["total_count"] == 10 assert stats["backend"] == "memory" # Flush (no-op for memory backend) - await actor_ref.ask(Message.from_json("Flush", {})) + await bucket.flush() # Data should still be there - stats_response = await actor_ref.ask(Message.from_json("Stats", {})) - stats = stats_response.to_json() + stats = await bucket.stats() assert stats["total_count"] == 10 +@pytest.mark.asyncio +async def test_bucket_storage_get(actor_system, temp_storage_path): + """Test BucketStorage get method via proxy.""" + bucket = await BucketStorage.local( + actor_system, + bucket_id=0, + storage_path=f"{temp_storage_path}/get_bucket", + batch_size=5, + backend="memory", + name="test_bucket_get", + ) + + # Put records + for i in range(10): + await bucket.put({"id": f"test_{i}", "value": i}) + + # Get records via proxy + records = await bucket.get(limit=10, offset=0) + assert len(records) == 10 + + # Get with limit + records = await bucket.get(limit=5) + assert len(records) == 5 + + +@pytest.mark.asyncio +async def test_bucket_storage_put_batch(actor_system, temp_storage_path): + """Test BucketStorage put_batch method via proxy.""" + bucket = await BucketStorage.local( + actor_system, + bucket_id=0, + storage_path=f"{temp_storage_path}/batch_bucket", + batch_size=100, + backend="memory", + name="test_bucket_batch", + ) + + # Put batch of records + records = [{"id": f"batch_{i}", "value": i * 10} for i in range(20)] + result = await bucket.put_batch(records) + + assert result["status"] == "ok" + assert result["count"] == 20 + + # Verify via stats + stats = await bucket.stats() + assert stats["total_count"] == 20 + + # ============================================================================ # Sync Queue Tests # ============================================================================ diff --git a/tests/python/test_queue_backends.py b/tests/python/test_queue_backends.py index 45ab2e72e..9d9b5ce85 100644 --- a/tests/python/test_queue_backends.py +++ b/tests/python/test_queue_backends.py @@ -249,28 +249,24 @@ class TestBucketStorageWithBackend: async def test_bucket_storage_with_memory_backend( self, actor_system, temp_storage_path ): - """Test BucketStorage with memory backend.""" - from pulsing.actor import Message - - storage = BucketStorage( + """Test BucketStorage with memory backend via proxy.""" + # Use BucketStorage.local() for proper @remote wrapping + bucket = await BucketStorage.local( + actor_system, bucket_id=0, storage_path=f"{temp_storage_path}/bucket_memory", batch_size=10, backend="memory", + name="bucket_memory_test", ) - actor_ref = await actor_system.spawn(storage, name="bucket_memory_test") - - # Put records + # Put records via proxy method for i in range(5): - response = await actor_ref.ask( - Message.from_json("Put", {"record": {"id": f"test_{i}", "value": i}}) - ) - assert response.to_json().get("status") == "ok" + result = await bucket.put({"id": f"test_{i}", "value": i}) + assert result["status"] == "ok" - # Get stats - stats_response = await actor_ref.ask(Message.from_json("Stats", {})) - stats = stats_response.to_json() + # Get stats via proxy method + stats = await bucket.stats() assert stats["bucket_id"] == 0 assert stats["total_count"] == 5 @@ -278,30 +274,25 @@ async def test_bucket_storage_with_memory_backend( @pytest.mark.asyncio async def test_bucket_storage_put_batch(self, actor_system, temp_storage_path): - """Test BucketStorage PutBatch message.""" - from pulsing.actor import Message - - storage = BucketStorage( + """Test BucketStorage put_batch method via proxy.""" + # Use BucketStorage.local() for proper @remote wrapping + bucket = await BucketStorage.local( + actor_system, bucket_id=0, storage_path=f"{temp_storage_path}/bucket_batch", batch_size=100, backend="memory", + name="bucket_batch_test", ) - actor_ref = await actor_system.spawn(storage, name="bucket_batch_test") - - # Put batch + # Put batch via proxy method records = [{"id": f"batch_{i}", "value": i} for i in range(10)] - response = await actor_ref.ask( - Message.from_json("PutBatch", {"records": records}) - ) - result = response.to_json() - assert result.get("status") == "ok" - assert result.get("count") == 10 + result = await bucket.put_batch(records) + assert result["status"] == "ok" + assert result["count"] == 10 - # Verify - stats_response = await actor_ref.ask(Message.from_json("Stats", {})) - stats = stats_response.to_json() + # Verify via stats + stats = await bucket.stats() assert stats["total_count"] == 10 @@ -472,8 +463,7 @@ def total_count(self) -> int: async def test_custom_backend_with_bucket_storage( self, actor_system, temp_storage_path ): - """Test custom backend with BucketStorage actor.""" - from pulsing.actor import Message + """Test custom backend with BucketStorage actor via proxy.""" class TrackingBackend: """Backend that tracks all operations.""" @@ -530,24 +520,24 @@ def total_count(self) -> int: # Register and use register_backend("tracking", TrackingBackend) - storage = BucketStorage( + # Use BucketStorage.local() for proper @remote wrapping + bucket = await BucketStorage.local( + actor_system, bucket_id=0, storage_path=f"{temp_storage_path}/tracking_test", batch_size=100, backend="tracking", + name="tracking_bucket", ) - actor_ref = await actor_system.spawn(storage, name="tracking_bucket") - - # Perform operations - await actor_ref.ask(Message.from_json("Put", {"record": {"id": "1"}})) - await actor_ref.ask(Message.from_json("Put", {"record": {"id": "2"}})) - await actor_ref.ask(Message.from_json("Get", {"limit": 10, "offset": 0})) - await actor_ref.ask(Message.from_json("Flush", {})) + # Perform operations via proxy methods + await bucket.put({"id": "1"}) + await bucket.put({"id": "2"}) + await bucket.get(limit=10, offset=0) + await bucket.flush() # Check tracking - stats_response = await actor_ref.ask(Message.from_json("Stats", {})) - stats = stats_response.to_json() + stats = await bucket.stats() assert stats["backend"] == "tracking" assert "put" in stats["operations"] diff --git a/tests/python/test_system_actor.py b/tests/python/test_system_actor.py index 5d602cbf6..527bdaf05 100644 --- a/tests/python/test_system_actor.py +++ b/tests/python/test_system_actor.py @@ -2,23 +2,16 @@ Tests for SystemActor functionality. Covers: -- Rust SystemActor (system/core) operations -- Python ActorService (_python_actor_service) operations -- System helper functions (list_actors, get_metrics, etc.) +- Rust SystemActor (system/core) operations via SystemActorProxy +- Python ActorService (system/python_actor_service) operations via PythonActorServiceProxy """ import asyncio import pytest import pulsing as pul from pulsing.actor import ( - Actor, - ActorId, - Message, - list_actors, - get_metrics, - get_node_info, - health_check, - ping, + get_python_actor_service, + get_system_actor, remote, ) @@ -36,6 +29,18 @@ async def system(): await system.shutdown() +@pytest.fixture +async def sys_proxy(system): + """Create a SystemActorProxy for the test system.""" + return await get_system_actor(system) + + +@pytest.fixture +async def service_proxy(system): + """Create a PythonActorServiceProxy for the test system.""" + return await get_python_actor_service(system) + + # ============================================================================ # Test: System Auto-Registration # ============================================================================ @@ -58,55 +63,45 @@ async def test_python_actor_service_auto_registered(system): # ============================================================================ -# Test: SystemActor Reference +# Test: SystemActorProxy # ============================================================================ @pytest.mark.asyncio -async def test_get_system_actor_reference(system): - """Should be able to get SystemActor reference.""" - sys_ref = await system.system() - assert sys_ref is not None - assert sys_ref.is_local() +async def test_get_system_actor_proxy(system): + """Should be able to get SystemActorProxy.""" + sys_proxy = await get_system_actor(system) + assert sys_proxy is not None + assert sys_proxy.ref is not None + assert sys_proxy.ref.is_local() # ============================================================================ -# Test: Ping +# Test: Ping via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_ping_local(system): - """Ping should return Pong with node info.""" - result = await ping(system) +async def test_ping_via_proxy(sys_proxy, system): + """Ping via SystemActorProxy should return Pong with node info.""" + result = await sys_proxy.ping() assert result["type"] == "Pong" assert "node_id" in result assert "timestamp" in result - assert result["node_id"] == system.node_id.id - - -@pytest.mark.asyncio -async def test_ping_direct_message(system): - """Direct ping message to SystemActor.""" - sys_ref = await system.system() - msg = Message.from_json("SystemMessage", {"type": "Ping"}) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - assert data["type"] == "Pong" - assert data["node_id"] == system.node_id.id + # node_id is serialized as string in JSON for u128 precision + assert int(result["node_id"]) == system.node_id.id # ============================================================================ -# Test: Health Check +# Test: Health Check via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_health_check(system): - """Health check should return healthy status.""" - result = await health_check(system) +async def test_health_check_via_proxy(sys_proxy): + """Health check via SystemActorProxy should return healthy status.""" + result = await sys_proxy.health_check() assert result["type"] == "Health" assert result["status"] == "healthy" @@ -114,38 +109,27 @@ async def test_health_check(system): assert "uptime_secs" in result -@pytest.mark.asyncio -async def test_health_check_direct_message(system): - """Direct health check message to SystemActor.""" - sys_ref = await system.system() - msg = Message.from_json("SystemMessage", {"type": "HealthCheck"}) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - assert data["type"] == "Health" - assert data["status"] == "healthy" - - # ============================================================================ -# Test: Get Node Info +# Test: Get Node Info via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_get_node_info(system): - """Should return node information.""" - result = await get_node_info(system) +async def test_get_node_info_via_proxy(sys_proxy, system): + """Should return node information via SystemActorProxy.""" + result = await sys_proxy.get_node_info() assert result["type"] == "NodeInfo" - assert result["node_id"] == system.node_id.id + # node_id is serialized as string in JSON for u128 precision + assert int(result["node_id"]) == system.node_id.id assert "addr" in result assert "uptime_secs" in result @pytest.mark.asyncio -async def test_get_node_info_address_format(system): +async def test_get_node_info_address_format(sys_proxy): """Node address should be in IP:port format.""" - result = await get_node_info(system) + result = await sys_proxy.get_node_info() addr = result["addr"] # Should contain port separator @@ -153,14 +137,14 @@ async def test_get_node_info_address_format(system): # ============================================================================ -# Test: Get Metrics +# Test: Get Metrics via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_get_metrics(system): - """Should return system metrics.""" - result = await get_metrics(system) +async def test_get_metrics_via_proxy(sys_proxy): + """Should return system metrics via SystemActorProxy.""" + result = await sys_proxy.get_metrics() assert result["type"] == "Metrics" assert "actors_count" in result @@ -171,113 +155,61 @@ async def test_get_metrics(system): @pytest.mark.asyncio -async def test_metrics_message_count_increases(system): +async def test_metrics_message_count_increases(sys_proxy): """Message count should increase with each message.""" # Get initial count - result1 = await get_metrics(system) + result1 = await sys_proxy.get_metrics() initial_count = result1["messages_total"] # Send a few more messages - await ping(system) - await ping(system) + await sys_proxy.ping() + await sys_proxy.ping() # Get new count - result2 = await get_metrics(system) + result2 = await sys_proxy.get_metrics() new_count = result2["messages_total"] assert new_count > initial_count # ============================================================================ -# Test: List Actors +# Test: List Actors via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_list_actors_empty_initially(system): +async def test_list_actors_via_proxy(sys_proxy): """Actor list should be empty initially (only system actors).""" - result = await list_actors(system) + result = await sys_proxy.list_actors() # Should be empty or only contain system actors assert isinstance(result, list) -@pytest.mark.asyncio -async def test_list_actors_direct_message(system): - """Direct ListActors message to SystemActor.""" - sys_ref = await system.system() - msg = Message.from_json("SystemMessage", {"type": "ListActors"}) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - assert data["type"] == "ActorList" - assert "actors" in data - - # ============================================================================ -# Test: GetActor +# Test: PythonActorServiceProxy # ============================================================================ @pytest.mark.asyncio -async def test_get_actor_not_found(system): - """GetActor should return error for non-existent actor.""" - sys_ref = await system.system() - msg = Message.from_json( - "SystemMessage", {"type": "GetActor", "name": "nonexistent"} - ) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - assert data["type"] == "Error" - assert "not found" in data["message"].lower() - - -# ============================================================================ -# Test: CreateActor (should fail in pure Rust mode) -# ============================================================================ +async def test_get_python_actor_service_proxy(system): + """Should be able to get PythonActorServiceProxy.""" + service_proxy = await get_python_actor_service(system) + assert service_proxy is not None + assert service_proxy.ref is not None @pytest.mark.asyncio -async def test_create_actor_not_supported_in_rust(system): - """CreateActor should return error in pure Rust SystemActor.""" - sys_ref = await system.system() - msg = Message.from_json( - "SystemMessage", - { - "type": "CreateActor", - "actor_type": "Counter", - "name": "test_counter", - "params": {}, - "public": True, - }, - ) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - assert data["type"] == "Error" - assert "not supported" in data["message"].lower() +async def test_list_registry_via_proxy(service_proxy): + """PythonActorServiceProxy should list registered actor classes.""" + classes = await service_proxy.list_registry() - -# ============================================================================ -# Test: PythonActorService -# ============================================================================ - - -@pytest.mark.asyncio -async def test_python_actor_service_list_registry(system): - """PythonActorService should list registered actor classes.""" - service_ref = await system.resolve_named("system/python_actor_service") - msg = Message.from_json("ListRegistry", {}) - resp = await service_ref.ask(msg) - data = resp.to_json() - - assert data.get("classes") is not None - assert isinstance(data["classes"], list) + assert classes is not None + assert isinstance(classes, list) # ============================================================================ -# Test: @remote with PythonActorService +# Test: @remote with PythonActorServiceProxy # ============================================================================ @@ -310,43 +242,40 @@ async def test_remote_local_creation(system): @pytest.mark.asyncio -async def test_remote_class_registered(system): +async def test_remote_class_registered(service_proxy): """@remote decorated class should be registered in global registry.""" - service_ref = await system.resolve_named("system/python_actor_service") - msg = Message.from_json("ListRegistry", {}) - resp = await service_ref.ask(msg) - data = resp.to_json() + classes = await service_proxy.list_registry() # TestCounter should be in the registry - class_names = data.get("classes", []) - assert any("TestCounter" in name for name in class_names) + assert any("TestCounter" in name for name in classes) # ============================================================================ -# Test: Multiple Concurrent Requests +# Test: Multiple Concurrent Requests via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_concurrent_ping_requests(system): - """SystemActor should handle concurrent requests.""" - tasks = [ping(system) for _ in range(10)] +async def test_concurrent_ping_requests(sys_proxy, system): + """SystemActor should handle concurrent requests via proxy.""" + tasks = [sys_proxy.ping() for _ in range(10)] results = await asyncio.gather(*tasks) for result in results: assert result["type"] == "Pong" - assert result["node_id"] == system.node_id.id + # node_id is serialized as string in JSON for u128 precision + assert int(result["node_id"]) == system.node_id.id @pytest.mark.asyncio -async def test_concurrent_mixed_requests(system): - """SystemActor should handle mixed concurrent requests.""" +async def test_concurrent_mixed_requests(sys_proxy): + """SystemActor should handle mixed concurrent requests via proxy.""" tasks = [ - ping(system), - health_check(system), - get_node_info(system), - get_metrics(system), - list_actors(system), + sys_proxy.ping(), + sys_proxy.health_check(), + sys_proxy.get_node_info(), + sys_proxy.get_metrics(), + sys_proxy.list_actors(), ] results = await asyncio.gather(*tasks) @@ -358,49 +287,50 @@ async def test_concurrent_mixed_requests(system): # ============================================================================ -# Test: Error Handling +# Test: Uptime via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_invalid_message_type(system): - """SystemActor should handle invalid message types gracefully.""" - sys_ref = await system.system() - msg = Message.from_json("SystemMessage", {"type": "InvalidType"}) - resp = await sys_ref.ask(msg) - data = resp.to_json() - - # Should return error for unknown message type - assert data["type"] == "Error" +async def test_uptime_increases(sys_proxy): + """Uptime should increase over time.""" + result1 = await sys_proxy.get_node_info() + uptime1 = result1["uptime_secs"] + await asyncio.sleep(1.1) -@pytest.mark.asyncio -async def test_malformed_message(system): - """SystemActor should handle malformed messages gracefully.""" - sys_ref = await system.system() - # Send a message without proper format - msg = Message.from_json("BadMessage", {"foo": "bar"}) - resp = await sys_ref.ask(msg) - data = resp.to_json() + result2 = await sys_proxy.get_node_info() + uptime2 = result2["uptime_secs"] - # Should return error - assert data["type"] == "Error" + assert uptime2 >= uptime1 # ============================================================================ -# Test: Uptime +# Test: Remote Node Access via Proxy # ============================================================================ @pytest.mark.asyncio -async def test_uptime_increases(system): - """Uptime should increase over time.""" - result1 = await get_node_info(system) - uptime1 = result1["uptime_secs"] +async def test_get_system_actor_for_remote_node(system): + """get_system_actor with node_id should work (for cluster scenarios).""" + # For local testing, use local node's ID + local_node_id = system.node_id.id - await asyncio.sleep(1.1) + # This should work even with local node_id + sys_proxy = await get_system_actor(system, node_id=local_node_id) + result = await sys_proxy.ping() - result2 = await get_node_info(system) - uptime2 = result2["uptime_secs"] + assert result["type"] == "Pong" - assert uptime2 >= uptime1 + +@pytest.mark.asyncio +async def test_get_python_actor_service_for_remote_node(system): + """get_python_actor_service with node_id should work (for cluster scenarios).""" + # For local testing, use local node's ID + local_node_id = system.node_id.id + + # This should work even with local node_id + service_proxy = await get_python_actor_service(system, node_id=local_node_id) + classes = await service_proxy.list_registry() + + assert isinstance(classes, list) diff --git a/tests/python/test_topic.py b/tests/python/test_topic.py index 431350611..2871eaf2d 100644 --- a/tests/python/test_topic.py +++ b/tests/python/test_topic.py @@ -729,7 +729,6 @@ async def test_double_start_stop(actor_system): async def test_topic_broker_via_storage_manager(actor_system): """Test that topic broker is created via StorageManager.""" from pulsing.queue.manager import get_storage_manager - from pulsing.actor import Message # Ensure StorageManager exists manager = await get_storage_manager(actor_system) @@ -738,9 +737,8 @@ async def test_topic_broker_via_storage_manager(actor_system): writer = await write_topic(actor_system, "sm_integration_topic") await writer.publish({"test": True}) - # Check stats include topics - response = await manager.ask(Message.from_json("GetStats", {})) - stats = response.to_json() + # Check stats include topics via proxy method + stats = await manager.get_stats() assert "topic_count" in stats assert stats["topic_count"] >= 1 @@ -751,7 +749,6 @@ async def test_topic_broker_via_storage_manager(actor_system): async def test_list_topics(actor_system): """Test listing topics via StorageManager.""" from pulsing.queue.manager import get_storage_manager - from pulsing.actor import Message # Create some topics await write_topic(actor_system, "list_topic_1") @@ -763,13 +760,12 @@ async def test_list_topics(actor_system): await w1.publish({"test": 1}) await w2.publish({"test": 2}) + # List topics via proxy method manager = await get_storage_manager(actor_system) - response = await manager.ask(Message.from_json("ListTopics", {})) - data = response.to_json() + topics = await manager.list_topics() - assert "topics" in data - assert "list_topic_1" in data["topics"] - assert "list_topic_2" in data["topics"] + assert "list_topic_1" in topics + assert "list_topic_2" in topics # ============================================================================ @@ -894,20 +890,11 @@ async def receive(self, msg): actor_name = "_topic_sub_timeout_error_topic_slow_sub" await actor_system.spawn(slow_actor, name=actor_name, public=True) - # Register with broker - from pulsing.queue.manager import get_topic_broker - from pulsing.actor import Message - - broker = await get_topic_broker(actor_system, "timeout_error_topic") - await broker.ask( - Message.from_json( - "Subscribe", - { - "subscriber_id": "slow_sub", - "actor_name": actor_name, - "node_id": actor_system.node_id.id, - }, - ) + # Register with broker using helper function + from pulsing.topic import subscribe_to_topic + + await subscribe_to_topic( + actor_system, "timeout_error_topic", "slow_sub", actor_name ) # Publish with very short timeout - should timeout @@ -1024,8 +1011,7 @@ async def test_subscriber_failure_threshold_eviction(actor_system): Verify P0-3 fix: Subscribers are automatically evicted after 3 consecutive failures. """ - from pulsing.actor import Actor, ActorId, Message - from pulsing.queue.manager import get_topic_broker + from pulsing.actor import Actor, ActorId from pulsing.topic.broker import MAX_CONSECUTIVE_FAILURES # Verify configuration constants @@ -1048,17 +1034,11 @@ async def receive(self, msg): actor_name = "_topic_sub_eviction_test_topic_failing" await actor_system.spawn(failing_actor, name=actor_name, public=True) - # Register failing subscriber with broker - broker = await get_topic_broker(actor_system, "eviction_test_topic") - await broker.ask( - Message.from_json( - "Subscribe", - { - "subscriber_id": "failing_sub", - "actor_name": actor_name, - "node_id": actor_system.node_id.id, - }, - ) + # Register failing subscriber with broker using helper function + from pulsing.topic import subscribe_to_topic + + await subscribe_to_topic( + actor_system, "eviction_test_topic", "failing_sub", actor_name ) # Get initial statistics