GeoServer

app-schema DescribeFeatureType is schema invalid

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.0-RC1
  • Fix Version/s: 2.1.x
  • Component/s: WFS
  • Labels:
    None
  • Testcase included:
    yes
  • Number of attachments :
    1

Description

This was wrongly raised in Geotools JIRA: http://jira.codehaus.org/browse/GEOT-2678

The response to an app-schema DescribeFeatureType request is schema-invalid:

For example:
http://services.auscope.org/pirsa-earthresource/wfs?request=DescribeFeatureType&typename=er:Commodity
yields

<?xml version="1.0" encoding="UTF-8"?>
<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema"
xmlns:er="urn:cgi:xmlns:GGIC:EarthResource:1.1" xmlns:gml="http://www.opengis.net/gml"
elementFormDefault="qualified" targetNamespace="urn:cgi:xmlns:GGIC:EarthResource:1.1">
<xsd:import namespace="http://www.opengis.net/gml"
schemaLocation="http://services.auscope.org:80/pirsa-earthresource/schemas/gml/3.1.1/base/gml.xsd" />
<xsd:import namespace="urn:cgi:xmlns:GGIC:EarthResource:1.1"
schemaLocation="http://www.earthresourceml.org/earthresourceml/1.1/xsd/earthResource.xsd" />
</xsd:schema>

This is schema-invalid because the import namespace is the same as the targetNamespace. Should this be an include?

http://www.w3.org/TR/2004/REC-xmlschema-1-20041028/structures.html#element-import
"1.1 If the namespace [attribute] is present, then its - actual value- must not match the - actual value- of the enclosing <schema>'s targetNamespace [attribute]."

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Rini Angreani added a comment -

Hi Justin,

Please review my patch when you have time.

Changes in FeatureTypeSchemaBuilder:

1. If there's only 1 namespace in the describeFeatureType request, targetNamespace would be encoded.
If schemaLocation is specified in app-schema mapping file, then it would only encode xsd:include pointing to the schema location.
Gml.xsd wouldn't be imported as previously done, because it was only needed for manually built schema. Not only that it's redundant for this case, it also causes XMLSpy validation error (that the user doesn't think is acceptable) because if the schema location that's included already imports gml.xsd, it would complain that some elements are declared twice.

If there's no schemaLocation, the schema is manually built as original, and gml.xsd is imported.

2. If there are more than 1 namespace in the request (or if no type name is specified, and there are more than 1 namespaces available), target namespace wouldn't be encoded (as original).
If schema locations are specified in app-schema mapping file, it would encode xsd:import point to the schema location.
Otherwise the original describe feature type URL for all the type names would be encoded.

Test cases are updated in FeatureChainingWfsTest testDescribeFeatureType(). All other tests passed.

Thank's!

Rini

Show
Rini Angreani added a comment - Hi Justin, Please review my patch when you have time. Changes in FeatureTypeSchemaBuilder: 1. If there's only 1 namespace in the describeFeatureType request, targetNamespace would be encoded. If schemaLocation is specified in app-schema mapping file, then it would only encode xsd:include pointing to the schema location. Gml.xsd wouldn't be imported as previously done, because it was only needed for manually built schema. Not only that it's redundant for this case, it also causes XMLSpy validation error (that the user doesn't think is acceptable) because if the schema location that's included already imports gml.xsd, it would complain that some elements are declared twice. If there's no schemaLocation, the schema is manually built as original, and gml.xsd is imported. 2. If there are more than 1 namespace in the request (or if no type name is specified, and there are more than 1 namespaces available), target namespace wouldn't be encoded (as original). If schema locations are specified in app-schema mapping file, it would encode xsd:import point to the schema location. Otherwise the original describe feature type URL for all the type names would be encoded. Test cases are updated in FeatureChainingWfsTest testDescribeFeatureType(). All other tests passed. Thank's! Rini
Hide
Permalink
Justin Deoliveira added a comment -

Generally the patch looks ok although it is a bit hard to follow in some parts. I would however be weary of such a change so close to a release candidate, as it does change the a lot of the core describe feature type core. Running cite tests to ensure no breakage would help mitigate a bit of the risk though.

That said I am not really involved with release planning or short term road mapping, so getting the fix in now may be fine. I will let Andrea comment to that.

Show
Justin Deoliveira added a comment - Generally the patch looks ok although it is a bit hard to follow in some parts. I would however be weary of such a change so close to a release candidate, as it does change the a lot of the core describe feature type core. Running cite tests to ensure no breakage would help mitigate a bit of the risk though. That said I am not really involved with release planning or short term road mapping, so getting the fix in now may be fine. I will let Andrea comment to that.
Hide
Permalink
Andrea Aime added a comment -

