serialized attribute returns ActiveRecord::AttributeMethods::Serialization::Attribute in subclass #4004

Closed
masterkain opened this Issue Dec 16, 2011 · 24 comments

Comments

Projects
None yet
7 participants
Contributor

masterkain commented Dec 16, 2011

class SubClass < MyClass
  serialize :search_params, Hash

1.9.3p0 :030 > SubClass.last.search_params
 => #<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007fd46b639a10 @object_class=Hash>, value="---\n:test: hi\n", state=:serialized> 

AFAIK it should return the typecasted value. Maybe related: f4853dc

Edited title and example.

@ghost ghost assigned jonleighton Dec 16, 2011

Member

jonleighton commented Dec 16, 2011

This is definitely a regression, that struct shouldn't be accessible externally.

Owner

tenderlove commented Dec 16, 2011

I can't seem to reproduce this. Can we get a more complete test case? I tried this test on master:

diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index dd0aead..6fcc671 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1233,6 +1233,13 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal(myobj, topic.content)
   end

+  def test_serialize_hash
+    Topic.serialize("content", Hash)
+
+    Topic.create("content" => { 'foo' => 'bar' })
+    assert_equal({'foo' => 'bar'}, Topic.last.content)
+  end
+
   def test_serialized_time_attribute
     myobj = Time.local(2008,1,1,1,0)
     topic = Topic.create("content" => myobj).reload
Member

jonleighton commented Dec 18, 2011

Are you directly accessing @attributes anywhere? If so that might be the problem. (@attributes is a private implementation detail.)

I will close this for now as we can't repro. Please do re-open if/when you produce a complete test case. Thanks.

Contributor

masterkain commented Dec 19, 2011

Ok guys, I have some more findings. I don't have permissions to reopen the ticket, but the code below should illustrate the issue.

require 'rubygems'
require 'active_record'
require 'logger'

puts "Using ActiveRecord #{ActiveRecord::VERSION::STRING}"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection(
:adapter  => 'sqlite3',
:database => ':memory:'
)

# Create the minimal database schema necessary to reproduce the bug
ActiveRecord::Schema.define do
  create_table :playlists, :force => true do |t|
    t.string :name
    t.string :type
    t.text :search_params # or string, doesn't matter
  end
end

class Playlist < ActiveRecord::Base
  attr_accessible :name
  validates :name, presence: true, length: { within: 1..20 }
end

class SmartPlaylist < Playlist
  serialize :search_params, Hash

  validates :search_params, presence: true

  attr_accessible :search_params
end

p = SmartPlaylist.new(name: 'test')
puts p.valid? # returns true
p.save!

p = SmartPlaylist.new(name: 'test')
p.search_params = { test: 'me'}
p.save!
puts p.search_params

This has been tested in current rails master, what happens is the following:

  1. as you can see SmartPlaylist is a subclass, but validates :search_params doesn't gets fired if serialize declaration is present.
  2. the output for p.search_params is
#<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007f978919a0f0 @object_class=Hash>, value="---\n:test: me\n", state=:serialized>

Full output follows:

