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

FIX Make sure nested transactions get reset on implicit commits

.appveyor.yml

@@ -19,6 +19,7 @@ before_test:
   - echo extension=php_curl.dll >> php.ini
   - echo extension=php_gd2.dll >> php.ini
   - echo extension=php_tidy.dll >> php.ini
+  - echo extension=php_fileinfo.dll >> php.ini
   - php -r "readfile('http://getcomposer.org/installer');" | php
   - php -r "readfile('https://dl.dropboxusercontent.com/u/7129062/sqlsrv_unofficial_3.0.2.2.zip');" > sqlsrv.zip
   - unzip sqlsrv.zip
@@ -40,4 +41,4 @@ test_script:
 
 environment:
   DB: MSSQL
-  CORE_RELEASE: 3
+  CORE_RELEASE: master

.upgrade.yml

@@ -0,0 +1,8 @@
+mappings:
+  MSSQLAzureDatabase: SilverStripe\MSSQL\MSSQLAzureDatabase
+  MSSQLDatabase: SilverStripe\MSSQL\MSSQLDatabase
+  MSSQLDatabaseConfigurationHelper: SilverStripe\MSSQL\MSSQLDatabaseConfigurationHelper
+  MSSQLQueryBuilder: SilverStripe\MSSQL\MSSQLQueryBuilder
+  MSSQLSchemaManager: SilverStripe\MSSQL\MSSQLSchemaManager
+  SQLServerConnector: SilverStripe\MSSQL\SQLServerConnector
+  SQLServerQuery: SilverStripe\MSSQL\SQLServerQuery

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.

README.md

@@ -9,18 +9,26 @@ Allows SilverStripe to use SQL Server databases.
  * Sean Harvey (Nickname: halkyon)
    <sean (at) silverstripe (dot) com>
    
+ * Damian Mooyman (@tractorcow)
+
 ## Requirements
 
- * SilverStripe 3.0+
+ * SilverStripe 4+
  * SQL Server 2008, 2008 R2, or 2012.
 
+`mssql` PHP api is no longer supported as of 2.0
+
 ### *nix
 
- * PHP with mssql extension and [FreeTDS](http://freetds.org)
+Linux support is only available via the PDO extension. This requires:
+
+ * [dblib](http://www.php.net/manual/en/ref.pdo-dblib.php)
+ * [FreeTDS](http://freetds.org)
 
 ### Windows
 
- * PHP with [SQL Server Driver for PHP](http://www.microsoft.com/en-us/download/details.aspx?id=20098) "sqlsrv" 3.0+
+On windows you can either connect via PDO or `sqlsrv`. Both options require the
+[SQL Server Driver for PHP](https://msdn.microsoft.com/library/dn865013.aspx?f=255&MSPPError=-2147217396). "sqlsrv" 3.0+
 
 Note: [SQL Server Express](http://www.microsoft.com/express/Database/) can also be used which is provided free by Microsoft. However, it has limitations such as 10GB maximum database storage.
 
@@ -29,7 +37,7 @@ Note: [SQL Server Express](http://www.microsoft.com/express/Database/) can also
 These steps will install the latest SilverStripe stable, along with this module using [Composer](http://getcomposer.org/):
 
  * Install SilverStripe: `composer create-project silverstripe/installer /my/website/folder`
- * Install module: `cd /my/website/folder && composer require silverstripe/mssql "*"`
+ * Install module: `cd /my/website/folder && composer require silverstripe/mssql ^2`
  * Open the SilverStripe installer by browsing to install.php, e.g. **http://localhost/silverstripe/install.php**
  * Select **SQL Server 2008+** in the database list and enter your SQL Server database details
 

_config/connectors.yml

@@ -1,25 +1,32 @@
 ---
 name: mssqlconnectors
 ---
-Injector:
+SilverStripe\Core\Injector\Injector:
 # Connect using PDO
   MSSQLPDODatabase:
-    class: 'MSSQLDatabase'
+    class: 'SilverStripe\MSSQL\MSSQLDatabase'
     properties:
       connector: %$PDOConnector
       schemaManager: %$MSSQLSchemaManager
       queryBuilder: %$MSSQLQueryBuilder
 # Uses sqlsrv_connect
   MSSQLDatabase:
-    class: 'MSSQLDatabase'
+    class: 'SilverStripe\MSSQL\MSSQLDatabase'
     properties:
       connector: %$SQLServerConnector
       schemaManager: %$MSSQLSchemaManager
       queryBuilder: %$MSSQLQueryBuilder
 # Uses sqlsrv_connect to connect to a MS Azure Database
   MSSQLAzureDatabase:
-    class: 'MSSQLAzureDatabase'
+    class: 'SilverStripe\MSSQL\MSSQLAzureDatabase'
     properties:
       connector: %$SQLServerConnector
       schemaManager: %$MSSQLSchemaManager
       queryBuilder: %$MSSQLQueryBuilder
+  SQLServerConnector:
+    class: 'SilverStripe\MSSQL\SQLServerConnector'
+    type: prototype
+  MSSQLSchemaManager:
+    class: 'SilverStripe\MSSQL\MSSQLSchemaManager'
+  MSSQLQueryBuilder:
+    class: 'SilverStripe\MSSQL\MSSQLQueryBuilder'

_register_database.php

@@ -1,11 +1,17 @@
 <?php
 
+use SilverStripe\Dev\Install\DatabaseAdapterRegistry;
+use SilverStripe\MSSQL\MSSQLDatabaseConfigurationHelper;
+
 // PDO connector for MS SQL Server
+/** @skipUpgrade */
 DatabaseAdapterRegistry::register(array(
 	'class' => 'MSSQLPDODatabase',
+    'module' => 'mssql',
 	'title' => 'SQL Server 2008 (using PDO)',
-	'helperPath' => dirname(__FILE__).'/code/MSSQLDatabaseConfigurationHelper.php',
-	'supported' => (class_exists('PDO') && in_array('sqlsrv', PDO::getAvailableDrivers())),
+	'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php',
+    'helperClass' => MSSQLDatabaseConfigurationHelper::class,
+	'supported' => !!MSSQLDatabaseConfigurationHelper::getPDODriver(),
 	'missingExtensionText' =>
 		'Either the <a href="http://www.php.net/manual/en/book.pdo.php">PDO Extension</a> or 
 		the <a href="http://www.php.net/manual/en/ref.pdo-sqlsrv.php">SQL Server PDO Driver</a> 
@@ -13,10 +19,13 @@ DatabaseAdapterRegistry::register(array(
 ));
 
 // Basic driver using sqlsrv connector
+/** @skipUpgrade */
 DatabaseAdapterRegistry::register(array(
 	'class' => 'MSSQLDatabase',
+    'module' => 'mssql',
 	'title' => 'SQL Server 2008 (using sqlsrv)',
-	'helperPath' => dirname(__FILE__).'/code/MSSQLDatabaseConfigurationHelper.php',
+	'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php',
+    'helperClass' => MSSQLDatabaseConfigurationHelper::class,
 	'supported' => function_exists('sqlsrv_connect'),
 	'missingExtensionText' =>
 		'The <a href="http://www.microsoft.com/sqlserver/2005/en/us/PHP-Driver.aspx">sqlsrv</a>
@@ -31,10 +40,13 @@ DatabaseAdapterRegistry::register(array(
 ));
 
 // MS Azure uses an online database
+/** @skipUpgrade */
 DatabaseAdapterRegistry::register(array(
 	'class' => 'MSSQLAzureDatabase',
+    'module' => 'mssql',
 	'title' => 'MS Azure Database (using sqlsrv)',
-	'helperPath' => dirname(__FILE__).'/code/MSSQLDatabaseConfigurationHelper.php',
+	'helperPath' => __DIR__.'/code/MSSQLDatabaseConfigurationHelper.php',
+    'helperClass' => MSSQLDatabaseConfigurationHelper::class,
 	'supported' => function_exists('sqlsrv_connect'),
 	'missingExtensionText' =>
 		'The <a href="http://www.microsoft.com/sqlserver/2005/en/us/PHP-Driver.aspx">sqlsrv</a>

code/MSSQLAzureDatabase.php

@@ -1,5 +1,7 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
 /**
  * Specific support for SQL Azure databases running on Windows Azure.
  * Currently only supports the SQLSRV driver from Microsoft.
@@ -16,7 +18,6 @@
  * Fulltext indexes are not supported.
  *
  * @author Sean Harvey <sean at silverstripe dot com>
- * @package mssql
  */
 class MSSQLAzureDatabase extends MSSQLDatabase
 {
@@ -47,7 +48,7 @@ class MSSQLAzureDatabase extends MSSQLDatabase
      *  - database: The database to connect to
      *  - windowsauthentication: Not supported for Azure
      */
-    protected function connect($parameters)
+    public function connect($parameters)
     {
         $this->parameters = $parameters;
         $this->connectDatabase($parameters['database']);
@@ -86,9 +87,10 @@ class MSSQLAzureDatabase extends MSSQLDatabase
      * database name.
      * @see http://msdn.microsoft.com/en-us/library/windowsazure/ee336288.aspx
      *
-     * @param type $name The database name to switch to
-     * @param type $create
-     * @param type $errorLevel
+     * @param string $name The database name to switch to
+     * @param bool $create
+     * @param bool|int $errorLevel
+     * @return bool
      */
     public function selectDatabase($name, $create = false, $errorLevel = E_USER_ERROR)
     {

code/MSSQLDatabase.php

@@ -1,4 +1,18 @@
 <?php
+
+namespace SilverStripe\MSSQL;
+
+use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Core\ClassInfo;
+use SilverStripe\ORM\ArrayList;
+use SilverStripe\ORM\Connect\Database;
+use SilverStripe\ORM\DataList;
+use SilverStripe\ORM\DB;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\PaginatedList;
+use SilverStripe\ORM\Queries\SQLSelect;
+
 /**
  * Microsoft SQL Server 2008+ connector class.
  *
@@ -35,11 +49,11 @@
  *
  * References:
  * @see http://freetds.org
- *
- * @package mssql
  */
-class MSSQLDatabase extends SS_Database
+class MSSQLDatabase extends Database
 {
+    use Configurable;
+    use Injectable;
 
     /**
      * Words that will trigger an error if passed to a SQL Server fulltext search
@@ -64,6 +78,11 @@ class MSSQLDatabase extends SS_Database
      */
     protected $fullTextEnabled = null;
 
+    /**
+     * @var bool
+     */
+    protected $transactionNesting = 0;
+
     /**
      * Set the default collation of the MSSQL nvarchar fields that we create.
      * We don't apply this to the database as a whole, so that we can use unicode collations.
@@ -72,7 +91,7 @@ class MSSQLDatabase extends SS_Database
      */
     public static function set_collation($collation)
     {
-        Config::inst()->update('MSSQLDatabase', 'collation', $collation);
+        static::config()->set('collation', $collation);
     }
 
     /**
@@ -84,7 +103,7 @@ class MSSQLDatabase extends SS_Database
      */
     public static function get_collation()
     {
-        return Config::inst()->get('MSSQLDatabase', 'collation');
+        return static::config()->get('collation');
     }
 
     /**
@@ -191,20 +210,25 @@ class MSSQLDatabase extends SS_Database
      * Picks up the fulltext-indexed tables from the database and executes search on all of them.
      * Results are obtained as ID-ClassName pairs which is later used to reconstruct the DataObjectSet.
      *
-     * @param array classesToSearch computes all descendants and includes them. Check is done via WHERE clause.
+     * @param array $classesToSearch computes all descendants and includes them. Check is done via WHERE clause.
      * @param string $keywords Keywords as a space separated string
-     * @return object DataObjectSet of result pages
+     * @param int $start
+     * @param int $pageLength
+     * @param string $sortBy
+     * @param string $extraFilter
+     * @param bool $booleanSearch
+     * @param string $alternativeFileFilter
+     * @param bool $invertedMatch
+     * @return PaginatedList DataObjectSet of result pages
      */
     public function searchEngine($classesToSearch, $keywords, $start, $pageLength, $sortBy = "Relevance DESC", $extraFilter = "", $booleanSearch = false, $alternativeFileFilter = "", $invertedMatch = false)
     {
-        if (isset($objects)) {
-            $results = new ArrayList($objects);
-        } else {
-            $results = new ArrayList();
-        }
+        $start = (int)$start;
+        $pageLength = (int)$pageLength;
+        $results = new ArrayList();
 
         if (!$this->fullTextEnabled()) {
-            return $results;
+            return new PaginatedList($results);
         }
         if (!in_array(substr($sortBy, 0, 9), array('"Relevanc', 'Relevance'))) {
             user_error("Non-relevance sort not supported.", E_USER_ERROR);
@@ -239,34 +263,33 @@ class MSSQLDatabase extends SS_Database
 
         // Create one query per each table, $columns not used. We want just the ID and the ClassName of the object from this query.
         foreach ($tables as $tableName => $columns) {
-            $baseClass = ClassInfo::baseDataClass($tableName);
-
+            $class = DataObject::getSchema()->tableClass($tableName);
             $join = $this->fullTextSearchMSSQL($tableName, $keywords);
             if (!$join) {
-                return $results;
+                return new PaginatedList($results);
             } // avoid "Null or empty full-text predicate"
 
             // Check if we need to add ShowInSearch
             $where = null;
-            if (strpos($tableName, 'SiteTree') === 0) {
+            if ($class === 'SilverStripe\\CMS\\Model\\SiteTree') {
                 $where = array("\"$tableName\".\"ShowInSearch\"!=0");
-            } elseif (strpos($tableName, 'File') === 0) {
+            } elseif ($class === 'SilverStripe\\Assets\\File') {
                 // File.ShowInSearch was added later, keep the database driver backwards compatible
                 // by checking for its existence first
-                $fields = $this->fieldList($tableName);
+                $fields = $this->getSchemaManager()->fieldList($tableName);
                 if (array_key_exists('ShowInSearch', $fields)) {
                     $where = array("\"$tableName\".\"ShowInSearch\"!=0");
                 }
             }
 
-            $queries[$tableName] = DataList::create($tableName)->where($where, '')->dataQuery()->query();
+            $queries[$tableName] = DataList::create($class)->where($where)->dataQuery()->query();
             $queries[$tableName]->setOrderBy(array());
 
             // Join with CONTAINSTABLE, a full text searcher that includes relevance factor
             $queries[$tableName]->setFrom(array("\"$tableName\" INNER JOIN $join AS \"ft\" ON \"$tableName\".\"ID\"=\"ft\".\"KEY\""));
             // Join with the base class if needed, as we want to test agains the ClassName
-            if ($tableName != $baseClass) {
-                $queries[$tableName]->setFrom("INNER JOIN \"$baseClass\" ON  \"$baseClass\".\"ID\"=\"$tableName\".\"ID\"");
+            if ($tableName != $tableName) {
+                $queries[$tableName]->setFrom("INNER JOIN \"$tableName\" ON  \"$tableName\".\"ID\"=\"$tableName\".\"ID\"");
             }
 
             $queries[$tableName]->setSelect(array("\"$tableName\".\"ID\""));
@@ -278,7 +301,7 @@ class MSSQLDatabase extends SS_Database
             if (count($allClassesToSearch)) {
                 $classesPlaceholder = DB::placeholders($allClassesToSearch);
                 $queries[$tableName]->addWhere(array(
-                    "\"$baseClass\".\"ClassName\" IN ($classesPlaceholder)" =>
+                    "\"$tableName\".\"ClassName\" IN ($classesPlaceholder)" =>
                     $allClassesToSearch
                 ));
             }
@@ -289,6 +312,7 @@ class MSSQLDatabase extends SS_Database
         $querySQLs = array();
         $queryParameters = array();
         foreach ($queries as $query) {
+            /** @var SQLSelect $query */
             $querySQLs[] = $query->sql($parameters);
             $queryParameters = array_merge($queryParameters, $parameters);
         }
@@ -327,8 +351,8 @@ class MSSQLDatabase extends SS_Database
      * Allow auto-increment primary key editing on the given table.
      * Some databases need to enable this specially.
      *
-     * @param $table The name of the table to have PK editing allowed on
-     * @param $allow True to start, false to finish
+     * @param string $table The name of the table to have PK editing allowed on
+     * @param bool $allow True to start, false to finish
      */
     public function allowPrimaryKeyEditing($table, $allow = true)
     {
@@ -338,11 +362,10 @@ class MSSQLDatabase extends SS_Database
     /**
      * Returns a SQL fragment for querying a fulltext search index
      *
-     * @param $tableName specific - table name
-     * @param $keywords string The search query
-     * @param $fields array The list of field names to search on, or null to include all
-     *
-     * @returns null if keyword set is empty or the string with JOIN clause to be added to SQL query
+     * @param string $tableName specific - table name
+     * @param string $keywords The search query
+     * @param array $fields The list of field names to search on, or null to include all
+     * @return string Clause, or null if keyword set is empty or the string with JOIN clause to be added to SQL query
      */
     public function fullTextSearchMSSQL($tableName, $keywords, $fields = null)
     {
@@ -429,14 +452,20 @@ class MSSQLDatabase extends SS_Database
 
     /**
      * Start transaction. READ ONLY not supported.
+     *
+     * @param bool $transactionMode
+     * @param bool $sessionCharacteristics
      */
     public function transactionStart($transactionMode = false, $sessionCharacteristics = false)
     {
-        if ($this->connector instanceof SQLServerConnector) {
+        if ($this->transactionNesting > 0) {
+            $this->transactionSavepoint('NESTEDTRANSACTION' . $this->transactionNesting);
+        } elseif ($this->connector instanceof SQLServerConnector) {
             $this->connector->transactionStart();
         } else {
             $this->query('BEGIN TRANSACTION');
         }
+        ++$this->transactionNesting;
     }
 
     public function transactionSavepoint($savepoint)
@@ -446,21 +475,71 @@ class MSSQLDatabase extends SS_Database
 
     public function transactionRollback($savepoint = false)
     {
+        // Named transaction
         if ($savepoint) {
             $this->query("ROLLBACK TRANSACTION \"$savepoint\"");
+            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)
     {
-        if ($this->connector instanceof SQLServerConnector) {
-            $this->connector->transactionEnd();
-        } else {
-            $this->query('COMMIT TRANSACTION');
+        // Fail if transaction isn't available
+        if (!$this->transactionNesting) {
+            return false;
+        }
+        --$this->transactionNesting;
+        if ($this->transactionNesting <= 0) {
+            $this->transactionNesting = 0;
+            if ($this->connector instanceof SQLServerConnector) {
+                $this->connector->transactionEnd();
+            } else {
+                $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();
         }
     }
 

code/MSSQLDatabaseConfigurationHelper.php

@@ -1,24 +1,31 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
+use SilverStripe\Dev\Install\DatabaseAdapterRegistry;
+use SilverStripe\Dev\Install\DatabaseConfigurationHelper;
+use PDO;
+use Exception;
+
 /**
  * This is a helper class for the SS installer.
  *
  * It does all the specific checking for MSSQLDatabase
  * to ensure that the configuration is setup correctly.
- * 
- * @package mssql
  */
 class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
 {
 
     protected function isAzure($databaseConfig)
     {
+        /** @skipUpgrade */
         return $databaseConfig['type'] === 'MSSQLAzureDatabase';
     }
 
     /**
      * Create a connection of the appropriate type
      *
+     * @skipUpgrade
      * @param array $databaseConfig
      * @param string $error Error message passed by value
      * @return mixed|null Either the connection object, or null if error
@@ -56,8 +63,14 @@ class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
                     }
                     return null;
                 case 'MSSQLPDODatabase':
+                    $driver = $this->getPDODriver();
+                    if (!$driver) {
+                        $error = 'No supported PDO driver';
+                        return null;
+                    }
+
                     // May throw a PDOException if fails
-                    $conn = @new PDO('sqlsrv:Server='.$databaseConfig['server'], $databaseConfig['username'], $databaseConfig['password']);
+                    $conn = @new PDO($driver.':Server='.$databaseConfig['server'], $databaseConfig['username'], $databaseConfig['password']);
                     if ($conn) {
                         return $conn;
                     } else {
@@ -65,7 +78,7 @@ class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
                         return null;
                     }
                 default:
-                    $error = 'Invalid connection type';
+                    $error = 'Invalid connection type: ' . $databaseConfig['type'];
                     return null;
             }
         } catch (Exception $ex) {
@@ -74,12 +87,29 @@ class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
         }
     }
 
+    /**
+     * Get supported PDO driver
+     *
+     * @return null
+     */
+    public static function getPDODriver() {
+        if (!class_exists('PDO')) {
+            return null;
+        }
+        foreach(PDO::getAvailableDrivers() as $driver) {
+            if(in_array($driver, array('sqlsrv', 'dblib'))) {
+                return $driver;
+            }
+        }
+        return null;
+    }
+
     /**
      * Helper function to quote a string value
      *
      * @param mixed $conn Connection object/resource
      * @param string $value Value to quote
-     * @return string Quoted strieng
+     * @return string Quoted string
      */
     protected function quote($conn, $value)
     {
@@ -92,6 +122,7 @@ class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
         } else {
             user_error('Invalid database connection', E_USER_ERROR);
         }
+        return null;
     }
 
     /**
@@ -189,6 +220,7 @@ class MSSQLDatabaseConfigurationHelper implements DatabaseConfigurationHelper
     public function requireDatabaseOrCreatePermissions($databaseConfig)
     {
         $conn = $this->createConnection($databaseConfig, $error);
+        /** @skipUpgrade */
         if (empty($conn)) {
             $success = false;
             $alreadyExists = false;

code/MSSQLQueryBuilder.php

@@ -1,9 +1,13 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
+use InvalidArgumentException;
+use SilverStripe\ORM\Queries\SQLSelect;
+use SilverStripe\ORM\Connect\DBQueryBuilder;
+
 /**
  * Builds a SQL query string from a SQLExpression object
- * 
- * @package mssql
  */
 class MSSQLQueryBuilder extends DBQueryBuilder
 {

code/MSSQLSchemaManager.php

@@ -1,9 +1,11 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
+use SilverStripe\ORM\Connect\DBSchemaManager;
+
 /**
  * Represents and handles all schema management for a MS SQL database
- * 
- * @package mssql
  */
 class MSSQLSchemaManager extends DBSchemaManager
 {
@@ -182,13 +184,14 @@ class MSSQLSchemaManager extends DBSchemaManager
 
     /**
      * Create a new table.
-     * @param $tableName The name of the table
-     * @param $fields A map of field names to field types
-     * @param $indexes A map of indexes
-     * @param $options An map of additional options.  The available keys are as follows:
+     * @param string $tableName The name of the table
+     * @param array $fields A map of field names to field types
+     * @param array $indexes A map of indexes
+     * @param array $options An map of additional options.  The available keys are as follows:
      *   - 'MSSQLDatabase'/'MySQLDatabase'/'PostgreSQLDatabase' - database-specific options such as "engine" for MySQL.
      *   - 'temporary' - If true, then a temporary table will be created
-     * @return The table name generated.  This may be different from the table name, for example with temporary tables.
+     * @param array $advancedOptions
+     * @return string The table name generated.  This may be different from the table name, for example with temporary tables.
      */
     public function createTable($tableName, $fields = null, $indexes = null, $options = null, $advancedOptions = null)
     {
@@ -227,18 +230,20 @@ class MSSQLSchemaManager extends DBSchemaManager
 
     /**
      * Alter a table's schema.
-     * @param $table The name of the table to alter
-     * @param $newFields New fields, a map of field name => field schema
-     * @param $newIndexes New indexes, a map of index name => index type
-     * @param $alteredFields Updated fields, a map of field name => field schema
-     * @param $alteredIndexes Updated indexes, a map of index name => index type
+     * @param string $tableName The name of the table to alter
+     * @param array $newFields New fields, a map of field name => field schema
+     * @param array $newIndexes New indexes, a map of index name => index type
+     * @param array $alteredFields Updated fields, a map of field name => field schema
+     * @param array $alteredIndexes Updated indexes, a map of index name => index type
+     * @param array $alteredOptions
+     * @param array $advancedOptions
      */
     public function alterTable($tableName, $newFields = null, $newIndexes = null, $alteredFields = null, $alteredIndexes = null, $alteredOptions=null, $advancedOptions=null)
     {
         $alterList = array();
 
         // drop any fulltext indexes that exist on the table before altering the structure
-        if ($this->fullTextIndexExists($tableName)) {
+        if ($this->fulltextIndexExists($tableName)) {
             $alterList[] = "\nDROP FULLTEXT INDEX ON \"$tableName\";";
         }
 
@@ -395,8 +400,10 @@ class MSSQLSchemaManager extends DBSchemaManager
         // drop *ALL* indexes on a table before proceeding
         // this won't drop primary keys, though
         $indexes = $this->indexNames($tableName);
+        $indexes = array_filter($indexes);
+
         foreach ($indexes as $indexName) {
-            $alterQueries[] = "DROP INDEX \"$indexName\" ON \"$tableName\";";
+            $alterQueries[] = "IF EXISTS (SELECT name FROM sys.indexes WHERE name = '$indexName' AND object_id = object_id(SCHEMA_NAME() + '.$tableName')) DROP INDEX \"$indexName\" ON \"$tableName\";";
         }
 
         $prefix = "ALTER TABLE \"$tableName\" ";
@@ -602,10 +609,13 @@ class MSSQLSchemaManager extends DBSchemaManager
         // Consolidate/Cleanup spec into array format
         $indexSpec = $this->parseIndexSpec($indexName, $indexSpec);
 
-        $drop = "IF EXISTS (SELECT name FROM sys.indexes WHERE name = '$index') DROP INDEX $index ON \"$tableName\";";
+        $drop = "IF EXISTS (SELECT name FROM sys.indexes WHERE name = '$index' AND object_id = object_id(SCHEMA_NAME() + '.$tableName')) DROP INDEX $index ON \"$tableName\";";
 
         // create a type-specific index
-        if ($indexSpec['type'] == 'fulltext' && $this->database->fullTextEnabled()) {
+        if ($indexSpec['type'] == 'fulltext') {
+            if(!$this->database->fullTextEnabled()) {
+                return '';
+            }
             // enable fulltext on this table
             $this->createFullTextCatalog();
             $primary_key = $this->getPrimaryKey($tableName);
@@ -687,6 +697,7 @@ class MSSQLSchemaManager extends DBSchemaManager
      * For a given table name, get all the internal index names,
      * except for those that are primary keys and fulltext indexes.
      *
+     * @param string $tableName
      * @return array
      */
     public function indexNames($tableName)
@@ -694,7 +705,7 @@ class MSSQLSchemaManager extends DBSchemaManager
         return $this->preparedQuery('
 			SELECT ind.name FROM sys.indexes ind
 			INNER JOIN sys.tables t ON ind.object_id = t.object_id
-			WHERE is_primary_key = 0 AND t.name = ?',
+			WHERE is_primary_key = 0 AND t.name = ? AND ind.name IS NOT NULL',
             array($tableName)
         )->column();
     }
@@ -712,7 +723,7 @@ class MSSQLSchemaManager extends DBSchemaManager
      * Return a boolean type-formatted string
      * We use 'bit' so that we can do numeric-based comparisons
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function boolean($values)
@@ -724,7 +735,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a date type-formatted string.
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function date($values)
@@ -735,7 +746,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a decimal type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function decimal($values)
@@ -758,7 +769,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a enum type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function enum($values)
@@ -776,6 +787,9 @@ class MSSQLSchemaManager extends DBSchemaManager
 
     /**
      * @todo Make this work like {@link MySQLDatabase::set()}
+     *
+     * @param array $values
+     * @return string
      */
     public function set($values)
     {
@@ -785,7 +799,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a float type-formatted string.
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function float($values)
@@ -796,7 +810,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a int type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function int($values)
@@ -807,7 +821,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a bigint type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function bigint($values)
@@ -819,10 +833,10 @@ class MSSQLSchemaManager extends DBSchemaManager
      * Return a datetime type-formatted string
      * For MS SQL, we simply return the word 'timestamp', no other parameters are necessary
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
-    public function ss_datetime($values)
+    public function datetime($values)
     {
         return 'datetime null';
     }
@@ -830,7 +844,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a text type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function text($values)
@@ -843,7 +857,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a time type-formatted string.
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function time($values)
@@ -854,7 +868,7 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Return a varchar type-formatted string
      *
-     * @params array $values Contains a tokenised list of info about this data type
+     * @param array $values Contains a tokenised list of info about this data type
      * @return string
      */
     public function varchar($values)
@@ -866,6 +880,8 @@ class MSSQLSchemaManager extends DBSchemaManager
 
     /**
      * Return a 4 digit numeric type.
+     *
+     * @param array $values
      * @return string
      */
     public function year($values)
@@ -875,6 +891,9 @@ class MSSQLSchemaManager extends DBSchemaManager
 
     /**
      * This returns the column which is the primary key for each table
+     *
+     * @param bool $asDbValue
+     * @param bool $hasAutoIncPK
      * @return string
      */
     public function IdColumn($asDbValue = false, $hasAutoIncPK = true)
@@ -899,6 +918,10 @@ class MSSQLSchemaManager extends DBSchemaManager
     /**
      * Returns the values of the given enum field
      * NOTE: Experimental; introduced for db-abstraction and may changed before 2.4 is released.
+     *
+     * @param string $tableName
+     * @param string $fieldName
+     * @return array
      */
     public function enumValuesForField($tableName, $fieldName)
     {
@@ -918,6 +941,9 @@ class MSSQLSchemaManager extends DBSchemaManager
      *
      * For instance, MSSQL uses 'BIGINT', while MySQL uses 'UNSIGNED'
      * and PostgreSQL uses 'INT'.
+     *
+     * @param string $type
+     * @return string
      */
     public function dbDataType($type)
     {

code/SQLServerConnector.php

@@ -1,9 +1,11 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
+use SilverStripe\ORM\Connect\DBConnector;
+
 /**
  * Database connector driver for sqlsrv_ library
- * 
- * @package mssql
  */
 class SQLServerConnector extends DBConnector
 {
@@ -195,7 +197,7 @@ class SQLServerConnector extends DBConnector
      * Quotes a string, including the "N" prefix so unicode
      * strings are saved to the database correctly.
      *
-     * @param string $string String to be encoded
+     * @param string $value String to be encoded
      * @return string Processed string ready for DB
      */
     public function quoteString($value)

code/SQLServerQuery.php

@@ -1,11 +1,14 @@
 <?php
 
+namespace SilverStripe\MSSQL;
+
+use DateTime;
+use SilverStripe\ORM\Connect\Query;
+
 /**
  * A result-set from a MSSQL database.
- * 
- * @package mssql
  */
-class SQLServerQuery extends SS_Query
+class SQLServerQuery extends Query
 {
 
     /**
@@ -40,13 +43,21 @@ class SQLServerQuery extends SS_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()
@@ -62,28 +73,4 @@ class SQLServerQuery extends SS_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

@@ -1,7 +1,7 @@
 {
 	"name": "silverstripe/mssql",
 	"description": "Adds MSSQL support to SilverStripe",
-	"type": "silverstripe-module",
+	"type": "silverstripe-vendormodule",
 	"keywords": ["silverstripe", "mssql", "database"],
 	"authors": [
 		{
@@ -14,11 +14,20 @@
 		}
 	],
 	"require": {
-		"silverstripe/framework": "^3.2"
+		"silverstripe/framework": "^4"
+	},
+	"suggest": {
+        "ext-sqlsrv": "Required to support MSSQLDatabase as the server type",
+        "ext-pdo_sqlsrv": "Required to support MSSQLPDODatabase as the server type"
 	},
 	"extra": {
 		"branch-alias": {
-			"dev-master": "1.0.x-dev"
+			"dev-master": "3.x-dev"
+		}
+	},
+	"autoload": {
+		"psr-4": {
+			"SilverStripe\\MSSQL\\": "code/"
 		}
 	},
 	"prefer-stable": true,

tests/MSSQLDatabaseQueryTest.php

@@ -1,4 +1,9 @@
 <?php
+
+use SilverStripe\ORM\DataObject;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Dev\TestOnly;
+
 class MSSQLDatabaseQueryTest extends SapphireTest
 {
 
@@ -23,8 +28,8 @@ class MSSQLDatabaseQueryTest extends SapphireTest
 class MSSQLDatabaseQueryTestDataObject extends DataObject implements TestOnly
 {
 
-    public static $db = array(
+    private static $db = array(
         'TestDate' => 'Date',
-        'TestDatetime' => 'SS_Datetime'
+        'TestDatetime' => 'Datetime'
     );
 }