Yeah, we shoudl be pushing out GS RC2 soon (next week the latest imho), but there is still no agreement so I don't know. If the patch is reviewed and does not break CITE I would ok for for applying it so that we can have a better working app-schema in 2.0 final

Show
Andrea Aime added a comment - Yeah, we shoudl be pushing out GS RC2 soon (next week the latest imho), but there is still no agreement so I don't know. If the patch is reviewed and does not break CITE I would ok for for applying it so that we can have a better working app-schema in 2.0 final
Hide
Permalink
Rini Angreani added a comment -

Thank's Justin and Andrea. The patch doesn't have to go into the release. I am trying to run CITE tests.

Show
Rini Angreani added a comment - Thank's Justin and Andrea. The patch doesn't have to go into the release. I am trying to run CITE tests.
Hide
Permalink
Rini Angreani added a comment -

Hi Justin,
Now that 2.0 has been released, can I commit my patch now and run CITE tests later? I've been trying to run CITE tests (with clean code) and it hasn't been successful. Or.. should I keep trying? I hit a dead end and clients have been pushing for this fix for a while. They're considering not using Geoserver (app-schema) because the output is invalid.

Thank's
Rini

Show
Rini Angreani added a comment - Hi Justin, Now that 2.0 has been released, can I commit my patch now and run CITE tests later? I've been trying to run CITE tests (with clean code) and it hasn't been successful. Or.. should I keep trying? I hit a dead end and clients have been pushing for this fix for a while. They're considering not using Geoserver (app-schema) because the output is invalid. Thank's Rini
Hide
Permalink
Justin Deoliveira added a comment -

Hi Rini,

Go ahead and commit. Thanks for your patience. Give me a shout when you commit and I will try to run cite tests over here to verify that nothing has broken..

Also, do you plan to commit on 2.0.x and trunk? Or just trunk?

Show
Justin Deoliveira added a comment - Hi Rini, Go ahead and commit. Thanks for your patience. Give me a shout when you commit and I will try to run cite tests over here to verify that nothing has broken.. Also, do you plan to commit on 2.0.x and trunk? Or just trunk?
Hide
Permalink
Rini Angreani added a comment -

Hi Justin,

Thank you so much for running the cite tests for me.
I committed my patch on both trunk and 2.0.
Let me know how the cite tests go...

Committed to trunk:
Modified: src\GeoServer-trunk\src\extension\app-schema\app-schema-test\src\test\java\org\geoserver\test\FeatureChainingWfsTest.java
Modified: src\GeoServer-trunk\src\wfs\src\main\java\org\geoserver\wfs\xml\FeatureTypeSchemaBuilder.java
Completed: At revision: 13785

Committed to 2.0:
Modified: src\Geoserver-2.0\src\extension\app-schema\app-schema-test\src\test\java\org\geoserver\test\FeatureChainingWfsTest.java
Modified: src\Geoserver-2.0\src\wfs\src\main\java\org\geoserver\wfs\xml\FeatureTypeSchemaBuilder.java
Completed: At revision: 13786

Show
Rini Angreani added a comment - Hi Justin, Thank you so much for running the cite tests for me. I committed my patch on both trunk and 2.0. Let me know how the cite tests go... Committed to trunk: Modified: src\GeoServer-trunk\src\extension\app-schema\app-schema-test\src\test\java\org\geoserver\test\FeatureChainingWfsTest.java Modified: src\GeoServer-trunk\src\wfs\src\main\java\org\geoserver\wfs\xml\FeatureTypeSchemaBuilder.java Completed: At revision: 13785 Committed to 2.0: Modified: src\Geoserver-2.0\src\extension\app-schema\app-schema-test\src\test\java\org\geoserver\test\FeatureChainingWfsTest.java Modified: src\Geoserver-2.0\src\wfs\src\main\java\org\geoserver\wfs\xml\FeatureTypeSchemaBuilder.java Completed: At revision: 13786
Hide
Permalink
Andrea Aime added a comment -

Mass closing all issues that have been in "resolved" state for 2 months or more without any feedback or update

Show
Andrea Aime added a comment - Mass closing all issues that have been in "resolved" state for 2 months or more without any feedback or update

People

  • Assignee:
    Justin Deoliveira
    Reporter:
    Rini Angreani
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: