From e075d015a05a73ded7ef641e234af8c3fc6b87f5 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Wed, 6 Jul 2022 16:59:16 +0200 Subject: [PATCH] Use Sequel's any_not_empty extension There are already several PRs (see below [1]) that change dataset.any? to !dataset.empty? or otherwise mention the difference between these two statements. Whereas the first one fetches all data from the database to then check if the resulting array contains at least one element, the second statement does a 'select 1 as one from table limit 1' query that has a far better performance. Fortunately there is a Sequel extension (see [2]) that 'fixes' this misleading behavior; so instead of searching for more places where we could change the implementation, using dataset.any? should now be equivalent to !dataset.empty? and everyone can choose freely which one to use. [1] https://github.com/cloudfoundry/cloud_controller_ng/pull/2117 https://github.com/cloudfoundry/cloud_controller_ng/pull/2533 https://github.com/cloudfoundry/cloud_controller_ng/pull/2803 https://github.com/cloudfoundry/cloud_controller_ng/pull/2855 [2] https://sequel.jeremyevans.net/rdoc-plugins/files/lib/sequel/extensions/any_not_empty_rb.html --- app/models/services/service_broker.rb | 4 ++-- lib/cloud_controller/db.rb | 1 + spec/unit/models/services/service_broker_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/services/service_broker.rb b/app/models/services/service_broker.rb index b8fd08314f5..fb471b81d1a 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? - !VCAP::CloudController::ServiceInstance. + VCAP::CloudController::ServiceInstance. join(:service_plans, id: :service_plan_id). join(:services, id: :service_id). where(services__service_broker_id: id). - empty? + any? end def self.user_visibility_filter(user) diff --git a/lib/cloud_controller/db.rb b/lib/cloud_controller/db.rb index f70c8f0776b..4be0141e8ae 100644 --- a/lib/cloud_controller/db.rb +++ b/lib/cloud_controller/db.rb @@ -94,6 +94,7 @@ def self.load_models_without_migrations_check(db_config, logger) Sequel::Model.plugin :validation_helpers Sequel::Database.extension(:current_datetime_timestamp) +Sequel::Database.extension(:any_not_empty) require 'cloud_controller/encryptor' Sequel::Model.include VCAP::CloudController::Encryptor::FieldEncryptor diff --git a/spec/unit/models/services/service_broker_spec.rb b/spec/unit/models/services/service_broker_spec.rb index 62808fbcbb6..59eba94ba4f 100644 --- a/spec/unit/models/services/service_broker_spec.rb +++ b/spec/unit/models/services/service_broker_spec.rb @@ -213,7 +213,7 @@ module VCAP::CloudController end it 'does a single db query' do - expect { service_broker.has_service_instances? }.to have_queried_db_times(/select/i, 1) + expect { service_broker.has_service_instances? }.to have_queried_db_times(/select 1.*limit 1/i, 1) end end @@ -223,7 +223,7 @@ module VCAP::CloudController end it 'does a single db query' do - expect { service_broker.has_service_instances? }.to have_queried_db_times(/select/i, 1) + expect { service_broker.has_service_instances? }.to have_queried_db_times(/select 1.*limit 1/i, 1) end end end