Skip Menu |
 

This queue is for tickets about the MooseX-Storage CPAN distribution.

Report information
The Basics
Id: 65733
Status: open
Priority: 0/
Queue: MooseX-Storage

People
Owner: Nobody in particular
Requestors: denis.baurain [...] ulg.ac.be
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



From denis.baurain [...] ulg.ac.be Tue Feb 15 02: 56:58 2011
MIME-Version: 1.0
X-Spam-Status: No, score=-6.9 tagged_above=-99.9 required=10 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_HI=-5] autolearn=ham
X-Mailer: CTM PowerMail version 6.0.6 build 4630 French (intel) <http://www.ctmdev.com>
X-Spam-Flag: NO
content-type: text/plain; charset="utf-8"
Message-ID: <20110215075646.72180570 [...] smtp.ulg.ac.be>
X-Virus-Scanned: Debian amavisd-new at bestpractical.com
X-Spam-Score: -6.9
Received: from localhost (localhost [127.0.0.1]) by hipster.bestpractical.com (Postfix) with ESMTP id CD2F961E003 for <cpan-bug+MooseX-Storage [...] hipster.bestpractical.com>; Tue, 15 Feb 2011 02:56:58 -0500 (EST)
Received: from hipster.bestpractical.com ([127.0.0.1]) by localhost (hipster.bestpractical.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id seD7glKY1iao for <cpan-bug+MooseX-Storage [...] hipster.bestpractical.com>; Tue, 15 Feb 2011 02:56:57 -0500 (EST)
Received: from la.mx.develooper.com (x1.develooper.com [207.171.7.70]) by hipster.bestpractical.com (Postfix) with SMTP id D441224156B for <bug-MooseX-Storage [...] rt.cpan.org>; Tue, 15 Feb 2011 02:56:56 -0500 (EST)
Received: (qmail 4495 invoked by uid 103); 15 Feb 2011 07:56:56 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 15 Feb 2011 07:56:56 -0000
Received: from serv108.segi.ulg.ac.be (HELO serv108.segi.ulg.ac.be) (139.165.32.111) by 16.mx.develooper.com (qpsmtpd/0.80) with ESMTP; Mon, 14 Feb 2011 23:56:53 -0800
Received: (qmail 5952 invoked from network); 15 Feb 2011 08:56:49 +0100
Received: from unknown (HELO [192.168.2.2]) (u016721 [...] [139.165.108.220]) (envelope-sender <denis.baurain [...] ulg.ac.be>) by serv108.segi.ulg.ac.be (qmail-ldap-1.03) with SMTP for <bug-MooseX-Storage [...] rt.cpan.org>; 15 Feb 2011 08:56:49 +0100
Delivered-To: cpan-bug+MooseX-Storage [...] hipster.bestpractical.com
Subject: issue with init_arg attribute option
Return-Path: <denis.baurain [...] ulg.ac.be>
X-RT-Mail-Extension: moosex-storage
X-Original-To: cpan-bug+MooseX-Storage [...] hipster.bestpractical.com
X-Spam-Check-BY: 16.mx.develooper.com
Date: Tue, 15 Feb 2011 08:56:46 +0100
X-Spam-Level:
To: <bug-MooseX-Storage [...] rt.cpan.org>
Content-Transfer-Encoding: quoted-printable
From: "Denis BAURAIN" <denis.baurain [...] ulg.ac.be>
X-RT-Original-Encoding: ISO-8859-1
Content-Length: 1902
Download (untitled) / with headers
text/plain 1.8k
Hi, Per perigrin's advice, I post here the test case that I posted earlier on stackoverflow.com. Sorry for the delay. I'm trying to add serialization to a Moose class that has required attributes using custom init_arg's (to prefix the attribute name with a dash for API consistency) and it seems that this causes unpacking to fail. I've setup a test case below to illustrate my point. use strict; use warnings; package MyClass1; use Moose; use MooseX::Storage; use namespace::autoclean; with Storage; has 'my_attr' => ( is => 'ro', isa => 'Str', required => 1, ); __PACKAGE__->meta->make_immutable; package MyClass2; use Moose; use MooseX::Storage; use namespace::autoclean; with Storage; has 'my_attr' => ( is => 'ro', isa => 'Str', required => 1, init_arg => '-my_attr', ); __PACKAGE__->meta->make_immutable; package main; my $inst1 = MyClass1->new(my_attr => 'The String'); my $packed1 = $inst1->pack; my $unpacked1 = MyClass1->unpack($packed1); # this works my $inst2 = MyClass2->new(-my_attr => 'The String'); my $packed2 = $inst2->pack; my $unpacked2 = MyClass2->unpack($packed2); # this fails with a ... # ... Attribute (my_attr) is required at ... Update: further investigation indicates that the issue is that init_arg is not taken into account when packing. Hence, even a non-required attribute using a custom init_arg is not correctly restored after unpacking. See this additional test case: package MyClass3; with Storage; has 'my_attr' => ( is => 'ro', isa => 'Str', init_arg => '-my_attr', ); # in main... my $inst3 = MyClass3->new(-my_attr => 'The String'); my $packed3 = $inst3->pack; my $unpacked3 = MyClass3->unpack($packed3); # this seems to work ... say $unpacked3->my_attr; # ... but my_attr stays undef Thanks a lot for your help, Denis
From denis.baurain [...] ulg.ac.be Mon Feb 28 04: 34:27 2011
MIME-Version: 1.0
X-Spam-Status: No, score=-6.9 tagged_above=-99.9 required=10 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_HI=-5] autolearn=ham
In-Reply-To: <rt-3.8.HEAD-9644-1297756619-1154.65733-3-0 [...] rt.cpan.org>
X-Mailer: CTM PowerMail version 6.0.6 build 4630 French (intel) <http://www.ctmdev.com>
X-Spam-Flag: NO
References: <RT-Ticket-65733 [...] rt.cpan.org> <20110215075646.72180570 [...] smtp.ulg.ac.be> <rt-3.8.HEAD-9644-1297756619-1154.65733-3-0 [...] rt.cpan.org>
X-Virus-Checked: Checked by ClamAV on 16.mx.develooper.com
X-Virus-Scanned: Debian amavisd-new at bestpractical.com
Message-ID: <20110228093413.883420042 [...] smtp.ulg.ac.be>
Content-Type: multipart/mixed; boundary="==_20110228093413.883420042-1_=="
X-Spam-Score: -6.9
Received: from localhost (localhost [127.0.0.1]) by hipster.bestpractical.com (Postfix) with ESMTP id 670BE241626 for <cpan-bug+MooseX-Storage [...] hipster.bestpractical.com>; Mon, 28 Feb 2011 04:34:27 -0500 (EST)
Received: from hipster.bestpractical.com ([127.0.0.1]) by localhost (hipster.bestpractical.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id uVEtIwLFAB3V for <cpan-bug+MooseX-Storage [...] hipster.bestpractical.com>; Mon, 28 Feb 2011 04:34:25 -0500 (EST)
Received: from la.mx.develooper.com (x1.develooper.com [207.171.7.70]) by hipster.bestpractical.com (Postfix) with SMTP id C7E4E241617 for <bug-MooseX-Storage [...] rt.cpan.org>; Mon, 28 Feb 2011 04:34:24 -0500 (EST)
Received: (qmail 27486 invoked by uid 103); 28 Feb 2011 09:34:23 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 28 Feb 2011 09:34:23 -0000
Received: from serv108.segi.ulg.ac.be (HELO serv108.segi.ulg.ac.be) (139.165.32.111) by 16.mx.develooper.com (qpsmtpd/0.80) with ESMTP; Mon, 28 Feb 2011 01:34:20 -0800
Received: (qmail 29442 invoked from network); 28 Feb 2011 10:34:14 +0100
Received: from unknown (HELO [192.168.2.2]) (u016721 [...] [139.165.108.220]) (envelope-sender <denis.baurain [...] ulg.ac.be>) by serv108.segi.ulg.ac.be (qmail-ldap-1.03) with SMTP for <bug-MooseX-Storage [...] rt.cpan.org>; 28 Feb 2011 10:34:14 +0100
Delivered-To: cpan-bug+MooseX-Storage [...] hipster.bestpractical.com
Subject: Re: [rt.cpan.org #65733] AutoReply: issue with init_arg attribute option
Return-Path: <denis.baurain [...] ulg.ac.be>
X-Spam-Check-BY: 16.mx.develooper.com
X-Original-To: cpan-bug+MooseX-Storage [...] hipster.bestpractical.com
X-RT-Mail-Extension: moosex-storage
Date: Mon, 28 Feb 2011 10:34:13 +0100
X-Spam-Level:
To: <bug-MooseX-Storage [...] rt.cpan.org>
From: "Denis BAURAIN" <denis.baurain [...] ulg.ac.be>
RT-Message-ID: <rt-3.8.HEAD-26834-1298885668-1898.65733-0-0 [...] rt.cpan.org>
Content-Length: 0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
X-RT-Original-Encoding: utf-8
Content-Length: 3369
Download (untitled) / with headers
text/plain 3.2k
Hi, I've written a patch for the issue I reported last month. I've also added a basic test file to check it works as expected. All other tests (even optional) of the current distribution (0.29) still pass. Not sure about the impact on performance though... I attach the files. Hope this helps (this helps me at least :-) Denis At Tue, 15 Feb 2011 02:56:59 -0500, bug-MooseX-Storage@rt.cpan.org wrote: Show quoted text
> >Greetings, > >This message has been automatically generated in response to the >creation of a trouble ticket regarding: > "issue with init_arg attribute option", >a summary of which appears below. > >There is no need to reply to this message right now. Your ticket has been >assigned an ID of [rt.cpan.org #65733]. Your ticket is accessible >on the web at: > > http://rt.cpan.org/Ticket/Display.html?id=65733 > >Please include the string: > > [rt.cpan.org #65733] > >in the subject line of all future correspondence about this issue. To do so, >you may reply to this message. > > Thank you, > bug-MooseX-Storage@rt.cpan.org > >------------------------------------------------------------------------- >Hi, > >Per perigrin's advice, I post here the test case that I posted earlier >on stackoverflow.com. Sorry for the delay. > >I'm trying to add serialization to a Moose class that has required >attributes using custom init_arg's (to prefix the attribute name with a >dash for API consistency) and it seems that this causes unpacking to >fail. I've setup a test case below to illustrate my point. > >use strict; >use warnings; > > >package MyClass1; > >use Moose; >use MooseX::Storage; >use namespace::autoclean; > >with Storage; > >has 'my_attr' => ( > is => 'ro', > isa => 'Str', > required => 1, >); > >__PACKAGE__->meta->make_immutable; > > >package MyClass2; > >use Moose; >use MooseX::Storage; >use namespace::autoclean; > >with Storage; > >has 'my_attr' => ( > is => 'ro', > isa => 'Str', > required => 1, > init_arg => '-my_attr', >); > >__PACKAGE__->meta->make_immutable; > > >package main; > >my $inst1 = MyClass1->new(my_attr => 'The String'); >my $packed1 = $inst1->pack; >my $unpacked1 = MyClass1->unpack($packed1); # this works > >my $inst2 = MyClass2->new(-my_attr => 'The String'); >my $packed2 = $inst2->pack; >my $unpacked2 = MyClass2->unpack($packed2); # this fails with a ... > # ... Attribute (my_attr) is required at ... > > >Update: further investigation indicates that the issue is that init_arg >is not taken into account when packing. Hence, even a non-required >attribute using a custom init_arg is not correctly restored after >unpacking. See this additional test case: > >package MyClass3; > >with Storage; > >has 'my_attr' => ( > is => 'ro', > isa => 'Str', > init_arg => '-my_attr', >); > ># in main... > >my $inst3 = MyClass3->new(-my_attr => 'The String'); >my $packed3 = $inst3->pack; >my $unpacked3 = MyClass3->unpack($packed3); # this seems to work ... >say $unpacked3->my_attr; # ... but my_attr stays undef > > >Thanks a lot for your help, >Denis > > > >
-- Denis BAURAIN, Ph.D. Unit of Animal Genomics GIGA-R & Faculty of Veterinary Medicine University of Liège GIGA Tower (B34 - 1° floor) CHU, Sart Tilman Campus 1 Avenue de l'Hopital 4000-Liège, Belgium
Content-Type: multipart/mixed; boundary="==_20110228093413.883420042-2_=="
Content-Length: 0
content-type: application/octet-stream; name="init_arg.diff"; x-mac-creator="00000000"; x-mac-type="00000000"
Content-Disposition: attachment
Content-Transfer-Encoding: base64
Content-Length: 789
Download init_arg.diff
text/x-diff 789b

Message body is not shown because sender requested not to inline it.

content-type: application/octet-stream; name="080_basic_initarg.t"; x-mac-creator="00000000"; x-mac-type="00000000"
Content-Disposition: attachment
Content-Transfer-Encoding: base64
Content-Length: 2241
Download 080_basic_initarg.t
text/x-perl 2.1k

Message body is not shown because sender requested not to inline it.

MIME-Version: 1.0
Subject: issue with init_arg attribute option: candidate patch
In-Reply-To: <20110215075646.72180570 [...] smtp.ulg.ac.be>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
References: <20110215075646.72180570 [...] smtp.ulg.ac.be>
Content-Type: multipart/mixed; boundary="----------=_1299771200-26834-162"
Message-ID: <rt-3.8.HEAD-26834-1299771200-314.65733-0-0 [...] rt.cpan.org>
X-RT-Original-Encoding: utf-8
Content-Length: 0
Content-Disposition: inline
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 660
Download (untitled) / with headers
text/plain 660b
Hi, I have written a more extensive patch to handle unpacking of attributes with custom init_arg options (either defined or not). I attach the patch file as well as a comprehensive test file. The rationale is given in the module POD (maybe not the best place). Though I am not sure about performance penalty, nor whether it is advisable to implement this solution "as is" in MooseX::Storage::Basic, my opinion is that the provided functionality is useful (at least to me :-) I apologize if I do not follow best practices to submit this patch. I would be happy to follow another route if someone points me in the right direction. Hope this helps, Denis
Subject: 080_basic_initarg.t
MIME-Version: 1.0
Content-Type: application/x-troff; name="080_basic_initarg.t"
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline; filename="080_basic_initarg.t"
Content-Transfer-Encoding: base64
Content-Length: 4500
Download 080_basic_initarg.t
text/x-perl 4.3k
#!/usr/bin/perl use strict; use warnings; use Test::More tests => 36; BEGIN { use_ok('MooseX::Storage'); } { package Foo; use Moose; use MooseX::Storage; with Storage; has 'number' => ( is => 'ro', isa => 'Int', init_arg => '-number' ); has 'string' => ( is => 'ro', isa => 'Str', init_arg => '-string' ); has 'boolean' => ( is => 'ro', isa => 'Bool', init_arg => '-boolean' ); has 'float' => ( is => 'ro', isa => 'Num', init_arg => '-float' ); has 'array' => ( is => 'ro', isa => 'ArrayRef', init_arg => '-array' ); has 'hash' => ( is => 'ro', isa => 'HashRef', init_arg => '-hash' ); has 'object' => ( is => 'ro', isa => 'Foo', init_arg => '-object' ); has 'union' => ( is => 'ro', isa => 'ArrayRef|Str', init_arg => '-union' ); has 'union2' => ( is => 'ro', isa => 'ArrayRef|Str', init_arg => '-union2' ); has 'requ_str' => ( is => 'ro', isa => 'Str', init_arg => '-requ_str', required => 1 ); has '_priv_str' => ( is => 'ro', isa => 'Str', init_arg => undef, writer => '_set_priv_str' ); has '_auto_str' => ( is => 'ro', isa => 'Str', init_arg => undef, builder => '_build_auto_str', lazy => 1 ); has '_dual_str' => ( is => 'ro', isa => 'Str', init_arg => undef, writer => '_set_dual_str', builder => '_build_dual_str', lazy => 1 ); has '_skip_str' => ( is => 'ro', isa => 'Str', init_arg => undef ); sub _build_auto_str { return 'auto string'; } sub _build_dual_str { return 'dual string'; } } { package Bar; use Moose; use MooseX::Storage; extends 'Foo'; has 'child_str' => ( is => 'ro', isa => 'Str', init_arg => '-child_str' ); } { my $packed = { __CLASS__ => 'Foo', number => 10, string => 'foo', boolean => 1, float => 10.5, array => [ 1 .. 10 ], hash => { map { $_ => undef } ( 1 .. 10 ) }, object => { __CLASS__ => 'Foo', number => 2, requ_str => 'inner required string', }, union => [ 1, 2, 3 ], union2 => 'A String', # exercise interaction between init_arg and other attr properties requ_str => 'required string', _priv_str => 'private string', _auto_str => 'overriden by builder', _dual_str => 'restored dual string', _skip_str => 'private string without writer', }; my $foo = Foo->unpack($packed); isa_ok( $foo, 'Foo' ); do_common_checks($foo); my $bar = Bar->unpack( { # exercise inherited attributes %$packed, child_str => 'a string in child class' } ); isa_ok( $bar, 'Bar' ); do_common_checks($bar); is( $bar->child_str, 'a string in child class', '... got the right string (child)' ); } sub do_common_checks { my $foo = shift; is( $foo->number, 10, '... got the right number' ); is( $foo->string, 'foo', '... got the right string' ); ok( $foo->boolean, '... got the right boolean' ); is( $foo->float, 10.5, '... got the right float' ); is_deeply( $foo->array, [ 1 .. 10 ], '... got the right array' ); is_deeply( $foo->hash, { map { $_ => undef } ( 1 .. 10 ) }, '... got the right hash' ); isa_ok( $foo->object, 'Foo' ); is( $foo->object->number, 2, '... got the right number (in the embedded object)' ); is( $foo->object->requ_str, 'inner required string', '... got the right string (required in the embedded object)' ); is_deeply( $foo->union, [ 1 .. 3 ], '... got the right array (in the union)' ); is( $foo->union2, 'A String', '... got the right string (in the union)' ); is( $foo->requ_str, 'required string', '... got the right string (required)' ); is( $foo->_priv_str, 'private string', '... got the right string (private)' ); is( $foo->_auto_str, 'auto string', '... got the right string (builder)' ); is( $foo->_dual_str, 'restored dual string', '... got the right string (writer/builder)' ); is( $foo->_skip_str, undef, '... skipped private string without writer' ); }
MIME-Version: 1.0
Subject: init_arg.diff
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: application/octet-stream; name="init_arg.diff"
Content-Disposition: inline; filename="init_arg.diff"
Content-Transfer-Encoding: base64
Content-Length: 4927
Download init_arg.diff
text/x-diff 4.8k
--- MooseX-Storage-0.29/lib/MooseX/Storage/Basic.pm 2010-11-17 14:51:35.000000000 +0100 +++ MooseX-Storage-0.29f/lib/MooseX/Storage/Basic.pm 2011-03-10 17:56:46.000000000 +0100 @@ -51,8 +51,35 @@ sub _storage_construct_instance { my ($class, $args, $opts) = @_; my %i = defined $opts->{'inject'} ? %{ $opts->{'inject'} } : (); - - $class->new( %$args, %i ); + + my %priv_attrs; + for my $arg (keys %$args) { + + # collect private attributes with undef init_arg but defined writer + # these will be restored after instance construction + my $init_arg = $class->meta->find_attribute_by_name($arg)->init_arg; + if (!defined $init_arg) { + my $writer = $class->meta->find_attribute_by_name($arg)->writer; + if (defined $writer) { + $priv_attrs{$writer} = $args->{$arg}; + delete $args->{$arg}; + } + } + + # replace attribute name by init_arg if defined + # this allows call to constructor below to work as expected + elsif ($init_arg ne $arg) { + $args->{$init_arg} = $args->{$arg}; + delete $args->{$arg}; + } + } + + # create new instance using public attributes ... + # and populate private attributes with corresponding writers + my $instance = $class->new( %$args, %i ); + $instance->$_( $priv_attrs{$_} ) for keys %priv_attrs; + + return $instance; } no Moose::Role; @@ -119,6 +146,71 @@ Providing the C<insert> argument let's you supply additional arguments to the class' C<new> function, or override ones from the serialized data. +Attributes with custom constructor parameters (specified with the C<init_arg> +option) are packed as other attributes but lead to more complex unpacking. The +current behavior is as follows. + +If C<init_arg> has a defined value (e.g., C<-start> for attribute C<start>), the +custom constructor parameter (C<-start>) is used when restoring the attribute +(even though it is still packed as C<start>). This is needed to ensure API +consistency when extending a large module distribution making use of custom +constructor parameters (e.g., BioPerl classes). + +If C<init_arg> is explicitly set to C<undef>, the unpacking behavior depends on +the value of the C<writer> option. When C<writer> is defined, the corresponding +method is used to restore the attribute after object creation (thus overriding +any attribute value possibly provided by C<default> or C<builder> options). This +is especially useful for lazy attributes that are very expensive to build and +thus prime candidates for serialization. Here is a use case that illustrates +this point. + +Consider a C<Store> class providing high-level access to an array of hashes. The +private C<_store> attribute of this class could be defined as: + + has '_store' => ( + is => 'ro', + isa => 'ArrayRef[HashRef[Num]]|ArrayRef', # union required for + init_arg => undef, # unpacking empty slots + default => sub { [] }, + writer => '_set_store', # for unserialization + ); + +Now, let's say that populating a C<Store> object requires processing hundreds of +large files. Ultimately, input data is stored in the object using methods like: + + sub add_coverage { + my $self = shift; + my $bin = shift; + my $tag = shift; + return ${$self->_store}[$bin]{$tag} += shift; # weight + } + +This is typically a preprocessing step that you only want to do once. In +contrast, accessing the C<Store> object might be needed several times (for +example with different sets of parameters for statistical analysis of the store +content). Thanks to the behavior described above, it is very easy to serialize +an object with such a private C<_store>. While the C<_store> attribute is +properly initialized on object creation (owing to its C<default> option), it is +also correctly restored on unpacking (due to its C<writer> option). Further, +this approach does not violate encapsulation of the private attribute since it +restores its value through explicitly defined methods. + +Finally, for attributes with C<init_arg> set to C<undef> but without a defined +C<writer>, attribute values are not restored at all (even if actually packed). +This is the desired behavior for lazy attributes that are cheap to compute from +other attributes (generally with a C<builder> option). In this case, it is +probably better (i.e., clearer) to explicitly add the 'DoNotSerialize' trait, as +below: + + has 'bin_width' => ( + is => 'ro', + isa => 'Num', + init_arg => undef, + lazy => 1, + builder => '_build_bin_width', + traits => [ 'DoNotSerialize' ], # will be recomputed on unpacking + ); + =back =head2 Introspection


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.