Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update @url and @model properties #19

Closed
mamund opened this issue Apr 3, 2015 · 19 comments
Closed

update @url and @model properties #19

mamund opened this issue Apr 3, 2015 · 19 comments
Assignees

Comments

@mamund
Copy link
Contributor

mamund commented Apr 3, 2015

@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:

  • the url property MUST ALWAYS be treated as a UriTemplate[1]
  • the model property MUST ONLY be used to construct the message BODY

question:

  • will this work? ALWAYS treat url as a template? that might be lots of needless work/processing
  • maybe it would be better to add a templated="true|false" property to apply to the url (when true the url MUST be processed as a UriTemplate)

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

@mamund mamund self-assigned this Apr 3, 2015
@mamund mamund added this to the April Update #1 milestone Apr 3, 2015
@inadarei
Copy link
Contributor

inadarei commented Apr 4, 2015

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
Copy link
Contributor Author

mamund commented Apr 4, 2015

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
Copy link
Contributor

inadarei commented Apr 4, 2015

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:
#19 (comment)

@mamund
Copy link
Contributor Author

mamund commented Apr 4, 2015

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

how is that done in UBER now?

@benhamill
Copy link

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
Copy link
Contributor Author

mamund commented Apr 4, 2015

@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:

  • When used for message bodies (append, partial, update) the completed model looks identical to an HTML body created from <form ... method="post"> w/ <input /> elements.
  • When used for modifying the url property (read, delete) the completed model looks identical to an HTTP query string created from <form ... method="get"> w/ <input /> elements.
  • What model does not do is support something like HTML's <label> element which adds prompts for HTML <input> elements. This is true for both use cases.

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
Copy link

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
Copy link
Contributor Author

mamund commented Apr 4, 2015

@benhamill

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

@inadarei
Copy link
Contributor

inadarei commented Apr 5, 2015

"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
Copy link

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
Copy link
Contributor

inadarei commented Apr 5, 2015

@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
Copy link
Contributor Author

mamund commented Apr 5, 2015

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
Copy link
Contributor

inadarei commented Apr 5, 2015

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
Copy link
Contributor Author

mamund commented Apr 5, 2015

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

mamund added a commit that referenced this issue Apr 6, 2015
@mamund
Copy link
Contributor Author

mamund commented Apr 6, 2015

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
Copy link
Contributor

inadarei commented Apr 6, 2015

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
Copy link
Contributor Author

mamund commented Apr 6, 2015

done!

@mamund
Copy link
Contributor Author

mamund commented Apr 7, 2015

@inadarei

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

@mamund
Copy link
Contributor Author

mamund commented Apr 8, 2015

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

@mamund mamund closed this as completed Apr 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants