Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ActiveRecord 4.2 #12

Open
fjl82 opened this issue Dec 23, 2014 · 21 comments · May be fixed by #26
Open

Support ActiveRecord 4.2 #12

fjl82 opened this issue Dec 23, 2014 · 21 comments · May be fixed by #26

Comments

@fjl82
Copy link

fjl82 commented Dec 23, 2014

The current version requires rgeo-activerecord 0.4.*, which does not work with AR 4.2:

E, [2014-12-23T09:54:57.557146 #3627] ERROR -- : undefined method `attribute_types_cached_by_default' for ActiveRecord::Base:Class (NoMethodError)
/var/lib/gems/2.0.0/gems/activerecord-4.2.0/lib/active_record/dynamic_matchers.rb:26:in `method_missing'
/var/lib/gems/2.0.0/gems/rgeo-activerecord-0.4.6/lib/rgeo/active_record/common_adapter_elements.rb:159:in `<module:ActiveRecord>'
/var/lib/gems/2.0.0/gems/rgeo-activerecord-0.4.6/lib/rgeo/active_record/common_adapter_elements.rb:47:in `<module:RGeo>'
/var/lib/gems/2.0.0/gems/rgeo-activerecord-0.4.6/lib/rgeo/active_record/common_adapter_elements.rb:45:in `<top (required)>'

Please update the adapter to support newer versions of rgeo-activerecord.

@teeparham
Copy link
Member

See #13

@fjl82
Copy link
Author

fjl82 commented Dec 24, 2014

Thanks for the heads-up. I don't want to become a maintainer (I don't code in my free time and I don't have time at work to take care of bug reports and things), but I had some time today to take a look at it myself and came up with this:
https://github.com/fjl82/activerecord-mysql2spatial-adapter
I can't give any guarantees that it works for everyone but it works for me on AR4.2 in the 2 projects that I use the adapter in. I don't have time to figure out how to make the tests run, so they won't work for now.

@benatkin
Copy link

benatkin commented Apr 9, 2015

@fjl82 nice work!

@teeparham we're using this at @townsquared. I'm willing to start maintaining it. I would start just by merging @fjl82's change and making the test suite run on rails 4.2.

@teeparham
Copy link
Member

@benatkin Hey good to hear from you! I added you to the rgeo organization. I'll add you as a gem owner so you can release new versions.

@benatkin
Copy link

benatkin commented Apr 9, 2015

@teeparham thanks! looking forward to getting this on travis-ci and cutting a new version.

@loganbu
Copy link

loganbu commented Apr 19, 2015

@fjl82 - Thanks for making that port! I noticed at least one issue in my project, where the adapter didn't properly look up 'point' columns - it thought they were 'int'. I sent you a pull request with fjl82#1.

@fjl82
Copy link
Author

fjl82 commented Apr 20, 2015

@agoln thanks for the fix, I merged the pull request. Looking forward to a new official release :)

@jdart
Copy link

jdart commented May 13, 2015

+1 looking forward to new release.

@geniousli
Copy link

does it support in AR4.2?

@agodel
Copy link

agodel commented Nov 30, 2015

Any updates on support for 4.2?

@dannyyu92
Copy link

+1

@dschweisguth
Copy link
Contributor

@fjl82's fork works for me, with one additional change that fixes a NoMethodError when a table has a spatial index: dschweisguth@eced87f

@stephenq
Copy link

stephenq commented Mar 7, 2017

Will this ever get worked into a release?

@januszm
Copy link
Contributor

januszm commented Mar 7, 2017

Sorry guys, this project was shelved for some time because of lack of resources and support. If I understand correctly you'd like to go with flj82's fork with some patches like dschweisguth/activerecord-mysql2spatial-adapter@eced87f ?

@dschweisguth
Copy link
Contributor

That's working for me, so it would be helpful to me to have it on master. However, I don't have deep understanding of the code, I just made it work, so it's possible that a different solution would be better.

@fjl82
Copy link
Author

fjl82 commented Mar 30, 2017

Would be cool if this project received some real support again :) We rely on good geo support in our projects and I currently use my own fork in production (there's a branch for AR 5.0 also), but it's only provided on a "works for me" basis, since I don't have sufficient time to process bugreports.

I think I did take a look at dschweisguth's patch but I think I had some issue with it (don't remember what though). I do have one table with a spatial index and that works fine for me (a 'geometry' column).

@dschweisguth
Copy link
Contributor

I have some time to work on this, but it isn't clear to me how to run the tests correctly. Judging from commit comments it looks like @januszm got the tests running on AR 4.1. What version(s) of Ruby work? What series of commands does one use to run the tests? So far the best I know how to do is

  • use current Ruby
  • ignore the Rakefile
  • gem build
  • gem install
  • ruby -Ilib -ractive_record/connection_adapters/mysql2spatial_adapter test/tc_basic.rb

with which 5/12 tests fail. Any suggestions? @fjl82 you got the tests running on your branch, so maybe you know how too.

@fjl82
Copy link
Author

fjl82 commented Jun 22, 2017

@dschweisguth No sorry, I never got the tests running either. I was considering just leaving it for the moment, or rebuilding them from scratch, but I didn't have time for massive non-functional changes. I made my fork because we rely on this in our production system and didn't want to get stuck on an old version of Rails (running 5.1 now), but I have too many other things to work on to spend a lot of time on the tests.

@januszm
Copy link
Contributor

januszm commented Jun 22, 2017

@dschweisguth @fjl82 I'd be happy to merge your updates and provide support for newer AR versions. As I mentioned before, I can't actively work on new features at the moment so I can only provide you with information and code review.

To make the test suite running with ActiveRecord 4.1, first create database config file test/database.yml , works for me with Ruby 2.1.9 or Ruby 2.3.3

adapter: mysql2spatial
encoding: utf8
reconnect: true
host:     localhost
database: activerecord_test
username: root
password:

make sure the database exists, feel free to adjust credentials and database name.

ActiveRecord 4.1 only works with mysql2 gem versions 0.3.x. Make sure you have this gem installed, I'll push a new commit to master, that helps with installing correct version (< 0.4.0)

(master *) activerecord-mysql2spatial-adapter: bundle exec rake test
Loaded testcase test/tc_basic.rb
Loaded testcase test/tc_spatial_queries.rb
Run options: --seed 18312

# Running:

..............

Finished in 0.273528s, 51.1831 runs/s, 102.3662 assertions/s.

14 runs, 28 assertions, 0 failures, 0 errors, 0 skips

I'll also add information about the test env setup to the Readme soon.

@dschweisguth
Copy link
Contributor

dschweisguth commented Jun 28, 2017

I encountered one more issue getting the tests to run, filed issue #24 to cover it, and fixed it in PR #23. I'll continue building on that towards 4.2 compatibility.

@januszm
Copy link
Contributor

januszm commented Jun 28, 2017

Thanks, I think that for future releases, we may just drop support for older AR versions ad clean up the code. Then we make it compatible with new AR and RGeo and bump the major gem version, those who need to work with older AR/rgeo will be able to still use adapter versions <= 0.5.0

@dschweisguth dschweisguth linked a pull request Jun 30, 2017 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.