Compatibility with active record 3.0 and 3.1

[sobrinho]

Calling super() makes the active record adapter compatible with older versions of active record.

[sobrinho]

Not sure why travis is not getting this commit, looking on this.

[karmi]

Not sure how this works without stack level too deep, just by reading the code? Also, ActiveRecord 3.0 is not very attractive to support, I'm quite hesitant to do it.

[sobrinho]

I know, but there is legacy applications out there and we have no other option since tire isn't compatible with elastic search 1.x.

This is the only necessary change right now and I supposed it was a bug since the gem allows it to be bundled on rails 3.0.

We may be blocked in the future because a missing feature on 3.0 or 3.1 but when we get to this, we may drop the support to 3.0 and 3.1 entirely.

Wdyt?

[karmi]

@sobrinho Yeah, I understand. I'll take a look at it next week -- I see how the call to super works, will just verify it with ActiveRecord code, so we don't end up doing some infinite recursion. I'll split the commit to better separate it, can you please sign the CLA in the meantime?

[sobrinho]

@karmi signed 👍

[sobrinho]

About the code, I've read somewhere (I don't remember where) that calling super on define_singleton_method does exactly what we want, without infinite recursion.

The only gotcha is that we must specify the params explicit, that's why I used super() instead of super (the latter raises an exception telling about the required explicitness).

[karmi]

@sobrinho Sounds like a nice trick, didn't knew about it :) Will split the commit and verify the code next weeks, thanks for the patch!

[dre-hh]

There are following issues with the 3.x Gemfiles

1. Active Record Connection not established
   I suppose Active Record 3.x needs the connection already established at declaration Time. And this is happening outside the setup block(which establishes the connection).
   E.g ActiveRecordAssociationsIntegrationTest

   class Category < ActiveRecord::Base

For some reason the error occurs only if you run the spec with the provided custom rake task.
If you run it directly from the rails_model subfolder with default minitest rake task, it works

#works
elasticsearch_model$ BUNDLE_GEMFILE=./gemfiles/3.0.gemfile bundle exec rake test  TEST=test/integration/acticve_record_associations_test.rb

#fails with connection not established error in acticve_record_association_test
elasticsearch_rails$ TEST_BUNDLE_GEMFILE=./gemfiles/3.0.gemfile bundle exec rake test:integration --trace 

I'd suggest we move the ActiveRecord declarations into the setup block or just call setup before any declaration.

@sobrinho
Should i make a pull request into your fork or will you fix it?

2. 3.0 and 3.1 issue with mongo conenction
   mongoid_basic_test.rb

Prior to mongoid 3, they used to have another way to connect to db and did not shipped
the Moped gem. So we would need some helper here, which establishes the connection
according to the mongoid version

  #mongoid_basic_test.rb#5 will throw errors with mongo 2.x
 session = Moped::Connection.new("localhost", 27017, 0.5)

[sobrinho]

@dre-hh I will work on this today and update the PR, thanks for the feedback!

[karmi]

So, started looking into this:

• First, it's nice of you @sobrinho to try test old Mongoid versions, but let's keep this sane and specific to ActiveRecord :) So I'll remove the Mongoid gems from the 3.0 and 3.1 gem files, leaving only ActiveRecord, which will skip the Mongoid tests. As @dre-hh pointed out, there's a completely different API on Mongoid 2.5.x, it's not worth to spend time with it.
• I'm getting lots of integration test failures and errors for the 3.1 gemfile, mainly around the gem not returning the expected documents, eg. <1> expected but was <0> and so on. This is troubling. Can you run it and figure out what's happening?

[karmi]

@sobrinho What should we do with this old story? :)

[sobrinho]

@karmi I'm on vacation right now but keep it open and I will fix the problems in a couple weeks :)

[karmi]

@sobrinho Sure thing, enjoy your vacation and ping me when you're back!

[dre-hh]

@karmi we've got rails 4.2 by the time now. I've personally upgraded some rails projects from 3.0 to 3.2. I don't remember wether there was some minor incompatibilities in AR. The whole process was actually pretty painless. I suggest we close this issue and encourage people to upgrade at least to AR 3.2

[karmi]

@dre-hh Agreed that Rails 3.0 is a legacy thing now, however, the gem works with that version, so I'm not sure if we should remove it... On the other hand, I'm all for adding support or 3.2.x and getting rid of 3.0.x if it would complicate things.

[sobrinho]

Closing, we finally upgraded the project, thanks!

[karmi]

@sobrinho, that's the best outcome! :) Best of luck!