Issue with `id` after upgrading from 0.30.1 to 0.31.0

[radostyle]

I have this issue after upgrading from 0.30.1 to 0.31.0 or 0.31.2
The reads work, but my update is failing.

curl -X PATCH -d '{ "data":{ "type": "email", "id": "100","attributes":{"status": "S"} } }' -H 'Content-Type: application/vnd.api+json' 'localhost:5000/v1/emails/100' | jq .

Is anyone else having this issue? Or maybe there is a change with the new dependency libraries?

{
  "errors": [
    {
      "detail": "Unknown field.",
      "source": {
        "pointer": "/data/id"
      },
      "status": "422",
      "title": "Validation error"
    }
  ],
  "jsonapi": {
    "version": "1.0"
  }
}

[akira-dev]

The id field is included into the attributes of the object before schema validation. So it seems that your schema does not include an id field or your id field get the attribute dump_only ?

[moet78]

I also hit this issue, and I tried remove the "dump_only" from id field and still get the same issue. Is there other possible reason for this issue to happen?

[benjaminhallouin]

@akira-dev I also face the same issue.
It can be reproduced by using your example (in /examples/api.py) and performing the patch call:

PATCH /computers/1 HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
  "data": {
    "type": "computer",
    "id": "1",
    "attributes": {
      "serial": "Amstrad 2"
    }
  }
}

It seems that the only way to avoid this is to remove the dump_only=True on the id field from the computer schema, but in that case, we can update the id with a patch call... which we obviously don't want. ;-)

[fweep]

Came here to report this. This seems like a bug to me. I've been deep in the flask-rest-jsonapi/marshmallow-jsonapi/marshmallow code all day, and I finally found this in marshmallow-jsonapi's schema.py:

def unwrap_item(self, item):
    if "type" not in item:
        raise ma.ValidationError(
            [
                {
                    "detail": "`data` object must include `type` key.",
                    "source": {"pointer": "/data"},
                }
            ]
        )
    if item["type"] != self.opts.type_:
        raise IncorrectTypeError(actual=item["type"], expected=self.opts.type_)

    payload = self.dict_class()
    if "id" in item:
        payload["id"] = item["id"]
# etc

This seems incorrect to me. My schema has dump_only=True for id, because it should never be part of attributes during a POST/PUT (create/update). As noted above, removing dump_only "solves" the problem, but then requests can modify id, and this is never desirable under any circumstances. I can't imagine a scenario where id should be set as part of the payload.

Modifying the method above and commenting out the last two quoted lines fixes the problem, and at least in my application has no negative side-effect at all. I'm going to play with this a bit more to confirm, but I'm reasonably-confident at this point that it's a bug, and plan to fork marshmallow-jsonapi and/or flask-rest-jsonapi to fix it.

I'm not sure if the maintainers of either of these repos are active as there haven't been any changes for a few months.

[fweep]

I was able to work around this in a much simpler way via the following:

class FooSchema(Schema):
    class Meta:
        type_ = 'foo'
        self_view = 'foo_detail'
        self_view_kwargs = {'id': '<id>'}
        self_view_many = 'foo_list'

    # id = fields.UUID(as_string=True, dump_only=True)
    id = fields.UUID(as_string=True)
    name = fields.Str(required=True)


class FooDetail(ResourceDetail):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'PATCH']

    def before_patch(self, *args, **kwargs):
        if 'id' in kwargs['data']:
            del kwargs['data']['id']

Similarly in my FooList resource with a before_post method. It isn't strictly required, because my model simply won't accept an id during creation, but rather than have an unhandled exception from the data layer, I'm handling it like this:

class FooList(ResourceList):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'POST']

    def before_post(self, *args, **kwargs):
        if 'id' in kwargs['data']:
            raise BadRequest('')

This is much cleaner/less-hacky than my proposal above. Thankfully flask-rest-jsonapi includes these nice hooks for manipulating the data.

[Gitigi]

I was able to work around this in a much simpler way via the following:

class FooSchema(Schema):
    class Meta:
        type_ = 'foo'
        self_view = 'foo_detail'
        self_view_kwargs = {'id': '<id>'}
        self_view_many = 'foo_list'

    # id = fields.UUID(as_string=True, dump_only=True)
    id = fields.UUID(as_string=True)
    name = fields.Str(required=True)


class FooDetail(ResourceDetail):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'PATCH']

    def before_patch(self, *args, **kwargs):
        if 'id' in kwargs['data']:
            del kwargs['data']['id']

Similarly in my FooList resource with a before_post method. It isn't strictly required, because my model simply won't accept an id during creation, but rather than have an unhandled exception from the data layer, I'm handling it like this:

class FooList(ResourceList):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'POST']

    def before_post(self, *args, **kwargs):
        if 'id' in kwargs['data']:
            raise BadRequest('')

This is much cleaner/less-hacky than my proposal above. Thankfully flask-rest-jsonapi includes these nice hooks for manipulating the data.

Better fix would be to remove the id using pre_load, which is called before deserializing

from marshmallow import pre_load

class FooSchema(Schema):
    class Meta:
        type_ = 'foo'
        self_view = 'foo_detail'
        self_view_kwargs = {'id': '<id>'}
        self_view_many = 'foo_list'

    @pre_load
    def remove_id_before_deserializing(self, data, **kwargs):
        if 'id' in data:
            del data['id']
        return data

    id = fields.UUID(as_string=True, dump_only=True)
    name = fields.Str(required=True)


class FooDetail(ResourceDetail):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'PATCH']

class FooList(ResourceList):
    schema = FooSchema
    data_layer = {
        'session': db.session,
        'model': Foo,
    }
    methods = ['GET', 'POST']

[igieon]

I don't know but for me you can safely remove dump_only, because in code of ResourceDetail is this part

        if (str(json_data['data']['id']) != str(kwargs[getattr(self._data_layer, 'url_field', 'id')])):
            raise BadRequest('Value of id does not match the resource identifier in url',
                             source={'pointer': '/data/id'})

For POST in specification you can allow or dissallow set id.