Skip to content

Commit 0a39f27

Browse files
tobiasKaminskytobiasKaminsky
authored
Merge pull request #2049 from nextcloud/backport/2044/stable-2.24.0
[stable-2.24.0] Improve large file download
2 parents 0502945 + 31861a4 commit 0a39f27

3 files changed

Lines changed: 226 additions & 103 deletions

File tree

‎library/src/androidTest/java/com/owncloud/android/lib/resources/files/DownloadFileRemoteOperationIT.kt‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,15 @@
88
*/
99
package com.owncloud.android.lib.resources.files
1010

11+
import com.nextcloud.common.NextcloudClient
1112
import com.owncloud.android.AbstractIT
13+
import okhttp3.Interceptor
14+
import okhttp3.Response
15+
import okhttp3.ResponseBody
16+
import okio.Buffer
17+
import okio.BufferedSource
18+
import okio.ForwardingSource
19+
import okio.buffer
1220
import org.junit.Assert.assertEquals
1321
import org.junit.Assert.assertFalse
1422
import org.junit.Assert.assertSame
@@ -167,4 +175,70 @@ class DownloadFileRemoteOperationIT : AbstractIT() {
167175
assertTrue("Downloaded file should exist at expected path", expectedFile.exists())
168176
assertTrue("Downloaded file should not be empty", expectedFile.length() >= 0)
169177
}
178+
179+
@Test
180+
fun downloadLargeFileSucceedsWithNoCallTimeout() {
181+
val filePath = createFile("large_no_call_timeout", 1000)
182+
val remotePath = "/large_no_call_timeout.txt"
183+
assertTrue(
184+
UploadFileRemoteOperation(filePath, remotePath, "text/plain", RANDOM_MTIME)
185+
.execute(client)
186+
.isSuccess
187+
)
188+
189+
val slowOkHttpClient =
190+
nextcloudClient.client
191+
.newBuilder()
192+
.addInterceptor(ChunkDelayInterceptor(delayMs = 100))
193+
.build()
194+
val slowNextcloudClient =
195+
NextcloudClient(
196+
url,
197+
nextcloudClient.getUserIdPlain(),
198+
nextcloudClient.credentials,
199+
slowOkHttpClient,
200+
nextcloudClient.context
201+
)
202+
203+
assertTrue(
204+
DownloadFileRemoteOperation(remotePath, cacheDir)
205+
.execute(slowNextcloudClient)
206+
.isSuccess
207+
)
208+
209+
assertEquals(File(filePath).length(), File(cacheDir + remotePath).length())
210+
}
211+
212+
/**
213+
* Used for create delay for test
214+
*/
215+
private class ChunkDelayInterceptor(
216+
private val delayMs: Long
217+
) : Interceptor {
218+
override fun intercept(chain: Interceptor.Chain): Response {
219+
val response = chain.proceed(chain.request())
220+
val body = response.body
221+
val slowSource =
222+
object : ForwardingSource(body.source()) {
223+
override fun read(
224+
sink: Buffer,
225+
byteCount: Long
226+
): Long {
227+
Thread.sleep(delayMs)
228+
return super.read(sink, byteCount)
229+
}
230+
}
231+
val slowBody =
232+
object : ResponseBody() {
233+
private val bufferedSource: BufferedSource = slowSource.buffer()
234+
235+
override fun contentType() = body.contentType()
236+
237+
override fun contentLength() = body.contentLength()
238+
239+
override fun source() = bufferedSource
240+
}
241+
return response.newBuilder().body(slowBody).build()
242+
}
243+
}
170244
}

‎library/src/main/java/com/nextcloud/common/NextcloudClient.kt‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
/*
22
* Nextcloud Android Library
33
*
4-
* SPDX-FileCopyrightText: 2019-2024 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-FileCopyrightText: 2019-2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-FileCopyrightText: 2026 Alper Ozturk <alper.ozturk@nextcloud.com>
56
* SPDX-FileCopyrightText: 2019-2024 Tobias Kaminsky
67
* SPDX-FileCopyrightText: 2023 Elv1zz <elv1zz.git@gmail.com>
78
* SPDX-FileCopyrightText: 2022 Álvaro Brey <alvaro@alvarobrey.com>
@@ -207,6 +208,18 @@ class NextcloudClient private constructor(
207208
}
208209
}
209210

211+
fun withSessionTimeOut(sessionTimeOut: SessionTimeOut): NextcloudClient {
212+
val newClient =
213+
client
214+
.newBuilder()
215+
.readTimeout(sessionTimeOut.readTimeOut.toLong(), TimeUnit.MILLISECONDS)
216+
.connectTimeout(sessionTimeOut.connectionTimeOut.toLong(), TimeUnit.MILLISECONDS)
217+
// needed to prevent cancellation, seems like default value not applied
218+
.callTimeout(0, TimeUnit.MILLISECONDS)
219+
.build()
220+
return NextcloudClient(delegate, credentials, newClient, context)
221+
}
222+
210223
fun getUserIdEncoded(): String = delegate.userIdEncoded!!
211224

212225
fun getUserIdPlain(): String = delegate.userId!!

‎library/src/main/java/com/owncloud/android/lib/resources/files/DownloadFileRemoteOperation.kt‎

Lines changed: 138 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ package com.owncloud.android.lib.resources.files
99
import android.os.Build
1010
import androidx.annotation.RequiresApi
1111
import com.nextcloud.common.NextcloudClient
12+
import com.nextcloud.common.SessionTimeOut
13+
import com.nextcloud.common.defaultSessionTimeOut
1214
import com.nextcloud.operations.GetMethod
1315
import com.owncloud.android.lib.common.network.OnDatatransferProgressListener
1416
import com.owncloud.android.lib.common.network.WebdavUtils
@@ -27,128 +29,162 @@ import java.util.concurrent.atomic.AtomicBoolean
2729

2830
@Suppress("NestedBlockDepth", "TooGenericExceptionCaught", "ThrowsCount")
2931
@RequiresApi(Build.VERSION_CODES.O)
30-
class DownloadFileRemoteOperation(
31-
private val remotePath: String,
32-
private val temporalFolderPath: String?
33-
) : RemoteOperation<Any>() {
34-
private val dataTransferListeners = ConcurrentHashMap.newKeySet<OnDatatransferProgressListener>()
35-
private val cancellationRequested = AtomicBoolean(false)
36-
var modificationTimestamp: Long = 0
37-
private set
38-
var etag: String = ""
39-
private set
40-
41-
@Suppress("DEPRECATION")
42-
override fun run(client: NextcloudClient): RemoteOperationResult<Any> {
43-
val targetPath = Paths.get(tmpPath)
44-
return try {
45-
val parent = targetPath.parent ?: throw IOException("No parent directory for: $targetPath")
46-
Files.createDirectories(parent)
47-
val getMethod = GetMethod(client.getFilesDavUri(remotePath), false)
48-
val status = downloadFile(getMethod, client, targetPath)
49-
RemoteOperationResult<Any>(isSuccess(status), getMethod).also {
50-
Log_OC.i(TAG, "Download of $remotePath to $targetPath: ${it.logMessage}")
51-
}
52-
} catch (e: Exception) {
53-
RemoteOperationResult<Any>(e).also {
54-
Log_OC.e(TAG, "Download of $remotePath to $targetPath: ${it.logMessage}", e)
32+
class DownloadFileRemoteOperation
33+
@JvmOverloads
34+
constructor(
35+
private val remotePath: String,
36+
private val temporalFolderPath: String?,
37+
private val fileSizeInBytes: Long? = null
38+
) : RemoteOperation<Any>() {
39+
private val dataTransferListeners = ConcurrentHashMap.newKeySet<OnDatatransferProgressListener>()
40+
private val cancellationRequested = AtomicBoolean(false)
41+
var modificationTimestamp: Long = 0
42+
private set
43+
var etag: String = ""
44+
private set
45+
46+
@Suppress("DEPRECATION")
47+
override fun run(client: NextcloudClient): RemoteOperationResult<Any> {
48+
val targetPath = Paths.get(tmpPath)
49+
return try {
50+
val parent = targetPath.parent ?: throw IOException("No parent directory for: $targetPath")
51+
Files.createDirectories(parent)
52+
val getMethod = GetMethod(client.getFilesDavUri(remotePath), false)
53+
val status = downloadFile(getMethod, client, targetPath)
54+
RemoteOperationResult<Any>(isSuccess(status), getMethod).also {
55+
Log_OC.i(TAG, "Download of $remotePath to $targetPath: ${it.logMessage}")
56+
}
57+
} catch (e: Exception) {
58+
RemoteOperationResult<Any>(e).also {
59+
Log_OC.e(TAG, "Download of $remotePath to $targetPath: ${it.logMessage}", e)
60+
}
5561
}
5662
}
57-
}
5863

59-
// region private methods
60-
@Throws(IOException::class, OperationCancelledException::class, CreateLocalFileException::class)
61-
private fun downloadFile(
62-
getMethod: GetMethod,
63-
client: NextcloudClient,
64-
targetPath: Path
65-
): Int {
66-
val status = client.execute(getMethod)
67-
if (!isSuccess(status)) {
68-
getMethod.releaseConnection()
64+
// region private methods
65+
@Throws(IOException::class, OperationCancelledException::class, CreateLocalFileException::class)
66+
private fun downloadFile(
67+
getMethod: GetMethod,
68+
client: NextcloudClient,
69+
targetPath: Path
70+
): Int {
71+
val sessionTimeOut = calculateSessionTimeOut(fileSizeInBytes)
72+
val downloadClient = client.withSessionTimeOut(sessionTimeOut)
73+
val status = downloadClient.execute(getMethod)
74+
if (!isSuccess(status)) {
75+
getMethod.releaseConnection()
76+
return status
77+
}
78+
79+
try {
80+
writeResponseToFile(getMethod, targetPath)
81+
readMetadata(getMethod)
82+
} finally {
83+
getMethod.releaseConnection()
84+
}
85+
6986
return status
7087
}
7188

72-
try {
73-
writeResponseToFile(getMethod, targetPath)
74-
readMetadata(getMethod)
75-
} finally {
76-
getMethod.releaseConnection()
89+
@Suppress("ReturnCount")
90+
private fun calculateSessionTimeOut(fileSizeInBytes: Long?): SessionTimeOut {
91+
fileSizeInBytes ?: return defaultSessionTimeOut
92+
if (fileSizeInBytes <= 0) return defaultSessionTimeOut
93+
val readTimeOut =
94+
(READ_TIMEOUT_PER_GB * fileSizeInBytes / BYTES_PER_GB_LONG)
95+
.coerceIn(READ_TIMEOUT_MIN, READ_TIMEOUT_MAX)
96+
.toInt()
97+
return SessionTimeOut(readTimeOut = readTimeOut, connectionTimeOut = CONNECTION_TIMEOUT)
7798
}
7899

79-
return status
80-
}
81-
82-
private fun writeResponseToFile(
83-
getMethod: GetMethod,
84-
targetPath: Path
85-
) {
86-
val responseStream =
87-
getMethod.getResponseBodyAsStream()
88-
?: throw IOException("Empty response body for $remotePath")
89-
90-
val outputStream =
91-
try {
92-
Files.newOutputStream(targetPath)
93-
} catch (ex: IOException) {
94-
Log_OC.e(TAG, "Error creating file $targetPath", ex)
95-
throw CreateLocalFileException(targetPath.toString(), ex)
96-
} catch (ex: SecurityException) {
97-
Log_OC.e(TAG, "Error creating file $targetPath", ex)
98-
throw CreateLocalFileException(targetPath.toString(), ex)
99-
}
100+
private fun writeResponseToFile(
101+
getMethod: GetMethod,
102+
targetPath: Path
103+
) {
104+
val responseStream =
105+
getMethod.getResponseBodyAsStream()
106+
?: throw IOException("Empty response body for $remotePath")
107+
108+
val outputStream =
109+
try {
110+
Files.newOutputStream(targetPath)
111+
} catch (ex: IOException) {
112+
Log_OC.e(TAG, "Error creating file $targetPath", ex)
113+
throw CreateLocalFileException(targetPath.toString(), ex)
114+
} catch (ex: SecurityException) {
115+
Log_OC.e(TAG, "Error creating file $targetPath", ex)
116+
throw CreateLocalFileException(targetPath.toString(), ex)
117+
}
100118

101-
BufferedInputStream(responseStream).use { bis ->
102-
outputStream.use { fos ->
103-
val buffer = ByteArray(BUFFER_SIZE)
104-
var bytesRead: Int
105-
while (bis.read(buffer).also { bytesRead = it } != -1) {
106-
if (cancellationRequested.get()) throw OperationCancelledException()
107-
fos.write(buffer, 0, bytesRead)
119+
val totalToTransfer = fileSizeInBytes ?: 0L
120+
var totalBytesRead = 0L
121+
122+
BufferedInputStream(responseStream).use { bis ->
123+
outputStream.use { fos ->
124+
val buffer = ByteArray(BUFFER_SIZE)
125+
var bytesRead: Int
126+
while (bis.read(buffer).also { bytesRead = it } != -1) {
127+
if (cancellationRequested.get()) throw OperationCancelledException()
128+
fos.write(buffer, 0, bytesRead)
129+
totalBytesRead += bytesRead
130+
dataTransferListeners.forEach { listener ->
131+
listener.onTransferProgress(
132+
bytesRead.toLong(),
133+
totalBytesRead,
134+
totalToTransfer,
135+
targetPath.toString()
136+
)
137+
}
138+
}
108139
}
109140
}
110141
}
111-
}
112142

113-
private fun readMetadata(getMethod: GetMethod) {
114-
val modificationTime =
115-
getMethod.getResponseHeader("Last-Modified")
116-
?: getMethod.getResponseHeader("last-modified")
143+
private fun readMetadata(getMethod: GetMethod) {
144+
val modificationTime =
145+
getMethod.getResponseHeader("Last-Modified")
146+
?: getMethod.getResponseHeader("last-modified")
117147

118-
if (modificationTime != null) {
119-
modificationTimestamp = WebdavUtils.parseResponseDate(modificationTime)?.time ?: 0
120-
} else {
121-
Log_OC.e(TAG, "Could not read modification time from response downloading $remotePath")
122-
}
148+
if (modificationTime != null) {
149+
modificationTimestamp = WebdavUtils.parseResponseDate(modificationTime)?.time ?: 0
150+
} else {
151+
Log_OC.e(TAG, "Could not read modification time from response downloading $remotePath")
152+
}
123153

124-
etag = WebdavUtils.getEtagFromResponse(getMethod)
125-
if (etag.isEmpty()) {
126-
Log_OC.e(TAG, "Could not read eTag from response downloading $remotePath")
154+
etag = WebdavUtils.getEtagFromResponse(getMethod)
155+
if (etag.isEmpty()) {
156+
Log_OC.e(TAG, "Could not read eTag from response downloading $remotePath")
157+
}
127158
}
128-
}
129159

130-
private fun isSuccess(status: Int) = (status == HttpStatus.SC_OK)
160+
private fun isSuccess(status: Int) = (status == HttpStatus.SC_OK)
131161

132-
private val tmpPath: String
133-
get() = temporalFolderPath + remotePath
134-
// endregion
162+
private val tmpPath: String
163+
get() = temporalFolderPath + remotePath
164+
// endregion
135165

136-
// region public methods
137-
fun addProgressListener(listener: OnDatatransferProgressListener) {
138-
dataTransferListeners.add(listener)
139-
}
166+
// region public methods
167+
fun addProgressListener(listener: OnDatatransferProgressListener) {
168+
dataTransferListeners.add(listener)
169+
}
140170

141-
fun removeProgressListener(listener: OnDatatransferProgressListener) {
142-
dataTransferListeners.remove(listener)
143-
}
171+
fun removeProgressListener(listener: OnDatatransferProgressListener) {
172+
dataTransferListeners.remove(listener)
173+
}
144174

145-
fun cancel() {
146-
cancellationRequested.set(true)
147-
}
148-
// endregion
175+
fun cancel() {
176+
cancellationRequested.set(true)
177+
}
178+
// endregion
179+
180+
companion object {
181+
private val TAG = DownloadFileRemoteOperation::class.java.simpleName
182+
private const val BUFFER_SIZE = 4096
149183

150-
companion object {
151-
private val TAG = DownloadFileRemoteOperation::class.java.simpleName
152-
private const val BUFFER_SIZE = 4096
184+
private const val BYTES_PER_GB_LONG = 1_000_000_000L
185+
private const val READ_TIMEOUT_MIN = 60 * 1000L // 1 min
186+
private const val READ_TIMEOUT_PER_GB = 3 * 60 * 1000L // 3 min per GB
187+
private const val READ_TIMEOUT_MAX = 30 * 60 * 1000L // 30 min cap
188+
private const val CONNECTION_TIMEOUT = 15 * 1000 // 15 s
189+
}
153190
}
154-
}

0 commit comments

Comments
 (0)