Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit be98bef4 authored by Ricki Hirner's avatar Ricki Hirner
Browse files

Sync adapter: add tests, move check of simultaneous running syncs to new ConcurrentUtils

parent 1747cb28
Loading
Loading
Loading
Loading
+129 −0
Original line number Diff line number Diff line
package at.bitfire.davdroid

import android.accounts.Account
import android.content.ContentProviderClient
import android.content.Context
import android.content.SyncResult
import android.os.Bundle
import androidx.test.platform.app.InstrumentationRegistry
import at.bitfire.davdroid.syncadapter.SyncAdapterService
import org.junit.Assert.*
import org.junit.Before
import org.junit.Test
import java.util.concurrent.atomic.AtomicInteger

class SyncAdapterTest {

    val context = InstrumentationRegistry.getInstrumentation().context
    val targetContext = InstrumentationRegistry.getInstrumentation().targetContext

    /** use our WebDAV provider as a mock provider because it's our own and we don't need any permissions for it */
    val mockAuthority = targetContext.getString(R.string.webdav_authority)
    val mockProvider = context.contentResolver.acquireContentProviderClient(mockAuthority)!!

    val account = Account("test", "com.example.test")

    lateinit var syncAdapter: TestSyncAdapter


    @Before
    fun createSyncAdapter() {
        syncAdapter = TestSyncAdapter(context)
    }


    @Test
    fun testPriorityCollections() {
        assertTrue(SyncAdapterService.SyncAdapter.priorityCollections(Bundle()).isEmpty())
        assertArrayEquals(arrayOf(1L,2L), SyncAdapterService.SyncAdapter.priorityCollections(Bundle(1).apply {
            putString(SyncAdapterService.SYNC_EXTRAS_PRIORITY_COLLECTIONS, "1,error,2")
        }).toTypedArray())
    }


    @Test
    fun testOnPerformSync_allowsSequentialSyncs() {
        for (i in 0 until 5)
            syncAdapter.onPerformSync(account, Bundle(), mockAuthority, mockProvider, SyncResult())
        assertEquals(5, syncAdapter.syncCalled.get())
    }

    @Test
    fun testOnPerformSync_allowsSimultaneousSyncs() {
        val extras = Bundle(1)
        extras.putLong(TestSyncAdapter.EXTRA_WAIT, 100)    // sync takes 100 ms

        val syncThreads = mutableListOf<Thread>()
        for (i in 0 until 100) {                            // within 100 ms, at least 2 threads should be created and run simultaneously
            syncThreads += Thread({
                syncAdapter.onPerformSync(account, extras, "$mockAuthority-$i", mockProvider, SyncResult())
            }).apply {
                start()
            }
        }

        // wait for all threads
        syncThreads.forEach { it.join() }

        assertEquals(100, syncAdapter.syncCalled.get())
    }

    @Test
    fun testOnPerformSync_preventsDuplicateSyncs() {
        val extras = Bundle(1)
        extras.putLong(TestSyncAdapter.EXTRA_WAIT, 500)    // sync takes 500 ms

        val syncThreads = mutableListOf<Thread>()
        for (i in 0 until 100) {        // creating 100 threads should be possible within in 500 ms
            syncThreads += Thread({
                syncAdapter.onPerformSync(account, extras, mockAuthority, mockProvider, SyncResult())
            }).apply {
                start()
            }
        }

        // wait for all threads
        syncThreads.forEach { it.join() }

        assertEquals(1, syncAdapter.syncCalled.get())
    }

    @Test
    fun testOnPerformSync_runsSyncAndSetsClassLoader() {
        syncAdapter.onPerformSync(account, Bundle(), mockAuthority, mockProvider, SyncResult())

        // check whether onPerformSync() actually calls sync()
        assertEquals(1, syncAdapter.syncCalled.get())

        // check whether contextClassLoader is set
        assertEquals(context.classLoader, Thread.currentThread().contextClassLoader)
    }


    class TestSyncAdapter(context: Context): SyncAdapterService.SyncAdapter(context) {

        companion object {
            /**
             * How long the sync() method shall wait
             */
            val EXTRA_WAIT = "waitMillis"
        }

        val syncCalled = AtomicInteger()

        override fun sync(
            account: Account,
            extras: Bundle,
            authority: String,
            provider: ContentProviderClient,
            syncResult: SyncResult
        ) {
            val wait = extras.getLong(EXTRA_WAIT)
            Thread.sleep(wait)

            syncCalled.incrementAndGet()
        }

    }

}
 No newline at end of file
+35 −0
Original line number Diff line number Diff line
package at.bitfire.davdroid

import java.util.*

object ConcurrentUtils {

    private val running = Collections.synchronizedSet(HashSet<Any>())


    /**
     * Guards a code block by a key – the block will only run when there is currently no
     * other running code block with the same key (compared by [Object.equals]).
     *
     * @param key       guarding key to determine whether the code block will be run
     * @param block     this code block will be run, but not more than one time at once per key
     *
     * @return  *true* if the code block was executed (i.e. there was no running code block with this key);
     *          *false* if there was already another running block with that key, so that the code block wasn't executed
     */
    fun runSingle(key: Any, block: () -> Unit): Boolean {
        if (!running.add(key))      // already running?
            return false            // this key is already in use, refuse execution
        // key is now in running

        try {
            block()
            return true

        } finally {
            running.remove(key)
            // key is now not in running anymore; further calls will succeed
        }
    }

}
 No newline at end of file
+32 −35
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ import android.net.NetworkCapabilities
import android.net.wifi.WifiManager
import android.os.Bundle
import androidx.core.content.getSystemService
import at.bitfire.davdroid.ConcurrentUtils
import at.bitfire.davdroid.InvalidAccountException
import at.bitfire.davdroid.PermissionUtils
import at.bitfire.davdroid.log.Logger
@@ -23,12 +24,9 @@ import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.ui.account.WifiPermissionsActivity
import kotlinx.coroutines.asCoroutineDispatcher
import java.util.*
import java.util.concurrent.ConcurrentHashMap
import java.util.concurrent.LinkedBlockingQueue
import java.util.concurrent.ThreadPoolExecutor
import java.util.concurrent.TimeUnit
import java.util.concurrent.locks.Lock
import java.util.concurrent.locks.ReentrantLock
import java.util.logging.Level

abstract class SyncAdapterService: Service() {
@@ -66,10 +64,6 @@ abstract class SyncAdapterService: Service() {
         */
        const val SYNC_EXTRAS_FULL_RESYNC = "full_resync"

        /** Keep a list of running syncs to block multiple calls at the same time, which
         * should not occur but sometimes do occur. */
        private val runningSyncs = ConcurrentHashMap<Pair<String, Account>, Lock>()

        /**
         * We use our own dispatcher to
         *
@@ -89,6 +83,14 @@ abstract class SyncAdapterService: Service() {
    override fun onBind(intent: Intent?) = syncAdapter().syncAdapterBinder!!


    /**
     * Base class for our sync adapters. Guarantees that
     *
     * 1. not more than one sync adapter per account and authority is running at a time,
     * 2. `Thread.currentThread().contextClassLoader` is set to the current context's class loader.
     *
     * Also provides some useful methods that can be used by derived sync adapters.
     */
    abstract class SyncAdapter(
        context: Context
    ): AbstractThreadedSyncAdapter(
@@ -98,6 +100,7 @@ abstract class SyncAdapterService: Service() {
    ) {

        companion object {

            fun priorityCollections(extras: Bundle): Set<Long> {
                val ids = mutableSetOf<Long>()
                extras.getString(SYNC_EXTRAS_PRIORITY_COLLECTIONS)?.let { rawIds ->
@@ -110,6 +113,7 @@ abstract class SyncAdapterService: Service() {
                }
                return ids
            }

        }

        abstract fun sync(account: Account, extras: Bundle, authority: String, provider: ContentProviderClient, syncResult: SyncResult)
@@ -117,13 +121,9 @@ abstract class SyncAdapterService: Service() {
        override fun onPerformSync(account: Account, extras: Bundle, authority: String, provider: ContentProviderClient, syncResult: SyncResult) {
            Logger.log.log(Level.INFO, "$authority sync of $account has been initiated", extras.keySet().joinToString(", "))

            /*
            Prevent multiple syncs of the same authority and account to run simultaneously.
             */
            val currentSync = Pair(authority, account)
            val currentSyncLock = runningSyncs.getOrPut(currentSync) { ReentrantLock() }
            if (currentSyncLock.tryLock())
                try {
            // prevent multiple syncs of the same authority and account to run simultaneously
            val currentSyncKey = Pair(authority, account)
            if (ConcurrentUtils.runSingle(currentSyncKey) {
                // required for ServiceLoader -> ical4j -> ical4android
                Thread.currentThread().contextClassLoader = context.classLoader

@@ -131,19 +131,16 @@ abstract class SyncAdapterService: Service() {
                    if (/* always true in open-source edition */ true)
                        sync(account, extras, authority, provider, syncResult)
                } catch (e: InvalidAccountException) {
                        Logger.log.log(Level.WARNING, "Account was removed during synchronization", e)
                    }

                    Logger.log.log(Level.INFO, "Sync for $currentSync finished", syncResult)
                } finally {
                    currentSyncLock.unlock()
                    // from now on, further threads can re-use the existing lock

                    runningSyncs -= currentSync
                    // from now on, further threads will create a new lock for the authority/account pair
                }
                    Logger.log.log(
                        Level.WARNING,
                        "Account was removed during synchronization",
                        e
                    )
                }
            })
                Logger.log.log(Level.INFO, "Sync for $currentSyncKey finished", syncResult)
            else
                Logger.log.warning("There's already another $authority sync running for $account, aborting")
                Logger.log.warning("There's already another running sync for $currentSyncKey, aborting")
        }

        override fun onSecurityException(account: Account, extras: Bundle, authority: String, syncResult: SyncResult) {
+57 −0
Original line number Diff line number Diff line
package at.bitfire.davdroid

import org.junit.Assert.assertEquals
import org.junit.Test
import java.util.concurrent.atomic.AtomicInteger

class ConcurrentUtilsTest {

    @Test
    fun testRunSingle_DifferentKeys_Sequentially() {
        var nrCalled = AtomicInteger()
        for (i in 0 until 10)
            ConcurrentUtils.runSingle(i) { nrCalled.incrementAndGet() }
        assertEquals(10, nrCalled.get())
    }

    @Test
    fun testRunSingle_DifferentKeys_Parallel() {
        var nrCalled = AtomicInteger()
        val threads = mutableListOf<Thread>()
        for (i in 0 until 10)
            threads += Thread() {
                ConcurrentUtils.runSingle(i) {
                    nrCalled.incrementAndGet()
                    Thread.sleep(100)
                }
            }.apply { start() }
        threads.forEach { it.join() }
        assertEquals(10, nrCalled.get())
    }

    @Test
    fun testRunSingle_SameKey_Sequentially() {
        val key = "a"
        var nrCalled = AtomicInteger()
        for (i in 0 until 10)
            ConcurrentUtils.runSingle(key) { nrCalled.incrementAndGet() }
        assertEquals(10, nrCalled.get())
    }

    @Test
    fun testRunSingle_SameKey_Parallel() {
        val key = "a"
        val nrCalled = AtomicInteger()
        val threads = mutableListOf<Thread>()
        for (i in 0 until 10)
            threads += Thread() {
                ConcurrentUtils.runSingle(key) {
                    nrCalled.incrementAndGet()
                    Thread.sleep(100)
                }
            }.apply { start() }
        threads.forEach { it.join() }
        assertEquals(1, nrCalled.get())
    }

}
 No newline at end of file