From 7ace7e26250bd0dc7ccfaa822ee43e4237282850 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Mon, 6 Mar 2023 11:47:12 +0100 Subject: [PATCH] First refactoring This got extracted from integrating Matomo tracking. We migrate to an event in order to remove Singleton instance. The event transports info and foreign code can react. We streamline architecture and split logic from integration. --- Classes/Cookie.php | 4 +- Classes/Events/SwitchedToVariant.php | 48 +++++++ Classes/Hook/TypoScriptFrontendController.php | 50 +++++++ Classes/Middleware/SetCookie.php | 9 ++ Classes/Switcher.php | 45 ++---- Configuration/Services.php | 10 +- README.md | 5 + Tests/Fixtures/BasicDatabase.csv | 20 +-- Tests/Functional/FrontendRenderingTest.php | 132 +++++++++--------- ext_localconf.php | 2 +- 10 files changed, 211 insertions(+), 114 deletions(-) create mode 100644 Classes/Events/SwitchedToVariant.php create mode 100644 Classes/Hook/TypoScriptFrontendController.php diff --git a/Classes/Cookie.php b/Classes/Cookie.php index 209de45..09000dd 100644 --- a/Classes/Cookie.php +++ b/Classes/Cookie.php @@ -23,9 +23,7 @@ declare(strict_types=1); namespace WerkraumMedia\ABTest; -use TYPO3\CMS\Core\SingletonInterface; - -class Cookie implements SingletonInterface +class Cookie { /** * @var int diff --git a/Classes/Events/SwitchedToVariant.php b/Classes/Events/SwitchedToVariant.php new file mode 100644 index 0000000..50eeda2 --- /dev/null +++ b/Classes/Events/SwitchedToVariant.php @@ -0,0 +1,48 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +namespace WerkraumMedia\ABTest\Events; + +final class SwitchedToVariant +{ + private array $originalPage; + private array $variantPage; + + public function __construct( + array $originalPage, + array $variantPage + ) { + $this->originalPage = $originalPage; + $this->variantPage = $variantPage; + } + + public function getOriginalPage(): array + { + return $this->originalPage; + } + + public function getVariantPage(): array + { + return $this->variantPage; + } +} diff --git a/Classes/Hook/TypoScriptFrontendController.php b/Classes/Hook/TypoScriptFrontendController.php new file mode 100644 index 0000000..0d69e16 --- /dev/null +++ b/Classes/Hook/TypoScriptFrontendController.php @@ -0,0 +1,50 @@ + + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +namespace WerkraumMedia\ABTest\Hook; + +use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController as Typo3TypoScriptFrontendController; +use WerkraumMedia\ABTest\Switcher; + +class TypoScriptFrontendController +{ + private Switcher $switcher; + + public function __construct( + Switcher $switcher + ) { + $this->switcher = $switcher; + } + + public function determineIdPostProc( + array $params, + Typo3TypoScriptFrontendController $frontendController + ): void { + $this->switcher->switch($frontendController); + } + + public static function register(): void + { + $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/class.tslib_fe.php']['determineId-PostProc'][self::class] = self::class . '->determineIdPostProc'; + } +} diff --git a/Classes/Middleware/SetCookie.php b/Classes/Middleware/SetCookie.php index 0ff4765..612e37e 100644 --- a/Classes/Middleware/SetCookie.php +++ b/Classes/Middleware/SetCookie.php @@ -29,6 +29,7 @@ use Psr\Http\Server\MiddlewareInterface; use Psr\Http\Server\RequestHandlerInterface; use Symfony\Component\HttpFoundation\Cookie as SymfonyCookie; use WerkraumMedia\ABTest\Cookie; +use WerkraumMedia\ABTest\Events\SwitchedToVariant; class SetCookie implements MiddlewareInterface { @@ -43,6 +44,14 @@ class SetCookie implements MiddlewareInterface $this->cookie = $cookie; } + public function handleVariant(SwitchedToVariant $event): void + { + $targetPage = $event->getVariantPage(); + $this->cookie->setRequestedPage((int)$event->getOriginalPage()['uid']); + $this->cookie->setActualPage((int)$targetPage['uid']); + $this->cookie->setLifetime((int)$targetPage['tx_abtest_cookie_time']); + } + public function process( ServerRequestInterface $request, RequestHandlerInterface $handler diff --git a/Classes/Switcher.php b/Classes/Switcher.php index 1d9d6ea..ef72bb2 100644 --- a/Classes/Switcher.php +++ b/Classes/Switcher.php @@ -24,9 +24,10 @@ declare(strict_types=1); namespace WerkraumMedia\ABTest; use DeviceDetector\DeviceDetector; +use TYPO3\CMS\Core\EventDispatcher\EventDispatcher; use TYPO3\CMS\Core\Http\ServerRequest; -use TYPO3\CMS\Core\Site\Entity\Site; use TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController; +use WerkraumMedia\ABTest\Events\SwitchedToVariant; /** * Will decide whether to switch to another variant. @@ -35,31 +36,28 @@ class Switcher { private PageRepository $pageRepository; - private Cookie $cookie; + private EventDispatcher $eventDispatcher; public function __construct( PageRepository $pageRepository, - Cookie $cookie + EventDispatcher $eventDispatcher ) { $this->pageRepository = $pageRepository; - $this->cookie = $cookie; + $this->eventDispatcher = $eventDispatcher; } - public function determineContentId( - array $params, - TypoScriptFrontendController $frontendController - ): void { + public function switch(TypoScriptFrontendController $frontendController): void + { if ($this->isRequestByBot()) { return; } $currentPageId = $frontendController->id; if (is_numeric($currentPageId) === false) { - $currentPageId = $this->getRootPageId(); - } else { - $currentPageId = (int)$currentPageId; + return; } + $currentPageId = (int)$currentPageId; if ($currentPageId === 0) { return; } @@ -85,9 +83,10 @@ class Switcher $this->pageRepository->updateCounter((int)$targetPage['uid'], ++$targetPage['tx_abtest_counter']); } - $this->cookie->setRequestedPage($currentPageId); - $this->cookie->setActualPage($targetPage['uid']); - $this->cookie->setLifetime($targetPage['tx_abtest_cookie_time']); + $this->eventDispatcher->dispatch(new SwitchedToVariant( + $currentPagePropertiesArray, + $targetPage + )); } private function isRequestByBot(): bool @@ -103,19 +102,6 @@ class Switcher return false; } - /** - * Returns 0 if no site could be fetched. - */ - private function getRootPageId(): int - { - $site = $this->getRequest()->getAttribute('site'); - if (!$site instanceof Site) { - return 0; - } - - return $site->getRootPageId(); - } - private function getRequest(): ServerRequest { return $GLOBALS['TYPO3_REQUEST']; @@ -141,9 +127,4 @@ class Switcher return $page; } - - public static function register(): void - { - $GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['tslib/class.tslib_fe.php']['determineId-PostProc'][self::class] = self::class . '->determineContentId'; - } } diff --git a/Configuration/Services.php b/Configuration/Services.php index e0d6052..85de5c9 100644 --- a/Configuration/Services.php +++ b/Configuration/Services.php @@ -5,7 +5,9 @@ declare(strict_types=1); namespace DanielSiepmann\Configuration; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; -use WerkraumMedia\ABTest\Switcher; +use WerkraumMedia\ABTest\Events\SwitchedToVariant; +use WerkraumMedia\ABTest\Hook\TypoScriptFrontendController; +use WerkraumMedia\ABTest\Middleware\SetCookie; use WerkraumMedia\ABTest\TCA\VariantFilter; return static function (ContainerConfigurator $containerConfigurator) { @@ -18,6 +20,10 @@ return static function (ContainerConfigurator $containerConfigurator) { $services->load('WerkraumMedia\\ABTest\\', '../Classes/'); - $services->set(Switcher::class)->public(); + $services->set(TypoScriptFrontendController::class)->public(); $services->set(VariantFilter::class)->public(); + $services->set(SetCookie::class)->tag('event.listener', [ + 'method' => 'handleVariant', + 'event' => SwitchedToVariant::class, + ]); }; diff --git a/README.md b/README.md index 7489cf0..71e7182 100644 --- a/README.md +++ b/README.md @@ -21,3 +21,8 @@ Additional header information may be specified both for the original version as ![Demo](https://raw.githubusercontent.com/werkraum-media/abtest/master/Documentation/Images/demo.gif) +### Known issues + +This extension currently does not support typeNum. + +It always checks requested page for a variant. diff --git a/Tests/Fixtures/BasicDatabase.csv b/Tests/Fixtures/BasicDatabase.csv index eaad2eb..33c29b0 100644 --- a/Tests/Fixtures/BasicDatabase.csv +++ b/Tests/Fixtures/BasicDatabase.csv @@ -1,10 +1,10 @@ -"pages",,,,,,,,,,,,,, -,"uid","pid","slug","title",tx_abtest_variant,hidden,tx_abtest_cookie_time,,,,,,, -,1,0,"/","Page 1 Title (No Variant)",0,0,604800,,,,,,, -,2,1,"/page-2","Page 2 Title (Variant A)",3,0,604800,,,,,,, -,3,1,"/page-3","Page 3 Title (Variant B)",0,0,604800,,,,,,, -,4,1,"/page-4","Page 4 Title (Variant A)",5,0,2419200,,,,,,, -,5,1,"/page-5","Page 5 Title (Variant B)",0,1,604800,,,,,,, -"sys_template",,,,,,,,,,,,,, -,"uid","pid","root","clear","constants","config",,,,,,,, -,1,1,1,3,"databasePlatform = mysql","",,,,,,,, +"pages" +,"uid","pid","slug","title",tx_abtest_variant,hidden,tx_abtest_cookie_time +,1,0,"/","Page 1 Title (No Variant)",0,0,604800 +,2,1,"/page-2","Page 2 Title (Variant A)",3,0,604800 +,3,1,"/page-3","Page 3 Title (Variant B)",0,0,604800 +,4,1,"/page-4","Page 4 Title (Variant A)",5,0,2419200 +,5,1,"/page-5","Page 5 Title (Variant B)",0,1,604800 +"sys_template" +,"uid","pid","root","clear","constants","config" +,1,1,1,3,"databasePlatform = mysql","" diff --git a/Tests/Functional/FrontendRenderingTest.php b/Tests/Functional/FrontendRenderingTest.php index 617d33b..612a2ff 100644 --- a/Tests/Functional/FrontendRenderingTest.php +++ b/Tests/Functional/FrontendRenderingTest.php @@ -54,12 +54,12 @@ class FrontendRenderingTest extends FunctionalTestCase { $request = new InternalRequest(); $request = $request->withPageId(1); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertSame('', $result->getHeaderLine('Set-Cookie')); - self::assertStringContainsString('Page 1 Title (No Variant)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); + self::assertSame(200, $response->getStatusCode()); + self::assertSame('', $response->getHeaderLine('Set-Cookie')); + self::assertStringContainsString('Page 1 Title (No Variant)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); $this->assertCounterOfPage(1, 0); } @@ -70,12 +70,12 @@ class FrontendRenderingTest extends FunctionalTestCase { $request = new InternalRequest(); $request = $request->withPageId(2); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); - $this->assertCookie($result, 'ab-2', '2'); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); + $this->assertCookie($response, 'ab-2', '2'); $this->assertCounterOfPage(2, 1); } @@ -88,12 +88,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(2); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 3 Title (Variant B)', $result->getBody()->__toString()); - $this->assertCookie($result, 'ab-2', '3'); - $this->assertPageIsNotCached($result); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 3 Title (Variant B)', $response->getBody()->__toString()); + $this->assertCookie($response, 'ab-2', '3'); + $this->assertPageIsNotCached($response); $this->assertCounterOfPage(2, 1); $this->assertCounterOfPage(3, 1); } @@ -108,12 +108,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('Cookie', 'ab-2=2'); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsCached($result); - $this->assertCookie($result, 'ab-2', '2'); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsCached($response); + $this->assertCookie($response, 'ab-2', '2'); // 1 from first visit, but not 2 as 2nd visit is via cookie. $this->assertCounterOfPage(2, 1, 'Opening from cookie should not increase counter.'); $this->assertCounterOfPage(3, 0, 'Opening from cookie should not increase counter.'); @@ -127,12 +127,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('User-Agent', 'Storebot-Google'); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); - $this->assertCookieWasNotSet($result); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); + $this->assertCookieWasNotSet($response); $this->assertCounterOfPage(2, 0); } @@ -144,12 +144,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(4); $request = $request->withAddedHeader('Cookie', 'ab-4=5'); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 4 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); - $this->assertCookie($result, 'ab-4', '4'); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 4 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); + $this->assertCookie($response, 'ab-4', '4'); $this->assertCounterOfPage(4, 1); $this->assertCounterOfPage(5, 0); } @@ -162,12 +162,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('Cookie', 'ab-2=5'); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); - $this->assertCookie($result, 'ab-2', '2'); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); + $this->assertCookie($response, 'ab-2', '2'); $this->assertCounterOfPage(2, 1); $this->assertCounterOfPage(5, 0); } @@ -182,12 +182,12 @@ class FrontendRenderingTest extends FunctionalTestCase $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('Cookie', 'ab-2=5'); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - self::assertStringContainsString('Page 3 Title (Variant B)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); - $this->assertCookie($result, 'ab-2', '3'); + self::assertSame(200, $response->getStatusCode()); + self::assertStringContainsString('Page 3 Title (Variant B)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); + $this->assertCookie($response, 'ab-2', '3'); $this->assertCounterOfPage(2, 1); $this->assertCounterOfPage(3, 1); } @@ -199,10 +199,10 @@ class FrontendRenderingTest extends FunctionalTestCase { $request = new InternalRequest(); $request = $request->withPageId(2); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - $cookie = Cookie::fromString($result->getHeaderLine('Set-Cookie')); + self::assertSame(200, $response->getStatusCode()); + $cookie = Cookie::fromString($response->getHeaderLine('Set-Cookie')); self::assertSame(604800, $cookie->getMaxAge()); } @@ -213,10 +213,10 @@ class FrontendRenderingTest extends FunctionalTestCase { $request = new InternalRequest(); $request = $request->withPageId(4); - $result = $this->executeFrontendRequest($request); + $response = $this->executeFrontendRequest($request); - self::assertSame(200, $result->getStatusCode()); - $cookie = Cookie::fromString($result->getHeaderLine('Set-Cookie')); + self::assertSame(200, $response->getStatusCode()); + $cookie = Cookie::fromString($response->getHeaderLine('Set-Cookie')); self::assertSame(2419200, $cookie->getMaxAge()); } @@ -232,29 +232,29 @@ class FrontendRenderingTest extends FunctionalTestCase { $request = new InternalRequest(); $request = $request->withPageId(2); - $result = $this->executeFrontendRequest($request); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); + $response = $this->executeFrontendRequest($request); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('Cookie', 'ab-2=2'); - $result = $this->executeFrontendRequest($request); - self::assertStringContainsString('Page 2 Title (Variant A)', $result->getBody()->__toString()); - $this->assertPageIsCached($result); + $response = $this->executeFrontendRequest($request); + self::assertStringContainsString('Page 2 Title (Variant A)', $response->getBody()->__toString()); + $this->assertPageIsCached($response); $request = new InternalRequest(); $request = $request->withPageId(2); - $result = $this->executeFrontendRequest($request); - self::assertStringContainsString('Page 3 Title (Variant B)', $result->getBody()->__toString()); - $this->assertPageIsNotCached($result); + $response = $this->executeFrontendRequest($request); + self::assertStringContainsString('Page 3 Title (Variant B)', $response->getBody()->__toString()); + $this->assertPageIsNotCached($response); $request = new InternalRequest(); $request = $request->withPageId(2); $request = $request->withAddedHeader('Cookie', 'ab-2=3'); - $result = $this->executeFrontendRequest($request); - self::assertStringContainsString('Page 3 Title (Variant B)', $result->getBody()->__toString()); - $this->assertPageIsCached($result); + $response = $this->executeFrontendRequest($request); + self::assertStringContainsString('Page 3 Title (Variant B)', $response->getBody()->__toString()); + $this->assertPageIsCached($response); } private function assertCounterOfPage( @@ -276,11 +276,11 @@ class FrontendRenderingTest extends FunctionalTestCase } private function assertCookie( - InternalResponse $result, + InternalResponse $response, string $name, string $value ): void { - $cookie = Cookie::fromString($result->getHeaderLine('Set-Cookie')); + $cookie = Cookie::fromString($response->getHeaderLine('Set-Cookie')); self::assertSame($name, $cookie->getName()); self::assertSame($value, $cookie->getValue()); self::assertSame('/', $cookie->getPath()); @@ -288,22 +288,22 @@ class FrontendRenderingTest extends FunctionalTestCase self::assertNull($cookie->getDomain()); } - private function assertCookieWasNotSet(InternalResponse $result): void + private function assertCookieWasNotSet(InternalResponse $response): void { self::assertSame( '', - $result->getHeaderLine('Set-Cookie'), + $response->getHeaderLine('Set-Cookie'), 'Cookie was set but was not expected to be set.' ); } - private function assertPageIsNotCached(InternalResponse $result): void + private function assertPageIsNotCached(InternalResponse $response): void { - self::assertSame('', $result->getHeaderLine('X-TYPO3-Debug-Cache')); + self::assertSame('', $response->getHeaderLine('X-TYPO3-Debug-Cache')); } - private function assertPageIsCached(InternalResponse $result): void + private function assertPageIsCached(InternalResponse $response): void { - self::assertStringStartsWith('Cached page generated', $result->getHeaderLine('X-TYPO3-Debug-Cache')); + self::assertStringStartsWith('Cached page generated', $response->getHeaderLine('X-TYPO3-Debug-Cache')); } } diff --git a/ext_localconf.php b/ext_localconf.php index 9077cde..80fd40c 100644 --- a/ext_localconf.php +++ b/ext_localconf.php @@ -2,4 +2,4 @@ defined('TYPO3_MODE') or die(); -\WerkraumMedia\ABTest\Switcher::register(); +\WerkraumMedia\ABTest\Hook\TypoScriptFrontendController::register();