FIX: ModelViews can have Resources whose models have unique fields.

ReadModelMixin and UpdateModelMixin store model instance as a property. This allows ModelResource to bind the ModelForm using the model instance making the form validate the input data against the model instance and not a brand new instance. When the latter happened and the model used unique fields, the form validation failed whenever a PUT was maintaining the previuos value of the unique field.

2 files changed, 39 insertions, 31 deletions

diff --git a/djangorestframework/mixins.py b/djangorestframework/mixins.py
index 1b3aa241..3d74ae19 100644
--- a/djangorestframework/mixins.py
+++ b/djangorestframework/mixins.py
@@ -491,17 +491,17 @@ class ReadModelMixin(object):
         try:
             if args:
                 # If we have any none kwargs then assume the last represents the primrary key
-                instance = model.objects.get(pk=args[-1], **kwargs)
+                self.model_instance = model.objects.get(pk=args[-1], **kwargs)
             else:
                 # Otherwise assume the kwargs uniquely identify the model
                 filtered_keywords = kwargs.copy()
                 if BaseRenderer._FORMAT_QUERY_PARAM in filtered_keywords:
                     del filtered_keywords[BaseRenderer._FORMAT_QUERY_PARAM]
-                instance = model.objects.get(**filtered_keywords)
+                self.model_instance = model.objects.get(**filtered_keywords)
         except model.DoesNotExist:
             raise ErrorResponse(status.HTTP_404_NOT_FOUND)
 
-        return instance
+        return self.model_instance
 
 
 class CreateModelMixin(object):
@@ -540,19 +540,19 @@ class UpdateModelMixin(object):
         try:
             if args:
                 # If we have any none kwargs then assume the last represents the primrary key
-                instance = model.objects.get(pk=args[-1], **kwargs)
+                self.model_instance = model.objects.get(pk=args[-1], **kwargs)
             else:
                 # Otherwise assume the kwargs uniquely identify the model
-                instance = model.objects.get(**kwargs)
+                self.model_instance = model.objects.get(**kwargs)
 
             for (key, val) in self.CONTENT.items():
-                setattr(instance, key, val)
+                setattr(self.model_instance, key, val)
         except model.DoesNotExist:
-            instance = model(**self.CONTENT)
-            instance.save()
+            self.model_instance = model(**self.CONTENT)
+            self.model_instance.save()
 
-        instance.save()
-        return instance
+        self.model_instance.save()
+        return self.model_instance
 
 
 class DeleteModelMixin(object):
diff --git a/djangorestframework/resources.py b/djangorestframework/resources.py
index b42bd952..be361ab8 100644
--- a/djangorestframework/resources.py
+++ b/djangorestframework/resources.py
@@ -177,14 +177,12 @@ class FormResource(Resource):
 
         # Return HTTP 400 response (BAD REQUEST)
         raise ErrorResponse(400, detail)
-  
 
-    def get_bound_form(self, data=None, files=None, method=None):
+
+    def get_form_class(self, method=None):
         """
-        Given some content return a Django form bound to that content.
-        If form validation is turned off (:attr:`form` class attribute is :const:`None`) then returns :const:`None`.
+        Returns the form class used to validate this resource.
         """
-
         # A form on the view overrides a form on the resource.
         form = getattr(self.view, 'form', None) or self.form
 
@@ -200,6 +198,16 @@ class FormResource(Resource):
             form = getattr(self, '%s_form' % method.lower(), form)
             form = getattr(self.view, '%s_form' % method.lower(), form)
 
+        return form
+  
+
+    def get_bound_form(self, data=None, files=None, method=None):
+        """
+        Given some content return a Django form bound to that content.
+        If form validation is turned off (:attr:`form` class attribute is :const:`None`) then returns :const:`None`.
+        """
+        form = self.get_form_class(method)
+
         if not form:
             return None
 
@@ -306,31 +314,31 @@ class ModelResource(FormResource):
         If the :attr:`form` class attribute has been explicitly set then that class will be used
         to create the Form, otherwise the model will be used to create a ModelForm.
         """
+        form = self.get_form_class(method)
 
-        form = super(ModelResource, self).get_bound_form(data, files, method=method)
-        
-        # Use an explict Form if it exists
-        if form:
-            return form
-
-        elif self.model:
+        if not form and self.model:
             # Fall back to ModelForm which we create on the fly
             class OnTheFlyModelForm(forms.ModelForm):
                 class Meta:
                     model = self.model
                     #fields = tuple(self._model_fields_set)
 
-            # Instantiate the ModelForm as appropriate
-            if data and isinstance(data, models.Model):
-                # Bound to an existing model instance
-                return OnTheFlyModelForm(instance=content)
-            elif data is not None:
-                return OnTheFlyModelForm(data, files)
-            return OnTheFlyModelForm()
+            form = OnTheFlyModelForm
 
         # Both form and model not set?  Okay bruv, whatevs...
-        return None
-    
+        if not form:
+            return None
+
+        # Instantiate the ModelForm as appropriate
+        if data is not None or files is not None:
+            if issubclass(form, forms.ModelForm) and hasattr(self.view, 'model_instance'):
+                # Bound to an existing model instance
+                return form(data, files, instance=self.view.model_instance)
+            else:
+                return form(data, files)
+
+        return form()
+
 
     def url(self, instance):
         """