Merge pull request #54 from dhensby/pulls/transaction-fixes

FIX Make sure nested transactions get reset on implicit commits

LICENSE

@@ -0,0 +1,29 @@
+BSD 3-Clause License
+
+Copyright (c) 2018, SilverStripe Ltd.
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+* Redistributions of source code must retain the above copyright notice, this
+  list of conditions and the following disclaimer.
+
+* Redistributions in binary form must reproduce the above copyright notice,
+  this list of conditions and the following disclaimer in the documentation
+  and/or other materials provided with the distribution.
+
+* Neither the name of the copyright holder nor the names of its
+  contributors may be used to endorse or promote products derived from
+  this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

code/MSSQLDatabase.php

@@ -475,22 +475,33 @@ class MSSQLDatabase extends Database
 
     public function transactionRollback($savepoint = false)
     {
+        // Named transaction
         if ($savepoint) {
             $this->query("ROLLBACK TRANSACTION \"$savepoint\"");
-        } else {
-            --$this->transactionNesting;
-            if ($this->transactionNesting > 0) {
-                $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
-            } elseif ($this->connector instanceof SQLServerConnector) {
-                $this->connector->transactionRollback();
-            } else {
-                $this->query('ROLLBACK TRANSACTION');
-            }
+            return true;
         }
+
+        // Fail if transaction isn't available
+        if (!$this->transactionNesting) {
+            return false;
+        }
+        --$this->transactionNesting;
+        if ($this->transactionNesting > 0) {
+            $this->transactionRollback('NESTEDTRANSACTION' . $this->transactionNesting);
+        } elseif ($this->connector instanceof SQLServerConnector) {
+            $this->connector->transactionRollback();
+        } else {
+            $this->query('ROLLBACK TRANSACTION');
+        }
+        return true;
     }
 
     public function transactionEnd($chain = false)
     {
+        // Fail if transaction isn't available
+        if (!$this->transactionNesting) {
+            return false;
+        }
         --$this->transactionNesting;
         if ($this->transactionNesting <= 0) {
             $this->transactionNesting = 0;
@@ -500,6 +511,36 @@ class MSSQLDatabase extends Database
                 $this->query('COMMIT TRANSACTION');
             }
         }
+        return true;
+    }
+
+    /**
+     * In error condition, set transactionNesting to zero
+     */
+    protected function resetTransactionNesting()
+    {
+        $this->transactionNesting = 0;
+    }
+
+    public function query($sql, $errorLevel = E_USER_ERROR)
+    {
+        $this->inspectQuery($sql);
+        return parent::query($sql, $errorLevel);
+    }
+
+    public function preparedQuery($sql, $parameters, $errorLevel = E_USER_ERROR)
+    {
+        $this->inspectQuery($sql);
+        return parent::preparedQuery($sql, $parameters, $errorLevel);
+    }
+
+    protected function inspectQuery($sql)
+    {
+        // Any DDL discards transactions.
+        $isDDL = $this->getConnector()->isQueryDDL($sql);
+        if ($isDDL) {
+            $this->resetTransactionNesting();
+        }
     }
 
     public function comparisonClause($field, $value, $exact = false, $negate = false, $caseSensitive = null, $parameterised = false)

code/SQLServerQuery.php

@@ -43,13 +43,21 @@ class SQLServerQuery extends Query
         }
     }
 
-    public function seek($row)
+    public function getIterator()
     {
-        if (!is_resource($this->handle)) {
-            return false;
-        }
+        if (is_resource($this->handle)) {
+            while ($data = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC)) {
+                // special case for sqlsrv - date values are DateTime coming out of the sqlsrv drivers,
+                // so we convert to the usual Y-m-d H:i:s value!
+                foreach ($data as $name => $value) {
+                    if ($value instanceof DateTime) {
+                        $data[$name] = $value->format('Y-m-d H:i:s');
+                    }
+                }
 
-        user_error('MSSQLQuery::seek() not supported in sqlsrv', E_USER_WARNING);
+                yield $data;
+            }
+        }
     }
 
     public function numRecords()
@@ -65,28 +73,4 @@ class SQLServerQuery extends Query
             user_error('MSSQLQuery::numRecords() not supported in this version of sqlsrv', E_USER_WARNING);
         }
     }
-
-    public function nextRecord()
-    {
-        if (!is_resource($this->handle)) {
-            return false;
-        }
-
-        if ($data = sqlsrv_fetch_array($this->handle, SQLSRV_FETCH_ASSOC)) {
-            // special case for sqlsrv - date values are DateTime coming out of the sqlsrv drivers,
-            // so we convert to the usual Y-m-d H:i:s value!
-            foreach ($data as $name => $value) {
-                if ($value instanceof DateTime) {
-                    $data[$name] = $value->format('Y-m-d H:i:s');
-                }
-            }
-            return $data;
-        } else {
-            // Free the handle if there are no more results - sqlsrv crashes if there are too many handles
-            sqlsrv_free_stmt($this->handle);
-            $this->handle = null;
-        }
-
-        return false;
-    }
 }

composer.json

@@ -22,7 +22,7 @@
 	},
 	"extra": {
 		"branch-alias": {
-			"dev-master": "2.x-dev"
+			"dev-master": "3.x-dev"
 		}
 	},
 	"autoload": {