Add versioning for GML format #2746
Conversation
|
@fgravin does this break backwards compatibility or not? Since ol.format.GML is marked stable. |
Yes it does, as the format to use now is either |
|
Maybe use |
|
Ok please have a look to the 2 last commits that ensure the backward compatibility. |
|
@bartvde what do you think about the global approach ? |
|
sorry for the delay @fgravin I'll try and do proper review on Monday |
| * @type {ol.format.GMLBase|undefined} | ||
| * @api | ||
| */ | ||
| olx.format.WFSWriteGetFeatureOptions.prototype.gmlFormat; |
There was a problem hiding this comment.
I'm not sure if we need this currently. Since WFS is only WFS 1.1, and it is kind of implicitly tied to GML 3. Unless you can think of a good use case here, I'd leave this out until the WFS format gets versioned?
There was a problem hiding this comment.
I added a test to read WFS 1.0 with just changing the GML format version. The parsing worked well without the need to version the WFS too.
Actually it is the main purpose of my PR : to be able to read a WFS with GML2.
There was a problem hiding this comment.
Okay so you might want to adapt: https://github.com/openlayers/ol3/blob/master/src/ol/format/wfsformat.js#L18 ?
I don't think this applies to WFS-T though.
|
@fgravin I've put in some (minor) comments but the bigger question here is about changing static functions to instance functions. I'm not sure if this is really desired, but I don't have a good alternative as well currently. Interested to hear other opinions on this. cc @elemoine @ahocevar @tschaub Also, can you please verify that for |
@tonio, @fgravin and I discussed it before this PR was created. This is the solution we found for the case where we have a base class with version-specific child classes. We (I mean @fgravin :-) can provide more details if necessary. |
|
okay thanks for confirming @elemoine would be good to write this up somewhere I guess when other people want to implement versioned formats? Currently all format content is still in the Wiki (such as https://github.com/openlayers/ol3/wiki/Parsing-XML-documents-with-ol.xml-and-ol.format.XML) |
| function(node, opt_options) { | ||
| var geometry = this.readGeometryElement(node, | ||
| [this.getReadOptions(node, goog.isDef(opt_options) ? opt_options : {})]); | ||
| return (goog.isDef(geometry) ? geometry : null); |
all writing methods & objects are defined in v3, as writing for v2 is not done yet
to be accessed from subclasses or static objects
for backward compatibility
to avoid issue with ol.format.GML alias ol.format.GMLBase ol.format.GML2 ol.format.GML3
also changes some comments and syntax (feedback from PR comments
This is to avoid the use of this.constructor that breaks advanced build
to avoid linter warning
|
I've updated my PR
All tests passes, examples work in avdanced mode and apidoc is well generated for the alias |
|
from my side no objections to merging this |
|
@tonio to make the final call on this one… :-) |
|
LGTM. I’m working on GFI based on this for another PR. On 14 October 2014 14:19, Éric Lemoine notifications@github.com wrote:
Antoine Abt Camptocamp France SAS Tel : 00 33 4 79 44 44 94 |
This PR proposes to add versoning for GML format.
It is based on a
gml/base.jsthat contains base GML format objects and methods.Then 2 versions
ol.format.GML.v2andol.format.GML.v3inherit from base to implement<gml:coordinates><gml:outerBoundaryIs><gml:innerBoundaryIs>Also, WFS format has been changed a little to use by default an
ol.format.GML.v3format to read features from the WFS response. You can change this default behavior by passing agmlFormatoption to theol.format.WFSconstructor. You can then read a WFS 1.0.0 with GML 2 for example. A test has been added in wfs format tests.