Respond to Proper Cancel Stanza

Christopher Vollick created 3 years ago

Previously cancels would be treated as an error, but then would be
caught so that they could respond with receipt of the cancel later.

But, we weren't setting @iq in these cases, because it was an error, so
we would actually respond to the wrong stanza; specifically the one
before the cancel.

This was bad and wrong and led to the bot sitting there waiting for the
cancel before moving on with its life.

Change summary

lib/command.rb            |  7 +++--
test/test_command.rb      | 54 +++++++++++++++++++++++++++++++++++++++++
test/test_helper.rb       | 13 +++++++++
test/test_registration.rb | 13 ---------
4 files changed, 71 insertions(+), 16 deletions(-)

Detailed changes

lib/command.rb

@@ -58,12 +58,13 @@ class Command
 		end
 
 		def reply(stanza=nil)
-			stanza ||= iq.reply.tap { |reply|
-				reply.status = :executing
-			}
+			stanza ||= iq.reply.tap { |reply| reply.status = :executing }
 			yield stanza if block_given?
 			COMMAND_MANAGER.write(stanza).then { |new_iq|
 				@iq = new_iq
+			}.catch_only(Blather::Stanza::Iq) { |new_iq|
+				@iq = new_iq if new_iq.set?
+				EMPromise.reject(new_iq)
 			}.catch_only(SessionManager::Timeout) do
 				EMPromise.reject(Timeout.new)
 			end

test/test_command.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "command"
+
+Command::COMMAND_MANAGER = Minitest::Mock.new
+
+class CommandTest < Minitest::Test
+	def test_cancel_finishes
+		blather = Minitest::Mock.new
+		iq1 = Blather::Stanza::Iq::Command.new
+		iq1.from = "[e-mail]"
+		iq1.id = "test-id"
+		iq1.sessionid = "session-id"
+
+		# The library digests the sessionID I give it...
+		session = iq1.sessionid
+
+		iq2 = Blather::Stanza::Iq::Command.new
+		iq2.from = "[e-mail]"
+		iq2.id = "cancel-id"
+		iq2.sessionid = "session-id"
+		iq2.action = :cancel
+
+		Command::COMMAND_MANAGER.expect(
+			:write,
+			EMPromise.reject(iq2),
+			[Matching.new do |iq|
+				assert_equal "test-id", iq.id
+				assert_equal session, iq.sessionid
+			end]
+		)
+
+		blather.expect(
+			:<<,
+			nil,
+			[Matching.new do |iq|
+				assert "canceled", iq.status
+				assert_equal "cancel-id", iq.id
+				assert_equal session, iq.sessionid
+			end]
+		)
+
+		execute_command(iq1, blather: blather) { |cmd|
+			cmd.reply.then do
+				raise "Should have cancelled"
+			end
+		}.then do
+			assert_mock blather
+			assert_mock Command::COMMAND_MANAGER
+		end
+	end
+	em :test_cancel_finishes
+end

test/test_helper.rb

@@ -118,6 +118,19 @@ BLATHER = Class.new {
 	def <<(*); end
 }.new.freeze
 
+def execute_command(
+	iq=Blather::Stanza::Iq::Command.new.tap { |i| i.from = "[e-mail]" },
+	blather: BLATHER,
+	&blk
+)
+	Command::Execution.new(
+		Minitest::Mock.new,
+		blather,
+		:to_s.to_proc,
+		iq
+	).execute(&blk).sync
+end
+
 class Matching
 	def initialize(&block)
 		@block = block

test/test_registration.rb

@@ -4,19 +4,6 @@ require "test_helper"
 require "customer"
 require "registration"
 
-def execute_command(
-	iq=Blather::Stanza::Iq::Command.new.tap { |i| i.from = "[e-mail]" },
-	blather: BLATHER,
-	&blk
-)
-	Command::Execution.new(
-		Minitest::Mock.new,
-		blather,
-		:to_s.to_proc,
-		iq
-	).execute(&blk).sync
-end
-
 class RegistrationTest < Minitest::Test
 	def test_for_registered
 		sgx = OpenStruct.new(