Skip to content

Commit

Permalink
Make constraint error details contain useful information for develope…
Browse files Browse the repository at this point in the history
…rs (#6796)
  • Loading branch information
aljazerzen authored Feb 19, 2024
1 parent 2ff0542 commit 8c0e5f9
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 47 deletions.
36 changes: 18 additions & 18 deletions edb/schema/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,31 +218,32 @@ def get_subject(self, schema: s_schema.Schema) -> ConsistencySubject:
self.get_field_value(schema, 'subject'),
)

def format_error(
def format_error_message(
self,
schema: s_schema.Schema,
) -> str:
subject = self.get_subject(schema)
titleattr = subject.get_annotation(schema, sn.QualName('std', 'title'))

if not titleattr:
subjname = subject.get_shortname(schema)
subjtitle = subjname.name
title_ann = subject.get_annotation(schema, sn.QualName('std', 'title'))
if title_ann:
subject_name = title_ann
else:
subjtitle = titleattr
short_name = subject.get_shortname(schema)
subject_name = short_name.name

return self.format_error_message(schema, subjtitle)
return self.format_error_text(schema, subject_name)

def format_error_message(
def format_error_text(
self,
schema: s_schema.Schema,
subjtitle: str,
subject_name: str,
) -> str:
errmsg: Optional[str] = self.get_errmessage(schema)
text = self.get_errmessage(schema)
assert text
args = self.get_args(schema)
if args:
args_ql: List[qlast.Base] = [
qlast.Path(steps=[qlast.ObjectRef(name=subjtitle)]),
qlast.Path(steps=[qlast.ObjectRef(name=subject_name)]),
]

args_ql.extend(arg.qlast for arg in args)
Expand All @@ -264,10 +265,9 @@ def format_error_message(
args_map = {name: edgeql.generate_source(val, pretty=False)
for name, val in index_parameters.items()}
else:
args_map = {'__subject__': subjtitle}
args_map = {'__subject__': subject_name}

assert errmsg is not None
return interpolate_errmessage(errmsg, args_map)
return interpolate_error_text(text, args_map)

def as_alter_delta(
self,
Expand Down Expand Up @@ -1578,7 +1578,7 @@ def _get_bases_for_ast(
return ()


def interpolate_errmessage(message: str, args: Dict[str, str]) -> str:
def interpolate_error_text(text: str, args: Dict[str, str]) -> str:
"""
Converts message template "hello {world}! {nope}{{world}}" and
arguments {"world": "Alice", "hell": "Eve"}
Expand All @@ -1589,8 +1589,8 @@ def interpolate_errmessage(message: str, args: Dict[str, str]) -> str:

formatted = ""
last_start = 0
for match in re.finditer(regex, message, flags=0):
formatted += message[last_start : match.start()]
for match in re.finditer(regex, text, flags=0):
formatted += text[last_start : match.start()]
last_start = match.end()

if match[1] is None:
Expand All @@ -1603,5 +1603,5 @@ def interpolate_errmessage(message: str, args: Dict[str, str]) -> str:
# arg not found
formatted += match[0]

formatted += message[last_start:]
formatted += text[last_start:]
return formatted
2 changes: 1 addition & 1 deletion edb/schema/scalars.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ def resolve_sql_type(
) -> Optional[str]:
type, scheme = self.resolve_sql_type_scheme(schema)
if scheme:
return constraints.interpolate_errmessage(
return constraints.interpolate_error_text(
scheme,
{
f'__arg_{i}__': v
Expand Down
12 changes: 9 additions & 3 deletions edb/server/compiler/errormech.py
Original file line number Diff line number Diff line change
Expand Up @@ -620,10 +620,16 @@ def _interpret_constraint_errors(
obj_ptr = obj_type.getptr(schema, sn.UnqualName('id'))
constraint = obj_ptr.get_exclusive_constraints(schema)[0]

msg = constraint.format_error(schema)
# msg is for the "end user" that should not mention pointers and object
# type it is also affected by setting `errmessage` in user schema.
msg = constraint.format_error_message(schema)

# details is for the "developer" that must explain what's going on
# under the hood. It contains verbose descriptions of object involved.
subject = constraint.get_subject(schema)
vname = subject.get_verbosename(schema, with_parent=True)
details = constraint.format_error_message(schema, vname)
subject_description = subject.get_verbosename(schema, with_parent=True)
constraint_description = constraint.get_verbosename(schema)
details = f'violated {constraint_description} on {subject_description}'

if from_graphql:
msg = gql_replace_type_names_in_text(msg)
Expand Down
12 changes: 1 addition & 11 deletions edb/testbase/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -763,21 +763,11 @@ async def assertRaisesRegexTx(self, exception, regex, msg=None, **kwargs):
"""A version of assertRaisesRegex with automatic transaction recovery
"""

with super().assertRaisesRegex(exception, regex, msg=msg):
with super().assertRaisesRegex(exception, regex, msg=msg, **kwargs):
try:
tx = self.con.transaction()
await tx.start()
yield
except BaseException as e:
if isinstance(e, exception):
for attr_name, expected_val in kwargs.items():
val = getattr(e, attr_name)
if val != expected_val:
raise self.failureException(
f'{exception.__name__} context attribute '
f'{attr_name!r} is {val} (expected '
f'{expected_val!r})') from e
raise
finally:
await tx.rollback()

Expand Down
51 changes: 37 additions & 14 deletions tests/test_constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1132,8 +1132,7 @@ async def test_constraints_ddl_04(self):
qry = r"""
CREATE ABSTRACT CONSTRAINT mymax3(max: std::int64) {
SET errmessage :=
'{__subject__} must be no longer ' ++
'than {max} characters.';
'My custom ' ++ 'message.';
USING (__subject__ <= max);
};
Expand All @@ -1148,14 +1147,26 @@ async def test_constraints_ddl_04(self):

# making sure the constraint was applied successfully
async with self.assertRaisesRegexTx(
edgedb.ConstraintViolationError,
'foo must be no longer than 3 characters.',
edgedb.ConstraintViolationError, 'My custom message.'
):
await self.con.execute("""
INSERT ConstraintOnTest3 {
foo := 'Test'
};
""")
try:
await self.con.execute(
"""
INSERT ConstraintOnTest3 {
foo := 'Test'
};
"""
)
except edgedb.ConstraintViolationError as e:
# edgedb-python does not expose a nicer access to details field
details = e._attrs[2].decode("utf-8")
self.assertEqual(
details,
"violated constraint 'default::mymax3' on "
"property 'foo' of "
"object type 'default::ConstraintOnTest3'",
)
raise

# testing interpolation
await self.con.execute(r"""
Expand All @@ -1168,12 +1179,24 @@ async def test_constraints_ddl_04(self):
};
""")
async with self.assertRaisesRegexTx(
edgedb.ConstraintViolationError,
'{"json": "{nope} {min} 4"}',
edgedb.ConstraintViolationError, '{"json": "{nope} {min} 4"}'
):
await self.con.execute("""
INSERT ConstraintOnTest4_2 { email := '' };
""")
try:
await self.con.execute(
"""
INSERT ConstraintOnTest4_2 { email := '' };
"""
)
except edgedb.ConstraintViolationError as e:
# edgedb-python does not expose a nicer access to details field
details = e._attrs[2].decode("utf-8")
self.assertEqual(
details,
"violated constraint 'std::min_len_value' on "
"property 'email' of "
"object type 'default::ConstraintOnTest4_2'",
)
raise

async def test_constraints_ddl_05(self):
# Test that constraint expression returns a boolean.
Expand Down

0 comments on commit 8c0e5f9

Please sign in to comment.