GeoTools

XSDTypeDefinition is incomplete for several GML 3.2 types

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 2.7-M4
  • Fix Version/s: None
  • Component/s: xml
  • Labels:
    None
  • Testcase included:
    yes

Description

The XSDTypeDefinition for GML 3.2 AbstractGMLType is incomplete because metaDataProperty has no type definition. (This is the complex type definition created dynamically from the schema, not the compiled types in GMLSchema.) This is likely caused by its type being hidden in deprecatedTypes.xsd and GML 3.2 relying on the single-point-of-entry gml.xsd.

One of the changes in GML 3.2 was to put a bunch of definitions in deprecatedTypes.xsd. This includes MetaDataPropertyType. This deprecated type is used in gmlBase.xsd, in particular, to define AbstractGMLType (via StandardObjectProperties). Yes, a deprecated type is used to define the base class of just about everything else. The particularly nasty thing about the way this was done is that deprecatedTypes.xsd is not imported by gmlBase.xsd. GML 3.2 is only usable if you import gml.xsd and allow it to import both gmlBase.xsd and deprecatedTypes.xsd. Parsing gmlBase.xsd will not work. You can see this in Eclipse. I don't think EMF is very happy about this either. I have no idea how to fix it.

This failure is blocking app-schema support for GML 3.2.

The attached unit test demonstrates the failure. The GML 3.1 test passes but the GML 3.2 test fails. It is also worth noting that the getQName() for metaDataProperty prints no namespace prefix because the "element" of this XSDNamedComponent is set to null. All the other children have types. 

Child element declaration types for {http://www.opengis.net/gml}AbstractGMLType :
QName: gml:metaDataProperty URI: http://www.opengis.net/gml#metaDataProperty Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@1493102 (element: [complexType: null]) (name: MetaDataPropertyType, targetNamespace: http://www.opengis.net/gml) (derivationMethod: <unset>, final: [], abstract: <unset>, contentTypeCategory: elementOnly, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)
QName: gml:description URI: http://www.opengis.net/gml#description Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@30ae41 (element: [complexType: null]) (name: StringOrRefType, targetNamespace: http://www.opengis.net/gml) (derivationMethod: extension, final: [], abstract: <unset>, contentTypeCategory: simple, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)
QName: gml:name URI: http://www.opengis.net/gml#name Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@4204 (element: [complexType: null]) (name: CodeType, targetNamespace: http://www.opengis.net/gml) (derivationMethod: extension, final: [], abstract: <unset>, contentTypeCategory: simple, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)

Child element declaration types for {http://www.opengis.net/gml/3.2}AbstractGMLType :
QName: metaDataProperty URI: http://www.opengis.net/gml/3.2#metaDataProperty Type: null <<< FAILURE
QName: gml:description URI: http://www.opengis.net/gml/3.2#description Type: org.eclipse.xsd.impl.XSDSimpleTypeDefinitionImpl@68cd79 (element: null) (name: StringOrRefType, targetNamespace: http://www.opengis.net/gml/3.2) (variety: atomic, final: null, lexicalFinal: null, validFacets: null)
QName: gml:descriptionReference URI: http://www.opengis.net/gml/3.2#descriptionReference Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@89e2f1 (element: [complexType: null]) (name: ReferenceType, targetNamespace: http://www.opengis.net/gml/3.2) (derivationMethod: <unset>, final: [], abstract: <unset>, contentTypeCategory: empty, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)
QName: gml:identifier URI: http://www.opengis.net/gml/3.2#identifier Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@92668c (element: [complexType: null]) (name: CodeWithAuthorityType, targetNamespace: http://www.opengis.net/gml/3.2) (derivationMethod: restriction, final: [], abstract: <unset>, contentTypeCategory: simple, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)
QName: gml:name URI: http://www.opengis.net/gml/3.2#name Type: org.eclipse.xsd.impl.XSDComplexTypeDefinitionImpl@18a9fc8 (element: [complexType: null]) (name: CodeType, targetNamespace: http://www.opengis.net/gml/3.2) (derivationMethod: extension, final: [], abstract: <unset>, contentTypeCategory: simple, prohibitedSubstitutions: [], lexicalFinal: null, block: null, mixed: <unset>)
  1. Text File
    GEOT-3327.app-schema.patch
    18/Apr/11 10:00 PM
    59 kB
    Ben Caradoc-Davies
  2. Text File
    GEOT-3327.xsd-gml3.patch
    18/Apr/11 10:00 PM
    55 kB
    Ben Caradoc-Davies

Issue Links

is related to

Bug - A problem which impairs or prevents the functions of the product. GEOT-3431 compatibility problem with emf

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.
relates to

Improvement - An improvement or enhancement to an existing feature or task. GEOT-3466 Upgrade EMF to 2.6.1

  • Major - Major loss of function.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ben Caradoc-Davies added a comment -
I should have mentioned: the attached test is for gt-xsd-gml3 src/test/java/org/geotools/gml3/v3_2 . I have attached the same file as a patch, for your convenience.
Show
Ben Caradoc-Davies added a comment - I should have mentioned: the attached test is for gt-xsd-gml3 src/test/java/org/geotools/gml3/v3_2 . I have attached the same file as a patch, for your convenience.
Hide
Permalink
Ben Caradoc-Davies added a comment -
(Fixed file name)
Show
Ben Caradoc-Davies added a comment - (Fixed file name)
Hide
Permalink
Justin Deoliveira added a comment -
Nasty. Well my first thought is to just hack the schema and make sure the deprecatedTypes.xsd is included by the base schema. I already had to heck the schema files in order to get them to generate properly due to the mass circular references and the like. Not pretty but I don't see much of an alternative.
Show
Justin Deoliveira added a comment - Nasty. Well my first thought is to just hack the schema and make sure the deprecatedTypes.xsd is included by the base schema. I already had to heck the schema files in order to get them to generate properly due to the mass circular references and the like. Not pretty but I don't see much of an alternative.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Thanks, Justin. Adding an extra include for deprecatedTypes.xsd in gmlBase.xsd fixes this issue; the attached unit test then passes. I am still encountering other problems integrating GML 3.2 into app-schema; I am investigating whether this EMF behaviour is causing problems in the application schema (GeoSciML 3.0rc1) as well.

Kind regards,
Ben.
Show
Ben Caradoc-Davies added a comment - Thanks, Justin. Adding an extra include for deprecatedTypes.xsd in gmlBase.xsd fixes this issue; the attached unit test then passes. I am still encountering other problems integrating GML 3.2 into app-schema; I am investigating whether this EMF behaviour is causing problems in the application schema (GeoSciML 3.0rc1) as well. Kind regards, Ben.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Justin, I have another missing type definition for you. ChildElementTypeTest in the attached GEOT-3327.ChildElementTypeTest.patch tests gts:TM_Primitive_PropertyType. I get this output:
Child element declaration types for {http://www.isotc211.org/2005/gts}TM_Primitive_PropertyType :
QName: AbstractTimePrimitive URI: http://www.opengis.net/gml/3.2#AbstractTimePrimitive Type: null <<< FAILURE

This is exactly the same failure I saw with the deprecated type, but I can't figure out why this type should fail. Adding an extra import to bring in the defining xsd does not help. I saw you have edited temporalObjects.xsd, but reverting your change has no effect.

Got any ideas?

The test also tests AbstractGMLType; as discussed before this is fixed by adding <include schemaLocation="deprecatedTypes.xsd"/> in gmlBase.xsd.
Show
Ben Caradoc-Davies added a comment - Justin, I have another missing type definition for you. ChildElementTypeTest in the attached GEOT-3327 .ChildElementTypeTest.patch tests gts:TM_Primitive_PropertyType. I get this output: Child element declaration types for { http://www.isotc211.org/2005/gts }TM_Primitive_PropertyType : QName: AbstractTimePrimitive URI: http://www.opengis.net/gml/3.2#AbstractTimePrimitive Type: null <<< FAILURE This is exactly the same failure I saw with the deprecated type, but I can't figure out why this type should fail. Adding an extra import to bring in the defining xsd does not help. I saw you have edited temporalObjects.xsd, but reverting your change has no effect. Got any ideas? The test also tests AbstractGMLType; as discussed before this is fixed by adding <include schemaLocation="deprecatedTypes.xsd"/> in gmlBase.xsd.
Hide
Permalink
Justin Deoliveira added a comment -
Nothing comes to mind immediately. But thanks for the test case. I will try to look into this asap.
Show
Justin Deoliveira added a comment - Nothing comes to mind immediately. But thanks for the test case. I will try to look into this asap.
Hide
Permalink
Justin Deoliveira added a comment -
Hi Ben,

So i looked into this and it makes sense why it is happening. Basically it comes down to the fact that the GML schema depends on GCO (though GMD) and the dependency back from GTS to GML creates a cycle that the xsd subsystem does not handle very well.

Unfortunately there is no easy way out here. Without breaking the cycle (see the GCO.addDependencies() method) the schema can simply not be built. While eclipse xsd is capable of handling this type of thing with proxies the way we build the schema objects by building dependencies first can not.

We could try to build the schema objects in a way that is more tolerant to cycles. But i am not sure exactly how that would work. Or we could try to break the cycle elsewhere so that the GCO can properly depend on GML.
Show
Justin Deoliveira added a comment - Hi Ben, So i looked into this and it makes sense why it is happening. Basically it comes down to the fact that the GML schema depends on GCO (though GMD) and the dependency back from GTS to GML creates a cycle that the xsd subsystem does not handle very well. Unfortunately there is no easy way out here. Without breaking the cycle (see the GCO.addDependencies() method) the schema can simply not be built. While eclipse xsd is capable of handling this type of thing with proxies the way we build the schema objects by building dependencies first can not. We could try to build the schema objects in a way that is more tolerant to cycles. But i am not sure exactly how that would work. Or we could try to break the cycle elsewhere so that the GCO can properly depend on GML.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Justin, I'm going to push our email discussion out here so others can see (and perhaps learn).

------- Original Message --------
Subject: Re: GML 3.2 problem
Date: Sat, 22 Jan 2011 00:26:14 +0800
From: Justin Deoliveira
To: Caradoc-Davies, Ben (CESRE, Kensington)

Hey Ben,

Sorry for the delayed response. Hope you had a nice vacation.

So yeah, this one will not be trivial by any means. As for fixing... i can think of two possible approaches.

The first would be to continue down the schema hacking path. Basically I had to break cycles to make things work properly. There are probably multiple points there the cycle can be broken. We could try to do it in a way that doesn't compromise the types in a way that they won't work for for you. Or possibly instead of just commenting out certain properties etc... dragging them inline. However this would change the namespace and probably lead to the same problems.

The second and more robust solution would be to make it so that we don't have to hack the schema object since XSD is perfectly capable of reading the underlying schemas with cycles. It is just the geotools XSD.getSchema() and XSD.getDepenencies() that have the issues with the cycles.

The idea would be to get fancy with the building and use some sort of proxying mechanism. So that when a cycle occurs in the XSD.getSchema chain a proxy object is returned for the time being and later resolved to the actual build object.

Furthermore I believe the SchemaGenerator class will also have to be modifying with the same proxying mechanism to avoid chocking on cycles.

So there you go. The second might actually be an easier route although the devil is always in the details.

Hope that helps :) Sorry for not having an easier answer.
Show
Ben Caradoc-Davies added a comment - Justin, I'm going to push our email discussion out here so others can see (and perhaps learn). ------- Original Message -------- Subject: Re: GML 3.2 problem Date: Sat, 22 Jan 2011 00:26:14 +0800 From: Justin Deoliveira To: Caradoc-Davies, Ben (CESRE, Kensington) Hey Ben, Sorry for the delayed response. Hope you had a nice vacation. So yeah, this one will not be trivial by any means. As for fixing... i can think of two possible approaches. The first would be to continue down the schema hacking path. Basically I had to break cycles to make things work properly. There are probably multiple points there the cycle can be broken. We could try to do it in a way that doesn't compromise the types in a way that they won't work for for you. Or possibly instead of just commenting out certain properties etc... dragging them inline. However this would change the namespace and probably lead to the same problems. The second and more robust solution would be to make it so that we don't have to hack the schema object since XSD is perfectly capable of reading the underlying schemas with cycles. It is just the geotools XSD.getSchema() and XSD.getDepenencies() that have the issues with the cycles. The idea would be to get fancy with the building and use some sort of proxying mechanism. So that when a cycle occurs in the XSD.getSchema chain a proxy object is returned for the time being and later resolved to the actual build object. Furthermore I believe the SchemaGenerator class will also have to be modifying with the same proxying mechanism to avoid chocking on cycles. So there you go. The second might actually be an easier route although the devil is always in the details. Hope that helps :) Sorry for not having an easier answer.
Hide
Permalink
Ben Caradoc-Davies added a comment -
-------- Original Message --------
Subject: Re: GML 3.2 problem
Date: Mon, 24 Jan 2011 17:19:26 +0800
From: Ben Caradoc-Davies
To: Justin Deoliveira

Thanks, Justin.

I have made several attempts at implementing proxies to break the cycle.
The problem I keep running into is that I do not know any way to build
an XSDSchema except by building it fully with with
XSDResourceImpl.getSchema(). As soon as I attempt to build an XSDSchema,
XSDResourceImpl/XSDSchemaImpl/XSDImportImpl call my locators and
resolvers and trigger the cycle. Any suggestions?

Kind regards,
Ben.
Show
Ben Caradoc-Davies added a comment - -------- Original Message -------- Subject: Re: GML 3.2 problem Date: Mon, 24 Jan 2011 17:19:26 +0800 From: Ben Caradoc-Davies To: Justin Deoliveira Thanks, Justin. I have made several attempts at implementing proxies to break the cycle. The problem I keep running into is that I do not know any way to build an XSDSchema except by building it fully with with XSDResourceImpl.getSchema(). As soon as I attempt to build an XSDSchema, XSDResourceImpl/XSDSchemaImpl/XSDImportImpl call my locators and resolvers and trigger the cycle. Any suggestions? Kind regards, Ben.
Hide
Permalink
Ben Caradoc-Davies added a comment -
-------- Original Message --------
Subject: Re: GML 3.2 problem
Date: Tue, 25 Jan 2011 14:06:23 +0800
From: Justin Deoliveira
To: Caradoc-Davies, Ben (CESRE, Kensington)

Well the way the org.geotools.xsd.XSD class does it is it looks through all the dependencies, creates the locators for them (that in turn the respective XSD.getSchema() for that specific schema) and then calls into parse the schema. This happens in XSD.buildSchema(). During the parsing cycle the locators are then called and the cycle introduces the issue.

A thought just occurred to me. If we didn't use the schema locator objects and just relied on the location resolvers we might avoid that cycle. And I believe xsd is smart enough internally to know when it is trying to parse an individual schema.

This scheme would be less efficient because it would involve more xsd schema objects being created but might work.

At this point I think i would need to experiment to come up with ideas that might work. I can try to carve out some time this week to do so but things are pretty busy right now. If you can try out the locator vs resolver thing that would be helpful.
Show
Ben Caradoc-Davies added a comment - -------- Original Message -------- Subject: Re: GML 3.2 problem Date: Tue, 25 Jan 2011 14:06:23 +0800 From: Justin Deoliveira To: Caradoc-Davies, Ben (CESRE, Kensington) Well the way the org.geotools.xsd.XSD class does it is it looks through all the dependencies, creates the locators for them (that in turn the respective XSD.getSchema() for that specific schema) and then calls into parse the schema. This happens in XSD.buildSchema(). During the parsing cycle the locators are then called and the cycle introduces the issue. A thought just occurred to me. If we didn't use the schema locator objects and just relied on the location resolvers we might avoid that cycle. And I believe xsd is smart enough internally to know when it is trying to parse an individual schema. This scheme would be less efficient because it would involve more xsd schema objects being created but might work. At this point I think i would need to experiment to come up with ideas that might work. I can try to carve out some time this week to do so but things are pretty busy right now. If you can try out the locator vs resolver thing that would be helpful.
Hide
Permalink
Ben Caradoc-Davies added a comment -
-------- Original Message --------
Subject: Re: GML 3.2 problem
Date: Tue, 25 Jan 2011 14:22:38 +0800
From: Ben Caradoc-Davies
To: Justin Deoliveira

Thanks, Justin. Yesterday, I tried hacking XSD as an experiment to get
rid of the locators at schema construction time (using just the
resolvers) and got an NPE. I did not understand why, but it seemed to be
having trouble using the resolver without the schema locator. I will
persevere, and check all the import paths in the schema files.

Looking through the EMF source, I think the patch() method is called to
update definitions as they are completed. I suspect you are right: EMF
can handle the cycles; it it just a matter of feeding it the schemas in
the right order.

One other thing I am doing is to remove the GML 3.2 stubs
(StubbedGMLXSD) and adding the buildTypeSchema methods as in GML (for
GML 3.2). This should make the implementation a bit more complete. If we
support cycles, the stubs should not be needed. :-)

Kind regards,
Ben.
Show
Ben Caradoc-Davies added a comment - -------- Original Message -------- Subject: Re: GML 3.2 problem Date: Tue, 25 Jan 2011 14:22:38 +0800 From: Ben Caradoc-Davies To: Justin Deoliveira Thanks, Justin. Yesterday, I tried hacking XSD as an experiment to get rid of the locators at schema construction time (using just the resolvers) and got an NPE. I did not understand why, but it seemed to be having trouble using the resolver without the schema locator. I will persevere, and check all the import paths in the schema files. Looking through the EMF source, I think the patch() method is called to update definitions as they are completed. I suspect you are right: EMF can handle the cycles; it it just a matter of feeding it the schemas in the right order. One other thing I am doing is to remove the GML 3.2 stubs (StubbedGMLXSD) and adding the buildTypeSchema methods as in GML (for GML 3.2). This should make the implementation a bit more complete. If we support cycles, the stubs should not be needed. :-) Kind regards, Ben.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Justin,

I have got rather a long report and a possible partial solution, in addition to some findings on EMF. I have attached two files:

xsd-gml3.patch modifies code and schemas and adds two unit tests that pass. It is not ready to go because it breaks at least one other module (wfs 2.0 FeatureCollectionEndingTest). I have added it here for discussion purposes. The patch restores the cyclic dependencies and adds the type schema methods. It fixes all of the import and include paths in the schemas (many were incorrect) and adds xlinks.xsd for completeness. It needs the two sub-task patches to run. All the XSDs now use a direct-to-file schema resolver, as we discussed. The NPE I ran into is because of problems preserving the correct context URL when mixing locators and resolvers (solved by not using a locator). Anyway, once these patches were in I could get some intermittent partial success in building a geosciml 3.0 core MappedFeatureType in app-schema. Still far from usable, but it is a (rather painful) start.

gts-temporalObjects-import-toplevel-gml.patch reverts part of the xsd-gml3.patch change to show that EMF does not correctly load cyclic schemas. The original gts temporalObjects.xsd imports the top-level gml.xsd. EmfXsdLoadTest uses pure EMF loading (following file based references) and can find gml:AbstractTimePrimitive when defining gts:TM_Primitive_PropertyType only if it directly imports gml 3.2 temporal.xsd. If it imports the top-level gml.xsd, it cannot find the definition of gml:AbstractTimePrimitive, despite gml.xsd including schemas that include it. I therefore conclude that EMF does not correctly include schemas in this cyclic import case. To see this behaviour, apply xsd-gml3.patch and the patches in the subtasks, then run EmfXsdLoadTest (passes). Then apply gts-temporalObjects-import-toplevel-gml.patch and run the tests again. You should see test_TM_Primitive_PropertyType fail.

