[IMP] convert PG errors through pg93 diag info

Previously availability of meta-information (failing constraint,
impacted table and column, …) in pg errors (e.g. constraint check
failure) was limited and only available through formatted error
messages, which could be localised (by PG itself) so not did getting
that information require string extraction it was brittle in the face of
localised instances.

Postgres 9.3 adds this meta-information to error diagnostic data
(`PQresultErrorField`), allowing easy programmatic access to it.
Psycopg2 [added a Diagnostic
object](http://initd.org/psycopg/docs/extensions.html#psycopg2.extensions.Diagnostics)
around the time pg9.3 was itself released.

Assuming Odoo now requires pg >= 9.3 we can remove the old string
munging extraction for diagnostic.

* funny story, the handling of 23505 didn't actually work because
  "duplicate key value violates unique constraint" is a literal part
  of the error message, I'd misunderstood "value" as a field name
  because my test model's field was called "value". Makes sense too
  since you can set UNIQUE constraints on multiple fields (and UNIQUE
  indices on expressions)
* for both errors, it should be possible to use the table_name or
  constraint metadata to provide messages about sub-model issues (a
  constraint on an m2o), but that's not currently handled

Fixes #15323
This commit is contained in:
xmo-odoo
2017-04-11 14:51:53 +02:00
committed by GitHub
parent a8994b4075
commit ca74b5e1c8
3 changed files with 80 additions and 31 deletions
+4
View File
@@ -3,6 +3,7 @@
from odoo import api, fields, models
def selection_fn(model):
return [(str(key), val) for key, val in enumerate(["Corge", "Grault", "Wheee", "Moog"])]
@@ -148,7 +149,10 @@ class OnlyOne(models.Model):
_name = 'export.unique'
value = fields.Integer()
value2 = fields.Integer()
value3 = fields.Integer()
_sql_constraints = [
('value_unique', 'unique (value)', "The value must be unique"),
('pair_unique', 'unique (value2, value3)', "The values must be unique"),
]
+34 -6
View File
@@ -3,6 +3,7 @@
import json
import pkgutil
import re
from odoo.tests import common
from odoo.tools.misc import mute_logger
@@ -1116,14 +1117,41 @@ class test_unique(ImporterCase):
])
self.assertFalse(result['ids'])
self.assertEqual(result['messages'], [
dict(message=u"The value for the field 'value' already exists. "
u"This might be 'Value' in the current model, "
u"or a field of the same name in an o2m.",
dict(message=u"The value for the field 'value' already exists "
u"(this is probably 'Value' in the current model).",
type='error', rows={'from': 1, 'to': 1},
record=1, field='value'),
dict(message=u"The value for the field 'value' already exists. "
u"This might be 'Value' in the current model, "
u"or a field of the same name in an o2m.",
dict(message=u"The value for the field 'value' already exists "
u"(this is probably 'Value' in the current model).",
type='error', rows={'from': 4, 'to': 4},
record=4, field='value'),
])
@mute_logger('odoo.sql_db')
def test_unique_pair(self):
result = self.import_(['value2', 'value3'], [
['0', '1'],
['1', '0'],
['1', '1'],
['1', '1'],
])
self.assertFalse(result['ids'])
self.assertEqual(len(result['messages']), 1)
message = result['messages'][0]
self.assertEqual(message['type'], 'error')
self.assertEqual(message['record'], 3)
self.assertEqual(message['rows'], {'from': 3, 'to': 3})
m = re.match(
r"The values for the fields '([^']+)' already exist "
r"\(they are probably '([^']+)' in the current model\)\.",
message['message']
)
self.assertIsNotNone(m)
self.assertItemsEqual(
m.group(1).split(', '),
['value2', 'value3']
)
self.assertItemsEqual(
m.group(2).split(', '),
['Value2', 'Value3']
)
+42 -25
View File
@@ -30,6 +30,7 @@ import operator
import pytz
import re
from collections import defaultdict, MutableMapping, OrderedDict
from contextlib import closing
from inspect import getmembers, currentframe
from operator import attrgetter, itemgetter
@@ -5090,45 +5091,61 @@ def itemgetter_tuple(items):
return lambda gettable: (gettable[items[0]],)
return operator.itemgetter(*items)
def convert_pgerror_23502(model, fields, info, e):
m = re.match(r'^null value in column "(?P<field>\w+)" violates '
r'not-null constraint\n',
tools.ustr(e))
field_name = m and m.group('field')
if not m or field_name not in fields:
def convert_pgerror_not_null(model, fields, info, e):
if e.diag.table_name != model._table:
return {'message': tools.ustr(e)}
message = _(u"Missing required value for the field '%s'.") % field_name
field = fields.get(field_name)
if field:
message = _(u"Missing required value for the field '%s' (%s)") % (field['string'], field_name)
field_name = e.diag.column_name
field = fields[field_name]
message = _(u"Missing required value for the field '%s' (%s)") % (field['string'], field_name)
return {
'message': message,
'field': field_name,
}
def convert_pgerror_23505(model, fields, info, e):
m = re.match(r'^duplicate key (?P<field>\w+) violates unique constraint',
tools.ustr(e))
field_name = m and m.group('field')
if not m or field_name not in fields:
def convert_pgerror_unique(model, fields, info, e):
# new cursor since we're probably in an error handler in a blown
# transaction which may not have been rollbacked/cleaned yet
with closing(model.env.registry.cursor()) as cr:
cr.execute("""
SELECT
conname AS "constraint name",
t.relname AS "table name",
ARRAY(
SELECT attname FROM pg_attribute
WHERE attrelid = conrelid
AND attnum = ANY(conkey)
) as "columns"
FROM pg_constraint
JOIN pg_class t ON t.oid = conrelid
WHERE conname = %s
""", [e.diag.constraint_name])
constraint, table, ufields = cr.fetchone() or (None, None, None)
# if the unique constraint is on an expression or on an other table
if not ufields or model._table != table:
return {'message': tools.ustr(e)}
message = _(u"The value for the field '%s' already exists.") % field_name
field = fields.get(field_name)
if field:
message = _(u"%s This might be '%s' in the current model, or a field "
u"of the same name in an o2m.") % (message, field['string'])
# TODO: add stuff from e.diag.message_hint? provides details about the constraint & duplication values but may be localized...
if len(ufields) == 1:
field_name = ufields[0]
field = fields[field_name]
message = _(u"The value for the field '%s' already exists (this is probably '%s' in the current model).") % (field_name, field['string'])
return {
'message': message,
'field': field_name,
}
field_strings = [fields[fname]['string'] for fname in ufields]
message = _(u"The values for the fields '%s' already exist (they are probably '%s' in the current model).") % (', '.join(ufields), ', '.join(field_strings))
return {
'message': message,
'field': field_name,
# no field, unclear which one we should pick and they could be in any order
}
PGERROR_TO_OE = defaultdict(
# shape of mapped converters
lambda: (lambda model, fvg, info, pgerror: {'message': tools.ustr(pgerror)}), {
# not_null_violation
'23502': convert_pgerror_23502,
# unique constraint error
'23505': convert_pgerror_23505,
'23502': convert_pgerror_not_null,
'23505': convert_pgerror_unique,
})
def _normalize_ids(arg, atoms=set(IdType)):