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 #223

Merged
merged 67 commits into from
Jan 10, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
d8cf934
add openapi schema generation for filters
dhaval-mehta Mar 15, 2020
a448b13
improve schema generation for filters
dhaval-mehta Mar 15, 2020
0859877
add code to generate schema from serializer
dhaval-mehta Mar 18, 2020
bce22da
rename some variables
dhaval-mehta Mar 19, 2020
75343a0
override schema of pagination
dhaval-mehta Apr 3, 2020
93b0f14
handle GeometrySerializerMethodField
dhaval-mehta Apr 3, 2020
9f94293
fix invalid schema for id field
dhaval-mehta Apr 3, 2020
c7421e4
add test models
dhaval-mehta Apr 3, 2020
69bc672
add test serializer
dhaval-mehta Apr 3, 2020
7b99c1e
add initial test case for schema generation
dhaval-mehta Apr 3, 2020
563ce0d
add model migration
dhaval-mehta Apr 3, 2020
9df1f4f
rename files
dhaval-mehta Apr 3, 2020
b0ff15c
fix handling of GeometrySerializerMethodField
dhaval-mehta Apr 3, 2020
99bf8cf
handle DRF version >= 3.12
dhaval-mehta Apr 8, 2020
255f160
add DEFAULT_SCHEMA_CLASS
dhaval-mehta Apr 8, 2020
5598003
upgrade djangorestframework to 3.11
dhaval-mehta Apr 8, 2020
b830a74
fix _map_serializer
dhaval-mehta Apr 8, 2020
2b7f862
revert DRF version to 3.10
dhaval-mehta Apr 8, 2020
e3f1494
fix schema for polygon
dhaval-mehta Apr 8, 2020
540a7c4
fix enum for polygon
dhaval-mehta Apr 8, 2020
2a8d1b0
add test cases for polygon
dhaval-mehta Apr 8, 2020
f9ab386
remove min items and change example generation logic
dhaval-mehta Apr 8, 2020
60682e7
add test cases for multi polygon
dhaval-mehta Apr 8, 2020
bbf392d
add test case for multi line string model
dhaval-mehta Apr 8, 2020
d2895d8
add test case for multi point model
dhaval-mehta Apr 8, 2020
14ceb6b
add test cases for pagination schema
dhaval-mehta Apr 8, 2020
af8773a
add test cases for filter schemas
dhaval-mehta Apr 8, 2020
a94a7bf
do not handle source attribute
dhaval-mehta Apr 8, 2020
dac80da
add missing dependency
dhaval-mehta Apr 8, 2020
4a47129
add number as type of array member for bbox
dhaval-mehta Apr 8, 2020
f1a0a2b
add test case fox auto bbox
dhaval-mehta Apr 8, 2020
d137946
fix bbox_geo_field handling
dhaval-mehta Apr 8, 2020
2bdba3d
add test case for schema generation of bbox_geo_field
dhaval-mehta Apr 8, 2020
73bda51
add support for DRF 3.12
dhaval-mehta May 3, 2020
e3cb217
Merge remote-tracking branch 'upstream/master'
dhaval-mehta Oct 18, 2020
02f34ca
change DRF version to 3.12
dhaval-mehta Oct 18, 2020
1c2e457
fix schema generation for 3.12
dhaval-mehta Oct 18, 2020
3e7966f
remove support for DRF < 3.12
dhaval-mehta Oct 18, 2020
e374ed4
fix support for distance to point filter
dhaval-mehta Oct 18, 2020
01c1e92
fix linting issues
dhaval-mehta Oct 18, 2020
59d346a
add name to AUTHORS
dhaval-mehta Oct 18, 2020
11430b4
fix linting issues
dhaval-mehta Oct 18, 2020
331f764
fix linting issues using black
dhaval-mehta Oct 18, 2020
1aefe46
[fix] Conflicts between black and flake8
dhaval-mehta Oct 19, 2020
8780ea6
[fix] Postgresql Setup
dhaval-mehta Oct 25, 2020
8de2e18
[fix] revert unnecessary changes made by black
dhaval-mehta Oct 25, 2020
27443e8
[fix] Wrong call to _map_serializer
dhaval-mehta Oct 25, 2020
4c1dc11
[change] Update documentation
dhaval-mehta Oct 25, 2020
e31c72d
add packaging dependency
dhaval-mehta Oct 25, 2020
f6ea625
fix has_geometry_distance
dhaval-mehta Oct 25, 2020
fb01e25
run schema generation tests only if DRF >= 3.12
dhaval-mehta Oct 25, 2020
83d3dd7
remove unnecessary check for DRF < 3.9
dhaval-mehta Oct 25, 2020
1d551ba
[fix] Fix tox.ini
dhaval-mehta Oct 25, 2020
644c379
[fix] Fix .travis.yml
dhaval-mehta Oct 25, 2020
6abd208
[fix] Fix linting issues
dhaval-mehta Oct 25, 2020
1f14ee5
[fix] revert unnecessary changes.
dhaval-mehta Oct 25, 2020
917b490
[fix] Linting issues
dhaval-mehta Oct 25, 2020
b1c30e6
[fix] Support for Django 2.1
dhaval-mehta Oct 25, 2020
0009d2a
[fix] travis env
dhaval-mehta Oct 25, 2020
5af7366
[fix] Fix tox.ini
dhaval-mehta Oct 25, 2020
218fec5
[Merge] Merge remote-tracking branch 'upstream/master'
dhaval-mehta Nov 10, 2020
c9ffd55
[Merge] Merge remote-tracking branch 'upstream/master'
dhaval-mehta Dec 24, 2020
438527c
[ci] Fix .travis.yml
dhaval-mehta Dec 25, 2020
f890c91
[code] Add support for GeometryField and GeometryCollectionField
dhaval-mehta Dec 30, 2020
d61949d
[docs] improve docs
dhaval-mehta Dec 31, 2020
88d0b12
[ci] Remove support for Django 2.1 and support for Python 3.9
dhaval-mehta Dec 31, 2020
d0f8848
[lint] Fix linting issues
dhaval-mehta Dec 31, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 76 additions & 7 deletions rest_framework_gis/filters.py
@@ -1,13 +1,12 @@
from math import cos, pi

from django.db.models import Q
from django.core.exceptions import ImproperlyConfigured
from django.contrib.gis import forms
from django.contrib.gis.db import models
from django.contrib.gis.geos import Polygon, Point
from django.contrib.gis import forms

from rest_framework.filters import BaseFilterBackend
from django.core.exceptions import ImproperlyConfigured
from django.db.models import Q
from rest_framework.exceptions import ParseError
from rest_framework.filters import BaseFilterBackend

from .tilenames import tile_edges

Expand All @@ -31,7 +30,6 @@
else:
gis_lookups = BaseSpatialField.get_lookups()


__all__ = [
'InBBoxFilter',
'InBBOXFilter',
Expand Down Expand Up @@ -73,6 +71,29 @@ def filter_queryset(self, request, queryset, view):
if not bbox:
return queryset
return queryset.filter(Q(**{'%s__%s' % (filter_field, geoDjango_filter): bbox}))

def get_schema_operation_parameters(self, view):
return [
{
'name': self.bbox_param,
'required': False,
'in': 'query',
'description': 'Specify a bounding box as filter: in_bbox=min_lon,min_lat,max_lon,max_lat',
'schema': {
'type': 'array',
'items': {
'type': 'float',
},
'minItems': 4,
'maxItems': 4,
'example': [0, 0, 10, 10]
},
'style': 'form',
'explode': False,
},
]


# backward compatibility
InBBOXFilter = InBBoxFilter

Expand Down Expand Up @@ -103,7 +124,7 @@ def __new__(cls, *args, **kwargs):


class TMSTileFilter(InBBoxFilter):
tile_param = 'tile' # The URL query paramater which contains the tile address
tile_param = 'tile' # The URL query parameter which contains the tile address

def get_filter_bbox(self, request):
tile_string = request.query_params.get(self.tile_param, None)
Expand All @@ -118,6 +139,20 @@ def get_filter_bbox(self, request):
bbox = Polygon.from_bbox(tile_edges(x, y, z))
return bbox

def get_schema_operation_parameters(self, view):
return [
{
'name': self.tile_param,
'required': False,
'in': 'query',
'description': 'Specify a bounding box filter defined by a TMS tile address: tile=Z/X/Y',
'schema': {
'type': 'string',
'example': '12/56/34'
},
},
]


class DistanceToPointFilter(BaseFilterBackend):
dist_param = 'dist'
Expand Down Expand Up @@ -188,3 +223,37 @@ def filter_queryset(self, request, queryset, view):
dist = self.dist_to_deg(dist, point[1])

return queryset.filter(Q(**{'%s__%s' % (filter_field, geoDjango_filter): (point, dist)}))

def get_schema_operation_parameters(self, view):
return [
{
'name': self.dist_param,
'required': False,
'in': 'query',
'schema': {
'type': 'number',
'format': 'float',
'default': 1000,
},
'description': 'Represents **Distance** in **Distance to point** filter. Default value is used only if '
'***{point_param}*** is passed.'.format(point_param=self.point_param)
},
{
'name': self.point_param,
'required': False,
'in': 'query',
'description': 'Point represented in **x,y** format. '
'Represents **point** in **Distance to point filter**',
'schema': {
'type': 'array',
'items': {
'type': 'float',
},
'minItems': 2,
'maxItems': 2,
'example': [0, 10]
},
'style': 'form',
'explode': False,
}
]
12 changes: 12 additions & 0 deletions rest_framework_gis/pagination.py
Expand Up @@ -18,3 +18,15 @@ def get_paginated_response(self, data):
('previous', self.get_previous_link()),
('features', data['features'])
]))

