Skip to content

Fix NameError on doorkeeper master by deferring AR model loading in run_hooks#241

Merged
nbulaj merged 1 commit into
masterfrom
copilot/fix-doorkeeper-gemfile-tests
Apr 13, 2026
Merged

Fix NameError on doorkeeper master by deferring AR model loading in run_hooks#241
nbulaj merged 1 commit into
masterfrom
copilot/fix-doorkeeper-gemfile-tests

Conversation

Copilot AI commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes failing tests when using doorkeeper_master.gemfile (doorkeeper ≥ 5.9.0).

Background

Doorkeeper #1804 (landed in v5.9.0) wrapped initialize_configured_associations inside ActiveSupport.on_load(:active_record) to prevent loading ActiveRecord models too early during the Rails boot process.

However, run_hooks in doorkeeper-openid_connect still called Doorkeeper.config.access_grant_model.prepend immediately during config.to_prepare, before ActiveRecord is fully initialized — causing:

NameError: uninitialized constant Doorkeeper::AccessGrant

Changes

lib/doorkeeper/openid_connect/orm/active_record.rb

Wraps the access_grant_model.prepend and establish_connection logic inside ActiveSupport.on_load(:active_record) in the run_hooks method, matching the pattern already used in initialize_models! and in doorkeeper's own updated initialize_configured_associations.

The existing version check is preserved:

  • doorkeeper ≥ 5.5.0 (including the new master): uses Doorkeeper.config.access_grant_model (the new configurable model API)
  • doorkeeper < 5.5.0 (older versions): falls back to Doorkeeper::AccessGrant directly

Copilot AI requested a review from nbulaj April 10, 2026 07:42
@55728 55728 mentioned this pull request Apr 10, 2026
@nbulaj nbulaj marked this pull request as ready for review April 13, 2026 10:05
@nbulaj nbulaj force-pushed the copilot/fix-doorkeeper-gemfile-tests branch from afe65ae to c5b220d Compare April 13, 2026 10:10

@nbulaj nbulaj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I added a few changes, now looks OK

@nbulaj nbulaj merged commit d60ec68 into master Apr 13, 2026
44 checks passed
@nbulaj nbulaj deleted the copilot/fix-doorkeeper-gemfile-tests branch May 20, 2026 11:29
@55728 55728 mentioned this pull request May 30, 2026
55728 added a commit to 55728/doorkeeper-openid_connect that referenced this pull request Jun 16, 2026
…eper-gem#306)

Namespaced custom access grant models (e.g. `Auth::OAuthAccessGrant <
ApplicationRecord`) crashed on Rails boot since v1.10.0 with
`NameError: uninitialized constant Auth::ApplicationRecord`.

v1.10.0 (doorkeeper-gem#241) wrapped the access-grant prepend in
`ActiveSupport.on_load(:active_record)`, following doorkeeper #1804.
doorkeeper reverted that approach in #1830 (v5.9.2): the load hook fires
while `ActiveRecord::Base` is first loaded — mid-evaluation of
`class ApplicationRecord < ActiveRecord::Base` — so constantizing a
namespaced grant model from the hook resolves `ApplicationRecord` before
`::ApplicationRecord` is registered, raising the NameError.

Follow doorkeeper #1830: drop the `run_hooks`/`initialize_models!`
overrides and the on_load block, and instead prepend an extension onto
the singleton class of `Doorkeeper::Orm::ActiveRecord::Mixins::AccessGrant`.
Its `included` callback adds the `openid_request` association when the
host model includes the mixin — at the model's own load time, with `base`
handed in by Ruby. Nothing constantizes the configured grant class, so
the re-entrant load window is gone. The string `class_name:` keeps the
association target resolved lazily by ActiveRecord. The prepend is
guarded with `base.is_a?(Class)`: when the mixin is included into an
intermediate ActiveSupport::Concern, the hook first fires with that
module as `base`, and the deferred include fires it again with the
actual model class.

The legacy `active_record_options[:establish_connection]` handling moves
into the OpenidRequest mixin's `included` block (guarded; a no-op on
doorkeeper 5.9.x, which no longer exposes `active_record_options`).

Adds regression coverage: a namespaced custom model that includes the
doorkeeper mixin is wired with the `openid_request` association.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants