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

Schema Generation for WagtailPagesAPIViewSet #141

Closed
uroybd opened this issue Aug 12, 2020 · 13 comments
Closed

Schema Generation for WagtailPagesAPIViewSet #141

uroybd opened this issue Aug 12, 2020 · 13 comments
Labels
low priority issues that would be nice to fix but have low impact

Comments

@uroybd
Copy link

uroybd commented Aug 12, 2020

I tried the extended method mentioned in the doc for Wagtail's PagesAPIViewSet with no success.

Here's the error:

Error #3: Exception raised while getting serializer from Fixed. Hint: Is get_serializer_class() returning None or is get_queryset() not working without a request? Ignoring the view for now. (Exception: 'NoneType' object has no attribute 'GET')

I browsed the code and tested with some loggers. In Fix class I'm not getting any self.request which is very important for Wagtail. In fact, everything falls apart if you don't get the request!

@tfranzel
Copy link
Owner

hi! i'm in the process of generalizing the mocked request already used in the special case of API versioning. i suppose it would address your issue.

in the meantime, if you use @extend_schema "thoroughly", that method should never be called. however, there has to be a complete mapping with request/response and all actions to ensure it is not called. it is rather easy to miss a case and produce this issue.

@uroybd
Copy link
Author

uroybd commented Aug 17, 2020

hi! i'm in the process of generalizing the mocked request already used in the special case of API versioning. i suppose it would address your issue.

Mocked request is the way to go I guess.

About @extend_schema, well I liked the extensions better since you don't need to subclass every time. I also want to create another package (or contribute to this one if you don't mind) for wagtail's schema generation.

Funny thing is, in case of the actual schema generation we don't even need the serializer or request in case of wagtail Pages. It has to be a polymorphic schema and can be generated using the model's APIFields.

@tfranzel
Copy link
Owner

hey @uroybd! i implemented mock requests. with a little bit of luck your issue may be solved with it. can you test it and let me know?

@uroybd
Copy link
Author

uroybd commented Sep 12, 2020

Great! I'll test it in next 1-2day(s) and will let you know

@tfranzel
Copy link
Owner

tfranzel commented Sep 13, 2020

i forgot to address one point. of course contributions are welcome! depending on the required changes, you can either contribute to

@tfranzel
Copy link
Owner

@uroybd i had a quick look into what wagtail is doing. they customize the request as they come in (attach wagtailapi_router as attribute to the request). i think it makes sense to hook into mock request generation, which i already prepared. after that, most things look fixable with extensions.

@uroybd
Copy link
Author

uroybd commented Sep 14, 2020

@uroybd i had a quick look into what wagtail is doing. they customize the request as they come in (attach wagtailapi_router as attribute to the request). i think it makes sense to hook into mock request generation, which i already prepared. after that, most things look fixable with extensions.

Can you give me an example of mock request usage? By far I can just hook extensions without errors which is surely an improvement.

Another question, Does OpenAPIViewExtension work only with APIView or ViewSet also?

@tfranzel
Copy link
Owner

tfranzel commented Sep 14, 2020

we introduced global mock requests a few commits ago. i just made it configurable (on master). i should have figured that it wouldn't take long before customization would come up.

Another question, Does OpenAPIViewExtension work only with APIView or ViewSet also?

that has not come up yet. viewsets by necessity provide more information for introspection so it was not an issue yet. got to check if ViewExtensions also work there. it may need a slight change, but we'll get to that.

i quickly put together an incomplete example. there are still exceptions going on. not exactly yet sure what the reason is. anyways just something to get you also started.

# settings.py
SPECTACULAR_SETTINGS = {
    'GET_MOCK_REQUEST': 'mysite.urls.build_wagtail_mock_request'
}
# urls.py
from drf_spectacular.plumbing import build_mock_request

# load extensions
from mysite.schema import *

api_router = WagtailAPIRouter('wagtailapi')

def build_wagtail_mock_request(method, path, view, original_request, **kwargs):
    # default mock request
    request = build_mock_request(method, path, view, original_request, **kwargs)
    # the added wagtail magic
    request.wagtailapi_router = api_router
    return request
# schema.py
from drf_spectacular.extensions import OpenApiSerializerFieldExtension, OpenApiViewExtension
from drf_spectacular.plumbing import build_basic_type, build_array_type
from drf_spectacular.types import OpenApiTypes


