Fix xml reports changed when node is not deleted (#1007) (#1012)

* Fix xml reports changed when node is not deleted

* Added changelog fragment

* Added tests for xml no change remove

* Added PR to changeling fragment

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 0243eabd30)

Co-authored-by: mklassen <lmklassen@gmail.com>

changelogs/fragments/xml-remove-changed.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - xml - fixed issue were changed was returned when removing non-existent xpath (https://github.com/ansible-collections/community.general/pull/1007).

plugins/modules/files/xml.py

@@ -411,8 +411,10 @@ def xpath_matches(tree, xpath, namespaces):
 
 def delete_xpath_target(module, tree, xpath, namespaces):
     """ Delete an attribute or element from a tree """
+    changed = False
     try:
         for result in tree.xpath(xpath, namespaces=namespaces):
+            changed = True
             # Get the xpath for this result
             if is_attribute(tree, xpath, namespaces):
                 # Delete an attribute
@@ -429,7 +431,7 @@ def delete_xpath_target(module, tree, xpath, namespaces):
     except Exception as e:
         module.fail_json(msg="Couldn't delete xpath target: %s (%s)" % (xpath, e))
     else:
-        finish(module, tree, xpath, namespaces, changed=True)
+        finish(module, tree, xpath, namespaces, changed=changed)
 
 
 def replace_children_of(children, match):

tests/integration/targets/xml/tasks/main.yml

@@ -43,7 +43,9 @@
   - include_tasks: test-count.yml
   - include_tasks: test-mutually-exclusive-attributes.yml
   - include_tasks: test-remove-attribute.yml
+  - include_tasks: test-remove-attribute-nochange.yml
   - include_tasks: test-remove-element.yml
+  - include_tasks: test-remove-element-nochange.yml
   - include_tasks: test-set-attribute-value.yml
   - include_tasks: test-set-children-elements.yml
   - include_tasks: test-set-children-elements-level.yml
@@ -53,6 +55,7 @@
   - include_tasks: test-pretty-print-only.yml
   - include_tasks: test-add-namespaced-children-elements.yml
   - include_tasks: test-remove-namespaced-attribute.yml
+  - include_tasks: test-remove-namespaced-attribute-nochange.yml
   - include_tasks: test-set-namespaced-attribute-value.yml
   - include_tasks: test-set-namespaced-element-value.yml
   - include_tasks: test-set-namespaced-children-elements.yml

tests/integration/targets/xml/tasks/test-remove-attribute-nochange.yml

@@ -0,0 +1,28 @@
+---
+  - name: Setup test fixture
+    copy:
+      src: results/test-remove-attribute.xml
+      dest: /tmp/ansible-xml-beers.xml
+
+
+  - name: Remove non-existing '/business/rating/@subjective'
+    xml:
+      path: /tmp/ansible-xml-beers.xml
+      xpath: /business/rating/@subjective
+      state: absent
+    register: remove_attribute
+
+  - name: Compare to expected result
+    copy:
+      src: results/test-remove-attribute.xml
+      dest: /tmp/ansible-xml-beers.xml
+    check_mode: yes
+    diff: yes
+    register: comparison
+
+  - name: Test expected result
+    assert:
+      that:
+      - remove_attribute.changed == false
+      - comparison.changed == false  # identical
+    #command: diff -u {{ role_path }}/results/test-remove-attribute.xml /tmp/ansible-xml-beers.xml

tests/integration/targets/xml/tasks/test-remove-element-nochange.yml

@@ -0,0 +1,28 @@
+---
+  - name: Setup test fixture
+    copy:
+      src: results/test-remove-element.xml
+      dest: /tmp/ansible-xml-beers.xml
+
+
+  - name: Remove non-existing '/business/rating'
+    xml:
+      path: /tmp/ansible-xml-beers.xml
+      xpath: /business/rating
+      state: absent
+    register: remove_element
+
+  - name: Compare to expected result
+    copy:
+      src: results/test-remove-element.xml
+      dest: /tmp/ansible-xml-beers.xml
+    check_mode: yes
+    diff: yes
+    register: comparison
+
+  - name: Test expected result
+    assert:
+      that:
+      - remove_element.changed == false
+      - comparison.changed == false  # identical
+    #command: diff -u {{ role_path }}/results/test-remove-element.xml /tmp/ansible-xml-beers.xml

tests/integration/targets/xml/tasks/test-remove-namespaced-attribute-nochange.yml

@@ -0,0 +1,33 @@
+---
+  - name: Setup test fixture
+    copy:
+      src: results/test-remove-namespaced-attribute.xml
+      dest: /tmp/ansible-xml-namespaced-beers.xml
+
+
+  - name: Remove non-existing namespaced '/bus:business/rat:rating/@attr:subjective'
+    xml:
+      path: /tmp/ansible-xml-namespaced-beers.xml
+      xpath: /bus:business/rat:rating/@attr:subjective
+      namespaces:
+        bus: http://test.business
+        ber: http://test.beers
+        rat: http://test.rating
+        attr: http://test.attribute
+      state: absent
+    register: remove_namespaced_attribute
+
+  - name: Compare to expected result
+    copy:
+      src: results/test-remove-namespaced-attribute.xml
+      dest: /tmp/ansible-xml-namespaced-beers.xml
+    check_mode: yes
+    diff: yes
+    register: comparison
+
+  - name: Test expected result
+    assert:
+      that:
+      - remove_namespaced_attribute.changed == false
+      - comparison.changed == false  # identical
+    #command: diff -u {{ role_path }}/results/test-remove-namespaced-attribute.xml /tmp/ansible-xml-namespaced-beers.xml

tests/integration/targets/xml/tasks/test-remove-namespaced-attribute.yml

@@ -28,6 +28,6 @@
   - name: Test expected result
     assert:
       that:
-      - remove_element.changed == true
+      - remove_namespaced_attribute.changed == true
       - comparison.changed == false  # identical
     #command: diff -u {{ role_path }}/results/test-remove-namespaced-attribute.xml /tmp/ansible-xml-namespaced-beers.xml

tests/integration/targets/xml/tasks/test-remove-namespaced-element-nochange.yml

@@ -0,0 +1,33 @@
+---
+  - name: Setup test fixture
+    copy:
+      src: results/test-remove-element.xml
+      dest: /tmp/ansible-xml-namespaced-beers.xml
+
+
+  - name: Remove non-existing namespaced '/bus:business/rat:rating'
+    xml:
+      path: /tmp/ansible-xml-namespaced-beers.xml
+      xpath: /bus:business/rat:rating
+      namespaces:
+        bus: http://test.business
+        ber: http://test.beers
+        rat: http://test.rating
+        attr: http://test.attribute
+      state: absent
+    register: remove_namespaced_element
+
+  - name: Compare to expected result
+    copy:
+      src: results/test-remove-element.xml
+      dest: /tmp/ansible-xml-namespaced-beers.xml
+    check_mode: yes
+    diff: yes
+    register: comparison
+
+  - name: Test expected result
+    assert:
+      that:
+      - remove_namespaced_element.changed == false
+      - comparison.changed == false  # identical
+    #command: diff -u {{ role_path }}/results/test-remove-element.xml /tmp/ansible-xml-namespaced-beers.xml