remove usage of django's lazy function for error messages

[mehdipourfar]

In my benchmarks, using Django's lazy function inside __init__ method of serializers makes serialization process 3 times slower. This commit:

1. Removes usages of this function.
2. Postpone error formatting to the steps after raising ValidationError.
3. Keeps the code compatible for users who already overrided Field classes to have their own error messages.

[auvipy]

I believe the proposed change will be easier to review wit proper test cases. thanks for your work!

[mehdipourfar]

Hi @auvipy
There is already a lot of tests for error message formatting. I don't know what to test. This commit contains no new feature. It only does some refactors and optimization.

[mehdipourfar]

And I don't know why Travis tests have failed. I have ran the tests with Tox in my local computer and everything seemed working.

[kevin-brown]

I am in the process of restarting your Travis jobs since you appear to have hit a temporal issue that affected non-Python 2.7 builds.

[rpkilby]

This was probably related to the temporary compat issue with tox/tox-venv

(tox-venv only runs on Python 3 builds)

[kevin-brown]

I've figured out what the rest of this is doing, but I can't figure out what this line is supposed to be doing.

It's not clear where this parameter was being previously injected, or if it was, and I also don't see anything explaining what this is supposed to be doing.

[mehdipourfar]

Django's BaseValidator takes a limit_value and that value is used for formatting messages. For example, this is the message template of MaxValueValidator.
message = _('Ensure this value is less than or equal to %(limit_value)s.')

But in Rest Framework, Serializers use code property of Validators as keyword for error message templates. This is IntegerField error message, for example:
'Ensure this value is less than or equal to {max_value}.

This block of code extracts needed keyword from it's Validator classes and adds them to params dict for error formatting.

[kevin-brown]

So, just to confirm, this is an issue with the current DRF logic and it's just being fixed in this PR alongside the stated removal of lazy()?

[mehdipourfar]

Yes. Actually a performance issue.

[kevin-brown]

Given that this is new logic, this is where I lean towards having new tests to cover this new logic. It's clear that there are no current tests that cover it (otherwise they would be failing), and I'd hate to see a regression in the future because they are not covered.

I'd also love to see it broken out into a separate PR, so it can be independently reviewed and not hold up the general refactor that is happening here, but I'm not stuck on that.

[bluetech]

Hi,

I also noticed this slowness. I made a different fix: #6644.

The approach here is problematic, because Django's ValidationError expects the error message to be formatted with %, but this changes to use .format. This is fine for the built-in messages, but does not account for other validators which raise Django's ValidationError.

[rpkilby]

Hi @mehdipourfar. Thanks for going to the effort to implement this. I realize this is based off my suggestion in #5452 (comment), but I ultimately agree that it's flawed. Ideally, we'd switch all validation messages to be %-based to be consistent with Django, but the headache isn't worth it.