[Bast-commits] r3288 - in branches/DBIx-Class-current:
lib/DBIx/Class/Storage t t/lib t/lib/DBICTest
blblack at dev.catalyst.perl.org
blblack at dev.catalyst.perl.org
Fri May 11 00:46:28 GMT 2007
Author: blblack
Date: 2007-05-11 00:46:23 +0100 (Fri, 11 May 2007)
New Revision: 3288
Modified:
branches/DBIx-Class-current/lib/DBIx/Class/Storage/DBI.pm
branches/DBIx-Class-current/t/19quotes_newstyle.t
branches/DBIx-Class-current/t/94versioning.t
branches/DBIx-Class-current/t/bindtype_columns.t
branches/DBIx-Class-current/t/lib/DBICTest.pm
branches/DBIx-Class-current/t/lib/DBICTest/Plain.pm
Log:
Got rid of effectively dead dbh_do code in the txn_{begin|end|rollback} funcs
Reworked the AutoCommit/transaction_depth stuff so that transaction_depth is always 1 or higher with AutoCommit off
Doc updates to recommend AutoCommit => 1 / txn_do
Warn if the user doesn't explicitly set AutoCommit
Added AutoCommit => 1 to some tests that were triggering the above warning
Modified: branches/DBIx-Class-current/lib/DBIx/Class/Storage/DBI.pm
===================================================================
--- branches/DBIx-Class-current/lib/DBIx/Class/Storage/DBI.pm 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/lib/DBIx/Class/Storage/DBI.pm 2007-05-10 23:46:23 UTC (rev 3288)
@@ -331,7 +331,12 @@
The arrayref can either contain the same set of arguments one would
normally pass to L<DBI/connect>, or a lone code reference which returns
-a connected database handle.
+a connected database handle. Please note that the L<DBI> docs
+recommend that you always explicitly set C<AutoCommit> to either
+C<0> or C<1>. L<DBIx::Class> further recommends that it be set
+to C<1>, and that you perform transactions via our L</txn_do>
+method. L<DBIx::Class> will emit a warning if you fail to explicitly
+set C<AutoCommit> one way or the other. See below for more details.
In either case, if the final argument in your connect_info happens
to be a hashref, C<connect_info> will look there for several
@@ -390,6 +395,21 @@
force this setting for you anyways. Setting HandleError to anything
other than simple exception object wrapper might cause problems too.
+Another Important Note:
+
+DBIC can do some wonderful magic with handling exceptions,
+disconnections, and transactions when you use C<AutoCommit => 1>
+combined with C<txn_do> for transaction support.
+
+If you set C<AutoCommit => 0> in your connect info, then you are always
+in an assumed transaction between commits, and you're telling us you'd
+like to manage that manually. A lot of DBIC's magic protections
+go away. We can't protect you from exceptions due to database
+disconnects because we don't know anything about how to restart your
+transactions. You're on your own for handling all sorts of exceptional
+cases if you choose the C<AutoCommit => 0> path, just as you would
+be with raw DBI.
+
Examples:
# Simple SQLite connection
@@ -404,7 +424,7 @@
'dbi:Pg:dbname=foo',
'postgres',
'my_pg_password',
- { AutoCommit => 0 },
+ { AutoCommit => 1 },
{ quote_char => q{"}, name_sep => q{.} },
]
);
@@ -415,7 +435,7 @@
'dbi:Pg:dbname=foo',
'postgres',
'my_pg_password',
- { AutoCommit => 0, quote_char => q{"}, name_sep => q{.} },
+ { AutoCommit => 1, quote_char => q{"}, name_sep => q{.} },
]
);
@@ -462,6 +482,18 @@
pop(@$info) if !keys %$last_info;
}
+ # Now check the (possibly new) final argument for AutoCommit,
+ # but not in the coderef case, obviously.
+ if(ref $info->[0] ne 'CODE') {
+ $last_info = $info->[3];
+
+ warn "You *really* should explicitly set AutoCommit "
+ . "(preferably to 1) in your db connect info"
+ if !$last_info
+ || ref $last_info ne 'HASH'
+ || !defined $last_info->{AutoCommit};
+ }
+
$self->_connect_info($info);
}
@@ -688,6 +720,10 @@
my @info = @{$self->_connect_info || []};
$self->_dbh($self->_connect(@info));
+ # Always set the transaction depth on connect, since
+ # there is no transaction in progress by definition
+ $self->{transaction_depth} = $self->_dbh->{AutoCommit} ? 0 : 1;
+
if(ref $self eq 'DBIx::Class::Storage::DBI') {
my $driver = $self->_dbh->{Driver}->{Name};
if ($self->load_optional_class("DBIx::Class::Storage::DBI::${driver}")) {
@@ -741,75 +777,61 @@
$dbh;
}
-sub _dbh_txn_begin {
- my ($self, $dbh) = @_;
- if ($dbh->{AutoCommit}) {
+
+sub txn_begin {
+ my $self = shift;
+ if($self->{transaction_depth}++ == 0) {
$self->debugobj->txn_begin()
- if ($self->debug);
- $dbh->begin_work;
+ if $self->debug;
+ # this isn't ->_dbh-> because
+ # we should reconnect on begin_work
+ # for AutoCommit users
+ $self->dbh->begin_work;
}
}
-sub txn_begin {
+sub txn_commit {
my $self = shift;
- $self->dbh_do($self->can('_dbh_txn_begin'))
- if $self->{transaction_depth}++ == 0;
-}
-
-sub _dbh_txn_commit {
- my ($self, $dbh) = @_;
- if ($self->{transaction_depth} == 0) {
- unless ($dbh->{AutoCommit}) {
- $self->debugobj->txn_commit()
- if ($self->debug);
- $dbh->commit;
- }
+ if ($self->{transaction_depth} == 1) {
+ my $dbh = $self->_dbh;
+ $self->debugobj->txn_commit()
+ if ($self->debug);
+ $dbh->commit;
+ $self->{transaction_depth} = 0
+ if $dbh->{AutoCommit};
}
- else {
- if (--$self->{transaction_depth} == 0) {
- $self->debugobj->txn_commit()
- if ($self->debug);
- $dbh->commit;
- }
+ elsif($self->{transaction_depth} > 1) {
+ $self->{transaction_depth}--
}
}
-sub txn_commit {
+sub txn_rollback {
my $self = shift;
- $self->dbh_do($self->can('_dbh_txn_commit'));
-}
-
-sub _dbh_txn_rollback {
- my ($self, $dbh) = @_;
- if ($self->{transaction_depth} == 0) {
- unless ($dbh->{AutoCommit}) {
+ my $dbh = $self->_dbh;
+ my $autocommit;
+ eval {
+ $autocommit = $dbh->{AutoCommit};
+ if ($self->{transaction_depth} == 1) {
$self->debugobj->txn_rollback()
if ($self->debug);
$dbh->rollback;
+ $self->{transaction_depth} = 0
+ if $autocommit;
}
- }
- else {
- if (--$self->{transaction_depth} == 0) {
- $self->debugobj->txn_rollback()
- if ($self->debug);
- $dbh->rollback;
+ elsif($self->{transaction_depth} > 1) {
+ $self->{transaction_depth}--;
}
else {
die DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION->new;
}
- }
-}
-
-sub txn_rollback {
- my $self = shift;
-
- eval { $self->dbh_do($self->can('_dbh_txn_rollback')) };
+ };
if ($@) {
my $error = $@;
my $exception_class = "DBIx::Class::Storage::NESTED_ROLLBACK_EXCEPTION";
$error =~ /$exception_class/ and $self->throw_exception($error);
- $self->{transaction_depth} = 0; # ensure that a failed rollback
- $self->throw_exception($error); # resets the transaction depth
+ # ensure that a failed rollback resets the transaction depth
+ $self->{transaction_depth} = $autocommit ? 0 : 1;
+ $self->throw_exception($error);
}
}
Modified: branches/DBIx-Class-current/t/19quotes_newstyle.t
===================================================================
--- branches/DBIx-Class-current/t/19quotes_newstyle.t 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/t/19quotes_newstyle.t 2007-05-10 23:46:23 UTC (rev 3288)
@@ -22,7 +22,13 @@
diag('Testing against ' . join(' ', map { $schema->storage->dbh->get_info($_) } qw/17 18/));
my $dsn = $schema->storage->connect_info->[0];
-$schema->connection($dsn, { quote_char => '`', name_sep => '.' });
+$schema->connection(
+ $dsn,
+ undef,
+ undef,
+ { AutoCommit => 1 },
+ { quote_char => '`', name_sep => '.' },
+);
my $sql = '';
$schema->storage->debugcb(sub { $sql = $_[1] });
@@ -47,7 +53,12 @@
eval { $rs->first };
like($sql, qr/ORDER BY \Q${order}\E/, 'did not quote ORDER BY with scalarref');
-$schema->connection($dsn, { quote_char => [qw/[ ]/], name_sep => '.' });
+$schema->connection(
+ $dsn,
+ undef,
+ undef,
+ { AutoCommit => 1, quote_char => [qw/[ ]/], name_sep => '.' }
+);
$schema->storage->debugcb(sub { $sql = $_[1] });
$schema->storage->debug(1);
@@ -62,7 +73,12 @@
order => '12'
);
-$schema->connection($dsn, { quote_char => '`', name_sep => '.' });
+$schema->connection(
+ $dsn,
+ undef,
+ undef,
+ { AutoCommit => 1, quote_char => '`', name_sep => '.' }
+);
is($schema->storage->sql_maker->update('group', \%data), 'UPDATE `group` SET `name` = ?, `order` = ?', 'quoted table names for UPDATE');
Modified: branches/DBIx-Class-current/t/94versioning.t
===================================================================
--- branches/DBIx-Class-current/t/94versioning.t 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/t/94versioning.t 2007-05-10 23:46:23 UTC (rev 3288)
@@ -21,7 +21,12 @@
mkdir("t/var") unless -d "t/var";
unlink('t/var/DBICVersion-Schema-1.0-SQLite.sql');
-my $schema_orig = DBICVersion::Schema->connect("dbi:SQLite:$db_file");
+my $schema_orig = DBICVersion::Schema->connect(
+ "dbi:SQLite:$db_file",
+ undef,
+ undef,
+ { AutoCommit => 1 },
+);
# $schema->storage->ensure_connected();
is($schema_orig->ddl_filename('SQLite', 't/var', '1.0'), File::Spec->catfile('t', 'var', 'DBICVersion-Schema-1.0-SQLite.sql'), 'Filename creation working');
@@ -35,7 +40,12 @@
is($schema_orig->exists($tvrs), 1, 'Created schema from DDL file');
eval "use DBICVersionNew";
-my $schema_new = DBICVersion::Schema->connect("dbi:SQLite:$db_file");
+my $schema_new = DBICVersion::Schema->connect(
+ "dbi:SQLite:$db_file",
+ undef,
+ undef,
+ { AutoCommit => 1 },
+);
unlink('t/var/DBICVersion-Schema-2.0-SQLite.sql');
unlink('t/var/DBICVersion-Schema-1.0-2.0-SQLite.sql');
@@ -43,7 +53,12 @@
ok(-f 't/var/DBICVersion-Schema-1.0-2.0-SQLite.sql', 'Created DDL upgrade file');
## create new to pick up filedata for upgrade files we just made (on_connect)
-my $schema_upgrade = DBICVersion::Schema->connect("dbi:SQLite:$db_file");
+my $schema_upgrade = DBICVersion::Schema->connect(
+ "dbi:SQLite:$db_file",
+ undef,
+ undef,
+ { AutoCommit => 1 },
+);
## do this here or let Versioned.pm do it?
$schema_upgrade->upgrade();
Modified: branches/DBIx-Class-current/t/bindtype_columns.t
===================================================================
--- branches/DBIx-Class-current/t/bindtype_columns.t 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/t/bindtype_columns.t 2007-05-10 23:46:23 UTC (rev 3288)
@@ -12,7 +12,7 @@
plan tests => 3;
-my $schema = DBICTest::Schema->connection($dsn, $dbuser, $dbpass);
+my $schema = DBICTest::Schema->connection($dsn, $dbuser, $dbpass, { AutoCommit => 1 });
my $dbh = $schema->storage->dbh;
Modified: branches/DBIx-Class-current/t/lib/DBICTest/Plain.pm
===================================================================
--- branches/DBIx-Class-current/t/lib/DBICTest/Plain.pm 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/t/lib/DBICTest/Plain.pm 2007-05-10 23:46:23 UTC (rev 3288)
@@ -15,7 +15,13 @@
my $dsn = "dbi:SQLite:${db_file}";
__PACKAGE__->load_classes("Test");
-my $schema = __PACKAGE__->compose_connection(__PACKAGE__, $dsn);
+my $schema = __PACKAGE__->compose_connection(
+ __PACKAGE__,
+ $dsn,
+ undef,
+ undef,
+ { AutoCommit => 1 }
+);
my $dbh = DBI->connect($dsn);
Modified: branches/DBIx-Class-current/t/lib/DBICTest.pm
===================================================================
--- branches/DBIx-Class-current/t/lib/DBICTest.pm 2007-05-10 22:09:56 UTC (rev 3287)
+++ branches/DBIx-Class-current/t/lib/DBICTest.pm 2007-05-10 23:46:23 UTC (rev 3288)
@@ -60,7 +60,7 @@
: 'compose_namespace');
my $schema = DBICTest::Schema->$compose_method('DBICTest')
- ->connect($dsn, $dbuser, $dbpass);
+ ->connect($dsn, $dbuser, $dbpass, { AutoCommit => 1 });
$schema->storage->on_connect_do(['PRAGMA synchronous = OFF']);
if ( !$args{no_deploy} ) {
__PACKAGE__->deploy_schema( $schema );
More information about the Bast-commits
mailing list