Active_model_serializers: Serializer for association does not look for siblings

Created on 19 Nov 2015  路  6Comments  路  Source: rails-api/active_model_serializers

I'm using master of the repo (currently locked to revision efe5128a2e6b8578d81bc7a807ace427c0b4d847). I really needed all of the include tree magic for the json adapter, and unfortunately, that's not in a release candidate yet (hint hint :)).

I have the current setup:

app/serializers/api/v1/lane_serializer.rb
class Api::V1::LanesSerializer < ActiveModel::Serializer

  has_many :sale_listings

end

app/serializers/api/v1/sale_listing_serializer.rb
class Api::V1::SaleListingSerializer < ActiveModel::Serializer
end

The lookup chain for the sale listing serializer is:

Api::V1::LanesSerializer::SaleListingSerializer (child)
::SaleListingSerializer (rooted)

I believe that it should check:
child
sibling
rooted

Interestingly enough, safe_constantize finds my sale listing serializer the FIRST time, but once it's loaded into memory, it doesn't find it again.

'Api::V1::LanesSerializer::SaleListingSerializer'.safe_constantize
=> Api::V1::SaleListingSerializer
'Api::V1::LanesSerializer::SaleListingSerializer'.safe_constantize
=> nil

This left me with a problem because my test env is not eager loaded, but production is. So, if the serializers are loaded in before ActiveModel::Serializer.get_serializer_for's safe_constantize call, I'm hosed.

Here's a monkey patch I put together that fixes the issue.

class ActiveModel::Serializer
  class << self
    alias_method :serializer_lookup_chain_for_without_sibling, :serializer_lookup_chain_for

    def serializer_lookup_chain_for(klass)
      self.serializer_lookup_chain_for_without_sibling(klass).
        insert(1, "#{self.name.deconstantize}::#{klass}Serializer")
    end
  end
end

Most helpful comment

The use case of controller and serializer namespace you've got laid out in this PR is exactly what I'm doing. Something along these lines would help my situation a lot.

That PR references a lot of issues of people asking for versioned serializer support, and I keep seeing people say that they should namespace the modules. But, without adding siblings into the lookup chain, that doesn't work.

I'd really like to help get this into a release somehow. I think it could help a lot of people, so what ever I can do to help, I'd be willing to do. Plus, encouraging a monkey patch in the docs feels really hacky and wrong to me. Though I don't know what situations you guys thought it would be more confusing in. Is there an issue you can link, so I can read more about it? Maybe I can somehow bring some new idea to the table.

As far as the autoloading issue is concerned:

The problem is with how ActiveSupport's constantize works.

Loading development environment (Rails 4.2.0)
irb(main):001:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
*** [DEBUG] Loading Api::V1::LaneSerializer file ***
*** [DEBUG] Loading Api::V1::SaleListingSerializer file ***
=> Api::V1::SaleListingSerializer
irb(main):002:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
NameError: uninitialized constant Api::V1::LaneSerializer::SaleListingSerializer

When it goes loading in files to try to find the constant I asked for, it actually loads in the file that I want and returns the constant I want (though, admittedly, not the constant I asked for). The second time I attempt to constantize that string, it doesn't go loading files, and instead raises an uninitialized constant error.

Since constantize finds it the first time, looking up siblings "just worked" in my test environment. This is because ActiveModel Serializer was what loaded in the serializer, and it then cached the class that it found.

This lazy-loading of the files is what happens when eager loading is disabled. It's nice for a test environment (and is the Rails default) because it uses less resources and your app boots faster because it only loads in what it needs.

Eager loading will actually go through and load your app into memory on boot. This will make your app perform faster because it doesn't have to go looking for classes.

Here's a quote from the default config/environments/test.rb file:

# Do not eager load code on boot. This avoids loading your whole application
# just for the purpose of running a single test. If you are using a tool that
# preloads Rails for running tests, you may have to set it to true.
config.eager_load = false

Meanwhile, config/environments/production.rb says the following by default:

# Eager load code on boot. This eager loads most of Rails and
# your application in memory, allowing both threaded web servers
# and those relying on copy on write to perform better.
# Rake tasks automatically ignore this option for performance.
config.eager_load = true

Here's an example of what happens if I require the file and then constantize.

Loading development environment (Rails 4.2.0)
irb(main):001:0> require 'api/v1/sale_listing_serializer'
*** [DEBUG] Loading Api::V1::SaleListingSerializer file ***
=> true
irb(main):002:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
*** [DEBUG] Loading Api::V1::LaneSerializer file ***
NameError: uninitialized constant Api::V1::LaneSerializer::SaleListingSerializer

So, in my case, as long as ActiveModelSerializers' serializer_lookup_chain is the first thing to load api/v1/sale_listing_serializer.rb into memory, then sibling lookup "just works" without monkey patching. But if it's not, then my app breaks in production and uses Rails' default to_json methods because it can't find the serializer class.

