🐛 fix infection

Kanti · Aug 14, 2023 · 6143e55 · 6143e55
1 parent 9a24141
commit 6143e55
Show file tree

Hide file tree

Showing 8 changed files with 97 additions and 35 deletions.
diff --git a/Classes/Service/ConfigService.php b/Classes/Service/ConfigService.php
@@ -4,17 +4,22 @@
 
 namespace Kanti\ServerTiming\Service;
 
+use Kanti\ServerTiming\Utility\TimingUtility;
+
 final class ConfigService
 {
+    /** @var int */
+    private const DEFAULT_STOP_WATCH_LIMIT = 100_000;
+
     public function stopWatchLimit(): int
     {
-        return (int)($this->getConfig('stop_watch_limit') ?? 100_000);
+        return (int)($this->getConfig('stop_watch_limit') ?: self::DEFAULT_STOP_WATCH_LIMIT);
     }
 
     public function tracesSampleRate(): ?float
     {
-        $tracesSampleRate = $this->getConfig('sentry_sample_rate');
-        return $tracesSampleRate === null ? null : (float)$tracesSampleRate;
+        $tracesSampleRate = $this->getConfig(TimingUtility::IS_CLI ? 'sentry_cli_sample_rate' : 'sentry_sample_rate');
+        return $tracesSampleRate === '' ? null : (float)$tracesSampleRate;
     }
 
     public function enableTracing(): ?bool
@@ -23,8 +28,8 @@ public function enableTracing(): ?bool
         return $tracesSampleRate === null ? null : (bool)$tracesSampleRate;
     }
 
-    private function getConfig(string $path): ?string
+    private function getConfig(string $path): string
     {
-        return $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['server_timing'][$path] ?? null;
+        return (string)($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['server_timing'][$path] ?? '');
     }
 }
diff --git a/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunction.php b/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunction.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Kanti\ServerTiming\Service\RegisterShutdownFunction;
+
+final class RegisterShutdownFunction implements RegisterShutdownFunctionInterface
+{
+    public function register(callable $callback): void
+    {
+        register_shutdown_function($callback);
+    }
+}
diff --git a/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunctionInterface.php b/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunctionInterface.php
@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Kanti\ServerTiming\Service\RegisterShutdownFunction;
+
+interface RegisterShutdownFunctionInterface
+{
+    public function register(callable $callback): void;
+}
diff --git a/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunctionNoop.php b/Classes/Service/RegisterShutdownFunction/RegisterShutdownFunctionNoop.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Kanti\ServerTiming\Service\RegisterShutdownFunction;
+
+final class RegisterShutdownFunctionNoop implements RegisterShutdownFunctionInterface
+{
+    public int $callCount = 0;
+
+    public function register(callable $callback): void
+    {
+        $this->callCount++;
+    }
+}
diff --git a/Classes/Utility/TimingUtility.php b/Classes/Utility/TimingUtility.php
@@ -4,43 +4,35 @@
 
 namespace Kanti\ServerTiming\Utility;
 
-use Closure;
 use Exception;
 use Kanti\ServerTiming\Dto\ScriptResult;
 use Kanti\ServerTiming\Dto\StopWatch;
+use Kanti\ServerTiming\Service\RegisterShutdownFunction\RegisterShutdownFunctionInterface;
 use Kanti\ServerTiming\Service\SentryService;
 use Kanti\ServerTiming\Service\ConfigService;
 use Psr\Http\Message\ResponseInterface;
 use SplObjectStorage;
 use TYPO3\CMS\Core\Context\Context;
 use TYPO3\CMS\Core\Core\Environment;
