Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

wrap and cache columns for typecasting

  • Loading branch information...
commit d592ea3b02c7e0952b87b876ad92bd1b453543c8 1 parent dd1eb78
@tenderlove tenderlove authored
View
10 activerecord/lib/active_record/attribute_methods/serialization.rb
@@ -10,6 +10,16 @@ module Serialization
self.serialized_attributes = {}
end
+ class Type # :nodoc:
+ def initialize(column)
+ @column = column
+ end
+
+ def type_cast(value)
+ value.unserialized_value
+ end
+ end
+
@pixeltrix Owner

This add a Type constant to every Active Record model and breaks constant autoloading for nested models, e.g:

$ rails c
Loading development environment (Rails 4.0.0.beta)
>> Product::Type
=> ActiveRecord::AttributeMethods::Serialization::Type

$ rails c
Loading development environment (Rails 3.2.8)
>> Product::Type
=> Product::Type(id: integer, name: string, created_at: datetime, updated_at: datetime)

The simple solution for this case is to rename the class to something like SerializationType however I was trying to think if there was some more generic solution. The problem is that there doesn't seem to be a hook to intercept the constant lookup - const_missing isn't called so we need some way of making the Type constant only visible inside the module where it's defined, however as far as I can tell there's no way of doing this. There's private_constant in 1.9.3 but that still sees the Type constant and returns a NameError: private constant Product::Type referenced error - any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
class Attribute < Struct.new(:coder, :value, :state)
def unserialized_value
state == :serialized ? unserialize : value
View
4 activerecord/lib/active_record/core.rb
@@ -165,7 +165,7 @@ def relation #:nodoc:
# User.new({ :first_name => 'Jamie', :is_admin => true }, :without_protection => true)
def initialize(attributes = nil, options = {})
@attributes = self.class.initialize_attributes(self.class.column_defaults.dup)
- @columns_hash = self.class.columns_hash.dup
+ @columns_hash = self.class.column_types.dup
init_internals
@@ -191,7 +191,7 @@ def initialize(attributes = nil, options = {})
# post.title # => 'hello world'
def init_with(coder)
@attributes = self.class.initialize_attributes(coder['attributes'])
- @columns_hash = self.class.columns_hash.merge(coder['column_types'] || {})
+ @columns_hash = self.class.column_types.merge(coder['column_types'] || {})
init_internals
View
21 activerecord/lib/active_record/model_schema.rb
@@ -206,6 +206,14 @@ def columns_hash
@columns_hash ||= Hash[columns.map { |c| [c.name, c] }]
end
+ def column_types
+ @column_types ||= columns_hash.dup.tap { |x|
+ serialized_attributes.keys.each do |key|
+ x[key] = AttributeMethods::Serialization::Type.new(x[key])
+ end
+ }
+ end
+
# Returns a hash where the keys are column names and the values are
# default values when instantiating the AR object for this table.
def column_defaults
@@ -268,9 +276,16 @@ def reset_column_information
undefine_attribute_methods
connection.schema_cache.clear_table_cache!(table_name) if table_exists?
- @column_names = @content_columns = @column_defaults = @columns = @columns_hash = nil
- @dynamic_methods_hash = @inheritance_column = nil
- @arel_engine = @relation = nil
+ @arel_engine = nil
+ @column_defaults = nil
+ @column_names = nil
+ @columns = nil
+ @columns_hash = nil
+ @column_types = nil
+ @content_columns = nil
+ @dynamic_methods_hash = nil
+ @inheritance_column = nil

Can we get this back if we clear it?

I don't think it's a "cache" like the other ivars here.

@tenderlove Owner

Get what back?

  1. User sets a custom inheritance column by saying self.inheritance_column = 'foobar'
  2. That sets @inheritance_column
  3. That ivar gets cleared by running .reset_column_information

I think the setting is lost unless the file is reloaded, which is not optimal.

@tenderlove Owner

Ah, that makes sense. Do you mind filing a ticket or sending a pull request?

Will do first thing tomorrow morning (Central time). Thanks!

@kennyj Collaborator
kennyj added a note

Relate to #5327 ?

@tenderlove Owner

@kennyj ya, I think it's the same thing. I've merged your pull request.

@seamusabshere can you verify that does what you want? If so, I'll backport to 3-2-stable (as I think this is a bug).

yes, perfect, thanks @kennyj !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ @relation = nil
end
def clear_cache! # :nodoc:
Please sign in to comment.
Something went wrong with that request. Please try again.