I'm creating api in Lumen framework and recently i read about DRY and service layer. To this day i didn't use any of these in my code and all logic was in the controllers. So i would like to start using it, but i have some problems with it.
This is part of my controller (UsersController.php), because whole code is too long.
<?php
namespace App\Http\Controllers;
use App\User;
use Illuminate\Http\Request;
class UsersController extends Controller
{
private $request;
public function __construct(Request $request) {
$this->request = $request;
}
public function destroy($id) {
$user = User::find($id);
if (!$user) {
return response()->json([
'error' => 'User not found'
], 404);
}
if ($user->role === 'admin') {
return response()->json([
'error' => 'You cant edit admin'
], 403);
}
$user->delete();
return response()->json([], 204);
}
}
After looking at this code i tried to change 2 things.
UserService.php
<?php
namespace App\Services;
use App\User;
class UserService
{
public function getUserById($id)
{
$user = User::find($id);
if (!$user) {
return response()->json([
'error' => 'User not found'
], 404);
}
if ($user->role === 'admin') {
return response()->json([
'error' => 'You cant edit admin'
], 403);
}
return $user;
}
}
Modified UsersController.php/destroy
public function destroy($id) {
$user = $this->userService->getUserById($id);
$user->delete(); // not working because sometimes it can return json response
return response()->json([], 204);
}
ResponderService.php
<?php
namespace App\Services;
class ResponderService
{
private function base($data, $status_code)
{
$data['status_code'] = $status_code;
return response()->json($data, $status_code);
}
public function error($message, $status_code)
{
$data['error'] = $message;
$data['status'] = 'error';
$this->base($data, $status_code);
}
}
I read about Repositories too, but i dont think this pattern would be good in my project. If you have other suggestions that could be improved in controller code, I'm open to them.
I don't see any issue with using Exceptions for your scenario.
<?php
namespace App\Services;
use App\User;
class UserService
{
public function getUserById($id)
{
$user = User::find($id);
if (!$user) {
throw UserNotFoundException('User not found');
}
if ($user->role === 'admin') {
throw EditAdminException("You can't edit admin.");
}
return $user;
}
}
Where those exceptions are your own custom exceptions defined in app\Exception if you want. Then the getUserById() method can only return a User otherwise an exception bubbles up and returns a JSON response to the client.
Laravel also already has a simple way to handle the first exception. You could do this:
<?php
namespace App\Services;
use App\User;
class UserService
{
public function getUserById($id)
{
$user = User::findOrFail($id);
if ($user->role === 'admin') {
throw EditAdminException("You can't edit admin.");
}
return $user;
}
}
And Laravel will handle throwing a Illuminate\Database\Eloquent\ModelNotFoundException if a User can't be found.
This way you don't need to worry about creating a ResponderService for what Exceptions can already do for you.
If you want to standardize on the responses of your resources you could leverage Eloquent Resources which work as a transformation layer for your API: https://laravel.com/docs/5.7/eloquent-resources
Lastly, if you find you're deleting a resource from more than one place and don't want to duplicate the response, you could put the response inside of an Event: https://laravel.com/docs/5.7/eloquent#events
The documentation shows the convoluted way of handling events, but I personally would only do it once your model starts to feel bloated.
You can do this as a simpler alternative to creating an Event and Observer class:
public static function boot()
{
parent::boot();
static::deleted(function ($model) {
return response()->json([], 204);
});
}
That method just goes on your User model. And the way I found that by the way is by looking inside the HasEvents trait on the Eloquent\Model.
Now, all of that said, I would actually put all of the deletion logic inside your UserService and rename the method from getUserById to deleteById. The alternative is a little weird because you're saying you don't want to be able to get a user by id if it's an admin.
Really what you're trying to do is encapsulate the logic of deleting a user, so just move it all into the method on the service, or better yet just use the delete event on the model and put all of the logic there. That way you don't even have to introduce a service yet.
Edit
Based on your comment below I think you may be misunderstanding how to user Exceptions in Laravel.
In a fresh Laravel project there is a class in app\Exeptions\Handler which catches all unhandled exceptions in your app. That class first checks if the Exception is a ModelNotFoundException and then returns a json response.
Else, it passes the caught Exception to the render method of it's parent.
So basically when you want to create a custom exception you simply make a class that extends Exception and implementes a handle method.
Here's an example Exception class:
<?php
namespace App\Exceptions;
use Exception;
class TicketNotPayableException extends Exception
{
/**
* Render an exception into an HTTP response.
*
* @param \Illuminate\Http\Request $request
* @param \Exception $exception
* @return \Illuminate\Http\Response
*/
public function render()
{
return response()->json([
'errors' => [
[
'title' => 'Ticket Not Payable Exception',
'description' =>
'This ticket has already been paid.'
],
],
'status' => '409'
], 409);
}
}
Now the response is totally reusable and I don't need a bunch of try-catch blocks in my code. Laravel's Exception Handler will catch it, and call the render method.
So, if I wanted to encapsulate the logic of paying for a ticket inside of a service I just have to throw App\Exceptions\TicketNotPayableException; and then my controller only needs to do something like: $ticketPaymentService->pay($ticket); and there's no need for a try-catch. If the exception gets thrown, it'll bubble up, get caught by the handler, and the render method will be called, which will return the appropriate JSON response - no need for a Responder.