Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support routing based on Accept Header #40

Open
rdohms opened this issue May 25, 2020 · 6 comments
Open

Support routing based on Accept Header #40

rdohms opened this issue May 25, 2020 · 6 comments
Labels
Improvement Improvement of existing feature New Feature New feature or request

Comments

@rdohms
Copy link
Contributor

rdohms commented May 25, 2020

We should be able to control routing based on a custom accept header.

So if we have
PUT /resource -> CreateResource
we would like to configure
PUT /resource + application/vnd.v2+json -> CreateResourceV2

Let's brainstorm possible solutions.

@rdohms rdohms added Improvement Improvement of existing feature New Feature New feature or request labels May 25, 2020
@lcobucci
Copy link
Member

lcobucci commented May 27, 2020

My idea on this is to have something a bit broader to define the strategy of choosing the command for a specific request.

Roughly speaking we would provide (on chimera/foundation) an interface (and perhaps some implementations) that would look like this:

namespace Chimera\Foundation;

interface MessageResolutionStrategy // not necessarily a good name but bear with me
{
    public function resolve(RequestInterface $request, array $arguments = []): string
}

An instance of that interface would be injected to the request handlers' actions, which would resolve things properly before creating the command/query.

That instance should be resolved via the DI container using the strategy class name as identifier - and failing if strategy isn't mapped.

This gives enough flexibility to cover your scenario and others (e.g. a PATCH request triggering different commands based on the payload - which was requested by @dsantang).


Your example would be mapped like this (combined some other ideas I have):

namespace MyCompany\MyApp;

use Chimera\Mapping;

/**
 * @Mapping\Routing\ExecuteEndpoint(
 *     "/blah",
 *     commandStrategy=AcceptVersionPromotion::class,
 *     commandStrategyArguments={
 *         "*": CreateResource::class,
 *         "application/vnd.v2+json": CreateResourceV2::class
 *      }
 * )
 */
final class CreateResourceHandler
{
    /** @Mapping\ServiceBus\CommandHandler */ // <~ yes, this isn't possible atm but it's something I'm brewing
    public function handle(CreateResource $command): void
    {
    }

    /** @Mapping\ServiceBus\CommandHandler */ // <~ yes, this isn't possible atm but it's something I'm brewing
    public function handleNewStuff(CreateResourceV2 $command): void
    {
    }
}

@rdohms
Copy link
Contributor Author

rdohms commented May 27, 2020

Overall I think its makes perfect sense.
One point would be that maybe we want separate handlers, but I presume that doing this the way we do now via DI not annotations would still be valid with just the message picking being affected, right?

@lcobucci
Copy link
Member

lcobucci commented May 27, 2020

@rdohms you can still have separate handlers (via annotations or DI tags). I suggested that snippet because it's shorter.

(side note) I observed that folks are finding it much easier to understand things when using annotations. Annotations combined with the DI auto wiring stuff reduced quite a lot of confusion of newcomers (hence me using it here).

@rdohms
Copy link
Contributor Author

rdohms commented Jun 11, 2020

Proposal

  1. Introduce a MessageResolutionStrategy interface
namespace Chimera\Foundation;

interface MessageResolutionStrategy
{
    public function resolve(RequestInterface $request, array $arguments = []): string
}
  1. In the foundation repo, change ExecuteQuery and ExecuteCommand to accept a $strategy parameter.

Here, when fetch or execute are called, and query or command are null (not set) we would first call the resolve method

Open points

  1. How do we deal with argument configurations in xml format, versus annotations
  2. How do we get the needed context ($request, $arguments) into the Execute* objects
  3. We can start with concrete implementations in usabilla-land and toss them back at Chimera, or start with default class for version handling, then we would need to figure out which repo to put this in.

Maybe the answer to 1/2 is that we define a separate service which is a configured Strategy and that is what gets injected, this way it can get the arguments already pre-set and only need the request at runtime? with annotations we can mimic this but the syntax would be simpler.

Context

The DI repo hooks this all together, for example here you can see a FetchOnly (used by Enrichment): https://github.com/chimeraphp/di-symfony/blob/master/src/Routing/Expressive/RegisterServices.php#L471, https://github.com/chimeraphp/di-symfony/blob/master/src/Routing/Expressive/RegisterServices.php#L421

@lcobucci
Copy link
Member

lcobucci commented Jun 24, 2020

Just to clarify things a bit... this is not about accept-based routing but rather have a condition-based message (command/query) execution. Which boils down to simply making things easier to our users.

This doesn't block users from implementing this logic themselves on plain RequestHandler. If there's any urgency on your end to implement, I'd highly recommend you to go with that solution instead of implementing a lot of things on top of Chimera which might not be compatible with how we move this forward.

To create a route like that you would have a class like this:

<?php
declare(strict_types=1);

namespace Me\MyApp\Context;

use Chimera\Mapping as Chimera;
use Chimera\MessageCreator;
use Chimera\Routing\HttpRequest;
use Chimera\ServiceBus;
use Psr\Http\Message\ResponseFactoryInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;
use function strpos;

/**
 * @Chimera\Routing\SimpleEndpoint("/my-header-based-stuff", name="doSomething", methods={"POST"})
 */
final class DoSomethingEndpoint implements RequestHandlerInterface
{
    private ServiceBus $commandBus;
    private MessageCreator $messageCreator;
    private ResponseFactoryInterface $responseFactory;

    public function __construct(
        ServiceBus $commandBus,
        MessageCreator $messageCreator,
        ResponseFactoryInterface $responseFactory
    ) {
        $this->commandBus = $commandBus;
        $this->messageCreator = $messageCreator;
        $this->responseFactory = $responseFactory;
    }

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        $this->commandBus->handle(
            $this->messageCreator->create(
                $this->resolveMessage($request),
                new HttpRequest($request)
            )
        );

        return $this->responseFactory->createResponse(204);
    }

    private function resolveMessage(ServerRequestInterface $request): string
    {
        if (strpos($request->getHeaderLine('Accept'), 'vnd.v2+json') !== false) {
            return DoSomethingV2::class;
        }

        return DoSomething::class;
    }
}

@rdohms
Copy link
Contributor Author

rdohms commented Feb 7, 2021

BTW, we will no longer be supporting this in our applications, so likely also not investing time into the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Improvement of existing feature New Feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants