Modules not isolated, rely on foreknowledge of includer

[atz]

How does a module with no ancestors call super in a method? (Multiple methods.)

Example: https://github.com/projecthydra/hydra-head/blob/master/hydra-access-controls/lib/hydra/policy_aware_access_controls_enforcement.rb#L9

I know this recurs generally in the Hydra stack but it is bad design. The module doesn't check at include time whether its needs are met and the includer shouldn't be forced to debug runtime errors or monkey-patch the module to inject its dependencies (in this case blacklight access controls enforcement).

This is not the composition pattern that was intended.

[jcoyne]

This is a common pattern in Rails: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/callbacks.rb#L292 And it was my impression that those programmers knew more than I did, so I copied them. I agree it's not the best, but I don't have the inclination or time to refactor it. Patches welcome!

[atz]

@jcoyne, that isn't comparable. ActiveRecord::Callbacks accounts for its dependencies here: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/callbacks.rb#L268-L277

    module ClassMethods # :nodoc:
      include ActiveModel::Callbacks
    end

    included do
      include ActiveModel::Validations::Callbacks

      define_model_callbacks :initialize, :find, :touch, only: :after
      define_model_callbacks :save, :create, :update, :destroy
    end

Now that is obfuscated and heavy on meta-programming, but isn't unmodular. But by the time it calls _run_touch_callbacks, it itself has done:

define_model_callbacks :initialize, :find, :touch

Given the context, we can tell that ends up here: https://github.com/rails/rails/blob/master/activemodel/lib/active_model/callbacks.rb#L103

It does not rely on the includer having already defined _run_touch_callbacks. So either you misunderstand what I'm describing, or you need a more serviceable example. I don't think what I'm indicating here is a common pattern in rails, or even an uncommon one: I expect it to appear zero times. If it does occur, I expect the maintainers would regard it as a mistake. Which is what I expect here.

[atz]

@jcoyne Have I described it clearly enough? Do you agree this is not just a "nice to have", but actually a flaw in design?

[atz]

See also: https://github.com/projectblacklight/blacklight/issues/1588