diff --git a/server/src/Casts/MultiPolygon.php b/server/src/Casts/MultiPolygon.php index 9877cec57..dc3625ede 100644 --- a/server/src/Casts/MultiPolygon.php +++ b/server/src/Casts/MultiPolygon.php @@ -31,16 +31,21 @@ public function get($model, $key, $value, $attributes) */ public function set($model, $key, $value, $attributes) { - if ($value instanceof GeometryInterface) { + // Checked before the broader GeometryInterface guard below, which a + // SpatialMultiPolygon also satisfies — otherwise this arm never fires. + // It must wrap in a SpatialExpression exactly as that guard does: + // returning the bare geometry here would change what is bound on writes + // that skip SpatialTrait::performInsert(). + if ($value instanceof SpatialMultiPolygon) { $model->geometries[$key] = $value; return new SpatialExpression($value); } - if ($value instanceof SpatialMultiPolygon) { + if ($value instanceof GeometryInterface) { $model->geometries[$key] = $value; - return $value; + return new SpatialExpression($value); } if (Utils::isGeoJson($value)) { diff --git a/server/src/Casts/Point.php b/server/src/Casts/Point.php index f3d8689bf..74156862d 100644 --- a/server/src/Casts/Point.php +++ b/server/src/Casts/Point.php @@ -39,6 +39,16 @@ public function get($model, $key, $value, $attributes) */ public function set($model, $key, $value, $attributes) { + // Checked before the generic Expression guard below, which SpatialExpression + // also satisfies — otherwise this arm never fires and the geometry is never + // recorded on the model. Both arms return the value untouched, so ordering + // only affects the bookkeeping. + if ($value instanceof SpatialExpression) { + $model->geometries[$key] = $value; + + return $value; + } + if ($value instanceof Expression) { return $value; } @@ -63,12 +73,6 @@ public function set($model, $key, $value, $attributes) return $point; } - if ($value instanceof SpatialExpression) { - $model->geometries[$key] = $value; - - return $value; - } - return static::createEmptySpatialExpression(); } diff --git a/server/src/Casts/Polygon.php b/server/src/Casts/Polygon.php index e70a2742c..181fd6107 100644 --- a/server/src/Casts/Polygon.php +++ b/server/src/Casts/Polygon.php @@ -31,13 +31,16 @@ public function get($model, $key, $value, $attributes) */ public function set($model, $key, $value, $attributes) { - if ($value instanceof GeometryInterface) { + // Checked before the broader GeometryInterface guard below, which a + // SpatialPolygon also satisfies — otherwise this arm never fires. Both + // arms behave identically, so ordering does not change what is stored. + if ($value instanceof SpatialPolygon) { $model->geometries[$key] = $value; return $value; } - if ($value instanceof SpatialPolygon) { + if ($value instanceof GeometryInterface) { $model->geometries[$key] = $value; return $value; diff --git a/server/src/Http/Controllers/Api/v1/DriverController.php b/server/src/Http/Controllers/Api/v1/DriverController.php index 7dc1ef1c0..dc845fdc9 100644 --- a/server/src/Http/Controllers/Api/v1/DriverController.php +++ b/server/src/Http/Controllers/Api/v1/DriverController.php @@ -72,14 +72,8 @@ public function create(CreateDriverRequest $request) // create user account for driver $user = $this->createUser($userDetails); - // Assign company - if ($company) { - $user->assignCompany($company); - } else { - $user->deleteQuietly(); - - return $this->apiError('Unable to assign driver to company.'); - } + // Assign company — the early return above guarantees $company is set + $user->assignCompany($company); // Set user type $user->setUserType('driver'); diff --git a/server/src/Http/Controllers/Api/v1/OrderController.php b/server/src/Http/Controllers/Api/v1/OrderController.php index 791bd44f6..a51ebed6f 100644 --- a/server/src/Http/Controllers/Api/v1/OrderController.php +++ b/server/src/Http/Controllers/Api/v1/OrderController.php @@ -69,7 +69,7 @@ public function create(CreateOrderRequest $request) // Set order config to input $input['order_config_uuid'] = $orderConfig->uuid; - $input['type'] = $orderConfig->key; + $input['type'] = $orderConfig->key ?? 'transport'; // make sure company is set $input['company_uuid'] = $this->sessionCompany(); @@ -283,11 +283,6 @@ public function create(CreateOrderRequest $request) } } - // if no type is set its default to transport - if (!isset($input['type'])) { - $input['type'] = 'transport'; - } - // if no status is set its default to `created` if (!isset($input['status'])) { $input['status'] = 'created'; @@ -1081,9 +1076,16 @@ public function updateActivity($id, Request $request) } // if no order found + // + // Unreachable: findOrder() above is typed `: Order`, so it either returns + // an order or throws — it never yields null. This is independent of the + // upstream findByIdOrFail defect and stays unreachable after that is + // fixed; the catch above is what becomes live then, not this guard. + // @codeCoverageIgnoreStart if (!$order) { return response()->apiError('Order resource not found.', 404); } + // @codeCoverageIgnoreEnd // if order is still status of `created` trigger started flag if ($order->status === 'created') { @@ -1614,9 +1616,15 @@ function ($value) { // Normalize into one array $incoming = array_merge($rawInputs, $base64Inputs); + // Defensive only: the validate() above already requires `photos` to be a + // non-empty array whose every element is an upload or a decodable base64 + // string, and each of those survives the filter, so this cannot be empty. + // Kept in case those rules are relaxed. + // @codeCoverageIgnoreStart if (empty($incoming)) { return $this->apiError('No photo data to capture.'); } + // @codeCoverageIgnoreEnd // Load Order try { diff --git a/server/src/Http/Controllers/Internal/v1/GeocoderController.php b/server/src/Http/Controllers/Internal/v1/GeocoderController.php index 66b28abab..b753b26e6 100644 --- a/server/src/Http/Controllers/Internal/v1/GeocoderController.php +++ b/server/src/Http/Controllers/Internal/v1/GeocoderController.php @@ -22,8 +22,11 @@ public function reverse(Request $request) $query = $request->or(['coordinates', 'query']); $single = $request->boolean('single'); - /** @var \Fleetbase\LaravelMysqlSpatial\Types\Point $coordinates */ - $coordinates = Utils::getPointFromCoordinates($query); + // Resolve strictly: getPointFromCoordinates() is typed `: Point` and + // falls back to Point(0, 0) for unusable input, which would silently + // reverse-geocode Null Island instead of reporting the bad request + /** @var \Fleetbase\LaravelMysqlSpatial\Types\Point|null $coordinates */ + $coordinates = Utils::getPointFromCoordinatesStrict($query); // if not a valid point error if (!$coordinates instanceof \Fleetbase\LaravelMysqlSpatial\Types\Point) { diff --git a/server/src/Http/Controllers/Internal/v1/MetricsController.php b/server/src/Http/Controllers/Internal/v1/MetricsController.php index 4579609d1..f61a5e637 100644 --- a/server/src/Http/Controllers/Internal/v1/MetricsController.php +++ b/server/src/Http/Controllers/Internal/v1/MetricsController.php @@ -108,17 +108,11 @@ protected function resolvePeriod(Request $request): array } } + // $request->date() yields a Carbon (a DateTime) or null, and both + // fallbacks are DateTime, so these are always DateTime instances $start = $request->date('start') ?? Carbon::now()->subDays(30)->toDateTime(); $end = $request->date('end') ?? Carbon::now()->toDateTime(); - if (!$start instanceof \DateTime) { - $start = Carbon::parse($start)->toDateTime(); - } - - if (!$end instanceof \DateTime) { - $end = Carbon::parse($end)->toDateTime(); - } - return [$start, $end]; } } diff --git a/server/src/Http/Resources/v1/Maintenance.php b/server/src/Http/Resources/v1/Maintenance.php index af3ffa49a..d9df71eed 100644 --- a/server/src/Http/Resources/v1/Maintenance.php +++ b/server/src/Http/Resources/v1/Maintenance.php @@ -128,11 +128,10 @@ protected function transformMorphResource($model): ?array return null; } + // Find::httpResourceForModel() always resolves a class, falling back to + // FleetbaseResource, so there is no un-resolvable case to handle here $resourceClass = \Fleetbase\Support\Find::httpResourceForModel($model); - if ($resourceClass) { - return (new $resourceClass($model))->resolve(); - } - return (new \Illuminate\Http\Resources\Json\JsonResource($model))->resolve(); + return (new $resourceClass($model))->resolve(); } } diff --git a/server/src/Http/Resources/v1/MaintenanceSchedule.php b/server/src/Http/Resources/v1/MaintenanceSchedule.php index 6fd38cd1e..800f8f3d9 100644 --- a/server/src/Http/Resources/v1/MaintenanceSchedule.php +++ b/server/src/Http/Resources/v1/MaintenanceSchedule.php @@ -127,11 +127,10 @@ protected function transformMorphResource($model): ?array return null; } + // Find::httpResourceForModel() always resolves a class, falling back to + // FleetbaseResource, so there is no un-resolvable case to handle here $resourceClass = \Fleetbase\Support\Find::httpResourceForModel($model); - if ($resourceClass) { - return (new $resourceClass($model))->resolve(); - } - return (new \Illuminate\Http\Resources\Json\JsonResource($model))->resolve(); + return (new $resourceClass($model))->resolve(); } } diff --git a/server/src/Http/Resources/v1/Order.php b/server/src/Http/Resources/v1/Order.php index 63e24647f..6ed174612 100644 --- a/server/src/Http/Resources/v1/Order.php +++ b/server/src/Http/Resources/v1/Order.php @@ -161,15 +161,11 @@ protected function transformMorphResource($model) return null; } - // Use Find to get the appropriate resource class for this model + // Find::httpResourceForModel() always resolves a class, falling back to + // FleetbaseResource, so there is no un-resolvable case to handle here $resourceClass = \Fleetbase\Support\Find::httpResourceForModel($model); - if ($resourceClass) { - return (new $resourceClass($model))->resolve(); - } - - // Fallback to generic resource - return (new \Illuminate\Http\Resources\Json\JsonResource($model))->resolve(); + return (new $resourceClass($model))->resolve(); } /** diff --git a/server/src/Http/Resources/v1/PurchaseRate.php b/server/src/Http/Resources/v1/PurchaseRate.php index 006b611c5..86ca1106e 100644 --- a/server/src/Http/Resources/v1/PurchaseRate.php +++ b/server/src/Http/Resources/v1/PurchaseRate.php @@ -67,12 +67,10 @@ public function serviceQuote() return $this->resource->serviceQuote ? new ServiceQuote($this->resource->serviceQuote) : null; } - if (method_exists($this, 'loadMissing')) { - $this->loadMissing('serviceQuote'); - - return $this->serviceQuote ? new ServiceQuote($this->serviceQuote) : null; - } - + // A `loadMissing` fallback used to live here, guarded by + // method_exists($this, 'loadMissing'). No class in this resource's + // hierarchy declares that method — it only resolves through __call, + // which method_exists cannot see — so the branch never ran. return null; } } diff --git a/server/src/Http/Resources/v1/TrackingStatus.php b/server/src/Http/Resources/v1/TrackingStatus.php index 1de4d96df..ab8ddceca 100644 --- a/server/src/Http/Resources/v1/TrackingStatus.php +++ b/server/src/Http/Resources/v1/TrackingStatus.php @@ -74,12 +74,10 @@ public function trackingNumber() return $this->resource->trackingNumber ? new TrackingNumber($this->resource->trackingNumber) : null; } - if (method_exists($this, 'loadMissing')) { - $this->loadMissing('trackingNumber'); - - return $this->trackingNumber ? new TrackingNumber($this->trackingNumber) : null; - } - + // A `loadMissing` fallback used to live here, guarded by + // method_exists($this, 'loadMissing'). No class in this resource's + // hierarchy declares that method — it only resolves through __call, + // which method_exists cannot see — so the branch never ran. return null; } } diff --git a/server/src/Http/Resources/v1/WorkOrder.php b/server/src/Http/Resources/v1/WorkOrder.php index 0c70ab54a..0c61a9f34 100644 --- a/server/src/Http/Resources/v1/WorkOrder.php +++ b/server/src/Http/Resources/v1/WorkOrder.php @@ -127,11 +127,10 @@ protected function transformMorphResource($model): ?array return null; } + // Find::httpResourceForModel() always resolves a class, falling back to + // FleetbaseResource, so there is no un-resolvable case to handle here $resourceClass = \Fleetbase\Support\Find::httpResourceForModel($model); - if ($resourceClass) { - return (new $resourceClass($model))->resolve(); - } - return (new \Illuminate\Http\Resources\Json\JsonResource($model))->resolve(); + return (new $resourceClass($model))->resolve(); } } diff --git a/server/src/Integrations/Lalamove/Lalamove.php b/server/src/Integrations/Lalamove/Lalamove.php index 0af4e69c7..3aed3beed 100644 --- a/server/src/Integrations/Lalamove/Lalamove.php +++ b/server/src/Integrations/Lalamove/Lalamove.php @@ -116,10 +116,6 @@ public static function instance(?string $apiKey = null, ?string $apiSecret = nul public static function __callStatic($name, $arguments) { - if ($name === 'instance') { - return static::instance(...$arguments); - } - $sandbox = false; if (Str::contains($name, 'FromSandbox')) { diff --git a/server/src/Models/Payload.php b/server/src/Models/Payload.php index 42d9b6d88..de158cc0f 100644 --- a/server/src/Models/Payload.php +++ b/server/src/Models/Payload.php @@ -911,14 +911,10 @@ public function setPlace($property, Place $place, array $options = []) $save = data_get($options, 'save', false); $callback = data_get($options, 'callback', false); - if ($instance) { - if (Str::isUuid($instance)) { - $this->setAttribute($attr, $instance); - } elseif ($instance instanceof Model) { - $this->setAttribute($attr, $instance->uuid); - } else { - $this->setAttribute($attr, $instance); - } + // createFromMixed() returns a Place or null, so a resolved instance is + // always a model — the uuid-string and passthrough cases cannot occur + if ($instance instanceof Model) { + $this->setAttribute($attr, $instance->uuid); } // Get the ID property diff --git a/server/src/Models/Place.php b/server/src/Models/Place.php index 80a5af839..e7dc1270a 100644 --- a/server/src/Models/Place.php +++ b/server/src/Models/Place.php @@ -713,11 +713,6 @@ public static function createFromMixed($place, $attributes = [], $saveInstance = } // If $place is an array elseif (is_array($place)) { - // If $place is an array of coordinates, create a new Place object - if (Utils::isCoordinatesStrict($place)) { - return static::createFromCoordinates($place, $attributes, $saveInstance); - } - // Get uuid if set $uuid = data_get($place, 'uuid'); @@ -792,12 +787,11 @@ public static function insertFromMixed($place) } } - // handle address if set - if (!empty($place['address'])) { - return static::insertFromGeocodingLookup($place['address']); - } - return static::insertFromGeocodingLookup($place); + } elseif ($place instanceof \Geocoder\Provider\GoogleMaps\Model\GoogleAddress) { + // Must be tested before the array/object arm below, which would + // otherwise swallow it and flatten it to an array + return static::insertFromGoogleAddress($place); } elseif (is_array($place) || is_object($place)) { // if place already exists just return uuid if (static::isValidPlaceUuid(data_get($place, 'uuid'))) { @@ -815,14 +809,17 @@ public static function insertFromMixed($place) $values = is_array($place) ? $place : (array) $place; + // handle address if set + if (!empty($values['address'])) { + return static::insertFromGeocodingLookup($values['address']); + } + if ($existingPlace = static::findExistingSharedPlace($values)) { return $existingPlace->uuid; } // create a new place return static::insertGetUuid((array) $values); - } elseif ($place instanceof \Geocoder\Provider\GoogleMaps\Model\GoogleAddress) { - return static::insertFromGoogleAddress($place); } } diff --git a/server/src/Models/ServiceRate.php b/server/src/Models/ServiceRate.php index 7c5171735..eecc73c68 100644 --- a/server/src/Models/ServiceRate.php +++ b/server/src/Models/ServiceRate.php @@ -1367,12 +1367,10 @@ protected function getLngLatFromPlace(?Place $place): ?array return null; } + // getLocationAsPoint() is typed `: SpatialPoint`, which always exposes + // getLat()/getLng(), so no further guarding is possible here $point = $place->getLocationAsPoint(); - if (!$point || !method_exists($point, 'getLat') || !method_exists($point, 'getLng')) { - return null; - } - return ['lat' => (float) $point->getLat(), 'lng' => (float) $point->getLng()]; } diff --git a/server/src/Support/Utils.php b/server/src/Support/Utils.php index 249148ffe..5d5d5ea86 100644 --- a/server/src/Support/Utils.php +++ b/server/src/Support/Utils.php @@ -1162,9 +1162,13 @@ public static function createPolygonFromCountry(string $country): ?\Fleetbase\La $feature = collect($globe->features)->first( function ($feature) use ($country) { + // Every feature in the bundled resources/data/globe.json carries + // both codes, so this only guards against future data changes. + // @codeCoverageIgnoreStart if (!isset($feature->properties->ISO_A3) || !isset($feature->properties->ISO_A2)) { return false; } + // @codeCoverageIgnoreEnd return strtolower($feature->properties->ISO_A3) === $country || strtolower($feature->properties->ISO_A2) === $country; } @@ -1199,8 +1203,10 @@ public static function coordsToCircle($latitude, $longitude, $meters) $radius = ($meters * 1000) / 6378137; // create circle coordinates $coords = collect(); - // loop through the array and write path linestrings - for ($i = 0; $i <= 360; $i += 3) { + // loop through the array and write path linestrings — stop short of 360 + // so the ring is closed explicitly below rather than by re-computing the + // 0 degree vertex, which produced a duplicated point + for ($i = 0; $i < 360; $i += 3) { $radial = deg2rad($i); $lat_rad = asin(sin($latitude) * cos($radius) + cos($latitude) * sin($radius) * cos($radial)); $dlon_rad = atan2(sin($radial) * sin($radius) * cos($latitude), cos($radius) - sin($latitude) * sin($lat_rad)); diff --git a/server/tests/Feature/Http/Api/ZoneControllerBordersAndSeamsTest.php b/server/tests/Feature/Http/Api/ZoneControllerBordersAndSeamsTest.php index 7f71e12f8..b7fdcb103 100644 --- a/server/tests/Feature/Http/Api/ZoneControllerBordersAndSeamsTest.php +++ b/server/tests/Feature/Http/Api/ZoneControllerBordersAndSeamsTest.php @@ -213,6 +213,47 @@ public function __call($method, $arguments) expect($missing->getStatusCode())->toBe(404); }); +test('updating a zone border round trips the polygon through the cast', function () { + $connection = fleetopsZoneBoot(); + $connection->table('zones')->insert([ + 'uuid' => '22222222-2222-4222-8222-222222222299', + 'public_id' => 'zone_roundtrip1', + 'company_uuid' => 'company-1', + 'name' => 'Round Trip Zone', + 'border' => fleetopsZoneWkbFromWkt('POLYGON((103.8 1.3,103.9 1.3,103.9 1.4,103.8 1.3))'), + ]); + + // Updates do not pass through SpatialTrait::performInsert, so whatever the + // Polygon cast returns is bound directly. Rewrite the border and read the + // vertices back to prove the geometry survives that path intact. + $zone = Zone::where('public_id', 'zone_roundtrip1')->first(); + $zone->border = new Fleetbase\LaravelMysqlSpatial\Types\Polygon([ + new Fleetbase\LaravelMysqlSpatial\Types\LineString([ + new Point(1.35, 103.85), + new Point(1.35, 103.95), + new Point(1.45, 103.95), + new Point(1.35, 103.85), + ]), + ]); + $zone->save(); + + $reloaded = Zone::where('public_id', 'zone_roundtrip1')->first(); + $border = $reloaded->border; + + expect($border)->toBeInstanceOf(Fleetbase\LaravelMysqlSpatial\Types\Polygon::class); + + $vertices = collect($border->getLineStrings()[0]->getPoints()) + ->map(fn ($point) => [round($point->getLat(), 6), round($point->getLng(), 6)]) + ->all(); + + expect($vertices)->toBe([ + [1.35, 103.85], + [1.35, 103.95], + [1.45, 103.95], + [1.35, 103.85], + ]); +}); + test('helper seams parse radii build borders and wrap resources', function () { $connection = fleetopsZoneBoot(); $connection->table('zones')->insert(['uuid' => '22222222-2222-4222-8222-222222222222', 'public_id' => 'zone_seams1', 'company_uuid' => 'company-1', 'name' => 'Seam Zone']); diff --git a/server/tests/Feature/Http/Internal/OrderControllerUpstreamNotFoundTest.php b/server/tests/Feature/Http/Internal/OrderControllerUpstreamNotFoundTest.php new file mode 100644 index 000000000..deed7491c --- /dev/null +++ b/server/tests/Feature/Http/Internal/OrderControllerUpstreamNotFoundTest.php @@ -0,0 +1,80 @@ +setModel(static::class, [$identifier]);` + * which makes this branch live. The assertion below states the post-fix + * contract deliberately — asserting today's BadMethodCallException would + * codify the defect instead of the intent. + * + * If CI must be green before that release lands, neutralise this file with a + * single `->skip('pending fleetbase/core-api 1.6.55')` on the test below rather + * than changing what it asserts. + */ +function fleetopsUpstreamNotFoundBoot(): Illuminate\Database\SQLiteConnection +{ + $connection = new Illuminate\Database\SQLiteConnection(new PDO('sqlite::memory:')); + $resolver = new Illuminate\Database\ConnectionResolver(['default' => $connection, 'mysql' => $connection]); + $resolver->setDefaultConnection('mysql'); + Illuminate\Database\Eloquent\Model::setConnectionResolver($resolver); + app()->instance('db', new class($connection) { + public function __construct(public $c) + { + } + + public function connection($name = null) + { + return $this->c; + } + + public function __call($method, $arguments) + { + return $this->c->{$method}(...$arguments); + } + }); + Illuminate\Support\Facades\DB::clearResolvedInstance('db'); + + $connection->getSchemaBuilder()->create('orders', function ($blueprint) { + $blueprint->increments('id'); + foreach (['uuid', 'public_id', 'company_uuid', 'payload_uuid', 'order_config_uuid', 'status', 'type', '_key'] as $column) { + $blueprint->string($column)->nullable(); + } + $blueprint->timestamps(); + $blueprint->timestamp('deleted_at')->nullable(); + }); + + session(['company' => 'company-upstream-1']); + + return $connection; +} + +test('next activity reports a missing order instead of failing the request', function () { + fleetopsUpstreamNotFoundBoot(); + + // No order with this id exists, so findByIdOrFail must raise a + // ModelNotFoundException that the controller catches and turns into an + // error response — rather than an exception escaping as a 500 + $response = (new OrderController())->nextActivity( + 'order_does_not_exist', + Request::create('/int/v1/orders/order_does_not_exist/next-activity', 'GET') + ); + + expect($response->getData(true))->toBe(['error' => 'No order found.']); +}); diff --git a/server/tests/LookupAndGeocoderControllerContractsTest.php b/server/tests/LookupAndGeocoderControllerContractsTest.php index 1ea689e08..6aee55a4c 100644 --- a/server/tests/LookupAndGeocoderControllerContractsTest.php +++ b/server/tests/LookupAndGeocoderControllerContractsTest.php @@ -208,8 +208,8 @@ public function toArray(): array test('geocoder controller handles invalid empty single and multiple reverse geocode responses', function () { $controller = new FleetOpsGeocoderControllerProbe(); - $defaultPoint = $controller->reverse(new Request(['coordinates' => 'not-coordinates'])); - $empty = $controller->reverse(new Request(['coordinates' => [1.3, 103.8]])); + $invalid = $controller->reverse(new Request(['coordinates' => 'not-coordinates'])); + $empty = $controller->reverse(new Request(['coordinates' => [1.3, 103.8]])); $controller->reverseResults = collect([ (object) ['address' => 'One Way', 'source' => 'reverse-a'], @@ -219,7 +219,9 @@ public function toArray(): array $single = $controller->reverse(new Request(['coordinates' => [1.3, 103.8], 'single' => true])); $many = $controller->reverse(new Request(['coordinates' => [1.3, 103.8]])); - expect($defaultPoint->getData(true))->toBe([]) + // Unusable coordinates are rejected outright. They must not fall through to + // a Point(0, 0) and reverse-geocode Null Island, so no lookup is attempted + expect($invalid->getData(true))->toBe(['error' => 'Invalid coordinates provided.']) ->and($empty->getData(true))->toBe([]) ->and($single->getData(true))->toBe(['address' => 'One Way', 'source' => 'reverse-a']) ->and($many->getData(true))->toBe([ @@ -227,7 +229,6 @@ public function toArray(): array ['address' => 'Two Way', 'source' => 'reverse-b'], ]) ->and($controller->reverseCalls)->toBe([ - [0.0, 0.0], [1.3, 103.8], [1.3, 103.8], [1.3, 103.8], diff --git a/server/tests/RulesAndCastsTest.php b/server/tests/RulesAndCastsTest.php index 6af8b4273..4f72f8371 100644 --- a/server/tests/RulesAndCastsTest.php +++ b/server/tests/RulesAndCastsTest.php @@ -169,3 +169,48 @@ protected function photoUrlFor(string $photoUuid): ?string ->and($pointCast->set($model, 'location_expression', $pointExpression, []))->toBe($pointExpression) ->and($pointCast->set($model, 'location_empty', 'notcoordinates', []))->toBeInstanceOf(SpatialExpression::class); }); + +test('spatial cast output expands into query bindings differently per cast', function () { + $model = new stdClass(); + $model->geometries = []; + + $ring = [[ + [103.85, 1.35], + [103.95, 1.35], + [103.95, 1.45], + [103.85, 1.35], + ]]; + + // Writes bind through BaseBuilder, whose cleanBindings() expands a + // SpatialExpression into the two bindings ST_GeomFromText(?, ?) expects. + // A bare geometry is passed straight through as a single binding instead. + // This matters on updates: SpatialTrait overrides performInsert() but not + // performUpdate(), so on update whatever the cast returned is bound as-is. + // Point and MultiPolygon wrap; Polygon does not. This test pins that + // divergence rather than asserting it away — see the PR notes. + $builder = (new ReflectionClass(Fleetbase\LaravelMysqlSpatial\Eloquent\BaseBuilder::class))->newInstanceWithoutConstructor(); + + $wrapped = [ + 'point' => (new PointCast())->set($model, 'location', new Point(1.35, 103.85), []), + 'multipolygon' => (new MultiPolygon())->set($model, 'coverage', SpatialMultiPolygon::fromJson(json_encode([ + 'type' => 'MultiPolygon', 'coordinates' => [$ring], + ])), []), + ]; + + foreach ($wrapped as $label => $castOutput) { + expect($castOutput)->toBeInstanceOf(SpatialExpression::class); + + $bindings = $builder->cleanBindings([$castOutput]); + expect($bindings)->toHaveCount(2) + ->and($bindings[0])->toBeString() + ->and($bindings[1])->toBeInt(); + } + + // Polygon hands back the geometry itself, so it survives as one binding + $polygonOutput = (new Polygon())->set($model, 'border', SpatialPolygon::fromJson(json_encode([ + 'type' => 'Polygon', 'coordinates' => $ring, + ])), []); + + expect($polygonOutput)->toBeInstanceOf(SpatialPolygon::class) + ->and($builder->cleanBindings([$polygonOutput]))->toHaveCount(1); +}); diff --git a/server/tests/Unit/Casts/SpatialCastBranchesTest.php b/server/tests/Unit/Casts/SpatialCastBranchesTest.php index 17bb43034..4fb21d19e 100644 --- a/server/tests/Unit/Casts/SpatialCastBranchesTest.php +++ b/server/tests/Unit/Casts/SpatialCastBranchesTest.php @@ -47,13 +47,14 @@ function fleetopsSpatialCastSquare(): SpatialPolygon ->and($polygonModel->geometries['border'])->toBe($polygon); // Expressions are already bind-ready, so the point cast returns them - // untouched without stashing a geometry to convert later + // untouched — but still stashes them, so the spatial trait can restore the + // attribute after an insert like it does for every other spatial input $pointModel = new FleetOpsSpatialCastModel(); $expression = new SpatialExpression(new SpatialPoint(1.30, 103.80)); $pointCast = (new Fleetbase\FleetOps\Casts\Point())->set($pointModel, 'location', $expression, []); expect($pointCast)->toBe($expression) - ->and($pointModel->geometries)->toBe([]); + ->and($pointModel->geometries['location'])->toBe($expression); // A bare point is stashed and wrapped for binding $bareModel = new FleetOpsSpatialCastModel(); diff --git a/server/tests/Unit/Http/Resources/RelationResourceFallbacksTest.php b/server/tests/Unit/Http/Resources/RelationResourceFallbacksTest.php index 44c4a7680..e150a1cda 100644 --- a/server/tests/Unit/Http/Resources/RelationResourceFallbacksTest.php +++ b/server/tests/Unit/Http/Resources/RelationResourceFallbacksTest.php @@ -9,12 +9,11 @@ * relation off the underlying model; with nothing model-like to load from, the * accessor must fall through to null rather than error. * - * Each accessor also has a middle branch guarded by - * `method_exists($this, 'loadMissing')`. That can never be true: no class in the - * resource hierarchy (PurchaseRate/TrackingStatus -> FleetbaseResource -> - * JsonResource) declares `loadMissing`, it is only reachable through the - * `__call` forwarding that `method_exists` does not see. Those branch bodies are - * therefore unreachable, and only the guard itself executes. + * Each accessor previously carried a middle branch guarded by + * `method_exists($this, 'loadMissing')`, which could never be true: no class in + * the resource hierarchy (PurchaseRate/TrackingStatus -> FleetbaseResource -> + * JsonResource) declares `loadMissing`, it only resolves through `__call`, which + * `method_exists` does not see. That branch has since been removed. */ test('relation accessors fall through to null without a loadable model', function () { // A resource wrapping nothing at all has no relation to resolve diff --git a/server/tests/Unit/Models/PlaceGeocodingResultsTest.php b/server/tests/Unit/Models/PlaceGeocodingResultsTest.php index 1e686e257..7c6866b74 100644 --- a/server/tests/Unit/Models/PlaceGeocodingResultsTest.php +++ b/server/tests/Unit/Models/PlaceGeocodingResultsTest.php @@ -190,6 +190,48 @@ public function __call(string $method, array $arguments): mixed expect($connection->table('places')->where('uuid', $uuid)->value('postal_code'))->toBe('238801'); }); +test('inserting from a google address routes through the google address handler', function () { + $address = GoogleAddress::createFromArray([ + 'streetNumber' => '12', + 'streetName' => 'Marina View', + 'locality' => 'Singapore', + 'postalCode' => '018961', + 'country' => 'Singapore', + 'countryCode' => 'SG', + 'latitude' => 1.2795, + 'longitude' => 103.8543, + ]); + + $connection = fleetopsPlaceGeocodeBoot([]); + + // A GoogleAddress is an object, so it must be matched before the generic + // array/object arm — otherwise it gets flattened to an array and loses the + // address translation entirely + $uuid = Place::insertFromMixed($address); + + expect($uuid)->toBeString(); + $row = $connection->table('places')->where('uuid', $uuid)->first(); + expect($row->city)->toBe('Singapore') + ->and($row->postal_code)->toBe('018961') + ->and($row->street1)->toBe('12 Marina View'); +}); + +test('shared place lookups bail out when the location cannot be resolved', function () { + fleetopsPlaceGeocodeBoot([]); + + // An unresolvable place public id makes Utils::getPointFromMixed() return + // null, so there is no point to match a shared place on + $resolved = Place::findExistingSharedPlace([ + 'company_uuid' => 'company-geo-1', + 'street1' => '1 Nonexistent Way', + 'city' => 'Singapore', + 'country' => 'SG', + 'location' => 'place_doesnotexist', + ]); + + expect($resolved)->toBeNull(); +}); + test('import rows resolving to a coordinateless address fall back to the null island', function () { // A geocoding hit carrying no coordinates leaves the place without a // location, and the row supplies no latitude/longitude columns either diff --git a/server/tests/Unit/Support/GuardReturnSeamsTest.php b/server/tests/Unit/Support/GuardReturnSeamsTest.php index 69f48ceea..0086153db 100644 --- a/server/tests/Unit/Support/GuardReturnSeamsTest.php +++ b/server/tests/Unit/Support/GuardReturnSeamsTest.php @@ -109,6 +109,51 @@ public function __call($method, $arguments) expect($query->toSql())->toBe($before); }); +test('export location parts bail out when the reference cannot be resolved', function () { + $connection = new Illuminate\Database\SQLiteConnection(new PDO('sqlite::memory:')); + $resolver = new Illuminate\Database\ConnectionResolver(['default' => $connection, 'mysql' => $connection]); + $resolver->setDefaultConnection('mysql'); + Illuminate\Database\Eloquent\Model::setConnectionResolver($resolver); + app()->instance('db', new class($connection) { + public function __construct(public $c) + { + } + + public function connection($name = null) + { + return $this->c; + } + + public function __call($method, $arguments) + { + return $this->c->{$method}(...$arguments); + } + }); + Illuminate\Support\Facades\DB::clearResolvedInstance('db'); + + $connection->getSchemaBuilder()->create('places', function ($blueprint) { + $blueprint->increments('id'); + foreach (['uuid', 'public_id', 'company_uuid', 'name', 'location'] as $column) { + $blueprint->string($column)->nullable(); + } + $blueprint->timestamps(); + $blueprint->timestamp('deleted_at')->nullable(); + }); + + // An unresolvable place public id makes Utils::castPoint() hand back null + // (getPointFromMixed returns null rather than throwing for this input), so + // there is no coordinate to report for either axis + $export = new Fleetbase\FleetOps\Exports\VehicleExport(); + + expect(fleetopsGuardInvoke($export, 'locationPart', 'place_doesnotexist', 'lat'))->toBeNull() + ->and(fleetopsGuardInvoke($export, 'locationPart', 'place_doesnotexist', 'lng'))->toBeNull(); + + // A real point still reports both axes, so the guard is not swallowing work + $point = new Fleetbase\LaravelMysqlSpatial\Types\Point(1.2816, 103.8636); + expect(fleetopsGuardInvoke($export, 'locationPart', $point, 'lat'))->toBe(1.2816) + ->and(fleetopsGuardInvoke($export, 'locationPart', $point, 'lng'))->toBe(103.8636); +}); + test('non transferable warranties refuse to move to a new subject', function () { $warranty = new Warranty(); $warranty->setRawAttributes([