How does Relay fit in with Radar and Spark?

[shadowhand]

UPDATE FOR LATER READERS: the Pipeline project was renamed to Relay shortly after announcement.

Taking our conversation from email to Github so that others can have some input...

On Jun 5, 2015, at 12:57, Woody Gilk woody.gilk@gmail.com wrote:

What's your vision for PipelinePHP? Do you want to roll Pipeline into Spark and share collaboration on it, or are you thinking that Pipeline should be the nexus between Spark and Radar?

I see Pipeline as the nexus between the two, and (one hopes) a wide range of other middleware systems. The idea would be, again one hopes, for a collection of middleware packages to spring up that follow the same signature and expectations. Then Radar, and Spark, and whatever-else-comes can say they are Pipeline compliant.

We're nearly ready to start prototyping with Spark and would like to get this sorted out soon so that we can collaborate more closely and work on core ADR stuff together, rather than duplicating efforts.

/me nods

As far as core ADR stuff, Pipeline might provide a good non-project-specific nexus, or maybe not. There's a case for publishing separate package that is not attached to any particular project.

Regardless, the fact that the middleware dispatching is separated out means each project gets to its own .env setup (before Pipeline), container setup (which creates Pipeline and other objects), and middleware objects (which Pipeline executes). Those surrounding pieces are where the fun and preferences live.

You've already talked about this, but I'll reiterate. I think the remaining pieces for an ADR set of interfaces are:

• An Action handler signature. It strikes me that this ought to be "just another middleware." It takes $request, $response, $next, and returns a $response. As such, it might not need an interface per se, but it would need a set of expectations on how to pick the classes for the action. Information from the routing phase might be passed as ServerRequest attributes: e.g., ['adr:input' => 'InputClassName', 'adr:domain' => 'DomainClassName', 'adr:responder' => 'ResponderClassName']. Actions would expect those to be there, and to use them appropriately (whatever that means).
• I don't know that a common Route interface would be needed, so long as the Action-related specifics of the route can be passed through to a later Action handler. Tough to type hint with a standard middleware signature in place. Alternatively, an ActionDescriptor might be good, and it would carry the input/domain/responder information, possibly along with the means to create those objects for the Action. It would still have to be passed via the $request as an attribute, maybe "adr:descriptor".
• A common Input interface sounds good. __invoke(ServerRequest $request) would be just fine, which you have in place. The return would have to be an array of params suitable for call_user_func_array().
• I don't know that a common Domain interface is really needed. call_user_func_array($domain, $input($request)) seems to cover every possible case.
• A common Responder interface is a little tricky. Although I like the idea of __invoke($request, $response, $payload = null), I have to wonder about the Payload interface at that point. Do we require one, or do we just say "$payload is whatever comes back from your Domain"?

Actual implementations of these, might go well in their own projects. That would mean an interface-only package, and then separate projects that implement them according to the desires of the implementors.

(/me wipes brow)

None of that is set in stone, obviously, but it seems like a good start combined with your spark/adr work.

Over to you!

[pmjones]

:+1:

[pmjones]

I don't know that a common Domain interface is really needed.

Well, other than that it be invokable/callable.

[dolfelt]

Information from the routing phase might be passed as ServerRequest attributes: e.g., ['adr:input' => 'InputClassName', 'adr:domain' => 'DomainClassName', 'adr:responder' => 'ResponderClassName']. Actions would expect those to be there, and to use them appropriately (whatever that means).

Do we not like the idea of using a RouteInterface and injecting it into the ActionHandlerMiddleware? The application determines which route is being used before the action is called. Is that too limiting?

[pmjones]

I think the Route gets returned too late for that. It would have to be passed to the action handler in some way. And then the action handler would have to actually create the objects specified in the route. Thus, my initial inclination toward an ActionDescriptor (or whatever) that has an injector/factory/builder, and gets information fed into it from the route about the input/domain/responder specifications. That ActionDescriptor then gets shoved into the Request as a uniquely "well-known" attribute for a later Action handler to extract and use to create the necessary objects. (Hope that made sense.)

[dolfelt]

@pmjones I guess I only suggest that since it's how it's happening now. The application resolves the route before the action handler is called.

What if we injected the RouteInterface / ActionDescriptor in the constructor of the ActionHandler class? This would allow use to use __invoke to match the Middleware signature, while also getting the route with all the Input, Domain and Responder information.

[pmjones]

/me ponders

That would also mean injecting the ActionDescriptor (or whatever) into the Routing/Router middle ware piece, as a shared object between the Routing/Router and the Action middleware. Starting to sound like a service of sorts there. Do we care about changing the state of shared objects in that way?

[dolfelt]

We wouldn't have to share the route necessarily. The flow I think could work is as follows.

1. Build Routes (attach custom domains/inputs/etc)
2. Router determines route from request
3. Application resolves the route information into an ActionDescriptor that contains the Input, Domain and Responder. If none are specified in the route, the defaults are used, so that the ActionHandler doesn't ever have to care about defaults.
4. Application injects ActionDescriptor into the ActionHandler and adds it to the bottom of the middleware stack.
5. Middleware stack is executed.

I'm sure I'm missing something in there, but from my thought process and the way Spark works, that flow seems to solve all the issues and allows ActionHandlers to just be middleware that follows the PipelinePHP interface.

[pmjones]

Thanks for that workflow! I think one disconnect with my way of thinking is that the Router IMO is "just another middleware." The Router middleware has no special relationship with the application as a whole, it just happens to get executed in the queue before the Action middleware. For example:

1. Load up dot-env values (if any)
2. Build/configure the Container or Injector (if any)
3. Add middleware to the queue (ResponseSender, ExceptionHandler, RoutingHandler, ActionHandler)
4. Add routes to the Router, which is shared with the RoutingHandler
5. Execute the middleware stack.

That's what I'm getting at when I say that the RoutingHandler needs to be able to communicate with the ActionHandler in some way; it's "just another middleware" and so is not special.

Hope that makes sense.

(p.s. I am beginning to really dislike the word "middleware." I so wish whoever had come up with that had just stuck with "decorator." Well, whatever.)

[dolfelt]

Now I'm seeing where you're coming from. How do you suggest resolving the Domain, Input, Responder classes in this flow? If the RouteHandler determines that information, would you have to put the Resolver/Injector inside that class?

My initial reaction to this is to think that the router should be outside the stack, but I suppose I could see advantages to this.

[pmjones]

Now I'm seeing where you're coming from.

Sweet.

How do you suggest resolving the Domain, Input, Responder classes in this flow? If the RouteHandler determines that information, would you have to put the Resolver/Injector inside that class?

Yeah, this is the tricky part. By way of example, this is what happens in Radar:

• The RoutingHandler is constructed with the Router map. When invoked, the RoutingHandler does all the routing work, then stuffs the Route object into a Request attribute named "radar/adr:route" and calls $next(); the ActionHandler is invoked later in the queue.
• The ActionHandler is constructed with the InjectionFactory from the DI container. When invoked, the ActionHandler reads the "radar/adr:route" Request attribute to get the Route object, then uses the InjectionFactory to create the input/domain/responder objects specified by that Route, and invokes them appropriately.

While I'm not enthusiastic about using a Request attribute to convey the routing information to the ActionHandler, it seemed the most obvious and straightforward approach.

[dolfelt]

Sounds like a good approach to me. I initially was inclined to keep the Injector out of the Handlers, but if you think that doesn't conflict with a service oriented architecture, then who am I to argue? :smile:

I do like the single attribute instead of multiple. Now what do we call it?

[shadowhand]

I'm still opposed to having the Injector exist anywhere except the Application. Auryn is strictly not a service locator and we shouldn't attempt to use it that way, nor should we use any service locator.

[shadowhand]

From my perspective, you have to start from the position that service location doesn't exist and that there is no way to "store" objects or "fetch" them later. Radar doesn't do this at all and it's one of my biggest issues with it.

[pmjones]

I initially was inclined to keep the Injector out of the Handlers, but if you think that doesn't conflict with a service oriented architecture, then who am I to argue?

Let's not overstate the case. ;-)

For the record, I'm not super-happy about dropping the Aura.Di InjectorFactory into the ActionHandler. The one thing that soothes me is that it's "only" a factory, and (@shadowhand take note) not a service locator or container, so I can avert my eyes a bit when I see it. I'm with @shadowhand on avoiding putting containers anywhere "inside" the system.

I do like the single attribute instead of multiple. Now what do we call it?

Whatever interface package name we come up with, I'd suggest that. The Aura naming convention where global keys are involved is "package/vendor:varname", but that may not be suitable here.

[pmjones]

you have to start from the position that service location doesn't exist and that there is no way to "store" objects or "fetch" them later

Agreed -- not sure why you think Radar does this, or where, but I may be too close to it to see it.

[dolfelt]

@shadowhand Is it possible to have both? In order to resolve the Domain/Input/Responder in the RouteHandler / ActionHandler, Auryn would need to be injected, just like we are doing in the Router currently.

you have to start from the position that service location doesn't exist and that there is no way to "store" objects or "fetch" them later

@shadowhand Are you suggesting that this method is using the ServerRequest object to store and fetch objects?

[shadowhand]

the ActionHandler reads the "radar/adr:route" Request attribute

Unless I misunderstand service location, this is clearly a service location.

[pmjones]

(/me thinks)

@shadowhand So that I'm clear, your position is that the passing of an object via the Request attributes counts as service location?

[dolfelt]

I think there are two ways to do it. Injector goes into the Handler, or the Router comes out of the stack. I'm not sure we can have both.

Pipeline would easily support both methods as it is right now.

[shadowhand]

@pmjones on second read, I think I had a knee-jerk reaction... I'll answer your question first, and then explain myself better:

your position is that the passing of an object via the Request attributes counts as service location?

I didn't initially realize it was with the Request, which is where the "knee-jerk" comes in. That said, I am still opposed to it, because the route is an implementation detail, not a property of HTTP.

If anything, I would suggest that the ActionHandlerInterface receive the RouteInterface as a collaborator:

class ActionHandler implements ActionHandlerInterface
{
    public function __construct(RouteInterface $route)
    {
        $this->route = $route;
    }

    public function __invoke($request, $response, $next)
    {
        $domain = $di_create_instance($this->route->getDomain());
        ...
    }

The implication here is that the middleware stack is created in a lazy fashion, so that the route can be injected. Thoughts?

[pmjones]

the route is an implementation detail, not a property of HTTP

Granted. Having said that, the PSR-7 ServerRequest $attributes property is for non-HTTP elements associated with the request. The original intent IIRC was to pass things like path-info values discovered by routing. Passing other request-related values seems a reasonable use.

I would suggest that the ActionHandlerInterface receive the RouteInterface as a collaborator

Which means it needs to be constructed with the discovered Route, which means the Route needs to be discovered before the ActionHandler is constructed, which means the RouterHandler needs to construct the ActionHandler perhaps via a factory ... ?

[shadowhand]

which means the RouterHandler needs to construct the ActionHandler perhaps via a factory ... ?

Or that middleware dispatcher would construct the middleware as it works through the stack?

[shadowhand]

@pmjones though I do see your point about ServerRequest::$attributes so perhaps that usage is totally acceptable in this situation. If we apply Occam's Razor, using the existing request interface works equally well and doesn't require assumptions about when a middleware handler is created.

tl;dr: I apologize for the diversion, carry on. 8-)

[pmjones]

Or that middleware dispatcher would construct the middleware as it works through the stack?

(/me nods)

It does (or "can do") that now via the $resolver, but it does so outside of any particular middleware, which leaves two problems: 1. How to get the Route out of the RouteHandler to the Resolver, and 2. When-and-how to specify arguments to pass to the $resolver mechanism.

Compare that with passing a Route object as a Request attribute for the ActionHandler to retrieve later.

[pmjones]

I apologize for the diversion, carry on.

Not at all.

... where does that leave us, BTW? ;-)

[shadowhand]

Going back to the topic before I derailed us... the discussion was about where to resolve the Domain, Input, and Responder (DIR). I see nothing wrong with allowing the ActionHandler to use a factory to create instances of the specs attached to the route. So long as the injector lives solely outside of the DIR, I will consider the architecture to be sound.

[dolfelt]

So do we want this project to contain an ActionDescriptorInterface for that object that needs to be passed to the ActionHandler?

[pmjones]

Hi again,

I've been working through the Spark interfaces to find the commonalities with Radar, and then extract them. Here's what I have so far (and I think this reiterates some of what I've said previously). None of this is in stone, of course, and I offer it here for feedback.

Action

I think @shadowhand's intuition toward a RouteInterface to retain the action specifications points in the right direction. Having worked it over some, I have concluded that what we want is not a RouteInterface per se, but an Action value object that carries the input/domain/responder specifications as extracted from elsewhere (likely a route). The Action VO is created by an earlier middleware using an ActionFactory, then attached to a request attribute for a later middleware to use.

Likewise, I think @shadowhand's intuition toward an ActionInterface and an ActionTrait also points in the right direction. Again, having worked it over some, it seems that an ActionHandler object proper, that is not itself a middleware piece, is the right way to go here. We can standardize the action work, without necessarily specifying how the middleware piece itself must be built. The middleware piece in charge of performing the action can then either extend, or compose, the ActionHandler. (UPDATE: the middleware piece would likely be project-specific, not something in this "standard package.")

You can see those four elements (Action, ActionFactory, ActionHandler, Middleware) here: https://gist.github.com/pmjones/1eca32131759ef600051 (UPDATE: The logic for these is derived from Radar.)

Incidentally, the ActionHander needs to create or resolve objects from the input/domain/responder specifications. That occurs via the $resolver, and is typehinted as a callable to allow wide flexibility.

Input, Domain, Responder

After that, it doesn't seem to me that we need (or even can fairly describe) interfaces for input, domain, and responder.

For example, when it comes to the domain, the actual domain work is outside the scope of ADR. The domain work hould be completely independent. The full extent of what we need is a plain old PHP callable specification that refers to some domain class and method, or a domain invokable object. It can be invoked used call_user_func_array().

Tied to that is the input specification. As long as it takes a ServerRequest and returns an array of params for call_user_func_array(), it can be an anonymous function for all we care. If we really wanted to, we could type-check the returned resolution of the input specification ...

$input = $this->resolve($action->getInput());
if (! $input instanceof InputInterface) {
    throw UnexpectedValueException();
}

... but I don't see that as being that big a deal.

As far as a standard InputTrait, the first thing that jumped out at me from the Spark implementation is that it's doing validation. IMO that belongs in the domain; let the domain validate the inputs, not the user interface. Aside from that, the merging of values from various request locations seems fine to me as a default; Radar does something similar. Providing a default Input object proper, instead of a trait, seems the more reasonable approach to me.

Finally, the responder. We know it needs a request and a response, but for the payload (if any) I don't think we can typehint to anything useful. The payload is going to come from the domain. The domain should not be dependent on anything from the user interface layer, which is what we're working with here. That alone makes it hard to typehint. Likewise, we may not even need a payload for some responders, as some routing may point directly to a responder without a domain payload. Even if we did this ...

interface Responder
{
    public function __invoke(Request $request, Response $response, $payload = null);
}

... it would prevent implementors from typehinting to the payload of their choice. (Same problem if we leave off $payload entirely; implementors adding a $payload param would get strictness errors.)

In all, I think saying "the ActionHandler has these expectations of input and responder" would be sufficient. For example:

// input, returns an array of params suitable for call_user_func_array()
function ($request);

// responder, returns a response
function ($request, $response, $payload = null);

That would allow implementors to typehint, or leave out entirely, the $payload as they wish.

Finally, to reiterate, the domain is specified with a callable, and is invoked with call_user_func_array($domain, $input($request))).

