Merge branch 'release/0.5.1'

.github/workflows/build.yml

@@ -0,0 +1,27 @@
+name: build
+on:
+    - push
+    - pull_request
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        ruby: ['2.1', '2.2', '2.3', '2.4', '2.5', '2.6', '2.7', 'jruby-9.1.17', 'jruby-9.2.21']
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Setup Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ${{ matrix.ruby }}
+          bundler: 1.17.3
+          bundler-cache: true
+
+      - name: Create build
+        run: bundle exec rake build
+
+      - name: Run Specs
+        run: bundle exec rake spec

.gitignore

@@ -1,3 +1,4 @@
 *.swp
 Gemfile.lock
 pkg
+.tool-versions

CHANGELOG.md

@@ -1,4 +1,12 @@
 # [develop](https://github.com/adhearsion/electric_slide)
+  * Bugfix: Conditionally clear agent's call object if the current
+    call is the same as the previous call so that call object information
+    is not lost when the call goes from queued to connected
+
+# [develop](https://github.com/adhearsion/electric_slide)
+  * Added `ElectricSlide::CallQueue#update_agent` to safely update a queued agent object's attributes
+  * Added `ElectricSlide::Agent#update` to update an agent object's attributes
+  * Added `ElectricSlide::Agent#callable?` to check if an agent can be called
 
 # [0.5.0](https://github.com/adhearsion/electric_slide/compare/v0.4.2...v0.5.0) - [2016-02-12](https://rubygems.org/gems/adhearsion/versions/0.5.0)
   * API Breakage: Fixed priority strategy now ensures that the same agent cannot be in multiple priorities

electric_slide.gemspec

@@ -23,9 +23,9 @@ Gem::Specification.new do |s|
   s.rubygems_version = "1.2.0"
   s.summary = "Automatic Call Distributor for Adhearsion"
 
-  s.add_runtime_dependency 'adhearsion'
+  s.add_runtime_dependency 'adhearsion', ['>= 2.5.0', '< 3.0.0']
   s.add_runtime_dependency 'countdownlatch'
-  s.add_runtime_dependency 'activesupport'
+  s.add_runtime_dependency 'activesupport', ['<= 3.2.13']
   s.add_development_dependency 'rspec', ['~> 3.0']
   s.add_development_dependency 'timecop'
   s.add_development_dependency 'ci_reporter'
@@ -34,5 +34,9 @@ Gem::Specification.new do |s|
   s.add_development_dependency 'simplecov'
   s.add_development_dependency 'simplecov-rcov'
 
+  # These two are needed to keep compatibility with Ruby >= 2.2.0, <= 2.2.3
+  s.add_development_dependency 'ruby_dep', ['= 1.3.1']
+  s.add_development_dependency 'listen', ['<= 3.1.1']
+
   s.specification_version = 2
 end

lib/electric_slide/agent.rb

@@ -15,6 +15,22 @@ def initialize(opts = {})
       @presence = opts[:presence] || :available
     end
 
+    # Updates an agent instance's attributes.
+    # @param [Hash] opts Agent attributes
+    # @return {Agent}
+    def update(attrs = {})
+      unless Hash === attrs
+        raise ArgumentError.new('Agent attributes must be a hash')
+      end
+
+      attrs.each do |attr, value|
+        setter = "#{attr}="
+        send setter, value
+      end
+
+      self
+    end
+
     def update_presence(new_presence, extra_params = {})
       old_presence = @presence
       @presence = new_presence
@@ -68,5 +84,10 @@ def join(queued_call)
     def from
       @call.from
     end
+
+    # Returns `true` if the agent can be called, i.e., has a contact address
+    def callable?
+      address.present?
+    end
   end
 end

lib/electric_slide/call_queue.rb

@@ -165,13 +165,7 @@ def add_agent(agent)
       abort DuplicateAgentError.new("Agent is already in the queue") if get_agent(agent.id)
 
       agent.queue = current_actor
-
-      case @connection_type
-      when :call
-        abort ArgumentError.new("Agent has no callable address") unless agent.address
-      when :bridge
-        bridged_agent_health_check agent
-      end
+      accept_agent! agent
 
       logger.info "Adding agent #{agent} to the queue"
       @agents << agent
