Skip to content

Commit 2561888

Browse files
Xfaider48ramunasd
Xfaider48
authored and
ramunasd
committed
fix: Fix mark channels closed when connection is closed
- Added unit tests - Method names are renamed
1 parent b37f7b4 commit 2561888

File tree

4 files changed

+161
-5
lines changed

4 files changed

+161
-5
lines changed

‎PhpAmqpLib/Channel/AMQPChannel.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,19 @@ public function close($reply_code = 0, $reply_text = '', $method_sig = array(0,
207207
), false, $this->channel_rpc_timeout);
208208
}
209209

210-
public function markClosed()
210+
/**
211+
* Closes a channel if no connection or a connection is closed
212+
*
213+
* @return bool
214+
*/
215+
public function closeIfDisconnected(): bool
211216
{
217+
if (!$this->connection || $this->connection->isConnected()) {
218+
return false;
219+
}
220+
212221
$this->do_close();
222+
return true;
213223
}
214224

215225
/**

‎PhpAmqpLib/Connection/AbstractConnection.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ protected function do_close()
424424
$this->frame_queue = new \SplQueue();
425425
$this->method_queue = [];
426426
$this->setIsConnected(false);
427-
$this->markChannelsClosed();
427+
$this->closeChannelsIfDisconnected();
428428
$this->close_input();
429429
$this->close_socket();
430430
}
@@ -1114,17 +1114,17 @@ protected function closeChannels()
11141114
}
11151115

11161116
/**
1117-
* Mark all available channels as closed
1117+
* Closes all available channels if disconnected
11181118
*/
1119-
protected function markChannelsClosed()
1119+
protected function closeChannelsIfDisconnected()
11201120
{
11211121
foreach ($this->channels as $key => $channel) {
11221122
// channels[0] is this connection object, so don't close it yet
11231123
if ($key === 0) {
11241124
continue;
11251125
}
11261126

1127-
$channel->markClosed();
1127+
$channel->closeIfDisconnected();
11281128
}
11291129
}
11301130

‎tests/Unit/Channel/AMQPChannelTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use PhpAmqpLib\Tests\Unit\Test\TestChannel;
1010
use PhpAmqpLib\Tests\Unit\Test\TestConnection;
1111
use PhpAmqpLib\Wire\AMQPWriter;
12+
use PHPUnit\Framework\MockObject\MockObject;
1213
use PHPUnit\Framework\TestCase;
1314

1415
class AMQPChannelTest extends TestCase
@@ -99,6 +100,72 @@ public function publish_batch_opened_connection(): void
99100
$channel->publish_batch();
100101
}
101102

