Jeff Hall wrote:
Hi all!
I have come into a strange issue in Rails that I am hoping someone can
shed some light.
To make a very long story short, I have been researching how to
override ActiveRecord::Base#read_attribute and write_attribute to
perform security checks at the model level (influenced by the
ModelSecurity generator). Shortly after implementing some code to
check this behavior, I began to see undefined behavior from rails.
The first request to read an attribute would be denied correctly,
however further requests would proceed without fail.
After a lot of searching, I think I have traced the problem down to
the generated read methods. When
ActiveRecord::Base#generate_read_methods is false, all is well. When
its true, my migraine returns.
More research, reading, and I come to this curiosity...
ActiveRecord::Base#define_read_method(symbol, attr_name, column)
generates reader code for an attribute after Rails realizes there
should be a reader method for it, correct? But the generated reader
method body does not call read_attribute... instead it accesses the
attribute directly and performs a type cast (if necessary)
Is this intended? Am I missing something that read_attribute is doing
that is not appropriate for a defined attribute reader method?
The chosen method is intentional, for performance reasons. The savings
are due to 3 factors:
1. method_missing is avoided.
2. analysis of the method name inside method_missing is avoided
3. case analysis of the column type is avoided. for string columns,
which are probably the vast majority of columns in any app, this means
that the generated code will simply access the attributes hash. And this
is way faster than doing this analysis over and over again.
Taken together, this saves a *lot* of Ruby code being executed on each
attribute access.
Now the first 2 improvements would still be in place if the generated
code would simply call read_attribute. But step 3 is very costly and
should be avoided, if possible.
I suggest that if you want to modify the generated reader code for your
app, go ahead and monkey patch the method "define_read_method". I
introduced this method specifically to be able to override it. I do that
in my own app as well (but for different reasons).
However, I strongly resist to introduce this change into core.
It's near trivial to draft a patch for this - however, I wanted to
make sure I wasn't vastly underestimating something.
Comments?
You're underestimating the performance impact.
I suggest you turn off reader generation and measure the impact using my
railsbench package.
Cheers,
-- stefan
_______________________________________________
Rails-core mailing list
Rails-core@lists.rubyonrails.org
http://lists.rubyonrails.org/mailman/listinfo/rails-core
