Commits

Anonymous committed 9515899

[svn r9912] XmlRpcPlugin: Added a ticket '_ts' token to detect mid-air collisions for ticket.update() calls.

Big thanks to stp for raising this issue and providing the patch + lots of new ticket tests to verify main ticket functionality.

Closes #5402

  • Participants
  • Parent commits 7c38495

Comments (0)

Files changed (3)

 
 setup(
     name='TracXMLRPC',
-    version='1.1.1',
+    version='1.1.2',
     license='BSD',
     author='Alec Thomas',
     author_email='alec@swapoff.org',

trunk/tracrpc/tests/ticket.py

         self.assertTrue(justnow <= changes[3][0])
         self.admin.ticket.delete(tid)
 
+    def test_update_non_existing(self):
+        try:
+            self.admin.ticket.update(3344, "a comment", {})
+            self.fail("Allowed to update non-existing ticket???")
+            self.admin.ticket.delete(3234)
+        except Exception, e:
+            self.assertTrue("Ticket 3344 does not exist." in str(e))
+
+    def test_update_basic(self):
+        import time
+        # Basic update check, no 'action' or 'time_changed'
+        tid = self.admin.ticket.create('test_update_basic1', 'ieidnsj', {
+                        'owner': 'osimons'})
+        # old-style (deprecated)
+        self.admin.ticket.update(tid, "comment1", {'component': 'component2'})
+        self.assertEquals(2, len(self.admin.ticket.changeLog(tid)))
+        # new-style with 'action'
+        time.sleep(1) # avoid "columns ticket, time, field are not unique"
+        self.admin.ticket.update(tid, "comment2", {'component': 'component1',
+                                                   'action': 'leave'})
+        self.assertEquals(4, len(self.admin.ticket.changeLog(tid)))
+        self.admin.ticket.delete(tid)
+
+    def test_update_time_changed(self):
+        # Update with collision check
+        import datetime
+        from tracrpc.xml_rpc import from_xmlrpc_datetime, to_xmlrpc_datetime
+        tid = self.admin.ticket.create('test_update_time_changed', '...', {})
+        tid, created, modified, attrs = self.admin.ticket.get(tid)
+        then = from_xmlrpc_datetime(modified) - datetime.timedelta(minutes=1)
+        # Unrestricted old-style update (to be removed soon)
+        try:
+            self.admin.ticket.update(tid, "comment1",
+                    {'_ts': to_xmlrpc_datetime(then)})
+        except Exception, e:
+            self.assertTrue("Ticket has been updated since last get" in str(e))
+        # Update with 'action' to test new-style update.
+        try:
+            self.admin.ticket.update(tid, "comment1",
+                    {'_ts': to_xmlrpc_datetime(then),
+                     'action': 'leave'})
+        except Exception, e:
+            self.assertTrue("modified by someone else" in str(e))
+        self.admin.ticket.delete(tid)
+
+    def test_update_time_same(self):
+        # Update with collision check
+        import datetime
+        from tracrpc.xml_rpc import from_xmlrpc_datetime, to_xmlrpc_datetime
+
+        # Unrestricted old-style update (to be removed soon)
+        tid = self.admin.ticket.create('test_update_time_same', '...', {})
+        tid, created, modified, attrs = self.admin.ticket.get(tid)
+        ts = attrs['_ts']
+        self.admin.ticket.update(tid, "comment1",
+                    {'_ts': ts})
+        self.admin.ticket.delete(tid)
+
+        # Update with 'action' to test new-style update.
+        tid = self.admin.ticket.create('test_update_time_same', '...', {})
+        tid, created, modified, attrs = self.admin.ticket.get(tid)
+        ts = attrs['_ts']
+        self.admin.ticket.update(tid, "comment1",
+                    {'_ts': ts, 'action': 'leave'})
+        self.admin.ticket.delete(tid)
+
+    def test_update_action(self):
+        # Updating with 'action' in attributes
+        tid = self.admin.ticket.create('test_update_action', 'ss')
+        current = self.admin.ticket.get(tid)
+        self.assertEqual('', current[3].get('owner', ''))
+        updated = self.admin.ticket.update(tid, "comment1",
+                {'action': 'reassign',
+                 'action_reassign_reassign_owner': 'user'})
+        self.assertEqual('user', updated[3].get('owner'))
+        self.admin.ticket.delete(tid)
+
+    def test_update_action_non_existing(self):
+        # Updating with non-existing 'action' in attributes
+        tid = self.admin.ticket.create('test_update_action_wrong', 'ss')
+        try:
+            self.admin.ticket.update(tid, "comment1",
+                {'action': 'reassign',
+                 'action_reassign_reassign_owner': 'user'})
+        except Exception, e:
+            self.assertTrue("invalid action" in str(e))
+        self.admin.ticket.delete(tid)
+
+    def test_update_field_non_existing(self):
+        tid = self.admin.ticket.create('test_update_field_non_existing', 'yw3')
+        try:
+            self.admin.ticket.update(tid, "comment1",
+                    {'does_not_exist': 'eiwrjoer'})
+        except Exception, e:
+            self.assertTrue("no such column" in str(e))
+        self.admin.ticket.delete(tid)
+
 
 class RpcTicketVersionTestCase(TracRpcTestCase):
     

trunk/tracrpc/ticket.py

         """ Fetch a ticket. Returns [id, time_created, time_changed, attributes]. """
         t = model.Ticket(self.env, id)
         req.perm(t.resource).require('TICKET_VIEW')
+        t['_ts'] = str(t.time_changed)
         return (t.id, t.time_created, t.time_changed, t.values)
 
     def create(self, req, summary, description, attributes={}, notify=False, when=None):
 
     def update(self, req, id, comment, attributes={}, notify=False, author='', when=None):
         """ Update a ticket, returning the new ticket in the same form as
-        getTicket(). Requires a valid 'action' in attributes to support workflow.
-        Overriding 'author' and 'when' requires admin permission. """
+        get(). 'New-style' call requires two additional items in attributes:
+        (1) 'action' for workflow support (including any supporting fields
+        as retrieved by getActions()),
+        (2) '_ts' changetime token for detecting update collisions (as received
+        from get() or update() calls).
+        ''Calling update without 'action' and '_ts' changetime token is
+        deprecated, and will raise errors in a future version.'' """
         t = model.Ticket(self.env, id)
         # custom author?
         if author and not (req.authname == 'anonymous' \
             self.log.warning("Rpc ticket.update for ticket %d by user %s " \
                     "has no workflow 'action'." % (id, req.authname))
             req.perm(t.resource).require('TICKET_MODIFY')
+            time_changed = attributes.pop('_ts', None)
+            if time_changed and str(time_changed) != str(t.time_changed):
+                raise TracError("Ticket has been updated since last get().")
             for k, v in attributes.iteritems():
                 t[k] = v
             t.save_changes(author, comment, when=when)
         else:
             ts = TicketSystem(self.env)
             tm = TicketModule(self.env)
+            # TODO: Deprecate update without time_changed timestamp
+            time_changed = str(attributes.pop('_ts', t.time_changed))
             action = attributes.get('action')
             avail_actions = ts.get_available_actions(req, t)
             if not action in avail_actions:
             # TicketModule reads req.args - need to move things there...
             req.args.update(attributes)
             req.args['comment'] = comment
-            req.args['ts'] = str(t.time_changed) # collision hack...
+            req.args['ts'] = time_changed
             changes, problems = tm.get_ticket_changes(req, t, action)
             for warning in problems:
                 add_warning(req, "Rpc ticket.update: %s" % warning)