Merge pull request #5 from whatwedo/fix/prevent-state-call-on-null

Introduces new interfaces for state management in sensors and metrics, and improves error handling and robustness across the monitoring system.
whatwedo · Aug 30, 2024 · c652627 · c652627
2 parents 82d093b + 5bd8905
commit c652627
Show file tree

Hide file tree

Showing 13 changed files with 197 additions and 33 deletions.
diff --git a/src/Command/CheckCommand.php b/src/Command/CheckCommand.php
@@ -43,6 +43,12 @@ private function printResult(SymfonyStyle $io, $result, $previousGroup = null, $
             $rows = [];
             $subs = [];
             foreach ($items as $subGroup => $row) {
+                if (is_array($row)) {
+                    $subs[$subGroup] = $row;
+
+                    continue;
+                }
+
                 if ($row instanceof AbstractSensor) {
                     $rows[] = [
                         sprintf('<fg=%s;options=bold>%s</>', $row->getState()->getCliColor(), $row->getState()->getIcon()),
@@ -55,8 +61,6 @@ private function printResult(SymfonyStyle $io, $result, $previousGroup = null, $
                         $row->getName(),
                         $row->getValue(),
                     ];
-                } elseif (is_array($row)) {
-                    $subs[$subGroup] = $row;
                 }
             }
 

diff --git a/src/Manager/MonitoringManager.php b/src/Manager/MonitoringManager.php
@@ -7,7 +7,8 @@
 use whatwedo\MonitorBundle\Enums\MetricStateEnum;
 use whatwedo\MonitorBundle\Enums\SensorStateEnum;
 use whatwedo\MonitorBundle\Monitoring\AttributeInterface;
-use whatwedo\MonitorBundle\Monitoring\Sensor\AbstractSensor;
+use whatwedo\MonitorBundle\Monitoring\Metric\MetricStateInterface;
+use whatwedo\MonitorBundle\Monitoring\Sensor\SensorStateInterface;
 
 class MonitoringManager
 {
@@ -106,15 +107,27 @@ public function getAttribute(string $className): AttributeInterface
 
     private function wasSuccessful(AttributeInterface $attribute): bool
     {
-        return $attribute instanceof AbstractSensor
-            ? $attribute->getState() === SensorStateEnum::SUCCESSFUL
-            : $attribute->getState() === MetricStateEnum::OK;
+        if ($attribute instanceof SensorStateInterface) {
+            return $attribute->getState() === SensorStateEnum::SUCCESSFUL;
+        }
+
+        if ($attribute instanceof MetricStateInterface) {
+            return $attribute->getState() === MetricStateEnum::OK;
+        }
+
+        return false;
     }
 
-    private function wasWarning(AttributeInterface $abstract): bool
+    private function wasWarning(AttributeInterface $attribute): bool
     {
-        return $abstract instanceof AbstractSensor
-            ? $abstract->getState() === SensorStateEnum::WARNING
-            : $abstract->getState() === MetricStateEnum::WARNING;
+        if ($attribute instanceof SensorStateInterface) {
+            return $attribute->getState() === SensorStateEnum::WARNING;
+        }
+
+        if ($attribute instanceof MetricStateInterface) {
+            return $attribute->getState() === MetricStateEnum::WARNING;
+        }
+
+        return false;
     }
 }
diff --git a/src/Monitoring/Metric/AbstractMetric.php b/src/Monitoring/Metric/AbstractMetric.php
@@ -7,7 +7,7 @@
 use whatwedo\MonitorBundle\Enums\MetricStateEnum;
 use whatwedo\MonitorBundle\Monitoring\AttributeInterface;
 
-abstract class AbstractMetric implements AttributeInterface
+abstract class AbstractMetric implements AttributeInterface, MetricStateInterface
 {
     public null|int|float $value = null;
 
@@ -16,7 +16,7 @@ abstract class AbstractMetric implements AttributeInterface
     public function getState(): MetricStateEnum
     {
         if ($this->state === null) {
-            throw new \RuntimeException(static::class.'::$state is not set.');
+            throw new \LogicException(static::class.'::$state is not set.');
         }
 
         return $this->state;

diff --git a/src/Monitoring/Metric/MetricStateInterface.php b/src/Monitoring/Metric/MetricStateInterface.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace whatwedo\MonitorBundle\Monitoring\Metric;
+
+use whatwedo\MonitorBundle\Enums\MetricStateEnum;
+
+interface MetricStateInterface
+{
+    /**
+     * @throws \LogicException
+     */
+    public function getState(): MetricStateEnum;
+}
diff --git a/src/Monitoring/Sensor/AbstractSensor.php b/src/Monitoring/Sensor/AbstractSensor.php
@@ -7,7 +7,7 @@
 use whatwedo\MonitorBundle\Enums\SensorStateEnum;
 use whatwedo\MonitorBundle\Monitoring\AttributeInterface;
 
-abstract class AbstractSensor implements AttributeInterface
+abstract class AbstractSensor implements AttributeInterface, SensorStateInterface
 {
     public ?SensorStateEnum $state = null;
 
@@ -16,7 +16,7 @@ abstract class AbstractSensor implements AttributeInterface
     public function getState(): SensorStateEnum
     {
         if ($this->state === null) {
-            throw new \RuntimeException(__CLASS__.'::$state is not set.');
+            throw new \LogicException(static::class.'::$state is not set.');
         }
 
         return $this->state;

diff --git a/src/Monitoring/Sensor/SensorStateInterface.php b/src/Monitoring/Sensor/SensorStateInterface.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace whatwedo\MonitorBundle\Monitoring\Sensor;
+
+use whatwedo\MonitorBundle\Enums\SensorStateEnum;
+
+interface SensorStateInterface
+{
+    /**
+     * @throws \LogicException
+     */
+    public function getState(): SensorStateEnum;
+}
diff --git a/src/Normalizer/AttributeNormalizer.php b/src/Normalizer/AttributeNormalizer.php
@@ -4,13 +4,19 @@
 
 namespace whatwedo\MonitorBundle\Normalizer;
 
+use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface;
+use Symfony\Component\Serializer\Normalizer\NormalizerAwareTrait;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 use whatwedo\MonitorBundle\Monitoring\AttributeInterface;
 use whatwedo\MonitorBundle\Monitoring\Metric\AbstractMetric;
+use whatwedo\MonitorBundle\Monitoring\Metric\MetricStateInterface;
 use whatwedo\MonitorBundle\Monitoring\Sensor\AbstractSensor;
+use whatwedo\MonitorBundle\Monitoring\Sensor\SensorStateInterface;
 
-class AttributeNormalizer implements NormalizerInterface
+class AttributeNormalizer implements NormalizerInterface, NormalizerAwareInterface
 {
+    use NormalizerAwareTrait;
+
     /**
      * @param AttributeInterface $object
      */
@@ -20,13 +26,18 @@ public function normalize($object, string $format = null, array $context = []):
             'name' => $object->getName(),
         ];
 
+        try {
+            if ($object instanceof MetricStateInterface || $object instanceof SensorStateInterface) {
+                $data['state'] = $this->normalizer->normalize($object->getState());
+            }
+        } catch (\LogicException) {
+        }
+
         if ($object instanceof AbstractSensor) {
-            $data['state'] = $object->getState();
             $data['details'] = $object->getDetails();
         }
 
         if ($object instanceof AbstractMetric) {
-            $data['state'] = $object->getState();
             $data['value'] = $object->getValue();
         }
 

diff --git a/tests/Command/CheckCommandTest.php b/tests/Command/CheckCommandTest.php
@@ -92,24 +92,24 @@ public function testCriticalCustomExitCode(): void
         self::assertEquals(-1, $commandTester->getStatusCode());
     }
 
-    public function testRuntimeError(): void
+    public function testLogicError(): void
     {
         $kernel = self::bootKernel([
             'config' => static function (TestKernel $kernel) {
-                $kernel->addTestConfig(__DIR__.'/../config/dummy_runtime_error.yml');
+                $kernel->addTestConfig(__DIR__.'/../config/dummy_logic_error.yml');
             },
         ]);
         $application = new Application($kernel);
         $command = $application->find('whatwedo:monitor:check');
 
         $commandTester = new CommandTester($command);
-        $runtimeException = false;
+        $logicException = false;
         try {
             $commandTester->execute([]);
-        } catch (\RuntimeException $e) {
-            $runtimeException = true;
+        } catch (\LogicException $e) {
+            $logicException = true;
         } finally {
-            self::assertTrue($runtimeException);
+            self::assertTrue($logicException);
         }
     }
 }
diff --git a/.../Metric/Dummy/RuntimeErrorDummyMetric.php → ...ng/Metric/Dummy/LogicErrorDummyMetric.php b/.../Metric/Dummy/RuntimeErrorDummyMetric.php → ...ng/Metric/Dummy/LogicErrorDummyMetric.php
@@ -31,11 +31,11 @@
 
 use whatwedo\MonitorBundle\Monitoring\Metric\AbstractMetric;
 
-class RuntimeErrorDummyMetric extends AbstractMetric
+class LogicErrorDummyMetric extends AbstractMetric
 {
     public function getName(): string
     {
-        return 'Runtime Error Test';
+        return 'Logic Error Test';
     }
 
     public function isEnabled(): bool

diff --git a/.../Sensor/Dummy/RuntimeErrorDummySensor.php → ...ng/Sensor/Dummy/LogicErrorDummySensor.php b/.../Sensor/Dummy/RuntimeErrorDummySensor.php → ...ng/Sensor/Dummy/LogicErrorDummySensor.php
@@ -31,11 +31,11 @@
 
 use whatwedo\MonitorBundle\Monitoring\Sensor\AbstractSensor;
 
-class RuntimeErrorDummySensor extends AbstractSensor
+class LogicErrorDummySensor extends AbstractSensor
 {
     public function getName(): string
     {
-        return 'Runtime Error Test';
+        return 'Logic Error Test';
     }
 
     public function isEnabled(): bool

diff --git a/tests/Normalizer/AttributeNormalizerTest.php b/tests/Normalizer/AttributeNormalizerTest.php
@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+
+namespace whatwedo\MonitorBundle\Tests\Normalizer;
+
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
+use whatwedo\MonitorBundle\Monitoring\AttributeInterface;
+use whatwedo\MonitorBundle\Tests\Monitoring\Metric\Dummy\CriticalDummyMetric;
+use whatwedo\MonitorBundle\Tests\Monitoring\Metric\Dummy\LogicErrorDummyMetric;
+use whatwedo\MonitorBundle\Tests\Monitoring\Metric\Dummy\OkDummyMetric;
+use whatwedo\MonitorBundle\Tests\Monitoring\Metric\Dummy\WarningDummyMetric;
+use whatwedo\MonitorBundle\Tests\Monitoring\Sensor\Dummy\CriticalDummySensor;
+use whatwedo\MonitorBundle\Tests\Monitoring\Sensor\Dummy\LogicErrorDummySensor;
+use whatwedo\MonitorBundle\Tests\Monitoring\Sensor\Dummy\SuccessfulDummySensor;
+use whatwedo\MonitorBundle\Tests\Monitoring\Sensor\Dummy\WarningDummySensor;
+use whatwedo\MonitorBundle\Tests\UseTestKernelTrait;
+
+final class AttributeNormalizerTest extends KernelTestCase
+{
+    use UseTestKernelTrait;
+
+    public static function provideDummyMetricsAndSensors(): array
+    {
+        return [
+            CriticalDummyMetric::class => [
+                'attribute' => new CriticalDummyMetric(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Critical Test',
+                    'state' => 'critical',
+                    'value' => 24,
+                ],
+            ],
+            OkDummyMetric::class => [
+                'attribute' => new OkDummyMetric(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Ok Test',
+                    'state' => 'ok',
+                    'value' => 80,
+                ],
+            ],
+            WarningDummyMetric::class => [
+                'attribute' => new WarningDummyMetric(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Warning Test',
+                    'state' => 'warning',
+                    'value' => 125,
+                ],
+            ],
+
+            CriticalDummySensor::class => [
+                'attribute' => new CriticalDummySensor(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Critical Test',
+                    'state' => 'critical',
+                    'details' => [],
+                ],
+            ],
+            SuccessfulDummySensor::class => [
+                'attribute' => new SuccessfulDummySensor(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Successful Test',
+                    'state' => 'successful',
+                    'details' => [],
+                ],
+            ],
+            WarningDummySensor::class => [
+                'attribute' => new WarningDummySensor(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Warning Test',
+                    'state' => 'warning',
+                    'details' => [],
+                ],
+            ],
+
+            LogicErrorDummyMetric::class => [
+                'attribute' => new LogicErrorDummyMetric(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Logic Error Test',
+                    'value' => null,
+                ],
+            ],
+            LogicErrorDummySensor::class => [
+                'attribute' => new LogicErrorDummySensor(),
+                'expectedNormalizedBody' => [
+                    'name' => 'Logic Error Test',
+                    'details' => [],
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideDummyMetricsAndSensors
+     */
+    public function testNormalizeWorksAsExpected(AttributeInterface $attribute, array $expectedNormalizedBody): void
+    {
+        $attribute->run();
+
+        self::assertSame(
+            $expectedNormalizedBody,
+            $this->getContainer()->get(NormalizerInterface::class)->normalize($attribute)
+        );
+    }
+}
diff --git a/tests/config/dummy_logic_error.yml b/tests/config/dummy_logic_error.yml
@@ -0,0 +1,6 @@
+services:
+  whatwedo\MonitorBundle\Tests\Monitoring\Sensor\Dummy\LogicErrorDummySensor:
+    tags: [ 'whatwedo_monitor.attribute' ]
+
+  whatwedo\MonitorBundle\Tests\Monitoring\Metric\Dummy\LogicErrorDummyMetric:
+    tags: [ 'whatwedo_monitor.attribute' ]
diff --git a/tests/config/dummy_runtime_error.yml b/tests/config/dummy_runtime_error.yml