GeoTools

Select XML attributes on the root element

Details

  • Type: Improvement Improvement
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: app-schema plugin
  • Labels:
    None
  • Environment:
  • Testcase included:
    yes

Description

The app-schema plugin doesn't allow the selection of attributes on the root element; i.e. the selection of client properties of the target element. For example the following x-path on a fictious schema will work:

ns:root_element/ns:nested_element/@ns:attribute

but the following x-path will not work:

ns:root_element/@ns:attribute

This x-path selection mechanism is not disallowed by the OGC Filter Encoding standard.

I have a use-case where a filter is required to be applied to the attribute of the root element; currently this is not supported by the app-schema plugin.

I have provided a patch which includes test cases that extends FeatureTypeMapping and XPath to allow my use-case to proceed. mvn clean install on trunk passes all tests.

In implementing XPath.rootElementSteps() I have tried to follow XPath.steps() as a template. FeatureTypeMapping.findMappingsFor() has been altered, as little as possible, to take advantage of XPath.rootElementSteps() where appropraite.

Does the app-schema documentation need to be amended? IMHO I think it doesn't. This is just a correction to a deficiency, overlooked by the original designers because they didn't have a specific use-case.

I'm happy to answer questions or provided additional details about my use-case.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Brett Walker added a comment -
I have supplied and improved patch. The new patch now deals with no namespace uri specified for the root element. A JUnit test has been written for this case. Please ignore the previous patch.
Show
Brett Walker added a comment - I have supplied and improved patch. The new patch now deals with no namespace uri specified for the root element. A JUnit test has been written for this case. Please ignore the previous patch.
Hide
Permalink
Rini Angreani added a comment -
Hi Brett,

Apologies for the late reply.
The patch looks good. Only a few minor things:

1. House keeping
- Commented out code should be removed in line 47 in your patch:
        } else { // if (propertyName.size() > 1)
- Please use GeoTools formatter and apply format to your changes (see http://docs.geotools.org/latest/developer/conventions/code/style.html#use-of-formatting-tools).

2. In rootElementSteps(), there's already an existing method to convert Name to QName, so there's no need to extract namespace prefix and create rootElementExpression.

QName qName = Types.toQName(rootElement.getName(), namespaces);

3. testPropertyNameWithGmlIdAttribute() could be improved by checking against the expression value (although I know you followed testPropertyNameWithXlinkAttribute() that doesn't do that).

Everything else looks good. Now, do you have commit access, or would you like me to commit the patch on your behalf?
Thanks again for your contribution.
Show
Rini Angreani added a comment - Hi Brett, Apologies for the late reply. The patch looks good. Only a few minor things: 1. House keeping - Commented out code should be removed in line 47 in your patch:         } else { // if (propertyName.size() > 1) - Please use GeoTools formatter and apply format to your changes (see http://docs.geotools.org/latest/developer/conventions/code/style.html#use-of-formatting-tools) . 2. In rootElementSteps(), there's already an existing method to convert Name to QName, so there's no need to extract namespace prefix and create rootElementExpression. QName qName = Types.toQName(rootElement.getName(), namespaces); 3. testPropertyNameWithGmlIdAttribute() could be improved by checking against the expression value (although I know you followed testPropertyNameWithXlinkAttribute() that doesn't do that). Everything else looks good. Now, do you have commit access, or would you like me to commit the patch on your behalf? Thanks again for your contribution.
Hide
Permalink
Brett Walker added a comment -
Hi Rini,

To answer you last question first; I don't have commit access. I want to submit a few more patches before requesting commit access.

If you could fix up the code according to the points you have mentioned that would be helpful. Point 1 and 2 seem almost trivial. Point 3 may involve more work.

If you are not able to find the time to do so then I am more than willing to help here. It is just that if I make a new patch, attach it to this case, get you to review it again and then committed to trunk, I think, would take more time than if you progressed this effort.yourself. Thoughts?

Thanks for your feedback,
Brett
Show
Brett Walker added a comment - Hi Rini, To answer you last question first; I don't have commit access. I want to submit a few more patches before requesting commit access. If you could fix up the code according to the points you have mentioned that would be helpful. Point 1 and 2 seem almost trivial. Point 3 may involve more work. If you are not able to find the time to do so then I am more than willing to help here. It is just that if I make a new patch, attach it to this case, get you to review it again and then committed to trunk, I think, would take more time than if you progressed this effort.yourself. Thoughts? Thanks for your feedback, Brett
Hide
Permalink
Rini Angreani added a comment -
No worries. I could fix it up myself and commit to trunk shortly. I'll put your name in the commit log message.
Show
Rini Angreani added a comment - No worries. I could fix it up myself and commit to trunk shortly. I'll put your name in the commit log message.
Hide
Permalink
Rini Angreani added a comment -
Committed to geotools trunk: Committed revision 38708.
Show
Rini Angreani added a comment - Committed to geotools trunk: Committed revision 38708.

People

  • Assignee:
    Rini Angreani
    Reporter:
    Brett Walker
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: