[forms] closes #2437859 - Detect and prevent concurrent edition of the same entity.
authorAnthony Truchet <anthony.truchet@logilab.fr>
Thu, 25 Apr 2013 10:16:25 +0200
changeset 10016 984505da8b89
parent 10015 57a16bef82c0
child 10017 58c7a075c793
[forms] closes #2437859 - Detect and prevent concurrent edition of the same entity. Add the timestamp of form generation to each entity's meta-information fields. On form validation, check that no concurrent change is overwritten and raises a ValidationError in case of concurrent change. A nicer handling with a message and a link to the new version of the entity would be a good thing...
i18n/de.po
i18n/en.po
i18n/es.po
i18n/fr.po
web/test/unittest_form.py
web/test/unittest_views_basecontrollers.py
web/views/editcontroller.py
web/views/forms.py
--- a/i18n/de.po	Wed Apr 09 16:58:58 2014 +0200
+++ b/i18n/de.po	Thu Apr 25 10:16:25 2013 +0200
@@ -483,6 +483,12 @@
 msgid "Entities"
 msgstr "Entitäten"
 
+#, python-format
+msgid ""
+"Entity %(eid)s has changed since you started to edit it. Reload the page and "
+"reapply your changes."
+msgstr ""
+
 msgid "Entity and relation supported by this source"
 msgstr ""
 
--- a/i18n/en.po	Wed Apr 09 16:58:58 2014 +0200
+++ b/i18n/en.po	Thu Apr 25 10:16:25 2013 +0200
@@ -461,6 +461,12 @@
 msgid "Entities"
 msgstr ""
 
+#, python-format
+msgid ""
+"Entity %(eid)s has changed since you started to edit it. Reload the page and "
+"reapply your changes."
+msgstr ""
+
 msgid "Entity and relation supported by this source"
 msgstr ""
 
--- a/i18n/es.po	Wed Apr 09 16:58:58 2014 +0200
+++ b/i18n/es.po	Thu Apr 25 10:16:25 2013 +0200
@@ -492,6 +492,12 @@
 msgid "Entities"
 msgstr "Entidades"
 
+#, python-format
+msgid ""
+"Entity %(eid)s has changed since you started to edit it. Reload the page and "
+"reapply your changes."
+msgstr ""
+
 msgid "Entity and relation supported by this source"
 msgstr "Entidades y relaciones aceptadas por esta fuente"
 
--- a/i18n/fr.po	Wed Apr 09 16:58:58 2014 +0200
+++ b/i18n/fr.po	Thu Apr 25 10:16:25 2013 +0200
@@ -4,7 +4,7 @@
 msgid ""
 msgstr ""
 "Project-Id-Version: cubicweb 2.46.0\n"
-"PO-Revision-Date: 2012-02-15 16:08+0100\n"
+"PO-Revision-Date: 2014-06-24 13:29+0200\n"
 "Last-Translator: Logilab Team <contact@logilab.fr>\n"
 "Language-Team: fr <contact@logilab.fr>\n"
 "Language: \n"
@@ -486,6 +486,12 @@
 msgid "Entities"
 msgstr "entités"
 
+#, python-format
+msgid ""
+"Entity %(eid)s has changed since you started to edit it. Reload the page and "
+"reapply your changes."
+msgstr "L'entité %(eid)s a été modifiée depuis votre demande d'édition. Veuillez recharger cette page et réappliquer vos changements."
+
 msgid "Entity and relation supported by this source"
 msgstr "Entités et relations supportés par cette source"
 
--- a/web/test/unittest_form.py	Wed Apr 09 16:58:58 2014 +0200
+++ b/web/test/unittest_form.py	Thu Apr 25 10:16:25 2013 +0200
@@ -16,7 +16,10 @@
 # You should have received a copy of the GNU Lesser General Public License along
 # with CubicWeb.  If not, see <http://www.gnu.org/licenses/>.
 
+import time
+
 from xml.etree.ElementTree import fromstring
+from lxml import html
 
 from logilab.common.testlib import unittest_main, mock_object
 
@@ -126,6 +129,24 @@
             self.assertIn('content_format', data)
 
 
+    def test_form_generation_time(self):
+        with self.admin_access.web_request() as req:
+            e = req.create_entity('BlogEntry', title=u'cubicweb.org', content=u"hop")
+            expected_field_name = '__form_generation_time:%d' % e.eid
+
+            ts_before = time.time()
+            form = self.vreg['forms'].select('edition', req, entity=e)
+            ts_after = time.time()
+
+            data = []
+            form.render(action='edit', w=data.append)
+            html_form = html.fromstring(''.join(data)).forms[0]
+            fields = dict(html_form.form_values())
+            self.assertIn(expected_field_name, fields)
+            ts = float(fields[expected_field_name])
+            self.assertTrue(ts_before < ts  < ts_after)
+
+
     # form tests ##############################################################
 
     def test_form_inheritance(self):
--- a/web/test/unittest_views_basecontrollers.py	Wed Apr 09 16:58:58 2014 +0200
+++ b/web/test/unittest_views_basecontrollers.py	Thu Apr 25 10:16:25 2013 +0200
@@ -23,7 +23,11 @@
     from urlparse import parse_qs as url_parse_query
 except ImportError:
     from cgi import parse_qs as url_parse_query
+
+import lxml
+
 from logilab.common.testlib import unittest_main
+
 from logilab.common.decorators import monkeypatch
 
 from cubicweb import Binary, NoSelectableObject, ValidationError
@@ -81,6 +85,49 @@
                 self.assertEqual({'login-subject': 'the value "admin" is already used, use another one'},
                                  cm.exception.errors)
 
+    def test_simultaneous_edition_only_one_commit(self):
+        """ Allow two simultaneous edit view of the same entity as long as only one commits
+        """
+        with self.admin_access.web_request() as req:
+            e = req.create_entity('BlogEntry', title=u'cubicweb.org', content=u"hop")
+            expected_path = e.rest_path()
+            req.cnx.commit()
+            form = self.vreg['views'].select('edition', req, rset=e.as_rset(), row=0)
+            html_form = lxml.html.fromstring(form.render(w=None, action='edit')).forms[0]
+
+        with self.admin_access.web_request() as req2:
+            form2 = self.vreg['views'].select('edition', req, rset=e.as_rset(), row=0)
+
+        with self.admin_access.web_request(**dict(html_form.form_values())) as req:
+            path, args = self.expect_redirect_handle_request(req, path='edit')
+            self.assertEqual(path, expected_path)
+
+    def test_simultaneous_edition_refuse_second_commit(self):
+        """ Disallow committing changes to an entity edited in between """
+        with self.admin_access.web_request() as req:
+            e = req.create_entity('BlogEntry', title=u'cubicweb.org', content=u"hop")
+            eid = e.eid
+            req.cnx.commit()
+            form = self.vreg['views'].select('edition', req, rset=e.as_rset(), row=0)
+            html_form = lxml.html.fromstring(form.render(w=None, action='edit')).forms[0]
+
+        with self.admin_access.web_request() as req2:
+            e = req2.entity_from_eid(eid)
+            e.cw_set(content = u"hip")
+            req2.cnx.commit()
+
+        form_field_name = "content-subject:%d" % eid
+        form_values = dict(html_form.form_values())
+        assert form_field_name in form_values
+        form_values[form_field_name] = u'yep'
+        with self.admin_access.web_request(**form_values) as req:
+            with self.assertRaises(ValidationError) as cm:
+                self.ctrl_publish(req)
+            reported_eid, dict_info = cm.exception.args
+            self.assertEqual(reported_eid, eid)
+            self.assertIn(None, dict_info)
+            self.assertIn("has changed since you started to edit it.", dict_info[None])
+
     def test_user_editing_itself(self):
         """checking that a manager user can edit itself
         """
--- a/web/views/editcontroller.py	Wed Apr 09 16:58:58 2014 +0200
+++ b/web/views/editcontroller.py	Thu Apr 25 10:16:25 2013 +0200
@@ -22,6 +22,8 @@
 from warnings import warn
 from collections import defaultdict
 
+from datetime import datetime
+
 from logilab.common.deprecation import deprecated
 from logilab.common.graph import ordered_nodes
 
@@ -266,6 +268,7 @@
         if eid is None: # creation or copy
             entity.eid = eid = self._insert_entity(etype, formparams['eid'], rqlquery)
         elif rqlquery.edited: # edition of an existant entity
+            self.check_concurrent_edition(formparams, eid)
             self._update_entity(eid, rqlquery)
         if is_main_entity:
             self.notify_edited(entity)
@@ -362,6 +365,23 @@
         else:
             self._cw.set_message(self._cw._('entity deleted'))
 
+
+    def check_concurrent_edition(self, formparams, eid):
+        req = self._cw
+        try:
+            form_ts = datetime.fromtimestamp(float(formparams['__form_generation_time']))
+        except KeyError:
+            # Backward and tests compatibility : if no timestamp consider edition OK
+            return
+        if req.execute("Any X WHERE X modification_date > %(fts)s, X eid %(eid)s",
+                       {'eid': eid, 'fts': form_ts}):
+            # We only mark the message for translation but the actual
+            # translation will be handled by the Validation mechanism...
+            msg = _("Entity %(eid)s has changed since you started to edit it."
+                    " Reload the page and reapply your changes.")
+            # ... this is why we pass the formats' dict as a third argument.
+            raise ValidationError(eid, {None: msg}, {'eid' : eid})
+
     def _action_apply(self):
         self._default_publish()
         self.reset()
--- a/web/views/forms.py	Wed Apr 09 16:58:58 2014 +0200
+++ b/web/views/forms.py	Thu Apr 25 10:16:25 2013 +0200
@@ -44,8 +44,11 @@
 
 __docformat__ = "restructuredtext en"
 
+
 from warnings import warn
 
+import time
+
 from logilab.common import dictattr, tempattr
 from logilab.common.decorators import iclassmethod, cached
 from logilab.common.textutils import splitstrip
@@ -349,7 +352,9 @@
         self.uicfg_affk = self._cw.vreg['uicfg'].select(
             'autoform_field_kwargs', self._cw, entity=self.edited_entity)
         self.add_hidden('__type', self.edited_entity.cw_etype, eidparam=True)
+
         self.add_hidden('eid', self.edited_entity.eid)
+        self.add_generation_time()
         # mainform default to true in parent, hence default to True
         if kwargs.get('mainform', True) or kwargs.get('mainentity', False):
             self.add_hidden(u'__maineid', self.edited_entity.eid)
@@ -363,6 +368,11 @@
             msgid = self._cw.set_redirect_message(msg)
             self.add_hidden('_cwmsgid', msgid)
 
+    def add_generation_time(self):
+        # NB repr is critical to avoid truncation of the timestamp
+        self.add_hidden('__form_generation_time', repr(time.time()),
+                        eidparam=True)
+
     def add_linkto_hidden(self):
         """add the __linkto hidden field used to directly attach the new object
         to an existing other one when the relation between those two is not