Permalink
Browse files

omg computer science!

Implement a mini state machine for serialized attributes. This means we
do not have to deserialize the values upon initialization, which means
that if we never actually access the attribute, we never have to
deserialize it.
  • Loading branch information...
1 parent 4f20eb5 commit 7895182d0fce11131024305f53d0cbb32817e65c @jonleighton jonleighton committed Nov 30, 2011
@@ -10,6 +10,26 @@ module Serialization
self.serialized_attributes = {}
end
+ class Attribute < Struct.new(:coder, :value, :state)
+ def unserialized_value
+ state == :serialized ? unserialize : value
+ end
+
+ def serialized_value
+ state == :unserialized ? serialize : value
+ end
+
+ def unserialize
+ self.state = :unserialized
@netshade
netshade Mar 31, 2016

cc @jonleighton

Hi Jon, I know it's been quite a while since you've written this code, but I'm curious if the order of operations in this method is what you intended. Specifically, if a coder threw an exception here, the state becomes corrupt, as the next attempt to get serialized_value or unserialized_value will attempt to operate on value as if this unserialize or serialize operation had completed successfully, when in fact it had not.

I know I'm performing some serious code necromancy asking you questions about this, I was just curious if my statement above rang true for you, or if you had a specific reason for ordering the operations in this way. Thanks for any help!

@jonleighton
jonleighton Apr 1, 2016 Member

@netshade I doubt I considered that scenario, but I think your reasoning sounds right, and in theory it would be good to protect against the coder throwing an exception here.

@netshade
netshade Apr 1, 2016

Awesome - thanks for confirming Jon! Very much appreciated, thank you. :)

+ self.value = coder.load(value)
+ end
+
+ def serialize
+ self.state = :serialized
+ self.value = coder.dump(value)
+ end
+ end
+
module ClassMethods
# If you have an attribute that needs to be saved to the database as an object, and retrieved as the same object,
# then specify the name of that attribute using this method and it will be handled automatically.
@@ -42,39 +62,35 @@ def define_method_attribute(attr_name)
if serialized_attributes.include?(attr_name)
generated_attribute_methods.module_eval(<<-CODE, __FILE__, __LINE__)
def _#{attr_name}
- @attributes_cache['#{attr_name}'] ||= @attributes['#{attr_name}']
+ @attributes['#{attr_name}'].unserialized_value
end
alias #{attr_name} _#{attr_name}
CODE
else
super
end
end
-
- def cacheable_column?(column)
- serialized_attributes.include?(column.name) || super
- end
end
def set_serialized_attributes
- sattrs = self.class.serialized_attributes
-
- sattrs.each do |key, coder|
- @attributes[key] = coder.load @attributes[key] if @attributes.key?(key)
+ self.class.serialized_attributes.each do |key, coder|
+ if @attributes.key?(key)
+ @attributes[key] = Attribute.new(coder, @attributes[key], :serialized)
+ end
end
end
def type_cast_attribute(column)
- coder = self.class.serialized_attributes[column.name]
-
- if column.text? && coder
- unserialized_object = coder.load(@attributes[column.name])
+ if column.text? && self.class.serialized_attributes.include?(column.name)
+ @attributes[column.name].unserialized_value
+ else
+ super
+ end
+ end
- if @attributes.frozen?
- unserialized_object
- else
- @attributes[column.name] = unserialized_object
- end
+ def type_cast_attribute_for_write(column, attr_name, value)
+ if column && coder = self.class.serialized_attributes[column.name]
+ Attribute.new(coder, value, :unserialized)
else
super
end
@@ -28,10 +28,8 @@ def write_attribute(attr_name, value)
@attributes_cache.delete(attr_name)
column = column_for_attribute(attr_name)
- if column && column.number?
- @attributes[attr_name] = convert_number_column_value(value)
- elsif column || @attributes.has_key?(attr_name)
- @attributes[attr_name] = value
+ if column || @attributes.has_key?(attr_name)
+ @attributes[attr_name] = type_cast_attribute_for_write(column, attr_name, value)
else
raise ActiveModel::MissingAttributeError, "can't write unknown attribute `#{attr_name}'"
end
@@ -43,6 +41,14 @@ def write_attribute(attr_name, value)
def attribute=(attribute_name, value)
write_attribute(attribute_name, value)
end
+
+ def type_cast_attribute_for_write(column, attr_name, value)
+ if column && column.number?
+ convert_number_column_value(value)
+ else
+ value
+ end
+ end
end
end
end
@@ -2003,8 +2003,8 @@ def arel_attributes_values(include_primary_key = true, include_readonly_attribut
if include_readonly_attributes || (!include_readonly_attributes && !self.class.readonly_attributes.include?(name))
- value = if coder = klass.serialized_attributes[name]
- coder.dump @attributes[name]
+ value = if klass.serialized_attributes.include?(name)
+ @attributes[name].serialized_value
else
# FIXME: we need @attributes to be used consistently.
# If the values stored in @attributes were already type
@@ -686,17 +686,13 @@ def test_dispatching_column_attributes_through_method_missing_deprecated
private
def cached_columns
- @cached_columns ||= (time_related_columns_on_topic + serialized_columns_on_topic).map(&:name)
+ @cached_columns ||= time_related_columns_on_topic.map(&:name)
end
def time_related_columns_on_topic
Topic.columns.select { |c| c.type.in?([:time, :date, :datetime, :timestamp]) }
end
- def serialized_columns_on_topic
- Topic.columns.select { |c| Topic.serialized_attributes.include?(c.name) }
- end
-
def in_time_zone(zone)
old_zone = Time.zone
old_tz = ActiveRecord::Base.time_zone_aware_attributes

0 comments on commit 7895182

Please sign in to comment.