| Subject: | CDBICompat/AccessorMapping.pm |
Hi!
The attached patch fixes two problems with CDBICompat::AccessorMapping.
1) mk_group_accessors() isn't prepared to handle arguments in the [
$field, $name ] format - in our installation this caused breakage in the
upgrade from 0.07006 to 0.0801.
2) The logic related to ro/wo accessors doesn't seem quite right. If my
"customer" class defines accessor_name() but not mutator_name(); and if
$customer->accessor_name transforms the field-name "customer_age" into
the method-name "age"; then I should get a r/w accessor called age().
But in this case CDBICompat is creating a read-only accessor called
age() and a write-only mutator called customer_age().
CDBI doesn't test for #2, so I included a standalone test to demonstrate
the problem.
Thanks,
Steve
| Subject: | cdbi-accessors.t |
use strict;
use Test::More;
$| = 1;
BEGIN {
eval "use DBIx::Class::CDBICompat;";
if ($@) {
plan (skip_all => 'Class::Trigger and DBIx::ContextualFetch required');
next;
}
eval "use DBD::SQLite";
plan $@ ? (skip_all => 'needs DBD::SQLite for testing') : (tests => 3);
}
INIT {
use lib 't/testlib';
require Film;
}
sub Film::accessor_name {
my ($class, $col) = @_;
return "sheep" if $col =~ /NumExplodingSheep/i;
return $col;
}
# According to CDBI -->
#
# mutator name will be the same as accessor name unless you override it.
#
# If both the accessor and mutator are to have the same method name,
# (which will always be true unless you override mutator_name_for), a
# read-write method is constructed for it.
#
# But CDBICompat's current behavior in this case is to create a read-only
# accessor with the "overridden" name, and a write-only mutator with the
# "original" name.
my $film = Film->create({sheep => 1});
eval { $film->sheep(2) };
is $@, '', 'Generated accessor is read-write';
eval { $film->NumExplodingSheep };
like $@, qr/Can't locate object method/, 'Original column name not a method';
unlike $@, qr/cannot access the value/, '... and certainly not a write-only mutator';
$film->discard_changes;
| Subject: | cdbi-accessors.patch |
--- lib/DBIx/Class/CDBICompat/AccessorMapping.pm.orig 2007-08-11 17:07:57.000000000 -0400
+++ lib/DBIx/Class/CDBICompat/AccessorMapping.pm 2008-06-18 16:01:03.000000000 -0400
@@ -9,19 +9,27 @@
unless ($class->can('accessor_name') || $class->can('mutator_name')) {
return $class->next::method($group => @cols);
}
+
+ # Per the CDBI manpage:
+ #
+ # If you override the mutator name, then the accessor method will be
+ # enforced as read-only, and the mutator as write-only.
+
foreach my $col (@cols) {
- my $ro_meth = ($class->can('accessor_name')
- ? $class->accessor_name($col)
- : $col);
- my $wo_meth = ($class->can('mutator_name')
- ? $class->mutator_name($col)
- : $col);
- #warn "$col $ro_meth $wo_meth";
- if ($ro_meth eq $wo_meth) {
- $class->next::method($group => [ $ro_meth => $col ]);
+ my ($name, $field) = ref $col ? @$col : ($col, $col);
+
+ my $accessor = $class->can('accessor_name')
+ ? $class->accessor_name($field)
+ : $name;
+ my $mutator = $class->can('mutator_name')
+ ? $class->mutator_name($field)
+ : $accessor;
+
+ if ($accessor eq $mutator) {
+ $class->next::method($group => [ $accessor => $field ]);
} else {
- $class->mk_group_ro_accessors($group => [ $ro_meth => $col ]);
- $class->mk_group_wo_accessors($group => [ $wo_meth => $col ]);
+ $class->mk_group_ro_accessors($group => [ $accessor => $field ]);
+ $class->mk_group_wo_accessors($group => [ $mutator => $field ]);
}
}
}