OpenLayers OpenLayers

Ticket #2164 (closed feature: fixed)

Opened 9 months ago

Last modified 6 months ago

complete parsing of WMSCapabilities document

Reported by: trondmm Assigned to: tschaub
Priority: minor Milestone: 2.9 Release
Component: Format Version: 2.8
Keywords: Cc:
State: Complete

Description

Add handling of AccessConstraints, Address, AddressType, AuthorityURL, BoundingBox, City, ContactAddress, ContactElectronicMailAddress, ContactFacsimileTelephone, ContactInformation, ContactOrganization, ContactPerson, ContactPersonPrimary, ContactPosition, ContactVoiceTelephone, Country, DataURL, DescribeLayer, Dimension, Exception, Extent, FeatureListURL, Fees, GetCapabilities, GetFeatureInfo, GetLegendGraphic, GetStyles, Identifier, PostCode, PutStyles, SRS, StateOrProvince and UserDefinedSymbolization tags, plus cascaded, fixedWidth, fixedHeight, opaque and noSubsets attributes of Layer tag.

This pretty much covers everything a WMS 1.1.1 GetCapabilities document can contain, except VendorSpecificCapabilities. I've also not attempted to parse the HTTP, Get or Post tags from DCPType, as that's likely to break compatibility.

Tests are included, and they pass on FF2 and FF3 on Linux, FF3 and IE6 on Windows

Attachments

ticket2164.patch (29.5 kB) - added by trondmm on 06/30/09 14:59:20.
complete parsing of WMSCapabilities document
2164.patch (28.6 kB) - added by tschaub on 09/11/09 20:03:40.
extra element parsing

Change History

06/30/09 14:59:20 changed by trondmm

  • attachment ticket2164.patch added.

complete parsing of WMSCapabilities document

07/16/09 15:14:01 changed by fvanderbiest

Thank's for the patch ! I've applied it successfully. Will send feedback about it in the next days.

(follow-up: ↓ 3 ) 07/17/09 09:25:34 changed by fvanderbiest

In read_cap_Layer, the layer's "identifiers" object is created empty, and does not seem to be filled later. Should'nt it be registered into the complexAttr array ?

(in reply to: ↑ 2 ) 07/17/09 11:53:41 changed by trondmm

Replying to fvanderbiest:

In read_cap_Layer, the layer's "identifiers" object is created empty, and does not seem to be filled later. Should'nt it be registered into the complexAttr array ?

identifiers is filled by the read_cap_Identifier method, if a corresponding authorityURL has been found by the read_cap_AuthorityURL method. Maybe it would've been better to check for its existence in the read_cap_Identifier method, and only create it if it's actually needed.

identifiers is not part of the complexAttr array because a layer does not inherit this value from its parent layer.

07/23/09 22:06:47 changed by tschaub

This is a really nice addition. Thanks for the work trondmm.

The reason I wasn't parsing SRS before was for performance issues in IE. With this patch, the wms cap tests look to run about 50% slower in IE6. I've tried this out in a few applications and I don't think the penalty is that bad in practice. This format is due for a rewrite - and it would be good to look for performance gains then. In the meantime, if we put this in and people complain, we could consider an option to disable srs parsing (and even consider disabling it by default).

Still reviewing.

09/11/09 20:03:40 changed by tschaub

  • attachment 2164.patch added.

extra element parsing

09/11/09 20:07:02 changed by tschaub

So, with r9659, we've got a simple way to create some WMS caps parsing benchmarks.

Without this patch applied, I'm seeing ~140ms per run in FF3.5. With the patch applied, this is bumped up to ~340ms per run. Both could be said to be negligible given other aspects of application loading. I need to poke around some more to back up the "boggy app" anecdotes (could be more significant in IE).

09/11/09 22:18:17 changed by tschaub

In IE6, running tests/speed/wmscaps.html shows parsing takes about 700ms without the patch applied and about 3.6s with the patch applied.

My feeling it that this is significant enough to make the default behavior a bit closer to what it was before. The parser could use a serious overhaul, but I'll look at just disabling the srs push for now.

09/11/09 22:37:36 changed by tschaub

Ok, if we leave layer.srs as an object, things aren't as bad in IE. It looks like ~1.3s instead of 3.6s per parse.

For most use cases, I actually think having an srs object makes more sense than having an array. Testing layer.srsepsg:4326? is more efficient, for example.

09/11/09 22:44:27 changed by tschaub

Ok, with a few minor changes, I think this is good to go.

Most of my changes were formatting & minor syntax changes. The one functional change was to remove the srs array push - leaving layer.srs an object.

I've updated the tests. All pass in FF3.5 and IE6.

Thanks for this excellent work. It will be nice to have a more complete representation of the capabilities available.

09/11/09 22:47:36 changed by tschaub

trondmm, it looks like we need a CLA from you. Please see the how-to for information on submitting one.

09/11/09 22:48:56 changed by tschaub

  • keywords deleted.
  • status changed from new to closed.
  • state changed from Review to Complete.
  • resolution set to fixed.

(In [9664]) Adding more complete WMS capabilities parsing. Thanks trondmm for the comprehensive patch. Some minor changes by me - including leaving the srs member value an object. r=me (closes #2164)