Merge pull request #117 from JMR-dev/refactor-folder-role-table

refactor(folders): derive roleOf and isServerSpecial from one attribute table

app/src/main/kotlin/org/libremail/domain/model/Folder.kt

@@ -33,6 +33,25 @@ enum class FolderRole {
     ;
 
     companion object {
+        /**
+         * Single source of truth mapping a lowercase IMAP SPECIAL-USE attribute to the role it implies:
+         * RFC 6154's six attributes, plus Gmail's `\All` and RFC 8457's `\Important`. A `null` role
+         * marks an attribute that flags a folder as server-provisioned (not user-created) without
+         * implying one of the friendly [FolderRole]s. [roleOf] returns the first role-bearing entry the
+         * folder advertises; [isServerSpecial] treats every key as special-use. Insertion order sets
+         * [roleOf]'s precedence when a folder advertises more than one role-bearing attribute.
+         */
+        private val ATTRIBUTE_ROLES: Map<String, FolderRole?> = linkedMapOf(
+            "\\sent" to SENT,
+            "\\drafts" to DRAFTS,
+            "\\junk" to SPAM,
+            "\\trash" to TRASH,
+            "\\archive" to ARCHIVE,
+            "\\all" to null,
+            "\\flagged" to null,
+            "\\important" to null,
+        )
+
         /**
          * Classifies a folder from its name and IMAP SPECIAL-USE attributes (RFC 6154). Prefers the
          * server-advertised attribute; falls back to a case-insensitive name match because many
@@ -41,27 +60,14 @@ enum class FolderRole {
         fun roleOf(fullName: String, displayName: String, attributes: List<String>): FolderRole {
             if (fullName.equals("INBOX", ignoreCase = true)) return INBOX
             val attrs = attributes.map { it.lowercase() }
-            val byAttribute = when {
-                "\\sent" in attrs -> SENT
-                "\\drafts" in attrs -> DRAFTS
-                "\\junk" in attrs -> SPAM
-                "\\trash" in attrs -> TRASH
-                "\\archive" in attrs -> ARCHIVE
-                else -> null
+            val byAttribute = ATTRIBUTE_ROLES.entries.firstNotNullOfOrNull { (attribute, role) ->
+                role?.takeIf { attribute in attrs }
             }
             return byAttribute ?: roleFromDisplayName(displayName)
         }
 
-        /**
-         * The RFC 6154 SPECIAL-USE attributes (plus Gmail's `\All`) that mark a folder as one the
-         * server provisions itself, as opposed to a user-created folder.
-         */
-        private val SPECIAL_USE_ATTRIBUTES =
-            setOf("\\all", "\\archive", "\\drafts", "\\flagged", "\\junk", "\\sent", "\\trash")
-
         /** True when the server advertises any SPECIAL-USE attribute for the folder (RFC 6154). */
-        fun isServerSpecial(attributes: List<String>): Boolean =
-            attributes.any { it.lowercase() in SPECIAL_USE_ATTRIBUTES }
+        fun isServerSpecial(attributes: List<String>): Boolean = attributes.any { it.lowercase() in ATTRIBUTE_ROLES }
 
         /** Best-effort role from a folder's display name, for servers without SPECIAL-USE flags. */
         private fun roleFromDisplayName(displayName: String): FolderRole = when (displayName.lowercase().trim()) {

app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt

@@ -42,11 +42,12 @@ class FolderMapperTest {
     }
 
     @Test
-    fun `the all-mail and starred attributes mark the folder special but drive no role of their own`() {
-        // Gmail's "All Mail" (\All) and "Starred" (\Flagged) are server special-use, yet roleOf maps
-        // neither to a role (the role still comes from the name — here neutral, so NORMAL). specialUse
-        // and role are independent axes; this documents the current wiring ahead of the #65 refactor.
-        listOf("\\All", "\\Flagged").forEach { attribute ->
+    fun `the all-mail, starred and important attributes mark the folder special but drive no role`() {
+        // Gmail's "All Mail" (\All), "Starred" (\Flagged) and "Important" (\Important, RFC 8457 — #62)
+        // are server special-use, yet roleOf maps none to a role (the role still comes from the name —
+        // here neutral, so NORMAL). specialUse and role are independent axes; the #65 unified table
+        // keeps them so.
+        listOf("\\All", "\\Flagged", "\\Important").forEach { attribute ->
             val entity = entityFor(listOf(attribute))
             assertTrue(entity.specialUse, "specialUse for $attribute")
             assertEquals(FolderRole.NORMAL.name, entity.role, "role for $attribute")

app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt

@@ -37,9 +37,51 @@ class FolderRoleTest {
     fun `isServerSpecial is true only for special-use attributes`() {
         assertTrue(FolderRole.isServerSpecial(listOf("\\Junk")))
         assertTrue(FolderRole.isServerSpecial(listOf("\\All")))
+        // Issue #62: \Important (RFC 8457) is what Gmail advertises on [Gmail]/Important.
+        assertTrue(FolderRole.isServerSpecial(listOf("\\Important")))
         // Case-insensitive, and ignores non-special-use flags mixed in.
         assertTrue(FolderRole.isServerSpecial(listOf("\\HasNoChildren", "\\drafts")))
         assertFalse(FolderRole.isServerSpecial(emptyList()))
         assertFalse(FolderRole.isServerSpecial(listOf("\\HasNoChildren")))
     }
+
+    // Guards the #65 unified attribute-to-role table: roleOf and isServerSpecial read from one map.
+    // Role-bearing attributes drive a role AND mark the folder special; \All, \Flagged and \Important
+    // are server-special but role-less (their role comes from the display name — here "Neutral", so
+    // NORMAL). Pins that the refactor preserved every existing mapping and added \Important (#62).
+    @Test
+    fun `every special-use attribute maps to its role and is server-special`() {
+        val expectedRole = mapOf(
+            "\\Sent" to FolderRole.SENT,
+            "\\Drafts" to FolderRole.DRAFTS,
+            "\\Junk" to FolderRole.SPAM,
+            "\\Trash" to FolderRole.TRASH,
+            "\\Archive" to FolderRole.ARCHIVE,
+            "\\All" to FolderRole.NORMAL,
+            "\\Flagged" to FolderRole.NORMAL,
+            "\\Important" to FolderRole.NORMAL,
+        )
+        expectedRole.forEach { (attribute, role) ->
+            assertEquals(role, FolderRole.roleOf("X", "Neutral", listOf(attribute)), "role for $attribute")
+            assertTrue(FolderRole.isServerSpecial(listOf(attribute)), "special-use for $attribute")
+        }
+    }
+
+    // A folder advertising two role-bearing attributes resolves to the earlier table entry (Sent
+    // precedes Archive), whatever order the server listed them — preserving the old when-ladder's
+    // precedence now that it is table-driven.
+    @Test
+    fun `role precedence follows the table order, not the folder's attribute order`() {
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Archive", "\\Sent")))
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Sent", "\\Archive")))
+    }
+
+    // A role-less special-use attribute must not suppress the display-name fallback: \All on a folder
+    // named "Sent" still classifies as SENT, and Gmail's [Gmail]/Important (\Important, RFC 8457) is
+    // special-use yet keeps NORMAL because "Important" is not a friendly role.
+    @Test
+    fun `a role-less special-use attribute still allows the name fallback`() {
+        assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Sent", listOf("\\All")))
+        assertEquals(FolderRole.NORMAL, FolderRole.roleOf("[Gmail]/Important", "Important", listOf("\\Important")))
+    }
 }