Courses

Laravel API Code Review and Refactor

OrderController with no try-catch: Final Version

In this lesson, we will finalize our big goal of shortening the OrderController and removing the other try-catch methods.

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

public function store(StoreOrderRequest $request)
{
    Gate::authorize('create', Order::class);
 
    try {
        $order = $this->orderService->createOrderHandleProducts($request);
 
        return response()->json(new OrderResource($order), Response::HTTP_CREATED);
    } catch (QueryException $eQueryException) {
        DB::rollback(); // Rollback transaction on database error
 
        return $this->responseDbError();
    } catch (Throwable $eTh) {
        DB::rollback(); // Rollback transaction on any other error
 
        return $this->responseUnexpectedError();
    }
}
 
public function update(UpdateOrderRequest $request, Order $order)
{
    Gate::authorize('update', $order);
 
    try {
        $this->orderService->updateOrderHandleProducts($request, $order);
 
        return response()->json(new OrderResource($order), Response::HTTP_OK);
    } catch (QueryException $eQueryException) {
        DB::rollback(); // Rollback transaction on database error
 
        return $this->responseDbError();
    } catch (Throwable $eTh) {
        DB::rollback(); // Rollback transaction on any other error
 
        return $this->responseUnexpectedError();
    }
}

Those try-catch blocks "clutter" the Controller, trying to catch Exceptions, which we can move to the bootstrap/app.php, like we did in the previous lessons with 404 and 403 status code Exceptions.

As a result, I hope to also entirely eliminate two Trait methods: responseDbError() and responseUnexpectedError(). Or maybe even remove that Trait altogether. We'll see.

Exceptions from Controller to bootstrap/app.php File

First, let's remove those from the Controller.

The methods are simplified to just THIS:

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

public function store(StoreOrderRequest $request)
{
    Gate::authorize('create', Order::class);
 
    $order = $this->orderService->createOrderHandleProducts($request);
 
    return response()->json(new OrderResource($order), Response::HTTP_CREATED);
}
 
public function update(UpdateOrderRequest $request, Order $order)
{
    Gate::authorize('update', $order);
 
    $this->orderService->updateOrderHandleProducts($request, $order);
 
    return response()->json(new OrderResource($order), Response::HTTP_OK);
}

Then, we handle those Exceptions in the same place as other Exceptions, globally.

bootstrap/app.php:

use Illuminate\Database\QueryException;
use Illuminate\Validation\ValidationException;
 
// ...
 
->withExceptions(function (Exceptions $exceptions) {
    $exceptions->renderable(function (Throwable $e) {
        if ($e instanceof AuthorizationException) {
             return response()->json([
                 'errors' => 'You are not authorized.',
                 'status' => Response::HTTP_FORBIDDEN,
             ], Response::HTTP_FORBIDDEN);
         }
 
        $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);
        }
 
        if ($e instanceof AccessDeniedHttpException) {
            return response()->json([
                'errors' => 'You are not authorized.',
                'status' => Response::HTTP_FORBIDDEN,
            ], Response::HTTP_FORBIDDEN);
        };
 
        if ($e instanceof QueryException) { 
            return response()->json([
                'errors' => 'Database error.',
                'status' => Response::HTTP_INTERNAL_SERVER_ERROR,
            ], Response::HTTP_INTERNAL_SERVER_ERROR);
        };
 
        if (! $e instanceof ValidationException) {
            return response()->json([
                'errors' => 'An unexpected error occurred.',
                'status' => Response::HTTP_INTERNAL_SERVER_ERROR,
            ], Response::HTTP_INTERNAL_SERVER_ERROR);
        } 
    });
})->create();

I've made a few changes here:

  1. Added two highlighted blocks: to catch the QueryException, and to catch all other Exceptions except ValidationException, because that should be handled by the Form Request validation.
  2. Re-grouped it all under one group of the Throwable interface, inside just checking $e instanceof class with if-statements.

See, now we have a complete list of Exceptions and their handling, all in one place, no matter which project Controller or other file they come from.

Here's the GitHub commit for this change so far.

Wait, but you removed DB::rollback()?

You may be wondering about the DB::rollback() method that I removed.

try {
    $this->orderService->updateOrderHandleProducts($request, $order);
 
    return response()->json(new OrderResource($order), Response::HTTP_OK);
} catch (QueryException $eQueryException) {
    DB::rollback(); 
 
    // ...
}

It's supposed to rollback the DB transaction started inside of that $this->orderService->updateOrderHandleProducts() method:

app/Services/V1/OrdersService.php:

public function createOrderHandleProducts(BaseOrderRequest $request)
{
    DB::beginTransaction(); // Start transaction
 
    // Create new order
    $order = Order::create($request->mappedAttributes());
 
    // Get products from request
    $incomingProducts = collect($request->input('data.relationships.products', []))->keyBy('id');
 
    foreach ($incomingProducts as $productId => $productData) {
        $quantity = $productData['quantity'];
        $price = $productData['price'];
 
        // Lock the product row for update to prevent race conditions
        $lockedProduct = Product::where('id', $productId)->lockForUpdate()->first();
 
        // Attach product to order with quantity and price
        $order->products()->attach($productId, ['quantity' => $quantity, 'price' => $price]);
 
        // Decrease stock
        $lockedProduct->decrement('stock', $quantity);
    }
 
    DB::commit(); // Commit transaction if everything succeeds
 
    return $order;
}

But this is again a problem with the single responsibility principle. Why is the successful transaction handled in the Service, but the failed transaction is rolled back in Controller?

And again, there's a better "Laravel way" to handle it automatically.

Instead of manual DB::commit(), we will combine it and change the Transaction mechanism to DB::transaction().

app/Services/V1/OrdersService.php:

public function createOrderHandleProducts(BaseOrderRequest $request): Order
{
    return DB::transaction(function () use ($request) {
        // Create new order
        $order = Order::create($request->mappedAttributes());
 
        // Get products from request
        $incomingProducts = collect($request->input('data.relationships.products', []))->keyBy('id');
 
        foreach ($incomingProducts as $productId => $productData) {
            $quantity = $productData['quantity'];
            $price = $productData['price'];
 
            // Lock the product row for update to prevent race conditions
            $lockedProduct = Product::where('id', $productId)->lockForUpdate()->first();
 
            // Attach product to order with quantity and price
            $order->products()->attach($productId, ['quantity' => $quantity, 'price' => $price]);
 
            // Decrease stock
            $lockedProduct->decrement('stock', $quantity);
        }
 
        return $order;
    });
}

A few things to note here:

  1. If something wrong happens inside of that DB::transaction() callback function, the transaction will be rolled back automatically. That's why we can safely remove that DB::rollback() from the Controller.
  2. You can also return the result from DB::transaction(), which I use here as a result of the entire method, without temporary variables.

I performed the same refactoring for the other two methods inside the same Service: updateOrderHandleProducts() and deleteOrderHandleProducts().

Here's the GitHub commit for this Transaction refactoring.

OrderController: Final Cleanup

There are a few small pieces we can shorten in this Controller:

  • Remove the unused classes from the use section
  • Remove the unnecessary response()->json() in a few more methods

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

use Illuminate\Database\QueryException; 
use Illuminate\Http\Response;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Gate;
use Throwable; 
 
// ...
 
class OrderController extends ApiController
{
 
    // ...
    public function store(StoreOrderRequest $request)
    {
        Gate::authorize('create', Order::class);
 
        $order = $this->orderService->createOrderHandleProducts($request);
 
        return response()->json(new OrderResource($order), Response::HTTP_CREATED); 
        return new OrderResource($order); 
    }
 
    public function update(UpdateOrderRequest $request, Order $order)
    {
        Gate::authorize('update', $order);
 
        $this->orderService->updateOrderHandleProducts($request, $order);
 
        return response()->json(new OrderResource($order), Response::HTTP_OK); 
        return new OrderResource($order); 
    }
}

Just to make sure... the tests are still green!

This is the GitHub commit for this cleanup.

Finished OrderController: BEFORE and AFTER

Finally, I think I've completed the refactoring of OrderController, simplifying it as much as possible, removing unnecessary parts, and moving them elsewhere.

Look at what we had in the very beginning: 159 lines of code.

BEFORE:

<?php
 
namespace App\Http\Controllers\Api\V1;
 
use App\Http\Filters\V1\OrderFilter;
use App\Http\Requests\Api\V1\ReplaceOrderRequest;
use App\Http\Requests\Api\V1\StoreOrderRequest;
use App\Http\Requests\Api\V1\UpdateOrderRequest;
use App\Http\Resources\V1\OrderCollection;
use App\Http\Resources\V1\OrderResource;
use App\Models\Order;
use App\Policies\V1\OrderPolicy;
use App\Services\V1\OrdersService;
use Illuminate\Auth\Access\AuthorizationException;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Database\QueryException;
use Illuminate\Http\Response;
use Illuminate\Support\Facades\DB;
use Throwable;
 
class OrderController extends ApiController
{
    protected $policyClass = OrderPolicy::class;
 
    public function __construct(
        protected OrdersService $orderService
    ) {}
 
    /**
     * Display a listing of the resource.
     */
    public function index(OrderFilter $filter)
    {
        try {
            return response()->json(new OrderCollection(
                Order::filter($filter)->paginate()
            ), Response::HTTP_OK);
        } catch (AuthorizationException $eAuthorizationException) {
            return $this->notAuthorized();
        }
    }
 
    /**
     * Display the specified resource.
     */
    public function show($order_id)
    {
        try {
            $order = Order::findOrFail($order_id);
            $this->isAble('view', $order); // policy
 
            if ($this->include('user')) {
                $order->load('user');
            }
 
            $order->load('products');
 
            return response()->json(new OrderResource($order), Response::HTTP_OK);
        } catch (ModelNotFoundException $eModelNotFound) {
            return $this->notFound('Order not found');
        } catch (AuthorizationException $eAuthorizationException) {
            return $this->notAuthorized();
        }
    }
 
    /**
     * Store a newly created resource in storage.
     */
    public function store(StoreOrderRequest $request)
    {
        try {
            $order = $this->orderService->createOrderHandleProducts($request);
 
            return response()->json(new OrderResource($order), Response::HTTP_CREATED);
        } catch (QueryException $eQueryException) {
            DB::rollback(); // Rollback transaction on database error
 
            return $this->dbError();
        } catch (Throwable $eTh) {
            DB::rollback(); // Rollback transaction on any other error
 
            return $this->unexpectedError();
        }
    }
 
    /**
     * Update the specified resource in storage.
     * PATCH
     */
    public function update(UpdateOrderRequest $request, $order_id)
    {
        try {
            $order = Order::findOrFail($order_id);
            $this->isAble('update', $order); // policy
            $this->orderService->updateOrderHandleProducts($request, $order);
 
            return response()->json(new OrderResource($order), Response::HTTP_OK);
        } catch (ModelNotFoundException $eModelNotFound) {
            return $this->notFound('Order not found');
        } catch (AuthorizationException $eAuthorizationException) {
            return $this->notAuthorized();
        } catch (QueryException $eQueryException) {
            DB::rollback(); // Rollback transaction on database error
 
            return $this->dbError();
        } catch (Throwable $eTh) {
            DB::rollback(); // Rollback transaction on any other error
 
            return $this->unexpectedError();
        }
    }
 
    /**
     * Replace the specified resource in storage.
     * PUT
     */
    public function replace(ReplaceOrderRequest $request, $order_id)
    {
        try {
            $order = Order::findOrFail($order_id);
            $this->isAble('update', $order); // policy
            $this->orderService->updateOrderHandleProducts($request, $order);
 
            return response()->json(new OrderResource($order), Response::HTTP_OK);
        } catch (ModelNotFoundException $eModelNotFound) {
            return $this->notFound('Order not found');
        } catch (AuthorizationException $eAuthorizationException) {
            return $this->notAuthorized();
        } catch (QueryException $eQueryException) {
            DB::rollback(); // Rollback transaction on database error
 
            return $this->dbError();
        } catch (Throwable $eTh) {
            DB::rollback(); // Rollback transaction on any other error
 
            return $this->unexpectedError();
        }
    }
 
    /**
     * Remove the specified resource from storage.
     */
    public function destroy($order_id)
    {
        try {
            $order = Order::findOrFail($order_id);
            $this->isAble('delete', $order); // policy
            $this->orderService->deleteOrderHandleProducts($order);
 
            return $this->ok('Order deleted successfully', [
                'status' => Response::HTTP_OK,
            ]);
        } catch (ModelNotFoundException $eModelNotFound) {
            return $this->notFound('Order not found');
        } catch (AuthorizationException $eAuthorizationException) {
            return $this->notAuthorized();
        }
    }
}

Look at what we have now: 84 lines of code. Almost 2x shorter.

AFTER:

<?php
 
namespace App\Http\Controllers\Api\V1;
 
use App\Http\Filters\V1\OrderFilter;
use App\Http\Requests\Api\V1\StoreOrderRequest;
use App\Http\Requests\Api\V1\UpdateOrderRequest;
use App\Http\Resources\V1\OrderCollection;
use App\Http\Resources\V1\OrderResource;
use App\Models\Order;
use App\Services\V1\OrdersService;
use Illuminate\Support\Facades\Gate;
 
class OrderController extends ApiController
{
    public function __construct(
        protected OrdersService $orderService
    ) {}
 
    /**
     * Display a listing of the resource.
     */
    public function index(OrderFilter $filter)
    {
        Gate::authorize('view-any', Order::class);
 
        return new OrderCollection(
            Order::filter($filter)->paginate()
        );
    }
 
    /**
     * Display the specified resource.
     */
    public function show(Order $order)
    {
        Gate::authorize('view', $order);
 
        if ($this->include('user')) {
            $order->load('user');
        }
 
        $order->load('products');
 
        return new OrderResource($order);
    }
 
    /**
     * Store a newly created resource in storage.
     */
    public function store(StoreOrderRequest $request)
    {
        Gate::authorize('create', Order::class);
 
        $order = $this->orderService->createOrderHandleProducts($request);
 
        return new OrderResource($order);
    }
 
    /**
     * Update the specified resource in storage.
     * PATCH
     */
    public function update(UpdateOrderRequest $request, Order $order)
    {
        Gate::authorize('update', $order);
 
        $this->orderService->updateOrderHandleProducts($request, $order);
 
        return new OrderResource($order);
    }
 
    /**
     * Remove the specified resource from storage.
     */
    public function destroy(Order $order)
    {
        Gate::authorize('delete', $order);
 
        $this->orderService->deleteOrderHandleProducts($order);
 
        return $this->responseSuccess('Order deleted successfully');
    }
}
Previous: Quick Fix for Test Factories

No comments yet…

avatar
You can use Markdown