Add support for User, Group, Driver, Legacy, and Transform actions in Manifest parsing

- Implement parsing and mapping of new action types: `User`, `Group`, `Driver`, `Legacy`, and `Transform` within `Manifest`.
- Introduce comprehensive test coverage for the newly supported actions, including edge cases for `ftpuser` property in manifests.
- Refactor to leverage `HashSet` for user `services` to avoid duplicates.
- Improve comments and error handling during action parsing for enhanced clarity and robustness.

libips/src/actions/mod.rs

@@ -14,7 +14,7 @@ use pest::Parser;
 use pest_derive::Parser;
 use serde::{Deserialize, Serialize};
 use std::clone::Clone;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::fs::read_to_string;
 use std::path::Path;
 use std::result::Result as StdResult;
@@ -50,7 +50,7 @@ pub enum ActionError {
 }
 
 pub trait FacetedAction {
-    // Add a facet to the action if the facet is already present the function returns false.
+    // Add a facet to the action if the facet is already present, the function returns false.
     fn add_facet(&mut self, facet: Facet) -> bool;
 
     // Remove a facet from the action.
@@ -470,6 +470,303 @@ impl From<Action> for Link {
     }
 }
 
+#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)]
+#[diff(attr(
+    #[derive(Debug, PartialEq)]
+))]
+pub struct User {
+    pub username: String,
+    pub uid: String,
+    pub group: String,
+    pub home_dir: String,
+    pub login_shell: String,
+    pub password: String,
+    pub services: HashSet<String>,
+    pub gcos_field: String,
+    pub properties: HashMap<String, Property>,
+    pub facets: HashMap<String, Facet>,
+}
+
+impl From<Action> for User {
+    fn from(act: Action) -> Self {
+        let mut user = User::default();
+        let mut props = act.properties;
+        if !act.payload_string.is_empty() {
+            let p_str = split_property(act.payload_string);
+            props.push(Property {
+                key: p_str.0,
+                value: p_str.1,
+            })
+        }
+        for prop in props {
+            match prop.key.as_str() {
+                "username" => user.username = prop.value,
+                "uid" => user.uid = prop.value,
+                "group" => user.group = prop.value,
+                "home-dir" => user.home_dir = prop.value,
+                "login-shell" => user.login_shell = prop.value,
+                "password" => user.password = prop.value,
+                "gcos-field" => user.gcos_field = prop.value,
+                "ftpuser" => {
+                    // Parse ftpuser property into services
+                    match string_to_bool(&prop.value) {
+                        // If it's a boolean value (backward compatibility)
+                        Ok(true) => { user.services.insert("ftp".to_string()); },
+                        Ok(false) => {}, // No services if false
+                        // If the value not a boolean, treat as a comma-separated list of services
+                        _ => {
+                            for service in prop.value.split(',') {
+                                let service = service.trim();
+                                if !service.is_empty() {
+                                    user.services.insert(service.to_string());
+                                }
+                            }
+                        }
+                    }
+                }
+                _ => {
+                    if is_facet(prop.key.clone()) {
+                        user.add_facet(Facet::from_key_value(prop.key, prop.value));
+                    } else {
+                        user.properties.insert(
+                            prop.key.clone(),
+                            Property {
+                                key: prop.key,
+                                value: prop.value,
+                            },
+                        );
+                    }
+                }
+            }
+        }
+        user
+    }
+}
+
+impl FacetedAction for User {
+    fn add_facet(&mut self, facet: Facet) -> bool {
+        self.facets.insert(facet.name.clone(), facet).is_none()
+    }
+
+    fn remove_facet(&mut self, facet: Facet) -> bool {
+        self.facets.remove(&facet.name) == Some(facet)
+    }
+}
+
+#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)]
+#[diff(attr(
+    #[derive(Debug, PartialEq)]
+))]
+pub struct Group {
+    pub groupname: String,
+    pub gid: String,
+    pub properties: HashMap<String, Property>,
+    pub facets: HashMap<String, Facet>,
+}
+
+impl From<Action> for Group {
+    fn from(act: Action) -> Self {
+        let mut group = Group::default();
+        let mut props = act.properties;
+        if !act.payload_string.is_empty() {
+            let p_str = split_property(act.payload_string);
+            props.push(Property {
+                key: p_str.0,
+                value: p_str.1,
+            })
+        }
+        for prop in props {
+            match prop.key.as_str() {
+                "groupname" => group.groupname = prop.value,
+                "gid" => group.gid = prop.value,
+                _ => {
+                    if is_facet(prop.key.clone()) {
+                        group.add_facet(Facet::from_key_value(prop.key, prop.value));
+                    } else {
+                        group.properties.insert(
+                            prop.key.clone(),
+                            Property {
+                                key: prop.key,
+                                value: prop.value,
+                            },
+                        );
+                    }
+                }
+            }
+        }
+        group
+    }
+}
+
+impl FacetedAction for Group {
+    fn add_facet(&mut self, facet: Facet) -> bool {
+        self.facets.insert(facet.name.clone(), facet).is_none()
+    }
+
+    fn remove_facet(&mut self, facet: Facet) -> bool {
+        self.facets.remove(&facet.name) == Some(facet)
+    }
+}
+
+#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)]
+#[diff(attr(
+    #[derive(Debug, PartialEq)]
+))]
+pub struct Driver {
+    pub name: String,
+    pub class: String,
+    pub perms: String,
+    pub clone_perms: String,
+    pub alias: String,
+    pub properties: HashMap<String, Property>,
+    pub facets: HashMap<String, Facet>,
+}
+
+impl From<Action> for Driver {
+    fn from(act: Action) -> Self {
+        let mut driver = Driver::default();
+        let mut props = act.properties;
+        if !act.payload_string.is_empty() {
+            let p_str = split_property(act.payload_string);
+            props.push(Property {
+                key: p_str.0,
+                value: p_str.1,
+            })
+        }
+        for prop in props {
+            match prop.key.as_str() {
+                "name" => driver.name = prop.value,
+                "class" => driver.class = prop.value,
+                "perms" => driver.perms = prop.value,
+                "clone_perms" => driver.clone_perms = prop.value,
+                "alias" => driver.alias = prop.value,
+                _ => {
+                    if is_facet(prop.key.clone()) {
+                        driver.add_facet(Facet::from_key_value(prop.key, prop.value));
+                    } else {
+                        driver.properties.insert(
+                            prop.key.clone(),
+                            Property {
+                                key: prop.key,
+                                value: prop.value,
+                            },
+                        );
+                    }
+                }
+            }
+        }
+        driver
+    }
+}
+
+impl FacetedAction for Driver {
+    fn add_facet(&mut self, facet: Facet) -> bool {
+        self.facets.insert(facet.name.clone(), facet).is_none()
+    }
+
+    fn remove_facet(&mut self, facet: Facet) -> bool {
+        self.facets.remove(&facet.name) == Some(facet)
+    }
+}
+
+#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)]
+#[diff(attr(
+    #[derive(Debug, PartialEq)]
+))]
+pub struct Legacy {
+    pub arch: String,
+    pub category: String,
+    pub desc: String,
+    pub hotline: String,
+    pub name: String,
+    pub pkg: String,
+    pub vendor: String,
+    pub version: String,
+    pub properties: HashMap<String, Property>,
+}
+
+impl From<Action> for Legacy {
+    fn from(act: Action) -> Self {
+        let mut legacy = Legacy::default();
+        let mut props = act.properties;
+        if !act.payload_string.is_empty() {
+            let p_str = split_property(act.payload_string);
+            props.push(Property {
+                key: p_str.0,
+                value: p_str.1,
+            })
+        }
+        for prop in props {
+            match prop.key.as_str() {
+                "arch" => legacy.arch = prop.value,
+                "category" => legacy.category = prop.value,
+                "desc" => legacy.desc = prop.value,
+                "hotline" => legacy.hotline = prop.value,
+                "name" => legacy.name = prop.value,
+                "pkg" => legacy.pkg = prop.value,
+                "vendor" => legacy.vendor = prop.value,
+                "version" => legacy.version = prop.value,
+                _ => {
+                    legacy.properties.insert(
+                        prop.key.clone(),
+                        Property {
+                            key: prop.key,
+                            value: prop.value,
+                        },
+                    );
+                }
+            }
+        }
+        legacy
+    }
+}
+
+#[derive(Debug, Default, PartialEq, Clone, Deserialize, Serialize, Diff)]
+#[diff(attr(
+    #[derive(Debug, PartialEq)]
+))]
+pub struct Transform {
+    pub transform_type: String,
+    pub pattern: String,
+    pub match_type: String,
+    pub operation: String,
+    pub value: String,
+    pub properties: HashMap<String, Property>,
+}
+
+impl From<Action> for Transform {
+    fn from(act: Action) -> Self {
+        let mut transform = Transform::default();
+        let mut props = act.properties;
+        if !act.payload_string.is_empty() {
+            let p_str = split_property(act.payload_string);
+            props.push(Property {
+                key: p_str.0,
+                value: p_str.1,
+            })
+        }
+        for prop in props {
+            match prop.key.as_str() {
+                "type" => transform.transform_type = prop.value,
+                "pattern" => transform.pattern = prop.value,
+                "match_type" => transform.match_type = prop.value,
+                "operation" => transform.operation = prop.value,
+                "value" => transform.value = prop.value,
+                _ => {
+                    transform.properties.insert(
+                        prop.key.clone(),
+                        Property {
+                            key: prop.key,
+                            value: prop.value,
+                        },
+                    );
+                }
+            }
+        }
+        transform
+    }
+}
+
 #[derive(Hash, Eq, PartialEq, Debug, Default, Clone, Deserialize, Serialize, Diff)]
 #[diff(attr(
     #[derive(Debug, PartialEq)]
@@ -490,6 +787,11 @@ pub struct Manifest {
     pub dependencies: Vec<Dependency>,
     pub licenses: Vec<License>,
     pub links: Vec<Link>,
+    pub users: Vec<User>,
+    pub groups: Vec<Group>,
+    pub drivers: Vec<Driver>,
+    pub legacies: Vec<Legacy>,
+    pub transforms: Vec<Transform>,
 }
 
 impl Manifest {
@@ -501,6 +803,11 @@ impl Manifest {
             dependencies: Vec::new(),
             licenses: Vec::new(),
             links: Vec::new(),
+            users: Vec::new(),
+            groups: Vec::new(),
+            drivers: Vec::new(),
+            legacies: Vec::new(),
+            transforms: Vec::new(),
         }
     }
 
@@ -523,13 +830,13 @@ impl Manifest {
                 self.dependencies.push(act.into());
             }
             ActionKind::User => {
-                todo!()
+                self.users.push(act.into());
             }
             ActionKind::Group => {
-                todo!()
+                self.groups.push(act.into());
             }
             ActionKind::Driver => {
-                todo!()
+                self.drivers.push(act.into());
             }
             ActionKind::License => {
                 self.licenses.push(act.into());
@@ -538,10 +845,10 @@ impl Manifest {
                 self.links.push(act.into());
             }
             ActionKind::Legacy => {
-                todo!()
+                self.legacies.push(act.into());
             }
             ActionKind::Transform => {
-                todo!()
+                self.transforms.push(act.into());
             }
             ActionKind::Unknown { action } => {
                 panic!("action {:?} not known", action)

libips/tests/test_manifest_parsing.rs

@@ -0,0 +1,59 @@
+extern crate libips;
+
+use libips::actions::Manifest;
+use std::path::Path;
+
+#[test]
+fn test_parse_postgre_common_manifest() {
+    let manifest_path = Path::new("/home/toasty/ws/illumos/ips/pkg6repo/postgre-common.manifest");
+    let manifest = Manifest::parse_file(manifest_path).expect("Failed to parse manifest");
+    
+    // Check that the manifest contains the expected actions
+    assert_eq!(manifest.attributes.len(), 11, "Expected 11 attributes");
+    assert_eq!(manifest.directories.len(), 1, "Expected 1 directory");
+    assert_eq!(manifest.groups.len(), 1, "Expected 1 group");
+    assert_eq!(manifest.users.len(), 1, "Expected 1 user");
+    assert_eq!(manifest.licenses.len(), 1, "Expected 1 license");
+    
+    // Check the group action
+    let group = &manifest.groups[0];
+    assert_eq!(group.groupname, "postgres", "Expected groupname to be 'postgres'");
+    assert_eq!(group.gid, "90", "Expected gid to be '90'");
+    
+    // Check the user action
+    let user = &manifest.users[0];
+    assert_eq!(user.username, "postgres", "Expected username to be 'postgres'");
+    assert_eq!(user.uid, "90", "Expected uid to be '90'");
+    assert_eq!(user.group, "postgres", "Expected group to be 'postgres'");
+    assert_eq!(user.home_dir, "/var/postgres", "Expected home_dir to be '/var/postgres'");
+    assert_eq!(user.login_shell, "/usr/bin/pfksh", "Expected login_shell to be '/usr/bin/pfksh'");
+    assert_eq!(user.password, "NP", "Expected password to be 'NP'");
+    assert!(user.services.is_empty(), "Expected no services for ftpuser=false");
+    assert_eq!(user.gcos_field, "PostgreSQL Reserved UID", "Expected gcos_field to be 'PostgreSQL Reserved UID'");
+    
+    // Check the directory action
+    let dir = &manifest.directories[0];
+    assert_eq!(dir.path, "var/postgres", "Expected path to be 'var/postgres'");
+    assert_eq!(dir.group, "postgres", "Expected group to be 'postgres'");
+    assert_eq!(dir.owner, "postgres", "Expected owner to be 'postgres'");
+    assert_eq!(dir.mode, "0755", "Expected mode to be '0755'");
+}
+
+#[test]
+fn test_parse_pgadmin_manifest() {
+    let manifest_path = Path::new("/home/toasty/ws/illumos/ips/pkg6repo/pgadmin.manifest");
+    let manifest = Manifest::parse_file(manifest_path).expect("Failed to parse manifest");
+    
+    // Check that the manifest contains the expected actions
+    assert!(manifest.attributes.len() > 0, "Expected attributes");
+    assert!(manifest.files.len() > 0, "Expected files");
+    assert_eq!(manifest.legacies.len(), 1, "Expected 1 legacy action");
+    
+    // Check the legacy action
+    let legacy = &manifest.legacies[0];
+    assert_eq!(legacy.arch, "i386", "Expected arch to be 'i386'");
+    assert_eq!(legacy.category, "system", "Expected category to be 'system'");
+    assert_eq!(legacy.pkg, "SUNWpgadmin3", "Expected pkg to be 'SUNWpgadmin3'");
+    assert_eq!(legacy.vendor, "Project OpenIndiana", "Expected vendor to be 'Project OpenIndiana'");
+    assert_eq!(legacy.version, "11.11.0,REV=2010.05.25.01.00", "Expected version to be '11.11.0,REV=2010.05.25.01.00'");
+}

libips/tests/test_user_services.rs

@@ -0,0 +1,124 @@
+extern crate libips;
+
+use libips::actions::Manifest;
+
+#[test]
+fn test_ftpuser_boolean_true() {
+    // Create a manifest string with ftpuser=true
+    let manifest_string = "user ftpuser=true".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that "ftp" service is added
+    assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added when ftpuser=true");
+    assert_eq!(user.services.len(), 1, "Expected exactly one service");
+}
+
+#[test]
+fn test_ftpuser_boolean_false() {
+    // Create a manifest string with ftpuser=false
+    let manifest_string = "user ftpuser=false".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that no services are added
+    assert!(user.services.is_empty(), "Expected no services when ftpuser=false");
+}
+
+#[test]
+fn test_ftpuser_services_list() {
+    // Create a manifest string with ftpuser=ssh,ftp,http
+    let manifest_string = "user ftpuser=ssh,ftp,http".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that all services are added
+    assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added");
+    assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added");
+    assert!(user.services.contains("http"), "Expected 'http' service to be added");
+    assert_eq!(user.services.len(), 3, "Expected exactly three services");
+}
+
+#[test]
+fn test_ftpuser_services_with_whitespace() {
+    // Create a manifest string with ftpuser=ssh, ftp, http
+    let manifest_string = "user ftpuser=\"ssh, ftp, http\"".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that all services are added with whitespace trimmed
+    assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added");
+    assert!(user.services.contains("ftp"), "Expected 'ftp' service to be added");
+    assert!(user.services.contains("http"), "Expected 'http' service to be added");
+    assert_eq!(user.services.len(), 3, "Expected exactly three services");
+}
+
+#[test]
+fn test_ftpuser_empty_string() {
+    // Create a manifest string with ftpuser=
+    let manifest_string = "user ftpuser=".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that no services are added
+    assert!(user.services.is_empty(), "Expected no services for empty string");
+}
+
+#[test]
+fn test_ftpuser_with_empty_elements() {
+    // Create a manifest string with ftpuser=ssh,,http
+    let manifest_string = "user ftpuser=ssh,,http".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check that only non-empty services are added
+    assert!(user.services.contains("ssh"), "Expected 'ssh' service to be added");
+    assert!(user.services.contains("http"), "Expected 'http' service to be added");
+    assert_eq!(user.services.len(), 2, "Expected exactly two services");
+}
+
+#[test]
+fn test_real_world_example() {
+    // Create a manifest string similar to the one in postgre-common.manifest
+    let manifest_string = "user username=postgres uid=90 group=postgres home-dir=/var/postgres login-shell=/usr/bin/pfksh password=NP gcos-field=\"PostgreSQL Reserved UID\" ftpuser=false".to_string();
+    
+    // Parse the manifest
+    let manifest = Manifest::parse_string(manifest_string).expect("Failed to parse manifest");
+    
+    // Get the user
+    let user = &manifest.users[0];
+    
+    // Check user properties
+    assert_eq!(user.username, "postgres");
+    assert_eq!(user.uid, "90");
+    assert_eq!(user.group, "postgres");
+    assert_eq!(user.home_dir, "/var/postgres");
+    assert_eq!(user.login_shell, "/usr/bin/pfksh");
+    assert_eq!(user.password, "NP");
+    assert_eq!(user.gcos_field, "PostgreSQL Reserved UID");
+    assert!(user.services.is_empty(), "Expected no services for ftpuser=false");
+}