From 02f27eb40759d577013d4bcb35f40179045d6b8c Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Wed, 9 Oct 2013 23:49:37 -0300 Subject: [PATCH] [BUGFIX] Ensure that hungup calls and merged dials can split/rejoin properly --- CHANGELOG.md | 2 + lib/adhearsion/call_controller/dial.rb | 59 ++++---- spec/adhearsion/call_controller/dial_spec.rb | 146 +++++++++++++++++++ 3 files changed, 180 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de298966..512099297 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # [develop](https://github.com/adhearsion/adhearsion) * Bugfix: Don't block shutdown waiting for the console to terminate * Bugfix: Ensure that splitting a dial rejoined to an alternative target (eg a mixer) or merged with another dial can still be split properly. + * Bugfix: Ensure that hungup calls don't prevent dial splits/merges. + * Bugfix: Ensure that merged dials which are split and rejoined all get rejoined to the mixer. # [2.4.0](https://github.com/adhearsion/adhearsion/compare/v2.3.5...v2.4.0) - [2013-08-29](https://rubygems.org/gems/adhearsion/versions/2.4.0) * Deprecation: Ruby 1.9.2 support is deprecated and will be dropped in a future version of Adhearsion diff --git a/lib/adhearsion/call_controller/dial.rb b/lib/adhearsion/call_controller/dial.rb index 9f2c61464..9ff4c50a2 100644 --- a/lib/adhearsion/call_controller/dial.rb +++ b/lib/adhearsion/call_controller/dial.rb @@ -184,33 +184,39 @@ def split(targets = {}) logger.info "Splitting calls apart" @splitting = true @calls.each do |call| - logger.info "Unjoining peer #{call.id} from #{join_target}" - ignoring_missing_joins { call.unjoin join_target } - if split_controller = targets[:others] - logger.info "Executing split controller #{split_controller} on #{call.id}" - call.execute_controller split_controller.new(call, 'current_dial' => self), targets[:others_callback] + ignoring_ended_calls do + logger.info "Unjoining peer #{call.id} from #{join_target}" + ignoring_missing_joins { call.unjoin join_target } + if split_controller = targets[:others] + logger.info "Executing split controller #{split_controller} on #{call.id}" + call.execute_controller split_controller.new(call, 'current_dial' => self), targets[:others_callback] + end end end - if join_target != @call - logger.info "Unjoining main call #{@call.id} from #{join_target}" - @call.unjoin join_target - end - if split_controller = targets[:main] - logger.info "Executing split controller #{split_controller} on main call" - @call.execute_controller split_controller.new(@call, 'current_dial' => self), targets[:main_callback] + ignoring_ended_calls do + if join_target != @call + logger.info "Unjoining main call #{@call.id} from #{join_target}" + @call.unjoin join_target + end + if split_controller = targets[:main] + logger.info "Executing split controller #{split_controller} on main call" + @call.execute_controller split_controller.new(@call, 'current_dial' => self), targets[:main_callback] + end end end # Rejoin parties that were previously split # @param [Call, String, Hash] target The target to join calls to. See Call#join for details. - def rejoin(target = @call) + def rejoin(target = join_target) logger.info "Rejoining to #{target}" - unless target == @call - @join_target = target - @call.join target + ignoring_ended_calls do + unless target == @call + @join_target = target + @call.join target + end end @calls.each do |call| - call.join target + ignoring_ended_calls { call.join target } end end @@ -256,9 +262,8 @@ def skip_cleanup # Hangup any remaining calls def cleanup_calls calls_to_hangup = @calls.map do |call| - begin + ignoring_ended_calls do [call.id, call] if call.active? - rescue Celluloid::DeadActorError end end.compact if calls_to_hangup.size.zero? @@ -270,11 +275,7 @@ def cleanup_calls else logger.info "#dial finished. Hanging up #{calls_to_hangup.size} outbound calls which are still active: #{calls_to_hangup.map(&:first).join ", "}." calls_to_hangup.each do |id, outbound_call| - begin - outbound_call.hangup - rescue Celluloid::DeadActorError - # This actor may previously have been shut down due to the call ending - end + ignoring_ended_calls { outbound_call.hangup } end end end @@ -320,11 +321,9 @@ def pre_join_tasks(call) def on_all_except(call) @calls.each do |target_call| - begin + ignoring_ended_calls do next if target_call.id == call.id yield target_call - rescue Celluloid::DeadActorError - # This actor may previously have been shut down due to the call ending end end end @@ -334,6 +333,12 @@ def ignoring_missing_joins rescue Punchblock::ProtocolError => e raise unless e.name == :service_unavailable end + + def ignoring_ended_calls + yield + rescue Celluloid::DeadActorError, Adhearsion::Call::Hangup, Adhearsion::Call::ExpiredError + # This actor may previously have been shut down due to the call ending + end end class ParallelConfirmationDial < Dial diff --git a/spec/adhearsion/call_controller/dial_spec.rb b/spec/adhearsion/call_controller/dial_spec.rb index 2b9bea54b..81a58bcfe 100644 --- a/spec/adhearsion/call_controller/dial_spec.rb +++ b/spec/adhearsion/call_controller/dial_spec.rb @@ -600,6 +600,79 @@ def run dial.status.result.should be == :answer end + it "should subsequently rejoin to a mixer" do + SecureRandom.stub uuid: 'foobar' + [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true } + + dial.merge other_dial + + waiter_thread = Thread.new do + dial.await_completion + latch.countdown! + end + + sleep 0.5 + + other_mock_call << mock_end + latch.wait(1).should be_false + + [call, other_mock_call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar') + end + end + + dial.split + + [call, other_mock_call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Joined.new(mixer_name: 'foobar') + end + end + + dial.rejoin + end + + context "if a call hangs up" do + before { SecureRandom.stub uuid: 'foobar' } + + it "should still allow splitting and rejoining" do + [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true } + + dial.merge other_dial + + waiter_thread = Thread.new do + dial.await_completion + latch.countdown! + end + + sleep 0.5 + + other_mock_call << mock_end + latch.wait(1).should be_false + + [call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar') + end + end + + other_mock_call.should_receive(:unjoin).and_raise Adhearsion::Call::ExpiredError + + dial.split + + [call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Joined.new(mixer_name: 'foobar') + end + end + + other_mock_call.should_receive(:join).and_raise Adhearsion::Call::Hangup + + dial.rejoin + end + end + context "if the calls were not joined" do it "should still join to mixer" do other_mock_call.should_receive(:unjoin).once.ordered.with(call).and_raise Punchblock::ProtocolError.new.setup(:service_unavailable) @@ -1639,6 +1712,79 @@ def run dial.status.result.should be == :answer end + it "should subsequently rejoin to a mixer" do + SecureRandom.stub uuid: 'foobar' + [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true } + + dial.merge other_dial + + waiter_thread = Thread.new do + dial.await_completion + latch.countdown! + end + + sleep 0.5 + + other_mock_call << mock_end + latch.wait(1).should be_false + + [call, other_mock_call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar') + end + end + + dial.split + + [call, other_mock_call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Joined.new(mixer_name: 'foobar') + end + end + + dial.rejoin + end + + context "if a call hangs up" do + before { SecureRandom.stub uuid: 'foobar' } + + it "should still allow splitting and rejoining" do + [call, other_mock_call, second_root_call, second_other_mock_call].each { |c| c.stub join: true, unjoin: true } + + dial.merge other_dial + + waiter_thread = Thread.new do + dial.await_completion + latch.countdown! + end + + sleep 0.5 + + other_mock_call << mock_end + latch.wait(1).should be_false + + [call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:unjoin).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Unjoined.new(mixer_name: 'foobar') + end + end + + other_mock_call.should_receive(:unjoin).and_raise Adhearsion::Call::ExpiredError + + dial.split + + [call, second_root_call, second_other_mock_call].each do |call| + call.should_receive(:join).once.with(mixer_name: 'foobar').and_return do + call << Punchblock::Event::Joined.new(mixer_name: 'foobar') + end + end + + other_mock_call.should_receive(:join).and_raise Adhearsion::Call::Hangup + + dial.rejoin + end + end + context "if the calls were not joined" do it "should still join to mixer" do other_mock_call.should_receive(:unjoin).once.ordered.with(call).and_raise Punchblock::ProtocolError.new.setup(:service_unavailable)