From e8d74a119c0a468878c113b6787150d82a157187 Mon Sep 17 00:00:00 2001 From: Nuwan Date: Thu, 11 Feb 2021 19:35:49 +0530 Subject: [PATCH] fix session overlapped duration with other users change calculation of MusicSessionUserHistory.duration_minutes to exactly get the number of minutes overlapped with other user sessions. previously it returned the entire music session time if a other user joined in and left without staying compleyely within the session. this commit also fixs an edge case of the query in MusicSessionUserHistory.overlapping_connections --- ruby/Gemfile | 3 +- ruby/Gemfile.lock | 12 +- .../models/music_session_user_history.rb | 52 +++++-- .../music_sessions_user_history_spec.rb | 143 +++++++++++++++++- 4 files changed, 192 insertions(+), 18 deletions(-) diff --git a/ruby/Gemfile b/ruby/Gemfile index 0f36ab103..ae1ce7ce6 100644 --- a/ruby/Gemfile +++ b/ruby/Gemfile @@ -51,7 +51,8 @@ gem "activerecord-import", "~> 0.4.1" gem "auto_strip_attributes" gem "json", "1.8.6" gem 'uuidtools', '2.1.2' -gem 'bcrypt-ruby', '3.0.1' +gem 'bcrypt', '3.1.15' +gem 'bcrypt-ruby' #, '3.0.1' gem 'ruby-protocol-buffers', '1.2.2' gem 'eventmachine', '1.0.4' gem 'amqp', '1.0.2' diff --git a/ruby/Gemfile.lock b/ruby/Gemfile.lock index c1f58a4c7..5f377da57 100644 --- a/ruby/Gemfile.lock +++ b/ruby/Gemfile.lock @@ -76,8 +76,9 @@ GEM aws-sdk-v1 (1.67.0) json (~> 1.4) nokogiri (~> 1) - bcrypt (3.1.16) - bcrypt-ruby (3.0.1) + bcrypt (3.1.15) + bcrypt-ruby (3.1.5) + bcrypt (>= 3.1.3) builder (3.2.4) carrierwave (0.9.0) activemodel (>= 3.2.0) @@ -493,7 +494,8 @@ DEPENDENCIES amqp (= 1.0.2) auto_strip_attributes aws-sdk (~> 1) - bcrypt-ruby (= 3.0.1) + bcrypt (= 3.1.15) + bcrypt-ruby builder carrierwave (= 0.9.0) database_cleaner (= 1.4.1) @@ -555,7 +557,7 @@ DEPENDENCIES zip-codes RUBY VERSION - ruby 2.3.1p112 + ruby 2.4.1p111 BUNDLED WITH - 1.17.2 + 1.17.3 diff --git a/ruby/lib/jam_ruby/models/music_session_user_history.rb b/ruby/lib/jam_ruby/models/music_session_user_history.rb index 3836b4231..0972f49e7 100644 --- a/ruby/lib/jam_ruby/models/music_session_user_history.rb +++ b/ruby/lib/jam_ruby/models/music_session_user_history.rb @@ -55,9 +55,28 @@ module JamRuby self.user ? self.user.email : '' end + # def duration_minutes + # end_time = self.session_removed_at || Time.now + # (end_time - self.created_at) / 60.0 + # end + + #take duration that this user session was with other + # def duration_minutes - end_time = self.session_removed_at || Time.now - (end_time - self.created_at) / 60.0 + query = <<-SQL + select sum(upper(diff) - lower(diff)) as duration + FROM ( + SELECT tsrange(created_at, session_removed_at, '[]') * tsrange('#{self.created_at.strftime '%Y-%m-%d %H:%M:%S'}', '#{self.session_removed_at.strftime '%Y-%m-%d %H:%M:%S'}', '[]') AS diff + FROM music_sessions_user_history + WHERE music_session_id = '#{self.music_session_id}' + AND id <> '#{self.id}' + + ) AS derivedTable +SQL + result = ActiveRecord::Base.connection.exec_query(query) + duration = result.cast_values[0] + return 0 unless duration + (duration.split(':').map(&:to_f).inject(0) { |a, b| a * 60 + b } / 60.0).round end def self.join_music_session(user_id, session_id, client_id) @@ -89,10 +108,10 @@ module JamRuby def end_history if self.session_removed_at.nil? self.session_removed_at = Time.now - self.max_concurrent_connections = determine_max_concurrent - self.update_attributes(:session_removed_at => self.session_removed_at, :max_concurrent_connections => self.max_concurrent_connections) - self.update_remaining_play_time end + self.max_concurrent_connections = determine_max_concurrent + self.update_attributes(:session_removed_at => self.session_removed_at, :max_concurrent_connections => self.max_concurrent_connections) + self.update_remaining_play_time end # update the users monthly play time @@ -116,20 +135,33 @@ module JamRuby # figures out what the peak amount of other clients the user saw while playing at any given time. # we use the database to get all other connections that occurred while their this user was connected, # and then sort through them using custom logic to increase/decrease the count as people come and go + def overlapping_connections + MusicSessionUserHistory.where("music_session_id = ? AND + ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR + (created_at > ? AND created_at < ? AND (session_removed_at is NULL OR session_removed_at > ?)) OR + (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))", + self.music_session_id, + self.created_at, self.session_removed_at, + self.created_at, self.session_removed_at, self.session_removed_at, + self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at) + end + def determine_max_concurrent - overlapping_connections = MusicSessionUserHistory.where("music_session_id = ? AND - ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR - (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))", - self.music_session_id, self.created_at, self.session_removed_at, self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at) + # overlapping_connections = MusicSessionUserHistory.where("music_session_id = ? AND + # ((created_at >= ? AND (session_removed_at is NULL OR session_removed_at <= ?)) OR + # (created_at <= ? AND (session_removed_at is NULL OR session_removed_at >= ?)))", + # self.music_session_id, self.created_at, self.session_removed_at, self.created_at, self.created_at).select('id, created_at, session_removed_at').order(:created_at) in_out_times = [] overlapping_connections.each do |connection| in_out_times.push([connection.created_at.nil? ? Time.at(0) : connection.created_at, true]) in_out_times.push([connection.session_removed_at.nil? ? Time.new(3000) : connection.session_removed_at, false]) #helpful to get rid of nulls for sorting end - + #raise ">>>>>> #{in_out_times.inspect}" count_concurrent(in_out_times) end + + def count_concurrent(in_out_times) in_out_times.sort! { |a,b| a[0] <=> b[0] } diff --git a/ruby/spec/jam_ruby/models/music_sessions_user_history_spec.rb b/ruby/spec/jam_ruby/models/music_sessions_user_history_spec.rb index edd19bbc1..be2316e1a 100644 --- a/ruby/spec/jam_ruby/models/music_sessions_user_history_spec.rb +++ b/ruby/spec/jam_ruby/models/music_sessions_user_history_spec.rb @@ -7,6 +7,7 @@ describe MusicSessionUserHistory do let(:user_history1) { FactoryGirl.create(:music_session_user_history, :history => music_session.music_session, :user => music_session.creator) } let(:user_history2) { FactoryGirl.create(:music_session_user_history, :history => music_session.music_session, :user => some_user) } + after { Timecop.return } @@ -19,6 +20,20 @@ describe MusicSessionUserHistory do describe "rating" do + def stub_app_config + #stubbing APP_CONFIG + app_config = Class.new{ + def rating_dialog_min_time + 60 + end + + def rating_dialog_min_num + 1 + end + } + stub_const("APP_CONFIG", app_config.new) + end + it "success" do user_history1.update_attribute(:rating, 1) expect( user_history1.errors.any? ).to eq(false) @@ -31,12 +46,15 @@ describe MusicSessionUserHistory do end it 'should rate success' do + stub_app_config users = [user_history1, user_history2] - Timecop.travel(Time.now + (MusicSessionUserHistory::MIN_SESSION_DURATION_RATING * 1.5).seconds) + #Timecop.travel(Time.now + (MusicSessionUserHistory::MIN_SESSION_DURATION_RATING * 1.5).seconds) + Timecop.travel(Time.now + (APP_CONFIG.rating_dialog_min_time * 1.5).seconds) expect( user_history1.should_rate_session? ).to eq(true) end it 'should rate fails' do + stub_app_config users = [user_history1] expect( user_history1.should_rate_session? ).to eq(false) users = [user_history1, user_history2] @@ -54,6 +72,13 @@ describe MusicSessionUserHistory do user_history1.max_concurrent_connections.should == 2 end + it "when history1 only" do + user_history1.save.should be_true + user_history1.reload + user_history1.end_history + user_history1.max_concurrent_connections.should == 1 + end + it "history2 ends before history1 starts" do user_history2.created_at = user_history1.created_at - 2 user_history2.session_removed_at = user_history1.created_at - 1 @@ -74,7 +99,37 @@ describe MusicSessionUserHistory do user_history1.max_concurrent_connections.should == 1 end - it "history2 is within bounds of history1 " do + it "history2 begin before history1 starts and terminates before history1 end" do + user_history1.session_removed_at = user_history1.created_at + 3 + user_history2.created_at = user_history1.created_at - 1 + user_history2.session_removed_at = user_history1.created_at + 2 + + user_history2.save.should be_true + + user_history1.reload + user_history2.reload + user_history1.end_history + + user_history1.max_concurrent_connections.should == 2 + end + + it "history2 begin after history1 starts and terminates after history1 end" do + user_history1.session_removed_at = user_history1.created_at + 3 + user_history2.created_at = user_history1.created_at + 1 + user_history2.session_removed_at = user_history1.created_at + 4 + + user_history1.save.should be_true + user_history2.save.should be_true + + user_history1.reload + user_history2.reload + user_history1.end_history + + user_history1.max_concurrent_connections.should == 2 + end + + + it "history2 is within bounds of history1 " do user_history1.session_removed_at = user_history1.created_at + 3 user_history2.created_at = user_history1.created_at + 1 user_history2.session_removed_at = user_history1.created_at + 2 @@ -147,6 +202,90 @@ describe MusicSessionUserHistory do user_history1.max_concurrent_connections.should == 3 end end + + describe "duration_minutes" do + it "returns zero when history2 ends before history1 starts" do + + user_history1.session_removed_at = user_history1.created_at + 5 + user_history2.created_at = user_history1.created_at - 2 + user_history2.session_removed_at = user_history1.created_at - 1 + + user_history2.save.should be_true + + user_history1.reload + user_history2.reload + user_history1.end_history + + expect(user_history1.duration_minutes).to eq 0 + + end + + + it "returns zero when history2 starts after history1 ends" do + + user_history1.session_removed_at = user_history1.created_at + 5 + user_history2.created_at = user_history1.session_removed_at + 1 + user_history2.session_removed_at = user_history1.session_removed_at + 2 + + user_history2.save.should be_true + + user_history1.reload + user_history2.reload + user_history1.end_history + + #user_history1.max_concurrent_connections.should == 1 + + expect(user_history1.duration_minutes).to eq 0 + + end + + + it "returns overlapped time when history2 is within bounds of history1" do + #pending + user_history1.session_removed_at = user_history1.created_at + 3*60 + user_history2.created_at = user_history1.created_at + 1*60 + user_history2.session_removed_at = user_history1.created_at + 3*60 + + user_history1.save.should be_true + user_history2.save.should be_true + + user_history1.reload + user_history2.reload + user_history1.end_history + #user_history1.max_concurrent_connections.should == 2 + expect(user_history1.duration_minutes).to eq 2 + end + + it "returns overlapped time when history2 begings before history1 start and terminates before history1 end" do + user_history1.session_removed_at = user_history1.created_at + 3*60 + user_history2.created_at = user_history1.created_at - 1*60 + user_history2.session_removed_at = user_history1.created_at + 2*60 + + expect(user_history1.save).to eq true + expect(user_history2.save).to eq true + + user_history1.reload + user_history2.reload + user_history1.end_history + + expect(user_history1.duration_minutes).to eq 2 + end + + + it "returns overlapped time when history2 begings after history1 start and terminates after history1 end" do + user_history1.session_removed_at = user_history1.created_at + 3*60 + user_history2.created_at = user_history1.created_at + 1*60 + user_history2.session_removed_at = user_history1.created_at + 4*60 + + expect(user_history1.save).to eq true + expect(user_history2.save).to eq true + + user_history1.end_history + + expect(user_history1.duration_minutes).to eq 2 + + end + end end