Justin Deoliveira wrote:
> Gabriel Roldan wrote:
>> Hi guys,
>>
>>
>> I'm wondering what to do since the fix for
>> <http://jira.codehaus.org/browse/GEOT-2403> breaks the
>> org.geotools.gml3.v3_2.GMLParsingTest.
>> But it actually exposes a bug in gml parsing, so it does not break
>> it, just uncovers it.
>>
>> Explanation:
>> the fix for GEOT-2403 stops SimpleFeatureImpl from returning "the
>> first non null geometry" when asked for the default geometry, but
>> instead returns the default geometry value, which may well be null.
>> Otherwise it is inconsistent with FeatureType.getGeometryAttribute.
>> So the patch is good.
>>
>> But, GMLParsingTest.testParseFeatureCollection, at line 75 does
>>
>> assertTrue( f.getDefaultGeometry() instanceof Point );
>>
>>
>> which is fine, but it sort of relied on SimpleFeatureImple
>> accidentally returning the first non null geometry instance found.
>>
>> The root of the problem being that the proper default geometry
>> attribute is not being set on the FeatureType parsed by
>> GMLParsingUtils.featureType.
>>
>> Since this method is not setting the default geometry attribute, and
>> since the SimpleFeatureType being built extends from
>> AbstractFeatureType, the SimpleFeatureTypeImpl implementation is
>> setting the default geometry to be gml:location instead.
>>
>> Now, why GMLParsingUtils.featureType is not setting the default
>> geometry? because of the namespace change in GML 3.2.
>>
>> Lines 254 to 263 in GMLParsingUtils:
>>
>> //set the default geometry explicitly
>> if (Geometry.class.isAssignableFrom(theClass)
>> &&
>> !GML.NAMESPACE.equals(property.getTargetNamespace())) {
>> //only set if non-gml, we do this because of
>> "gml:location",
>> // we dont want that to be the default if the user has
>> another
>> // geometry attribute
>> if (ftBuilder.getDefaultGeometry() == null) {
>> ftBuilder.setDefaultGeometry(property.getName());
>> }
>> }
>>
>> The GML.NAMESPACE being compared is the one in the gml2 package, and
>> though it matches the GML 3.1.2 namespace, it does not match the GML
>> 3.2 namespace, hence the default geom property is not set.
>>
>>
>> As I don't quite see a straightforward solution, I'm asking you for
>> any clue on how to proceed. What I thought so far is to change the
>> static reference to GML.NAMESPACE for the namespace of the XSD
>> instance being used, whether it is gml2, gml3.1 or gml3.2... But I
>> don't see how to get to that XSD instance from inside that method?
>> any idea?
> HMmm... yeah, there is no great way to get the actual URI. You could
> get the XSDSchema object from the XSDElementDeclaration... but getting
> the "gml" namespace would be a heuristic.
>
> What would probably work just as well (although a bit of a hack) would
> be just to make the comparison startsWith(), rather than equals. That
> would pick up both gml namespaces.
yup, I was kind of wanting to find a cleaner solution but if the
following is ok for you I can go for it:
Index: src/main/java/org/geotools/gml2/bindings/GML2ParsingUtils.java
===================================================================
--- src/main/java/org/geotools/gml2/bindings/GML2ParsingUtils.java
(revision 32695)
+++ src/main/java/org/geotools/gml2/bindings/GML2ParsingUtils.java
(working copy)
@@ -251,9 +251,11 @@
// create the type
ftBuilder.minOccurs(min).maxOccurs(max).add(property.getName(), theClass);
- //set the default geometry explicitly
+ //set the default geometry explicitly. Note we're comparing
the GML namespace
+ //with String.startsWith to catch up on the GML 3.2
namespace too, which is hacky.
+ final String propNamespace = property.getTargetNamespace();
if (Geometry.class.isAssignableFrom(theClass)
- &&
!GML.NAMESPACE.equals(property.getTargetNamespace())) {
+ && (propNamespace == null ||
!propNamespace.startsWith(GML.NAMESPACE))) {
//only set if non-gml, we do this because of
"gml:location",
// we dont want that to be the default if the user has
another
// geometry attribute
>>
>> Also, how can I proceed to commit the change in main for GEOT-2403
>> without breaking the build while this is sorted out?
>>
>> Cheers,
>>
>> Gabriel.
>>
>>
>>
>>
>>
>
>
------------------------------------------------------------------------------
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
