Repository navigation
Thin the injection layer: one universal discrimination seam - #14
Merged
Merged
Conversation
…stance_of seam Rails 6.0+ added a homogeneous fast path (instantiate_instance_of) that skips discriminate_class_for_record when the result lacks the STI inheritance_column; MTI discriminates on tableoid, so it was forced back onto the discriminating path by overriding _load_from_sql (7.0+) and *reimplementing* find_by_sql (6.x) behind an ActiveRecord.version gate. Both eras funnel through the same signature-stable instantiate_instance_of(klass, attributes, column_types, &block). Intercept that one method instead: re-route to `instantiate` when the row's tableoid maps to a different class. The `klass == self` guard matches only AR's homogeneous call, so post-discrimination re-entry terminates (and STI-within-MTI resolves via super). 5.x has no fast path -> feature detection skips the hook. Removes the version if/elsif, the find_by_sql reimplementation, and force_mti_instantiation: ~55 lines -> ~10, version-detection -> feature-detection. Verified green on AR 7.0 and 8.1 (order-independent).
Both the mutable Hash (Rails <= 7.1) and the read-only IndexedRow (7.2+) support #[], and the mti_class != self guard already terminates the recursion, so the respond_to?(:delete) branch (and the mutation it implied) is gone. Verified on AR 7.0 (Hash) and 8.1 (IndexedRow).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the version-gated discrimination dispatch with a single feature-detected
instantiate_instance_ofseam, and simplifiesdiscriminate_class_for_recordto read the oid via[]universally._load_from_sql(7.0+) / reimplemented-find_by_sql(6.x)ActiveRecord.versionsplit +force_mti_instantiation. AR 6.0+ funnel the homogeneous fast path through one signature-stable method; intercept it and re-route throughinstantiatewhen the row's tableoid maps to a different class.klass == selfbounds the recursion; 5.x (no fast path) is feature-detected out.[]; themti_class != selfguard already terminates recursion, so therespond_to?(:delete)branch is gone.Net -47 lines in core_extension.rb; version-detection -> feature-detection for the dispatch.
Verified locally: Rails 7.0/7.1/7.2/8.0/8.1 green + order-independent (26 seeds); composes clean with active_record-progresql. This PR runs the full 5.2->8.1 matrix.
(Note: dropping add_tableoid_column was attempted and reverted — its load_schema! hook is the eager-registration trigger, not cosmetic.)