[impr-OpenMage#187] Ensure all db transactions are either committed o…

…r rolled back
edannenberg · Dec 6, 2017 · a734880 · a734880
1 parent d863412
commit a734880
Show file tree

Hide file tree

Showing 23 changed files with 176 additions and 145 deletions.
diff --git a/app/code/core/Mage/Admin/Model/Resource/User.php b/app/code/core/Mage/Admin/Model/Resource/User.php
@@ -212,14 +212,14 @@ public function delete(Mage_Core_Model_Abstract $user)
 
             $adapter->delete($this->getMainTable(), $conditions);
             $adapter->delete($this->getTable('admin/role'), $conditions);
+            $adapter->commit();
         } catch (Mage_Core_Exception $e) {
+            $adapter->rollBack();
             throw $e;
-            return false;
         } catch (Exception $e) {
             $adapter->rollBack();
             return false;
         }
-        $adapter->commit();
         $this->_afterDelete($user);
         return true;
     }
@@ -270,6 +270,7 @@ public function _saveRelations(Mage_Core_Model_Abstract $user)
             }
             $adapter->commit();
         } catch (Mage_Core_Exception $e) {
+            $adapter->rollBack();
             throw $e;
         } catch (Exception $e) {
             $adapter->rollBack();

diff --git a/app/code/core/Mage/Api/Model/Resource/Rules.php b/app/code/core/Mage/Api/Model/Resource/Rules.php
@@ -79,6 +79,7 @@ public function saveRel(Mage_Api_Model_Rules $rule)
 
             $adapter->commit();
         } catch (Mage_Core_Exception $e) {
+            $adapter->rollBack();
             throw $e;
         } catch (Exception $e) {
             $adapter->rollBack();

diff --git a/app/code/core/Mage/Api/Model/Resource/User.php b/app/code/core/Mage/Api/Model/Resource/User.php
@@ -245,14 +245,14 @@ public function delete(Mage_Core_Model_Abstract $user)
         try {
             $dbh->delete($this->getTable('api/user'), array('user_id = ?' => $uid));
             $dbh->delete($this->getTable('api/role'), array('user_id = ?' => $uid));
+            $dbh->commit();
         } catch (Mage_Core_Exception $e) {
+            $dbh->rollBack();
             throw $e;
-            return false;
         } catch (Exception $e) {
             $dbh->rollBack();
             return false;
         }
-        $dbh->commit();
         return true;
     }
 
@@ -298,6 +298,7 @@ public function _saveRelations(Mage_Core_Model_Abstract $user)
             }
             $adapter->commit();
         } catch (Mage_Core_Exception $e) {
+            $adapter->rollBack();
             throw $e;
         } catch (Exception $e) {
             $adapter->rollBack();

diff --git a/app/code/core/Mage/Backup/Model/Db.php b/app/code/core/Mage/Backup/Model/Db.php
@@ -112,52 +112,54 @@ public function createBackup(Mage_Backup_Model_Backup $backup)
         $backup->open(true);
 
         $this->getResource()->beginTransaction();
-
-        $tables = $this->getResource()->getTables();
-
-        $backup->write($this->getResource()->getHeader());
-
-        $ignoreDataTablesList = $this->getIgnoreDataTablesList();
-
-        foreach ($tables as $table) {
-            $backup->write($this->getResource()->getTableHeader($table)
-                . $this->getResource()->getTableDropSql($table) . "\n");
-            $backup->write($this->getResource()->getTableCreateSql($table, false) . "\n");
-
-            $tableStatus = $this->getResource()->getTableStatus($table);
-
-            if ($tableStatus->getRows() && !in_array($table, $ignoreDataTablesList)) {
-                $backup->write($this->getResource()->getTableDataBeforeSql($table));
-
-                if ($tableStatus->getDataLength() > self::BUFFER_LENGTH) {
-                    if ($tableStatus->getAvgRowLength() > 0 && $tableStatus->getAvgRowLength() < self::BUFFER_LENGTH) {
-                        // Process rows in batches
-                        $limit = floor(self::BUFFER_LENGTH / $tableStatus->getAvgRowLength());
-                        $multiRowsLength = ceil($tableStatus->getRows() / $limit);
+        try {
+            $tables = $this->getResource()->getTables();
+
+            $backup->write($this->getResource()->getHeader());
+
+            $ignoreDataTablesList = $this->getIgnoreDataTablesList();
+
+            foreach ($tables as $table) {
+                $backup->write($this->getResource()->getTableHeader($table)
+                    . $this->getResource()->getTableDropSql($table) . "\n");
+                $backup->write($this->getResource()->getTableCreateSql($table, false) . "\n");
+
+                $tableStatus = $this->getResource()->getTableStatus($table);
+
+                if ($tableStatus->getRows() && !in_array($table, $ignoreDataTablesList)) {
+                    $backup->write($this->getResource()->getTableDataBeforeSql($table));
+
+                    if ($tableStatus->getDataLength() > self::BUFFER_LENGTH) {
+                        if ($tableStatus->getAvgRowLength() > 0 && $tableStatus->getAvgRowLength() < self::BUFFER_LENGTH) {
+                            // Process rows in batches
+                            $limit = floor(self::BUFFER_LENGTH / $tableStatus->getAvgRowLength());
+                            $multiRowsLength = ceil($tableStatus->getRows() / $limit);
+                        } else {
+                            // Process rows one by one
+                            $limit = 1;
+                            $multiRowsLength = $tableStatus->getRows();
+                        }
+                    } else {
+                        // Process all rows at once
+                        $limit = $tableStatus->getRows();
+                        $multiRowsLength = 1;
                     }
-                    else {
-                        // Process rows one by one
-                        $limit = 1;
-                        $multiRowsLength = $tableStatus->getRows();
+
+                    for ($i = 0; $i < $multiRowsLength; $i++) {
+                        $backup->write($this->getResource()->getTableDataSql($table, $limit, $i * $limit));
                     }
-                }
-                else {
-                    // Process all rows at once
-                    $limit = $tableStatus->getRows();
-                    $multiRowsLength = 1;
-                }
 
-                for ($i = 0; $i < $multiRowsLength; $i ++) {
-                    $backup->write($this->getResource()->getTableDataSql($table, $limit, $i*$limit));
+                    $backup->write($this->getResource()->getTableDataAfterSql($table));
                 }
-
-                $backup->write($this->getResource()->getTableDataAfterSql($table));
             }
-        }
-        $backup->write($this->getResource()->getTableForeignKeysSql());
-        $backup->write($this->getResource()->getFooter());
+            $backup->write($this->getResource()->getTableForeignKeysSql());
+            $backup->write($this->getResource()->getFooter());
 
-        $this->getResource()->commitTransaction();
+            $this->getResource()->commitTransaction();
+        } catch (Exception $e) {
+            $this->getResource()->rollBackTransaction();
+            throw $e;
+        }
 
         $backup->close();
 

diff --git a/app/code/core/Mage/Bundle/Model/Resource/Price/Index.php b/app/code/core/Mage/Bundle/Model/Resource/Price/Index.php
@@ -253,9 +253,14 @@ protected function _savePriceIndex($productId, $websiteId, $groupId, $minPrice,
     {
         $adapter = $this->_getWriteAdapter();
         $adapter->beginTransaction();
-        $bind = array($productId, $websiteId, $groupId, $minPrice, $maxPrice);
-        $adapter->insertOnDuplicate($this->getMainTable(), $bind, array('min_price', 'max_price'));
-        $adapter->commit();
+        try {
+            $bind = array($productId, $websiteId, $groupId, $minPrice, $maxPrice);
+            $adapter->insertOnDuplicate($this->getMainTable(), $bind, array('min_price', 'max_price'));
+            $adapter->commit();
+        } catch (Exception $e) {
+            $adapter->rollBack();
+            throw $e;
+        }
 
         return $this;
     }

diff --git a/app/code/core/Mage/Catalog/Model/Category.php b/app/code/core/Mage/Catalog/Model/Category.php
@@ -245,12 +245,13 @@ public function move($parentId, $afterCategoryId)
 
             Mage::dispatchEvent($this->_eventPrefix.'_move_after', $eventParams);
             Mage::dispatchEvent('catalog_category_tree_move_after', $eventParams);
-            $this->_getResource()->commit();
 
             // Set data for indexer
             $this->setAffectedCategoryIds(array($this->getId(), $this->getParentId(), $parentId));
 
             $moveComplete = true;
+
+            $this->_getResource()->commit();
         } catch (Exception $e) {
             $this->_getResource()->rollBack();
             throw $e;

diff --git a/app/code/core/Mage/Catalog/Model/Resource/Product/Indexer/Price.php b/app/code/core/Mage/Catalog/Model/Resource/Product/Indexer/Price.php
@@ -590,13 +590,18 @@ protected function _prepareWebsiteDateTable()
         }
 
         $write->beginTransaction();
-        $table = $this->_getWebsiteDateTable();
-        $write->delete($table);
+        try {
+            $table = $this->_getWebsiteDateTable();
+            $write->delete($table);
 
-        if ($data) {
-            $write->insertMultiple($table, $data);
+            if ($data) {
+                $write->insertMultiple($table, $data);
+            }
+            $write->commit();
+        } catch (Exception $e) {
+            $write->rollBack();
+            throw $e;
         }
-        $write->commit();
 
         return $this;
     }

diff --git a/app/code/core/Mage/Catalog/Model/Resource/Product/Website.php b/app/code/core/Mage/Catalog/Model/Resource/Product/Website.php
@@ -106,9 +106,10 @@ public function addProducts($websiteIds, $productIds)
 
         $this->_getWriteAdapter()->beginTransaction();
 
-        // Before adding of products we should remove it old rows with same ids
-        $this->removeProducts($websiteIds, $productIds);
         try {
+            // Before adding of products we should remove it old rows with same ids
+            $this->removeProducts($websiteIds, $productIds);
+
             foreach ($websiteIds as $websiteId) {
                 foreach ($productIds as $productId) {
                     if (!$productId) {

diff --git a/app/code/core/Mage/CatalogIndex/Model/Indexer.php b/app/code/core/Mage/CatalogIndex/Model/Indexer.php
@@ -497,34 +497,44 @@ protected function _walkCollection($collection, $store, $attributes = array(), $
 
         for ($i=0;$i<$productCount/self::STEP_SIZE;$i++) {
             $this->_getResource()->beginTransaction();
+            try {
+                $deleteKill = false;
 
-            $stepData = $collection->getAllIds(self::STEP_SIZE, $i*self::STEP_SIZE);
+                $stepData = $collection->getAllIds(self::STEP_SIZE, $i*self::STEP_SIZE);
 
-            /**
-             * Reindex EAV attributes if required
-             */
-            if (count($attributes)) {
-                $this->_getResource()->reindexAttributes($stepData, $attributes, $store);
-            }
+                /**
+                 * Reindex EAV attributes if required
+                 */
+                if (count($attributes)) {
+                    $this->_getResource()->reindexAttributes($stepData, $attributes, $store);
+                }
 
-            /**
-             * Reindex prices if required
-             */
-            if (count($prices)) {
-                $this->_getResource()->reindexPrices($stepData, $prices, $store);
-                $this->_getResource()->reindexTiers($stepData, $store);
-                $this->_getResource()->reindexMinimalPrices($stepData, $store);
-                $this->_getResource()->reindexFinalPrices($stepData, $store);
-            }
+                /**
+                 * Reindex prices if required
+                 */
+                if (count($prices)) {
+                    $this->_getResource()->reindexPrices($stepData, $prices, $store);
+                    $this->_getResource()->reindexTiers($stepData, $store);
+                    $this->_getResource()->reindexMinimalPrices($stepData, $store);
+                    $this->_getResource()->reindexFinalPrices($stepData, $store);
+                }
 
-            Mage::getResourceSingleton('catalog/product')->refreshEnabledIndex($store, $stepData);
+                Mage::getResourceSingleton('catalog/product')->refreshEnabledIndex($store, $stepData);
 
-            $kill = Mage::getModel('catalogindex/catalog_index_kill_flag')->loadSelf();
-            if ($kill->checkIsThisProcess()) {
+                $kill = Mage::getModel('catalogindex/catalog_index_kill_flag')->loadSelf();
+                if ($kill->checkIsThisProcess()) {
+                    $this->_getResource()->rollBack();
+                    $deleteKill = true;
+                } else {
+                    $this->_getResource()->commit();
+                }
+            } catch (Exception $e) {
                 $this->_getResource()->rollBack();
+                throw $e;
+            }
+
+            if ($deleteKill && isset($kill)) {
                 $kill->delete();
-            } else {
-                $this->_getResource()->commit();
             }
         }
         return $this;

diff --git a/app/code/core/Mage/CatalogInventory/Model/Resource/Indexer/Stock.php b/app/code/core/Mage/CatalogInventory/Model/Resource/Indexer/Stock.php
@@ -115,11 +115,11 @@ public function reindexProducts($productIds)
                     $indexer->reindexEntity($byType[$indexer->getTypeId()]);
                 }
             }
+            $adapter->commit();
         } catch (Exception $e) {
             $adapter->rollback();
             throw $e;
         }
-        $adapter->commit();
 
         return $this;
     }
@@ -149,13 +149,12 @@ public function catalogProductDelete(Mage_Index_Model_Event $event)
             foreach ($parentIds as $parentType => $entityIds) {
                 $this->_getIndexer($parentType)->reindexEntity($entityIds);
             }
+            $adapter->commit();
         } catch (Exception $e) {
             $adapter->rollback();
             throw $e;
         }
 
-        $adapter->commit();
-
         return $this;
     }
 
@@ -223,11 +222,11 @@ public function catalogProductMassAction(Mage_Index_Model_Event $event)
                     $indexer->reindexEntity($byType[$indexer->getTypeId()]);
                 }
             }
+            $adapter->commit();
         } catch (Exception $e) {
             $adapter->rollback();
             throw $e;
         }
-        $adapter->commit();
 
         return $this;
     }

diff --git a/app/code/core/Mage/CatalogInventory/Model/Resource/Stock.php b/app/code/core/Mage/CatalogInventory/Model/Resource/Stock.php
@@ -181,8 +181,13 @@ public function correctItemsQty($stock, $productQtys, $operator = '-')
         );
 
         $adapter->beginTransaction();
-        $adapter->update($this->getTable('cataloginventory/stock_item'), array('qty' => $value), $where);
-        $adapter->commit();
+        try {
+            $adapter->update($this->getTable('cataloginventory/stock_item'), array('qty' => $value), $where);
+            $adapter->commit();
+        } catch (Exception $e) {
+            $adapter->rollBack();
+            throw $e;
+        }
 
         return $this;
     }

diff --git a/app/code/core/Mage/CatalogInventory/Model/Stock.php b/app/code/core/Mage/CatalogInventory/Model/Stock.php
@@ -135,21 +135,25 @@ public function registerProductsSale($items)
         $qtys = $this->_prepareProductQtys($items);
         $item = Mage::getModel('cataloginventory/stock_item');
         $this->_getResource()->beginTransaction();
-        $stockInfo = $this->_getResource()->getProductsStock($this, array_keys($qtys), true);
-        $fullSaveItems = array();
-        foreach ($stockInfo as $itemInfo) {
-            $item->setData($itemInfo);
-            if (!$item->checkQty($qtys[$item->getProductId()])) {
-                $this->_getResource()->commit();
-                Mage::throwException(Mage::helper('cataloginventory')->__('Not all products are available in the requested quantity'));
-            }
-            $item->subtractQty($qtys[$item->getProductId()]);
-            if (!$item->verifyStock() || $item->verifyNotification()) {
-                $fullSaveItems[] = clone $item;
+        try {
+            $stockInfo = $this->_getResource()->getProductsStock($this, array_keys($qtys), true);
+            $fullSaveItems = array();
+            foreach ($stockInfo as $itemInfo) {
+                $item->setData($itemInfo);
+                if (!$item->checkQty($qtys[$item->getProductId()])) {
+                    Mage::throwException(Mage::helper('cataloginventory')->__('Not all products are available in the requested quantity'));
+                }
+                $item->subtractQty($qtys[$item->getProductId()]);
+                if (!$item->verifyStock() || $item->verifyNotification()) {
+                    $fullSaveItems[] = clone $item;
+                }
             }
+            $this->_getResource()->correctItemsQty($this, $qtys, '-');
+            $this->_getResource()->commit();
+        } catch (Exception $e) {
+            $this->_getResource()->rollBack();
+            throw $e;
         }
-        $this->_getResource()->correctItemsQty($this, $qtys, '-');
-        $this->_getResource()->commit();
         return $fullSaveItems;
     }