Skip to content

Commit

Permalink
Fix loading all Tasks with namespace collision
Browse files Browse the repository at this point in the history
After upgrading from Rails 7 to 7.1, we started seeing an error when
trying to load the Maintenance Task UI:

```
undefined method `instance_of?` for #<BasicObject:... >
```

We ended up identifying the BasicObject as PRIMARY_KEY_NOT_SET, which
was added to Active Record in 7.1. The reason for attempting to load
this constant was that we have a model in our app named Maintenance
which inherits from ActiveRecord::Base, and so all of the constants
below ActiveRecord::Base get loaded by load_constants. (This clearly
isn't a common case, nor something I would recommend other applications
do).

To fix this, we can use the Rails application's loader to load all of
the application's tasks instead of traversing the constant tree. This
approach has multiple benefits:
- it will only load constants managed by the application's loader (which
  is the intention anyway)
- load_constants will be faster in production because the loader will
  recognize that the namespace has already been loaded and so will not
  have to traverse the tree

Since `#eager_load_namespace` was introduced in Zeitwerk 2.6.2 but the
minimum required by Rails is 2.6, the higher minimum version was added
to the gemspec as well.
  • Loading branch information
skipkayhil committed Dec 19, 2023
1 parent 280bf6d commit 735843d
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 7 deletions.
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ PATH
activerecord (>= 6.0)
job-iteration (>= 1.3.6)
railties (>= 6.0)
zeitwerk (>= 2.6.2)

GEM
remote: https://rubygems.org/
Expand Down
8 changes: 1 addition & 7 deletions app/models/maintenance_tasks/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,13 +187,7 @@ def load_constants
namespace = MaintenanceTasks.tasks_module.safe_constantize
return unless namespace

load_const = lambda do |root|
root.constants.each do |name|
object = root.const_get(name)
load_const.call(object) if object.instance_of?(Module)
end
end
load_const.call(namespace)
Rails.autoloaders.main.eager_load_namespace(namespace)
end
end

Expand Down
1 change: 1 addition & 0 deletions maintenance_tasks.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ Gem::Specification.new do |spec|
spec.add_dependency("activerecord", ">= 6.0")
spec.add_dependency("job-iteration", ">= 1.3.6")
spec.add_dependency("railties", ">= 6.0")
spec.add_dependency("zeitwerk", ">= 2.6.2")
end

0 comments on commit 735843d

Please sign in to comment.