radarphp / Radar.Adr

The Action-Domain-Responder core for Radar.
MIT License
55 stars 7 forks source link

Double-indirection in input array #23

Closed cxj closed 8 years ago

cxj commented 8 years ago

This seems a little odd. Couldn't we just have an array of values, rather than an array of an array of values? :-)

class GenericPostInput
{
    public function __invoke(ServerRequestInterface $request)
    {
        $stream = $request->getBody();

        $data = $stream->read(99999);

        return array(array($data));
    }
}
pmjones commented 8 years ago

Hiya -- the return has to be a sequential array, because it gets sent through call_user_func_array() as the second argument when invoking the domain callable. E.g.: $payload = call_user_func_array($domainCallable, $inputReturn).

However, I don't see why you'd need array(array($data)) instead of just array($data). What's the signature of the domain callable being invoked?

cxj commented 8 years ago

The Route looks like this:

$adr->post('270', '/270', '\Clx\Api270')
    ->input('\Clx\GenericPostInput')
    ->responder('\Clx\X12responder');

So I think that means I'm required to implement Action with an __invoke() method like this:

    public function __invoke(array $input = array()) {

My Domain method called from the above expects just a string as an input. So I have this:

$domain = new Domain();
$output = $domain->lookup($input[0]);   // Note dereference of the first input element.

return $payload->setStatus(PayLoadStatus::SUCCESS)->setOutput($output);

I think what caused me to return array(array($data)) was to make the __invoke() signature match the examples in your documentation. I'm not actually sure how I arrived at that input filter any longer, but doing that indirection is what finally made it work, I think.

cxj commented 8 years ago

Or in more general terms, I'm just trying to read a POSTed string of data, send it to my domain, and have the web server output a returned string to the client. It's supposed to be a simple RESTful web service.

pmjones commented 8 years ago

Coming back around to this after way too long. Is it still trouble for you?

When you say "I think that means I'm required to implement Action" -- in Radar, you're not really coding any Action classes. The Action logic is standardized via an extension of Arbiter's ActionHandler. All you should need to implement is an Input callable, a Domain callable, and Responder callable.

cxj commented 8 years ago

scratches head -- Well, my app is still coded as described above. The Domain gets its input from $input[0] and the Input class still does the odd return array(array($data)) thing. If it ain't broke, don't fix it.

I'd have to mess around with it some more to see if I can get one layer of array out of it.

I could'a sworn the original Radar README.md called the third argument to the route an Action, but I see the current, more expanded documentation calls it a Domain. Yeah, that thing. ;-)

pmjones commented 8 years ago

Heh. :-) I suppose we can look at it together soon enough? Meanwhile, I'm trying to do some administrative cleanup; do you mind if we close this issue pending further review? (We can leave it open if you like though.)

cxj commented 8 years ago

Just so it doesn't get forgotten by me. Maybe I'll just create an internal ticket for myself in our Redmine instance to nag me. So go ahead and close for now I guess.

pmjones commented 8 years ago

No, I can leave it open, no problem. If you can create a test case, or some other working example, demonstrating the issue, that will help me figure out what's going on.

cxj commented 8 years ago

Let me see what I can do.

cxj commented 8 years ago

I think this demonstrates the peculiarity: https://github.com/cxj/Issue23Radar.Adr. In particular, look at GenericPostInput.php and X12ApiAction.php (misnamed, I know -- should be X12ApiDomain.php :-)

You can run it with php -S localhost.8000 -t web/ and then use something like ncat or other simple socket connection to throw a string at it.

pmjones commented 8 years ago

I'm an idiot. If it was a snake it woulda bit me. Yes, you need the array wrapper around the "core" input array, and yes, it's intentional. It's because Arbiter ActionHandler invokes the domain callable using call_user_func_array().

By way of example, let's say the "core" input array is ['foo', 'bar', 'baz']. If you return just that from your input class, Arbiter will do the equivalent of $domain('foo', 'bar', 'baz'). That's because call_user_func_array() passes each element of the array as a separate argument. If you have a domain callable that uses separate arguments, that's exactly what you want.

However, your particular domain callable requires an array as the first parameter. That means you have to wrap the "core" input array in a surrounding array, so that the entire core array gets passed as a single argument.

You can see exactly the same thing in https://github.com/radarphp/Radar.Adr/blob/1.x/src/Input.php. The different data sources are merged into a single array, and then returned as the first element of a wrapping array, so that you can slap that array of data against the domain.

So you're doing the right thing. Hope that all makes sense. Sorry for my blindness to the obvious.

cxj commented 8 years ago

Got it. So I could simplify my sample code by removing one array() wrapper in my Input class and by then expecting a scalar string as the argument to my Domain class. At least, it appears to continue to work correctly when I do that.

I thought I had tried something like that before and then received some PHP error about mismatched types. But not now. Probably a symptom of a bad memory. Hope you didn't spend too much time on this thing.

pmjones commented 8 years ago

I could simplify my sample code by removing one array() wrapper in my Input class and by then expecting a scalar string as the argument to my Domain class.

Yup.

Hope you didn't spend too much time on this thing.

Any time I spent on it past the first couple of comments above is my own fault. Thanks, as ever, for your patience. :-)