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

Suggestion: Move params attribute to rel #2

Open
franzliedke opened this issue Apr 28, 2016 · 2 comments
Open

Suggestion: Move params attribute to rel #2

franzliedke opened this issue Apr 28, 2016 · 2 comments

Comments

@franzliedke
Copy link
Collaborator

This would obviously be a candidate for an eventual v0.6 release.

I've seen (and experienced) some confusion in regards to the params attribute in the get/post etc. methods.

From the README:

def get(params = {})
def post(data = {}, params = {})

All these methods accept at least one argument, namely the hash of params that will be interpolated / added to the URL when making a request.

For GET requests, this is perfectly straightforward and expected. For e.g. POST requests, it is confusing - which request data belongs where, and why? (Mostly when reading source code, I guess, one can figure it out when writing.)

Since params is actually a part of every request method, I'd suggest to move it to the rel method instead. In fact, I'd argue this even makes sense from a domain perspective: the data passed to params is typically substituted for the placeholders in the relation's URL template, or are optional URL parameters (such as filters).

Thus, when calling rel with some params, the resulting object would not so much be the description of a relation, but rather a handle for a resource in the REST terminology - a relation, bound (identified) to a distinct resource through the params.

Any additional data passed to the various request helper methods would then be sent in the way corresponding to the HTTP method: for GET requests, it would be serialized and appended to the URL, for POST/PUT/DELETE it would be sent as form data / JSON in the request body.

TL;DR Turn this:

repo = client.rel(:repository)
repo = repositories.get(owner: 'jgraichen', repo: 'restify').value

Into this:

client.rel(:repository, owner: 'jgraichen', repo: 'restify').get.value

(One could think about having separate concepts for the generic relation - i.e. the description of a relation with placeholders - and an instantiated relation, i.e. a resource - although that name is already taken...)

What do you think?

@franzliedke
Copy link
Collaborator Author

P.S.: The benefits are a little easier to see with e.g. PUT requests - fictitious example:

my_account = client.rel(:employee, name: 'franzliedke')

# Ensure fair payment
if my_acccount.get.value.salary < 100000000
  my_account.patch(salary: 100000000).value
end

With the current signatures, that second request would have to be made like this:

client.rel(:employee).patch({salary: 100000000}, name: 'franzliedke').value

@jgraichen
Copy link
Owner

Originally I wanted to only have a data parameter and have URL query parameters extracted automatically.

As a relation like /path{?a,b,c} defines that there are the query params a, b, and c they were simply taken from the data attributes. Unfortunately it failed when the API did not export query parameters on it's relations.

As it's ongoing more important to e.g. pass headers too I think about making the signatures like (data, **opts) with options being like params: {undefined_query_param: 4}, headers: {'Authorization': 'Token 46573'}. Known query parameters would also be striped from data.

I'll think about it but I do like your idea of merging things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants