Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#21003 closed Cleanup/optimization (fixed)

BaseGeometryWidget is not idempotent

Reported by: Mathieu Leplatre Owned by: nobody
Component: GIS Version: 1.6-beta-1
Severity: Normal Keywords:
Cc: Claude Paroz Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Currently, the geometry data is serialized as wkt :

https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/widgets.py#L69
https://github.com/django/django/blob/1.6b2/django/contrib/gis/templates/gis/openlayers.html#L20

And is read back here:

https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/fields.py#L95

But transform() will fail, because the instantiated Geometry won't have any projection associated (wkt does not carry srid).

`
GEOSException at ...
Calling transform() with no SRID set is not supported
`

https://github.com/django/django/blob/1.6b2/django/contrib/gis/forms/fields.py#L96

There are several possible solutions :

Attachments (3)

patch-widget-srid.patch (648 bytes ) - added by Mathieu Leplatre 11 years ago.
Fix if widget srid != field srid
patch-widget-srid.patch​ (1.5 KB ) - added by Mathieu Leplatre 11 years ago.
Fix if widget srid != field srid
21003-3.diff (3.1 KB ) - added by Claude Paroz 11 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 by Claude Paroz, 11 years ago

Triage Stage: UnreviewedAccepted

What about:

diff --git a/django/contrib/gis/forms/fields.py b/django/contrib/gis/forms/fields.py
index 2fb8b54..04a19fd 100644
--- a/django/contrib/gis/forms/fields.py
+++ b/django/contrib/gis/forms/fields.py
@@ -87,6 +87,8 @@ class GeometryField(forms.Field):
         # Only do a geographic comparison if both values are available
         if initial and data:
             data = fromstr(data)
+            if not data.srid and self.srid:
+                data.srid = self.srid
             data.transform(initial.srid)
             # If the initial value was not added by the browser, the geometry
             # provided may be slightly different, the first time it is saved.

comment:2 by Mathieu Leplatre, 11 years ago

Hum, unless I don't get it, it's wrong.

In your proposition, you assign the database field srid to the data coming for the widget, whereas you should be using the widget's srid !

Something like this :

::

if not data.srid:

data.srid = self.widget.map_srid

If you want to see another approach, for example, in django-leaflet, I did that to fix that problem:
https://github.com/makinacorpus/django-leaflet/blob/add_leaflet_form_widget_and_admin/leaflet/templates/leaflet/widget.html#L34
Which is a consequence of the lack of modularity mentionned in #20998: because in this particular case, what is transmitted between the field and and the widgets is EWKT !

by Mathieu Leplatre, 11 years ago

Attachment: patch-widget-srid.patch added

Fix if widget srid != field srid

comment:3 by Mathieu Leplatre, 11 years ago

Has patch: set

by Mathieu Leplatre, 11 years ago

Attachment: patch-widget-srid.patch​ added

Fix if widget srid != field srid

comment:4 by Mathieu Leplatre, 11 years ago

Since SRID is ignored while parsing GeoJSON ("crs" attribute), this bug prevents from using other serialization format than e/wkt.

Indeed, data coming from widget is read without assigning srid during deserialization. If the serialized format cannot carry it, the resulting geometry has no srid. Which fails at first transform() operation...

by Claude Paroz, 11 years ago

Attachment: 21003-3.diff added

comment:5 by Claude Paroz, 11 years ago

Here's a slightly different patch. I didn't like that we had to reconstruct a Geometry object at two different places in the same file. And I prefer to test the srid after having created the object, because you never know if the client has provided a value containing a parsable srid through a clever thing. Criticism welcome!

comment:6 by Mathieu Leplatre, 11 years ago

Indeed, it's a lot better to use to_python() ! Great, thanks !

comment:7 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In dd656073ad80d0e8ec9e9dfea30da75a660d759c:

Fixed #21003 -- Ensured geometry widget return value has SRID

Thanks Mathieu Leplatre for the report and initial patch.

comment:8 by Claude Paroz <claude@…>, 11 years ago

In 0514fbb2f37b530e46f54e205cb8c9d722e142d6:

[1.6.x] Fixed #21003 -- Ensured geometry widget return value has SRID

Thanks Mathieu Leplatre for the report and initial patch.
Backport of dd656073ad from master.

Note: See TracTickets for help on using tickets.
Back to Top