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

More useful create_with / update_with #153

Closed
jeremyevans opened this issue May 1, 2011 · 6 comments
Closed

More useful create_with / update_with #153

jeremyevans opened this issue May 1, 2011 · 6 comments
Assignees

Comments

@jeremyevans
Copy link
Owner

I think it is a really useful feature to be able to pass params into models that are not neccerserily
directly columns.

I also believe that Sequel:Model is missing a way to update from params that also triggers the
callbacks/hooks.

attached is a monkey-patch that I current run on my version of Sequel:Model to help me out.

I removed the column checking, since this is really the job of your own logic (sometimes I want
params that are not columns) and most web-frameworks have a way to pre-check for valid/safe
parameters.

record.update_with_params( {'name' => 'a' } )

by using method assignments instead of directly filling values, we can have special methods that
can accept parameters and do something useful with them.

This behavior is very desirable when working towards the fat-model-skinny-controller ideal.

Google Code Info:
Issue #: 128
Author: chrisfa...@gmail.com
Created On: 2008-01-17T09:41:11.000Z
Closed On: 2008-01-23T13:45:39.000Z

@ghost ghost assigned jeremyevans May 1, 2011
@jeremyevans
Copy link
Owner Author

I believe it needs a respond_to? check if there're some model non-related
parameters (let's say params[:controller] in merb or params[:page] or something like
that)

Google Code Info:
Author: invi...@gmail.com
Created On: 2008-01-18T01:17:43.000Z

@jeremyevans
Copy link
Owner Author

possibly ... but that does assume that you want your params assignment to go ahead even if you have passed
parameters that are of no use?

I think the case that you bring up is because you are considering parsing in the entire params hash. This is
probably not a great idea anyway, and you should try to group your params together using the "group[name]"
syntax of naming params supported by most frameworks.

Google Code Info:
Author: chrisfa...@gmail.com
Created On: 2008-01-18T09:30:50.000Z

@jeremyevans
Copy link
Owner Author

That's right. I was wrong, i should use grouppings.

BUT.

What if a hacker passes malicious parameters to the group by editing the query
string? He can trigger any setter method.

I'd check if

  1. setter exists (to avoid exceptions)
  2. setter is public (by using private setters we can make forbid some attributes to
    be changed easily)

Google Code Info:
Author: invi...@gmail.com
Created On: 2008-01-19T12:25:48.000Z

@jeremyevans
Copy link
Owner Author

I like the checking if method is public... that is a very smart move. But I don't think silently eating errors for
non-existing methods would be a good idea.

at the moment, the #create_with_params method will throw an error if the column doesn't exist.... this is
correct behavior since it should be up to your application [not sequel] to decide what to do in these situations.

ways to handle evil params:

Merb: http://merb.devjavu.com/browser/plugins/merb_param_protection/README
AR/Rails: http://api.rubyonrails.org/classes/ActiveRecord/Base.html#M001390

Google Code Info:
Author: chrisfa...@gmail.com
Created On: 2008-01-21T11:13:35.000Z

@jeremyevans
Copy link
Owner Author

+1

Would still be nice to have callbacks trigger on set/update
Thanks for the patch though

Google Code Info:
Author: altere...@gmail.com
Created On: 2008-01-22T07:46:18.000Z

@jeremyevans
Copy link
Owner Author

I'd like to keep the current behavior of #create_with_params so it wouldn't throw
errors on invalid parameters, but rather ignore them. For the time being I think it
would be wise to check with respond_to? and set only those attributes which the model
provides. This is now in the trunk.

Google Code Info:
Author: cico...@gmail.com
Created On: 2008-01-23T13:45:39.000Z

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

1 participant