[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...
--- 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