Refactoring

* use Kotlin getters/setters, if possible
* simplify interfaces
* reduce importance of ContactsStorageException (now used as a separate exception only,
  not as a wrapper for RemoteException)

src/androidTest/java/at/bitfire/vcard4android/AndroidAddressBookTest.kt

@@ -51,16 +51,16 @@ class AndroidAddressBookTest {
         var values = ContentValues()
         values.put(ContactsContract.Settings.SHOULD_SYNC, false)
         values.put(ContactsContract.Settings.UNGROUPED_VISIBLE, false)
-        addressBook.updateSettings(values)
-        values = addressBook.getSettings()
+        addressBook.settings = values
+        values = addressBook.settings
         assertFalse(values.getAsInteger(ContactsContract.Settings.SHOULD_SYNC) != 0)
         assertFalse(values.getAsInteger(ContactsContract.Settings.UNGROUPED_VISIBLE) != 0)
 
         values = ContentValues()
         values.put(ContactsContract.Settings.SHOULD_SYNC, true)
         values.put(ContactsContract.Settings.UNGROUPED_VISIBLE, true)
-        addressBook.updateSettings(values)
-        values = addressBook.getSettings()
+        addressBook.settings = values
+        values = addressBook.settings
         assertTrue(values.getAsInteger(ContactsContract.Settings.SHOULD_SYNC) != 0)
         assertTrue(values.getAsInteger(ContactsContract.Settings.UNGROUPED_VISIBLE) != 0)
     }
@@ -69,12 +69,12 @@ class AndroidAddressBookTest {
     fun testSyncState() {
 		val addressBook = TestAddressBook(testAccount, provider)
 
-        addressBook.writeSyncState(ByteArray(0))
-        assertEquals(0, addressBook.readSyncState()!!.size)
+        addressBook.syncState = ByteArray(0)
+        assertEquals(0, addressBook.syncState!!.size)
 
         val random = byteArrayOf(1, 2, 3, 4, 5)
-        addressBook.writeSyncState(random)
-        assertArrayEquals(random, addressBook.readSyncState())
+        addressBook.syncState = random
+        assertArrayEquals(random, addressBook.syncState)
     }
 
 }

src/androidTest/java/at/bitfire/vcard4android/AndroidContactTest.kt

@@ -78,7 +78,7 @@ class AndroidContactTest {
         val contact = AndroidContact(addressBook, vcard, null, null)
         contact.create()
 
-        val contact2 = AndroidContact(addressBook, contact.id!!, null, null)
+        val contact2 = addressBook.findContactByID(contact.id!!)
         try {
             val vcard2 = contact2.contact!!
             assertEquals(vcard.displayName, vcard2.displayName)
@@ -110,7 +110,7 @@ class AndroidContactTest {
         val dbContact = AndroidContact(addressBook, contacts.first(), null, null)
         dbContact.create()
 
-        val dbContact2 = AndroidContact(addressBook, dbContact.id!!, null, null)
+        val dbContact2 = addressBook.findContactByID(dbContact.id!!)
         try {
             val contact2 = dbContact2.contact!!
             assertEquals("Test", contact2.displayName)
@@ -132,7 +132,7 @@ class AndroidContactTest {
         val contact = AndroidContact(addressBook, vcard, null, null)
         contact.create()
 
-        val contact2 = AndroidContact(addressBook, contact.id!!, null, null)
+        val contact2 = addressBook.findContactByID(contact.id!!)
         try {
             val vcard2 = contact2.contact!!
             assertEquals(4000, vcard2.emails.size)
@@ -191,7 +191,7 @@ class AndroidContactTest {
         val contact = AndroidContact(addressBook, vcard, null, null)
         contact.create()
 
-        val contact2 = AndroidContact(addressBook, contact.id!!, null, null)
+        val contact2 = addressBook.findContactByID(contact.id!!)
         try {
             val vcard2 = contact2.contact!!
             assertEquals(vcard.displayName, vcard2.displayName)

src/androidTest/java/at/bitfire/vcard4android/impl/TestAddressBook.kt

@@ -10,6 +10,7 @@ package at.bitfire.vcard4android.impl
 
 import android.accounts.Account
 import android.content.ContentProviderClient
+import android.content.ContentValues
 import at.bitfire.vcard4android.*
 
 class TestAddressBook(
@@ -19,22 +20,16 @@ class TestAddressBook(
 
     object ContactFactory: AndroidContactFactory<AndroidContact> {
 
-        override fun newInstance(addressBook: AndroidAddressBook<AndroidContact, *>, id: Long, fileName: String?, eTag: String?) =
-                AndroidContact(addressBook, id, fileName, eTag)
-
-        override fun newInstance(addressBook: AndroidAddressBook<AndroidContact, *>, contact: Contact, fileName: String?, eTag: String?): AndroidContact =
-                AndroidContact(addressBook, contact, fileName, eTag)
+        override fun fromProvider(addressBook: AndroidAddressBook<AndroidContact, *>, values: ContentValues) =
+                AndroidContact(addressBook, values)
 
     }
 
 
     object GroupFactory: AndroidGroupFactory<AndroidGroup> {
 
-        override fun newInstance(addressBook: AndroidAddressBook<*, AndroidGroup>, id: Long, fileName: String?, eTag: String?) =
-                AndroidGroup(addressBook, id, fileName, eTag)
-
-        override fun newInstance(addressBook: AndroidAddressBook<*, AndroidGroup>, contact: Contact, fileName: String?, eTag: String?) =
-                AndroidGroup(addressBook, contact, fileName, eTag)
+        override fun fromProvider(addressBook: AndroidAddressBook<*, AndroidGroup>, values: ContentValues) =
+                AndroidGroup(addressBook, values)
 
     }
     

src/main/java/at/bitfire/vcard4android/AndroidAddressBook.kt

@@ -26,50 +26,49 @@ open class AndroidAddressBook<T1: AndroidContact, T2: AndroidGroup>(
         val groupFactory: AndroidGroupFactory<T2>
 ) {
 
-    // account-specific address book settings
-
-    /**
-     * Retrieves [ContactsContract.Settings] for the current address book.
-     * @throws FileNotFoundException if the settings row couldn't be fetched.
-     */
-    fun getSettings(): ContentValues {
-        provider!!.query(syncAdapterURI(ContactsContract.Settings.CONTENT_URI), null, null, null, null)?.use { cursor ->
-            if (cursor.moveToNext()) {
-                val values = ContentValues(cursor.columnCount)
-                DatabaseUtils.cursorRowToContentValues(cursor, values)
-                return values
+    var settings: ContentValues
+        /**
+         * Retrieves [ContactsContract.Settings] for the current address book.
+         * @throws FileNotFoundException if the settings row couldn't be fetched.
+         * @throws android.os.RemoteException on content provider errors
+         */
+        get() {
+            provider!!.query(syncAdapterURI(ContactsContract.Settings.CONTENT_URI), null, null, null, null)?.use { cursor ->
+                if (cursor.moveToNext()) {
+                    val values = ContentValues(cursor.columnCount)
+                    DatabaseUtils.cursorRowToContentValues(cursor, values)
+                    return values
+                }
             }
+            throw FileNotFoundException()
         }
-        throw FileNotFoundException()
-    }
-
-    /**
-     * Updates [ContactsContract.Settings] by inserting the given values into
-     * the current address book.
-     * @param values settings to be updated
-     */
-    fun updateSettings(values: ContentValues) {
-        values.put(ContactsContract.Settings.ACCOUNT_NAME, account.name)
-        values.put(ContactsContract.Settings.ACCOUNT_TYPE, account.type)
-        provider!!.insert(syncAdapterURI(ContactsContract.Settings.CONTENT_URI), values)
-    }
-
 
-    // account-specific address book sync state
-
-    fun readSyncState(): ByteArray? = ContactsContract.SyncState.get(provider, account)
-    fun writeSyncState(data: ByteArray) = ContactsContract.SyncState.set(provider, account, data)
+        /**
+         * Updates [ContactsContract.Settings] by inserting the given values into
+         * the current address book.
+         * @param values settings to be updated
+         * @throws android.os.RemoteException on content provider errors
+         */
+        set(values) {
+            values.put(ContactsContract.Settings.ACCOUNT_NAME, account.name)
+            values.put(ContactsContract.Settings.ACCOUNT_TYPE, account.type)
+            provider!!.insert(syncAdapterURI(ContactsContract.Settings.CONTENT_URI), values)
+        }
 
+    var syncState: ByteArray?
+        get() = ContactsContract.SyncState.get(provider, account)
+        set(data: ByteArray?) = ContactsContract.SyncState.set(provider, account, data)
 
-    // groups
 
-    protected fun queryContacts(where: String?, whereArgs: Array<String>?): List<T1> {
+    fun queryContacts(where: String?, whereArgs: Array<String>?): List<T1> {
         val contacts = LinkedList<T1>()
         provider!!.query(rawContactsSyncUri(),
-                arrayOf(RawContacts._ID, AndroidContact.COLUMN_FILENAME, AndroidContact.COLUMN_ETAG),
-                where, whereArgs, null)?.let { cursor ->
-            while (cursor.moveToNext())
-                contacts += contactFactory.newInstance(this, cursor.getLong(0), cursor.getString(1), cursor.getString(2))
+                null, where, whereArgs, null)?.let { cursor ->
+            while (cursor.moveToNext()) {
+                val values = ContentValues(cursor.columnCount)
+                DatabaseUtils.cursorRowToContentValues(cursor, values)
+                contacts += contactFactory.fromProvider(this, values)
+            }
         }
         return contacts
     }
@@ -79,20 +78,30 @@ open class AndroidAddressBook<T1: AndroidContact, T2: AndroidGroup>(
         provider!!.query(groupsSyncUri(),
                 arrayOf(Groups._ID, AndroidGroup.COLUMN_FILENAME, AndroidGroup.COLUMN_ETAG),
                 where, whereArgs, null)?.use { cursor ->
-            while (cursor.moveToNext())
-                groups += groupFactory.newInstance(this, cursor.getLong(0), cursor.getString(1), cursor.getString(2))
+            while (cursor.moveToNext()) {
+                val values = ContentValues(cursor.columnCount)
+                DatabaseUtils.cursorRowToContentValues(cursor, values)
+                groups += groupFactory.fromProvider(this, values)
+            }
         }
         return groups
     }
 
+    fun findContactByID(id: Long) =
+            queryContacts("${RawContacts._ID}=?", arrayOf(id.toString())).firstOrNull()
+                    ?: throw FileNotFoundException()
+
+    fun findContactByUID(uid: String) =
+            queryContacts("${AndroidContact.COLUMN_UID}=?", arrayOf(uid)).firstOrNull()
+
 
     // helpers
 
     fun syncAdapterURI(uri: Uri) = uri.buildUpon()
-                .appendQueryParameter(RawContacts.ACCOUNT_NAME, account.name)
-                .appendQueryParameter(RawContacts.ACCOUNT_TYPE, account.type)
-                .appendQueryParameter(ContactsContract.CALLER_IS_SYNCADAPTER, "true")
-                .build()!!
+            .appendQueryParameter(ContactsContract.CALLER_IS_SYNCADAPTER, "true")
+            .appendQueryParameter(RawContacts.ACCOUNT_NAME, account.name)
+            .appendQueryParameter(RawContacts.ACCOUNT_TYPE, account.type)
+            .build()!!
 
     fun rawContactsSyncUri() = syncAdapterURI(RawContacts.CONTENT_URI)
     fun groupsSyncUri() = syncAdapterURI(Groups.CONTENT_URI)

src/main/java/at/bitfire/vcard4android/AndroidContactFactory.kt

@@ -8,9 +8,10 @@
 
 package at.bitfire.vcard4android
 
+import android.content.ContentValues
+
 interface AndroidContactFactory<T: AndroidContact> {
 
-    fun newInstance(addressBook: AndroidAddressBook<T, out AndroidGroup>, id: Long, fileName: String?, eTag: String?): T
-    fun newInstance(addressBook: AndroidAddressBook<T, out AndroidGroup>, contact: Contact, fileName: String?, eTag: String?): T
+    fun fromProvider(addressBook: AndroidAddressBook<T, out AndroidGroup>, values: ContentValues): T
 
 }

src/main/java/at/bitfire/vcard4android/AndroidGroup.kt

@@ -35,10 +35,10 @@ open class AndroidGroup(
     var fileName: String? = null
     var eTag: String? = null
 
-	constructor(addressBook: AndroidAddressBook<out AndroidContact, out AndroidGroup>, id: Long, fileName: String?, eTag: String?): this(addressBook) {
-		this.id = id
-        this.fileName = fileName
-        this.eTag = eTag
+	constructor(addressBook: AndroidAddressBook<out AndroidContact, out AndroidGroup>, values: ContentValues): this(addressBook) {
+		this.id = values.getAsLong(Groups._ID)
+        this.fileName = values.getAsString(COLUMN_FILENAME)
+        this.eTag = values.getAsString(COLUMN_ETAG)
 	}
 
     constructor(addressBook: AndroidAddressBook<out AndroidContact, out AndroidGroup>, contact: Contact, fileName: String?  = null, eTag: String? = null): this(addressBook) {
@@ -47,21 +47,20 @@ open class AndroidGroup(
         this.eTag = eTag
 	}
 
-
     var contact: Contact? = null
     /**
-     * Creates a {@link Contact} (representation of a VCard) from the group.
-     * @throws IllegalArgumentException if group is not persistent yet ({@link #id} is null)
+     * Creates a [Contact] (representation of a vCard) from the group.
+     * @throws IllegalArgumentException if group has not been saved yet
+     * @throws FileNotFoundException when the group is not available (anymore)
+     * @throws RemoteException on contact provider errors
      */
-    @Throws(FileNotFoundException::class, ContactsStorageException::class)
-    get() {
-        field?.let { return field }
+        get() {
+            field?.let { return field }
 
-        val id = requireNotNull(id)
-        val c = Contact()
-        try {
-			addressBook.provider!!.query(addressBook.syncAdapterURI(ContentUris.withAppendedId(Groups.CONTENT_URI, id)),
-					arrayOf(COLUMN_UID, Groups.TITLE, Groups.NOTES), null, null, null)?.use { cursor ->
+            val id = requireNotNull(id)
+            val c = Contact()
+            addressBook.provider!!.query(addressBook.syncAdapterURI(ContentUris.withAppendedId(Groups.CONTENT_URI, id)),
+                    arrayOf(COLUMN_UID, Groups.TITLE, Groups.NOTES), null, null, null)?.use { cursor ->
                 if (!cursor.moveToNext())
                     throw FileNotFoundException("Contact group not found")
 
@@ -95,13 +94,9 @@ open class AndroidGroup(
 
             field = c
             return c
-		} catch (e: RemoteException) {
-			throw ContactsStorageException("Couldn't read contact group", e)
-		}
-	}
+        }
 
 
-    @Throws(FileNotFoundException::class, ContactsStorageException::class)
     protected open fun contentValues(): ContentValues {
         val values = ContentValues()
         values.put(COLUMN_FILENAME, fileName)
@@ -117,57 +112,35 @@ open class AndroidGroup(
     /**
      * Creates a group with data taken from the constructor.
      * @return number of affected rows
-     * @throws ContactsStorageException in case of content provider exception
+     * @throws RemoteException on contact provider errors
      */
-    @Throws(ContactsStorageException::class)
     fun create(): Uri {
         val values = contentValues()
 		values.put(Groups.ACCOUNT_TYPE, addressBook.account.type)
 		values.put(Groups.ACCOUNT_NAME, addressBook.account.name)
         values.put(Groups.SHOULD_SYNC, 1)
         // read-only: values.put(Groups.GROUP_VISIBLE, 1);
-		try {
-			val uri = addressBook.provider!!.insert(addressBook.syncAdapterURI(Groups.CONTENT_URI), values)
-			id = ContentUris.parseId(uri)
-			return uri
-		} catch (e: RemoteException) {
-			throw ContactsStorageException("Couldn't create contact group", e)
-		}
+        val uri = addressBook.provider!!.insert(addressBook.syncAdapterURI(Groups.CONTENT_URI), values)
+        id = ContentUris.parseId(uri)
+        return uri
 	}
 
-    @Throws(ContactsStorageException::class)
-    fun delete() =
-		try {
-			addressBook.provider!!.delete(groupSyncURI(), null, null)
-		} catch (e: RemoteException) {
-			throw ContactsStorageException("Couldn't delete contact group", e)
-		}
-
-    @Throws(ContactsStorageException::class)
-    fun update(values: ContentValues) =
-        try {
-            addressBook.provider!!.update(groupSyncURI(), values, null, null)
-        } catch (e: RemoteException) {
-            throw ContactsStorageException("Couldn't delete contact group", e)
-        }
+    fun update(values: ContentValues) = addressBook.provider!!.update(groupSyncURI(), values, null, null)
+    fun delete() = addressBook.provider!!.delete(groupSyncURI(), null, null)
 
     /**
-     * Updates a group from a {@link Contact}, which represents a VCard received from the
+     * Updates a group from a [Contact], which represents a vCard received from the
      * CardDAV server.
      * @param contact data object to take group title, members etc. from
      * @return number of affected rows
-     * @throws ContactsStorageException in case of a content provider exception
+     * @throws RemoteException on contact provider errors
      */
-    @Throws(ContactsStorageException::class)
     fun updateFromServer(contact: Contact): Int {
         this.contact = contact
         return update(contentValues())
     }
 
 
-    override fun toString() = ToStringBuilder.reflectionToString(this)!!
-
-
     // helpers
 
     private fun groupSyncURI(): Uri {
@@ -175,4 +148,6 @@ open class AndroidGroup(
         return addressBook.syncAdapterURI(ContentUris.withAppendedId(ContactsContract.Groups.CONTENT_URI, id))
     }
 
+    override fun toString() = ToStringBuilder.reflectionToString(this)!!
+
 }

src/main/java/at/bitfire/vcard4android/AndroidGroupFactory.kt

@@ -8,9 +8,10 @@
 
 package at.bitfire.vcard4android
 
+import android.content.ContentValues
+
 interface AndroidGroupFactory<T: AndroidGroup> {
 
-    fun newInstance(addressBook: AndroidAddressBook<out AndroidContact, T>, id: Long, fileName: String?, eTag: String?): T
-    fun newInstance(addressBook: AndroidAddressBook<out AndroidContact, T>, contact: Contact, fileName: String?, eTag: String?): T
+    fun fromProvider(addressBook: AndroidAddressBook<out AndroidContact, T>, values: ContentValues): T
 
 }

src/main/java/at/bitfire/vcard4android/BatchOperation.kt

@@ -25,14 +25,13 @@ class BatchOperation(
 
     fun enqueue(operation: Operation) = queue.add(operation)
 
-    @Throws(ContactsStorageException::class)
     fun commit(): Int {
         var affected = 0
         if (!queue.isEmpty())
             try {
                 Constants.log.fine("Committing ${queue.size} operations …")
 
-                results = Array<ContentProviderResult?>(queue.size, { null })
+                results = Array(queue.size, { null })
                 runBatch(0, queue.size)
 
                 for (result in results.filterNotNull())
@@ -54,16 +53,14 @@ class BatchOperation(
 
 
     /**
-     * Runs a subset of the operations in {@link #queue} using {@link #providerClient} in a transaction.
-     * Catches {@link TransactionTooLargeException} and splits the operations accordingly.
-     * @param start    index of first operation which will be run (inclusive)
-     * @param end      index of last operation which will be run (exclusive!)
-     * @throws RemoteException  if the provider clients throws a {@link RemoteException}, or
-     *                          if the transaction is too large and can't be split
-     * @throws OperationApplicationException
-     * @throws ContactsStorageException
+     * Runs a subset of the operations in [queue] using [providerClient] in a transaction.
+     * Catches [TransactionTooLargeException] and splits the operations accordingly.
+     * @param start index of first operation which will be run (inclusive)
+     * @param end   index of last operation which will be run (exclusive!)
+     * @throws RemoteException on contact provider errors
+     * @throws OperationApplicationException when the batch can't be processed
+     * @throws ContactsStorageException if the transaction is too large or if the batch operation failed partially
      */
-    @Throws(RemoteException::class, OperationApplicationException::class, ContactsStorageException::class)
     private fun runBatch(start: Int, end: Int) {
         if (end == start)
             return     // nothing to do
@@ -80,7 +77,7 @@ class BatchOperation(
         } catch(e: TransactionTooLargeException) {
             if (end <= start + 1)
             // only one operation, can't be split
-                throw RemoteException("Can't transfer data to content provider (data row too large)")
+                throw ContactsStorageException("Can't transfer data to content provider (data row too large)")
 
             Constants.log.warning("Transaction too large, splitting (losing atomicity)")
             val mid = start + (end - start)/2
@@ -89,7 +86,7 @@ class BatchOperation(
         }
     }
 
-    fun toCPO(start: Int, end: Int): ArrayList<ContentProviderOperation> {
+    private fun toCPO(start: Int, end: Int): ArrayList<ContentProviderOperation> {
         val cpo = ArrayList<ContentProviderOperation>(end - start)
 
         for ((i, op) in queue.subList(start, end).withIndex()) {
@@ -116,7 +113,7 @@ class BatchOperation(
     }
 
 
-    class Operation @JvmOverloads constructor(
+    class Operation constructor(
             val builder: ContentProviderOperation.Builder,
             val backrefKey: String? = null,
             val backrefIdx: Int = -1

src/main/java/at/bitfire/vcard4android/ContactsStorageException.kt

@@ -8,5 +8,4 @@
 
 package at.bitfire.vcard4android
 
-@Deprecated("Unnecessary generic wrapper around RemoteException")
-class ContactsStorageException @JvmOverloads constructor(message: String?, ex: Throwable? = null): Exception(message, ex)
+class ContactsStorageException constructor(message: String?, ex: Throwable? = null): Exception(message, ex)