ColtonProvias / sqlalchemy-jsonapi

JSONAPI implementation for use with SQLAlchemy
MIT License
71 stars 27 forks source link

Patch_relationship is not raising BadRequestError when payload data is not array of dictionaries #55

Open kaitj91 opened 7 years ago

kaitj91 commented 7 years ago

It is very important that the library is able to return some sort of response that let's the user know they submitted an invalid request by raising a BadRequestError and returning a 400 and some sort of description explaining why.

Through testing patch_relationship, I found that we are not raising a BadRequestError when we send a payload where data is not an array of dictionaries.

For example:

       def test_patch_relationship_with_invalid_data_format(self):
        """Patch relationship when data is not an array of dictionaries returns 400.

        A BadRequestError is raised.
        """
        user = models.User(
            first='Sally', last='Smith',
            password='password', username='SallySmith1')
        self.session.add(user)
        blog_post = models.Post(
            title='This Is A Title', content='This is the content',
            author_id=user.id, author=user)
        self.session.add(blog_post)
        comment = models.Comment(
            content='This is comment 1', author_id=user.id,
            post_id=blog_post.id, author=user, post=blog_post)
        self.session.add(comment)
        self.session.commit()
        payload = {
            'data': ['foo']
        }

        with self.assertRaises(errors.BadRequestError) as error:
            models.serializer.patch_relationship(
                self.session, payload, 'posts', blog_post.id, 'comments')

        self.assertEqual(error.exception.detail, 'data must be an array of objects')
        self.assertEqual(error.exception.status_code, 400)

This test fails because there is no check for this in place in patch_relationship. Thus when I try to run this test a TypeError occurs. Specifically the TypeError is occuring because we are failing to check

if not isinstance(item, dict):
    raise BadRequestError('data must be an array of objects')

at the beginning of the for loop below:

        for item in json_data['data']:
            to_relate = self._fetch_resource(
                session, item['type'], item['id'], Permissions.EDIT)

Because we do not have this check a TypeError occurs because we are trying to look up item['type'] and because it is a list we would need to look up by integer indices and not strings. This is an easy fix that would provide an informative error message to users who send badly formatted payloads.