fix: keep only the from_strings method to create an AgentType (#288)

# Description

Allow direct creation of agent type only via the from_strings method.

Signed-off-by: Zsolt Kacsándi <[email protected]>
Signed-off-by: Mauro Sardara <[email protected]>
Co-authored-by: Mauro Sardara <[email protected]>

Author: zsoltkacsandi

data-plane/gateway/datapath/src/messages/encoder.rs

@@ -64,16 +64,6 @@ impl From<&ProtoAgent> for AgentType {
 }
 
 impl AgentType {
-    /// Create a new AgentType
-    pub fn new(organization: u64, namespace: u64, agent_type: u64) -> Self {
-        Self {
-            organization,
-            namespace,
-            agent_type,
-            strings: None,
-        }
-    }
-
     pub fn from_strings(organization: &str, namespace: &str, agent_type: &str) -> Self {
         Self {
             organization: calculate_hash(organization),
@@ -87,21 +77,6 @@ impl AgentType {
         }
     }
 
-    pub fn with_organization(self, organization: u64) -> Self {
-        Self {
-            organization,
-            ..self
-        }
-    }
-
-    pub fn with_namespace(self, namespace: u64) -> Self {
-        Self { namespace, ..self }
-    }
-
-    pub fn with_agent_type(self, agent_type: u64) -> Self {
-        Self { agent_type, ..self }
-    }
-
     pub fn organization(&self) -> u64 {
         self.organization
     }

data-plane/gateway/datapath/src/messages/utils.rs

@@ -942,7 +942,7 @@ mod tests {
 
     #[test]
     fn test_conversions() {
-        // ProtoAgent to Agent
+        // Agent to ProtoAgent
         let agent = Agent::from_strings("org", "ns", "type", 1);
         let proto_agent = ProtoAgent::from(&agent);
 
@@ -951,6 +951,22 @@ mod tests {
         assert_eq!(proto_agent.agent_type, agent.agent_type().agent_type());
         assert_eq!(proto_agent.agent_id.unwrap(), agent.agent_id());
 
+        // ProtoAgent to Agent
+        let agent_from_proto = Agent::from(&proto_agent);
+        assert_eq!(
+            agent_from_proto.agent_type().organization(),
+            proto_agent.organization
+        );
+        assert_eq!(
+            agent_from_proto.agent_type().namespace(),
+            proto_agent.namespace
+        );
+        assert_eq!(
+            agent_from_proto.agent_type().agent_type(),
+            proto_agent.agent_type
+        );
+        assert_eq!(agent_from_proto.agent_id(), proto_agent.agent_id.unwrap());
+
         // AgentType to ProtoAgent
         let agent_type = AgentType::from_strings("org", "ns", "type");
         let proto_agent = ProtoAgent::from((&agent_type, Some(1)));

data-plane/testing/README.md

@@ -22,15 +22,15 @@ Options:
 #### Workload file structure
 The names used by the gateway are in the form
 ```
-organinzation/namespace/agent_class/agent_id
+organization/namespace/agent_class/agent_id
 ```
 These four components are strings decided by the application. For performance reason the gateway internally encodes these strings in u64 values. The workload generator application ignores the strings and creates random names by generating four random u64.
 
 A subscription is represented as follow
 ```
 SUB 24 7 9025227877545173655 4093485893050047688 12129358409937561971 3376091243880048110
 ```
-- ```SUB``` indicates that the line is a subscription 
+- ```SUB``` indicates that the line is a subscription
 - ```24``` is the index of the subscription
 - ```7``` is the ID of the subscriber that will generate the subscription. This is randomly generated and is a value between 0 and ```AGENTS``` that is provided as input. Be sure to spawn enough subscriber apps (see below) to cover all the IDs
 - the last for numbers are random generated and are the subscription name in the form ```organization/namespace/agent_class/agent_id```

data-plane/testing/src/bin/workload-gen.rs

@@ -157,7 +157,25 @@ fn main() {
     let bar = ProgressBar::new(max_subscriptions as u64);
     while subscriptions < max_subscriptions {
         // create a new type
-        let agent_type = AgentType::new(rng.random(), rng.random(), rng.random());
+        let org = rand::rng()
+            .sample_iter(&rand::distr::Alphanumeric)
+            .take(7)
+            .map(char::from)
+            .collect::<String>();
+
+        let ns = rand::rng()
+            .sample_iter(&rand::distr::Alphanumeric)
+            .take(7)
+            .map(char::from)
+            .collect::<String>();
+
+        let atype = rand::rng()
+            .sample_iter(&rand::distr::Alphanumeric)
+            .take(7)
+            .map(char::from)
+            .collect::<String>();
+
+        let agent_type = AgentType::from_strings(&org, &ns, &atype);
 
         let mut type_state = TypeState::default();
 

data-plane/testing/src/lib.rs

@@ -1,3 +1,6 @@
+// Copyright AGNTCY Contributors (https://github.com/agntcy)
+// SPDX-License-Identifier: Apache-2.0
+
 use std::str::SplitWhitespace;
 
 use agp_datapath::messages::{Agent, AgentType};
@@ -28,6 +31,35 @@ pub struct ParsedMessage {
     pub receivers: Vec<u64>,
 }
 
+fn parse_ids(iter: &mut SplitWhitespace<'_>) -> Result<Agent, ParsingError> {
+    let org = iter
+        .next()
+        .ok_or(ParsingError::ParsingError(
+            "missing organization".to_string(),
+        ))?
+        .parse::<String>()
+        .map_err(|e| ParsingError::ParsingError(format!("failed to parse organization: {}", e)))?;
+    let namespace = iter
+        .next()
+        .ok_or(ParsingError::ParsingError("missing namespace".to_string()))?
+        .parse::<String>()
+        .map_err(|e| ParsingError::ParsingError(format!("failed to parse namespace: {}", e)))?;
+    let agent_type_val = iter
+        .next()
+        .ok_or(ParsingError::ParsingError("missing agent_type".to_string()))?
+        .parse::<String>()
+        .map_err(|e| ParsingError::ParsingError(format!("failed to parse agent type: {}", e)))?;
+    let agent_id = iter
+        .next()
+        .ok_or(ParsingError::ParsingError("missing agent_id".to_string()))?
+        .parse::<u64>()
+        .map_err(|e| ParsingError::ParsingError(format!("failed to parse agent id: {}", e)))?;
+
+    let a_type = AgentType::from_strings(&org, &namespace, &agent_type_val);
+
+    Ok(Agent::new(a_type, agent_id))
+}
+
 pub fn parse_sub(mut iter: SplitWhitespace<'_>) -> Result<ParsedMessage, ParsingError> {
     let mut subscription = ParsedMessage {
         msg_type: "SUB".to_string(),
@@ -54,44 +86,7 @@ pub fn parse_sub(mut iter: SplitWhitespace<'_>) -> Result<ParsedMessage, Parsing
         },
     }
 
-    let mut a_type = AgentType::default();
-    let mut sub = Agent::default();
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_organization(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_namespace(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_agent_type(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            sub = sub.with_agent_id(x);
-            sub = sub.with_agent_type(a_type);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
+    let sub = parse_ids(&mut iter)?;
 
     subscription.name = sub;
     Ok(subscription)
@@ -120,44 +115,7 @@ pub fn parse_pub(mut iter: SplitWhitespace<'_>) -> Result<ParsedMessage, Parsing
     }
 
     // get the publication name
-    let mut a_type = AgentType::default();
-    let mut pub_name = Agent::default();
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_organization(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_namespace(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            a_type = a_type.with_agent_type(x);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
-
-    match iter.next().unwrap().parse::<u64>() {
-        Ok(x) => {
-            pub_name = pub_name.with_agent_id(x);
-            pub_name = pub_name.with_agent_type(a_type);
-        }
-        Err(e) => {
-            return Err(ParsingError::ParsingError(e.to_string()));
-        }
-    }
+    let pub_name = parse_ids(&mut iter)?;
 
     // get the len of the possible receivers
     let size = match iter.next().unwrap().parse::<u64>() {
@@ -191,16 +149,17 @@ pub fn parse_pub(mut iter: SplitWhitespace<'_>) -> Result<ParsedMessage, Parsing
 
 pub fn parse_line(line: &str) -> Result<ParsedMessage, ParsingError> {
     let mut iter = line.split_whitespace();
-    let prefix = iter.next();
-
-    if prefix == Some("SUB") {
-        return parse_sub(iter);
+    let msg_type = iter
+        .next()
+        .ok_or_else(|| ParsingError::ParsingError("missing type".to_string()))?
+        .to_string();
+
+    match msg_type.as_str() {
+        "SUB" => parse_sub(iter),
+        "PUB" => parse_pub(iter),
+        _ => Err(ParsingError::ParsingError(format!(
+            "unknown type: {}",
+            msg_type
+        ))),
     }
-
-    if prefix == Some("PUB") {
-        return parse_pub(iter);
-    }
-
-    // unable to parse this line
-    Err(ParsingError::ParsingError("unknown prefix".to_string()))
 }