Fragment Caching + ActiveModel::Serializer::Type

[bdmac]

I am using the type class method provided by ActiveModel::Serializer::Type to override the type for a certain serializer. This works fine with normal AMS caching. As soon as I add the except keyword on my cache call (triggering Fragment Caching), the class-level type setting in my serializer gets ignored and it reverts to inferring the type from the serialized object's class.

This does not seem to be expected behavior. I am trying to dig into FragmentCache adapter (I was surprised this was an adapter) to see what is going on. It looks like we dynamically create cached and non-cached serializers when fragment caching is invoked and my current best guess without having completed my investigation is that the class attribute type sets on the actual serializer is not being pulled forward into the generated fragment cache serializers... but I'm not sure yet.

[bdmac]

I believe I could fix this by modifying FragmentCache around L#97 to be:

cached.constantize.cache(klass._cache_options)
cached.constantize.type(klass._type)

The first line is already there to proxy forward the _cache_options but that is the only internal setting that is handled. I also see a few other possible problems similar to type that use serializer.class_attribute:

• _reflections
• _attributes_data
• _links

[bdmac]

Do not see a clean way to ensure that any serializer DSL settings such as type are brought forward in FragmentCache since none of them are setting any kind of common structure. If they were all being merged into something like dsl_options or whatever then we could set that on the cached serializer in FragmentCache.

[bdmac]

Actually the above almost works but not quite because I guess we wind up using deep_merge which is order dependent as to which hash's values win. Options seem to be either set it on both the cached and non-cached serializers OR only set it on the non-cached serializer which feels arbitrary.

[bdmac]

I have a fix for this in #1458

[beauby]

Thanks for looking into this @bdmac. I believe caching is indeed not in sync with the latest features added to AMS. I'm a bit busy currently but just wanted to let you know that I'll review #1458 asap.

[beauby]

#1458 seemd to have been merged. @bdmac, can you confirm the issue is resolved?