From d045ae4b53f057530db1fd43f33d3cd34e0a995d Mon Sep 17 00:00:00 2001 From: Jonathan Kolyer Date: Sun, 3 Nov 2013 01:43:09 -0500 Subject: [PATCH] vrfs-774: geocoding tests --- ruby/lib/jam_ruby/models/max_mind_geo.rb | 16 ++---- ruby/lib/jam_ruby/models/user.rb | 29 ++++++---- ruby/spec/factories.rb | 12 ++-- .../spec/jam_ruby/models/max_mind_geo_spec.rb | 28 ++++------ .../jam_ruby/models/user_location_spec.rb | 55 +++++++++++++++++++ 5 files changed, 96 insertions(+), 44 deletions(-) create mode 100644 ruby/spec/jam_ruby/models/user_location_spec.rb diff --git a/ruby/lib/jam_ruby/models/max_mind_geo.rb b/ruby/lib/jam_ruby/models/max_mind_geo.rb index d651e05a3..81868c4cd 100644 --- a/ruby/lib/jam_ruby/models/max_mind_geo.rb +++ b/ruby/lib/jam_ruby/models/max_mind_geo.rb @@ -1,9 +1,14 @@ -require 'csv' module JamRuby class MaxMindGeo < ActiveRecord::Base self.table_name = 'max_mind_geo' + def self.ip_lookup(ip_addy) + self.where(["ip_start <= ? AND ip_end >= ?", + ip_addy, ip_addy]) + .limit(1) + .first + end def self.import_from_max_mind(file) # File Geo-139 @@ -36,15 +41,6 @@ module JamRuby end User.find_each { |usr| usr.update_lat_lng } end - - - # Make an IP address fit in a signed int. Just divide it by 2, as the least significant part - # just can't possibly matter. We can verify this if needed. My guess is the entire bottom octet is - # actually irrelevant - def self.ip_address_to_int(ip) - ip.split('.').inject(0) {|total,value| (total << 8 ) + value.to_i} / 2 - end end - end diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 5f5845880..de3abc56c 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -11,7 +11,7 @@ module JamRuby include Geokit::ActsAsMappable::Glue unless defined?(acts_as_mappable) acts_as_mappable - after_update :check_lat_lng + after_save :check_lat_lng attr_accessible :first_name, :last_name, :email, :city, :password, :password_confirmation, :state, :country, :birth_date, :subscribe_email, :terms_of_service, :original_fpfile, :cropped_fpfile, :cropped_s3_path, :photo_url, :crop_selection, :lat, :lng @@ -969,24 +969,29 @@ module JamRuby end def check_lat_lng - update_lat_lng if city_changed? || state_changed? || country_changed? + update_lat_lng if (city_changed? || state_changed? || country_changed?) && !lat_changed? end - def update_lat_lng(ip_addy=nil) + def update_lat_lng(ip_addy=nil, force=false) yn = false if provides_location? # ip_addy argument ignored in this case - query = {:city => self.city} - query[:region] = self.state unless self.state.blank? - query[:country] = self.country unless self.country.blank? - if geo = MaxMindGeo.where(query).limit(1).first - self.update_attributes({:lat => geo.lat, :lng => geo.lng}) - yn = true + if force || lat.nil? || lng.nil? + query = { :city => self.city } + query[:region] = self.state unless self.state.blank? + query[:country] = self.country unless self.country.blank? + if geo = MaxMindGeo.where(query).limit(1).first + self.update_attributes({ :lat => geo.lat, :lng => geo.lng }) + yn = true + end end elsif ip_addy - if geo = MaxMindGeo.where(['ip_start >= ? AND ip_end <= ?',ip_addy,ip_addy]).limit(1).first - self.update_attributes({:lat => geo.lat, :lng => geo.lng}) + if geo = MaxMindGeo.ip_lookup(ip_addy) + self.update_attributes({ :lat => geo.lat, :lng => geo.lng }) yn = true - end + end + else + self.update_attributes({ :lat => nil, :lng => nil }) + yn = true end yn end diff --git a/ruby/spec/factories.rb b/ruby/spec/factories.rb index 4337e13da..3a0fa30d4 100644 --- a/ruby/spec/factories.rb +++ b/ruby/spec/factories.rb @@ -130,12 +130,12 @@ FactoryGirl.define do factory :geocoder, :class => JamRuby::MaxMindGeo do country 'US' - region 'NC' - city 'Apex' - ip_start '1.0.0.0' - ip_end '1.1.1.1.' - lat 35.73265 - lng -78.85029 + sequence(:region) { |n| ['NC', 'CA'][(n-1).modulo(2)] } + sequence(:city) { |n| ['Apex', 'San Francisco'][(n-1).modulo(2)] } + sequence(:ip_start) { |n| ['1.1.0.0', '1.1.255.255'][(n-1).modulo(2)] } + sequence(:ip_end) { |n| ['1.2.0.0', '1.2.255.255'][(n-1).modulo(2)] } + sequence(:lat) { |n| [35.73265, 37.7742075][(n-1).modulo(2)] } + sequence(:lng) { |n| [-78.85029, -122.4155311][(n-1).modulo(2)] } end end diff --git a/ruby/spec/jam_ruby/models/max_mind_geo_spec.rb b/ruby/spec/jam_ruby/models/max_mind_geo_spec.rb index e5d4ad81f..1df42817d 100644 --- a/ruby/spec/jam_ruby/models/max_mind_geo_spec.rb +++ b/ruby/spec/jam_ruby/models/max_mind_geo_spec.rb @@ -9,10 +9,6 @@ describe MaxMindGeo do in_directory_with_file(GEO_CSV) before do - -# content_for_file('0.116.0.0,0.119.255.255,"AT","","","",47.3333,13.3333,, -# 1.0.0.0,1.0.0.255,"AU","","","",-27.0000,133.0000,, -# 1.0.1.0,1.0.1.255,"CN","07","Fuzhou","",26.0614,119.3061,,'.encode(Encoding::ISO_8859_1)) content_for_file('startIpNum,endIpNum,country,region,city,postalCode,latitude,longitude,dmaCode,areaCode 0.116.0.0,0.119.255.255,"AT","","","",47.3333,13.3333,123,123 1.0.0.0,1.0.0.255,"AU","","","",-27.0000,133.0000,, @@ -23,20 +19,20 @@ describe MaxMindGeo do it { MaxMindGeo.count.should == 3 } - # let(:first) { MaxMindGeo.find_by_ip_start('0.116.0.0') } - # let(:second) { MaxMindGeo.find_by_ip_start('1.0.0.0') } - # let(:third) { MaxMindGeo.find_by_ip_start('1.0.1.0') } + let(:first) { MaxMindGeo.find_by_ip_start('0.116.0.0') } + let(:second) { MaxMindGeo.find_by_ip_start('1.0.0.0') } + let(:third) { MaxMindGeo.find_by_ip_start('1.0.1.0') } - # it { first.country.should == 'AT' } - # it { first.ip_start.should == '0.116.0.0' } - # it { first.ip_end.should == '0.119.255.255' } + it { first.country.should == 'AT' } + it { first.ip_start.should == '0.116.0.0' } + it { first.ip_end.should == '0.119.255.255' } - # it { second.country.should == 'AU' } - # it { second.ip_start.should == '1.0.0.0' } - # it { second.ip_end.should == '1.0.0.255' } + it { second.country.should == 'AU' } + it { second.ip_start.should == '1.0.0.0' } + it { second.ip_end.should == '1.0.0.255' } - # it { third.country.should == 'CN' } - # it { third.ip_start.should == '1.0.1.0' } - # it { third.ip_end.should == '1.0.1.255' } + it { third.country.should == 'CN' } + it { third.ip_start.should == '1.0.1.0' } + it { third.ip_end.should == '1.0.1.255' } end diff --git a/ruby/spec/jam_ruby/models/user_location_spec.rb b/ruby/spec/jam_ruby/models/user_location_spec.rb new file mode 100644 index 000000000..11bd1226a --- /dev/null +++ b/ruby/spec/jam_ruby/models/user_location_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe User do + +=begin +X If user provides profile location data, that will be used for lat/lng lookup +X If the user changes their profile location, we update their lat/lng address +X If no profile location is provided, then we populate lat/lng from their IP address +X If no profile location is provided, and the user creates/joins a music session, then we update their lat/lng from the IP address +=end + + before do + @geocode1 = FactoryGirl.create(:geocoder) + @geocode2 = FactoryGirl.create(:geocoder) + @user = User.new(first_name: "Example", last_name: "User", email: "user@example.com", + password: "foobar", password_confirmation: "foobar", + city: "Apex", state: "NC", country: "US", + terms_of_service: true, musician: true) + @user.save! + end + + describe "with profile location data" do + it "should have lat/lng values" do + @user.lat.should == @geocode1.lat + @user.lng.should == @geocode1.lng + end + it "should have updated lat/lng values" do + @user.update_attributes({ :city => @geocode2.city, + :state => @geocode2.region, + :country => @geocode2.country, + }) + @user.lat.should == @geocode2.lat + @user.lng.should == @geocode2.lng + end + end + + describe "without profile location data" do + it "should have lat/lng values from ip_address" do + @user.update_attributes({ :city => nil, + :state => nil, + :country => nil, + }) + @user.lat.should == nil + @user.lng.should == nil + geo = JamRuby::MaxMindGeo.ip_lookup('1.1.0.0') + geo.should_not be_nil + geo = JamRuby::MaxMindGeo.ip_lookup('1.1.0.255') + geo.should_not be_nil + @user.update_lat_lng('1.1.0.255') + @user.lat.should == @geocode1.lat + @user.lng.should == @geocode1.lng + end + end + +end