XPath NamespaceSupport

Jody Garnett added a comment - 11/Jan/11 6:12 AM So one of the issues was Query not being able to see CommonFactory finder. I have prepared a patch to move CommonFactory finder to gt-api. I actually like the result there a lot better; leaving gt-main to be strictly implementation. There were two areas of difficulty in moving CommonFactoryFinder: - Some old references to FunctionImpl in CommonFactoryFinder - Explicit reference to LienientFeatureFactoryImpl

Jody Garnett added a comment - 11/Jan/11 5:15 PM Patch reviewed (thanks justin) and now applied to trunk. Ball back in your court Niels.

Niels Charlier added a comment - 12/Jan/11 3:58 AM Okay, here is the patch. Some remarks: - In Query, the old methods are called get-/setPropertyNames (with String[]), the new ones are called get-/setProperties (with List<PropertyName>). There were special constants defined to use for the old methods called NO_NAMES and ALL_NAMES, I added new constants called NO_PROPERTIES and ALL_PROPERTIES to fulfill the same role for the new methods. - Previously there was a hint key for NamespaceSupport defined in the XpathPropertyAccessorFactory and another hint key for the same purpose in FeaturePropertyAccessorFactory. To avoid confusion and error (there should be one and only hint key as they will not be recognized as equal!), I removed both and created a new one in the PropertyAccessorFactory interface (the advantage being that any existing references to the old ones will still work). - The FilterFactoryImplNamespaceAware in app-schema has not been removed, but updated to be built on top of the existing namespace support in FF. The reason being that it works with one single NamespaceSupport object for the whole filter factory and this ff is passed on to other classes and it would take some work to change that. This can be optionally done within app-schema at a later stage, but it's not really necessary. - I needed to make a small change to a class in the 'feature-pregeneralized' package to adapt to the change in the PropertyName interface.e

Jody Garnett added a comment - 12/Jan/11 7:35 AM reviewed patch; added javadocs and a few null checks in the implementation of Query; seems good

Niels Charlier added a comment - 12/Jan/11 8:58 PM Jody, your changes look good, but just one comment. You did this: in FilterFactoryImpl:      public PropertyName property( Name name ) { - return property( name.toString() ); // uses full URI for justin + return new AttributeExpressionImpl( name ); + } in AttributeExpressionImpl: + public AttributeExpressionImpl( Name name ){ + this( name.getLocalPart() ); // uses full URI for justin + } Note that you are using only the Local Part rather than the full string with namespace included. I thought we trying to move away from throwing namespaces in the bin... However, this reminds me of another issue I haven't been able to address yet. Attribute Expressions of the form "full_namespace:property_name" currently cannot be evaluated by the feature property accessors because X-path doesn't accept full namespaces. Perhaps to solve this issue we can use the NamespaceSupport object and the concept of the default namespace? Something like this: public AttributeExpressionImpl( Name name ){         attPath = name.getLocalPart();         schema = null;         this.namespaceSupport = new NamespaceSupport();         namespaceSupport.addPrefix("", name.getNamespaceURI());         this.expressionType = ATTRIBUTE;   }

Niels Charlier added a comment - 13/Jan/11 1:26 AM - edited Found this little error in your patch. You wrote this: + if( namespaceSupport != null && hints != null ){ + hints = new Hints(PropertyAccessorFactory.NAMESPACE_CONTEXT, namespaceSupport); + } While I'm guessing it has to be + if( namespaceSupport != null && hints == null ){ + hints = new Hints(PropertyAccessorFactory.NAMESPACE_CONTEXT, namespaceSupport); + } Will upload new patch soon

Niels Charlier added a comment - 13/Jan/11 2:21 AM new patch

Niels Charlier added a comment - 13/Jan/11 9:52 PM Committed, revision 36470 The commit included, besides what is in the patch, a reworked version of DefaultQueryTest called QueryTest, which tests the new functionality of Query.

Andrea Aime added a comment - 06/Mar/11 11:53 AM Mass closing all issues in resolved state that have not been reopened nor commented over in the last month