@@ -182,6 +176,24 @@ def add_agent(agent)
       async.check_for_connections
     end
 
+    # Updates a queued agent's attributes
+    def update_agent(agent, agent_attrs)
+      abort ArgumentError.new('Agent must not be `nil`') unless agent
+      unless get_agent(agent.id)
+        abort MissingAgentError.new('Agent is not in the queue')
+      end
+
+      # check if the agent is allowed to have the given set of attributes using
+      # a dupe, to preserve the state of the original in case of failure
+      agent.dup.tap do |double_agent|
+        double_agent.update agent_attrs
+        accept_agent! double_agent
+      end
+
+      agent.update agent_attrs
+      return_agent agent, agent.presence
+    end
+
     # Marks an agent as available to take a call. To be called after an agent completes a call
     # and is ready to take the next call.
     # @param [Agent] agent The {Agent} that is being returned to the queue
@@ -276,9 +288,10 @@ def remove_call(call)
     # @param [Agent] agent Agent to be connected
     # @param [Adhearsion::Call] call Caller to be connected
     def connect(agent, queued_call)
-      unless queued_call.active?
+      unless queued_call && queued_call.active?
         logger.warn "Inactive queued call found in #connect"
         return_agent agent
+        return
       end
 
       queued_call[:agent] = agent
@@ -288,15 +301,11 @@ def connect(agent, queued_call)
       when :call
         call_agent agent, queued_call
       when :bridge
-        unless agent.call && agent.call.active?
-          logger.warn "Inactive agent call found in #connect, returning caller to queue"
-          priority_enqueue queued_call
-        end
         bridge_agent agent, queued_call
       end
     rescue *ENDED_CALL_EXCEPTIONS
       ignoring_ended_calls do
-        if queued_call.active?
+        if queued_call && queued_call.active?
           logger.warn "Dead call exception in #connect but queued_call still alive, reinserting into queue"
           priority_enqueue queued_call
         end
@@ -392,9 +401,9 @@ def call_agent(agent, queued_call)
 
       agent_call.on_end do |end_event|
         # Ensure we don't return an agent that was removed or paused
+        old_call = agent.call
         conditionally_return_agent agent
-
-        agent.call = nil
+        agent.call = nil if agent_return_method == :manual || agent.call == old_call 
 
         agent.callback :disconnect, queue, agent_call, queued_call
 
@@ -418,6 +427,13 @@ def call_agent(agent, queued_call)
     end
 
     def bridge_agent(agent, queued_call)
+      unless agent.call && agent.call.active?
+        logger.warn "Inactive agent call found for Agent #{agent.id} while bridging. Logging out agent and returning caller to queue."
+        priority_enqueue queued_call
+        remove_agent agent
+        return
+      end
+
       # Stash caller ID to make log messages work even if calls end
       queued_caller_id = remote_party queued_call
       agent.call[:queued_call] = queued_call
@@ -455,6 +471,17 @@ def bridge_agent(agent, queued_call)
       end
     end
 
+    def accept_agent!(agent)
+      case @connection_type
+      when :call
+        unless agent.callable?
+          abort ArgumentError.new('Agent has no callable address')
+        end
+      when :bridge
+        bridged_agent_health_check agent
+      end
+    end
+
     # @private
     def bridged_agent_health_check(agent)
       if agent.presence == :available && @connection_type == :bridge

lib/electric_slide/version.rb

@@ -1,4 +1,4 @@
 # encoding: utf-8
 class ElectricSlide
-  VERSION = '0.5.0'
+  VERSION = '0.5.1'
 end

spec/electric_slide/agent_spec.rb

@@ -63,4 +63,56 @@ def foo
     expect(old_presence).to eq :unavailable
     expect(extra_params[:triggered_by]).to eq 'auto'
   end
+
+  describe '#update' do
+    it 'returns the agent' do
+      expect(subject.update).to eq(subject)
+    end
+
+    context 'when given a hash with an agent attribute as key' do
+      it "sets the corresponding agent's attribute to the given value" do
+        expect {
+          subject.update(address: '[email protected]')
+        }.to change(subject, :address).from('[email protected]').to('[email protected]')
+      end
+    end
+
+    context 'when given an non-hash argument' do
+      it 'raises an error' do
+        expect {
+          subject.update(nil)
+        }.to raise_error(ArgumentError, 'Agent attributes must be a hash')
+      end
+    end
+
+    context 'when given a hash with a key that does not correspond to any agent attribute' do
+      it "raises an error" do
+        expect {
+          subject.update(blah: 1)
+        }.to raise_error(NoMethodError)
+      end
+    end
+  end
+
+  describe '#callable?' do
+    context 'when the agent has an address' do
+      before do
+        subject.address = 'Baker St.'
+      end
+
+      it 'returns `true`' do
+        expect(subject).to be_callable
+      end
+    end
+
+    context 'when the agent has no address' do
+      before do
+        subject.address = ''
+      end
+
+      it 'returns `false`' do
+        expect(subject).to_not be_callable
+      end
+    end
+  end
 end

spec/electric_slide/call_queue_spec.rb

@@ -54,6 +54,7 @@
     let(:agent_id) { '123' }
     let(:agent) { ElectricSlide::Agent.new id: agent_id, address: '123', presence: :available }
     let!(:agent_call) { Adhearsion::OutboundCall.new }
+    let!(:new_agent_call) { Adhearsion::OutboundCall.new }
     let(:queued_call) { dummy_call }
     let(:connected_time) { DateTime.now }
 
@@ -71,6 +72,7 @@
 
       before do
         allow(agent_call).to receive(:dial)
+        allow(new_agent_call).to receive(:dial)
         queue.add_agent agent
         queue.enqueue queued_call
       end
@@ -101,6 +103,16 @@
           }.to change(agent, :call).from(agent_call).to(nil)
         end
 
+        it "does not unsets the agent's `call` attribute when the agent connects to another call" do
+          queue.remove_agent double_agent # We only want one agent in the queue so they get the next call
+          allow(Adhearsion::OutboundCall).to receive(:new) { new_agent_call }
+          queue.enqueue dummy_call # We want another call to be waiting after this call
+
+          expect {
+            agent_call << Punchblock::Event::End.new(reason: :hangup)
+          }.to change(agent, :call).from(agent_call).to(new_agent_call)
+        end
+
         context "when the return strategy is :auto" do
           let(:agent_return_method) { :auto }
 
@@ -287,6 +299,54 @@
     end
   end
 
+  describe '#update_agent' do
+    let(:queue) { ElectricSlide::CallQueue.new(connection_type: :call) }
+    let(:agent) { ElectricSlide::Agent.new(id: '1', address: '[email protected]', presence: :on_call) }
+
+    before do
+      queue.add_agent(agent)
+    end
+
+    it 'updates the agent with the given attributes' do
+      expect {
+        queue.update_agent(agent, address: '[email protected]')
+      }.to change(agent, :address).from('[email protected]').to('[email protected]')
+    end
+
+    it 'returns the agent to the queue' do
+      expect(queue.wrapped_object).to receive(:return_agent).with(agent, :on_call)
+      queue.update_agent(agent, address: '[email protected]')
+    end
+
+    context 'when given a set of attributes that makes the agent unacceptable in the queue' do
+      it 'raises an error' do
+        expect {
+          queue.update_agent(agent, address: '')
+        }.to raise_error(ArgumentError, 'Agent has no callable address')
+      end
+
+      it "does not change the agent's attributes" do
+        expect { queue.update_agent(agent, address: '') }.to raise_error(ArgumentError)
+
+        expect(agent.id).to eq('1')
+        expect(agent.address).to eq('[email protected]')
+        expect(agent.presence).to eq(:on_call)
+      end
+    end
+
+    context 'when given an agent not in the queue' do
+      before do
+        queue.remove_agent agent
+      end
+
+      it 'raises an error' do
+        expect {
+          queue.update_agent(agent, address: '[email protected]')
+        }.to raise_error(ElectricSlide::CallQueue::MissingAgentError, 'Agent is not in the queue')
+      end
+    end
+  end
+
   describe '#return_agent' do
     let(:queue) { ElectricSlide::CallQueue.new }
     let(:agent) { ElectricSlide::Agent.new(id: '1', address: '[email protected]', presence: :on_call) }