From 9424ffa6aa02ca904894edc53b8f3f947cdbae4f Mon Sep 17 00:00:00 2001 From: Seth Call Date: Wed, 7 Aug 2013 10:35:27 -0500 Subject: [PATCH] * VRFS-484; more changes needed but this helps a lot of common leave/join scenarios --- lib/jam_ruby/connection_manager.rb | 172 ++++++++++++----------- lib/jam_ruby/message_factory.rb | 17 ++- lib/jam_ruby/models/notification.rb | 22 ++- spec/jam_ruby/connection_manager_spec.rb | 19 +-- 4 files changed, 133 insertions(+), 97 deletions(-) diff --git a/lib/jam_ruby/connection_manager.rb b/lib/jam_ruby/connection_manager.rb index f400c780d..c820abe74 100644 --- a/lib/jam_ruby/connection_manager.rb +++ b/lib/jam_ruby/connection_manager.rb @@ -11,6 +11,10 @@ module JamRuby # This may make sense in the short term if we are still managing connections in the database, but # we move to the node-js in the websocket gateway (because the websocket gateway needs to call some of these methods). # Or of course we could just port the relevant methods to node-js +# +# Also we don't send notifications from ConnectionManager; +# we just return enough data so that a caller can make the determination if it needs to + class ConnectionManager < BaseManager def initialize(options={}) @@ -33,32 +37,64 @@ module JamRuby return friend_ids end - def reconnect(conn) - sql =< 0) + # If a blk is passed in, on success, count is also passed back an the db connection, allowing for + # notifications to go out within the table log. music_session_id is also passed, if the music_session still exists + # and this connection was in a session + def delete_connection(client_id, &blk) + ConnectionManager.active_record_transaction do |connection_manager| - conn = connection_manager.pg_conn - + count = 0 user_id = nil music_session_id = nil @@ -152,7 +202,7 @@ SQL return elsif result.cmd_tuples == 1 user_id = result[0]['user_id'] - music_session_id = result[0]['client_id'] + music_session_id = result[0]['music_session_id'] else raise Exception, 'uniqueness constraint has been lost on client_id' @@ -164,17 +214,19 @@ SQL # since we did delete a row, check and see if any more connections for that user exist # if we are down to zero, send out user gone message conn.exec("SELECT count(user_id) FROM connections where user_id = $1", [user_id]) do |result| - count = result.getvalue(0, 0) - if count == "0" - # send notification - Notification.send_friend_update(user_id, false, conn) - end + count = result.getvalue(0, 0).to_i end # same for session-if we are down to the last participant, delete the session unless music_session_id.nil? - conn.exec("DELETE FROM music_sessions id = $1 AND 0 = (SELECT count(music_session_id) FROM connections where music_session_id = $1)", [music_session_id]).clear + result = conn.exec("DELETE FROM music_sessions WHERE id = $1 AND 0 = (select count(music_session_id) FROM connections where music_session_id = $1)", [music_session_id]) + if result.cmd_tuples == 1 + music_session_id = nil + end end + + blk.call(conn, count, music_session_id) unless blk.nil? + return count end end @@ -224,7 +276,9 @@ SQL end end - def join_music_session(user, client_id, music_session, as_musician, tracks) + # if a blk is passed in, upon success, it will be called and you can issue notifications + # within the connection table lock + def join_music_session(user, client_id, music_session, as_musician, tracks, &blk) connection = nil user_id = user.id music_session_id = music_session.id @@ -242,10 +296,7 @@ SQL if connection.errors.any? raise ActiveRecord::Rollback else - if as_musician && music_session.musician_access - Notification.send_musician_session_join(music_session, connection, user) - Notification.send_friend_session_join(db_conn, connection, user) - end + blk.call(db_conn, connection) unless blk.nil? MusicSessionUserHistory.save(music_session_id, user_id, client_id) end end @@ -253,57 +304,9 @@ SQL return connection end - def join_music_session_old(user_id, client_id, music_session_id, as_musician) - conn = @pg_conn - - lock_connections(conn) - - previous_music_session_id = check_already_session(conn, client_id) - - user = User.find(user_id) - - if as_musician != true && as_musician != false # checks that a boolean was passed in - raise JamArgumentError, "as_musician incorrectly specified" - end - - # determine if the user can join; if not, throw a PermissionError - music_session = MusicSession.find(music_session_id) - - - unless music_session.can_join?(user, as_musician) - @log.debug "user can not join a session user_id=#{user_id} and client_id=#{client_id}" - raise PermissionError, "unable to join the specified session" - end - - begin - # we include user_id in the query as an act of security, so that a user can't access someone else's client connection - conn.exec("UPDATE connections SET music_session_id = $1, as_musician = $2 WHERE client_id = $3 and user_id = $4", [music_session_id, as_musician, client_id, user_id]) do |result| - if result.cmd_tuples == 1 - @log.debug "associated music_session with connection for client=#{client_id}, music_session=#{music_session_id}, and user=#{user_id}" - - session_checks(conn, previous_music_session_id, user_id) - elsif result.cmd_tuples == 0 - @log.debug "join_music_session no connection found with client_id=#{client_id} and user_id=#{user_id}" - raise ActiveRecord::RecordNotFound - else - @log.error "database failure or logic error; this path should be impossible if the table is locked (join_music_session)" - raise Exception, "locked table changed state" - end - end - rescue PG::Error => pg_error - if pg_error.to_s.include?("insert or update on table \"connections\" violates foreign key constraint \"connections_music_session_id_fkey\"") - # if there is no music session that exists, we will receive this message - @log.debug "music_session does not exist. music_session=#{music_session_id}" - raise StateError, "music_session does not exist" - else - raise pg_error - end - - end - # end - end - - def leave_music_session(user, connection, music_session) + # if a blk is passed in, upon success, it will be called and you can issue notifications + # within the connection table lock + def leave_music_session(user, connection, music_session, &blk) ConnectionManager.active_record_transaction do |connection_manager| conn = connection_manager.pg_conn @@ -331,7 +334,8 @@ SQL session_checks(conn, previous_music_session_id, user_id) - Notification.send_musician_session_depart(music_session, connection, user) + blk.call() unless blk.nil? + elsif result.cmd_tuples == 0 @log.debug "leave_music_session no connection found with client_id=#{client_id}" raise ActiveRecord::RecordNotFound diff --git a/lib/jam_ruby/message_factory.rb b/lib/jam_ruby/message_factory.rb index 82391f4f4..144df8e35 100644 --- a/lib/jam_ruby/message_factory.rb +++ b/lib/jam_ruby/message_factory.rb @@ -35,8 +35,8 @@ end # create a login ack (login was successful) - def login_ack(public_ip, client_id, token, heartbeat_interval, music_session_id) - login_ack = Jampb::LoginAck.new(:public_ip => public_ip, :client_id => client_id, :token => token, :heartbeat_interval => heartbeat_interval, :music_session_id => music_session_id) + def login_ack(public_ip, client_id, token, heartbeat_interval, music_session_id, reconnected) + login_ack = Jampb::LoginAck.new(:public_ip => public_ip, :client_id => client_id, :token => token, :heartbeat_interval => heartbeat_interval, :music_session_id => music_session_id, :reconnected => reconnected) return Jampb::ClientMessage.new(:type => ClientMessage::Type::LOGIN_ACK, :route_to => CLIENT_TARGET, :login_ack => login_ack) end @@ -112,6 +112,19 @@ return Jampb::ClientMessage.new(:type => ClientMessage::Type::MUSICIAN_SESSION_DEPART, :route_to => CLIENT_TARGET, :musician_session_depart => left) end + # create a musician fresh session message + def musician_session_fresh(session_id, user_id, username, photo_url) + fresh = Jampb::MusicianSessionFresh.new(:session_id => session_id, :user_id => user_id, :username => username, :photo_url => photo_url) + return Jampb::ClientMessage.new(:type => ClientMessage::Type::MUSICIAN_SESSION_FRESH, :route_to => CLIENT_TARGET, :musician_session_fresh => fresh) + end + + # create a musician stale session message + def musician_session_stale(session_id, user_id, username, photo_url) + stale = Jampb::MusicianSessionStale.new(:session_id => session_id, :user_id => user_id, :username => username, :photo_url => photo_url) + return Jampb::ClientMessage.new(:type => ClientMessage::Type::MUSICIAN_SESSION_STALE, :route_to => CLIENT_TARGET, :musician_session_stale => stale) + end + + # create a user-joined session message def join_request(session_id, join_request_id, username, text) join_request = Jampb::JoinRequest.new(:join_request_id => join_request_id, :username => username, :text => text) diff --git a/lib/jam_ruby/models/notification.rb b/lib/jam_ruby/models/notification.rb index 6d82c2190..0afc93ad5 100644 --- a/lib/jam_ruby/models/notification.rb +++ b/lib/jam_ruby/models/notification.rb @@ -223,13 +223,31 @@ module JamRuby @@mq_router.server_publish_to_session(music_session, msg, sender = {:client_id => connection.client_id}) end - def send_musician_session_depart(music_session, connection, user) + def send_musician_session_depart(music_session, client_id, user) # (1) create notification msg = @@message_factory.musician_session_depart(music_session.id, user.id, user.name, user.photo_url) # (2) send notification - @@mq_router.server_publish_to_session(music_session, msg, sender = {:client_id => connection.client_id}) + @@mq_router.server_publish_to_session(music_session, msg, sender = {:client_id => client_id}) + end + + def send_musician_session_fresh(music_session, client_id, user) + + # (1) create notification + msg = @@message_factory.musician_session_fresh(music_session.id, user.id, user.name, user.photo_url) + + # (2) send notification + @@mq_router.server_publish_to_session(music_session, msg, sender = {:client_id => client_id}) + end + + def send_musician_session_stale(music_session, client_id, user) + + # (1) create notification + msg = @@message_factory.musician_session_stale(music_session.id, user.id, user.name, user.photo_url) + + # (2) send notification + @@mq_router.server_publish_to_session(music_session, msg, sender = {:client_id => client_id}) end def send_friend_session_join(db_conn, connection, user) diff --git a/spec/jam_ruby/connection_manager_spec.rb b/spec/jam_ruby/connection_manager_spec.rb index 8b1239c55..0056e9486 100644 --- a/spec/jam_ruby/connection_manager_spec.rb +++ b/spec/jam_ruby/connection_manager_spec.rb @@ -62,8 +62,8 @@ describe ConnectionManager do client_id = "client_id2" user_id = create_user("test", "user2", "user2@jamkazam.com") - connection = @connman.create_connection(user_id, client_id, "1.1.1.1") - + count = @connman.create_connection(user_id, client_id, "1.1.1.1") + count.should == 1 # make sure the connection is seen @conn.exec("SELECT count(*) FROM connections where user_id = $1", [user_id]) do |result| @@ -73,7 +73,8 @@ describe ConnectionManager do cc = Connection.find_by_client_id!(client_id) cc.connected?.should be_true - @connman.delete_connection(client_id) + count = @connman.delete_connection(client_id) + count.should == 0 @conn.exec("SELECT count(*) FROM connections where user_id = $1", [user_id]) do |result| result.getvalue(0, 0).should == "0" @@ -377,9 +378,9 @@ describe ConnectionManager do user = User.find(user_id) dummy_music_session = MusicSession.new - connection = @connman.create_connection(user_id, client_id, "1.1.1.1") + @connman.create_connection(user_id, client_id, "1.1.1.1") - expect { @connman.leave_music_session(user, connection, dummy_music_session) }.to raise_error(JamRuby::StateError) + expect { @connman.leave_music_session(user, Connection.find_by_client_id(client_id), dummy_music_session) }.to raise_error(JamRuby::StateError) end it "leave_music_session fails if in different music_session" do @@ -393,9 +394,9 @@ describe ConnectionManager do dummy_music_session = MusicSession.new - connection = @connman.create_connection(user_id, client_id, "1.1.1.1") + @connman.create_connection(user_id, client_id, "1.1.1.1") @connman.join_music_session(user, client_id, music_session, true, TRACKS) - expect { @connman.leave_music_session(user, connection, dummy_music_session) }.to raise_error(JamRuby::StateError) + expect { @connman.leave_music_session(user, Connection.find_by_client_id(client_id), dummy_music_session) }.to raise_error(JamRuby::StateError) end it "leave_music_session works" do @@ -407,7 +408,7 @@ describe ConnectionManager do user = User.find(user_id) music_session = MusicSession.find(music_session_id) - connection = @connman.create_connection(user_id, client_id, "1.1.1.1") + @connman.create_connection(user_id, client_id, "1.1.1.1") @connman.join_music_session(user, client_id, music_session, true, TRACKS) assert_session_exists(music_session_id, true) @@ -416,7 +417,7 @@ describe ConnectionManager do result.getvalue(0, 0).should == music_session_id end - @connman.leave_music_session(user, connection, music_session) + @connman.leave_music_session(user, Connection.find_by_client_id(client_id), music_session) @conn.exec("SELECT music_session_id FROM connections WHERE client_id = $1", [client_id]) do |result| result.getvalue(0, 0).should == nil