GeoServer
  1. GeoServer
  2. GEOS-3553

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

        Hide
        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
        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
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: