wlanslovenija / django-tastypie-mongoengine

MongoEngine support for django-tastypie.
Other
73 stars 59 forks source link

Document validation on full_hydrate #20

Closed leo-naeka closed 11 years ago

leo-naeka commented 12 years ago

Hi,

Since c9bedc3, Mongoengine's validate() method is called before returning the bundle. I understand that is used to handle validation errors and to return the proper error message, but this awkward when doing auto-allocation on required fields in document's save() method for example.

Won't the validation reporting be in a better place in obj_create() and obj_update() methods ? If you're agree, just say it and I'll make a pull request ;)

mitar commented 12 years ago

I am not sure anymore why I put that in full_hydrate. There is a comment, but from code I don't see that it is called before full_hydrate. Strange. Maybe this was before I implemented the rest in a more proper way. Anyway, there are quite some tests for this, so we can try to refactor it and we will see if errors are still from correct layer (from validation and not MongoEngine). I want fail-fast behavior here.

So I think you could try to move this code to is_valid (first call parent, then call this code). Then you can update obj_create and obj_update methods to call it, similar to how it is done here.

Please write tests for your use case.

leo-naeka commented 12 years ago

The fact is that tastypie doesn't care of models validation: neither full_clean() nor clean_fields() are called. I suppose that they encourage the use of Validation or FormValidation or even overriding the obj_create() and/or obj_update() methods and calling clean_fields() on the desired fields.

We can't check for Mongoengine's validate() on is_valid() since bundle.obj is not ready and this would anyway be blocking with auto-allocated fields in doc's save() as described before.

I think that wrapping object and related saves in obj_create() and obj_update() in a try/except would be a better way to handle validation errors... I'll do a pull request, it will be easier to discuss on this.

mitar commented 11 years ago

OK. That validation was really a hack and it broke even more with newer MongoEngine versions. I moved it as you suggested.

mitar commented 11 years ago

Thanks.