Trap IntegrityError when adding and renaming items on ticket admin

[Ryan J Ollos]

When attempting to add an item that differs from an existing item only by whitespace, the error shown below results. For example, if attempting to add milestone5 , but milestone5 already exists, the traceback results. We should trap this condition and raise the TracError, which is the typical situation when adding an existing item.

How to Reproduce

While doing a POST operation on /admin/ticket/milestones, Trac issued an internal error.

(please provide additional details here)

Request parameters:

{'__FORM_TOKEN': u'366d21614f5d1c6c88ba9da3',
 'add': u'Add',
 'cat_id': u'ticket',
 'duedate': u'11/18/2013 12:00:00 AM',
 'name': u'  milestone5  ',
 'panel_id': u'milestones',
 'path_info': None}

User agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/30.0.1599.114 Chrome/30.0.1599.114 Safari/537.36

System Information

Trac	1.1.2dev
Genshi	0.8dev (without speedups)
pysqlite	2.3.2
Python	2.5.6 (r256:88840, Oct 28 2013, 23:50:06) [GCC 4.7.3]
setuptools	0.6
SQLite	3.7.15.2
jQuery	1.8.3

Enabled Plugins

Python Traceback

Traceback (most recent call last):
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/web/main.py", line 497, in _dispatch_request
    dispatcher.dispatch(req)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/web/main.py", line 214, in dispatch
    resp = chosen_handler.process_request(req)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/admin/web_ui.py", line 125, in process_request
    path_info)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/ticket/admin.py", line 53, in render_admin_panel
    return self._render_admin_panel(req, cat, page, version)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/ticket/admin.py", line 297, in _render_admin_panel
    mil.insert()
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/ticket/model.py", line 1091, in insert
    to_utimestamp(self.completed), self.description))
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/db/util.py", line 65, in execute
    return self.cursor.execute(sql_escape_percent(sql), args)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/db/sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/db/sqlite_backend.py", line 56, in execute
    args or [])
  File "/home/user/Workspace/t_component_owner/teo-rjollos.git/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
IntegrityError: column name is not unique

[Ryan J Ollos]

Also there are some issues mentioned in comment:2:ticket:11351 and comment:4:ticket:11297 that we may want to explore in this ticket.

[Ryan J Ollos]

Issue reported in ​SO:23956175.

[Ryan J Ollos]

See related comment:5:ticket:11351.

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[strk@…]

I just got hit by this issue too, annoying indeed as it's not easy to see there's a space in the name (dirname is unquoted in trac-admin error message too)

[Jun Omae]

Replying to Ryan J Ollos:

When attempting to add an item that differs from an existing item only by whitespace, the error shown below results. For example, if attempting to add milestone5 , but milestone5 already exists, the traceback results. We should trap this condition and raise the TracError, which is the typical situation when adding an existing item.

In trac/ticket/admin.py, we trap IntegrityError on saving an item but have not trapped on adding an item for milestone, component, version, enum record.

trac/ticket/admin.py

@@ -286 +286 @@
                             mil.due = user_time(req, parse_date,
                                                 req.args.get('duedate'),
                                                 hint='datetime')
-                        mil.insert()
-                        add_notice(req, _('The milestone "%(name)s" has been '
-                                          'added.', name=name))
+                        try:
+                            mil.insert()
+                        except self.env.db_exc.IntegrityError:
+                            raise TracError(_('Milestone "%(name)s" already '
+                                              'exists.', name=name))
+                        else:
+                            add_notice(req, _('The milestone "%(name)s" has '
+                                              'been added.', name=name))
                     else:
                         if mil.name is None:
                             raise TracError(_('Invalid milestone name.'))

Proposed changes in [a18914a42/jomae.git] (jomae.git@t11371).

[Ryan J Ollos]

comment:11 changes look good. For trunk, we'll need to catch ResourceExistsError (#11419).

[Jun Omae]

Revised proposed changes:

1.0-stable	[47fd9b5fd/jomae.git] (jomae.git@t11371_1.0)
1.2-stable	[37fc02f18/jomae.git] (jomae.git@t11371_1.2)
trunk	[a0dd4b9c9/jomae.git] (jomae.git@t11371_trunk)

[Ryan J Ollos]

The comment:13 changes look good to me.

[Jun Omae]

Thanks for the reviewing. Applied in [16502] and merged in [16503-16504].

[Ryan J Ollos]

Created #13011 to handle some outstanding issues that were original in description.

[Ryan J Ollos]

Added some trac.ticket.model documentation on trunk in r16507.