+use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
-final class TimingUtility
+final class TimingUtility implements SingletonInterface
 {
-    private static ?TimingUtility $instance = null;
-
     private bool $registered = false;
 
     /** @var bool */
     public const IS_CLI = PHP_SAPI === 'cli';
 
     private bool $alreadyShutdown = false;
 
-    private readonly Closure $registerShutdownFunction;
-
-    private readonly ConfigService $configService;
-
-    public function __construct(Closure $registerShutdownFunction = null, ConfigService $configService = null)
+    public function __construct(private readonly RegisterShutdownFunctionInterface $registerShutdownFunction, private readonly ConfigService $configService)
     {
-        $this->registerShutdownFunction = $registerShutdownFunction ?? register_shutdown_function(...);
-        $this->configService = $configService ?? new ConfigService();
     }
 
-    public static function getInstance(Closure $registerShutdownFunction = null): TimingUtility
+    public static function getInstance(): TimingUtility
     {
-        // to not use GeneralUtility::makeInstance( as this is maybe called to early in the stack)
-        return self::$instance ??= new self($registerShutdownFunction);
+        return GeneralUtility::makeInstance(TimingUtility::class);
     }
 
     /** @var StopWatch[] */
@@ -115,8 +107,7 @@ public function stopWatchInternal(string $key, string $info = ''): StopWatch
             }
 
             if (!$this->registered) {
-                $x = $this->registerShutdownFunction;
-                $x(fn(): ?ResponseInterface => $this->shutdown(ScriptResult::fromShutdown()));
+                $this->registerShutdownFunction->register(fn(): ?ResponseInterface => $this->shutdown(ScriptResult::fromShutdown()));
                 $this->registered = true;
             }
         }

diff --git a/Configuration/Services.yaml b/Configuration/Services.yaml
@@ -9,6 +9,9 @@ services:
     # SqlLogging can be removed from exclude if only TYPO3 12 and above are required
     exclude: '../Classes/{Dto,SqlLogging}/*'
 
+  Kanti\ServerTiming\Service\RegisterShutdownFunction\RegisterShutdownFunctionInterface:
+    class: Kanti\ServerTiming\Service\RegisterShutdownFunction\RegisterShutdownFunction
+
   Kanti\ServerTiming\EventListener\ConsoleCommandEventListener:
     tags:
       -

diff --git a/Tests/TimingUtilityTest.php b/Tests/TimingUtilityTest.php
@@ -6,23 +6,31 @@
 
 use Generator;
 use Kanti\ServerTiming\Service\ConfigService;
+use Kanti\ServerTiming\Service\RegisterShutdownFunction\RegisterShutdownFunctionNoop;
 use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use Kanti\ServerTiming\Dto\StopWatch;
 use Kanti\ServerTiming\Utility\TimingUtility;
 use PHPUnit\Framework\TestCase;
 use ReflectionClass;
+use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 #[CoversClass(TimingUtility::class)]
 #[CoversClass(StopWatch::class)]
 #[CoversClass(ConfigService::class)]
