From f699650b5fdb0c53b14484b5c40e76d812c0be64 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 27 Jun 2017 10:19:51 +1200 Subject: [PATCH] Update based on feedback --- .../02_Controllers/05_Middlewares.md | 3 +-- src/Control/Director.php | 2 +- .../Middleware/TrustedProxyMiddleware.php | 23 +++++-------------- src/Core/Injector/Injector.php | 12 ++++------ tests/php/Core/Injector/InjectorTest.php | 18 ++------------- 5 files changed, 14 insertions(+), 44 deletions(-) diff --git a/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md b/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md index 50c5015a0..1069cc84c 100644 --- a/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md +++ b/docs/en/02_Developer_Guides/02_Controllers/05_Middlewares.md @@ -111,9 +111,8 @@ property. The controller which does the work should be registered under the SilverStripe\Control\Director: rules: special\section: - Controller: SpecialRouteMiddleware + Controller: %$SpecialRouteMiddleware ## API Documentation * [api:SilverStripe\Control\HTTPMiddleware] - diff --git a/src/Control/Director.php b/src/Control/Director.php index 3921b8164..1977cffa6 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -372,7 +372,7 @@ class Director implements TemplateGlobalProvider // Note that if a different request was previously registered, this will now be lost // In these cases it's better to use Kernel::nest() prior to kicking off a nested request - Injector::inst()->unregisterNamedObject(HTTPRequest::class, true); + Injector::inst()->unregisterNamedObject(HTTPRequest::class); return $response; } diff --git a/src/Control/Middleware/TrustedProxyMiddleware.php b/src/Control/Middleware/TrustedProxyMiddleware.php index 39ab45141..0d865e6c3 100644 --- a/src/Control/Middleware/TrustedProxyMiddleware.php +++ b/src/Control/Middleware/TrustedProxyMiddleware.php @@ -82,17 +82,13 @@ class TrustedProxyMiddleware implements HTTPMiddleware } /** - * Set the array of headers from which to lookup the hostname - * Can also specify comma-separated list as a single string. + * Set the array of headers from which to lookup the hostname. * - * @param array|string $proxyHostHeaders + * @param array $proxyHostHeaders * @return $this */ public function setProxyHostHeaders($proxyHostHeaders) { - if (is_string($proxyHostHeaders)) { - $proxyHostHeaders = preg_split('/ *, */', $proxyHostHeaders); - } $this->proxyHostHeaders = $proxyHostHeaders ?: []; return $this; } @@ -108,17 +104,13 @@ class TrustedProxyMiddleware implements HTTPMiddleware } /** - * Set the array of headers from which to lookup the client IP - * Can also specify comma-separated list as a single string. + * Set the array of headers from which to lookup the client IP. * - * @param array|string $proxyIPHeaders + * @param array $proxyIPHeaders * @return $this */ public function setProxyIPHeaders($proxyIPHeaders) { - if (is_string($proxyIPHeaders)) { - $proxyIPHeaders = preg_split('/ *, */', $proxyIPHeaders); - } $this->proxyIPHeaders = $proxyIPHeaders ?: []; return $this; } @@ -137,14 +129,11 @@ class TrustedProxyMiddleware implements HTTPMiddleware * Set array of headers from which to lookup the client scheme (http/https) * Can also specify comma-separated list as a single string. * - * @param array|string $proxySchemeHeaders + * @param array $proxySchemeHeaders * @return $this */ public function setProxySchemeHeaders($proxySchemeHeaders) { - if (is_string($proxySchemeHeaders)) { - $proxySchemeHeaders = preg_split('/ *, */', $proxySchemeHeaders); - } $this->proxySchemeHeaders = $proxySchemeHeaders ?: []; return $this; } @@ -210,7 +199,7 @@ class TrustedProxyMiddleware implements HTTPMiddleware // Validate IP address $ip = $request->getIP(); if ($ip) { - return IPUtils::checkIP($ip, preg_split('/ *, */', $trustedIPs)); + return IPUtils::checkIP($ip, preg_split('/\s*,\s*/', $trustedIPs)); } return false; diff --git a/src/Core/Injector/Injector.php b/src/Core/Injector/Injector.php index 02754bf46..2156c1969 100644 --- a/src/Core/Injector/Injector.php +++ b/src/Core/Injector/Injector.php @@ -851,15 +851,12 @@ class Injector implements ContainerInterface * by the inject * * @param string $name The name to unregister - * @param bool $flushSpecs Set to true to clear spec for this service * @return $this */ - public function unregisterNamedObject($name, $flushSpecs = false) + public function unregisterNamedObject($name) { unset($this->serviceCache[$name]); - if ($flushSpecs) { - unset($this->specs[$name]); - } + unset($this->specs[$name]); return $this; } @@ -867,10 +864,9 @@ class Injector implements ContainerInterface * Clear out objects of one or more types that are managed by the injetor. * * @param array|string $types Base class of object (not service name) to remove - * @param bool $flushSpecs Set to true to clear spec for this service * @return $this */ - public function unregisterObjects($types, $flushSpecs = false) + public function unregisterObjects($types) { if (!is_array($types)) { $types = [ $types ]; @@ -884,7 +880,7 @@ class Injector implements ContainerInterface throw new InvalidArgumentException("Global unregistration is not allowed"); } if ($object instanceof $filterClass) { - $this->unregisterNamedObject($key, $flushSpecs); + $this->unregisterNamedObject($key); break; } } diff --git a/tests/php/Core/Injector/InjectorTest.php b/tests/php/Core/Injector/InjectorTest.php index bca527c10..4501bbaa1 100644 --- a/tests/php/Core/Injector/InjectorTest.php +++ b/tests/php/Core/Injector/InjectorTest.php @@ -811,15 +811,8 @@ class InjectorTest extends SapphireTest $this->assertTrue($injector->has('NamedService')); $this->assertEquals($service, $injector->get('NamedService')); - // Unregister by name only: New instance of the - // old class will be constructed + // Unregister service by name $injector->unregisterNamedObject('NamedService'); - $this->assertTrue($injector->has('NamedService')); - $this->assertNotEquals($service, $injector->get(TestObject::class)); - - // Unregister name and spec, injector forgets about this - // service spec altogether - $injector->unregisterNamedObject('NamedService', true); $this->assertFalse($injector->has('NamedService')); // Test registered with class name @@ -827,15 +820,8 @@ class InjectorTest extends SapphireTest $this->assertTrue($injector->has(TestObject::class)); $this->assertEquals($service, $injector->get(TestObject::class)); - // Unregister by name only: New instance of the - // old class will be constructed + // Unregister service by class $injector->unregisterNamedObject(TestObject::class); - $this->assertTrue($injector->has(TestObject::class)); - $this->assertNotEquals($service, $injector->get(TestObject::class)); - - // Unregister name and spec, injector forgets about this - // service spec altogether - $injector->unregisterNamedObject(TestObject::class, true); $this->assertFalse($injector->has(TestObject::class)); }