Skip to content

Commit

Permalink
Use Sequel's any_not_empty extension
Browse files Browse the repository at this point in the history
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]
cloudfoundry#2117
cloudfoundry#2533
cloudfoundry#2803
cloudfoundry#2855

[2] https://sequel.jeremyevans.net/rdoc-plugins/files/lib/sequel/extensions/any_not_empty_rb.html
  • Loading branch information
philippthun authored and will-gant committed Dec 16, 2022
1 parent ea3671d commit a510deb
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 4 deletions.
4 changes: 2 additions & 2 deletions app/models/services/service_broker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions lib/cloud_controller/db.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/models/services/service_broker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit a510deb

Please sign in to comment.