Courses

Laravel API Code Review and Refactor

Route Model Binding

The next piece of code I didn't like is the OrderController methods NOT using Route Model Binding:

app/Http/Controllers/Api/V1/OrderController.php

public function show($order_id)
{
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}
 
public function update(UpdateOrderRequest $request, $order_id)
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}
 
public function destroy($order_id)
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}

We shouldn't need to call the ModelNotFoundException manually, Laravel can do that internally.

That said, we need to preserve the same response with the Order not found message, so this refactoring is not as easy as you may expect.


Controller to Route Model Binding

This part is simple:

  • Type-hint the Model
  • Remove the findOrFail()
  • Remove one of the catch parts

app/Http/Controllers/Api/V1/OrderController.php

public function show($order_id)
public function show(Order $order)
{
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}
 
public function update(UpdateOrderRequest $request, $order_id)
public function update(UpdateOrderRequest $request, Order $order)
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}
 
public function destroy($order_id)
public function destroy(Order $order)
try {
$order = Order::findOrFail($order_id);
 
// ...
} catch (ModelNotFoundException $eModelNotFound) {
return $this->responseNotFound('Order not found');
}
}

Catch the 404 Exception Globally

Next, we need to catch that ModelNotFoundException, showing exactly the same error message as in the Trait method responseNotFound():

app/Traits/V1/ApiResponses.php:

public function responseNotFound(string $message = 'Not Found.'): JsonResponse
{
return $this->responseError($message, Response::HTTP_NOT_FOUND);
}
 
protected function responseError($message, int $statusCode = 500): JsonResponse
{
return response()->json([
'errors' => $message,
'status' => $statusCode
], $statusCode);
}

Since Laravel 11, the global settings for Exceptions are defined in the bootstrap/app.php file.

Here's the code, and I will explain it below:

bootstrap/app.php:

use Illuminate\Database\Eloquent\ModelNotFoundException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpFoundation\Response;
 
// ...
 
return Application::configure(basePath: dirname(__DIR__))
->withRouting(
// ...
})
->withExceptions(function (Exceptions $exceptions) {
$exceptions->renderable(function (NotFoundHttpException $e, $request) {
$previous = $e->getPrevious();
 
if ($previous instanceof ModelNotFoundException) {
return response()->json([
'errors' => str($previous->getModel())->afterLast('\\') . ' not found',
'status' => Response::HTTP_NOT_FOUND,
], Response::HTTP_NOT_FOUND);
}
});

When trying to refactor it, I realized that Laravel doesn't catch the ModelNotFoundException here for some reason.

So, here's what "workaround" I needed to do:

  1. Go one level up and catch the Symfony NotFoundHttpException
  2. Trace back to the $previous->getModel() to show the correct error of which Model was not found so that the Exception would work for any Eloquent model in the future.

At this point, I was tempted to remove the method responseNotFound() from the Trait, but other controllers still used it, so we will get to that in future lessons.


Fixing Validation Rule

To make the automated tests pass, I realized we must also add Route Model binding to the Custom validation rule used inside the Controller.

app/Rules/V1/UpdateOrderStockValidation.php

use App\Models\Order;
 
class UpdateOrderStockValidation implements ValidationRule
{
public function __construct(
protected $order_id
protected Order $order
) {}
 
public function validate(string $attribute, mixed $value, Closure $fail): void
{
$order = Order::findOrFail($this->order_id)?->load('products');
$order = Order::findOrFail($this->order->id)?->load('products');
 
// ...
}
}

New Test: Model Not Found

In the OrdersShowTest, I added one more method:

tests/Feature/OrdersShowTest.php:

public function test_returns_404_for_non_existing_order()
{
$user = User::factory()->create();
 
$response = $this
->actingAs($user)
->getJson('/api/v1/orders/999');
 
$response->assertStatus(404)
->assertJson([
'errors' => 'Order not found'
]);
}

And the tests are passing!

Here's the GitHub commit for these changes.

Previous: Duplicate Code: Make Controller Resourceful

No comments yet…

avatar
You can use Markdown