Add ol.format.WMSCapabilities format #1610

Merged
merged 9 commits into from Mar 11, 2014

Projects

None yet

6 participants

Owner
fredj commented Jan 28, 2014

work in progress

Owner
bartvde commented Jan 28, 2014

I'm assuming the end version won't have an interface like readFeatures etc. ? Since we're not reading features with this format.

Owner
bartvde commented Jan 28, 2014

is the plan to support multiple versions of WMS Capabilities? i.e. a versioned format?

Contributor
twpayne commented Jan 28, 2014

@fredj, as @bartvde says, ol.format.WMSCapabilities should not inherit from ol.format.Format since it does not contain features or geometries.

Owner
fredj commented Jan 28, 2014

Should it stays ol.parser.ogc.WMSCapabilities?

Contributor
twpayne commented Jan 28, 2014

Should it stays ol.parser.ogc.WMSCapabilities?

Opinion is divided :) I personally would put all feature/geometry formats in ol.format and keep ol.parser for everything else (styles, capabilities, transactions, etc.) but @tschaub disagrees.

For the moment, it might be easiest to keep it in ol.format, but not inherit from ol.format.Format.

Owner
bartvde commented Jan 28, 2014

Personally I would be okay with having the separation between format (geometries and features) and parser (everything else).

Contributor
htulipe commented Feb 12, 2014

IMHO, @tschaub is right on terminology, "format" and "parser" are not related to geometry/anything else.

Here is a proposition from an outside look:

  • Drop the "format" word, it relates to a type of data in an abstract way.
  • Keep "parser" because this is what those modules are about, they parse stuff. This is more concrete.

This scenario implies to:

  • rename ol.format.Format to ol.parser.FeaturesParser (or ol.parser.feature.Parser)
  • rename ol.format.XML to ol.parser.XMLFeaturesParser (or ol.parser.feature.xml.Parser)

And so on for current format modules. This way, the WMS getcap parser could be named ol.parser.WmsGetCapabilitesParser (or ol.parser.layer.wms.GetCapabilitiesParser).

I think this makes more sense since right now the name ol.format.XML is misleading. One can think it is supposed to read XML whereas it reads features in XML.

Owner
fredj commented Feb 19, 2014

The Capability part is ready, continuing with the Service part

Owner
fredj commented Feb 25, 2014

Service part finished, cleaning up ...

Contributor
twpayne commented Feb 28, 2014

@fredj, at the hangout yesterday we agreed that we should use a single ol.format namespace, including for non-feature formats. Instead, feature formats inherit from ol.format.Feature. So, please can you rename ol.parser.WMSGetCapabilities to ol.format.WMSGetCapabilities. Thanks!

Owner
fredj commented Mar 5, 2014

Class renamed, ready to review.

@bartvde bartvde and 1 other commented on an outdated diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+ for (var n = doc.firstChild; !goog.isNull(n); n = n.nextSibling) {
+ if (n.nodeType == goog.dom.NodeType.ELEMENT) {
+ return this.readFromNode(n);
+ }
+ }
+ return null;
+};
+
+
+/**
+ * @param {Node} node Node.
+ * @return {Object} WMS Capability object.
+ */
+ol.format.WMSCapabilities.prototype.readFromNode = function(node) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ goog.asserts.assert(node.localName == 'WMS_Capabilities');
bartvde
bartvde Mar 5, 2014 Owner

So I assume this means this format is only targeting WMS 1.3 right?

fredj
fredj Mar 5, 2014 Owner

Fixed, thanks

@twpayne twpayne commented on the diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+
+
+/**
+ * @private
+ * @param {Node} node Node.
+ * @param {Array.<*>} objectStack Object stack.
+ * @return {ol.Extent|undefined} Bounding box object.
+ */
+ol.format.WMSCapabilities.readEXGeographicBoundingBox_ =
+ function(node, objectStack) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ goog.asserts.assert(node.localName == 'EX_GeographicBoundingBox');
+ var geographicBoundingBox = ol.xml.pushParseAndPop(
+ /** @type {ol.format.EXGeographicBoundingBoxType} */ ({}),
+ ol.format.WMSCapabilities.EX_GEOGRAPHIC_BOUNDING_BOX_PARSERS_,
+ node, objectStack);
twpayne
twpayne Mar 5, 2014 Contributor

If parsing the geographic bounding box fails then o.xml.pushParseAndPop will return undefined. In this case, this function should also return undefined. This should also allow you to remove the typecast in the following line.

