Simplified code to use FCM token type as default by martin-morek · Pull Request #727 · ably/ably-pubsub-java

martin-morek · 2021-10-21T09:22:08Z

fixes #703
By default FCM token type will be used, deprecated GCM type is kept only for cases when it's loaded from storage

uxduck · 2021-10-22T09:43:01Z

This is a breaking change and when users update, they will immediately have to change their code. If we deprecate the original method first, temporarily keep the original for a few releases, and add a new method which users should use instead, users will find the version upgrade process more comfortable.

martin-morek · 2021-10-22T15:39:46Z

I believe my changes were done carefully mostly affecting codebase creating a new token witch cannot have GCM type anymore. I've tried to kept possibility to read old token type to don't cause exception but on the other hand simplify a code. @ben-xD if you see any places where it can go wrong please point to them directly so I fix them. However I can definitely add deprecated annotation at the top of old constructor to make it obvious.

uxduck

Just a couple of comments, thanks :)

…ecated whole Type enum

- Improves the implementation, moving checks up to the LocalDevice. - Adds test coverages for constant FCM values we rely upon going forwards.

QuintinWillison · 2022-01-19T12:25:21Z

Emulation failing. 🙄 ... I'm going to have a play in #738 to see if I can get it more stable, before coming back to this PR.

QuintinWillison · 2022-01-19T21:08:10Z

Emulation run failed:

io.ably.lib.test.android.AndroidPushTest > push_syncOnStartup[test(AVD) - 7.0] FAILED 
	java.lang.AssertionError: expected null, but was:<GotPushDeviceDetails>
	at org.junit.Assert.fail(Assert.java:88)

io.ably.lib.test.android.AndroidPushTest > NotActivated_on_CalledActivate_with_registrationToken[test(AVD) - 7.0] FAILED 
	java.lang.AssertionError: expected collection to have size 1, got 0: [] expected:<1> but was:<0>
	at org.junit.Assert.fail(Assert.java:88)

QuintinWillison · 2022-01-20T10:06:38Z

Emulation run failed:

io.ably.lib.test.android.AndroidPushTest > push_syncOnStartup[test(AVD) - 7.0] FAILED 
	java.lang.AssertionError: expected null, but was:<GotPushDeviceDetails>
	at org.junit.Assert.fail(Assert.java:88)

io.ably.lib.test.android.AndroidPushTest > NotActivated_on_CalledActivate_with_registrationToken[test(AVD) - 7.0] FAILED 
	java.lang.AssertionError: expected collection to have size 1, got 0: [] expected:<1> but was:<0>
	at org.junit.Assert.fail(Assert.java:88)

QuintinWillison · 2022-01-20T10:07:32Z

Given the same test failed twice in a row I think it's back to the drawing board with this PR, alas. 😢

Remove deprecated test RegistrationTokenTypeTest Refactor token type usage across LocalDevice as FCM only exist Refactor generateSecret method for better try

qsdigor · 2022-07-25T11:58:46Z

I have removed all instances and mentions of GCM as it is long past deprecated and removed in 2019.

KacperKluka

Almost there 😉

Co-authored-by: KacperKluka <62378170+KacperKluka@users.noreply.github.com>

…en received can be in any format

KacperKluka

LGTM 👍

Simplified code to use FCM token type as default

6131366

martin-morek requested review from QuintinWillison and uxduck October 21, 2021 09:22

Added deprecated annotation

03a8fe5

uxduck reviewed Nov 2, 2021

View reviewed changes

Comment thread android/src/main/java/io/ably/lib/types/RegistrationToken.java Resolved

Comment thread android/src/main/java/io/ably/lib/push/ActivationContext.java Resolved

Improved onNewRegistrationToken() handling if token is GCM type. Depr…

93a312d

…ecated whole Type enum

QuintinWillison self-assigned this Jan 19, 2022

QuintinWillison removed their request for review January 19, 2022 09:31

Fully deprecate interfaces supporting GCM.

4b84d84

- Improves the implementation, moving checks up to the LocalDevice. - Adds test coverages for constant FCM values we rely upon going forwards.

