From a6f1a0f7c5977ed9c2aef7ba43199eb7a9577a49 Mon Sep 17 00:00:00 2001 From: Oleh Blynnykov Date: Wed, 15 Apr 2020 19:13:41 +0300 Subject: [PATCH 01/10] Added optional description field to the tag in webapi.xml --- .../Model/ConfigInterface.php | 11 +++++ .../Model/MassSchedule.php | 6 ++- .../Magento/Webapi/Model/Config/Converter.php | 6 +++ app/code/Magento/Webapi/etc/webapi_base.xsd | 1 + .../Rest/AsynchronousRequestProcessor.php | 21 ++++++++- app/code/Magento/WebapiAsync/Model/Config.php | 28 +++++++++-- .../Model/MassScheduleTest.php | 47 ++++++++++++++++++- .../AsynchronousOperations/_files/webapiA.xml | 16 +++++++ 8 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 dev/tests/integration/testsuite/Magento/AsynchronousOperations/_files/webapiA.xml diff --git a/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php b/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php index de0f89a71650a..320255b008ece 100644 --- a/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php +++ b/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php @@ -29,6 +29,7 @@ interface ConfigInterface const SERVICE_PARAM_KEY_INTERFACE = 'interface'; const SERVICE_PARAM_KEY_METHOD = 'method'; const SERVICE_PARAM_KEY_TOPIC = 'topic'; + const SERVICE_PARAM_KEY_DESCRIPTION = 'description'; const DEFAULT_HANDLER_NAME = 'async'; const SYSTEM_TOPIC_NAME = 'async.system.required.wrapper.topic'; const SYSTEM_TOPIC_CONFIGURATION = [ @@ -57,4 +58,14 @@ public function getServices(); * @throws \Magento\Framework\Exception\LocalizedException */ public function getTopicName($routeUrl, $httpMethod); + + /** + * Get topic description from webapi_async_config services config array by route url and http method + * + * @param string $routeUrl + * @param string $httpMethod GET|POST|PUT|DELETE + * @return string + * @throws \Magento\Framework\Exception\LocalizedException + */ + public function getTopicDescription($routeUrl, $httpMethod); } diff --git a/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php b/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php index 4dcaf7279a570..f35506453f6b2 100644 --- a/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php +++ b/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php @@ -113,15 +113,19 @@ public function __construct( * * @param string $topicName * @param array $entitiesArray + * @param string $topicDescription * @param string $groupId * @param string $userId * @return AsyncResponseInterface * @throws BulkException * @throws LocalizedException */ - public function publishMass($topicName, array $entitiesArray, $groupId = null, $userId = null) + public function publishMass($topicName, array $entitiesArray, $topicDescription = '', $groupId = null, $userId = null) { $bulkDescription = __('Topic %1', $topicName); + if ($topicDescription !== '') { + $bulkDescription = $topicDescription; + } if ($userId == null) { $userId = $this->userContext->getUserId(); diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 837a0f84423ad..3c4cda0c25492 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -55,6 +55,11 @@ public function convert($source) $soapMethod = trim($soapOperationNode->nodeValue); } $url = trim($route->attributes->getNamedItem('url')->nodeValue); + + $description = ''; + if ($descriptionNode = $route->attributes->getNamedItem('description')) { + $description = trim($descriptionNode->nodeValue); + } $version = $this->convertVersion($url); $serviceClassData = []; @@ -104,6 +109,7 @@ public function convert($source) ], self::KEY_ACL_RESOURCES => $resourceReferences, self::KEY_DATA_PARAMETERS => $data, + self::KEY_DESCRIPTION => $description ]; $serviceSecure = false; diff --git a/app/code/Magento/Webapi/etc/webapi_base.xsd b/app/code/Magento/Webapi/etc/webapi_base.xsd index 7d1a5a14ba78f..82844355e65fc 100644 --- a/app/code/Magento/Webapi/etc/webapi_base.xsd +++ b/app/code/Magento/Webapi/etc/webapi_base.xsd @@ -30,6 +30,7 @@ + diff --git a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php index 81235e335b8fa..8f933c330ed7e 100644 --- a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php +++ b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php @@ -99,11 +99,13 @@ public function process(\Magento\Framework\Webapi\Rest\Request $request) $entitiesParamsArray = $this->inputParamsResolver->resolve(); $topicName = $this->getTopicName($request); + $topicDescription = $this->getTopicDescription($request); try { $asyncResponse = $this->asyncBulkPublisher->publishMass( $topicName, - $entitiesParamsArray + $entitiesParamsArray, + $topicDescription ); } catch (BulkException $bulkException) { $asyncResponse = $bulkException->getData(); @@ -121,6 +123,8 @@ public function process(\Magento\Framework\Webapi\Rest\Request $request) /** * @param \Magento\Framework\Webapi\Rest\Request $request * @return string + * @throws \Magento\Framework\Exception\InputException + * @throws \Magento\Framework\Exception\LocalizedException */ private function getTopicName($request) { @@ -132,6 +136,21 @@ private function getTopicName($request) ); } + /** + * @param \Magento\Framework\Webapi\Rest\Request $request + * @return string + * @throws \Magento\Framework\Exception\LocalizedException + */ + private function getTopicDescription($request) + { + $route = $this->inputParamsResolver->getRoute(); + + return $this->webapiAsyncConfig->getTopicDescription( + $route->getRoutePath(), + $request->getHttpMethod() + ); + } + /** * {@inheritdoc} */ diff --git a/app/code/Magento/WebapiAsync/Model/Config.php b/app/code/Magento/WebapiAsync/Model/Config.php index 7329862ca528c..9a6ebbfdf9828 100644 --- a/app/code/Magento/WebapiAsync/Model/Config.php +++ b/app/code/Magento/WebapiAsync/Model/Config.php @@ -94,6 +94,26 @@ public function getTopicName($routeUrl, $httpMethod) return $services[$lookupKey][self::SERVICE_PARAM_KEY_TOPIC]; } + /** + * @inheritdoc + */ + public function getTopicDescription($routeUrl, $httpMethod) + { + $services = $this->getServices(); + $lookupKey = $this->generateLookupKeyByRouteData( + $routeUrl, + $httpMethod + ); + + if (array_key_exists($lookupKey, $services) === false) { + throw new LocalizedException( + __('WebapiAsync config for "%lookupKey" does not exist.', ['lookupKey' => $lookupKey]) + ); + } + + return $services[$lookupKey][self::SERVICE_PARAM_KEY_DESCRIPTION]; + } + /** * Generate topic data for all defined services * @@ -110,6 +130,7 @@ private function generateTopicsDataFromWebapiConfig() if ($httpMethod !== \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_GET) { $serviceInterface = $httpMethodData[Converter::KEY_SERVICE][Converter::KEY_SERVICE_CLASS]; $serviceMethod = $httpMethodData[Converter::KEY_SERVICE][Converter::KEY_SERVICE_METHOD]; + $topicDescription = $httpMethodData[Converter::KEY_DESCRIPTION]; $lookupKey = $this->generateLookupKeyByRouteData( $routeUrl, @@ -123,9 +144,10 @@ private function generateTopicsDataFromWebapiConfig() ); $services[$lookupKey] = [ - self::SERVICE_PARAM_KEY_INTERFACE => $serviceInterface, - self::SERVICE_PARAM_KEY_METHOD => $serviceMethod, - self::SERVICE_PARAM_KEY_TOPIC => $topicName, + self::SERVICE_PARAM_KEY_INTERFACE => $serviceInterface, + self::SERVICE_PARAM_KEY_METHOD => $serviceMethod, + self::SERVICE_PARAM_KEY_TOPIC => $topicName, + self::SERVICE_PARAM_KEY_DESCRIPTION => $topicDescription ]; } } diff --git a/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php index c0cc1763b2654..b425708686e1d 100644 --- a/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php +++ b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php @@ -23,6 +23,8 @@ use Magento\Catalog\Model\ResourceModel\Product\Collection; use Magento\Catalog\Api\ProductRepositoryInterface; use Magento\Framework\ObjectManagerInterface; +use Magento\AsynchronousOperations\Model\ResourceModel\Bulk\Collection as BulkCollection; +use Magento\Webapi\Model\Config\Reader as ConfigReader; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -69,8 +71,15 @@ class MassScheduleTest extends \PHPUnit\Framework\TestCase */ private $registry; + /** @var \PHPUnit_Framework_MockObject_MockObject */ + protected $fileResolverMock; + + /** @var ConfigReader */ + protected $configReader; + protected function setUp() { + $this->fileResolverMock = $this->createMock(\Magento\Framework\Config\FileResolverInterface::class); $this->objectManager = Bootstrap::getObjectManager(); $this->registry = $this->objectManager->get(Registry::class); $this->massSchedule = $this->objectManager->create(MassSchedule::class); @@ -83,6 +92,10 @@ protected function setUp() 'logFilePath' => $this->logFilePath, 'appInitParams' => \Magento\TestFramework\Helper\Bootstrap::getInstance()->getAppInitParams() ]); + $this->configReader = $this->objectManager->create( + \Magento\Webapi\Model\Config\Reader::class, + ['fileResolver' => $this->fileResolverMock] + ); try { $this->publisherConsumerController->initialize(); @@ -121,6 +134,8 @@ public function testScheduleMass($products) public function sendBulk($products) { $this->skus = []; + $expectedDescription = 'Save Products Test Description'; + $description = $this->readDescription(); foreach ($products as $data) { if (isset($data['product'])) { $this->skus[] = $data['product']->getSku(); @@ -130,7 +145,8 @@ public function sendBulk($products) $result = $this->massSchedule->publishMass( 'async.magento.catalog.api.productrepositoryinterface.save.post', - $products + $products, + $description ); //assert bulk accepted with no errors @@ -138,6 +154,13 @@ public function sendBulk($products) //assert number of products sent to queue $this->assertCount(count($this->skus), $result->getRequestItems()); + + //assert topic description + $this->assertEquals( + $expectedDescription, + $this->getDescription($result->getBulkUuid()), + 'Description is wrong' + ); } public function tearDown() @@ -189,6 +212,18 @@ public function assertProductExists($productsSkus, $count) return $size == $count; } + public function getDescription($uuid) + { + $bulkDescription = ''; + $collection = $this->objectManager->create(BulkCollection::class) + ->addFieldToFilter('uuid', ['in' => $uuid]) + ->load(); + if (!empty($collection->getFirstItem()->getData())) { + $bulkDescription = $collection->getFirstItem()->getDescription(); + } + return $bulkDescription; + } + /** * @dataProvider productExceptionDataProvider * @param ProductInterface[] $products @@ -300,4 +335,14 @@ public function productExceptionDataProvider() ], ]; } + + public function readDescription() + { + $configFiles = [ + file_get_contents(realpath(__DIR__ . '/../_files/webapiA.xml')) + ]; + $this->fileResolverMock->expects($this->any())->method('get')->will($this->returnValue($configFiles)); + $value = $this->configReader->read(); + return $value['routes']['/V1/products']['POST']['description']; + } } diff --git a/dev/tests/integration/testsuite/Magento/AsynchronousOperations/_files/webapiA.xml b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/_files/webapiA.xml new file mode 100644 index 0000000000000..6ec8bdae8809d --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/_files/webapiA.xml @@ -0,0 +1,16 @@ + + + + + + + + + + From 8396884febecf91698647e6545c18f47e31cf3c1 Mon Sep 17 00:00:00 2001 From: Oleh Blynnykov Date: Thu, 16 Apr 2020 11:39:50 +0300 Subject: [PATCH 02/10] Added optional description field to the tag in webapi.xml [adjusted tests] --- .../Magento/Webapi/Model/Config/_files/webapi.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php index 57c8fbf45c63c..232b92ac36700 100644 --- a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php +++ b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php @@ -138,6 +138,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmoduleMSC' => [ @@ -152,6 +153,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmodule1/:id' => [ @@ -166,6 +168,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmodule1' => [ @@ -184,6 +187,7 @@ 'value' => null, ], ], + 'description' => '' ], 'POST' => [ 'secure' => false, @@ -200,6 +204,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V2/testmodule1/:id' => [ @@ -215,6 +220,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -228,6 +234,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'PUT' => [ 'secure' => false, @@ -241,6 +248,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V2/testmodule1' => [ @@ -260,6 +268,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V2/testmoduleMSC/itemPreconfigured' => [ @@ -274,6 +283,7 @@ 'Magento_TestModuleMSC::resource2' => true, ], 'parameters' => [], + 'description' => '' ] ] ], From 93a6900d214528445d0069098a1b0c42e932175d Mon Sep 17 00:00:00 2001 From: Oleh Blynnykov Date: Thu, 16 Apr 2020 19:12:09 +0300 Subject: [PATCH 03/10] Static and Unit tests --- .../AsynchronousOperations/Model/MassSchedule.php | 9 +++++++-- .../Webapi/Test/Unit/Model/Config/_files/webapi.php | 6 ++++++ .../Controller/Rest/AsynchronousRequestProcessor.php | 10 ++++++++-- .../Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php | 3 ++- .../AsynchronousOperations/Model/MassScheduleTest.php | 7 ++++++- 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php b/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php index f35506453f6b2..6ebbd8f01e67d 100644 --- a/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php +++ b/app/code/Magento/AsynchronousOperations/Model/MassSchedule.php @@ -120,8 +120,13 @@ public function __construct( * @throws BulkException * @throws LocalizedException */ - public function publishMass($topicName, array $entitiesArray, $topicDescription = '', $groupId = null, $userId = null) - { + public function publishMass( + $topicName, + array $entitiesArray, + $topicDescription = '', + $groupId = null, + $userId = null + ) { $bulkDescription = __('Topic %1', $topicName); if ($topicDescription !== '') { $bulkDescription = $topicDescription; diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php index 49c794bf773b9..b2d1160884585 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php @@ -67,6 +67,7 @@ 'value' => '%customer_id%', ], ], + 'description' => '' ], ], '/V1/customers/me' => [ @@ -85,6 +86,7 @@ 'value' => null, ], ], + 'description' => '' ], 'PUT' => [ 'secure' => true, @@ -101,6 +103,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V1/customers' => [ @@ -115,6 +118,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/customers/:id' => [ @@ -129,6 +133,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -142,6 +147,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], ], diff --git a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php index 8f933c330ed7e..4be036bd64038 100644 --- a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php +++ b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php @@ -87,7 +87,7 @@ public function __construct( } /** - * {@inheritdoc} + * @inheritdoc */ public function process(\Magento\Framework\Webapi\Rest\Request $request) { @@ -121,6 +121,8 @@ public function process(\Magento\Framework\Webapi\Rest\Request $request) } /** + * Get topic name from request + * * @param \Magento\Framework\Webapi\Rest\Request $request * @return string * @throws \Magento\Framework\Exception\InputException @@ -137,6 +139,8 @@ private function getTopicName($request) } /** + * Get topic description from request + * * @param \Magento\Framework\Webapi\Rest\Request $request * @return string * @throws \Magento\Framework\Exception\LocalizedException @@ -152,7 +156,7 @@ private function getTopicDescription($request) } /** - * {@inheritdoc} + * @inheritdoc */ public function canProcess(\Magento\Framework\Webapi\Rest\Request $request) { @@ -167,6 +171,8 @@ public function canProcess(\Magento\Framework\Webapi\Rest\Request $request) } /** + * Check if path is bulk + * * @param \Magento\Framework\Webapi\Rest\Request $request * @return bool */ diff --git a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php index 47b75b2057316..8ad69b17214a8 100644 --- a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php @@ -63,7 +63,8 @@ public function testGetServicesSetsTopicFromServiceContractName() 'service' => [ 'class' => \Magento\Catalog\Api\ProductRepositoryInterface::class, 'method' => 'save', - ] + ], + 'description' => '' ] ] ] diff --git a/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php index b425708686e1d..4478fab4a3551 100644 --- a/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php +++ b/dev/tests/integration/testsuite/Magento/AsynchronousOperations/Model/MassScheduleTest.php @@ -71,12 +71,17 @@ class MassScheduleTest extends \PHPUnit\Framework\TestCase */ private $registry; - /** @var \PHPUnit_Framework_MockObject_MockObject */ + /** @var \PHPUnit\Framework\MockObject\MockObject */ protected $fileResolverMock; /** @var ConfigReader */ protected $configReader; + /** + * @var string + */ + protected $logFilePath; + protected function setUp() { $this->fileResolverMock = $this->createMock(\Magento\Framework\Config\FileResolverInterface::class); From f3cb88a56e7825cfc96912d84e87601a356fcef7 Mon Sep 17 00:00:00 2001 From: Oleh Blynnykov Date: Thu, 16 Apr 2020 20:08:15 +0300 Subject: [PATCH 04/10] Static tests --- app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php index 8ad69b17214a8..ab7003bd8d3eb 100644 --- a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php @@ -22,17 +22,17 @@ class ConfigTest extends \PHPUnit\Framework\TestCase private $config; /** - * @var Webapi|\PHPUnit_Framework_MockObject_MockObject + * @var Webapi|\PHPUnit\Framework\MockObject\MockObject */ private $webapiCacheMock; /** - * @var WebapiConfig|\PHPUnit_Framework_MockObject_MockObject + * @var WebapiConfig|\PHPUnit\Framework\MockObject\MockObject */ private $configMock; /** - * @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject + * @var SerializerInterface|\PHPUnit\Framework\MockObject\MockObject */ private $serializerMock; From 39d95f0ed5c60351067bc81a6b31c3fa2860d813 Mon Sep 17 00:00:00 2001 From: Oleh Blynnykov Date: Tue, 21 Apr 2020 19:05:10 +0300 Subject: [PATCH 05/10] Static tests --- .../Magento/Webapi/Model/Config/Converter.php | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 3c4cda0c25492..1bf0210237091 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -70,6 +70,8 @@ public function convert($source) $resources = $route->getElementsByTagName('resource'); $resourceReferences = []; $resourcePermissionSet = []; + $data = $this->convertMethodParameters($route->getElementsByTagName('parameter')); + $serviceData = $data; /** @var \DOMElement $resource */ foreach ($resources as $resource) { if ($resource->nodeType != XML_ELEMENT_NODE) { @@ -79,22 +81,13 @@ public function convert($source) $resourceReferences[$ref] = true; // For SOAP $resourcePermissionSet[] = $ref; + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES][] = $ref; } - $data = $this->convertMethodParameters($route->getElementsByTagName('parameter')); - $serviceData = $data; - if (!isset($serviceClassData[self::KEY_METHODS][$soapMethod])) { - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = $resourcePermissionSet; - } else { - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = - array_unique( - array_merge( - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES], - $resourcePermissionSet - ) - ); - $serviceData = []; - } + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = + array_unique( + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] + ); $method = $route->attributes->getNamedItem('method')->nodeValue; $secureNode = $route->attributes->getNamedItem('secure'); From 8ce76e6279db19ba60cb7527c07a0b8e751b8df9 Mon Sep 17 00:00:00 2001 From: Lyzun Oleksandr Date: Fri, 19 Mar 2021 11:18:56 +0100 Subject: [PATCH 06/10] Some code alignments --- .../Model/ConfigInterface.php | 6 +-- app/code/Magento/Catalog/etc/webapi.xml | 2 +- .../Magento/Webapi/Model/Config/Converter.php | 22 +++++---- .../Rest/AsynchronousRequestProcessor.php | 1 - app/code/Magento/WebapiAsync/Model/Config.php | 45 ++++++++++--------- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php b/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php index bf3dbbc69a08b..6e063fdb950db 100644 --- a/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php +++ b/app/code/Magento/AsynchronousOperations/Model/ConfigInterface.php @@ -49,7 +49,7 @@ interface ConfigInterface * @return array * @since 100.2.3 */ - public function getServices(); + public function getServices(): array; /** * Get topic name from webapi_async_config services config array by route url and http method @@ -60,7 +60,7 @@ public function getServices(); * @throws \Magento\Framework\Exception\LocalizedException * @since 100.2.3 */ - public function getTopicName($routeUrl, $httpMethod); + public function getTopicName(string $routeUrl, string $httpMethod): string; /** * Get topic description from webapi_async_config services config array by route url and http method @@ -70,5 +70,5 @@ public function getTopicName($routeUrl, $httpMethod); * @return string * @throws \Magento\Framework\Exception\LocalizedException */ - public function getTopicDescription($routeUrl, $httpMethod); + public function getTopicDescription(string $routeUrl, string $httpMethod): string; } diff --git a/app/code/Magento/Catalog/etc/webapi.xml b/app/code/Magento/Catalog/etc/webapi.xml index 5e799cd9f426d..a2e41cb3de6de 100644 --- a/app/code/Magento/Catalog/etc/webapi.xml +++ b/app/code/Magento/Catalog/etc/webapi.xml @@ -9,7 +9,7 @@ xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd"> - + diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 1bf0210237091..4d1010f0a2a2b 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -55,7 +55,6 @@ public function convert($source) $soapMethod = trim($soapOperationNode->nodeValue); } $url = trim($route->attributes->getNamedItem('url')->nodeValue); - $description = ''; if ($descriptionNode = $route->attributes->getNamedItem('description')) { $description = trim($descriptionNode->nodeValue); @@ -70,8 +69,6 @@ public function convert($source) $resources = $route->getElementsByTagName('resource'); $resourceReferences = []; $resourcePermissionSet = []; - $data = $this->convertMethodParameters($route->getElementsByTagName('parameter')); - $serviceData = $data; /** @var \DOMElement $resource */ foreach ($resources as $resource) { if ($resource->nodeType != XML_ELEMENT_NODE) { @@ -81,13 +78,22 @@ public function convert($source) $resourceReferences[$ref] = true; // For SOAP $resourcePermissionSet[] = $ref; - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES][] = $ref; } + $data = $this->convertMethodParameters($route->getElementsByTagName('parameter')); + $serviceData = $data; - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = - array_unique( - $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] - ); + if (!isset($serviceClassData[self::KEY_METHODS][$soapMethod])) { + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = $resourcePermissionSet; + } else { + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES] = + array_unique( + array_merge( + $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_ACL_RESOURCES], + $resourcePermissionSet + ) + ); + $serviceData = []; + } $method = $route->attributes->getNamedItem('method')->nodeValue; $secureNode = $route->attributes->getNamedItem('secure'); diff --git a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php index 4be036bd64038..20b83647ab979 100644 --- a/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php +++ b/app/code/Magento/WebapiAsync/Controller/Rest/AsynchronousRequestProcessor.php @@ -125,7 +125,6 @@ public function process(\Magento\Framework\Webapi\Rest\Request $request) * * @param \Magento\Framework\Webapi\Rest\Request $request * @return string - * @throws \Magento\Framework\Exception\InputException * @throws \Magento\Framework\Exception\LocalizedException */ private function getTopicName($request) diff --git a/app/code/Magento/WebapiAsync/Model/Config.php b/app/code/Magento/WebapiAsync/Model/Config.php index 9a6ebbfdf9828..7e9ea57d68fe8 100644 --- a/app/code/Magento/WebapiAsync/Model/Config.php +++ b/app/code/Magento/WebapiAsync/Model/Config.php @@ -59,7 +59,7 @@ public function __construct( /** * @inheritdoc */ - public function getServices() + public function getServices(): array { if (null === $this->asyncServices) { $services = $this->cache->load(self::CACHE_ID); @@ -77,27 +77,29 @@ public function getServices() /** * @inheritdoc */ - public function getTopicName($routeUrl, $httpMethod) + public function getTopicName(string $routeUrl, string $httpMethod): string { - $services = $this->getServices(); - $lookupKey = $this->generateLookupKeyByRouteData( - $routeUrl, - $httpMethod - ); - - if (array_key_exists($lookupKey, $services) === false) { - throw new LocalizedException( - __('WebapiAsync config for "%lookupKey" does not exist.', ['lookupKey' => $lookupKey]) - ); - } - - return $services[$lookupKey][self::SERVICE_PARAM_KEY_TOPIC]; + return $this->getServiceParameter($routeUrl, $httpMethod, self::SERVICE_PARAM_KEY_TOPIC); } /** * @inheritdoc */ - public function getTopicDescription($routeUrl, $httpMethod) + public function getTopicDescription(string $routeUrl, string $httpMethod): string + { + return $this->getServiceParameter($routeUrl, $httpMethod, self::SERVICE_PARAM_KEY_DESCRIPTION); + } + + /** + * Get service parameter by name + * + * @param string $routeUrl + * @param string $httpMethod + * @param string $attributeName + * @return string + * @throws LocalizedException + */ + private function getServiceParameter(string $routeUrl, string $httpMethod, string $attributeName): string { $services = $this->getServices(); $lookupKey = $this->generateLookupKeyByRouteData( @@ -111,9 +113,10 @@ public function getTopicDescription($routeUrl, $httpMethod) ); } - return $services[$lookupKey][self::SERVICE_PARAM_KEY_DESCRIPTION]; + return $services[$lookupKey][$attributeName]; } + /** * Generate topic data for all defined services * @@ -121,7 +124,7 @@ public function getTopicDescription($routeUrl, $httpMethod) * * @return array */ - private function generateTopicsDataFromWebapiConfig() + private function generateTopicsDataFromWebapiConfig(): array { $webApiConfig = $this->webApiConfig->getServices(); $services = []; @@ -166,7 +169,7 @@ private function generateTopicsDataFromWebapiConfig() * @param string $httpMethod * @return string */ - private function generateLookupKeyByRouteData($routeUrl, $httpMethod) + private function generateLookupKeyByRouteData(string $routeUrl, string $httpMethod): string { return self::TOPIC_PREFIX . $this->generateKey($routeUrl, $httpMethod, '/', false); } @@ -183,7 +186,7 @@ private function generateLookupKeyByRouteData($routeUrl, $httpMethod) * @param string $httpMethod * @return string */ - private function generateTopicNameFromService($serviceInterface, $serviceMethod, $httpMethod) + private function generateTopicNameFromService(string $serviceInterface, string $serviceMethod, string $httpMethod): string { $typeName = strtolower(sprintf('%s.%s', $serviceInterface, $serviceMethod)); return strtolower(self::TOPIC_PREFIX . $this->generateKey($typeName, $httpMethod, '\\', false)); @@ -198,7 +201,7 @@ private function generateTopicNameFromService($serviceInterface, $serviceMethod, * @param bool $lcfirst * @return string */ - private function generateKey($typeName, $methodName, $delimiter = '\\', $lcfirst = true) + private function generateKey(string $typeName, string $methodName, string $delimiter = '\\', bool $lcfirst = true): string { $parts = explode($delimiter, trim($typeName, $delimiter)); foreach ($parts as &$part) { From 40fd30cd5ca8c743d868b0d629aa504eba7acafd Mon Sep 17 00:00:00 2001 From: Lyzun Oleksandr Date: Fri, 19 Mar 2021 12:08:30 +0100 Subject: [PATCH 07/10] Align code, so Description is more a opt-out in Converter and not opt-in --- app/code/Magento/Webapi/Model/Config/Converter.php | 7 +++++-- .../Webapi/Test/Unit/Model/Config/_files/webapi.php | 6 ------ app/code/Magento/WebapiAsync/Model/Config.php | 7 +++++-- .../Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php | 3 +-- .../Magento/Webapi/Model/Config/_files/webapi.php | 10 ---------- 5 files changed, 11 insertions(+), 22 deletions(-) diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 4d1010f0a2a2b..43b2875b9e7ae 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -107,10 +107,13 @@ public function convert($source) self::KEY_SERVICE_METHOD => $serviceMethod, ], self::KEY_ACL_RESOURCES => $resourceReferences, - self::KEY_DATA_PARAMETERS => $data, - self::KEY_DESCRIPTION => $description + self::KEY_DATA_PARAMETERS => $data ]; + if ($description === '') { + $result[self::KEY_ROUTES][$url][$method][self::KEY_DESCRIPTION] = $description; + } + $serviceSecure = false; if (isset($serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_SECURE])) { $serviceSecure = $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_SECURE]; diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php index da468ee58e89d..bd8fcc78953a4 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php @@ -70,7 +70,6 @@ 'value' => '%customer_id%', ], ], - 'description' => '' ], ], '/V1/customers/me' => [ @@ -89,7 +88,6 @@ 'value' => null, ], ], - 'description' => '' ], 'PUT' => [ 'secure' => true, @@ -106,7 +104,6 @@ 'value' => null, ], ], - 'description' => '' ], ], '/V1/customers' => [ @@ -121,7 +118,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], '/V1/customers/:id' => [ @@ -136,7 +132,6 @@ ], 'parameters' => [ ], - 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -150,7 +145,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], ], diff --git a/app/code/Magento/WebapiAsync/Model/Config.php b/app/code/Magento/WebapiAsync/Model/Config.php index 7e9ea57d68fe8..b20fe3f69082d 100644 --- a/app/code/Magento/WebapiAsync/Model/Config.php +++ b/app/code/Magento/WebapiAsync/Model/Config.php @@ -116,7 +116,6 @@ private function getServiceParameter(string $routeUrl, string $httpMethod, strin return $services[$lookupKey][$attributeName]; } - /** * Generate topic data for all defined services * @@ -133,7 +132,11 @@ private function generateTopicsDataFromWebapiConfig(): array if ($httpMethod !== \Magento\Framework\Webapi\Rest\Request::HTTP_METHOD_GET) { $serviceInterface = $httpMethodData[Converter::KEY_SERVICE][Converter::KEY_SERVICE_CLASS]; $serviceMethod = $httpMethodData[Converter::KEY_SERVICE][Converter::KEY_SERVICE_METHOD]; - $topicDescription = $httpMethodData[Converter::KEY_DESCRIPTION]; + + $topicDescription = ''; + if (isset($httpMethodData[Converter::KEY_DESCRIPTION])) { + $topicDescription = $httpMethodData[Converter::KEY_DESCRIPTION]; + } $lookupKey = $this->generateLookupKeyByRouteData( $routeUrl, diff --git a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php index 007f7ee795e1d..df8367afe1775 100644 --- a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php @@ -67,8 +67,7 @@ public function testGetServicesSetsTopicFromServiceContractName() 'service' => [ 'class' => ProductRepositoryInterface::class, 'method' => 'save', - ], - 'description' => '' + ] ] ] ] diff --git a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php index 232b92ac36700..57c8fbf45c63c 100644 --- a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php +++ b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php @@ -138,7 +138,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], '/V1/testmoduleMSC' => [ @@ -153,7 +152,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], '/V1/testmodule1/:id' => [ @@ -168,7 +166,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], '/V1/testmodule1' => [ @@ -187,7 +184,6 @@ 'value' => null, ], ], - 'description' => '' ], 'POST' => [ 'secure' => false, @@ -204,7 +200,6 @@ 'value' => null, ], ], - 'description' => '' ], ], '/V2/testmodule1/:id' => [ @@ -220,7 +215,6 @@ ], 'parameters' => [ ], - 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -234,7 +228,6 @@ ], 'parameters' => [ ], - 'description' => '' ], 'PUT' => [ 'secure' => false, @@ -248,7 +241,6 @@ ], 'parameters' => [ ], - 'description' => '' ], ], '/V2/testmodule1' => [ @@ -268,7 +260,6 @@ 'value' => null, ], ], - 'description' => '' ], ], '/V2/testmoduleMSC/itemPreconfigured' => [ @@ -283,7 +274,6 @@ 'Magento_TestModuleMSC::resource2' => true, ], 'parameters' => [], - 'description' => '' ] ] ], From e1baf63f18a0c1d5cf0505042d80a5b0d4292077 Mon Sep 17 00:00:00 2001 From: Lyzun Oleksandr Date: Fri, 19 Mar 2021 13:51:58 +0100 Subject: [PATCH 08/10] Fix Unit tests --- app/code/Magento/Webapi/Model/Config/Converter.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 43b2875b9e7ae..033d21338d586 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -110,7 +110,7 @@ public function convert($source) self::KEY_DATA_PARAMETERS => $data ]; - if ($description === '') { + if ($description !== '') { $result[self::KEY_ROUTES][$url][$method][self::KEY_DESCRIPTION] = $description; } From cb5c8cbcd79260ab345df0feb50046077cf33bd1 Mon Sep 17 00:00:00 2001 From: Lyzun Oleksandr Date: Fri, 19 Mar 2021 16:39:00 +0100 Subject: [PATCH 09/10] Revert change in default webapi --- app/code/Magento/Catalog/etc/webapi.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/etc/webapi.xml b/app/code/Magento/Catalog/etc/webapi.xml index a2e41cb3de6de..5e799cd9f426d 100644 --- a/app/code/Magento/Catalog/etc/webapi.xml +++ b/app/code/Magento/Catalog/etc/webapi.xml @@ -9,7 +9,7 @@ xsi:noNamespaceSchemaLocation="urn:magento:module:Magento_Webapi:etc/webapi.xsd"> - + From e0a54e98d7a324503a223ef4b91217540d0696cf Mon Sep 17 00:00:00 2001 From: Lyzun Oleksandr Date: Mon, 22 Mar 2021 09:30:25 +0100 Subject: [PATCH 10/10] Manage tests and change a logic of parameter --- app/code/Magento/Webapi/Model/Config/Converter.php | 7 ++----- .../Webapi/Test/Unit/Model/Config/_files/webapi.php | 6 ++++++ .../Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php | 3 ++- .../Magento/Webapi/Model/Config/_files/webapi.php | 10 ++++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Webapi/Model/Config/Converter.php b/app/code/Magento/Webapi/Model/Config/Converter.php index 033d21338d586..4d1010f0a2a2b 100644 --- a/app/code/Magento/Webapi/Model/Config/Converter.php +++ b/app/code/Magento/Webapi/Model/Config/Converter.php @@ -107,13 +107,10 @@ public function convert($source) self::KEY_SERVICE_METHOD => $serviceMethod, ], self::KEY_ACL_RESOURCES => $resourceReferences, - self::KEY_DATA_PARAMETERS => $data + self::KEY_DATA_PARAMETERS => $data, + self::KEY_DESCRIPTION => $description ]; - if ($description !== '') { - $result[self::KEY_ROUTES][$url][$method][self::KEY_DESCRIPTION] = $description; - } - $serviceSecure = false; if (isset($serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_SECURE])) { $serviceSecure = $serviceClassData[self::KEY_METHODS][$soapMethod][self::KEY_SECURE]; diff --git a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php index bd8fcc78953a4..da468ee58e89d 100644 --- a/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php +++ b/app/code/Magento/Webapi/Test/Unit/Model/Config/_files/webapi.php @@ -70,6 +70,7 @@ 'value' => '%customer_id%', ], ], + 'description' => '' ], ], '/V1/customers/me' => [ @@ -88,6 +89,7 @@ 'value' => null, ], ], + 'description' => '' ], 'PUT' => [ 'secure' => true, @@ -104,6 +106,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V1/customers' => [ @@ -118,6 +121,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/customers/:id' => [ @@ -132,6 +136,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -145,6 +150,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], ], diff --git a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php index df8367afe1775..007f7ee795e1d 100644 --- a/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php +++ b/app/code/Magento/WebapiAsync/Test/Unit/Model/ConfigTest.php @@ -67,7 +67,8 @@ public function testGetServicesSetsTopicFromServiceContractName() 'service' => [ 'class' => ProductRepositoryInterface::class, 'method' => 'save', - ] + ], + 'description' => '' ] ] ] diff --git a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php index 57c8fbf45c63c..232b92ac36700 100644 --- a/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php +++ b/dev/tests/integration/testsuite/Magento/Webapi/Model/Config/_files/webapi.php @@ -138,6 +138,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmoduleMSC' => [ @@ -152,6 +153,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmodule1/:id' => [ @@ -166,6 +168,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V1/testmodule1' => [ @@ -184,6 +187,7 @@ 'value' => null, ], ], + 'description' => '' ], 'POST' => [ 'secure' => false, @@ -200,6 +204,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V2/testmodule1/:id' => [ @@ -215,6 +220,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'DELETE' => [ 'secure' => false, @@ -228,6 +234,7 @@ ], 'parameters' => [ ], + 'description' => '' ], 'PUT' => [ 'secure' => false, @@ -241,6 +248,7 @@ ], 'parameters' => [ ], + 'description' => '' ], ], '/V2/testmodule1' => [ @@ -260,6 +268,7 @@ 'value' => null, ], ], + 'description' => '' ], ], '/V2/testmoduleMSC/itemPreconfigured' => [ @@ -274,6 +283,7 @@ 'Magento_TestModuleMSC::resource2' => true, ], 'parameters' => [], + 'description' => '' ] ] ],