From 12e62c63296a904126f02b0e1e3fb7a1d5cf9174 Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Sat, 22 Nov 2008 03:51:04 +0000 Subject: [PATCH] ENHANCEMENT: Change MySQLDatabase connection to operate in ANSI SQL mode, to ease the transition to DB abstraction git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@66399 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- cli-script.php | 2 ++ core/model/DataObject.php | 46 ++++++++++++++++---------------- core/model/MySQLDatabase.php | 2 ++ core/model/SQLQuery.php | 13 ++++----- tests/DataObjectTest.php | 12 ++++----- tests/SQLQueryTest.php | 4 +-- tests/forms/RequirementsTest.php | 1 + tests/security/SecurityTest.php | 10 +++---- 8 files changed, 48 insertions(+), 42 deletions(-) diff --git a/cli-script.php b/cli-script.php index 5b417d102..587401d07 100755 --- a/cli-script.php +++ b/cli-script.php @@ -55,6 +55,8 @@ if(isset($_SERVER['argv'][2])) { */ require_once("core/Core.php"); +global $databcaseConfig; + // We don't have a session in cli-script, but this prevents errors $_SESSION = null; diff --git a/core/model/DataObject.php b/core/model/DataObject.php index 20aeede72..559056905 100644 --- a/core/model/DataObject.php +++ b/core/model/DataObject.php @@ -743,7 +743,7 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP if((!isset($this->record['ID']) || !$this->record['ID']) && isset($ancestry[0])) { $baseTable = $ancestry[0]; - DB::query("INSERT INTO `{$baseTable}` SET Created = NOW()"); + DB::query("INSERT INTO \"{$baseTable}\" SET Created = NOW()"); $this->record['ID'] = DB::getGeneratedID($baseTable); $this->changed['ID'] = 2; @@ -880,7 +880,7 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP if(self::has_own_table($ancestor)) { $sql = new SQLQuery(); $sql->delete = true; - $sql->from[$ancestor] = "`$ancestor`"; + $sql->from[$ancestor] = "\"$ancestor\""; $sql->where[] = "ID = $this->ID"; $this->extend('augmentSQL', $sql); $sql->execute(); @@ -1171,12 +1171,12 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP $query = $componentObj->extendedSQL( - "`$table`.$parentField = $this->ID", // filter + "\"$table\".$parentField = $this->ID", // filter $sort, $limit, - "INNER JOIN `$table` ON `$table`.$componentField = `$componentBaseClass`.ID" // join + "INNER JOIN \"$table\" ON \"$table\".$componentField = \"$componentBaseClass\".ID" // join ); - array_unshift($query->select, "`$table`.*"); + array_unshift($query->select, "\"$table\".*"); if($filter) $query->where[] = $filter; if($join) $query->from[] = $join; @@ -1218,16 +1218,16 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP $baseComponentClass = ClassInfo::baseDataClass($componentClass); if($baseTable == $parentClass) { - return "LEFT JOIN `$table` ON (`$table`.`$parentField` = `$parentClass`.`ID` AND `$table`.`$componentField` = '{$this->ID}')"; + return "LEFT JOIN \"$table\" ON (\"$table\".\"$parentField\" = \"$parentClass\".\"ID\" AND \"$table\".\"$componentField\" = '{$this->ID}')"; } else { - return "LEFT JOIN `$table` ON (`$table`.`$componentField` = `$baseComponentClass`.`ID` AND `$table`.`$parentField` = '{$this->ID}')"; + return "LEFT JOIN \"$table\" ON (\"$table\".\"$componentField\" = \"$baseComponentClass\".\"ID\" AND \"$table\".\"$parentField\" = '{$this->ID}')"; } } function getManyManyFilter($componentName, $baseTable) { list($parentClass, $componentClass, $parentField, $componentField, $table) = $this->many_many($componentName); - return "`$table`.`$parentField` = '{$this->ID}'"; + return "\"$table\".\"$parentField\" = '{$this->ID}'"; } /** @@ -1865,17 +1865,17 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP $groupList = implode(', ', $groups->column("ID")); $query = new SQLQuery( - "`Page_Can$perm`.PageID", - array("`Page_Can$perm`"), + "\"Page_Can$perm\".PageID", + array("\"Page_Can$perm\""), "GroupID IN ($groupList)"); $permissionCache[$memberID][$perm] = $query->execute()->column(); if($perm == "View") { - $query = new SQLQuery("`SiteTree`.ID", array( - "`SiteTree`", - "LEFT JOIN `Page_CanView` ON `Page_CanView`.PageID = `SiteTree`.ID" - ), "`Page_CanView`.PageID IS NULL"); + $query = new SQLQuery("\"SiteTree\".ID", array( + "\"SiteTree\"", + "LEFT JOIN \"Page_CanView\" ON \"Page_CanView\".PageID = \"SiteTree\".ID" + ), "\"Page_CanView\".PageID IS NULL"); $unsecuredPages = $query->execute()->column(); if($permissionCache[$memberID][$perm]) { @@ -2070,11 +2070,11 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP } $baseClass = array_shift($tableClasses); - $select = array("`$baseClass`.*"); + $select = array("\"$baseClass\".*"); // Build our intial query $query = new SQLQuery($select); - $query->from("`$baseClass`"); + $query->from("\"$baseClass\""); $query->where($filter); $query->orderby($sort); $query->limit($limit); @@ -2091,8 +2091,8 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP // Join all the tables if($tableClasses && self::$subclass_access) { foreach($tableClasses as $tableClass) { - $query->from[$tableClass] = "LEFT JOIN `$tableClass` ON `$tableClass`.ID = `$baseClass`.ID"; - $query->select[] = "`$tableClass`.*"; + $query->from[$tableClass] = "LEFT JOIN \"$tableClass\" ON \"$tableClass\".ID = \"$baseClass\".ID"; + $query->select[] = "\"$tableClass\".*"; // Add SQL for multi-value fields $SNG = singleton($tableClass); @@ -2107,8 +2107,8 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP } } - $query->select[] = "`$baseClass`.ID"; - $query->select[] = "if(`$baseClass`.ClassName,`$baseClass`.ClassName,'$baseClass') AS RecordClassName"; + $query->select[] = "\"$baseClass\".ID"; + $query->select[] = "if(\"$baseClass\".ClassName,\"$baseClass\".ClassName,'$baseClass') AS RecordClassName"; // Get the ClassName values to filter to $classNames = ClassInfo::subclassesFor($this->class); @@ -2125,7 +2125,7 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP user_error("DataObject::get() Can't find data sub-classes for '$callerClass'"); } - $query->where[] = "`$baseClass`.ClassName IN ('" . implode("','", $classNames) . "')"; + $query->where[] = "\"$baseClass\".ClassName IN ('" . implode("','", $classNames) . "')"; } if($having) { @@ -2352,11 +2352,11 @@ class DataObject extends ViewableData implements DataObjectInterface,i18nEntityP if(singleton($callerClass) instanceof DataObject) { $tableClasses = ClassInfo::dataClassesFor($callerClass); $baseClass = array_shift($tableClasses); - return DataObject::get_one($callerClass,"`$baseClass`.`ID` = $id"); + return DataObject::get_one($callerClass,"\"$baseClass\".\"ID\" = $id"); // This simpler code will be used by non-DataObject classes that implement DataObjectInterface } else { - return DataObject::get_one($callerClass,"`ID` = $id"); + return DataObject::get_one($callerClass,"\"ID\" = $id"); } } else { user_error("DataObject::get_by_id passed a non-numeric ID #$id", E_USER_WARNING); diff --git a/core/model/MySQLDatabase.php b/core/model/MySQLDatabase.php index 44d03ddb7..02bd3c426 100644 --- a/core/model/MySQLDatabase.php +++ b/core/model/MySQLDatabase.php @@ -38,6 +38,8 @@ class MySQLDatabase extends Database { if(!$this->dbConn) { $this->databaseError("Couldn't connect to MySQL database"); } + + $this->query("SET sql_mode = 'ANSI'"); parent::__construct(); } diff --git a/core/model/SQLQuery.php b/core/model/SQLQuery.php index cbc2f4368..243d37d7b 100755 --- a/core/model/SQLQuery.php +++ b/core/model/SQLQuery.php @@ -85,7 +85,7 @@ class SQLQuery extends Object { function __construct($select = "*", $from = array(), $where = "", $orderby = "", $groupby = "", $having = "", $limit = "") { $this->select($select); // @todo - $this->from = is_array($from) ? $from : array(str_replace('`','',$from) => $from); + $this->from = is_array($from) ? $from : array(str_replace(array('"','`'),'',$from) => $from); $this->where($where); $this->orderby($orderby); $this->groupby($groupby); @@ -130,7 +130,7 @@ class SQLQuery extends Object { * @return SQLQuery This instance */ public function from($table) { - $this->from[str_replace('`','',$table)] = $table; + $this->from[str_replace(array('"','`'),'',$table)] = $table; return $this; } @@ -141,7 +141,7 @@ class SQLQuery extends Object { * @return SQLQuery This instance */ public function leftJoin($table, $onPredicate) { - $this->from[$table] = "LEFT JOIN `$table` ON $onPredicate"; + $this->from[$table] = "LEFT JOIN \"$table\" ON $onPredicate"; return $this; } @@ -205,9 +205,9 @@ class SQLQuery extends Object { if(!array_key_exists('sort', $orderby)) user_error('SQLQuery::orderby(): Wrong format for $orderby array', E_USER_ERROR); if(isset($orderby['sort']) && !empty($orderby['sort']) && isset($orderby['dir']) && !empty($orderby['dir'])) { - $combinedOrderby = "`" . Convert::raw2sql($orderby['sort']) . "` " . Convert::raw2sql(strtoupper($orderby['dir'])); + $combinedOrderby = "\"" . Convert::raw2sql($orderby['sort']) . "\" " . Convert::raw2sql(strtoupper($orderby['dir'])); } elseif(isset($orderby['sort']) && !empty($orderby['sort'])) { - $combinedOrderby = "`" . Convert::raw2sql($orderby['sort']) . "`"; + $combinedOrderby = "\"" . Convert::raw2sql($orderby['sort']) . "\""; } else { $combinedOrderby = false; } @@ -344,6 +344,7 @@ class SQLQuery extends Object { */ function renameTable($old, $new) { $this->replaceText("`$old`", "`$new`"); + $this->replaceText("\"$old\"", "\"$new\""); } /** @@ -433,7 +434,7 @@ class SQLQuery extends Object { */ function filtersOnID() { return ($this->where && count($this->where) == 1 && - (strpos($this->where[0], ".`ID` = ") || strpos($this->where[0], ".ID = ") || strpos($this->where[0], "ID = ") ) + (strpos($query->where[0], ".\"ID\" = ") || strpos($query->where[0], ".`ID` = ") || strpos($query->where[0], ".ID = ") || strpos($query->where[0], "ID = ") ) ); } diff --git a/tests/DataObjectTest.php b/tests/DataObjectTest.php index 9512a5592..ffbf54ef4 100644 --- a/tests/DataObjectTest.php +++ b/tests/DataObjectTest.php @@ -57,7 +57,7 @@ class DataObjectTest extends SapphireTest { $this->assertEquals(8, $comments->Count()); // Test WHERE clause - $comments = DataObject::get('PageComment', 'Name="Bob"'); + $comments = DataObject::get('PageComment', "Name='Bob'"); $this->assertEquals(2, $comments->Count()); foreach($comments as $comment) { $this->assertEquals('Bob', $comment->Name); @@ -72,7 +72,7 @@ class DataObjectTest extends SapphireTest { $this->assertEquals('Joe', $comments->First()->Name); // Test join - $comments = DataObject::get('PageComment', '`SiteTree`.Title="First Page"', '', 'INNER JOIN SiteTree ON PageComment.ParentID = SiteTree.ID'); + $comments = DataObject::get('PageComment', "`SiteTree`.Title='First Page'", '', 'INNER JOIN SiteTree ON PageComment.ParentID = SiteTree.ID'); $this->assertEquals(2, $comments->Count()); $this->assertEquals('Bob', $comments->First()->Name); $this->assertEquals('Bob', $comments->Last()->Name); @@ -100,15 +100,15 @@ class DataObjectTest extends SapphireTest { $this->assertEquals($homepageID, $page->ID); // Test get_one() without caching - $comment1 = DataObject::get_one('PageComment', 'Name="Joe"', false); + $comment1 = DataObject::get_one('PageComment', "Name='Joe'", false); $comment1->Comment = "Something Else"; - $comment2 = DataObject::get_one('PageComment', 'Name="Joe"', false); + $comment2 = DataObject::get_one('PageComment', "Name='Joe'", false); $this->assertNotEquals($comment1->Comment, $comment2->Comment); // Test get_one() with caching - $comment1 = DataObject::get_one('PageComment', 'Name="Jane"', true); + $comment1 = DataObject::get_one('PageComment', "Name='Jane'", true); $comment1->Comment = "Something Else"; - $comment2 = DataObject::get_one('PageComment', 'Name="Jane"', true); + $comment2 = DataObject::get_one('PageComment', "Name='Jane'", true); $this->assertEquals((string)$comment1->Comment, (string)$comment2->Comment); // Test get_one() with order by without caching diff --git a/tests/SQLQueryTest.php b/tests/SQLQueryTest.php index 76407b4ea..0aa44ced2 100644 --- a/tests/SQLQueryTest.php +++ b/tests/SQLQueryTest.php @@ -100,13 +100,13 @@ class SQLQueryTest extends SapphireTest { $query = new SQLQuery(); $query->from[] = "MyTable"; $query->orderby(array('sort'=>'MyName')); - $this->assertEquals("SELECT * FROM MyTable ORDER BY `MyName`", $query->sql()); + $this->assertEquals('SELECT * FROM MyTable ORDER BY "MyName"', $query->sql()); // array limit with start (MySQL specific) $query = new SQLQuery(); $query->from[] = "MyTable"; $query->orderby(array('sort'=>'MyName','dir'=>'desc')); - $this->assertEquals("SELECT * FROM MyTable ORDER BY `MyName` DESC", $query->sql()); + $this->assertEquals('SELECT * FROM MyTable ORDER BY "MyName" DESC', $query->sql()); } function testSelectWithComplexOrderbyClause() { diff --git a/tests/forms/RequirementsTest.php b/tests/forms/RequirementsTest.php index b6f8791c3..6246c2831 100644 --- a/tests/forms/RequirementsTest.php +++ b/tests/forms/RequirementsTest.php @@ -44,6 +44,7 @@ class RequirementsTest extends SapphireTest { $html = Requirements::includeInHTML(false, self::$html_template); /* COMBINED JAVASCRIPT FILE IS INCLUDED IN HTML HEADER */ + Debug::message($html); $this->assertTrue((bool)preg_match('/src=".*\/RequirementsTest_bc\.js/', $html), 'combined javascript file is included in html header'); /* COMBINED JAVASCRIPT FILE EXISTS */ diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index 623192914..2ea24c34f 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -151,16 +151,16 @@ class SecurityTest extends FunctionalTest { /* UNSUCCESSFUL ATTEMPTS WITH WRONG PASSWORD FOR EXISTING USER ARE LOGGED */ $this->doTestLoginForm('sam@silverstripe.com', 'wrongpassword'); - $attempt = DataObject::get_one('LoginAttempt', 'Email = "sam@silverstripe.com"'); + $attempt = DataObject::get_one('LoginAttempt', "Email = 'sam@silverstripe.com'"); $this->assertTrue(is_object($attempt)); - $member = DataObject::get_one('Member', 'Email = "sam@silverstripe.com"'); + $member = DataObject::get_one('Member', "Email = 'sam@silverstripe.com'"); $this->assertEquals($attempt->Status, 'Failure'); $this->assertEquals($attempt->Email, 'sam@silverstripe.com'); $this->assertEquals($attempt->Member(), $member); /* UNSUCCESSFUL ATTEMPTS WITH NONEXISTING USER ARE LOGGED */ $this->doTestLoginForm('wronguser@silverstripe.com', 'wrongpassword'); - $attempt = DataObject::get_one('LoginAttempt', 'Email = "wronguser@silverstripe.com"'); + $attempt = DataObject::get_one('LoginAttempt', "Email = 'wronguser@silverstripe.com'"); $this->assertTrue(is_object($attempt)); $this->assertEquals($attempt->Status, 'Failure'); $this->assertEquals($attempt->Email, 'wronguser@silverstripe.com'); @@ -171,8 +171,8 @@ class SecurityTest extends FunctionalTest { /* SUCCESSFUL ATTEMPTS ARE LOGGED */ $this->doTestLoginForm('sam@silverstripe.com', '1nitialPassword'); - $attempt = DataObject::get_one('LoginAttempt', 'Email = "sam@silverstripe.com"'); - $member = DataObject::get_one('Member', 'Email = "sam@silverstripe.com"'); + $attempt = DataObject::get_one('LoginAttempt', "Email = 'sam@silverstripe.com'"); + $member = DataObject::get_one('Member', "Email = 'sam@silverstripe.com'"); $this->assertTrue(is_object($attempt)); $this->assertEquals($attempt->Status, 'Success'); $this->assertEquals($attempt->Email, 'sam@silverstripe.com');