cognitect-labs / vase

Data driven microservices
Eclipse Public License 1.0
374 stars 40 forks source link

Query parameters, path and parameters in request entity are mixed together #12

Closed ordnungswidrig closed 8 years ago

ordnungswidrig commented 8 years ago

Pedestal merges all parameters related to a request, i.e. query parameters, path parameters and request entity data. One problem is that the priority of the different parameters sources is arbitrary (however documented). This problem is a fundamental one, which I like to point out because vase declares to be "way to describe and run RESTful APIs". The query and path parameters are part of the URL addressing a resource and not part of the state that a client wants to be transferred as the new state of the resource. In my view, mixing all parameters together does not play well with a clean REST architecture.

In the context of pedestal this means that all the data for a transact action should come from the request entity, i.e. the json or edn encoded body.

ohpauleez commented 8 years ago

This seems more like a philosophical issue. Is there a technological issue that is preventing you from delivering value?

ordnungswidrig commented 8 years ago

Interesting take, one technical issue that comes to my mind is that passing entity representation as URL parameters interferes with the cache invalidation guarantees of the HTTP methods.

ordnungswidrig commented 8 years ago

More background on this is in rfc7234 §3.4.

ohpauleez commented 8 years ago

So don't pass the representation on the URL.

It would helpful for me to hear a problem statement in the form of: "I would like to do X with Vase to deliver Y value to the end-user/service. Because the parameters are merged, this isn't possible." It allows me to consider a concrete usecase where Vase isn't delivering what it should.

On the surface, the only thing that isn't possible is having two parameters from different "sources" have the same name, which I would argue is an anti-pattern anyway. There's also a rule of "Don't do things you know are wrong" like hacking params into the query string when they shouldn't be passed there. But that said, those misbehaviors are all on the client-side and Vase can do little to enforce anything.

ordnungswidrig commented 8 years ago

Ok, I understand your reasoning. I still think that vase should not allow this antipattern and those interceptors that process an entity from the request, e.g. transact, should only use the value in the request body, not the query string.

ohpauleez commented 8 years ago

Closing this issue for now, but if this becomes an issue in the wild, we should re-open it.