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

Fixes WFS EXP_FILTER parsing in GetFeature request #47029

Merged
merged 8 commits into from
Feb 3, 2022

Conversation

pblottiere
Copy link
Member

... for using this kind of request: TYPENAME=countries,places&EXP_FILTER="name"='France';"name"='Paris'.

(note that the ; separator is used to be homogeneous with the filter implementation in WMS 1.3.0 service).

This PR is related to https://www.mail-archive.com/qgis-user@lists.osgeo.org/msg50029.html.

@pblottiere pblottiere added Bug Either a bug report, or a bug fix. Let's hope for the latter! Server Related to QGIS server labels Jan 27, 2022
@github-actions github-actions bot added this to the 3.24.0 milestone Jan 27, 2022
@pblottiere
Copy link
Member Author

Hello @nboisteault,

FYI, I think this PR will fix your issue raised some days ago about the EXP_FILTER filter in WFS.

@rouault
Copy link
Contributor

rouault commented Jan 27, 2022

... for using this kind of request: TYPENAME=countries,places&EXP_FILTER="name"='France';"name"='Paris'.

(note that the ; separator is used to be homogeneous with the filter implementation in WMS 1.3.0 service).

I'd just note that isn't consistent with the WFS spec regarding usage of FILTER for multiple layers.

For WFS 2, see Example 10 of http://docs.opengeospatial.org/is/09-025r2/09-025r2.html#391

TYPENAMES=(InWaterA_1M)(BuiltUpA_1M)&
       FILTER=(<Filter><Within><PropertyName>InWaterA_1M/wkbGeom
              <PropertyName><gml:Envelope><gml:lowerCorner>43.5707 -79.5797</gml:lowerCorner>
              <gml:upperCorner>43.8219 -79.2693</gml:upperCorner></gml:Envelope>
              </Within></Filter>)(<Filter><Within><PropertyName>
              BuiltUpA_1M/wkbGeom<PropertyName><gml:Envelope><gml:lowerCorner>43.5705 -79.5797
              </gml:lowerCorner><gml:upperCorner>43.8219 -79.2693</gml:upperCorner>
              </gml:Envelope></Within></Filter>)

and WFS 1.1 example 13 of http://docs.opengeospatial.org/is/04-094r1/04-094r1.html :

       TYPENAME=InWaterA_1M,BuiltUpA_1M&
       FILTER=(<Filter><Within><PropertyName>InWaterA_1M/wkbGeom
              <PropertyName><gml:Envelope><gml:lowerCorner>10 10</gml:lowerCorner>
              <gml:upperCorner>20 20</gml:upperCorner></gml:Envelope>
              </Within></Filter>)(<Filter><Within><PropertyName>
              BuiltUpA_1M/wkbGeom<PropertyName><gml:Envelope><gml:lowerCorner>10 10
              </gml:lowerCorner><gml:upperCorner>20 20</gml:upperCorner>
              </gml:Envelope></Within></Filter>)

Interestingly I just discover that in WFS 2 TYPENAMES=A,B means a join layer, whereas TYPENAMES=(A)(B) means get the 2 layers separately...

@rldhont
Copy link
Contributor

rldhont commented Jan 27, 2022

@pblottiere backport ?

@nboisteault
Copy link

@rouault very interesting indeed.
@pblottiere Many thanks for this PR. Do you know if QGIS Server is covering every cases @rouault has shown in his comment?

@pblottiere
Copy link
Member Author

@rouault thanks a lot for your input 👍

Do you know if QGIS Server is covering every cases @rouault has shown in his comment?

@nboisteault Considering the comment of Even and the corresponding spec, I think I should probably update the regex to support the (filter1)(filter2) notation instead of filter1;filter2. This way we'll be consistent with the WFS spec even if it's a vendor parameter.

BTW, I'm willing to bet that the FILTER parameter is also buggy...

@pblottiere
Copy link
Member Author

pblottiere commented Jan 31, 2022

By taking a look to the WMS FILTER parameter implementation, I noticed that the notation for multiple OGC filters is already something like (filter1)(filter2) and the ; delimitor is used to split QGIS expressions. In the interests of consistency accross services (WMS and WFS), I think it's best to keep the same approach here.

So I moved the filter parser in QgsServerParameterDefintion in order to use the same part of code in WFS and WMS services. I also added a whole bunch of unit tests in TestQgsServerParameter (empty OGC filters, several OGC filters, multiple QGIS expressions, ...).

The FILTER parameter in WFS seems broken too in case of multiple OGC filters but I'll take a look at it in another PR (this PR is only about EXP_FILTER).

@pblottiere pblottiere added the Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. label Feb 1, 2022
@github-actions
Copy link

github-actions bot commented Feb 1, 2022

@pblottiere
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@pblottiere
Copy link
Member Author

I'll merge this PR in the coming days if I don't have more comments in the meantime.

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

@pblottiere
A documentation ticket has been opened at qgis/QGIS-Documentation#7329
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

elpaso added a commit to elpaso/QGIS that referenced this pull request Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Server Related to QGIS server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants