Re: Warning if primary key is changed (was Re: Ignorance prevention)

[prev] [thread] [next] [Date index for 2004/10/08]

--eVzOFob/8UvintSX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Oct 08, 2004 at 02:30:37AM +0100, Tony Bowden wrote:
> On Thu, Oct 07, 2004 at 06:40:32PM -0400, William McKee wrote:
> > I'm not sure where the method generation code is located 
> 
> _mk_column_accessors(). It goes over all the columns and calls
> make_accessor (or make_ro_accessor) in Class::Accessor for us. 

Thanks, now I see it.


> It should just be a matter of adding an extra conditional to the
> pre-existing check for whether the method should be read only.

OK, I see where you are looping through the two types of accessor--ro
and wo. After the $acc_type value is assigned, I'm overriding it to
make_ro_accessor if we're dealing with the primary key. This may cause
problems with the mutator but even if you're overriding the
mutator_name(), it still should not allow writing to the primary key so
I think it's OK. Thoughts?

One thing that I had to do was check that $class->primary_key was
defined. If I didn't do this t/99-misc.t would fail. I'm not sure why
that's the case as no tests were able to run. I suppose it has to do
with how the CDBI subclass was being constructed. Good thing that test
was in there!

I realized that it is still possible to set the primary key via the
set() command and added a test that proves this. We have a trade-off
between performance and some error checking vs. better error checking
and decreased performance. I think I'm in favor of the former at the
moment. I've updated the CAVEATS to reflect these changes.


> [1] See - I can care about performance too :)

;)

I've attached the updated test script and the updated DBI.pm along with
doc updates.


Thanks,
William

        -- 
        Knowmad Services Inc.
http://www.knowmad.com

--eVzOFob/8UvintSX
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="DBI.pm.diff"

--- ../Class-DBI-0.96/lib/Class/DBI.pm	2004-04-30 03:22:12.000000000 -0400
+++ lib/Class/DBI.pm	2004-10-08 15:54:41.000000000 -0400
@@ -390,6 +390,9 @@
 		foreach my $type (keys %method) {
 			my $name     = $method{$type};
 			my $acc_type = $both ? "make_accessor" : "make_${type}_accessor";
+			if ( defined $class->primary_column and $obj->name eq $class->primary_column->name ) {
+				$acc_type = "make_ro_accessor";
+			}
 			my $accessor = $class->$acc_type($obj->name_lc);
 			$class->_make_method($_, $accessor) for ($name, "_${name}_accessor");
 		}
@@ -2014,7 +2017,8 @@
 	}
 
 If you override the mutator_name, then the accessor method will be
-enforced as read-only, and the mutator as write-only.
+enforced as read-only, and the mutator as write-only (except for the primary
+key which will always be read-only).
 
 =head2 update vs auto update
 
@@ -2926,8 +2930,10 @@
 
 =head2 Don't change or inflate the value of your primary columns
 
-Altering your primary key column currently causes Bad Things to happen.
-I should really protect against this.
+Altering your primary key column currently causes Bad Things to happen. If you
+try to change the primary key via the accessor, an exception will be thrown.
+It is still possible to change the primary key if you use the set() method.
+You have been warned!
 
 =head1 SUPPORTED DATABASES
 

--eVzOFob/8UvintSX
Content-Type: application/x-troff
Content-Disposition: attachment; filename="02-Film.t"
Content-Transfer-Encoding: quoted-printable

--- ../Class-DBI-0.96/t/02-Film.t	2004-04-25 11:33:36.000000000 -0400=0A+++=
 t/02-Film.t	2004-10-08 15:50:29.000000000 -0400=0A@@ -4,7 +4,7 @@=0A =0A B=
EGIN {=0A 	eval "use DBD::SQLite";=0A-	plan $@ ? (skip_all =3D> 'needs DBD:=
:SQLite for testing') : (tests =3D> 90);=0A+	plan $@ ? (skip_all =3D> 'need=
s DBD::SQLite for testing') : (tests =3D> 94);=0A }=0A =0A INIT {=0A@@ -298=
,6 +298,18 @@=0A 	is "$blrunner", "Bladerunner:R", "Provide stringify_self(=
)";=0A }=0A =0A+# Changing primary key causes error=0A+is $blrunner->id, $b=
lrunner->title, "Verify id";=0A+eval {=0A+  $blrunner->title('ScissorHands'=
);=0A+};=0A+like $@, qr/cannot alter the value/, "Changing the primary key =
via the accessor throws an exception";=0A+eval {=0A+  $blrunner->set(title =
=3D> 'Willy Wonka and the Chocolate Factory');=0A+};=0A+is $@, '', "Changin=
g the primary key via set() does not throw an exception";=0A+is $blrunner->=
title, 'Willy Wonka and the Chocolate Factory', "The primary key can be cha=
nged by set()";=0A+=0A {=0A 	{=0A 		ok my $byebye =3D DeletingFilm->create(=
{=0A
--eVzOFob/8UvintSX--