How do you handle controllers with partial identical business logic?

I usually reach for repositories only for aggregating different data retrieval methods. I never use them for data manipulation (insert/update/delete).

I know some people do use repositories for everything, but I like separating data manipulation actions into dedicated classes.

There was a thread about a month discussing the concept of repositories:

https://laracasts.com/discuss/channels/laravel/how-to-refactor-large-repositories

Again, I am not saying one approach is better than the other. I am just stating my preference.

I will illustrate how I would approach using repositories and action classes.

Part one - Lights, Camera, Action!

Using your use-case as an example I would have:

• A UserRepository that has dedicated methods for the different queries I want to retrieve User records
• A CreateUserAction for creating User records
• A UpdateUserAction for updating User records
• A RemoveUserAction for deleting User records
• A RestoreUserAction for restoring User records (as I see you have a route for this)

As a side note, if I have specific one-off updates, such as changing a user is_active from true to false, I might have specific action classes for this tasks, such as ActivateUserAction, DeactivateUserAction, and so on.

Considering this my controllers would be:

class LdapUserController extends Controller
{
    public function index(LdapRequest $request, UserRepository $repository)
    {
        $data = $repository->forIndex($request->filters());

        return view('user::pages.users.adldap.index', $data);
    }

    public function create()
    {
        // passing user as a parameter in case you share
        // a form partial template with create and edit views
        return view('user::pages.users.adldap.create', ['user' => new User()]);
    }

    public function store(CreateUserRequest $request, CreateUserAction $action)
    {
        // assuming you are using FormRequests with validation rules
        $action->execute($request->validated());

        return redirect()
            ->route('users.index')
            ->with('success', 'user created');
    }

    public function show(UserRepository $repository, User $user)
    {
        $repository->loadRelations($user);

        return view('user::pages.users.adldap.show', $user);
    }

    public function edit(User $user)
    {
        return view('user::pages.users.adldap.edit', ['user' => $user]);
    }

    public function update(UpdateUserRequest $request, UpdateUserAction $action, User $user)
    {
        // assuming you are using FormRequests with validation rules
        $action->execute($user, $request->validated());

        return redirect()
            ->route('users.index')
            ->with('success', 'user updated');
    }

    public function destroy(RemoveUserAction $action, User $user)
    {
        $action->execute($user);

        return redirect()
            ->route('users.index')
            ->with('success', 'user removed');
    }

    public function restore(RestoreUserAction $action, User $user)
    {
        $action->execute($user);

        return redirect()
            ->route('users.index')
            ->with('success', 'user restored');
    }
}

And similar code for the EloquentUserController.

From your responses I assume the repository class is a familiar concept to you.

A action class, on the other hand is a dedicated class that performs one tasks, for example let's see a naïve implementation of the UpdateUserAction

<?php

namespace App\Actions;

use App\Models\User;

class UpdateUserAction
{
    public function execute(User $user, array $data)
    {
        $user->update($data);
    }
}

Oh my! Just an over-engineered piece of code!

I agree 100% if all you need is to simply call the ->update(...) method on the user instance. If that is the case, keep it simple and call it directly in the controller.

But let's assume this requirements:

• You want to log whenever an user update occurs.
• Your project requirements doesn't allow mass-assignments.
• You want to send an email when an admin record is updated.
• When a password is provided you want to hash it.

Now let's see our updated action class:

<?php

namespace App\Actions;

use App\Mail\AdminWasUpdatedMail;
use App\Models\User;
use Illuminate\Contracts\Hashing\Hasher;
use Illuminate\Contracts\Mail\Mailer;
use Illuminate\Support\Arr;
use Psr\Log\LoggerInterface;

class UpdateUserAction
{
    // these will be auto-injected by the container
    // when you typehint this action class in the controller
    private LoggerInterface $logger;
    private Hasher $hasher;
    private Mailer $mailer;

    public function __construct(LoggerInterface $logger, Hasher $hasher, Mailer $mailer)
    {
        $this->logger = $logger;
        $this->hasher = $hasher;
        $this->mailer = $mailer;
    }

