From b195a4ce34e3787bca3a14aa59131b7e9586a24c Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Wed, 6 Jul 2022 11:15:22 +0200 Subject: [PATCH] Improve query to check if broker has service instances Instead of doing (more than) m times n queries (number of services * number of service plans) a single query joining the required tables is sufficient. before ------ 1* SELECT * FROM `services` WHERE (`service_broker_id` = ...) m* SELECT * FROM `service_plans` WHERE (`service_id` = ...) m*n* SELECT * FROM `service_instances` WHERE (`service_plan_id` = ...) after ----- SELECT 1 AS `one` FROM `service_instances` INNER JOIN `service_plans` ON (`service_plans`.`id` = `service_instances`.`service_plan_id`) INNER JOIN `services` ON (`services`.`id` = `service_plans`.`service_id`) WHERE (`services`.`service_broker_id` = 1) LIMIT 1 --- app/models/services/service_broker.rb | 10 ++++---- .../models/services/service_broker_spec.rb | 25 +++++++++++++++---- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/models/services/service_broker.rb b/app/models/services/service_broker.rb index 536507f4576..b8fd08314f5 100644 --- a/app/models/services/service_broker.rb +++ b/app/models/services/service_broker.rb @@ -47,11 +47,11 @@ def space_scoped? end def has_service_instances? - services.select do |service| - service.service_plans.select { |plan| - plan.service_instances.any? - }.any? - end.any? + !VCAP::CloudController::ServiceInstance. + join(:service_plans, id: :service_plan_id). + join(:services, id: :service_id). + where(services__service_broker_id: id). + empty? end def self.user_visibility_filter(user) diff --git a/spec/unit/models/services/service_broker_spec.rb b/spec/unit/models/services/service_broker_spec.rb index 0c151a4d8be..62808fbcbb6 100644 --- a/spec/unit/models/services/service_broker_spec.rb +++ b/spec/unit/models/services/service_broker_spec.rb @@ -203,13 +203,28 @@ module VCAP::CloudController let(:service) { Service.make(service_broker: service_broker) } let!(:service_plan) { ServicePlan.make(service: service) } - it 'returns true when there are service instances' do - ManagedServiceInstance.make(service_plan: service_plan) - expect(service_broker.has_service_instances?).to eq(true) + context 'when there are service instances' do + before do + ManagedServiceInstance.make(service_plan: service_plan) + end + + it 'returns true' do + expect(service_broker.has_service_instances?).to eq(true) + end + + it 'does a single db query' do + expect { service_broker.has_service_instances? }.to have_queried_db_times(/select/i, 1) + end end - it 'return false when there are no service instances' do - expect(service_broker.has_service_instances?).to eq(false) + context 'when there are no service instances' do + it 'returns false' do + expect(service_broker.has_service_instances?).to eq(false) + end + + it 'does a single db query' do + expect { service_broker.has_service_instances? }.to have_queried_db_times(/select/i, 1) + end end end end