Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

API Make DataList and ArrayList immutable

In 3.0 there was some confusion about whether DataLists and ArrayLists
were mutable or not. If DataLists were immutable, they'd return the result, and your code
would look like

  $list = $list->filter(....);

If DataLists were mutable, they'd operate on themselves, returning nothing, and your code
would look like

 $list->filter(....);

This makes all DataLists and ArrayList immutable for all _searching_ operations.
Operations on DataList that modify the underlying SQL data store remain mutating.

- These functions no longer mutate the existing object, and if you do not capture the value
returned by them will have no effect:

  ArrayList#reverse
  ArrayList#sort
  ArrayList#filter
  ArrayList#exclude

  DataList#dataQuery (use DataList#alterDataQuery to modify dataQuery in a safe manner)
  DataList#where
  DataList#limit
  DataList#sort
  DataList#addFilter
  DataList#applyFilterContext
  DataList#innerJoin
  DataList#leftJoin
  DataList#find
  DataList#byIDs
  DataList#reverse

- DataList#setDataQueryParam has been added as syntactic sugar around the most common
cause of accessing the dataQuery directly - setting query parameters

- RelationList#setForeignID has been removed. Always use RelationList#forForeignID
when querying, and overload RelationList#foreignIDList when subclassing.

- Relatedly,the protected variable RelationList->foreignID has been removed, as the ID is
now stored on a query parameter. Use RelationList#getForeignID to read it.
  • Loading branch information...
