Schema generation overriding

[Igonato]

I'm happy overall with the auto generated schema and have no desire to write it by hand. Sometimes there are issues though and it would be nice if there was a way to override it partially.

This is a proof-of-concept solution for Fields, that adds schema argument. Here we tell it that PrimaryKeyRelatedField should have integer values and not String:

class CatSanctuarySerializer(ModelSerializer):

    favorite_cat = PrimaryKeyRelatedField(
        required=False,
        queryset=Cat.objects.all(),
        schema=coreschema.Integer(title='Cat.'),
    )

    cats = PrimaryKeyRelatedField(
        many=True,
        required=False,
        queryset=Cat.objects.all(),
        schema=coreschema.Array(
            items=coreschema.Integer(),
            title='Cats',
            description='My cats'
        ),
    )

    class Meta:
        model = MyCatSanctuary
        fields = 'id', 'favorite_cat', 'cats'

Should be useful in many other cases when SchemaGenerator doesn't quite cut it. What do you think?
Also, if something similar is implemented for Serializers, Views and ViewSets it should solve #4685

[carltongibson]

@Igonato Thanks for this. I'll have a look at this properly this week. Bottom line is we need to do something in this area, so all ideas are very helpful.

I'm happy overall with the auto generated schema and have no desire to write it by hand. Sometimes there are issues though and it would be nice if there was a way to override it partially.

Exactly. 🙂

[Igonato]

the comment is no longer relevant after #5337

I took a deeper look into the generator logic. As far as it's concerned a serializer is just a bunch of Fields, so not sure if there is really a use in overriding schemas on the serializer level if you can override fields, which this PR covers.

With views/endpoints, however, there are path arguments, pagination and filter fields involved, it can take a request argument and determine a bunch of stuff dynamically, so fine overriding gets complicated. My best suggestion is schema field / get_schema(self, schema_request=None) method for views (similar to serialzer_class/get_serializer_class), which can override get_links in the generator:

class CatViewSet(ModelViewSet):
    queryset = Cat.objects.all()
    serializer_class = CatSerializer

    schema = {
        'list': coreapi.Link(
            url='/',
            action='get',
            fields=[
                *schemas.get_serializer_fields(CatSerializer),
                coreapi.Field(
                    name='search',
                    required=False,
                    location='query',
                    description='Filter cats by name'
                )
            ]
            description='List all cats'
        ),
        # ...
    }

    def get_schema(self, schema_request=None):
        # do somehting with the schema_request, check
        # authentication, permissions, etc...
        return self.schema

I think it is possible to drop urls there and add generate just them with the schema generator.

---

Also, decorators like this should help document function based views:

SOME_SCHEMA = coreapi.Link(
    action='get',
    fields=[
        # ...
    ],
    # ...
)


def get_some_schema(request=None):
    return SOME_SCHEMA


@decorators.api_view(['GET'])
@decorators.schema(SOME_SCHEMA)
def some_function_based_view(request, format=None):

    return Response({
       # ...
    })


@decorators.api_view(['GET'])
@decorators.get_schema(get_some_schema)
def another_function_based_view(request, format=None):

    return Response({
       # ...
    })

Let me know if all this seems adequate, I'll gladly spend part of my weekend contributing to the project.

[Igonato]

Just saw #5337 +1 for "Migrate introspection logic from SchemaGenerator to descriptor class for APIView"

[carltongibson]

@Igonato I'll leave this open pending #5337. We'll roll what we can/need to from here into that once we have the descriptor in place. It maybe that just overriding get_serializer_fields gives us enough (but we'll see).

Function based views get turned into APIViews under the hood so I'm hoping that we'll just need a decorator (or an argument) to allow passing a custom descriptor-generator-class (name pending) there.

Follow-along: all feedback/input appreciated!

[carltongibson]

@Igonato Can you give #5354 a go to implement your needs here?

You'll need to subclass for now (kwargs will come later)

Let me know here you get on. We can discuss and move key points to #5354.

Thanks! Your experience will be really helpful.

[Igonato]

@carltongibson how about limiting this PR to the field-level overriding then? And #5354 will be a view-level solution.

It works as is for the field schema. I can add tests and documentation at the end of the week.

To clarify, current PR is enough for my needs, I was just looking for a way to adjust some fields schema. The reason I brought up the view-level overriding is that I encountered this question on SO about documenting non-model based views and two answers are, basically, change api classes to some that support serializers and write it by hand. It seems that there should be a better way

[carltongibson]

#5354 will cover all needs—the various fields, description, encoding etc.

My question here is for you to implement what you need — if I read right by overriding get_serializer_fields on the descriptor class — and report back how that went.

[Igonato]

My only need at the time is to change schema for one field. Overriding #5354 descriptor's get_serializer_fields works but is really cumbersome since it is view-specific and if the same serializer is used on multiple views I need to add the descriptor to all of the views and what if each view needs some other adjustment? Also, views may have multiple serializers with the use of get_serializer_class method and if so in the get_serializer_fields I first need to find out which serializer is it and does it have the field I want to alter, so no, it doesn't really work that well. For my need, the field-level schema adjustment is the way to go.

[carltongibson]

OK. Those points are all good.

The kind of one field case is exactly where we'd be looking to allow an amount of kwarg based specification. ("More complex needs? Override!")

On the multiple use case you'd use a custom base view class, setting the descriptor once there.

The multiple serialisers case is at the moment a known limitation. The logic we provide doesn't (yet) handle that at all. It's on the list. 🙂

I suspect we'll be reluctant to add to the serializers.Fields api in the core framework. If you want to add API to serialisers so that you can ask them for their schema fields that may well work for your project. (The whole point about the current batch of changes is to make that customisation possible.)

We'd be likely to steer you towards either adjusting the generated schema (for just the serialiser if that's your need) or having a manually specified schema returned instead. (This matches your later examples — the only differences then are where that would be specified, and how it would be fetched.)

I'm going to close this PR as is for now. It's out of scope of what we going to/able to add for "Out of the Box" in this cycle.

Once the schema refactoring drops (so it'll be 3.7 in all likelihood) we'll be interested, here and on the mailing list, in looking at different use-case and how they can be handled.

Thanks for your input! It's very informative.

[Igonato]

The kind of one field case is exactly where we'd be looking to allow an amount of kwarg based specification. ("More complex needs? Override!")

Sorry for being overly-annoying I'll just state one last time: the problem isn't kwargs or subclassing, but doing it in the view scope which normally isn't even aware of any serializer fields since they are managed by serializers. It really feels like a wrong place for overriding a field schema and there should be some separate, serializer-related place for it.

Thank you for your time, I'll try to keep my eye on the project and get back on this when 3.7 is under way

[carltongibson]

Sure. And maybe you’re right. (But do we then do the same for Paginators and ... etc?)

But right now we’re inspecting the serialiser to get its fields. In this next phase we’re going to go from a situation where that’s not customisable to one where it is.

We may change how we do things after that. (Of course we may not.)