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

Adds single table inheritance support for models #536

Merged
merged 2 commits into from
Jun 9, 2016

Conversation

jfinkels
Copy link
Owner

This commit adds support for fetching, creating, updating, and deleting
at endpoints corresponding to APIs created for polymorphic models
defined using single table inheritance.

This also changes the default serializer so that it supports
heterogeneous collections directly.

Fixes issue #200.

This is currently a work-in-progress: it needs documentation to explain what operations are supported. This could be an opportunity to create a special section of the documentation on "Association proxies, polymorphic tables, hybrid properties, etc."

@philliproso
Copy link

This is great. What can I do to get joined table inheritance

@jfinkels
Copy link
Owner Author

It may already work with this pull request, by pure luck, but there are no tests for that configuration so we don't know. If you want to help Flask-Restless support joined table inheritance, the first thing we need are unit tests, similar to the ones for single-table inheritance that you can see at tests/test_polymorphism.py in the current pull request.

In any case, I'd like to finish this pull request before merging in others for other polymorphism setups.

@philliproso
Copy link

To get this branch to work on joined table inheritance structures all I need do is remove the following two line. I have written test and will make a pull request once this branch is merged.

    # Exclude column names that are foreign keys.
    foreign_key_columns = foreign_keys(model)
    columns = (c for c in columns if c not in foreign_key_columns)

Maybe this is a bit late to enter into the discussion. But why are foreign_keys dropped from the model. I know it is likely to have something to do with the assumption that all foreign_keys will be linked to a relationship, and you 'should' be manipulating this via the relationship api. I found in v0.1x of flask-restless it was very useful to sometimes bypass relationships and just set and update the foreignkey. Plus sometimes in a databases (designed by other people) one has foreignkeys that are not just GUIDs or id's but actually represent something and therefore don't need a relationship to represent them.

I guess if you don't drop foreign_keys you need have a rule describing if the foreignkey or relationship takes precedence. In mind the foreign_key should take precedence.

That being said I can change those two lines of code to drop all foreignkeys that are not primary keys and submit a pull request that makes this compatible with joined table inheritance.

@philliproso
Copy link

line 302 of /flask-restless/flask_restless/serialization can be changed to
columns = (c for c in columns if c not in foreign_key_columns or c == primary_key_for(model))

@philliproso
Copy link

Referring to the previous comment, I see the json api spec says this.

Although has-one foreign keys (e.g. author_id) are often stored internally alongside other information to be represented in a resource object, these keys SHOULD NOT appear as attributes.

@jfinkels
Copy link
Owner Author

jfinkels commented Jun 5, 2016

As for the foreign keys, yes I'm following the JSON API specification "SHOULD NOT" statement. If you want to propose allowing foreign keys, you can open a separate issue. I think it may be possible to use the additional_attributes keyword argument to APIManager.create_api to expose foreign key IDs if desired.

As for joined table inheritance, I'll merge this shortly and give you the heads-up to submit a new pull request, if you'd like to do it.

This commit adds support for fetching, creating, updating, and deleting
at endpoints corresponding to APIs created for polymorphic models
defined using single table inheritance.

This also changes the default serializer so that it supports
heterogeneous collections directly.
This commit also adds some info about association proxies in the same
document.
@jfinkels jfinkels merged commit f91a6b9 into master Jun 9, 2016
@jfinkels jfinkels deleted the polymorphic-models branch June 9, 2016 02:48
@jfinkels
Copy link
Owner Author

jfinkels commented Jun 9, 2016

@philliproso I have merged this pull request. If you'd like to prepare a new one for joined table inheritance, feel free to do so whenever you like.

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

Successfully merging this pull request may close these issues.

None yet

2 participants