Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃摬 Unifiedpush #2213

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

馃摬 Unifiedpush #2213

wants to merge 4 commits into from

Conversation

koo5
Copy link

@koo5 koo5 commented Jul 13, 2022

This PR adds a new build variant: "unifiedpush".

problems:

  • does not work with UP-NextPush yet... (Up-Example does not work with it either)
  • currently does not work when there are multiple distributors available - we think this is because UnifiedPush.registerAppWithDialog expects to be called from an activity..
  • another design issue is that unifiedpush.ChatAndCallMessagingService is largely a copy of firebase.ChatAndCallMessagingService - not sure where to create a common ancestor.
  • "ClosedInterface" was a strange name to begin with...

feedback, ideas and corrections appreciated!

koo5 added 4 commits July 13, 2022 05:04
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
Signed-off-by: Jindrich Kolman <kolman.jindrich@gmail.com>
@nextcloud-android-bot
Copy link
Collaborator

Lint

TypemasterPR
Warnings116117
Errors11

SpotBugs (new)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 15
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 61
Total 154

SpotBugs (master)

Warning Type Number
Bad practice Warnings 8
Correctness Warnings 35
Experimental Warnings 2
Internationalization Warnings 9
Malicious code vulnerability Warnings 15
Performance Warnings 22
Security Warnings 2
Dodgy code Warnings 61
Total 154

Lint increased!

@nickvergessen nickvergessen added this to the 15.0.0 milestone Aug 9, 2022
@nickvergessen nickvergessen changed the title WIP: Unifiedpush 馃敂馃摠 Unifiedpush Aug 9, 2022
@nickvergessen nickvergessen changed the title 馃敂馃摠 Unifiedpush 馃摬 Unifiedpush Aug 9, 2022
@AlvaroBrey
Copy link
Member

AlvaroBrey commented Aug 9, 2022

Hey @koo5, thank you very much for the time and effort gone into this. We would love to have this implemented in Talk, as it can help our users become more independent from Google infrastructure.

Can you write some instructions on how to test this (server + android setup)?

@AlvaroBrey
Copy link
Member

  • another design issue is that unifiedpush.ChatAndCallMessagingService is largely a copy of firebase.ChatAndCallMessagingService - not sure where to create a common ancestor.

You can create an ancestor class in the main sourceset; it doesn't do any harm as it will only be registered in the specific flavor's manifests.

@ASerbinski
Copy link
Contributor

Unified push is interesting, but I think that this is the wrong approach. There are more than one option for not-google push message delivery, another that is probably more helpful is https://gitlab.com/Nextcloud-Push which operates server side as a plugin to nextcloud itself, meaning that you install the proxy into your nextcloud instance, and then Nextcloud can push the messages directly to the client itself.

Another option would be to work somehow with Nextcloud Files HPB.

So the problem then becomes that this approach would lead to the need to publish multiple separate variants of Nextcloud Talk, whereas in my view, a more ideal solution would be to be able to support all of the different push message delivery mechanisms within the SAME variant.

Obviously a google variant has to be kept separately from an "everything else" variant in order to include on F-Droid, but in my opinion, it would be more proper for all of the additional delivery mechanisms to be available on BOTH variants.

The user should be asked which push delivery mechanism to use during initial setup.

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Aug 9, 2022

Sorry for potentially widening the field of option, but linking to the discussion of the Android files app regarding supporting an alternative push solution, see also: nextcloud/android#5510 as well as nextcloud/android#8684

@mahibi mahibi removed this from the 15.0.0 milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants