Skip to content
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.

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

[MessageQueue]: add support for first class queue configuration. #30111

Open
wants to merge 1 commit into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions app/code/Magento/Ui/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@
<item name="array" xsi:type="object">arrayArgumentInterpreterProxy</item>
<item name="boolean" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Boolean</item>
<item name="number" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Number</item>
<item name="int" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Integer</item>
Copy link
Member

@damienwebdev damienwebdev Sep 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't precisely know the full scope of how why this interpreter injection seems to occur in various hierarchies, but potentially this isn't in the scope of this PR?

cc: @paliarush

<item name="string" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\StringUtils</item>
<item name="null" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\NullType</item>
<item name="url" xsi:type="object">Magento\Ui\Config\Argument\Parser\Url</item>
Expand Down
1 change: 1 addition & 0 deletions app/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@
<item name="array" xsi:type="object">layoutArrayArgumentReaderInterpreterProxy</item>
<item name="boolean" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Boolean</item>
<item name="number" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Number</item>
<item name="int" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\Integer</item>
Copy link
Member

@damienwebdev damienwebdev Sep 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't precisely know the full scope of how why this interpreter injection seems to occur in various hierarchies, but potentially this isn't in the scope of this PR?

cc: @paliarush

<item name="string" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\StringUtils</item>
<item name="null" xsi:type="object">Magento\Framework\Data\Argument\Interpreter\NullType</item>
<item name="object" xsi:type="object">Magento\Framework\View\Layout\Argument\Interpreter\Passthrough</item>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ protected function createArgumentInterpreter(
'boolean' => new \Magento\Framework\Data\Argument\Interpreter\Boolean($booleanUtils),
'string' => new \Magento\Framework\Data\Argument\Interpreter\BaseStringUtils($booleanUtils),
'number' => new \Magento\Framework\Data\Argument\Interpreter\Number(),
'int' => new \Magento\Framework\Data\Argument\Interpreter\Integer(),
'null' => new \Magento\Framework\Data\Argument\Interpreter\NullType(),
'object' => new \Magento\Framework\Data\Argument\Interpreter\DataObject($booleanUtils),
'const' => $constInterpreter,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Framework\Data\Argument\Interpreter;

use Magento\Framework\Data\Argument\InterpreterInterface;

/**
* Interpreter of numeric data, such as integer or float
*/
class Integer implements InterpreterInterface
{
/**
* {@inheritdoc}
* @return int|float
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're returning an int, the return value should just be int right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I thought this was new stuff, but clearly confused 😄

* @throws \InvalidArgumentException
*/
public function evaluate(array $data)
{
if (!isset($data['value']) || !is_numeric($data['value'])) {
throw new \InvalidArgumentException('Numeric value is expected.');
}
$result = $data['value'];
return (int)$result;
}
}
6 changes: 6 additions & 0 deletions lib/internal/Magento/Framework/Data/etc/argument/types.xsd
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@
</xs:complexContent>
</xs:complexType>

<xs:complexType name="int">
<xs:complexContent>
<xs:extension base="argumentType"/>
</xs:complexContent>
</xs:complexType>

<xs:complexType name="null">
<xs:complexContent>
<xs:restriction base="argumentType"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,20 @@ public function __construct(
) {
parent::__construct($reader, $cache, $cacheId, $serializer);
}

public function getQueues(): array {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ->get() loses backward compatibility (it returns both exchanges and queues when it used to only return exchanges), how should we consider the compatibility of this API?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return array_filter($this->get(),function($item){
if($item['type'] == 'queue'){
return $item;
}
});
}

public function getExchanges(): array {
return array_filter($this->get(),function($item){
if($item['type'] != 'queue'){
return $item;
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

use Magento\Framework\MessageQueue\Topology\Config\ExchangeConfigItem\BindingInterface;
use Magento\Framework\MessageQueue\Topology\Config\ExchangeConfigItem\Binding\IteratorFactory;
use Magento\Framework\MessageQueue\Topology\Config\ExchangeConfigItem\Binding\Iterator;
use Magento\Framework\MessageQueue\Topology\Config\ExchangeConfigItem\BindingFactory;

/**
* {@inheritdoc}
Expand Down Expand Up @@ -37,7 +39,7 @@ class ExchangeConfigItem implements ExchangeConfigItemInterface
/**
* Exchange bindings.
*
* @var BindingInterface[]
* @var Iterator
*/
private $bindings;

Expand Down Expand Up @@ -69,6 +71,11 @@ class ExchangeConfigItem implements ExchangeConfigItemInterface
*/
private $isInternal;

/**
* @var BindingFactory
*/
public $bindingFactory;

/**
* Initialize dependencies.
*
Expand Down Expand Up @@ -158,6 +165,11 @@ public function setData(array $data)
$this->isDurable = $data['durable'];
$this->isAutoDelete = $data['autoDelete'];
$this->arguments = $data['arguments'];
$this->bindings->setData($data['bindings']);
if( $this->type != 'queue'){
$this->bindings->setData($data['bindings']);
}
else{
$this->bindings->setData([]);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Iterator implements \Iterator, \ArrayAccess
*/
public function __construct(Data $configData, ExchangeConfigItemFactory $itemFactory)
{
$this->data = $configData->get();
$this->data = $configData->getExchanges();
$this->object = $itemFactory->create();
$this->rewind();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\MessageQueue\Topology\Config\QueueConfigItem;

use Magento\Framework\MessageQueue\Topology\Config\Data;
Expand Down Expand Up @@ -64,16 +66,22 @@ public function getMappedData()
{
if (null === $this->mappedData) {
$this->mappedData = [];
foreach ($this->configData->get() as $exchange) {

$queues = $this->createQueueItemsFromQueues($this->configData->getQueues());
foreach ($queues as $key => $value) {
$this->mappedData[$key] = $value;
}
foreach ($this->configData->getExchanges() as $exchange) {
$connection = $exchange['connection'];
foreach ($exchange['bindings'] as $binding) {
if ($binding['destinationType'] === 'queue') {
if ($binding['destinationType'] === 'queue' && !array_key_exists($binding['destination'] . '--' . $connection, $this->mappedData)) {
$queueItems = $this->createQueueItems($binding['destination'], $binding['topic'], $connection);
$this->mappedData = array_merge($this->mappedData, $queueItems);
}
}
}
}

return $this->mappedData;
}

Expand All @@ -83,13 +91,54 @@ public function getMappedData()
* @param string $name
* @param string $topic
* @param string $connection
* @deprecated
* @return array
*/
private function createQueueItems($name, $topic, $connection)
{
return $this->createQueueItemsFromTopic($name, $topic, $connection);
}

private function createQueueItemsFromQueues(array $queues)
{
$output = [];
$synchronousTopics = [];
foreach ($queues as $queue) {
$output[$queue['name'] . '--' . $queue['connection']] =
$this->createQueue(
$queue['name'],
$queue['connection'],
$queue['arguments'],
$queue['durable'],
$queue['autoDelete']
);
}
return $output;
}

private function createQueue($name, $connection, $arguments = [], $durable = true, $autoDelete = false)
{
return [
'name' => $name,
'connection' => $connection,
'durable' => isset($durable) ? $durable : true,
'autoDelete' => isset($autoDelete) ? $autoDelete : false,
'arguments' => isset($arguments) ? $arguments : [],
];
}

/**
* Create queue config item.
*
* @param string $name
* @param string $topic
* @param string $connection
* @return array
*/
private function createQueueItemsFromTopic($name, $topic, $connection)
{
$output = [];
$synchronousTopics = [];

if (strpos($topic, '*') !== false || strpos($topic, '#') !== false) {
$synchronousTopics = $this->matchSynchronousTopics($topic);
} elseif ($this->isSynchronousTopic($topic)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\MessageQueue\Topology\Config\Validator;

use Magento\Framework\MessageQueue\Topology\Config\ValidatorInterface;
Expand All @@ -19,9 +21,11 @@ public function validate($configData)
{
$errors = [];
foreach ($configData as $name => $data) {
foreach ((array)$data['bindings'] as $binding) {
if (isset($data['type']) && $data['type'] == 'topic' && !isset($binding['topic'])) {
$errors[] = 'Topic name is required for topic based exchange: ' . $name;
if ($data['type'] != 'queue') {
foreach ((array)$data['bindings'] as $binding) {
if (isset($data['type']) && $data['type'] == 'topic' && !isset($binding['topic'])) {
$errors[] = 'Topic name is required for topic based exchange: ' . $name;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class FieldsTypes implements ValidatorInterface
public function validate($configData)
{
foreach ($configData as $exchangeName => $exchangeConfig) {
$this->validateFieldsTypes($exchangeName, $exchangeConfig);
$exchangeConfig['type'] != 'queue' ?
$this->validateFieldsTypes($exchangeName, $exchangeConfig) : $this->validateQueueTypes($exchangeName, $exchangeConfig);
}
}

Expand Down Expand Up @@ -128,4 +129,16 @@ private function validateBindings($exchangeName, $exchangeConfig, $bindingFields
}
}
}

/**
* Make sure types of all fields in the queue item config are correct.
*
* @param string $queueName
* @param array $queueConfig
* @return void
* @throws \LogicException
*/
private function validateQueueTypes($queueName, $queueConfig)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What validation should be provided here?

}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
<?php

/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\MessageQueue\Topology\Config\Validator;

use Magento\Framework\MessageQueue\Topology\Config\ValidatorInterface;
Expand All @@ -17,24 +19,34 @@ class Format implements ValidatorInterface
*/
public function validate($configData)
{
$requiredFields = ['name', 'type', 'connection', 'durable', 'autoDelete', 'internal', 'bindings', 'arguments'];
$requiredExchangeFields = ['name', 'type', 'connection', 'durable', 'autoDelete', 'internal', 'bindings', 'arguments'];
$requiredQueueFields = ['name', 'type', 'connection', 'durable', 'autoDelete', 'internal', 'arguments'];

$requiredBindingFields = ['id', 'destinationType', 'destination', 'disabled', 'topic', 'arguments'];
$errors = [];
foreach ($configData as $name => $data) {
$diff = array_diff($requiredFields, array_keys($data));
foreach ($diff as $field) {
$errors[] = sprintf('Missing [%s] field for exchange %s.', $field, $name);
}
if ($data['type'] != 'queue') {
$diff = array_diff($requiredExchangeFields, array_keys($data));
foreach ($diff as $field) {
$errors[] = sprintf('Missing [%s] field for exchange %s.', $field, $name);
}

if (!array_key_exists('bindings', $data) || !is_array($data['bindings'])) {
$errors[] = sprintf('Invalid bindings format for exchange %s.', $name);
continue;
}
if (!array_key_exists('bindings', $data) || !is_array($data['bindings'])) {
$errors[] = sprintf('Invalid bindings format for exchange %s.', $name);
continue;
}

foreach ($data['bindings'] as $bindingConfig) {
$diff = array_diff($requiredBindingFields, array_keys($bindingConfig));
foreach ($data['bindings'] as $bindingConfig) {
$diff = array_diff($requiredBindingFields, array_keys($bindingConfig));
foreach ($diff as $field) {
$errors[] = sprintf('Missing [%s] field for binding %s in exchange config.', $field, $name);
}
}
}
else{
$diff = array_diff($requiredQueueFields, array_keys($data));
foreach ($diff as $field) {
$errors[] = sprintf('Missing [%s] field for binding %s in exchange config.', $field, $name);
$errors[] = sprintf('Missing [%s] field for queue %s.', $field, $name);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,41 @@ public function convert($source)
'arguments' => $exchangeArguments,
];
}

foreach ($source->getElementsByTagName('queue') as $queue) {
$name = $this->getAttributeValue($queue, 'name');
$connection = $this->getAttributeValue($queue, 'connection');

$bindings = [];
$queueArguments = [];
/** @var \DOMNode $node */
foreach ($queue->childNodes as $node) {
if (!in_array($node->nodeName, ['binding', 'arguments']) || $node->nodeType != XML_ELEMENT_NODE) {
continue;
}
switch ($node->nodeName) {
case 'binding':
$bindings = $this->processBindings($node, $bindings);
break;

case 'arguments':
$queueArguments = $this->processArguments($node);
break;
}
}

$autoDelete = $this->getAttributeValue($queue, 'autoDelete', false);
$result[$name . '--' . $connection] = [
'name' => $name,
'type' => $this->getAttributeValue($queue, 'type'),
'connection' => $connection,
'durable' => $this->booleanUtils->toBoolean($this->getAttributeValue($queue, 'durable', true)),
'autoDelete' => $this->booleanUtils->toBoolean($autoDelete),
'internal' => $this->booleanUtils->toBoolean($this->getAttributeValue($queue, 'internal', false)),
'arguments' => $queueArguments,
];
}

return $result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ class Reader extends \Magento\Framework\Config\Reader\Filesystem implements Read
'/config/exchange/binding' => 'id',
'/config/exchange/binding/arguments/argument' => 'name',
'/config/exchange/binding/arguments/argument(/item)+' => 'name',
'/config/queue' => ['name', 'connection'],
'/config/queue/arguments/argument' => 'name',
'/config/queue/arguments/argument(/item)+' => 'name'
];

/**
Expand Down
Loading