From 9a93acb589d794ecc3a52b515018c91409b298ed Mon Sep 17 00:00:00 2001 From: "Brian D. Burns" Date: Sun, 18 Mar 2012 07:04:46 -0400 Subject: [PATCH] update attr_deprecate to accept a value parameter allow setting a specific value on the replacement attribute, or using a lambda to determine the replacement value to be set based on the value originally set by the user on the deprecated attribute. --- lib/backup/configuration/helpers.rb | 54 ++++++- spec/configuration/helpers_spec.rb | 218 +++++++++++++++++++++++----- spec/database/mongodb_spec.rb | 15 +- spec/database/mysql_spec.rb | 13 +- spec/database/postgresql_spec.rb | 13 +- spec/database/redis_spec.rb | 13 +- spec/database/riak_spec.rb | 13 +- 7 files changed, 265 insertions(+), 74 deletions(-) diff --git a/lib/backup/configuration/helpers.rb b/lib/backup/configuration/helpers.rb index 34c6626f0..88da108a6 100644 --- a/lib/backup/configuration/helpers.rb +++ b/lib/backup/configuration/helpers.rb @@ -48,10 +48,28 @@ def log_deprecation_warning(name, deprecation) protected + ## + # Method to deprecate an attribute. + # + # :version should be set to the backup version which will first + # introduce the deprecation. + # :replacement may be set to another attr_accessor name to set + # the value for instead of the deprecated accessor + # :value may be used to specify the value set on :replacement. + # If :value is nil, the value set on the deprecated accessor + # will be used to set the value for the :replacement. + # If :value is a lambda, it will be passed the value the user + # set on the deprecated accessor, and should return the value + # to be set on the :replacement. + # Therefore, to cause the replacement accessor not to be set, + # use the lambda form to return nil. This is only way to specify + # a :replacement without transferring a value. + # e.g. :replacement => :new_attr, :value => Proc.new {} def attr_deprecate(name, args = {}) deprecations[name] = { :version => nil, - :replacement => nil + :replacement => nil, + :value => nil }.merge(args) end @@ -70,16 +88,46 @@ def load_defaults! end end + ## + # Check missing methods for deprecations + # + # Note that OpenStruct (used for setting defaults) does not allow + # multiple arguments when assigning values for members. + # So, we won't allow it here either, even though an attr_accessor + # will accept and convert them into an Array. Therefore, setting + # an option value using multiple values, whether as a default or + # directly on the class' accessor, should not be supported. + # i.e. if an option will accept being set as an Array, then it + # should be explicitly set as such. e.g. option = [val1, val2] def method_missing(name, *args) deprecation = nil if method = name.to_s.chomp!('=') + if (len = args.count) != 1 + raise ArgumentError, + "wrong number of arguments (#{ len } for 1)", caller(1) + end deprecation = self.class.deprecations[method.to_sym] end if deprecation self.class.log_deprecation_warning(method, deprecation) - replacement = deprecation[:replacement] - send(:"#{ replacement }=", *args) if replacement + if replacement = deprecation[:replacement] + value = + case deprecation[:value] + when nil + args[0] + when Proc + deprecation[:value].call(args[0]) + else + deprecation[:value] + end + unless value.nil? + Logger.warn( + "#{ self.class }.#{ replacement } is being set to '#{ value }'" + ) + send(:"#{ replacement }=", value) + end + end else super end diff --git a/spec/configuration/helpers_spec.rb b/spec/configuration/helpers_spec.rb index 3e450d196..a4d04310e 100644 --- a/spec/configuration/helpers_spec.rb +++ b/spec/configuration/helpers_spec.rb @@ -10,9 +10,23 @@ class Foo include Backup::Configuration::Helpers attr_accessor :accessor attr_reader :reader - attr_deprecate :removed_accessor, :version => '1.1' - attr_deprecate :replaced_accessor, :version => '1.2', + attr_deprecate :removed, + :version => '1.1' + attr_deprecate :replaced, + :version => '1.2', :replacement => :accessor + attr_deprecate :replaced_with_value, + :version => '1.3', + :replacement => :accessor, + :value => 'new_value' + attr_deprecate :replaced_with_lambda, + :version => '1.4', + :replacement => :accessor, + :value => lambda {|val| val ? '1' : '0' } + attr_deprecate :replaced_with_lambda_nil, + :version => '1.4', + :replacement => :accessor, + :value => Proc.new {} end end end @@ -60,10 +74,8 @@ class Foo describe '.deprecations' do it 'should return @deprecations' do - Backup::Foo.deprecations.should == { - :removed_accessor => { :version => '1.1', :replacement => nil }, - :replaced_accessor => { :version => '1.2', :replacement => :accessor } - } + Backup::Foo.deprecations.should be_a(Hash) + Backup::Foo.deprecations.keys.count.should be(5) end it 'should set @deprecations to an empty hash if not set' do @@ -80,15 +92,30 @@ class Foo it 'should add deprected attributes' do Backup::Foo.send(:attr_deprecate, :attr1) Backup::Foo.send(:attr_deprecate, :attr2, :version => '1.3') - Backup::Foo.send(:attr_deprecate, :attr3, :replacement => :foo) + Backup::Foo.send(:attr_deprecate, :attr3, :replacement => :new_attr3) Backup::Foo.send(:attr_deprecate, :attr4, - :version => '1.4', :replacement => :foobar) + :version => '1.4', :replacement => :new_attr4) + Backup::Foo.send(:attr_deprecate, :attr5, + :version => '1.5', + :replacement => :new_attr5, + :value => 'new_value') Backup::Foo.deprecations.should == { - :attr1 => { :version => nil, :replacement => nil }, - :attr2 => { :version => '1.3', :replacement => nil }, - :attr3 => { :version => nil, :replacement => :foo }, - :attr4 => { :version => '1.4', :replacement => :foobar } + :attr1 => { :version => nil, + :replacement => nil, + :value => nil }, + :attr2 => { :version => '1.3', + :replacement => nil, + :value => nil }, + :attr3 => { :version => nil, + :replacement => :new_attr3, + :value => nil }, + :attr4 => { :version => '1.4', + :replacement => :new_attr4, + :value => nil }, + :attr5 => { :version => '1.5', + :replacement => :new_attr5, + :value => 'new_value' } } end end @@ -99,11 +126,11 @@ class Foo Backup::Logger.expects(:warn).with do |err| err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Foo.removed_accessor has been deprecated as of backup v.1.1" + " Backup::Foo.removed has been deprecated as of backup v.1.1" end - deprecation = Backup::Foo.deprecations[:removed_accessor] - Backup::Foo.log_deprecation_warning(:removed_accessor, deprecation) + deprecation = Backup::Foo.deprecations[:removed] + Backup::Foo.log_deprecation_warning(:removed, deprecation) end end @@ -112,13 +139,13 @@ class Foo Backup::Logger.expects(:warn).with do |err| err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Foo.replaced_accessor has been deprecated as of backup v.1.2\n" + + " Backup::Foo.replaced has been deprecated as of backup v.1.2\n" + " This setting has been replaced with:\n" + " Backup::Foo.accessor" end - deprecation = Backup::Foo.deprecations[:replaced_accessor] - Backup::Foo.log_deprecation_warning(:replaced_accessor, deprecation) + deprecation = Backup::Foo.deprecations[:replaced] + Backup::Foo.log_deprecation_warning(:replaced, deprecation) end end end @@ -161,26 +188,92 @@ class Foo context 'when the missing method is an attribute set operator' do context 'and the method has been deprecated' do context 'and the deprecated method has a replacement' do - it 'should set the value on the replacement and log a warning' do - Backup::Foo.defaults do |f| - f.replaced_accessor = 'attr_value' + context 'and a replacement value is specified' do + it 'should set the the replacement value on the replacement' do + Backup::Foo.defaults do |f| + f.replaced_with_value = 'attr_value' + end + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to 'new_value'" + ) + + klass = Backup::Foo.new + klass.send(:load_defaults!) + klass.accessor.should == 'new_value' end + end - Backup::Logger.expects(:warn) + context 'and the replacement value is a lambda' do + it 'should set replacement value using the lambda' do + value = [true, false].shuffle.first + new_value = value ? '1' : '0' + + Backup::Foo.defaults do |f| + f.replaced_with_lambda = value + end + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to '#{ new_value }'" + ) + + klass = Backup::Foo.new + klass.send(:load_defaults!) + klass.accessor.should == new_value + end - klass = Backup::Foo.new - klass.send(:load_defaults!) - klass.accessor.should == 'attr_value' + it 'should not set the replacement if the lambda returns nil' do + Backup::Foo.defaults do |f| + f.replaced_with_lambda_nil = 'foo' + end + + Backup::Foo.any_instance.expects(:accessor=).never + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + + klass = Backup::Foo.new + klass.send(:load_defaults!) + klass.accessor.should be_nil + end + end + + context 'and no replacement value is specified' do + it 'should set the original value on the replacement' do + Backup::Foo.defaults do |f| + f.replaced = 'attr_value' + end + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to 'attr_value'" + ) + + klass = Backup::Foo.new + klass.send(:load_defaults!) + klass.accessor.should == 'attr_value' + end end end context 'and the deprecated method has no replacement' do it 'should only log a warning' do Backup::Foo.defaults do |f| - f.removed_accessor = 'attr_value' + f.removed = 'attr_value' end - Backup::Logger.expects(:warn) + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) klass = Backup::Foo.new klass.send(:load_defaults!) @@ -210,21 +303,76 @@ class Foo context 'when the missing method is an attribute set operator' do context 'and the method has been deprecated' do context 'and the deprecated method has a replacement' do - it 'should set the value on the replacement and log a warning' do - Backup::Logger.expects(:warn) + context 'and a replacement value is specified' do + it 'should set the the replacement value on the replacement' do + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to 'new_value'" + ) + + klass = Backup::Foo.new + klass.replaced_with_value = 'attr_value' + klass.accessor.should == 'new_value' + end + end - klass = Backup::Foo.new - klass.replaced_accessor = 'attr_value' - klass.accessor.should == 'attr_value' + context 'and the replacement value is a lambda' do + it 'should set replacement value using the lambda' do + value = [true, false].shuffle.first + new_value = value ? '1' : '0' + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to '#{ new_value }'" + ) + + klass = Backup::Foo.new + klass.replaced_with_lambda = value + klass.accessor.should == new_value + end + + it 'should not set the replacement if the lambda returns nil' do + Backup::Foo.any_instance.expects(:accessor=).never + + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + + klass = Backup::Foo.new + klass.replaced_with_lambda_nil = 'foo' + klass.accessor.should be_nil + end + end + + context 'and no replacement value is specified' do + it 'should set the original value on the replacement' do + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Foo.accessor is being set to 'attr_value'" + ) + + klass = Backup::Foo.new + klass.replaced = 'attr_value' + klass.accessor.should == 'attr_value' + end end end + context 'and the deprecated method has no replacement' do it 'should only log a warning' do - Backup::Logger.expects(:warn) + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) klass = Backup::Foo.new - klass.removed_accessor = 'attr_value' + klass.removed = 'attr_value' klass.accessor.should be_nil end end @@ -248,7 +396,7 @@ class Foo klass = Backup::Foo.new expect do - klass.removed_accessor + klass.removed end.to raise_error(NoMethodError) end end diff --git a/spec/database/mongodb_spec.rb b/spec/database/mongodb_spec.rb index 9ec52f902..e11afbbdc 100644 --- a/spec/database/mongodb_spec.rb +++ b/spec/database/mongodb_spec.rb @@ -479,14 +479,13 @@ describe '#utility_path' do before do - Backup::Database::MongoDB.any_instance.stubs(:utility).returns('blah') - Backup::Logger.expects(:warn).with do |err| - err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Database::MongoDB.utility_path has been deprecated " + - "as of backup v.3.0.21\n" + - " This setting has been replaced with:\n" + - " Backup::Database::MongoDB.mongodump_utility" - end + Backup::Database::MongoDB.any_instance.stubs(:utility) + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Database::MongoDB.mongodump_utility is being set to 'foo'" + ) end context 'when set directly' do diff --git a/spec/database/mysql_spec.rb b/spec/database/mysql_spec.rb index 6bf2ebd05..28686f3fa 100644 --- a/spec/database/mysql_spec.rb +++ b/spec/database/mysql_spec.rb @@ -381,13 +381,12 @@ describe '#utility_path' do before do Backup::Database::MySQL.any_instance.stubs(:utility) - Backup::Logger.expects(:warn).with do |err| - err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Database::MySQL.utility_path has been deprecated " + - "as of backup v.3.0.21\n" + - " This setting has been replaced with:\n" + - " Backup::Database::MySQL.mysqldump_utility" - end + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Database::MySQL.mysqldump_utility is being set to 'foo'" + ) end context 'when set directly' do diff --git a/spec/database/postgresql_spec.rb b/spec/database/postgresql_spec.rb index f25e011e2..6b8ca96e8 100644 --- a/spec/database/postgresql_spec.rb +++ b/spec/database/postgresql_spec.rb @@ -323,13 +323,12 @@ describe '#utility_path' do before do Backup::Database::PostgreSQL.any_instance.stubs(:utility) - Backup::Logger.expects(:warn).with do |err| - err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Database::PostgreSQL.utility_path has been deprecated " + - "as of backup v.3.0.21\n" + - " This setting has been replaced with:\n" + - " Backup::Database::PostgreSQL.pg_dump_utility" - end + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Database::PostgreSQL.pg_dump_utility is being set to 'foo'" + ) end context 'when set directly' do diff --git a/spec/database/redis_spec.rb b/spec/database/redis_spec.rb index c834f2b7c..ef575c425 100644 --- a/spec/database/redis_spec.rb +++ b/spec/database/redis_spec.rb @@ -304,13 +304,12 @@ describe '#utility_path' do before do Backup::Database::Redis.any_instance.stubs(:utility) - Backup::Logger.expects(:warn).with do |err| - err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Database::Redis.utility_path has been deprecated " + - "as of backup v.3.0.21\n" + - " This setting has been replaced with:\n" + - " Backup::Database::Redis.redis_cli_utility" - end + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Database::Redis.redis_cli_utility is being set to 'foo'" + ) end context 'when set directly' do diff --git a/spec/database/riak_spec.rb b/spec/database/riak_spec.rb index 45b8741a7..4e8067161 100644 --- a/spec/database/riak_spec.rb +++ b/spec/database/riak_spec.rb @@ -146,13 +146,12 @@ describe '#utility_path' do before do Backup::Database::Riak.any_instance.stubs(:utility) - Backup::Logger.expects(:warn).with do |err| - err.message.should == "ConfigurationError: [DEPRECATION WARNING]\n" + - " Backup::Database::Riak.utility_path has been deprecated " + - "as of backup v.3.0.21\n" + - " This setting has been replaced with:\n" + - " Backup::Database::Riak.riak_admin_utility" - end + Backup::Logger.expects(:warn).with( + instance_of(Backup::Errors::ConfigurationError) + ) + Backup::Logger.expects(:warn).with( + "Backup::Database::Riak.riak_admin_utility is being set to 'foo'" + ) end context 'when set directly' do