+#[CoversClass(RegisterShutdownFunctionNoop::class)]
 final class TimingUtilityTest extends TestCase
 {
     protected function setUp(): void
     {
-        // do not register a real shutdown function:
-        TimingUtility::getInstance(static fn(): int => 0);
+        GeneralUtility::setSingletonInstance(TimingUtility::class, $this->getTestInstance());
+    }
+
+    protected function tearDown(): void
+    {
+        unset($GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['server_timing']);
+        GeneralUtility::resetSingletonInstances([]);
     }
 
     #[Test]
@@ -75,14 +83,14 @@ public function stopWatchStopIfNot(): void
     #[Test]
     public function stopWatchInternal(): void
     {
-        (new TimingUtility(static fn(): int => 0))->stopWatchInternal('test');
+        TimingUtility::getInstance()->stopWatchInternal('test');
         self::assertTrue(true, 'isCallable');
     }
 
     #[Test]
     public function stopWatchFirstIsAlwaysPhp(): void
     {
-        $timingUtility = new TimingUtility(static fn(): int => 0);
+        $timingUtility = $this->getTestInstance();
         $timingUtility->stopWatchInternal('test');
 
         $watches = $timingUtility->getStopWatches();
@@ -91,19 +99,31 @@ public function stopWatchFirstIsAlwaysPhp(): void
         self::assertSame('test', $watches[1]->key);
     }
 
+    #[Test]
+    public function stopWatchLimit(): void
+    {
+        $GLOBALS['TYPO3_CONF_VARS']['EXTENSIONS']['server_timing']['stop_watch_limit'] = 3;
+        $timingUtility = $this->getTestInstance();
+        $timingUtility->stopWatchInternal('test');
+        $timingUtility->stopWatchInternal('test');
+        $timingUtility->stopWatchInternal('test');
+
+        $watches = $timingUtility->getStopWatches();
+        self::assertCount(3, $watches);
+        self::assertSame('php', $watches[0]->key);
+        self::assertSame('test', $watches[1]->key);
+        self::assertSame('test', $watches[2]->key);
+    }
+
     #[Test]
     public function didRegisterShutdownFunctionOnce(): void
     {
-        $called = 0;
-        $timingUtility = new TimingUtility(static function ($callback) use (&$called): void {
-            self::assertIsCallable($callback);
-            $called++;
-        });
+        $timingUtility = new TimingUtility($registerShutdownFunction = new RegisterShutdownFunctionNoop(), new ConfigService());
         $timingUtility->stopWatchInternal('test');
         $timingUtility->stopWatchInternal('test');
         $timingUtility->stopWatchInternal('test');
         $timingUtility->stopWatchInternal('test');
-        self::assertSame(1, $called);
+        self::assertSame(1, $registerShutdownFunction->callCount);
     }
 
     #[Test]
@@ -127,7 +147,7 @@ public function timingString(string $expected, array $args): void
         $reflection = new ReflectionClass(TimingUtility::class);
         $reflectionMethod = $reflection->getMethod('timingString');
 
-        $result = $reflectionMethod->invoke(new TimingUtility(static fn(): int => 0), ...$args);
+        $result = $reflectionMethod->invoke($this->getTestInstance(), ...$args);
         self::assertSame($expected, $result);
     }
 
@@ -163,7 +183,7 @@ public function combineIfToMuch(array $expected, array $initalStopWatches): void
         $initalStopWatches = array_map(static fn(StopWatch $el): StopWatch => clone $el, $initalStopWatches);
         $reflectionMethod = $reflection->getMethod('combineIfToMuch');
 
-        $result = $reflectionMethod->invoke(new TimingUtility(static fn(): int => 0), $initalStopWatches);
+        $result = $reflectionMethod->invoke($this->getTestInstance(), $initalStopWatches);
         self::assertEqualsWithDelta($expected, $result, 0.00001);
     }
 
@@ -239,12 +259,17 @@ public function shouldTrack(): void
         $reflection = new ReflectionClass(TimingUtility::class);
         $isAlreadyShutdown = $reflection->getProperty('alreadyShutdown');
 
-        $timingUtility = new TimingUtility(static fn(): int => 0);
+        $timingUtility = $this->getTestInstance();
 
         $isAlreadyShutdown->setValue($timingUtility, false);
         self::assertTrue($timingUtility->shouldTrack());
 
         $isAlreadyShutdown->setValue($timingUtility, true);
         self::assertFalse($timingUtility->shouldTrack());
     }
+
+    private function getTestInstance(): TimingUtility
+    {
+        return new TimingUtility(new RegisterShutdownFunctionNoop(), new ConfigService());
+    }
 }
diff --git a/composer.json b/composer.json
@@ -20,7 +20,7 @@
     "infection/infection": "^0.26.13",
     "phpstan/extension-installer": "^1.1",
     "phpunit/phpunit": "^10",
-    "pluswerk/grumphp-config": "^6.7",
+    "pluswerk/grumphp-config": "^6.8.0",
     "saschaegerer/phpstan-typo3": "^1.1",
     "sentry/sdk": "^3.5",
     "spatie/phpunit-watcher": "^1.23",