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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If a usable archive exists and the invalidation is for a specific plugin, delete the invalidation #17918

Merged

Conversation

justinvelluppillai
Copy link
Contributor

Description:

Delete plugin invalidations that are ignored because a newer archive exists, otherwise invalidations get stuck there indefinitely.

Review

…gin, delete the invalidation so it doesn't stay for ever
@justinvelluppillai justinvelluppillai added this to the 4.5.0 milestone Aug 25, 2021
@justinvelluppillai
Copy link
Contributor Author

Fixes #17752

@justinvelluppillai justinvelluppillai marked this pull request as ready for review August 25, 2021 21:21
@justinvelluppillai justinvelluppillai added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Aug 25, 2021
@diosmosis
Copy link
Member

The code looks good. A test for this could be added to QueueConsumerTest (or an existing test modified).

Most of those tests work the same way, they:

  1. setup the QueueConsumer instance
  2. insert some archive_invalidations entries
  3. then select everything through the QueueConsumer instance, collecting the invalidations in an array
  4. and check that the result of that array equals a hardcoded expected value

You could add some invalidations to an existing test (ie, add a plugin invalidation for date/period, then add a recent archive 'done' row so it thinks there's a usable archive (note Date::$now can be set to make it seem recent), and check that it is both not selected and deleted afterwards). You can also do this in a new test by copy-pasting test_invalidateConsumeOrder() and changing $invalidations and $expectedInvalidationsFound, and adding the extra archive row insertion.

@diosmosis
Copy link
Member

Sorry, github notifications said my review was requested, but i think that might have been the original request.

@diosmosis diosmosis merged commit 5d4925d into 4.x-dev Aug 29, 2021
@diosmosis diosmosis deleted the 17752-archiving-delete-superseded-plugin-invalidations branch August 29, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically delete plugin specific/report specific invalidations that do not need to run
2 participants