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

Permalinks: look up from index if cache empty #6568

Draft
wants to merge 3 commits into
base: v5/develop
Choose a base branch
from

Conversation

distantnative
Copy link
Member

Description

@lukasbestle Implemented mostly as discussed in issue, though with Cache::isEmpty() instead of a special key to flag. However, I am very unsure if this really is a solution. Consider the scenario:

  • Cache doesn't exist
  • Permalink request now is allowed to use the index to resolve the UUID
  • Finds the model which also populates the cache for this UUID
  • Other permalink comes around
  • Cache isn't empty anymore
  • Returns false cause it won't find the UUID in cache and is not allowed to use index

So are we really solving the problem of the issue or in reality only postponing it one further request?

Summary of changes

  • If the cache is empty, actually allow searching for UUID in index when resolving permalinks
  • Cache::isEmpty() methods
  • Cleaned up Cache classes (method order)

Reasoning

Additional context

Review commits one by one as the method reordering clutters the general diff.

Changelog

Fixes

Breaking changes

  • Cache handler must implement ::isIndex(): bool method

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added needs: discussion 🗣 Requires further discussion to proceed type: enhancement ✨ Suggests an enhancement; improves Kirby labels Jul 24, 2024
if (empty($this->options['prefix']) === false) {
$iterator = new APCUIterator('!^' . preg_quote($this->options['prefix']) . '!');
} else {
$iterator = new APCUIterator();

Check failure

Code scanning / Psalm

TooFewArguments Error

Too few arguments for APCUIterator::__construct - expecting search to be passed
@distantnative distantnative linked an issue Jul 24, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: discussion 🗣 Requires further discussion to proceed type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File Permalinks throw 404 if UUID cache is empty
2 participants