Skip to content

fix(cleanup): remove expired wopi tokens with single delete query#5688

Open
Copilot wants to merge 1 commit into
mainfrom
copilot/delete-expired-tokens
Open

fix(cleanup): remove expired wopi tokens with single delete query#5688
Copilot wants to merge 1 commit into
mainfrom
copilot/delete-expired-tokens

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

  • Resolves: #
  • Target version: main

Summary

Delete expired WOPI tokens directly with a single DELETE ... WHERE expiry < ... query in the cleanup background job, instead of fetching/deleting only one 1000-token chunk.

Also replaced the hardcoded 60-second value with a named constant to document the grace period used for cleanup.

TODO

  • Implement cleanup change for expired WOPI tokens
  • Run PHP lint checks
  • Link related issue in Resolves (if applicable)

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Documentation (manuals or wiki) has been updated or is not required

Co-authored-by: Julius Knorr <jus@bitgrid.net>
Signed-off-by: Julius Knorr <jus@bitgrid.net>
@juliusknorr juliusknorr force-pushed the copilot/delete-expired-tokens branch from 869d23f to e6801ac Compare May 21, 2026 14:37
Copy link
Copy Markdown
Contributor

@chrip chrip left a comment

Choose a reason for hiding this comment

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

Nice simplification. One concern: the old query used getExpiredTokenIds(1000) which deleted in batches of 1000. The new query deletes all expired tokens in a single statement, which could be expensive on large instances with many stale WOPI records.
Consider either:

  • Keeping the batched approach (e.g. LIMIT 1000), or
  • Adding a note in the PR description about expected scale and whether a single DELETE is safe for large deployments

Otherwise LGTM — the constant is a nice touch for readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants