uber-hypermedia / specification

UBER Hypermedia Specification
http://uberhypermedia.org
24 stars 5 forks source link

update @url and @model properties #19

Closed mamund closed 9 years ago

mamund commented 9 years ago

@inadarei

currently the model property is supposed to be used to modify the url property (when action="read") and for creating a BODY (when action="append|partial|remove|replace").

i propose the following:

question:

comments?

NOTE: this will be a BREAKING CHANGE since old clients will MAY not treat the url or model properties correctly

[1] https://tools.ietf.org/html/rfc6570

inadarei commented 9 years ago

I think assuming all urls are URITemplate's is totally fine. I don't think processing will be issue. If it does become issue we can add optional templated attribute (it will have to default to true due to backwards compatibility, however).

I am not sure about only allowing model for message bodies, however. I like that it removes duality in using the property (makes things much easier to reason about) but leaves a void in functionality, possibly? URITemplate doesn't have the level of functionality that model does.

Model allows rich support of describing forms, correct? And forms can be GET or POST/PUT. If we don't allow using models for GET, how will we be able to define labels etc. for GET forms? URITemplate doesn't really allow that, does it?

Am I missing something badly?

mamund commented 9 years ago

couple things:

1) with the current spec you can't use template for both the query string and the body in a single request (e.g. url="/users/{id}" model="{name}&{address}&{phone}"). i think that's a problem.

2) i think we can cover both for the same request by using templating in the url property AND using model for the body. i don't think we're losing functionality here. do you see something that is now not possible that you could do before?

inadarei commented 9 years ago

You are right: the two cannot be used together, currently, so my bad on that one

URITemplate is super convenient for GET because all major platforms have highly optimized parser implementations (even PHP has a C extension) but: how would you document placeholder parameters in URITemplate for a client? Meaning: the labels of those params, maybe some constraints?

On Fri, Apr 3, 2015 at 8:18 PM, Mike Amundsen notifications@github.com wrote:

couple things: 1) with the current spec you can't use template for both the query string and the body in a single request (e.g. url="/users/{id}" model="{name}&{address}&{phone}"). i think that's a problem.

2) i think we can cover both for the same request by using templating in the url property AND using model for the body. i don't think we're losing functionality here. do you see something that is now not possible that you could do before?

Reply to this email directly or view it on GitHub: https://github.com/uber-hypermedia/specification/issues/19#issuecomment-89471975

mamund commented 9 years ago

"how would you document placeholder parameters in URITemplate for a client?"

how is that done in UBER now?

benhamill commented 9 years ago

I think UBER's model doesn't currently have anything approaching the richness of HTML's <input> or <select> or whatever, right? You can't say "send me one of: {true,false}" or "this is a number" or anything, yeah? model is just defined as a giant UTI Template, as it were.

So saying that model is for body doesn't make it any less expressive for GET requests.

Or am I missing something? (I'm intentionally ignoring the template proposal, for now).

mamund commented 9 years ago

@benhamill

Right, ignoring the stalled template proposal for now....

As you say, model is a bare-minimum templating element -- all functionality is lifted directly from UriTemplate spec.

In the current UBER spec:

Right now, without addressing the <label> shortcoming, I am trying to work out a way to support both modifying the url property AND constructing a message body in the same data element.

benhamill commented 9 years ago

Right, yeah, cool. I think your line of reasoning about making model always for the body and treating all url values as templated is sound. Some examples with that in mind, in case the reveals either my incomprehension or some new aspect:

{
  "uber": {
    "version": "1.x",
    "data": [
      {
        "rel": ["self"],
        "url": "http://api.example.com/"
      },
      {
        "name": "list friends",
        "rel": ["http://example.com/rels#list_friends"],
        "url": "http://api.example.com/{user_id}/friends",
        "action": "read"
      },
      {
        "name": "search friends",
        "rel": ["http://example.com/rels#search_friends"],
        "url": "http://api.example.com/{user_id}{?name,page,per_page}",
        "action": "read"
      },
      {
        "name": "add friend",
        "rel": ["http://example.com/rels#add_friend"],
        "url": "http://api.example.com/{user_id}/friends",
        "action": "append",
        "model": "friend={friend_id}"
      }
    ]
  }
}

Doing manual URI Template expanding, of course, I could...

$ # Follow the self link...
$ curl "http://api.example.com"
$ # Assuming I already have some user ids handy for whatever reason,
$ # I could list someone's friends...
$ curl "http://api.example.com/1234/friends"
$ # Or search their freinds...
$ curl "http://api.example.com/1234/friends?name=mike" # default paging stuff
$ # Or, lastly, make an introduction...
$ curl -d "friend=7889" "http://api.example.com/1234/friends"

Seem legit? I'm guessing, by the way, on how curl handles the -d option, but you get my point, I hope.

mamund commented 9 years ago

@benhamill

yep -- all looking good. that last example cuts right to the heart of it all, right?

inadarei commented 9 years ago

"how would you document placeholder parameters in URITemplate for a client?"

how is that done in UBER now?

You are right, there's not "official" way of doing it. I will open an issue about that, but it's a completely separate thing. This proposal of using URITemplate for "read" and only using models for body is 100% great.

Since there seems to be no doubt remaining, in principle, can we talk about the details please?

  1. @mamund, you convinced me that a way of distinguishing simple ulrs from url-templates is needed, if for no other reason, because "{" and "}" are not reserved characters in a URL. Are we set on templated attribute? Does it default to false?
  2. Quick question:

    Should we say anything about the possibility of using templates in URL's query section for actions other than "read" and also using model at the same time?

    Currently this behavior is certainly possible in HTTP, as the functionality of allowing query params and http body at the same time for an HTTP POST is supported by all major web-servers. I believe doing such things is at least frowned upon, if not outright an undocumented behavior in HTTP spec, but "oh, well".

    Anything we should say about it? Or leave it to the implementations if they want to implement "dirty hacks" that may make their APIs less portable to other protocols? Not our problem?

benhamill commented 9 years ago

Currently this behavior is certainly possible in HTTP, as the functionality of allowing query params and http body at the same time for an HTTP POST is supported by all major web-servers.

I think you might be underestimating the applicability of URI Templates. See my above example of "add friend" to see a case where you'd want to use a URI Template with a POST body (as append). It's not just for generating query parameters, but is a more generally useful templating syntax.

"{" and "}" are not reserved characters in a URL.

Ugh. I hadn't considered that. I agree, then, that we should have a way to indicate whether a given url is templated or not. I'd prefer a templated attribute that defaults to true. For use in the rare case where you have a url that really does have a literal { and/or } in it. Maybe there's yet another angle I'm not thinking of, though.

inadarei commented 9 years ago

@benhamill yeah, you skillfully avoided the trap in your example :), but I am asking about the cases in which URITemplate will be used for query params (which may still not be our problem, this is just a question).

Re: templated thing, there is another option:

since we are introducing new attribute, anyway, we might as well introduce url-template attribute. This could make parsing slightly easier, as you only need to check one property, rather than combination of two.

mamund commented 9 years ago

yeah - when i write up the url/model PR this weekend, i'll include a templated="true|false" property. i think that will be easier to support than two url properties (one for plain, one for templated URLs).

i can definitely see cases where i want BOTH in the same request. HTML doesn't does this, but lots of other APIs do.

inadarei commented 9 years ago

If we go with templated="true|false" then I think it's important to make the false be the default. That would make the new version backwards-compatible with the existing version. I don't know how many people use UBER in production, but we should assume there already are existing installations, right?

mamund commented 9 years ago

excellent point. will make sure to include "false" as the default.

mamund commented 9 years ago

i've updated the docs for issue #19 (+url+ and +model+ and +templated+) check it out and let me know what you think.

i might do some clean up before creating the PR

inadarei commented 9 years ago

Awesome!

Do you mind if I go ahead and create a PR? Makes it much easier to review changes. I can label it "pending review fixes" to mark as "not ready for merge" and any changes you make to the branch will show up automatically, anyway.

mamund commented 9 years ago

done!

mamund commented 9 years ago

@inadarei

made changes as suggested. let me know if you see any other needed mods.

mamund commented 9 years ago

closed as resolved w/ issue #19 and PR #25