Removed leading backslash in xml

[srsathish92]

Description (*)

Removed leading backslash in namespace & class in XML

xsi:type="object">\Magento\... xsi:type="string">\Magento\... xsi:type="const">\Magento\... <type name="\Magento\... type="\Magento\... <virtualType name="\Magento\... for="\Magento\...

is replaced to

xsi:type="object">Magento\.. xsi:type="string">Magento\... xsi:type="const">Magento\... <type name="Magento\... type="Magento\... <virtualType name="Magento\... for="Magento\...

Questions or comments

If any please let me know your feedback's

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] Removed leading backslash in xml #29614: Removed leading backslash in xml

[m2-assistant]

Hi @srsathish92. 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

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

[ihor-sviziev]

Please test on test instance the cases that fails on functional tests, seems that your changes break functionality somehow. It requires additional debug to explain why

Pull Request state was updated. Re-review required.

[srsathish92]

@magento give me test instance

[magento-engcom-team]

Hi @srsathish92. Thank you for your request. I'm working on Magento instance for you

[magento-engcom-team]

Hi @srsathish92, here is your new Magento instance.
Admin access: https://pr-26948.instances.magento-community.engineering/admin_ee88
Login: 490a6f0a Password: 62684c25c79d
Instance will be terminated in up to 3 hours.

Pull Request state was updated. Re-review required.

[ihor-sviziev]

Waiting approval for SVC failure

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7013 has been created to process this Pull Request
✳️ @ihor-sviziev, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[srsathish92]

@magento run Semantic Version Checker

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[Den4ik]

I believe that Failed Semantic Version check could be ignored for ES module

[ihor-sviziev]

It seems like SVC failure should be fixed already magento/magento-semver#59
@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[ihor-sviziev]

@sidolov, are we using latest version of the SVC checker? Feels like the issue should be resolved already

[sidolov]

@ihor-sviziev according to the composer.json - we always use the latest released version of the package

[ihor-sviziev]

Oh, I see that that commit wasn't added to release yet :(

[ihor-sviziev]

Hi @srsathish92,
I did additional research, and it seems like removing leading backslash from the XML files in general - is a good practice, but it might lead to unexpected issues during the upgrade.
Reason - in the XML - two nodes

    <type name="Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Magento\CatalogSearch\Model\ResourceModel\EngineInterface::CONFIG_ENGINE_PATH</argument>
            <argument name="scopeType" xsi:type="const">Magento\Store\Model\ScopeInterface::SCOPE_STORE</argument>
        </arguments>
    </type>

and

    <type name="\Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Some\Another\Path::CONFIG_ENGINE_PATH</argument>
        </arguments>
    </type>

Will be just merged to something like this (instead of overriding the value):

    <type name="Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Magento\CatalogSearch\Model\ResourceModel\EngineInterface::CONFIG_ENGINE_PATH</argument>
            <argument name="scopeType" xsi:type="const">Magento\Store\Model\ScopeInterface::SCOPE_STORE</argument>
        </arguments>
    </type>
    <type name="\Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Some\Another\Path::CONFIG_ENGINE_PATH</argument>
        </arguments>
    </type>

In this case, Magento will use only one of these sections that might lead to unexpected results.

It's a limitation of XML, so I think it's better not to touch these lines to prevent breaking backward compatibility rather than do this cleanup.
I'm closing this Pull Request.

Thank you so much for your contribution!

[m2-assistant]

Hi @srsathish92, 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.

[Den4ik]

@ihor-sviziev Do you mean merge during prepare Magento release or merging of xml files during app execution?

[srsathish92]

Hi @srsathish92, I did additional research, and it seems like removing leading backslash from the XML files in general - is a good practice, but it might lead to unexpected issues during the upgrade. Reason - in the XML - two nodes

    <type name="Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Magento\CatalogSearch\Model\ResourceModel\EngineInterface::CONFIG_ENGINE_PATH</argument>
            <argument name="scopeType" xsi:type="const">Magento\Store\Model\ScopeInterface::SCOPE_STORE</argument>
        </arguments>
    </type>

and

    <type name="\Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Some\Another\Path::CONFIG_ENGINE_PATH</argument>
        </arguments>
    </type>

Will be just merged to something like this (instead of overriding the value):

    <type name="Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Magento\CatalogSearch\Model\ResourceModel\EngineInterface::CONFIG_ENGINE_PATH</argument>
            <argument name="scopeType" xsi:type="const">Magento\Store\Model\ScopeInterface::SCOPE_STORE</argument>
        </arguments>
    </type>
    <type name="\Magento\AdvancedSearch\Model\Client\ClientResolver">
        <arguments>
            <argument name="path" xsi:type="const">Some\Another\Path::CONFIG_ENGINE_PATH</argument>
        </arguments>
    </type>

In this case, Magento will use only one of these sections that might lead to unexpected results.

It's a limitation of XML, so I think it's better not to touch these lines to prevent breaking backward compatibility rather than do this cleanup. I'm closing this Pull Request.

Thank you so much for your contribution!

@ihor-sviziev Did you tried the viceversa? What will be output? - If it is the same behaviour as aforementioned then we need to update the document somewhere.

[ihor-sviziev]

@srsathish92, basically the same result. The reason is pretty simple - at the time of XML merge, the nodes aren't getting merged, similar to PHP. After that, Magento trying to retrieve the data, and it's using the only one section

Example:

var_dump(array_merge(['\test' => 0], ['test' => 1]));

outputs

array(2) {
  ["\test"]=>
  int(0)
  ["test"]=>
  int(1)
}

not the

array(1) {
  ["test"]=>
  int(1)
}

[Den4ik]

@ihor-sviziev Could you little bit explain when you have time. One place where we have merging is config load. Are you see problems here?