    public function execute(User $user, array $data)
    {
        $user->name = $data['name'];
        $user->email = $data['email'];

        if (Arr::has($data, 'password')) {
            $user->password = $this->hasher->make($data['password']);
        }

        $user->save();

        $this->logger->info('User updated', ['user' => $user->getKey(), 'payload' => $data]);
        
        // ->isAdmin() is a method on the User model
        // that checks is a user is an admin
        if ($user->isAdmin()) {
            $this->mailer->send(new AdminWasUpdatedMail($user));
        }
    }
}

We added a lot of features to our UpdateUserAction without changing anything on both controllers. Hope you see the value on adopting this approach.

It is sure more work (more classes, more code), but they are easier to test, and are easier to extend when you need to add features.

Of course, if your app requirements are simple that a $user->update($request->validated()) is sufficient, keep it simple, and start refactoring only if you need to add features later.

Also, another advantage on not having these actions within the Repository is that different actions can have different requirements, and thus different dependencies.

By having them in dedicated classes we don't need to clutter our controllers or repository with dependencies that will only be used by a single action.

In some projects, I would even wrap the data from the request into a DTO (Data-Transfer Object), so the action receives a payload it knows is type-safe. But I guess that would be out-of-scope, and again, if your app won; t benefit of having DTOs keep it simple.

Part two - Why not jobs?

One thing you might be asking, is why not use job classes instead of action classes. Indeed they are very similar concepts, and if your job class does not implement the ShouldQueue interface it will be run synchronously.

Only difference between them is that in a Laravel job class the dependencies you want injected from the container should be a method dependency on the handle method and not on the constructor. I think code illustrate better what I mean. Let's see the "Job" version of the UpdateUserAction class:

<?php

namespace App\Jobs;

use App\Mail\AdminWasUpdatedMail;
use App\Models\User;
use Illuminate\Contracts\Hashing\Hasher;
use Illuminate\Contracts\Mail\Mailer;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Support\Arr;
use Psr\Log\LoggerInterface;

class UpdateUserAction
{
    use Dispatchable;

    private User $user;
    private array $data;

    public function __construct(User $user, array $data)
    {
        $this->user = $user;
        $this->data = $data;
    }

    public function handle(LoggerInterface $logger, Hasher $hasher, Mailer $mailer)
    {
        $this->user->name = $this->data['name'];
        $this->user->email = $this->data['email'];

        if (Arr::has($this->data, 'password')) {
            $this->user->password = $hasher->make($this->data['password']);
        }

        $this->user->save();

        $logger->info('User updated', [
            'user' => $this->user->getKey(),
            'payload' => $this->data,
        ]);

        if ($this->user->isAdmin()) {
            $mailer->send(new AdminWasUpdatedMail($this->user));
        }
    }
}

As you can see, very similar, just the constructor and "execute/handle" methods parameters are swapped.

To use this you'd need to change your controller action to:

use App\Jobs\UpdateUserAction;

public function update(UpdateUserRequest $request, User $user)
{
    UpdateUserAction::dispatch($user, $request->validated());

    return redirect()
        ->route('users.index')
        ->with('success', 'user updated');
}

One advantage of using Jobs for dedicated classes is that if you want to queue the action processing, the change is much easier.

But I prefer to have actions the first way, with constructor injected dependencies, and if I need some part of the action queued I can dispatch a job from the action. This way the controller doesn't need to know how an action works.

Part three - Who else does that?

Laravel itself adopted a similar pattern with JetStream and Fortify.

If you look at Fortify's source code you will find an Actions folder:

https://github.com/laravel/fortify/tree/1.x/src/Actions

Each action class is very focused and perform an specific task, for example lets look at CompletePasswordReset code:

<?php

namespace Laravel\Fortify\Actions;

use Illuminate\Auth\Events\PasswordReset;
use Illuminate\Contracts\Auth\StatefulGuard;
use Illuminate\Support\Str;

class CompletePasswordReset
{
    /**
     * Complete the password reset process for the given user.
     *
     * @param  \Illuminate\Contracts\Auth\StatefulGuard  $guard
     * @param  mixed  $user
     * @return void
     */
    public function __invoke(StatefulGuard $guard, $user)
    {
        $user->setRememberToken(Str::random(60));

        $user->save();

        event(new PasswordReset($user));

        // $guard->login($user);
    }
}

Permalink: https://github.com/laravel/fortify/blob/66323c4e695b63dc9a13e361d16f0fd9e3ecec92/src/Actions/CompletePasswordReset.php#L1-L28

