diff --git a/docs/en/changelogs/3.1.0.md b/docs/en/changelogs/3.1.0.md index 38e3a0271..95ebdc5dc 100644 --- a/docs/en/changelogs/3.1.0.md +++ b/docs/en/changelogs/3.1.0.md @@ -18,7 +18,7 @@ including `Email_BounceHandler` and `Email_BounceRecord` classes, as well as the `Member->Bounced` property. * Deprecated global email methods `htmlEmail()` and `plaintextEmail`, as well as various email helper methods like `encodeMultipart()`. Use the `Email` API, or the `Mailer` class where applicable. - * Removed non-functional `$inlineImages` option for sending emails + * Removed non-functional `$inlineImages` option for sending emails * Removed support for keyed arrays in `SelectionGroup`, use new `SelectionGroup_Item` object to populate the list instead (see [API docs](api:SelectionGroup)). * Removed `Form->Name()`: Use getName() @@ -29,4 +29,20 @@ * Removed `Member->sendInfo()`: use Member_ChangePasswordEmail or Member_ForgotPasswordEmail directly * Removed `SQLMap::map()`: Use DataList::("Member")->map() * Removed `SQLMap::mapInGroups()`: Use Member::map_in_groups() - * Removed `PasswordEncryptor::register()/unregister()`: Use config system instead \ No newline at end of file + * Removed `PasswordEncryptor::register()/unregister()`: Use config system instead + * Methods on DataList and ArrayList that used to both modify the existing list & return a new version now just return a new version. Make sure you change statements like `$list->filter(...)` to $`list = $list->filter(...)` for these methods: + - `ArrayList#reverse` + - `ArrayList#sort` + - `ArrayList#filter` + - `ArrayList#exclude` + - `DataList#where` + - `DataList#limit` + - `DataList#sort` + - `DataList#addFilter` + - `DataList#applyFilterContext` + - `DataList#innerJoin` + - `DataList#leftJoin` + - `DataList#find` + - `DataList#byIDs` + - `DataList#reverse` + * `DataList#dataQuery` has been changed to return a clone of the query, and so can't be used to modify the list's query directly. Use `DataList#alterDataQuery` instead to modify dataQuery in a safe manner. diff --git a/model/ArrayList.php b/model/ArrayList.php index dccd6b3e2..f84de0a56 100644 --- a/model/ArrayList.php +++ b/model/ArrayList.php @@ -2,6 +2,17 @@ /** * A list object that wraps around an array of objects or arrays. * + * Note that (like DataLists), the implementations of the methods from SS_Filterable, SS_Sortable and + * SS_Limitable return a new instance of ArrayList, rather than modifying the existing instance. + * + * For easy reference, methods that operate in this way are: + * + * - limit + * - reverse + * - sort + * - filter + * - exclude + * * @package framework * @subpackage model */ @@ -309,8 +320,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta * @return ArrayList */ public function reverse() { - // TODO 3.1: This currently mutates existing array - $list = /* clone */ $this; + $list = clone $this; $list->items = array_reverse($this->items); return $list; @@ -376,8 +386,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta $multisortArgs[] = &$sortDirection[$column]; } - // TODO 3.1: This currently mutates existing array - $list = /* clone */ $this; + $list = clone $this; // As the last argument we pass in a reference to the items that all the sorting will be applied upon $multisortArgs[] = &$list->items; call_user_func_array('array_multisort', $multisortArgs); @@ -440,8 +449,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta } } - // TODO 3.1: This currently mutates existing array - $list = /* clone */ $this; + $list = clone $this; $list->items = $itemsToKeep; return $list; } @@ -504,8 +512,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta } } - // TODO 3.1: This currently mutates existing array - $list = /* clone */ $this; + $list = clone $this; $list->items = $itemsToKeep; return $list; } diff --git a/model/DataList.php b/model/DataList.php index b39b32529..8734e1563 100644 --- a/model/DataList.php +++ b/model/DataList.php @@ -3,21 +3,21 @@ * Implements a "lazy loading" DataObjectSet. * Uses {@link DataQuery} to do the actual query generation. * - * todo 3.1: In 3.0 the below is not currently true for backwards compatible reasons, but code should not rely on - * current behaviour. + * DataLists are _immutable_ as far as the query they represent is concerned. When you call a method that + * alters the query, a new DataList instance is returned, rather than modifying the existing instance * - * DataLists have two sets of methods. + * When you add or remove an element to the list the query remains the same, but because you have modified + * the underlying data the contents of the list changes. These are some of those methods: * - * 1). Selection methods (SS_Filterable, SS_Sortable, SS_Limitable) change the way the list is built, but does not - * alter underlying data. There are no external affects from selection methods once this list instance is - * destructed. + * - add + * - addMany + * - remove + * - removeMany + * - removeByID + * - removeByFilter + * - removeAll * - * 2). Mutation methods change the underlying data. The change persists into the underlying data storage layer. - * - * DataLists are _immutable_ as far as selection methods go - they all return new instances of DataList, rather - * than change the current list. - * - * DataLists are _mutable_ as far as mutation methods go - they all act on the existing DataList instance. + * Subclasses of DataList may add other methods that have the same effect. * * @package framework * @subpackage model @@ -85,17 +85,13 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab /** * Return a copy of the internal {@link DataQuery} object * - * todo 3.1: In 3.0 the below is not currently true for backwards compatible reasons, but code should not rely on - * this - * * Because the returned value is a copy, modifying it won't affect this list's contents. If * you want to alter the data query directly, use the alterDataQuery method * * @return DataQuery */ public function dataQuery() { - // TODO 3.1: This method potentially mutates self - return /* clone */ $this->dataQuery; + return clone $this->dataQuery; } /** @@ -122,7 +118,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab if ($this->inAlterDataQueryCall) { $list = $this; - $res = $callback($list->dataQuery, $list); + $res = call_user_func($callback, $list->dataQuery, $list); if ($res) $list->dataQuery = $res; return $list; @@ -132,7 +128,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab $list->inAlterDataQueryCall = true; try { - $res = $callback($list->dataQuery, $list); + $res = call_user_func($callback, $list->dataQuery, $list); if ($res) $list->dataQuery = $res; } catch (Exception $e) { @@ -145,39 +141,6 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab } } - /** - * In 3.0.0 some methods in DataList mutate their list. We don't want to change that in the 3.0.x - * line, but we don't want people relying on it either. This does the same as alterDataQuery, but - * _does_ mutate the existing list. - * - * todo 3.1: All methods that call this need to call alterDataQuery instead - */ - protected function alterDataQuery_30($callback) { - Deprecation::notice('3.1', 'DataList will become immutable in 3.1'); - - if ($this->inAlterDataQueryCall) { - $res = $callback($this->dataQuery, $this); - if ($res) $this->dataQuery = $res; - - return $this; - } - else { - $this->inAlterDataQueryCall = true; - - try { - $res = $callback($this->dataQuery, $this); - if ($res) $this->dataQuery = $res; - } - catch (Exception $e) { - $this->inAlterDataQueryCall = false; - throw $e; - } - - $this->inAlterDataQueryCall = false; - return $this; - } - } - /** * Return a new DataList instance with the underlying {@link DataQuery} object changed * @@ -190,6 +153,21 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab return $clone; } + public function setDataQueryParam($keyOrArray, $val = null) { + $clone = clone $this; + + if(is_array($keyOrArray)) { + foreach($keyOrArray as $key => $val) { + $clone->dataQuery->setQueryParam($key, $val); + } + } + else { + $clone->dataQuery->setQueryParam($keyOrArray, $val); + } + + return $clone; + } + /** * Returns the SQL query that will be used to get this DataList's records. Good for debugging. :-) * @@ -206,7 +184,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab * @return DataList */ public function where($filter) { - return $this->alterDataQuery_30(function($query) use ($filter){ + return $this->alterDataQuery(function($query) use ($filter){ $query->where($filter); }); } @@ -243,7 +221,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab if(!$limit && !$offset) { return $this; } - return $this->alterDataQuery_30(function($query) use ($limit, $offset){ + return $this->alterDataQuery(function($query) use ($limit, $offset){ $query->limit($limit, $offset); }); } @@ -281,7 +259,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab $sort = func_get_arg(0); } - return $this->alterDataQuery_30(function($query, $list) use ($sort, $col, $dir){ + return $this->alterDataQuery(function($query, $list) use ($sort, $col, $dir){ if ($col) { // sort('Name','Desc') @@ -346,25 +324,24 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab throw new InvalidArgumentException('Incorrect number of arguments passed to filter()'); } - // TODO 3.1: Once addFilter doesn't mutate self, this results in a double clone - $clone = clone $this; - $clone->addFilter($filters); - return $clone; + return $this->addFilter($filters); } /** * Return a new instance of the list with an added filter */ public function addFilter($filterArray) { + $list = $this; + foreach($filterArray as $field => $value) { $fieldArgs = explode(':', $field); $field = array_shift($fieldArgs); $filterType = array_shift($fieldArgs); $modifiers = $fieldArgs; - $this->applyFilterContext($field, $filterType, $modifiers, $value); + $list = $list->applyFilterContext($field, $filterType, $modifiers, $value); } - return $this; + return $list; } /** @@ -485,7 +462,6 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab * @todo Deprecated SearchContexts and pull their functionality into the core of the ORM */ private function applyFilterContext($field, $comparisators, $modifiers, $value) { - $t = singleton($this->dataClass())->dbObject($field); if($comparisators) { $className = "{$comparisators}Filter"; } else { @@ -496,7 +472,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab array_unshift($modifiers, $comparisators); } $t = new $className($field, $value, $modifiers); - $t->apply($this->dataQuery()); + + return $this->alterDataQuery(array($t, 'apply')); } /** @@ -581,7 +558,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab * @return DataList */ public function innerJoin($table, $onClause, $alias = null) { - return $this->alterDataQuery_30(function($query) use ($table, $onClause, $alias){ + return $this->alterDataQuery(function($query) use ($table, $onClause, $alias){ $query->innerJoin($table, $onClause, $alias); }); } @@ -595,7 +572,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab * @return DataList */ public function leftJoin($table, $onClause, $alias = null) { - return $this->alterDataQuery_30(function($query) use ($table, $onClause, $alias){ + return $this->alterDataQuery(function($query) use ($table, $onClause, $alias){ $query->leftJoin($table, $onClause, $alias); }); } @@ -810,9 +787,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab $SQL_col = sprintf('"%s"', Convert::raw2sql($key)); } - // todo 3.1: In 3.1 where won't be mutating, so this can be on $this directly - $clone = clone $this; - return $clone->where("$SQL_col = '" . Convert::raw2sql($value) . "'")->First(); + return $this->where("$SQL_col = '" . Convert::raw2sql($value) . "'")->First(); } /** @@ -836,9 +811,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab public function byIDs(array $ids) { $ids = array_map('intval', $ids); // sanitize $baseClass = ClassInfo::baseDataClass($this->dataClass); - $this->where("\"$baseClass\".\"ID\" IN (" . implode(',', $ids) .")"); - - return $this; + return $this->where("\"$baseClass\".\"ID\" IN (" . implode(',', $ids) .")"); } /** @@ -849,10 +822,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab */ public function byID($id) { $baseClass = ClassInfo::baseDataClass($this->dataClass); - - // todo 3.1: In 3.1 where won't be mutating, so this can be on $this directly - $clone = clone $this; - return $clone->where("\"$baseClass\".\"ID\" = " . (int)$id)->First(); + return $this->where("\"$baseClass\".\"ID\" = " . (int)$id)->First(); } /** @@ -1028,7 +998,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab * @return DataList */ public function reverse() { - return $this->alterDataQuery_30(function($query){ + return $this->alterDataQuery(function($query){ $query->reverseSort(); }); } diff --git a/model/DataObject.php b/model/DataObject.php index 6c271d81c..962533bc5 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -2722,9 +2722,9 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if($limit && strpos($limit, ',') !== false) { $limitArguments = explode(',', $limit); - $result->limit($limitArguments[1],$limitArguments[0]); + $result = $result->limit($limitArguments[1],$limitArguments[0]); } elseif($limit) { - $result->limit($limit); + $result = $result->limit($limit); } if($join) $result = $result->join($join); diff --git a/model/Filterable.php b/model/Filterable.php index a508f65ef..72d0d918f 100644 --- a/model/Filterable.php +++ b/model/Filterable.php @@ -19,8 +19,9 @@ interface SS_Filterable { public function canFilterBy($by); /** - * Filter the list to include items with these charactaristics - * + * Return a new instance of this list that only includes items with these charactaristics + * + * @return SS_Filterable * @example $list = $list->filter('Name', 'bob'); // only bob in the list * @example $list = $list->filter('Name', array('aziz', 'bob'); // aziz and bob in list * @example $list = $list->filter(array('Name'=>'bob, 'Age'=>21)); // bob with the age 21 @@ -31,8 +32,9 @@ interface SS_Filterable { public function filter(); /** - * Exclude the list to not contain items with these charactaristics + * Return a new instance of this list that excludes any items with these charactaristics * + * @return SS_Filterable * @example $list = $list->exclude('Name', 'bob'); // exclude bob from list * @example $list = $list->exclude('Name', array('aziz', 'bob'); // exclude aziz and bob from list * @example $list = $list->exclude(array('Name'=>'bob, 'Age'=>21)); // exclude bob that has Age 21 diff --git a/model/HasManyList.php b/model/HasManyList.php index c39efceb4..8d30a5482 100644 --- a/model/HasManyList.php +++ b/model/HasManyList.php @@ -21,14 +21,16 @@ class HasManyList extends RelationList { $this->foreignKey = $foreignKey; } - protected function foreignIDFilter() { + protected function foreignIDFilter($id = null) { + if ($id === null) $id = $this->getForeignID(); + // Apply relation filter - if(is_array($this->foreignID)) { - return "\"$this->foreignKey\" IN ('" . - implode("', '", array_map('Convert::raw2sql', $this->foreignID)) . "')"; - } else if($this->foreignID !== null){ + if(is_array($id)) { + return "\"$this->foreignKey\" IN ('" . + implode("', '", array_map('Convert::raw2sql', $id)) . "')"; + } else if($id !== null){ return "\"$this->foreignKey\" = '" . - Convert::raw2sql($this->foreignID) . "'"; + Convert::raw2sql($id) . "'"; } } @@ -44,18 +46,20 @@ class HasManyList extends RelationList { user_error("HasManyList::add() expecting a $this->dataClass object, or ID value", E_USER_ERROR); } + $foreignID = $this->getForeignID(); + // Validate foreignID - if(!$this->foreignID) { + if(!$foreignID) { user_error("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING); return; } - if(is_array($this->foreignID)) { + if(is_array($foreignID)) { user_error("ManyManyList::add() can't be called on a list linked to mulitple foreign IDs", E_USER_WARNING); return; } $fk = $this->foreignKey; - $item->$fk = $this->foreignID; + $item->$fk = $foreignID; $item->write(); } diff --git a/model/Hierarchy.php b/model/Hierarchy.php index 4fa8f98d9..117c3dd95 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -591,12 +591,13 @@ class Hierarchy extends DataExtension { $id = $this->owner->ID; $children = DataObject::get($baseClass) - ->where("\"{$baseClass}\".\"ParentID\" = $id AND \"{$baseClass}\".\"ID\" != $id"); - if(!$showAll) $children = $children->where('"ShowInMenus" = 1'); + ->where("\"{$baseClass}\".\"ParentID\" = $id AND \"{$baseClass}\".\"ID\" != $id") + ->setDataQueryParam(array( + 'Versioned.mode' => $onlyDeletedFromStage ? 'stage_unique' : 'stage', + 'Versioned.stage' => 'Live' + )); - // Query the live site - $children->dataQuery()->setQueryParam('Versioned.mode', $onlyDeletedFromStage ? 'stage_unique' : 'stage'); - $children->dataQuery()->setQueryParam('Versioned.stage', 'Live'); + if(!$showAll) $children = $children->where('"ShowInMenus" = 1'); return $children; } diff --git a/model/Limitable.php b/model/Limitable.php index 8e062de1e..633762d2a 100644 --- a/model/Limitable.php +++ b/model/Limitable.php @@ -11,11 +11,11 @@ interface SS_Limitable { /** - * Returns a filtered version of this where no more than $limit records are included. + * Returns a new instance of this list where no more than $limit records are included. * If $offset is specified, then that many records at the beginning of the list will be skipped. * This matches the behaviour of the SQL LIMIT clause. * - * @return SS_List + * @return SS_Limitable */ public function limit($limit, $offset = 0); diff --git a/model/ManyManyList.php b/model/ManyManyList.php index 1cfb5069c..7b7ecd90e 100644 --- a/model/ManyManyList.php +++ b/model/ManyManyList.php @@ -9,7 +9,7 @@ class ManyManyList extends RelationList { protected $localKey; - protected $foreignKey, $foreignID; + protected $foreignKey; protected $extraFields; @@ -48,19 +48,35 @@ class ManyManyList extends RelationList { } /** - * Return a filter expression for the foreign ID. + * Return a filter expression for when getting the contents of the relationship for some foreign ID + * @return string */ - protected function foreignIDFilter() { + protected function foreignIDFilter($id = null) { + if ($id === null) $id = $this->getForeignID(); + // Apply relation filter - if(is_array($this->foreignID)) { + if(is_array($id)) { return "\"$this->joinTable\".\"$this->foreignKey\" IN ('" . - implode("', '", array_map('Convert::raw2sql', $this->foreignID)) . "')"; - } else if($this->foreignID !== null){ + implode("', '", array_map('Convert::raw2sql', $id)) . "')"; + } else if($id !== null){ return "\"$this->joinTable\".\"$this->foreignKey\" = '" . - Convert::raw2sql($this->foreignID) . "'"; + Convert::raw2sql($id) . "'"; } } + /** + * Return a filter expression for the join table when writing to the join table + * + * When writing (add, remove, removeByID), we need to filter the join table to just the relevant + * entries. However some subclasses of ManyManyList (Member_GroupSet) modify foreignIDFilter to + * include additional calculated entries, so we need different filters when reading and when writing + * + * @return string + */ + protected function foreignIDWriteFilter($id = null) { + return $this->foreignIDFilter($id); + } + /** * Add an item to this many_many relationship * Does so by adding an entry to the joinTable. @@ -73,22 +89,25 @@ class ManyManyList extends RelationList { throw new InvalidArgumentException("ManyManyList::add() expecting a $this->dataClass object, or ID value", E_USER_ERROR); } - + + $foreignIDs = $this->getForeignID(); + $foreignFilter = $this->foreignIDWriteFilter(); + // Validate foreignID - if(!$this->foreignID) { + if(!$foreignIDs) { throw new Exception("ManyManyList::add() can't be called until a foreign ID is set", E_USER_WARNING); } - if($filter = $this->foreignIDFilter()) { + if($foreignFilter) { $query = new SQLQuery("*", array("\"$this->joinTable\"")); - $query->setWhere($filter); + $query->setWhere($foreignFilter); $hasExisting = ($query->count() > 0); } else { $hasExisting = false; } // Insert or update - foreach((array)$this->foreignID as $foreignID) { + foreach((array)$foreignIDs as $foreignID) { $manipulation = array(); if($hasExisting) { $manipulation[$this->joinTable]['command'] = 'update'; @@ -134,8 +153,8 @@ class ManyManyList extends RelationList { $query = new SQLQuery("*", array("\"$this->joinTable\"")); $query->setDelete(true); - - if($filter = $this->foreignIDFilter()) { + + if($filter = $this->foreignIDWriteFilter($this->getForeignID())) { $query->setWhere($filter); } else { user_error("Can't call ManyManyList::remove() until a foreign ID is set", E_USER_WARNING); @@ -177,7 +196,7 @@ class ManyManyList extends RelationList { if($this->extraFields) { foreach($this->extraFields as $fieldName => $dbFieldSpec) { $query = new SQLQuery("\"$fieldName\"", array("\"$this->joinTable\"")); - if($filter = $this->foreignIDFilter()) { + 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); diff --git a/model/RelationList.php b/model/RelationList.php index a09366b56..5e11f5180 100644 --- a/model/RelationList.php +++ b/model/RelationList.php @@ -7,44 +7,44 @@ * @todo Is this additional class really necessary? */ abstract class RelationList extends DataList { - protected $foreignID; - - /** - * Set the ID of the record that this ManyManyList is linking *from*. - * - * This is the mutatable version of this function, and will be protected only - * from 3.1. Use forForeignID instead - * - * @param $id A single ID, or an array of IDs - */ - public function setForeignID($id) { - // If already filtered on foreign ID, remove that first - if($this->foreignID !== null) { - $oldFilter = $this->foreignIDFilter(); - try { - $this->dataQuery->removeFilterOn($oldFilter); - } - catch(InvalidArgumentException $e) { /* NOP */ } - } - // Turn a 1-element array into a simple value - if(is_array($id) && sizeof($id) == 1) $id = reset($id); - $this->foreignID = $id; - - $this->dataQuery->where($this->foreignIDFilter()); - - return $this; + public function getForeignID() { + return $this->dataQuery->getQueryParam('Foreign.ID'); } - + /** * Returns a copy of this list with the ManyMany relationship linked to the given foreign ID. * @param $id An ID or an array of IDs. */ public function forForeignID($id) { - return $this->alterDataQuery_30(function($query, $list) use ($id){ - $list->setForeignID($id); + // Turn a 1-element array into a simple value + if(is_array($id) && sizeof($id) == 1) $id = reset($id); + + // Calculate the new filter + $filter = $this->foreignIDFilter($id); + + $list = $this->alterDataQuery(function($query, $list) use ($id, $filter){ + // Check if there is an existing filter, remove if there is + $currentFilter = $query->getQueryParam('Foreign.Filter'); + if($currentFilter) { + try { + $query->removeFilterOn($currentFilter); + } + catch (Exception $e) { /* NOP */ } + } + + // Add the new filter + $query->setQueryParam('Foreign.ID', $id); + $query->setQueryParam('Foreign.Filter', $filter); + $query->where($filter); }); + + return $list; } - - abstract protected function foreignIDFilter(); + + /** + * Returns a where clause that filters the members of this relationship to just the related items + * @param $id (optional) An ID or an array of IDs - if not provided, will use the current ids as per getForeignID + */ + abstract protected function foreignIDFilter($id = null); } diff --git a/model/Sortable.php b/model/Sortable.php index 952e9d8a8..c3432a945 100644 --- a/model/Sortable.php +++ b/model/Sortable.php @@ -19,9 +19,10 @@ interface SS_Sortable { public function canSortBy($by); /** - * Sorts this list by one or more fields. You can either pass in a single + * Return a new instance of this list that is sorted by one or more fields. You can either pass in a single * field name and direction, or a map of field names to sort directions. * + * @return SS_Sortable * @example $list = $list->sort('Name'); // default ASC sorting * @example $list = $list->sort('Name DESC'); // DESC sorting * @example $list = $list->sort('Name', 'ASC'); @@ -31,11 +32,10 @@ interface SS_Sortable { /** - * Reverses the list based on reversing the current sort. + * Return a new instance of this list based on reversing the current sort. * + * @return SS_Sortable * @example $list = $list->reverse(); - * - * @return array */ public function reverse(); } diff --git a/model/UnsavedRelationList.php b/model/UnsavedRelationList.php index 39b52e973..2a7ef640d 100644 --- a/model/UnsavedRelationList.php +++ b/model/UnsavedRelationList.php @@ -239,25 +239,14 @@ class UnsavedRelationList extends ArrayList { return $list->column('ID'); } - /** - * Set the ID of the record that this RelationList is linking. - * - * Adds the - * - * @param $id A single ID, or an array of IDs - */ - public function setForeignID($id) { - $class = singleton($this->baseClass); - $class->ID = 1; - return $class->{$this->relationName}()->setForeignID($id); - } - /** * Returns a copy of this list with the relationship linked to the given foreign ID. * @param $id An ID or an array of IDs. */ public function forForeignID($id) { - return $this->setForeignID($id); + $class = singleton($this->baseClass); + $class->ID = 1; + return $class->{$this->relationName}()->forForeignID($id); } /** diff --git a/model/Versioned.php b/model/Versioned.php index a74928408..560358475 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -1003,10 +1003,10 @@ class Versioned extends DataExtension { $containerClass = 'DataList') { $result = DataObject::get($class, $filter, $sort, $join, $limit, $containerClass); - $dq = $result->dataQuery(); - $dq->setQueryParam('Versioned.mode', 'stage'); - $dq->setQueryParam('Versioned.stage', $stage); - return $result; + return $result->setDataQueryParam(array( + 'Versioned.mode' => 'stage', + 'Versioned.stage' => $stage + )); } public function deleteFromStage($stage) { @@ -1049,8 +1049,10 @@ class Versioned extends DataExtension { */ public static function get_latest_version($class, $id) { $baseClass = ClassInfo::baseDataClass($class); - $list = DataList::create($baseClass)->where("\"$baseClass\".\"RecordID\" = $id"); - $list->dataQuery()->setQueryParam("Versioned.mode", "latest_versions"); + $list = DataList::create($baseClass) + ->where("\"$baseClass\".\"RecordID\" = $id") + ->setDataQueryParam("Versioned.mode", "latest_versions"); + return $list->First(); } @@ -1077,8 +1079,11 @@ class Versioned extends DataExtension { * In particular, this will query deleted records as well as active ones. */ public static function get_including_deleted($class, $filter = "", $sort = "") { - $list = DataList::create($class)->where($filter)->sort($sort); - $list->dataQuery()->setQueryParam("Versioned.mode", "latest_versions"); + $list = DataList::create($class) + ->where($filter) + ->sort($sort) + ->setDataQueryParam("Versioned.mode", "latest_versions"); + return $list; } @@ -1093,8 +1098,9 @@ class Versioned extends DataExtension { $baseClass = ClassInfo::baseDataClass($class); $list = DataList::create($baseClass) ->where("\"$baseClass\".\"RecordID\" = $id") - ->where("\"$baseClass\".\"Version\" = " . (int)$version); - $list->dataQuery()->setQueryParam('Versioned.mode', 'all_versions'); + ->where("\"$baseClass\".\"Version\" = " . (int)$version) + ->setDataQueryParam("Versioned.mode", 'all_versions'); + return $list->First(); } @@ -1104,8 +1110,10 @@ class Versioned extends DataExtension { */ public static function get_all_versions($class, $id) { $baseClass = ClassInfo::baseDataClass($class); - $list = DataList::create($class)->where("\"$baseClass\".\"RecordID\" = $id"); - $list->dataQuery()->setQueryParam('Versioned.mode', 'all_versions'); + $list = DataList::create($class) + ->where("\"$baseClass\".\"RecordID\" = $id") + ->setDataQueryParam('Versioned.mode', 'all_versions'); + return $list; } diff --git a/search/SearchContext.php b/search/SearchContext.php index de34b4086..fcce9dc33 100644 --- a/search/SearchContext.php +++ b/search/SearchContext.php @@ -153,7 +153,7 @@ class SearchContext extends Object { $filter->setModel($this->modelClass); $filter->setValue($value); if(! $filter->isEmpty()) { - $filter->apply($query->dataQuery()); + $query = $query->alterDataQuery(array($filter, 'apply')); } } } diff --git a/security/Group.php b/security/Group.php index 108bd2348..837143c37 100755 --- a/security/Group.php +++ b/security/Group.php @@ -236,7 +236,9 @@ class Group extends DataObject { // Remove the default foreign key filter in prep for re-applying a filter containing all children groups. // Filters are conjunctive in DataQuery by default, so this filter would otherwise overrule any less specific // ones. - $result->dataQuery()->removeFilterOn('Group_Members'); + $result = $result->alterDataQuery(function($query){ + $query->removeFilterOn('Group_Members'); + }); // Now set all children groups as a new foreign key $groups = Group::get()->byIDs($this->collateFamilyIDs()); $result = $result->forForeignID($groups->column('ID'))->where($filter)->sort($sort)->limit($limit); diff --git a/security/Member.php b/security/Member.php index b531efb43..964b329c8 100644 --- a/security/Member.php +++ b/security/Member.php @@ -1420,14 +1420,12 @@ class Member_GroupSet extends ManyManyList { /** * Link this group set to a specific member. */ - public function setForeignID($id) { - // Turn a 1-element array into a simple value - if(is_array($id) && sizeof($id) == 1) $id = reset($id); - $this->foreignID = $id; - + public function foreignIDFilter($id = null) { + if ($id === null) $id = $this->getForeignID(); + // Find directly applied groups - $manymanyFilter = $this->foreignIDFilter(); - $groupIDs = DB::query('SELECT "GroupID" FROM "Group_Members" WHERE ' . $manymanyFilter)->column(); + $manyManyFilter = parent::foreignIDFilter($id); + $groupIDs = DB::query('SELECT "GroupID" FROM "Group_Members" WHERE ' . $manyManyFilter)->column(); // Get all ancestors $allGroupIDs = array(); @@ -1438,8 +1436,16 @@ class Member_GroupSet extends ManyManyList { } // Add a filter to this DataList - if($allGroupIDs) $this->byIDs($allGroupIDs); - else $this->byIDs(array(0)); + if($allGroupIDs) { + return "\"Group\".\"ID\" IN (" . implode(',', $allGroupIDs) .")"; + } + else { + return "\"Group\".\"ID\" = 0"; + } + } + + public function foreignIDWriteFilter($id = null) { + return parent::foreignIDFilter($id); } } diff --git a/tests/model/ArrayListTest.php b/tests/model/ArrayListTest.php index 0d568f05d..bbb57e75e 100644 --- a/tests/model/ArrayListTest.php +++ b/tests/model/ArrayListTest.php @@ -229,14 +229,14 @@ class ArrayListTest extends SapphireTest { )); } - public function testSortSimpleDefualtIsSortedASC() { + public function testSortSimpleDefaultIsSortedASC() { $list = new ArrayList(array( array('Name' => 'Steve'), (object) array('Name' => 'Bob'), array('Name' => 'John') )); - $list->sort('Name'); + $list = $list->sort('Name'); $this->assertEquals($list->toArray(), array( (object) array('Name' => 'Bob'), array('Name' => 'John'), @@ -250,7 +250,8 @@ class ArrayListTest extends SapphireTest { (object) array('Name' => 'Bob'), array('Name' => 'John') )); - $list->sort('Name','asc'); + + $list = $list->sort('Name','asc'); $this->assertEquals($list->toArray(), array( (object) array('Name' => 'Bob'), array('Name' => 'John'), @@ -265,7 +266,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'John') )); - $list->sort('Name', 'DESC'); + $list = $list->sort('Name', 'DESC'); $this->assertEquals($list->toArray(), array( array('Name' => 'Steve'), array('Name' => 'John'), @@ -280,8 +281,8 @@ class ArrayListTest extends SapphireTest { array('Name' => 'Steve') )); - $list->sort('Name', 'ASC'); - $list->reverse(); + $list = $list->sort('Name', 'ASC'); + $list = $list->reverse(); $this->assertEquals($list->toArray(), array( array('Name' => 'Steve'), @@ -297,11 +298,11 @@ class ArrayListTest extends SapphireTest { (object) array('Name'=>'Object3', 'F1'=>5, 'F2'=>2, 'F3'=>2), )); - $list->sort('F3', 'ASC'); + $list = $list->sort('F3', 'ASC'); $this->assertEquals($list->first()->Name, 'Object3', 'Object3 should be first in the list'); $this->assertEquals($list->last()->Name, 'Object2', 'Object2 should be last in the list'); - $list->sort('F3', 'DESC'); + $list = $list->sort('F3', 'DESC'); $this->assertEquals($list->first()->Name, 'Object2', 'Object2 should be first in the list'); $this->assertEquals($list->last()->Name, 'Object3', 'Object3 should be last in the list'); } @@ -313,11 +314,11 @@ class ArrayListTest extends SapphireTest { (object) array('ID'=>2, 'Name'=>'Aron', 'Importance'=>1), )); - $list->sort(array('Name'=>'ASC', 'Importance'=>'ASC')); + $list = $list->sort(array('Name'=>'ASC', 'Importance'=>'ASC')); $this->assertEquals($list->first()->ID, 2, 'Aron.2 should be first in the list'); $this->assertEquals($list->last()->ID, 3, 'Bert.3 should be last in the list'); - $list->sort(array('Name'=>'ASC', 'Importance'=>'DESC')); + $list = $list->sort(array('Name'=>'ASC', 'Importance'=>'DESC')); $this->assertEquals($list->first()->ID, 1, 'Aron.2 should be first in the list'); $this->assertEquals($list->last()->ID, 3, 'Bert.3 should be last in the list'); } @@ -331,7 +332,7 @@ class ArrayListTest extends SapphireTest { (object) array('Name' => 'Bob'), array('Name' => 'John') )); - $list->filter('Name','Bob'); + $list = $list->filter('Name','Bob'); $this->assertEquals(array((object)array('Name'=>'Bob')), $list->toArray(), 'List should only contain Bob'); } @@ -349,7 +350,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'Steve'), array('Name' => 'John') ); - $list->filter('Name',array('Steve','John')); + $list = $list->filter('Name',array('Steve','John')); $this->assertEquals($expected, $list->toArray(), 'List should only contain Steve and John'); } @@ -362,7 +363,7 @@ class ArrayListTest extends SapphireTest { (object) array('Name' => 'Steve', 'ID' => 2), array('Name' => 'John', 'ID' => 2) )); - $list->filter(array('Name'=>'Clair')); + $list = $list->filter(array('Name'=>'Clair')); $this->assertEquals(array(), $list->toArray(), 'List should be empty'); } @@ -375,7 +376,7 @@ class ArrayListTest extends SapphireTest { (object) array('Name' => 'Steve', 'ID' => 2), array('Name' => 'John', 'ID' => 2) )); - $list->filter(array('Name'=>'Steve', 'ID'=>2)); + $list = $list->filter(array('Name'=>'Steve', 'ID'=>2)); $this->assertEquals(array((object)array('Name'=>'Steve', 'ID'=>2)), $list->toArray(), 'List should only contain object Steve'); } @@ -389,7 +390,7 @@ class ArrayListTest extends SapphireTest { (object) array('Name' => 'Steve', 'ID' => 2), array('Name' => 'John', 'ID' => 2) )); - $list->filter(array('Name'=>'Steve', 'ID'=>4)); + $list = $list->filter(array('Name'=>'Steve', 'ID'=>4)); $this->assertEquals(array(), $list->toArray(), 'List should be empty'); } @@ -404,7 +405,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'Steve', 'ID' => 3, 'Age'=>43) )); - $list->filter(array('Name'=>'Steve','Age'=>array(21, 43))); + $list = $list->filter(array('Name'=>'Steve','Age'=>array(21, 43))); $expected = array( array('Name' => 'Steve', 'ID' => 1, 'Age'=>21), @@ -426,7 +427,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'Steve', 'ID' => 3, 'Age'=>43) )); - $list->filter(array('Name'=>array('Steve','Clair'),'Age'=>array(21, 43))); + $list = $list->filter(array('Name'=>array('Steve','Clair'),'Age'=>array(21, 43))); $expected = array( array('Name' => 'Steve', 'ID' => 1, 'Age'=>21), @@ -448,7 +449,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'John') )); - $list->exclude('Name', 'Bob'); + $list = $list->exclude('Name', 'Bob'); $expected = array( array('Name' => 'Steve'), array('Name' => 'John') @@ -467,7 +468,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'John') )); - $list->exclude('Name', 'Clair'); + $list = $list->exclude('Name', 'Clair'); $expected = array( array('Name' => 'Steve'), array('Name' => 'Bob'), @@ -485,7 +486,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'Bob'), array('Name' => 'John') )); - $list->exclude('Name', array('Steve','John')); + $list = $list->exclude('Name', array('Steve','John')); $expected = array(array('Name' => 'Bob')); $this->assertEquals(1, $list->count()); $this->assertEquals($expected, $list->toArray(), 'List should only contain Bob'); @@ -501,7 +502,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'John', 'Age' => 21) )); - $list->exclude(array('Name' => 'Bob', 'Age' => 21)); + $list = $list->exclude(array('Name' => 'Bob', 'Age' => 21)); $expected = array( array('Name' => 'Bob', 'Age' => 32), @@ -527,7 +528,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'phil', 'Age' => 16) )); - $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16))); + $list = $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16))); $expected = array( array('Name' => 'phil', 'Age' => 11), array('Name' => 'bob', 'Age' => 12), @@ -553,7 +554,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'phil', 'Age' => 16) )); - $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'Bananas'=>true)); + $list = $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'Bananas'=>true)); $expected = array( array('Name' => 'bob', 'Age' => 10), array('Name' => 'phil', 'Age' => 11), @@ -584,7 +585,7 @@ class ArrayListTest extends SapphireTest { array('Name' => 'clair','Age' => 16, 'HasBananas'=>true) )); - $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'HasBananas'=>true)); + $list = $list->exclude(array('Name'=>array('bob','phil'),'Age'=>array(10, 16),'HasBananas'=>true)); $expected = array( array('Name' => 'bob', 'Age' => 10, 'HasBananas'=>false), array('Name' => 'phil','Age' => 11, 'HasBananas'=>true), diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index 0ebc0c68d..385fb9ea9 100644 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -79,9 +79,14 @@ class DataListTest extends SapphireTest { public function testInnerJoin() { $db = DB::getConn(); + $list = DataObjectTest_TeamComment::get(); - $list->innerJoin('DataObjectTest_Team', '"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"', - 'Team'); + $list = $list->innerJoin( + 'DataObjectTest_Team', + '"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"', + 'Team' + ); + $expected = 'SELECT DISTINCT "DataObjectTest_TeamComment"."ClassName", "DataObjectTest_TeamComment"."Created",' . ' "DataObjectTest_TeamComment"."LastEdited", "DataObjectTest_TeamComment"."Name",' . ' "DataObjectTest_TeamComment"."Comment", "DataObjectTest_TeamComment"."TeamID",' @@ -89,14 +94,20 @@ class DataListTest extends SapphireTest { . ' THEN "DataObjectTest_TeamComment"."ClassName" ELSE '.$db->prepStringForDB('DataObjectTest_TeamComment') . ' END AS "RecordClassName" FROM "DataObjectTest_TeamComment" INNER JOIN "DataObjectTest_Team" AS "Team"' . ' ON "DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"'; + $this->assertEquals($expected, $list->sql()); } public function testLeftJoin() { $db = DB::getConn(); + $list = DataObjectTest_TeamComment::get(); - $list->leftJoin('DataObjectTest_Team', '"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"', - 'Team'); + $list = $list->leftJoin( + 'DataObjectTest_Team', + '"DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"', + 'Team' + ); + $expected = 'SELECT DISTINCT "DataObjectTest_TeamComment"."ClassName", "DataObjectTest_TeamComment"."Created",' . ' "DataObjectTest_TeamComment"."LastEdited", "DataObjectTest_TeamComment"."Name",' . ' "DataObjectTest_TeamComment"."Comment", "DataObjectTest_TeamComment"."TeamID",' @@ -104,14 +115,16 @@ class DataListTest extends SapphireTest { . ' THEN "DataObjectTest_TeamComment"."ClassName" ELSE '.$db->prepStringForDB('DataObjectTest_TeamComment') . ' END AS "RecordClassName" FROM "DataObjectTest_TeamComment" LEFT JOIN "DataObjectTest_Team" AS "Team"' . ' ON "DataObjectTest_Team"."ID" = "DataObjectTest_TeamComment"."TeamID"'; + $this->assertEquals($expected, $list->sql()); // Test with namespaces (with non-sensical join, but good enough for testing) $list = DataObjectTest_TeamComment::get(); - $list->leftJoin( + $list = $list->leftJoin( 'DataObjectTest\NamespacedClass', '"DataObjectTest\NamespacedClass"."ID" = "DataObjectTest_TeamComment"."ID"' ); + $expected = 'SELECT DISTINCT "DataObjectTest_TeamComment"."ClassName", ' . '"DataObjectTest_TeamComment"."Created", ' . '"DataObjectTest_TeamComment"."LastEdited", ' @@ -126,6 +139,7 @@ class DataListTest extends SapphireTest { . 'LEFT JOIN "DataObjectTest\NamespacedClass" ON ' . '"DataObjectTest\NamespacedClass"."ID" = "DataObjectTest_TeamComment"."ID"'; $this->assertEquals($expected, $list->sql(), 'Retains backslashes in namespaced classes'); + } public function testToNestedArray() {