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

Add raise_on_blank_find_param resource setting #344

Conversation

stokarenko
Copy link
Contributor

The problem comes from #329

That is a bit curios, but json_api_client returns an array from .find method, always.
The history of this fact was discussed here

So, when we searching for a single resource by primary key, we typically write the things like

admin = User.find(id).first

The next thing which we need to notice - json_api_client will just interpolate the incoming .find param to the end of API URL, just like that:

http://somehost/api/v1/users/{id}

What will happen if we pass the blank id (nil or empty string) to the .find method then?.. Yeah, json_api_client will try to call the INDEX API endpoint instead of SHOW one:

http://somehost/api/v1/users/

Lets sum all together - in case if id comes blank (from CGI for instance), we can silently receive the admin variable equal to some existing resource, with all the consequences.

Even worse, admin variable can equal to random resource, depends on ordering applied by INDEX endpoint.

If you prefer to get JsonApiClient::Errors::NotFound raised, please define in your base Resource class:

class Resource < JsonApiClient::Resource
  self.raise_on_blank_find_param = true
end

Copy link
Collaborator

@gaorlov gaorlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! Thanks for taking the time!

test/unit/query_builder_test.rb Show resolved Hide resolved
@stokarenko stokarenko force-pushed the add-raise_on_blank_find_param-resource-setting branch from b9006c3 to 7df6daf Compare May 24, 2019 17:24
Copy link
Collaborator

@gaorlov gaorlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! As soon as tests pass, I'll release this as 1.11.0.
Thanks again for taking the time!

@gaorlov
Copy link
Collaborator

gaorlov commented May 24, 2019

Oh. one last thing: can you please add a brief description to CHANGELOG.md? Thanks!

@stokarenko
Copy link
Contributor Author

on my way! )

@stokarenko stokarenko force-pushed the add-raise_on_blank_find_param-resource-setting branch from 7df6daf to 3b48cfc Compare May 24, 2019 17:31
@stokarenko
Copy link
Contributor Author

@gaorlov done )

@gaorlov gaorlov merged commit 53264d9 into JsonApiClient:master May 24, 2019
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

Successfully merging this pull request may close these issues.

2 participants