From bd13d27e386c76db070e480a42ff784e1fc06e14 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Tue, 4 Nov 2025 16:38:22 +0100 Subject: [PATCH] HttpClient: remove unnecessary `close()` (#1792) * Remove unnecessary AutoCloseable implementations and client.close() calls - Remove AutoCloseable from NextcloudLoginFlow and DavResourceFinder - Remove client.close() calls in various classes and tests - Update HttpClient to remove close() method * Fix test * Fix annotations / KDoc --- .../at/bitfire/davdroid/db/CollectionTest.kt | 6 -- .../davdroid/network/HttpClientTest.kt | 1 - .../davdroid/network/OkhttpClientTest.kt | 19 ++--- .../CollectionsWithoutHomeSetRefresherTest.kt | 1 - .../servicedetection/DavResourceFinderTest.kt | 1 - .../servicedetection/HomeSetRefresherTest.kt | 1 - .../PrincipalsRefresherTest.kt | 1 - .../servicedetection/ServiceRefresherTest.kt | 1 - .../QueryChildDocumentsOperationTest.kt | 1 - .../at/bitfire/davdroid/network/HttpClient.kt | 7 +- .../davdroid/network/NextcloudLoginFlow.kt | 6 +- .../davdroid/push/PushRegistrationManager.kt | 44 +++++----- .../repository/DavCollectionRepository.kt | 45 +++++----- .../servicedetection/DavResourceFinder.kt | 6 +- .../RefreshCollectionsWorker.kt | 44 +++++----- .../davdroid/sync/ContactsSyncManager.kt | 28 +++---- .../kotlin/at/bitfire/davdroid/sync/Syncer.kt | 2 - .../davdroid/ui/setup/LoginScreenModel.kt | 5 +- .../davdroid/ui/setup/NextcloudLoginModel.kt | 4 - .../davdroid/webdav/RandomAccessCallback.kt | 3 +- .../webdav/StreamingFileDescriptor.kt | 5 +- .../davdroid/webdav/WebDavMountRepository.kt | 14 ++-- .../webdav/operation/CopyDocumentOperation.kt | 55 ++++++------ .../operation/CreateDocumentOperation.kt | 75 +++++++++-------- .../operation/DeleteDocumentOperation.kt | 25 +++--- .../webdav/operation/MoveDocumentOperation.kt | 27 +++--- .../OpenDocumentThumbnailOperation.kt | 27 +++--- .../operation/QueryChildDocumentsOperation.kt | 83 +++++++++---------- .../operation/RenameDocumentOperation.kt | 41 +++++---- 29 files changed, 261 insertions(+), 317 deletions(-) diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/db/CollectionTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/db/CollectionTest.kt index 56166c88c..9e1350b8c 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/db/CollectionTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/db/CollectionTest.kt @@ -14,7 +14,6 @@ import dagger.hilt.android.testing.HiltAndroidTest import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer -import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -45,11 +44,6 @@ class CollectionTest { Assume.assumeTrue(NetworkSecurityPolicy.getInstance().isCleartextTrafficPermitted) } - @After - fun teardown() { - httpClient.close() - } - @Test @SmallTest diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt index 1c1209904..4c1088e02 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/HttpClientTest.kt @@ -45,7 +45,6 @@ class HttpClientTest { @After fun tearDown() { server.shutdown() - httpClient.close() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/OkhttpClientTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/OkhttpClientTest.kt index 760332dbe..7ca00868e 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/network/OkhttpClientTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/network/OkhttpClientTest.kt @@ -31,16 +31,15 @@ class OkhttpClientTest { @Test @SdkSuppress(maxSdkVersion = 34) fun testIcloudWithSettings() { - httpClientBuilder.build().use { client -> - client.okHttpClient - .newCall( - Request.Builder() - .get() - .url("https://icloud.com") - .build() - ) - .execute() - } + val client = httpClientBuilder.build() + client.okHttpClient + .newCall( + Request.Builder() + .get() + .url("https://icloud.com") + .build() + ) + .execute() } } \ No newline at end of file diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionsWithoutHomeSetRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionsWithoutHomeSetRefresherTest.kt index aa4fcf5e4..f1422a697 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionsWithoutHomeSetRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/CollectionsWithoutHomeSetRefresherTest.kt @@ -80,7 +80,6 @@ class CollectionsWithoutHomeSetRefresherTest { @After fun tearDown() { - client.close() mockServer.shutdown() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt index a77ca849f..f915b526c 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinderTest.kt @@ -83,7 +83,6 @@ class DavResourceFinderTest { @After fun tearDown() { - client.close() server.shutdown() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/HomeSetRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/HomeSetRefresherTest.kt index 5607b756b..37a763a14 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/HomeSetRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/HomeSetRefresherTest.kt @@ -86,7 +86,6 @@ class HomeSetRefresherTest { @After fun tearDown() { - client.close() mockServer.shutdown() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/PrincipalsRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/PrincipalsRefresherTest.kt index 231f73473..d8bded34a 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/PrincipalsRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/PrincipalsRefresherTest.kt @@ -81,7 +81,6 @@ class PrincipalsRefresherTest { @After fun tearDown() { - client.close() mockServer.shutdown() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/ServiceRefresherTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/ServiceRefresherTest.kt index 24ed9126b..43cc6afa5 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/ServiceRefresherTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/servicedetection/ServiceRefresherTest.kt @@ -68,7 +68,6 @@ class ServiceRefresherTest { @After fun tearDown() { - client.close() mockServer.shutdown() } diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperationTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperationTest.kt index e3f320431..39a7a0fed 100644 --- a/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperationTest.kt +++ b/app/src/androidTest/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperationTest.kt @@ -84,7 +84,6 @@ class QueryChildDocumentsOperationTest { @After fun tearDown() { - client.close() server.shutdown() runBlocking { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt index b5728619c..ab383570d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/HttpClient.kt @@ -42,12 +42,7 @@ import javax.net.ssl.SSLContext class HttpClient( val okHttpClient: OkHttpClient -): AutoCloseable { - - override fun close() { - // nothing to do, can be removed - } - +) { // builder diff --git a/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt b/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt index 9ed2836b5..324b09f7c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/network/NextcloudLoginFlow.kt @@ -33,7 +33,7 @@ import javax.inject.Inject */ class NextcloudLoginFlow @Inject constructor( httpClientBuilder: HttpClient.Builder -): AutoCloseable { +) { companion object { const val FLOW_V1_PATH = "index.php/login/flow" @@ -46,10 +46,6 @@ class NextcloudLoginFlow @Inject constructor( val httpClient = httpClientBuilder .build() - override fun close() { - httpClient.close() - } - // Login flow state var loginUrl: HttpUrl? = null diff --git a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt index ad4e882d4..645ad4908 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/push/PushRegistrationManager.kt @@ -180,25 +180,23 @@ class PushRegistrationManager @Inject constructor( return val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() + val httpClient = httpClientBuilder.get() .fromAccountAsync(account) .build() - .use { httpClient -> - for (collection in subscribeTo) - try { - val expires = collection.pushSubscriptionExpires - // calculate next run time, but use the duplicate interval for safety (times are not exact) - val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) - if (expires != null && expires >= nextRun.epochSecond) - logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") - else { - // no existing subscription or expiring soon - logger.fine("Registering push subscription for ${collection.url}") - subscribe(httpClient, collection, endpoint) - } - } catch (e: Exception) { - logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) - } + for (collection in subscribeTo) + try { + val expires = collection.pushSubscriptionExpires + // calculate next run time, but use the duplicate interval for safety (times are not exact) + val nextRun = Instant.now() + Duration.ofDays(2 * WORKER_INTERVAL_DAYS) + if (expires != null && expires >= nextRun.epochSecond) + logger.fine("Push subscription for ${collection.url} is still valid until ${collection.pushSubscriptionExpires}") + else { + // no existing subscription or expiring soon + logger.fine("Registering push subscription for ${collection.url}") + subscribe(httpClient, collection, endpoint) + } + } catch (e: Exception) { + logger.log(Level.WARNING, "Couldn't register subscription at CalDAV/CardDAV server", e) } } @@ -294,15 +292,13 @@ class PushRegistrationManager @Inject constructor( return val account = accountRepository.get().fromName(service.accountName) - httpClientBuilder.get() + val httpClient = httpClientBuilder.get() .fromAccountAsync(account) .build() - .use { httpClient -> - for (collection in from) - collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> - logger.info("Unsubscribing Push from ${collection.url}") - unsubscribe(httpClient, collection, url) - } + for (collection in from) + collection.pushSubscription?.toHttpUrlOrNull()?.let { url -> + logger.info("Unsubscribing Push from ${collection.url}") + unsubscribe(httpClient, collection, url) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt index 2a1247d39..ef1fd9c05 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/repository/DavCollectionRepository.kt @@ -173,21 +173,20 @@ class DavCollectionRepository @Inject constructor( val service = serviceRepository.getBlocking(collection.serviceId) ?: throw IllegalArgumentException("Service not found") val account = Account(service.accountName, context.getString(R.string.account_type)) - httpClientBuilder.get().fromAccount(account).build().use { httpClient -> - runInterruptible(ioDispatcher) { - try { - DavResource(httpClient.okHttpClient, collection.url).delete { - // success, otherwise an exception would have been thrown → delete locally, too - delete(collection) - } - } catch (e: HttpException) { - if (e is NotFoundException || e is GoneException) { - // HTTP 404 Not Found or 410 Gone (collection is not there anymore) -> delete locally, too - logger.info("Collection ${collection.url} not found on server, deleting locally") - delete(collection) - } else - throw e + val httpClient = httpClientBuilder.get().fromAccount(account).build() + runInterruptible(ioDispatcher) { + try { + DavResource(httpClient.okHttpClient, collection.url).delete { + // success, otherwise an exception would have been thrown → delete locally, too + delete(collection) } + } catch (e: HttpException) { + if (e is NotFoundException || e is GoneException) { + // HTTP 404 Not Found or 410 Gone (collection is not there anymore) -> delete locally, too + logger.info("Collection ${collection.url} not found on server, deleting locally") + delete(collection) + } else + throw e } } } @@ -290,19 +289,17 @@ class DavCollectionRepository @Inject constructor( // helpers private suspend fun createOnServer(account: Account, url: HttpUrl, method: String, xmlBody: String) { - httpClientBuilder.get() + val httpClient = httpClientBuilder.get() .fromAccount(account) .build() - .use { httpClient -> - runInterruptible(ioDispatcher) { - DavResource(httpClient.okHttpClient, url).mkCol( - xmlBody = xmlBody, - method = method - ) { - // success, otherwise an exception would have been thrown - } - } + runInterruptible(ioDispatcher) { + DavResource(httpClient.okHttpClient, url).mkCol( + xmlBody = xmlBody, + method = method + ) { + // success, otherwise an exception would have been thrown } + } } private fun generateMkColXml( diff --git a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinder.kt b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinder.kt index 494d9c115..f6d098af3 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinder.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/DavResourceFinder.kt @@ -63,7 +63,7 @@ class DavResourceFinder @AssistedInject constructor( @ApplicationContext val context: Context, private val dnsRecordResolver: DnsRecordResolver, httpClientBuilder: HttpClient.Builder -): AutoCloseable { +) { @AssistedFactory interface Factory { @@ -93,10 +93,6 @@ class DavResourceFinder @AssistedInject constructor( } .build() - override fun close() { - httpClient.close() - } - private fun initLogging(): StringHandler { // don't use more than 1/4 of the available memory for a log string val activityManager = context.getSystemService()!! diff --git a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt index a2a7a65de..4dce7083b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/servicedetection/RefreshCollectionsWorker.kt @@ -153,34 +153,32 @@ class RefreshCollectionsWorker @AssistedInject constructor( .cancel(serviceId.toString(), NotificationRegistry.NOTIFY_REFRESH_COLLECTIONS) // create authenticating OkHttpClient (credentials taken from account settings) - httpClientBuilder + val httpClient = httpClientBuilder .fromAccount(account) .build() - .use { httpClient -> - runInterruptible { - val httpClient = httpClient.okHttpClient - val refresher = collectionsWithoutHomeSetRefresherFactory.create(service, httpClient) + runInterruptible { + val httpClient = httpClient.okHttpClient + val refresher = collectionsWithoutHomeSetRefresherFactory.create(service, httpClient) - // refresh home set list (from principal url) - service.principal?.let { principalUrl -> - logger.fine("Querying principal $principalUrl for home sets") - val serviceRefresher = serviceRefresherFactory.create(service, httpClient) - serviceRefresher.discoverHomesets(principalUrl) - } - - // refresh home sets and their member collections - homeSetRefresherFactory.create(service, httpClient) - .refreshHomesetsAndTheirCollections() - - // also refresh collections without a home set - refresher.refreshCollectionsWithoutHomeSet() - - // Lastly, refresh the principals (collection owners) - val principalsRefresher = principalsRefresherFactory.create(service, httpClient) - principalsRefresher.refreshPrincipals() - } + // refresh home set list (from principal url) + service.principal?.let { principalUrl -> + logger.fine("Querying principal $principalUrl for home sets") + val serviceRefresher = serviceRefresherFactory.create(service, httpClient) + serviceRefresher.discoverHomesets(principalUrl) } + // refresh home sets and their member collections + homeSetRefresherFactory.create(service, httpClient) + .refreshHomesetsAndTheirCollections() + + // also refresh collections without a home set + refresher.refreshCollectionsWithoutHomeSet() + + // Lastly, refresh the principals (collection owners) + val principalsRefresher = principalsRefresherFactory.create(service, httpClient) + principalsRefresher.refreshPrincipals() + } + } catch(e: InvalidAccountException) { logger.log(Level.SEVERE, "Invalid account", e) return Result.failure() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/ContactsSyncManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/ContactsSyncManager.kt index fe53601dc..af67d1435 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/ContactsSyncManager.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/ContactsSyncManager.kt @@ -485,25 +485,23 @@ class ContactsSyncManager @AssistedInject constructor( } // authenticate only against a certain host, and only upon request - httpClientBuilder + val hostHttpClient = httpClientBuilder .fromAccount(account, onlyHost = baseUrl.host) .followRedirects(true) // allow redirects .build() - .use { httpClient -> - try { - val response = httpClient.okHttpClient.newCall(Request.Builder() - .get() - .url(httpUrl) - .build()).execute() + try { + val response = hostHttpClient.okHttpClient.newCall(Request.Builder() + .get() + .url(httpUrl) + .build()).execute() - if (response.isSuccessful) - return response.body.bytes() - else - logger.warning("Couldn't download external resource") - } catch(e: IOException) { - logger.log(Level.SEVERE, "Couldn't download external resource", e) - } - } + if (response.isSuccessful) + return response.body.bytes() + else + logger.warning("Couldn't download external resource") + } catch(e: IOException) { + logger.log(Level.SEVERE, "Couldn't download external resource", e) + } return null } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt index 30389b24e..191ebe451 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/sync/Syncer.kt @@ -273,8 +273,6 @@ abstract class Syncer, CollectionType: syncResult.numUnclassifiedErrors++ // Hard sync error } finally { - if (httpClient.isInitialized()) - httpClient.value.close() logger.info("${dataStore.authority} sync of $account finished") } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginScreenModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginScreenModel.kt index f1b16d63c..92b37820b 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginScreenModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/LoginScreenModel.kt @@ -195,9 +195,8 @@ class LoginScreenModel @AssistedInject constructor( detectResourcesJob = viewModelScope.launch { val result = withContext(Dispatchers.IO) { runInterruptible { - resourceFinderFactory.create(loginInfo.baseUri!!, loginInfo.credentials).use { finder -> - finder.findInitialConfiguration() - } + val finder = resourceFinderFactory.create(loginInfo.baseUri!!, loginInfo.credentials) + finder.findInitialConfiguration() } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/NextcloudLoginModel.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/NextcloudLoginModel.kt index bee507252..62c940aaa 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/NextcloudLoginModel.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/NextcloudLoginModel.kt @@ -102,10 +102,6 @@ class NextcloudLoginModel @AssistedInject constructor( state[STATE_TOKEN] = value }*/ - override fun onCleared() { - loginFlow.close() - } - /** * Starts the Login Flow. diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt index a4c391b22..16273451c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/RandomAccessCallback.kt @@ -43,7 +43,7 @@ import javax.annotation.WillClose @RequiresApi(26) class RandomAccessCallback @AssistedInject constructor( - @Assisted @WillClose private val httpClient: HttpClient, + @Assisted private val httpClient: HttpClient, @Assisted private val url: HttpUrl, @Assisted private val mimeType: MediaType?, @Assisted headResponse: HeadResponse, @@ -127,7 +127,6 @@ class RandomAccessCallback @AssistedInject constructor( // free resources ioThread.quitSafely() - httpClient.close() } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt index 50cb1f4e5..3abe7a223 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/StreamingFileDescriptor.kt @@ -27,10 +27,10 @@ import java.util.logging.Logger import javax.annotation.WillClose /** - * @param client HTTP client ([StreamingFileDescriptor] is responsible to close it) + * @param client HTTP client to use */ class StreamingFileDescriptor @AssistedInject constructor( - @Assisted @WillClose private val client: HttpClient, + @Assisted private val client: HttpClient, @Assisted private val url: HttpUrl, @Assisted private val mimeType: MediaType?, @Assisted private val externalScope: CoroutineScope, @@ -75,7 +75,6 @@ class StreamingFileDescriptor @AssistedInject constructor( writeFd.close() } catch (_: IOException) {} - client.close() finishedCallback.onFinished(transferred, success) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/WebDavMountRepository.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/WebDavMountRepository.kt index def00da37..7799b9599 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/WebDavMountRepository.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/WebDavMountRepository.kt @@ -127,21 +127,19 @@ class WebDavMountRepository @Inject constructor( val validVersions = arrayOf("1", "2", "3") val builder = httpClientBuilder.get() - if (credentials != null) builder.authenticate( host = null, getCredentials = { credentials } ) + val httpClient = builder.build() var webdavUrl: HttpUrl? = null - builder.build().use { httpClient -> - val dav = DavResource(httpClient.okHttpClient, url) - runInterruptible { - dav.options(followRedirects = true) { davCapabilities, response -> - if (davCapabilities.any { it in validVersions }) - webdavUrl = dav.location - } + val dav = DavResource(httpClient.okHttpClient, url) + runInterruptible { + dav.options(followRedirects = true) { davCapabilities, response -> + if (davCapabilities.any { it in validVersions }) + webdavUrl = dav.location } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CopyDocumentOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CopyDocumentOperation.kt index a031edd43..6192c7b43 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CopyDocumentOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CopyDocumentOperation.kt @@ -40,38 +40,37 @@ class CopyDocumentOperation @Inject constructor( if (srcDoc.mountId != dstFolder.mountId) throw UnsupportedOperationException("Can't COPY between WebDAV servers") - httpClientBuilder.build(srcDoc.mountId).use { client -> - val dav = DavResource(client.okHttpClient, srcDoc.toHttpUrl(db)) - val dstUrl = dstFolder.toHttpUrl(db).newBuilder() - .addPathSegment(name) - .build() + val client = httpClientBuilder.build(srcDoc.mountId) + val dav = DavResource(client.okHttpClient, srcDoc.toHttpUrl(db)) + val dstUrl = dstFolder.toHttpUrl(db).newBuilder() + .addPathSegment(name) + .build() - try { - runInterruptible(ioDispatcher) { - dav.copy(dstUrl, false) { - // successfully copied - } + try { + runInterruptible(ioDispatcher) { + dav.copy(dstUrl, false) { + // successfully copied } - } catch (e: HttpException) { - e.throwForDocumentProvider(context) } - - val dstDocId = documentDao.insertOrReplace( - WebDavDocument( - mountId = dstFolder.mountId, - parentId = dstFolder.id, - name = name, - isDirectory = srcDoc.isDirectory, - displayName = srcDoc.displayName, - mimeType = srcDoc.mimeType, - size = srcDoc.size - ) - ).toString() - - DocumentProviderUtils.notifyFolderChanged(context, targetParentDocumentId) - - /* return */ dstDocId + } catch (e: HttpException) { + e.throwForDocumentProvider(context) } + + val dstDocId = documentDao.insertOrReplace( + WebDavDocument( + mountId = dstFolder.mountId, + parentId = dstFolder.id, + name = name, + isDirectory = srcDoc.isDirectory, + displayName = srcDoc.displayName, + mimeType = srcDoc.mimeType, + size = srcDoc.size + ) + ).toString() + + DocumentProviderUtils.notifyFolderChanged(context, targetParentDocumentId) + + /* return */ dstDocId } } \ No newline at end of file diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CreateDocumentOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CreateDocumentOperation.kt index bd5e3b33b..d76d6d5d6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CreateDocumentOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/CreateDocumentOperation.kt @@ -41,45 +41,44 @@ class CreateDocumentOperation @Inject constructor( val createDirectory = mimeType == Document.MIME_TYPE_DIR var docId: Long? - httpClientBuilder.build(parent.mountId).use { client -> - for (attempt in 0..DocumentProviderUtils.MAX_DISPLAYNAME_TO_MEMBERNAME_ATTEMPTS) { - val newName = displayNameToMemberName(displayName, attempt) - val parentUrl = parent.toHttpUrl(db) - val newLocation = parentUrl.newBuilder() - .addPathSegment(newName) - .build() - val doc = DavResource(client.okHttpClient, newLocation) - try { - runInterruptible(ioDispatcher) { - if (createDirectory) - doc.mkCol(null) { - // directory successfully created - } - else - doc.put(RequestBody.EMPTY, ifNoneMatch = true) { - // document successfully created - } - } - - docId = documentDao.insertOrReplace( - WebDavDocument( - mountId = parent.mountId, - parentId = parent.id, - name = newName, - isDirectory = createDirectory, - mimeType = mimeType.toMediaTypeOrNull(), - eTag = null, - lastModified = null, - size = if (createDirectory) null else 0 - ) - ) - - DocumentProviderUtils.notifyFolderChanged(context, parentDocumentId) - - return@runBlocking docId.toString() - } catch (e: HttpException) { - e.throwForDocumentProvider(context, ignorePreconditionFailed = true) + val client = httpClientBuilder.build(parent.mountId) + for (attempt in 0..DocumentProviderUtils.MAX_DISPLAYNAME_TO_MEMBERNAME_ATTEMPTS) { + val newName = displayNameToMemberName(displayName, attempt) + val parentUrl = parent.toHttpUrl(db) + val newLocation = parentUrl.newBuilder() + .addPathSegment(newName) + .build() + val doc = DavResource(client.okHttpClient, newLocation) + try { + runInterruptible(ioDispatcher) { + if (createDirectory) + doc.mkCol(null) { + // directory successfully created + } + else + doc.put(RequestBody.EMPTY, ifNoneMatch = true) { + // document successfully created + } } + + docId = documentDao.insertOrReplace( + WebDavDocument( + mountId = parent.mountId, + parentId = parent.id, + name = newName, + isDirectory = createDirectory, + mimeType = mimeType.toMediaTypeOrNull(), + eTag = null, + lastModified = null, + size = if (createDirectory) null else 0 + ) + ) + + DocumentProviderUtils.notifyFolderChanged(context, parentDocumentId) + + return@runBlocking docId.toString() + } catch (e: HttpException) { + e.throwForDocumentProvider(context, ignorePreconditionFailed = true) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/DeleteDocumentOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/DeleteDocumentOperation.kt index 9736e90c1..2db5cdd4c 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/DeleteDocumentOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/DeleteDocumentOperation.kt @@ -34,21 +34,20 @@ class DeleteDocumentOperation @Inject constructor( logger.fine("WebDAV removeDocument $documentId") val doc = documentDao.get(documentId.toLong()) ?: throw FileNotFoundException() - httpClientBuilder.build(doc.mountId).use { client -> - val dav = DavResource(client.okHttpClient, doc.toHttpUrl(db)) - try { - runInterruptible(ioDispatcher) { - dav.delete { - // successfully deleted - } + val client = httpClientBuilder.build(doc.mountId) + val dav = DavResource(client.okHttpClient, doc.toHttpUrl(db)) + try { + runInterruptible(ioDispatcher) { + dav.delete { + // successfully deleted } - logger.fine("Successfully removed") - documentDao.delete(doc) - - DocumentProviderUtils.notifyFolderChanged(context, doc.parentId) - } catch (e: HttpException) { - e.throwForDocumentProvider(context) } + logger.fine("Successfully removed") + documentDao.delete(doc) + + DocumentProviderUtils.notifyFolderChanged(context, doc.parentId) + } catch (e: HttpException) { + e.throwForDocumentProvider(context) } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/MoveDocumentOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/MoveDocumentOperation.kt index c9faa3fa4..d1b883453 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/MoveDocumentOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/MoveDocumentOperation.kt @@ -42,22 +42,21 @@ class MoveDocumentOperation @Inject constructor( .addPathSegment(doc.name) .build() - httpClientBuilder.build(doc.mountId).use { client -> - val dav = DavResource(client.okHttpClient, doc.toHttpUrl(db)) - try { - runInterruptible(ioDispatcher) { - dav.move(newLocation, false) { - // successfully moved - } + val client = httpClientBuilder.build(doc.mountId) + val dav = DavResource(client.okHttpClient, doc.toHttpUrl(db)) + try { + runInterruptible(ioDispatcher) { + dav.move(newLocation, false) { + // successfully moved } - - documentDao.update(doc.copy(parentId = dstParent.id)) - - DocumentProviderUtils.notifyFolderChanged(context, sourceParentDocumentId) - DocumentProviderUtils.notifyFolderChanged(context, targetParentDocumentId) - } catch (e: HttpException) { - e.throwForDocumentProvider(context) } + + documentDao.update(doc.copy(parentId = dstParent.id)) + + DocumentProviderUtils.notifyFolderChanged(context, sourceParentDocumentId) + DocumentProviderUtils.notifyFolderChanged(context, targetParentDocumentId) + } catch (e: HttpException) { + e.throwForDocumentProvider(context) } doc.id.toString() diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/OpenDocumentThumbnailOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/OpenDocumentThumbnailOperation.kt index d267ed47a..c8d2b3d20 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/OpenDocumentThumbnailOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/OpenDocumentThumbnailOperation.kt @@ -76,24 +76,23 @@ class OpenDocumentThumbnailOperation @Inject constructor( // create thumbnail val job = accessScope.async { withTimeout(THUMBNAIL_TIMEOUT_MS) { - httpClientBuilder.build(doc.mountId, logBody = false).use { client -> - val url = doc.toHttpUrl(db) - val dav = DavResource(client.okHttpClient, url) - var result: ByteArray? = null - runInterruptible(ioDispatcher) { - dav.get("image/*", null) { response -> - response.body.byteStream().use { data -> - BitmapFactory.decodeStream(data)?.let { bitmap -> - val thumb = ThumbnailUtils.extractThumbnail(bitmap, sizeHint.x, sizeHint.y) - val baos = ByteArrayOutputStream() - thumb.compress(Bitmap.CompressFormat.JPEG, 95, baos) - result = baos.toByteArray() - } + val client = httpClientBuilder.build(doc.mountId, logBody = false) + val url = doc.toHttpUrl(db) + val dav = DavResource(client.okHttpClient, url) + var result: ByteArray? = null + runInterruptible(ioDispatcher) { + dav.get("image/*", null) { response -> + response.body.byteStream().use { data -> + BitmapFactory.decodeStream(data)?.let { bitmap -> + val thumb = ThumbnailUtils.extractThumbnail(bitmap, sizeHint.x, sizeHint.y) + val baos = ByteArrayOutputStream() + thumb.compress(Bitmap.CompressFormat.JPEG, 95, baos) + result = baos.toByteArray() } } } - result } + result } } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperation.kt index 74157e648..c1dc91090 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/QueryChildDocumentsOperation.kt @@ -128,58 +128,57 @@ class QueryChildDocumentsOperation @Inject constructor( val newChildrenList = hashMapOf() val parentUrl = parent.toHttpUrl(db) - httpClientBuilder.build(parent.mountId).use { client -> - val folder = DavCollection(client.okHttpClient, parentUrl) + val client = httpClientBuilder.build(parent.mountId) + val folder = DavCollection(client.okHttpClient, parentUrl) - try { - runInterruptible(ioDispatcher) { - folder.propfind(1, *DAV_FILE_FIELDS) { response, relation -> - logger.fine("$relation $response") + try { + runInterruptible(ioDispatcher) { + folder.propfind(1, *DAV_FILE_FIELDS) { response, relation -> + logger.fine("$relation $response") - val resource: WebDavDocument = - when (relation) { - Response.HrefRelation.SELF -> // it's about the parent - parent + val resource: WebDavDocument = + when (relation) { + Response.HrefRelation.SELF -> // it's about the parent + parent - Response.HrefRelation.MEMBER -> // it's about a member - WebDavDocument(mountId = parent.mountId, parentId = parent.id, name = response.hrefName()) + Response.HrefRelation.MEMBER -> // it's about a member + WebDavDocument(mountId = parent.mountId, parentId = parent.id, name = response.hrefName()) - else -> { - // we didn't request this; log a warning and ignore it - logger.warning("Ignoring unexpected $response $relation in $parentUrl") - return@propfind - } + else -> { + // we didn't request this; log a warning and ignore it + logger.warning("Ignoring unexpected $response $relation in $parentUrl") + return@propfind } - - val updatedResource = resource.copy( - isDirectory = response[ResourceType::class.java]?.types?.contains(ResourceType.COLLECTION) - ?: resource.isDirectory, - displayName = response[DisplayName::class.java]?.displayName, - mimeType = response[GetContentType::class.java]?.type, - eTag = response[GetETag::class.java]?.takeIf { !it.weak }?.eTag, - lastModified = response[GetLastModified::class.java]?.lastModified?.toEpochMilli(), - size = response[GetContentLength::class.java]?.contentLength, - mayBind = response[CurrentUserPrivilegeSet::class.java]?.mayBind, - mayUnbind = response[CurrentUserPrivilegeSet::class.java]?.mayUnbind, - mayWriteContent = response[CurrentUserPrivilegeSet::class.java]?.mayWriteContent, - quotaAvailable = response[QuotaAvailableBytes::class.java]?.quotaAvailableBytes, - quotaUsed = response[QuotaUsedBytes::class.java]?.quotaUsedBytes, - ) - - if (resource == parent) - documentDao.update(updatedResource) - else { - documentDao.insertOrUpdate(updatedResource) - newChildrenList[resource.name] = updatedResource } - // remove resource from known child nodes, because not found on server - oldChildren.remove(resource.name) + val updatedResource = resource.copy( + isDirectory = response[ResourceType::class.java]?.types?.contains(ResourceType.COLLECTION) + ?: resource.isDirectory, + displayName = response[DisplayName::class.java]?.displayName, + mimeType = response[GetContentType::class.java]?.type, + eTag = response[GetETag::class.java]?.takeIf { !it.weak }?.eTag, + lastModified = response[GetLastModified::class.java]?.lastModified?.toEpochMilli(), + size = response[GetContentLength::class.java]?.contentLength, + mayBind = response[CurrentUserPrivilegeSet::class.java]?.mayBind, + mayUnbind = response[CurrentUserPrivilegeSet::class.java]?.mayUnbind, + mayWriteContent = response[CurrentUserPrivilegeSet::class.java]?.mayWriteContent, + quotaAvailable = response[QuotaAvailableBytes::class.java]?.quotaAvailableBytes, + quotaUsed = response[QuotaUsedBytes::class.java]?.quotaUsedBytes, + ) + + if (resource == parent) + documentDao.update(updatedResource) + else { + documentDao.insertOrUpdate(updatedResource) + newChildrenList[resource.name] = updatedResource } + + // remove resource from known child nodes, because not found on server + oldChildren.remove(resource.name) } - } catch (e: Exception) { - logger.log(Level.WARNING, "Couldn't query children", e) } + } catch (e: Exception) { + logger.log(Level.WARNING, "Couldn't query children", e) } // Delete child nodes which were not rediscovered (deleted serverside) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/RenameDocumentOperation.kt b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/RenameDocumentOperation.kt index 26dd38549..66bc99808 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/RenameDocumentOperation.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/webdav/operation/RenameDocumentOperation.kt @@ -35,29 +35,28 @@ class RenameDocumentOperation @Inject constructor( logger.fine("WebDAV renameDocument $documentId $displayName") val doc = documentDao.get(documentId.toLong()) ?: throw FileNotFoundException() - httpClientBuilder.build(doc.mountId).use { client -> - for (attempt in 0..DocumentProviderUtils.MAX_DISPLAYNAME_TO_MEMBERNAME_ATTEMPTS) { - val newName = displayNameToMemberName(displayName, attempt) - val oldUrl = doc.toHttpUrl(db) - val newLocation = oldUrl.newBuilder() - .removePathSegment(oldUrl.pathSegments.lastIndex) - .addPathSegment(newName) - .build() - try { - val dav = DavResource(client.okHttpClient, oldUrl) - runInterruptible(ioDispatcher) { - dav.move(newLocation, false) { - // successfully renamed - } + val client = httpClientBuilder.build(doc.mountId) + for (attempt in 0..DocumentProviderUtils.MAX_DISPLAYNAME_TO_MEMBERNAME_ATTEMPTS) { + val newName = displayNameToMemberName(displayName, attempt) + val oldUrl = doc.toHttpUrl(db) + val newLocation = oldUrl.newBuilder() + .removePathSegment(oldUrl.pathSegments.lastIndex) + .addPathSegment(newName) + .build() + try { + val dav = DavResource(client.okHttpClient, oldUrl) + runInterruptible(ioDispatcher) { + dav.move(newLocation, false) { + // successfully renamed } - documentDao.update(doc.copy(name = newName)) - - DocumentProviderUtils.notifyFolderChanged(context, doc.parentId) - - return@runBlocking doc.id.toString() - } catch (e: HttpException) { - e.throwForDocumentProvider(context, true) } + documentDao.update(doc.copy(name = newName)) + + DocumentProviderUtils.notifyFolderChanged(context, doc.parentId) + + return@runBlocking doc.id.toString() + } catch (e: HttpException) { + e.throwForDocumentProvider(context, true) } }