Railtie's use of loader is not STI aware, causing operations to load before needed models

[rosskevin]

This relates and is a follow-on to #14 Engine concepts are not loaded. This is not engine specific, this is for any namespaced model/operation.

We have found that namespaced models are not loaded prior to concepts, because ModelFile has a hardcoded path and doesn't attempt to look at namespace in the directory structure resolution.

From source:

    # Prepend model file, before the concept files like operation.rb get loaded.
    ModelFile = ->(input, options) do
      model = "app/models/#{options[:name]}.rb"
      File.exist?(model) ? [model]+input : input
    end

I'm working on gathering more information and a potential fix. The above is speculation based on the result of the logging of the loaded classes.

[rosskevin]

The issue may be invalid. What was purported to be a namespace issue may be an issue with inheritance, and loading of leaf concepts > leaf models > base concept > base model, and the order. Still looking.

[rosskevin]

So this code seems to add the direct model previous to the concept, but does not take into into account inheritance of an STI model:

Loader.new.({debug: true, insert: [ModelFile, before: Loader::AddConceptFiles]})

It looks like we need to re-sort the final result of this prior to loading to push all models up.

[rosskevin]

This may sound like a crazy question, but why isn't trailblazer-rails waiting until rails is done loading models then loading concepts? That would remove this issue altogether, because models would have been loaded by the standard rails mechanism.

[rosskevin]

Workaround (so far) puts models first and incorporates the patch included in #14

module Trailblazer
  class Railtie < ::Rails::Railtie
    def self.load_root(root)
      # This is a fix/workaround for "loader is not STI aware, causing operations to load before needed models"
      #  @see https://github.com/trailblazer/trailblazer-rails/issues/30

      # Loader has no concept for base dir, must chdir so that it can find files
      Dir.chdir(root) do
        # Loader has no concept of STI models, so gather it's intended set of files for re-sorting
        files = []
        Loader.new.({debug: false, insert: [ModelFile, before: Loader::AddConceptFiles]}) { |file|
          files << file
        }

        # now re-sort the files, models-first
        files = files.sort_by { |file|
          (file =~ /(app\/models\/.*)/) || 1000 # model index otherwise 1000 to keep models first.
        }

        files.each do |file|
          require_dependency("#{root}/#{file}")
        end
      end
    end

    def self.load_concepts(app)

      # This is a fix/workaround for "Engine concepts are not loaded"
      #  @see https://github.com/trailblazer/trailblazer-rails/issues/14
      engines = ::Rails::Engine.subclasses.map(&:instance)
      engines.each do |engine|
        load_root(engine.root)
      end

      Dir.chdir(app.root) do # necessary to guarantee context for tests
        load_root(app.root)
      end
    end
  end
end

I'm still testing so I'm not really putting my stamp of approval on this re-sort as a working solution.

[rosskevin]

So far the above works, the only caveat being that if the basic sorting algorithm doesn't get your dependencies quite right, e.g. I had a subscription downgrade operation that inherited from change in the operation.rb file, I was able to add require_dependency File.expand_path("../operation", __FILE__) to the leaf operation and resolution worked fine. This particular inherited operation + STI is somewhat of an outlier because the operations are in separate files due to complexity.

This seems to be working well.

[apotonick]

I really don't want to make this step of trying to be "smarter" than Rails' autoloading - which is pure madness in itself.

Why don't you just manually require_dependency where the loader doesn't pick them up?

[rosskevin]

I didn't see an obvious way to make TB wait for rails to load everything else, then load, so we do need to make the loading code a bit smarter.

I think the above solution is a good one because it requires all models first. This solved 99% of my problems, where I could do an explicit require_dependency for the rest.