rgeo / activerecord-mysql2spatial-adapter

ActiveRecord connection adapter for MySQL Spatial Extensions, based on mysql2 and rgeo
48 stars 64 forks source link

Support ActiveRecord 4.2 #12

Open fjl82 opened 9 years ago

fjl82 commented 9 years ago

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 commented 9 years ago

See #13

fjl82 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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

loganbu commented 9 years ago

@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 https://github.com/fjl82/activerecord-mysql2spatial-adapter/pull/1.

fjl82 commented 9 years ago

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

jdart commented 9 years ago

+1 looking forward to new release.

geniousli commented 9 years ago

does it support in AR4.2?

agodel commented 8 years ago

Any updates on support for 4.2?

dannyyu92 commented 8 years ago

+1

dschweisguth commented 8 years ago

@fjl82's fork works for me, with one additional change that fixes a NoMethodError when a table has a spatial index: https://github.com/dschweisguth/activerecord-mysql2spatial-adapter/commit/eced87fecec340284743a3d80eea453001b2ce88

stephenq commented 7 years ago

Will this ever get worked into a release?

januszm commented 7 years ago

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@eced87f ?

dschweisguth commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

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

fjl82 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

I encountered one more issue getting the tests to run, filed issue #24 to cover it, and fixed it in PR https://github.com/rgeo/activerecord-mysql2spatial-adapter/pull/23. I'll continue building on that towards 4.2 compatibility.

januszm commented 7 years ago

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