Repository navigation
fix(find): key the filter and request caches by the controller's own class - #315
Merged
roncodes merged 2 commits intoOct 15, 2026
Merged
Conversation
…class Find::httpFilterForModel() cached its answer per model class for the life of the process, ignoring the filter a controller sets on its model and whether the request was internal. Under Octane, the first controller to list a model decided the filter for every other: Fleet-Ops' order filter was applied to Storefront's order lists, so a network's orders page listed every order in the company. The cache is now keyed by the model's own filter, and a filter found by convention is picked per request audience. httpRequestForModel() is keyed by the model's own request class too.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release/v1.6.71 #315 +/- ##
===================================================
Coverage 100.00% 100.00%
Complexity 8885 8885
===================================================
Files 472 472
Lines 27934 27925 -9
===================================================
- Hits 27934 27925 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On production, a brand-new Storefront network with no stores listed every order in the company on its Orders page. The page asks for
GET storefront/int/v1/orders?network=network_rw2htfer7j, and Storefront's order filter limits that to the network's orders. The HAR of that request showed:meta.storefront_id. Storefront's filter excludes every such order (queryForInternalrequiresmeta->storefront_id), so Storefront's filter can't have run.x-cache-status: HIT, so the API list cache kept serving the wrong result for its TTL.Cause
FleetbaseController::setApiModel()gives the model the controller's filter ($model->filter = $this->filter), andModel::getFilter()returns it. ButFind::httpFilterForModel()keeps its answer in a function-levelstatic $cachekeyed only by model class, namespace and version, and readsgetFilter()only on a miss.Fleet-Ops and Storefront both list
Fleetbase\FleetOps\Models\Order, with different filters. In an Octane worker, whichever order list it serves first decides the filter for every later one:network,storefrontandstorefront_idscopes never apply. This is the reported bug.Reproduced in tinker, one process:
The convention fallback (
Http\Filter\Internal\v1\Modelversusv1\Model) depends on whether the request is internal, and that wasn't part of the key either.httpResourceForModel()already keys on the model's explicit resource.httpRequestForModel()had the same gap for$model->request.Fix
httpFilterForModel(): the cache key includes the model's own filter (getFilter()). When the filter comes from the convention fallback, both the internal and public answers are cached, and the request picks between them on read, the wayhttpResourceForModel()does.httpRequestForModel(): the cache key includes the model's own request class (getRequest()). Its fallback still reads the audience only on a miss, as before. Request classes are resolved in controller constructors, and reading the audience there on every call recursed throughUserControllerTest's route stub, which builds a controller on eachroute()call.Find::versionedClass(), shared by both. It returns the first existing class amongInternal\vN\Model(internal routes only),vN\ModelandModel, matching the previousstr_replacefallbacks.Tests
Three new cases in
tests/Unit/FindHttpRequestForModelTest.php:All three fail on
release/v1.6.71and pass with the fix.Validation, run locally on PHP 8.4:
pest(full suite): 2245 tests, 0 failed.php-cs-fixeron the changed files: clean.php -lon PHP 8.2: clean.Release
RELEASE.mdfor v1.6.71.octane:reload).