Don't convert unknown TYPE values into explicit X-ABLabels and vice versa for compatibility

* Don't convert unknown TYPE values into explicit X-ABLabels
* Don't convert X-ABLabels into TYPE=x- values

There are clients/servers which don't understand X-ABLabels and the vCard
group concept and drop such entries. In this case, it's OK when labeled
properties don't work, but other properties shouldn't be dropped in
such environments.

5 files changed, 76 insertions, 144 deletions

diff --git a/build.gradle b/build.gradle
index b9d4de0..68b10b0 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1,7 +1,7 @@
 
 buildscript {
     ext.versions = [
-        kotlin: '1.4.21',
+        kotlin: '1.4.31',
         dokka: '0.10.1',
         // latest Apache Commons versions that don't require Java 8 (Android 7)
         commonsIO: '2.6',
@@ -14,7 +14,7 @@ buildscript {
     }
 
     dependencies {
-        classpath 'com.android.tools.build:gradle:4.1.1'
+        classpath 'com.android.tools.build:gradle:4.1.3'
         classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:${versions.kotlin}"
         classpath "org.jetbrains.dokka:dokka-gradle-plugin:${versions.dokka}"
     }
@@ -31,7 +31,7 @@ apply plugin: 'org.jetbrains.dokka'
 
 android {
     compileSdkVersion 30
-    buildToolsVersion '30.0.2'
+    buildToolsVersion '30.0.3'
 
     defaultConfig {
         minSdkVersion 16        // Android 4.1
@@ -75,7 +75,7 @@ android {
 dependencies {
     implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:${versions.kotlin}"
 
-    implementation 'androidx.annotation:annotation:1.1.0'
+    implementation 'androidx.annotation:annotation:1.2.0'
     // noinspection GradleDependency
     implementation "commons-io:commons-io:${versions.commonsIO}"
     // noinspection GradleDependency
@@ -93,5 +93,5 @@ dependencies {
     androidTestImplementation 'androidx.test:runner:1.3.0'
     androidTestImplementation 'androidx.test:rules:1.3.0'
 
-    testImplementation 'junit:junit:4.13.1'
+    testImplementation 'junit:junit:4.13.2'
 }
diff --git a/src/androidTest/java/at/bitfire/vcard4android/AndroidContactTest.kt b/src/androidTest/java/at/bitfire/vcard4android/AndroidContactTest.kt
index 66f13a5..2244a26 100644
--- a/src/androidTest/java/at/bitfire/vcard4android/AndroidContactTest.kt
+++ b/src/androidTest/java/at/bitfire/vcard4android/AndroidContactTest.kt
@@ -258,7 +258,7 @@ class AndroidContactTest {
 
             contact2.emails[5].let { email ->
                 assertEquals("withlabel@example.com", email.property.value)
-                assertEquals(EmailType.get("x-with-label-and-x-type"), email.property.types.first())
+                assertTrue(email.property.types.isEmpty())
                 assertEquals("With label and x-type", email.label)
             }
         } finally {
@@ -297,14 +297,15 @@ class AndroidContactTest {
                 assertEquals("work", url.type)
             }
 
-            contact2.urls[2].property.let { url ->
-                assertEquals("http://Custom.example", url.value)
-                assertEquals("x-custom", url.type)
+            contact2.urls[2].let { url ->
+                assertEquals("http://Custom.example", url.property.value)
+                assertNull(url.property.type)
+                assertNull(url.label)
             }
 
             contact2.urls[3].let { url ->
                 assertEquals("http://Custom.example", url.property.value)
-                assertEquals("x-custom-with-label", url.property.type)
+                assertEquals(null, url.property.type)
                 assertEquals("Custom (with label)", url.label)
             }
         } finally {
@@ -312,12 +313,6 @@ class AndroidContactTest {
         }
     }
 
-
-    @Test
-    fun testLabelToXName() {
-        assertEquals("x-aunties-home", AndroidContact.labelToXName("auntie's home"))
-    }
-
     @Test
     fun testToURIScheme() {
         assertEquals("testp+csfgh-ewt4345.2qiuz4", AndroidContact.toURIScheme("02 34test#ä{☺}ö p[]ß+csfgh()-e_wt4\\345.2qiuz4"))
@@ -325,10 +320,4 @@ class AndroidContactTest {
         assertEquals("CyanogenModForum", AndroidContact.toURIScheme("CyanogenMod_Forum"))
     }
 
-    @Test
-    fun testXNameToLabel() {
-        assertEquals("Aunties Home", AndroidContact.xNameToLabel("X-AUNTIES-HOME"))
-        assertEquals("Aunties Home", AndroidContact.xNameToLabel("X-AUNTIES_HOME"))
-    }
-
 }
diff --git a/src/main/java/at/bitfire/vcard4android/AndroidContact.kt b/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
index 0356ef8..4514862 100644
--- a/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
+++ b/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
@@ -32,7 +32,6 @@ import ezvcard.util.PartialDate
 import org.apache.commons.io.IOUtils
 import org.apache.commons.lang3.StringUtils
 import org.apache.commons.lang3.builder.ToStringBuilder
-import org.apache.commons.text.WordUtils
 import java.io.ByteArrayOutputStream
 import java.io.FileNotFoundException
 import java.io.IOException
@@ -52,21 +51,6 @@ open class AndroidContact(
         const val COLUMN_UID = RawContacts.SYNC1
         const val COLUMN_ETAG = RawContacts.SYNC2
 
-        fun labelToXName(label: String) = "x-" + label
-                .replace(' ','-')
-                .replace(Regex("[^\\p{L}\\p{Nd}\\-_]"), "")
-                .toLowerCase()
-
-        fun xNameToLabel(xname: String): String {
-            // "x-my_property"
-            var s = xname.toLowerCase(Locale.getDefault())    // 1. ensure lower case -> "x-my_property"
-            if (s.startsWith("x-"))                    // 2. remove x- from beginning -> "my_property"
-                s = s.substring(2)
-            s = s   .replace('_', ' ')       // 3. replace "_" and "-" by " " -> "my property"
-                    .replace('-', ' ')
-            return WordUtils.capitalize(s)                   // 4. capitalize -> "My Property"
-        }
-
         fun toURIScheme(s: String?) =
                 // RFC 3986 3.1
                 // scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
@@ -255,7 +239,6 @@ open class AndroidContact(
             Phone.TYPE_CUSTOM -> {
                     row.getAsString(Phone.LABEL)?.let {
                         labeledNumber.label = it
-                        number.types += TelephoneType.get(labelToXName(it))
                     }
                 }
         }
@@ -279,7 +262,6 @@ open class AndroidContact(
             Email.TYPE_CUSTOM ->
                 row.getAsString(Email.LABEL)?.let {
                     labeledEmail.label = it
-                    email.types += EmailType.get(labelToXName(it))
                 }
         }
         if (row.getAsInteger(Email.IS_PRIMARY) != 0)
@@ -356,28 +338,29 @@ open class AndroidContact(
                 }
         }
 
-        impp?.let { impp ->
-            val labeledImpp = LabeledProperty(impp)
-
-            when (row.getAsInteger(Im.TYPE)) {
-                Im.TYPE_HOME ->
-                    impp.types += ImppType.HOME
-                Im.TYPE_WORK ->
-                    impp.types += ImppType.WORK
-                Im.TYPE_CUSTOM ->
-                    row.getAsString(Im.LABEL)?.let {
-                        labeledImpp.label = it
-                        impp.types.add(ImppType.get(labelToXName(it)))
-                    }
-            }
-
-            contact!!.impps += labeledImpp
+        if (impp == null)
+            return
+        val labeledImpp = LabeledProperty(impp)
+
+        when (row.getAsInteger(Im.TYPE)) {
+            Im.TYPE_HOME ->
+                impp.types += ImppType.HOME
+            Im.TYPE_WORK ->
+                impp.types += ImppType.WORK
+            Im.TYPE_CUSTOM ->
+                row.getAsString(Im.LABEL)?.let {
+                    labeledImpp.label = it
+                }
         }
+
+        contact!!.impps += labeledImpp
     }
 
     protected open fun populateNickname(row: ContentValues) {
         row.getAsString(Nickname.NAME)?.let { name ->
             val nick = ezvcard.property.Nickname()
+            val labeledNick = LabeledProperty(nick)
+
             nick.values += name
 
             when (row.getAsInteger(Nickname.TYPE)) {
@@ -390,10 +373,10 @@ open class AndroidContact(
                 Nickname.TYPE_OTHER_NAME ->
                     nick.type = Contact.NICKNAME_TYPE_OTHER_NAME
                 Nickname.TYPE_CUSTOM ->
-                    row.getAsString(Nickname.LABEL)?.let { nick.type = labelToXName(it) }
+                    row.getAsString(Nickname.LABEL)?.let { labeledNick.label = it }
             }
 
-            contact!!.nickName = nick
+            contact!!.nickName = labeledNick
         }
     }
 
@@ -414,7 +397,6 @@ open class AndroidContact(
             StructuredPostal.TYPE_CUSTOM -> {
                 row.getAsString(StructuredPostal.LABEL)?.let {
                     labeledAddress.label = it
-                    address.types += AddressType.get(labelToXName(it))
                 }
             }
         }
@@ -447,7 +429,6 @@ open class AndroidContact(
                 url.type = Contact.URL_TYPE_FTP
             Website.TYPE_CUSTOM ->
                 row.getAsString(Website.LABEL)?.let {
-                    url.type = labelToXName(it)
                     labeledUrl.label = it
                 }
         }
@@ -527,7 +508,6 @@ open class AndroidContact(
                 SipAddress.TYPE_CUSTOM ->
                     row.getAsString(SipAddress.LABEL)?.let {
                         labeledImpp.label = it
-                        impp.types += ImppType.get(labelToXName(it))
                     }
             }
             contact!!.impps.add(labeledImpp)
@@ -701,7 +681,7 @@ open class AndroidContact(
             typeLabel = labeledNumber.label
         } else {
             when {
-                // 1 Android type <-> 2 VCard types: fax, cell, pager
+                // 1 Android type <-> 2 vCard types: fax, cell, pager
                 types.contains(TelephoneType.FAX) ->
                     typeCode = when {
                         types.contains(TelephoneType.HOME) -> Phone.TYPE_FAX_HOME
@@ -738,16 +718,6 @@ open class AndroidContact(
                     typeCode = Phone.TYPE_ASSISTANT
                 types.contains(Contact.PHONE_TYPE_MMS) ->
                     typeCode = Phone.TYPE_MMS
-
-                types.contains(Contact.PHONE_TYPE_OTHER) ||
-                types.contains(TelephoneType.VOICE) ||
-                types.contains(TelephoneType.TEXT) -> {}
-
-                types.isNotEmpty() -> {
-                    val type = types.first()
-                    typeCode = Phone.TYPE_CUSTOM
-                    typeLabel = xNameToLabel(type.value)
-                }
             }
         }
 
@@ -763,11 +733,7 @@ open class AndroidContact(
 
     protected open fun insertEmail(batch: BatchOperation, labeledEmail: LabeledProperty<ezvcard.property.Email>) {
         val email = labeledEmail.property
-
-        // drop TYPE=internet and TYPE=x400 because Android only knows Internet email addresses
-        // drop TYPE=other for compatibility, too (non-standard type which is only used by some clients and not useful as an explicit value)
         val types = email.types
-        types.removeAll(arrayOf(EmailType.INTERNET, EmailType.X400, Contact.EMAIL_TYPE_OTHER))
 
         // preferred email address?
         var pref: Int? = null
@@ -782,7 +748,7 @@ open class AndroidContact(
             types -= EmailType.PREF
         }
 
-        var typeCode = 0
+        var typeCode = Email.TYPE_OTHER
         var typeLabel: String? = null
         if (labeledEmail.label != null) {
             typeCode = Email.TYPE_CUSTOM
@@ -794,14 +760,6 @@ open class AndroidContact(
                     EmailType.WORK -> typeCode = Email.TYPE_WORK
                     Contact.EMAIL_TYPE_MOBILE -> typeCode = Email.TYPE_MOBILE
                 }
-            if (typeCode == 0) {    // we still didn't find a known type
-                if (email.types.isEmpty())
-                    typeCode = Email.TYPE_OTHER
-                else {
-                    typeCode = Email.TYPE_CUSTOM
-                    typeLabel = xNameToLabel(types.first().value)
-                }
-            }
         }
 
         val builder = insertDataBuilder(Email.RAW_CONTACT_ID)
@@ -855,10 +813,6 @@ open class AndroidContact(
                     ImppType.WORK,
                     ImppType.BUSINESS -> typeCode = Im.TYPE_WORK
                 }
-            if (typeCode == Im.TYPE_OTHER && impp.types.isNotEmpty()) {
-                typeCode = Im.TYPE_CUSTOM
-                typeLabel = xNameToLabel(impp.types.first().value)
-            }
         }
 
         val protocol = impp.protocol
@@ -910,23 +864,25 @@ open class AndroidContact(
     }
 
     protected open fun insertNickname(batch: BatchOperation) {
-        val nick = contact!!.nickName
-        if (nick == null || nick.values.isEmpty())
+        val labeledNick = contact!!.nickName ?: return
+        val nick = labeledNick.property
+        if (nick.values.isEmpty())
             return
 
         val typeCode: Int
         var typeLabel: String? = null
 
-        val type = nick.type?.toLowerCase()
-        typeCode = when (type) {
-            Contact.NICKNAME_TYPE_MAIDEN_NAME -> Nickname.TYPE_MAIDEN_NAME
-            Contact.NICKNAME_TYPE_SHORT_NAME ->  Nickname.TYPE_SHORT_NAME
-            Contact.NICKNAME_TYPE_INITIALS ->    Nickname.TYPE_INITIALS
-            Contact.NICKNAME_TYPE_OTHER_NAME ->  Nickname.TYPE_OTHER_NAME
-            null                             ->  Nickname.TYPE_DEFAULT
-            else -> {
-                typeLabel = xNameToLabel(type)
-                Nickname.TYPE_CUSTOM
+        if (labeledNick.label != null) {
+            typeCode = Nickname.TYPE_CUSTOM
+            typeLabel = labeledNick.label
+        } else {
+            val type = nick.type?.toLowerCase()
+            typeCode = when (type) {
+                Contact.NICKNAME_TYPE_MAIDEN_NAME -> Nickname.TYPE_MAIDEN_NAME
+                Contact.NICKNAME_TYPE_SHORT_NAME ->  Nickname.TYPE_SHORT_NAME
+                Contact.NICKNAME_TYPE_INITIALS ->    Nickname.TYPE_INITIALS
+                null ->                              Nickname.TYPE_DEFAULT
+                else ->                              Nickname.TYPE_OTHER_NAME
             }
         }
 
@@ -979,22 +935,17 @@ open class AndroidContact(
         }
 
         val types = address.types
-        var typeCode = StructuredPostal.TYPE_OTHER
+        val typeCode: Int
         var typeLabel: String? = null
         if (labeledAddress.label != null) {
             typeCode = StructuredPostal.TYPE_CUSTOM
             typeLabel = labeledAddress.label
-        } else {
-            when {
-                types.contains(AddressType.HOME) -> typeCode = StructuredPostal.TYPE_HOME
-                types.contains(AddressType.WORK) -> typeCode = StructuredPostal.TYPE_WORK
-                types.contains(Contact.ADDRESS_TYPE_OTHER) -> {}
-                types.isNotEmpty() -> {
-                    typeCode = StructuredPostal.TYPE_CUSTOM
-                    typeLabel = xNameToLabel(address.types.first().value)
-                }
+        } else
+            typeCode = when {
+                types.contains(AddressType.HOME) -> StructuredPostal.TYPE_HOME
+                types.contains(AddressType.WORK) -> StructuredPostal.TYPE_WORK
+                else -> StructuredPostal.TYPE_OTHER
             }
-        }
 
         val builder = insertDataBuilder(StructuredPostal.RAW_CONTACT_ID)
                 .withValue(StructuredPostal.MIMETYPE, StructuredPostal.CONTENT_ITEM_TYPE)
@@ -1028,11 +979,7 @@ open class AndroidContact(
                 "home" ->                    Website.TYPE_HOME
                 "work" ->                    Website.TYPE_WORK
                 Contact.URL_TYPE_FTP ->      Website.TYPE_FTP
-                null ->                      Website.TYPE_OTHER
-                else -> {
-                    typeLabel = xNameToLabel(type)
-                    Website.TYPE_CUSTOM
-                }
+                else ->                      Website.TYPE_OTHER
             }
         }
 
diff --git a/src/main/java/at/bitfire/vcard4android/Contact.kt b/src/main/java/at/bitfire/vcard4android/Contact.kt
index 56c1f58..0bd7dca 100644
--- a/src/main/java/at/bitfire/vcard4android/Contact.kt
+++ b/src/main/java/at/bitfire/vcard4android/Contact.kt
@@ -48,11 +48,12 @@ class Contact {
     var phoneticMiddleName: String? = null
     var phoneticFamilyName: String? = null
 
-    var nickName: Nickname? = null
+    /** vCard NICKNAME – Android only supports one nickname **/
+    var nickName: LabeledProperty<Nickname>? = null
 
     var organization: Organization? = null
-    var jobTitle: String? = null           // VCard TITLE
-    var jobDescription: String? = null     // VCard ROLE
+    var jobTitle: String? = null           // vCard TITLE
+    var jobDescription: String? = null     // vCard ROLE
 
     val phoneNumbers = LinkedList<LabeledProperty<Telephone>>()
     val emails = LinkedList<LabeledProperty<Email>>()
@@ -69,7 +70,7 @@ class Contact {
 
     var photo: ByteArray? = null
 
-    /** unknown properties in text VCARD format */
+    /** unknown properties in text vCard format */
     var unknownProperties: String? = null
 
 
@@ -86,21 +87,15 @@ class Contact {
         const val PROPERTY_PHONETIC_LAST_NAME = "X-PHONETIC-LAST-NAME"
         const val PROPERTY_SIP = "X-SIP"
 
+        // TEL x-types to store Android types
         val PHONE_TYPE_CALLBACK = TelephoneType.get("x-callback")!!
         val PHONE_TYPE_COMPANY_MAIN = TelephoneType.get("x-company_main")!!
         val PHONE_TYPE_RADIO = TelephoneType.get("x-radio")!!
         val PHONE_TYPE_ASSISTANT = TelephoneType.get("X-assistant")!!
         val PHONE_TYPE_MMS = TelephoneType.get("x-mms")!!
-        /** Sometimes used to denote an "other" phone numbers. Only for compatibility – don't use it yourself! */
-        val PHONE_TYPE_OTHER = TelephoneType.get("other")!!
 
-        /** Custom email type to denote "mobile" email addresses. */
+        // EMAIL x-types to store Android types
         val EMAIL_TYPE_MOBILE = EmailType.get("x-mobile")!!
-        /** Sometimes used to denote an "other" email address. Only for compatibility – don't use it yourself! */
-        val EMAIL_TYPE_OTHER = EmailType.get("other")!!
-
-        /** Sometimes used to denote an "other" postal address. Only for compatibility – don't use it yourself! */
-        val ADDRESS_TYPE_OTHER = AddressType.get("other")!!
 
         const val NICKNAME_TYPE_MAIDEN_NAME = "x-maiden-name"
         const val NICKNAME_TYPE_SHORT_NAME = "x-short-name"
@@ -162,7 +157,7 @@ class Contact {
                         c.familyName = StringUtils.trimToNull(prop.family)
                         c.suffix = StringUtils.trimToNull(prop.suffixes.joinToString(" "))
                     }
-                    is Nickname -> c.nickName = prop
+                    is Nickname -> c.nickName = LabeledProperty(prop, findLabel(prop.group))
 
                     is Organization -> c.organization = prop
                     is Title -> c.jobTitle = StringUtils.trimToNull(prop.value)
@@ -341,7 +336,7 @@ class Contact {
                 }
             }
         if (fn.isNullOrEmpty())
-            nickName?.let { fn = it.values.firstOrNull() }
+            nickName?.let { fn = it.property.values.firstOrNull() }
         if (fn.isNullOrEmpty())
             emails.firstOrNull()?.let { fn = it.property.value }
         if (fn.isNullOrEmpty())
@@ -370,9 +365,6 @@ class Contact {
                 vCard.structuredName = StructuredName()
         }
 
-        // NICKNAME
-        nickName?.let { vCard.nickname = it }
-
         // phonetic names
         phoneticGivenName?.let { vCard.addExtendedProperty(PROPERTY_PHONETIC_FIRST_NAME, it) }
         phoneticMiddleName?.let { vCard.addExtendedProperty(PROPERTY_PHONETIC_MIDDLE_NAME, it) }
@@ -385,7 +377,7 @@ class Contact {
 
         // will be used to count "davdroidXX." property groups
         val labelIterator = AtomicInteger()
-
+        // TODO outside function outside; make clear that it modifies labeledProperty.property
         fun addLabel(labeledProperty: LabeledProperty<VCardProperty>) {
             labeledProperty.label?.let {
                 val group = "group${labelIterator.incrementAndGet()}"
@@ -396,24 +388,27 @@ class Contact {
             }
         }
 
+        // NICKNAME
+        nickName?.let { labeledNickName ->
+            vCard.addNickname(labeledNickName.property)
+            addLabel(labeledNickName)
+        }
+
         // TEL
         for (labeledPhone in phoneNumbers) {
-            val phone = labeledPhone.property
-            vCard.addTelephoneNumber(phone)
+            vCard.addTelephoneNumber(labeledPhone.property)
             addLabel(labeledPhone)
         }
 
         // EMAIL
         for (labeledEmail in emails) {
-            val email = labeledEmail.property
-            vCard.addEmail(email)
+            vCard.addEmail(labeledEmail.property)
             addLabel(labeledEmail)
         }
 
         // IMPP
         for (labeledImpp in impps) {
-            val impp = labeledImpp.property
-            vCard.addImpp(impp)
+            vCard.addImpp(labeledImpp.property)
             addLabel(labeledImpp)
         }
 
diff --git a/src/test/java/at/bitfire/vcard4android/ContactTest.kt b/src/test/java/at/bitfire/vcard4android/ContactTest.kt
index 571511b..ea2c6af 100644
--- a/src/test/java/at/bitfire/vcard4android/ContactTest.kt
+++ b/src/test/java/at/bitfire/vcard4android/ContactTest.kt
@@ -164,8 +164,9 @@ class ContactTest {
         assertTrue(toString(c, GroupMethod.GROUP_VCARDS, VCardVersion.V3_0).contains("\nFN:test@example.com\r\n"))
 
         // nick name available
-        c.nickName = Nickname()
-        c.nickName!!.values += "Nikki"
+        c.nickName = LabeledProperty(Nickname().apply {
+            values += "Nikki"
+        })
         assertTrue(toString(c, GroupMethod.GROUP_VCARDS, VCardVersion.V3_0).contains("\nFN:Nikki\r\n"))
     }
 
@@ -268,7 +269,7 @@ class ContactTest {
         // NICKNAME
         assertEquals(
                 listOf("Nick1", "Nick2"),
-                c.nickName!!.values
+                c.nickName!!.property.values
         )
 
         // ADR