Merge pull request sympa-community#1335 from ikedas/issue-1330 by ikedas

Database: Nested transaction crashes
LANFDETROY · Jan 23, 2022 · af26abc · af26abc
2 parents 7ba9313 + 09bcf96
commit af26abc
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 59 deletions.
diff --git a/src/lib/Sympa/Database.pm b/src/lib/Sympa/Database.pm
@@ -413,53 +413,56 @@ sub prepare_query_log_values {
 # DEPRECATED: Use tools::eval_in_time() and fetchall_arrayref().
 #sub fetch();
 
+# As most of DBMS do not support nested transactions, these are not
+# effective during when {_sdbTransactionLevel} attribute is
+# positive, i.e. only the outermost transaction will be available.
 sub begin {
     my $self = shift;
 
-    my $dbh = $self->__dbh;
-    return undef unless $dbh;
-
-    return undef unless $dbh->begin_work;
-
     $self->{_sdbTransactionLevel} //= 0;
-    unless ($self->{_sdbTransactionLevel}++) {
-        $self->{_sdbPrevPersistency} = $self->set_persistent(0);
+    if ($self->{_sdbTransactionLevel}++) {
+        return 1;
     }
 
+    my $dbh = $self->__dbh;
+    return undef unless $dbh;
+
+    $dbh->begin_work or die $DBI::errstr;
+    $self->{_sdbPrevPersistency} = $self->set_persistent(0);
     return 1;
 }
 
-sub _finalize_transaction {
+sub commit {
     my $self = shift;
 
-    unless (defined $self->{_sdbTransactionLevel}) {
-        return;
-    }
     unless ($self->{_sdbTransactionLevel}) {
         die 'bug in logic. Ask developer';
     }
-    unless (--$self->{_sdbTransactionLevel}) {
-        $self->set_persistent($self->{_sdbPrevPersistency});
+    if (--$self->{_sdbTransactionLevel}) {
+        return 1;
     }
-}
-
-sub commit {
-    my $self = shift;
 
     my $dbh = $self->__dbh;
     return undef unless $dbh;
 
-    $self->_finalize_transaction;
+    $self->set_persistent($self->{_sdbPrevPersistency});
     return $dbh->commit;
 }
 
 sub rollback {
     my $self = shift;
 
+    unless ($self->{_sdbTransactionLevel}) {
+        die 'bug in logic. Ask developer';
+    }
+    if (--$self->{_sdbTransactionLevel}) {
+        return 1;
+    }
+
     my $dbh = $self->__dbh;
     return undef unless $dbh;
 
-    $self->_finalize_transaction;
+    $self->set_persistent($self->{_sdbPrevPersistency});
     return $dbh->rollback;
 }
 

diff --git a/src/lib/Sympa/DatabaseDriver/SQLite.pm b/src/lib/Sympa/DatabaseDriver/SQLite.pm
@@ -520,44 +520,6 @@ sub translate_type {
     return $type;
 }
 
-# As SQLite does not support nested transactions, these are not effective
-# during when {_sdbSQLiteTransactionLevel} attribute is positive, i.e. only
-# the outermost transaction will be available.
-sub begin {
-    my $self = shift;
-
-    $self->{_sdbSQLiteTransactionLevel} //= 0;
-
-    if ($self->{_sdbSQLiteTransactionLevel}++) {
-        return 1;
-    }
-    return $self->SUPER::begin;
-}
-
-sub commit {
-    my $self = shift;
-
-    unless ($self->{_sdbSQLiteTransactionLevel}) {
-        die 'bug in logic. Ask developer';
-    }
-    if (--$self->{_sdbSQLiteTransactionLevel}) {
-        return 1;
-    }
-    return $self->SUPER::commit;
-}
-
-sub rollback {
-    my $self = shift;
-
-    unless ($self->{_sdbSQLiteTransactionLevel}) {
-        die 'bug in logic. Ask developer';
-    }
-    if (--$self->{_sdbSQLiteTransactionLevel}) {
-        return 1;
-    }
-    return $self->SUPER::rollback;
-}
-
 # Note:
 # - To prevent "database is locked" error, acquire "immediate" lock
 #   by each query.  Most queries excluding "SELECT" need to lock in this
@@ -572,7 +534,7 @@ sub do_query {
     my $need_lock =
         ($_[0] =~
             /^\s*(ALTER|CREATE|DELETE|DROP|INSERT|REINDEX|REPLACE|UPDATE)\b/i)
-        unless $self->{_sdbSQLiteTransactionLevel};
+        unless $self->{_sdbTransactionLevel};
 
     ## acquire "immediate" lock
     unless (!$need_lock or $self->__dbh->begin_work) {
@@ -612,7 +574,7 @@ sub do_prepared_query {
     my $need_lock =
         ($_[0] =~
             /^\s*(ALTER|CREATE|DELETE|DROP|INSERT|REINDEX|REPLACE|UPDATE)\b/i)
-        unless $self->{_sdbSQLiteTransactionLevel};
+        unless $self->{_sdbTransactionLevel};
 
     ## acquire "immediate" lock
     unless (!$need_lock or $self->__dbh->begin_work) {