Eliminate the use of singleton classes on CPK relations.

[justinweiss]

Previously, CPK overrode ActiveRecord::Relation on creation and
injected a singleton class into the object hierarchy, which caused the
relation (and the model holding it) to be undumpable.

I've moved the overridden methods into their own class, and overrode
ActiveRecord::Relation#new to act as a factory method, returning either
an ActiveRecord::Relation or a CompositePrimaryKeys::CompositeRelation
as needed.

[justinweiss]

This resolves #116.

[cfis]

Thanks for the patch - looks like a good idea. Will try and review it this weekend.

[pdf]

It's a great idea - just spent hours tracking down the source of my unmarshallable objects. Working fine with @justinweiss's patch.

[cfis]

I am reverting this commit because it breaks a ton of tests. To be exact:

127 tests, 127 assertions, 8 failures, 74 errors, 0 skips

Unfortunately this approach will not work due to code in relation/delegation.rb:

  def new(klass, *args)
    relation = relation_class_for(klass).allocate
    relation.__send__(:initialize, klass, *args)
    relation
  end

By overriding the picking of the class, all relations are broken by this pull request.

[justinweiss]

Sorry about that. I’m really not sure how I missed testing this under Rails 4, since it clearly doesn’t work with some of the activerecord changes. I don’t have any Rails 4 projects yet, so most of the writing and testing of this patch happened under 3.2.

I’ll take another look, it shouldn’t be too hard to fix.

On Nov 2, 2013, at 4:46 PM, Charlie Savage notifications@github.com wrote:

I am reverting this commit because it breaks a ton of tests. To be exact:

127 tests, 127 assertions, 8 failures, 74 errors, 0 skips

Unfortunately this approach will not work due to code in relation/delegation.rb:

def new(klass, *args)
relation = relation_class_for(klass).allocate
relation.send(:initialize, klass, *args)
relation
end
By overriding the picking of the class, all relations are broken by this pull request.

—
Reply to this email directly or view it on GitHub.

[cfis]

Sounds good...I think you're right on the tests aren't looking good on Rails 4, so some of those failures aren't due to this commit.

[justinweiss]

I almost had this working under Rails 4.0.1, but the code I was hooking is getting rewritten again: https://github.com/rails/rails/commits/master/activerecord/lib/active_record/relation/delegation.rb. I'm going to hold off looking at this until the changes settle down.

[cfis]

Ok, sounds good, will cut a new release then.

[bdurand]

Any updates on this fix? I'm still running into the problem and hopefully the upstream changes in ActiveRecord 4 have settled down by now.

[justinweiss]

Nope -- we've stopped using composite keys, so I haven't invested any more time in this. If someone wants to try to do something with the code, though, I'd be happy to answer questions about it!