Merge pull request #5940 from open-sausages/pulls/4.0/filter-updates

API Change behaviour of filter API to support injected search filter classes

ORM/DataList.php

@@ -2,6 +2,9 @@
 
 namespace SilverStripe\ORM;
 
+use BadMethodCallException;
+use SearchFilter;
+use SilverStripe\ORM\Queries\SQLConditionGroup;
 use ViewableData;
 use Exception;
 use InvalidArgumentException;
@@ -34,6 +37,7 @@ use ArrayIterator;
  * @subpackage orm
  */
 class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortable, SS_Limitable {
+
 	/**
 	 * The DataObject class name that this data list is querying
 	 *
@@ -403,12 +407,9 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	public function addFilter($filterArray) {
 		$list = $this;
 
-		foreach($filterArray as $field => $value) {
+		foreach($filterArray as $expression => $value) {
-			$fieldArgs = explode(':', $field);
+			$filter = $this->createSearchFilter($expression, $value);
-			$field = array_shift($fieldArgs);
+			$list = $list->alterDataQuery(array($filter, 'apply'));
-			$filterType = array_shift($fieldArgs);
-			$modifiers = $fieldArgs;
-			$list = $list->applyFilterContext($field, $filterType, $modifiers, $value);
 		}
 
 		return $list;
@@ -454,21 +455,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 			$subquery = $query->disjunctiveGroup();
 
 			foreach($whereArguments as $field => $value) {
-				$fieldArgs = explode(':',$field);
+				$filter = $this->createSearchFilter($field, $value);
-				$field = array_shift($fieldArgs);
-				$filterType = array_shift($fieldArgs);
-				$modifiers = $fieldArgs;
-
-				if($filterType) {
-					$className = "{$filterType}Filter";
-				} else {
-					$className = 'ExactMatchFilter';
-				}
-				if(!class_exists($className)){
-					$className = 'ExactMatchFilter';
-					array_unshift($modifiers, $filterType);
-				}
-				$filter = Injector::inst()->create($className, $field, $value, $modifiers);
 				$filter->apply($subquery);
 			}
 		});
@@ -550,30 +537,40 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	}
 
 	/**
-	 * Translates a filter type to a SQL query.
+	 * Given a filter expression and value construct a {@see SearchFilter} instance
 	 *
-	 * @param string $field - the fieldname in the db
+	 * @param string $filter E.g. `Name:ExactMatch:not`, `Name:ExactMatch`, `Name:not`, `Name`
-	 * @param string $filter - example StartsWith, relates to a filtercontext
+	 * @param mixed $value Value of the filter
-	 * @param array $modifiers - Modifiers to pass to the filter, ie not,nocase
+	 * @return SearchFilter
-	 * @param string $value - the value that the filtercontext will use for matching
-	 * @return static
 	 */
-	private function applyFilterContext($field, $filter, $modifiers, $value) {
+	protected function createSearchFilter($filter, $value) {
-		if($filter) {
+		// Field name is always the first component
-			$className = "{$filter}Filter";
+		$fieldArgs = explode(':', $filter);
+		$fieldName = array_shift($fieldArgs);
+
+		// Inspect type of second argument to determine context
+		$secondArg = array_shift($fieldArgs);
+		$modifiers = $fieldArgs;
+		if(!$secondArg) {
+			// Use default filter if none specified. E.g. `->filter(['Name' => $myname])`
+			$filterServiceName = 'DataListFilter.default';
 		} else {
-			$className = 'ExactMatchFilter';
+			// The presence of a second argument is by default ambiguous; We need to query
+			// Whether this is a valid modifier on the default filter, or a filter itself.
+			/** @var SearchFilter $defaultFilterInstance */
+			$defaultFilterInstance = Injector::inst()->get('DataListFilter.default');
+			if (in_array(strtolower($secondArg), $defaultFilterInstance->getSupportedModifiers())) {
+				// Treat second (and any subsequent) argument as modifiers, using default filter
+				$filterServiceName = 'DataListFilter.default';
+				array_unshift($modifiers, $secondArg);
+			} else {
+				// Second argument isn't a valid modifier, so assume is filter identifier
+				$filterServiceName = "DataListFilter.{$secondArg}";
+			}
 		}
 
-		if(!class_exists($className)) {
+		// Build instance
-			$className = 'ExactMatchFilter';
+		return Injector::inst()->create($filterServiceName, $fieldName, $value, $modifiers);
-
-			array_unshift($modifiers, $filter);
-		}
-
-		$t = new $className($field, $value, $modifiers);
-
-		return $this->alterDataQuery(array($t, 'apply'));
 	}
 
 	/**
@@ -608,21 +605,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 			$subquery = $query->disjunctiveGroup();
 
 			foreach($whereArguments as $field => $value) {
-				$fieldArgs = explode(':', $field);
+				$filter = $this->createSearchFilter($field, $value);
-				$field = array_shift($fieldArgs);
-				$filterType = array_shift($fieldArgs);
-				$modifiers = $fieldArgs;
-
-				if($filterType) {
-					$className = "{$filterType}Filter";
-				} else {
-					$className = 'ExactMatchFilter';
-				}
-				if(!class_exists($className)){
-					$className = 'ExactMatchFilter';
-					array_unshift($modifiers, $filterType);
-				}
-				$filter = Injector::inst()->create($className, $field, $value, $modifiers);
 				$filter->exclude($subquery);
 			}
 		});

_config/model.yml

@@ -1,3 +1,6 @@
+---
+Name: corefieldtypes
+---
 Injector:
   Boolean:
     class: SilverStripe\ORM\FieldType\DBBoolean
@@ -55,3 +58,28 @@ Injector:
     class: SilverStripe\ORM\FieldType\DBVarchar
   Year:
     class: SilverStripe\ORM\FieldType\DBYear
+---
+Name: coresearchfilters
+---
+Injector:
+  DataListFilter.default: %$DataListFilter.ExactMatch
+  DataListFilter.EndsWith:
+    class: EndsWithFilter
+  DataListFilter.ExactMatch:
+    class: ExactMatchFilter
+  DataListFilter.Fulltext:
+    class: FulltextFilter
+  DataListFilter.GreaterThan:
+    class: GreaterThanFilter
+  DataListFilter.GreaterThanOrEqual:
+    class: GreaterThanOrEqualFilter
+  DataListFilter.LessThan:
+    class: LessThanFilter
+  DataListFilter.LessThanOrEqual:
+    class: LessThanOrEqualFilter
+  DataListFilter.PartialMatch:
+    class: PartialMatchFilter
+  DataListFilter.StartsWith:
+    class: StartsWithFilter
+  DataListFilter.WithinRange:
+    class: WithinRangeFilter

docs/en/02_Developer_Guides/00_Model/06_SearchFilters.md

@@ -36,6 +36,17 @@ These suffixes can also take modifiers themselves. The modifiers currently suppo
 comparison uses the database's default. For MySQL and MSSQL, this is case-insensitive. For PostgreSQL, this is 
 case-sensitive.
 
+Note that all search filters (e.g. `:PartialMatch`) refer to services registered with [api:Injector]
+within the `DataListFilter.` prefixed namespace. New filters can be registered using the below yml
+config:
+
+
+	:::yaml
+	Injector:
+	  DataListFilter.CustomMatch:
+	    class: MyVendor/Search/CustomMatchFilter
+
+
 The following is a query which will return everyone whose first name starts with "S", either lowercase or uppercase:
 
 	:::php

docs/en/02_Developer_Guides/12_Search/02_FulltextSearch.md

@@ -71,7 +71,7 @@ Example DataObject:
 Performing the search:
 
 	:::php
-	SearchableDataObject::get()->filter('SearchFields:fulltext', 'search term');
+	SearchableDataObject::get()->filter('SearchFields:Fulltext', 'search term');
 
 If your search index is a single field size, then you may also specify the search filter by the name of the
 field instead of the index.

docs/en/04_Changelogs/4.0.0.md

@@ -59,6 +59,10 @@
  * `UpgradeSiteTreePermissionSchemaTask` is removed.
  * `$action` parameter to `Controller::Link()` method is standardised.
  * Removed `UpgradeSiteTreePermissionSchemaTask`
+ * Removed `DataList::applyFilterContext` private method
+ * Search filter classes (e.g. `ExactMatchFilter`) are now registered with `Injector`
+   via a new `DataListFilter.` prefix convention.
+   see [search filter documentation](/developer_guides/model/searchfilters) for more information.
 
 ## New API
 
@@ -1003,4 +1007,4 @@ The default `admin/` URL to access the CMS interface can now be changed via a cu
 `AdminRootController`. If your website or module has hard coded `admin` URLs in PHP, templates or JavaScript, make sure 
 to update those with the appropriate function or config call. See 
 [CMS architecture](/developer_guides/customising_the_admin_interface/cms-architecture#the-admin-url) for language 
-specific functions.
+specific functions.

search/filters/ExactMatchFilter.php

@@ -18,13 +18,9 @@ use SilverStripe\ORM\DB;
  */
 class ExactMatchFilter extends SearchFilter {
 
-	public function setModifiers(array $modifiers) {
+	public function getSupportedModifiers()
-		if(($extras = array_diff($modifiers, array('not', 'nocase', 'case'))) != array()) {
+	{
-			throw new InvalidArgumentException(
+		return ['not', 'nocase', 'case'];
-				get_class($this) . ' does not accept ' . implode(', ', $extras) . ' as modifiers');
-		}
-
-		parent::setModifiers($modifiers);
 	}
 
 	/**

search/filters/PartialMatchFilter.php

@@ -15,13 +15,9 @@ use SilverStripe\ORM\DB;
  */
 class PartialMatchFilter extends SearchFilter {
 
-	public function setModifiers(array $modifiers) {
+	public function getSupportedModifiers()
-		if(($extras = array_diff($modifiers, array('not', 'nocase', 'case'))) != array()) {
+	{
-			throw new InvalidArgumentException(
+		return ['not', 'nocase', 'case'];
-				get_class($this) . ' does not accept ' . implode(', ', $extras) . ' as modifiers');
-		}
-
-		parent::setModifiers($modifiers);
 	}
 
 	/**

search/filters/SearchFilter.php

@@ -2,12 +2,22 @@
 
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataQuery;
+
 /**
  * Base class for filtering implementations,
  * which work together with {@link SearchContext}
  * to create or amend a query for {@link DataObject} instances.
  * See {@link SearchContext} for more information.
  *
+ * Each search filter must be registered in config as an "Injector" service with
+ * the "DataListFilter." prefix. E.g.
+ *
+ * <code>
+ * Injector:
+ *   DataListFilter.EndsWith:
+ *     class: EndsWithFilter
+ * </code>
+ *
  * @package framework
  * @subpackage search
  */
@@ -53,7 +63,8 @@ abstract class SearchFilter extends Object {
 	 * @param mixed $value
 	 * @param array $modifiers
 	 */
-	public function __construct($fullName, $value = false, array $modifiers = array()) {
+	public function __construct($fullName = null, $value = false, array $modifiers = array()) {
+		parent::__construct();
 		$this->fullName = $fullName;
 
 		// sets $this->name and $this->relation
@@ -112,7 +123,29 @@ abstract class SearchFilter extends Object {
 	 * @param array $modifiers
 	 */
 	public function setModifiers(array $modifiers) {
-		$this->modifiers = array_map('strtolower', $modifiers);
+		$modifiers = array_map('strtolower', $modifiers);
+
+		// Validate modifiers are supported
+		$allowed = $this->getSupportedModifiers();
+		$unsupported = array_diff($modifiers, $allowed);
+		if ($unsupported) {
+			throw new InvalidArgumentException(
+				get_class($this) . ' does not accept ' . implode(', ', $unsupported) . ' as modifiers'
+			);
+		}
+
+		$this->modifiers = $modifiers;
+	}
+
+	/**
+	 * Gets supported modifiers for this filter
+	 *
+	 * @return array
+	 */
+	public function getSupportedModifiers()
+	{
+		// By default support 'not' as a modifier for all filters
+		return ['not'];
 	}
 
 	/**

tests/model/DataListTest.php

@@ -649,9 +649,22 @@ class DataListTest extends SapphireTest {
 	}
 
 	public function testSimpleFilterWithNonExistingComparisator() {
-		$this->setExpectedException('InvalidArgumentException');
+		$this->setExpectedException(
+			'ReflectionException',
+			'Class DataListFilter.Bogus does not exist'
+		);
 		$list = DataObjectTest_TeamComment::get();
-		$list = $list->filter('Comment:Bogus', 'team comment');
+		$list->filter('Comment:Bogus', 'team comment');
+	}
+
+	public function testInvalidModifier() {
+		// Invalid modifiers are treated as failed filter construction
+		$this->setExpectedException(
+			'ReflectionException',
+			'Class DataListFilter.invalidmodifier does not exist'
+		);
+		$list = DataObjectTest_TeamComment::get();
+		$list->filter('Comment:invalidmodifier', 'team comment');
 	}
 
 	/**

tests/search/FulltextFilterTest.php

@@ -19,25 +19,25 @@ class FulltextFilterTest extends SapphireTest {
 			$this->assertEquals(3, $baseQuery->count(), "FulltextFilterTest_DataObject count does not match.");
 
 			// First we'll text the 'SearchFields' which has been set using an array
-			$search = $baseQuery->filter("SearchFields:fulltext", 'SilverStripe');
+			$search = $baseQuery->filter("SearchFields:Fulltext", 'SilverStripe');
 			$this->assertEquals(1, $search->count());
 
-			$search = $baseQuery->exclude("SearchFields:fulltext", "SilverStripe");
+			$search = $baseQuery->exclude("SearchFields:Fulltext", "SilverStripe");
 			$this->assertEquals(2, $search->count());
 
 			// Now we'll run the same tests on 'OtherSearchFields' which should yield the same resutls
 			// but has been set using a string.
-			$search = $baseQuery->filter("OtherSearchFields:fulltext", 'SilverStripe');
+			$search = $baseQuery->filter("OtherSearchFields:Fulltext", 'SilverStripe');
 			$this->assertEquals(1, $search->count());
 
-			$search = $baseQuery->exclude("OtherSearchFields:fulltext", "SilverStripe");
+			$search = $baseQuery->exclude("OtherSearchFields:Fulltext", "SilverStripe");
 			$this->assertEquals(2, $search->count());
 
 			// Search on a single field
-			$search = $baseQuery->filter("ColumnE:fulltext", 'Dragons');
+			$search = $baseQuery->filter("ColumnE:Fulltext", 'Dragons');
 			$this->assertEquals(1, $search->count());
 
-			$search = $baseQuery->exclude("ColumnE:fulltext", "Dragons");
+			$search = $baseQuery->exclude("ColumnE:Fulltext", "Dragons");
 			$this->assertEquals(2, $search->count());
 		} else {
 			$this->markTestSkipped("FulltextFilter only supports MySQL syntax.");