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

Spec-to-serializer #76

Merged
merged 41 commits into from
Jan 13, 2023
Merged

Spec-to-serializer #76

merged 41 commits into from
Jan 13, 2023

Conversation

j4mie
Copy link
Member

@j4mie j4mie commented Dec 20, 2022

This is quite a big change, but it should be entirely backwards-compatible and optional. It's isolated to the rest_framework "layer" of django-readers. If DRF isn't being used in a project, this change won't affect anything.

The DRF layer of django-readers does not use Serializers at all. Instead it uses a class (ProjectionSerializer) that quacked just enough like a real serializer to work with DRF's generic views.

However, some bits of the DRF ecosystem (specifically schema generation, but I'm sure there are others) depend on having a serializer class for introspection to work.

The core of this change is a new utility function serializer_class_for_spec. Given a name prefix, a model class, and a spec, this function returns a "real" DRF serializer that is the same "shape" as the spec, including nested serializers to represent relationships.

The fields on this serializer are not actually used during request/response handling (we still just project the object being serialized). Instead, the purpose of this is essentially to enable automatic schema generation (see #70) via introspection of views. But it may also make django-readers more compatible with any other parts of the DRF ecosystem that require actual serializer classes.

A couple of other changes are needed to enable this:

Specifying field types

For spec fields that are simple model fields or relationships, we can use DRF's ModelSerializer machinery to automatically infer their types from the model. However, for custom pairs, we need a way to explicitly declare the return type. To enable this, a utility function out is provided. This can be used in two ways.

As a decorator:

@out(serializers.CharField())
def upper_name():
    return qs.include_fields(name), lambda instance: instance.name.upper()

Or, with some special DSL-ish syntax, inline in a spec (for use with built-in library functions):

spec = [
    "name",
    {"type": pairs.field_display("type") >> out(serialiers.CharField())},
]

Dynamic behaviour

Second, if a spec is in any way "dynamic" (ie depends on the request in some way), the previously documented way to handle this was to override get_spec on your view class and return the spec, rather than using a static spec property:

def widget_is_mine(request):
    return qs.include_fields("owner"), lambda instance: instance.owner == request.user


class WidgetListView(SpecMixin, ListAPIView):
    def get_spec(self):
        return [
            "name",
            "size",
            {"is_mine": widget_is_mine(self.request)},
        ]

Of course, during introspection no request object is available, and so this approach wouldn't work. Instead, during request processing, the DRF layer in django-readers now pre-processes the spec and any callable objects it finds are automatically called and passed the request object. So there's no need to override get_spec at all:

@out(serializers.BooleanField())
def widget_is_mine(request):
    return qs.include_fields("owner"), lambda instance: instance.owner == request.user

class WidgetListView(SpecMixin, ListAPIView):
    spec = [
        "name",
        "size",
        {"is_mine": widget_is_mine},
    ]

TODO

Open question (answered: see comments below):

Do we actually want to avoid changing the behaviour of SpecMixin here at all (other than pre-processing for callables), and instead simply document spec_to_serializer_class as a util for use in custom AutoSchema subclasses? My concerns are:

  • it's hard to guess how people are going to use this and so maybe trying to do anything automatically is a mistake
  • it might be confusing that get_serializer_class returns a real Serializer, but this serializer isn't actually used during serialization
  • the memory/performance overhead of constructing a Serializer (as opposed to the tiny "fake serializer" that we used before).
  • Changelog
  • Docs

j4mie added 27 commits December 1, 2022 14:12
@j4mie
Copy link
Member Author

j4mie commented Jan 10, 2023

OK, with the latest change this PR is much more additive and optional. The change in behaviour of the existing SpecMixin is limited to just the call-callables-with-request stuff. The serializer behaviour is not changed at all.

Instead, I've introduced another higher-level utility function, serializer_class_for_view, which takes a view instance and returns a serializer class built from its spec. This can then be used from a custom AutoSchema subclass to generate appropriate serializers at schema generation time.

This makes this change much more of a "handy added utils" type of thing, rather than a fundamental change to the behaviour of the SpecMixin.

@j4mie j4mie marked this pull request as ready for review January 12, 2023 09:34
Copy link

@charlie-certn charlie-certn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥳

@@ -30,7 +30,7 @@ pip install django-readers

## What is django-readers?

`django-readers` is both a **small library** (less than 500 lines of Python) and a **collection of recommended patterns** for structuring your code. It is intended to help with code that performs _reads_: querying your database and presenting the data to the user. It can be used with views that render HTML templates as well as [Django REST framework](https://www.django-rest-framework.org/) API views, and indeed anywhere else in your project where data is retrieved from the database.
`django-readers` is both a **small library** and a **collection of recommended patterns** for structuring your code. It is intended to help with code that performs _reads_: querying your database and presenting the data to the user. It can be used with views that render HTML templates as well as [Django REST framework](https://www.django-rest-framework.org/) API views, and indeed anywhere else in your project where data is retrieved from the database.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight 😢 at the removal of (less than 500 lines of Python) but all for the greater good!

@j4mie j4mie merged commit 1059b80 into main Jan 13, 2023
@j4mie j4mie deleted the spec-to-serializer branch January 13, 2023 16:06
@j4mie j4mie mentioned this pull request Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants