Add AmqpQueryConsumerCallback

[nikmx]

Is the query consumer missing or is it rejected due to some technical reasons? There was a AmqpRpcMessageConsumerCallback included in early versions of this package.

[nikmx]

There might also be some inconsistencies concerning query producers.

The producer docs suggest that in the humus config message producers are treated universally. But as the query producer is utilizing the json-rpc-client the humus rpc docs show that configuration would be different.

Additionally as the docs mentioned suggest that producers would be configured universally and resolved via the same factory (e.g. not differing between rpc and regular producers) the factory would need to be sensitive to that, which seems not the case.

[prolic]

@nikmx thanks for your comments.

Let's go to the second part first: You're right, that for the query producer, the factory should check which key it currently reads from and instantiate the query producer instead of the message producer. This is obviously a bug. I mark this issue accordingly. Do you want to provide a PR to improve the factory?

I'm not sure about this AmqpRpcMessageConsumerCallback. Can find it on older releases (v1.1 and v1.0) as well. But maybe I simply forgot what I implemented in the past. If you can link it for me that would be great, so I can tell more about this specifics. Generally speaking, for RPC queries, I don't think providing a consumer callback that sends messages to the query bus again makes any sense. HumusAmqp is already capable of responding to RPC calls itself, so there is no need for that. See also https://humusamqp.readthedocs.io/en/latest/articles/rpc.html#setup-the-json-rpc-server

[nikmx]

@prolic thanks for your reply

I may provide a PR if I resolve my issue utilizing the package. I'll get back to that next week.

Concerning the rpc consumer, find the files attached as text files. A AmqpJsonRpcServer was also included in the package, which now (and maybe back then too) resides in the HumusAmqp package. The consumer I'm thinking of would of course build up on that, but integrating back into the service-bus context. Like directional bridging buses. Like what that package provides for commands and events. I was also reflecting if the communication pattern is valid for queries, but the dedicated callback queues of rpc calls (like rabbitmq integrates) seem reasonable.

AmqpRpcMessageConsumerCallback.php.txt AmqpJsonRpcServer.php.txt

[prolic]

I mean if you want to push the queries to the query bus again (which doesn't make any sense imho), then you can do this with the rpc server implementation of humus-amqp. To quote a part from the readme:

function (\Humus\Amqp\JsonRpc\Request $request): \Humus\Amqp\JsonRpc\Response {
    switch ($request->method()) {
        case 'times2':
            return \Humus\Amqp\JsonRpc\JsonRpcResponse::withResult($request->id(), $request->params() * 2);
        case 'times3:
            return \Humus\Amqp\JsonRpc\JsonRpcResponse::withResult($request->id(), $request->params() * 3);
        case 'plus5':
            return \Humus\Amqp\JsonRpc\JsonRpcResponse::withResult($request->id(), $request->params() + 5);
        default:
            return \Humus\Amqp\JsonRpc\JsonRpcResponse::withError($request->id(), new \Humus\Amqp\JsonRpc\JsonRpcError(32601));
    }
}

as you can see routing is already accomplished there, so what's the point of having the query bus additionally?

[nikmx]

Hm. If I would have a service bus context here I'd want the messages extracted and passed to the handlers. The routing wouldn't need to be done here, but would integrate in the service-bus implementation and keep everything in service-bus context.

[prolic]

Sure you can do that, but I think that's your custom implementation, doesn't need to be in the package by default.

On Fri, Jul 20, 2018, 02:20 nikmx notifications@github.com wrote:

Hm. If I would have a service bus context here I'd want the messages extracted and passed to the handlers. The routing wouldn't need to be done here, but would integrate in the service-bus implementation and keep everything in service-bus context.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prooph/humus-amqp-producer/issues/22#issuecomment-406369312, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYEvE1mtZkUexnl3X1tN9tiPawQKoQGks5uIM3UgaJpZM4VV9xc .

[nikmx]

Ok. Thought it might integrate well, like as bridge for all message types. As the integration was already there, I guess at some point you had that idea too. Thanks for your input.