This bad behaviour of EMF may have serious consequences for app-schema users because we rely on EMF to parse application schemas supplied by users. They are community-governed input data, and the users should not have to modify them. I do not know bad the problem is. Smaller scale cycles do not repeat the failure.

As an aside, did you ever consider an EMF upgrade? Some of my colleagues wonder if this might help. You once mentioned an upgrade; I expect it would be a lot of work.

Justin, I would be very interested in your thoughts.

Kind regards,
Ben.
Show
Ben Caradoc-Davies added a comment - Justin, I have got rather a long report and a possible partial solution, in addition to some findings on EMF. I have attached two files: xsd-gml3.patch modifies code and schemas and adds two unit tests that pass. It is not ready to go because it breaks at least one other module (wfs 2.0 FeatureCollectionEndingTest). I have added it here for discussion purposes. The patch restores the cyclic dependencies and adds the type schema methods. It fixes all of the import and include paths in the schemas (many were incorrect) and adds xlinks.xsd for completeness. It needs the two sub-task patches to run. All the XSDs now use a direct-to-file schema resolver, as we discussed. The NPE I ran into is because of problems preserving the correct context URL when mixing locators and resolvers (solved by not using a locator). Anyway, once these patches were in I could get some intermittent partial success in building a geosciml 3.0 core MappedFeatureType in app-schema. Still far from usable, but it is a (rather painful) start. gts-temporalObjects-import-toplevel-gml.patch reverts part of the xsd-gml3.patch change to show that EMF does not correctly load cyclic schemas. The original gts temporalObjects.xsd imports the top-level gml.xsd. EmfXsdLoadTest uses pure EMF loading (following file based references) and can find gml:AbstractTimePrimitive when defining gts:TM_Primitive_PropertyType only if it directly imports gml 3.2 temporal.xsd. If it imports the top-level gml.xsd, it cannot find the definition of gml:AbstractTimePrimitive, despite gml.xsd including schemas that include it. I therefore conclude that EMF does not correctly include schemas in this cyclic import case. To see this behaviour, apply xsd-gml3.patch and the patches in the subtasks, then run EmfXsdLoadTest (passes). Then apply gts-temporalObjects-import-toplevel-gml.patch and run the tests again. You should see test_TM_Primitive_PropertyType fail. This bad behaviour of EMF may have serious consequences for app-schema users because we rely on EMF to parse application schemas supplied by users. They are community-governed input data, and the users should not have to modify them. I do not know bad the problem is. Smaller scale cycles do not repeat the failure. As an aside, did you ever consider an EMF upgrade? Some of my colleagues wonder if this might help. You once mentioned an upgrade; I expect it would be a lot of work. Justin, I would be very interested in your thoughts. Kind regards, Ben.
Hide
Permalink
Ben Caradoc-Davies added a comment -
For good measure I have attached the app-schema part of my investigation. It includes some package poms that have to be hand created to provide geosciml 3.0 and its dependencies, and one unit test that seems to pass intermittently.
Show
Ben Caradoc-Davies added a comment - For good measure I have attached the app-schema part of my investigation. It includes some package poms that have to be hand created to provide geosciml 3.0 and its dependencies, and one unit test that seems to pass intermittently.
Hide
Permalink
Justin Deoliveira added a comment -
Hi Ben,

Great work on putting this patch together. It definitely looks like a step in the right direction. What breaks in FeatureCollectionEncodingTest exactly?

As for an update to EMF, yes this is something i am very interested in for a number of reasons, just never got around to completing it. I did however to do the upgrade locally to figure out what would break code wise, and it only introduced a few compile errors due to generics but they were all trivial to fix. So if you think an upgrade to EMF will help I would be all for it.
Show
Justin Deoliveira added a comment - Hi Ben, Great work on putting this patch together. It definitely looks like a step in the right direction. What breaks in FeatureCollectionEncodingTest exactly? As for an update to EMF, yes this is something i am very interested in for a number of reasons, just never got around to completing it. I did however to do the upgrade locally to figure out what would break code wise, and it only introduced a few compile errors due to generics but they were all trivial to fix. So if you think an upgrade to EMF will help I would be all for it.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Justin,

please review GEOT-3327.xsd-gml3.patch, which adds support for cyclic dependencies in GML 3.2. This only works because the EMF upgrade GEOT-3466 is complete, fixing EMF cyclic import behaviour. The patch:
- Forces GML to use a resolver, not a locator, so it can locate all its subsidiary schemas. This allows the XSDSchema for GML to be build before its subsidiaries, including their definitions, and resolving all cycles.
- Makes the six subsidiary XSD implementations delegate to GML's XSDSchema, which contains all the definitions. This solution allows the GeoTools one-XSD-per-namespace contracts to be preserved, while resolving cyclcic definitions.
- Fixes the relative import paths in the schemas to allow direct resolution, and adds a private copy of xlinks.xsd (as we cannot use a locator in this case).
- Adds an include for deprecatedTypes.xsd to make one schema valid in its own right. (I spoke to Simon C about this, and he will make sure valid-in-own-right is OGC policy for future GML schema releases.)
- Adds two unit tests to show that both pure EMF and GeoTools SchemaIndexImpl handle previously-broken types that used cyclic dependencies.

