diff --git a/ruby/lib/jam_ruby/connection_manager.rb b/ruby/lib/jam_ruby/connection_manager.rb index 0b33be407..53a75dc72 100644 --- a/ruby/lib/jam_ruby/connection_manager.rb +++ b/ruby/lib/jam_ruby/connection_manager.rb @@ -72,9 +72,8 @@ module JamRuby if block.nil? then locid = 0 else locid = block.locid end location = GeoIpLocations.find_by_locid(locid) - if location.nil? - # todo what's a better default location? - locidispid = 0 + if location.nil? || isp.nil? || block.nil? + locidispid = nil else locidispid = locid*1000000+ispid end @@ -207,9 +206,8 @@ SQL if block.nil? then locid = 0 else locid = block.locid end location = GeoIpLocations.find_by_locid(locid) - if location.nil? - # todo what's a better default location? - locidispid = 0 + if location.nil? || isp.nil? || block.nil? + locidispid = nil else locidispid = locid*1000000+ispid end diff --git a/ruby/lib/jam_ruby/models/connection.rb b/ruby/lib/jam_ruby/models/connection.rb index 3a8d3c0dd..27d715238 100644 --- a/ruby/lib/jam_ruby/models/connection.rb +++ b/ruby/lib/jam_ruby/models/connection.rb @@ -163,7 +163,7 @@ module JamRuby # if user joins the session as a musician, update their addr and location if as_musician - user.update_addr_loc(self, 'j') + user.update_addr_loc(self, User::JAM_REASON_JOIN) user.update_audio_latency(self, audio_latency) end end diff --git a/ruby/lib/jam_ruby/models/geo_ip_locations.rb b/ruby/lib/jam_ruby/models/geo_ip_locations.rb index 782c8c2da..490ad79ca 100644 --- a/ruby/lib/jam_ruby/models/geo_ip_locations.rb +++ b/ruby/lib/jam_ruby/models/geo_ip_locations.rb @@ -20,12 +20,16 @@ module JamRuby # This is a class method because it doesn't need to be in a transaction. def self.lookup(ip_address) - city = state = country = nil - locid = ispid = 0 + city = state = country = locid = ispid = nil - unless ip_address.nil? || ip_address !~ /^\d+\.\d+\.\d+\.\d+$/ + if !ip_address.nil? || ip_address =~ /^\d+\.\d+\.\d+\.\d+$/ || ip_address.class == Fixnum + + if ip_address.class == Fixnum + addr = ip_address + else + addr = ip_address_to_int(ip_address) + end - addr = ip_address_to_int(ip_address) block = GeoIpBlocks.lookup(addr) if block @@ -47,7 +51,7 @@ module JamRuby end end - {city: city, state: state, country: country, addr: addr, locidispid: locid*1000000+ispid} + {city: city, state: state, country: country, addr: addr, locidispid: (locid.nil? || ispid.nil?) ? nil : locid*1000000+ispid} end def self.createx(locid, countrycode, region, city, postalcode, latitude, longitude, metrocode, areacode) diff --git a/ruby/lib/jam_ruby/models/latency_tester.rb b/ruby/lib/jam_ruby/models/latency_tester.rb index 4e3c71b44..ae5c041a3 100644 --- a/ruby/lib/jam_ruby/models/latency_tester.rb +++ b/ruby/lib/jam_ruby/models/latency_tester.rb @@ -53,9 +53,8 @@ module JamRuby block = GeoIpBlocks.lookup(addr) if block.nil? then locid = 0 else locid = block.locid end location = GeoIpLocations.find_by_locid(locid) - if location.nil? - # todo what's a better default location? - locidispid = 0 + if location.nil? || isp.nil? || block.nil? + locidispid = nil else locidispid = locid*1000000+ispid end diff --git a/ruby/lib/jam_ruby/models/user.rb b/ruby/lib/jam_ruby/models/user.rb index 15a71826e..2c046df78 100644 --- a/ruby/lib/jam_ruby/models/user.rb +++ b/ruby/lib/jam_ruby/models/user.rb @@ -5,12 +5,15 @@ module JamRuby #devise: for later: :trackable + @@log = Logging.logger[User] + VALID_EMAIL_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i JAM_REASON_REGISTRATION = 'r' JAM_REASON_NETWORK_TEST = 'n' - JAM_REASON_FTUE = 'f' + JAM_REASON_FTUE = 'g' JAM_REASON_JOIN = 'j' JAM_REASON_IMPORT = 'i' + JAM_REASON_LOGIN = 'l' devise :database_authenticatable, :recoverable, :rememberable @@ -135,7 +138,7 @@ module JamRuby validates :show_whats_next, :inclusion => {:in => [nil, true, false]} validates :mods, json: true validates_numericality_of :last_jam_audio_latency, greater_than:0, :allow_nil => true - validates :last_jam_updated_reason, :inclusion => {:in => [nil, JAM_REASON_REGISTRATION, JAM_REASON_NETWORK_TEST, JAM_REASON_FTUE, JAM_REASON_JOIN, JAM_REASON_IMPORT] } + validates :last_jam_updated_reason, :inclusion => {:in => [nil, JAM_REASON_REGISTRATION, JAM_REASON_NETWORK_TEST, JAM_REASON_FTUE, JAM_REASON_JOIN, JAM_REASON_IMPORT, JAM_REASON_LOGIN] } # custom validators validate :validate_musician_instruments @@ -987,6 +990,27 @@ module JamRuby save! end + def update_addr_loc(connection, reason) + unless connection + @@log.warn("no connection specified in update_addr_loc with reason #{reason}") + return + end + + # we don't use a websocket login to update the user's record unless there is no addr + if reason == JAM_REASON_LOGIN && last_jam_addr + return + end + + self.last_jam_addr = connection.addr + self.last_jam_locidispid = connection.locidispid + self.last_jam_updated_reason = reason + self.last_jam_updated_at = Time.now + unless self.save + puts "OK?" + @@log.warn("unable to update user #{self} with last_jam_reason #{reason}. errors: #{self.errors.inspect}") + end + end + def escape_filename(path) dir = File.dirname(path) file = File.basename(path) @@ -1212,14 +1236,6 @@ module JamRuby self.city end - def update_addr_loc(connection, reason) - self.last_jam_addr = connection.addr - self.last_jam_locidispid = connection.locidispid - self.last_jam_updated_reason = reason - self.last_jam_updated_at = Time.now - self.save - end - def update_audio_latency(connection, audio_latency) # updating the connection is best effort diff --git a/ruby/spec/support/maxmind.rb b/ruby/spec/support/maxmind.rb index 981f8c98a..4ac3166e2 100644 --- a/ruby/spec/support/maxmind.rb +++ b/ruby/spec/support/maxmind.rb @@ -182,4 +182,14 @@ def create_score(a_geoip, b_geoip, a_addr = a_geoip[:jamisp].beginip, b_addr = b Score.createx(Score.create_locidispid(a_geoip[:geoiplocation], a_geoip[:jamisp]), a_client_id, a_addr, Score.create_locidispid(b_geoip[:geoiplocation], b_geoip[:jamisp]), b_client_id, b_addr, score, score_dt, score_data) +end + +def locidispid_from_ip(ip_address) + location = GeoIpLocations.lookup(ip_address) + + if location + location[:locidispid] + else + nil + end end \ No newline at end of file diff --git a/web/app/assets/javascripts/wizard/gear_test.js b/web/app/assets/javascripts/wizard/gear_test.js index cb9b01355..032a427f2 100644 --- a/web/app/assets/javascripts/wizard/gear_test.js +++ b/web/app/assets/javascripts/wizard/gear_test.js @@ -443,7 +443,7 @@ renderIOScore(null, null, null, 'skip', 'skip', 'skip'); } - rest.userCertifiedGear({success: false}); + rest.userCertifiedGear({success: false, client_id: app.clientId}); if(data.reason == "latency") { context.JK.GA.trackAudioTestData(uniqueDeviceName(), context.JK.GA.AudioTestDataReasons.latencyFail, data.latencyScore); diff --git a/web/app/controllers/api_diagnostics_controller.rb b/web/app/controllers/api_diagnostics_controller.rb index ac8a17ce9..945a05b4a 100644 --- a/web/app/controllers/api_diagnostics_controller.rb +++ b/web/app/controllers/api_diagnostics_controller.rb @@ -4,13 +4,20 @@ class ApiDiagnosticsController < ApiController respond_to :json def create + data = params[:data].to_json if params[:data] @diagnostic = Diagnostic.new @diagnostic.type = params[:type] - @diagnostic.data = params[:data].to_json if params[:data] + @diagnostic.data = data @diagnostic.user = current_user @diagnostic.creator = Diagnostic::CLIENT @diagnostic.save + # update last jam info if network test attempted + if @diagnostic.type == 'NETWORK_TEST_RESULT' && params[:data] && params[:data]['client_id'] + connection = Connection.find_by_client_id(params[:data]['client_id']) + current_user.update_addr_loc(connection, User::JAM_REASON_NETWORK_TEST) + end + respond_with_model(@diagnostic, new: true) end end diff --git a/web/app/controllers/api_users_controller.rb b/web/app/controllers/api_users_controller.rb index 99601d900..27f5f268d 100644 --- a/web/app/controllers/api_users_controller.rb +++ b/web/app/controllers/api_users_controller.rb @@ -530,7 +530,7 @@ class ApiUsersController < ApiController connection = Connection.find_by_client_id(params[:client_id]) # update last_jam location information - @user.update_addr_loc(connection, 'g') if connection + @user.update_addr_loc(connection, User::JAM_REASON_FTUE) if connection if !@user.errors.any? # update audio gear latency information diff --git a/web/spec/features/network_test_spec.rb b/web/spec/features/network_test_spec.rb index b9cb2f92b..4f8df53ce 100644 --- a/web/spec/features/network_test_spec.rb +++ b/web/spec/features/network_test_spec.rb @@ -15,6 +15,14 @@ describe "Network Test", :js => true, :type => :feature, :capybara_feature => tr end it "success" do + + # verify last_jam fields get updated too + + user.last_jam_updated_at.should be_nil + user.last_jam_updated_reason.should be_nil + user.last_jam_addr.should be_nil + user.last_jam_locidispid.should be_nil + FactoryGirl.create(:latency_tester) open_user_dropdown @@ -25,6 +33,16 @@ describe "Network Test", :js => true, :type => :feature, :capybara_feature => tr find('.start-network-test').trigger(:click) find('.user-btn', text: 'RUN NETWORK TEST ANYWAY').trigger(:click) find('.scored-clients', text: '5') + + wait_for_ajax # waiting for Diagnostic post to finish + + sleep 1 # even after AJAX is done, because we are in a different connection, we have to give time to fully commit data + + user.reload + user.last_jam_updated_at.should_not be_nil + user.last_jam_updated_reason.should == User::JAM_REASON_NETWORK_TEST + user.last_jam_addr.should == ip_address_to_int('127.0.0.1') + user.last_jam_locidispid.should == locidispid_from_ip('127.0.0.1') end end diff --git a/web/spec/requests/user_progression_spec.rb b/web/spec/requests/user_progression_spec.rb index 7def02080..390aa151b 100644 --- a/web/spec/requests/user_progression_spec.rb +++ b/web/spec/requests/user_progression_spec.rb @@ -42,14 +42,22 @@ describe "User Progression", :type => :api do end it "qualified gear" do + connection = FactoryGirl.create(:connection, user: user, ip_address: '127.0.0.1', locidispid: locidispid_from_ip('127.0.0.1')) + user.first_certified_gear_at.should be_nil - post "/api/users/progression/certified_gear.json", { :success => true}.to_json, "CONTENT_TYPE" => 'application/json' + user.last_jam_addr.should be_nil + user.last_jam_locidispid.should be_nil + post "/api/users/progression/certified_gear.json", { :success => true, client_id: connection.client_id}.to_json, "CONTENT_TYPE" => 'application/json' last_response.status.should eql(200) JSON.parse(last_response.body).should eql({ }) user.reload user.first_certified_gear_at.should_not be_nil user.last_failed_certified_gear_at.should be_nil user.last_failed_certified_gear_reason.should be_nil + user.last_jam_addr.should == ip_address_to_int('127.0.0.1') + user.last_jam_updated_reason.should == User::JAM_REASON_FTUE + user.last_jam_updated_at.should_not be_nil + user.last_jam_locidispid.should == locidispid_from_ip('127.0.0.1') end it "failed qualified gear" do diff --git a/web/spec/support/maxmind.rb b/web/spec/support/maxmind.rb new file mode 100644 index 000000000..89df561a9 --- /dev/null +++ b/web/spec/support/maxmind.rb @@ -0,0 +1,195 @@ + +ISO3166_COUNTRYCODE_INDEX = 0 +ISO3166_COUNTRYNAME_INDEX = 1 + +REGIONCODES_COUNTRYCODE_INDEX = 0 +REGIONCODES_REGIONCODE_INDEX = 1 +REGIONCODES_REGIONNAME_INDEX = 2 + +GEOIPBLOCKS_BEGINIP_INDEX = 0 +GEOIPBLOCKS_ENDIP_INDEX = 1 +GEOIPBLOCKS_LOCID_INDEX = 2 + +GEOIPLOCATIONS_LOCID_INDEX = 0 +GEOIPLOCATIONS_COUNTRY_INDEX = 1 +GEOIPLOCATIONS_REGION_INDEX = 2 +GEOIPLOCATIONS_CITY_INDEX = 3 +GEOIPLOCATIONS_POSTALCODE_INDEX = 4 +GEOIPLOCATIONS_LATITUDE_INDEX = 5 +GEOIPLOCATIONS_LONGITUDE_INDEX = 6 +GEOIPLOCATIONS_METROCODE_INDEX = 7 +GEOIPLOCATIONS_AREACODE_INDEX = 8 + +GEOIPISP_BEGINIP_INDEX = 0 +GEOIPISP_ENDIP_INDEX = 1 +GEOIPISP_COMPANY_INDEX = 2 + +JAMISP_BEGINIP_INDEX = 0 +JAMISP_ENDIP_INDEX = 1 +JAMISP_COMPANY_INDEX = 2 + +# the goal is to specify just enough data to build out a score (a 'leaf' in maxmind data) +def tiny_maxmind_dataset + + region_codes = [ + ["US","TX","Texas"] + ] + + iso3166 = [ + ["US", "United States"] + ] + + # table=max_mind_isp + geo_ip_isp_142 = [ + ["1.0.0.0","1.0.0.255","US","Google"], + ["1.0.1.0","1.0.1.255","US","Time Warner"], + ["1.0.2.0","1.0.2.255","US","AT&T"] + ] + + # table=max_mind_geo + geo_ip_city_139 = [ + ["1.0.0.0","1.0.0.255","US","TX","Austin","78759","30.4000","-97.7528","635","512"], # original: 4.15.0.0, 4.15.0.255 (68091904, 68092159), locid=1504 + ["1.0.1.0","1.0.1.255","US","TX","Austin","78701","30.2678","-97.7426","635","512"], # original: 4.28.169.0, 4.28.169.255 (68987136, 68987391), locid=1102 + ["1.0.2.0","1.0.2.255","US","TX","Austin","78729","30.4549","-97.7565","635","512"] # original: 4.30.69.0, 4.30.69.127 (69092608, 69092735), locid=14655 + ] + + # table=geoipblocks, file=GeoIPCity-134-Blocks.csv + geo_ip_city_134_blocks = [ + [68091904, 68092159, 1504], + [68987136, 68987391, 1102], + [69092608, 69092735, 14655] + ] + + # table=geoiplocations, file=GeoIpCity-134-Locations.csv + geo_ip_city_134_locations = [ + [1504,"US","TX","Austin","78759",30.4000,-97.7528,635,512], + [1102,"US","TX","Austin","78701",30.2678,-97.7426,635,512], + [14655,"US","TX","Austin","78729",30.4549,-97.7565,635,512] + ] + + #table=geoipisp + geo_ip_isp = [ + [401604608,401866751,"Time Warner Cable"] + ] + + { + region_codes: region_codes, + iso3166: iso3166, + geo_ip_isp_142: geo_ip_isp_142, + geo_ip_city_139: geo_ip_city_139, + geo_ip_city_134_blocks: geo_ip_city_134_blocks, + geo_ip_city_134_locations: geo_ip_city_134_locations, + geo_ip_isp: geo_ip_isp + } +end + +def write_content_to_tmp_file(name, content, prefix = '') + file = Tempfile.new([name.to_s, '.csv']) + File.open(file, 'w') {|f| f.write(to_csv(content, prefix)) } + file +end + +def dataset_to_tmp_files(dataset=tiny_maxmind_dataset) + tmp_files = {} + + geo_ip_isp_124 = write_content_to_tmp_file(:geo_ip_isp, dataset[:geo_ip_isp]) + tmp_files[:geo_ip_124_files] = { 'GeoIPISP.csv' => geo_ip_isp_124 } + + geo_ip_isp_134_blocks = write_content_to_tmp_file(:geo_ip_city_134_blocks, dataset[:geo_ip_city_134_blocks], + "Copyright (c) 2011 MaxMind Inc. All Rights Reserved.\n" + + "startIpNum,endIpNum,locId\n") + geo_ip_isp_134_locations = write_content_to_tmp_file(:geo_ip_city_134_locations, dataset[:geo_ip_city_134_locations], + "Copyright (c) 2012 MaxMind LLC. All Rights Reserved.\n" + + "locId,country,region,city,postalCode,latitude,longitude,metroCode,areaCode\n") + tmp_files[:geo_ip_134_files] = { 'GeoIPCity-134-Blocks.csv' => geo_ip_isp_134_blocks , 'GeoIPCity-134-Location.csv' => geo_ip_isp_134_locations } + + tmp_files[:region_codes] = write_content_to_tmp_file(:region_codes, dataset[:region_codes]) + tmp_files[:iso3166] = write_content_to_tmp_file(:iso3166, dataset[:iso3166]) + + tmp_files +end + +# to be used with maxmind datasets (should be an array of arrays) +def to_csv(content, prefix = '') + buffer = prefix + count = 0 + while count < content.length + buffer += content[count].to_csv + count = count + 1 + end + buffer[0..buffer.length-2] # take off last trailing \n +end + +# from here: http://stackoverflow.com/questions/2204058/show-which-columns-an-index-is-on-in-postgresql +def list_indexes(table_name) + result = GeoIpBlocks.connection.execute("select + i.relname as index_name, + array_to_string(array_agg(a.attname), ', ') as column_names +from + pg_class t, + pg_class i, + pg_index ix, + pg_attribute a +where + t.oid = ix.indrelid + and i.oid = ix.indexrelid + and a.attrelid = t.oid + and a.attnum = ANY(ix.indkey) + and t.relkind = 'r' + and t.relname = '#{table_name}' +group by + t.relname, + i.relname +order by + t.relname, + i.relname;") + + result.values.map { |row| row[0] } +end + +def table_exists?(table_name) + GeoIpBlocks.connection.select_value("SELECT 1 + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' + AND c.relname = '#{table_name}'"); +end + +def create_phony_database + GeoIpBlocks.connection.execute("select generate_scores_dataset()").check +end + +# gets related models for an IP in the 1st block from the scores_better_test_data.sql +def austin_geoip + geoiplocation = GeoIpLocations.find_by_locid(17192) + geoipblock = GeoIpBlocks.find_by_locid(17192) + jamisp = JamIsp.find_by_beginip(geoipblock.beginip) + {jamisp: jamisp, geoiplocation: geoiplocation, geoipblock: geoipblock } +end + +# gets related models for an IP in the 1st block from the scores_better_test_data.sql +def dallas_geoip + geoiplocation = GeoIpLocations.find_by_locid(667) + geoipblock = GeoIpBlocks.find_by_locid(667) + jamisp = JamIsp.find_by_beginip(geoipblock.beginip) + {jamisp: jamisp, geoiplocation: geoiplocation, geoipblock: geoipblock} +end + +# attempts to make the creation of a score more straightforward. +# a_geoip and b_geoip are hashes with keys jamisp and geoiplocation (like those created by austin_geoip and dallas_geoip) +def create_score(a_geoip, b_geoip, a_addr = a_geoip[:jamisp].beginip, b_addr = b_geoip[:jamisp].beginip, + a_client_id = 'a_client_id', b_client_id = 'b_client_id', score = 10, score_dt = Time.now, score_data = nil) + Score.createx(Score.create_locidispid(a_geoip[:geoiplocation], a_geoip[:jamisp]), a_client_id, a_addr, + Score.create_locidispid(b_geoip[:geoiplocation], b_geoip[:jamisp]), b_client_id, b_addr, + score, score_dt, score_data) +end + +def locidispid_from_ip(ip_address) + location = GeoIpLocations.lookup(ip_address) + + if location + location[:locidispid] + else + nil + end +end \ No newline at end of file diff --git a/websocket-gateway/lib/jam_websockets/router.rb b/websocket-gateway/lib/jam_websockets/router.rb index f39852132..686cf06f0 100644 --- a/websocket-gateway/lib/jam_websockets/router.rb +++ b/websocket-gateway/lib/jam_websockets/router.rb @@ -647,6 +647,7 @@ module JamWebsockets # log this connection in the database ConnectionManager.active_record_transaction do |connection_manager| connection_manager.create_connection(user.id, client.client_id, client.channel_id, remote_ip, client_type, connection_stale_time, connection_expire_time) do |conn, count| + user.update_addr_loc(Connection.find_by_client_id(client.client_id), User::JAM_REASON_LOGIN) if count == 1 Notification.send_friend_update(user.id, true, conn) end diff --git a/websocket-gateway/spec/jam_websockets/router_spec.rb b/websocket-gateway/spec/jam_websockets/router_spec.rb index 31020c738..cafd818ba 100644 --- a/websocket-gateway/spec/jam_websockets/router_spec.rb +++ b/websocket-gateway/spec/jam_websockets/router_spec.rb @@ -228,6 +228,33 @@ describe Router do @user = FactoryGirl.create(:user, :password => "foobar", :password_confirmation => "foobar") client1 = login(@router, @user, "foobar", "1", @user.remember_token, "client") + @user.reload + + # verify that last_jam_reason, last_jam_addr, and last_jam_updated_at were updated + @user.last_jam_addr.should == ip_address_to_int('127.0.0.1') + @user.last_jam_updated_reason.should == User::JAM_REASON_LOGIN + @user.last_jam_updated_at.should_not be_nil + puts @user.last_jam_addr + @user.last_jam_locidispid.should == GeoIpLocations.lookup(@user.last_jam_addr)[:locidispid] + done + end + + it "should not update last_jam info if already set", :mq => true do + last_jam_addr = ip_address_to_int('10.0.0.1') + last_jam_updated_reason = User::JAM_REASON_REGISTRATION + last_jam_updated_at = 5.hours.ago + last_jam_locidispid = 1 + @user = FactoryGirl.create(:user, + :password => "foobar", :password_confirmation => "foobar", + last_jam_addr: last_jam_addr, last_jam_updated_reason: last_jam_updated_reason, last_jam_updated_at: last_jam_updated_at, last_jam_locidispid: last_jam_locidispid) + client1 = login(@router, @user, "foobar", "1", @user.remember_token, "client") + @user.reload + + # verify that last_jam_reason, last_jam_addr, and last_jam_updated_at were updated + @user.last_jam_addr.should == last_jam_addr + @user.last_jam_updated_reason.should == last_jam_updated_reason + @user.last_jam_updated_at.should == last_jam_updated_at + @user.last_jam_locidispid.should == last_jam_locidispid done end