Relax redundant source name restriction for Fields

[james-mchugh]

Similar to as suggested and accepted in #2420, could the assertion for source be relaxed as well?

In the use case I'm currently working, I have several serializers for different representations of the same underlying model. Since all of these serializers ultimately map to the the same model fields, they require a lot of the same validation logic. I've been wrestling with the best way of doing this, given that I can't really put that validation logic in the central location of model as discussed in #7850.

One of the ways I have landed on doing this was to create a common fields.py module that contains common DRF Fields with validation logic for each of the model fields. Below is an example:

def NameField():
    return serializers.CharField(
        max_length=128,
        validators=[
            validators.MinLengthValidator(
                1, message="The name must contain at least one character."
            ),
            validators.RegexValidator(
                regex=r"[a-zA-Z-]+",
                message="The name must only contain letters and dashes.",
            ),
        ],
        source="name"
    )

This should solve many of the common validation issues I have, except for the fact that I cannot use it in a serializer if source is the same as the field name. Consider the dummy example below:

class MySerializerV1(serializers.Serializer):
    old_name = fields.NameField()

class MySerializerV2(serializers.Serializer):
    name = fields.NameField()

Because the source argument is redundant, it triggers the AssertionError at https://github.com/encode/django-rest-framework/blob/master/rest_framework/fields.py#L360 when running bind for MySerializerV2.

One of the other approaches I considered was putting all of this validation logic in my model and using a ModelSerializer, since DRF automatically pulls the validators from Django model fields. This is fine if my representations are an exact mapping to my model, but they're not. I attempted to work around this by creating a base class that inherits from ModelSerializer and overrides get_fields so child classes can redefine mappings between the representation fields and model fields. This worked, but I wasn't super happy with how the resulting code looked. I can share more details on this if interested, but I don't want to distract from my main question.

Is there a better way of doing what I'm trying to do? Otherwise, what are your thoughts on relaxing the assertion around redundant sources?

[browniebroke]

If you really insist on having source in your custom field, how about changing your NameField to:

class NameField(serializers.CharField):
    def __init__(
        self,
        max_length=128,
        source="name",
        **kwargs,
    ):
        kwargs.setdefault("validators", [
            validators.MinLengthValidator(
                1, message="The name must contain at least one character."
            ),
            validators.RegexValidator(
                regex=r"[a-zA-Z-]+",
                message="The name must only contain letters and dashes.",
            ),
        ])
        super().__init__(max_length=max_length, source=source, **kwargs)

Then, you can disable this assertion by passing None to the source:

class MySerializerV1(serializers.Serializer):
    old_name = fields.NameField()

class MySerializerV2(serializers.Serializer):
    name = fields.NameField(source=None)

That also enables you to override other keyword arguments:

class MySerializerV3(serializers.Serializer):
    short_name = fields.NameField(max_length=100)

Alternative solution

Although, personally, I would probably remove the source from the custom field:

class NameField(serializers.CharField):
    def __init__(
            self,
            max_length=128,
            **kwargs,
    ):
        kwargs.setdefault("validators", [...])
        super().__init__(max_length=max_length, **kwargs)

In which case, the 2 serializers would look like this:

class MySerializerV1(serializers.Serializer):
    old_name = fields.NameField(source="name")

class MySerializerV2(serializers.Serializer):
    name = fields.NameField()

I find it more explicit and clearer in the first one where the old_name comes from. I recognise that it might be because the example is simplified...

Hope that helps.

[james-mchugh]

Thank you for the reply!

I considered both of these options as well. My code currently uses option 1, but I may switch it to option 2 because I don't think explicitly setting source to None really helps with readability or maintainability. I also don't think it necessarily helps with the style trying to be enforced by DRF.

My main concern with option 2 is that each of these fields represent a specific field of a specific model. They are very specific to the domain that model is covering, so the validation logic they use will likely not be applicable to other models in my project. I could see omitting the source field in them if they were expected to be general fields that could be used by any model, but that is currently not the case and each field has a direct connection to a model field. Having to explicitly specify a source everytime a field is used across N serializers and M fields just dramatically increases the chance of an easily avoidable programmer error. I understand the desire to be explicit, but isn't that kind of the issue with the assertion? It doesn't allow you to be explicit because the source implicitly comes from the field name.

I definitely see the desire for maintaining a consistent style for DRF. Many technologies also have style guides, such as of course PEP8 for Python. However, PEP8 is just a guide, and there may be reasons you want to not use that guide when it may help with readability/maintainability of the code. For example, in my code above, I used camelcase for a function name because I was trying to achieve a style where the functions resembled Field classes for consistency with other fields being used by the serializer. There are some downsides to this, but for my project, not having to maintain more verbose class definitions for each of the fields while achieving a consistent code style in the serializers makes it worth it. The point is, Python doesn't prevent me from running code that doesn't follow its recommended style.

If enforcing a certain style can result in workarounds that ultimately reduce code maintainability/readability, wouldn't it be worth considering relaxing those requirements a bit? I feel like there are other ways of informing users when they're not following recommended styles without forcing unnecessary restrictions onto them, such as warnings for example.

[james-mchugh]

Sorry, off topic, but funny enough, your icon matches the thumbnails for the one of the tabs I have open. It turns out I currently have one of your blog posts open (this one to be specific). Thank you for writing that post and your work on DRF!