bullet icon indicating copy to clipboard operation
bullet copied to clipboard

nil primary_key with single-table inheritance

Open jbodah opened this issue 5 years ago • 1 comments

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

jbodah avatar Sep 03 '19 22:09 jbodah

I ran into the same issue and it was not related to this gem, it's related to composite_primary_keys gem https://github.com/composite-primary-keys/composite_primary_keys.

When bullet runs this extension https://github.com/flyerhzm/bullet/blob/master/lib/bullet/ext/object.rb it calls self.class.primary_keys and for some reason (you can follow the stacktrace) in that logic composite_primary_keys is doing self.class.primary_key = keys if keys is not an Array just for inherited classes, in that case keys is nil so self.class.primary_key become nil. Then we run into many issues because primary_key from our inherited classes is nil.

Instead of messing with composite_primary_keys I forked and monkey patched the bullet object extension to recover from this bug:

def primary_key_value
  class_primary_key = self.class.primary_key if self.class.respond_to?(:primary_key)

  if self.class.respond_to?(:primary_keys) && self.class.primary_keys
    return self.class.primary_keys.map { |primary_key| self.send primary_key }.join(',')
  end

  if self.class.respond_to?(:primary_key)
    self.class.primary_key ||= class_primary_key
    return self.send(self.class.primary_key) if self.class.primary_key
  end

  self.id
end

I hope this can help you...

efrenvalfonso avatar Jul 27 '22 19:07 efrenvalfonso