GEOT-3327.app-schema.patch adds the test schema artifacts, adds app-schema support for GML 3.2 (with GML version autodetection) and a unit test based on GeoSciML 3.0rc1 (a GML 3.2 application schema). You do not have to review this patch.
Show
Ben Caradoc-Davies added a comment - Justin, please review GEOT-3327 .xsd-gml3.patch, which adds support for cyclic dependencies in GML 3.2. This only works because the EMF upgrade GEOT-3466 is complete, fixing EMF cyclic import behaviour. The patch: - Forces GML to use a resolver, not a locator, so it can locate all its subsidiary schemas. This allows the XSDSchema for GML to be build before its subsidiaries, including their definitions, and resolving all cycles. - Makes the six subsidiary XSD implementations delegate to GML's XSDSchema, which contains all the definitions. This solution allows the GeoTools one-XSD-per-namespace contracts to be preserved, while resolving cyclcic definitions. - Fixes the relative import paths in the schemas to allow direct resolution, and adds a private copy of xlinks.xsd (as we cannot use a locator in this case). - Adds an include for deprecatedTypes.xsd to make one schema valid in its own right. (I spoke to Simon C about this, and he will make sure valid-in-own-right is OGC policy for future GML schema releases.) - Adds two unit tests to show that both pure EMF and GeoTools SchemaIndexImpl handle previously-broken types that used cyclic dependencies. GEOT-3327 .app-schema.patch adds the test schema artifacts, adds app-schema support for GML 3.2 (with GML version autodetection) and a unit test based on GeoSciML 3.0rc1 (a GML 3.2 application schema). You do not have to review this patch.
Hide
Permalink
Ben Caradoc-Davies added a comment -
By the way, the thing that broke the WFS 2.0 FeatureCollectionEncodingTest was my original GML resolver implementation, which used URI.resolve for relative includes/imports. This worked fine in Eclipse, when the path to the context schema is a file: URI, but in maven while building gt-xsd-wfs the path is a jar:file: URI, which is opaque, and URI.resolve does not resolve against opaque URI contexts, so relative includes/imports fail. The solution was the the implementation in the attached patch, which uses a URL constructor to resolve relative includes/imports if the context URI is opaque.
Show
Ben Caradoc-Davies added a comment - By the way, the thing that broke the WFS 2.0 FeatureCollectionEncodingTest was my original GML resolver implementation, which used URI.resolve for relative includes/imports. This worked fine in Eclipse, when the path to the context schema is a file: URI, but in maven while building gt-xsd-wfs the path is a jar:file: URI, which is opaque, and URI.resolve does not resolve against opaque URI contexts, so relative includes/imports fail. The solution was the the implementation in the attached patch, which uses a URL constructor to resolve relative includes/imports if the context URI is opaque.
Hide
Permalink
Justin Deoliveira added a comment -
Bravo! Ben. Very nice work.

