From 434c2eda4132c9a97ecf7ea58a24631fe72105fc Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 13 Dec 2021 15:37:46 +0100 Subject: [PATCH] Fixes #34141 - Make validators modern Ruby compatible In old Ruby versions a hash was pushed as the last argument but in modern Ruby there is first class support for this via kwargs. This worked if a single argument was provided but if they were combined it failed: validate_presence :setting, if: ->(settings) { false } This then tried to create a validator of a hash rather than as a predicate. It then also unconditionally validates :setting. --- lib/proxy/pluggable.rb | 13 +++++++------ test/plugins/module_loader_test.rb | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/proxy/pluggable.rb b/lib/proxy/pluggable.rb index 4bd6060ab..3efef4cb2 100644 --- a/lib/proxy/pluggable.rb +++ b/lib/proxy/pluggable.rb @@ -42,16 +42,17 @@ def initialize_after(*module_names) raise "#{plugin_name}: 'initialize_after' method has been removed." end - def validate_readable(*settings) - validate(*settings.push(:file_readable => true)) + def validate_readable(*settings, **validator_params) + validator_params[:file_readable] = true + validate(*settings, **validator_params) end - def validate_presence(*settings) - validate(*settings.push(:presence => true)) + def validate_presence(*settings, **validator_params) + validator_params[:presence] = true + validate(*settings, **validator_params) end - def validate(*settings) - validator_params = settings.pop + def validate(*settings, **validator_params) predicate = validator_params.delete(:if) validator_name = validator_params.keys.first validator_args = validator_params[validator_name] diff --git a/test/plugins/module_loader_test.rb b/test/plugins/module_loader_test.rb index ba83233ca..877645e83 100644 --- a/test/plugins/module_loader_test.rb +++ b/test/plugins/module_loader_test.rb @@ -70,6 +70,21 @@ def test_presence_validators_called_on_each_of_default_settings assert results.include?(:class => ::Proxy::PluginValidators::Presence, :setting => :default_2, :args => nil, :predicate => nil) end + VALIDATOR_PREDICATE = ->(settings) { false } + class TestPluginWithBuiltInValidators < ::Proxy::Plugin + default_settings :default_1 => "one", :default_2 => "two" + validate_presence :missing_setting, if: VALIDATOR_PREDICATE + validate_readable :missing_path, if: VALIDATOR_PREDICATE + end + def test_presence_validator_called_with_predicate + loader = ::Proxy::DefaultModuleLoader.new(TestPluginWithBuiltInValidators, nil) + results = loader.validate_settings(TestPluginWithBuiltInValidators, :default_1 => "one", :default_2 => "two") + assert_includes results, {:class => ::Proxy::PluginValidators::Presence, :setting => :default_1, :args => nil, :predicate => nil} + assert_includes results, {:class => ::Proxy::PluginValidators::Presence, :setting => :default_2, :args => nil, :predicate => nil} + assert_includes results, {:class => ::Proxy::PluginValidators::Presence, :setting => :missing_setting, :args => true, :predicate => VALIDATOR_PREDICATE} + assert_includes results, {:class => ::Proxy::PluginValidators::FileReadable, :setting => :missing_path, :args => true, :predicate => VALIDATOR_PREDICATE} + end + class TestValidator < ::Proxy::PluginValidators::Base def validate!(settings) true