[autoform] Avoid two calls to field.process_form for the same field in some cases
authorSylvain Thénault <sylvain.thenault@logilab.fr>
Thu, 18 Feb 2016 14:22:07 +0100
changeset 11897 2ceb0bfa4b3f
parent 11892 08cf02efc7ce
child 11898 c5d3382f14e9
[autoform] Avoid two calls to field.process_form for the same field in some cases when some entity is being created and data include non-inlined relations, the values for this relation are now stored for later usage, avoiding two calls to field's process_form method, which may be unexpected for custom fields. This has been discovered in saem_ref#f5444b1f9770. Reorganize imports in the test along the way.
cubicweb/web/test/unittest_views_basecontrollers.py
cubicweb/web/views/editcontroller.py
--- a/cubicweb/web/test/unittest_views_basecontrollers.py	Thu Nov 24 15:36:26 2016 +0100
+++ b/cubicweb/web/test/unittest_views_basecontrollers.py	Thu Feb 18 14:22:07 2016 +0100
@@ -1,4 +1,4 @@
-# copyright 2003-2014 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
+# copyright 2003-2016 LOGILAB S.A. (Paris, FRANCE), all rights reserved.
 # contact http://www.logilab.fr/ -- mailto:contact@logilab.fr
 #
 # This file is part of CubicWeb.
@@ -27,22 +27,20 @@
 from logilab.common.testlib import unittest_main
 from logilab.common.decorators import monkeypatch
 
-from cubicweb import Binary, NoSelectableObject, ValidationError, AuthenticationError
+from cubicweb import Binary, NoSelectableObject, ValidationError, transaction as tx
 from cubicweb.schema import RRQLExpression
+from cubicweb.predicates import is_instance
 from cubicweb.devtools.testlib import CubicWebTC
 from cubicweb.devtools.webtest import CubicWebTestTC
 from cubicweb.devtools.httptest import CubicWebServerTC
 from cubicweb.utils import json_dumps
 from cubicweb.uilib import rql_for_eid
-from cubicweb.web import Redirect, RemoteCallFailed, http_headers
-import cubicweb.server.session
-from cubicweb.server.session import Connection
+from cubicweb.web import Redirect, RemoteCallFailed, http_headers, formfields as ff
 from cubicweb.web.views.autoform import get_pending_inserts, get_pending_deletes
 from cubicweb.web.views.basecontrollers import JSonController, xhtmlize, jsonize
 from cubicweb.web.views.ajaxcontroller import ajaxfunc, AjaxFunction
-import cubicweb.transaction as tx
+from cubicweb.server.session import Connection
 from cubicweb.server.hook import Hook, Operation
-from cubicweb.predicates import is_instance
 
 
 class ViewControllerTC(CubicWebTestTC):
@@ -617,6 +615,58 @@
             finally:
                 blogentry.__class__.cw_skip_copy_for = []
 
+    def test_avoid_multiple_process_posted(self):
+        # test that when some entity is being created and data include non-inlined relations, the
+        # values for this relation are stored for later usage, without calling twice field's
+        # process_form method, which may be unexpected for custom fields
+
+        orig_process_posted = ff.RelationField.process_posted
+
+        def count_process_posted(self, form):
+            res = list(orig_process_posted(self, form))
+            nb_process_posted_calls[0] += 1
+            return res
+
+        ff.RelationField.process_posted = count_process_posted
+
+        try:
+            with self.admin_access.web_request() as req:
+                gueid = req.execute('CWGroup G WHERE G name "users"')[0][0]
+                req.form = {
+                    'eid': 'X',
+                    '__type:X': 'CWUser',
+                    '_cw_entity_fields:X': 'login-subject,upassword-subject,in_group-subject',
+                    'login-subject:X': u'adim',
+                    'upassword-subject:X': u'toto', 'upassword-subject-confirm:X': u'toto',
+                    'in_group-subject:X': repr(gueid),
+                }
+                nb_process_posted_calls = [0]
+                self.expect_redirect_handle_request(req, 'edit')
+                self.assertEqual(nb_process_posted_calls[0], 1)
+                user = req.find('CWUser', login=u'adim').one()
+                self.assertEqual(set(g.eid for g in user.in_group), set([gueid]))
+                req.form = {
+                    'eid': ['X', 'Y'],
+                    '__type:X': 'CWUser',
+                    '_cw_entity_fields:X': 'login-subject,upassword-subject,in_group-subject',
+                    'login-subject:X': u'dlax',
+                    'upassword-subject:X': u'toto', 'upassword-subject-confirm:X': u'toto',
+                    'in_group-subject:X': repr(gueid),
+
+                    '__type:Y': 'EmailAddress',
+                    '_cw_entity_fields:Y': 'address-subject,use_email-object',
+                    'address-subject:Y': u'dlax@cw.org',
+                    'use_email-object:Y': 'X',
+                }
+                nb_process_posted_calls = [0]
+                self.expect_redirect_handle_request(req, 'edit')
+                self.assertEqual(nb_process_posted_calls[0], 3)  # 3 = 1 (in_group) + 2 (use_email)
+                user = req.find('CWUser', login=u'dlax').one()
+                self.assertEqual(set(e.address for e in user.use_email), set(['dlax@cw.org']))
+
+        finally:
+            ff.RelationField.process_posted = orig_process_posted
+
     def test_nonregr_eetype_etype_editing(self):
         """non-regression test checking that a manager user can edit a CWEType entity
         """
@@ -669,7 +719,6 @@
             self.assertEqual(e.title, '"13:03:40"')
             self.assertEqual(e.content, '"13:03:43"')
 
-
     def test_nonregr_multiple_empty_email_addr(self):
         with self.admin_access.web_request() as req:
             gueid = req.execute('CWGroup G WHERE G name "users"')[0][0]
--- a/cubicweb/web/views/editcontroller.py	Thu Nov 24 15:36:26 2016 +0100
+++ b/cubicweb/web/views/editcontroller.py	Thu Feb 18 14:22:07 2016 +0100
@@ -187,12 +187,13 @@
             req.transaction_data['__maineid'] = form['__maineid']
         # no specific action, generic edition
         self._to_create = req.data['eidmap'] = {}
-        # those two data variables are used to handle relation from/to entities
+        # those three data variables are used to handle relation from/to entities
         # which doesn't exist at time where the entity is edited and that
         # deserves special treatment
         req.data['pending_inlined'] = defaultdict(set)
         req.data['pending_others'] = set()
         req.data['pending_composite_delete'] = set()
+        req.data['pending_values'] = dict()
         try:
             for formparams in self._ordered_formparams():
                 self.edit_entity(formparams)
@@ -205,10 +206,20 @@
         # treated now (pop to ensure there are no attempt to add new ones)
         pending_inlined = req.data.pop('pending_inlined')
         assert not pending_inlined, pending_inlined
+        pending_values = req.data.pop('pending_values')
         # handle all other remaining relations now
         while req.data['pending_others']:
             form_, field = req.data['pending_others'].pop()
-            self.handle_formfield(form_, field)
+            # attempt to retrieve values and original values if they have already gone through
+            # handle_formfield (may not if there has been some not yet known eid at the first
+            # processing round). In the later case we've to go back through handle_formfield.
+            try:
+                values, origvalues = pending_values.pop((form_, field))
+            except KeyError:
+                self.handle_formfield(form_, field)
+            else:
+                self.handle_relation(form_, field, values, origvalues)
+        assert not pending_values, 'unexpected remaining pending values %s' % pending_values
         del req.data['pending_others']
         # then execute rql to set all relations
         for querydef in self.relations_rql:
@@ -330,6 +341,7 @@
                     self.handle_relation(form, field, value, origvalues)
                 else:
                     form._cw.data['pending_others'].add((form, field))
+                    form._cw.data['pending_values'][(form, field)] = (value, origvalues)
 
         except ProcessFormError as exc:
             self.errors.append((field, exc))