Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deserializer throws away fields named type and links #559

Closed
raiskila opened this issue Jul 10, 2016 · 6 comments
Closed

Deserializer throws away fields named type and links #559

raiskila opened this issue Jul 10, 2016 · 6 comments
Labels

Comments

@raiskila
Copy link

If my SQLAlchemy models have a field called type, and I pass a value for it in the attributes hash in a JSON API request, the strings_to_datetimes function inside helpers.py throws it away prior to updating the SQLAlchemy model object.

@AlmightyOatmeal
Copy link

AlmightyOatmeal commented Jul 10, 2016

@janneraiskila, what version of flask-restless are you using?

string_to_datetime() builds a dictionary where-as string_to_datetime() does the dirty work.

Can you define "throws it away"? Is there a value missing or is it a NoneType? Can you provide a little more information on any errors you get, steps to reproduce, data you're having trouble with, anything other than "it doesn't work"?

@raiskila
Copy link
Author

Hi @AlmightyOatmeal,

I'm using HEAD. Notice that the list comprehension inside strings_to_datetimes has an if clause which causes items to be removed from the list. That's the issue.

@AlmightyOatmeal
Copy link

AlmightyOatmeal commented Jul 10, 2016

@janneraiskila, list comprehension is not used in strings_to_datetimes(); dictionary comprehension, via sets, is:

return dict((k, string_to_datetime(model, k, v)) for k, v in dictionary.items() if k not in ('type', 'links'))

If you look at the output JSON, "type" and "links" are being used in the resulting JSON. I will see if it is necessary to exclude those fields from the dictionary parsing.

@raiskila
Copy link
Author

raiskila commented Jul 10, 2016

Hi @AlmightyOatmeal,

That is a Python list comprehension generator expression and its output value is passed to the dict constructor.

Here's a reproduction gist modified from the "Quickstart" tutorial code on the documentation site.
https://gist.github.com/janneraiskila/7873c6903c863248ec83bce1607e01a8

Just attempt to POST to /api/person, e.g.:

{
  "data": {
    "type": "person",
    "attributes": {
      "name": "John",
      "birth_date": "2016-01-01",
      "type": 5
    }
  }
}

@AlmightyOatmeal
Copy link

AlmightyOatmeal commented Jul 10, 2016

@janneraiskila, that gist and output similar to this would have saved a lot of time in the beginning. I've recreated the error:

╭─jamie@BeefCurtainMoses  ~
╰─$ curl -X POST -H 'Accept: application/vnd.api+json' -H 'Content-Type: application/vnd.api+json' 'http://127.0.01:5000/api/person' -d '{
  "data": {
    "type": "person",
    "attributes": {
      "name": "John",
      "birth_date": "2016-01-01",
      "type": 5
    }
  }
}'
{
  "errors": [
    {
      "code": null,
      "detail": "(sqlite3.IntegrityError) NOT NULL constraint failed: person.type [SQL: u'INSERT INTO person (name, birth_date, type) VALUES (?, ?, ?)'] [parameters: (u'John', '2016-01-01', None)]",
      "id": null,
      "links": null,
      "meta": null,
      "source": null,
      "status": 400,
      "title": "Integrity Error"
    }
  ],
  "jsonapi": {
    "version": "1.0"
  },
  "meta": {}
}
╭─jamie@BeefCurtainMoses  ~
╰─$ curl -X GET -H 'Accept: application/vnd.api+json' -H 'Content-Type: application/vnd.api+json' 'http://127.0.01:5000/api/person'
{
  "data": [
    {
      "attributes": {
        "birth_date": "2016-01-01",
        "name": "John",
        "type": 5
      },
      "id": "1",
      "links": {
        "self": "http://127.0.01:5000/api/person/1"
      },
      "type": "person"
    }
  ],
  "included": [],
  "jsonapi": {
    "version": "1.0"
  },
  "links": {
    "first": "http://127.0.01:5000/api/person?page[number]=1&page[size]=10",
    "last": "http://127.0.01:5000/api/person?page[number]=1&page[size]=10",
    "next": "http://127.0.01:5000/api/person?page[number]=2&page[size]=10",
    "prev": "http://127.0.01:5000/api/person?page[number]=0&page[size]=10",
    "self": "/api/person"
  },
  "meta": {
    "total": 1
  }
}

Looking through every instance where strings_to_datetimes() is called and it would only affect attributes. Referencing the JSON API specifications, there should not be a JSON API "type" reference within an "attributes" object (http://jsonapi.org/format/#document-resource-object-attributes) so I see no reason why "type" is being omitted.

@janneraiskila, I would recommend a sqlite URI of just 'sqlite://' to create an in-memory database for the sake if testing so there aren't random database files laying about :)

AlmightyOatmeal added a commit to AlmightyOatmeal/flask-restless that referenced this issue Jul 10, 2016
* Fixes jfinkels#559 regarding omission of 'type' field from attributes object. No tests failed due to this change.
* Changed list comprehension wrapped in dict() constructor with dict comprehension.
* Changed comprehension variables to something more human-readable.
AlmightyOatmeal added a commit to AlmightyOatmeal/flask-restless that referenced this issue Jul 10, 2016
* Fixes jfinkels#559 by removing the included portion instead of trying to map the entire table.
AlmightyOatmeal added a commit to AlmightyOatmeal/flask-restless that referenced this issue Jul 10, 2016
* Fixes jfinkels#559 by removing the included portion instead of trying to map the entire table.
@jfinkels jfinkels added the bug label Aug 16, 2016
@jfinkels
Copy link
Owner

This should be fixed by pull request #569.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants