ActiveRecord woe - implicit order and association

lzap · April 29, 2020, 3:40pm

Hey,

I’ve spent the whole day debugging this:

> DiscoveryRule.first.hosts.first
undefined local variable or method `implicit_order_column' for #<ActiveRecord::Associations::CollectionProxy []>
 Backtrace for 'Action failed' error (ActionView::Template::Error): undefined local variable or method `implicit_order_column' for #<ActiveRecord::Associations::CollectionProxy []>
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/relation/delegation.rb:108:in `method_missing'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/relation/finder_methods.rb:554:in `ordered_relation'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/relation/finder_methods.rb:520:in `find_nth_with_limit'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/associations/collection_proxy.rb:1107:in `find_nth_with_limit'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/relation/finder_methods.rb:513:in `find_nth'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.1/lib/active_record/relation/finder_methods.rb:120:in `first'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/helpers.rb:41:in `data_name'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:173:in `child'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:116:in `block in compile_settings'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:115:in `each'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:115:in `compile_settings'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:36:in `engines'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:121:in `merge_engines_into_result'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:57:in `block in to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:252:in `cache_results'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:51:in `to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:90:in `to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:50:in `block in render'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:385:in `cache_results'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:49:in `render'
 | /home/lzap/work/foreman/config/initializers/rabl_init.rb:49:in `render'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:129:in `block in merge_engines_into_result'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:121:in `each'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:121:in `merge_engines_into_result'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:57:in `block in to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:252:in `cache_results'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/builder.rb:51:in `to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/multi_builder.rb:27:in `map'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/multi_builder.rb:27:in `to_a'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:92:in `to_hash'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:103:in `to_dumpable'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:112:in `to_json'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:50:in `block in render'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:385:in `cache_results'
 | /home/lzap/.rbenv/versions/2.6.3/lib/ruby/gems/2.6.0/gems/rabl-0.14.2/lib/rabl/engine.rb:49:in `render'
 | /home/lzap/work/foreman/config/initializers/rabl_init.rb:49:in `render'
 | /home/lzap/work/foreman_discovery/app/views/api/v2/discovery_rules/index.json.rabl:3:in `_0c11c3c03ea8ac65e99adfb1b2e500c2'

Rails 6 added new model class attribute implicit_order_column where you can specify default:

class User < ApplicationRecord
  self.implicit_order_column = "updated_at"
end

Problem is, there is some misbehavior with this when used with DiscoveredHost. Probably STI has something to do with this, but I’ve been unable to simulate this in pure Rails. It must be some monkey patching or order of dependencies loaded.

Now the funny part, when I put PRY to the place in RABL where it calls the first method - wait for it - it works just fine!

          if object_name.nil? && data.respond_to?(:first)
--          #require "pry"; binding.pry
            first = data.first
            object_name = data_name(first).to_s.pluralize if first.present?
          end

When I comment it out, it shows the error again. Someone more clever than me help me with this.

ezr-ondrej · April 29, 2020, 4:55pm

I’ve debug this and found an issue. As the relation is has_many :hosts Rails automatically assumes the association class being Host then Rails for relations is being clever with class methods and it asks the class if it responds to the method here: https://github.com/rails/rails/blob/92d03850f3bb4c44103d0b06b43e14d6e270e646/activerecord/lib/active_record/relation/delegation.rb#L105

But as our Host class doesn’t inherit from ActiveRecord::Base it says nope, and Rails fails.

I’ve opened a PR that solves it in this case:

github.com/theforeman/foreman_discovery

Fixes #29658 - discovery_rule hosts association on Rails 6

theforeman:develop ← ezr-ondrej:discovery_rule_rails6

opened 04:51PM - 29 Apr 20 UTC

ezr-ondrej

+1 -1

@lzap as of https://community.theforeman.org/t/activerecord-woe-implicit-order-a…nd-association/18492 this solves it, but there might be other places. I would love if you could think of getting rid of `Host` factory in favor of `Host` model from `Host::Discovered` perspective, as it could be only blocker.

but there might be other cases, I’ll keep digging, though to me it feels like our Host is very bad implementation of Factory Pattern and time has proven it’s not very needed Factory. I’d love if we could get rid of it in favor of one Host class using facets to separate some logic away of the main class.

lzap · April 30, 2020, 11:14am

I’ve merged, it fixed the issue. Thanks.

I don’t understand. Is this something that should be fixed in Rails too?