It's worth noting, of course, that this isn't specifically a rails anti-pattern. Many ORM layers offer the ability to update from a dictionary or associative array, and this is an anti-pattern elsewhere, too.
Just a minor correction update_attributes and friends are completely safe and not an anti pattern. What's an anti pattern is not using one of attr_protected and attr_accessible.
It's a shame the article was a little sensational before pointing this out, but it's in there. It's also a bit of a disaster that it's clearly not as widely used as it should be...
Even considering the existence of attr_protected and attr_accessible I still think it's an anti-pattern. You're accepting an arbitrary hash from the user and blindly using it to update a bunch of fields on your model, which in my opinion just invites security issues. attr_protected and attr_accessible feel like band-aids - they address the symptom, but it's the concept idea of assigning a user provided hash to your model's attributes that is the core problem.
attr_protected and attr_accessible also feel too coarsely grained to me. What about the case where you assign the "created_by" user when an object is first created, but don't want that to be re-assigned later on? How about situations where super-users can edit specific fields that regular users can't?
I don't think the convenience provided here over explicit assignment is worth the trade-off in terms of security risk, which is why I consider this an anti-pattern.
To suggest that because you *can* invent scenarios where the pattern doesn't apply, that no one should *ever* use it is ... well ... pythonic I suppose ;)
In all seriousness though, there's clearly a documentation or user-education gap here given the number of open source apps that didn't use this functionality. But forcing every single user to spell out every single attribute every single time, isn't really a decent solution. The majority of users shouldn't have to pay a tax when the coarse-grained security and simple approach works fine for them.
You bet it's pythonic... "Explicit is better than implicit" is line two of the Zen of Python!
There's a middle ground - rather than having attr_accessible on the ActiveRecord model itself, have some kind of intermediary class which handles safely copying the correct subset of attributes from form data to the model.
In Django that's what django.forms (and in particular the ModelForm class) does - provides one place that controls the interaction between data from forms and Django's ORM. Since it's also used to generate the forms it reduces boilerplate while also ensuring that only form fields that were actually served up on the page can be used to auto-populate fields on the model. Extra values hacked in to new hidden form fields will be silently ignored.
Indeed, ActiveRecord::Base is kinda ActionController::Form and a persistence object smooshed together. This makes some stuff a little tricky such as your created_by case without manually slicing the hash. But every time I've played with an alternative design, it just became ... too verbose!
I feel we should focus on making Easy things easy, hard things possible. Ouch, I quoted a perlism...
It's still something I intend to investigate, perhaps I'll borrow from djangos design when I next experiment.
Models are where your domain logic lives; when you're writing your model you use attr_accessible, just like you declare your validations, it's all part of the process. If you have special logic like not reassigning "created_by," then you set that up in the model, that's where those rules are supposed to live. You can't couple that kind of logic to a form object, especially when building web services.
In Grok/Zope 3 the same pattern as Django's forms is used. A Form class has an attribute which is a list of fields, which are used to render the form. When the form is updated, it only updates those fields which were specifically exposed in the form.
Except unlike Django, Grok's Form classes are specializations of a base View class. The inheritance is AddForm and EditForm inherits from Form which inherits from View. This way if you are constructing a form you don't need to have a separate View object and Form object. A little simpler way of working with forms, but it does requires all Views to be class-based, which makes all Views a little bit more complex. Ruby on Rails makes all Views class-based (aka "Controllers") but they usually route several different Views into the same class. Combining several Views into one class means making something like AddForm inherits from Form inherits from Controller would feel less natural I think (but I haven't done that much work with Ruby on Rails so maybe I'm wrong?). Zope 3 let's you use either approach, routing to either a class-based view or to individual methods, but that kind of flexibility also means you need to "go to the xml" and declare everything. Grok's purely class-based Views means that you end up creating quite a lot of classes - fortunately in Python classes can have a very low syntactical overhead (a complete class can be implemented in one line as easy as "class MySimpleView(grok.View): pass") so I've being enjoying that approach in my projects.
Protecting your Model objects using something like attr_accessible will work for simpler use cases, but in many applications you'll want a more sophisticated security model, with roles and permissions. It'd be pretty hard to implement Model level security that satisfies more complex use cases without making it so complex that people with simpler uses cases didn't absolutely hate it. And even if you did make something everyone liked, "Model-level" would likely mean something tied to into ActiveRecord or the Django ORM, and as soon as your Model needed to encompass data that lived outside a relational database you'd could either be out of luck or stuck monkey patchin'.
It's worth noting, of course, that this isn't specifically a rails anti-pattern. Many ORM layers offer the ability to update from a dictionary or associative array, and this is an anti-pattern elsewhere, too.
Completely agree - this is an anti-pattern where-ever it occurs.
Just a minor correction update_attributes and friends are completely safe and not an anti pattern. What's an anti pattern is not using one of attr_protected and attr_accessible.
It's a shame the article was a little sensational before pointing this out, but it's in there. It's also a bit of a disaster that it's clearly not as widely used as it should be...
Even considering the existence of attr_protected and attr_accessible I still think it's an anti-pattern. You're accepting an arbitrary hash from the user and blindly using it to update a bunch of fields on your model, which in my opinion just invites security issues. attr_protected and attr_accessible feel like band-aids - they address the symptom, but it's the concept idea of assigning a user provided hash to your model's attributes that is the core problem.
attr_protected and attr_accessible also feel too coarsely grained to me. What about the case where you assign the "created_by" user when an object is first created, but don't want that to be re-assigned later on? How about situations where super-users can edit specific fields that regular users can't?
I don't think the convenience provided here over explicit assignment is worth the trade-off in terms of security risk, which is why I consider this an anti-pattern.
To suggest that because you *can* invent scenarios where the pattern doesn't apply, that no one should *ever* use it is ... well ... pythonic I suppose ;)
In all seriousness though, there's clearly a documentation or user-education gap here given the number of open source apps that didn't use this functionality. But forcing every single user to spell out every single attribute every single time, isn't really a decent solution. The majority of users shouldn't have to pay a tax when the coarse-grained security and simple approach works fine for them.
You bet it's pythonic... "Explicit is better than implicit" is line two of the Zen of Python!
There's a middle ground - rather than having attr_accessible on the ActiveRecord model itself, have some kind of intermediary class which handles safely copying the correct subset of attributes from form data to the model.
In Django that's what django.forms (and in particular the ModelForm class) does - provides one place that controls the interaction between data from forms and Django's ORM. Since it's also used to generate the forms it reduces boilerplate while also ensuring that only form fields that were actually served up on the page can be used to auto-populate fields on the model. Extra values hacked in to new hidden form fields will be silently ignored.
Indeed, ActiveRecord::Base is kinda ActionController::Form and a persistence object smooshed together. This makes some stuff a little tricky such as your created_by case without manually slicing the hash. But every time I've played with an alternative design, it just became ... too verbose!
I feel we should focus on making Easy things easy, hard things possible. Ouch, I quoted a perlism...
It's still something I intend to investigate, perhaps I'll borrow from djangos design when I next experiment.
sandro - 24th September 2008 01:21 - #
In Grok/Zope 3 the same pattern as Django's forms is used. A Form class has an attribute which is a list of fields, which are used to render the form. When the form is updated, it only updates those fields which were specifically exposed in the form.
Except unlike Django, Grok's Form classes are specializations of a base View class. The inheritance is AddForm and EditForm inherits from Form which inherits from View. This way if you are constructing a form you don't need to have a separate View object and Form object. A little simpler way of working with forms, but it does requires all Views to be class-based, which makes all Views a little bit more complex. Ruby on Rails makes all Views class-based (aka "Controllers") but they usually route several different Views into the same class. Combining several Views into one class means making something like AddForm inherits from Form inherits from Controller would feel less natural I think (but I haven't done that much work with Ruby on Rails so maybe I'm wrong?). Zope 3 let's you use either approach, routing to either a class-based view or to individual methods, but that kind of flexibility also means you need to "go to the xml" and declare everything. Grok's purely class-based Views means that you end up creating quite a lot of classes - fortunately in Python classes can have a very low syntactical overhead (a complete class can be implemented in one line as easy as "class MySimpleView(grok.View): pass") so I've being enjoying that approach in my projects.
Protecting your Model objects using something like attr_accessible will work for simpler use cases, but in many applications you'll want a more sophisticated security model, with roles and permissions. It'd be pretty hard to implement Model level security that satisfies more complex use cases without making it so complex that people with simpler uses cases didn't absolutely hate it. And even if you did make something everyone liked, "Model-level" would likely mean something tied to into ActiveRecord or the Django ORM, and as soon as your Model needed to encompass data that lived outside a relational database you'd could either be out of luck or stuck monkey patchin'.