From 10203d8ee9db97f1a89b2309ed78ebed493ef2b8 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 02:49:16 -0500 Subject: [PATCH] refactor(folders): derive roleOf and isServerSpecial from one attribute table Replaces the two hand-maintained RFC 6154 tables in FolderRole's companion -- roleOf's attribute when-ladder and the separate SPECIAL_USE_ATTRIBUTES set, which had already drifted (\All and \Flagged were special-use but had no role branch) -- with a single ordered ATTRIBUTE_ROLES map from a lowercase SPECIAL-USE attribute to the FolderRole it implies (null = server-special but role-less). roleOf returns the first role-bearing entry the folder advertises (insertion order preserves the old ladder's precedence); isServerSpecial treats every key as special-use. One source of truth, so the two can no longer diverge. Also adds \Important (RFC 8457) as a role-less special-use key, so Gmail's [Gmail]/Important is recognized as server-provisioned and the drawer de-dup renders "Important - Gmail" instead of leaking the raw "Important ([Gmail])" namespace form (#62). Purely additive: no role/specialUse mapping changed for any existing attribute, and specialUse stays a plain Boolean column re-derived on the next folder refresh -- no Room migration needed. Tests: extends the #64 fidelity fixtures (FolderRoleTest, FolderMapperTest) with the \Important case, and adds table-order precedence and role-less-fallback guards pinning the refactor behavior-for-behavior. Closes #65 Closes #62 Co-Authored-By: Claude Fable 5 --- .../org/libremail/domain/model/Folder.kt | 38 ++++++++++------- .../libremail/data/local/FolderMapperTest.kt | 11 ++--- .../libremail/domain/model/FolderRoleTest.kt | 42 +++++++++++++++++++ 3 files changed, 70 insertions(+), 21 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt index 0b8de14..3cbb648 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt +++ b/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 = 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): 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): Boolean = - attributes.any { it.lowercase() in SPECIAL_USE_ATTRIBUTES } + fun isServerSpecial(attributes: List): 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()) { diff --git a/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt b/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt index ca01e64..75fd901 100644 --- a/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt +++ b/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") diff --git a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt index d426cfd..d1ccc50 100644 --- a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt +++ b/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"))) + } }