new _get_default_configfile method, as the preferred way to set a default
Karen Etheridge [Sat, 9 Feb 2013 19:34:36 +0000 (11:34 -0800)]
Changes
lib/MooseX/ConfigFromFile.pm
t/05_default_sub.t

diff --git a/Changes b/Changes
index bc58688..1701b13 100644 (file)
--- a/Changes
+++ b/Changes
@@ -3,6 +3,10 @@ Revision history for {{$dist->name}}
 {{$NEXT}}
           - allow configfiles called "0"
           - configfile value now passed through to new()
+          - new _get_default_configfile method added, which consumers can
+            override to provide a default value without having to redefine the
+            attribute itself (via RT#79746) -- PLEASE READ THE DOCUMENTATION
+            if you override the configfile attribute!
 
 0.07      2013-02-04 (Karen Etheridge)
           - fixed tests to not load optional dependencies
index 481839e..6214934 100644 (file)
@@ -2,18 +2,28 @@ package MooseX::ConfigFromFile;
 
 use Moose::Role;
 use MooseX::Types::Path::Tiny 'Path';
-use Try::Tiny qw/ try /;
+use MooseX::Types::Moose 'Undef';
+use Try::Tiny;
 use Carp qw(croak);
 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,
+    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 },
 );
 
 sub new_with_config {
@@ -26,7 +36,8 @@ sub new_with_config {
     }
     else {
         # This would only succeed if the consumer had defined a new configfile
-        # sub to override the generated reader
+        # sub to override the generated reader - as suggested in old
+        # documentation
         $configfile = try { $class->configfile };
 
         # this is gross, but since a lot of users have swapped in their own
@@ -96,7 +107,7 @@ MooseX::ConfigFromFile - An abstract Moose role for setting attributes from a co
   with 'MooseX::SomeSpecificConfigRole';
 
   # optionally, default the configfile:
-  around configfile => sub { '/tmp/foo.yaml' };
+  sub _get_default_configfile { '/tmp/foo.yaml' }
 
   # ... insert your stuff here ...
 
@@ -131,15 +142,8 @@ during its normal C<new_with_options>.
 This is a L<Path::Tiny> object which can be coerced from a regular path
 string or any object that supports stringification.
 This is the file your attributes are loaded from.  You can add a default
-configfile in the consuming class and it will be honored at the appropriate time
-(note that a simple sub declaration is not sufficient, as there is already a
-sub by that name being added by Moose as the attribute reader)
-
-  around configfile => sub { '/etc/myapp.yaml' };
-
-Note that you can alternately just provide a C<configfile> method which returns
-the config file when called - this will be used in preference to the default of
-the attribute.
+configfile in the consuming class and it will be honored at the appropriate
+time; see below at L</_get_default_configfile>.
 
 If you have L<MooseX::Getopt> installed, this attribute will also have the
 C<Getopt> trait supplied, so you can also set the configfile from the
@@ -164,6 +168,12 @@ classes or roles that consume this role.
 Its two arguments are the class name and the configfile, and it is expected to return
 a hashref of arguments to pass to C<new()> 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
+passed into the constructor explicitly).
+
 =head1 COPYRIGHT
 
 Copyright (c) - the MooseX::ConfigFromFile "AUTHOR" and "CONTRIBUTORS" as listed below.
index 6c41191..0160167 100644 (file)
@@ -2,13 +2,14 @@ use strict;
 use warnings FATAL => 'all';
 
 use Test::Requires 'MooseX::SimpleConfig';      # skip all if not reuqired
-use Test::More tests => 19;
+use Test::More tests => 33;
 use Test::Fatal;
 use Test::Deep '!blessed';
 use Test::NoWarnings 1.04 ':early';
 use Scalar::Util 'blessed';
 
 my %loaded_file;
+my %configfile_sub;
 my %constructor_args;
 
 
@@ -31,6 +32,7 @@ my %constructor_args;
     sub __my_configfile
     {
         my $class = blessed($_[0]) || $_[0];
+        $configfile_sub{$class}++;
         $class . ' file'
     }
 }
@@ -72,6 +74,27 @@ is(
     'no exceptions',
 );
 
+{
+    package OverriddenDefaultMethod;
+    use Moose;
+    extends 'Generic';
+    has '+configfile' => (
+        default => sub { shift->__my_configfile },
+    );
+}
+
+is(
+    exception {
+        my $obj = OverriddenDefaultMethod->new_with_config;
+        is($obj->configfile, blessed($obj) . ' file', 'configfile set via overridden default');
+        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',
+);
+
+
 # legacy usecase, and configfile init_arg has been changed
 {
     package OverriddenDefaultAndChangedName;
@@ -110,6 +133,8 @@ is(
     exception {
         my $obj = OverriddenMethod->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,
@@ -137,6 +162,60 @@ is(
             {  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 WrapperSub;
+    use Moose;
+    extends 'Generic';
+    sub _get_default_configfile { shift->__my_configfile }
+}
+
+is(
+    exception {
+        my $obj = WrapperSub->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
+{
+    package WrapperSubAndChangedName;
+    use Moose;
+    extends 'Generic';
+    has '+configfile' => (
+        init_arg => 'my_configfile',
+    );
+    sub _get_default_configfile { shift->__my_configfile }
+}
+
+is(
+    exception {
+        my $obj = WrapperSubAndChangedName->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',
+        );
+        is($configfile_sub{blessed($obj)}, 1, 'configfile was calculated just once');
         is($loaded_file{blessed($obj) . ' file'}, 1, 'correct file was loaded from');
     },
     undef,