Skip to content

Commit afbeaaa

Browse files
markstorymarkstory
authored
Merge pull request #57 from cakephp/fix-55
Fix worker compatibility in PHP8
2 parents 2495966 + bf65a8d commit afbeaaa

File tree

5 files changed

+153
-44
lines changed

5 files changed

+153
-44
lines changed

‎phpunit.xml.dist‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
11
<?xml version="1.0" encoding="UTF-8"?>
22
<phpunit
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
34
colors="true"
45
bootstrap="tests/bootstrap.php"
5-
>
6-
7-
<testsuites>
8-
<testsuite name="queue">
9-
<directory>tests/TestCase</directory>
10-
</testsuite>
11-
</testsuites>
12-
13-
<!-- Setup a listener for fixtures -->
14-
<listeners>
15-
<listener class="Cake\TestSuite\Fixture\FixtureInjector">
16-
<arguments>
17-
<object class="Cake\TestSuite\Fixture\FixtureManager"/>
18-
</arguments>
19-
</listener>
20-
</listeners>
21-
22-
<!-- Prevent coverage reports from looking in tests, vendors, config folders -->
23-
<filter>
24-
<whitelist>
25-
<directory suffix=".php">src/</directory>
26-
</whitelist>
27-
</filter>
28-
6+
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
7+
<!-- Only collect coverage for src/ -->
8+
<coverage>
9+
<include>
10+
<directory suffix=".php">src/</directory>
11+
</include>
12+
</coverage>
13+
<testsuites>
14+
<testsuite name="queue">
15+
<directory>tests/TestCase</directory>
16+
</testsuite>
17+
</testsuites>
18+
<!-- Setup a listener for fixtures -->
19+
<listeners>
20+
<listener class="Cake\TestSuite\Fixture\FixtureInjector">
21+
<arguments>
22+
<object class="Cake\TestSuite\Fixture\FixtureManager"/>
23+
</arguments>
24+
</listener>
25+
</listeners>
2926
</phpunit>

‎src/Job/Message.php‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
namespace Cake\Queue\Job;
1818

1919
use Cake\Utility\Hash;
20+
use Closure;
2021
use Interop\Queue\Context;
2122
use Interop\Queue\Message as QueueMessage;
2223
use JsonSerializable;
24+
use RuntimeException;
2325

2426
class Message implements JsonSerializable
2527
{
@@ -38,6 +40,11 @@ class Message implements JsonSerializable
3840
*/
3941
protected $parsedBody;
4042

43+
/**
44+
* @var \Closure|null
45+
*/
46+
protected $callable;
47+
4148
/**
4249
* @param \Interop\Queue\Message $originalMessage Queue message.
4350
* @param \Interop\Queue\Context $context Context.
@@ -74,11 +81,35 @@ public function getParsedBody(): array
7481
}
7582

7683
/**
77-
* @return mixed
84+
* Get a closure containing the callable in the job.
85+
*
86+
* Supported callables include:
87+
*
88+
* - strings for global functions, or static method names.
89+
* - array of [class, method]. The class will be constructed with no constructor parameters.
90+
*
91+
* @return \Closure
7892
*/
7993
public function getCallable()
8094
{
81-
return $this->parsedBody['class'] ?? null;
95+
if ($this->callable) {
96+
return $this->callable;
97+
}
98+
$target = $this->parsedBody['class'] ?? null;
99+
100+
$callable = null;
101+
if (is_array($target) && count($target) === 2) {
102+
$instance = new $target[0]();
103+
$callable = Closure::fromCallable([$instance, $target[1]]);
104+
} elseif (is_string($target)) {
105+
/** @psalm-suppress InvalidArgument */
106+
$callable = Closure::fromCallable($target);
107+
} else {
108+
throw new RuntimeException(sprintf('Could not create callable from `%s`', json_encode($target)));
109+
}
110+
$this->callable = $callable;
111+
112+
return $this->callable;
82113
}
83114

84115
/**

‎src/Queue/Processor.php‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
use Cake\Event\EventDispatcherTrait;
2020
use Cake\Log\LogTrait;
2121
use Cake\Queue\Job\Message;
22+
use Error;
2223
use Exception;
2324
use Interop\Queue\Context;
2425
use Interop\Queue\Message as QueueMessage;
2526
use Interop\Queue\Processor as InteropProcessor;
2627
use Psr\Log\LoggerInterface;
2728
use Psr\Log\NullLogger;
29+
use RuntimeException;
2830

2931
class Processor implements InteropProcessor
3032
{
@@ -58,7 +60,9 @@ public function process(QueueMessage $message, Context $context)
5860
$this->dispatchEvent('Processor.message.seen', ['queueMessage' => $message]);
5961

6062
$jobMessage = new Message($message, $context);
61-
if (!is_callable($jobMessage->getCallable())) {
63+
try {
64+
$jobMessage->getCallable();
65+
} catch (RuntimeException | Error $e) {
6266
$this->logger->debug('Invalid callable for message. Rejecting message from queue.');
6367
$this->dispatchEvent('Processor.message.invalid', ['message' => $jobMessage]);
6468

@@ -106,18 +110,7 @@ public function process(QueueMessage $message, Context $context)
106110
public function processMessage(Message $message)
107111
{
108112
$callable = $message->getCallable();
109-
110-
$response = InteropProcessor::REQUEUE;
111-
if (is_array($callable) && count($callable) == 2) {
112-
$className = $callable[0];
113-
$methodName = $callable[1];
114-
$instance = new $className();
115-
$response = $instance->$methodName($message);
116-
} elseif (is_string($callable)) {
117-
/** @psalm-suppress InvalidArgument */
118-
$response = call_user_func($callable, $message);
119-
}
120-
113+
$response = $callable($message);
121114
if ($response === null) {
122115
$response = InteropProcessor::ACK;
123116
}

‎tests/TestCase/Job/MessageTest.php‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@
1818

1919
use Cake\Queue\Job\Message;
2020
use Cake\TestSuite\TestCase;
21+
use Closure;
2122
use Enqueue\Null\NullConnectionFactory;
2223
use Enqueue\Null\NullMessage;
24+
use Error;
25+
use RuntimeException;
2326

2427
class MessageTest extends TestCase
2528
{
@@ -30,7 +33,7 @@ class MessageTest extends TestCase
3033
*/
3134
public function testConstructorAndGetters()
3235
{
33-
$callable = ['App\\Job\\ExampleJob','execute'];
36+
$callable = ['TestApp\WelcomeMailer', 'welcome'];
3437
$data = 'sample data ' . time();
3538
$id = 7;
3639
$args = compact('id', 'data');
@@ -49,7 +52,7 @@ public function testConstructorAndGetters()
4952
$this->assertSame($context, $message->getContext());
5053
$this->assertSame($originalMessage, $message->getOriginalMessage());
5154
$this->assertSame($parsedBody, $message->getParsedBody());
52-
$this->assertSame($callable, $message->getCallable());
55+
$this->assertInstanceOf(Closure::class, $message->getCallable());
5356
$this->assertSame($args, $message->getArgument());
5457
$this->assertSame($id, $message->getArgument('id'));
5558
$this->assertSame($data, $message->getArgument('data', 'ignore_this'));
@@ -60,4 +63,50 @@ public function testConstructorAndGetters()
6063
$actualToStringValue = (string)$message;
6164
$this->assertSame($messageBody, $actualToStringValue);
6265
}
66+
67+
/**
68+
* Test that invalid classes cannot be made into callables.
69+
*
70+
* @return void
71+
*/
72+
public function testGetCallableInvalidClass()
73+
{
74+
$parsedBody = [
75+
'queue' => 'default',
76+
'class' => ['Trash', 'trash'],
77+
'args' => [],
78+
];
79+
$messageBody = json_encode($parsedBody);
80+
$connectionFactory = new NullConnectionFactory();
81+
82+
$context = $connectionFactory->createContext();
83+
$originalMessage = new NullMessage($messageBody);
84+
$message = new Message($originalMessage, $context);
85+
86+
$this->expectException(Error::class);
87+
$message->getCallable();
88+
}
89+
90+
/**
91+
* Test that invalid classes cannot be made into callables.
92+
*
93+
* @return void
94+
*/
95+
public function testGetCallableInvalidType()
96+
{
97+
$parsedBody = [
98+
'queue' => 'default',
99+
'class' => ['TestApp\WelcomeMailer', 'trash', 'oops'],
100+
'args' => [],
101+
];
102+
$messageBody = json_encode($parsedBody);
103+
$connectionFactory = new NullConnectionFactory();
104+
105+
$context = $connectionFactory->createContext();
106+
$originalMessage = new NullMessage($messageBody);
107+
$message = new Message($originalMessage, $context);
108+
109+
$this->expectException(RuntimeException::class);
110+
$message->getCallable();
111+
}
63112
}

‎tests/TestCase/Queue/ProcessorTest.php‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
use Cake\Event\EventList;
2020
use Cake\Log\Engine\ArrayLog;
21+
use Cake\Log\Log;
2122
use Cake\Queue\Job\Message;
2223
use Cake\Queue\Queue\Processor;
2324
use Cake\TestSuite\TestCase;
@@ -84,11 +85,16 @@ public function testProcess($jobMethod, $expected, $logMessage, $dispatchedEvent
8485
$this->assertSame('Processor.message.seen', $events[0]->getName());
8586
$this->assertEquals(['queueMessage' => $queueMessage], $events[0]->getData());
8687

88+
// Events should contain a message with the same payload.
8789
$this->assertSame('Processor.message.start', $events[1]->getName());
88-
$this->assertEquals(['message' => $message], $events[1]->getData());
90+
$data = $events[1]->getData();
91+
$this->assertArrayHasKey('message', $data);
92+
$this->assertSame($message->jsonSerialize(), $data['message']->jsonSerialize());
8993

9094
$this->assertSame($dispatchedEvent, $events[2]->getName());
91-
$this->assertEquals(['message' => $message], $events[2]->getData());
95+
$data = $events[2]->getData();
96+
$this->assertArrayHasKey('message', $data);
97+
$this->assertSame($message->jsonSerialize(), $data['message']->jsonSerialize());
9298
}
9399

94100
/**
@@ -163,6 +169,39 @@ public function testProcessWillRequeueOnException()
163169
$this->assertSame($expected, $actual);
164170
}
165171

172+
/**
173+
* Test processJobMessage method.
174+
*
175+
* @return void
176+
*/
177+
public function testProcessJobObject()
178+
{
179+
Log::setConfig('debug', [
180+
'className' => 'Array',
181+
'levels' => ['notice', 'info', 'debug'],
182+
]);
183+
184+
$messageBody = [
185+
'queue' => 'default',
186+
'class' => ['TestApp\WelcomeMailer', 'welcome'],
187+
'args' => [],
188+
];
189+
$connectionFactory = new NullConnectionFactory();
190+
$context = $connectionFactory->createContext();
191+
$queueMessage = new NullMessage(json_encode($messageBody));
192+
$processor = new Processor();
193+
194+
$actual = $processor->process($queueMessage, $context);
195+
$logs = Log::engine('debug')->read();
196+
Log::drop('debug');
197+
198+
$this->assertCount(1, $logs);
199+
$this->assertStringContainsString('Welcome mail sent', $logs[0]);
200+
201+
$expected = InteropProcessor::ACK;
202+
$this->assertSame($expected, $actual);
203+
}
204+
166205
/**
167206
* Test processMessage method.
168207
*
@@ -190,7 +229,7 @@ public function testProcessMessage()
190229
/**
191230
* Job to be used in test testProcessMessageCallableIsString
192231
*
193-
* @param Message $message The message to process
232+
* @param \Cake\Queue\Job\Message $message The message to process
194233
* @return null
195234
*/
196235
public static function processReturnNull(Message $message)

0 commit comments

Comments
 (0)