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

Add support for WMS 1.1.1 + other WMS improvements #6173

Merged
merged 18 commits into from
Mar 10, 2022
Merged

Conversation

nf-s
Copy link
Contributor

@nf-s nf-s commented Mar 7, 2022

Fixes TerriaJS/neii-viewer#218 #5410 #4777 #2590

  • Improve WMS 1.1.1 support
    • Added useWmsVersion130 trait - Use WMS version 1.3.0. True by default (unless url has "version=1.1.1" or "version=1.1.0"), if false, then WMS version 1.1.1 will be used.
    • Added getFeatureInfoFormat trait - Format parameter to pass to GetFeatureInfo requests (as info_format query parameter). Defaults to "application/json", "application/vnd.ogc.gml", "text/html" or "text/plain" - depending on GetCapabilities response
  • Add legendBackgroundColor to LegendOwnerTraits and backgroundColor to LegendTraits
  • Add sld_version=1.1.0 to GetLegendGraphics requests
  • Filter "styles","version","format","srs","crs" conflicting query parameters from WMS url
  • WMS styles, tileWidth, tileHeight and crs/srs will use value in url if it is defined (similar to existing behavior with layers and version)
  • WMS will now show warning if invalid layers (eg if the specified layers don't exist in GetCapabilities)

Notes about WMS query changes

WMS Exceptions

application/vnd.ogc.se_xml is the default value for WMS 1.1.1
XML is the new default for WMS 1.3.0

From WMS 1.1.1 spec

image

From WMS 1.3.0 spec

image

Add sld_version to GetLegendGraphics requests

As it is mandatory

From WMS-SLD 1.1.0 spec

image

Test: WMS 1.1.1 and Legend backgroundColor

Catalog

{
  "workbench": [],
  "catalog": [
    {
      "type": "wms",
      "name": "Legend with background test (and WMS 1.1.1)",
      "url": "http://www.bom.gov.au/cgi-bin/ws/gis/ncc/cdio/wxs?service=WMS&version=1.1.1&request=GetCapabilities",
      "layers": "IDCJCM001900"
    }
  ]
}

After vs Before

image

You can ignore error in main CI link

Note in main

  • Feature picking doesn't work
  • No Legend present

Test: random WMS working before and after

Some random other WMS layers in NationalMap - should be unchanged for both branches

Test: WMS with invalid layers

After vs Before

image

Checklist

  • There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • I've updated relevant documentation in doc/.
  • I've updated CHANGES.md with what I changed.
  • I've provided instructions in the PR description on how to test this PR.

@nf-s nf-s marked this pull request as draft March 8, 2022 02:49
@nf-s nf-s changed the title update WMS default parameters to 1.3.0 Add support for WMS 1.1.1 Mar 8, 2022
@nf-s nf-s changed the title Add support for WMS 1.1.1 Add support for WMS 1.1.1 + other WMS improvements Mar 9, 2022
@nf-s nf-s mentioned this pull request Mar 9, 2022
4 tasks
Comment on lines +190 to +199
if (this.invalidLayers.length > 0)
throw new TerriaError({
sender: this,
title: i18next.t("models.webMapServiceCatalogItem.noLayerFoundTitle"),
message: i18next.t(
"models.webMapServiceCatalogItem.noLayerFoundMessage",
{ name: getName(this), layers: this.invalidLayers.join(", ") }
)
});
}
Copy link
Collaborator

@na9da na9da Mar 10, 2022

Choose a reason for hiding this comment

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

Doubt: So we fail the dataset even if we have valid layers? Is that the intention?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think for the moment this is fine - as it is what would happen previous to the PR.

But in the future it might be worth showing a warning message for invalid layers, and then just showing valid layers anyway

Copy link
Collaborator

@na9da na9da left a comment

Choose a reason for hiding this comment

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

Great work @nf-s.

Just one comment.

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.

New v8 testing: some service layers cannot be displayed/error
2 participants