WIP: Serializer doesn't behave well with an attribute called options

[thiagogabriel]

I have an attribute called options being serialized, but it causes a
stack level too deep on my controller and it started on 0.10.0.rc3.

Using AMS test suit I checked it happens because of options attribute.

The expected is:
{:name=>"Name 1", :description=>"Description 1", :options=>'one'}

but with @adapter.serializable_hash(options) I get:
{:name=>"Name 1", :description=>"Description 1", :options=>{}}

and with @serializable_resource.serializable_hash(options) I get:
{:name=>"Name 1", :description=>"Description 1"}

[thiagogabriel]

@joaomdmoura is there any temporary recommendation (workaround) for this case different than rename my options attribute?

[thiagogabriel]

Update:
The problem is not exactly with SerializableResource, but was introduced on rc3.

[bf4]

I think I know what this is. There's some confusion in the code between options locals vs. instance variables/methods.. Want me to take a stab at it? Thanks for the tests!

[thiagogabriel]

@bf4 Sure. Thanks 👍
Maybe it is better to move or include new tests on the proper place, since the bug is not exactly on SerializableResource.

[bf4]

And you said this broke between rc2 and rc3? I haven't isolated it as quickly as I thought

[bf4]

The PR passes if we correct

+   @serializer = ProfileWithOptionsSerializer.new(@resource)
    @adapter = ActiveModel::Serializer::Adapter.create(@serializer)            
-   @serializable_resource = ActiveModel::SerializableResource.new(@resource)
+   @serializable_resource = ActiveModel::SerializableResource.new(@resource, serializer: ProfileWithOptionsSerializer)

So, I'm inclined to think this test doesn't reproduce your issue. :(

[bf4]

There's a chance this PR might fix it #1166

[thiagogabriel]

You are right about my wrong test. The test passed but the assertions are both wrong

[1] pry(#<ActiveModel::SerializableResourceTest>)> @adapter.serializable_hash(options)
=> {:name=>"Name 1", :description=>"Description 1", :options=>{}}
[2] pry(#<ActiveModel::SerializableResourceTest>)> @serializable_resource.serializable_hash(options)
=> {:name=>"Name 1", :description=>"Description 1", :options=>{}}

It is fixed on master, probably #1166

@adapter.serializable_hash(options)
=> {:name=>"Name 1", :description=>"Description 1", :options=>"one"}
[2] pry(#<ActiveModel::SerializableResourceTest>)> @serializable_resource.serializable_hash(options)
=> {:name=>"Name 1", :description=>"Description 1", :options=>"one"}

[thiagogabriel]

@bf4 I changed where my problem is happening but it broke many tests. Can you take a look on the new broken tests? Replacing try! with try fixes all the broken tests but the options values are ignored.

[bf4]

such meta (why try! over try?)

[thiagogabriel]

Because try checks for @object.respond_to? which returns false.

[bf4]

@thiagogabriel see #1165 (comment) for the reason for the test failure

Also, attributes and attribute both define methods on the Serializer that delegate to the object, so there should be no need to check the object for the method

[thiagogabriel]

@bf4 I'll take a better look on you PR #1165 tomorrow.

[bf4]

@thiagogabriel cool, and see if we can get a good failing test.. you might need to bring over more of your business logic, as well, as your Ruby version, Rails version, AMS version, backtrace, etc.

[thiagogabriel]

@bf4 I'm checking master now. Sorry for my delay.