From 39a29fa2f65ebd9a3ad486c6d85249322bcb9a64 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Tue, 5 Mar 2019 16:59:32 +1300 Subject: [PATCH 1/5] ENHANCEMENT: has_extension() should allow injector overrides --- src/Core/Extensible.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index 79fe536b6..805aab2dd 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -388,6 +388,9 @@ trait Extensible if (!$strict && is_subclass_of($extension, $requiredExtension)) { return true; } + if (Injector::inst()->get($extension) instanceof $requiredExtension) { + return true; + } } return false; From aa491d92940282a62748bbfe4f69a1e56b0ce2d8 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 20 Mar 2019 12:32:11 +1300 Subject: [PATCH 2/5] Fix tests --- src/Control/Director.php | 5 +++-- src/Control/Middleware/HTTPMiddlewareAware.php | 11 ++++++++++- src/Core/Extensible.php | 5 +++-- tests/php/Core/ObjectTest.php | 8 ++++++++ tests/php/Core/ObjectTest/ExtendTest5.php | 11 +++++++++++ 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 tests/php/Core/ObjectTest/ExtendTest5.php diff --git a/src/Control/Director.php b/src/Control/Director.php index 8b85e1fee..51827f628 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -365,12 +365,13 @@ class Director implements TemplateGlobalProvider }; break; } - +$start = time(); // Call the handler with the configured middlewares $response = $this->callMiddleware($request, $handler); - +$end = time(); // 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 + $elapsed = $end - $start; Injector::inst()->unregisterNamedObject(HTTPRequest::class); return $response; diff --git a/src/Control/Middleware/HTTPMiddlewareAware.php b/src/Control/Middleware/HTTPMiddlewareAware.php index e7e861898..36ff486c1 100644 --- a/src/Control/Middleware/HTTPMiddlewareAware.php +++ b/src/Control/Middleware/HTTPMiddlewareAware.php @@ -54,12 +54,21 @@ trait HTTPMiddlewareAware */ protected function callMiddleware(HTTPRequest $request, callable $last) { + $classes = array_map(function ($middle) { + return get_class($middle); + }, $this->getMiddlewares()); + + // Reverse middlewares $next = $last; /** @var HTTPMiddleware $middleware */ foreach (array_reverse($this->getMiddlewares()) as $middleware) { $next = function ($request) use ($middleware, $next) { - return $middleware->process($request, $next); + $start = time(); + $value = $middleware->process($request, $next); + $end = time(); + $elapsed = $end - $start; + return $value; }; } return $next($request); diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index 805aab2dd..b8e2c7fe7 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -388,8 +388,9 @@ trait Extensible if (!$strict && is_subclass_of($extension, $requiredExtension)) { return true; } - if (Injector::inst()->get($extension) instanceof $requiredExtension) { - return true; + $inst = Injector::inst()->get($extension); + if ($inst instanceof $requiredExtension) { + return $strict ? get_class($inst) === $requiredExtension : true; } } diff --git a/tests/php/Core/ObjectTest.php b/tests/php/Core/ObjectTest.php index 886a5cd0a..b3971e987 100644 --- a/tests/php/Core/ObjectTest.php +++ b/tests/php/Core/ObjectTest.php @@ -11,6 +11,7 @@ use SilverStripe\Core\Tests\ObjectTest\ExtendTest1; use SilverStripe\Core\Tests\ObjectTest\ExtendTest2; use SilverStripe\Core\Tests\ObjectTest\ExtendTest3; use SilverStripe\Core\Tests\ObjectTest\ExtendTest4; +use SilverStripe\Core\Tests\ObjectTest\ExtendTest5; use SilverStripe\Core\Tests\ObjectTest\ExtensionRemoveTest; use SilverStripe\Core\Tests\ObjectTest\ExtensionTest; use SilverStripe\Core\Tests\ObjectTest\ExtensionTest2; @@ -289,6 +290,13 @@ class ObjectTest extends SapphireTest $objectTest_ExtensionTest->hasExtension(ExtendTest3::class), "Extensions are detected with instance hasExtension() when added through add_extension()" ); + + // load in a custom implementation + Injector::inst()->registerService(new ExtendTest5(), ExtendTest4::class); + $this->assertTrue( + ExtensionTest3::has_extension(ExtendTest5::class), + "Injected sub-extensions are detected with static has_extension() when added through add_extension()" + ); // @todo At the moment, this does NOT remove the extension due to parameterized naming, // meaning the extension will remain added in further test cases diff --git a/tests/php/Core/ObjectTest/ExtendTest5.php b/tests/php/Core/ObjectTest/ExtendTest5.php new file mode 100644 index 000000000..814c192b7 --- /dev/null +++ b/tests/php/Core/ObjectTest/ExtendTest5.php @@ -0,0 +1,11 @@ + Date: Wed, 20 Mar 2019 13:08:56 +1300 Subject: [PATCH 3/5] Fix linting --- src/Control/Director.php | 5 ++--- src/Control/Middleware/HTTPMiddlewareAware.php | 11 +---------- tests/behat/src/CmsUiContext.php | 6 +++--- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/Control/Director.php b/src/Control/Director.php index 51827f628..8b85e1fee 100644 --- a/src/Control/Director.php +++ b/src/Control/Director.php @@ -365,13 +365,12 @@ class Director implements TemplateGlobalProvider }; break; } -$start = time(); + // Call the handler with the configured middlewares $response = $this->callMiddleware($request, $handler); -$end = time(); + // 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 - $elapsed = $end - $start; Injector::inst()->unregisterNamedObject(HTTPRequest::class); return $response; diff --git a/src/Control/Middleware/HTTPMiddlewareAware.php b/src/Control/Middleware/HTTPMiddlewareAware.php index 36ff486c1..e7e861898 100644 --- a/src/Control/Middleware/HTTPMiddlewareAware.php +++ b/src/Control/Middleware/HTTPMiddlewareAware.php @@ -54,21 +54,12 @@ trait HTTPMiddlewareAware */ protected function callMiddleware(HTTPRequest $request, callable $last) { - $classes = array_map(function ($middle) { - return get_class($middle); - }, $this->getMiddlewares()); - - // Reverse middlewares $next = $last; /** @var HTTPMiddleware $middleware */ foreach (array_reverse($this->getMiddlewares()) as $middleware) { $next = function ($request) use ($middleware, $next) { - $start = time(); - $value = $middleware->process($request, $next); - $end = time(); - $elapsed = $end - $start; - return $value; + return $middleware->process($request, $next); }; } return $next($request); diff --git a/tests/behat/src/CmsUiContext.php b/tests/behat/src/CmsUiContext.php index 6797dc65f..1946f30cc 100644 --- a/tests/behat/src/CmsUiContext.php +++ b/tests/behat/src/CmsUiContext.php @@ -58,9 +58,9 @@ class CmsUiContext implements Context $timeoutMs = $this->getMainContext()->getAjaxTimeout(); $this->getSession()->wait( $timeoutMs, - "(". - "document.getElementsByClassName('cms-content-loading-overlay').length +". - "document.getElementsByClassName('cms-loading-container').length". + "(" . + "document.getElementsByClassName('cms-content-loading-overlay').length +" . + "document.getElementsByClassName('cms-loading-container').length" . ") == 0" ); } From f6a7bddd8dbc3d769830b3f394c7c2e88b7336f8 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 20 Mar 2019 14:33:34 +1300 Subject: [PATCH 4/5] Linting --- src/Forms/GridField/GridFieldLevelup.php | 2 +- src/includes/constants.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Forms/GridField/GridFieldLevelup.php b/src/Forms/GridField/GridFieldLevelup.php index d16d41a88..84d92af19 100644 --- a/src/Forms/GridField/GridFieldLevelup.php +++ b/src/Forms/GridField/GridFieldLevelup.php @@ -88,7 +88,7 @@ class GridFieldLevelup implements GridField_HTMLProvider $template = SSViewer::get_templates_by_class($this, '', __CLASS__); return array( 'before' => $forTemplate->renderWith($template), - ); + ); } public function setAttributes($attrs) diff --git a/src/includes/constants.php b/src/includes/constants.php index a619db5f2..277230b3d 100644 --- a/src/includes/constants.php +++ b/src/includes/constants.php @@ -139,11 +139,11 @@ if (!defined('BASE_URL')) { ) { $requestURI = $_SERVER['REQUEST_URI']; // Check if /base/public or /base are in the request - foreach ([$baseURL, dirname($baseURL)] as $candidate) { - if (stripos($requestURI, $candidate) === 0) { - return $candidate; - } + foreach ([$baseURL, dirname($baseURL)] as $candidate) { + if (stripos($requestURI, $candidate) === 0) { + return $candidate; } + } // Ambiguous return ''; } From 9eac374b1371a80ddf6e7c57b212b876efc788a9 Mon Sep 17 00:00:00 2001 From: Aaron Carlino Date: Wed, 27 Mar 2019 11:41:23 +1300 Subject: [PATCH 5/5] Use strcasecmp --- src/Core/Extensible.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index b8e2c7fe7..24b623912 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -390,7 +390,7 @@ trait Extensible } $inst = Injector::inst()->get($extension); if ($inst instanceof $requiredExtension) { - return $strict ? get_class($inst) === $requiredExtension : true; + return $strict ? strcasecmp(get_class($inst), $requiredExtension) === 0 : true; } }