def get_paginated_response_schema(self, view):
schema = super().get_paginated_response_schema(view)
schema['properties']['features'] = schema['properties'].pop('results')
schema['properties'] = {
'type': {
'type': 'string',
'enum': ['FeatureCollection']
},
**schema['properties'],
}
return schema
172 changes: 172 additions & 0 deletions rest_framework_gis/schema.py
@@ -0,0 +1,172 @@
import warnings

from django.contrib.gis.db import models
from rest_framework.schemas.openapi import AutoSchema
from rest_framework.utils import model_meta

from rest_framework_gis.fields import GeometrySerializerMethodField
from rest_framework_gis.serializers import GeoFeatureModelListSerializer, GeoFeatureModelSerializer


class GeoFeatureAutoSchema(AutoSchema):
Copy link

Choose a reason for hiding this comment

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

Requiring users to set REST_FRAMEWORK.DEFAULT_SCHEMA_CLASS to a custom GIS specific class will limit the utility of this. The default DRF schema generator is rarely used for serious projects because there are so many unsolved problems in it. Most medium to large projects will use drf-yasg, or similar (including drf-spectacular which is new but quite good), and they use their own mechanism, either requiring that DEFAULT_SCHEMA_CLASS is set to their own class, or they ignore DEFAULT_SCHEMA_CLASS altogether. And many projects need to sub-class DRF's AutoSchema to add their own behaviour. I've tried to do a WIP for drf-spectacular to use the standard AutoSchema inheritance at tfranzel/drf-spectacular#46 , but it has has some curly problems.

Keeping GeoFeatureAutoSchema is ok, but it would be better as a thin wrapper only, using functionality that exists outside of the class.

i.e. the schemas and functions that are embedded here inside this class should be in the module namespace, so they can be used with other generators. The code in this custom AutoSchema only uses self. to reference the new code below -- it doesnt rely on any of the inner workings of DRF AutoSchema.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. Can you tell me what are the unresolved problems with the current master branch of DRF? OpenAPI3 proposal for enhancements/fixes encode/django-rest-framework#7088 has listed 5 problems. 3 problems are already solved. Also, the DRF schema generator is maturing with time.
  2. drf-yasg is based on open API specification 2.0. Also, I would love to know which are the additional features that are present in drf-yasg but missing in the DRF schema generator.
  3. Django-rest-framework-gis should override DEFAULT_SCHEMA_CLASS is the solution of Carlton Gibson, maintainer of schema generation part. Thread link: allow serializers to override schema encode/django-rest-framework#7202 (comment)
  4. All methods are starting with _. I think it is a clear instruction to others, not to use these methods. Also, this is an unreviewed code. I have already requested @auvipy to review this code.

Copy link

Choose a reason for hiding this comment

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

I have no comment on DRF master. I expected released versions of DRF to be usable, and in this case it isnt. Usually the problem is that it falls apart when an installable app using DRF isnt 'perfect', which is 80%+ of the time, and often they are slowly maintained, so it will be a long time before they will work - usually it is exceptions all over the place.

While drf-yasg is openapi 2.0, it is usable for most people, and they dont care much about v2 vs v3

Django-rest-framework-gis should override DEFAULT_SCHEMA_CLASS is the solution of Carlton Gibson

I am not saying that drf-gis shouldnt provide a AutoSchema class. I just hope that the core functionality of this PR is outside of the AutoSchema class, and the AutoSchema class is a thin wrapper. Especially the schema definitions, which are first class entities that drf-gis should expose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have noted down your points. @auvipy Eagerly waiting for your review.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm actually waiting for drf 3.12 release and celery 4.4.3[due by 5th of may]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @auvipy for the quick reply.
I have added a lot of extra code to make it compatible with DRF 3.10 and 3.11.
Should I remove that code and make schema generation only compatible with DRF 3.12?

Copy link
Collaborator

Choose a reason for hiding this comment

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

DRF 3.10 can be removed without any hesitation!

COORDINATES_SCHEMA_FOR_POINT = {
'type': 'array',
'items': {
'type': 'number',
'format': 'float',
},
'example': [12.9721, 77.5933],
'minItems': 2,
'maxItems': 3,
}

COORDINATES_SCHEMA_FOR_LINE_STRING = {
'type': 'array',
'items': COORDINATES_SCHEMA_FOR_POINT,
'example': [[22.4707, 70.0577], [12.9721, 77.5933]],
'minItems': 2,
}

GEO_FIELD_TO_SCHEMA = {
models.PointField: {
'type': {
'type': 'string',
'enum': ['Point'],
},
'coordinates': COORDINATES_SCHEMA_FOR_POINT,
},
models.LineStringField: {
'type': {
'type': 'string',
'enum': ['LineString'],
},
'coordinates': COORDINATES_SCHEMA_FOR_LINE_STRING,
},
models.PolygonField: {
'type': {
'type': 'string',
'enum': ['LineString'],
},
'coordinates': {
'type': 'array',
'items': {
**COORDINATES_SCHEMA_FOR_LINE_STRING,
'minItems': 4,
},
'minItems': 1,
'example': [[[0.0, 0.0], [0.0, 1.0], [1.0, 1.0], [1.0, 0.0], [0.0, 0.0]],
[[0.4, 0.4], [0.4, 0.6], [0.6, 0.6], [0.6, 0.4], [0.4, 0.4]]]
}
}
}

MULTI_FIELD_MAPPING = {
Copy link

Choose a reason for hiding this comment

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

It looks like GeometryCollection isnt being covered by this PR. It uses geometries rather than geometry, and I dont see "geometries" mentioned anywhere in this PR.

Copy link

Choose a reason for hiding this comment

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

ping @dhaval-mehta . Still no "geometries"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jayvdb man, I am running very busy. I have just merged the latest code in my branch and resolved merge conflicts.
I will sure look into your suggestion but it may take 2-3 months.

Copy link

Choose a reason for hiding this comment

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

2-3 months? Do you mean after this PR is merged?

Unless you have tests passing that include "geometries", it is very likely that merging this PR will cause problems for anyone using GeometryCollection in their gis data.

Copy link
Collaborator

Choose a reason for hiding this comment

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

2-3 months? Do you mean after this PR is merged?

Unless you have tests passing that include "geometries", it is very likely that merging this PR will cause problems for anyone using GeometryCollection in their gis data.

@dhaval-mehta can you clear this concern please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jayvdb Added support for GeometryCollectionField and GeometryField

models.PointField: models.MultiPointField,
models.LineStringField: models.MultiLineStringField,
models.PolygonField: models.MultiPolygonField
}

for singular_field, multi_field in MULTI_FIELD_MAPPING.items():
GEO_FIELD_TO_SCHEMA[multi_field] = {
'type': {
'type': 'string',
'enum': [multi_field.geom_class.__name__],
},
'coordinates': {
'type': 'array',
'items': GEO_FIELD_TO_SCHEMA[singular_field]['coordinates'],
'minItems': 1,
'example': [
GEO_FIELD_TO_SCHEMA[singular_field]['coordinates']['example'],
GEO_FIELD_TO_SCHEMA[singular_field]['coordinates']['example']
]
}
}

def _map_geo_field(self, serializer, geo_field_name):
field = serializer.fields[geo_field_name]
if isinstance(field, GeometrySerializerMethodField):
warnings.warn('Geometry generation for GeometrySerializerMethodField is not supported.'.format(field=field))
return {}

if field.source:
model_field_name = field.source
else:
model_field_name = geo_field_name

geo_field = model_meta.get_field_info(serializer.Meta.model).fields[model_field_name]
try:
return self.GEO_FIELD_TO_SCHEMA[geo_field.__class__]
except KeyError:
warnings.warn('Geometry generation for {field} is not supported.'.format(field=field))
return {}

def _map_field(self, field):
if isinstance(field, GeoFeatureModelListSerializer):
Copy link

Choose a reason for hiding this comment

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

this looks incorrect. _map_field is a method from DRF, and the arg field is not a serializer. I have to wonder how a serializer is appearing here.

Anyway, _map_field should handle if isinstance(field, GeometryField):.
I've added a commit jayvdb@85983bc with some of the things that seem needed for that.

I needed this when using spatialite, which may be a bit different under the cover than the postgres driver.

I have a travis build https://travis-ci.org/github/jayvdb/django-rest-framework-gis/jobs/682327850 and test_point_field_inner_geo_list_serializer might be a real breakage caused by my changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this looks incorrect. _map_field is a method from DRF, and the arg field is not a serializer. I have to wonder how a serializer is appearing here.

Have a look at line no 364 of rest_framework/schemas/openapi.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyway, _map_field should handle if isinstance(field, GeometryField):.

Line no 369 of rest_framework/schemas/openapi.py will handle this case automatically.

return self._map_geo_feature_model_list_serializer(field)

return super()._map_field(field)

def _map_geo_feature_model_list_serializer(self, serializer):
return {
'type': 'object',
'properties': {
'type': {
'type': 'string',
'enum': ['FeatureCollection']
},
'features': {
'type': 'array',
'items': self._map_serializer(serializer.child)
}
}
}

def _map_geo_feature_model_serializer(self, serializer):
schema = super()._map_serializer(serializer)

geo_json_schema = {
'type': 'object',
'properties': {
'type': {
'type': 'string',
'enum': ['Feature']
},
}
}

if serializer.Meta.id_field:
geo_json_schema['properties']['id'] = schema['properties'].pop(serializer.Meta.id_field)

geo_field = serializer.Meta.geo_field
geo_json_schema['properties']['geometry'] = {
'type': 'object',
'properties': self._map_geo_field(serializer, geo_field),
}
schema['properties'].pop(geo_field)

if serializer.Meta.auto_bbox or serializer.Meta.bbox_geo_field:
geo_json_schema['properties']['bbox'] = {
'type': 'array',
'minItems': 4,
'maxItems': 4,
'example': [12.9721, 77.5933, 12.9721, 77.5933],
}
if serializer.Meta.bbox_geo_field in schema:
schema.pop(serializer.Meta.bbox_geo_field)

geo_json_schema['properties']['properties'] = schema

return geo_json_schema

def map_serializer(self, serializer):
dhaval-mehta marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(serializer, GeoFeatureModelListSerializer):
return self._map_geo_feature_model_list_serializer(serializer)

if isinstance(serializer, GeoFeatureModelSerializer):
return self._map_geo_feature_model_serializer(serializer)

return super()._map_serializer(serializer)

def _map_serializer(self, serializer):
return self.map_serializer(serializer)
@@ -0,0 +1,86 @@
# Generated by Django 3.0.4 on 2020-04-03 06:52

import django.contrib.gis.db.models.fields
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('django_restframework_gis_tests', '0002_nullable'),
]

operations = [
migrations.CreateModel(
name='LineStringModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('points', django.contrib.gis.db.models.fields.LineStringField(srid=4326)),
],
options={
'abstract': False,
},
),
migrations.CreateModel(
name='MultiLineStringModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('points', django.contrib.gis.db.models.fields.MultiLineStringField(srid=4326)),
],
options={
'abstract': False,
},
),
migrations.CreateModel(
name='MultiPointModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('points', django.contrib.gis.db.models.fields.MultiPointField(srid=4326)),
],
options={
'abstract': False,
},
),
migrations.CreateModel(
name='MultiPolygonModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('polygon', django.contrib.gis.db.models.fields.MultiPolygonField(srid=4326)),
],
options={
'abstract': False,
},
),
migrations.CreateModel(
name='PointModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('location', django.contrib.gis.db.models.fields.PointField(srid=4326)),
],
options={
'abstract': False,
},
),
migrations.CreateModel(
name='PolygonModel',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('random_field1', models.CharField(max_length=32)),
('random_field2', models.IntegerField()),
('polygon', django.contrib.gis.db.models.fields.PolygonField(srid=4326)),
],
options={
'abstract': False,
},
),
]