Reifier#reify raise NameError with overridden find_sti_class

[moyashipan]

• [x] This is not a usage question, this is a bug report
• [x] This bug can be reproduced with the script I provide below
• [x] This bug can be reproduced in the latest release of the paper_trail gem

# frozen_string_literal: true

# Use this template to report PaperTrail bugs.
# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "2.6.6"
  source "https://rubygems.org"
  gem "activerecord", "5.2.4.3"
  gem "minitest", "5.14.2"
  gem "paper_trail", "11.0.0", require: false
  gem "sqlite3", "1.4.2"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Please use sqlite for your bug reports, if possible.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = nil
ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :animals, force: true do |t|
    t.string :type, null: false
    t.string :name
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.text :object, limit: 1_073_741_823
    t.text :object_changes, limit: 1_073_741_823
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail"

# STEP FOUR: Define your AR models here.
class Animal < ActiveRecord::Base
  has_paper_trail

  class << self
    def find_sti_class(type_name)
      type_name.camelize.constantize
    end

    def sti_name
      name.underscore
    end
  end
end

class Cat < Animal
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_1
    cat = nil
    assert_difference(-> { PaperTrail::Version.count }, +1) {
      cat = Cat.create
    }
    assert_difference(-> { PaperTrail::Version.count }, +1) {
      cat.update(name: 'Tanjiro')
    }
    assert_nothing_raised do
      PaperTrail::Version.find(1).next.reify
    end
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`

  1) Error:
BugTest#test_1:
NameError: wrong constant name cat
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/inflector/methods.rb:283:in `const_get'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/inflector/methods.rb:283:in `block in constantize'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/inflector/methods.rb:281:in `each'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/inflector/methods.rb:281:in `inject'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/inflector/methods.rb:281:in `constantize'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/core_ext/string/inflections.rb:68:in `constantize'
    /usr/local/bundle/gems/paper_trail-11.0.0/lib/paper_trail/reifier.rb:126:in `version_reification_class'
    /usr/local/bundle/gems/paper_trail-11.0.0/lib/paper_trail/reifier.rb:55:in `init_model'
    /usr/local/bundle/gems/paper_trail-11.0.0/lib/paper_trail/reifier.rb:15:in `reify'
    /usr/local/bundle/gems/paper_trail-11.0.0/lib/paper_trail/version_concern.rb:232:in `reify'
    my_bug_report.rb:75:in `block in test_1'
    /usr/local/bundle/gems/activesupport-5.2.4.3/lib/active_support/testing/assertions.rb:32:in `assert_nothing_raised'
    my_bug_report.rb:74:in `test_1'

1 runs, 2 assertions, 0 failures, 1 errors, 0 skips

https://github.com/paper-trail-gem/paper_trail/blob/294b7188e95c288b212d997b3a6b4e3759dadf43/lib/paper_trail/reifier.rb#L122-L127

I think the above code should be below.

def version_reification_class(version, attrs)
  klass = version.item_type.constantize
  inher_col_value = attrs[klass.inheritance_column]
  inher_col_value.blank? ? klass : klass.find_sti_class(inher_col_value)
end

And then

1 runs, 2 assertions, 0 failures, 0 errors, 0 skips

See also: https://github.com/rails/rails/blob/90216abe121aa9da12e9338722f8f25d252dcf75/activerecord/lib/active_record/inheritance.rb#L246-L250

[tlynam]

Hi @moyashipan, thank you for submitting this bug report! I'm curious are find_sti_class and sti_name private methods? Does this work using inheritance_column?

[moyashipan]

Hi @tlynam, thank you for your reply.

I'm curious are find_sti_class and sti_name private methods?

Only find_sti_class is private method. Overriding of find_sti_class is a way known for a long time. https://stackoverflow.com/questions/23293177/rails-sti-how-to-change-mapping-between-class-name-value-of-the-type-column

Yep, I missed it. My code must be like this:

 def version_reification_class(version, attrs)
   klass = version.item_type.constantize
   inher_col_value = attrs[klass.inheritance_column]
-  inher_col_value.blank? ? klass : klass.find_sti_class(inher_col_value)
+  inher_col_value.blank? ? klass : klass.send(:find_sti_class, inher_col_value)
 end

However, In Rails 6.1, sti_class_for will be added and we will be able to call the public method to get STI Class. https://github.com/rails/rails/pull/37500/commits/3e2822684f90bfc03f35e0a172a60175a62102d7

Does this work using inheritance_column?

Yes. My new suggested code works fine with or without overriding find_sti_code and inheritance_column.

script with overridden inheritance_column

```ruby # frozen_string_literal: true # Use this template to report PaperTrail bugs. # Please include only the minimum code necessary to reproduce your issue. require "bundler/inline" # STEP ONE: What versions are you using? gemfile(true) do ruby "2.6.6" source "https://rubygems.org" gem "activerecord", "5.2.4.3" gem "minitest", "5.14.2" gem "paper_trail", "11.0.0", require: false gem "sqlite3", "1.4.2" end require "active_record" require "minitest/autorun" require "logger" # Please use sqlite for your bug reports, if possible. ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:") ActiveRecord::Base.logger = nil ActiveRecord::Schema.define do # STEP TWO: Define your tables here. create_table :animals, force: true do |t| t.string :my_type, null: false # <<<<<<<<<<<<<<<<<<<< HERE (1/2) t.string :name t.timestamps null: false end create_table :versions do |t| t.string :item_type, null: false t.integer :item_id, null: false t.string :event, null: false t.string :whodunnit t.text :object, limit: 1_073_741_823 t.text :object_changes, limit: 1_073_741_823 t.datetime :created_at end add_index :versions, %i[item_type item_id] end ActiveRecord::Base.logger = Logger.new(STDOUT) require "paper_trail" # STEP FOUR: Define your AR models here. class Animal < ActiveRecord::Base has_paper_trail self.inheritance_column = :my_type # <<<<<<<<<<<<<<<<<<<< HERE (2/2) class << self def find_sti_class(type_name) type_name.camelize.constantize end def sti_name name.underscore end end end class Cat < Animal end # STEP FIVE: Please write a test that demonstrates your issue. class BugTest < ActiveSupport::TestCase def test_1 cat = nil assert_difference(-> { PaperTrail::Version.count }, +1) { cat = Cat.create } assert_difference(-> { PaperTrail::Version.count }, +1) { cat.update(name: 'Tanjiro') } assert_nothing_raised do PaperTrail::Version.find(1).next.reify end end end # STEP SIX: Run this script using `ruby my_bug_report.rb` ```

[tlynam]

Thank you @moyashipan for the explanation and updated script!

So you're proposing we implement this fix? I'm curious why you chose klass.send(:find_sti_class, inher_col_value) compared to klass.find_sti_class(inher_col_value)?

      def version_reification_class(version, attrs)
        klass = version.item_type.constantize
        inher_col_value = attrs[klass.inheritance_column]
        inher_col_value.blank? ? klass : klass.send(:find_sti_class, inher_col_value)
      end

[github-actions[bot]]

This issue has been automatically marked as stale due to inactivity. The resources of our volunteers are limited. Bug reports must provide a script that reproduces the bug, using our template. Feature suggestions must include a promise to build the feature yourself. Thank you for all your contributions.