Support the ongoing development of Laravel.io →
Authentication Architecture
Last updated 1 year ago.
0

I don't personally feel like redirects should be inside of service classes. As a personal preference, I like my services to be "black boxes" - they should do one thing and do them well. In your case the redirect is coupled to the service and would make your service unworkable in non-http situations. For example, how would you use the service to authenticate a single command line action where you might want credentials passed as command arguments?

The purpose of a controller is to direct application flow - that includes handling redirects based on the results of a service call.

Also, if you have an aggregate service (one that does many things) - remember that your service can inject and use other services if necessary. Every part of a system that you can make smaller and less coupled is a win for the architecture of your project.

Last updated 1 year ago.
0

I agree with abbajbryant, your service shouldn't know anything about the http layer, or any layer above it (the object owner).

You are almost there though with your current implementation. Using a listener or the Delegation Pattern is nice way to handle this but it needs to be implemented in a way that is flexible with any object that instantiates it. Therefor we can't rely on $listener->actionFails() returning an object that has a method called withErrors (unless specifically docblocked because this is not a static language). You could have Listener->actionFails expect an $errors parameter which then does what it wants with those errors e.g. pass it to a redirectResponse.

E.g. Controller

<?php
    public function postSignin (){
        //...
        return $this->authenticator->attempt($input , $remember, $this);
    }

    public function actionFails(MessageBag $errors)
    {
        return $this->redirector->route('route.name')-withError($errors);
    }

Service

<?php
class Authenticator {
    //....
    public function attempt( $credentials , $remember, ListenerInterface $listener )
    {
        if ( ! $this->signin_validation->isValid( $credentials )){
            $messages = $this->signinValidation->getMessages();
            return $listener->actionFails($messages);
        }
        //...
    }

}

One last thing is your listening interface is very generic. Perhaps you need an AuthenticationListenerInterface instead. Instead of implementing the generic actionFails method it could implement authenticationFails. This way each of your services can gain more control over what each listener can listen for and what parameters are passed to those listener methods.

So to answer your questions

Should redirects be in the service or the controller ? Controller

Should I have a different ListenerInterface for each service ? I would, but it depends on the needs of the system.

Is there any other misunderstanding of the pattern in the code ? Your pretty close. It just needs some refining.

Last updated 1 year ago.
0

Thanks damienadermann & abbajbryant , Nice explanation :D

Last updated 1 year ago.
0

Sign in to participate in this thread!

Eventy

Your banner here too?

reshadman reshadman Joined 15 Feb 2014

Moderators

We'd like to thank these amazing companies for supporting us

Your logo here?

Laravel.io

The Laravel portal for problem solving, knowledge sharing and community building.

© 2024 Laravel.io - All rights reserved.