Skip to content

Commit 7cec00f

Browse files
committed
Pull define_virtual_{include,arel} out of define_virtual_attribute
instead of defining arel and includes at runtime, we're defining them up front. Introducing the arel and includes up front simplifies the virtual attribute definition to just registering with the attributes api. If you note, the code only uses half of the attribute call, specifically `attribute_type`. This is causing problems in 7.2. We simplified it to here so it will be easier in the transition to 7.2
1 parent 9b5a57d commit 7cec00f

File tree

3 files changed

+16
-18
lines changed

3 files changed

+16
-18
lines changed

lib/active_record/virtual_attributes.rb

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,15 @@ def virtual_column(name, type:, **options)
5656
virtual_attribute(name, type, **options)
5757
end
5858

59-
def virtual_attribute(name, type, **options)
59+
def virtual_attribute(name, type, uses: nil, arel: nil, **options)
6060
name = name.to_s
6161
reload_schema_from_cache
6262

6363
self.virtual_attributes_to_define =
6464
virtual_attributes_to_define.merge(name => [type, options])
65+
66+
define_virtual_include(name, uses)
67+
define_virtual_arel(name, arel)
6568
end
6669

6770
#
@@ -91,20 +94,18 @@ def load_schema!
9194

9295
virtual_attributes_to_define.each do |name, (type, options)|
9396
type = type.call if type.respond_to?(:call)
94-
type = ActiveRecord::Type.lookup(type, **options.except(:uses, :arel)) if type.kind_of?(Symbol)
97+
type = ActiveRecord::Type.lookup(type, **options) if type.kind_of?(Symbol)
9598

96-
define_virtual_attribute(name, type, **options.slice(:uses, :arel))
99+
define_virtual_attribute(name, type)
97100
end
98101

99102
virtual_delegates_to_define.each do |method_name, (method, options)|
100103
define_virtual_delegate(method_name, method, options)
101104
end
102105
end
103106

104-
def define_virtual_attribute(name, cast_type, uses: nil, arel: nil)
107+
def define_virtual_attribute(name, cast_type)
105108
attribute_types[name.to_s] = cast_type
106-
define_virtual_include(name, uses)
107-
define_virtual_arel(name, arel)
108109
end
109110
end
110111
end

lib/active_record/virtual_attributes/virtual_delegates.rb

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ module ClassMethods
1818
# Definition
1919
#
2020

21-
def virtual_delegate(*methods, to:, type:, prefix: nil, allow_nil: nil, default: nil, **options) # rubocop:disable Naming/MethodParameterName
21+
def virtual_delegate(*methods, to:, type:, prefix: nil, allow_nil: nil, default: nil, uses: nil, **options) # rubocop:disable Naming/MethodParameterName
2222
to = to.to_s
2323
if to.include?(".") && (methods.size > 1 || prefix)
2424
raise ArgumentError, 'Delegation only supports specifying a target method name when defining a single virtual method with no prefix'
@@ -32,8 +32,13 @@ def virtual_delegate(*methods, to:, type:, prefix: nil, allow_nil: nil, default:
3232
# This better supports reloading of the class and changing the definitions
3333
methods.each do |method|
3434
method_name, to, method = determine_method_names(method, to, prefix)
35-
define_delegate(method_name, method, :to => to, :allow_nil => allow_nil, :default => default)
35+
unless (to_ref = reflection_with_virtual(to))
36+
raise ArgumentError, "Delegation needs an association. Association #{to} does not exist"
37+
end
3638

39+
define_delegate(method_name, method, :to => to, :allow_nil => allow_nil, :default => default)
40+
define_virtual_include(method_name, uses || to)
41+
define_virtual_arel(method_name, virtual_delegate_arel(method, to_ref))
3742
self.virtual_delegates_to_define =
3843
virtual_delegates_to_define.merge(method_name.to_s => [method, options.merge(:to => to, :type => type)])
3944
end
@@ -52,16 +57,10 @@ def virtual_delegate(*methods, to:, type:, prefix: nil, allow_nil: nil, default:
5257
# @option options :uses [Array|Symbol|Hash] sql includes hash. (default: to)
5358
# @option options :type [Symbol|ActiveModel::Type::Value] type for the attribute
5459
def define_virtual_delegate(method_name, col, options)
55-
unless (to = options[:to]) && (to_ref = reflection_with_virtual(to.to_s))
56-
raise ArgumentError, 'Delegation needs an association. Supply an options hash with a :to key as the last argument (e.g. delegate :hello, to: :greeter).'
57-
end
58-
59-
col = col.to_s
6060
type = options[:type]
6161
type = ActiveRecord::Type.lookup(type) if type.kind_of?(Symbol)
6262

63-
arel = virtual_delegate_arel(col, to_ref)
64-
define_virtual_attribute(method_name, type, :uses => (options[:uses] || to), :arel => arel)
63+
define_virtual_attribute(method_name, type)
6564
end
6665

6766
# see activesupport module/delegation.rb

spec/virtual_delegates_spec.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,7 @@ def self.connection
241241
end
242242

243243
it "catches invalid references" do
244-
TestOtherClass.virtual_delegate :col4, :to => :others, :type => :integer
245-
246-
expect { model.new }.to raise_error(NameError)
244+
expect { TestOtherClass.virtual_delegate :col4, :to => :others, :type => :integer }.to raise_error(ArgumentError)
247245
end
248246

249247
it "catches invalid column" do

0 commit comments

Comments
 (0)