openwisp / django-rest-framework-gis

Geographic add-ons for Django REST Framework. Maintained by the OpenWISP Project.
http://openwisp.org
MIT License
1.07k stars 200 forks source link

Openapi schema generation does not support serializers not subclassing GeoFeatureModelSerializer #257

Closed AndrewGuenther closed 3 years ago

AndrewGuenther commented 3 years ago

I have a pretty simple model:

from django.contrib.gis.db import models

class StoredRaster(models.Model):
   file_arn = models.CharField(max_length=1024, null=True)
   footprint = models.PolygonField(srid=4326, null=True)

If I try to now generate a schema of this model, it will declare the footprint field as a string type, rather than the GeoJSON representation that the API would actually return. This causes clients to fail type validation on responses.

You can see that the schema generation logic explicitly checks for this serializer in both of these places: https://github.com/openwisp/django-rest-framework-gis/blob/master/rest_framework_gis/schema.py#L162-L169 https://github.com/openwisp/django-rest-framework-gis/blob/master/rest_framework_gis/schema.py#L109-L113

The logic for map_field should be adapted to also handle GeometryFields

nemesifier commented 3 years ago

@dhaval-mehta what do you think?

AndrewGuenther commented 3 years ago

I was able to fix this. The diff is pretty simple, but I haven't looked into tests yet. I'll follow up with a PR soon.

dhaval-mehta commented 3 years ago

@AndrewGuenther Can you please share the serializer as well? Does your serializer class extends GeoFeatureModelSerializer?

AndrewGuenther commented 3 years ago

No, my serializer does not extend GeoFeatureModelSerializer and that's an intentional choice. I don't want the restructuring that is provided by that class.

from core_api.models.StoredRaster import StoredRaster
from rest_framework import serializers

class StoredRasterSerializer(serializers.ModelSerializer):
    class Meta:
        model = StoredRaster
        fields = '__all__'
dhaval-mehta commented 3 years ago

I got your point. Checking serializer is a subclass of GeoFeatureModelSerializer is just for performance optimizations. I will raise a PR soon.

dhaval-mehta commented 3 years ago

I had some medical emergency and because of it, I couldn’t pick this task. Sorry for the inconvenience caused. I will try to complete this in the current week only.

AndrewGuenther commented 3 years ago

I went ahead and submitted the change that I have. I just couldn't figure out how to test it properly. Would love some feedback on that.

AndrewGuenther commented 3 years ago

I went ahead and added a test that covers this, but still would like feedback if it's the preferred way.

dhaval-mehta commented 3 years ago

I will review this PR.