[FEATURE] Add AbstractCondition for TS conditions 95/32695/11
authorBenjamin Mack <benni@typo3.org>
Tue, 9 Sep 2014 19:42:37 +0000 (21:42 +0200)
committerWouter Wolters <typo3@wouterwolters.nl>
Wed, 24 Sep 2014 18:27:41 +0000 (20:27 +0200)
In order to streamline conditions and make
TypoScript conditions more flexible, an abstract class
AbstractCondition is introduced. Any class can extend
this abstract class, and TypoScript seamlessly calls
the class's main function that can completely flexibly
deal with the value that is set after the class.

possible conditions are:

[ACME\TypoScriptLovePackage\BennisCondition]

[ACME\TypoScriptLovePackage\BennisCondition = 7]

[ACME\TypoScriptLovePackage\BennisCondition = 7, != 6]

where the TypoScript Condition can deal with =/!= etc
by itself.

Additionally, this change fixes one bug, where the
AbstractConditionMatcher would check on a condition name
and would not match, then there is an additional
run through the subclass although not needed.

Releases: master
Resolves: #61489
Change-Id: I891edd367bb29b042cbbf6aa368cbf173dc3815d
Reviewed-on: http://review.typo3.org/32695
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractCondition.php [new file with mode: 0644]
typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcher.php
typo3/sysext/core/Classes/Configuration/TypoScript/Exception/InvalidTypoScriptConditionException.php [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/master/Feature-61489-AbstractTypoScriptCondition.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Configuration/TypoScript/ConditionMatching/AbstractConditionMatcherTest.php
typo3/sysext/frontend/Classes/Configuration/TypoScript/ConditionMatching/ConditionMatcher.php
typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/ConditionMatcherTest.php
typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestCondition.php [new file with mode: 0644]
typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestConditionException.php [new file with mode: 0644]

diff --git a/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractCondition.php b/typo3/sysext/core/Classes/Configuration/TypoScript/ConditionMatching/AbstractCondition.php
new file mode 100644 (file)
index 0000000..c3cf431
--- /dev/null
@@ -0,0 +1,56 @@
+<?php
+namespace TYPO3\CMS\Core\Configuration\TypoScript\ConditionMatching;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * Abstract class to define own custom TypoScript conditions.
+ *
+ * Used with the TypoScript parser.
+ */
+abstract class AbstractCondition {
+
+       /**
+        * @var \TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching\AbstractConditionMatcher
+        */
+       protected $conditionMatcherInstance;
+
+       /**
+        * @return \TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching\AbstractConditionMatcher
+        */
+       protected function getConditionMatcherInstance() {
+               return $this->conditionMatcherInstance;
+       }
+
+       /**
+        * @param \TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching\AbstractConditionMatcher $conditionMatcherInstance
+        */
+       public function setConditionMatcherInstance($conditionMatcherInstance) {
+               $this->conditionMatcherInstance = $conditionMatcherInstance;
+       }
+
+       /**
+        * Main method handling the evaluation.
+        * Any given parameters given within the condition in form of
+        * [ACME\MyPackageName\MyCondition = value1, = value2]
+        * will be given as parameter in form of a numeric array, each entry
+        * containing the strings that are split by the commas
+        * e.g. array('= value1', '= value2')
+        *
+        * @param array $conditionParameters
+        * @return bool
+        */
+       abstract public function matchCondition(array $conditionParameters);
+
+}
index 6e8adcb..7c677cd 100644 (file)
@@ -210,6 +210,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'browser':
                                $values = GeneralUtility::trimExplode(',', $value, TRUE);
@@ -226,6 +227,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'version':
                                $values = GeneralUtility::trimExplode(',', $value, TRUE);
@@ -252,6 +254,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'system':
                                $values = GeneralUtility::trimExplode(',', $value, TRUE);
@@ -263,6 +266,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'device':
                                if (!isset($this->deviceInfo)) {
@@ -274,11 +278,14 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'useragent':
                                $test = trim($value);
                                if ($test !== '') {
                                        return $this->searchStringWildcard((string)$browserInfo['useragent'], $test);
+                               } else {
+                                       return FALSE;
                                }
                                break;
                        case 'language':
@@ -296,20 +303,17 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'IP':
                                if ($value === 'devIP') {
                                        $value = trim($GLOBALS['TYPO3_CONF_VARS']['SYS']['devIPmask']);
                                }
 
-                               if (GeneralUtility::cmpIP(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $value)) {
-                                       return TRUE;
-                               }
+                               return (bool) GeneralUtility::cmpIP(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $value);
                                break;
                        case 'hostname':
-                               if (GeneralUtility::cmpFQDN(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $value)) {
-                                       return TRUE;
-                               }
+                               return (bool) GeneralUtility::cmpFQDN(GeneralUtility::getIndpEnv('REMOTE_ADDR'), $value);
                                break;
                        case 'hour':
 
@@ -360,6 +364,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'compatVersion':
                                return GeneralUtility::compat_version($value);
@@ -375,6 +380,7 @@ abstract class AbstractConditionMatcher {
                                } elseif ($value === '') {
                                        return TRUE;
                                }
+                               return FALSE;
                                break;
                        case 'page':
                                if ($keyParts[1]) {
@@ -384,6 +390,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'globalVar':
                                $values = GeneralUtility::trimExplode(',', $value, TRUE);
@@ -396,6 +403,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'globalString':
                                $values = GeneralUtility::trimExplode(',', $value, TRUE);
@@ -408,6 +416,7 @@ abstract class AbstractConditionMatcher {
                                                return TRUE;
                                        }
                                }
+                               return FALSE;
                                break;
                        case 'userFunc':
                                $matches = array();
@@ -417,6 +426,7 @@ abstract class AbstractConditionMatcher {
                                if (function_exists($funcName) && call_user_func_array($funcName, $funcValues)) {
                                        return TRUE;
                                }
+                               return FALSE;
                                break;
                }
                return NULL;
diff --git a/typo3/sysext/core/Classes/Configuration/TypoScript/Exception/InvalidTypoScriptConditionException.php b/typo3/sysext/core/Classes/Configuration/TypoScript/Exception/InvalidTypoScriptConditionException.php
new file mode 100644 (file)
index 0000000..5045cdb
--- /dev/null
@@ -0,0 +1,25 @@
+<?php
+namespace TYPO3\CMS\Core\Configuration\TypoScript\Exception;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * A "Your TypoScript condition is invalid" exception
+ * used when a TypoScript condition is called but not extending
+ * from the AbstractCondition class.
+ *
+ * @api
+ */
+class InvalidTypoScriptConditionException extends \TYPO3\CMS\Core\Exception {
+}
\ No newline at end of file
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Feature-61489-AbstractTypoScriptCondition.rst b/typo3/sysext/core/Documentation/Changelog/master/Feature-61489-AbstractTypoScriptCondition.rst
new file mode 100644 (file)
index 0000000..d6e2835
--- /dev/null
@@ -0,0 +1,36 @@
+================================================================
+Feature: #61489 - Allow own TypoScript Condition implementations
+================================================================
+
+Description
+===========
+
+It is now possible to add own TypoScript conditions via a
+separate API.
+
+An extension / package can now ship an implementation of a new
+abstract class AbstractCondition. Via the existing TypoScript
+Condition Syntax the class is called by the simple full namespaced
+class name.
+The class's main function "matchCondition" can flexibly evaluate
+any parameters given after the class name.
+
+Usage:
+
+.. code-block:: typoscript
+
+       [BigCompanyName\TypoScriptLovePackage\BennisTypoScriptCondition]
+
+       [BigCompanyName\TypoScriptLovePackage\BennisTypoScriptCondition = 7]
+
+       [BigCompanyName\TypoScriptLovePackage\BennisTypoScriptCondition = 7, != 6]
+
+       [BigCompanyName\TypoScriptLovePackage\BennisTypoScriptCondition = {$mysite.myconstant}]
+
+where the TypoScript Condition class deals with =/!= etc itself.
+
+Impact
+======
+
+If you've previously used the "userFunc" condition, it is encouraged
+to use this new API for your own TypoScript conditions.
index 704e251..c8ac10f 100644 (file)
@@ -133,7 +133,7 @@ class AbstractConditionMatcherTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                );
                $method->setAccessible(TRUE);
 
-               $this->assertNull(
+               $this->assertFalse(
                        $method->invokeArgs($abstractConditionMatcherMock, array('applicationContext', $notMatchingApplicationContextCondition))
                );
        }
@@ -171,22 +171,22 @@ class AbstractConditionMatcherTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                        'IP does not match' => array(
                                '127.0.0.1',
                                '127.0.0.2',
-                               NULL
+                               FALSE
                        ),
                        'ipv4 subnet does not match' => array(
                                '127.0.0.1/8',
                                '126.0.0.1',
-                               NULL
+                               FALSE
                        ),
                        'ipv6 subnet does not match' => array(
                                '::1/127',
                                '::2',
-                               NULL
+                               FALSE
                        ),
                        'List of addresses does not match' => array(
                                '127.0.0.1, ::1',
                                '::2',
-                               NULL
+                               FALSE
                        ),
                );
        }
