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

Checks for bbox CRS definition are generally invalid #249

Open
aaime opened this issue Nov 12, 2024 · 2 comments
Open

Checks for bbox CRS definition are generally invalid #249

aaime opened this issue Nov 12, 2024 · 2 comments
Assignees

Comments

@aaime
Copy link

aaime commented Nov 12, 2024

Describe the bug
BBoxCrsParameter#verifyBboxCrsParameterWithDefaultCrs performs two requests with two different bounding boxes:

  • The first one in the "default CRS"
  • The second one in the same bbox transformed onto a different, target CRS

The test then goes to check the list of returned feature ids in both cases, and checks they are the same set of ids.

However, there are a few issues in this:

  • If the original BBOX covers the entire world (maxExtent, lin 144), then the bbox is "reduced a bit", 5 degrees per side. This reduction implies features returned by the original bounds might not be in the reprojected result. (Also, the test is very weak, for example, a Countries dataset will touch the south pole, due to Antarctica, but not the north pole, as there is not land there... leading to south pole being projected to infinite if one is using 3857).
  • The requests performed are limited to the first 10 items. But it's not safe to assume that two different requests against the same spatial area will return the same first 10 features, depending on how the spatial filter execution is performed, the order of the results could be different (one should really check the entire possible set of features instead)
  • As reported in the past already, envelope reprojection is a difficult matter, as the lines of the bbox could be turned into curves during reprojection, while the test suite is just reprojecting the corners of the envelope instead. This is especially true if the datasets involved have large area coverage (eventually the whole world), while this issue would be less of a problem over small areas (e.g, a city).

I don't believe the current test logic can be easily fixed. I would suggest to change it:

  • Add the "crs" parameter so that the returned geometries are in the target CRS
  • Verify that the returned features footprints are crossing the reprojected bbox

Even this will be subject to issues (e.g Antartica will fail to reproject to 3857).

Maybe the test suite should ask for a "safe bbox" for all layers instead, and use it to issue requests? Just thinking out loud, mind.

@dstenger
Copy link
Contributor

Thank you for reporting. We will do further investigation and check if the logic of the test can be updated.

@dstenger dstenger added this to CITE Nov 13, 2024
@github-project-automation github-project-automation bot moved this to To do in CITE Nov 13, 2024
@dstenger dstenger moved this from To do to In progress in CITE Nov 13, 2024
@dstenger
Copy link
Contributor

We reviewed your thoughts and agree that there is room for improvement.
An update of the logic of the test suite might be the solution here making the tests more robust against edge cases.

If this problem prevents certification of your API, please get in touch with us.

We will work on a solution as soon as there are free resources again.

@dstenger dstenger moved this from In progress to To do in CITE Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To do
Development

No branches or pull requests

3 participants