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

The Decision Graph is Incomplete #211

Open
timvisher opened this issue May 8, 2015 · 7 comments
Open

The Decision Graph is Incomplete #211

timvisher opened this issue May 8, 2015 · 7 comments

Comments

@timvisher
Copy link

The Decision Graph implies that GETs can't reach 404. In fact, it implies that only POST can reach a 404.

This is false in that a GET that defines an :exists? decision which returns false does in fact return a 404 with the body being controlled by :handle-not-found.

Also, none of the handlers are listed so it's difficult to see where they get called.

@ordnungswidrig
Copy link
Member

I cannot follow. :exists? is called for all allowed methods. Additionally the rectangular nodes in the graph are the handlers.

@timvisher
Copy link
Author

@ordnungswidrig I had worked out that the rectangular boxes are the handlers. I guess what I'm saying is stylistically it might be better to have them be something like handle-ok (200 OK) rather than just 200 OK but that's very minor and you guys are consistent enough in your naming that it's not hard to find what you're looking for.

I think my major point is that just looking at the decision graph, I don't see a line from exists? to 200 OK. The decision graph only seems to talk about PUT, POST, and DELETE. Obviously, GET works, but I have to figure out how it works without the aid of the decision graph.

Does that make any more sense?

@andrewmcveigh
Copy link
Contributor

Hi Tim,

Initially I had some similar niggles with the graph as you. I think the problem is that you're expecting to see explicit GET/POST paths, but they're somewhat implicit.

There is no 'method-post? decision, but 'post-to-missing? and 'post-to-existing? (and others) basically deal with that, if it's not already determined implicitly.

So there is a path from exists? to 200 OK:

exists? => true
...
method-delete? => false
method-patch? => false
post-to-existing? => false
put-to-existing? => false
multiple-representations? => false

Perhaps if there were a method-post? decision between method-patch? and post-to-existing? the path would be more explicit. You'd then need another method-put? decision which would probably start to confuse things.

@ordnungswidrig
Copy link
Member

@andrewmcveigh yes, I think that's a very good explanation. I don't see any value on additional decision points for dispatch based on the method, actually the method-patch? and post-to-existing? are kind of "internal" decision points, not thought to be overridden by the resource definition.

@timvisher
Copy link
Author

@andrewmcveigh Yes. That's very clear. I had figured that out sort of on my own, but your explanation is clearer than the one that was floating around in my brain. :)

@ordnungswidrig I think it would be useful to put somewhere (if at all possible, on the decision graph itself) that internal decision points are not represented and that it shouldn't be implied that the decision graph represents all possible routes through it.

That's a pretty wordy way to put it though…

@ordnungswidrig
Copy link
Member

@timvisher I see. To be honest, I'm not too happy about the "internal decision points" at all. We should consider marking them, well, "internal" and to grey them out in the graph or like that.

@ordnungswidrig
Copy link
Member

@timvisher I've prepared a branch to remove some of the "internal decision points": #229

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