I literally had written all of my tests, and everything worked, and when I deployed, it was all broken.

This is why I think that it should look up siblings, too. Then, one of the constant names that it looks up will actually be the class I want.

All 6 comments

The initial idea behind the serializer_lookup_chain_for method was that it could be monkey-patched for specific/complicated use-cases. There has been talks about adding the siblings somewhere in the lookup chain, but we ended up deciding that it would make some situations too confusing. If you had time to make a doc PR to explain how to monkey-patch the lookup chain, that would be awesome.

Concerning the autoloading issue, it already popped up in the past, and I started investigating but couldn't really find a solution (I'm not very familiar with Rails' autoloading). Do you have any information/knowledge that could help us solve it? (ref. #1331)

The use case of controller and serializer namespace you've got laid out in this PR is exactly what I'm doing. Something along these lines would help my situation a lot.

That PR references a lot of issues of people asking for versioned serializer support, and I keep seeing people say that they should namespace the modules. But, without adding siblings into the lookup chain, that doesn't work.

I'd really like to help get this into a release somehow. I think it could help a lot of people, so what ever I can do to help, I'd be willing to do. Plus, encouraging a monkey patch in the docs feels really hacky and wrong to me. Though I don't know what situations you guys thought it would be more confusing in. Is there an issue you can link, so I can read more about it? Maybe I can somehow bring some new idea to the table.

As far as the autoloading issue is concerned:

The problem is with how ActiveSupport's constantize works.

Loading development environment (Rails 4.2.0)
irb(main):001:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
*** [DEBUG] Loading Api::V1::LaneSerializer file ***
*** [DEBUG] Loading Api::V1::SaleListingSerializer file ***
=> Api::V1::SaleListingSerializer
irb(main):002:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
NameError: uninitialized constant Api::V1::LaneSerializer::SaleListingSerializer

When it goes loading in files to try to find the constant I asked for, it actually loads in the file that I want and returns the constant I want (though, admittedly, not the constant I asked for). The second time I attempt to constantize that string, it doesn't go loading files, and instead raises an uninitialized constant error.

Since constantize finds it the first time, looking up siblings "just worked" in my test environment. This is because ActiveModel Serializer was what loaded in the serializer, and it then cached the class that it found.

This lazy-loading of the files is what happens when eager loading is disabled. It's nice for a test environment (and is the Rails default) because it uses less resources and your app boots faster because it only loads in what it needs.

Eager loading will actually go through and load your app into memory on boot. This will make your app perform faster because it doesn't have to go looking for classes.

Here's a quote from the default config/environments/test.rb file:

# Do not eager load code on boot. This avoids loading your whole application
# just for the purpose of running a single test. If you are using a tool that
# preloads Rails for running tests, you may have to set it to true.
config.eager_load = false

Meanwhile, config/environments/production.rb says the following by default:

# Eager load code on boot. This eager loads most of Rails and
# your application in memory, allowing both threaded web servers
# and those relying on copy on write to perform better.
# Rake tasks automatically ignore this option for performance.
config.eager_load = true

Here's an example of what happens if I require the file and then constantize.

Loading development environment (Rails 4.2.0)
irb(main):001:0> require 'api/v1/sale_listing_serializer'
*** [DEBUG] Loading Api::V1::SaleListingSerializer file ***
=> true
irb(main):002:0> 'Api::V1::LaneSerializer::SaleListingSerializer'.constantize
*** [DEBUG] Loading Api::V1::LaneSerializer file ***
NameError: uninitialized constant Api::V1::LaneSerializer::SaleListingSerializer

So, in my case, as long as ActiveModelSerializers' serializer_lookup_chain is the first thing to load api/v1/sale_listing_serializer.rb into memory, then sibling lookup "just works" without monkey patching. But if it's not, then my app breaks in production and uses Rails' default to_json methods because it can't find the serializer class.

I literally had written all of my tests, and everything worked, and when I deployed, it was all broken.

This is why I think that it should look up siblings, too. Then, one of the constant names that it looks up will actually be the class I want.

Did you have any thoughts on this @beauby?

FWIW, all you need to do is put that monkey patch in an initializer. It works great now in prod and dev for us.

@randallb, I know you edited your post. Yes, I did put that in an initializer. I also did require 'active_model/serializer' before the monkey patch, for safety. (And I added a big comment in the initializer to explain what it's doing.)

Anyone wanna make a PR to document how to add this behavior?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kapso picture kapso  路  4Comments

JohnMerlino2 picture JohnMerlino2  路  3Comments

steverob picture steverob  路  4Comments

Tybot204 picture Tybot204  路  3Comments

yjukaku picture yjukaku  路  5Comments