(Fix submitted as GitHub PR #4641)

Overview

PR #4633 (commit 71351cd) can cause association classes to be prematurely loaded under some circumstances. If those classes interact apart from the associations themselves, this can cause classes to be incompletely defined. Further, 71351cd also eats any underlying NameError, making the problem hard unnecessarily hard to debug.

This is a breaking regression in 7.0.3 and was not present in 7.0.2. The original commit was an attempt to fix a separate regression in 7.0.2 (itself not present in 7.0.1).

Part 1 - Hiding NameError

The method relation_class_name (relatable.rb:174-184 [all line numbers are before to this PR]) eats NameError exceptions caused during resolve_name. Unfortunately, this consumes not only NameError's caused by missing objects, but also NameError's caused by typos or any other legitimate, but unresolvable (at the moment) reference in the middle of the class definition.

For example, if an unresolvable reference is 50% of the way through a model definition, that model class will end up only 50% defined, with everything in the latter half simply missing. The NameError will be eaten, so there's no evidence of what when wrong, nor where, until a method or other item from that latter half is used and found to be missing.

The case of 'unresolvable at the moment' is particularly interesting because it might be, for example, a constant reference in the parent class. Normally this is fine, but the premature loading issue (below) causes circular reference loading (of sorts), which means a dependent class may be defined when its parent class is only partially defined right then.

Part 2 - Premature loading

Part 1 is particularly relevant because the referenced commit also causes model definitions to be prematurely loaded as part of an association definition, hence making it possible for the rescue NameError to become a problem.

This is triggered within define_dependency! (depending.rb:59-71), specifically by the include? (:67), which checks equality of associations (== [relatable:76]), which uses relation_class_name.

Because it's checking for duplicate associations, it's only triggered when there are 2+ associations with :dependent and the model/class definition for the not-last association is a) unloaded prior to the current model, and b) triggers a NameError.

As noted above, a NameError could happen because premature loading has caused a child association to load while its parent association is in the middle of being defined (and not yet complete). If the parent association had finished loading first, before loading the child association, no NameError would have occurred.

Further, any other exception caused by premature loading is also a potential problem. It just wouldn't be hidden like NameError.

Example

We've experienced several nuances to this in a production app. I've distilled down a highly simplified example to show one way possible way to cause this.

https://github.com/zarqman/mongoid-autoload-test

This PR: #4641

The comments on relation_class_name expressly indicate that the class need not be defined yet. The final included note also indicates that "the return value is not always a fully qualified class name".

7.0.3 changed this to always return a fully qualified class name, and this is the source of the premature loading, as a class/model may be autoloaded as part fully qualifying the class name.

This PR restores the behavior to match the original comments. This successfully resolves the issue in our own full-featured apps, while preserving the scope-search that was added in the original commit.

In any instance where the fully qualified class name is still needed, it's possible to use relation_class.name instead.

Alternatives

1. It might be possible to instead remove the duplicate association check (depending.rb:67), assuming this is the only source of premature loading. Since any call to {{relation_class_name}}would trigger such a load, I'm not at all positive that is the case though. There could also be other problems caused by eliminating the dupe check.

2. Reverting both 71351cd and the commit it was attempting to fix. In essence, restoring class loading behavior to 7.0.1's behavior. Requiring certain module/namespace combinations to use the :class_name option seems acceptable. (In this regard, even 7.0.2's functionality was acceptable had it not been a breaking change in a patch release.)

If any further changes are to be made to loading/module identification, defer until the next major version, v8. Even then, I would then recommend attempting to use Ruby's natural class/module identification rather than reimplement it.

3. Rewrite association logic to defer most setup until first use, instead of at the time of definition. This would hopefully allow any/all circular class references and autoloading to be handled first. I am unsure whether this is practically possible or not.