django-uuidfield
django-uuidfield copied to clipboard
dcramer
Fixed #46 and #49: get_db_prep_value() returns arbitrary UUID hex for badly-formatted ...
...UUIDs, and uniformly returns hex representation
Throwing ValueError or TypeError as discussed in #46 is not consistent with how Django ORM interface, which raises ObjectDoesNotExist for get and evaluates to empty list for filter. It suggests, method get_db_prep_value() should return a non-existing value, where an arbitrary value is the best candidate.
Sorry, I should have commented here instead of https://github.com/dcramer/django-uuidfield/issues/49. I don't understand how raising a ValueError is not consistent with the Django ORM interface. Could you explain? And how is the fact that the UUID field is an auto field related to this?
@ksonbol get_db_prep_value() itself is not expected to raise an exception. This method links Python interface of object.get(), object.filter() and object.save() to underlying database value, not user provided query/update parameter to underlying database value, which is handled by Django form or Django REST framework serializers. Querying SomeModel.objects.filter(boolean_field=255), for example, will be treated as boolean_field=True instead of throwing an error.
I was wrong about the auto field... the test test_raises_exception expects an IntegrityError because there is no null=True set on ManualUUIDField. null=True should be supported and get_db_prep_value should not return a null value when the UUID is invalid.
Calling SomeModel.objects.get(id="string") will cause get_prep_value to raise a ValueError. So I think it's normal to raise a ValueError there, and maybe I should move the code to get_prep_value since it is not directly related to a specific database.
@ksonbol Yes it does. Django's way of handling invalid value is to write URL regex that excludes all non-integers from getting into the ORM interface by returning 404. URL exclusion may be too complex for the case of UUID, but it can work.
Actually disputation on designing interfaces is the most tricky issue as it is mostly a matter of coding taste ;)
I pulled in https://github.com/dcramer/django-uuidfield/pull/48 as it was more concise in resolving one of these issues. If you want to rebase to cover the other explicit issue I'll take a look.
