On Thursday 16 March 2006 00:39, Geoffrey Young wrote: > >>>in order for $o to maintain it's internal state. is that right? if so, > >>> I don't like that very much. > > > > It's not right. You are confusing scalars with references in that > > example. > > ook, my bad - I thought the proposed "solution" was to always dereference > pnotes behind the scenes, making the additional storage a requirement. > > but, just in case I'm not the only one confused, I'd propose a path like > this... > > o create some (many?) additional pnotes tests that exercise the different > ways we think people will be using pnotes in almost all normal > circumstances we can think of. digging through CPAN might be a good idea. > > o offer that any fix shouldn't break the tests
Here are some tests that cover your concerns so far:
package TestPn::pn;
use strict;
use warnings FATAL => 'all';
use Apache::Test;
use Apache::TestUtil;
use Apache2::RequestUtil;
use Apache2::Const -compile => 'OK';
{
package _O;
sub new {bless {}=>__PACKAGE__}
sub member {$_[0]->{x}=$_[1] if @_>1; $_[0]->{x}}
}
sub handler {
my $r = shift;
Apache::Test::test_pm_refresh();
plan $r, tests => 20;
my $x;
# pnotes as hash ref
# this 1st set of tests are all ok, because $r->pnotes returns a normal
# hash ref
$x=1;
$r->pnotes->{x}=$x;
$x++;
ok t_cmp $r->pnotes('x'), 1, q{hash ref1};
ok t_cmp $r->pnotes->{x}, 1, q{hash ref2};
delete $r->pnotes->{x};
# pnotes method interface
# these two fail
$x=1;
$r->pnotes(x=>$x);
$x++;
ok t_cmp $r->pnotes('x'), 1, q{method1};
ok t_cmp $r->pnotes->{x}, 1, q{method2};
delete $r->pnotes->{x};
# pnotes as hash: storing and retrieving objects
my $o1=_O->new;
$o1->member(1);
my $o2=_O->new;
$o2->member(2);
# check if the objects work properly
ok t_cmp $o1->member, 1, q{o1 setup};
ok t_cmp $o2->member, 2, q{o2 setup};
# these tests succeed. they show normal object behavior.
$r->pnotes->{o}=$o1;
$o1->member(10);
ok t_cmp $r->pnotes('o')->member, 10, q{hash ref obj 1};
ok t_cmp $r->pnotes->{o}->member, 10, q{hash ref obj 2};
$r->pnotes('o')->member(100);
ok t_cmp $r->pnotes('o')->member, 100, q{hash ref obj 3};
ok t_cmp $r->pnotes->{o}->member, 100, q{hash ref obj 4};
# the following two succeed only because the hash ref interface
# was used to assing $r->pnotes->{o}.
$o1=$o2;
ok t_cmp $r->pnotes('o')->member, 100, q{hash ref obj 5};
ok t_cmp $r->pnotes->{o}->member, 100, q{hash ref obj 6};
delete $r->pnotes->{o};
# pnotes method interface: storing and retrieving objects
$o1=_O->new;
$o1->member(1);
$o2=_O->new;
$o2->member(2);
ok t_cmp $o1->member, 1, q{o1 setup};
ok t_cmp $o2->member, 2, q{o2 setup};
$r->pnotes(o=>$o1);
$o1->member(10);
ok t_cmp $r->pnotes('o')->member, 10, q{method obj 1};
ok t_cmp $r->pnotes->{o}->member, 10, q{method obj 2};
$r->pnotes('o')->member(100);
ok t_cmp $r->pnotes('o')->member, 100, q{method obj 3};
ok t_cmp $r->pnotes->{o}->member, 100, q{method obj 4};
# these two fail, because the assignment $o1=$o2 modifies $r->pnotes->{o}.
$o1=$o2;
ok t_cmp $r->pnotes('o')->member, 100, q{method obj 5};
ok t_cmp $r->pnotes->{o}->member, 100, q{method obj 6};
delete $r->pnotes->{o};
Apache2::Const::OK;
}
1;
The patch is IMO really simple:
In xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h and
xs/Apache2/RequestUtil/Apache2__RequestUtil.h
if (val) {
retval = *hv_store(ccfg->pnotes, k, len,
- SvREFCNT_inc(val), 0);
+ newSVsv(val), 0);
}
else if (hv_exists(ccfg->pnotes, k, len)) {
retval = *hv_fetch(ccfg->pnotes, k, len, FALSE);
The attached patch was made a few days ago. It applies to r384596 with the
connection-pnotes patch applied before. All tests succeed.
Torsten
diff -Naur r384596-cnotes/t/modperl/pnotes.t pnotes/t/modperl/pnotes.t
--- r384596-cnotes/t/modperl/pnotes.t 2006-03-10 21:23:28.465092006 +0100
+++ pnotes/t/modperl/pnotes.t 2006-03-10 21:32:55.647505579 +0100
@@ -10,7 +10,7 @@
t_debug("connecting to $url");
-plan tests => (22 * 3);
+plan tests => (24 * 3);
# first with keepalives
Apache::TestRequest::user_agent(reset => 1, keep_alive => 1);
diff -Naur r384596-cnotes/t/response/TestModperl/pnotes.pm pnotes/t/response/TestModperl/pnotes.pm
--- r384596-cnotes/t/response/TestModperl/pnotes.pm 2006-03-10 21:23:28.462092410 +0100
+++ pnotes/t/response/TestModperl/pnotes.pm 2006-03-10 21:33:07.117987774 +0100
@@ -17,8 +17,8 @@
# make it ok to call ok() here while plan()ing elsewhere
Apache::Test::init_test_pm($r);
- $Test::ntest = 1 + (22 * ($r->args - 1));
- $Test::planned = 22;
+ $Test::ntest = 1 + (24 * ($r->args - 1));
+ $Test::planned = 24;
my $c = $r->connection;
@@ -80,6 +80,11 @@
ok t_cmp($o->pnotes('pnotes_foo'), undef,
"deleted $type contents");
ok !exists $o->pnotes->{'pnotes_foo'};
+
+ my $x=1;
+ $o->pnotes(x=>$x);
+ $x++;
+ ok $o->pnotes('x'), 1, 'pass parameters by value'
}
# set pnotes so we can test unset on later connections
diff -Naur r384596-cnotes/xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h pnotes/xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h
--- r384596-cnotes/xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h 2006-03-10 21:23:28.464092141 +0100
+++ pnotes/xs/Apache2/ConnectionUtil/Apache2__ConnectionUtil.h 2006-03-10 21:22:39.106742419 +0100
@@ -39,7 +39,7 @@
if (val) {
retval = *hv_store(ccfg->pnotes, k, len,
- SvREFCNT_inc(val), 0);
+ newSVsv(val), 0);
}
else if (hv_exists(ccfg->pnotes, k, len)) {
retval = *hv_fetch(ccfg->pnotes, k, len, FALSE);
diff -Naur r384596-cnotes/xs/Apache2/RequestUtil/Apache2__RequestUtil.h pnotes/xs/Apache2/RequestUtil/Apache2__RequestUtil.h
--- r384596-cnotes/xs/Apache2/RequestUtil/Apache2__RequestUtil.h 2006-03-10 21:09:59.082272981 +0100
+++ pnotes/xs/Apache2/RequestUtil/Apache2__RequestUtil.h 2006-03-10 21:21:56.121535121 +0100
@@ -227,7 +227,7 @@
if (val) {
retval = *hv_store(rcfg->pnotes, k, len,
- SvREFCNT_inc(val), 0);
+ newSVsv(val), 0);
}
else if (hv_exists(rcfg->pnotes, k, len)) {
retval = *hv_fetch(rcfg->pnotes, k, len, FALSE);
pgpnay7aqg0zk.pgp
Description: PGP signature
