From 279e89e1febe424f1fcdb95083d7bef48f29ce9b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Fri, 24 Jan 2020 14:25:43 +0100
Subject: [PATCH] misc: use helper function to get xml node text (#39240)

---
 wcs/fields.py             | 22 ++++++++--------
 wcs/formdef.py            | 18 ++++++-------
 wcs/qommon/misc.py        |  6 +++++
 wcs/qommon/xml_storage.py |  4 +--
 wcs/workflows.py          | 55 ++++++++++++++++++---------------------
 5 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/wcs/fields.py b/wcs/fields.py
index d0297ee3f..4de7f9db4 100644
--- a/wcs/fields.py
+++ b/wcs/fields.py
@@ -38,7 +38,7 @@ from django.utils.six.moves.html_parser import HTMLParser
 from .qommon import _, force_str
 from .qommon import evalutils
 from .qommon.form import *
-from .qommon.misc import localstrftime, strftime, date_format, ellipsize, can_thumbnail
+from .qommon.misc import localstrftime, strftime, date_format, ellipsize, can_thumbnail, xml_node_text
 from .qommon.template import Template, TemplateError
 from .qommon import get_cfg, get_logger
 
@@ -272,11 +272,11 @@ class Field(object):
                 continue
             if list(el):
                 if type(getattr(self, attribute)) is list:
-                    v = [force_str(x.text) for x in el]
+                    v = [xml_node_text(x) for x in el]
                 elif type(getattr(self, attribute)) is dict:
                     v = {}
                     for e in el:
-                        v[e.tag] = force_str(e.text)
+                        v[e.tag] = xml_node_text(e)
                 else:
                     print('currently:', self.__dict__)
                     print('  attribute:', attribute)
@@ -291,10 +291,10 @@ class Field(object):
                 elif type(getattr(self, attribute)) is int:
                     setattr(self, attribute, int(el.text))
                 else:
-                    setattr(self, attribute, force_str(el.text))
+                    setattr(self, attribute, xml_node_text(el))
         if include_id:
             try:
-                self.id = force_str(elem.find('id').text)
+                self.id = xml_node_text(elem.find('id'))
             except:
                 pass
 
@@ -304,8 +304,8 @@ class Field(object):
             return
         if node.findall('type'):
             self.condition = {
-                'type': force_str(node.find('type').text),
-                'value': force_str(node.find('value').text),
+                'type': xml_node_text(node.find('type')),
+                'value': xml_node_text(node.find('value')),
             }
         elif node.text:
             self.condition = {'type': 'python', 'value': force_str(node.text).strip()}
@@ -1788,19 +1788,19 @@ class PageField(Field):
         for post_condition_node in node.findall('post_condition'):
             if post_condition_node.findall('condition/type'):
                 condition = {
-                    'type': force_str(post_condition_node.find('condition/type').text),
-                    'value': force_str(post_condition_node.find('condition/value').text),
+                    'type': xml_node_text(post_condition_node.find('condition/type')),
+                    'value': xml_node_text(post_condition_node.find('condition/value')),
                 }
             elif post_condition_node.find('condition').text:
                 condition = {
                     'type': 'python',
-                    'value': force_str(post_condition_node.find('condition').text),
+                    'value': xml_node_text(post_condition_node.find('condition')),
                 }
             else:
                 continue
             self.post_conditions.append({
                 'condition': condition,
-                'error_message': force_str(post_condition_node.find('error_message').text),
+                'error_message': xml_node_text(post_condition_node.find('error_message')),
             })
 
     def post_conditions_export_to_xml(self, node, charset, include_id=False):
diff --git a/wcs/formdef.py b/wcs/formdef.py
index c4929d834..fa5fa97be 100644
--- a/wcs/formdef.py
+++ b/wcs/formdef.py
@@ -35,7 +35,7 @@ from .qommon import _, force_str, PICKLE_KWARGS
 from .qommon.storage import StorableObject, fix_key
 from .qommon.cron import CronJob
 from .qommon.form import *
-from .qommon.misc import simplify, get_as_datetime
+from .qommon.misc import simplify, get_as_datetime, xml_node_text
 from .qommon import get_cfg
 from .qommon.substitution import Substitutions
 from .qommon.publisher import get_publisher_class
@@ -1061,7 +1061,7 @@ class FormDef(StorableObject):
             value = tree.find(text_attribute)
             if value is None or value.text is None:
                 continue
-            setattr(formdef, text_attribute, force_str(value.text))
+            setattr(formdef, text_attribute, xml_node_text(value))
 
         for boolean_attribute in cls.BOOLEAN_ATTRIBUTES:
             value = tree.find(boolean_attribute)
@@ -1095,10 +1095,10 @@ class FormDef(StorableObject):
             if option.attrib.get('type') == 'date':
                 option_value = time.strptime(option.text, '%Y-%m-%d')
             elif option.text:
-                option_value = force_str(option.text)
+                option_value = xml_node_text(option)
             elif option.findall('filename'):
-                filename = option.find('filename').text
-                upload = Upload(filename, content_type=option.find('content_type').text)
+                filename = xml_node_text(option.find('filename'))
+                upload = Upload(filename, content_type=xml_node_text(option.find('content_type')))
                 option_value = UploadedFile(get_publisher().app_dir, filename, upload)
                 option_value.set_content(base64.decodestring(force_bytes(option.find('content').text)))
             formdef.workflow_options[option.attrib.get('varname')] = option_value
@@ -1116,7 +1116,7 @@ class FormDef(StorableObject):
                 if Category.has_key(category_id):
                     formdef.category_id = category_id
             else:
-                category = force_str(category_node.text)
+                category = xml_node_text(category_node)
                 for c in Category.select():
                     if c.name == category:
                         formdef.category_id = c.id
@@ -1130,7 +1130,7 @@ class FormDef(StorableObject):
                 if Workflow.has_key(workflow_id):
                     formdef.workflow_id = workflow_id
             else:
-                workflow = force_str(workflow_node.text)
+                workflow = xml_node_text(workflow_node)
                 for w in Workflow.select():
                     if w.name == workflow:
                         formdef.workflow_id = w.id
@@ -1138,7 +1138,7 @@ class FormDef(StorableObject):
 
         def get_role_by_node(role_node):
             role_id = None
-            value = force_str(role_node.text)
+            value = xml_node_text(role_node)
             if value.startswith('_') or value == 'logged-users':
                 role_id = value
             elif include_id:
@@ -1183,7 +1183,7 @@ class FormDef(StorableObject):
             formdef.geolocations = {}
             for child in geolocations_node:
                 geoloc_key = child.attrib['key']
-                geoloc_value = force_str(child.text)
+                geoloc_value = xml_node_text(child)
                 formdef.geolocations[geoloc_key] = geoloc_value
 
         if tree.find('required_authentication_contexts') is not None:
diff --git a/wcs/qommon/misc.py b/wcs/qommon/misc.py
index bc18b33e7..b61b07e33 100644
--- a/wcs/qommon/misc.py
+++ b/wcs/qommon/misc.py
@@ -495,6 +495,12 @@ def indent_xml(elem, level=0):
     return elem
 
 
+def xml_node_text(node):
+    if node is None or node.text is None:
+        return None
+    return force_str(node.text)
+
+
 def preprocess_struct_time(obj):
     if isinstance(obj, time.struct_time):
         dt = datetime.datetime(*obj[:6])
diff --git a/wcs/qommon/xml_storage.py b/wcs/qommon/xml_storage.py
index e21210580..1dd0705dc 100644
--- a/wcs/qommon/xml_storage.py
+++ b/wcs/qommon/xml_storage.py
@@ -21,7 +21,7 @@ from django.utils.encoding import force_text
 from quixote import get_publisher
 
 from . import force_str
-from .misc import indent_xml
+from .misc import indent_xml, xml_node_text
 from .storage import StorableObject
 
 
@@ -106,7 +106,7 @@ class XmlStorableObject(StorableObject):
         return obj
 
     def import_str_from_xml(self, element, charset):
-        return force_str(element.text)
+        return xml_node_text(element)
 
     def import_int_from_xml(self, element, charset):
         return int(element.text)
diff --git a/wcs/workflows.py b/wcs/workflows.py
index c9f9efb43..4cee17d62 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -31,7 +31,7 @@ from django.utils.encoding import force_text
 from quixote import get_request, get_response, redirect
 
 from .qommon import _, force_str
-from .qommon.misc import C_, get_as_datetime, file_digest, get_foreground_colour
+from .qommon.misc import C_, get_as_datetime, file_digest, get_foreground_colour, xml_node_text
 from .qommon.storage import StorableObject, atomic_write, NotEqual, Contains, Null, pickle_2to3_conversion
 from .qommon.form import *
 from .qommon.humantime import seconds2humanduration
@@ -600,12 +600,12 @@ class Workflow(StorableObject):
         if include_id and tree.attrib.get('id'):
             workflow.id = tree.attrib.get('id')
 
-        workflow.name = force_str(tree.find('name').text)
+        workflow.name = xml_node_text(tree.find('name'))
 
         if tree.find('roles') is not None:
             workflow.roles = {}
             for role_node in tree.findall('roles/role'):
-                workflow.roles[role_node.attrib['id']] = force_str(role_node.text)
+                workflow.roles[role_node.attrib['id']] = xml_node_text(role_node)
 
         if tree.find('last_modification') is not None:
             node = tree.find('last_modification')
@@ -877,11 +877,11 @@ class XmlSerialisable(object):
                 continue
             if list(el):
                 if type(getattr(self, attribute)) is list:
-                    v = [force_str(x.text) if x.text is not None else '' for x in el]
+                    v = [xml_node_text(x) or '' for x in el]
                 elif type(getattr(self, attribute)) is dict:
                     v = {}
                     for e in el:
-                        v[e.tag] = force_str(e.text)
+                        v[e.tag] = xml_node_text(e)
                 else:
                     # ???
                     raise AssertionError
@@ -894,7 +894,7 @@ class XmlSerialisable(object):
                 elif type(getattr(self, attribute)) is int:
                     setattr(self, attribute, int(el.text))
                 else:
-                    setattr(self, attribute, force_str(el.text))
+                    setattr(self, attribute, xml_node_text(el))
 
     def _roles_export_to_xml(self, attribute, item, charset, include_id=False):
         if not hasattr(self, attribute) or not getattr(self, attribute):
@@ -945,7 +945,7 @@ class XmlSerialisable(object):
         if elem is None:
             return None
 
-        value = force_str(elem.text)
+        value = xml_node_text(elem) or ''
 
         # look for known static values
         if value.startswith('_') or value == 'logged-users':
@@ -1362,10 +1362,10 @@ class WorkflowGlobalAction(object):
         return status
 
     def init_with_xml(self, elem, charset, include_id=False):
-        self.id = force_str(elem.find('id').text)
-        self.name = force_str(elem.find('name').text)
+        self.id = xml_node_text(elem.find('id'))
+        self.name = xml_node_text(elem.find('name'))
         if elem.find('backoffice_info_text') is not None:
-            self.backoffice_info_text = force_str(elem.find('backoffice_info_text').text)
+            self.backoffice_info_text = xml_node_text(elem.find('backoffice_info_text'))
 
         self.items = []
         for item in elem.find('items'):
@@ -1403,10 +1403,10 @@ class WorkflowCriticalityLevel(object):
         return level
 
     def init_with_xml(self, elem, charset, include_id=False):
-        self.id = force_str(elem.find('id').text)
-        self.name = force_str(elem.find('name').text)
+        self.id = xml_node_text(elem.find('id'))
+        self.name = xml_node_text(elem.find('name'))
         if elem.find('colour') is not None:
-            self.colour = force_str(elem.find('colour').text)
+            self.colour = xml_node_text(elem.find('colour'))
 
 
 class WorkflowStatus(object):
@@ -1652,16 +1652,16 @@ class WorkflowStatus(object):
         return status
 
     def init_with_xml(self, elem, charset, include_id=False):
-        self.id = force_str(elem.find('id').text)
-        self.name = force_str(elem.find('name').text)
+        self.id = xml_node_text(elem.find('id'))
+        self.name = xml_node_text(elem.find('name'))
         if elem.find('colour') is not None:
-            self.colour = force_str(elem.find('colour').text)
+            self.colour = xml_node_text(elem.find('colour'))
         if elem.find('extra_css_class') is not None:
-            self.extra_css_class = force_str(elem.find('extra_css_class').text)
+            self.extra_css_class = xml_node_text(elem.find('extra_css_class'))
         if elem.find('forced_endpoint') is not None:
             self.forced_endpoint = (elem.find('forced_endpoint').text == 'true')
         if elem.find('backoffice_info_text') is not None:
-            self.backoffice_info_text = force_str(elem.find('backoffice_info_text').text)
+            self.backoffice_info_text = xml_node_text(elem.find('backoffice_info_text'))
 
         self.visibility = []
         for visibility_role in elem.findall('visibility/role'):
@@ -2015,12 +2015,12 @@ class WorkflowStatusItem(XmlSerialisable):
             return
         if node.findall('type'):
             self.condition = {
-                'type': force_str(node.find('type').text),
-                'value': force_str(node.find('value').text),
+                'type': xml_node_text(node.find('type')),
+                'value': xml_node_text(node.find('value')),
             }
         elif node.text:
             # backward compatibility
-            self.condition = {'type': 'python', 'value': force_str(node.text)}
+            self.condition = {'type': 'python', 'value': xml_node_text(node)}
 
     def q_admin_lookup(self, workflow, status, component, html_top):
         return None
@@ -2035,7 +2035,7 @@ class WorkflowStatusItem(XmlSerialisable):
         if elem is None:
             self.attachments = None
         else:
-            self.attachments = [force_str(item.text) for item in elem.findall('attachment')]
+            self.attachments = [xml_node_text(item) for item in elem.findall('attachment')]
 
     def get_attachments_options(self):
         attachments_options = [(None, '---', None)]
@@ -2303,12 +2303,9 @@ class CommentableWorkflowStatusItem(WorkflowStatusItem):
     def button_label_init_with_xml(self, element, charset, include_id=False):
         if element is None:
             return
-        if element.text is None:
-            # this means element is self-closing, <button_label />, which maps
-            # to None, meaning "no button".
-            self.button_label = None
-        else:
-            self.button_label = force_str(element.text)
+        # this can be None if element is self-closing, <button_label />, which
+        # then maps to None, meaning "no button".
+        self.button_label = xml_node_text(element)
 
 register_item_class(CommentableWorkflowStatusItem)
 
@@ -2461,7 +2458,7 @@ class SendmailWorkflowStatusItem(WorkflowStatusItem):
         # override to allow for destination set with computed values.
         if elem is None:
             return None
-        value = force_str(elem.text)
+        value = xml_node_text(elem)
 
         if self.get_expression(value)['type'] != 'text' or '@' in value:
             return value
-- 
2.25.0

