Fix several exceptions when calling ZPublisher.utils.fix_properties.

I created this function in Zope 5.4 and it worked for the databases I tried it on.
But on another project I ran into several exceptions.
See tracebacks in Plone:
plone/plone.app.upgrade#270

Note that `plone.app.upgrade` has a temporary copy of the same function.
I fixed the function there as well today:
plone/plone.app.upgrade#271

CHANGES.rst

@@ -11,6 +11,8 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
 5.4.1 (unreleased)
 ------------------
 
+- Fix several exceptions when calling ``ZPublisher.utils.fix_properties``.
+
 - Update to newest compatible versions of dependencies.
 
 - Use intermediate ``str`` representation for non-bytelike response data unless

src/ZPublisher/tests/test_utils.py

@@ -36,6 +36,55 @@ def test_utf_8(self):
         self.assertEqual(self._makeOne(b'test\xc2\xae'), 'test\xae')
 
 
+class NoUpdatePropertyManager:
+    """PropertyManager without _updateProperty method.
+
+    This is a simplified version of the original PropertyManager,
+    with only the methods we need.
+    """
+    _properties = ()
+
+    def _setPropValue(self, id, value):
+        if type(value) == list:
+            value = tuple(value)
+        setattr(self, id, value)
+
+    def _setProperty(self, id, value, type='string'):
+        self._properties = self._properties + ({'id': id, 'type': type},)
+        self._setPropValue(id, value)
+
+    def hasProperty(self, id):
+        for p in self._properties:
+            if id == p['id']:
+                return 1
+        return 0
+
+    def getProperty(self, id, d=None):
+        if self.hasProperty(id):
+            return getattr(self, id)
+        return d
+
+    def getPropertyType(self, id):
+        for md in self._properties:
+            if md['id'] == id:
+                return md.get('type', 'string')
+        return None
+
+    def _propertyMap(self):
+        return self._properties
+
+    def propertyMap(self):
+        return tuple(dict.copy() for dict in self._propertyMap())
+
+
+class NoPropertiesManager(NoUpdatePropertyManager):
+    """PropertyManager with _updateProperty method but without _properties."""
+    _properties = None
+
+    def _updateProperty(self, id, value):
+        self._setPropValue(id, value)
+
+
 class FixPropertiesTests(unittest.TestCase):
 
     def _makeOne(self):
@@ -102,3 +151,28 @@ def test_ustring(self):
         fix_properties(obj)
         self.assertEqual(obj.getProperty("prop1"), "single line")
         self.assertEqual(obj.getPropertyType("prop1"), "string")
+
+    def test_no_update(self):
+        # Test that an object without _updateProperty method does not trip up
+        # our code.
+        from ZPublisher.utils import fix_properties
+
+        obj = NoUpdatePropertyManager()
+        obj._setProperty("mixed", ["text and", b"bytes"], "lines")
+        self.assertEqual(obj.getProperty("mixed"), ("text and", b"bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "lines")
+
+        # This should not raise an error.
+        fix_properties(obj)
+        # The properties should have remained the same.
+        self.assertEqual(obj.getProperty("mixed"), ("text and", b"bytes"))
+        self.assertEqual(obj.getPropertyType("mixed"), "lines")
+
+    def test_no_properties(self):
+        # Test that an object with a failing propertyMap method,
+        # due to _properties=None, does not trip up our code.
+        from ZPublisher.utils import fix_properties
+
+        obj = NoPropertiesManager()
+        # This should not raise an error.
+        fix_properties(obj)

src/ZPublisher/utils.py

@@ -145,11 +145,19 @@ def fix_properties(obj, path=None):
             # We don't care about the path then, it is only shown in logs.
             path = "/dummy"
 
+    if not hasattr(obj, "_updateProperty"):
+        # Seen with portal_url tool, most items in portal_skins,
+        # catalog lexicons, workflow states/transitions/variables, etc.
+        return
     try:
         prop_map = obj.propertyMap()
-    except AttributeError:
-        # Object does not inherit from PropertyManager.
-        # For example 'MountedObject'.
+    except (AttributeError, TypeError, KeyError, ValueError):
+        # If getting the property map fails, there is nothing we can do.
+        # Problems seen in practice:
+        # - Object does not inherit from PropertyManager,
+        #   for example 'MountedObject'.
+        # - Object is a no longer existing skin layer.
+        logger.warning("Error getting property map from %s", path)
         return
 
     for prop_info in prop_map: