From bff79b561f5a9b91348f1ed0b9b51cfe1bf78810 Mon Sep 17 00:00:00 2001
From: Christopher Vollick <0@psycoti.ca>
Date: Tue, 22 Mar 2022 17:16:36 -0400
Subject: [PATCH] Respond to Proper Cancel Stanza

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.
---
 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(-)
 create mode 100644 test/test_command.rb

diff --git a/lib/command.rb b/lib/command.rb
index 30b69d9411c3a0802e0fbefedc595281ba3bbe10..4292d22bf62efc879a74e1f747523866cc7d8ac4 100644
--- a/lib/command.rb
+++ b/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
diff --git a/test/test_command.rb b/test/test_command.rb
new file mode 100644
index 0000000000000000000000000000000000000000..da8809c44583ccef46ddf780d33ccc9c92f19ae3
--- /dev/null
+++ b/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 = "test@example.com"
+		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 = "test@example.com"
+		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
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 1e6bb86529df34269766adf8eee82af3a6e42975..f397d1555d0275e6d0e5671843551219f73db4ad 100644
--- a/test/test_helper.rb
+++ b/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 = "test@example.com" },
+	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
diff --git a/test/test_registration.rb b/test/test_registration.rb
index 00b75e51761fbad390c517783391c723f2df9f7f..08b18fc0f5f30f3517d6149c381ea9698b30bcd6 100644
--- a/test/test_registration.rb
+++ b/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 = "test@example.com" },
-	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(