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 >> >> > ------------------------------------------------------------------------------ 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
