Skip to content

Commit b34b319

Browse files
andrestejerina97andrestejerina97
authored
fix: add null check matchesStrategy (#513)
1 parent 2db6e9b commit b34b319

3 files changed

Lines changed: 173 additions & 2 deletions

File tree

‎.github/workflows/push.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ jobs:
3030
run: |
3131
echo "running OpenTelemetry Formatter tests"
3232
vendor/bin/phpunit tests/OpenTelemetry/Formatters/ --log-junit results_opentelemetry_tests.xml
33+
echo "running AuditLogFormatterFactoryTest"
34+
vendor/bin/phpunit tests/OpenTelemetry/AuditLogFormatterFactoryTest.php --log-junit results_audit_formatter_tests.xml
3335
3436
- name: Upload OpenTelemetry Tests Output
3537
uses: actions/upload-artifact@v4
@@ -38,6 +40,13 @@ jobs:
3840
path: results_opentelemetry_tests.xml
3941
retention-days: 5
4042

43+
- name: Upload AuditLogFormatterFactory Tests Output
44+
uses: actions/upload-artifact@v4
45+
with:
46+
name: results_audit_formatter_tests
47+
path: results_audit_formatter_tests.xml
48+
retention-days: 5
49+
4150
integration-tests:
4251
runs-on: ubuntu-latest
4352
strategy:

‎app/Audit/AuditLogFormatterFactory.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,13 @@ private function getFormatterByContext(object $subject, string $event_type, Audi
163163

164164
private function matchesStrategy(array $strategy, AuditContext $ctx): bool
165165
{
166-
if (isset($strategy['route']) && !$this->routeMatches($strategy['route'], $ctx->rawRoute)) {
167-
return false;
166+
if (isset($strategy['route'])) {
167+
if ($ctx->rawRoute === null) {
168+
return false;
169+
}
170+
if (!$this->routeMatches($strategy['route'], $ctx->rawRoute)) {
171+
return false;
172+
}
168173
}
169174

170175
return true;

‎tests/OpenTelemetry/AuditLogFormatterFactoryTest.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
<?php
2+
namespace Tests\OpenTelemetry;
3+
4+
/**
5+
* Copyright 2026 OpenStack Foundation
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
**/
16+
17+
use App\Audit\AuditContext;
18+
use App\Audit\AuditLogFormatterFactory;
19+
use PHPUnit\Framework\TestCase;
20+
21+
/**
22+
* Class AuditLogFormatterFactoryTest
23+
* Tests for AuditLogFormatterFactory::matchesStrategy() null guard
24+
*/
25+
class AuditLogFormatterFactoryTest extends TestCase
26+
{
27+
private const ROUTE_CREATE_EVENT = 'POST|api/v1/summits/{id}/events';
28+
private const ROUTE_UPDATE_PRESENTATION = 'PUT|api/v1/summits/{id}/presentations';
29+
private const FORMATTER_CLASS = 'TestFormatterClass';
30+
31+
private AuditLogFormatterFactory $factory;
32+
33+
protected function setUp(): void
34+
{
35+
parent::setUp();
36+
$this->factory = new AuditLogFormatterFactory();
37+
}
38+
39+
40+
public function testMatchesStrategyHandlesNullRawRouteWithRouteRequired(): void
41+
{
42+
$strategy = [
43+
'route' => self::ROUTE_CREATE_EVENT,
44+
'formatter' => self::FORMATTER_CLASS
45+
];
46+
47+
$ctx = new AuditContext(
48+
userId: null,
49+
userEmail: null,
50+
userFirstName: null,
51+
userLastName: null,
52+
uiApp: null,
53+
uiFlow: null,
54+
route: null,
55+
rawRoute: null, // null rawRoute from console command
56+
httpMethod: null,
57+
clientIp: null,
58+
userAgent: null
59+
);
60+
61+
$reflection = new \ReflectionClass($this->factory);
62+
$method = $reflection->getMethod('matchesStrategy');
63+
$method->setAccessible(true);
64+
65+
$result = $method->invoke($this->factory, $strategy, $ctx);
66+
$this->assertFalse($result, 'matchesStrategy should return false when rawRoute is null and route is required');
67+
}
68+
69+
70+
public function testMatchesStrategyReturnsTrueWhenNoRouteRequiredAndRawRouteNull(): void
71+
{
72+
$strategy = [
73+
'formatter' => self::FORMATTER_CLASS
74+
];
75+
76+
$ctx = new AuditContext(
77+
userId: null,
78+
userEmail: null,
79+
userFirstName: null,
80+
userLastName: null,
81+
uiApp: null,
82+
uiFlow: null,
83+
route: null,
84+
rawRoute: null,
85+
httpMethod: null,
86+
clientIp: null,
87+
userAgent: null
88+
);
89+
90+
$reflection = new \ReflectionClass($this->factory);
91+
$method = $reflection->getMethod('matchesStrategy');
92+
$method->setAccessible(true);
93+
94+
$result = $method->invoke($this->factory, $strategy, $ctx);
95+
$this->assertTrue($result, 'matchesStrategy should return true when no route is required');
96+
}
97+
98+
99+
public function testMatchesStrategyReturnsTrueWhenRouteMatches(): void
100+
{
101+
$strategy = [
102+
'route' => self::ROUTE_CREATE_EVENT,
103+
'formatter' => self::FORMATTER_CLASS
104+
];
105+
106+
$ctx = new AuditContext(
107+
userId: null,
108+
userEmail: null,
109+
userFirstName: null,
110+
userLastName: null,
111+
uiApp: null,
112+
uiFlow: null,
113+
route: null,
114+
rawRoute: self::ROUTE_CREATE_EVENT, // matching rawRoute
115+
httpMethod: null,
116+
clientIp: null,
117+
userAgent: null
118+
);
119+
120+
$reflection = new \ReflectionClass($this->factory);
121+
$method = $reflection->getMethod('matchesStrategy');
122+
$method->setAccessible(true);
123+
124+
$result = $method->invoke($this->factory, $strategy, $ctx);
125+
$this->assertTrue($result, 'matchesStrategy should return true when routes match');
126+
}
127+
128+
129+
public function testMatchesStrategyReturnsFalseWhenRouteDoesNotMatch(): void
130+
{
131+
$strategy = [
132+
'route' => self::ROUTE_CREATE_EVENT,
133+
'formatter' => self::FORMATTER_CLASS
134+
];
135+
136+
$ctx = new AuditContext(
137+
userId: null,
138+
userEmail: null,
139+
userFirstName: null,
140+
userLastName: null,
141+
uiApp: null,
142+
uiFlow: null,
143+
route: null,
144+
rawRoute: self::ROUTE_UPDATE_PRESENTATION, // non-matching rawRoute
145+
httpMethod: null,
146+
clientIp: null,
147+
userAgent: null
148+
);
149+
150+
$reflection = new \ReflectionClass($this->factory);
151+
$method = $reflection->getMethod('matchesStrategy');
152+
$method->setAccessible(true);
153+
154+
$result = $method->invoke($this->factory, $strategy, $ctx);
155+
$this->assertFalse($result, 'matchesStrategy should return false when routes do not match');
156+
}
157+
}

0 commit comments

Comments
 (0)