From f0bb1d03a85d032a1a3353a6a0d8c27dfd03b164 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Fri, 16 Sep 2022 14:59:54 +0200 Subject: [PATCH] Fix broken recordview introduced with last commit Relates: #86 --- Classes/Domain/ExpressionLanguage/Factory.php | 5 +-- .../SymfonyExpressionLanguage.php | 8 +--- Classes/Domain/Pageview/Factory.php | 12 +++--- Classes/Domain/Recordview/Factory.php | 19 +++++++-- Classes/Middleware/Pageview.php | 15 +++++-- Classes/Middleware/Recordview.php | 15 +++++-- .../Domain/Recordview/FactoryTest.php | 40 ++++++++++++++----- .../recordview/Configuration/Services.yaml | 4 +- Tests/Unit/Domain/Pageview/FactoryTest.php | 28 +++++++++---- 9 files changed, 101 insertions(+), 45 deletions(-) rename Tests/{Unit => Functional}/Domain/Recordview/FactoryTest.php (88%) diff --git a/Classes/Domain/ExpressionLanguage/Factory.php b/Classes/Domain/ExpressionLanguage/Factory.php index a30363f..e8fb483 100644 --- a/Classes/Domain/ExpressionLanguage/Factory.php +++ b/Classes/Domain/ExpressionLanguage/Factory.php @@ -24,14 +24,11 @@ declare(strict_types=1); namespace DanielSiepmann\Tracking\Domain\ExpressionLanguage; use DanielSiepmann\Tracking\Domain\Model\Expression; -use Psr\Http\Message\ServerRequestInterface; -use TYPO3\CMS\Core\Context\Context; interface Factory { public function create( string $expression, - ServerRequestInterface $request, - Context $context + array $variables ): Expression; } diff --git a/Classes/Domain/ExpressionLanguage/SymfonyExpressionLanguage.php b/Classes/Domain/ExpressionLanguage/SymfonyExpressionLanguage.php index 1487cbe..7a45b2e 100644 --- a/Classes/Domain/ExpressionLanguage/SymfonyExpressionLanguage.php +++ b/Classes/Domain/ExpressionLanguage/SymfonyExpressionLanguage.php @@ -36,18 +36,14 @@ class SymfonyExpressionLanguage implements Factory { public function create( string $expression, - ServerRequestInterface $request, - Context $context + array $variables ): Expression { $language = new ExpressionLanguage(); $this->registerTraverseFunction($language); return new SymfonyExpression( $expression, - [ - 'context' => $context, - 'request' => $request, - ], + $variables, $language ); } diff --git a/Classes/Domain/Pageview/Factory.php b/Classes/Domain/Pageview/Factory.php index 3b5ccb1..74f4a40 100644 --- a/Classes/Domain/Pageview/Factory.php +++ b/Classes/Domain/Pageview/Factory.php @@ -41,13 +41,13 @@ class Factory $this->siteFinder = $siteFinder; } - public static function fromRequest(ServerRequestInterface $request): Pageview + public function fromRequest(ServerRequestInterface $request): Pageview { return new Pageview( - self::getRouting($request)->getPageId(), - self::getLanguage($request), + $this->getRouting($request)->getPageId(), + $this->getLanguage($request), new \DateTimeImmutable(), - (int) self::getRouting($request)->getPageType(), + (int) $this->getRouting($request)->getPageType(), (string) $request->getUri(), $request->getHeader('User-Agent')[0] ?? '' ); @@ -66,7 +66,7 @@ class Factory ); } - private static function getLanguage(ServerRequestInterface $request): SiteLanguage + private function getLanguage(ServerRequestInterface $request): SiteLanguage { $language = $request->getAttribute('language'); @@ -77,7 +77,7 @@ class Factory return $language; } - private static function getRouting(ServerRequestInterface $request): PageArguments + private function getRouting(ServerRequestInterface $request): PageArguments { $routing = $request->getAttribute('routing'); diff --git a/Classes/Domain/Recordview/Factory.php b/Classes/Domain/Recordview/Factory.php index 06ebbea..f8d9cfe 100644 --- a/Classes/Domain/Recordview/Factory.php +++ b/Classes/Domain/Recordview/Factory.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace DanielSiepmann\Tracking\Domain\Recordview; +use DanielSiepmann\Tracking\Domain\ExpressionLanguage\Factory as ExpressionFactory; use DanielSiepmann\Tracking\Domain\Model\RecordRule; use DanielSiepmann\Tracking\Domain\Model\Recordview; use Psr\Http\Message\ServerRequestInterface; @@ -32,15 +33,25 @@ use TYPO3\CMS\Core\Site\Entity\SiteLanguage; class Factory { - public static function fromRequest( + /** + * @var ExpressionFactory + */ + private $expressionFactory; + + public function __construct( + ExpressionFactory $expressionFactory + ) { + $this->expressionFactory = $expressionFactory; + } + + public function fromRequest( ServerRequestInterface $request, RecordRule $rule ): Recordview { - // Need silent, as expression language doens't provide a way to check for array keys - $recordUid = @(new ExpressionLanguage())->evaluate( + $recordUid = $this->expressionFactory->create( $rule->getUidExpression(), ['request' => $request] - ); + )->evaluate(); if (is_numeric($recordUid) === false) { throw new \UnexpectedValueException( diff --git a/Classes/Middleware/Pageview.php b/Classes/Middleware/Pageview.php index 63888fc..f32cce9 100644 --- a/Classes/Middleware/Pageview.php +++ b/Classes/Middleware/Pageview.php @@ -44,6 +44,11 @@ class Pageview implements MiddlewareInterface */ private $context; + /** + * @var Factory + */ + private $factory; + /** * @var ExpressionFactory */ @@ -57,11 +62,13 @@ class Pageview implements MiddlewareInterface public function __construct( Repository $repository, Context $context, + Factory $factory, ExpressionFactory $expressionFactory, string $rule ) { $this->repository = $repository; $this->context = $context; + $this->factory = $factory; $this->expressionFactory = $expressionFactory; $this->rule = $rule; } @@ -71,7 +78,7 @@ class Pageview implements MiddlewareInterface RequestHandlerInterface $handler ): ResponseInterface { if ($this->shouldTrack($request, $this->context)) { - $this->repository->add(Factory::fromRequest($request)); + $this->repository->add($this->factory->fromRequest($request)); } return $handler->handle($request); @@ -83,8 +90,10 @@ class Pageview implements MiddlewareInterface ): bool { return (bool) $this->expressionFactory->create( $this->rule, - $request, - $context + [ + 'request' => $request, + 'context' => $context, + ] )->evaluate(); } } diff --git a/Classes/Middleware/Recordview.php b/Classes/Middleware/Recordview.php index 4449715..651b470 100644 --- a/Classes/Middleware/Recordview.php +++ b/Classes/Middleware/Recordview.php @@ -46,6 +46,11 @@ class Recordview implements MiddlewareInterface */ private $context; + /** + * @var Factory + */ + private $factory; + /** * @var ExpressionFactory */ @@ -59,11 +64,13 @@ class Recordview implements MiddlewareInterface public function __construct( Repository $repository, Context $context, + Factory $factory, ExpressionFactory $expressionFactory, array $rules ) { $this->repository = $repository; $this->context = $context; + $this->factory = $factory; $this->expressionFactory = $expressionFactory; $this->rules = RecordRule::multipleFromArray($rules); @@ -75,7 +82,7 @@ class Recordview implements MiddlewareInterface ): ResponseInterface { foreach ($this->rules as $rule) { if ($this->shouldTrack($request, $this->context, $rule)) { - $this->repository->add(Factory::fromRequest($request, $rule)); + $this->repository->add($this->factory->fromRequest($request, $rule)); } } @@ -89,8 +96,10 @@ class Recordview implements MiddlewareInterface ): bool { return (bool) $this->expressionFactory->create( $rule->getMatchesExpression(), - $request, - $context + [ + 'request' => $request, + 'context' => $context, + ] )->evaluate(); } } diff --git a/Tests/Unit/Domain/Recordview/FactoryTest.php b/Tests/Functional/Domain/Recordview/FactoryTest.php similarity index 88% rename from Tests/Unit/Domain/Recordview/FactoryTest.php rename to Tests/Functional/Domain/Recordview/FactoryTest.php index 50efccb..a1977e2 100644 --- a/Tests/Unit/Domain/Recordview/FactoryTest.php +++ b/Tests/Functional/Domain/Recordview/FactoryTest.php @@ -28,15 +28,19 @@ use Prophecy\PhpUnit\ProphecyTrait; use Psr\Http\Message\ServerRequestInterface; use TYPO3\CMS\Core\Routing\PageArguments; use TYPO3\CMS\Core\Site\Entity\SiteLanguage; -use TYPO3\TestingFramework\Core\Unit\UnitTestCase as TestCase; +use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase; /** * @covers DanielSiepmann\Tracking\Domain\Recordview\Factory */ -class FactoryTest extends TestCase +class FactoryTest extends FunctionalTestCase { use ProphecyTrait; + protected $testExtensionsToLoad = [ + 'typo3conf/ext/tracking', + ]; + /** * @test */ @@ -60,7 +64,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertInstanceOf(Recordview::class, $result); } @@ -87,7 +93,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame('Some User Agent', $result->getUserAgent()); } @@ -114,7 +122,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame('https://example.com', $result->getUrl()); } @@ -141,7 +151,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertInstanceOf(\DateTimeImmutable::class, $result->getCrdate()); } @@ -168,7 +180,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame($language->reveal(), $result->getLanguage()); } @@ -195,7 +209,9 @@ class FactoryTest extends TestCase 'category' => 10, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame(10, $result->getPageUid()); } @@ -222,7 +238,9 @@ class FactoryTest extends TestCase 'category' => 20, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame(20, $result->getRecordUid()); } @@ -249,7 +267,9 @@ class FactoryTest extends TestCase 'category' => 20, ]); - $result = Factory::fromRequest($request->reveal(), $rule->reveal()); + $subject = $this->get(Factory::class); + + $result = $subject->fromRequest($request->reveal(), $rule->reveal()); static::assertSame('sys_category', $result->getTableName()); } } diff --git a/Tests/Functional/Fixtures/Extensions/recordview/Configuration/Services.yaml b/Tests/Functional/Fixtures/Extensions/recordview/Configuration/Services.yaml index 18ca140..7f75e15 100644 --- a/Tests/Functional/Fixtures/Extensions/recordview/Configuration/Services.yaml +++ b/Tests/Functional/Fixtures/Extensions/recordview/Configuration/Services.yaml @@ -9,6 +9,6 @@ services: arguments: $rules: topics: - matches: 'request.getQueryParams()["topic_id"] > 0' - recordUid: 'request.getQueryParams()["topic_id"]' + matches: 'traverse(request.getQueryParams(), "topic_id") > 0' + recordUid: 'traverse(request.getQueryParams(), "topic_id")' tableName: 'sys_category' diff --git a/Tests/Unit/Domain/Pageview/FactoryTest.php b/Tests/Unit/Domain/Pageview/FactoryTest.php index e84ea5e..51b7966 100644 --- a/Tests/Unit/Domain/Pageview/FactoryTest.php +++ b/Tests/Unit/Domain/Pageview/FactoryTest.php @@ -56,7 +56,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn(''); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertInstanceOf(Pageview::class, $result); } @@ -79,7 +81,9 @@ class FactoryTest extends TestCase 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0' ]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertSame( 'Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0', $result->getUserAgent() @@ -103,7 +107,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn('https://example.com/path?query=params&some=more#anchor'); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertSame( 'https://example.com/path?query=params&some=more#anchor', $result->getUrl() @@ -127,7 +133,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn(''); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertSame( 50, $result->getPageType() @@ -151,7 +159,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn(''); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertInstanceOf(\DateTimeImmutable::class, $result->getCrdate()); } @@ -172,7 +182,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn(''); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertInstanceOf(SiteLanguage::class, $result->getLanguage()); } @@ -193,7 +205,9 @@ class FactoryTest extends TestCase $request->getUri()->willReturn(''); $request->getHeader('User-Agent')->willReturn([]); - $result = Factory::fromRequest($request->reveal()); + $subject = new Factory($this->prophesize(SiteFinder::class)->reveal()); + + $result = $subject->fromRequest($request->reveal()); static::assertSame( 10, $result->getPageUid()