Merge pull request #409 from plone/davisagli-perf-lif

Performance: Avoid unnecessary work copying unchanged language-independent fields

news/408.bugfix

@@ -0,0 +1 @@
+When an item is modified, only copy language-independent field values to translations if they have changed. [davisagli]

src/plone/app/multilingual/dx/cloner.py

@@ -50,19 +50,26 @@ def copy_relation(self, relation_value, target_language):
         return RelationValue(intids.getId(obj))
 
     def copy_fields(self, translation):
-        doomed = False
+        changed = False
 
         target_language = queryAdapter(translation, ILanguage).get_language()
 
         for schema in iterSchemata(self.context):
+            context_adapter = None
+            translation_adapter = None
             for field_name in schema:
                 if ILanguageIndependentField.providedBy(schema[field_name]):
-                    value = getattr(schema(self.context), field_name, _marker)
+                    if context_adapter is None:
+                        context_adapter = schema(self.context)
+                    value = getattr(context_adapter, field_name, _marker)
+                    field_changed = None
                     if value == _marker:
                         continue
                     elif IRelationValue.providedBy(value):
+                        field_changed = True
                         value = self.copy_relation(value, target_language)
                     elif IRelationList.providedBy(schema[field_name]):
+                        field_changed = True
                         if not value:
                             value = []
                         else:
@@ -75,10 +82,21 @@ def copy_fields(self, translation):
                                     new_value.append(copied_relation)
                             value = new_value
 
-                    doomed = True
-                    setattr(schema(translation), field_name, safe_unicode(value))
+                    if translation_adapter is None:
+                        translation_adapter = schema(translation)
+
+                    # We only want to store a new value if it has changed.
+                    # In general we can compare equality of the new value to the one on the translation.
+                    # But RelationValue.__eq__ is broken if the relation doesn't have a from_object,
+                    # so for now we force field_changed to True for relations above.
+                    if field_changed is None:
+                        translation_value = getattr(translation_adapter, field_name, _marker)
+                        field_changed = value != translation_value
+                    if field_changed:
+                        changed = True
+                        setattr(translation_adapter, field_name, safe_unicode(value))
 
         # If at least one field has been copied over to the translation
         # we need to inform subscriber to trigger an ObjectModifiedEvent
         # on that translation.
-        return doomed
+        return changed

src/plone/app/multilingual/tests/test_lif.py

@@ -14,11 +14,13 @@
 from zope.event import notify
 from zope.interface import alsoProvides
 from zope.interface import noLongerProvides
+from zope.lifecycleevent import modified
 from zope.lifecycleevent import ObjectModifiedEvent
 from zope.pagetemplate.interfaces import IPageTemplate
 from zope.publisher.interfaces.browser import IDefaultBrowserLayer
 from zope.schema._bootstrapinterfaces import RequiredMissing
 
+import transaction
 import unittest
 
 
@@ -148,6 +150,26 @@ def test_input_widget_does_not_render_textarea_but_span(self):
         )
         self.assertNotIn("<textarea", widget_template(self.widget))
 
+    def test_field_copied_to_translation_when_modified(self):
+        en_feedback = self.portal.en["test-feedback"]
+        ca_feedback = api.translate(en_feedback, "ca")
+        del en_feedback.REQUEST.translation_info
+
+        en_feedback.mandatory_feedback = "modified"
+        modified(en_feedback)
+        self.assertEqual(ca_feedback.mandatory_feedback, "modified")
+
+    def test_field_not_copied_if_not_modified(self):
+        en_feedback = self.portal.en["test-feedback"]
+        ca_feedback = api.translate(en_feedback, "ca")
+        del en_feedback.REQUEST.translation_info
+
+        # commit changes so far, so that we can check to see
+        # if the translation was updated
+        transaction.commit()
+        modified(en_feedback)
+        self.assertNotIn(ca_feedback, self.portal._p_jar._registered_objects)
+
 
 class TestLanguageIndependentRelationField(unittest.TestCase):