Then in the controller it delegates to these action classes, for example this CompletePasswordReset is called from the NewPasswordController's store method.

https://github.com/laravel/fortify/blob/66323c4e695b63dc9a13e361d16f0fd9e3ecec92/src/Http/Controllers/NewPasswordController.php#L55-L80

Again, I am not saying one approach is better than the other.

Laravel recently launched a new authentication package called laravel/breeze in response to JetStream criticism with a more traditional controllers/request approach.

Use whatever you think fits better your project's requirements and your dev team's workflow.

I personally like this Action/Repository workflow as I find easier to test, maintain, and add features in the long-run.

As an example this last week a client asked for changing a feature in a old project I didn't touch in more than 6 months. As everything is broken in smaller isolated pieces, it was very easy to find where it needed to be changed, and to remember the concept around how it worked.

In contrast, for this same client we launched a very small project that will be short-lived (a landing page that collects leads for a new building). For this other project I went with "tuck everything in the controller" as the requirements are much simpler, and the project is not expected to have a long life.

Hope it helps. Have a nice day =)

Hey @martinbean , nice idea!

Would love to hear your opinion about my last response, if you have time to do so.

I've learned a lot from your responses throughout the years in Laracasts forums, and on Discord, although I am not an active responder there.

Thanks in advance!

@rodrigo.pedra Thanks! To be honest, I skimmed it on first read with how long it was, but your understanding of repositories is certainly correct: they should only really be used to fetch entities and not the place for all CRUD operations like creating, updating, and deleting entities.

I seldom use repositories myself, but I’d use them where the same storage mechanism used is used throughout the application, as you’d usually bind a single implementation to an interface, and then use the interface for dependency injection.

If there are parts of the application that use different storage mechanisms but should adhere to the same interface, then I’d create a class that then accepts a particular implementation for that storage mechanism. The base class would then just call methods on that particular adapter:

class UserService
{
    protected $store;

    public function __construct(UserStore $store)
    {
        $this->store = $store;
    }

    public function export($id)
    {
        return $this->store->export($id);
    }

    public function restore($id)
    {
        return $this->store->restore($id);
    }

    public function destroy($id)
    {
        return $this->store->destroy($id);
    }
}

So the UserService class just delegates method calls to a particular implementation. You can then inject different implementations based on which controller is wanting the UserService using contextual binding:

$this->app
    ->when(EloquentUserController::class)
    ->needs(UserService::class)
    ->give(function () {
        return new UserService(EloquentUserStore());
    });

$this->app
    ->when(LdapUserController::class)
    ->needs(UserService::class)
    ->give(function () {
        return new UserService(LdapUserStore());
    });

The two controllers can then just extend a base one, because it’ll get the user service with the correct store injected in its constructor:

abstract class BaseUserController extends Controller
{
    protected $service;

    public function __construct(UserService $service)
    {
        $this->service = $service;
    }

    public function export($id)
    {
        $this->service->export($id);
    }

    public function restore($id)
    {
        $this->service->restore($id);
    }

    public function destroy($id)
    {
        $this->service->destroy($id);
    }
}

class EloquentUserController extends BaseUserController
{
    // No body needed
}

class LdapUserController extends BaseUserController
{
    // No body needed
}

Thanks for the response @martinbean !

I usually don't reach for the repositories myself, but I used them when I wanted to share custom queries between several components in an app.

These days I prefer dedicated query classes for larger complicated queries (reports for example), or I usually return a custom eloquent query builder from a overridden newEloquentBuilder(...) method in a model, for stronger type hints in local scopes in an approach similar to the one described here:

https://timacdonald.me/dedicated-eloquent-model-query-builders/

But I thought using a repository would fit the issue described by OP of needing to share some logic between two similar controllers.

But after reading your approach of having a generic service class that accepts a specific store implementation by leveraging the container, I was like: how come I never thought of it before !?

It indeed looks like a better, and more flexible solution.

The conclusion I get is that I still have a lot to learn, and am glad I am able to share knowledge from folks like you.

Thanks for your time and feedback, and have a nice day =)

You're welcome @lazos99 !

By reading @martinbean 's response to my feedback request I think you could consider using a service class similar to what he described there.

EDIT After reading your more recent response, I think you already figured out how to improve you solution.