In some case ActiveResource fails to handle serialized attributes with Ruby 1.9 and crashes

[marcgg]

Hi,

First of all, this is my first attempt at contributing to rails. I tried to follow the Rails guide, but I might be off. Feel free to give me guidelines ^^

I'll take the example of a blog with posts & comments to illustrate the issue.

ActiveResource takes all attributes of an object to create an object out of it. For instance if the JSON passed around is:

post = {
  id: 123,
  title: "Hey!",
  comments: [{id: 456, content: "Loving this"}]
}

It will create the correct Comment object associated with the Post. This works fine.

The problem is when we try to pass via ARes an object with serialized attributes. Let's say that we have:

class Post
  serialize :updates, Array
end

with updates defined as such: [{:user_updating_id => :update_date_to_i}, ...]

We could now have something like that:

post = {
  id: 123,
  title: "Hey!",
  updates: [
    {1 => 5464848414}, {2 => 9787987421}
  ]
}

The problem arise when we try to do a .to_sym on the parameters. In 1.8.7 it would just return nil:

ruby-1.8.7-p299 :001 > 1.to_sym
 => nil 

But in 1.9.2 it crashes:

ruby-1.9.2-p290 :001 > 1.to_sym
NoMethodError: undefined method `to_sym' for 1:Fixnum

I'm suggesting skipping attributes that are not valid to have the same behavior between Ruby 1.8.7 and 1.9.2. I inspired myself from the fact that we "next" blank attributes, so why not unsuitable ones?

This is not ideal, since it would be better to be able to determine if something is a serialized attribute or another model embeded and then have a different logic, but at least this would prevent ARes from crashing for Ruby 1.9.2 as it does right now if we have the situation detailed above.

Let me know if you need more detail or if you need something else to accept this patch!

[dmathieu]

You could also change the key into a string here, therefore allowing the to_sym.

(prefix_parameters.include?(key.to_s.to_sym) ? prefix_options : query_options)[key.to_s.to_sym] = value

[marcgg]

I don't think this would be best since it would try to create attributes based on integers or dates, like my_object.42

[dmathieu]

Please add a test case :)

[marcgg]

Okay I'll add a test case asap
Thanks for the feedback

[marcgg]

I just added some tests and a minor refactoring after @dmathieu advices.

The two tests cases I added are currently failing if ran on master with Ruby 1.9.2

It would be awesome if someone could take a look since this is causing issue for some apps my company is working on currently. Any directions on how this could be improved or any notes are appreciated.