encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.4k stars 6.84k forks source link

Using SerializerMethodField with many=True on CreateView results in error #6084

Open matteing opened 6 years ago

matteing commented 6 years ago

Checklist

Steps to reproduce

When issuing a POST request from the Rest Framework UI to an endpoint that allows lists (many=True) to be created, if the serializer has a MethodField, the object passes as dictionary and errors with 'collections.OrderedDict'.

Background

class TaskSerializer(serializers.ModelSerializer):
    user = UserSerializer(
        read_only=True,
        default=serializers.CurrentUserDefault()
    )
    project_set = ProjectSerializer(many=True, read_only=True)
    praise = serializers.SerializerMethodField(read_only=True)
    user_praised = serializers.SerializerMethodField(read_only=True)

    def get_praise(self, obj):
        # BUG WORKAROUND
        # Hack otherwise breaks /smart/. I don't know why. Rest framework bug.
        if isinstance(obj, Task):
            return obj.praise_amount
        else:
            return 0

    def get_user_praised(self, obj):
       # Unmodified bug case
        request = self.context.get('request', False)
        if request and request.user.is_authenticated:
            return obj.user_praise_amount(request.user)
        else:
            return None

    @staticmethod
    def setup_eager_loading(queryset):
        """ Perform necessary eager loading of data. """
        # select_related for "to-one" relationships
        queryset = queryset.select_related('user')

        # prefetch_related for "to-many" relationships
        queryset = queryset.prefetch_related(
            'project_set',
            'project_set__user')

        return queryset

    class Meta:
        model = Task
        fields = (
            'id',
            'event',
            'done',
            'content',
            'created_at',
            'updated_at',
            'done_at',
            'user',
            'project_set',
            'praise',
            'user_praised',
            'attachment',
        )
        read_only_fields = (
            'event',
            'done_at',
        )

View code:

class SmartTaskView(CreateAPIView):
    serializer_class = TaskSerializer

    def create(self, request, *args, **kwargs):
        serializer = self.get_serializer(data=request.data, many=True)
        serializer.is_valid(raise_exception=True)
        self.perform_create(serializer)
        headers = self.get_success_headers(serializer.data)
        for t in serializer.data:
            process_task_tags(Task.objects.get(pk=t['id']), request.user)
        return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)

Expected behavior

On regular JS post requests using axios and JS arrays, the POST request works as expected, adding everywhere from 1 to unlimited tasks. It returns 201 and the Task object with correct fields.

It is expected for the Rest Framework UI to do the same, yet it does not.

Working request body: screen shot 2018-07-11 at 11 15 51 pm

Actual behavior

An AttributeError occurs because a dict is passed to MethodField instead of an instance, the latter being expected behavior. This does not happen when using Many=True on GET views.

AttributeError at /smart/
'collections.OrderedDict' object has no attribute 'user_praise_amount'

screen shot 2018-07-11 at 11 10 02 pm

matteing commented 6 years ago

Any help on this?

rpkilby commented 6 years ago

Hi @matteing, the full stack trace would be helpful in pinpointing where the exception is occurring.

matteing commented 6 years ago

Absolutely, I've included it below:

Environment:

Request Method: POST
Request URL: http://localhost:8000/smart/

Django Version: 2.0.1
Python Version: 3.6.5
Installed Applications:
['django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'rest_framework',
 'rest_framework.authtoken',
 'corsheaders',
 'django_q',
 'makerlog',
 'stats',
 'accounts',
 'pages',
 'projects',
 'integrations',
 'memoize',
 'django_extensions',
 'invites',
 'sensors',
 'anymail',
 'notifications',
 'discussions']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'corsheaders.middleware.CorsMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'makerlog.middleware.timezone.TimezoneMiddleware']

Traceback:

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/django/core/handlers/exception.py" in inner
  35.             response = get_response(request)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  158.                 response = self.process_exception_by_middleware(e, request)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/django/core/handlers/base.py" in _get_response
  156.                 response = response.render()

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/django/template/response.py" in render
  106.             self.content = self.rendered_content

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/response.py" in rendered_content
  72.         ret = renderer.render(self.data, accepted_media_type, context)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/renderers.py" in render
  724.         context = self.get_context(data, accepted_media_type, renderer_context)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/renderers.py" in get_context
  697.             'post_form': self.get_rendered_html_form(data, view, 'POST', request),

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/renderers.py" in get_rendered_html_form
  520.             return self.render_form_for_serializer(serializer)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/renderers.py" in render_form_for_serializer
  528.             serializer.data,

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/serializers.py" in data
  560.         ret = super(Serializer, self).data

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/serializers.py" in data
  264.                 self._data = self.to_representation(self.validated_data)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/serializers.py" in to_representation
  527.                 ret[field.field_name] = field.to_representation(attribute)

File "/Users/sergio/.virtualenvs/makerlog-server-B0FiQlpH/lib/python3.6/site-packages/rest_framework/fields.py" in to_representation
  1855.         return method(value)

File "/Users/sergio/Projects/makerlog-server/projects/serializers.py" in get_user_praised
  55.             return obj.user_praise_amount(request.user)

Exception Type: AttributeError at /smart/
Exception Value: 'collections.OrderedDict' object has no attribute 'user_praise_amount'
rpkilby commented 6 years ago

If you disable the many flag and create a single object, do you receive the same error?

matteing commented 6 years ago

@rpkilby Indeed, error does disappear - although it kills the desired functionality, which is posting arrays of objects from JS.

The error does not occur when posting through JS though, strangely. Only through the browsable interface.

rpkilby commented 6 years ago

Of course - I just wanted to do a quick sanity check that this wasn't a general usage issue, and was related to the many usage.

matteing commented 6 years ago

Any updates?

tomchristie commented 6 years ago

The error does not occur when posting through JS though, strangely. Only through the browsable interface.

Indeed. I guess the first thing to do here would be for us to figure out if it's always been an issue as long as we've supported bulk-create, or if the issue has been introduced along the way.

tomchristie commented 6 years ago

Any updates?

If anyone wants to help progress this, I'd suggest:

  1. Reducing the issue down to the simplest possible case. (Single field. Ideally just Serializer rather than ModelSerializer.)
  2. Figuring out which versions it applies to.
carltongibson commented 6 years ago

Looks to me like this could never have worked.

The Browsable API is rendering the form from the serialised data (which is a collections.OrderedDict) and not an object instance.

This kind of thing always happens with the Browsable API — we get to a certain complexity and we're asking it to do too much...

bufpal commented 6 years ago

I had same problem... so I had to seperate to two serializers and two views. One is for the get request(fields with SerializerMethodField), the other one is for the post request.