From e8256a9e571aefa3f95afd5bd673f95cfe7b8f53 Mon Sep 17 00:00:00 2001
From: James Magahern <james@magahern.com>
Date: Wed, 10 Sep 2025 14:06:54 -0700
Subject: [PATCH] core: attachment mime: prefer jpg instead of jfif

---
 .../src/daemon/attachment_store.rs            | 22 ++++++++++++++++---
 .../src/daemon/models/attachment.rs           | 19 ++++++++++++++--
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/core/kordophoned/src/daemon/attachment_store.rs b/core/kordophoned/src/daemon/attachment_store.rs
index 61151a1..42fced1 100644
--- a/core/kordophoned/src/daemon/attachment_store.rs
+++ b/core/kordophoned/src/daemon/attachment_store.rs
@@ -165,6 +165,7 @@ impl AttachmentStore {
         // Check if any in-progress temporary file exists already for this attachment
         if let Some(in_progress) = Self::find_in_progress_download(&attachment, preview) {
             log::warn!(target: target::ATTACHMENTS, "Temporary file already exists: {}, assuming download is in progress", in_progress.display());
+            // Treat as a non-fatal condition so we don't spam errors.
             return Err(AttachmentStoreError::DownloadAlreadyInProgress.into());
         }
 
@@ -183,8 +184,7 @@ impl AttachmentStore {
 
         let guessed_ext = normalized_mime
             .as_deref()
-            .and_then(|m| mime_guess::get_mime_extensions_str(m))
-            .and_then(|list| list.first().copied())
+            .and_then(|m| Attachment::preferred_extension_for_mime(m))
             .unwrap_or("bin");
 
         let final_path = attachment
@@ -327,7 +327,23 @@ impl AttachmentStore {
                                     ).await;
 
                                     if let Err(e) = result {
-                                        log::error!(target: target::ATTACHMENTS, "Error downloading attachment {}: {}", &_guid, e);
+                                        // Downgrade noise for expected cases
+                                        if let Some(kind) = e.downcast_ref::<AttachmentStoreError>() {
+                                            match kind {
+                                                AttachmentStoreError::AttachmentAlreadyDownloaded => {
+                                                    log::debug!(target: target::ATTACHMENTS, "Attachment already downloaded: {}", &_guid);
+                                                }
+                                                AttachmentStoreError::DownloadAlreadyInProgress => {
+                                                    // Already logged a warning where detected
+                                                    log::debug!(target: target::ATTACHMENTS, "Download already in progress: {}", &_guid);
+                                                }
+                                                _ => {
+                                                    log::error!(target: target::ATTACHMENTS, "Error downloading attachment {}: {}", &_guid, kind);
+                                                }
+                                            }
+                                        } else {
+                                            log::error!(target: target::ATTACHMENTS, "Error downloading attachment {}: {}", &_guid, e);
+                                        }
                                     }
                                 });
 
diff --git a/core/kordophoned/src/daemon/models/attachment.rs b/core/kordophoned/src/daemon/models/attachment.rs
index cd38601..6adb261 100644
--- a/core/kordophoned/src/daemon/models/attachment.rs
+++ b/core/kordophoned/src/daemon/models/attachment.rs
@@ -20,6 +20,22 @@ pub struct Attachment {
 }
 
 impl Attachment {
+    pub(crate) fn preferred_extension_for_mime(mime: &str) -> Option<&'static str> {
+        let normalized = mime.split(';').next().unwrap_or(mime).trim();
+        // Prefer common, user-friendly extensions over obscure ones
+        match normalized {
+            "image/jpeg" | "image/pjpeg" => Some("jpg"),
+            _ => mime_guess::get_mime_extensions_str(normalized)
+                .and_then(|list| {
+                    // If jpg is one of the candidates, prefer it
+                    if list.iter().any(|e| *e == "jpg") {
+                        Some("jpg")
+                    } else {
+                        list.first().copied()
+                    }
+                }),
+        }
+    }
     pub fn get_path(&self) -> PathBuf {
         self.get_path_for_preview_scratch(false, false)
     }
@@ -45,8 +61,7 @@ impl Attachment {
         let ext_from_mime = self
             .mime_type
             .as_ref()
-            .and_then(|m| mime_guess::get_mime_extensions_str(m.split(';').next().unwrap_or(m)))
-            .and_then(|list| list.first().copied());
+            .and_then(|m| Self::preferred_extension_for_mime(m));
 
         let base_ext = match ext_from_mime {
             Some(ext) => format!("{}.{}", kind, ext),