GeoTools

Bounding box query when specifying property name fails for complex features

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.6.1
  • Fix Version/s: 2.6.2
  • Component/s: main
  • Labels:
    None
  • Testcase included:
    yes

Description

In complex features, namespaces are used for the property names, eg:

wfs?request=GetFeature&typeName=gsml:MappedFeature&propertyName=gsml:shape&bbox=132,-27,133,-26

This fails, because in FilterFactoryImpl, it resets the property name and loses the namespace information.
I removed line 311 in this method, since it did the damage, and it's redundant.

public BBOX bbox(Expression e, double minx, double miny, double maxx, double maxy, String srs) {

PropertyName name = null;
if ( e instanceof PropertyName )

{ name = (PropertyName) e; }

else

{ throw new IllegalArgumentException(); }

BBoxExpression bbox = null;
try

{ bbox = createBBoxExpression(new Envelope(minx,maxx,miny,maxy)); }


catch (IllegalFilterException ife)

{ new IllegalArgumentException().initCause(ife); }

BBOXImpl box = new BBOXImpl(this,e,bbox);

>>> removed >>> box.setPropertyName( name.getPropertyName() );

By passing name.getPropertyName() as String, it loses the namespace hints from the PropertyName name.
It also doesn't make sense to set property name (expression1) again because it's already passed in through BBoxImpl constructor as e. If you look higher, name = e.

My test in gt-app-schema now passes (not yet committed obviously).

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Rini Angreani added a comment -
Test committed in src\Geotools\modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java

Just add @test in testBBoxWithPropertyName() to activate it.
This fix is essential to allow bbox queries for features with >1 geometries.
Show
Rini Angreani added a comment - Test committed in src\Geotools\modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java Just add @test in testBBoxWithPropertyName() to activate it. This fix is essential to allow bbox queries for features with >1 geometries.
Hide
Permalink
Rini Angreani added a comment - - edited
Related conversation:
http://www.mail-archive.com/geotools-devel@lists.sourceforge.net/msg20242.html

Hi list,

I submitted a patch for http://jira.codehaus.org/browse/GEOT-2827 more than a
week ago, and it's still unassigned.
I'm not sure who the module maintainer is for that module (core filter).
It's a one line change, but quite important for bounding box queries in complex
features.
If you want to run bounding box query with a specific property name (in the
case where there are more than one geometries per feature), it'd fail because
it accidentally (and unnecessarily) removes the namespace for the property
names.
Can anyone advise?

Thank's

Rini
================================================================================================
Tue, 24 Nov 2009 07:37:14 -0800

Hi Rini,

I think Jody is the unofficial maintainer of the filter stuff but he is
traveling at teh moment so I will give it my 2c.

All in all I think the patch is ok. It does however change some behavior
. Basically if code was doing this before:

ff.bbox(null, 1, 2 , 3 ,4, "name")

It will now not have the desired affect unless i am missing something. I
  would say instead of removing the line guard it with a check:

if (e == null) {
   bbox.setPropertyName(name.getPropertyName());
}

2c.

-Justin


Show
Rini Angreani added a comment - - edited Related conversation: http://www.mail-archive.com/geotools-devel@lists.sourceforge.net/msg20242.html Hi list, I submitted a patch for http://jira.codehaus.org/browse/GEOT-2827 more than a week ago, and it's still unassigned. I'm not sure who the module maintainer is for that module (core filter). It's a one line change, but quite important for bounding box queries in complex features. If you want to run bounding box query with a specific property name (in the case where there are more than one geometries per feature), it'd fail because it accidentally (and unnecessarily) removes the namespace for the property names. Can anyone advise? Thank's Rini ================================================================================================ Tue, 24 Nov 2009 07:37:14 -0800 Hi Rini, I think Jody is the unofficial maintainer of the filter stuff but he is traveling at teh moment so I will give it my 2c. All in all I think the patch is ok. It does however change some behavior . Basically if code was doing this before: ff.bbox(null, 1, 2 , 3 ,4, "name") It will now not have the desired affect unless i am missing something. I   would say instead of removing the line guard it with a check: if (e == null) {    bbox.setPropertyName(name.getPropertyName()); } 2c. -Justin
Hide
Permalink
Rini Angreani added a comment -
Committed to GT-trunk:
======================
Modified: modules\library\main\src\main\java\org\geotools\filter\FilterFactoryImpl.java
Modified: modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java
Completed: At revision: 34796

Committed to GT-2.6.x:
======================
Modified: modules\library\main\src\main\java\org\geotools\filter\FilterFactoryImpl.java
Modified: modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java
Completed: At revision: 34797
Show
Rini Angreani added a comment - Committed to GT-trunk: ====================== Modified: modules\library\main\src\main\java\org\geotools\filter\FilterFactoryImpl.java Modified: modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java Completed: At revision: 34796 Committed to GT-2.6.x: ====================== Modified: modules\library\main\src\main\java\org\geotools\filter\FilterFactoryImpl.java Modified: modules\unsupported\app-schema\app-schema\src\test\java\org\geotools\filter\BBoxTest.java Completed: At revision: 34797
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:
    Rini Angreani
    Reporter:
    Rini Angreani
Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: