Write photos to asset file instead of PHOTO blob (#7)

* Write photos to asset file instead of PHOTO blob (closes #6)
* Writing contact photos: validate and throw exception when a photo can't be written
* Tests

8 files changed, 265 insertions, 90 deletions

diff --git a/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoBuilderTest.kt b/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoBuilderTest.kt
index e4aca82..8b5ce30 100644
--- a/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoBuilderTest.kt
+++ b/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoBuilderTest.kt
@@ -4,47 +4,120 @@
 
 package at.bitfire.vcard4android.contactrow
 
+import android.Manifest
+import android.accounts.Account
+import android.content.ContentProviderClient
+import android.content.ContentUris
+import android.graphics.BitmapFactory
 import android.net.Uri
-import android.provider.ContactsContract.CommonDataKinds.Photo
+import android.provider.ContactsContract
+import android.provider.ContactsContract.RawContacts
 import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.rule.GrantPermissionRule
+import at.bitfire.vcard4android.AndroidContact
 import at.bitfire.vcard4android.Contact
+import at.bitfire.vcard4android.impl.TestAddressBook
 import org.apache.commons.io.IOUtils
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
+import org.junit.Assert
+import org.junit.Assert.*
+import org.junit.BeforeClass
+import org.junit.ClassRule
 import org.junit.Test
 import kotlin.random.Random
 
 class PhotoBuilderTest {
 
+    companion object {
+        @JvmField
+        @ClassRule
+        val permissionRule = GrantPermissionRule.grant(Manifest.permission.READ_CONTACTS, Manifest.permission.WRITE_CONTACTS)!!
+
+        private val testAccount = Account("AndroidContactTest", "at.bitfire.vcard4android")
+
+        val testContext = InstrumentationRegistry.getInstrumentation().context
+        private lateinit var provider: ContentProviderClient
+        private lateinit var addressBook: TestAddressBook
+
+        @BeforeClass
+        @JvmStatic
+        fun connect() {
+            provider = testContext.contentResolver.acquireContentProviderClient(ContactsContract.AUTHORITY)!!
+            Assert.assertNotNull(provider)
+
+            addressBook = TestAddressBook(testAccount, provider)
+        }
+
+        @BeforeClass
+        @JvmStatic
+        fun disconnect() {
+            @Suppress("DEPRECATION")
+            provider.release()
+        }
+    }
+
+
     @Test
-    fun testEmpty() {
+    fun testBuild_NoPhoto() {
         PhotoBuilder(Uri.EMPTY, null, Contact()).build().also { result ->
             assertEquals(0, result.size)
         }
     }
 
-
     @Test
-    fun testPhoto_NoResize() {
+    fun testBuild_Photo() {
         val blob = ByteArray(1024) { Random.nextInt().toByte() }
-        assertTrue(blob.size < PhotoBuilder.MAX_PHOTO_BLOB_SIZE)
         PhotoBuilder(Uri.EMPTY, null, Contact().apply {
             photo = blob
         }).build().also { result ->
-            assertEquals(Photo.CONTENT_ITEM_TYPE, result[0].values[Photo.MIMETYPE])
-            assertEquals(blob, result[0].values[Photo.PHOTO])
+            // no row because photos have to be inserted with a separate call to insertPhoto()
+            assertEquals(0, result.size)
         }
     }
 
+
     @Test
-    fun testPhoto_Resize() {
-        val blob = IOUtils.readFully(InstrumentationRegistry.getInstrumentation().context.assets.open("large.jpg"), 3519652)
-        assertTrue(blob.size > PhotoBuilder.MAX_PHOTO_BLOB_SIZE)
-        PhotoBuilder(Uri.EMPTY, null, Contact().apply {
-            photo = blob
-        }).build().also { result ->
-            assertEquals(Photo.CONTENT_ITEM_TYPE, result[0].values[Photo.MIMETYPE])
-            assertTrue((result[0].values[Photo.PHOTO] as ByteArray).size < PhotoBuilder.MAX_PHOTO_BLOB_SIZE)
+    fun testInsertPhoto() {
+        val contact = AndroidContact(addressBook, Contact().apply { displayName = "Contact with photo" }, null, null)
+        val contactUri = contact.add()
+        val rawContactId = ContentUris.parseId(contactUri)
+
+        try {
+            val photo = IOUtils.resourceToByteArray("/large.jpg")
+            val photoUri = PhotoBuilder.insertPhoto(provider, testAccount, rawContactId, photo)
+            assertNotNull(photoUri)
+
+            // the photo is processed and often resized by the contacts provider
+            val contact2 = addressBook.findContactById(rawContactId)
+            val photo2 = contact2.getContact().photo!!
+
+            // verify that the image is in JPEG format (some Samsung devices seem to save as PNG)
+            val options = BitmapFactory.Options()
+            options.inJustDecodeBounds = true
+            BitmapFactory.decodeByteArray(photo2, 0, photo2.size, options)
+            assertEquals("image/jpeg", options.outMimeType)
+
+            // verify that contact is not dirty
+            provider.query(
+                ContentUris.withAppendedId(RawContacts.CONTENT_URI, rawContactId),
+                arrayOf(RawContacts.DIRTY),
+                null, null, null
+            )!!.use { cursor ->
+                assertTrue(cursor.moveToNext())
+                assertEquals(0, cursor.getInt(0))
+            }
+        } finally {
+            contact.delete()
+        }
+    }
+
+    @Test(expected = IllegalArgumentException::class)
+    fun testInsertPhoto_Invalid() {
+        val contact = AndroidContact(addressBook, Contact().apply { displayName = "Contact with photo" }, null, null)
+        contact.add()
+        try {
+            val photoUri = PhotoBuilder.insertPhoto(provider, testAccount, contact.id!!, ByteArray(100) /* invalid photo  */)
+        } finally {
+            contact.delete()
         }
     }
 
diff --git a/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoHandlerTest.kt b/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoHandlerTest.kt
index 0ac09b3..fea2217 100644
--- a/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoHandlerTest.kt
+++ b/src/androidTest/java/at/bitfire/vcard4android/contactrow/PhotoHandlerTest.kt
@@ -4,16 +4,58 @@
 
 package at.bitfire.vcard4android.contactrow
 
+import android.Manifest
+import android.accounts.Account
+import android.content.ContentProviderClient
+import android.content.ContentUris
 import android.content.ContentValues
+import android.provider.ContactsContract
 import android.provider.ContactsContract.CommonDataKinds.Photo
+import android.provider.ContactsContract.RawContacts
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.rule.GrantPermissionRule
+import at.bitfire.vcard4android.AndroidContact
 import at.bitfire.vcard4android.Contact
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertNull
+import at.bitfire.vcard4android.impl.TestAddressBook
+import org.apache.commons.io.IOUtils
+import org.junit.Assert
+import org.junit.Assert.*
+import org.junit.BeforeClass
+import org.junit.ClassRule
 import org.junit.Test
 import kotlin.random.Random
 
 class PhotoHandlerTest {
 
+    companion object {
+        @JvmField
+        @ClassRule
+        val permissionRule = GrantPermissionRule.grant(Manifest.permission.READ_CONTACTS, Manifest.permission.WRITE_CONTACTS)!!
+
+        private val testAccount = Account("AndroidContactTest", "at.bitfire.vcard4android")
+
+        val testContext = InstrumentationRegistry.getInstrumentation().context
+        private lateinit var provider: ContentProviderClient
+        private lateinit var addressBook: TestAddressBook
+
+        @BeforeClass
+        @JvmStatic
+        fun connect() {
+            provider = testContext.contentResolver.acquireContentProviderClient(ContactsContract.AUTHORITY)!!
+            Assert.assertNotNull(provider)
+
+            addressBook = TestAddressBook(testAccount, provider)
+        }
+
+        @BeforeClass
+        @JvmStatic
+        fun disconnect() {
+            @Suppress("DEPRECATION")
+            provider.release()
+        }
+    }
+
+
     @Test
     fun testPhoto_Empty() {
         val contact = Contact()
@@ -33,6 +75,38 @@ class PhotoHandlerTest {
         assertEquals(blob, contact.photo)
     }
 
-    // TODO testPhoto_FileId
+    @Test
+    fun testPhoto_FileId() {
+        val contact = Contact().apply {
+            displayName = "Contact with photo"
+            photo = IOUtils.resourceToByteArray("/large.jpg")
+        }
+        val androidContact = AndroidContact(addressBook, contact, null, null)
+        val rawContactId = ContentUris.parseId(androidContact.add())
+
+        val dataUri = ContentUris.withAppendedId(RawContacts.CONTENT_URI, rawContactId).buildUpon()
+            .appendPath(RawContacts.Data.CONTENT_DIRECTORY)
+            .build()
+        val thumbnail = provider.query(dataUri, arrayOf(Photo.PHOTO_FILE_ID, Photo.PHOTO),
+            "${RawContacts.Data.MIMETYPE}=?", arrayOf(Photo.CONTENT_ITEM_TYPE),
+            null
+        )!!.use { cursor ->
+            assertTrue(cursor.moveToNext())
+
+            val fileId = cursor.getLong(0)
+            assertNotNull(fileId)
+
+            val blob = cursor.getBlob(1)
+            assertNotNull(blob)
+            blob!!
+        }
+
+        val contact2 = addressBook.findContactById(rawContactId)
+        // now PhotoHandler handles the PHOTO_FILE_ID
+        val photo2 = contact2.getContact().photo
+        assertNotNull(photo2)
+        // make sure PhotoHandler didn't just return the thumbnail blob
+        assertNotEquals(thumbnail, photo2)
+    }
 
 }
\ No newline at end of file
diff --git a/src/androidTest/assets/large.jpg b/src/androidTest/resources/large.jpg
index ef071b9..ef071b9 100644
--- a/src/androidTest/assets/large.jpg
+++ b/src/androidTest/resources/large.jpg
diff --git a/src/androidTest/assets/large.jpg.LICENSE b/src/androidTest/resources/large.jpg.LICENSE
index 2254977..2254977 100644
--- a/src/androidTest/assets/large.jpg.LICENSE
+++ b/src/androidTest/resources/large.jpg.LICENSE
diff --git a/src/main/java/at/bitfire/vcard4android/AndroidContact.kt b/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
index decc0ba..31556e5 100644
--- a/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
+++ b/src/main/java/at/bitfire/vcard4android/AndroidContact.kt
@@ -15,6 +15,7 @@ import android.provider.ContactsContract.RawContacts
 import android.provider.ContactsContract.RawContacts.Data
 import androidx.annotation.CallSuper
 import at.bitfire.vcard4android.contactrow.ContactProcessor
+import at.bitfire.vcard4android.contactrow.PhotoBuilder
 import org.apache.commons.lang3.builder.ToStringBuilder
 import java.io.FileNotFoundException
 
@@ -112,7 +113,8 @@ open class AndroidContact(
 
 
     fun add(): Uri {
-        val batch = BatchOperation(addressBook.provider!!)
+        val provider = addressBook.provider!!
+        val batch = BatchOperation(provider)
 
         val builder = BatchOperation.CpoBuilder.newInsert(addressBook.syncAdapterURI(RawContacts.CONTENT_URI))
         buildContact(builder, false)
@@ -124,13 +126,18 @@ open class AndroidContact(
         val resultUri = batch.getResult(0)?.uri ?: throw ContactsStorageException("Empty result from content provider when adding contact")
         id = ContentUris.parseId(resultUri)
 
+        getContact().photo?.let { photo ->
+            PhotoBuilder.insertPhoto(provider, addressBook.account, id!!, photo)
+        }
+
         return resultUri
     }
 
     fun update(data: Contact): Uri {
         setContact(data)
 
-        val batch = BatchOperation(addressBook.provider!!)
+        val provider = addressBook.provider!!
+        val batch = BatchOperation(provider)
         val uri = rawContactSyncURI()
         val builder = BatchOperation.CpoBuilder.newUpdate(uri)
         buildContact(builder, true)
@@ -151,6 +158,10 @@ open class AndroidContact(
         insertDataRows(batch)
         batch.commit()
 
+        getContact().photo?.let { photo ->
+            PhotoBuilder.insertPhoto(provider, addressBook.account, id!!, photo)
+        }
+
         return uri
     }
 
diff --git a/src/main/java/at/bitfire/vcard4android/Utils.kt b/src/main/java/at/bitfire/vcard4android/Utils.kt
index b0aa5c2..5e667bf 100644
--- a/src/main/java/at/bitfire/vcard4android/Utils.kt
+++ b/src/main/java/at/bitfire/vcard4android/Utils.kt
@@ -4,9 +4,12 @@
 
 package at.bitfire.vcard4android
 
+import android.accounts.Account
 import android.content.ContentValues
 import android.database.Cursor
 import android.database.DatabaseUtils
+import android.net.Uri
+import android.provider.ContactsContract
 
 object Utils {
 
@@ -16,4 +19,10 @@ object Utils {
         return values
     }
 
+    fun Uri.asSyncAdapter(account: Account): Uri = buildUpon()
+        .appendQueryParameter(ContactsContract.RawContacts.ACCOUNT_NAME, account.name)
+        .appendQueryParameter(ContactsContract.RawContacts.ACCOUNT_TYPE, account.type)
+        .appendQueryParameter(ContactsContract.CALLER_IS_SYNCADAPTER, "true")
+        .build()
+
 }
\ No newline at end of file
diff --git a/src/main/java/at/bitfire/vcard4android/contactrow/PhotoBuilder.kt b/src/main/java/at/bitfire/vcard4android/contactrow/PhotoBuilder.kt
index 6fb567e..f91f80e 100644
--- a/src/main/java/at/bitfire/vcard4android/contactrow/PhotoBuilder.kt
+++ b/src/main/java/at/bitfire/vcard4android/contactrow/PhotoBuilder.kt
@@ -4,93 +4,100 @@
 
 package at.bitfire.vcard4android.contactrow
 
-import android.graphics.Bitmap
+import android.accounts.Account
+import android.content.ContentProviderClient
+import android.content.ContentUris
+import android.content.ContentValues
 import android.graphics.BitmapFactory
-import android.media.ThumbnailUtils
 import android.net.Uri
 import android.provider.ContactsContract.CommonDataKinds.Photo
+import android.provider.ContactsContract.RawContacts
 import at.bitfire.vcard4android.BatchOperation
 import at.bitfire.vcard4android.Constants
 import at.bitfire.vcard4android.Contact
-import java.io.ByteArrayOutputStream
-import java.util.*
-import kotlin.math.min
+import at.bitfire.vcard4android.ContactsStorageException
+import at.bitfire.vcard4android.Utils.asSyncAdapter
+import java.util.logging.Level
 
 class PhotoBuilder(dataRowUri: Uri, rawContactId: Long?, contact: Contact)
     : DataRowBuilder(Factory.mimeType(), dataRowUri, rawContactId, contact) {
 
     companion object {
-        const val MAX_PHOTO_BLOB_SIZE = 950*1024    // IPC limit 1 MB, minus 50 kB for the protocol itself = 950 kB
-        const val MAX_RESIZE_PASSES = 10
-    }
-
-    override fun build(): List<BatchOperation.CpoBuilder> {
-        // The following approach would be correct, but it doesn't work:
-        // the ContactsProvider handler will process the image in background and update
-        // the raw contact with the new photo ID when it's finished, setting it to dirty again!
-        // See https://code.google.com/p/android/issues/detail?id=226875
-
-        /*Uri photoUri = addressBook.syncAdapterURI(Uri.withAppendedPath(
-                ContentUris.withAppendedId(RawContacts.CONTENT_URI, id),
-                RawContacts.DisplayPhoto.CONTENT_DIRECTORY));
-        Constants.log.debug("Setting local photo " + photoUri);
-        try {
-            @Cleanup AssetFileDescriptor fd = addressBook.provider.openAssetFile(photoUri, "w");
-            @Cleanup OutputStream stream = fd.createOutputStream();
-            if (stream != null)
-                stream.write(photo);
-            else
-                Constants.log.warn("Couldn't create local contact photo file");
-        } catch (IOException|RemoteException e) {
-            Constants.log.warn("Couldn't write local contact photo file", e);
-        }*/
-
-        val result = LinkedList<BatchOperation.CpoBuilder>()
-        contact.photo?.let { photo ->
-            val resized = resizeIfNecessary(photo)
-            if (resized != null)
-                result += newDataRow().withValue(Photo.PHOTO, resized)
-        }
-        return result
-    }
-
-    private fun resizeIfNecessary(blob: ByteArray): ByteArray? {
-        if (blob.size > MAX_PHOTO_BLOB_SIZE) {
-            Constants.log.fine("Photo larger than $MAX_PHOTO_BLOB_SIZE bytes, resizing")
 
-            val bitmap = BitmapFactory.decodeByteArray(blob, 0, blob.size)
-            if (bitmap == null) {
-                Constants.log.warning("Image decoding failed")
-                return null
+        /**
+         * Inserts a raw contact photo and resets [RawContacts.DIRTY] to 0 then.
+         *
+         * If the contact provider needs more than 7 seconds to insert the photo, this
+         * method will time out and throw a [ContactsStorageException]. In this case, the
+         * [RawContacts.DIRTY] flag may be set asynchronously by the contacts provider
+         * as soon as it finishes the operation.
+         *
+         * @param provider      client to access contacts provider
+         * @param account       account of the contact, used to create sync adapter URIs
+         * @param rawContactId  ID of the raw contact ([RawContacts._ID]])
+         * @param data          contact photo (binary data in a supported format like JPEG or PNG)
+         *
+         * @return URI of the raw contact display photo ([Photo.PHOTO_URI])
+         *
+         * @throws ContactsStorageException when the image couldn't be written
+         */
+        fun insertPhoto(provider: ContentProviderClient, account: Account, rawContactId: Long, data: ByteArray): Uri? {
+            // verify that data can be decoded by BitmapFactory, so that the contacts provider can process it
+            val valid = BitmapFactory.decodeByteArray(data, 0, data.size) != null
+            if (!valid)
+                throw IllegalArgumentException("Image can't be decoded")
+
+            // write file to contacts provider
+            val uri = RawContacts.CONTENT_URI.buildUpon()
+                .appendPath(rawContactId.toString())
+                .appendPath(RawContacts.DisplayPhoto.CONTENT_DIRECTORY)
+                .build()
+            Constants.log.log(Level.FINE, "Writing photo to $uri (${data.size} bytes)")
+            provider.openAssetFile(uri, "w")?.use { fd ->
+                fd.createOutputStream()?.use { os ->
+                    os.write(data)
+                }
             }
 
-            var size = min(bitmap.width, bitmap.height).toFloat()
-            var resized: ByteArray = blob
-            var count = 0
-            var quality = 98
-            do {
-                if (++count > MAX_RESIZE_PASSES) {
-                    Constants.log.warning("Couldn't resize photo within $MAX_RESIZE_PASSES passes")
-                    return null
+            // photo is now processed in the background; wait until it is available
+            var photoUri: Uri? = null
+            for (i in 1..70) {      // wait max. 70x100 ms = 7 seconds
+                val dataRowUri = RawContacts.CONTENT_URI.buildUpon()
+                    .appendPath(rawContactId.toString())
+                    .appendPath(RawContacts.Data.CONTENT_DIRECTORY)
+                    .build()
+                provider.query(dataRowUri, arrayOf(Photo.PHOTO_URI), "${RawContacts.Data.MIMETYPE}=?", arrayOf(Photo.CONTENT_ITEM_TYPE), null)?.use { cursor ->
+                    if (cursor.moveToNext())
+                        cursor.getString(0)?.let { uriStr ->
+                            photoUri = Uri.parse(uriStr)
+                        }
                 }
+                if (photoUri != null)
+                    break
+                Thread.sleep(100)
+            }
 
-                val sizeInt = size.toInt()
-                val thumb = ThumbnailUtils.extractThumbnail(bitmap, sizeInt, sizeInt)
-                val baos = ByteArrayOutputStream()
-                if (thumb.compress(Bitmap.CompressFormat.JPEG, quality, baos))
-                    resized = baos.toByteArray()
+            // reset dirty flag in any case (however if we didn't wait long enough, the dirty flag will then be set again)
+            val notDirty = ContentValues(1)
+            notDirty.put(RawContacts.DIRTY, 0)
+            val rawContactUri = ContentUris.withAppendedId(RawContacts.CONTENT_URI, rawContactId).asSyncAdapter(account)
+            provider.update(rawContactUri, notDirty, null, null)
 
-                size *= .9f
-                quality--
-            } while (resized.size >= MAX_PHOTO_BLOB_SIZE)
+            if (photoUri != null)
+                Constants.log.log(Level.FINE, "Photo has been inserted: $photoUri")
+            else
+                throw ContactsStorageException("Couldn't store contact photo")
 
-            return resized
+            return photoUri
+        }
 
-        } else
-            return blob
     }
 
 
+    override fun build(): List<BatchOperation.CpoBuilder> =
+        emptyList()     // data row must be inserted by calling insertPhoto()
+
+
     object Factory: DataRowBuilder.Factory<PhotoBuilder> {
         override fun mimeType() = Photo.CONTENT_ITEM_TYPE
         override fun newInstance(dataRowUri: Uri, rawContactId: Long?, contact: Contact) =
diff --git a/src/main/java/at/bitfire/vcard4android/contactrow/PhotoHandler.kt b/src/main/java/at/bitfire/vcard4android/contactrow/PhotoHandler.kt
index 01d58c1..de17724 100644
--- a/src/main/java/at/bitfire/vcard4android/contactrow/PhotoHandler.kt
+++ b/src/main/java/at/bitfire/vcard4android/contactrow/PhotoHandler.kt
@@ -22,8 +22,7 @@ class PhotoHandler(val provider: ContentProviderClient?): DataRowHandler() {
     override fun handle(values: ContentValues, contact: Contact) {
         super.handle(values, contact)
 
-        val photoId = values.getAsLong(Photo.PHOTO_FILE_ID)
-        if (photoId != null) {
+        values.getAsLong(Photo.PHOTO_FILE_ID)?.let { photoId ->
             val photoUri = ContentUris.withAppendedId(ContactsContract.DisplayPhoto.CONTENT_URI, photoId)
             try {
                 provider?.openAssetFile(photoUri, "r")?.let { file ->
@@ -34,7 +33,9 @@ class PhotoHandler(val provider: ContentProviderClient?): DataRowHandler() {
             } catch(e: IOException) {
                 Constants.log.log(Level.WARNING, "Couldn't read local contact photo file", e)
             }
-        } else
+        }
+
+        if (contact.photo == null)
             contact.photo = values.getAsByteArray(Photo.PHOTO)
     }