Fixes #1784 - properties in property file take precedence when they m…

…ask existing properties
phingofficial · Jan 30, 2024 · bae7450 · bae7450
1 parent d20f817
commit bae7450
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 13 deletions.
diff --git a/src/Phing/Task/System/PropertyTask.php b/src/Phing/Task/System/PropertyTask.php
@@ -564,23 +564,26 @@ protected function resolveAllProperties(Properties $props)
                         throw new BuildException('Property ' . $propertyName . ' was circularly defined.');
                     }
 
-                    $fragment = $this->getProject()->getProperty($propertyName);
-                    if (null !== $fragment) {
-                        $sb .= $fragment;
-
-                        continue;
-                    }
-
                     if ($props->containsKey($propertyName)) {
                         $fragment = $props->getProperty($propertyName);
                         if (false !== strpos($fragment, '${')) {
                             $resolveStack[] = $propertyName;
                             $resolved = false; // parse again (could have been replaced w/ another var)
                         }
-                    } else {
-                        $fragment = '${' . $propertyName . '}';
+
+                        $sb .= $fragment;
+
+                        continue;
+                    }
+
+                    $fragment = $this->getProject()->getProperty($propertyName);
+                    if (null !== $fragment) {
+                        $sb .= $fragment;
+
+                        continue;
                     }
 
+                    $fragment = '${' . $propertyName . '}';
                     $sb .= $fragment;
                 }
 

diff --git a/tests/Phing/Test/Task/System/PropertyTaskTest.php b/tests/Phing/Test/Task/System/PropertyTaskTest.php
@@ -47,9 +47,22 @@ public function test2(): void
         $this->expectLog('test2', 'testprop1=aa, testprop3=xxyy, testprop4=aazz');
     }
 
-    public function test4(): void
+    public function testPropertyInFileShouldShadowExistingPropertyWithSameName(): void
     {
-        $this->expectLog('test4', 'http.url is http://localhost:999');
+        $this->expectLog(__FUNCTION__, 'http.url is http://localhost:80');
+        $this->assertPropertyEquals('http.port', '999');
+    }
+
+    public function testOverrideExistingPropertyWithNewProperty(): void
+    {
+        $this->executeTarget(__FUNCTION__);
+        $this->assertPropertyEquals('http.port', '80');
+    }
+
+    public function testOverrideExistingPropertyWithNewPropertyFromFile(): void
+    {
+        $this->executeTarget(__FUNCTION__);
+        $this->assertPropertyEquals('http.port', '80');
     }
 
     public function testPrefixSuccess(): void

diff --git a/tests/etc/tasks/property.xml b/tests/etc/tasks/property.xml
@@ -16,12 +16,22 @@
     <property file="property2.properties"/>
   </target>
 
-  <target name="test4">
-    <property name="http.port" value="999" />
+  <target name="testPropertyInFileShouldShadowExistingPropertyWithSameName">
+    <property name="http.port" value="999"/>
     <property file="property3.properties"/>
     <echo message="http.url is ${http.url}"/>
   </target>
 
+  <target name="testOverrideExistingPropertyWithNewProperty">
+    <property name="http.port" value="999"/>
+    <property name="http.port" value="80" override="true" />
+  </target>
+
+  <target name="testOverrideExistingPropertyWithNewPropertyFromFile">
+    <property name="http.port" value="999" />
+    <property file="property3.properties" override="true"/>
+  </target>
+
   <target name="prefix.success">
     <property file="property3.properties" prefix="server1"/>
   </target>