BUG Revert some changes to ManyManyList

BUG Fix incompatibility in Member_GroupList
Fix regressions in merges from 3.1
BUG Fix Security failing on test classes
BUG Fix postgresql compatibility
Clarify sql encoding of table names

control/RequestProcessor.php

@@ -2,7 +2,7 @@
 
 /**
  * Represents a request processer that delegates pre and post request handling to nested request filters
- * 
+ *
  * @package framework
  * @subpackage control
  */
@@ -21,7 +21,7 @@ class RequestProcessor implements RequestFilter {
 
 	/**
 	 * Assign a list of request filters
-	 * 
+	 *
 	 * @param array $filters
 	 */
 	public function setFilters($filters) {

dev/FixtureFactory.php

@@ -91,9 +91,9 @@ class FixtureFactory {
 	public function createRaw($table, $identifier, $data) {
 		$fields = array();
 		foreach($data as $fieldName => $fieldVal) {
-			$fields["\"$fieldName\""] = $this->parseValue($fieldVal);
+			$fields["\"{$fieldName}\""] = $this->parseValue($fieldVal);
 		}
-		$insert = new SQLInsert($table, $fields);
+		$insert = new SQLInsert("\"{$table}\"", $fields);
 		$insert->execute();
 		$id = DB::get_generated_id($table);
 		$this->fixtures[$table][$identifier] = $id;

docs/en/changelogs/3.2.0.md

@@ -9,6 +9,8 @@
  * UploadField "Select from files" shows files in all folders by default
  * UploadField won't display an overwrite warning unless Upload:replaceFile is true
  * HtmlEditorField no longer substitutes `<blockquote />` for indented text
+ * ClassInfo::dataClassesFor now returns classes which should have tables, regardless of whether those
+   tables actually exist.
 
 ### CMS
 

model/ManyManyList.php

@@ -59,10 +59,16 @@ class ManyManyList extends RelationList {
 		$this->foreignKey = $foreignKey;
 		$this->extraFields = $extraFields;
 
-		$baseClass = ClassInfo::baseDataClass($dataClass);
+		$this->linkJoinTable();
+	}
 
+	/**
+	 * Setup the join between this dataobject and the necessary mapping table
+	 */
+	protected function linkJoinTable() {
 		// Join to the many-many join table
-		$this->dataQuery->innerJoin($joinTable, "\"$joinTable\".\"$this->localKey\" = \"$baseClass\".\"ID\"");
+		$baseClass = ClassInfo::baseDataClass($this->dataClass);
+		$this->dataQuery->innerJoin($this->joinTable, "\"{$this->joinTable}\".\"{$this->localKey}\" = \"{$baseClass}\".\"ID\"");
 
 		// Add the extra fields to the query
 		if($this->extraFields) {
@@ -86,12 +92,12 @@ class ManyManyList extends RelationList {
 				$this->_compositeExtraFields[$field] = array();
 
 				// append the composite field names to the select
-				foreach($obj->compositeDatabaseFields() as $k => $f) {
-					$col = $field . $k;
+				foreach($obj->compositeDatabaseFields() as $subField => $subSpec) {
+					$col = $field . $subField;
 					$finalized[] = $col;
 
 					// cache
-					$this->_compositeExtraFields[$field][] = $k;
+					$this->_compositeExtraFields[$field][] = $subField;
 				}
 			} else {
 				$finalized[] = $field;
@@ -113,16 +119,15 @@ class ManyManyList extends RelationList {
 
 		if($this->_compositeExtraFields) {
 			foreach($this->_compositeExtraFields as $fieldName => $composed) {
-				// convert joined extra fields into their composite field
-				// types.
+				// convert joined extra fields into their composite field types.
 				$value = array();
 
-				foreach($composed as $i => $k) {
-					if(isset($row[$fieldName . $k])) {
-						$value[$k] = $row[$fieldName . $k];
+				foreach($composed as $subField => $subSpec) {
+					if(isset($row[$fieldName . $subSpec])) {
+						$value[$subSpec] = $row[$fieldName . $subSpec];
 
 						// don't duplicate data in the record
-						unset($row[$fieldName . $k]);
+						unset($row[$fieldName . $subSpec]);
 					}
 				}
 
@@ -156,7 +161,7 @@ class ManyManyList extends RelationList {
 		}
 
 		// Apply relation filter
-		$key = "\"$this->joinTable\".\"$this->foreignKey\"";
+		$key = "\"{$this->joinTable}\".\"{$this->foreignKey}\"";
 		if(is_array($id)) {
 			return array("$key IN (".DB::placeholders($id).")"  => $id);
 		} else if($id !== null){
@@ -184,7 +189,8 @@ class ManyManyList extends RelationList {
 	 * Does so by adding an entry to the joinTable.
 	 * 
 	 * @param mixed $item
-	 * @param $extraFields A map of additional columns to insert into the joinTable
+	 * @param array $extraFields A map of additional columns to insert into the joinTable.
+	 * Column names should be ANSI quoted.
 	 */
 	public function add($item, $extraFields = array()) {
 		// Ensure nulls or empty strings are correctly treated as empty arrays
@@ -208,43 +214,61 @@ class ManyManyList extends RelationList {
 
 		// Apply this item to each given foreign ID record
 		if(!is_array($foreignIDs)) $foreignIDs = array($foreignIDs);
-		$baseClass = ClassInfo::baseDataClass($this->dataClass);
 		foreach($foreignIDs as $foreignID) {
-			
 			// Check for existing records for this item
-			if($foreignFilter = $this->foreignIDFilter($foreignID)) {
+			if($foreignFilter = $this->foreignIDWriteFilter($foreignID)) {
 				// With the current query, simply add the foreign and local conditions
 				// The query can be a bit odd, especially if custom relation classes
 				// don't join expected tables (@see Member_GroupSet for example).
-				$query = $this->dataQuery->query()->toSelect();
+				$query = new SQLSelect("*", "\"{$this->joinTable}\"");
 				$query->addWhere($foreignFilter);
 				$query->addWhere(array(
-					"\"$baseClass\".\"ID\"" => $itemID
+					"\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID
 				));
 				$hasExisting = ($query->count() > 0);
 			} else {
 				$hasExisting = false;	
 			}
 			
-			// Determine entry type
-			if(!$hasExisting) {
-				// Field values for new record
-				$fieldValues = array_merge($extraFields, array(
-					"\"{$this->foreignKey}\"" => $foreignID,
-					"\"{$this->localKey}\"" => $itemID
-				));
-				// Create new record
-				$insert = new SQLInsert("\"{$this->joinTable}\"", $fieldValues);
-				$insert->execute();
-			} elseif(!empty($extraFields)) {
-				// For existing records, simply update any extra data supplied
-				$foreignWriteFilter = $this->foreignIDWriteFilter($foreignID);
-				$update = new SQLUpdate("\"{$this->joinTable}\"", $extraFields, $foreignWriteFilter);
-				$update->addWhere(array(
+			$manipulation = array();
+			if($hasExisting) {
+				$manipulation[$this->joinTable]['command'] = 'update';
+				$manipulation[$this->joinTable]['where'] = array(
+					"\"{$this->joinTable}\".\"{$this->foreignKey}\"" => $foreignID,
 					"\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID
-				));
-				$update->execute();
+				);
+			} else {
+				$manipulation[$this->joinTable]['command'] = 'insert';
 			}
+
+			if($extraFields) {
+				foreach($extraFields as $fieldName => $fieldValue) {
+					if(is_null($fieldValue)) {
+						$manipulation[$this->joinTable]['fields'][$fieldName] = null;
+					} elseif($fieldValue instanceof DBField) {
+						// rely on writeToManipulation to manage the changes
+						// required for this field.
+						$working = array('fields' => array());
+
+						// create a new instance of the field so we can
+						// modify the field name to the correct version.
+						$field = DBField::create_field(get_class($fieldValue), $fieldValue);
+						$field->setName($fieldName);
+						$field->writeToManipulation($working);
+
+						foreach($working['fields'] as $extraName => $extraValue) {
+							$manipulation[$this->joinTable]['fields'][$extraName] = $extraValue;
+						}
+					} else {
+						$manipulation[$this->joinTable]['fields'][$fieldName] =  $fieldValue;
+					}
+				}
+			}
+
+			$manipulation[$this->joinTable]['fields'][$this->localKey] = $itemID;
+			$manipulation[$this->joinTable]['fields'][$this->foreignKey] = $foreignID;
+
+			DB::manipulate($manipulation);
 		}
 	}
 
@@ -275,7 +299,7 @@ class ManyManyList extends RelationList {
 	public function removeByID($itemID) {
 		if(!is_numeric($itemID)) throw new InvalidArgumentException("ManyManyList::removeById() expecting an ID");
 
-		$query = new SQLDelete("\"$this->joinTable\"");
+		$query = new SQLDelete("\"{$this->joinTable}\"");
 
 		if($filter = $this->foreignIDWriteFilter($this->getForeignID())) {
 			$query->setWhere($filter);
@@ -283,7 +307,7 @@ class ManyManyList extends RelationList {
 			user_error("Can't call ManyManyList::remove() until a foreign ID is set", E_USER_WARNING);
 		}
 		
-		$query->addWhere(array("\"$this->localKey\"" => $itemID));
+		$query->addWhere(array("\"{$this->localKey}\"" => $itemID));
 		$query->execute();
 	}
 
@@ -302,7 +326,7 @@ class ManyManyList extends RelationList {
 		$query->removeFilterOn($foreignFilter);
 
 		$selectQuery = $query->query();
-		$selectQuery->setSelect("\"$base\".\"ID\"");
+		$selectQuery->setSelect("\"{$base}\".\"ID\"");
 
 		$from = $selectQuery->getFrom();
 		unset($from[$this->joinTable]);
@@ -313,11 +337,11 @@ class ManyManyList extends RelationList {
 		// Use a sub-query as SQLite does not support setting delete targets in
 		// joined queries.
 		$delete = new SQLDelete();
-		$delete->setFrom("\"$this->joinTable\"");
+		$delete->setFrom("\"{$this->joinTable}\"");
 		$delete->addWhere($this->foreignIDFilter());
 		$subSelect = $selectQuery->sql($parameters);
 		$delete->addWhere(array(
-			"\"$this->joinTable\".\"$this->localKey\" IN ($subSelect)" => $parameters
+			"\"{$this->joinTable}\".\"{$this->localKey}\" IN ($subSelect)" => $parameters
 		));
 		$delete->execute();
 	}
@@ -341,13 +365,13 @@ class ManyManyList extends RelationList {
 		// @todo Optimize into a single query instead of one per extra field
 		if($this->extraFields) {
 			foreach($this->extraFields as $fieldName => $dbFieldSpec) {
-				$query = new SQLSelect("\"$fieldName\"", "\"$this->joinTable\"");
+				$query = new SQLSelect("\"{$fieldName}\"", "\"{$this->joinTable}\"");
 				if($filter = $this->foreignIDWriteFilter($this->getForeignID())) {
 					$query->setWhere($filter);
 				} else {
 					user_error("Can't call ManyManyList::getExtraData() until a foreign ID is set", E_USER_WARNING);
 				}
-				$query->addWhere("\"$this->localKey\" = {$itemID}");
+				$query->addWhere("\"{$this->localKey}\" = {$itemID}");
 				$result[$fieldName] = $query->execute()->value();
 			}
 		}

model/connect/DBSchemaManager.php

@@ -130,16 +130,17 @@ abstract class DBSchemaManager {
 
 			// End schema update
 			foreach ($this->schemaUpdateTransaction as $tableName => $changes) {
+				$advancedOptions = isset($changes['advancedOptions']) ? $changes['advancedOptions'] : null;
 				switch ($changes['command']) {
 					case 'create':
 						$this->createTable($tableName, $changes['newFields'], $changes['newIndexes'],
-										$changes['options'], @$changes['advancedOptions']);
+										$changes['options'], $advancedOptions);
 						break;
 
 					case 'alter':
 						$this->alterTable($tableName, $changes['newFields'], $changes['newIndexes'],
 										$changes['alteredFields'], $changes['alteredIndexes'],
-										$changes['alteredOptions'], @$changes['advancedOptions']);
+										$changes['alteredOptions'], $advancedOptions);
 						break;
 				}
 			}

model/queries/SQLConditionalExpression.php

@@ -54,10 +54,10 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	/**
 	 * Sets the list of tables to query from or update
 	 *
-	 * @example $query->setFrom("MyTable"); // SELECT * FROM MyTable
+	 * @example $query->setFrom('"MyTable"'); // SELECT * FROM "MyTable"
 	 *
-	 * @param string|array $from Escaped SQL statement, usually an unquoted table name
-	 * @return SQLSelect
+	 * @param string|array $from Single, or list of, ANSI quoted table names
+	 * @return self
 	 */
 	public function setFrom($from) {
 		$this->from = array();
@@ -75,9 +75,9 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	/**
 	 * Add a table to include in the query or update
 	 *
-	 * @example $query->addFrom("MyTable"); // UPDATE MyTable
+	 * @example $query->addFrom('"MyTable"'); // SELECT * FROM "MyTable"
 	 *
-	 * @param string|array $from Escaped SQL statement, usually an unquoted table name
+	 * @param string|array $from Single, or list of, ANSI quoted table names
 	 * @return self Self reference
 	 */
 	public function addFrom($from) {
@@ -189,7 +189,7 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	/**
 	 * Add an additional filter (part of the ON clause) on a join.
 	 *
-	 * @param string $table Table to join on from the original join
+	 * @param string $table Table to join on from the original join (unquoted)
 	 * @param string $filter The "ON" SQL fragment (escaped)
 	 * @return self Self reference
 	 */
@@ -201,7 +201,7 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	/**
 	 * Set the filter (part of the ON clause) on a join.
 	 *
-	 * @param string $table Table to join on from the original join
+	 * @param string $table Table to join on from the original join (unquoted)
 	 * @param string $filter The "ON" SQL fragment (escaped)
 	 * @return self Self reference
 	 */
@@ -213,6 +213,7 @@ abstract class SQLConditionalExpression extends SQLExpression {
 	/**
 	 * Returns true if we are already joining to the given table alias
 	 * 
+	 * @param string $tableAlias Table name
 	 * @return boolean
 	 */
 	public function isJoinedTo($tableAlias) {

model/queries/SQLDelete.php

@@ -23,6 +23,7 @@ class SQLDelete extends SQLConditionalExpression {
 	 * Construct a new SQLDelete.
 	 *
 	 * @param array|string $from An array of Tables (FROM clauses). The first one should be just the table name.
+	 * Each should be ANSI quoted.
 	 * @param array $where An array of WHERE clauses.
 	 * @param array|string $delete The table(s) to delete, if multiple tables are queried from
 	 * @return self Self reference
@@ -35,6 +36,7 @@ class SQLDelete extends SQLConditionalExpression {
 	 * Construct a new SQLDelete.
 	 *
 	 * @param array|string $from An array of Tables (FROM clauses). The first one should be just the table name.
+	 * Each should be ANSI quoted.
 	 * @param array $where An array of WHERE clauses.
 	 * @param array|string $delete The table(s) to delete, if multiple tables are queried from
 	 */

model/queries/SQLInsert.php

@@ -26,7 +26,7 @@ class SQLInsert extends SQLExpression implements SQLWriteExpression {
 	/**
 	 * Construct a new SQLInsert object
 	 *
-	 * @param string $into Table name to insert into
+	 * @param string $into Table name to insert into (ANSI quoted)
 	 * @param array $assignments List of column assignments
 	 * @return static
 	 */
@@ -37,7 +37,7 @@ class SQLInsert extends SQLExpression implements SQLWriteExpression {
 	/**
 	 * Construct a new SQLInsert object
 	 *
-	 * @param string $into Table name to insert into
+	 * @param string $into Table name to insert into (ANSI quoted)
 	 * @param array $assignments List of column assignments
 	 */
 	function __construct($into = null, $assignments = array()) {
@@ -50,7 +50,7 @@ class SQLInsert extends SQLExpression implements SQLWriteExpression {
 	/**
 	 * Sets the table name to insert into
 	 *
-	 * @param string $into Single table name
+	 * @param string $into Single table name (ANSI quoted)
 	 * @return self The self reference to this query
 	 */
 	public function setInto($into) {

model/queries/SQLSelect.php

@@ -63,6 +63,7 @@ class SQLSelect extends SQLConditionalExpression {
 	 *
 	 * @param array $select An array of SELECT fields.
 	 * @param array|string $from An array of FROM clauses. The first one should be just the table name.
+	 * Each should be ANSI quoted.
 	 * @param array $where An array of WHERE clauses.
 	 * @param array $orderby An array ORDER BY clause.
 	 * @param array $groupby An array of GROUP BY clauses.
@@ -80,6 +81,7 @@ class SQLSelect extends SQLConditionalExpression {
 	 *
 	 * @param array $select An array of SELECT fields.
 	 * @param array|string $from An array of FROM clauses. The first one should be just the table name.
+	 * Each should be ANSI quoted.
 	 * @param array $where An array of WHERE clauses.
 	 * @param array $orderby An array ORDER BY clause.
 	 * @param array $groupby An array of GROUP BY clauses.

model/queries/SQLUpdate.php

@@ -19,7 +19,7 @@ class SQLUpdate extends SQLConditionalExpression implements SQLWriteExpression {
 	/**
 	 * Construct a new SQLUpdate object
 	 *
-	 * @param string $table Table name to update
+	 * @param string $table Table name to update (ANSI quoted)
 	 * @param array $assignment List of column assignments
 	 * @param array $where List of where clauses
 	 * @return static
@@ -31,7 +31,7 @@ class SQLUpdate extends SQLConditionalExpression implements SQLWriteExpression {
 	/**
 	 * Construct a new SQLUpdate object
 	 *
-	 * @param string $table Table name to update
+	 * @param string $table Table name to update (ANSI quoted)
 	 * @param array $assignment List of column assignments
 	 * @param array $where List of where clauses
 	 */

security/Member.php

@@ -1497,15 +1497,11 @@ class Member extends DataObject implements TemplateGlobalProvider {
  */
 class Member_GroupSet extends ManyManyList {
 	
-	public function __construct($dataClass, $joinTable, $localKey, $foreignKey, $extraFields = array()) {
-		
-		// Bypass the many-many constructor
-		DataList::__construct($dataClass);
-
-		$this->joinTable = $joinTable;
-		$this->localKey = $localKey;
-		$this->foreignKey = $foreignKey;
-		$this->extraFields = $extraFields;
+	protected function linkJoinTable() {
+		// Do not join the table directly
+		if($this->extraFields) {
+			user_error('Member_GroupSet does not support many_many_extraFields', E_USER_ERROR);
+		}
 	}
 	
 	/**

security/Security.php

@@ -937,6 +937,9 @@ class Security extends Controller implements TemplateGlobalProvider {
 		$requiredTables[] = 'Permission';
 		
 		foreach($requiredTables as $table) {
+			// Skip test classes, as not all test classes are scaffolded at once
+			if(is_subclass_of($table, 'TestOnly')) continue;
+
 			// if any of the tables aren't created in the database
 			if(!ClassInfo::hasTable($table)) return false;
 

tests/model/ManyManyListTest.php

@@ -253,8 +253,8 @@ class ManyManyListTest extends SapphireTest {
 		$db = DB::getConn();
 		$expected = 'SELECT DISTINCT "ManyManyListTest_ExtraFields_Clients"."WorthCurrency",'
 			.' "ManyManyListTest_ExtraFields_Clients"."WorthAmount", "ManyManyListTest_ExtraFields_Clients"."Reference",'
-			.' "ManyManyListTest_ExtraFields"."ClassName", "ManyManyListTest_ExtraFields"."Created",'
-			.' "ManyManyListTest_ExtraFields"."LastEdited", "ManyManyListTest_ExtraFields"."ID",'
+			.' "ManyManyListTest_ExtraFields"."ClassName", "ManyManyListTest_ExtraFields"."LastEdited",'
+			.' "ManyManyListTest_ExtraFields"."Created", "ManyManyListTest_ExtraFields"."ID",'
 			.' CASE WHEN "ManyManyListTest_ExtraFields"."ClassName" IS NOT NULL THEN'
 			.' "ManyManyListTest_ExtraFields"."ClassName" ELSE '. $db->prepStringForDB('ManyManyListTest_ExtraFields')
 			.' END AS "RecordClassName" FROM "ManyManyListTest_ExtraFields" INNER JOIN'
@@ -262,7 +262,7 @@ class ManyManyListTest extends SapphireTest {
 			.' "ManyManyListTest_ExtraFields_Clients"."ManyManyListTest_ExtraFieldsID" ='
 			.' "ManyManyListTest_ExtraFields"."ID"';
 
-		$this->assertEquals($expected, $list->sql());
+		$this->assertSQLEquals($expected, $list->sql($parameters));
 	}