Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Push subscription verification is not always executed on first install #7236

Closed
jonalmeida opened this issue Jun 4, 2020 · 0 comments · Fixed by #7239
Closed

Push subscription verification is not always executed on first install #7236

jonalmeida opened this issue Jun 4, 2020 · 0 comments · Fixed by #7239
Assignees
Labels
🐞 bug Something isn't working E2 Estimation points: 2 <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab

Comments

@jonalmeida
Copy link
Contributor

jonalmeida commented Jun 4, 2020

To avoid hitting our push servers too often, we check our last successful verification first and verify again after the PERIODIC_INTERVAL_MILLISECONDS interval.

A problem arises when we first access the prefLastVerified: if the value does not exist, we use System.currentTimeMillis() which should be fine in most cases, except on it's first usage:

private fun shouldVerifyNow(): Boolean {
    return (System.currentTimeMillis() - prefLastVerified) >= PERIODIC_INTERVAL_MILLISECONDS
}

Here, we compare the current time with the pref value (which doesn't exist), so we query the current time again. The difference of that is therefore a negative value which is always fail the verify check on that first attempt.

┆Issue is synchronized with this Jira Task

@jonalmeida jonalmeida added 🐞 bug Something isn't working <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab E2 Estimation points: 2 labels Jun 4, 2020
@jonalmeida jonalmeida self-assigned this Jun 4, 2020
jonalmeida added a commit to jonalmeida/android-components that referenced this issue Jun 4, 2020
…ture

When we first access the `lastVerified` pref we do not have an value
stored so we get the value from the current time.

The first usage is where we compare the delta of the current time with
the `lastVerified` so this lead us to have a negative value which fails.
jonalmeida added a commit to jonalmeida/android-components that referenced this issue Jun 4, 2020
…ture

When we first access the `lastVerified` pref we do not have an value
stored so we get the value from the current time.

The first usage is where we compare the delta of the current time with
the `lastVerified` so this lead us to have a negative value which fails.
bors bot pushed a commit that referenced this issue Jun 8, 2020
7239: Close #7236: Fix last verification check in AutoPushFeature r=Amejia481 a=jonalmeida

When we first access the `lastVerified` pref we do not have an value
stored so we get the value from the current time.

The first usage is where we compare the delta of the current time with
the `lastVerified` so this lead us to have a negative value which fails.




Co-authored-by: Jonathan Almeida <[email protected]>
@bors bors bot closed this as completed in de6f222 Jun 8, 2020
@jonalmeida jonalmeida added this to the 45.0.0 milestone Jun 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Something isn't working E2 Estimation points: 2 <push> Components: concept-push, lib-push-adm, lib-push-firebase, feature-push, feature-sendtab
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant