interagent / prmd

JSON Schema tools and doc generation for HTTP APIs
MIT License
2.1k stars 172 forks source link

Adding querystrings to links fails on regex unless there's a {()} before the question mark #253

Open henrythor opened 9 years ago

henrythor commented 9 years ago

This fails: `"href": "/wallet/statements?start={(%2Fschemata%2Faccount%23%2Fdefinitions%2Fstart)}&end={(%2Fschemata%2Faccount%23%2Fdefinitions%2Fend)}"`` With this error:

#/definitions/account/links/3/href: failed schema #/definitions/resourceLink/properties/href: /wallet/statements?start={(%23%2Fdefinitions%2Faccount%2Fdefinitions%2Fstart)}&end={(%23%2Fdefinitions%2Faccount%2Fdefinitions%2Fend)} does not match /^(\/([a-z0-9][a-z0-9_\-]*[a-z0-9]|\{\(.*\)\}))+$/.

If I use this: "href": "/wallet/{()}/statements?start={(%2Fschemata%2Faccount%23%2Fdefinitions%2Fstart)}&end={(%2Fschemata%2Faccount%23%2Fdefinitions%2Fend)}",

It's fine. Breaks my documentation, but atleast it parses.

I'm using prmd 0.8.0

geemus commented 9 years ago

Sorry to hear that. Are these for GET requests? I suspect that it should just render properties as query in the examples if it is a GET, which might help you? If it is not GET, things are slightly more complicated. I think maybe in that case we might need/want to expand the second [] in the first part of the regex to also include the query reserved characters (ie ?&=). What do you think?

JulienPalard commented 8 years ago

I'm having the same problem I have an URI with a query string and it fails against ^(\/((?!-|_)[a-z0-9_\\-]+(?<!-|_)|\\{\\(.*\\)\\}))+$".

I can understand you don't like query string in REST APIs, yet they are widly used (think ?page=, ?q=, whatever), and I think you should allow them.

To be honest, I don't think we can provide a regex that covers all legitimates URI patterns of REST APIs, why not simply drop it ? I'm sure in a certain specific domain and context we can find a legitimate use of _ after a /, like, /doc/functions/__init__/ which is a bad example because it should probably be a {(...)}.

geemus commented 8 years ago

I guess I would prefer to keep some level of validation, even if it is a looser one, to avoid altogether invalid URIs, that said, it does seem problematic to pin down what exactly that regex should be given the many possible edge cases. My question was around GET because parameters in that context for a link are presumed to actually appear in the query string already (so you can define them that way without actually putting them into the path of the link). I thought perhaps that might be a better way to go through this than putting it all in the path and agree there are legitimate cases for using that, but I think largely or wholly in GET (for POST/etc I would expect body params instead). Does that make sense/help?

JulienPalard commented 8 years ago

I understand and can only agree with the preference of keeping a level of validation.

I, personally, think that REST as we understand it is only a human presentation of the 2616, and my understanding of the 2616 highly diverge with "PHP grown" developpers (no insult here):

I mean, I'm OK to GET information with curl -XGET http://... -d '{"huge": "query DSL like ES describing in a lot of bytes exactly what I want}' and I'm OK to curl -XPOST http://...?some_hints_about_how_to_alter_the_collection without body to "add an empty (default ?) document to a collection".

As REST is more a "nice presentation of the 2616" than a "PHP oriented presentation of doing an API", I can only think that we should NOT correlate GET with query string and POST with request body.

Finally, the query string is a well defined list of key value pairs, but the request body is let to the discretion of the implementor, accepting the Content-Type they want, so I'm not sure it's the role of hyperschema to describe request body (no more than allowed content-types and an example ?).

geemus commented 8 years ago

I agree that 2616 doesn't make a strong stance on this, but the json-schema definition does. It quite explicitly says that GET schema becomes query and POST schema becomes body, see: http://json-schema.org/latest/json-schema-hypermedia.html#anchor38

If that pattern is followed, the nice part is that it provides a more detailed way to specify what the different query params are, what values are allowed, etc. But it certainly doesn't provide this niceness if using POST with query. Adding it to the path directly is certainly one way around this, though as you note it isn't currently supported.

Hope that helps clarify some of the reasoning behind why this tooling is setup this way (more to match json-schema expectations/usage than broader 2616).

goto100 commented 8 years ago

need this feature +1

geemus commented 8 years ago

Perhaps we should just loosen the regex so that it can be accommodated in the URL. Anyone up for a PR?