diff --git a/ruby/lib/jam_ruby.rb b/ruby/lib/jam_ruby.rb index c5376cea8..d301a7cea 100755 --- a/ruby/lib/jam_ruby.rb +++ b/ruby/lib/jam_ruby.rb @@ -28,6 +28,7 @@ require "jam_ruby/constants/validation_messages" require "jam_ruby/errors/jam_permission_error" require "jam_ruby/errors/state_error" require "jam_ruby/errors/jam_argument_error" +require "jam_ruby/errors/jam_record_not_found" require "jam_ruby/errors/conflict_error" require "jam_ruby/lib/app_config" require "jam_ruby/lib/s3_manager_mixin" diff --git a/ruby/lib/jam_ruby/connection_manager.rb b/ruby/lib/jam_ruby/connection_manager.rb index e0324a77a..64b84ddbb 100644 --- a/ruby/lib/jam_ruby/connection_manager.rb +++ b/ruby/lib/jam_ruby/connection_manager.rb @@ -408,7 +408,15 @@ SQL ConnectionManager.active_record_transaction do |connection_manager| db_conn = connection_manager.pg_conn - connection = Connection.find_by_client_id_and_user_id!(client_id, user.id) + connection = Connection.find_by_client_id(client_id) + + if connection.nil? + raise JamRecordNotFound.new("Unable to find connection by client_id #{client_id}", 'Connection') + elsif connection.user_id.nil? + raise JamPermissionError, "no user_id associated with connection #{client_id}" + elsif connection.user_id != user.id + raise JamPermissionError, "wrong user_id associated with connection #{client_id}" + end connection.join_the_session(music_session, as_musician, tracks, user, audio_latency, video_sources) JamRuby::MusicSessionUserHistory.join_music_session(user.id, music_session.id) diff --git a/ruby/lib/jam_ruby/errors/jam_record_not_found.rb b/ruby/lib/jam_ruby/errors/jam_record_not_found.rb new file mode 100644 index 000000000..d000f58d5 --- /dev/null +++ b/ruby/lib/jam_ruby/errors/jam_record_not_found.rb @@ -0,0 +1,12 @@ +module JamRuby + class JamRecordNotFound < StandardError + + attr_accessor :missing_message, :record_type + + def initialize(message, record_type) + @message = message + @missing_message = message + @record_type = record_type + end + end +end \ No newline at end of file diff --git a/ruby/spec/jam_ruby/connection_manager_spec.rb b/ruby/spec/jam_ruby/connection_manager_spec.rb index 33bb6d517..694323832 100644 --- a/ruby/spec/jam_ruby/connection_manager_spec.rb +++ b/ruby/spec/jam_ruby/connection_manager_spec.rb @@ -107,7 +107,7 @@ describe ConnectionManager, no_transaction: true do cc.locidispid.should == 17192000002 cc.udp_reachable.should == true - @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, false, GATEWAY, false) + @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, false, GATEWAY) cc = Connection.find_by_client_id!(client_id) cc.connected?.should be_true @@ -147,7 +147,7 @@ describe ConnectionManager, no_transaction: true do cc.locidispid.should == 17192000002 cc.udp_reachable.should == false - @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, nil, GATEWAY, false) # heartbeat passes nil in for udp_reachable + @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, nil, GATEWAY) # heartbeat passes nil in for udp_reachable cc = Connection.find_by_client_id!(client_id) cc.connected?.should be_true @@ -356,7 +356,7 @@ describe ConnectionManager, no_transaction: true do user = User.find(user_id) - expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) }.to raise_error(ActiveRecord::RecordNotFound) + expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) }.to raise_error(JamRuby::JamRecordNotFound) end @@ -429,7 +429,7 @@ describe ConnectionManager, no_transaction: true do @connman.create_connection(user_id, client_id, channel_id, "1.1.1.1", 'client', STALE_TIME, EXPIRE_TIME, REACHABLE, GATEWAY, false) # specify real user id, but not associated with this session - expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(ActiveRecord::RecordNotFound) + expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(JamRuby::JamPermissionError) end it "join_music_session fails if no music_session" do @@ -455,7 +455,7 @@ describe ConnectionManager, no_transaction: true do @connman.create_connection(user_id, client_id, channel_id, "1.1.1.1", 'client', STALE_TIME, EXPIRE_TIME, REACHABLE, GATEWAY, false) # specify real user id, but not associated with this session - expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(ActiveRecord::RecordNotFound) + expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(JamRuby::JamPermissionError) end diff --git a/ruby/spec/jam_ruby/models/active_music_session_spec.rb b/ruby/spec/jam_ruby/models/active_music_session_spec.rb index 827dcaaf7..2dc75cfc8 100644 --- a/ruby/spec/jam_ruby/models/active_music_session_spec.rb +++ b/ruby/spec/jam_ruby/models/active_music_session_spec.rb @@ -17,7 +17,14 @@ describe ActiveMusicSession do it "fails gracefully when no connection" do music_session = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false) - expect { ActiveMusicSession.participant_create(user, music_session.id, "junk", true, nil, 5) }.to raise_error(ActiveRecord::RecordNotFound) + begin + ActiveMusicSession.participant_create(user, music_session.id, "junk", true, nil, 5) + false.should be_true + rescue JamRuby::JamRecordNotFound => e + e.record_type.should eql "Connection" + e.missing_message.should eql "Unable to find connection by client_id junk" + end + end it "succeeds no active music session" do @@ -34,12 +41,6 @@ describe ActiveMusicSession do expect { ActiveMusicSession.participant_create(user, 'bad music session ID', conn.client_id, true, nil, 5) }.to raise_error(ActiveRecord::RecordNotFound) end - it "fails gracefully when invalid music session Id" do - music_session = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false) - conn = FactoryGirl.create(:connection, :user => user) - ActiveMusicSession.participant_create(user, 'bad music session ID', conn.client_id, true, nil, 5) - end - it "pulls out of other session" do music_session1 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false) music_session2 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false) @@ -52,7 +53,8 @@ describe ActiveMusicSession do it "user-less connection bails appropriately" do music_session1 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false) conn = FactoryGirl.create(:connection) - expect { ActiveMusicSession.participant_create(user, music_session1.id, conn.client_id, true, nil, 5) }.to raise_error(ActiveRecord::RecordNotFound) + ActiveRecord::Base.connection.execute("update connections set user_id = NULL where connections.id = '#{conn.id}'") + expect { ActiveMusicSession.participant_create(user, music_session1.id, conn.client_id, true, nil, 5) }.to raise_error(JamRuby::JamPermissionError) end end diff --git a/web/app/controllers/api_controller.rb b/web/app/controllers/api_controller.rb index 7aa20de41..3d102c9b9 100644 --- a/web/app/controllers/api_controller.rb +++ b/web/app/controllers/api_controller.rb @@ -19,6 +19,10 @@ class ApiController < ApplicationController @exception = exception render "errors/permission_error", :status => 403 end + rescue_from 'JamRuby::JamRecordNotFound' do |exception| + @exception = exception + render "errors/record_not_found", :status => 404 + end rescue_from 'JamRuby::ConflictError' do |exception| @exception = exception render "errors/conflict_error", :status => 409 diff --git a/web/app/views/errors/record_not_found.rabl b/web/app/views/errors/record_not_found.rabl new file mode 100644 index 000000000..c64f78d6e --- /dev/null +++ b/web/app/views/errors/record_not_found.rabl @@ -0,0 +1,11 @@ +object @exception + +attributes :record_type + +node "message" do |e| + e.missing_message +end + +node "type" do + "JamRecordNotFound" +end