103+
/**
104+
* @test
105+
*/
106+
public function close_if_disconnected_false(): void
107+
{
108+
$mockBuilder = $this->getMockBuilder(TestConnection::class)
109+
->disableOriginalConstructor();
110+
111+
$methodsToMock = [
112+
'isConnected',
113+
];
114+
115+
if (!method_exists($mockBuilder, 'onlyMethods')) {
116+
$mockBuilder->setMethods($methodsToMock);
117+
} else {
118+
$mockBuilder->onlyMethods($methodsToMock);
119+
}
120+
121+
/** @var MockObject&TestConnection $connectionMock */
122+
$connectionMock = $mockBuilder->getMock();
123+
$channel = new TestChannel($connectionMock, 1);
124+
125+
$connectionMock->expects(self::once())
126+
->method('isConnected')
127+
->willReturn(true);
128+
129+
// Should return `false` because $this->connection->isConnected()
130+
$firstResult = $channel->closeIfDisconnected();
131+
$this->assertFalse($firstResult);
132+
}
133+
134+
/**
135+
* @test
136+
*/
137+
public function close_if_disconnected_true(): void
138+
{
139+
$mockBuilder = $this->getMockBuilder(TestConnection::class)
140+
->disableOriginalConstructor();
141+
142+
$methodsToMock = [
143+
'isConnected',
144+
];
145+
146+
if (!method_exists($mockBuilder, 'onlyMethods')) {
147+
$mockBuilder->setMethods($methodsToMock);
148+
} else {
149+
$mockBuilder->onlyMethods($methodsToMock);
150+
}
151+
152+
/** @var MockObject&TestConnection $connectionMock */
153+
$connectionMock = $mockBuilder->getMock();
154+
$channel = new TestChannel($connectionMock, 1);
155+
156+
$connectionMock->expects(self::once())
157+
->method('isConnected')
158+
->willReturn(false);
159+
160+
// Should return true
161+
$firstResult = $channel->closeIfDisconnected();
162+
$this->assertTrue($firstResult);
163+
164+
// Should return `false` because no $this->connection
165+
$secondResult = $channel->closeIfDisconnected();
166+
$this->assertFalse($secondResult);
167+
}
168+
102169
public function basic_consume_invalid_arguments_provider()
103170
{
104171
return [

‎tests/Unit/Connection/AbstractConnectionTest.php‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,14 @@
22

33
namespace PhpAmqpLib\Tests\Unit\Connection;
44

5+
use PhpAmqpLib\Channel\Frame;
6+
use PhpAmqpLib\Connection\AbstractConnection;
57
use PhpAmqpLib\Connection\AMQPConnectionConfig;
68
use PhpAmqpLib\Connection\AMQPConnectionFactory;
9+
use PhpAmqpLib\Exception\AMQPConnectionClosedException;
710
use PhpAmqpLib\Tests\Unit\Test\TestConnection;
11+
use PhpAmqpLib\Wire\IO\AbstractIO;
12+
use PHPUnit\Framework\MockObject\MockObject;
813
use PHPUnit\Framework\TestCase;
914

1015
class AbstractConnectionTest extends TestCase
@@ -39,4 +44,78 @@ public function connection_login_method_external(): void
3944
$property->setAccessible(true);
4045
self::assertEquals($response, $property->getValue($connection));
4146
}
47+
48+
/**
49+
* @test
50+
*/
51+
public function close_channels_if_disconnected(): void
52+
{
53+
$ioMock = $this->createMock(AbstractIO::class);
54+
$config = new AMQPConnectionConfig();
55+
$config->setIsLazy(false);
56+
57+
$args = [
58+
$user = null,
59+
$password = null,
60+
$vhost = '/',
61+
$insist = false,
62+
$login_method = 'AMQPLAIN',
63+
$login_response = null,
64+
$locale = 'en_US',
65+
$ioMock,
66+
$heartbeat = 0,
67+
$connectionTimeout = 0,
68+
$channelRpcTimeout = 0.0,
69+
$config
70+
];
71+
72+
/** @var MockObject&AbstractConnection $connection */
73+
$connection = $this->getMockForAbstractClass(
74+
AbstractConnection::class,
75+
$args,
76+
$mockClassName = '',
77+
$callOriginalConstructor = true,
78+
$callOriginalClone = true,
79+
$callAutoload = true,
80+
$mockedMethods = [
81+
'connect',
82+
'send_channel_method_frame',
83+
'wait_channel',
84+
]
85+
);
86+
87+
// Emulate channel.open_ok
88+
$payload = pack('n2', 20, 11);
89+
$connection
90+
->method('wait_channel')
91+
->willReturn(new Frame(
92+
Frame::TYPE_METHOD,
93+
1,
94+
mb_strlen($payload),
95+
$payload
96+
));
97+
$newChannel = $connection->channel(1);
98+
$this->assertTrue($newChannel->is_open());
99+
100+
// Emulate error to call do_close
101+
$ioMock
102+
->expects($this->once())
103+
->method('select')
104+
->willThrowException(new AMQPConnectionClosedException('test'));
105+
$ioMock
106+
->expects($this->once())
107+
->method('close');
108+
109+
$exception = null;
110+
try {
111+
$connection->select(3);
112+
} catch (AMQPConnectionClosedException $exception) {
113+
}
114+
115+
$this->assertInstanceOf(AMQPConnectionClosedException::class, $exception);
116+
$this->assertEquals('test', $exception->getMessage());
117+
118+
// After do_close is called, channel must be inactive too
119+
$this->assertFalse($newChannel->is_open());
120+
}
42121
}

0 commit comments

Comments
 (0)