From a0670efa07fa15ade828cdfa500da2572a09f4eb Mon Sep 17 00:00:00 2001 From: Tim Weisenberger Date: Mon, 6 Feb 2023 18:18:56 +0100 Subject: [PATCH 1/4] FEATURE: Add advanced logging rules --- Classes/Integration/NetlogixIntegration.php | 28 ++++- Classes/LoggingRule/AllowListRule.php | 27 +++++ Classes/LoggingRule/DenyListRule.php | 27 +++++ .../ExceptionHandlerRenderingGroupsRule.php | 24 +++++ Classes/LoggingRule/LoggingRule.php | 12 +++ Configuration/Settings.LoggingRules.yaml | 13 +++ .../Integration/NetlogixIntegrationTest.php | 102 +++++++++++++----- Tests/Unit/LoggingRule/AllowListRuleTest.php | 38 +++++++ Tests/Unit/LoggingRule/DenyListRuleTest.php | 38 +++++++ ...xceptionHandlerRenderingGroupsRuleTest.php | 85 +++++++++++++++ 10 files changed, 362 insertions(+), 32 deletions(-) create mode 100644 Classes/LoggingRule/AllowListRule.php create mode 100644 Classes/LoggingRule/DenyListRule.php create mode 100644 Classes/LoggingRule/ExceptionHandlerRenderingGroupsRule.php create mode 100644 Classes/LoggingRule/LoggingRule.php create mode 100644 Configuration/Settings.LoggingRules.yaml create mode 100644 Tests/Unit/LoggingRule/AllowListRuleTest.php create mode 100644 Tests/Unit/LoggingRule/DenyListRuleTest.php create mode 100644 Tests/Unit/LoggingRule/ExceptionHandlerRenderingGroupsRuleTest.php diff --git a/Classes/Integration/NetlogixIntegration.php b/Classes/Integration/NetlogixIntegration.php index 1460752..3bd43bc 100644 --- a/Classes/Integration/NetlogixIntegration.php +++ b/Classes/Integration/NetlogixIntegration.php @@ -4,10 +4,11 @@ namespace Netlogix\Sentry\Integration; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Configuration\ConfigurationManager; use Neos\Flow\Core\Bootstrap; use Neos\Flow\ObjectManagement\CompileTimeObjectManager; use Neos\Utility\Files; -use Netlogix\Sentry\ExceptionHandler\ExceptionRenderingOptionsResolver; +use Neos\Utility\PositionalArraySorter; use Netlogix\Sentry\Scope\ScopeProvider; use Sentry\Event; use Sentry\EventHint; @@ -54,10 +55,27 @@ public static function handleEvent(Event $event, EventHint $hint): ?Event return $event; } - if ($hint->exception instanceof Throwable - && ($optionsResolver = Bootstrap::$staticObjectManager->get(ExceptionRenderingOptionsResolver::class)) !== null) { - $options = $optionsResolver->resolveRenderingOptionsForThrowable($hint->exception); - if (!($options['logException'] ?? true)) { + if ( + $hint->exception instanceof Throwable && + ($configurationManager = Bootstrap::$staticObjectManager->get(ConfigurationManager::class)) !== null + ) { + $rules = $configurationManager->getConfiguration( + ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules' + ) ?? []; + + $decision = true; + + $positionalArraySorter = new PositionalArraySorter($rules); + $sortedRules = $positionalArraySorter->toArray(); + + foreach (array_keys($sortedRules) as $rule) { + $decision = Bootstrap::$staticObjectManager + ->get($rule) + ->decide($hint->exception, $decision); + } + + if (!$decision) { return null; } } diff --git a/Classes/LoggingRule/AllowListRule.php b/Classes/LoggingRule/AllowListRule.php new file mode 100644 index 0000000..a6eb9ab --- /dev/null +++ b/Classes/LoggingRule/AllowListRule.php @@ -0,0 +1,27 @@ +allowList as $allowedThrowableClassName) { + if ($throwable instanceof $allowedThrowableClassName) { + return true; + } + } + + return $previousDecision; + } +} diff --git a/Classes/LoggingRule/DenyListRule.php b/Classes/LoggingRule/DenyListRule.php new file mode 100644 index 0000000..b1f8b59 --- /dev/null +++ b/Classes/LoggingRule/DenyListRule.php @@ -0,0 +1,27 @@ +denyList as $deniedThrowableClassName) { + if ($throwable instanceof $deniedThrowableClassName) { + return false; + } + } + + return $previousDecision; + } +} diff --git a/Classes/LoggingRule/ExceptionHandlerRenderingGroupsRule.php b/Classes/LoggingRule/ExceptionHandlerRenderingGroupsRule.php new file mode 100644 index 0000000..a13a515 --- /dev/null +++ b/Classes/LoggingRule/ExceptionHandlerRenderingGroupsRule.php @@ -0,0 +1,24 @@ +optionsResolver->resolveRenderingOptionsForThrowable($throwable); + return $options['logException'] ?? $previousDecision; + } +} diff --git a/Classes/LoggingRule/LoggingRule.php b/Classes/LoggingRule/LoggingRule.php new file mode 100644 index 0000000..da63c4e --- /dev/null +++ b/Classes/LoggingRule/LoggingRule.php @@ -0,0 +1,12 @@ +getMockBuilder(ObjectManagerInterface::class) ->getMock(); - $optionsResolver = new ExceptionRenderingOptionsResolver(); - $optionsResolver->setOptions([ - 'renderingGroups' => [ - 'netlogixSentryTest' => [ - 'matchingExceptionClassNames' => [Test::class], - 'options' => $options - ] - ] - ]); + $configurationManager = $this->getMockBuilder(ConfigurationManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationManager + ->method('getConfiguration') + ->with( ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules') + ->willReturn([ + 'Netlogix\Sentry\LoggingRule\ExceptionHandlerRenderingGroupsRule' => '10' + ]); + + $exceptionHandlerRenderingGroupsRule = $this->getMockBuilder(ExceptionHandlerRenderingGroupsRule::class) + ->disableOriginalConstructor() + ->getMock(); + + $exceptionHandlerRenderingGroupsRule + ->method('decide') + ->willReturn($ruleResult); $objectManager ->method('get') - ->with(ExceptionRenderingOptionsResolver::class) - ->willReturn($optionsResolver); + ->with($this->logicalOr( + $this->equalTo( ConfigurationManager::class), + $this->equalTo(ExceptionHandlerRenderingGroupsRule::class) + )) + ->will($this->returnCallback(function($class) use ($configurationManager, $exceptionHandlerRenderingGroupsRule) { + if ($class === ConfigurationManager::class) { + return $configurationManager; + } else if ($class === ExceptionHandlerRenderingGroupsRule::class) { + return $exceptionHandlerRenderingGroupsRule; + } + + return null; + })); + Bootstrap::$staticObjectManager = $objectManager; @@ -88,24 +112,48 @@ public function Event_is_logged_depending_on_logException(array $options, bool $ } } - public function provideLogExceptionExpectations(): iterable + /** + * @test + */ + public function Event_is_logged_when_no_rules_are_defined(): void { - yield 'If logException is false, null is returned' => [ - 'options' => [ - 'logException' => false, - ], - 'isLogged' => false, - ]; + $objectManager = $this->getMockBuilder(ObjectManagerInterface::class) + ->getMock(); - yield 'If logException is true, the exception is logged' => [ - 'options' => [ - 'logException' => true, - ], - 'isLogged' => true, + $configurationManager = $this->getMockBuilder(ConfigurationManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationManager + ->method('getConfiguration') + ->with( ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules') + ->willReturn([]); + + $objectManager + ->method('get') + ->with(ConfigurationManager::class) + ->willReturn($configurationManager); + + Bootstrap::$staticObjectManager = $objectManager; + + $throwable = new Test('foo', 1612089648); + + $event = Event::createEvent(); + $hint = EventHint::fromArray(['exception' => $throwable]); + + self::assertSame($event, NetlogixIntegration::handleEvent($event, $hint)); + } + + public function provideExceptionLoggingExpectations(): iterable + { + yield 'If ruleResult is false, null is returned' => [ + 'ruleResult' => false, + 'isLogged' => false, ]; - yield 'If logException is unset, the exception is logged' => [ - 'options' => [], + yield 'If ruleResult is true, the exception is logged' => [ + 'ruleResult' => true, 'isLogged' => true, ]; } diff --git a/Tests/Unit/LoggingRule/AllowListRuleTest.php b/Tests/Unit/LoggingRule/AllowListRuleTest.php new file mode 100644 index 0000000..88ef1c0 --- /dev/null +++ b/Tests/Unit/LoggingRule/AllowListRuleTest.php @@ -0,0 +1,38 @@ +getAccessibleMock(AllowListRule::class, ['dummy']); + $allowListRule->_set('allowList', [ + Test::class + ]); + + self::assertEquals(true, $allowListRule->decide(new Test(), false)); + } + + /** + * @test + */ + public function if_allow_list_not_contains_class_decision_should_be_equal_to_previous_decision(): void + { + $allowListRule = $this->getAccessibleMock(AllowListRule::class, ['dummy']); + $allowListRule->_set('allowList', []); + + $previousDecision = false; + + self::assertEquals($previousDecision, $allowListRule->decide(new Test(), $previousDecision)); + } +} diff --git a/Tests/Unit/LoggingRule/DenyListRuleTest.php b/Tests/Unit/LoggingRule/DenyListRuleTest.php new file mode 100644 index 0000000..3661e30 --- /dev/null +++ b/Tests/Unit/LoggingRule/DenyListRuleTest.php @@ -0,0 +1,38 @@ +getAccessibleMock(DenyListRule::class, ['dummy']); + $denyListRule->_set('denyList', [ + Test::class + ]); + + self::assertFalse($denyListRule->decide(new Test(), true)); + } + + /** + * @test + */ + public function if_deny_list_not_contains_class_decision_should_be_equal_to_previous_decision(): void + { + $denyListRule = $this->getAccessibleMock(DenyListRule::class, ['dummy']); + $denyListRule->_set('denyList', []); + + $previousDecision = true; + + self::assertEquals($previousDecision, $denyListRule->decide(new Test(), $previousDecision)); + } +} diff --git a/Tests/Unit/LoggingRule/ExceptionHandlerRenderingGroupsRuleTest.php b/Tests/Unit/LoggingRule/ExceptionHandlerRenderingGroupsRuleTest.php new file mode 100644 index 0000000..023102f --- /dev/null +++ b/Tests/Unit/LoggingRule/ExceptionHandlerRenderingGroupsRuleTest.php @@ -0,0 +1,85 @@ +setOptions([ + 'renderingGroups' => [ + 'netlogixSentryTest' => [ + 'matchingExceptionClassNames' => [Test::class], + 'options' => [ + 'logException' => true + ] + ] + ] + ]); + + $exceptionHandlerRenderingGroupsRule = $this->getAccessibleMock(ExceptionHandlerRenderingGroupsRule::class, ['dummy']); + $exceptionHandlerRenderingGroupsRule->_set('optionsResolver', $optionsResolver); + + self::assertTrue($exceptionHandlerRenderingGroupsRule->decide(new Test(), false)); + } + + /** + * @test + */ + public function if_log_exception_is_false_decision_should_be_false(): void + { + $optionsResolver = new ExceptionRenderingOptionsResolver(); + $optionsResolver->setOptions([ + 'renderingGroups' => [ + 'netlogixSentryTest' => [ + 'matchingExceptionClassNames' => [Test::class], + 'options' => [ + 'logException' => false + ] + ] + ] + ]); + + $exceptionHandlerRenderingGroupsRule = $this->getAccessibleMock(ExceptionHandlerRenderingGroupsRule::class, ['dummy']); + $exceptionHandlerRenderingGroupsRule->_set('optionsResolver', $optionsResolver); + + self::assertFalse($exceptionHandlerRenderingGroupsRule->decide(new Test(), true)); + } + + /** + * @test + */ + public function if_log_exception_is_null_decision_should_be_equal_to_previous_decision(): void + { + $optionsResolver = new ExceptionRenderingOptionsResolver(); + $optionsResolver->setOptions([ + 'renderingGroups' => [ + 'netlogixSentryTest' => [ + 'matchingExceptionClassNames' => [Test::class], + 'options' => [] + ] + ] + ]); + + $exceptionHandlerRenderingGroupsRule = $this->getAccessibleMock(ExceptionHandlerRenderingGroupsRule::class, ['dummy']); + $exceptionHandlerRenderingGroupsRule->_set('optionsResolver', $optionsResolver); + + $previousDecision = true; + + self::assertEquals( + $previousDecision, + $exceptionHandlerRenderingGroupsRule->decide(new Test(), $previousDecision) + ); + } +} From 867ee5f44db9722fbd708c35991bddebb02f13b1 Mon Sep 17 00:00:00 2001 From: Tim Weisenberger Date: Tue, 7 Feb 2023 18:42:09 +0100 Subject: [PATCH 2/4] BUGFIX: Remove typed properties to be compatible with PHP v7.3 --- Classes/LoggingRule/AllowListRule.php | 2 +- Classes/LoggingRule/DenyListRule.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Classes/LoggingRule/AllowListRule.php b/Classes/LoggingRule/AllowListRule.php index a6eb9ab..88387f0 100644 --- a/Classes/LoggingRule/AllowListRule.php +++ b/Classes/LoggingRule/AllowListRule.php @@ -12,7 +12,7 @@ class AllowListRule implements LoggingRule * @Flow\InjectConfiguration(path="loggingRules.allowList") * @var array */ - protected array $allowList = []; + protected $allowList = []; public function decide(Throwable $throwable, bool $previousDecision): bool { diff --git a/Classes/LoggingRule/DenyListRule.php b/Classes/LoggingRule/DenyListRule.php index b1f8b59..54eef4b 100644 --- a/Classes/LoggingRule/DenyListRule.php +++ b/Classes/LoggingRule/DenyListRule.php @@ -12,7 +12,7 @@ class DenyListRule implements LoggingRule * @Flow\InjectConfiguration(path="loggingRules.denyList") * @var array */ - protected array $denyList = []; + protected $denyList = []; public function decide(Throwable $throwable, bool $previousDecision): bool { From 2c973fae168f6dca759fabd99bdfa372f60aae3a Mon Sep 17 00:00:00 2001 From: Tim Weisenberger Date: Wed, 8 Feb 2023 08:40:36 +0100 Subject: [PATCH 3/4] BUGFIX: Ensure correct value are returned from ObjectManager mock --- .../Integration/NetlogixIntegrationTest.php | 68 +++++++++++++------ 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/Tests/Unit/Integration/NetlogixIntegrationTest.php b/Tests/Unit/Integration/NetlogixIntegrationTest.php index 0ecb5b7..2fffafd 100644 --- a/Tests/Unit/Integration/NetlogixIntegrationTest.php +++ b/Tests/Unit/Integration/NetlogixIntegrationTest.php @@ -83,20 +83,10 @@ public function Event_is_logged_depending_on_loggingRules(bool $ruleResult, bool $objectManager ->method('get') - ->with($this->logicalOr( - $this->equalTo( ConfigurationManager::class), - $this->equalTo(ExceptionHandlerRenderingGroupsRule::class) - )) - ->will($this->returnCallback(function($class) use ($configurationManager, $exceptionHandlerRenderingGroupsRule) { - if ($class === ConfigurationManager::class) { - return $configurationManager; - } else if ($class === ExceptionHandlerRenderingGroupsRule::class) { - return $exceptionHandlerRenderingGroupsRule; - } - - return null; - })); - + ->will($this->returnValueMap([ + [ConfigurationManager::class, $configurationManager], + [ExceptionHandlerRenderingGroupsRule::class, $exceptionHandlerRenderingGroupsRule] + ])); Bootstrap::$staticObjectManager = $objectManager; @@ -204,10 +194,23 @@ public function Event_is_enriched_with_ScopeProvider_data(): void ->method('collectUser') ->willReturn(['username' => 'lars']); + $configurationManager = $this->getMockBuilder(ConfigurationManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationManager + ->method('getConfiguration') + ->with( ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules') + ->willReturn([]); + $objectManager ->method('get') - ->withConsecutive([ExceptionRenderingOptionsResolver::class], [ScopeProvider::class]) - ->willReturnOnConsecutiveCalls(new ExceptionRenderingOptionsResolver(), $scopeProvider); + ->will($this->returnValueMap([ + [ConfigurationManager::class, $configurationManager], + [ExceptionHandlerRenderingGroupsRule::class, new ExceptionRenderingOptionsResolver()], + [ScopeProvider::class, $scopeProvider], + ])); Bootstrap::$staticObjectManager = $objectManager; @@ -264,10 +267,22 @@ public function Events_are_also_enriched_if_hint_does_not_contain_a_throwable(): ->method('collectUser') ->willReturn([]); + $configurationManager = $this->getMockBuilder(ConfigurationManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationManager + ->method('getConfiguration') + ->with( ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules') + ->willReturn([]); + $objectManager ->method('get') - ->with(ScopeProvider::class) - ->willReturn($scopeProvider); + ->will($this->returnValueMap([ + [ConfigurationManager::class, $configurationManager], + [ScopeProvider::class, $scopeProvider], + ])); Bootstrap::$staticObjectManager = $objectManager; @@ -299,10 +314,23 @@ public function ScopeProvider_receives_the_current_Exception(): void ->method('withThrowable') ->with($throwable); + $configurationManager = $this->getMockBuilder(ConfigurationManager::class) + ->disableOriginalConstructor() + ->getMock(); + + $configurationManager + ->method('getConfiguration') + ->with( ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.loggingRules.rules') + ->willReturn([]); + $objectManager ->method('get') - ->withConsecutive([ExceptionRenderingOptionsResolver::class], [ScopeProvider::class]) - ->willReturnOnConsecutiveCalls(new ExceptionRenderingOptionsResolver(), $scopeProvider); + ->will($this->returnValueMap([ + [ConfigurationManager::class, $configurationManager], + [ExceptionRenderingOptionsResolver::class, new ExceptionRenderingOptionsResolver()], + [ScopeProvider::class, $scopeProvider], + ])); Bootstrap::$staticObjectManager = $objectManager; From 46b65ae0fa50c86335bbf1451d8b7eb9f4dd791e Mon Sep 17 00:00:00 2001 From: Tim Weisenberger Date: Wed, 8 Feb 2023 08:41:13 +0100 Subject: [PATCH 4/4] BUGFIX: Use correct namespace for unit tests --- Tests/Unit/Scope/ScopeProviderTest.php | 2 +- Tests/Unit/Scope/User/FlowAccountTest.php | 2 +- composer.json | 8 ++++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Tests/Unit/Scope/ScopeProviderTest.php b/Tests/Unit/Scope/ScopeProviderTest.php index d87187b..75c74de 100644 --- a/Tests/Unit/Scope/ScopeProviderTest.php +++ b/Tests/Unit/Scope/ScopeProviderTest.php @@ -1,7 +1,7 @@