Andreas,

On Fri, Mar 14, 2008 at 3:22 AM, Andreas Veithen (JIRA) <jira@apache.org> wrote:

   [ https://issues.apache.org/jira/browse/SYNAPSE-235?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578506#action_12578506 ]

Andreas Veithen commented on SYNAPSE-235:
-----------------------------------------

Some more comments and suggestions:

1) SynapseXPath extends AXIOMXPath. I would prefer composition instead of inheritance here. Indeed, given the way we extend the functionality of AXIOMXPath, there is no longer an "is a" relation between SynapseXPath and AXIOMPath. Not having SynapseXPath extending AXIOMXPath directly would give us more control over how SynapseXPath objects are used. For example this would have prevented the problem I pointed out in my previous comment. Another example: for the moment nothing prevents the code from calling setVariableContext on a SynapseXPath object, but this would lead to unexpected results.

I'm aware that this requires additional changes to SynapseXPathFactory and OMElementUtils, but I think that from a design point of view the effort is worth it.

Wont this limit the capabilities of the SynapseXPath? I think SynapseXPath "is a" AXIOMXPath rather than SynapseXPath "contains a" AXIOMXPath.

Any way in my initial code I did this using composition and thought that it is good to use inheritance here and also if you look at the current code, there if you provide the MessageContext to evaluate the xpath then the variables like $trp, $ctx will be available to the xpath. In the case of SOAPEnvelope being evaluated by the xpath, the above variables does not be effective (because there is no meaning for them when evaluated with envelope) and the $body and $header variables are still effective and if you provide any other object to the evaluate, then non of the variables are effective because there is no concept of header and body in arbitrary XMLNode? So I don't see any problem of using inheritance here....
 


2) We also need a ThreadSafeDelegatingFunctionContext to make SynapseXPath thread safe.

+1 I will add that.
 


3) SynapseVariableContext objects are stored in ThreadLocals and at the same time hold references to MessageContext and/or SOAPEnvelope objects. To avoid memory leaks, we need to release the reference to SynapseVariableContext after the evaluation. There should be a try-finally block with a call to ThreadSafeDelegatingFunctionContext #setDelegate(null). Idem for the function contexts.

+1, will do this too :)
 


4) There are hardcoded namespace prefix-URI mappings in SynapseXPath#evaluate and SynapseVariableContext#getVariableValue. I really don't like this because it is in contradiction with the fact that normally namespace prefixes can be chosen arbitrarily. I think we should only specify well defined namespace URIs and let the user define the prefix-URI mapping in the usual way. The config file would then look like this:

<definitions xmlns="http://ws.apache.org/ns/synapse" xmlns:t="http://ws.apache.org/ns/synapse/xpath-vars/transport-headers">
 ...
   <... expression="$t:content-type"/>
 ...
</definitions>

This is a bit more complicated, but it is the same approach as in XML Schema and especially XSLT. Also when reading a config file, somebody not familiar with our implicit XPath variables (but otherwise experienced with XML) would almost certainly try to find the namespace mapping to get an idea about where the variable comes for. If he sees something like "http://ws.apache.org/ns/synapse/xpath-vars/transport-headers" as the URI, this will become clear to him immediately. He can then get rest of the information from Google...

I agree with you, but I did this without a name space for the simplicity of the configuration, other wise these name spaces are going to hang around in nodes where there are xpaths (yes we can bring them to the top level definition element) If all others are OK with going for a name space aware variables that is you have to define the NS that you use for the variables I am OK with this modification.

Thanks,
Ruwan




> Allow XPath expressions to be specified relative to envelope or body via an attribute
> -------------------------------------------------------------------------------------
>
>                 Key: SYNAPSE-235
>                 URL: https://issues.apache.org/jira/browse/SYNAPSE-235
>             Project: Synapse
>          Issue Type: Improvement
>            Reporter: Asankha C. Perera
>            Assignee: Ruwan Linton
>             Fix For: 1.2
>
>
> This would make XPath expressions simpler without consideration for SOAP 1.1 or 1.2 or REST etc
> Default could be envelope (i.e. what we have now - for backward compatibility), and an optional attribute could specify if it should be relative to the body

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@synapse.apache.org
For additional commands, e-mail: dev-help@synapse.apache.org




--
Ruwan Linton
http://www.wso2.org - "Oxygenating the Web Services Platform"