[ADD] allow QueryMiddleware with get request

[Orkin]

16

[prolic]

@basz Can you check this PR?

[prolic]

ping @codeliner

[basz]

I don't get this RouteParamsExtractor - What is it good for? Why do we need it?

If I understand it correctly, the RouteParamsExtractor would extract GET vars into a payload the MessageFactory can deal with. Since this might vary across Routers I guess an Adapter would be appropriate.

@Orkin Can you clarify whether you indent to support multiple queries on a single GET requests? I'm worried that for multiple queries the request uri might get too long too quickly.

I'll can look further into this by tonight.

[Orkin]

@prolic RouteParamsExtractor is here to retrieve params form route url for exemple to retrieve the user_id from this url /users/{user_id} because only aura router retrieve value options from the route definition to set a default query / command name.

[Orkin]

@basz I would like to allow multiple queries only with POST because like you told the request uri might get too long and allow single query for get parameter.

The problem is that I'm not really agree with the fact that we have to do a POST request to retrieve a resource in case of REST ... And the other thing is that in POST request body we have a mention of prooph component. Maybe I'm too strict with that but it's an information about the back-end implementation and can lead to bug exploit or security issue if one is found. I think about production we can't always update dependencies as soon as possible :/

[basz]

@orkin I have the same issues as you mentioned (wanted to propose to rename the key from prooph_query_name to query_name)

So, one GET route, just one query. I assume just one response. Not an array of responses like POST.

I'm cool with that. Would hate to change all my GET request client side at this point.

So with this in mind I'll review tonight. Thanks

[Orkin]

@basz Yes that's it ! Thanks !

[Orkin]

@basz I'm agree with you it's really hard to determine which parameters is the most important if there is many params with the same name. That why I add a query entries because with get request we don't have body so we have to lead only with route and query params. For me route params are most of the case mandatory so we can think they are part of the payload for example with posts we can have an uri like this : /users/{user_id}/posts/{post_id}?format=json With this uri we have user_id and post_id that are part of the payload and the format parameter a query param. We can imagine that default format is html. It's an optional param and that make sense to put it in the metadata key instead of query. What do you think about that ?

[basz]

That makes sense. The counter argument is off course that when you use queryParam to refine a Query it could/should be considered part of the payload and not metadata about how to return the response. For example some/uri/to/a/resource?prop=value&page=2&sortBy=x. You might consider the property filter to be part of a queries payload, and the page/sorting metadata like. Indeed this is all just semantics...

So with that said I think my preference in this instance would be to keep in sync with the old behaviour. routeParam => payload. queryParams => metadata.

I just feel there is a generic solution which should apply to all http requests to message cases lingering somewhere... Perhaps I should open a different issue to discuss this further

[prolic]

I'm a little undecisive on this PR. @codeliner can you take over? I'll be happy with whatever you decide.

[codeliner]

I would like to keep things simple. HTTP request handling is not the primary target of prooph. It is not a web framework but instead sits behind one like Zend Expressive or Symfony. Our goal with the QueryMiddleware is not to provide a "one fits all" solution, because this is not possible with the available time we have and it goes against the idea of middlewares:

If POST requests for queries is not something you want to do then simply use your own middleware(s) for the query side. Middlewares are small and composable. The prooph project focuses on messaging and event sourcing. For messaging a single unified format is much easier to use and to understand:

POST request body => message payload Query params => message metadata

We can provide different middlewares, though. But that's something @basz should decide. I can only say that I won't have time to maintain it. We're fine with POST requests for all message types as this makes it super easy to develop clients that send messages to a single message box. See Event Machine It uses a single messagebox endpoint and generates an OpenAPI v3 schema that can be load into SwaggerUI. We have a JS implementation that works with such an endpoint and also let microservices communicate through messages send over HTTP.

If a REST-ish API is required I'd always build a custom one in front of the prooph backend and don't use the middleware package because it is not designed (on purpose) to provide REST-like semantics but rather messaging semantics.

[prolic]

@Orkin @codeliner This ticket is on hold since April 30. Will it procede or shall we close it?

[basz]

let's close this one