class Fix1(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.api.v2.serializers.DetailUrlField'

    def map_serializer_field(self, auto_schema, direction):
        return build_basic_type(OpenApiTypes.URI)


class Fix2(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.api.v2.serializers.TagsField'

    def map_serializer_field(self, auto_schema, direction):
        return build_array_type(build_basic_type(OpenApiTypes.STR))


class Fix3(Fix1):
    target_class = 'wagtail.images.api.v2.serializers.ImageDownloadUrlField'


class Fix4(Fix1):
    target_class = 'wagtail.documents.api.v2.serializers.DocumentDownloadUrlField'


class Fix5(Fix1):
    target_class = 'wagtail.api.v2.serializers.PageHtmlUrlField'


class Fix6(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.api.v2.serializers.TypeField'

    def map_serializer_field(self, auto_schema, direction):
        return build_basic_type(OpenApiTypes.STR)


class Fix7(Fix6):
    target_class = 'wagtail.api.v2.serializers.PageTypeField'


class Fix8(OpenApiViewExtension):
    target_class = 'wagtail.api.v2.views.BaseAPIViewSet'
    match_subclasses = True

    def view_replacement(self):
        class Fixed(self.target):
            def get_object(self):
                return self.get_queryset().model()

        return Fixed


class Fix9(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.admin.api.serializers.PageChildrenField'

    def map_serializer_field(self, auto_schema, direction):
        return build_object_type(
            properties={
                'count': build_basic_type(OpenApiTypes.INT),
                'listing_url': build_basic_type(OpenApiTypes.URI),
            }
        )


class Fix10(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.admin.api.serializers.PageStatusField'

    def map_serializer_field(self, auto_schema, direction):
        return build_object_type(
            properties={
                'status': build_basic_type(OpenApiTypes.STR),
                'live': build_basic_type(OpenApiTypes.BOOL),
                'has_unpublished_changes': build_basic_type(OpenApiTypes.BOOL),
            }
        )


class Fix11(OpenApiSerializerFieldExtension):
    target_class = 'wagtail.images.api.fields.ImageRenditionField'

    def map_serializer_field(self, auto_schema, direction):
        return build_object_type(
            properties={
                'url': build_basic_type(OpenApiTypes.URI),
                'width': build_basic_type(OpenApiTypes.INT),
                'height': build_basic_type(OpenApiTypes.INT),
                'alt': build_basic_type(OpenApiTypes.STR),
            }
        )

@tfranzel
Copy link
Owner

had the wrong issue number in commit: 8856ab7 add the mock request function

@tfranzel
Copy link
Owner

i have a few warnings left, which are quite puzzling. wagtail is doing something really strange regarding module modification i think. they somehow inject their serializers in the rest_framework namespace/module. here is a list of things i noticed.

  • they use action name 'listing_view' instead of the default 'list'
  • they modify the rest_framework namespace unexpectedly (at least that is what i think is going on). stuff like rest_framework.serializers.PageSerializer does not actually exist there.
    • this leads to component collisions that are actually fine (false warnings)
    • OpenApiSerializerExtension cannot register for the correct class so it stays dormant.
  • spectacular schema endpoints break because request is passed through and that request does not contain the "router" modificaiton. so only CLI works acceptably.

the schema already looks somewhat acceptable, but the warnings are not that easy to squash.

@uroybd
Copy link
Author

uroybd commented Sep 16, 2020

It actually has a PageSerializer but that is too dynamic and context-aware to get a strict output. I think generating a schema for documents and images should be easier while Pages will be a pain. A very important thing is, no single request is enough to get the actual schema for the pages. It should be a collection of various page types. Here you can find an alternate approach to get data from wagtail page models of instead which I think, will be of less hassle. But, of course, not ideal. However, Wagtail is not an ideal API provider too. https://gitlab.com/codesigntheory/drftypegen/-/blob/master/drftypegen/adapters.py

@tfranzel
Copy link
Owner

as i dove in deeper i noticed that too. it is quite "meta". feel free to contribute a blueprint for wagtail if you like, though i'm not sure how and if it is helpful in practice. though currently i don't see much that can be improved in the core to improve wagtail detection.

@tfranzel tfranzel added the low priority issues that would be nice to fix but have low impact label Sep 17, 2020
@tfranzel
Copy link
Owner

tfranzel commented Oct 6, 2020

closing due to missing action points and being stale. feel free to come back to this if you like and maybe do a blueprint. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low priority issues that would be nice to fix but have low impact
Projects
None yet
Development

No branches or pull requests

2 participants