From 11c9756d9cd1ef5f5365a45acdbc18849b945071 Mon Sep 17 00:00:00 2001
From: Dag Wieers <dag@wieers.com>
Date: Fri, 25 Aug 2017 15:01:24 +0200
Subject: [PATCH] xml module: Make user interface much more restricted (#28657)

So the xml module has a lot of parameters that combined together do
specific things. however it does not really describe which parameters
are required together, or which ones are required.

This PR fixes that situation and makes it impossible to provide
confussing parameters combinations.

Also, `pretty_print` was used as a flag to modify the output, but also
as an action to reformat the file (without other change). This is
problematic the way it was designed. This fixes that too.

This fixes #28194
---
 lib/ansible/modules/files/xml.py | 62 +++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/lib/ansible/modules/files/xml.py b/lib/ansible/modules/files/xml.py
index 0712fe38053..7ed5ad36ac3 100644
--- a/lib/ansible/modules/files/xml.py
+++ b/lib/ansible/modules/files/xml.py
@@ -224,13 +224,13 @@ xmlstring:
     returned: when parameter 'xmlstring' is set
 '''
 
+import copy
 import json
 import os
 import re
 import traceback
 
 from collections import MutableMapping
-import copy
 from distutils.version import LooseVersion
 from io import BytesIO
 
@@ -675,11 +675,11 @@ def main():
         argument_spec=dict(
             path=dict(type='path', aliases=['dest', 'file']),
             xmlstring=dict(type='str'),
-            xpath=dict(type='str', default='/'),
+            xpath=dict(type='str'),
             namespaces=dict(type='dict', default={}),
             state=dict(type='str', default='present', choices=['absent', 'present'], aliases=['ensure']),
-            value=dict(),
-            attribute=dict(),
+            value=dict(type='raw'),
+            attribute=dict(type='raw'),
             add_children=dict(type='list'),
             set_children=dict(type='list'),
             count=dict(type='bool', default=False),
@@ -690,15 +690,27 @@ def main():
             backup=dict(type='bool', default=False),
         ),
         supports_check_mode=True,
+        # TODO: Implement this as soon as #28662 (required_by functionality) is merged
+        # required_by=dict(
+        #    add_children=['xpath'],
+        #    attribute=['value'],
+        #    set_children=['xpath'],
+        #    value=['xpath'],
+        # ),
+        required_if=[
+            ['content', 'attribute', ['xpath']],
+            ['content', 'text', ['xpath']],
+            ['count', True, ['xpath']],
+            ['print_match', True, ['xpath']],
+        ],
+        required_one_of=[
+            ['path', 'xmlstring'],
+            ['add_children', 'content', 'count', 'pretty_print', 'print_match', 'set_children', 'value'],
+        ],
         mutually_exclusive=[
-            ['value', 'set_children'],
-            ['value', 'add_children'],
-            ['set_children', 'add_children'],
+            ['add_children', 'content', 'count', 'print_match', 'set_children', 'value'],
             ['path', 'xmlstring'],
-            ['content', 'set_children'],
-            ['content', 'add_children'],
-            ['content', 'value'],
-        ]
+        ],
     )
 
     xml_file = module.params['path']
@@ -734,7 +746,7 @@ def main():
         module.fail_json(msg="The target XML source '%s' does not exist." % xml_file)
 
     # Parse and evaluate xpath expression
-    if xpath:
+    if xpath is not None:
         try:
             etree.XPath(xpath)
         except etree.XPathSyntaxError as e:
@@ -755,24 +767,25 @@ def main():
 
     if print_match:
         do_print_match(module, doc, xpath, namespaces)
+        # exit
 
     if count:
         count_nodes(module, doc, xpath, namespaces)
+        # exit
 
     if content == 'attribute':
         get_element_attr(module, doc, xpath, namespaces)
+        # exit
     elif content == 'text':
         get_element_text(module, doc, xpath, namespaces)
-
-    # module.fail_json(msg="OK. Well, etree parsed the xml file...")
-
-    # module.exit_json(what_did={"foo": "bar"}, changed=True)
+        # exit
 
     # File exists:
     if state == 'absent':
         # - absent: delete xpath target
         delete_xpath_target(module, doc, xpath, namespaces)
-        # Exit
+        # exit
+
     # - present: carry on
 
     # children && value both set?: should have already aborted by now
@@ -781,23 +794,32 @@ def main():
     # set_children set?
     if set_children:
         set_target_children(module, doc, xpath, namespaces, set_children, input_type)
+        # exit
 
     # add_children set?
     if add_children:
         add_target_children(module, doc, xpath, namespaces, add_children, input_type)
+        # exit
 
     # No?: Carry on
 
     # Is the xpath target an attribute selector?
     if value is not None:
         set_target(module, doc, xpath, namespaces, attribute, value)
+        # exit
+
+    # If an xpath was provided, we need to do something with the data
+    if xpath is not None:
+        ensure_xpath_exists(module, doc, xpath, namespaces)
+        # exit
 
-    # Format the xml only?
+    # Otherwise only reformat the xml data?
     if pretty_print:
+        xpath = '/'
         pretty(module, doc)
+        # exit
 
-    ensure_xpath_exists(module, doc, xpath, namespaces)
-    # module.fail_json(msg="don't know what to do")
+    module.fail_json(msg="Don't know what to do")
 
 
 if __name__ == '__main__':