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

Fix #9685 Add possibility to use vector layers inside GeoProcessing tool #9691

Merged
merged 7 commits into from
Dec 6, 2023

Conversation

MV88
Copy link
Contributor

@MV88 MV88 commented Nov 10, 2023

Description

  • refactored the way the counter for each tool is handled, plus some clean up
  • add possibility to configure a wpsUrl to be used as default source in the plugin otherwise the url from the layer will be used
  • changed the way the groups for buffered and intersected layers are generated
  • remember to test with and without wpsUrl property

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Issue

What is the current behavior?

#9685

What is the new behavior?

you can use vector layers inside GeoProcessing plugin

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

Other useful information

MV88 added 4 commits November 10, 2023 14:17
… Geo_Processing tool

* refactored the way the counter for each tool is handled
* add possibility to configure a wpsUrl to be used as default source in the plugin otherwise the url from the layer will be used
@MV88 MV88 added BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch C265-ATOLCD-2022-SUPPORT labels Nov 10, 2023
@MV88 MV88 added this to the 2023.02.01 milestone Nov 10, 2023
@MV88 MV88 requested a review from offtherailz November 10, 2023 14:20
@MV88 MV88 self-assigned this Nov 10, 2023
@MV88 MV88 linked an issue Nov 10, 2023 that may be closed by this pull request
10 tasks
Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

  • Zoom to feature selected incorrect
geoprocessing_zoom_incorect.mp4
  • When switch layer, old feature remain selected
selected_.webm
  • I tried to create a buffer layer, and then use this buffer layer to intersect with states layer. The result looks to be the buffer layer itself, instead of the intersection.
Intersection_wrong.webm

Please test extensively before submitting reviews.

* better clean up of highlighted features
* fixed layer used in intersection process
* fiz zoom to feature when selected
@MV88
Copy link
Contributor Author

MV88 commented Nov 21, 2023

@offtherailz PR is ready again for review.

  • when fixing last point i encountered a bug in geoserver (using stable)

basically it's this request (trying to intersect two multipolygon)
failing intersection request.txt

with this error

{
    "type": "FeatureCollection",
    "features": []
}<?xml version="1.0" encoding="UTF-8"?><ows:ExceptionReport xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:ows="http://www.opengis.net/ows/1.1" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" version="1.1.0" xsi:schemaLocation="http://www.opengis.net/ows/1.1 https://gs-stable.geosolutionsgroup.com/geoserver/schemas/ows/1.1.0/owsAll.xsd">
<ows:Exception exceptionCode="NoApplicableCode">
<ows:ExceptionText>An error occurred while encoding the results of the process
found non-noded intersection between LINESTRING ( -109.0472 35.9967,
-109.046 34.9546 ) and LINESTRING ( -109.0463 34.9052,
-109.046 34.9546 ) [ (-109.046,
    34.9546, NaN)
]</ows:ExceptionText>
</ows:Exception>
</ows:ExceptionReport>

currently it tells you there is no intersection between the two. but we can better parse the result by dispatching a more appropriate notification

@MV88 MV88 requested a review from offtherailz November 21, 2023 15:07
@offtherailz
Copy link
Member

@MV88 please fix the tests

@MV88
Copy link
Contributor Author

MV88 commented Nov 22, 2023

@MV88 please fix the tests

done

@offtherailz
Copy link
Member

offtherailz commented Nov 27, 2023

Extracting from the request you posted the geometries,I found that the geometry sent look quite strange.

geometries_used.zip

image

ANYWAY

Trying to intersect the geometries of the map attached (I repeat, obtained by copying and pasting your requests json and adding them to the map) using the geoprocessing tool, it worked.

image

If you select the whole vector layers (for both) it works, doing an (probably unnecessary) collect WPS request.
Instead selecting feature 0 of both layers, the WPS returns an error.

So I'd ask you to check better if this is not a bug of mapstore instead.

General overview

I see in anycase you send a collect geometries for each layer:

  • In case of vector layers, I'd like to know the reason of this.
  • In case of WFS layers, again, I think there is not a specific reason.

We should change strategy to avoid upload/download/upload roundrip. Maybe this is an optimization, so it is not strictly mandatory, but could be useful to move complexity.

We should use, whenever possible, WFS directly or sending directly the geometry when vector data locally is available.
What are the counter-sides for this strategy?

Here in this example (even if collect geometry is used as a wrapper), intersect is used with WFS input directly, this could be more efficient in general.

https://docs.geoserver.org/main/en/user/services/wps/processes/chaining.html

@offtherailz
Copy link
Member

offtherailz commented Nov 27, 2023

As discussed with @aaime, @MV88 is right, this issue on JTS causes the problems reported on some geometries
@aaime also suggest to try to activate OverlayNG to workaround it. @tdipisa can you take care of this? Need to set a system property on demo

-Djts.overlay=ng

@tdipisa
Copy link
Member

tdipisa commented Dec 4, 2023

@offtherailz thank you

-Djts.overlay=ng

Let's enable this in gs stable to complete the review.

We should change strategy to avoid upload/download/upload roundrip. Maybe this is an optimization, so it is not strictly mandatory, but could be useful to move complexity.

We should use, whenever possible, WFS directly or sending directly the geometry when vector data locally is available.
What are the counter-sides for this strategy?

Let's wait for the review to be completed. Then we will consider to do this optimization as part of this PR or later.

@randomorder
Copy link
Member

randomorder commented Dec 4, 2023

JAVAT_OPTS updated on gs-stable @tdipisa

@tdipisa
Copy link
Member

tdipisa commented Dec 4, 2023

Thank you @randomorder.
@offtherailz you can proceed and complete the review.

@offtherailz
Copy link
Member

offtherailz commented Dec 6, 2023

It works perfectly now 👍

Applied various buffer and intersection combination and it produced the expected result every time.

image
map (46).json

@MV88 MV88 merged commit 468775b into geosolutions-it:master Dec 6, 2023
4 checks passed
@MV88
Copy link
Contributor Author

MV88 commented Dec 6, 2023

@ElenaGallo please test it in DEV

@MV88 MV88 modified the milestones: 2023.02.01, 2023.02.02 Dec 6, 2023
@ElenaGallo
Copy link
Contributor

@MV88 It is not possible to buffer or intersect a measurement layer.

measurament.mp4

MV88 added a commit to MV88/MapStore2 that referenced this pull request Dec 19, 2023
…GeoProcessing tool (geosolutions-it#9691)

* Fix geosolutions-it#9685 Add possibility tyo use vector layers inside Geo_Processing tool

* refactored the way the counter for each tool is handled
* add possibility to configure a wpsUrl to be used as default source in the plugin otherwise the url from the layer will be used

* refactor: moved some utilities in dedicated utils file

* improved feedback in case wpsUrl is missing

* improve feedback also for buffer tool

* better handling misconfigured layer in buffer tool

* Fixed review points

* better clean up of highlighted features
* fixed layer used in intersection process
* fiz zoom to feature when selected

* fix test
@ElenaGallo ElenaGallo removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend the GeoProcessing tool to work with client side Vector layers as WPS inputs
5 participants