client: Don't over-abstract locked/unlocked items

cli/src/main.rs

@@ -62,86 +62,130 @@ impl Termination for Error {
 #[derive(Serialize)]
 struct ItemOutput {
     label: String,
-    secret: String,
-    created_at: String,
-    modified_at: String,
+    /// Those are None if the item is locked
+    #[serde(skip_serializing_if = "Option::is_none")]
+    secret: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    created_at: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    modified_at: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     schema: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     content_type: Option<String>,
-    attributes: HashMap<String, String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    attributes: Option<HashMap<String, String>>,
+    is_locked: bool,
 }
 
 impl ItemOutput {
     fn new(
-        secret: &oo7::Secret,
+        secret: Option<&oo7::Secret>,
         label: &str,
-        mut attributes: HashMap<String, String>,
-        created: Duration,
-        modified: Duration,
+        mut attributes: Option<HashMap<String, String>>,
+        created: Option<Duration>,
+        modified: Option<Duration>,
+        is_locked: bool,
         as_hex: bool,
     ) -> Self {
-        let bytes = secret.as_bytes();
         let local_offset = UtcOffset::current_local_offset().unwrap_or(UtcOffset::UTC);
 
-        let created = OffsetDateTime::from_unix_timestamp(created.as_secs() as i64)
-            .unwrap()
-            .to_offset(local_offset);
-        let modified = OffsetDateTime::from_unix_timestamp(modified.as_secs() as i64)
-            .unwrap()
-            .to_offset(local_offset);
+        let created = created.map(|created| {
+            OffsetDateTime::from_unix_timestamp(created.as_secs() as i64)
+                .unwrap()
+                .to_offset(local_offset)
+        });
+        let modified = modified.map(|modified| {
+            OffsetDateTime::from_unix_timestamp(modified.as_secs() as i64)
+                .unwrap()
+                .to_offset(local_offset)
+        });
 
         let format = time::format_description::parse_borrowed::<2>(
             "[year]-[month]-[day] [hour]:[minute]:[second]",
         )
         .unwrap();
 
-        let secret_str = if as_hex {
-            hex::encode(bytes)
-        } else {
-            match std::str::from_utf8(bytes) {
-                Ok(s) => s.to_string(),
-                Err(_) => hex::encode(bytes),
+        let secret_str = secret.map(|s| {
+            let bytes = s.as_bytes();
+            if as_hex {
+                hex::encode(bytes)
+            } else {
+                match std::str::from_utf8(bytes) {
+                    Ok(s) => s.to_string(),
+                    Err(_) => hex::encode(bytes),
+                }
             }
-        };
+        });
 
-        let schema = attributes.remove(oo7::XDG_SCHEMA_ATTRIBUTE);
-        let content_type = attributes.remove(oo7::CONTENT_TYPE_ATTRIBUTE);
+        let schema = attributes
+            .as_mut()
+            .and_then(|attrs| attrs.remove(oo7::XDG_SCHEMA_ATTRIBUTE));
+        let content_type = attributes
+            .as_mut()
+            .and_then(|attrs| attrs.remove(oo7::CONTENT_TYPE_ATTRIBUTE));
 
         Self {
             label: label.to_string(),
             secret: secret_str,
-            created_at: created.format(&format).unwrap(),
-            modified_at: modified.format(&format).unwrap(),
+            created_at: created.map(|created| created.format(&format).unwrap()),
+            modified_at: modified.map(|modified| modified.format(&format).unwrap()),
             schema,
             content_type,
             attributes,
+            is_locked,
         }
     }
 
-    fn from_file_item(item: &oo7::file::Item, as_hex: bool) -> Self {
-        let unlocked = item.as_unlocked();
+    fn from_file_item(item: &oo7::file::UnlockedItem, as_hex: bool) -> Self {
         Self::new(
-            &unlocked.secret(),
-            unlocked.label(),
-            unlocked
-                .attributes()
-                .iter()
-                .map(|(k, v)| (k.to_string(), v.to_string()))
-                .collect(),
-            unlocked.created(),
-            unlocked.modified(),
+            Some(&item.secret()),
+            item.label(),
+            Some(
+                item.attributes()
+                    .iter()
+                    .map(|(k, v)| (k.to_string(), v.to_string()))
+                    .collect(),
+            ),
+            Some(item.created()),
+            Some(item.modified()),
+            false,
             as_hex,
         )
     }
 
     async fn from_dbus_item(item: &oo7::dbus::Item, as_hex: bool) -> Result<Self, Error> {
+        use oo7::dbus::ServiceError;
+
+        let is_locked = item.is_locked().await?;
+        let secret = match item.secret().await {
+            Ok(secret) => Ok(Some(secret)),
+            Err(oo7::dbus::Error::Service(ServiceError::IsLocked(_))) => Ok(None),
+            Err(e) => Err(e),
+        }?;
+        let attributes = match item.attributes().await {
+            Ok(attributes) => Ok(Some(attributes)),
+            Err(oo7::dbus::Error::Service(ServiceError::IsLocked(_))) => Ok(None),
+            Err(e) => Err(e),
+        }?;
+        let created = match item.created().await {
+            Ok(created) => Ok(Some(created)),
+            Err(oo7::dbus::Error::Service(ServiceError::IsLocked(_))) => Ok(None),
+            Err(e) => Err(e),
+        }?;
+        let modified = match item.modified().await {
+            Ok(modified) => Ok(Some(modified)),
+            Err(oo7::dbus::Error::Service(ServiceError::IsLocked(_))) => Ok(None),
+            Err(e) => Err(e),
+        }?;
+
         Ok(Self::new(
-            &item.secret().await?,
+            secret.as_ref(),
             &item.label().await?,
-            item.attributes().await?,
-            item.created().await?,
-            item.modified().await?,
+            attributes,
+            created,
+            modified,
+            is_locked,
             as_hex,
         ))
     }
@@ -150,16 +194,29 @@ impl ItemOutput {
 impl fmt::Display for ItemOutput {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         writeln!(f, "[{}]", self.label)?;
-        writeln!(f, "secret = {}", self.secret)?;
-        writeln!(f, "created = {}", self.created_at)?;
-        writeln!(f, "modified = {}", self.modified_at)?;
+        if let Some(ref secret) = self.secret {
+            writeln!(f, "secret = {}", secret)?;
+        }
+        if let Some(ref created_at) = self.created_at {
+            writeln!(f, "created = {}", created_at)?;
+        }
+        if let Some(ref modified_at) = self.modified_at {
+            writeln!(f, "modified = {}", modified_at)?;
+        }
         if let Some(schema) = &self.schema {
             writeln!(f, "schema = {schema}")?;
         }
         if let Some(content_type) = &self.content_type {
             writeln!(f, "content_type = {content_type}")?;
         }
-        writeln!(f, "attributes = {:?}", self.attributes)?;
+        if let Some(attributes) = &self.attributes {
+            writeln!(f, "attributes = {:?}", attributes)?;
+        }
+        if self.is_locked {
+            writeln!(f, "locked = true")?;
+        } else {
+            writeln!(f, "locked = false")?;
+        }
         Ok(())
     }
 }
@@ -378,7 +435,7 @@ impl Commands {
                     let items = keyring.search_items(&attributes).await?;
                     if let Some(item) = items.first() {
                         if secret_only {
-                            Output::SecretOnly(vec![item.as_unlocked().secret().clone()], hex)
+                            Output::SecretOnly(vec![item.secret().clone()], hex)
                         } else {
                             Output::Items(vec![ItemOutput::from_file_item(item, hex)], json)
                         }
@@ -405,7 +462,7 @@ impl Commands {
                     if secret_only {
                         let secrets = items_to_print
                             .into_iter()
-                            .map(|item| item.as_unlocked().secret().clone())
+                            .map(|item| item.secret().clone())
                             .collect();
 
                         Output::SecretOnly(secrets, hex)
@@ -472,14 +529,15 @@ impl Commands {
             Commands::List { hex, json } => {
                 let items = match keyring {
                     Keyring::File(keyring) => {
-                        let items = keyring.items().await?;
+                        let items = keyring.all_items().await?;
                         let mut outputs = Vec::new();
                         for item in items {
-                            if let Ok(item) = item {
-                                outputs.push(ItemOutput::from_file_item(&item, hex));
-                            } else if !json {
-                                // Only print error message in text mode, skip in JSON mode
-                                println!("Item is not valid and cannot be decrypted");
+                            match item {
+                                Ok(item) => outputs.push(ItemOutput::from_file_item(&item, hex)),
+                                Err(_) if !json => {
+                                    println!("Item is not valid and cannot be decrypted");
+                                }
+                                Err(_) => {} // Skip invalid items in JSON mode
                             }
                         }
                         outputs

client/examples/file_tracing.rs

@@ -294,7 +294,7 @@ async fn test_search_performance() -> oo7::Result<()> {
     let start = Instant::now();
     let items = keyring.items().await?;
     let all_items_time = start.elapsed();
-    let valid_items = items.iter().filter(|r| r.is_ok()).count();
+    let valid_items = items.iter().count();
     info!(
         "Get all items: {:?} (found {} valid items)",
         all_items_time, valid_items

client/examples/schema.rs

@@ -52,15 +52,14 @@ async fn main() -> oo7::Result<()> {
     println!("Found {} item(s)", items.len());
 
     for item in &items {
-        let unlocked = item.as_unlocked();
-        println!("  Label: {}", unlocked.label());
-        println!("  Secret: {:?}", unlocked.secret());
+        println!("  Label: {}", item.label());
+        println!("  Secret: {:?}", item.secret());
     }
 
     println!("\n=== Typed attributes ===");
 
     if let Some(item) = items.first() {
-        let schema = item.as_unlocked().attributes_as::<PasswordSchema>()?;
+        let schema = item.attributes_as::<PasswordSchema>()?;
         println!("Username: {}", schema.username);
         println!("Server: {}", schema.server);
         println!("Port: {:?}", schema.port);

client/src/file/locked_keyring.rs

@@ -18,8 +18,8 @@ use tokio::{
     sync::{Mutex, RwLock},
 };
 
-use super::{Error, Item, LockedItem, UnlockedKeyring, api};
-use crate::{Secret, file::InvalidItemError};
+use super::{Error, LockedItem, UnlockedKeyring, api};
+use crate::Secret;
 
 /// A locked keyring that requires a secret to unlock.
 #[derive(Debug)]
@@ -56,16 +56,14 @@ impl LockedKeyring {
 
     /// Retrieve the list of available [`LockedItem`]s without decrypting them.
     #[cfg_attr(feature = "tracing", tracing::instrument(skip(self)))]
-    pub async fn items(&self) -> Result<Vec<Result<Item, InvalidItemError>>, Error> {
+    pub async fn items(&self) -> Result<Vec<LockedItem>, Error> {
         let keyring = self.keyring.read().await;
 
         Ok(keyring
             .items
             .iter()
-            .map(|encrypted_item| {
-                Ok(Item::Locked(LockedItem {
-                    inner: encrypted_item.clone(),
-                }))
+            .map(|encrypted_item| LockedItem {
+                inner: encrypted_item.clone(),
             })
             .collect())
     }

client/src/file/mod.rs

@@ -10,10 +10,7 @@
 //!     .await?;
 //!
 //! let items = keyring.search_items(&[("account", "alice")]).await?;
-//! assert_eq!(
-//!     items[0].as_unlocked().secret(),
-//!     oo7::Secret::blob("My Password")
-//! );
+//! assert_eq!(items[0].secret(), oo7::Secret::blob("My Password"));
 //!
 //! keyring.delete(&[("account", "alice")]).await?;
 //! #   Ok(())
@@ -46,6 +43,18 @@ pub enum Item {
     Unlocked(UnlockedItem),
 }
 
+impl From<UnlockedItem> for Item {
+    fn from(item: UnlockedItem) -> Self {
+        Self::Unlocked(item)
+    }
+}
+
+impl From<LockedItem> for Item {
+    fn from(item: LockedItem) -> Self {
+        Self::Locked(item)
+    }
+}
+
 impl Item {
     pub const fn is_locked(&self) -> bool {
         matches!(self, Self::Locked(_))
@@ -102,6 +111,18 @@ pub enum Keyring {
     Unlocked(UnlockedKeyring),
 }
 
+impl From<LockedKeyring> for Keyring {
+    fn from(keyring: LockedKeyring) -> Self {
+        Self::Locked(keyring)
+    }
+}
+
+impl From<UnlockedKeyring> for Keyring {
+    fn from(keyring: UnlockedKeyring) -> Self {
+        Self::Unlocked(keyring)
+    }
+}
+
 impl Keyring {
     /// Validate that a secret can decrypt the items in this keyring.
     #[cfg_attr(feature = "tracing", tracing::instrument(skip(self, secret)))]
@@ -136,10 +157,20 @@ impl Keyring {
             .and_then(|time| time.duration_since(std::time::SystemTime::UNIX_EPOCH).ok())
     }
 
-    pub async fn items(&self) -> Result<Vec<Result<Item, InvalidItemError>>, Error> {
+    pub async fn items(&self) -> Result<Vec<Item>, Error> {
         match self {
-            Self::Locked(keyring) => keyring.items().await,
-            Self::Unlocked(keyring) => keyring.items().await,
+            Self::Locked(keyring) => Ok(keyring
+                .items()
+                .await?
+                .into_iter()
+                .map(Item::Locked)
+                .collect()),
+            Self::Unlocked(keyring) => Ok(keyring
+                .items()
+                .await?
+                .into_iter()
+                .map(Item::Unlocked)
+                .collect()),
         }
     }