enhance tests, use real v2 service endpoints

Author: kathap

middleware/service_broker_rate_limiter.rb

@@ -6,7 +6,7 @@ module Middleware
     RateLimitEndpoint = Struct.new(:endpoint_pattern, :request_methods)
 
     RATE_LIMITED_ENDPOINTS = [
-      RateLimitEndpoint.new(%r{\A/v2/(service_instances|service_credential_bindings|service_route_bindings)}, %w[PUT POST DELETE PATCH]),
+      RateLimitEndpoint.new(%r{\A/v2/(service_instances|service_bindings|service_keys)}, %w[PUT POST DELETE]),
       RateLimitEndpoint.new(%r{\A/v3/(service_instances|service_credential_bindings|service_route_bindings)/.+/parameters\z}, %w[GET])
     ].freeze
 

spec/unit/middleware/service_broker_rate_limiter_spec.rb

@@ -5,10 +5,12 @@ module Middleware
     RSpec.describe ServiceBrokerRateLimiter do
       let(:app) { double(:app) }
       let(:logger) { double }
-      let(:path_info) { '/v3/service_instances' }
+      let(:instance) { VCAP::CloudController::Service.make(instances_retrievable: true) }
+      let(:path_info) { '/v2/service_instances' }
       let(:user_guid) { SecureRandom.uuid }
       let(:user_env) { { 'cf.user_guid' => user_guid, 'PATH_INFO' => path_info } }
-      let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3/service_instances', method: 'POST') }
+      let(:env) { { 'cf.user_guid' => user_guid, 'PATH_INFO' => path, 'REQUEST_METHOD' => request_method } }
+      let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v2/service_instances', method: 'POST') }
       let(:max_concurrent_requests) { 1 }
       let(:broker_timeout) { 60 }
       let(:middleware) do
@@ -39,16 +41,6 @@ module Middleware
           expect(status).to eq(200)
         end
 
-        it 'does not allow more than the max number of concurrent requests' do
-          threads = 2.times.map { Thread.new { Thread.current[:status], = middleware.call(user_env) } }
-          statuses = threads.map { |t| t.join[:status] }
-
-          expect(statuses).to include(200)
-          expect(statuses).to include(429)
-          expect(app).to have_received(:call).once
-          expect(logger).to have_received(:info).with("Service broker concurrent rate limit exceeded for user '#{user_guid}'")
-        end
-
         it 'counts concurrent requests per user' do
           other_user_env = { 'cf.user_guid' => 'other_user_guid', 'PATH_INFO' => path_info }
           threads = [user_env, other_user_env].map do |env|
@@ -117,20 +109,79 @@ module Middleware
             end
           end
         end
+
+        shared_examples 'rate-limited endpoint' do
+          it 'does not allow more than the max number of concurrent requests' do
+            allow(app).to receive(:call) do
+              sleep(1)
+              [200, {}, 'a body']
+            end
+
+            threads = 2.times.map { Thread.new { middleware.call(env) } }
+            statuses = threads.map(&:join).map(&:value).map(&:first)
+
+            expect(statuses).to include(200)
+            expect(statuses).to include(429)
+            expect(app).to have_received(:call).once
+            expect(logger).to have_received(:info).with("Service broker concurrent rate limit exceeded for user '#{user_guid}'")
+          end
+        end
+
+        v2_rate_limited_service_endpoints = %w[/v2/service_instances /v2/service_bindings /v2/service_keys]
+
+        methods = %w[PUT POST DELETE]
+
+        v2_rate_limited_service_endpoints.each do |endpoint|
+          methods.each do |method|
+            context "#{endpoint} #{method} endpoint" do
+              let(:path) { endpoint }
+              let(:request_method) { method }
+              let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
+
+              include_examples 'rate-limited endpoint'
+            end
+          end
+        end
+
+        v3_rate_limited_service_endpoints = %w[/v3/service_instances/:guid/parameters /v3/service_credential_bindings/:guid/parameters /v3/service_route_bindings/:guid/parameters]
+
+        v3_rate_limited_service_endpoints.each do |endpoint|
+          context "#{endpoint} GET" do
+            let(:path) { endpoint.gsub(':guid', instance.guid.to_s) }
+            let(:request_method) { 'GET' }
+            let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: path, method: request_method) }
+
+            include_examples 'rate-limited endpoint'
+          end
+        end
       end
 
       describe 'excluded requests' do
-        let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: '/v3/service_instances', method: 'POST') }
+        shared_examples 'non-rate-limited endpoint' do
+          it 'allows concurrent requests without limits' do
+            threads = 2.times.map { Thread.new { middleware.call(env) } }
+            statuses = threads.map(&:join).map(&:value).map(&:first)
+
+            expect(statuses).to all(eq(200))
+            expect(app).to have_received(:call).twice
+            expect(logger).not_to have_received(:info)
+          end
+        end
 
-        it 'allows requests that are no longer rate limited' do
-          status, = middleware.call(user_env)
-          expect(status).to eq(200)
+        v3_not_rate_limited_service_endpoints = %w[/v3/service_instances /v3/service_credential_bindings /v3/service_route_bindings]
 
-          status, = middleware.call(user_env)
-          expect(status).to eq(200)
+        methods = %w[POST PATCH DELETE]
+
+        v3_not_rate_limited_service_endpoints.each do |endpoint|
+          methods.each do |method|
+            context "#{endpoint} #{method} - non-rate-limited" do
+              let(:path) { endpoint }
+              let(:request_method) { method }
+              let(:fake_request) { instance_double(ActionDispatch::Request, fullpath: endpoint, method: method) }
 
-          statuses = 2.times.map { middleware.call(user_env).first }
-          expect(statuses).to contain_exactly(200, 200)
+              include_examples 'non-rate-limited endpoint'
+            end
+          end
         end
       end