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

misc qgis server wfs getFeature fixes when crs is defined as a ogc urn #58355

Merged
merged 6 commits into from
Oct 3, 2024

Conversation

ptitjano
Copy link
Contributor

Description

This fixes 3 issues when the crs parameter is defined with the a ogc urn syntax. From the commit messages:

Fix outputCrs in startGetFeature if defined as ogc urn

The `srsName` used in `QgsWfsGetFeature::startGetFeature` can be
defined with the `urn:ogc:def:crs:EPSG::X` syntax. In that case, the
destination crs is not valid because it is created with the default
`QgsCoordinateReferenceSystem` constructor which does not handle this
syntax.
Therefore, in that case, the requested transformation fails and the
envelope coordinates are always in the input crs (the layer one).

This issue is fixed by using
`QgsCoordinateReferenceSystem::fromOgcWmsCrs` which handles the ogcUrn
syntax.

The test suite already covers the `urn:ogc:def:crs:EPSG::X` syntax
case. However, this issue is not detected because the requested output
crs is the same as the input one.

Properly check axis inversion in startGetFeature

The axis inversion needs to be checked on the output crs, not the
input one.

The existing test does not detect this issue because the requested
output crs is the same as the input one.
To cover this case, a new test with a output crs different from the
input one is added.

Properly handle ogc urns in post requests

There are 2 different parameters to check to define the output crs:
- the request parameter (`SRSNAME` from the request)
- the query crs parameter

For a `GET` request, if `SRSNAME` is set, both parameters are set and
equal.
For a `POST` request, only the query parameter is set if defined.

In `writeGetFeature()`, the `outputCrs` correctly takes into account
the query parameter. However, this is not the case for the output srs
name (`srsName`) which only takes into account the query CRS as an
auth id.
Therefore, the output srsName will always be defined as an authid
even if the parameter is an ogc urn.

This issue is fixed by first computing the output srs
name (`outputSrsName`) by taking
into account the query and the query parameters. Then, this name is
used to compute `outputCrs`.

cc @elpaso @troopa81

@ptitjano ptitjano self-assigned this Aug 10, 2024
@github-actions github-actions bot added this to the 3.40.0 milestone Aug 10, 2024
@ptitjano ptitjano added Server Related to QGIS server backport queued_ltr_backports Queued Backports Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Aug 10, 2024
Copy link

github-actions bot commented Aug 10, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit c492df9)

Copy link

github-actions bot commented Sep 3, 2024

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 3, 2024
@ptitjano ptitjano removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 3, 2024
Copy link

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 18, 2024
@ptitjano ptitjano removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Sep 18, 2024
Copy link

github-actions bot commented Oct 3, 2024

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@github-actions github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 3, 2024
The `srsName` used in `QgsWfsGetFeature::startGetFeature` can be
defined with the `urn:ogc:def:crs:EPSG::X` syntax. In that case, the
destination crs is not valid because it is created with the default
`QgsCoordinateReferenceSystem` constructor which does not handle this
syntax.
Therefore, in that case, the requested transformation fails and the
envelope coordinates are always in the input crs (the layer one).

This issue is fixed by using
`QgsCoordinateReferenceSystem::setDestinationCrs` which handles the
ogc urn syntax.

The test suite already covers the `urn:ogc:def:crs:EPSG::X` syntax
case. However, this issue is not detected because the requested output
crs is the same as the input one.
The axis inversion needs to be checked on the output crs, not the
input one.

The existing test does not detect this issue because the requested
output crs is the same as the input one.
To cover this case, a new test with a output crs different from the
input one is added.
This makes it easier to understand its usage.
There are 2 different parameters to check to define the output crs:
- the request parameter (`SRSNAME` from the request)
- the query crs parameter

For a `GET` request, if `SRSNAME` is set, both parameters are set and
equal.
For a `POST` request, only the query parameter is set if defined.

In `writeGetFeature()`, the `outputCrs` correctly takes into account
the query parameter. However, this is not the case for the output srs
name (`srsName`) which only takes into account the query CRS as an
auth id.
Therefore, the output srsName will always be defined as an authid
even if the parameter is an ogc urn.

This issue is fixed by first computing the output srs
name (`outputSrsName`) by taking
into account the query and the query parameters. Then, this name is
used to compute `outputCrs`.
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Oct 3, 2024
@troopa81 troopa81 merged commit b3aad56 into qgis:master Oct 3, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport queued_ltr_backports Queued Backports Bug Either a bug report, or a bug fix. Let's hope for the latter! Server Related to QGIS server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants