Fix for communication.xml Handlers merging processs

[matiashidalgo]

Description (*)

This changes fixes the "handler" merging process into communication.xml files which currently overrides all the handlers with the last one created, ignoring it's different names as expected. Issue found during custom handler development for MCOM module where there was already a handler for a message.

Manual testing scenarios (*)

1. Please see Fix for communication.xml Handlers merging processs #28926 (comment)

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] Fix for communication.xml Handlers merging processs #29528: Fix for communication.xml Handlers merging processs

[m2-assistant]

Hi @matiashidalgo. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

[sidolov]

@magento create issue

[lenaorobei]

Thanks for fixing it.

Could you please add integration test to cover this?

[lenaorobei]

@magento run all tests

[matiashidalgo]

Thanks for fixing it.

Could you please add integration test to cover this?

Hi @lenaorobei since this is a fix to Framework I can not find any example of an Integration test, would you mind to point me out to an example of this and tell me which module I'm supposed to use for place my integration test?

[lenaorobei]

Sure! Please see example dev/tests/integration/testsuite/Magento/Framework/Communication/ConfigTest.php.

I also noticed that Static Tests build failed. Here is the report https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/28926/f2b73dfd3095f563cfde67e1f874d8e1/Statics/allure-report-b2b/index.html#suites/51d3ab4d5d77a0bc6a7ea344e9e7e6be/dccf717ef048c46e/

Looks like this particular case needs to be suppressed, because default parameters are different.

[lenaorobei]

@magento run all tests

[matiashidalgo]

Sure! Please see example dev/tests/integration/testsuite/Magento/Framework/Communication/ConfigTest.php.

I also noticed that Static Tests build failed. Here is the report https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/28926/f2b73dfd3095f563cfde67e1f874d8e1/Statics/allure-report-b2b/index.html#suites/51d3ab4d5d77a0bc6a7ea344e9e7e6be/dccf717ef048c46e/

Looks like this particular case needs to be suppressed, because default parameters are different.

Thank you a lot for pointing me to the right place, I've found the reason which makes the integration test not detect this issues so now it is covered.

About the static code error, I didn't found how to fix it, can you help me once again and point me out where this can be fixed?

[lbajsarowicz]

@magento run all tests

[matiashidalgo]

@magento run all tests

[lbajsarowicz]

@matiashidalgo Perfect timing! I was just sending the recommendations to you.

[ghost]

Got it, thanks.

…

Sent from my Apple Watch

[matiashidalgo]

@lenaorobei @lbajsarowicz I can't find the issue on this failed tests, can you check?

[matiashidalgo]

@matiashidalgo I suppose that you fixed the next scenario:

1. Create sample ModuleA with communication.xml

<topic name="product_action_attribute.update" request="Magento\AsynchronousOperations\Api\Data\OperationInterface">
    <handler name="handler.moduleA" type="Magento\Catalog\Model\Attribute\Backend\Consumer" method="process" />
</topic>

1. Create sample ModuleB with communication.xml

<topic name="product_action_attribute.update" request="Magento\AsynchronousOperations\Api\Data\OperationInterface">
    <handler name="handler.moduleB" type="Magento\Catalog\Model\Attribute\Backend\Consumer" method="process" />
</topic>

1. Clean cache config
2. Make the following

$reader = ObjectManager::getInstance()->create(\Magento\Framework\Communication\Config\Reader\XmlReader::class);
$result = $reader->read();

Before changes
we can see only one handler for product_action_attribute.update topic

After changes
for product_action_attribute.update topic we can see previously created handlers

• handler.moduleA
• handler.moduleB

Exactly, this is a correct test @engcom-Charlie

[engcom-Charlie]

@matiashidalgo could you please resolve the merge conflict?
Thank you.

[matiashidalgo]

@matiashidalgo could you please resolve the merge conflict?
Thank you.

Conflicts resolved! @engcom-Charlie Thank you too!

[engcom-Charlie]

@magento run all tests

[gabrieldagama]

@magento run all tests

[gabrieldagama]

@magento run Functional Tests CE, WebAPI Tests

[matiashidalgo]

@magento run Functional Tests CE

[matiashidalgo]

@engcom-Charlie @gabrieldagama all checks passed now.

Please do let me know if something else is needed from me.

[gabrieldagama]

Hi @matiashidalgo, thanks for your contribution, the changes look good to me!

[magento-engcom-team]

Hi @gabrieldagama, thank you for the review.
ENGCOM-8232 has been created to process this Pull Request

[engcom-Delta]

✔️ QA passed
Was checked case from Manual testing scenarios
❌ Before:

✔️ After:

[engcom-Delta]

Note: Automation tests are passed

[m2-assistant]

Hi @matiashidalgo, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.