Skip to content

Deny Envelope::fromData() usage #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
vjik wants to merge 1 commit into from
Closed

Deny Envelope::fromData() usage #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions src/Message/DelayEnvelope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ public function __construct(MessageInterface $message, float $delaySeconds)
parent::__construct($message, [self::META_DELAY_SECONDS => $delaySeconds]);
}

public static function fromMessage(MessageInterface $message): static
{
/** @var float|int|string $delaySeconds */
$delaySeconds = $message->getMetadata()[self::META_DELAY_SECONDS] ?? 0.0;
return new self($message, (float) $delaySeconds);
}

public function getDelaySeconds(): float
{
return $this->metadata[self::META_DELAY_SECONDS];
Expand Down
8 changes: 5 additions & 3 deletions src/Message/Envelope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace Yiisoft\Queue\Message;

use LogicException;

abstract class Envelope implements MessageInterface
{
/**
Expand All @@ -25,11 +27,11 @@ public function __construct(MessageInterface $message, array $metadata)

final public static function fromData(string $type, mixed $data, array $metadata = []): static
{
return static::fromMessage(Message::fromData($type, $data, $metadata));
throw new LogicException(
'Envelopes cannot be created via "fromData()". Wrap an existing "MessageInterface" instance instead.',
);
}

abstract public static function fromMessage(MessageInterface $message): static;

final public function getMessage(): MessageInterface
{
return $this->message;
Expand Down
19 changes: 0 additions & 19 deletions src/Message/IdEnvelope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

namespace Yiisoft\Queue\Message;

use function is_int;
use function is_object;
use function is_string;

/**
* ID envelope allows to identify a message.
*/
Expand All @@ -20,21 +16,6 @@ public function __construct(MessageInterface $message, string|int|null $id)
parent::__construct($message, [self::MESSAGE_ID_KEY => $id]);
}

public static function fromMessage(MessageInterface $message): static
{
$rawId = $message->getMetadata()[self::MESSAGE_ID_KEY] ?? null;

$id = match (true) {
$rawId === null => null, // don't remove this branch: it's important for compute speed
is_string($rawId),
is_int($rawId) => $rawId,
is_object($rawId) && method_exists($rawId, '__toString') => (string) $rawId,
default => null,
};

return new self($message, $id);
}

public function getId(): string|int|null
{
return $this->metadata[self::MESSAGE_ID_KEY];
Expand Down
8 changes: 0 additions & 8 deletions src/Middleware/FailureHandling/FailureEnvelope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,4 @@ public function __construct(MessageInterface $message, array $metadata = [])
self::FAILURE_META_KEY => ArrayHelper::merge($message->getMetadata()[self::FAILURE_META_KEY] ?? [], $metadata),
]);
}

public static function fromMessage(MessageInterface $message): static
{
/** @var array $metadata */
$metadata = $message->getMetadata()[self::FAILURE_META_KEY] ?? [];

return new self($message, $metadata);
}
}
5 changes: 0 additions & 5 deletions tests/App/DummyEnvelope.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,4 @@ public function __construct(MessageInterface $message)
{
parent::__construct($message, []);
}

public static function fromMessage(MessageInterface $message): static
{
return new self($message);
}
}
33 changes: 0 additions & 33 deletions tests/Benchmark/MetadataBench.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,39 +31,6 @@ public function benchEnvelopeRead(): void
$id = $message->getId();
}

/**
* Create metadata as an array and read its value from an envelope object.
*/
public function benchEnvelopeReadRestored(): void
{
$message = IdEnvelope::fromMessage(new Message('foo', 'bar', ['id' => 1]));
$id = $message->getId();
}

public function provideEnvelopeStack(): Generator
{
$config = [1 => 'one', 5 => 'five', 15 => 'fifteen'];
$message = new IdEnvelope(new Message('foo', 'bar'), 1);

for ($i = 1; $i <= max(...array_keys($config)); $i++) {
if (isset($config[$i])) {
yield $config[$i] => ['message' => $message];
}
$message = new FailureEnvelope($message, ["fail$i" => "fail$i"]);
}
}

/**
* Read metadata value from an envelope object restored from an envelope stacks of different depth
*
* @psalm-param array{message: MessageInterface} $params
*/
#[ParamProviders('provideEnvelopeStack')]
public function benchEnvelopeReadFromStack(array $params): void
{
$id = IdEnvelope::fromMessage($params['message'])->getId();
}

public function provideEnvelopeStackCounts(): Generator
{
yield 'one' => [1];
Expand Down
18 changes: 0 additions & 18 deletions tests/Unit/Message/DelayEnvelopeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,4 @@ public function testDelayEnvelope(): void
self::assertArrayHasKey(DelayEnvelope::META_DELAY_SECONDS, $metadata);
self::assertSame(300.5, $metadata[DelayEnvelope::META_DELAY_SECONDS]);
}

public function testFromMessage(): void
{
$message = new Message('test', ['data' => 'value'], [DelayEnvelope::META_DELAY_SECONDS => 150]);
$delayEnvelope = DelayEnvelope::fromMessage($message);

self::assertSame(150.0, $delayEnvelope->getDelaySeconds());
self::assertSame('test', $delayEnvelope->getType());
self::assertSame(['data' => 'value'], $delayEnvelope->getData());
}

