-
Notifications
You must be signed in to change notification settings - Fork 451
Open
Labels
Description
We have an older app on Rail 4.1.16 and we noticed that our update queries suddenly started having the literal string id
stripped out of them. On some digging it turns out that there is a data race with initializing the value of @primary_key
. I haven't checked this behavior is head Rails
I was able to catch this with TracePoint and pry. Here is the backtrace from where the break condition fails:
From: /usr/local/bundle/gems/activerecord-4.1.16/lib/active_record/dynamic_matchers.rb @ line 38 ActiveRecord::DynamicMatchers::Method.match:
36: def match(model, name)
37: klass = matchers.find { |k| name =~ k.pattern }
=> 38: klass.new(model, name) if klass
39: end
[1] pry(ActiveRecord::DynamicMatchers::Method)> up
From: /usr/local/bundle/gems/activerecord-4.1.16/lib/active_record/dynamic_matchers.rb @ line 12 ActiveRecord::DynamicMatchers#respond_to?:
8: def respond_to?(name, include_private = false)
9: if self == Base
10: super
11: else
=> 12: match = Method.match(self, name)
13: match && match.valid? || super
14: end
15: end
[1] pry(MyObject)> up
From: /usr/local/bundle/gems/bullet-6.0.2/lib/bullet/ext/object.rb @ line 13 Object#bullet_primary_key_value:
8: def bullet_primary_key_value
9: return if respond_to?(:persisted?) && !persisted?
10:
11: if self.class.respond_to?(:primary_keys) && self.class.primary_keys
12: self.class.primary_keys.map { |primary_key| send primary_key }.join(',')
=> 13: elsif self.class.respond_to?(:primary_key) && self.class.primary_key
14: send self.class.primary_key
15: else
16: id
17: end
18: end
Steps to repro:
class Animal < ActiveRecord::Base
end
class Dog < Animal
end
# NOTE: a Dog record must exist
$ rails c
irb> Bullet.profile { Animal.first; Dog.respond_to?(:primary_key); Dog.primary_key }
The source of the issue seems to be that Dog.respond_to?
is initializing the @primary_key
instance variable in Dog as nil which is causing the lazy accessor #primary_key
to think that the primary key was already defined
jaysonvirissimo