Merge branch 'dm-connection-proxy-hash-arg' into 'master'

Fix ConnectionProxy#method_missing for trailing hash arguments

Closes #3166

See merge request !2682

lib/gitlab/database/load_balancing/connection_proxy.rb

@@ -25,34 +25,34 @@ module Gitlab
         end
 
         def select(*args)
-          read_using_load_balancer(:select, *args)
+          read_using_load_balancer(:select, args)
         end
 
         def select_all(arel, name = nil, binds = [])
           if arel.respond_to?(:locked) && arel.locked
             # SELECT ... FOR UPDATE queries should be sent to the primary.
-            write_using_load_balancer(:select_all, arel, name, binds,
+            write_using_load_balancer(:select_all, [arel, name, binds],
                                       sticky: true)
           else
-            read_using_load_balancer(:select_all, arel, name, binds)
+            read_using_load_balancer(:select_all, [arel, name, binds])
           end
         end
 
         STICKY_WRITES.each do |name|
           define_method(name) do |*args, &block|
-            write_using_load_balancer(name, *args, sticky: true, &block)
+            write_using_load_balancer(name, args, sticky: true, &block)
           end
         end
 
         # Delegates all unknown messages to a read-write connection.
         def method_missing(name, *args, &block)
-          write_using_load_balancer(name, *args, &block)
+          write_using_load_balancer(name, args, &block)
         end
 
         # Performs a read using the load balancer.
         #
         # name - The name of the method to call on a connection object.
-        def read_using_load_balancer(name, *args, &block)
+        def read_using_load_balancer(name, args, &block)
           method = Session.current.use_primary? ? :read_write : :read
 
           @load_balancer.send(method) do |connection|
@@ -65,7 +65,7 @@ module Gitlab
         # name - The name of the method to call on a connection object.
         # sticky - If set to true the session will stick to the master after
         #          the write.
-        def write_using_load_balancer(name, *args, sticky: false, &block)
+        def write_using_load_balancer(name, args, sticky: false, &block)
           result = @load_balancer.read_write do |connection|
             # Sticking has to be enabled before calling the method. Not doing so
             # could lead to methods called in a block still being performed on a

spec/lib/gitlab/database/load_balancing/connection_proxy_spec.rb

@@ -5,7 +5,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
 
   describe '#select' do
     it 'performs a read' do
-      expect(proxy).to receive(:read_using_load_balancer).with(:select, 'foo')
+      expect(proxy).to receive(:read_using_load_balancer).with(:select, ['foo'])
 
       proxy.select('foo')
     end
@@ -17,7 +17,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         arel = double(:arel)
 
         expect(proxy).to receive(:read_using_load_balancer)
-          .with(:select_all, arel, 'foo', [])
+          .with(:select_all, [arel, 'foo', []])
 
         proxy.select_all(arel, 'foo')
       end
@@ -28,7 +28,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         arel = double(:arel, locked: true)
 
         expect(proxy).to receive(:write_using_load_balancer)
-          .with(:select_all, arel, 'foo', [], sticky: true)
+          .with(:select_all, [arel, 'foo', []], sticky: true)
 
         proxy.select_all(arel, 'foo')
       end
@@ -39,7 +39,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
     describe "#{name}" do
       it 'runs the query on the primary and sticks to it' do
         expect(proxy).to receive(:write_using_load_balancer)
-          .with(name, 'foo', sticky: true)
+          .with(name, ['foo'], sticky: true)
 
         proxy.send(name, 'foo')
       end
@@ -75,10 +75,19 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
   describe '#method_missing' do
     it 'runs the query on the primary without sticking to it' do
       expect(proxy).to receive(:write_using_load_balancer)
-        .with(:foo, 'foo')
+        .with(:foo, ['foo'])
 
       proxy.foo('foo')
     end
+
+    it 'properly forwards trailing hash arguments' do
+      allow(proxy.load_balancer).to receive(:read_write)
+
+      expect(proxy).to receive(:write_using_load_balancer).and_call_original
+
+      expect { proxy.case_sensitive_comparison(:table, :attribute, :column, { value: :value, format: :format }) }
+        .not_to raise_error
+    end
   end
 
   describe '#read_using_load_balancer' do
@@ -97,7 +106,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         expect(connection).to receive(:foo).with('foo')
         expect(proxy.load_balancer).to receive(:read).and_yield(connection)
 
-        proxy.read_using_load_balancer(:foo, 'foo')
+        proxy.read_using_load_balancer(:foo, ['foo'])
       end
     end
 
@@ -110,7 +119,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
         expect(proxy.load_balancer).to receive(:read_write)
           .and_yield(connection)
 
-        proxy.read_using_load_balancer(:foo, 'foo')
+        proxy.read_using_load_balancer(:foo, ['foo'])
       end
     end
   end
@@ -129,7 +138,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
       expect(connection).to receive(:foo).with('foo')
       expect(session).not_to receive(:write!)
 
-      proxy.write_using_load_balancer(:foo, 'foo')
+      proxy.write_using_load_balancer(:foo, ['foo'])
     end
 
     it 'sticks to the primary when sticking is enabled' do
@@ -137,7 +146,7 @@ describe Gitlab::Database::LoadBalancing::ConnectionProxy do
       expect(connection).to receive(:foo).with('foo')
       expect(session).to receive(:write!)
 
-      proxy.write_using_load_balancer(:foo, 'foo', sticky: true)
+      proxy.write_using_load_balancer(:foo, ['foo'], sticky: true)
     end
   end
 end