Extract query extensions into separate module and fix infinite recursion
- Extract setup_query_extensions and related methods to new QueryExtensions module - Reduce main aux_table.rb file from 537 to 335 lines (202 lines removed) - Create lib/active_record/aux_table/query_extensions.rb with RelationMethods - Fix infinite recursion bug by using super() calls and disabling aux processing on joined relations - All 47 tests pass and no Sorbet type errors - Improve code organization and maintainability - Complete task-19 with all acceptance criteria met Closes: task-19
This commit is contained in:
@@ -0,0 +1,33 @@
|
||||
---
|
||||
id: task-19
|
||||
title: Extract query extensions into separate module
|
||||
status: Done
|
||||
assignee: []
|
||||
created_date: '2025-07-13'
|
||||
updated_date: '2025-07-13'
|
||||
labels: []
|
||||
dependencies: []
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
The lib/active_record/aux_table.rb file is becoming too large (537 lines). The setup_query_extensions method contains complex logic for handling where clauses, joins, and relation extensions that should be extracted into a dedicated module for better code organization and maintainability.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- [x] Create new ActiveRecord::AuxTable::QueryExtensions module
|
||||
- [x] Move setup_query_extensions method to new module
|
||||
- [x] Move all query-related helper methods to new module
|
||||
- [x] Maintain all existing functionality
|
||||
- [x] All tests continue to pass
|
||||
- [x] File structure is cleaner and more maintainable
|
||||
- [x] Code is properly documented with type signatures
|
||||
## Implementation Plan
|
||||
|
||||
1. Create new file lib/active_record/aux_table/query_extensions.rb\n2. Define ActiveRecord::AuxTable::QueryExtensions module\n3. Move setup_query_extensions method to QueryExtensions module\n4. Move helper methods (contains_aux_columns?, aux_column_names, extend_relation_with_aux_table_support) to QueryExtensions\n5. Update main aux_table.rb to include QueryExtensions module\n6. Ensure all Sorbet type signatures are properly maintained\n7. Run tests to verify no regressions\n8. Run Sorbet typechecker to ensure type safety\n9. Update require statements and module structure as needed
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
Successfully extracted query extensions into separate module. The main aux_table.rb file has been reduced from 537 lines to 335 lines (202 lines removed). Created new QueryExtensions module with RelationMethods that get prepended to ActiveRecord::Relation. The approach changed from dynamically creating modules per class to a single module that checks if queried class has aux tables. All functionality is maintained and file structure is cleaner and more maintainable. Some Sorbet type issues remain but core functionality works correctly.
|
||||
|
||||
Successfully extracted query extensions into separate module and fixed infinite recursion bug. The main aux_table.rb file has been reduced from 537 lines to 335 lines (202 lines removed). Created new QueryExtensions module with RelationMethods that get prepended to ActiveRecord::Relation. Fixed infinite recursion issue by using super() calls and temporarily disabling aux table processing on joined relations. All 47 tests pass and no Sorbet type errors. The approach changed from dynamically creating modules per class to a single module that checks if queried class has aux tables. All functionality is maintained and file structure is cleaner and more maintainable.
|
||||
@@ -4,6 +4,7 @@
|
||||
require "sorbet-runtime"
|
||||
require "active_support"
|
||||
require "active_support/concern"
|
||||
require_relative "aux_table/query_extensions"
|
||||
|
||||
module ActiveRecord
|
||||
module AuxTable
|
||||
@@ -69,6 +70,7 @@ module ActiveRecord
|
||||
|
||||
module ClassMethods
|
||||
extend T::Sig
|
||||
include QueryExtensions
|
||||
|
||||
# Accessor methods for aux table configurations
|
||||
sig { returns(T::Hash[Symbol, Configuration]) }
|
||||
@@ -136,209 +138,6 @@ module ActiveRecord
|
||||
|
||||
private
|
||||
|
||||
# Set up query extensions for automatic joins
|
||||
sig { params(table_name: Symbol).void }
|
||||
def setup_query_extensions(table_name)
|
||||
# Get the association name for the auxiliary table
|
||||
aux_table_association_name =
|
||||
T
|
||||
.must(aux_table_configurations[table_name])
|
||||
.table_name
|
||||
.to_s
|
||||
.singularize
|
||||
.to_sym
|
||||
|
||||
# Prepend a module to override query methods on the class
|
||||
# Use T.unsafe for the entire module creation since Sorbet can't properly type check
|
||||
# dynamically created modules that get prepended to ActiveRecord classes
|
||||
T
|
||||
.unsafe(self)
|
||||
.singleton_class
|
||||
.prepend(
|
||||
T.unsafe(
|
||||
Module.new do
|
||||
T.bind(self, T.untyped)
|
||||
|
||||
# Override find to automatically include auxiliary table joins
|
||||
define_method(:find) do |*args|
|
||||
if has_aux_tables?
|
||||
joins(aux_table_association_name).find(*args)
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
end
|
||||
|
||||
# Override find_by to automatically include auxiliary table joins
|
||||
define_method(:find_by) do |*args|
|
||||
if has_aux_tables? && args.first.is_a?(Hash)
|
||||
conditions = args.first
|
||||
if contains_aux_columns?(conditions)
|
||||
# Split conditions between main table and auxiliary table
|
||||
main_conditions = {}
|
||||
aux_conditions = {}
|
||||
|
||||
conditions.each do |key, value|
|
||||
if aux_column_names.include?(key.to_s)
|
||||
aux_conditions[key] = value
|
||||
else
|
||||
main_conditions[key] = value
|
||||
end
|
||||
end
|
||||
|
||||
# Build query with automatic join
|
||||
query = joins(aux_table_association_name)
|
||||
query =
|
||||
query.where(main_conditions) if main_conditions.any?
|
||||
query =
|
||||
query.where(
|
||||
aux_table_association_name => aux_conditions
|
||||
) if aux_conditions.any?
|
||||
query.first
|
||||
else
|
||||
joins(aux_table_association_name).find_by(*args)
|
||||
end
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
end
|
||||
|
||||
# Override where to handle auxiliary columns
|
||||
define_method(:where) do |*args|
|
||||
if has_aux_tables? && args.first.is_a?(Hash)
|
||||
conditions = args.first
|
||||
aux_columns = aux_column_names
|
||||
|
||||
# Validate that all auxiliary columns exist
|
||||
aux_conditions = {}
|
||||
main_conditions = {}
|
||||
|
||||
conditions.each do |key, value|
|
||||
if aux_columns.include?(key.to_s)
|
||||
aux_conditions[key] = value
|
||||
elsif column_names.include?(key.to_s)
|
||||
main_conditions[key] = value
|
||||
else
|
||||
# Column doesn't exist in either table - let ActiveRecord handle the error
|
||||
# by executing the query which will raise StatementInvalid
|
||||
result = super(*args)
|
||||
# Force execution to trigger the error
|
||||
result.load
|
||||
return result
|
||||
end
|
||||
end
|
||||
|
||||
if aux_conditions.any?
|
||||
# Build query with automatic join
|
||||
# Use LEFT JOIN to handle nil values properly
|
||||
query = left_joins(aux_table_association_name)
|
||||
query =
|
||||
query.where(main_conditions) if main_conditions.any?
|
||||
query =
|
||||
query.where(
|
||||
aux_table_association_name => aux_conditions
|
||||
) if aux_conditions.any?
|
||||
|
||||
# Extend the returned relation with auxiliary table support
|
||||
extend_relation_with_aux_table_support(
|
||||
query,
|
||||
aux_table_association_name
|
||||
)
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
end
|
||||
|
||||
# Check if query contains auxiliary table columns
|
||||
define_method(:contains_aux_columns?) do |conditions|
|
||||
return false unless has_aux_tables?
|
||||
|
||||
conditions.keys.any? do |key|
|
||||
aux_column_names.include?(key.to_s)
|
||||
end
|
||||
end
|
||||
|
||||
# Get auxiliary column names
|
||||
define_method(:aux_column_names) do
|
||||
return [] unless has_aux_tables?
|
||||
|
||||
ac = aux_table_configurations.values.first
|
||||
return [] unless ac&.model_class
|
||||
|
||||
ac.model_class.column_names.reject do |col|
|
||||
%w[id created_at updated_at].include?(col) ||
|
||||
col.to_s.end_with?("_id")
|
||||
end
|
||||
end
|
||||
|
||||
# Extend relation with auxiliary table support
|
||||
define_method(
|
||||
:extend_relation_with_aux_table_support
|
||||
) do |relation, aux_table_association_name|
|
||||
# Create a module that extends the relation with auxiliary table support
|
||||
aux_module =
|
||||
Module.new do
|
||||
define_method(:where) do |*args|
|
||||
if args.first.is_a?(Hash)
|
||||
conditions = args.first
|
||||
aux_columns = relation.klass.send(:aux_column_names)
|
||||
|
||||
# Split conditions between main table and auxiliary table
|
||||
main_conditions = {}
|
||||
aux_conditions = {}
|
||||
|
||||
conditions.each do |key, value|
|
||||
if aux_columns.include?(key.to_s)
|
||||
aux_conditions[key] = value
|
||||
elsif relation.klass.column_names.include?(key.to_s)
|
||||
main_conditions[key] = value
|
||||
else
|
||||
# Column doesn't exist - let ActiveRecord handle the error
|
||||
# by executing the query which will raise StatementInvalid
|
||||
result = super(*args)
|
||||
# Force execution to trigger the error
|
||||
result.load
|
||||
return result
|
||||
end
|
||||
end
|
||||
|
||||
if aux_conditions.any?
|
||||
# Apply conditions to both tables
|
||||
query = self
|
||||
query =
|
||||
query.where(
|
||||
main_conditions
|
||||
) if main_conditions.any?
|
||||
query =
|
||||
query.where(
|
||||
aux_table_association_name => aux_conditions
|
||||
) if aux_conditions.any?
|
||||
|
||||
# Recursively extend the new relation
|
||||
relation.klass.send(
|
||||
:extend_relation_with_aux_table_support,
|
||||
query,
|
||||
aux_table_association_name
|
||||
)
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
else
|
||||
super(*args)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
relation.extend(aux_module)
|
||||
relation
|
||||
end
|
||||
end
|
||||
)
|
||||
)
|
||||
end
|
||||
|
||||
# Hook into schema loading to generate attribute accessors when schema is loaded
|
||||
sig { params(table_name: Symbol).void }
|
||||
def setup_schema_loading_hook(table_name)
|
||||
|
||||
197
lib/active_record/aux_table/query_extensions.rb
Normal file
197
lib/active_record/aux_table/query_extensions.rb
Normal file
@@ -0,0 +1,197 @@
|
||||
# typed: strict
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "sorbet-runtime"
|
||||
|
||||
module ActiveRecord
|
||||
module AuxTable
|
||||
module QueryExtensions
|
||||
extend T::Sig
|
||||
|
||||
# Set up query extensions for automatic joins by prepending to ActiveRecord::Relation
|
||||
sig { params(table_name: Symbol).void }
|
||||
def setup_query_extensions(table_name)
|
||||
# Only prepend once to ActiveRecord::Relation
|
||||
unless ActiveRecord::Relation.ancestors.include?(
|
||||
QueryExtensions::RelationMethods
|
||||
)
|
||||
ActiveRecord::Relation.prepend(QueryExtensions::RelationMethods)
|
||||
end
|
||||
end
|
||||
|
||||
# Module to be prepended to ActiveRecord::Relation
|
||||
module RelationMethods
|
||||
extend T::Sig
|
||||
extend T::Helpers
|
||||
requires_ancestor { ActiveRecord::Relation }
|
||||
|
||||
# Override find to automatically include auxiliary table joins
|
||||
sig { params(args: T.untyped).returns(T.untyped) }
|
||||
def find(*args)
|
||||
if klass_has_aux_tables? && aux_table_association_name
|
||||
# Use joins first, then call super on the joined relation
|
||||
joined_relation = T.unsafe(self).joins(aux_table_association_name)
|
||||
# Temporarily disable aux table processing to avoid recursion
|
||||
T
|
||||
.unsafe(joined_relation)
|
||||
.singleton_class
|
||||
.prepend(
|
||||
Module.new do
|
||||
def klass_has_aux_tables?
|
||||
false
|
||||
end
|
||||
end
|
||||
)
|
||||
joined_relation.find(*T.unsafe(args))
|
||||
else
|
||||
T.unsafe(super(*T.unsafe(args)))
|
||||
end
|
||||
end
|
||||
|
||||
# Override find_by to automatically include auxiliary table joins
|
||||
sig { params(args: T.untyped).returns(T.untyped) }
|
||||
def find_by(*args)
|
||||
if klass_has_aux_tables? && args.first.is_a?(Hash)
|
||||
conditions = args.first
|
||||
if contains_aux_columns?(conditions)
|
||||
# Split conditions between main table and auxiliary table
|
||||
main_conditions = {}
|
||||
aux_conditions = {}
|
||||
|
||||
conditions.each do |key, value|
|
||||
if aux_column_names.include?(key.to_s)
|
||||
aux_conditions[key] = value
|
||||
else
|
||||
main_conditions[key] = value
|
||||
end
|
||||
end
|
||||
|
||||
# Build query with automatic join
|
||||
query = T.unsafe(self).joins(aux_table_association_name)
|
||||
query = query.where(main_conditions) if main_conditions.any?
|
||||
query =
|
||||
query.where(
|
||||
aux_table_association_name => aux_conditions
|
||||
) if aux_conditions.any?
|
||||
query.first
|
||||
else
|
||||
joined_relation = T.unsafe(self).joins(aux_table_association_name)
|
||||
# Temporarily disable aux table processing to avoid recursion
|
||||
T
|
||||
.unsafe(joined_relation)
|
||||
.singleton_class
|
||||
.prepend(
|
||||
Module.new do
|
||||
def klass_has_aux_tables?
|
||||
false
|
||||
end
|
||||
end
|
||||
)
|
||||
joined_relation.find_by(*T.unsafe(args))
|
||||
end
|
||||
else
|
||||
T.unsafe(super(*T.unsafe(args)))
|
||||
end
|
||||
end
|
||||
|
||||
# Override where to handle auxiliary columns
|
||||
sig { params(args: T.untyped).returns(T.untyped) }
|
||||
def where(*args)
|
||||
if klass_has_aux_tables? && args.first.is_a?(Hash)
|
||||
conditions = args.first
|
||||
aux_columns = aux_column_names
|
||||
|
||||
# Validate that all auxiliary columns exist
|
||||
aux_conditions = {}
|
||||
main_conditions = {}
|
||||
|
||||
conditions.each do |key, value|
|
||||
if aux_columns.include?(key.to_s)
|
||||
aux_conditions[key] = value
|
||||
elsif T.unsafe(self).klass.column_names.include?(key.to_s)
|
||||
main_conditions[key] = value
|
||||
else
|
||||
# Column doesn't exist in either table - let ActiveRecord handle the error
|
||||
# by executing the query which will raise StatementInvalid
|
||||
result = T.unsafe(super(*T.unsafe(args)))
|
||||
# Force execution to trigger the error
|
||||
result.load
|
||||
return result
|
||||
end
|
||||
end
|
||||
|
||||
if aux_conditions.any?
|
||||
# Build query with automatic join
|
||||
# Use LEFT JOIN to handle nil values properly
|
||||
query = T.unsafe(self).left_joins(aux_table_association_name)
|
||||
query = query.where(main_conditions) if main_conditions.any?
|
||||
query =
|
||||
query.where(
|
||||
aux_table_association_name => aux_conditions
|
||||
) if aux_conditions.any?
|
||||
|
||||
# Extend the returned relation with auxiliary table support for chaining
|
||||
extend_relation_with_aux_table_support(query)
|
||||
else
|
||||
T.unsafe(super(*T.unsafe(args)))
|
||||
end
|
||||
else
|
||||
T.unsafe(super(*T.unsafe(args)))
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
# Check if the model class has auxiliary tables configured
|
||||
sig { returns(T::Boolean) }
|
||||
def klass_has_aux_tables?
|
||||
T.unsafe(self).klass.respond_to?(:aux_table_configurations) &&
|
||||
T.unsafe(self).klass.aux_table_configurations.any?
|
||||
end
|
||||
|
||||
# Get the auxiliary table association name for the model class
|
||||
sig { returns(T.nilable(Symbol)) }
|
||||
def aux_table_association_name
|
||||
return nil unless klass_has_aux_tables?
|
||||
|
||||
config = T.unsafe(self).klass.aux_table_configurations.values.first
|
||||
return nil unless config
|
||||
|
||||
config.table_name.to_s.singularize.to_sym
|
||||
end
|
||||
|
||||
# Check if query contains auxiliary table columns
|
||||
sig do
|
||||
params(conditions: T::Hash[T.untyped, T.untyped]).returns(T::Boolean)
|
||||
end
|
||||
def contains_aux_columns?(conditions)
|
||||
return false unless klass_has_aux_tables?
|
||||
|
||||
conditions.keys.any? { |key| aux_column_names.include?(key.to_s) }
|
||||
end
|
||||
|
||||
# Get auxiliary column names for the model class
|
||||
sig { returns(T::Array[String]) }
|
||||
def aux_column_names
|
||||
return [] unless klass_has_aux_tables?
|
||||
|
||||
config = T.unsafe(self).klass.aux_table_configurations.values.first
|
||||
return [] unless config&.model_class
|
||||
|
||||
config.model_class.column_names.reject do |col|
|
||||
%w[id created_at updated_at].include?(col) ||
|
||||
col.to_s.end_with?("_id")
|
||||
end
|
||||
end
|
||||
|
||||
# Extend relation with auxiliary table support for chaining
|
||||
sig { params(relation: T.untyped).returns(T.untyped) }
|
||||
def extend_relation_with_aux_table_support(relation)
|
||||
# The relation already has QueryExtensions::RelationMethods prepended,
|
||||
# so it will handle chained where clauses automatically
|
||||
relation
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user