Fix related google calendars not being found (bitfireAT/davx5#409)

* Minor changes
- update kdoc
- rename method and variables

* Add proxy parents to related resource detection

* Rename argument, query ResourceType

* Remove unnecessary utility method

* Change parentOf to extension function; Always return URL with trailing slash

* Use calendar-proxy-read/write ResourceType from new dav4jvm

* Use max. two levels of recursion to detect shared Google calendars

* Revise test and adapt method

* Simplify HttpUrl.parent()

---------

Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Sunik Kupfer 2023-10-26 11:33:53 +02:00 committed by Ricki Hirner
parent 0215e98326
commit 1e6a457a0d
No known key found for this signature in database
GPG key ID: 79A019FCAAEDD3AA
5 changed files with 106 additions and 48 deletions

View file

@ -143,13 +143,13 @@ class RefreshCollectionsWorkerTest {
}
@Test
fun testQueryHomesets() {
fun testDiscoverHomesets() {
val service = createTestService(Service.TYPE_CARDDAV)!!
val baseUrl = mockServer.url(PATH_CARDDAV + SUBPATH_PRINCIPAL)
// Query home sets
RefreshCollectionsWorker.Refresher(db, service, settings, client.okHttpClient)
.queryHomeSets(baseUrl)
.discoverHomesets(baseUrl)
// Check home sets have been saved to database
assertEquals(mockServer.url("$PATH_CARDDAV$SUBPATH_ADDRESSBOOK_HOMESET/"), db.homeSetDao().getByService(service.id).first().url)

View file

@ -63,6 +63,7 @@ import at.bitfire.davdroid.ui.DebugInfoActivity
import at.bitfire.davdroid.ui.NotificationUtils
import at.bitfire.davdroid.ui.NotificationUtils.notifyIfPossible
import at.bitfire.davdroid.ui.account.SettingsActivity
import at.bitfire.davdroid.util.DavUtils.parent
import com.google.common.util.concurrent.ListenableFuture
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
@ -192,7 +193,7 @@ class RefreshCollectionsWorker @AssistedInject constructor(
// refresh home set list (from principal url)
service.principal?.let { principalUrl ->
Logger.log.fine("Querying principal $principalUrl for home sets")
refresher.queryHomeSets(principalUrl)
refresher.discoverHomesets(principalUrl)
}
// refresh home sets and their member collections
@ -280,28 +281,25 @@ class RefreshCollectionsWorker @AssistedInject constructor(
val httpClient: OkHttpClient
) {
val alreadyQueried = mutableSetOf<HttpUrl>()
/**
* Checks if the given URL defines home sets and adds them to given home set list.
* Starting at current-user-principal URL, tries to recursively find and save all user relevant home sets.
*
* @param principalUrl Principal URL to query
* @param forPersonalHomeset Whether this is the first call of this recursive method.
* Indicates that these found home sets are considered "personal", as they belong to the
* current-user-principal.
*
* Note: This is not be be confused with the DAV:owner attribute. Home sets can be owned by
* other principals and still be considered "personal" (belonging to the current-user-principal).
* @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):
*
* *true* = found home sets belong to the current-user-principal; recurse if
* calendar proxies or group memberships are found
*
* *false* = found home sets don't directly belong to the current-user-principal; don't recurse
* - 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
*
* @throws java.io.IOException
* @throws HttpException
* @throws at.bitfire.dav4jvm.exception.DavException
*/
internal fun queryHomeSets(principalUrl: HttpUrl, forPersonalHomeset: Boolean = true) {
val related = mutableSetOf<HttpUrl>()
internal fun discoverHomesets(principalUrl: HttpUrl, level: Int = 0) {
Logger.log.fine("Discovering homesets of $principalUrl")
val relatedResources = mutableSetOf<HttpUrl>()
// Define homeset class and properties to look for
val homeSetClass: Class<out HrefListProperty>
@ -309,48 +307,62 @@ class RefreshCollectionsWorker @AssistedInject constructor(
when (service.type) {
Service.TYPE_CARDDAV -> {
homeSetClass = AddressbookHomeSet::class.java
properties = arrayOf(DisplayName.NAME, AddressbookHomeSet.NAME, GroupMembership.NAME)
properties = arrayOf(DisplayName.NAME, AddressbookHomeSet.NAME, GroupMembership.NAME, ResourceType.NAME)
}
Service.TYPE_CALDAV -> {
homeSetClass = CalendarHomeSet::class.java
properties = arrayOf(DisplayName.NAME, CalendarHomeSet.NAME, CalendarProxyReadFor.NAME, CalendarProxyWriteFor.NAME, GroupMembership.NAME)
properties = arrayOf(DisplayName.NAME, CalendarHomeSet.NAME, CalendarProxyReadFor.NAME, CalendarProxyWriteFor.NAME, GroupMembership.NAME, ResourceType.NAME)
}
else -> throw IllegalArgumentException()
}
val dav = DavResource(httpClient, principalUrl)
// Query the URL
val principal = DavResource(httpClient, principalUrl)
val personal = level == 0
try {
// Query for the given service with properties
dav.propfind(0, *properties) { davResponse, _ ->
principal.propfind(0, *properties) { davResponse, _ ->
alreadyQueried += davResponse.href
// Check we got back the right service and save it
davResponse[homeSetClass]?.let { homeSet ->
for (href in homeSet.hrefs)
dav.location.resolve(href)?.let {
val foundUrl = UrlUtils.withTrailingSlash(it)
// 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)
// 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 and still be considered "personal" (belonging to the current-user-principal).
db.homeSetDao().insertOrUpdateByUrl(
HomeSet(0, service.id, forPersonalHomeset, foundUrl)
HomeSet(0, service.id, personal, resolvedHomeSetUrl)
)
}
}
// If personal (outer call of recursion), find/refresh related resources
if (forPersonalHomeset) {
val relatedResourcesTypes = mapOf(
CalendarProxyReadFor::class.java to "read-only proxy for", // calendar-proxy-read-for
CalendarProxyWriteFor::class.java to "read/write proxy for ", // calendar-proxy-read/write-for
GroupMembership::class.java to "member of group") // direct group memberships
for ((type, logString) in relatedResourcesTypes) {
// 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) {
Logger.log.fine("Principal is a $logString for $href, checking for home sets")
dav.location.resolve(href)?.let { url ->
related += url
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) {
@ -360,9 +372,13 @@ class RefreshCollectionsWorker @AssistedInject constructor(
throw e
}
// query related homesets (those that do not belong to the current-user-principal)
for (resource in related)
queryHomeSets(resource, false)
// query related resources
if (level <= 1)
for (resource in relatedResources)
if (alreadyQueried.contains(resource))
Logger.log.warning("$resource already queried, skipping")
else
discoverHomesets(resource, level + 1)
}
/**

View file

@ -8,8 +8,8 @@ import android.content.Context
import android.net.ConnectivityManager
import android.os.Build
import androidx.core.content.getSystemService
import at.bitfire.davdroid.network.Android10Resolver
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.network.Android10Resolver
import okhttp3.HttpUrl
import okhttp3.MediaType
import okhttp3.MediaType.Companion.toMediaType
@ -151,6 +151,30 @@ object DavUtils {
// extension methods
/**
* Returns parent URL (parent folder). Always with trailing slash
*/
fun HttpUrl.parent(): HttpUrl {
if (pathSegments.size == 1 && pathSegments[0] == "")
// already root URL
return this
val builder = newBuilder()
if (pathSegments[pathSegments.lastIndex] == "") {
// URL ends with a slash ("/some/thing/" -> ["some","thing",""]), remove two segments ("" at lastIndex and "thing" at lastIndex - 1)
builder.removePathSegment(pathSegments.lastIndex)
builder.removePathSegment(pathSegments.lastIndex - 1)
} else
// URL doesn't end with a slash ("/some/thing" -> ["some","thing"]), remove one segment ("thing" at lastIndex)
builder.removePathSegment(pathSegments.lastIndex)
// append trailing slash
builder.addPathSegment("")
return builder.build()
}
/**
* Compares MIME type and subtype of two MediaTypes. Does _not_ compare parameters
* like `charset` or `version`.

View file

@ -5,8 +5,11 @@
package at.bitfire.davdroid
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.DavUtils.parent
import okhttp3.HttpUrl.Companion.toHttpUrl
import org.junit.Assert.*
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.xbill.DNS.DClass
import org.xbill.DNS.Name
@ -31,6 +34,21 @@ class DavUtilsTest {
assertEquals("file.html", DavUtils.lastSegmentOfUrl((exampleURL + "dir/file.html").toHttpUrl()))
}
@Test
fun testParent() {
// with trailing slash
assertEquals("http://example.com/1/2/".toHttpUrl(), "http://example.com/1/2/3/".toHttpUrl().parent())
assertEquals("http://example.com/1/".toHttpUrl(), "http://example.com/1/2/".toHttpUrl().parent())
assertEquals("http://example.com/".toHttpUrl(), "http://example.com/1/".toHttpUrl().parent())
assertEquals("http://example.com/".toHttpUrl(), "http://example.com/".toHttpUrl().parent())
// without trailing slash
assertEquals("http://example.com/1/2/".toHttpUrl(), "http://example.com/1/2/3".toHttpUrl().parent())
assertEquals("http://example.com/1/".toHttpUrl(), "http://example.com/1/2".toHttpUrl().parent())
assertEquals("http://example.com/".toHttpUrl(), "http://example.com/1".toHttpUrl().parent())
assertEquals("http://example.com/".toHttpUrl(), "http://example.com".toHttpUrl().parent())
}
@Test
fun testSelectSRVRecord() {
assertNull(DavUtils.selectSRVRecord(emptyArray()))

View file

@ -10,7 +10,7 @@ buildscript {
hilt: '2.48.1',
kotlin: '1.9.10', // keep in sync with * app/build.gradle composeOptions.kotlinCompilerExtensionVersion
// * com.google.devtools.ksp at the end of this file
okhttp: '4.11.0',
okhttp: '4.12.0',
room: '2.5.2',
workManager: '2.9.0-rc01',
// Apache Commons versions
@ -19,7 +19,7 @@ buildscript {
commonsText: '1.10.0',
// own libraries
cert4android: '2bb3898',
dav4jvm: 'da94a8b',
dav4jvm: '1ed89c1',
ical4android: '916f222',
vcard4android: 'b376d2e'
]