@twpayne twpayne commented on an outdated diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+ return ol.xml.pushParseAndPop(
+ {}, ol.format.WMSCapabilities.ATTRIBUTION_PARSERS_, node, objectStack);
+};
+
+
+/**
+ * @private
+ * @param {Node} node Node.
+ * @param {Array.<*>} objectStack Object stack.
+ * @return {Object} Bounding box object.
+ */
+ol.format.WMSCapabilities.readBoundingBox_ = function(node, objectStack) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ goog.asserts.assert(node.localName == 'BoundingBox');
+
+ var extent = /** @type {ol.Extent} */ ([
twpayne
twpayne Mar 5, 2014 Contributor

Is the typecast really needed?

@twpayne twpayne commented on an outdated diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+ * @param {Node} node Node.
+ * @param {Array.<*>} objectStack Object stack.
+ * @return {Object} Bounding box object.
+ */
+ol.format.WMSCapabilities.readBoundingBox_ = function(node, objectStack) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ goog.asserts.assert(node.localName == 'BoundingBox');
+
+ var extent = /** @type {ol.Extent} */ ([
+ ol.format.XSD.readDecimalString(node.getAttribute('minx')),
+ ol.format.XSD.readDecimalString(node.getAttribute('miny')),
+ ol.format.XSD.readDecimalString(node.getAttribute('maxx')),
+ ol.format.XSD.readDecimalString(node.getAttribute('maxy'))
+ ]);
+
+ var resolutions = /** @type {Array.<number>} */ ([
twpayne
twpayne Mar 5, 2014 Contributor

As above, is the typecast needed?

@twpayne twpayne commented on the diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+
+/**
+ * @private
+ * @param {Node} node Node.
+ * @param {Array.<*>} objectStack Object stack.
+ * @return {Object|undefined} Layer object.
+ */
+ol.format.WMSCapabilities.readLayer_ = function(node, objectStack) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ goog.asserts.assert(node.localName == 'Layer');
+ var parentLayerObject = /** @type {Object.<string,*>} */
+ (objectStack[objectStack.length - 1]);
+
+ var layerObject = /** @type {Object.<string,*>} */ (ol.xml.pushParseAndPop(
+ {}, ol.format.WMSCapabilities.LAYER_PARSERS_, node, objectStack));
+
twpayne
twpayne Mar 5, 2014 Contributor

(Comment applies to line below, but am putting it here for clarity)

If you write:

if (!goog.isDef(layerObject)) {
  return undefined;
}

You can then un-indent the block below and ensure that the function always explicitly returns a value.

@twpayne twpayne commented on an outdated diff Mar 5, 2014
src/ol/format/wmscapabilitiesformat.js
+};
+
+
+/**
+ * @private
+ * @param {Node} node Node.
+ * @param {Array.<*>} objectStack Object stack.
+ * @return {Object|undefined} Online resource object.
+ */
+ol.format.WMSCapabilities.readSizedFormatOnlineresource_ =
+ function(node, objectStack) {
+ goog.asserts.assert(node.nodeType == goog.dom.NodeType.ELEMENT);
+ var formatOnlineresource =
+ ol.format.WMSCapabilities.readFormatOnlineresource_(node, objectStack);
+ if (goog.isDef(formatOnlineresource)) {
+ var size = /** @type {ol.Size} */ ([
twpayne
twpayne Mar 5, 2014 Contributor

Not sure if the typecast is needed here.

Contributor
twpayne commented Mar 5, 2014

Very nice! I've added a few fairly minor comments.

Contributor
htulipe commented Mar 5, 2014

Cool! Look forward to see that on master.

Contributor
twpayne commented Mar 11, 2014

Thanks for addressing the comments @fredj. This looks great! Please merge.

Owner
bartvde commented Mar 11, 2014

+1 on getting this in as well, thanks for the work

Owner
fredj commented Mar 11, 2014

Thanks, merging

@fredj fredj merged commit 7b7bc78 into openlayers:master Mar 11, 2014

1 check passed

default The Travis CI build passed
Details
@fredj fredj deleted the fredj:wms-capabilities branch Mar 11, 2014
Owner

I haven't looked at the code, but thank you all for the work on this. Very nice to see this merged.

Owner

A post-commit +1 from me too. Thanks!

This was referenced Mar 12, 2014
Owner
bartvde commented Apr 20, 2015

@fredj can you clarify if this parser is supposed to work for version 1.1.1? Or only 1.3.0?

Owner
bartvde commented Apr 20, 2015

Looking at the output of the parser it seems a larger part of a WMS 1.1.1 doc is parsed, but this is probably only due to the common aspects / elements between 1.1.1 and 1.3.0? So my guess is this code is not intended to be used on 1.1.1, and adding 1.1.1 would mean splitting the parser up into a base and versioned classes similar to what has been done for GML.

Owner
fredj commented Apr 20, 2015

If I remember correctly it has only be tested for 1.3.0

Owner
bartvde commented Apr 20, 2015

thanks for clarifying @fredj

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment