zendframework / zend-expressive

PSR-15 middleware in minutes!
BSD 3-Clause "New" or "Revised" License
711 stars 197 forks source link

AbstractAction or ActionPlugin #514

Closed Moln closed 7 years ago

Moln commented 7 years ago

Now, zend-expressive has more components to use in Action

Then the problem is I use them more frequently in Action.

Will create an AbstractAction and ActionPlugin like zend-mvc?

Example:

/**
 * @method \Zend\Log\Logger  logger()
 * @method \Zend\Expressive\Template\TemplateRendererInterface view()
 * @method \HalFactory hal()
 * @method \ZendPloblemDetailFactory ploblem()
 */
class AbstractAction
    public function __call($method, $params)
    {
        $plugin = $this->plugins()->get($method);
        return $plugin($params);
    }
}

class FetchAllBooksAction extends AbstractAction
    public function __construct(SomeRepository $repository) {
        $this->repository = $repository;
    }

    public function __invoke(ServerRequestInterface $request, DelegateInterface $delegate)
    {
        if (/*Some condition*/ true) {
            return $this->ploblem($request, 'message', 403);
        }
        try {
           $result = $this->resource->fetchAll();
        } catch (\Exception $e) {
            $this->logger()->warn($e->getMessage());
            return $this->ploblem($request, $e);
        }
        return $this->hal($result);
    }
}
weierophinney commented 7 years ago

No, we likely will not.

These various classes have very different interfaces and functionality, and thus would not work with a plugin system. They are not defining __invoke(), and many of these have multiple entry points (e.g., the ProblemDetailsResponseFactory defines both createResponse() and createResponseFromThrowable() methods). While we could have a plugin system return the requested instance instead (e.g., $this->problemDetails()->createResponse(/* ... */)), this is still a level of indirection for the user, and requires they know what the plugin is and what methods it exposes; why not just compose it directly?

Additionally, we do not like hiding dependencies; if you need to render HAL, compose the HalResponseFactory; if you need to render via a template, compose a TemplateRendererInterface. Doing so helps the other developers on your team know immediately what the class will be consuming. It also means that if you have too many dependencies, you're more likely to evaluate whether or not the middleware is doing too much, and refactor to multiple middleware with fewer dependencies.

One approach you can take is to create a number of custom abstract action classes for your own use, and a common factory to use with each. This approach simplifies injection of dependencies, while retaining the explicitness of having them passed via constructor. Alternately, use something like the ReflectionBasedAbstractFactory from zend-servicemanager; assuming your dependencies are all typed against classes, this can automate creation of your instances, and eliminate the need of writing a ton of discrete factories.

Moln commented 7 years ago

Get it! Thanks for reply.