commit 27113f82c30dd08a2d60bf0f481ae06054363ad0 1 parent 644cc79
@hafriedlander hafriedlander authored
View
20 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
+ * 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.
View
23 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 @@ public function canSortBy($by) {
* @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 @@ public function sort() {
$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 @@ public function filter() {
}
}
- // TODO 3.1: This currently mutates existing array
- $list = /* clone */ $this;
+ $list = clone $this;
$list->items = $itemsToKeep;
return $list;
}
@@ -504,8 +512,7 @@ public function exclude() {
}
}
- // TODO 3.1: This currently mutates existing array
- $list = /* clone */ $this;
+ $list = clone $this;
$list->items = $itemsToKeep;
return $list;
}
View
122 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 @@ public function __clone() {
/**
* 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 @@ public function alterDataQuery($callback) {
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 @@ public function alterDataQuery($callback) {
$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) {
@@ -146,39 +142,6 @@ public function alterDataQuery($callback) {
}
/**
- * 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
*
* @param DataQuery $dataQuery
@@ -190,6 +153,21 @@ public function setDataQuery(DataQuery $dataQuery) {
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 @@ public function sql() {
* @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 @@ public function limit($limit, $offset = 0) {
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 @@ public function sort() {
$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 @@ public function filter() {
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 @@ public function getRelationName($field) {
* @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 @@ private function applyFilterContext($field, $comparisators, $modifiers, $value)
array_unshift($modifiers, $comparisators);
}
$t = new $className($field, $value, $modifiers);
- $t->apply($this->dataQuery());
+
+ return $this->alterDataQuery(array($t, 'apply'));
}
/**
@@ -581,7 +558,7 @@ public function subtract(SS_List $list) {
* @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 @@ public function innerJoin($table, $onClause, $alias = null) {
* @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 @@ public function find($key, $value) {
$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 @@ public function setQueriedColumns($queriedColumns) {
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 @@ public function byIDs(array $ids) {
*/
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 @@ public function removeByID($itemID) {
* @return DataList
*/
public function reverse() {
- return $this->alterDataQuery_30(function($query){
+ return $this->alterDataQuery(function($query){
$query->reverseSort();
});
}
View
4 model/DataObject.php
@@ -2722,9 +2722,9 @@ public static function get($callerClass = null, $filter = "", $sort = "", $join
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);
View
8 model/Filterable.php
@@ -19,8 +19,9 @@
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 @@ public function canFilterBy($by);
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
View
22 model/HasManyList.php
@@ -21,14 +21,16 @@ public function __construct($dataClass, $foreignKey) {
$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 @@ public function add($item) {
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();
}
View
11 model/Hierarchy.php
@@ -591,12 +591,13 @@ public function liveChildren($showAll = false, $onlyDeletedFromStage = false) {
$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;
}
View
4 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);
View
49 model/ManyManyList.php
@@ -9,7 +9,7 @@ class ManyManyList extends RelationList {
protected $localKey;
- protected $foreignKey, $foreignID;
+ protected $foreignKey;
protected $extraFields;
@@ -48,20 +48,36 @@ public function __construct($dataClass, $joinTable, $localKey, $foreignKey, $ext
}
/**
- * 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.
* @param $extraFields A map of additional columns to insert into the joinTable
@@ -73,22 +89,25 @@ public function add($item, $extraFields = null) {
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 @@ public function removeByID($itemID) {
$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 @@ function getExtraData($componentName, $itemID) {
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);
View
62 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);
}
View
8 model/Sortable.php
@@ -19,9 +19,10 @@
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 @@ public function sort();
/**
- * 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();
}
View
17 model/UnsavedRelationList.php
@@ -240,24 +240,13 @@ public function column($colName = '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);
}
/**
View
32 model/Versioned.php
@@ -1003,10 +1003,10 @@ public static function get_by_stage($class, $stage, $filter = '', $sort = '', $j
$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 @@ public function doRollbackTo($version) {
*/
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 @@ public function isLatestVersion() {
* 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 @@ public static function get_version($class, $id, $version) {
$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 @@ public static function get_version($class, $id, $version) {
*/
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;
}
View
2  search/SearchContext.php
@@ -153,7 +153,7 @@ public function getQuery($searchParams, $sort = false, $limit = false, $existing
$filter->setModel($this->modelClass);
$filter->setValue($value);
if(! $filter->isEmpty()) {
- $filter->apply($query->dataQuery());
+ $query = $query->alterDataQuery(array($filter, 'apply'));
}
}
}
View
4 security/Group.php
@@ -236,7 +236,9 @@ public function Members($filter = "", $sort = "", $join = "", $limit = "") {
// 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);
View
24 security/Member.php
@@ -1420,14 +1420,12 @@ public function __construct($dataClass, $joinTable, $localKey, $foreignKey, $ext
/**
* 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 @@ public function setForeignID($id) {
}
// 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);
}
}
View
49 tests/model/ArrayListTest.php
@@ -229,14 +229,14 @@ public function testColumn() {
));
}
- 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 @@ public function testSortSimpleASCOrder() {
(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 @@ public function testSortSimpleDESCOrder() {
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 @@ public function testReverse() {
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 @@ public function testSimpleMultiSort() {
(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 @@ public function testMultiSort() {
(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 @@ public function testSimpleFilter() {
(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 @@ public function testSimpleFilterWithMultiple() {
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 @@ public function testSimpleFilterWithMultipleNoMatch() {
(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 @@ public function testMultipleFilter() {
(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 @@ public function testMultipleFilterNoMatch() {
(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 @@ public function testMultipleWithArrayFilter() {
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 @@ public function testMultipleWithArrayFilterAdvanced() {
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 @@ public function testSimpleExclude() {
array('Name' => 'John')
));
- $list->exclude('Name', 'Bob');
+ $list = $list->exclude('Name', 'Bob');
$expected = array(
array('Name' => 'Steve'),
array('Name' => 'John')
@@ -467,7 +468,7 @@ public function testSimpleExcludeNoMatch() {
array('Name' => 'John')
));
- $list->exclude('Name', 'Clair');
+ $list = $list->exclude('Name', 'Clair');
$expected = array(
array('Name' => 'Steve'),
array('Name' => 'Bob'),
@@ -485,7 +486,7 @@ public function testSimpleExcludeWithArray() {
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 @@ public function testExcludeWithTwoArrays() {
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 @@ public function testMultipleExclude() {
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 @@ public function testMultipleExcludeNoMatch() {
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 @@ public function testMultipleExcludeThreeArguments() {
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),
View
24 tests/model/DataListTest.php
@@ -79,9 +79,14 @@ public function testSql() {
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 @@ public function testInnerJoin() {
. ' 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 @@ public function testLeftJoin() {
. ' 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 @@ public function testLeftJoin() {
. 'LEFT JOIN "DataObjectTest\NamespacedClass" ON '
. '"DataObjectTest\NamespacedClass"."ID" = "DataObjectTest_TeamComment"."ID"';
$this->assertEquals($expected, $list->sql(), 'Retains backslashes in namespaced classes');
+
}
public function testToNestedArray() {
Please sign in to comment.
Something went wrong with that request. Please try again.