Skip to content

Conversation

@ArnyminerZ
Copy link
Member

@ArnyminerZ ArnyminerZ commented Feb 25, 2025

Purpose

Upgrade the UnifiedPush connector version to the latest one to support VAPID and message encryption.

Also includes the Google FCM embedded distributor so that without any additional distributor, DAVx5 works with FCM out of the box.

Checklist

  • The PR has a proper title, description and label.
  • I have self-reviewed the PR.
  • I have added documentation to complex functions and functions that can be used by other modules.
  • I have added reasonable tests or consciously decided to not add tests.

@ArnyminerZ ArnyminerZ added enhancement New feature or request push related to WebDAV-Push labels Feb 25, 2025
@ArnyminerZ ArnyminerZ self-assigned this Feb 25, 2025
@ArnyminerZ ArnyminerZ linked an issue Feb 25, 2025 that may be closed by this pull request
1 task
@p1gp1g
Copy link

p1gp1g commented Mar 31, 2025

It breaks the build because of duplicated libraries, so I've had to exclude it from UnifiedPush. I guess that when tink-crypto/tink-java-apps#5 is merged they will release a new version.

Even when using the upstream version you'd have the same issue. It means you have another dependency using tink (or another dependency). You'll probably want to see what other dependency depends on it to check it uses an updated version of tink. You may want to explicitly depend on tink to control the version.

By the way, the upstream tink:webpush-app uses an outdated tink, see: https://codeberg.org/UnifiedPush/android-connector/issues/5#issue-1037639

@rfc2822 rfc2822 self-assigned this Apr 13, 2025
@rfc2822 rfc2822 force-pushed the 1245-push-support-unifiedpush-connector-3x branch from ece5a48 to d93ed99 Compare April 18, 2025 08:13
@rfc2822 rfc2822 changed the title Support UnifiedPush Connector 3.x [Push] Support UnifiedPush Connector 3.x Apr 21, 2025
@rfc2822 rfc2822 changed the title [Push] Support UnifiedPush Connector 3.x [Push] Support UnifiedPush Connector 3.x, VAPID, Encryption, Google FCM Apr 21, 2025
@rfc2822 rfc2822 requested a review from Copilot April 21, 2025 06:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the UnifiedPush connector to version 3.x to support VAPID, message encryption, and embedded distributor support for Google FCM. It updates dependency versions and module imports in build files, refactors and consolidates push registration logic in several application classes and services, and adjusts database queries to include service-specific parameters.

Reviewed Changes

Copilot reviewed 15 out of 17 changed files in this pull request and generated no comments.

Show a summary per file
File Description
gradle/libs.versions.toml Upgraded UnifiedPush dependency versions and added the unifiedpush-fcm dependency.
AppSettingsModel.kt Refactored push registration handling by renaming variables and delegating registration to a new manager.
AccountSettingsMigration18.kt Wrapped database operations in runBlocking to ensure synchronous migration steps.
RefreshCollectionsWorker.kt Integrated push registration updates post collection refresh.
PreferenceRepository.kt Removed UnifiedPush endpoint management functions.
DavServiceRepository.kt Added a new suspend function to fetch all services.
DavCollectionRepository.kt Updated push-related queries to require an explicit serviceId parameter.
UnifiedPushService.kt Updated push service implementation with new API signatures and delegated endpoint processing.
PushRegistrationWorker.kt Simplified the worker by delegating push logic to PushRegistrationManager.
PushRegistrationManager.kt New file that consolidates all push registration, subscription, and periodic worker management logic.
ServiceDao.kt & CollectionDao.kt Updated queries to support retrieval of push-specific data using serviceId.
Android tests & TestModules.kt Adjusted tests for the revised push logic and removed obsolete registration worker modules.
Files not reviewed (2)
  • app/build.gradle.kts: Language not supported
  • app/src/main/AndroidManifest.xml: Language not supported
Comments suppressed due to low confidence (3)