QuintinWillison marked this pull request as draft January 19, 2022 12:25

Merge branch 'tweak-CI-20220119' into 703-remove-GMC-references

1ebc55b

QuintinWillison changed the base branch from main to tweak-CI-20220119 January 19, 2022 16:26

Base automatically changed from tweak-CI-20220119 to main January 19, 2022 19:09

QuintinWillison marked this pull request as ready for review January 19, 2022 19:11

QuintinWillison requested review from KacperKluka and ikbalkaya January 19, 2022 19:11

QuintinWillison removed request for KacperKluka and ikbalkaya January 20, 2022 10:07

QuintinWillison marked this pull request as draft January 20, 2022 10:07

QuintinWillison mentioned this pull request Jul 20, 2022

Remove references to GCM and simplify code #703

Closed

Merge branch 'integration/version-2' into 703-remove-GMC-references

dec3941

qsdigor changed the base branch from main to integration/version-2 July 22, 2022 08:45

QSD_igor added 2 commits July 25, 2022 13:43

Remove Type from RegistrationToken as only FCM is supported,

98ced2f

Remove deprecated test RegistrationTokenTypeTest Refactor token type usage across LocalDevice as FCM only exist Refactor generateSecret method for better try

Fix test which checks non-existing registration token type

68fcb23

qsdigor requested review from KacperKluka, QuintinWillison and ikbalkaya July 25, 2022 11:56

qsdigor assigned qsdigor and unassigned QuintinWillison Jul 25, 2022

qsdigor added the enhancement New feature or improved functionality. label Jul 25, 2022

KacperKluka suggested changes Jul 27, 2022

View reviewed changes

Comment thread android/src/main/java/io/ably/lib/push/LocalDevice.java Resolved

Comment thread android/src/main/java/io/ably/lib/push/LocalDevice.java Resolved

qsdigor and others added 2 commits July 28, 2022 09:20

Update android/src/main/java/io/ably/lib/push/LocalDevice.java

dc44847

Co-authored-by: KacperKluka <62378170+KacperKluka@users.noreply.github.com>

Compare FCM token tag with equalsIgnoreCase() instead equals() as tok…

c377629

…en received can be in any format

qsdigor requested a review from KacperKluka July 28, 2022 07:30

KacperKluka approved these changes Jul 28, 2022

View reviewed changes

deanna-seta marked this pull request as ready for review August 8, 2022 12:56

QuintinWillison merged commit d3cd6dc into integration/version-2 Sep 14, 2022

QuintinWillison deleted the 703-remove-GMC-references branch September 14, 2022 13:57

Navigation Menu

Repository navigation

Simplified code to use FCM token type as default - #727

Simplified code to use FCM token type as default#727
QuintinWillison merged 10 commits into
integration/version-2from
703-remove-GMC-references

martin-morek commented Oct 21, 2021

Uh oh!

uxduck commented Oct 22, 2021

Uh oh!

martin-morek commented Oct 22, 2021

Uh oh!

uxduck left a comment

Uh oh!

Uh oh!

Uh oh!

QuintinWillison commented Jan 19, 2022

Uh oh!

QuintinWillison commented Jan 19, 2022

Uh oh!

QuintinWillison commented Jan 20, 2022

Uh oh!

QuintinWillison commented Jan 20, 2022

Uh oh!

qsdigor commented Jul 25, 2022

Uh oh!

KacperKluka left a comment

Uh oh!

Uh oh!

Uh oh!

KacperKluka left a comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

Conversation

martin-morek commented Oct 21, 2021

Uh oh!

uxduck commented Oct 22, 2021

Uh oh!

martin-morek commented Oct 22, 2021

Uh oh!

uxduck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

QuintinWillison commented Jan 19, 2022

Uh oh!

QuintinWillison commented Jan 19, 2022

Uh oh!

QuintinWillison commented Jan 20, 2022

Uh oh!

QuintinWillison commented Jan 20, 2022

Uh oh!

qsdigor commented Jul 25, 2022

Uh oh!

KacperKluka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KacperKluka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

5 participants