Opened 23 months ago

Closed 22 months ago

Last modified 22 months ago

#21003 closed Cleanup/optimization (fixed)

BaseGeometryWidget is not idempotent

Reported by: leplatrem Owned by: nobody
Component: GIS Version: 1.6-beta-1
Severity: Normal Keywords:
Cc: claudep 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 leplatrem 22 months ago.
Fix if widget srid != field srid
patch-widget-srid.patch​ (1.5 KB) - added by leplatrem 22 months ago.
Fix if widget srid != field srid
21003-3.diff (3.1 KB) - added by claudep 22 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 23 months ago by claudep

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 22 months ago by leplatrem

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 !

Changed 22 months ago by leplatrem

Fix if widget srid != field srid

comment:3 Changed 22 months ago by leplatrem

  • Has patch set

Changed 22 months ago by leplatrem

Fix if widget srid != field srid

comment:4 Changed 22 months ago by leplatrem

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...

Changed 22 months ago by claudep

comment:5 Changed 22 months ago by claudep

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 Changed 22 months ago by leplatrem

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

comment:7 Changed 22 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In dd656073ad80d0e8ec9e9dfea30da75a660d759c:

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

Thanks Mathieu Leplatre for the report and initial patch.

comment:8 Changed 22 months ago by Claude Paroz <claude@…>

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