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

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

Closed
AndrewGuenther opened this issue May 25, 2021 · 9 comments · Fixed by #259

Comments

@AndrewGuenther
Copy link
Contributor

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
Copy link
Member

@dhaval-mehta what do you think?

@AndrewGuenther
Copy link
Contributor Author

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.

AndrewGuenther pushed a commit to AndrewGuenther/django-rest-framework-gis that referenced this issue May 26, 2021
The intial implementation of schema generation only supported
serializers which subclassed GeoFeatureModel*Serializer. This meant that
models which have standalone GeometryFields would not properly generate
a schema. This change adds support for that use case.

Fixes openwisp#257
@dhaval-mehta
Copy link
Collaborator

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

@AndrewGuenther
Copy link
Contributor Author

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
Copy link
Collaborator

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

@dhaval-mehta
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@dhaval-mehta
Copy link
Collaborator

I will review this PR.

AndrewGuenther pushed a commit to AndrewGuenther/django-rest-framework-gis that referenced this issue Jul 14, 2021
…#257

The intial implementation of schema generation only supported
serializers which subclassed GeoFeatureModel*Serializer. This meant that
models which have standalone GeometryFields would not properly generate
a schema. This change adds support for that use case.

Closes openwisp#257
AndrewGuenther pushed a commit to AndrewGuenther/django-rest-framework-gis that referenced this issue Jul 14, 2021
…#257

The intial implementation of schema generation only supported
serializers which subclassed GeoFeatureModel*Serializer. This meant that
models which have standalone GeometryFields would not properly generate
a schema. This change adds support for that use case.

Closes openwisp#257
auvipy pushed a commit that referenced this issue Jul 15, 2021
The intial implementation of schema generation only supported
serializers which subclassed GeoFeatureModel*Serializer. This meant that
models which have standalone GeometryFields would not properly generate
a schema. This change adds support for that use case.

Closes #257
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 a pull request may close this issue.

3 participants