Skip to content

[PB-6281]: add encrypted file upload flow with multipart support#464

Merged
terrerox merged 3 commits into
release-1.9.1from
feature/PB-6281-upload-file
Jun 10, 2026
Merged

[PB-6281]: add encrypted file upload flow with multipart support#464
terrerox merged 3 commits into
release-1.9.1from
feature/PB-6281-upload-file

Conversation

@terrerox

@terrerox terrerox commented May 27, 2026

Copy link
Copy Markdown
  • add bridge API methods/models for upload start, multipart finish, and file entry creation
  • introduce encrypted uploader with SHA-256 part hashing and RIPEMD160 shard hash
  • add upload token helpers in DocumentId and JSON parsing utilities
  • add upload HTTP client config and Bouncy Castle RIPEMD160 dependency
  • add crypto callback await helper and pending upload model

@terrerox terrerox requested a review from TamaraFinogina as a code owner May 27, 2026 05:05
@terrerox terrerox marked this pull request as draft May 27, 2026 05:05
@terrerox terrerox self-assigned this May 27, 2026
@terrerox terrerox force-pushed the feature/PB-6281-upload-file branch from 3a60331 to 0506824 Compare May 28, 2026 04:31
@terrerox terrerox changed the title feat: Implement file upload functionality with encryption and progres… [PB-6281]: add encrypted file upload flow with multipart support May 28, 2026
@terrerox terrerox force-pushed the feature/PB-6281-upload-file branch from 0506824 to 5b81068 Compare May 28, 2026 04:39
@terrerox terrerox marked this pull request as ready for review May 28, 2026 14:27
@terrerox terrerox requested a review from CandelR May 28, 2026 14:37
@@ -0,0 +1,15 @@
package com.internxt.cloud.documents.api.model

const val ENCRYPT_VERSION_AES03 = "Aes03"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, i missed that. Thanks!

@terrerox terrerox force-pushed the feature/PB-5928-download-decryption branch from 03be86e to e3fbcb8 Compare June 1, 2026 20:51
@terrerox terrerox force-pushed the feature/PB-6281-upload-file branch 2 times, most recently from 19346e5 to 5b81068 Compare June 1, 2026 22:25
- add bridge API methods/models for upload start, multipart finish, and file entry creation
- introduce encrypted uploader with SHA-256 part hashing and RIPEMD160 shard hash
- add upload token helpers in DocumentId and JSON parsing utilities
- add upload HTTP client config and Bouncy Castle RIPEMD160 dependency
- add crypto callback await helper and pending upload model
@terrerox terrerox force-pushed the feature/PB-6281-upload-file branch from 5b81068 to abd0357 Compare June 1, 2026 22:37
@terrerox terrerox requested a review from TamaraFinogina June 1, 2026 23:11
Base automatically changed from feature/PB-5928-download-decryption to release-1.9.1 June 4, 2026 20:29

@CandelR CandelR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@terrerox After reviewing the upload implementation and taking another look at the download implementation, I have noticed that the entire module (EncryptedFileDownloader,EncryptedFileUploader, InternxtDocumentsProvider) relies on blocking threads:

  • call.execute() blocks a thread for the duration of the transfer
  • CompletableFuture.get() blocks another thread during encryption
  • openExecutor exists solely to offload work from the binder thread, so it seems that waster a third thread. Opening a file ties up 3 threads, 2 of which do nothing but wait.

Migrating to coroutines solves this at the root:

  • CryptoService is pure native Java, so it can be wrapped directly with suspendCancellableCoroutine
  • call.execute() becomes call.await() via enqueue + suspendCancellableCoroutine
  • All the AtomicReference + setOnCancelListener + OperationCanceledException wiring disappears, coroutine cancellation handles it automatically
  • openExecutor can be removed entirely, runBlocking at the boundary of openDocument is the appropriate bridge

If you’d like to give this a go, it might be worth creating a ticket to address this issue before continue with iOS tasks, as coroutines are the standard concurrency model in modern Kotlin for Android and would significantly simplify the cancellation logic

failureMessage: String,
invoke: (OnlyErrorCallback) -> Unit,
) {
val done = CompletableFuture<Unit>()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to use coroutines, Kotlin and the entire Android ecosystem have moved to coroutines as the standard async model.CompletableFuture.get() blocks the thread while waiting, coroutines suspend instead, freeing the thread for other work, and cancellation is automatic when the coroutine scope is cancelled . Replace with suspendCancellableCoroutine

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will create another task to address this issue

etag = response.header("ETag") ?: response.header("Etag")
?: throw IOException("Part ${index + 1} missing ETag in response")
}
parts.add(UploadedPart(partNumber = index + 1, etag = etag!!))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not force the no nullable values (!!), this could lead in unexpected errors. Return the resutl to etag directly

@sonarqubecloud

sonarqubecloud Bot commented Jun 5, 2026

Copy link
Copy Markdown

@terrerox terrerox requested a review from CandelR June 5, 2026 14:51
@terrerox

terrerox commented Jun 7, 2026

Copy link
Copy Markdown
Author

@terrerox After reviewing the upload implementation and taking another look at the download implementation, I have noticed that the entire module (EncryptedFileDownloader,EncryptedFileUploader, InternxtDocumentsProvider) relies on blocking threads:

  • call.execute() blocks a thread for the duration of the transfer
  • CompletableFuture.get() blocks another thread during encryption
  • openExecutor exists solely to offload work from the binder thread, so it seems that waster a third thread. Opening a file ties up 3 threads, 2 of which do nothing but wait.

Migrating to coroutines solves this at the root:

  • CryptoService is pure native Java, so it can be wrapped directly with suspendCancellableCoroutine
  • call.execute() becomes call.await() via enqueue + suspendCancellableCoroutine
  • All the AtomicReference + setOnCancelListener + OperationCanceledException wiring disappears, coroutine cancellation handles it automatically
  • openExecutor can be removed entirely, runBlocking at the boundary of openDocument is the appropriate bridge

If you’d like to give this a go, it might be worth creating a ticket to address this issue before continue with iOS tasks, as coroutines are the standard concurrency model in modern Kotlin for Android and would significantly simplify the cancellation logic

Implemented coroutines here

@terrerox terrerox merged commit 8f61a1c into release-1.9.1 Jun 10, 2026
2 of 3 checks passed
@terrerox terrerox deleted the feature/PB-6281-upload-file branch June 10, 2026 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants