From 09bcf963f048331f1a718a535c3af89feaab4386 Mon Sep 17 00:00:00 2001
From: IKEDA Soji <ikeda@conversion.co.jp>
Date: Sat, 22 Jan 2022 18:37:15 +0900
Subject: [PATCH] As most of RDBMS do not support nested transactions, only the
 outermost transaction should be available.

---
 src/lib/Sympa/Database.pm              | 41 +++++++++++++------------
 src/lib/Sympa/DatabaseDriver/SQLite.pm | 42 ++------------------------
 2 files changed, 24 insertions(+), 59 deletions(-)

diff --git a/src/lib/Sympa/Database.pm b/src/lib/Sympa/Database.pm
index e02cf94e2..99d2e9716 100644
--- 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
index caf45b863..d42417e2f 100644
--- 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) {