public function testFromMessageWithoutDelay(): void
{
$message = new Message('test', ['data' => 'value']);
$delayEnvelope = DelayEnvelope::fromMessage($message);

self::assertSame(0.0, $delayEnvelope->getDelaySeconds());
}
}
26 changes: 0 additions & 26 deletions tests/Unit/Message/EnvelopeTest.php
View file
Open in desktop

This file was deleted.

68 changes: 0 additions & 68 deletions tests/Unit/Message/IdEnvelopeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,58 +22,6 @@ public function testConstructor(): void
$this->assertSame($id, $envelope->getId());
}

public function testFromMessageWithStringId(): void
{
$id = 'test-id';
$message = $this->createMessage([IdEnvelope::MESSAGE_ID_KEY => $id]);

$envelope = IdEnvelope::fromMessage($message);

$this->assertSame($id, $envelope->getId());
}

public function testFromMessageWithIntId(): void
{
$id = 123;
$message = $this->createMessage([IdEnvelope::MESSAGE_ID_KEY => $id]);

$envelope = IdEnvelope::fromMessage($message);

$this->assertSame($id, $envelope->getId());
}

public function testFromMessageWithNullId(): void
{
$message = $this->createMessage();

$envelope = IdEnvelope::fromMessage($message);

$this->assertNull($envelope->getId());
}

public function testFromMessageWithObjectHavingToString(): void
{
$stringableObject = new class {
public function __toString(): string
{
return 'object-id';
}
};
$message = $this->createMessage([IdEnvelope::MESSAGE_ID_KEY => $stringableObject]);
$envelope = IdEnvelope::fromMessage($message);

$this->assertSame('object-id', $envelope->getId());
}

public function testFromMessageWithInvalidIdType(): void
{
$invalidId = ['array-cannot-be-id'];
$message = $this->createMessage([IdEnvelope::MESSAGE_ID_KEY => $invalidId]);
$message = IdEnvelope::fromMessage($message);

$this->assertNull($message->getId());
}

public function testGetEnvelopeMetadata(): void
{
$id = 'test-id';
Expand All @@ -86,22 +34,6 @@ public function testGetEnvelopeMetadata(): void
$this->assertSame($id, $metadata[IdEnvelope::MESSAGE_ID_KEY]);
}

public function testFromData(): void
{
$type = 'test-handler';
$data = ['key' => 'value'];
$metadata = ['meta' => 'data', IdEnvelope::MESSAGE_ID_KEY => 'test-id'];

$envelope = IdEnvelope::fromData($type, $data, $metadata);

$this->assertInstanceOf(IdEnvelope::class, $envelope);
$this->assertSame($type, $envelope->getType());
$this->assertSame($data, $envelope->getData());
$this->assertArrayHasKey('meta', $envelope->getMetadata());
$this->assertSame('data', $envelope->getMetadata()['meta']);
$this->assertSame('test-id', $envelope->getId());
}

private function createMessage(array $metadata = []): MessageInterface
{
return new Message('test-handler', ['test-data'], $metadata);
Expand Down
39 changes: 0 additions & 39 deletions tests/Unit/Middleware/FailureHandling/FailureEnvelopeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,6 @@ public function testConstructor(): void
$this->assertSame($metadata, $envelope->getMetadata()[FailureEnvelope::FAILURE_META_KEY]);
}

public function testFromMessageWithExistingMetadata(): void
{
$existingMetadata = ['attempt' => 1];
$message = $this->createMessage([FailureEnvelope::FAILURE_META_KEY => $existingMetadata]);

$envelope = FailureEnvelope::fromMessage($message);

$this->assertSame($existingMetadata, $envelope->getMetadata()[FailureEnvelope::FAILURE_META_KEY]);
}

public function testFromMessageWithoutMetadata(): void
{
$message = $this->createMessage();

$envelope = FailureEnvelope::fromMessage($message);

$this->assertArrayHasKey(FailureEnvelope::FAILURE_META_KEY, $envelope->getMetadata());
$this->assertSame([], $envelope->getMetadata()[FailureEnvelope::FAILURE_META_KEY]);
}

public function testMetadataMerging(): void
{
$existingMetadata = ['attempt' => 1, 'firstError' => 'First error'];
Expand All @@ -57,25 +37,6 @@ public function testMetadataMerging(): void
$this->assertSame('Last error', $mergedMetadata['lastError']);
}

public function testFromData(): void
{
$type = 'test-handler';
$data = ['key' => 'value'];
$metadata = [
'meta' => 'data',
FailureEnvelope::FAILURE_META_KEY => ['attempt' => 1],
];

$envelope = FailureEnvelope::fromData($type, $data, $metadata);

$this->assertInstanceOf(FailureEnvelope::class, $envelope);
$this->assertSame($type, $envelope->getType());
$this->assertSame($data, $envelope->getData());
$this->assertArrayHasKey('meta', $envelope->getMetadata());
$this->assertSame('data', $envelope->getMetadata()['meta']);
$this->assertSame(['attempt' => 1], $envelope->getMetadata()[FailureEnvelope::FAILURE_META_KEY]);
}

private function createMessage(array $metadata = []): MessageInterface
{
return new Message('test-handler', ['test-data'], $metadata);
Expand Down
Loading