diff --git a/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php b/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php index c88e1ea..ea3b818 100644 --- a/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php +++ b/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php @@ -36,26 +36,23 @@ class ChainRequestPolicy implements ChainRequestPolicyInterface { * {@inheritdoc} */ public function check(Request $request) { - $deny = FALSE; - $allow = FALSE; + $overall_result = NULL; foreach ($this->rules as $rule) { $result = $rule->check($request); if ($result === static::ALLOW) { - $allow = $result; + $overall_result = $result; } elseif ($result === static::DENY) { - $deny = $result; + $overall_result = $result; break; } + elseif (isset($result)) { + throw new \UnexpectedValueException('Return value of RequestPolicyInterface::check() must be one of RequestPolicyInterface::ALLOW, RequestPolicyInterface::DENY or NULL'); + } } - if ($deny) { - return $deny; - } - elseif ($allow) { - return $allow; - } + return $overall_result; } /** diff --git a/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php b/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php index c9a334e..20d5e39 100644 --- a/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php +++ b/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php @@ -35,8 +35,12 @@ class ChainResponsePolicy implements ChainResponsePolicyInterface { */ public function check(Response $response, Request $request) { foreach ($this->rules as $rule) { - if ($rule->check($response, $request) === static::DENY) { - return static::DENY; + $result = $rule->check($response, $request); + if ($result === static::DENY) { + return $result; + } + elseif (isset($result)) { + throw new \UnexpectedValueException('Return value of ResponsePolicyInterface::check() must be one of ResponsePolicyInterface::DENY or NULL'); } } } diff --git a/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php b/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php index fd41185..e3e779c 100644 --- a/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php +++ b/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php @@ -48,12 +48,31 @@ public function testEmptyChain() { } /** - * Asserts that check() returns NULL if none of the rules returns an actionable result. + * Asserts that check() returns NULL if a rule returns NULL. * - * @dataProvider providerChainNoResultOnInvalidReturnValue * @covers ::check */ - public function testChainNoResultOnInvalidReturnValue($return_value) { + public function testNullRuleChain() { + $rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $rule->expects($this->once()) + ->method('check') + ->with($this->request) + ->will($this->returnValue(NULL)); + + $this->policy->addPolicy($rule); + + $result = $this->policy->check($this->request); + $this->assertSame(NULL, $result); + } + + /** + * Asserts that check() throws an exception if a rule returns an invalid value. + * + * @expectedException \UnexpectedValueException + * @dataProvider providerChainExceptionOnInvalidReturnValue + * @covers ::check + */ + public function testChainExceptionOnInvalidReturnValue($return_value) { $rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); $rule->expects($this->once()) ->method('check') @@ -62,19 +81,17 @@ public function testChainNoResultOnInvalidReturnValue($return_value) { $this->policy->addPolicy($rule); - $actual_result = $this->policy->check($this->request); - $this->assertSame(NULL, $actual_result); + $this->policy->check($this->request); } /** - * Provides test data for testChainNoResultOnInvalidReturnValue. + * Provides test data for testChainExceptionOnInvalidReturnValue. * * @return array * Test input and expected result. */ - public function providerChainNoResultOnInvalidReturnValue() { + public function providerChainExceptionOnInvalidReturnValue() { return [ - [NULL], [FALSE], [0], [1], @@ -115,12 +132,6 @@ public function providerAllowIfAnyRuleReturnedAllow() { return [ [[RequestPolicyInterface::ALLOW]], [[NULL, RequestPolicyInterface::ALLOW]], - [[RequestPolicyInterface::ALLOW, FALSE]], - [[0, RequestPolicyInterface::ALLOW]], - [[RequestPolicyInterface::ALLOW, 1]], - [[TRUE, RequestPolicyInterface::ALLOW]], - [[RequestPolicyInterface::ALLOW, [1, 2, 3]]], - [[new \stdClass(), RequestPolicyInterface::ALLOW]], ]; } diff --git a/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php b/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php index 8195e4c..2dbfba3 100644 --- a/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php +++ b/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php @@ -57,12 +57,31 @@ public function testEmptyChain() { } /** - * Asserts that check() returns NULL if none of the rules returns an actionable result. + * Asserts that check() returns NULL if a rule returns NULL. * - * @dataProvider providerChainNoResultOnInvalidReturnValue * @covers ::check */ - public function testChainNoResultOnInvalidReturnValue($return_value) { + public function testNullRuleChain() { + $rule = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); + $rule->expects($this->once()) + ->method('check') + ->with($this->response, $this->request) + ->will($this->returnValue(NULL)); + + $this->policy->addPolicy($rule); + + $result = $this->policy->check($this->response, $this->request); + $this->assertSame(NULL, $result); + } + + /** + * Asserts that check() throws an exception if a rule returns an invalid value. + * + * @expectedException \UnexpectedValueException + * @dataProvider providerChainExceptionOnInvalidReturnValue + * @covers ::check + */ + public function testChainExceptionOnInvalidReturnValue($return_value) { $rule = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); $rule->expects($this->once()) ->method('check') @@ -76,14 +95,13 @@ public function testChainNoResultOnInvalidReturnValue($return_value) { } /** - * Provides test data for testChainNoResultOnInvalidReturnValue. + * Provides test data for testChainExceptionOnInvalidReturnValue. * * @return array * Test input and expected result. */ - public function providerChainNoResultOnInvalidReturnValue() { + public function providerChainExceptionOnInvalidReturnValue() { return [ - [NULL], [FALSE], [0], [1],