Question about the GML.DelegatingXSD class and having GCO, etc... other schemas extend it. So if I understand correctly those schemas simply return the GML schema as their own schema? Do you forsee any issues with them resolving to a schema in which the target namespace is not their own namespace? Regardless I don't think anything outside of app schema would be affected but just wanted to clarify for my own understanding.

Other then that it all looks great. A very nice cleanup. Question... since this depends on the EMF upgrade I assume this is trunk only? Definitely +1 for the trunk but I would have some reservations about the stable branch at the moment...
Show
Justin Deoliveira added a comment - Bravo! Ben. Very nice work. Question about the GML.DelegatingXSD class and having GCO, etc... other schemas extend it. So if I understand correctly those schemas simply return the GML schema as their own schema? Do you forsee any issues with them resolving to a schema in which the target namespace is not their own namespace? Regardless I don't think anything outside of app schema would be affected but just wanted to clarify for my own understanding. Other then that it all looks great. A very nice cleanup. Question... since this depends on the EMF upgrade I assume this is trunk only? Definitely +1 for the trunk but I would have some reservations about the stable branch at the moment...
Hide
Permalink
Ben Caradoc-Davies added a comment -
Thanks, Justin. This is for trunk at this time.

I have not been able to detect any problems with using the only XSDSchema for multiple namespaces. As far as I can tell, the implementation only expects it to provide type definitions for the namespace, which it does. I don't see any problems caused by the target namespace not matching that of the containing XSD.
Show
Ben Caradoc-Davies added a comment - Thanks, Justin. This is for trunk at this time. I have not been able to detect any problems with using the only XSDSchema for multiple namespaces. As far as I can tell, the implementation only expects it to provide type definitions for the namespace, which it does. I don't see any problems caused by the target namespace not matching that of the containing XSD.
Hide
Permalink
Ben Caradoc-Davies added a comment -
Committed on trunk in r36987.
Show
Ben Caradoc-Davies added a comment - Committed on trunk in r36987.

People

  • Assignee:
    Justin Deoliveira
    Reporter:
    Ben Caradoc-Davies
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: