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

Unverified Commit 288583bf authored by Sunik Kupfer's avatar Sunik Kupfer Committed by GitHub
Browse files

Extract `discoverHomesets()` to `ServiceRefresher` (#1604)



* Extract discoverHomesets to ServiceRefresher

Signed-off-by: default avatarSunik Kupfer <kupfer@bitfire.at>

* Move hiltRule to top; Add space

Signed-off-by: default avatarSunik Kupfer <kupfer@bitfire.at>

* Log every request with method and path

Signed-off-by: default avatarSunik Kupfer <kupfer@bitfire.at>

---------

Signed-off-by: default avatarSunik Kupfer <kupfer@bitfire.at>
parent 98c0b0c3
Loading
Loading
Loading
Loading
+0 −27
Original line number Diff line number Diff line
@@ -92,33 +92,6 @@ class CollectionListRefresherTest {
    }


    @Test
    fun testDiscoverHomesets() {
        val baseUrl = mockServer.url(PATH_CARDDAV + SUBPATH_PRINCIPAL)

        // Query home sets
        refresherFactory.create(service, client.okHttpClient).discoverHomesets(baseUrl)

        // Check home set has been saved correctly to database
        val savedHomesets = db.homeSetDao().getByService(service.id)
        assertEquals(2, savedHomesets.size)

        // Home set from current-user-principal
        val personalHomeset = savedHomesets[1]
        assertEquals(mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL/"), personalHomeset.url)
        assertEquals(service.id, personalHomeset.serviceId)
        // personal should be true for homesets detected at first query of current-user-principal (Even if they occur in a group principal as well!!!)
        assertEquals(true, personalHomeset.personal)

        // Home set found in a group principal
        val groupHomeset = savedHomesets[0]
        assertEquals(mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET_NON_PERSONAL/"), groupHomeset.url)
        assertEquals(service.id, groupHomeset.serviceId)
        // personal should be false for homesets not detected at the first query of current-user-principal (IE. in groups)
        assertEquals(false, groupHomeset.personal)
    }


    // refreshHomesetsAndTheirCollections

    @Test
+163 −0
Original line number Diff line number Diff line
/*
 * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
 */

package at.bitfire.davdroid.servicedetection

import android.security.NetworkSecurityPolicy
import at.bitfire.davdroid.db.AppDatabase
import at.bitfire.davdroid.db.Service
import at.bitfire.davdroid.network.HttpClient
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import okhttp3.mockwebserver.Dispatcher
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.MockWebServer
import okhttp3.mockwebserver.RecordedRequest
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assume
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.util.logging.Logger
import javax.inject.Inject

@HiltAndroidTest
class ServiceRefresherTest {

    @get:Rule
    val hiltRule = HiltAndroidRule(this)

    @Inject
    lateinit var db: AppDatabase

    @Inject
    lateinit var httpClientBuilder: HttpClient.Builder

    @Inject
    lateinit var logger: Logger

    @Inject
    lateinit var serviceRefresherFactory: ServiceRefresher.Factory

    private lateinit var client: HttpClient
    private lateinit var mockServer: MockWebServer
    private lateinit var service: Service

    @Before
    fun setUp() {
        hiltRule.inject()

        // Start mock web server
        mockServer = MockWebServer().apply {
            dispatcher = TestDispatcher(logger)
            start()
        }

        // build HTTP client
        client = httpClientBuilder.build()
        Assume.assumeTrue(NetworkSecurityPolicy.getInstance().isCleartextTrafficPermitted)

        // insert test service
        val serviceId = db.serviceDao().insertOrReplace(
            Service(id = 0, accountName = "test", type = Service.TYPE_CARDDAV, principal = null)
        )
        service = db.serviceDao().get(serviceId)!!
    }

    @After
    fun tearDown() {
        client.close()
        mockServer.shutdown()
    }


    @Test
    fun testDiscoverHomesets() {
        val baseUrl = mockServer.url(PATH_CARDDAV + SUBPATH_PRINCIPAL)

        // Query home sets
        serviceRefresherFactory.create(service, client.okHttpClient)
            .discoverHomesets(baseUrl)

        // Check home set has been saved correctly to database
        val savedHomesets = db.homeSetDao().getByService(service.id)
        assertEquals(2, savedHomesets.size)

        // Home set from current-user-principal
        val personalHomeset = savedHomesets[1]
        assertEquals(mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL/"), personalHomeset.url)
        assertEquals(service.id, personalHomeset.serviceId)
        // personal should be true for homesets detected at first query of current-user-principal (Even if they occur in a group principal as well!!!)
        assertEquals(true, personalHomeset.personal)

        // Home set found in a group principal
        val groupHomeset = savedHomesets[0]
        assertEquals(mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET_NON_PERSONAL/"), groupHomeset.url)
        assertEquals(service.id, groupHomeset.serviceId)
        // personal should be false for homesets not detected at the first query of current-user-principal (IE. in groups)
        assertEquals(false, groupHomeset.personal)
    }


    companion object {
        private const val PATH_CARDDAV = "/carddav"

        private const val SUBPATH_PRINCIPAL = "/principal"
        private const val SUBPATH_GROUPPRINCIPAL_0 = "/groups/0"
        private const val SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL = "/addressbooks-homeset"
        private const val SUBPATH_ADDRESSBOOK_HOMESET_NON_PERSONAL = "/addressbooks-homeset-non-personal"

    }

    class TestDispatcher(
        private val logger: Logger
    ) : Dispatcher() {

        override fun dispatch(request: RecordedRequest): MockResponse {
            val path = request.path!!.trimEnd('/')
            logger.info("Query: ${request.method} on $path ")

            if (request.method.equals("PROPFIND", true)) {
                val properties = when (path) {
                    PATH_CARDDAV + SUBPATH_PRINCIPAL ->
                        "<resourcetype><principal/></resourcetype>" +
                                "<displayname>Mr. Wobbles</displayname>" +
                                "<CARD:addressbook-home-set>" +
                                "   <href>${PATH_CARDDAV}${SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL}</href>" +
                                "</CARD:addressbook-home-set>" +
                                "<group-membership>" +
                                "   <href>${PATH_CARDDAV}${SUBPATH_GROUPPRINCIPAL_0}</href>" +
                                "</group-membership>"

                    PATH_CARDDAV + SUBPATH_GROUPPRINCIPAL_0 ->
                        "<resourcetype><principal/></resourcetype>" +
                                "<displayname>All address books</displayname>" +
                                "<CARD:addressbook-home-set>" +
                                "   <href>${PATH_CARDDAV}${SUBPATH_ADDRESSBOOK_HOMESET_PERSONAL}</href>" +
                                "   <href>${PATH_CARDDAV}${SUBPATH_ADDRESSBOOK_HOMESET_NON_PERSONAL}</href>" +
                                "</CARD:addressbook-home-set>"

                    else -> ""
                }
                return MockResponse()
                    .setResponseCode(207)
                    .setBody(
                        "<multistatus xmlns='DAV:' xmlns:CARD='urn:ietf:params:xml:ns:carddav' xmlns:CAL='urn:ietf:params:xml:ns:caldav'>" +
                                "<response>" +
                                "   <href>$path</href>" +
                                "   <propstat><prop>" +
                                properties +
                                "   </prop></propstat>" +
                                "</response>" +
                                "</multistatus>"
                    )
            }

            return MockResponse().setResponseCode(404)
        }

    }

}
 No newline at end of file
+0 −141
Original line number Diff line number Diff line
@@ -7,25 +7,18 @@ package at.bitfire.davdroid.servicedetection
import at.bitfire.dav4jvm.DavResource
import at.bitfire.dav4jvm.Property
import at.bitfire.dav4jvm.Response
import at.bitfire.dav4jvm.UrlUtils
import at.bitfire.dav4jvm.exception.HttpException
import at.bitfire.dav4jvm.property.caldav.CalendarColor
import at.bitfire.dav4jvm.property.caldav.CalendarDescription
import at.bitfire.dav4jvm.property.caldav.CalendarHomeSet
import at.bitfire.dav4jvm.property.caldav.CalendarProxyReadFor
import at.bitfire.dav4jvm.property.caldav.CalendarProxyWriteFor
import at.bitfire.dav4jvm.property.caldav.CalendarTimezone
import at.bitfire.dav4jvm.property.caldav.CalendarTimezoneId
import at.bitfire.dav4jvm.property.caldav.Source
import at.bitfire.dav4jvm.property.caldav.SupportedCalendarComponentSet
import at.bitfire.dav4jvm.property.carddav.AddressbookDescription
import at.bitfire.dav4jvm.property.carddav.AddressbookHomeSet
import at.bitfire.dav4jvm.property.common.HrefListProperty
import at.bitfire.dav4jvm.property.push.PushTransports
import at.bitfire.dav4jvm.property.push.Topic
import at.bitfire.dav4jvm.property.webdav.CurrentUserPrivilegeSet
import at.bitfire.dav4jvm.property.webdav.DisplayName
import at.bitfire.dav4jvm.property.webdav.GroupMembership
import at.bitfire.dav4jvm.property.webdav.Owner
import at.bitfire.dav4jvm.property.webdav.ResourceType
import at.bitfire.davdroid.db.AppDatabase
@@ -37,11 +30,9 @@ import at.bitfire.davdroid.repository.DavCollectionRepository
import at.bitfire.davdroid.repository.DavHomeSetRepository
import at.bitfire.davdroid.settings.Settings
import at.bitfire.davdroid.settings.SettingsManager
import at.bitfire.davdroid.util.DavUtils.parent
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import okhttp3.HttpUrl
import okhttp3.OkHttpClient
import java.util.logging.Level
import java.util.logging.Logger
@@ -72,37 +63,6 @@ class CollectionListRefresher @AssistedInject constructor(
        ResourceType.NAME
    )

    /**
     * Home-set class to use depending on the given service type.
     */
    private val homeSetClass: Class<out HrefListProperty> =
        when (service.type) {
            Service.TYPE_CARDDAV -> AddressbookHomeSet::class.java
            Service.TYPE_CALDAV -> CalendarHomeSet::class.java
            else -> throw IllegalArgumentException()
        }

    /**
     * Home-set properties to ask for in a PROPFIND request to the principal URL,
     * depending on the given service type.
     */
    private val homeSetProperties: Array<Property.Name> =
        arrayOf(                        // generic WebDAV properties
            DisplayName.NAME,
            GroupMembership.NAME,
            ResourceType.NAME
        ) + when (service.type) {       // service-specific CalDAV/CardDAV properties
                Service.TYPE_CARDDAV -> arrayOf(
                    AddressbookHomeSet.NAME,
                )
                Service.TYPE_CALDAV -> arrayOf(
                    CalendarHomeSet.NAME,
                    CalendarProxyReadFor.NAME,
                    CalendarProxyWriteFor.NAME
                )
                else -> throw IllegalArgumentException()
            }

    /**
     * Collection properties to ask for in a PROPFIND request on a collection.
     */
@@ -129,107 +89,6 @@ class CollectionListRefresher @AssistedInject constructor(
            else -> throw IllegalArgumentException()
        }



    /**
     * Starting at given principal URL, tries to recursively find and save all user relevant home sets.
     *
     * @param principalUrl              URL of principal to query (user-provided principal or current-user-principal)
     * @param level                     Current recursion level (limited to 0, 1 or 2):
     *   - 0: We assume found home sets belong to the current-user-principal
     *   - 1 or 2: We assume found home sets don't directly belong to the current-user-principal
     * @param alreadyQueriedPrincipals  The HttpUrls of principals which have been queried already, to avoid querying principals more than once.
     * @param alreadySavedHomeSets      The HttpUrls of home sets which have been saved to database already, to avoid saving home sets
     * more than once, which could overwrite the already set "personal" flag with `false`.
     *
     * @throws java.io.IOException                          on I/O errors
     * @throws HttpException                                on HTTP errors
     * @throws at.bitfire.dav4jvm.exception.DavException    on application-level or logical errors
     */
    internal fun discoverHomesets(
        principalUrl: HttpUrl,
        level: Int = 0,
        alreadyQueriedPrincipals: MutableSet<HttpUrl> = mutableSetOf(),
        alreadySavedHomeSets: MutableSet<HttpUrl> = mutableSetOf()
    ) {
        logger.fine("Discovering homesets of $principalUrl")
        val relatedResources = mutableSetOf<HttpUrl>()

        // Query the URL
        val principal = DavResource(httpClient, principalUrl)
        val personal = level == 0
        try {
            principal.propfind(0, *homeSetProperties) { davResponse, _ ->
                alreadyQueriedPrincipals += davResponse.href

                // If response holds home sets, save them
                davResponse[homeSetClass]?.let { homeSets ->
                    for (homeSetHref in homeSets.hrefs)
                        principal.location.resolve(homeSetHref)?.let { homesetUrl ->
                            val resolvedHomeSetUrl = UrlUtils.withTrailingSlash(homesetUrl)
                            if (!alreadySavedHomeSets.contains(resolvedHomeSetUrl)) {
                                homeSetRepository.insertOrUpdateByUrlBlocking(
                                    // HomeSet is considered personal if this is the outer recursion call,
                                    // This is because we assume the first call to query the current-user-principal
                                    // Note: This is not be be confused with the DAV:owner attribute. Home sets can be owned by
                                    // other principals while still being considered "personal" (belonging to the current-user-principal)
                                    // and an owned home set need not always be personal either.
                                    HomeSet(0, service.id, personal, resolvedHomeSetUrl)
                                )
                                alreadySavedHomeSets += resolvedHomeSetUrl
                            }
                        }
                }

                // Add related principals to be queried afterwards
                if (personal) {
                    val relatedResourcesTypes = listOf(
                        // current resource is a read/write-proxy for other principals
                        CalendarProxyReadFor::class.java,
                        CalendarProxyWriteFor::class.java,
                        // current resource is a member of a group (principal that can also have proxies)
                        GroupMembership::class.java
                    )
                    for (type in relatedResourcesTypes)
                        davResponse[type]?.let {
                            for (href in it.hrefs)
                                principal.location.resolve(href)?.let { url ->
                                    relatedResources += url
                                }
                        }
                }

                // If current resource is a calendar-proxy-read/write, it's likely that its parent is a principal, too.
                davResponse[ResourceType::class.java]?.let { resourceType ->
                    val proxyProperties = arrayOf(
                        ResourceType.CALENDAR_PROXY_READ,
                        ResourceType.CALENDAR_PROXY_WRITE,
                    )
                    if (proxyProperties.any { resourceType.types.contains(it) })
                        relatedResources += davResponse.href.parent()
                }
            }
        } catch (e: HttpException) {
            if (e.code/100 == 4)
                logger.log(Level.INFO, "Ignoring Client Error 4xx while looking for ${service.type} home sets", e)
            else
                throw e
        }

        // query related resources
        if (level <= 1)
            for (resource in relatedResources)
                if (alreadyQueriedPrincipals.contains(resource))
                    logger.warning("$resource already queried, skipping")
                else
                    discoverHomesets(
                        principalUrl = resource,
                        level = level + 1,
                        alreadyQueriedPrincipals = alreadyQueriedPrincipals,
                        alreadySavedHomeSets = alreadySavedHomeSets
                    )
    }

    /**
     * Refreshes home-sets and their collections.
     *
+3 −1
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ class RefreshCollectionsWorker @AssistedInject constructor(
    private val logger: Logger,
    private val notificationRegistry: NotificationRegistry,
    private val pushRegistrationManager: PushRegistrationManager,
    private val serviceRefresherFactory: ServiceRefresher.Factory,
    serviceRepository: DavServiceRepository
): CoroutineWorker(appContext, workerParams) {

@@ -161,7 +162,8 @@ class RefreshCollectionsWorker @AssistedInject constructor(
                        // refresh home set list (from principal url)
                        service.principal?.let { principalUrl ->
                            logger.fine("Querying principal $principalUrl for home sets")
                            refresher.discoverHomesets(principalUrl)
                            val serviceRefresher = serviceRefresherFactory.create(service, httpClient)
                            serviceRefresher.discoverHomesets(principalUrl)
                        }

                        // refresh home sets and their member collections
+178 −0

File added.

Preview size limit exceeded, changes collapsed.