Don't break on ActionDispatch reload.

[sporkmonger]

Rails reloads stuff in development environments. When it does, it creates new classes in memory with the updated version of the class. Old references to these classes can potentially leak memory, but more importantly, they're stale, pointing to the old version of the code. The previous implementation kept direct references to the classes after any types involved in coercion had been reloaded. This just uses a String as the cache key instead. Reloads generally won't change the name of the class, so this should also be more reliable as a cache. And if a class name does change, it'll be an entirely new key with no collision issues. This does assume that the value side of the cache does not need to be reloaded – but since Rails won't typically auto-reload that code anyway, it shouldn't be an issue.

[sporkmonger]

It expected #<Examples::BookCollectionAttribute> to be an instance of Examples::BookCollectionAttribute... and that failed? Am I reading that wrong?

This code probably did cause that error, but I would argue that the problem here is that the cache should be being cleared for tests like this and right now there's no mechanism for clearing the cache. I'm also kind of curious how slow it is without the cache vs with the cache?

[solnic]

IIRC this cache is much more important in coercible than in Virtus. It'll be removed in Virtus 2.0 anyway as the whole const missing magic will be removed.

On 28 Nov 2014, at 13:12, Bob Aman [email protected] wrote:

It expected #Examples::BookCollectionAttribute to be an instance of Examples::BookCollectionAttribute... and that failed? Am I reading that wrong?

This code probably did cause that error, but I would argue that the problem here is that the cache should be being cleared for tests like this and right now there's no mechanism for clearing the cache. I'm also kind of curious how slow it is without the cache vs with the cache?

—
Reply to this email directly or view it on GitHub.

[sporkmonger]

Ah, interesting and good to know.

The scenario we're hitting issues with is in Grape, which uses Virtus for coercion. If you declare a Rails model as the type of a parameter, and build an attribute for coercing from a Hash into an unpersisted instance of that model, that's where you run into issues with this caching by class reference stuff in the development environment. It manifests as this lovely behavior where everything works great the first time you try, but after you make a code modification, suddenly Grape insists the previously valid Hash value is now invalid. Even if all you changed was a comment. Can't even tell you how long it took to track that one down. :-P