index 70b0c52..b6cf6c3 100644 (file)
@@ -13,6 +13,9 @@ namespace TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 /**
  * Matching TypoScript conditions for frontend disposal.
  *
@@ -29,9 +32,10 @@ class ConditionMatcher extends \TYPO3\CMS\Core\Configuration\TypoScript\Conditio
         * @param string $string The condition to match against its criterias.
         * @return boolean Whether the condition matched
         * @see \TYPO3\CMS\Core\TypoScript\Parser\TypoScriptParser::parse()
+        * @throws \TYPO3\CMS\Core\Configuration\TypoScript\Exception\InvalidTypoScriptConditionException
         */
        protected function evaluateCondition($string) {
-               list($key, $value) = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode('=', $string, FALSE, 2);
+               list($key, $value) = GeneralUtility::trimExplode('=', $string, FALSE, 2);
                $result = parent::evaluateConditionCommon($key, $value);
                if (is_bool($result)) {
                        return $result;
@@ -41,16 +45,16 @@ class ConditionMatcher extends \TYPO3\CMS\Core\Configuration\TypoScript\Conditio
                                        $groupList = $this->getGroupList();
                                        // '0,-1' is the default usergroups when not logged in!
                                        if ($groupList != '0,-1') {
-                                               $values = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $value, TRUE);
+                                               $values = GeneralUtility::trimExplode(',', $value, TRUE);
                                                foreach ($values as $test) {
-                                                       if ($test == '*' || \TYPO3\CMS\Core\Utility\GeneralUtility::inList($groupList, $test)) {
+                                                       if ($test == '*' || GeneralUtility::inList($groupList, $test)) {
                                                                return TRUE;
                                                        }
                                                }
                                        }
                                        break;
                                case 'treeLevel':
-                                       $values = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $value, TRUE);
+                                       $values = GeneralUtility::trimExplode(',', $value, TRUE);
                                        $treeLevel = count($this->rootline) - 1;
                                        foreach ($values as $test) {
                                                if ($test == $treeLevel) {
@@ -61,7 +65,7 @@ class ConditionMatcher extends \TYPO3\CMS\Core\Configuration\TypoScript\Conditio
                                case 'PIDupinRootline':
 
                                case 'PIDinRootline':
-                                       $values = \TYPO3\CMS\Core\Utility\GeneralUtility::trimExplode(',', $value, TRUE);
+                                       $values = GeneralUtility::trimExplode(',', $value, TRUE);
                                        if ($key == 'PIDinRootline' || !in_array($this->pageId, $values)) {
                                                foreach ($values as $test) {
                                                        foreach ($this->rootline as $rl_dat) {
@@ -72,6 +76,26 @@ class ConditionMatcher extends \TYPO3\CMS\Core\Configuration\TypoScript\Conditio
                                                }
                                        }
                                        break;
+                               default:
+                                       list($conditionClassName, $conditionParameters) = GeneralUtility::trimExplode(' ', $string, FALSE, 2);
+
+                                       // Check if the condition class name is a valid class
+                                       // This is necessary to not stop here for the conditions ELSE and GLOBAL
+                                       if (class_exists($conditionClassName)) {
+                                               // Use like this: [MyCompany\MyPackage\ConditionMatcher\MyOwnConditionMatcher = myvalue]
+                                               /** @var \TYPO3\CMS\Core\Configuration\TypoScript\ConditionMatching\AbstractCondition $conditionObject */
+                                               $conditionObject = GeneralUtility::makeInstance($conditionClassName);
+                                               if (($conditionObject instanceof \TYPO3\CMS\Core\Configuration\TypoScript\ConditionMatching\AbstractCondition) === FALSE) {
+                                                       throw new \TYPO3\CMS\Core\Configuration\TypoScript\Exception\InvalidTypoScriptConditionException(
+                                                               '"' . $key . '" is not a valid TypoScript Condition object.',
+                                                               1410286153
+                                                       );
+                                               }
+
+                                               $conditionParameters = $this->parseUserFuncArguments($conditionParameters);
+                                               $conditionObject->setConditionMatcherInstance($this);
+                                               return $conditionObject->matchCondition($conditionParameters);
+                                       }
                        }
                }
                return FALSE;
index 4918e5b..f79d773 100644 (file)
@@ -15,9 +15,7 @@ namespace TYPO3\CMS\Frontend\Tests\Unit\Configuration\TypoScript\ConditionMatchi
  */
 
 /**
- * Testcase for class \TYPO3\CMS\Frontend\Configuration\TypoScript\ConditionMatching\ConditionMatcher.
- *
- * @author     Oliver Hader <oliver@typo3.org>
+ * Test case
  */
 class ConditionMatcherTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
 
@@ -706,4 +704,19 @@ class ConditionMatcherTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                $this->assertTrue($this->matchCondition->match('[globalString = ' . $this->testGlobalNamespace . '|second|third = testThird]'));
        }
 
+       /**
+        * @test
+        * @expectedException \TYPO3\CMS\Core\Configuration\TypoScript\Exception\InvalidTypoScriptConditionException
+        */
+       public function matchThrowsExceptionIfConditionClassDoesNotInheritFromAbstractCondition() {
+               $this->matchCondition->match('[stdClass = foo]');
+       }
+
+       /**
+        * @test
+        * @expectedException \TYPO3\CMS\Frontend\Tests\Unit\Configuration\TypoScript\ConditionMatching\Fixtures\TestConditionException
+        */
+       public function matchCallsTestConditionAndHandsOverParameters() {
+               $this->matchCondition->match('[TYPO3\\CMS\\Frontend\\Tests\\Unit\\Configuration\\TypoScript\\ConditionMatching\\Fixtures\\TestCondition = 7, != 6]');
+       }
 }
diff --git a/typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestCondition.php b/typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestCondition.php
new file mode 100644 (file)
index 0000000..e72c23a
--- /dev/null
@@ -0,0 +1,35 @@
+<?php
+namespace TYPO3\CMS\Frontend\Tests\Unit\Configuration\TypoScript\ConditionMatching\Fixtures;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * Fixture for custom conditions
+ */
+class TestCondition extends \TYPO3\CMS\Core\Configuration\TypoScript\ConditionMatching\AbstractCondition {
+
+       /**
+        * Test matcher tests input parameters.
+        *
+        * @param array $conditionParameters
+        * @throws \RuntimeException
+        * @return bool
+        */
+       public function matchCondition(array $conditionParameters) {
+               // Throw an exception if everything is fine, this exception is *expected* in the according unit test
+               if ($conditionParameters[0] === '= 7' && $conditionParameters[1] === '!= 6') {
+                       throw new TestConditionException('All Ok', 1411581139);
+               }
+       }
+}
\ No newline at end of file
diff --git a/typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestConditionException.php b/typo3/sysext/frontend/Tests/Unit/Configuration/TypoScript/ConditionMatching/Fixtures/TestConditionException.php
new file mode 100644 (file)
index 0000000..fffa0e1
--- /dev/null
@@ -0,0 +1,21 @@
+<?php
+namespace TYPO3\CMS\Frontend\Tests\Unit\Configuration\TypoScript\ConditionMatching\Fixtures;
+
+/**
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * Exception thrown by TestCondition
+ */
+class TestConditionException extends \Exception {
+}
\ No newline at end of file