From 3231cc98550a704b3eb065c59dffb705dd569a1b Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 00:54:20 +0100 Subject: [PATCH 01/16] Proof of concept: apply LazyRequired when cloning and inheriting attributes. --- .../Meta/Attribute/Trait/LazyRequire.pm | 25 +++++++++++++++++++ t/rt76054_inheritance.t | 19 +++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm index 3ddb6a4..9a7d297 100644 --- a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm +++ b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm @@ -23,6 +23,12 @@ after _process_options => sub { return unless $options->{lazy_required}; + $class->_enable_lazy_required($name, $options); +}; + +sub _enable_lazy_required { + my ($class, $name, $options) = @_; + # lazy_required + default or builder doesn't make sense because if there # is a default/builder, the reader will always be able to return a value. Moose->throw_error( @@ -37,6 +43,25 @@ after _process_options => sub { }; }; +around clone_and_inherit_options => sub { + my ($orig, $self, %options) = @_; + + if ($options{lazy_required}) { + $self->_enable_lazy_required($self->name, \%options); + } else { + for my $boolean_option (qw(lazy required)) { + if (!exists $options{$boolean_option}) { + $options{$boolean_option} = 0; + } + } + ### FIXME: can we say "no, don't provide a default"? + if (!exists $options{default}) { + $options{default} = undef; + } + } + $self->$orig(%options); +}; + package # hide Moose::Meta::Attribute::Custom::Trait::LazyRequire; diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index f1f0375..f598228 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -4,8 +4,6 @@ use warnings; use Test::More 0.88; use Test::Fatal; -local $TODO = 'RT#75054'; - { package Account; use Moose; @@ -31,15 +29,28 @@ local $TODO = 'RT#75054'; lazy_required => 1, ); + package AccountExt::Lax; + + use Moose; + extends 'AccountExt'; + use MooseX::LazyRequire; + + has '+password' => ( + lazy_required => 0, + default => sub { 'hunter2' }, + ); } my $r = AccountExt->new; my $e = exception { $r->password }; -isnt($e, undef, 'works on inherited attributes') && +isnt($e, undef, 'works on inherited attributes: exception') && like( exception { $r->password }, qr/Attribute 'password' must be provided before calling reader/, - 'works on inherited attributes' + 'works on inherited attributes: mentions password by name' ); +my $lax = AccountExt::Lax->new; +is($lax->password, 'hunter2', 'We can override LazyRequired *off* as well'); + done_testing; From c4ee29bf39f6c789cf8e096fc23b3eb1313583ef Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 01:03:51 +0100 Subject: [PATCH 02/16] Add comments to the bare-bones test that was provided as part of the initial bug report. --- t/rt76054_inheritance.t | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index f598228..ced7b42 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -5,6 +5,7 @@ use Test::More 0.88; use Test::Fatal; { + # Our base class has an attribute that's nothing special. package Account; use Moose; use MooseX::LazyRequire; @@ -14,6 +15,7 @@ use Test::Fatal; isa => 'Str', ); + # The extended class wants you to specify a password. package AccountExt; use Moose; @@ -29,7 +31,8 @@ use Test::Fatal; lazy_required => 1, ); - package AccountExt::Lax; + # A further subclass will supply one for you if you don't specify one. + package AccountExt::Lax::Default; use Moose; extends 'AccountExt'; @@ -40,8 +43,9 @@ use Test::Fatal; default => sub { 'hunter2' }, ); } -my $r = AccountExt->new; +# In the extension class, asking about a password generates an exception. +my $r = AccountExt->new; my $e = exception { $r->password }; isnt($e, undef, 'works on inherited attributes: exception') && like( @@ -50,6 +54,7 @@ like( 'works on inherited attributes: mentions password by name' ); +# The lax subclass is happy to provide you with a default password. my $lax = AccountExt::Lax->new; is($lax->password, 'hunter2', 'We can override LazyRequired *off* as well'); From 4370351aebad83906f3a94a39b070596f2f4c612 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 01:09:02 +0100 Subject: [PATCH 03/16] Get rid of algebraic variable names. --- t/rt76054_inheritance.t | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index ced7b42..b04ff9f 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -45,17 +45,19 @@ use Test::Fatal; } # In the extension class, asking about a password generates an exception. -my $r = AccountExt->new; -my $e = exception { $r->password }; -isnt($e, undef, 'works on inherited attributes: exception') && +my $account_ext = AccountExt->new; +my $exception_ext = exception { $account_ext->password }; +isnt($exception_ext, undef, 'works on inherited attributes: exception') && like( - exception { $r->password }, + $exception_ext, qr/Attribute 'password' must be provided before calling reader/, 'works on inherited attributes: mentions password by name' ); # The lax subclass is happy to provide you with a default password. -my $lax = AccountExt::Lax->new; -is($lax->password, 'hunter2', 'We can override LazyRequired *off* as well'); +my $account_ext_lax_default = AccountExt::Lax::Default->new; +is($account_ext_lax_default->password, + 'hunter2', + 'We can override LazyRequired *off* as well'); done_testing; From 7bb448ebef2a9b11cc98428011b30a58a143de85 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 01:41:40 +0100 Subject: [PATCH 04/16] Clarification: MooseX::LazyRequire doesn't add a trait to attributes. --- t/basic.t | 31 +++++++++++++++++++++++++++++++ t/rt76054_inheritance.t | 11 ++++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/t/basic.t b/t/basic.t index cc10e7f..69e9781 100644 --- a/t/basic.t +++ b/t/basic.t @@ -3,6 +3,21 @@ use warnings; use Test::More 0.88; use Test::Fatal; +{ + package Vanilla; + use Moose; + + has flavour => ( is => 'ro' ); +} + +{ + my $vanilla = Vanilla->new; + my $attribute = $vanilla->meta->find_attribute_by_name('flavour'); + ok(!$attribute->can('lazy_required'), + q{If MooseX::LazyRequired isn't loaded, lazy_required isn't a method} + ); +} + { package Foo; use Moose; @@ -12,6 +27,8 @@ use Test::Fatal; is => 'ro', lazy_required => 1, ); + + has other_attribute => (is => 'ro'); } { @@ -26,6 +43,20 @@ use Test::Fatal; qr/Attribute 'bar' must be provided/, 'lazy_required value was not provided', ); + + my $foo = Foo->new; + my $attribute = $foo->meta->find_attribute_by_name('bar'); + ok($attribute->can('lazy_required'), + 'MooseX::LazyRequire is loaded, so we have a lazy_required method' + ); + ok($attribute->lazy_required, 'This attribute is indeed lazy-required'); + + my $other_attribute = $foo->meta->find_attribute_by_name('other_attribute'); + ok($other_attribute->can('lazy_required'), + 'The other attribute therefore also has a lazy_required method'); + ok(!$other_attribute->lazy_required, + q{The other attribute isn't lazy-required, though}, + ); } { diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index b04ff9f..3d67862 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -15,7 +15,7 @@ use Test::Fatal; isa => 'Str', ); - # The extended class wants you to specify a password. + # The extended class wants you to specify a password, eventually. package AccountExt; use Moose; @@ -25,9 +25,6 @@ use Test::Fatal; has '+password' => ( is => 'ro', - # Probably there also should be: - # traits => ['LazyRequire'], - # but I'm not sure lazy_required => 1, ); @@ -44,7 +41,8 @@ use Test::Fatal; ); } -# In the extension class, asking about a password generates an exception. +# In the extension class, asking about a password generates an exception, +# when you ask about it. my $account_ext = AccountExt->new; my $exception_ext = exception { $account_ext->password }; isnt($exception_ext, undef, 'works on inherited attributes: exception') && @@ -53,6 +51,9 @@ like( qr/Attribute 'password' must be provided before calling reader/, 'works on inherited attributes: mentions password by name' ); +my $attribute_ext = $account_ext->meta->find_attribute_by_name('password'); +ok($attribute_ext->lazy_required, + 'The inherited attribute is now lazy-required'); # The lax subclass is happy to provide you with a default password. my $account_ext_lax_default = AccountExt::Lax::Default->new; From 5f76509ad0de82d6ffbdfc96a7c4b7f780232dd6 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 01:53:23 +0100 Subject: [PATCH 05/16] Make sure that you can override the required option if you disable lazy_required. --- t/rt76054_inheritance.t | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 3d67862..6a44fd0 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -21,14 +21,26 @@ use Test::Fatal; use Moose; extends 'Account'; use MooseX::LazyRequire; - use Carp; has '+password' => ( is => 'ro', lazy_required => 1, ); - # A further subclass will supply one for you if you don't specify one. + # A further subclass insists that you supply the password immediately. + package AccountExt::Harsh; + + use Moose; + extends 'AccountExt'; + use MooseX::LazyRequire; + + has '+password' => ( + is => 'ro', + lazy_required => 0, + required => 1, + ); + + # Another subclass will supply one for you if you don't specify one. package AccountExt::Lax::Default; use Moose; @@ -55,6 +67,12 @@ my $attribute_ext = $account_ext->meta->find_attribute_by_name('password'); ok($attribute_ext->lazy_required, 'The inherited attribute is now lazy-required'); +# The harsh subclass generates an exception as soon as you don't provide a +# password. +my $exception_harsh_constructor = exception { AccountExt::Harsh->new }; +isnt($exception_harsh_constructor, undef, + 'Cannot create a harsh object without a password'); + # The lax subclass is happy to provide you with a default password. my $account_ext_lax_default = AccountExt::Lax::Default->new; is($account_ext_lax_default->password, From c1ed89ef6fe7245c37c46aa06aa642744abfe86c Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 02:10:44 +0100 Subject: [PATCH 06/16] Test that we can say "not lazy-required, but still lazy". --- t/rt76054_inheritance.t | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 6a44fd0..2797b7e 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -40,6 +40,20 @@ use Test::Fatal; required => 1, ); + # Another subclass makes the attribute lazy. + package AccountExt::Lazy; + + use Moose; + extends 'AccountExt'; + use MooseX::LazyRequire; + + $AccountExt::Lazy::default_password = 'password'; + has '+password' => ( + lazy_required => 0, + lazy => 1, + default => sub { $AccountExt::Lazy::default_password }, + ); + # Another subclass will supply one for you if you don't specify one. package AccountExt::Lax::Default; @@ -73,6 +87,15 @@ my $exception_harsh_constructor = exception { AccountExt::Harsh->new }; isnt($exception_harsh_constructor, undef, 'Cannot create a harsh object without a password'); +# The lazy subclass will use a lazily-generated value. +my $lazy = AccountExt::Lazy->new; +{ + local $AccountExt::Lazy::default_password = 'qwerty'; + is($lazy->password, 'qwerty', + 'The lazy object resolves its default value as late as possible' + ); +} + # The lax subclass is happy to provide you with a default password. my $account_ext_lax_default = AccountExt::Lax::Default->new; is($account_ext_lax_default->password, From 64078b8605815dfb2844bc192ba9d9d9bc0b8fa9 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 02:23:22 +0100 Subject: [PATCH 07/16] If you disable lazy_required but don't provide an alternative default, and the type you're using doesn't allow undef, you're in a world of hurt. --- t/rt76054_inheritance.t | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 2797b7e..2c94296 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -65,15 +65,25 @@ use Test::Fatal; lazy_required => 0, default => sub { 'hunter2' }, ); + + # But if you don't override the default, you're SOL. + package AccountExt::Lax::Woo; + + use Moose; + extends 'AccountExt'; + use MooseX::LazyRequire; + + has '+password' => (lazy_required => 0); } # In the extension class, asking about a password generates an exception, # when you ask about it. my $account_ext = AccountExt->new; -my $exception_ext = exception { $account_ext->password }; -isnt($exception_ext, undef, 'works on inherited attributes: exception') && -like( - $exception_ext, +my $exception_ext_password = exception { $account_ext->password }; +isnt($exception_ext_password, undef, + 'works on inherited attributes: exception') +&& like( + $exception_ext_password, qr/Attribute 'password' must be provided before calling reader/, 'works on inherited attributes: mentions password by name' ); @@ -102,4 +112,13 @@ is($account_ext_lax_default->password, 'hunter2', 'We can override LazyRequired *off* as well'); +# The woo subclass really wants to be the base subclass, but can't, because +# a default option got in the way in the inheritance hierarchy. +my $exception_ext_woo_password = exception { AccountExt::Lax::Woo->new }; +like( + $exception_ext_woo_password, + qr/Attribute .+ password .+ \Qdoes not pass the type constraint\E/x, + 'Falling back to undef as a last resort violates type constraints' +); + done_testing; From fea0aa740bfad01b0dd248819870114e5d67c30b Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 02:34:43 +0100 Subject: [PATCH 08/16] Document the limitations of saying lazy_required and then "wait, never mind". --- lib/MooseX/LazyRequire.pm | 18 ++++++++++++++++++ .../Meta/Attribute/Trait/LazyRequire.pm | 12 +++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/MooseX/LazyRequire.pm b/lib/MooseX/LazyRequire.pm index 91ac010..ae94077 100644 --- a/lib/MooseX/LazyRequire.pm +++ b/lib/MooseX/LazyRequire.pm @@ -33,6 +33,16 @@ use namespace::autoclean; Foo->new(bar => 42); # succeeds, bar will be 42 Foo->new; # fails, neither foo nor bare were given + package Foo::Nevermind; + + use Moose; + use MooseX::LazyRequire; + extends 'Foo'; + + has foo => ( lazy_required => 0 ); + + Foo::Nevermind->new; # succeeds + =head1 DESCRIPTION This module adds a C option to Moose attribute declarations. @@ -41,6 +51,14 @@ The reader methods for all attributes with that option will throw an exception unless a value for the attributes was provided earlier by a constructor parameter or through a writer method. +You can override an attribute declaration in a parent class, either enabling +or disabling lazy-required. Note, though, that becaus lazy_required works by +declaring a default value that's evaluated lazily, if you say "never mind, this +attribute isn't lazy-required any more", you I need to provide a +default value or coderef. B if C isn't a valid value for +your attribute, because that's what MooseX::LazyRequire will try to enforce +in the absence of any better guidance. + =head1 CAVEATS Prior to Moose 1.9900, roles didn't have an attribute metaclass, so this module can't diff --git a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm index 9a7d297..ea6a5cb 100644 --- a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm +++ b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm @@ -13,6 +13,9 @@ has lazy_required => ( default => 0, ); +# This will be called when a new attribute is created - i.e. when someone in +# a Moose class says has attribute => ... for the first time. + after _process_options => sub { my ($class, $name, $options) = @_; @@ -43,18 +46,25 @@ sub _enable_lazy_required { }; }; +# This will be called when someone says, in a subclass of a Moose class, +# has '+attribute' => ... + around clone_and_inherit_options => sub { my ($orig, $self, %options) = @_; if ($options{lazy_required}) { $self->_enable_lazy_required($self->name, \%options); } else { + # Disable lazy and required, unless we were told "actually, I like + # that part of lazy-required". for my $boolean_option (qw(lazy required)) { if (!exists $options{$boolean_option}) { $options{$boolean_option} = 0; } } - ### FIXME: can we say "no, don't provide a default"? + # In desperation, if we haven't specified an alternative default + # value or coderef, claim that undef is fine. This may well not be, + # if the type of the attribute doesn't accept undef as a legal value. if (!exists $options{default}) { $options{default} = undef; } From 8dff0fee44b857d39ed63332004b54173becb342 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 02:43:20 +0100 Subject: [PATCH 09/16] Whitespace shenanigans. --- lib/MooseX/LazyRequire.pm | 6 +++--- .../Meta/Attribute/Trait/LazyRequire.pm | 2 +- t/basic.t | 2 +- t/rt76054_inheritance.t | 14 +++++++------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/MooseX/LazyRequire.pm b/lib/MooseX/LazyRequire.pm index ae94077..20ada6a 100644 --- a/lib/MooseX/LazyRequire.pm +++ b/lib/MooseX/LazyRequire.pm @@ -34,13 +34,13 @@ use namespace::autoclean; Foo->new; # fails, neither foo nor bare were given package Foo::Nevermind; - + use Moose; use MooseX::LazyRequire; extends 'Foo'; - + has foo => ( lazy_required => 0 ); - + Foo::Nevermind->new; # succeeds =head1 DESCRIPTION diff --git a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm index ea6a5cb..e8f14e3 100644 --- a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm +++ b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm @@ -31,7 +31,7 @@ after _process_options => sub { sub _enable_lazy_required { my ($class, $name, $options) = @_; - + # lazy_required + default or builder doesn't make sense because if there # is a default/builder, the reader will always be able to return a value. Moose->throw_error( diff --git a/t/basic.t b/t/basic.t index 69e9781..e06d385 100644 --- a/t/basic.t +++ b/t/basic.t @@ -6,7 +6,7 @@ use Test::Fatal; { package Vanilla; use Moose; - + has flavour => ( is => 'ro' ); } diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 2c94296..8d8fe42 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -29,11 +29,11 @@ use Test::Fatal; # A further subclass insists that you supply the password immediately. package AccountExt::Harsh; - + use Moose; extends 'AccountExt'; use MooseX::LazyRequire; - + has '+password' => ( is => 'ro', lazy_required => 0, @@ -42,11 +42,11 @@ use Test::Fatal; # Another subclass makes the attribute lazy. package AccountExt::Lazy; - + use Moose; extends 'AccountExt'; use MooseX::LazyRequire; - + $AccountExt::Lazy::default_password = 'password'; has '+password' => ( lazy_required => 0, @@ -56,7 +56,7 @@ use Test::Fatal; # Another subclass will supply one for you if you don't specify one. package AccountExt::Lax::Default; - + use Moose; extends 'AccountExt'; use MooseX::LazyRequire; @@ -68,11 +68,11 @@ use Test::Fatal; # But if you don't override the default, you're SOL. package AccountExt::Lax::Woo; - + use Moose; extends 'AccountExt'; use MooseX::LazyRequire; - + has '+password' => (lazy_required => 0); } From 98fda820ca3917f8f622b1269abe0b8babb19ec1 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 15:58:57 +0100 Subject: [PATCH 10/16] Don't confuse explicitly saying lazy_required => 0 and not mentioning lazy_required at all. All a class needs to do is say use MooseX::LazyRequire; and the clone_and_inherit_options method of Moose::Meta::Attribute will be wrapped - for *all* attributes. --- .../Meta/Attribute/Trait/LazyRequire.pm | 2 +- t/rt76054_inheritance.t | 39 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm index e8f14e3..8da7504 100644 --- a/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm +++ b/lib/MooseX/LazyRequire/Meta/Attribute/Trait/LazyRequire.pm @@ -54,7 +54,7 @@ around clone_and_inherit_options => sub { if ($options{lazy_required}) { $self->_enable_lazy_required($self->name, \%options); - } else { + } elsif (exists $options{lazy_required}) { # Disable lazy and required, unless we were told "actually, I like # that part of lazy-required". for my $boolean_option (qw(lazy required)) { diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 8d8fe42..27097a9 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -4,8 +4,10 @@ use warnings; use Test::More 0.88; use Test::Fatal; +my $default_description = 'Clearly nothing that you care about'; { - # Our base class has an attribute that's nothing special. + # Our base class has an attribute that's nothing special, and another one + # that most of these tests will ignore. package Account; use Moose; use MooseX::LazyRequire; @@ -14,6 +16,12 @@ use Test::Fatal; is => 'rw', isa => 'Str', ); + has description => ( + is => 'rw', + isa => 'Str', + lazy => 1, + default => sub { $default_description } + ); # The extended class wants you to specify a password, eventually. package AccountExt; @@ -74,6 +82,26 @@ use Test::Fatal; use MooseX::LazyRequire; has '+password' => (lazy_required => 0); + + # If you don't mention lazy_required *at all* when overriding an + # attribute, that's fine. + package Account::Logged; + + use Moose; + extends 'Account'; + use MooseX::LazyRequire; + + has 'description_history' => ( + is => 'rw', + isa => 'ArrayRef', + default => sub { [] }, + ); + has '+description' => ( + trigger => sub { + my ($self, $new_value, $old_value) = @_; + push @{ $self->description_history }, $new_value; + } + ); } # In the extension class, asking about a password generates an exception, @@ -121,4 +149,13 @@ like( 'Falling back to undef as a last resort violates type constraints' ); +# We don't mess with attributes in classes that use MooseX::LazyRequire but +# don't set lazy_require explicitly, not even when subclassing attributes. +my $logged = Account::Logged->new; +is($logged->description, $default_description, + 'When lazy_required is omitted entirely, the default etc. is unaffected'); +$logged->description('Now for something else'); +is($logged->description_history->[0], 'Now for something else', + 'The trigger fired, though, so we *did* change that attribute'); + done_testing; From 2fbb5b6e8574eac26dd35b91331226332b1cff08 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:06:20 +0100 Subject: [PATCH 11/16] More whitespace shenanigans. --- t/rt76054_inheritance.t | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 27097a9..0e168c2 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -86,11 +86,11 @@ my $default_description = 'Clearly nothing that you care about'; # If you don't mention lazy_required *at all* when overriding an # attribute, that's fine. package Account::Logged; - + use Moose; extends 'Account'; use MooseX::LazyRequire; - + has 'description_history' => ( is => 'rw', isa => 'ArrayRef', From 86307d911fc5fe8f9f2a05bb614f79c9a98edd3a Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:06:36 +0100 Subject: [PATCH 12/16] Typo. --- lib/MooseX/LazyRequire.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/MooseX/LazyRequire.pm b/lib/MooseX/LazyRequire.pm index 20ada6a..7c45a2d 100644 --- a/lib/MooseX/LazyRequire.pm +++ b/lib/MooseX/LazyRequire.pm @@ -52,7 +52,7 @@ unless a value for the attributes was provided earlier by a constructor parameter or through a writer method. You can override an attribute declaration in a parent class, either enabling -or disabling lazy-required. Note, though, that becaus lazy_required works by +or disabling lazy-required. Note, though, that because lazy_required works by declaring a default value that's evaluated lazily, if you say "never mind, this attribute isn't lazy-required any more", you I need to provide a default value or coderef. B if C isn't a valid value for From b2d5363674ecbc74ac4b5ed69a291a137c41e131 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:07:00 +0100 Subject: [PATCH 13/16] Actually mention what changed. --- Changes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Changes b/Changes index 1f0db30..9d2f35b 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,8 @@ Change history for distribution MooseX-LazyRequire {{$NEXT}} + - Let people specify lazy_required when inheriting attributes (Sam Kington) + This is RT#76054. 0.11 2014-08-16 20:49:14Z - lots of cleanup of metadata From 32d032e023183a2d735dbecb0c315f1776a70b69 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:47:42 +0100 Subject: [PATCH 14/16] Documentation fix: we're overriding an attribute, not replacing it. --- lib/MooseX/LazyRequire.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/MooseX/LazyRequire.pm b/lib/MooseX/LazyRequire.pm index 7c45a2d..f5a2279 100644 --- a/lib/MooseX/LazyRequire.pm +++ b/lib/MooseX/LazyRequire.pm @@ -39,7 +39,7 @@ use namespace::autoclean; use MooseX::LazyRequire; extends 'Foo'; - has foo => ( lazy_required => 0 ); + has '+foo' => ( lazy_required => 0 ); Foo::Nevermind->new; # succeeds From 878c6bed7f09b2f2846acac135dc179eae314533 Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:48:02 +0100 Subject: [PATCH 15/16] Trivial documentation fix. --- Changes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Changes b/Changes index 9d2f35b..7e27d84 100644 --- a/Changes +++ b/Changes @@ -1,7 +1,7 @@ Change history for distribution MooseX-LazyRequire {{$NEXT}} - - Let people specify lazy_required when inheriting attributes (Sam Kington) + - Let people specify lazy_required when inheriting attributes (Sam Kington). This is RT#76054. 0.11 2014-08-16 20:49:14Z From 54eb46723ad328948eb972e8c27d7e7682b604af Mon Sep 17 00:00:00 2001 From: Sam Kington Date: Sat, 13 Jun 2020 16:54:22 +0100 Subject: [PATCH 16/16] Move this test to a slightly more logical place. --- t/rt76054_inheritance.t | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/t/rt76054_inheritance.t b/t/rt76054_inheritance.t index 0e168c2..0e0e03b 100644 --- a/t/rt76054_inheritance.t +++ b/t/rt76054_inheritance.t @@ -119,6 +119,14 @@ my $attribute_ext = $account_ext->meta->find_attribute_by_name('password'); ok($attribute_ext->lazy_required, 'The inherited attribute is now lazy-required'); +# These subclasses turn lazy_required *off* again, sometimes adding in elements +# that lazy_required provides. +# The lax subclass is happy to provide you with a default password. +my $account_ext_lax_default = AccountExt::Lax::Default->new; +is($account_ext_lax_default->password, + 'hunter2', + 'We can override LazyRequired *off* as well'); + # The harsh subclass generates an exception as soon as you don't provide a # password. my $exception_harsh_constructor = exception { AccountExt::Harsh->new }; @@ -134,12 +142,6 @@ my $lazy = AccountExt::Lazy->new; ); } -# The lax subclass is happy to provide you with a default password. -my $account_ext_lax_default = AccountExt::Lax::Default->new; -is($account_ext_lax_default->password, - 'hunter2', - 'We can override LazyRequired *off* as well'); - # The woo subclass really wants to be the base subclass, but can't, because # a default option got in the way in the inheritance hierarchy. my $exception_ext_woo_password = exception { AccountExt::Lax::Woo->new };