nextcloud / nextcloud/notifications

Verify token caching

Open
#2,625 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug technical debt
Dominant language
JavaScript
Stars
153
Forks
67
Avg merge
15h 3m
Merged PRs (30d)
96

Description

There's chance that caching is being handled in an inconsistent way in the Push class:

  • neither of the deletePushToken*() methods directly update the cache; both rely on the caller to handle that
  • not all instances where deletePushToken*() methods are called update the cache
  • the PushController class has some overlapping functions with Push that do direct DB updates/inserts. That further complicates things.
  • the PushController class isn't even aware of the cache used by the Push class

Impact:

  • based on the expiration time used for the cache in Push::validateToken() at a minimum it's possible for there to be a discrepancy for re-registered devices of ~10 minutes (I think)
  • duplicate code in Push and PushController classes increases code maintenance burden as well as inconsistencies/bugs

Disclaimer: It's possible the cache matter is prevented in some other way; I don't have time to get too deep into this today, but wanted to note it since it seems to be the case.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the token-deletion and validation paths in the Push and PushController classes, including every caller of deletePushToken*(). Compare their cache and database behavior before deciding on a single ownership model. Done means re-registration and deletion keep cache and database state consistent across all callers, with coverage for the affected paths.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
backend, database
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.