Define new hooks for ActiveRecord::Reflection::AssociationReflection

[lowjoel]

Needed for me to do reflection on reflection properties.

I've not managed to run the tests yet. The tests here are complete stabs in the dark...

[lowjoel]

@ronen Finally got postgresql/ruby2.2/AR4.2 tested on Windows. Not the full matrix, but at least it's some testing.

[coveralls]

Coverage decreased (-1.31%) to 98.69% when pulling 4e45be1 on lowjoel:master into 0643b7e on SchemaPlus:master.

[lowjoel]

@ronen Any comments on this? I've no clue why coverage goes down on those, I didn't remove anything...

[ronen]

@lowjoel [sorry for the slow response, my (limited) schema_plus time has been mostly taken up by a small flurry of bug reports, hadn't had a chance to look into this until now.]

I'm concerned that the stack is being wrapped around an undocumented internal method -- and given where that method is called, after and around, and implement middleware callbacks wouldn't have expected semantics.

I think for your inverse-checking gem you want to be able to hook into the declaration of associations (rather than, say, looking up associations via reflect_on_association). So I think the hook should be named something like Model::Association::Declare. And unfortunately looking at the AR source I don't see just one place it can be put -- I think there'd need to be a wrapper around all four of belongs_to, has_one, and has_many and has_and_belongs_to_many, along the lines of:

def belongs_to(name, scope=nil, options={})
  SchemaMonkey::Middleware::Model::Association::Declare.start model:self, name: name, scope: scope, options: options, type: :belongs_to do |env|
    super env.name, env.scope, env.options
  end
end

and similarly for the others.

Given that, for your inverse-checking gem, I think you'd use an after middleware callback, something like:

def after(env)
  return if env.type == :has_and_belongs_to_many  # no :inverse_of needed for HABTM
  reflection = env.model.reflections[env.name]
  # ...rest of your logic here..
end

Does this make sense?

[lowjoel]

Good idea. That was the callsite I identified needing hooking, so that was the first thing I came up with. I'll change it.

[coveralls]

Coverage remained the same at 100.0% when pulling f9d9657 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[lowjoel]

@ronen updated, how does this look? I'll port my gem over now.

[coveralls]

Coverage decreased (-1.51%) to 98.49% when pulling d15de61 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[ronen]

@lowjoel for the stack to allow middleware to examine (and potentially modify) the arguments, they need to be captured in the env object, and passed to the underlying implementation from the env object. Also typically middleware will need to know which model this is acting on. So the overrided method needs to have this form:

def belongs_to(name, scope=nil, options={})   # duplicates the signature of the overrided method
  SchemaMonkey::Middleware::Model::Association::Declare.start model:self, name: name, scope: scope, options: options, type: :belongs_to do |env|   # captures model and all args in env
    super env.name, env.scope, env.options  # call implementation using values from env (potentially modified by middleware)
  end
end

[lowjoel]

Yeah I realised after pushing that self isn't captured. Spent a lot of time messing around with gemfiles

[lowjoel]

The problem I foresee is that across different versions of Rails, the parameter list might change and the gem will break for some versions but not others. Declaring the list of parameters in the environment isn't sustainable...

I'm fixing the PR now.

[coveralls]

Coverage remained the same at 100.0% when pulling edc6c53 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[coveralls]

Coverage remained the same at 100.0% when pulling edc6c53 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[lowjoel]

@ronen pushed. I'm not sure if you're going to bump the gem to 0.3

[ronen]

@lowjoel good catch adding &extensions -- I had forgotten about that!

Care to update the README too?

Re parameter list changing across different versions of AR: Yeah, not much can be done about that -- we need to expose all the args to the middleware in the stack. As versions change, schema_plus_core is going to accumulate version-specific conditionals and other cruft, (just as schema_plus 1.x did). But at least all that ugliness will (hopefully) be hidden in schema_plus_core so all the other gems would stay cleaner.

And yes, this will be a minor version bump.

[lowjoel]

Pushed, have a look.

[ronen]

Pushed, have a look.

@lowjoel Did you mean to push something new?

[lowjoel]

crap, push was rejected, sorry. really pushed now.

[coveralls]

Coverage remained the same at 100.0% when pulling 19a2d67 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 19a2d67 on lowjoel:master into 03bc7a4 on SchemaPlus:master.

[ronen]

Cool, thanks. Merging.

[ronen]

...and I've released 0.3.0

[lowjoel]

👍 https://github.com/lowjoel/schema_plus_association_inverses