[ruby-1.9.3-p0@global] 12:13 ~ $ ruby test.rb 
Using ActiveRecord 3.2.0.beta
-- create_table(:playlists, {:force=>true})
D, [2011-12-19T12:13:34.154962 #15198] DEBUG -- :    (1.4ms)  select sqlite_version(*)
D, [2011-12-19T12:13:34.156497 #15198] DEBUG -- :    (0.5ms)  CREATE TABLE "playlists" ("id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, "name" varchar(255), "type" varchar(255), "search_params" varchar(255)) 
   -> 0.0134s
true
D, [2011-12-19T12:13:34.212691 #15198] DEBUG -- :    (0.1ms)  begin transaction
D, [2011-12-19T12:13:34.219903 #15198] DEBUG -- :   SQL (5.6ms)  INSERT INTO "playlists" ("name", "search_params", "type") VALUES (?, ?, ?)  [["name", "test"], ["search_params", nil], ["type", "SmartPlaylist"]]
D, [2011-12-19T12:13:34.220441 #15198] DEBUG -- :    (0.1ms)  commit transaction
D, [2011-12-19T12:13:34.221097 #15198] DEBUG -- :    (0.0ms)  begin transaction
D, [2011-12-19T12:13:34.222825 #15198] DEBUG -- :   SQL (0.1ms)  INSERT INTO "playlists" ("name", "search_params", "type") VALUES (?, ?, ?)  [["name", "test"], ["search_params", "---\n:test: me\n"], ["type", "SmartPlaylist"]]
D, [2011-12-19T12:13:34.223089 #15198] DEBUG -- :    (0.1ms)  commit transaction
#<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007f978919a0f0 @object_class=Hash>, value="---\n:test: me\n", state=:serialized>

@jonleighton jonleighton reopened this Dec 19, 2011

Contributor

alvarobp commented Dec 21, 2011

I had the same problem. After seeing this issue and @masterkain 's example code I managed to reproduce it and write a failing test case.

Find the patch here http://pastie.org/private/imjnb6tx214e141pocmzxg (I'm not sure about making a pull request just to share a failing test case. Should I have made one?)

My guess is that there is no problem as long as the serialize class method is called in the base class. However, if the serialize method is called in the subclass the accessor method does not deserialize the attribute and retrieves the value directly from the attributes hash.

I think this happens because the generated_external_attribute_methods method is delegated to the base class so the reader method is defined in that base class, on which the serialized attribute hasn't been declared.

Two solutions come to mind (from ignorance):

  • Rewrite the generated_external_attribute_methods method to somehow check for attribute methods in self (class) first and then in the base class if not present.
  • Ignore this and always declare serialized attributes in the base class.

I just moved the serialize call to my base class since it doesn't actually make any difference for me.

Contributor

masterkain commented Dec 21, 2011

@alvarobp I can confirm your findings, yesterday I spent some time hardening my tests and effectively declaring serialize (or store, for that matter) in superclass is working, even if it's kinda out of place because in my case the behavior really belongs only to a subclass.

Member

jonleighton commented Dec 21, 2011

@alvarobp you're dead on. I changed the code assuming that all subclasses that share a single DB table would have exactly the same attribute methods, but turns out I'm wrong. I'll fix this within a few days.

Member

jonleighton commented Dec 23, 2011

I have applied @alvarobp's test patch and then fixed in 7bb754e. (The bug reference in the commit message is wrong)

troex commented Jan 27, 2012

Looks like a have related problem or maybe the same:

I have simple model

class Graph < ActiveRecord::Base
  serialize :config, Hash
  serialize :metrics, Hash
end

later when I use this model's attributes it returns ActiveRecord::AttributeMethods::Serialization::Attribute and even more:


$ rails c
Loading development environment (Rails 3.2.1)

irb(main):001:0> Graph.find(289)
  Graph Load (0.3ms)  SELECT `graphs`.* FROM `graphs` WHERE `graphs`.`id` = 289 LIMIT 1
=> #<Graph id: 289, account_id: 1, host_id: 10, kind: "snmp", name: "uptime", category: "system", config: {"graph_title"=>"Uptime", "graph_vlabel"=>"days", "graph_base"=>"1000", "graph_lower_limit"=>"0"}, metrics: {"uptime"=>{"oid"=>"1.3.6.1.2.1.1.3.0", "label"=>"uptime", "dst"=>"g", "cdef"=>"uptime,8640000,/", "draw"=>"AREA"}}, hide: "auto", created_at: "2011-11-12 20:11:51", updated_at: "2011-11-12 20:14:22">

irb(main):002:0> Graph.find(289).attributes
  Graph Load (0.3ms)  SELECT `graphs`.* FROM `graphs` WHERE `graphs`.`id` = 289 LIMIT 1
=> {"id"=>289, "account_id"=>1, "host_id"=>10, "kind"=>"snmp", "name"=>"uptime", "category"=>"system", "config"=>#<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007fd807967c10 @object_class=Hash>, value="---\ngraph_title: Uptime\ngraph_vlabel: days\ngraph_base: '1000'\ngraph_lower_limit: '0'\n", state=:serialized>, "metrics"=>#<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007fd8079671e8 @object_class=Hash>, value="---\nuptime:\n  oid: 1.3.6.1.2.1.1.3.0\n  label: uptime\n  dst: g\n  cdef: uptime,8640000,/\n  draw: AREA\n", state=:serialized>, "hide"=>"auto", "created_at"=>2011-11-12 20:11:51 UTC, "updated_at"=>2011-11-12 20:14:22 UTC, "timezone"=>"St. Petersburg"}

That lead to that I cannot use Model.attributes.to_query if one of attributes is serialized

Member

jonleighton commented Jan 27, 2012

@Toex: what does graph[:config] return? can you provide steps to reproduce? have you tried disabling all plugins? thanks.

Member

jonleighton commented Jan 27, 2012

sorry, that should be @troex ^^

troex commented Jan 27, 2012

@jonleighton I've tried to reproduce with the script above but no luck it does not happen.

graph[:config]

what works okay if I access any attribute directly but not if using model.attributes

irb(main):024:0> graph[:config]
=> {"graph_title"=>"Uptime", "graph_vlabel"=>"days", "graph_base"=>"1000", "graph_lower_limit"=>"0"}

irb(main):025:0> graph.attributes['config']
=> #<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#<ActiveRecord::Coders::YAMLColumn:0x007fa2459a0da0 @object_class=Hash>, value={"graph_title"=>"Uptime", "graph_vlabel"=>"days", "graph_base"=>"1000", "graph_lower_limit"=>"0"}, state=:unserialized>

I'll try once again to reproduce it, but looks like it does not happen without full rails enviroment.

Member

jonleighton commented Jan 27, 2012

@troex: what does p graph.method(:attributes) show? also p graph.method(:read_attribute)?

troex commented Jan 27, 2012

@jonleighton thanks for directing me this way I've found the problem, my attributes method was redefined:

  def attributes
    @attributes['timezone'] ||= self.host.timezone
    @attributes
  end

This is kind a hack to define attribute value upon creation of object

My code worked in 3.1, but I think this my application's bug not rails, sorry for your time.
Here is reproduction code:

require 'rubygems'
require 'active_record'
require 'logger'

puts "Using ActiveRecord #{ActiveRecord::VERSION::STRING}"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection(
:adapter  => 'sqlite3',
:database => ':memory:'
)

# Create the minimal database schema necessary to reproduce the bug
ActiveRecord::Schema.define do
  create_table :graphs, :force => true do |t|
    t.string :name
    t.string :metrics
  end
end

class Graph < ActiveRecord::Base
  serialize :metrics, Hash
  attr_accessible :metrics, :timezone

  def attributes
    @attributes['timezone'] ||= 'UTC'
    @attributes
  end
end

p = Graph.new
p.name = 'test'
p.metrics = {'key' => 'value'}
p.save

#p = Graph.first
p.reload
puts p.attributes
Member

jonleighton commented Jan 27, 2012

@troex yes, you're not supposed to access @attributes - it's an implementation details of AR. You can do this instead:

def attributes
  attrs = super
  attrs['timezone'] ||= self.host.timezone
  attrs
end

troex commented Jan 27, 2012

Many thanks, now it works great. Hope to cu on 10th Feb in Moscow ;)

Raven24 commented Jan 31, 2012

I have a problem that seems to stem from the serialization refactoring discussed in this issue:
if you have a form that displays a serialized AR attribute, instead of the actual value you get

#<struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=..., value=..., state=:unserialized>

displayed inside the text field (as string). Every form view helper consistently shows that string. Echoing the form.object.myattribute directly inside the same partial yields the expected value, though.

This used to work with rails 3.1... (are there so few people using serialization?)

Member

jonleighton commented Jan 31, 2012

@Raven24 are you using Kaminari? If so, please upgrade for the fix. There will also be a workaround in the next point release.

Raven24 commented Jan 31, 2012

No, in the project that I want to upgrade I use nested_forms (by Ryan Bates, the railscast guy) where neccessary and normal forms, if that is important. But the issue appears with both. (I'm on rails 3.2.1, ruby 1.9.3)
So far everything else works fine, it's just the forms that don't seem to handle the serialization...

Member

jonleighton commented Jan 31, 2012

@Raven24: sorry, got my wires crossed. The Kaminari is something else.

Regarding your problem, do you still experience it after disabling all plugins? Are you able to help me by providing some code to reproduce the issue?

Raven24 commented Feb 1, 2012

Sure, here you go:
(shamelessly adapted from the one posted earlier in this thread)

require "rubygems"
require "action_view"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection(
  :adapter => 'sqlite3', 
  :database => ':memory:')

ActiveRecord::Schema.define do
  create_table :items, :force=>true do |t|
    t.string :name
  end
end

class MySerializer
  def load str
    str.swapcase
  end

  def dump str
    str.swapcase
  end
end

class Item < ActiveRecord::Base
  serialize :name, MySerializer.new
end

@t = Item.new
@t.name = "lowerUPPER"
@t.save!

include ActionView::Helpers::FormHelper

puts text_field( :t, :name)

the output of the text_field view helper is

<input 
id="t_name" 
name="t[name]" 
size="30" 
type="text"
value="#&lt;struct ActiveRecord::AttributeMethods::Serialization::Attribute coder=#&lt;MySerializer:0x00000002358e58 @_routes=nil&gt;, value=&quot;lowerUPPER&quot;, state=:unserialized&gt;" />

on my system (added a little formatting, for legibility here)

Member

jonleighton commented Feb 1, 2012

Aha, that helper accesses name_before_type_cast, which is not handled correctly. I'll make sure a fix gets into 3.2.2.

Member

jonleighton commented Feb 1, 2012

I've created #4837 to track this new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment