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

Supporting TCRS configuration through gridset selector #375

Closed
wants to merge 46 commits into from

Conversation

dromagnoli
Copy link
Member

Checklist

For core and extension modules:

  • New unit tests have been added covering the changes.
  • Documentation has been updated (if change is visible to end users).
  • The REST API docs have been updated (when changing configuration objects or the REST controllers).
  • There is an issue in the GeoServer Jira (except for changes that do not affect administrators or end users in any way).
  • Commit message(s) must be in the form [GEOS-XYZWV] Title of the Jira ticket.
  • Bug fixes and small new features are presented as a single commit.
  • Each commit has a single objective (if there are multiple commits, each has a separate JIRA ticket describing its goal).

The dependency `AWS SDK` of the `cog-s3` plugin requires additional libraries after it was upgraded from version 2.24.13 to 2.27.23.

This fixes the issue discussed in https://discourse.osgeo.org/t/missing-jars-for-cog-s3-plugin-in-assembly-xml/110332
@dromagnoli
Copy link
Member Author

Code depends on this GeoTools PR adding PROJ formatting support: geosolutions-it/geotools#84

MapML XML/XSD has been updated to modify the projection elements.
It was tied to ProjType which is an enum only supporting the 4 Built-in Projections (WGS84,CBMTILE,APSTILE,OSMTILE).
I have created a WrappingProjType which encapsulates the ProjType and modified projection elements of xsd/xml to be simple String instead of ProjType type. Please, let me know if that's ok.

Side Question on the next task related to Projection support:
SW.4.1.19 requires allowing GeoServer admin to set the default projection MapML URL.
At the moment I think is it already supported.
Admin can specify default Projection during layer configuration, by specifying a MapML:CRS as Declared CRS and select Reproject Native to Declared.
image

The generated LayerPreview will use that CRS.
http://localhost:8080/geoserver/topp/wms?service=WMS&version=1.1.0&request=GetMap&layers=topp%3Astates&bbox=-2154706.877127081%2C2763762.2272634404%2C3844204.014332743%2C5981537.174078367&width=768&height=411&srs=MapML%3AUTM14WGS84Quad&styles=&format=text%2Fhtml%3B%20subtype%3Dmapml

image
image

Is it what is requested?
Or do we actually need a different feature where we want the possibility to specify a Declared CRS and specify a different Default output CRS?

@prushforth
Copy link
Collaborator

prushforth commented Oct 21, 2024

MapML XML/XSD has been updated to modify the projection elements.
It was tied to ProjType which is an enum only supporting the 4 Built-in Projections (WGS84,CBMTILE,APSTILE,OSMTILE).
I have created a WrappingProjType which encapsulates the ProjType and modified projection elements of xsd/xml to be simple String instead of ProjType type. Please, let me know if that's ok.

That is ok! One thing I don't want to have happen is to allow the projection name string to contain :, as I don't want users to use for example, "EPSG:4326" as the name of a TCRS, since they are definitely not the same kind of thing. I noticed that the data type used in the rnc was string but in the xsd it's anyURI which might lead to trouble, but I suppose even a plain string without the : can be considered a URI. If I use the example pictured, we want to use MapML:UTM14WGS84QUAD as the value required by GeoServer (GetMap srsName parameter requirement), but inside the HTML and MapML document it should be used without the prefix:

<mapml-viewer projection="UTM14WGS84QUAD" ...>...</mapml-viewer>

Side Question on the next task related to Projection support:
SW.4.1.19 requires allowing GeoServer admin to set the default projection MapML URL.
At the moment I think is it already supported.

You are correct.

Admin can specify default Projection during layer configuration, by specifying a MapML:CRS as Declared CRS and select Reproject Native to Declared.

This is done, thanks!

@dromagnoli
Copy link
Member Author

One thing I don't want to have happen is to allow the projection name string to contain :

This is already addressed, there is also a note in the GridSet to TCRS Selector documentation that reports that:
https://github.com/geosolutions-it/geoserver/pull/375/files#diff-3bb5273b2d308af6af5f3b5f39e968a4307b50fe4084adf3e6dda3f2c8935fe8R73

Quoting:
Gridsets containing ":" character in the name won't be listed

@dromagnoli dromagnoli requested a review from aaime October 28, 2024 09:34
aaime and others added 4 commits October 28, 2024 10:50
…ver#7984)

Bumps [org.springframework.security:spring-security-web](https://github.com/spring-projects/spring-security) from 5.8.14 to 5.8.15.
- [Release notes](https://github.com/spring-projects/spring-security/releases)
- [Changelog](https://github.com/spring-projects/spring-security/blob/main/RELEASE.adoc)
- [Commits](spring-projects/spring-security@5.8.14...5.8.15)

---
updated-dependencies:
- dependency-name: org.springframework.security:spring-security-web
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Copy link
Member

@aaime aaime left a comment

Choose a reason for hiding this comment

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

Overall looks good, see comments below


@Test
public void testGridsetToTiledCRS() throws Exception {
CoordinateReferenceSystem crs = CRS.decode("MapML:UTM31WGS84Quad");
Copy link
Member

Choose a reason for hiding this comment

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

Good. I would also check that:

  • The code shows up in "CRS.getSupportedCodes()
  • A GUI test that adds a built-in valid griset as a valid TRCS, and after the submission, the CRS class can find it

Copy link
Member Author

Choose a reason for hiding this comment

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

Tests Added.

jodygarnett and others added 14 commits October 29, 2024 19:13
The docuemntation was restructured to seperate out administrator from worksapce administrator in response to security vulnerability on log file traversal.
geoserver#7961)

fall back to using the URL of the capabilities document itself.

Update MapMLBaseProxyTest.java to check the cascaded request against the
request URL from the capabilities document OR if that is null, use the
URL OF the capabilities document as the base.

Update assertCascading in MapMLWMTSProxyTest.java to compare against
resource request URL from capabilities document OR if that is null,
use the URL OF the capabilities document as the base.
…ment section

* Centralize the declaration of geotools dependencies to the root pom's
  dependencyManagement and remove `<version>${gt.version}</version>` in
  module poms for consistency.

* Exclude `xml-apis` and apache `fop` in the root pom dependency management section
  Of especial interes for the `gt-app-schema` module not to carry over the
  unnecessary (sice Java 1.6) `xml-apis` jar, whose presence also makes the
  eclipse IDE complain there are multiple sources for classes
  in the `javax.xml.namespace`, `org.w3c.dom`, and other packages.

commit 397981a4e9dc310219270f82b2e560f6d3de4127
Author: Gabriel Roldan <[email protected]>
Date:   Tue Aug 6 12:05:38 2024 -0300
…version_from_root_pom

Declare all GeoTools dependencies in the root pom's dependency management section
aaime and others added 10 commits November 6, 2024 17:08
The way it is stated now is misleading, because the LEGEND_OPTIONS params are required, but they are mentioned after the optional parameters.
Add a data directory under build/cite/ogcapi-features10.

This is an extract of the [release](../../../data/release/) data directory
without the raster layers and their corresponding datasets, for the purpose
of executing the ogcapi-features-1.0 conformance test suite against this
well known datasets.

Note although it adds the shapefiles and geopackage again, they won't incur
in any size overhead for the repository given the way git stores files.
This commit adds support for running CITE (Compliance & Interoperability Testing Evaluation)
tests for `OGC API - Features 1.0`. It includes updates to dependencies, configuration changes,
a new CI workflow, and test data handling to ensure proper validation of GeoServer¿s conformance.

* The new `ogcapi-features10` test suite runs against the latest version of the
  [ogccite/ets-ogcapi-features10:1.7.1-teamengine-5.4.1](https://hub.docker.com/r/ogccite/ets-ogcapi-features10/tags)
   "TEAM Engine and OGC API - Features test suite", using the TeamEngine¿s REST API. Refer to the "Using the REST API"
   section in the [User Guide](https://opengeospatial.github.io/teamengine/users.html) for more details.
* Introduced a new GitHub Actions workflow that builds and runs the CITE tests.
* The CITE tests run against all feature types present in the `release` data directory.
    * Configured the WFS to declare `EPSG:4326` and `EPSG:3857` for all FeatureTypes,
      ensuring that tests do not attempt reprojection between incompatible coordinate systems or boundaries.
    * Disabled the `tiger:giant_polygon` FeatureType as the test suite tries to reproject it
      to CRSs that can't cover its single world-wide polygon.
* Updated the CITE build and run scripts located in `build/cite/` to ensure they can be executed.
    * The existing CITE tests continue to run via CLI commands against the `geoserver-docker.osgeo.org/geoserver-cite:teamengine_latest` image.
      to include only the necessary dependencies for this specific test suite.
    * Updated the `geoserver.war` file used for testing to default to the
      locally built version. A `make war` target has been introduced.
    * Added an `ogcapi-features` Maven profile in `src/web/app/pom.xml`
@dromagnoli
Copy link
Member Author

dromagnoli commented Nov 15, 2024

@aaime.
All the feedbacks have been addressed, converting to publish PR to the official Repo.

@aaime
Copy link
Member

aaime commented Nov 15, 2024

Go for the official repo!

groldan and others added 10 commits November 17, 2024 13:53
`EPSG:4087` (WGS 84 / World Equidistant Cylindrical) is a global
projection, while `EPSG:3857` (WGS 84 / Pseudo-Mercator) is not.

Since the `ogcapi-features-1.0` ETS fails while doing coordinate
transformations on itself, despite GeoServer having returned
transformed geometries (albeit outside the area of validity of
the target CRS), let's use two default published CRS's that
cover the whole globe.

Re-enable the `tiger:giant_polygon` layer, since now it can be
reprojected across the declared global CRS's.
…imitations

The ets-ogcapi-features10 test suite will chek the transformed collection items
fall inside the collection's declared spatialExtent (WGS84). In doing so, the test
themselves are limited to 7 decimals for geographic coordinates, and hence will fail
with GeoServer's higher accuracy, despite GeoServer responses being correct.
…nal fixes

Use a patched version of `ogccite/ets-ogcapi-features10:1.8-SNAPSHOT-teamengine-5.4.1`.

Currently, we require the following pull request to be applied:
[Fix global crs codes lookup in discovery collections crs tests](opengeospatial/ets-ogcapi-features10#255)
to the test suite.

This may happen again, hence we're introducing a build step to generate
the required `ogccite/ets-ogcapi-features10` image out of the
`geoserver/ets-ogcapi-features10` fork, using a branch named `geoserver/integration`.

The procedure to maintain that branch is to rebase on top of the upstream
master branch as required, and apply the additional branches with `git merge --no-ff`.
Use a patched ogccite/ets-ogcapi-features10 Docker image with additional fixes
Test ogcapi-features both

* Respects the configured number of decimal places when encoding GeoJSON,
both in the Collection document bounds and for items coordinates (this last
part is performed already by the WFS  GeoJSON encoder)
* The encoded collection bounds fully covers the items, as asserted by
the CITE tests,
@dromagnoli dromagnoli force-pushed the mapml-tcrs-proj branch 2 times, most recently from c239c1d to b7cfb49 Compare November 19, 2024 13:01
@aaime
Copy link
Member

aaime commented Dec 20, 2024

@prushforth fix for axis order merged here: geoserver#8143

@aaime aaime closed this Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants