Repository navigation
fix: declare relation return types the console requests and filter resource expansions through core - #392
Merged
Conversation
Core's RequestRelations now loads a client-requested `with`/`expand`/`with_count` name only when the method (or Expandable closure) declares an Eloquent Relation return type, or the model lists it in `$withable`. These relations are requested by the console and the public v1 expansions but were untyped, so they would stop loading: - Fleet: photo, serviceArea, zone, vendor, parentFleet (BelongsTo); subFleets (HasMany); drivers, vehicles (HasManyThrough) - Issue: reporter, assignee, vehicle, driver (BelongsTo) - FuelReport: driver, vehicle, reporter (BelongsTo) - ServiceRate: rateFees, parcelFees (HasMany); serviceArea, zone (BelongsTo) - Driver: vehicle (BelongsTo) - Payload: entities (HasMany); dropoff, pickup, return (BelongsTo); waypoints (HasManyThrough) - UserExpansion closures: driver, currentDriverSession, customer, contact (HasOne); driverProfiles (HasMany). CompanyExpansion: drivers (HasMany) Test fakes that override these methods now declare the same return type and extend the matching relation class, since an untyped override of a typed parent method is a fatal error at class load.
ResolvesPublicRelationFields::requestedRelations() kept any `with` name whose root was a method on the model, and the Fleet and Vehicle resources pass the result to loadMissing(). Eloquent loads a name by calling it on a blank model, so `?with=saveInstance` ran that method. When Fleetbase\Support\RequestRelations is available, an Eloquent resource now keeps only the paths RequestRelations::eagerLoads() accepts, named as the model's methods, which also matches the canonical names publicRelationObject() compares against. The method_exists check stays as the fallback for core releases without RequestRelations and for non-Eloquent resources.
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.
Today the API passes every name in a client's
with,expandorwith_countstraight to Eloquent. Eloquent loads a name by calling$model->newInstance()->$name(), so any public method that takes no required arguments runs on a blank model. For example,GET /int/v1/orders?with[]=dispatchrunsOrder::dispatch(), which saves a blank order and queuesOrderDispatchedand its webhooks.core-api#303 restricts these names to methods that declare an Eloquent
Relationreturn type, or that the model lists in$withable. Union types likeBelongsTo|Buildercount. This PR keeps fleetops working under that rule and closes one fleetops-side bypass.1. Relation return types (
e4a85d74)These relations are untyped today. The console requests them, or they are documented public expansions, so they would stop loading under #303:
fleets?with=(all eight are documented in Postman and fleetbase.io)driverAssigned.vehicle); public v1drivers?with=vehicleUserCompany::drivers()already takes precedenceI checked every entry in the public allowlists (
FleetController::EXPANDABLE,DriverController,VehicleController,TrailerController). Each target is now typed or already was. Vehicle and Trailer were already typed, and the union-typed Order and Driver relations (driverAssigned,vehicleAssigned,facilitator,currentJob,jobs) already pass.Fatal-error check: adding a return type is fatal for any subclass that overrides the method without one. I scanned every package and followed subclass chains (82 subclasses). No
srcclass overrides a typed method. The only subclass outside fleetops is storefront's testServiceRateQuoteStub, which overrides none of them. Several fleetops test fakes did override these methods untyped. They now declare the matching type, and their fake relations extend the real relation class.2. Resource bypass (
45fab851)ResolvesPublicRelationFields::requestedRelations()checks the request'swithnames with onlymethod_existsbeforeFleetandVehiclecallloadMissing($with). That path also renders nested inside other responses, so?with=saveInstancereached the loader there. When core'sRequestRelationsexists, Eloquent models now go throughRequestRelations::eagerLoads(), which returns names as the model's methods. Older core releases and non-Eloquent fixtures keep the previous check.A new test in
VehicleResourceContractsTestrequests[driver, saveInstance, untypedDriver, delete, driver.saveInstance, parent_vehicle, payload]and gets back['driver', 'parentVehicle'], without ever callingsaveInstance. It is skipped whenRequestRelationsis absent.Merge order
Merge this with core-api#303 or before it. On today's core, part 1 changes nothing at runtime and part 2 falls back to the old check.
Docs / API spec
Every documented expansion stays requestable, and unknown names are still ignored, so no Postman change is needed.
fleetbase.ioextension-development/backend/expansions-and-observers.mdxshould say that an Expansion relation needs a declared closure return type to be requestable throughwith. That page is tracked in core-api#303.Validation
php -lon all 16 changed files: clean.SocketCluster\SocketToken,SocketChannelRegistry,VerificationCode::CHECK_VALID. None of them touch the changed files.RequestRelations: I copied the class into the vendored core temporarily and reran the 99 resource andwith-related files. All pass, including the new test, except 3 with the same SocketToken failure.