Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Fixed

- **A controller's filter is used for every query it serves.** `Find::httpFilterForModel()` cached its answer per model class for the life of the process, ignoring the filter a controller sets on its model (`$filter`) 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. `Find::httpRequestForModel()` is keyed by the model's own request class too. (#315)
- **Live and test-mode data no longer share API cache entries.** List, model and relationship cache keys now carry the database connection (`{api_query}:mysql:…` / `{api_query}:sandbox:…`). Before, a console request in test mode could be served the cached live response for the same query (and the reverse), because console sessions carry no API credential to tell the two apart. (#312)
- **Driver-assigned lifecycle events reach `driver.{public_id}` and `driver.{uuid}`.** Order events for records with an assigned driver were only published on `driverAssigned.*`, which clients following the `{type}.{id}` convention never subscribe to. `driverAssigned.*` is still published and is now deprecated. (#311)

Expand Down
108 changes: 50 additions & 58 deletions src/Support/Find.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,47 +105,31 @@ protected static function httpResourceByConvention(string $baseNamespace, string
*/
public static function httpRequestForModel(Model $model, ?string $namespace = null, ?int $version = 1): ?string
{
// Create a unique cache key based on parameters.
$cacheKey = md5(get_class($model) . '|' . ($namespace ?? '') . '|' . $version . '|' . Http::action());
// A controller can name its model's request class (`$model->request`). It belongs in the key:
// the cache lives as long as the process, so in an Octane worker one controller's answer was
// otherwise served to every other controller of the same model.
$explicit = method_exists($model, 'getRequest') ? $model->getRequest() : null;
$cacheKey = md5(get_class($model) . '|' . ($namespace ?? '') . '|' . $version . '|' . Http::action() . '|' . (is_string($explicit) ? $explicit : ''));

static $cache = [];
if (isset($cache[$cacheKey])) {
return $cache[$cacheKey];
}

$requestNamespace = null;
$requestNamespace = $explicit;
$defaultRequestNS = '\\Fleetbase\\Http\\Requests\\';
$requestNS = $baseNamespace = $namespace ? $namespace . '\\Http\\Requests\\' : $defaultRequestNS;
$requestNS = $namespace ? $namespace . '\\Http\\Requests\\' : $defaultRequestNS;
$modelName = Utils::classBasename($model);

if (method_exists($model, 'getRequest')) {
$requestNamespace = $model->getRequest();
}

if ($requestNamespace === null) {
$requestNamespace = $requestNS . Str::studly(ucfirst(Http::action()) . ucfirst($modelName) . 'Request');
}

if (!Utils::classExists($requestNamespace)) {
$internal = Http::isInternalRequest();
if ($internal) {
$baseNamespace .= 'Internal\\';
}
$requestNamespace = $baseNamespace . "v{$version}\\" . $modelName;

if (!Utils::classExists($requestNamespace)) {
$requestNamespace = str_replace('Internal\\', '', $requestNamespace);
}
if (!Utils::classExists($requestNamespace)) {
$requestNamespace = str_replace("v{$version}\\", '', $requestNamespace);
}
$requestNamespace = static::versionedClass($requestNS, $modelName, $version, Http::isInternalRequest());
}

try {
if (!Utils::classExists($requestNamespace)) {
throw new \Exception('Missing resource');
}
} catch (\Error|\Exception $e) {
if (!Utils::classExists($requestNamespace)) {
$requestNamespace = $defaultRequestNS . 'FleetbaseRequest';
}

Expand All @@ -168,47 +152,55 @@ public static function httpRequestForModel(Model $model, ?string $namespace = nu
*/
public static function httpFilterForModel(Model $model, ?string $namespace = null, ?int $version = 1): ?string
{
// Create a unique cache key based on the model, namespace, version, and a filter flag.
$cacheKey = md5(get_class($model) . '|' . ($namespace ?? '') . '|' . $version . '|filter');
static $cache = [];
if (isset($cache[$cacheKey])) {
return $cache[$cacheKey];
}

$namespaceSegments = explode('Models', get_class($model));
$baseNS = '\\' . rtrim($namespaceSegments[0], '\\');
$filterNamespace = null;
$defaultFilterNS = $baseNS . '\\Http\\Filter\\';
$filterNs = $namespace ? $namespace . '\\Http\\Filter\\' : $defaultFilterNS;
$modelName = Utils::classBasename($model);

if (method_exists($model, 'getFilter')) {
$filterNamespace = $model->getFilter();
}
// A controller names its model's filter (`$model->filter`): Fleet-Ops and Storefront both list
// orders with different filters. It belongs in the key: the cache lives as long as the process,
// so in an Octane worker the first controller's filter was otherwise applied to every later
// controller of the same model.
$explicit = method_exists($model, 'getFilter') ? $model->getFilter() : null;
$cacheKey = md5(get_class($model) . '|' . ($namespace ?? '') . '|' . $version . '|filter|' . (is_string($explicit) ? $explicit : ''));

if (!array_key_exists($cacheKey, $cache)) {
$namespaceSegments = explode('Models', get_class($model));
$baseNS = '\\' . rtrim($namespaceSegments[0], '\\');
$filterNs = $namespace ? $namespace . '\\Http\\Filter\\' : $baseNS . '\\Http\\Filter\\';
$modelName = Utils::classBasename($model);
$filterNamespace = $explicit ?? $filterNs . Str::studly(ucfirst($modelName) . 'Filter');
$orNull = fn (string $class) => Utils::classExists($class) ? $class : null;

$cache[$cacheKey] = Utils::classExists($filterNamespace)
? $filterNamespace
// The versioned fallback differs for internal routes, so both are kept and the request picks.
: [
'internal' => $orNull(static::versionedClass($filterNs, $modelName, $version, true)),
'public' => $orNull(static::versionedClass($filterNs, $modelName, $version, false)),
];
}

$cached = $cache[$cacheKey];

return is_array($cached) ? (Http::isInternalRequest() ? $cached['internal'] : $cached['public']) : $cached;
}

if ($filterNamespace === null) {
$filterNamespace = $filterNs . Str::studly(ucfirst($modelName) . 'Filter');
/**
* The versioned class a model's request or filter falls back to: `Internal\v1\Model` for internal
* routes, then `v1\Model`, then `Model`, the first that exists (the last when none does).
*/
protected static function versionedClass(string $baseNamespace, string $modelName, ?int $version, bool $internal): string
{
$candidates = [$baseNamespace . "v{$version}\\" . $modelName, $baseNamespace . $modelName];
if ($internal) {
array_unshift($candidates, $baseNamespace . "Internal\\v{$version}\\" . $modelName);
}

if (!Utils::classExists($filterNamespace)) {
$internal = Http::isInternalRequest();
$baseNamespace = $filterNs;
if ($internal) {
$baseNamespace = $filterNs . 'Internal\\';
}
$filterNamespace = $baseNamespace . "v{$version}\\" . $modelName;

if (!Utils::classExists($filterNamespace)) {
$filterNamespace = str_replace('Internal\\', '', $filterNamespace);
}
if (!Utils::classExists($filterNamespace)) {
$filterNamespace = str_replace("v{$version}\\", '', $filterNamespace);
foreach ($candidates as $candidate) {
if (Utils::classExists($candidate)) {
return $candidate;
}
}

$cache[$cacheKey] = Utils::classExists($filterNamespace) ? $filterNamespace : null;

return $cache[$cacheKey];
return end($candidates);
}

public static function getModelPackage(Model $model): ?string
Expand Down
88 changes: 88 additions & 0 deletions tests/Unit/FindHttpRequestForModelTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,27 @@ class FilterResolutionRecord extends \Illuminate\Database\Eloquent\Model
{
}

/** A model whose controller may name its filter and request, as FleetbaseController does. */
class ControllerChosenResolutionRecord extends \Illuminate\Database\Eloquent\Model
{
public ?string $filter = null;
public ?string $request = null;

public function getFilter(): ?string
{
return $this->filter;
}

public function getRequest(): ?string
{
return $this->request;
}
}

class AudienceFilterResolutionRecord extends \Illuminate\Database\Eloquent\Model
{
}

class ExplicitResourceResolutionRecord extends \Illuminate\Database\Eloquent\Model
{
public function getResource(): string
Expand Down Expand Up @@ -66,6 +87,34 @@ class ExplicitResource
class FilterResolutionRecordFilter
{
}

/** The model's own filter, found by convention. */
class ControllerChosenResolutionRecordFilter
{
}

/** Another module's filter for the same model (Storefront's order filter over Fleet-Ops orders). */
class OtherModuleFilter
{
}
}

namespace Fleetbase\Tests\Fixtures\Http\Filter\v1 {
class AudienceFilterResolutionRecord
{
}
}

namespace Fleetbase\Tests\Fixtures\Http\Filter\Internal\v1 {
class AudienceFilterResolutionRecord
{
}
}

namespace Fleetbase\Tests\Fixtures\Http\Requests {
class OtherModuleRequest
{
}
}

namespace Fleetbase\Tests\Fixtures\Http\Requests\Internal\v1 {
Expand All @@ -83,14 +132,21 @@ class PackageResolutionRecord extends \Illuminate\Database\Eloquent\Model
namespace {
use Fleetbase\Http\Requests\FleetbaseRequest;
use Fleetbase\Support\Find;
use Fleetbase\Tests\Fixtures\Http\Filter\ControllerChosenResolutionRecordFilter;
use Fleetbase\Tests\Fixtures\Http\Filter\FilterResolutionRecordFilter;
use Fleetbase\Tests\Fixtures\Http\Filter\Internal\v1\AudienceFilterResolutionRecord as AudienceFilterInternal;
use Fleetbase\Tests\Fixtures\Http\Filter\OtherModuleFilter;
use Fleetbase\Tests\Fixtures\Http\Filter\v1\AudienceFilterResolutionRecord as AudienceFilterPublic;
use Fleetbase\Tests\Fixtures\Http\Requests\CreateAssetValidationRecordRequest;
use Fleetbase\Tests\Fixtures\Http\Requests\Internal\v1\InternalRequestResolutionRecord as InternalRequestResolutionRecordRequest;
use Fleetbase\Tests\Fixtures\Http\Requests\OtherModuleRequest;
use Fleetbase\Tests\Fixtures\Http\Requests\UpdateAssetValidationRecordRequest;
use Fleetbase\Tests\Fixtures\Http\Resources\ExplicitResource;
use Fleetbase\Tests\Fixtures\Http\Resources\Internal\v1\InternalResourceResolutionRecord as InternalResourceResolutionRecordResource;
use Fleetbase\Tests\Fixtures\Http\Resources\v1\ResourceResolutionRecord as ResourceResolutionRecordResource;
use Fleetbase\Tests\Fixtures\Models\AssetValidationRecord;
use Fleetbase\Tests\Fixtures\Models\AudienceFilterResolutionRecord;
use Fleetbase\Tests\Fixtures\Models\ControllerChosenResolutionRecord;
use Fleetbase\Tests\Fixtures\Models\ExplicitResourceResolutionRecord;
use Fleetbase\Tests\Fixtures\Models\FilterResolutionRecord;
use Fleetbase\Tests\Fixtures\Models\InternalRequestResolutionRecord;
Expand Down Expand Up @@ -189,4 +245,36 @@ function find_test_request(string $path, string $routeUri, string $namespace = '
->and(Find::getModelPackage(new Fleetbase\Storefront\Models\PackageResolutionRecord()))
->toBe('Storefront');
});

test('a filter a controller sets on its model is used even after the model resolved another filter', function () {
$fleetOps = new ControllerChosenResolutionRecord();

$storefront = new ControllerChosenResolutionRecord();
$storefront->filter = OtherModuleFilter::class;

// The first controller's answer used to be cached for the model class and served to the second.
expect(Find::httpFilterForModel($fleetOps))->toBe('\\' . ControllerChosenResolutionRecordFilter::class)
->and(Find::httpFilterForModel($storefront))->toBe(OtherModuleFilter::class)
->and(Find::httpFilterForModel(new ControllerChosenResolutionRecord()))->toBe('\\' . ControllerChosenResolutionRecordFilter::class);
});

test('a convention filter is resolved for the request audience, not the first audience asked', function () {
app()->instance('request', find_test_request('/v1/records', 'v1/records'));
$public = Find::httpFilterForModel(new AudienceFilterResolutionRecord());

app()->instance('request', find_test_request('/int/v1/records', 'int/v1/records'));
$internal = Find::httpFilterForModel(new AudienceFilterResolutionRecord());

expect($public)->toBe('\\' . AudienceFilterPublic::class)
->and($internal)->toBe('\\' . AudienceFilterInternal::class);
});

test('a request class a controller sets on its model is used even after the model resolved another', function () {
$_SERVER['REQUEST_METHOD'] = 'POST';
$named = new ControllerChosenResolutionRecord();
$named->request = OtherModuleRequest::class;

expect(Find::httpRequestForModel(new ControllerChosenResolutionRecord(), '\Fleetbase\Tests\Fixtures'))->toBe('\\' . FleetbaseRequest::class)
->and(Find::httpRequestForModel($named, '\Fleetbase\Tests\Fixtures'))->toBe(OtherModuleRequest::class);
});
}
Loading