Fix includes end excludes for subresources

[o3bvv]

Hello!

I have experienced same problem as described at #211.

That issue tells that if we have API like /resource/<:id>/subresource then all fields of subresource are exposed. It seems like includes and excludes which were defined for subresource are not applied.

However, the problem is more complicated. Actually, what we have is that includes and excludes of resource are applied for subresource. This is a terrible issue, because to_dict can try to include fields/methods/relations which are not defines for subresource. And this will just result into HTTP 500.

This patch resolves the issue. I'm not sure this is done in a perfect way, so we can make a discussion if needed.

[o3bvv]

Oops, sorry. The build has failed.

[elvismacak]

it's bug cause the the update of sqlalchemy1.0.2
i rollback to 0.9.8 in commit elvismacak@0f13fc1 and that works, you may pull-request again(also rollback the sqlalchemy)

[o3bvv]

I think there is no need for this pull request currently. I figured out that with current implementation we loose the ability to create multiple endpoints for one resource via manager.create_api().

For example, you have a User and Book models where each book has a FK relation with user.

Then you create an endpoint for User with full list of columns exposed. It's OK. If you want to create a lite endpoint with less columns exposed, than we have a problem: currently we store mapping of views for a single model via dict inside API manager, e.g. Model -> View. So, later we can ask a view to tell us about includes and excludes. But, obviously, if we use dict and a Model as a key, then only the last created view will be stored in our mapping and that's bad.

It may seem to be OK to use something else from dict, or at least to store views as a list. But this makes things worse: when you serialize a book which has a user, then we will need to choose which API view to ask for includes and excludes.

And that's a big deal. I even not sure if this can be resolved or resolved without major architectural changes.

Please, tell me if you have some thoughts on this.

[elvismacak]

yeah， i thought about another problem.

as the example you described, we have two users, user-1 has book-1, book-2, book-3. user-2 has book-4.

/api/user ==> list all the the user
/api/user/1 ==> get the user 1 and also expose the books it has (book-1, book-2, book-3)
/api/user/1/books ==> list the books it has (book-1, book-2, book-3)
/api/user/1books/3 ==> get the book-3
/api/user/1books/4 ==> get the book-4 this is really unacceptable

I have read the code, and found it hard or unable to resolve these, temparly, i prefer to disable the subresouce query.

also, your thoghts are on the same point: how to expose the subresource.

i thought about yours.
let's assume

• we have user, author, book.
• book has two FK to user and author
• query /api/book will include and exclude book columns in style-A
• query /api/user/1 will include and exclude book columns in style-B
• query /api/auhtor/1 will include and exclude book columns in style-C
• query /api/user/1/books/1 will include and exclude book columns in style-D
• query /api/auhtor/1/books/1 will include and exclude book columns in style-E

generally speaking, style-D is the same with style-B, style-E is the same with style-C, here we will talk about style-A, B, C

i think a better logic or solution is：

1. if the book is in resource book query mode: use style-A
2. if the book is in resource user query mode: use style-B
3. if the book is in resource author query mode: use style-C
   like the picture below:
   

that requires each api-blueprint will handle its own dict for how to expose the subresource, if it's none, use the self.manager._created_apis you defined.

it's too long, help it'll help you, and i enjoy it very much as you do

[o3bvv]

Oh, thank you.

It's getting a bit complicated. I need some time to think about this and see what can be done.

[o3bvv]

Well, I've thought about this a little.

First of all, I'll need to remove last commits from this pull request and extract some of them into separate pull requests or just to create a new one.

Second, it seems to be Ok the following idea: instead of guessing on something it's better to set things explicitly. And here is what I mean.

I suppose it will be OK to allow user to pass serializer argument for create_api(), where serializer is a callable and it accepts 1 parameter: an object to be serialized. So, user will be able to use include_columns+include_methods OR exclude_columns OR serializer (honestly, it would be great to leave only serializer but backward compatibility may not allow to do so).

Currently we can create serializers using partial, e.g.:

from functools import partial
from flask.ext.restless.helpers import to_dict


USER_INCLUDE_COLUMNS = ['id', 'login', 'name', 'email', 'role', 'active', ]

user_serializer = partial(to_dict, include=USER_INCLUDE_COLUMNS)

And then we will be able to use it:

api_manager.create_api(
    model=User,
    collection_name='users',
    methods=['GET', 'POST', 'DELETE', 'PUT', ],
    serializer=user_serializer,
)

The cool thing about this is that we can extract serializers into a separate module and then reuse them. And not only for views. And this is really great.

This leads us to the following idea: we can define a mapping "collection name" -> "serializer" for subresourses, e.g.:

api_manager.create_api(
    model=ReasouceA,
    collection_name='a_resouces',
    serializers={
        'b_resouces': resource_b_serializer,
        'c_resouces': resource_c_serializer,
    }
)

Or we can define an universal parameter for setting up serializers, e.g.:

api_manager.create_api(
    model=ResouceA,
    collection_name='a_resouces',
    serializers={
        'a_resouces': resource_a_serializer,
        'a_resouces.b_resouces': resource_b_serializer,
        'a_resouces.c_resouces': resource_c_serializer,
        'a_resouces.c_resouces.d_resouces': resource_d_serializer,
    }
)

In case ResouceA has some ResourceX as subresource and serializers does not define serializer for it, then default (current) approach can be used.

Please, let me know about your thoughts on that. It will be cool if we can communicate in some more direct way, e.g. via Skype, Gitter.im or so.

[elvismacak]

sorry, i'm busy these days.
hope discuss with you this weekend, thx

[jfinkels]

On the master branch, the correct serializers are now called for each type of resource; see http://flask-restless.readthedocs.org/en/latest/customizing.html#per-model-serialization

If you can't solve your problem using that, please open a new issue, as the code has recently changed significantly. Thanks!