Sounds good; one clarification for performance considerations.
I am not sure the API will let you check the x-path against the feature or
feature type; the canHandle method is supposed to be fast; and should just
check that the xpath expression is valid.
You will need to check that the xpath points to a property when the expression
is evaluated; at that point you could throw an exception (or otherwise fail).
Jody
On 03/09/2010, at 3:11 PM, Niels wrote:
> Hi,
>
> Thanks for the advice.
>
> So my plan of attack will be:
>
> 1. Implement the 'canHandle' of class 'XPathPropertyAcessor' to make it
> return false when the x-path doesn't resolve to a property
> 2. Throw an exception in AttributeExpressionImpl.evaluate if no property
> accessor can be found.
> 3. Go through all the geotools test cases to be compatible with the changes.
>
> regards,
> Niels
>
> On 03/09/10 11:40, Jody Garnett wrote:
>>
>> I expect that JD (ie Justin's) comment was with respect to preserving
>> compatibility with the existing test cases when transitioning to the
>> GeoTools 2.3 Filter implementation (ie with Property Accessors).
>>
>> Now that the transition is mostly completed; throwing a good
>> exception is possible (but some work will be needed to update test
>> cases).
>>
>> This GeoTools 2.3 Filter transition is our oldest "technical debt") in
>> the library; there are a number of test cases that make use of the the
>> FilterImpl method directly (rather than using filter factory).
>>
>> Jody
>>
>> On Fri, Sep 3, 2010 at 1:36 PM, Jody Garnett <[email protected]> wrote:
>>
>>> I think you could throw an exception there; and update the geotools test
>>> cases.
>>> Jody
>>>
>>>
>>> On Thu, Sep 2, 2010 at 4:02 PM, Niels <[email protected]> wrote:
>>>
>>>> Hi,
>>>>
>>>> I am a new geotools developer who just started working on app-schema for
>>>> CSIRO last week.
>>>> I was asked to look at this problem:
>>>> http://jira.codehaus.org/browse/GEOT-3066 ("No error is returned if I gave
>>>> an invalid column name in app-schema mapping file").
>>>>
>>>> I discovered the issue is intertwined with other parts of the geotools
>>>> code.
>>>> The issue is in the evaluation of expressions, more specifically in the
>>>> AttributeExpressionImpl class. The evaluate method will return a null value
>>>> when it cannot evaluate the attribute expression. This makes it impossible
>>>> for App-schema to distinguish between an actual attribute with a real null
>>>> value and a non-existing attribute. I think an evaluation method should in
>>>> principle distinguish between an expression that cannot be evaluated and an
>>>> expression that evaluates to a null value. However, there are explicit
>>>> comments in the code that say this would cause issues with backward
>>>> compatibility:
>>>>
>>>> public Object evaluate(Object obj, Class target) {
>>>> PropertyAccessor accessor = getPropertyAccessor(obj, target);
>>>>
>>>> if ( accessor == null ) {
>>>> return null; //JD:not throwing exception to remain backwards
>>>> compatabile, just returnign null
>>>> }
>>>> Object value = accessor.get( obj, attPath, target );
>>>>
>>>> if(target == null)
>>>> return value;
>>>>
>>>> return Converters.convert( value, target );
>>>>
>>>> }
>>>> Although I am not sure why, because the FunctionExpressionImpl class does
>>>> throw exceptions when it cannot evaluate.
>>>> I considered bypassing the evaluate method altogether from app-schema for
>>>> the mapping, but this is not a serious option because expressions can be
>>>> quite complex, with functions etc. and all that functionality cannot be
>>>> lost.
>>>>
>>>> I think the most optimal solution that respects backward compatibility,
>>>> would be to introduce some flag somewhere that can turn error checking on
>>>> for the evaluation of attribute expressions (but is off by default). My
>>>> first idea to implement this is : creating an alternative implementation of
>>>> AttributeExpression, and then passing an optional boolean value to the CQL
>>>> compiler that specifies which attribute expression implementation to use.
>>>> Ben Caradoc-Davies however suggested the use of the 'Hints', but I am not
>>>> familiar yet with how this exactly works.
>>>>
>>>> What are your thoughts?
>>>>
>>>> One more thing, another issue that would need to be resolved is in the
>>>> XPathPropertyAccessor class in the xml package.
>>>> When evaluating an attribute expression, an iteration through all property
>>>> accessors determines which one to use.
>>>> When an invalid column name is specified, he will end up with
>>>> XPathPropertyAccessor as last choice. The problem is this code:
>>>> JXPathContext context =
>>>> JXPathContextFactory.newInstance().newContext(null, object);
>>>> context.setLenient(true);
>>>>
>>>> The Lenient property will make sure that no exception is thrown if the
>>>> xpath
>>>> cannot be resolved and a null is returned instead, again making it
>>>> impossible to distinguish between an error and a real null value. I don't
>>>> know why it was chosen to do it this way?
>>>> That too would need to change.
>>>>
>>>>
>>>> With Regards,
>>>> Niels
>>>>
>>>>
>>>> --
>>>> Niels Charlier
>>>>
>>>> Software Engineer
>>>> CSIRO Earth Science and Resource Engineering
>>>> Phone: +61 8 6436 8914
>>>>
>>>> Australian Resources Research Centre
>>>> 26 Dick Perry Avenue, Kensington WA 6151
>>>>
>>>> ------------------------------------------------------------------------------
>>>> This SF.net Dev2Dev email is sponsored by:
>>>>
>>>> Show off your parallel programming skills.
>>>> Enter the Intel(R) Threading Challenge 2010.
>>>> http://p.sf.net/sfu/intel-thread-sfd
>>>> _______________________________________________
>>>> Geotools-devel mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>
>>>>
>>>>
>>>
>
>
> --
> Niels Charlier
>
> Software Engineer
> CSIRO Earth Science and Resource Engineering
> Phone: +61 8 6436 8914
>
> Australian Resources Research Centre
> 26 Dick Perry Avenue, Kensington WA 6151
> ------------------------------------------------------------------------------
> This SF.net Dev2Dev email is sponsored by:
>
> Show off your parallel programming skills.
> Enter the Intel(R) Threading Challenge 2010.
> http://p.sf.net/sfu/intel-thread-sfd_______________________________________________
> Geotools-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/geotools-devel
------------------------------------------------------------------------------
This SF.net Dev2Dev email is sponsored by:
Show off your parallel programming skills.
Enter the Intel(R) Threading Challenge 2010.
http://p.sf.net/sfu/intel-thread-sfd
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
