From 6c77d09b44456bc1770f0264f41c1238557c64fa Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Fri, 15 Mar 2024 14:41:51 +0100 Subject: [PATCH] Fixes #37273 - Use delegation syntax for resource_scope In Ruby 3 you need to explicitly delegate keyword arguments, or if you don't care about Ruby 2.6 or prior then you can use the new delegation syntax[1]. It deals with the difference in calling where. When calling `.where([])` it returns an array while `.where(*[])` returns an instance of `WhereChain` which consumers of the method can't deal with. Because of that it calls `.all`. [1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ --- .rubocop.yml | 3 +++ app/controllers/api/base_controller.rb | 8 ++++---- .../api/v2/auth_sources_controller.rb | 2 +- .../api/v2/common_parameters_controller.rb | 2 +- .../api/v2/config_reports_controller.rb | 4 ++-- app/controllers/api/v2/filters_controller.rb | 2 +- .../api/v2/host_statuses_controller.rb | 2 +- .../api/v2/instance_hosts_controller.rb | 2 +- .../api/v2/personal_access_tokens_controller.rb | 2 +- app/controllers/api/v2/settings_controller.rb | 2 +- .../api/v2/smart_proxy_hosts_controller.rb | 2 +- .../concerns/api/v2/taxonomies_controller.rb | 4 ++-- app/controllers/concerns/find_common.rb | 17 +++++++++++------ .../foreman/controller/bookmark_common.rb | 2 +- .../foreman/controller/taxonomies_controller.rb | 2 +- .../concerns/foreman/controller/user_aware.rb | 2 +- .../concerns/foreman/controller/users_mixin.rb | 4 ++-- .../application_controller_subclass_test.rb | 2 +- 18 files changed, 36 insertions(+), 28 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index c24cd85b240..4fd273a6ad0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,6 +5,9 @@ inherit_gem: inherit_from: .rubocop_todo.yml +AllCops: + TargetRubyVersion: 2.7 + Bundler/OrderedGems: Enabled: false diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index a3f93c5a265..ea2f064370a 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -70,8 +70,8 @@ def controller_permission end # overwrites resource_scope in FindCommon to consider nested objects - def resource_scope(options = {}) - super(options).merge(parent_scope).readonly(false) + def resource_scope(...) + super(...).merge(parent_scope).readonly(false) end def parent_scope @@ -110,8 +110,8 @@ def resource_class_join(association, scope) resource_class.joins(association.name).merge(scope) end - def resource_scope_for_index(options = {}) - scope = resource_scope(options).search_for(*search_options) + def resource_scope_for_index(...) + scope = resource_scope(...).search_for(*search_options) return scope if paginate_options[:per_page] == 'all' scope.paginate(**paginate_options) end diff --git a/app/controllers/api/v2/auth_sources_controller.rb b/app/controllers/api/v2/auth_sources_controller.rb index 05000a23121..de7f9de17de 100644 --- a/app/controllers/api/v2/auth_sources_controller.rb +++ b/app/controllers/api/v2/auth_sources_controller.rb @@ -14,7 +14,7 @@ def index @auth_sources = resource_scope_for_index.except_hidden end - def resource_scope(*args) + def resource_scope(...) super.except_hidden end diff --git a/app/controllers/api/v2/common_parameters_controller.rb b/app/controllers/api/v2/common_parameters_controller.rb index 4bdc3dbedb8..261b158f2d3 100644 --- a/app/controllers/api/v2/common_parameters_controller.rb +++ b/app/controllers/api/v2/common_parameters_controller.rb @@ -59,7 +59,7 @@ def controller_permission 'params' end - def resource_scope(*args, &block) + def resource_scope(...) super.where(:type => 'CommonParameter') end diff --git a/app/controllers/api/v2/config_reports_controller.rb b/app/controllers/api/v2/config_reports_controller.rb index 35a6d069dc8..5f219c534d6 100644 --- a/app/controllers/api/v2/config_reports_controller.rb +++ b/app/controllers/api/v2/config_reports_controller.rb @@ -72,9 +72,9 @@ def setup_search_options params[:search] += " host = " + params[:host_id] if params[:host_id] end - def resource_scope(options = {}) + def resource_scope(*args, **options) options[:permission] = :view_config_reports - super(options).my_reports + super(*args, **options).my_reports end def action_permission diff --git a/app/controllers/api/v2/filters_controller.rb b/app/controllers/api/v2/filters_controller.rb index 03662b59202..0dc73113c24 100644 --- a/app/controllers/api/v2/filters_controller.rb +++ b/app/controllers/api/v2/filters_controller.rb @@ -55,7 +55,7 @@ def destroy process_response @filter.destroy end - def resource_scope(*args) + def resource_scope(...) resource_class.unscoped end diff --git a/app/controllers/api/v2/host_statuses_controller.rb b/app/controllers/api/v2/host_statuses_controller.rb index e2d53806364..34ff10c896f 100644 --- a/app/controllers/api/v2/host_statuses_controller.rb +++ b/app/controllers/api/v2/host_statuses_controller.rb @@ -12,7 +12,7 @@ def index private - def resource_scope(*args, &block) + def resource_scope(...) HostStatusPresenter.all end diff --git a/app/controllers/api/v2/instance_hosts_controller.rb b/app/controllers/api/v2/instance_hosts_controller.rb index e3639ff4d15..4c7b5192e37 100644 --- a/app/controllers/api/v2/instance_hosts_controller.rb +++ b/app/controllers/api/v2/instance_hosts_controller.rb @@ -7,7 +7,7 @@ def resource_class @resource_class ||= Host::Managed end - def resource_scope(*args) + def resource_scope(...) super.authorized(:view_hosts).joins(:infrastructure_facet).merge(::HostFacets::InfrastructureFacet.where(foreman_instance: true)) end diff --git a/app/controllers/api/v2/personal_access_tokens_controller.rb b/app/controllers/api/v2/personal_access_tokens_controller.rb index 89c420dcc88..c78b564ae39 100644 --- a/app/controllers/api/v2/personal_access_tokens_controller.rb +++ b/app/controllers/api/v2/personal_access_tokens_controller.rb @@ -67,7 +67,7 @@ def parent_permission(child_perm) end end - def resource_scope(*args) + def resource_scope(...) if editing_self? resource_class.where(user: @user).readonly(false) else diff --git a/app/controllers/api/v2/settings_controller.rb b/app/controllers/api/v2/settings_controller.rb index 1452c61a2a5..f1ce5fd22b7 100644 --- a/app/controllers/api/v2/settings_controller.rb +++ b/app/controllers/api/v2/settings_controller.rb @@ -57,7 +57,7 @@ def update render_error :custom_error, :locals => { :message => e.bare_message }, :status => :unprocessable_entity end - def resource_scope(_options = {}) + def resource_scope(...) Foreman.settings end end diff --git a/app/controllers/api/v2/smart_proxy_hosts_controller.rb b/app/controllers/api/v2/smart_proxy_hosts_controller.rb index 689601d8e85..0e4f123a5d0 100644 --- a/app/controllers/api/v2/smart_proxy_hosts_controller.rb +++ b/app/controllers/api/v2/smart_proxy_hosts_controller.rb @@ -8,7 +8,7 @@ def resource_class @resource_class ||= Host::Managed end - def resource_scope(*args) + def resource_scope(...) resource_class.authorized(:view_hosts).joins(:infrastructure_facet).merge(::HostFacets::InfrastructureFacet.where(smart_proxy_id: @proxy.id)) end diff --git a/app/controllers/concerns/api/v2/taxonomies_controller.rb b/app/controllers/concerns/api/v2/taxonomies_controller.rb index 39c960ee148..1e94c457984 100644 --- a/app/controllers/concerns/api/v2/taxonomies_controller.rb +++ b/app/controllers/concerns/api/v2/taxonomies_controller.rb @@ -93,8 +93,8 @@ def destroy end # overriding public FindCommon#resource_scope to scope only to user's taxonomies - def resource_scope(*args) - @resource_scope ||= scope_for(resource_class, args).send("my_#{taxonomies_plural}") + def resource_scope(*args, **kwargs) + @resource_scope ||= scope_for(resource_class, *args, **kwargs).send("my_#{taxonomies_plural}") end private diff --git a/app/controllers/concerns/find_common.rb b/app/controllers/concerns/find_common.rb index 9c57fba973f..236ab66b369 100644 --- a/app/controllers/concerns/find_common.rb +++ b/app/controllers/concerns/find_common.rb @@ -32,22 +32,27 @@ def resource_class @resource_class end - def resource_scope(options = {}) - @resource_scope ||= scope_for(resource_class, options) + def resource_scope(*args, **kwargs) + @resource_scope ||= scope_for(resource_class, *args, **kwargs) end - def scope_for(resource, options = {}) - controller = options.delete(:controller) { controller_permission } + def scope_for(resource, *args, **kwargs) + controller = kwargs.delete(:controller) { controller_permission } # don't call the #action_permission method here, we are not sure if the resource is authorized at this point # calling #action_permission here can cause an exception, in order to avoid this, ensure :authorized beforehand - permission = options.delete(:permission) + permission = kwargs.delete(:permission) if resource.respond_to?(:authorized) permission ||= "#{action_permission}_#{controller}" resource = resource.authorized(permission, resource) end - resource.where(options) + # Callers rely on a plain array + if args.empty? && kwargs.empty? + resource.all + else + resource.where(*args, **kwargs) + end end def resource_class_for(resource) diff --git a/app/controllers/concerns/foreman/controller/bookmark_common.rb b/app/controllers/concerns/foreman/controller/bookmark_common.rb index 4f7d448bb37..358f9d3fcd5 100644 --- a/app/controllers/concerns/foreman/controller/bookmark_common.rb +++ b/app/controllers/concerns/foreman/controller/bookmark_common.rb @@ -3,7 +3,7 @@ def resource_base Bookmark.my_bookmarks end - def resource_scope(*args) + def resource_scope(...) Bookmark.my_bookmarks end end diff --git a/app/controllers/concerns/foreman/controller/taxonomies_controller.rb b/app/controllers/concerns/foreman/controller/taxonomies_controller.rb index bb8a22578a2..edafddc60aa 100644 --- a/app/controllers/concerns/foreman/controller/taxonomies_controller.rb +++ b/app/controllers/concerns/foreman/controller/taxonomies_controller.rb @@ -194,7 +194,7 @@ def find_resource end end - def resource_scope + def resource_scope(...) taxonomy_class.send("my_#{taxonomies_plural}") end diff --git a/app/controllers/concerns/foreman/controller/user_aware.rb b/app/controllers/concerns/foreman/controller/user_aware.rb index 3781f7802c8..cce5fa7392d 100644 --- a/app/controllers/concerns/foreman/controller/user_aware.rb +++ b/app/controllers/concerns/foreman/controller/user_aware.rb @@ -8,7 +8,7 @@ module Foreman::Controller::UserAware private - def resource_scope(*args, &block) + def resource_scope(...) super.where(:user => @user) end diff --git a/app/controllers/concerns/foreman/controller/users_mixin.rb b/app/controllers/concerns/foreman/controller/users_mixin.rb index 7bde3268cd9..0045acad048 100644 --- a/app/controllers/concerns/foreman/controller/users_mixin.rb +++ b/app/controllers/concerns/foreman/controller/users_mixin.rb @@ -6,8 +6,8 @@ module Foreman::Controller::UsersMixin before_action :clear_session_locale_on_update, :only => :update end - def resource_scope(options = {}) - super(options).except_hidden + def resource_scope(...) + super.except_hidden end protected diff --git a/test/controllers/application_controller_subclass_test.rb b/test/controllers/application_controller_subclass_test.rb index 3401f8f264d..b4c74366098 100644 --- a/test/controllers/application_controller_subclass_test.rb +++ b/test/controllers/application_controller_subclass_test.rb @@ -212,7 +212,7 @@ def @sso.authenticated? mock_scope = mock('mock_scope') auth_scope = mock('auth_scope') - auth_scope.stubs(:where).returns(mock_scope) + auth_scope.stubs(:all).returns(mock_scope) resource_class = mock('authorized_resource') resource_class.stubs(:authorized).returns(auth_scope)