[OpenLayers-Trac] Re: [OpenLayers] #3368: Make Protocol.WFS work with just featureType and featurePrefix

[OpenLayers]

#3368: Make Protocol.WFS work with just featureType and featurePrefix
--------------------------+-------------------------------------------------
 Reporter:  ahocevar      |       Owner:  tschaub     
     Type:  feature       |      Status:  new         
 Priority:  minor         |   Milestone:  2.12 Release
Component:  Protocol.WFS  |     Version:  2.11 RC1    
 Keywords:                |       State:  Commit      
--------------------------+-------------------------------------------------

Comment(by ahocevar):

 Thanks Bart for the thorough review. Some follow-up questions and
 explanations:

 Replying to [comment:5 bartvde]:
 >  * A question about the change you made in
 lib/OpenLayers/Format/WFST/v1_0_0.js (and 1_1_0.js as well), are you 100%
 that in all cases both prefix and featureNS will be set? Imagine the
 probably hypothetical case where only prefix is set, we would get a
 prefixed node, but no xmlns declaration leading to invalid XML. Is it not
 safer to keep the current code or check for both prefix and featureNS?

 I don't see this problem. The typeName attribute of the Query is not a
 node, so XML doesn't care what goes in there. Note that the whole point of
 this change is to make things work when '''only''' the featurePrefix and
 '''no''' featureNS is set.

 >  * Maybe add a comment to the change in
 lib/OpenLayers/Format/Filter/v1_0_0.js stating this will only happen if
 you configure geometryName with null, so it's not default behaviour to
 create invalid WFS 1.0 requests.

 Will do.

 >  * lib/OpenLayers/Protocol/WFS/v1.js: the comment about default prefix
 is this not implementation (i.e. GeoServer) specific? I don't think e.g.
 Mapserver has something equivalent to default namespace/workspace. Why not
 just state if the prefix matches the prefix of the geometry node's
 "featureNode"?

 Good point. But it's not just the prefix of the geometry node's
 "featureNode", it's "the feature prefix used by the server".

 >  * GML/Base talks about a default value of "geometry", but Protocol WFS
 mentions "the_geom". So isn't the default value geometry?

 This really depends on the inheritance chain and the mixins used. I just
 used what the docs already said before my change. If you can confirm that
 it is "geometry" in this case (which I doubt, because otherwise we
 wouldn't have been able to use this with GeoServer ever without setting
 the geometryName), I'll change it.

 >  * Put {{{var format = this.readFormat || this.format; }}} before the
 read call to simplify.

 I had this already, but changed it back because one read call is with
 options and the other without, and I didn't investigate if the options do
 harm in the case where we currently don't use them. If you can confirm
 that they don't, I'll change it.

 >  * Trailing comma in tests/Format/WFST/v1_0_0.html and
 tests/Format/WFST/v1_1_0.html
 >
 >  * tests/Format/Filter/v1_0_0.html: typo in test function name
 test_BBOX_noGeometrName
 >
 >  * tests/Protocol/WFS.html: trailing comma

 Thanks, will change them.

 >  * was the change in wfs:Delete made to pass the tests? So this mean we
 cannot write out a request anymore with: <ogc:PropertyName/> ? Or would
 this require setting geometryName to an empty string?

 Yes, this change was made to make the test pass. Do you see any side
 effects of this? Would it be desirable being able to serialize a Query
 that has an empty PropertyName element? What would be the point of doing
 so?

 I'm waiting for your feedback before I commit this.

-- 
Ticket URL: <http://trac.openlayers.org/ticket/3368#comment:6>
OpenLayers <http://openlayers.org/>
A free AJAX map viewer