From: Karen Etheridge Date: Tue, 12 Feb 2013 20:41:10 +0000 (-0800) Subject: Avoid "Due to a method name conflict in roles..." errors X-Git-Tag: v0.10~1 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=gitmo%2FMooseX-ConfigFromFile.git;a=commitdiff_plain;h=eb27f7e786f04d44cdc18de8b9dc16eae4c6f66c Avoid "Due to a method name conflict in roles..." errors When composing into a role that chooses to define a _get_default_configfile method, we would conflict with the existing method already defined here. Simply not defining one, and checking if one exists with ->can, is nicer than forcing users to say { -excludes => '_get_default_configfile' }. --- diff --git a/Changes b/Changes index 0d5e5ad..0bc5f94 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,7 @@ Revision history for {{$dist->name}} {{$NEXT}} + - allow clean composition into a role, without requiring an -exclude 0.09 2013-02-11 09:35:50 PST-0800 (Karen Etheridge) - removed prereqs which are only used for optional tests diff --git a/dist.ini b/dist.ini index 6edb886..1fcb4ad 100644 --- a/dist.ini +++ b/dist.ini @@ -28,7 +28,7 @@ version_regexp = ^v([\d._]+)(-TRIAL)?$ [AutoPrereqs] skip = ^A$ -skip = ^Generic$ +skip = ^Generic skip = ^MooseX::SimpleConfig$ skip = ^MooseX::Getopt$ diff --git a/lib/MooseX/ConfigFromFile.pm b/lib/MooseX/ConfigFromFile.pm index 6214934..5fa8cac 100644 --- a/lib/MooseX/ConfigFromFile.pm +++ b/lib/MooseX/ConfigFromFile.pm @@ -9,11 +9,6 @@ use namespace::autoclean; requires 'get_config_from_file'; -# overridable in consuming class or role to provide a default value -# This is called before instantiation, so it must be a class method, -# and not depend on any other attributes -sub _get_default_configfile { } - has configfile => ( is => 'ro', isa => Path|Undef, @@ -23,7 +18,10 @@ has configfile => ( lazy => 1, # it sucks that we have to do this rather than using a builder, but some old code # simply swaps in a new default sub into the attr definition - default => sub { shift->_get_default_configfile }, + default => sub { + my $class = shift; + $class->_get_default_configfile if $class->can('_get_default_configfile'); + }, ); sub new_with_config { @@ -170,8 +168,8 @@ a hashref of arguments to pass to C which are sourced from the configfile =head2 _get_default_configfile -This class method returns nothing by default, but can and should be redefined -in a consuming class to return the default value of the configfile (if not +This class method is not implemented in this role, but can and should be defined +in a consuming class or role to return the default value of the configfile (if not passed into the constructor explicitly). =head1 COPYRIGHT diff --git a/t/05_default_sub.t b/t/05_default_sub.t index f5a44f9..b19be3d 100644 --- a/t/05_default_sub.t +++ b/t/05_default_sub.t @@ -1,7 +1,7 @@ use strict; use warnings FATAL => 'all'; -use Test::More tests => 33; +use Test::More tests => 49; use Test::Fatal; use Test::Deep '!blessed'; use Test::NoWarnings 1.04 ':early'; @@ -51,6 +51,50 @@ is( 'no exceptions', ); +{ + package Base; + use Moose; +} +{ + package GenericRole; + use Moose::Role; + with 'MooseX::ConfigFromFile'; + sub get_config_from_file + { + my ($class, $file) = @_; + $loaded_file{$file}++; + +{} + } + around BUILDARGS => sub { + my ($orig, $class) = (shift, shift); + my $args = $class->$orig(@_); + $constructor_args{$class} = $args; + }; + sub __my_configfile + { + my $class = blessed($_[0]) || $_[0]; + $configfile_sub{$class}++; + $class . ' file' + } +} + +is( + exception { + my $obj = Moose::Meta::Class->create_anon_class( + superclasses => ['Base'], + roles => ['GenericRole'], + )->name->new_with_config; + is($obj->configfile, undef, 'no configfile set'); + cmp_deeply(\%loaded_file, {}, 'no files loaded'); + cmp_deeply( + $constructor_args{blessed($obj)}, + { }, + 'correct constructor args passed', + ); + }, + undef, + 'no exceptions', +); # this is a classic legacy usecase from old documentation that we must # continue to support @@ -140,6 +184,28 @@ is( 'no exceptions', ); +{ + package OverriddenMethodAsRole; + use Moose::Role; + with 'GenericRole'; + around configfile => sub { my $orig = shift; shift->__my_configfile }; +} + +is( + exception { + my $obj = Moose::Meta::Class->create_anon_class( + superclasses => ['Base'], + roles => ['OverriddenMethodAsRole'], + )->name->new_with_config; + is($obj->configfile, blessed($obj) . ' file', 'configfile set via overridden sub'); + # this is not fixable - the reader method has been shadowed + # is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once'); + is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from'); + }, + undef, + 'no exceptions', +); + # overridable method for configfile default, and configfile init_arg is changed { @@ -169,6 +235,51 @@ is( 'no exceptions', ); +{ + package OverriddenMethodAndChangedNameAsRole; + use Moose::Role; + with 'GenericRole'; + use MooseX::Types::Path::Tiny 'Path'; + use MooseX::Types::Moose 'Undef'; + use Try::Tiny; + has configfile => ( + is => 'ro', + isa => Path|Undef, + coerce => 1, + predicate => 'has_configfile', + do { try { require MooseX::Getopt; (traits => ['Getopt']) } }, + lazy => 1, + # it sucks that we have to do this rather than using a builder, but some old code + # simply swaps in a new default sub into the attr definition + default => sub { shift->_get_default_configfile }, + + # this is the overridden bit + init_arg => 'my_configfile', + ); + around configfile => sub { my $orig = shift; shift->__my_configfile }; +} + +is( + exception { + my $obj = Moose::Meta::Class->create_anon_class( + superclasses => ['Base'], + roles => ['OverriddenMethodAndChangedNameAsRole'], + )->name->new_with_config; + is($obj->configfile, blessed($obj) . ' file', 'configfile set via overridden sub'); + cmp_deeply( + $constructor_args{blessed($obj)}, + { my_configfile => blessed($obj) . ' file' }, + 'correct constructor args passed', + ); + # this is not fixable - the reader method has been shadowed + # is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once'); + is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from'); + }, + undef, + 'no exceptions', +); + + # newly-supported overridable method for configfile default { package NewSub; @@ -193,6 +304,33 @@ is( 'no exceptions', ); +{ + package NewSubAsRole; + use Moose::Role; + with 'GenericRole'; + sub _get_default_configfile { shift->__my_configfile } +} + +is( + exception { + my $obj = Moose::Meta::Class->create_anon_class( + superclasses => ['Base'], + roles => ['NewSubAsRole'], + )->name->new_with_config; + is($obj->configfile, blessed($obj) . ' file', 'configfile set via new sub'); + cmp_deeply( + $constructor_args{blessed($obj)}, + { configfile => blessed($obj) . ' file' }, + 'correct constructor args passed', + ); + is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once'); + is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from'); + }, + undef, + 'no exceptions', +); + + # newly-supported overridable method for configfile default, and configfile # init_arg has been changed {