refactor: tweak transaction method and test (#279)

Author: supercaracal

lib/redis_client/cluster.rb

@@ -90,10 +90,7 @@ def pipelined
     end
 
     def multi(watch: nil, &block)
-      ::RedisClient::Cluster::Transaction
-        .new(@router, @command_builder)
-        .find_node(&block)
-        .multi(watch: watch, &block)
+      ::RedisClient::Cluster::Transaction.new(@router, @command_builder).execute(watch: watch, &block)
     end
 
     def pubsub

lib/redis_client/cluster/transaction.rb

@@ -33,11 +33,11 @@ def call_once_v(command, &_)
         ensure_node_key(command)
       end
 
-      def find_node
+      def execute(watch: nil, &block)
         yield self
         raise ArgumentError, 'empty transaction' if @node_key.nil?
 
-        @router.find_node(@node_key)
+        @router.find_node(@node_key).multi(watch: watch, &block)
       end
 
       private

test/redis_client/test_cluster.rb

@@ -188,15 +188,14 @@ def test_pubsub_without_subscription
       end
 
       def test_transaction_with_single_key
-        want = ['OK', 1, 2, '2']
         got = @client.multi do |t|
           t.call('SET', 'counter', '0')
           t.call('INCR', 'counter')
           t.call('INCR', 'counter')
-          t.call('GET', 'counter')
         end
 
-        assert_equal(want, got)
+        assert_equal(['OK', 1, 2], got)
+        assert_equal('2', @client.call('GET', 'counter'))
       end
 
       def test_transaction_with_multiple_key
@@ -215,25 +214,41 @@ def test_transaction_with_multiple_key
 
       def test_transaction_with_empty_block
         assert_raises(ArgumentError) { @client.multi {} }
+        assert_raises(LocalJumpError) { @client.multi }
+      end
+
+      def test_transaction_with_keyless_commands
+        assert_raises(::RedisClient::Cluster::Transaction::ConsistencyError) do
+          @client.multi do |t|
+            t.call('ECHO', 'foo')
+            t.call('ECHO', 'bar')
+          end
+        end
       end
 
       def test_transaction_with_hashtag
-        want = ['OK', 'OK', %w[1 2 3 4]]
         got = @client.multi do |t|
           t.call('MSET', '{key}1', '1', '{key}2', '2')
           t.call('MSET', '{key}3', '3', '{key}4', '4')
-          t.call('MGET', '{key}1', '{key}2', '{key}3', '{key}4')
         end
 
-        assert_equal(want, got)
+        assert_equal(%w[OK OK], got)
+        assert_equal(%w[1 2 3 4], @client.call('MGET', '{key}1', '{key}2', '{key}3', '{key}4'))
       end
 
       def test_transaction_without_hashtag
         assert_raises(::RedisClient::Cluster::Transaction::ConsistencyError) do
           @client.multi do |t|
             t.call('MSET', 'key1', '1', 'key2', '2')
             t.call('MSET', 'key3', '3', 'key4', '4')
-            t.call('MGET', 'key1', 'key2', 'key3', 'key4')
+          end
+        end
+
+        assert_raises(::RedisClient::CommandError, 'CROSSSLOT keys') do
+          @client.multi do |t|
+            t.call('MSET', 'key1', '1', 'key2', '2')
+            t.call('MSET', 'key1', '1', 'key3', '3')
+            t.call('MSET', 'key1', '1', 'key4', '4')
           end
         end