app/src/main/kotlin/at/bitfire/davdroid/push/UnifiedPushService.kt:59

  • Using runBlocking in the onNewEndpoint callback can block the processing thread if processSubscription takes longer than expected. Consider refactoring this to use a non-blocking coroutine approach to improve responsiveness.
runBlocking { pushRegistrationManager.processSubscription(serviceId, endpoint) }

app/src/main/kotlin/at/bitfire/davdroid/ui/AppSettingsModel.kt:136

  • Changing loadPushDistributors from a suspend function to a regular function may introduce blocking behavior if asynchronous operations are expected. Ensure this change is intentional and that all operations within are non-blocking.
private fun loadPushDistributors() {

app/src/main/kotlin/at/bitfire/davdroid/db/CollectionDao.kt:38

  • Ensure that the new query for getFirstVapidKey is compatible with the current database schema and that appropriate migration scripts are provided, if necessary.
@Query("SELECT pushVapidKey FROM collection WHERE serviceId=:serviceId AND pushVapidKey IS NOT NULL LIMIT 1")

@rfc2822 rfc2822 force-pushed the 1245-push-support-unifiedpush-connector-3x branch from 1082b94 to 57c64bf Compare April 22, 2025 15:53
@rfc2822 rfc2822 force-pushed the 1245-push-support-unifiedpush-connector-3x branch from ffff4e5 to 5b3df43 Compare April 23, 2025 14:20
@rfc2822 rfc2822 mentioned this pull request Apr 23, 2025
4 tasks
@rfc2822 rfc2822 force-pushed the 1245-push-support-unifiedpush-connector-3x branch from 93023bf to 6ebe97c Compare April 24, 2025 10:56
@rfc2822 rfc2822 requested a review from Copilot April 24, 2025 11:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades the UnifiedPush connector to version 3.x by adding support for VAPID, message encryption, and embedding the Google FCM distributor for out‐of‐the‐box DAVx⁵ FCM support. Key changes include:

  • Removal of legacy unified push endpoint functions and related code in PreferenceRepository.
  • Updating repository interfaces (DavServiceRepository, DavCollectionRepository, ServiceDao, CollectionDao) to use suspend functions for asynchronous operations.
  • Introducing new push-related components (UnifiedPushService, PushRegistrationManager, PushRegistrationWorker, PushMessageHandler) and corresponding test updates.

Reviewed Changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
PreferenceRepository.kt Removed legacy unified push endpoint variable and functions.
DavServiceRepository.kt Added suspend functions for asynchronous service retrieval.
DavCollectionRepository.kt Converted several functions into suspend functions with added serviceId parameters.
AccountRepository.kt Adjusted import order to reintroduce InvalidAccountException correctly.
UnifiedPushService.kt New push service implementation using runBlocking to invoke push registration manager calls.
PushRegistrationWorker.kt Refactored to use PushRegistrationManager for periodic update logic.
PushRegistrationManager.kt New manager handling registration updates, subscriptions, and unsubscriptions with mutex synchronization.
PushMessageHandler.kt New handler for processing incoming decrypted push messages and triggering syncs.
HttpClient.kt Added asynchronous builder function to support suspend-friendly account retrieval.
ServiceDao.kt & CollectionDao.kt Updated DAO interfaces with suspend query methods and additional parameters.
Test Files Modified tests to use coroutine test scopes and remove obsolete push receiver/parser files.
Files not reviewed (2)
  • app/build.gradle.kts: Language not supported
  • app/src/main/AndroidManifest.xml: Language not supported

@rfc2822 rfc2822 marked this pull request as ready for review April 24, 2025 11:05
@rfc2822 rfc2822 merged commit f62509e into main-ose Apr 24, 2025
12 checks passed
@rfc2822 rfc2822 deleted the 1245-push-support-unifiedpush-connector-3x branch April 24, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request push related to WebDAV-Push

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Push] Support UnifiedPush Connector 3.x

3 participants