/me wipes brow

Over to you!

[pmjones]

As a followup, both Radar and Spark should be able to use the Action* classes as-is via composer. The classes should probably be in a separate package so they don't "identify" with either Radar or Spark; hell, they probably ought to be separate from Relay, since the ActionHandler doesn't require a particular middleware signature.

[dolfelt]

@pmjones Where do you think something like this should live? We already separate out our handler here, as well as other Spark dependencies. Also, our handler operates without an injector, as we prefer to inject everything at the application level. I'm sure there are merits to each.

[pmjones]

Where do you think something like this should live?

I say it ought to be in a separate project as a standalone library. That keeps it from being "a particular framework's handler." (It's the same idea as having Relay be a standalone library.)

our handler operates without an injector, as we prefer to inject everything at the application level

I confess I don't see how the Input/Domain/Responder objects are being instantiated there; does the Route object deal with that?

[pmjones]

On further consideration, the $resolver could default to null as an optional injection, and the resolve() method could just return the spec directly if no $resolver has been injected. I have updated the gist to allow for that: https://gist.github.com/pmjones/1eca32131759ef600051

[dolfelt]

@pmjones I'm starting to add in Relay to Spark, and I'm running into some issues. It's probably just my brain thinking about it one way, but what do you see the order being of these middlewares?

I'm thinking it will be ExceptionHandler --> {all other middleware} --> RouteHandler --> ActionHandler --> ResponseSender.

Does that make sense?

[pmjones]

ResponseSender, ExceptionHandler, Router, Action. Cf. https://github.com/relayphp/Relay.Middleware for the reasoning. To wit:

The ResponseSender is intended to go at the top of the Relay queue, so that it is the middleware with the last opportunity to do something with the returned response. ... The ExceptionHandler is intended to go near the top of the Relay queue, but after the ResponseSender, so that the ResponseSender can then send the returned $response.

[dolfelt]

Well silly me. I should have read through that whole document first. :smiley:

[pmjones]

@shadowhand @dolfelt et al: Any further thoughts on this, especially the extraction of the Action-specific elements?

[dolfelt]

@pmjones I've done some work on it and got it to what I think it a functional state. https://github.com/sparkphp/Spark/pull/17

[pmjones]

I like it. I think your work proves the concept of Relay and its middleware as a reusable package. I see that you've got your own ExceptionHandler, but it totally makes sense given that you're trying to present different/extended information. So as far as Relay itself is concerned I think https://github.com/sparkphp/Spark/pull/17 is a success.

I opine that you could go a little further, if you had the inclination, by doing these things in reference to the action-related gist above (https://gist.github.com/pmjones/1eca32131759ef600051):

• Have the Spark\Router retain Action objects instead of Spark\Route objects; they carry the exact same data.
• You could then have the Spark\ActionHandler either extend or compose the ActionHandler. The extended or composed class would need a $resolver based on Auryn, which I expect would be trivial. Something as straightforward as the following might work:

class AurynResolver
{
    public function __construct(Injector $injector)
    {
        $this->injector = $injector;
    }

    public function __invoke($spec)
    {
        return $this->injector->make($spec);
    }
}

So you would pass the AurynResolver instance to the Spark\ActionHandler constructor.

The upshot is that you can remove even more code from Spark, and depend on the Action stuff from the gist (which itself would need to become yet another separate package, probably not something in Relay). FWIW, I have a Radar branch using the Action stuff from the gist, and it seems to be working fine.

Again, the action-related stuff is over-and-above the Relay-related work, and may not be related to your goals.

Hope all that made sense.

[pmjones]

I have converted the action-related gist above to a separate repo, tentatively Elemental: https://github.com/elementalphp/Elemental.Elemental

(Apparently the vendor name "elemental" is already taken on Packagist, so I'll have to change the name; the purpose was to fully extract the functionality and test it separately.)

[pmjones]

Renamed to Arbiter, hopefully to indicate an arbitrator/mediator: https://github.com/arbiterphp/Arbiter.Arbiter

[pmjones]

Oh and @dolfelt @shadowhand I have successfully integrated Arbiter into Radar per a commit this morning: https://github.com/radarphp/Radar.Adr/commit/cfe4eade52a1794be7af7d613e405d34275852cb

[pmjones]

That leaves us with Input and Responder elements.

Regarding input, I asserted above:

Providing a default Input object proper, instead of a trait, seems the more reasonable approach to me.

To get to that, It might be fair to merge the default Radar input object and the Spark trait (without making allowance for validation) into an Arbiter input object of some sort.

Responders are a tougher nut to crack. For the reasons I note above, I think the best we could do would be to come up with a responder package (probably yet another separate package) that type hints against _Aura\PayloadInterface\PayloadInterface (or some other interface-only package) for its payloads. That avoids having the payload type embedded in the responder package, and thus tying domain-related elements to a UI-relate package. Then we could collect the Radar responder and accepts-aware interface, and the two Spark responders, into that separate package, on which Radar and Spark could then depend. But I'm really not sure about that one; responders are a heavy-lifting element in ADR, and they do a ton of payload-specific work, which may make them harder to generalize.

At that point I think we'll have extracted everything possible, leaving only the project-specific elements that go around those things: config/setup, routing, etc.

@dolfelt @shadowhand etc, thoughts?

[dolfelt]

@pmjones Finally got time to implement the new Relay stuff into my middleware branch (https://github.com/sparkphp/Spark/pull/17).

I'm still debating the need for all these packages. Arbiter seems to make sense, but trying to create all these separate packages for defaults for the Domain, Input, Responder stuff seems overly complicated and unnecessary. Spark will probably have sane defaults for Input and Responder, but I would expect most people to just write their own. Also, the less requirements we have for this kind of stuff the better.

I'd also like to find a way to not require a certain payload to come back. Maybe one for the default responder, but again, extending it to use whatever payload you want should be easy.

For the responder portion, I'm leaning towards no. I don't want to limit the payload that needs to come back. It should be up to the application to decide that.

For the Input portion, I'm on the fence. I don't think it really matters, but could be good to include something simple in the Arbiter package.

P.S. I might give a crack at adding the Arbiter package later today. Looks simple enough.

[pmjones]

Finally got time to implement the new Relay stuff into my middleware branch

Nice.

For the responder portion, I'm leaning towards no.

As am I, mostly because I can't see any way clear to the goal of not being dependent on a particular payload type.

For the Input portion, I'm on the fence. I don't think it really matters, but could be good to include something simple in the Arbiter package.

I'm leaning toward "no" here as well. The input to the Domain is going to be very Domain-dependent (same way output from the Domain is Domain-dependent) and although a basic no-frills Input object is possible I think it would serve little purpose other than as a naive example for newcomers.

In all, it seems that Input and Responder should be specific to the combination of framework and domain, and cannot be reliably extracted without specifying a third dependency (esp. the payload).

I might give a crack at adding the Arbiter package later today. Looks simple enough.

Right on. Let me know how it goes. With Radar it was dead-stupid-easy.

---

With that, unless there are further comments from @shadowhand et al, I think we have reached the end of this issue. The middleware queue and processing, and the action processing, have been extracted to independent packages, and input/domain/responder portions remain Domain-specific concerns to be addressed on a case-by-case basis.

Further thoughts?

[pmjones]

I think this issue has served its purpose. I am closing it, pending later desires to reopen it.