hajkmap / Hajk

A modern, full-featured OpenLayers based map viewer and editor
MIT License
122 stars 46 forks source link

Edit ignores WFS layer's projection when it reads and writes features #927

Open jacobwod opened 2 years ago

jacobwod commented 2 years ago

Describe the bug

The Edit plugin expects the WFS layers to be in the same projection as the View to work flawlessly.

To Reproduce

Preparations - setup the map, WFS and WMS layers:

  1. Setup a map in one projection (e.g. EPSG:3008) Skärmavbild 2021-11-23 kl  10 10 15
  2. Setup a Edit service (WFS) in another projection (e.g. EPSG:3006) Skärmavbild 2021-11-23 kl  10 11 10
  3. Setup the same layer as a regular WMS layer (so we can view it via LayerSwitcher). Use the same projection as WFS (OpenLayes will reproject it to the View's projection) Skärmavbild 2021-11-23 kl  10 11 55

Preview the layer using WMS and PostGIS

Open the map in Client UI. Show the WMS layer. In my example, the layer has 4 geometries. They're in 3006 but get correctly placed (confirmed both in Hajk and pgAdmin):

Skärmavbild 2021-11-23 kl  10 15 27

Skärmavbild 2021-11-23 kl  10 16 31

Preview using WFS

Now try opening the WFS service in Edit plugin. The layer is fetched correctly, Hajks specifies that we want 3006 (that's the layer's projection):

GET https://myurl.com?service=WFS&version=1.1.0&request=GetFeature&typename=sitac_angrepp&srsname=EPSG:3006

And we do get back a valid GML response with geometries in 3006:

bild

In spite of the correct response, the geometries are somehow lost (they're added to the View as if they were in the Views projection, 3008, and end up way outside their correct placement [which should be exactly as the WMS service returned]). This is the number 1 issue.

Add a new feature using WFS Edit

There is also a number 2 issue: as we've seen in issue number 1, the Edit plugin seems to ignore the WFS service's projection (3006) and just expects that the geometries are in View's projection (3008). So the four existing features won't show. This error works the other way around too: when we add a geometry using Hajk, its coordinates will be read as if they were in the Views projection (3008), but saved in the layer's projection (3006), without any transformation occuring. Our new feature ends up in an unexpected place.

Let's draw a feature. It will be placed just between 3 of the original features: Skärmavbild 2021-11-23 kl  10 26 57

The POST looks fine, we want the geometry to be saved in 3006, which is the WFS service's projection and it's correct:

<Transaction
    xmlns="http://www.opengis.net/wfs" service="WFS" version="1.1.0" xsi:schemaLocation="http://www.opengis.net/wfs http://schemas.opengis.net/wfs/1.1.0/wfs.xsd"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <Insert>
        <sitac_angrepp
            xmlns="https://rtj.halmstad.se">
            <geometry>
                <LineString
                    xmlns="http://www.opengis.net/gml" srsName="EPSG:3006">
                    <posList srsDimension="2">123497.55967540591 6286955.857225036 119521.55967540592 6281523.857225036 126577.55967540591 6281243.857225036 123833.55967540591 6285611.857225036</posList>
                </LineString>
            </geometry>
            <kategori>Åtgärder</kategori>
            <typ>Angrepp</typ>
            <beskrivning>Genomfört direkt angrepp</beskrivning>
        </sitac_angrepp>
    </Insert>
</Transaction>

I can see the new geometry in PostGIS too: Skärmavbild 2021-11-23 kl  10 30 10

But Hajk took the 3008 coordinates and just saved them into the 3006 layer, without transformation. This is the issue number 2.

Conclusion

I don't feel comfortable rolling out 3.8 with Edit being broken like this, what's your opinion @jesade-vbg @hitomin21?

jacobwod commented 2 years ago

Tests

Below is a comprehension of Edit functionality using our configuration between v3.7.0 and v3.8.0-rc.1.

Disclaimer: all data shown in the videos below is for test purposes only. Neither the geometries, names nor feature attributes correspond to any real-world data.

Hajk 3.7.0 (tag commit)

Scenario 1 - WFS from GeoServer, map and WFS layer in the same projection

In the video below I demonstrate how both add, update and delete work flawlessly.

https://user-images.githubusercontent.com/110222/143398461-c24abb51-b3e8-4283-8af7-b5af36fffa2d.mov

Scenario 2 - WFS from QGIS Server, map and WFS layer in the same projections

In the video below data served from QGIS Server can be added and removed flawlessly and is instantaneously updated in the WMS service too. The update however doesn't work, the geometry is lost.

https://user-images.githubusercontent.com/110222/143400069-c2e61981-a8e2-458e-b061-1c848bfc9195.mov

Hajk 3.8.0-rc1+ (commit 0b72207a9304bd2806ba1b723363bd283d7bd503)

Scenario 1 - WFS from GeoServer, map and WFS layer in the same projection

In the video below I demonstrate how both add, update and delete work flawlessly.

https://user-images.githubusercontent.com/110222/143402441-8a60c277-4471-429c-b02b-bc8bbfb982d8.mov

Scenario 2 - WFS from QGIS Server, map and WFS layer in the same projections

Just like in 3.7, the update operation moves the geometry to an unknown location. Other than that it seems to work well, as long as both the map and WFS layers are in the same SRS. https://user-images.githubusercontent.com/110222/143402158-4b5db922-48b3-435c-b2b4-b9898bd5c9f0.mov

Conclusion

I can not get the update operation to work. Can we see some results from other organisations?

hitomin21 commented 2 years ago

We are discussing a bit internally about this, we will get back to you @jacobwod. Thanks for a very detail description!

hitomin21 commented 2 years ago

I think the simplest solution would be to use the view's projection in the request to the WFS and only use that in the edit tool.

At least Geoserver (and I assume QGIS Server as well) converts the response to the projection specified in the request so this would save us some trouble since we do not have to handle it in the code.

@jacobwod what do you think?

jacobwod commented 2 years ago

I think that we'll leave it for now.

This is still one of the major drawbacks in our current Edit plugin, however, and should be taken care of.

jacobwod commented 2 years ago

@hitomin21 @jesade-vbg What happens when you try to edit/move an existing feature that has some null values in the database? (It's a completely valid use case.)

This is what I see: Skärmavbild 2021-11-30 kl  10 45 28

Hallbergs commented 2 years ago

@hitomin21 @jesade-vbg What happens when you try to edit/move an existing feature that has some null values in the database? (It's a completely valid use case.)

This is what I see: Skärmavbild 2021-11-30 kl 10 45 28

The code below is probably the cause. let value = field.defaultValue; might be undefined? I might be wrong though.

updateFeature() {
    const featureProps = this.props.model.editFeature.getProperties();
    Object.keys(this.state.formValues).forEach((key) => {
      let value = this.state.formValues[key];
      if (value === "") value = null;
      if (Array.isArray(value)) {
        value = value
          .filter((v) => v.checked)
          .map((v) => v.name)
          .join(";");
      }
      featureProps[key] = value;
    });
jacobwod commented 2 years ago

That's right, featureProps[key] = value; sets the value to undefined which is an Object, and upon writing as a String the default toString() method is invoked which leads to "[Object object]".

Skärmavbild 2021-11-30 kl  11 00 42

Please fix this as soon as possible @hitomin21, @jesade-vbg so @Hallbergs and I can prepare a RC2.

hitomin21 commented 2 years ago

I'm working on that fix now, thanks for the info.

hitomin21 commented 2 years ago

let value = field.defaultValue

Would only be executed for noneditable fields so the bug must be in both noneditable and editable fields. I've added a check for if value is undefined.

I also added so the default value (or empty string) will be added if we load a editable field with a null value (in initFormValues).

I'm still unable to reproduce the error with Geoserver so can't verify my fixes, sorry for making you re-test this over and over.

Pushed in branch https://github.com/hajkmap/Hajk/tree/fix/3.8-edittool