Skip to content

Commit 099501f

Browse files
author
klapaudius
committed
Add PSR/log support, custom exception, and SSE transport tests
Introduced `psr/log` as a dependency to enhance logging capabilities. Added `SseTransportException` for more specific error handling within the SSE transport. Implemented comprehensive unit tests for SSE transport to validate behavior, including message sending, initialization, and error handling.
1 parent 4a17636 commit 099501f

File tree

4 files changed

+329
-16
lines changed

4 files changed

+329
-16
lines changed

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"php": "^8.2",
99
"ext-json": "*",
1010
"ext-redis": "*",
11+
"psr/log": "^3.0",
1112
"symfony/console": "~7.0",
1213
"symfony/dependency-injection": "~7.0",
1314
"symfony/http-foundation": "~7.0"

‎src/Transports/SseTransport.php‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,29 +104,28 @@ public function initialize(): void
104104
*/
105105
private function sendEvent(string $event, string $data): void
106106
{
107-
// 헤더 설정이 이미 전송되었는지 확인
107+
// Check if the headers have already been sent
108108
if (! headers_sent()) {
109-
// 버퍼링 비활성화
109+
// Disable buffering
110110
ini_set('output_buffering', 'off');
111111
ini_set('zlib.output_compression', false);
112112

113-
// 필수 SSE 헤더 추가
113+
// Add required SSE headers
114114
header('Content-Type: text/event-stream');
115115
header('Cache-Control: no-cache');
116116
header('X-Accel-Buffering: no');
117117
header('Connection: keep-alive');
118118
}
119119

120-
// 모든 버퍼 비우기
121-
// while (ob_get_level() > 0) {
122-
// ob_end_flush();
123-
// }
120+
// Just ensure output gets flushed
121+
ob_flush(); // Flushes the active buffer
122+
flush(); // Flushes the system-level buffer (important for real-time outputs)
124123

125124
echo sprintf('event: %s', $event).PHP_EOL;
126125
echo sprintf('data: %s', $data).PHP_EOL;
127126
echo PHP_EOL;
128127

129-
flush();
128+
flush(); // Ensure the data is sent to the client
130129
}
131130

132131
/**
@@ -176,11 +175,7 @@ public function close(): void
176175
}
177176
}
178177

179-
try {
180-
$this->sendEvent(event: 'close', data: '{"reason":"server_closed"}');
181-
} catch (Exception $e) {
182-
$this->logger?->info('Could not send final SSE close event: '.$e->getMessage());
183-
}
178+
$this->sendEvent(event: 'close', data: '{"reason":"server_closed"}');
184179
}
185180

186181
/**
@@ -305,17 +300,17 @@ public function processMessage(string $clientId, array $message): void
305300
* @param string $clientId The target client ID.
306301
* @param array $message The message payload (as an array).
307302
*
308-
* @throws Exception If adapter is not set, JSON encoding fails, or adapter push fails.
303+
* @throws SseTransportException If adapter is not set, JSON encoding fails, or adapter push fails.
309304
*/
310305
public function pushMessage(string $clientId, array $message): void
311306
{
312307
if ($this->adapter === null) {
313-
throw new Exception('Cannot push message: SSE Adapter is not configured.');
308+
throw new SseTransportException('Cannot push message: SSE Adapter is not configured.');
314309
}
315310

316311
$messageString = json_encode($message);
317312
if ($messageString === false) {
318-
throw new Exception('Failed to JSON encode message for pushing: '.json_last_error_msg());
313+
throw new SseTransportException('Failed to JSON encode message for pushing: '.json_last_error_msg());
319314
}
320315

321316
$this->adapter->pushMessage(clientId: $clientId, message: $messageString);

‎src/Transports/SseTransportException.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
namespace KLP\KlpMcpServer\Transports;
4+
5+
class SseTransportException extends \Exception
6+
{
7+
8+
}

‎tests/Transports/SseTransportTest.php‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,309 @@
1+
<?php
2+
3+
namespace KLP\KlpMcpServer\Tests\Transports;
4+
5+
use KLP\KlpMcpServer\Transports\SseAdapters\SseAdapterInterface;
6+
use KLP\KlpMcpServer\Transports\SseTransport;
7+
use PHPUnit\Framework\Attributes\Small;
8+
use PHPUnit\Framework\MockObject\MockObject;
9+
use PHPUnit\Framework\TestCase;
10+
use Psr\Log\LoggerInterface;
11+
12+
#[Small]
13+
class SseTransportTest extends TestCase
14+
{
15+
private LoggerInterface|MockObject $loggerMock;
16+
private SseTransport $instance;
17+
18+
protected function setUp(): void
19+
{
20+
parent::setUp();
21+
$this->loggerMock = $this->createMock(LoggerInterface::class);
22+
$this->instance = new SseTransport('/default-path', $this->loggerMock);
23+
}
24+
25+
/**
26+
* Test that sending a string message calls the sendEvent method with 'message' event.
27+
*/
28+
public function test_send_string_message(): void
29+
{
30+
// Arrange
31+
$message = 'Test string message';
32+
33+
// Act
34+
ob_start();
35+
try {
36+
$this->instance->send($message);
37+
$output = ob_get_contents(); // Capture the output before clearing the buffer
38+
} finally {
39+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
40+
}
41+
42+
// Assert
43+
$this->assertEquals(
44+
'event: message' . PHP_EOL
45+
. 'data: ' .$message . PHP_EOL . PHP_EOL,
46+
$output
47+
);
48+
}
49+
50+
/**
51+
* Test that initialize generates a unique client ID and sends an 'endpoint' event.
52+
*/
53+
public function test_initialize_generates_client_id_and_sends_endpoint(): void
54+
{
55+
// Act
56+
ob_start();
57+
try {
58+
$this->instance->initialize();
59+
$output = ob_get_contents(); // Capture the output before clearing the buffer
60+
} finally {
61+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
62+
}
63+
64+
// Assert
65+
$expectedClientId = $this->getProtectedProperty($this->instance, 'clientId');
66+
$expectedOutput = 'event: endpoint' . PHP_EOL
67+
. 'data: /default-path/message?sessionId=' . $expectedClientId . PHP_EOL . PHP_EOL;
68+
69+
$this->assertNotNull($expectedClientId);
70+
$this->assertEquals($expectedOutput, $output);
71+
}
72+
73+
/**
74+
* Test that initialize does not overwrite an existing client ID.
75+
*/
76+
public function test_initializeD_does_not_overwrite_existing_client_id(): void
77+
{
78+
// Arrange
79+
$existingClientId = 'predefined-client-id';
80+
$this->setProtectedProperty($this->instance, 'clientId', $existingClientId);
81+
82+
// Act
83+
ob_start();
84+
try {
85+
$this->instance->initialize();
86+
$output = ob_get_contents(); // Capture the output before clearing the buffer
87+
} finally {
88+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
89+
}
90+
91+
// Assert
92+
$currentClientId = $this->getProtectedProperty($this->instance, 'clientId');
93+
$expectedOutput = 'event: endpoint' . PHP_EOL
94+
. 'data: /default-path/message?sessionId=' . $existingClientId . PHP_EOL . PHP_EOL;
95+
96+
$this->assertSame($existingClientId, $currentClientId);
97+
$this->assertEquals($expectedOutput, $output);
98+
}
99+
100+
/**
101+
* Test that sending an array message encodes it to JSON and calls sendEvent with 'message' event.
102+
*/
103+
public function test_send_array_message(): void
104+
{
105+
// Define the input array and expected JSON
106+
// Arrange
107+
$messageArray = ['key' => 'value', 'anotherKey' => 123];
108+
$messageJson = json_encode($messageArray);
109+
110+
// Act
111+
ob_start();
112+
try {
113+
$this->instance->send($messageArray);
114+
$output = ob_get_contents(); // Capture the output before clearing the buffer
115+
} finally {
116+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
117+
}
118+
119+
// Assert
120+
$this->assertEquals(
121+
'event: message' . PHP_EOL
122+
. 'data: ' .$messageJson . PHP_EOL . PHP_EOL,
123+
$output
124+
);
125+
}
126+
127+
private function getProtectedProperty(SseTransport $instance, string $propertyName)
128+
{
129+
$reflection = new \ReflectionClass($instance);
130+
$prop = $reflection->getProperty($propertyName);
131+
132+
return $prop->getValue($instance);
133+
}
134+
135+
private function setProtectedProperty(SseTransport $instance, string $propertyName, string $propertyValue):void
136+
{
137+
$reflection = new \ReflectionClass($instance);
138+
$prop = $reflection->getProperty($propertyName);
139+
$prop->setValue($instance, $propertyValue);
140+
}
141+
142+
/**
143+
* Test that the start() method sets the connected flag to true and initializes the transport.
144+
*/
145+
public function test_start_sets_connected_flag_and_initializes_transport(): void
146+
{
147+
// Act
148+
ob_start();
149+
try {
150+
$this->instance->start();
151+
} finally {
152+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
153+
}
154+
155+
// Assert
156+
$this->assertTrue($this->getProtectedProperty($this->instance, 'connected'));
157+
}
158+
159+
/**
160+
* Test that the start() method does not reinitialize the transport if already connected.
161+
*/
162+
public function test_start_does_not_reinitialize_transport_when_already_connected(): void
163+
{
164+
// Arrange
165+
$this->setProtectedProperty($this->instance, 'connected', true);
166+
167+
// Act
168+
ob_start();
169+
try {
170+
$this->instance->start();
171+
} finally {
172+
ob_end_clean(); // Ensure the buffer is cleaned even if an exception occurs
173+
}
174+
175+
// Assert
176+
$this->assertTrue($this->getProtectedProperty($this->instance, 'connected'));
177+
}
178+
179+
/**
180+
* Test that the registered close handlers are executed when `close` is called.
181+
*/
182+
public function test_close_executes_registered_handlers(): void
183+
{
184+
// Arrange
185+
$this->setProtectedProperty($this->instance, 'connected', true);
186+
$handlerExecuted = false;
187+
$this->instance->onClose(function () use (&$handlerExecuted) {
188+
$handlerExecuted = true;
189+
});
190+
191+
// Act
192+
ob_start();
193+
try {
194+
$this->instance->close();
195+
} finally {
196+
ob_end_clean();
197+
}
198+
199+
// Assert
200+
$this->assertTrue($handlerExecuted, 'The close handler was not executed.');
201+
}
202+
203+
/**
204+
* Test that the adapter's resources are cleaned up when `close` is called with an adapter set.
205+
*/
206+
public function test_close_removes_adapter_resources(): void
207+
{
208+
// Arrange
209+
$this->setProtectedProperty($this->instance, 'connected', true);
210+
$adapterMock = $this->createMock(SseAdapterInterface::class);
211+
$this->instance->setAdapter($adapterMock);
212+
$this->setProtectedProperty($this->instance, 'clientId', 'test-client-id');
213+
$adapterMock->expects($this->once())
214+
->method('removeAllMessages')
215+
->with('test-client-id');
216+
217+
// Act
218+
ob_start();
219+
try {
220+
$this->instance->close();
221+
} finally {
222+
ob_end_clean();
223+
}
224+
}
225+
226+
/**
227+
* Test that the `close` method sends a close event with the correct data.
228+
*/
229+
public function test_close_sends_close_event(): void
230+
{
231+
// Arrange
232+
$this->setProtectedProperty($this->instance, 'connected', true);
233+
234+
// Act
235+
ob_start();
236+
try {
237+
$this->instance->close();
238+
$output = ob_get_contents();
239+
} finally {
240+
ob_end_clean();
241+
}
242+
243+
// Assert
244+
$this->assertEquals(
245+
'event: close' . PHP_EOL
246+
. 'data: {"reason":"server_closed"}' . PHP_EOL . PHP_EOL,
247+
$output
248+
);
249+
}
250+
251+
/**
252+
* Test that the `close` method does nothing if the connection is already closed.
253+
*/
254+
public function test_close_does_nothing_when_already_closed(): void
255+
{
256+
// Arrange
257+
$this->setProtectedProperty($this->instance, 'connected', false);
258+
259+
// Act
260+
ob_start();
261+
try {
262+
$this->instance->close();
263+
$output = ob_get_contents();
264+
} finally {
265+
ob_end_clean();
266+
}
267+
268+
// Assert
269+
$this->assertEmpty($output, 'Output should be empty when already closed.');
270+
}
271+
272+
/**
273+
* Test that exceptions during close handlers or adapter cleanup are logged correctly.
274+
*/
275+
public function test_close_logs_exceptions_in_handlers_or_cleanup(): void
276+
{
277+
// Arrange
278+
$this->setProtectedProperty($this->instance, 'connected', true);
279+
$invocations = [
280+
'Error in SSE close handler: Handler Exception',
281+
'Error cleaning up SSE adapter resources on close: Adapter Exception'
282+
];
283+
$this->loggerMock->expects($matcher = $this->exactly(2))
284+
->method('error')
285+
->with($this->callback(function ($arg) use (&$invocations, $matcher) {
286+
$this->assertEquals($arg, $invocations[$matcher->numberOfInvocations() - 1]);
287+
return true;
288+
}));
289+
290+
$this->instance->onClose(function () {
291+
throw new \Exception('Handler Exception');
292+
});
293+
294+
$adapterMock = $this->createMock(SseAdapterInterface::class);
295+
$this->instance->setAdapter($adapterMock);
296+
$this->setProtectedProperty($this->instance, 'clientId', 'test-client-id');
297+
$adapterMock->expects($this->once())
298+
->method('removeAllMessages')
299+
->willThrowException(new \Exception('Adapter Exception'));
300+
301+
// Act
302+
ob_start();
303+
try {
304+
$this->instance->close();
305+
} finally {
306+
ob_end_clean();
307+
}
308+
}
309+
}

0 commit comments

Comments
 (0)