bug #58299 [DependencyInjection] Fix XmlFileLoader not respecting when env for services (Bradley Zeggelaar)

This PR was merged into the 5.4 branch.

Discussion
----------

[DependencyInjection] Fix `XmlFileLoader` not respecting when env for services

| Q             | A
| ------------- | ---
| Branch?       | 5.4 <!-- see below -->
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Issues        | Fix #58293 <!-- prefix each issue number with "Fix #", no need to create an issue if none exists, explain below instead -->
| License       | MIT

This PR resolves a bug inside the `XmlFileLoader` that causes the loading of all services inside a `<when env="..." />` elements even when not in the defined environment.

My knowledge about xPath is very limited, but it seems like the parser interpets `.//` as `//` (anywhere) while most like `./` (relative) was meant, due to the passing of an `$root` element. Based on the added test and passing of existing test this change seems like sufficient to respect the `<when env="...." />`

<!--
Replace this notice by a description of your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Commits
-------

2ec78d90909 [DependencyInjection] Fix `XmlFileLoader` not respecting when env for services

Author: nicolas-grekas

Loader/XmlFileLoader.php

@@ -123,7 +123,7 @@ private function parseImports(\DOMDocument $xml, string $file, ?\DOMNode $root =
         $xpath = new \DOMXPath($xml);
         $xpath->registerNamespace('container', self::NS);
 
-        if (false === $imports = $xpath->query('.//container:imports/container:import', $root)) {
+        if (false === $imports = $xpath->query('./container:imports/container:import', $root)) {
             return;
         }
 
@@ -139,14 +139,14 @@ private function parseDefinitions(\DOMDocument $xml, string $file, Definition $d
         $xpath = new \DOMXPath($xml);
         $xpath->registerNamespace('container', self::NS);
 
-        if (false === $services = $xpath->query('.//container:services/container:service|.//container:services/container:prototype|.//container:services/container:stack', $root)) {
+        if (false === $services = $xpath->query('./container:services/container:service|./container:services/container:prototype|./container:services/container:stack', $root)) {
             return;
         }
         $this->setCurrentDir(\dirname($file));
 
         $this->instanceof = [];
         $this->isLoadingInstanceof = true;
-        $instanceof = $xpath->query('.//container:services/container:instanceof', $root);
+        $instanceof = $xpath->query('./container:services/container:instanceof', $root);
         foreach ($instanceof as $service) {
             $this->setDefinition((string) $service->getAttribute('id'), $this->parseDefinition($service, $file, new Definition()));
         }
@@ -197,7 +197,7 @@ private function getServiceDefaults(\DOMDocument $xml, string $file, ?\DOMNode $
         $xpath = new \DOMXPath($xml);
         $xpath->registerNamespace('container', self::NS);
 
-        if (null === $defaultsNode = $xpath->query('.//container:services/container:defaults', $root)->item(0)) {
+        if (null === $defaultsNode = $xpath->query('./container:services/container:defaults', $root)->item(0)) {
             return new Definition();
         }
 

Tests/Fixtures/RemoteCaller.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+interface RemoteCaller
+{
+}

Tests/Fixtures/RemoteCallerHttp.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class RemoteCallerHttp implements RemoteCaller
+{
+}

Tests/Fixtures/RemoteCallerSocket.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures;
+
+class RemoteCallerSocket implements RemoteCaller
+{
+}

Tests/Fixtures/xml/when-env-services.xml

@@ -0,0 +1,23 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+    <services>
+        <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCaller"
+                 alias="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerHttp"/>
+
+        <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerHttp"
+                 class="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerHttp"/>
+
+        <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerSocket"
+                 class="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerSocket"/>
+    </services>
+
+    <when env="dev">
+        <services>
+            <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCaller"
+                     alias="Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerSocket"/>
+        </services>
+    </when>
+</container>

Tests/Loader/XmlFileLoaderTest.php

@@ -44,6 +44,9 @@
 use Symfony\Component\DependencyInjection\Tests\Fixtures\FooWithAbstractArgument;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCaller;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerHttp;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\RemoteCallerSocket;
 use Symfony\Component\ExpressionLanguage\Expression;
 
 class XmlFileLoaderTest extends TestCase
@@ -1167,4 +1170,19 @@ public function testWhenEnv()
 
         $this->assertSame(['foo' => 234, 'bar' => 345], $container->getParameterBag()->all());
     }
+
+    public function testLoadServicesWithEnvironment()
+    {
+        $container = new ContainerBuilder();
+
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'), 'prod');
+        $loader->load('when-env-services.xml');
+
+        self::assertInstanceOf(RemoteCallerHttp::class, $container->get(RemoteCaller::class));
+
+        $loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'), 'dev');
+        $loader->load('when-env-services.xml');
+
+        self::assertInstanceOf(RemoteCallerSocket::class, $container->get(RemoteCaller::class));
+    }
 }