On Tue, Oct 25, 2011 at 3:48 PM, Daniel Kulp <dk...@apache.org> wrote: > On Tuesday, October 25, 2011 3:41:29 PM Claus Ibsen wrote: >> On Tue, Oct 25, 2011 at 3:31 PM, Daniel Kulp <dk...@apache.org> wrote: >> > Is there a reason the default for this would be false? We claim to >> > support Java 6 as a requirement and the XSLT transform engine in the >> > JDK DOES support StAXSource. Thus, by default, it should work. If >> > the user replaces the XSLT engine with one that doesn't support StAX, >> > that's really on them. >> > >> > Alternatively, is there a way we can better detect if we cannot use >> > StAX? >> > >> > That said, the XSLT component needs some more work. On my TODO list >> > once I get CXF 2.5 out. It currently converts the body to an >> > InputStream which, in many cases, is sub-optimal. For example, if >> > using CXF with the new streaming stuff, the body is already a Source >> > object of some sort. Converting to an InputStream is a bad idea from >> > a performance perspective. >> >> Ah we should most likely check if the body is Source already without >> converting. Then use the body as is, without the input stream. > > > Was just about to suggest that. :-) Also check for a DOM Element if > that's already there, String, etc.... At least the "common" types to avoid > creating the InputStream. >
Okay I have committed an improvement so we only convert to InputStream if really needed. I backported to 2.8 as well. And gave it a test on my XP box as well, which was the OS that reported the "leaked" resource issues. In terms of the allowStAX option I think we could switch the default to true, because the 2.8.2 release kinda tipped the balance a bit by introducing the StAX converters. We can then add a note on the XSLT wiki page about they can disable StAX in case they get this "Not supported exception". > > Dan > > > > >> >> > Actually, I wonder if just: >> > exchange.getIn().getBody(Source.class) >> > would work better. Hmm..... something to try. >> > >> > Dan >> > >> > On Tuesday, October 25, 2011 1:06:49 PM davscl...@apache.org wrote: >> >> Author: davsclaus >> >> Date: Tue Oct 25 13:06:49 2011 >> >> New Revision: 1188642 >> >> >> >> URL: http://svn.apache.org/viewvc?rev=1188642&view=rev >> >> Log: >> >> CAMEL-4579: Added option allowStAX to XSLT component. >> >> >> >> Added: >> >> >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteAllowStAXTest.java - copied, changed from r1188579, >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteTest.java Modified: >> >> >> >> camel/trunk/camel-core/src/main/java/org/apache/camel/builder/xml/Xslt >> >> Build er.java >> >> >> >> Modified: >> >> camel/trunk/camel-core/src/main/java/org/apache/camel/builder/xml/Xslt >> >> Build er.java URL: >> >> http://svn.apache.org/viewvc/camel/trunk/camel-core/src/main/java/org/ >> >> apach >> >> e/camel/builder/xml/XsltBuilder.java?rev=1188642&r1=1188641&r2=118864 >> >> 2&view= diff >> >> ====================================================================== >> >> ===== === --- >> >> camel/trunk/camel-core/src/main/java/org/apache/camel/builder/xml/Xslt >> >> Build er.java (original) +++ >> >> camel/trunk/camel-core/src/main/java/org/apache/camel/builder/xml/Xslt >> >> Build er.java Tue Oct 25 13:06:49 2011 @@ -32,6 +32,8 @@ import >> >> javax.xml.transform.Transformer; >> >> import javax.xml.transform.TransformerConfigurationException; >> >> import javax.xml.transform.TransformerFactory; >> >> import javax.xml.transform.URIResolver; >> >> +import javax.xml.transform.dom.DOMSource; >> >> +import javax.xml.transform.sax.SAXSource; >> >> import javax.xml.transform.stax.StAXSource; >> >> import javax.xml.transform.stream.StreamSource; >> >> >> >> @@ -46,6 +48,8 @@ import org.apache.camel.support.Synchron >> >> import org.apache.camel.util.ExchangeHelper; >> >> import org.apache.camel.util.FileUtil; >> >> import org.apache.camel.util.IOHelper; >> >> +import org.slf4j.Logger; >> >> +import org.slf4j.LoggerFactory; >> >> >> >> import static org.apache.camel.util.ObjectHelper.notNull; >> >> >> >> @@ -59,6 +63,7 @@ import static org.apache.camel.util.Obje >> >> * @version >> >> */ >> >> public class XsltBuilder implements Processor { >> >> + private final static Logger LOG = >> >> LoggerFactory.getLogger(XsltBuilder.class); private Map<String, >> >> Object> >> >> parameters = new HashMap<String, Object>(); private XmlConverter >> >> converter = new XmlConverter(); >> >> private Templates template; >> >> @@ -67,6 +72,7 @@ public class XsltBuilder implements Proc >> >> private URIResolver uriResolver; >> >> private boolean deleteOutputFile; >> >> private ErrorListener errorListener = new XsltErrorListener(); >> >> + private boolean allowStAX; >> >> >> >> public XsltBuilder() { >> >> } >> >> @@ -104,7 +110,9 @@ public class XsltBuilder implements Proc >> >> try { >> >> is = exchange.getIn().getBody(InputStream.class); >> >> Source source = getSource(exchange, is); >> >> + LOG.trace("Using {} as source", source); >> >> transformer.transform(source, result); >> >> + LOG.trace("Transform complete with result {}", result); >> >> resultHandler.setBody(out); >> >> } finally { >> >> IOHelper.close(is); >> >> @@ -211,6 +219,16 @@ public class XsltBuilder implements Proc >> >> return this; >> >> } >> >> >> >> + /** >> >> + * Enables to allow using StAX. >> >> + * <p/> >> >> + * When enabled StAX is preferred as the first choice as {@link >> >> Source}. + */ >> >> + public XsltBuilder allowStAX() { >> >> + setAllowStAX(true); >> >> + return this; >> >> + } >> >> + >> >> // Properties >> >> // >> >> ---------------------------------------------------------------------- >> >> --- >> >> >> >> @@ -246,6 +264,14 @@ public class XsltBuilder implements Proc >> >> this.resultHandlerFactory = resultHandlerFactory; >> >> } >> >> >> >> + public boolean isAllowStAX() { >> >> + return allowStAX; >> >> + } >> >> + >> >> + public void setAllowStAX(boolean allowStAX) { >> >> + this.allowStAX = allowStAX; >> >> + } >> >> + >> >> /** >> >> * Sets the XSLT transformer from a Source >> >> * >> >> @@ -333,16 +359,32 @@ public class XsltBuilder implements Proc >> >> /** >> >> * Converts the inbound stream to a {@link Source}. >> >> * <p/> >> >> - * This implementation will prefer StAX first, and fallback to >> >> other kinds of Source types. + * This implementation will prefer >> >> to source in the following order: + * <ul> >> >> + * <li>StAX - Is StAX is allowed</li> >> >> + * <li>SAX - SAX as 2nd choice</li> >> >> + * <li>Stream - Stream as 3rd choice</li> >> >> + * <li>DOM - DOM as 4th choice</li> >> >> + * </ul> >> >> */ >> >> protected Source getSource(Exchange exchange, InputStream is) { >> >> - // try StAX first >> >> - Source source = >> >> exchange.getContext().getTypeConverter().convertTo(StAXSource.class, >> >> exchange, is); - if (source == null) { >> >> - // fallback and try other kind of source >> >> + Source source = null; >> >> + if (isAllowStAX()) { >> >> source = >> >> exchange.getContext().getTypeConverter().convertTo(StAXSource.class, >> >> exchange, is); } >> >> if (source == null) { >> >> + // then try SAX >> >> + source = >> >> exchange.getContext().getTypeConverter().convertTo(SAXSource.class, >> >> exchange, is); + } >> >> + if (source == null) { >> >> + // then try stream >> >> + source = >> >> exchange.getContext().getTypeConverter().convertTo(StreamSource.class, >> >> exchange, is); + } >> >> + if (source == null) { >> >> + // and fallback to DOM >> >> + source = >> >> exchange.getContext().getTypeConverter().convertTo(DOMSource.class, >> >> exchange, is); + } >> >> + if (source == null) { >> >> if (isFailOnNullBody()) { >> >> throw new ExpectedBodyTypeException(exchange, >> >> Source.class); } else { >> >> @@ -383,6 +425,7 @@ public class XsltBuilder implements Proc >> >> String key = entry.getKey(); >> >> Object value = entry.getValue(); >> >> if (value != null) { >> >> + LOG.trace("Transformer set parameter {} -> {}", key, >> >> value); transformer.setParameter(key, value); >> >> } >> >> } >> >> >> >> Copied: >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteAllowStAXTest.java (from r1188579, >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteTest.java) URL: >> >> http://svn.apache.org/viewvc/camel/trunk/camel-core/src/test/java/org/ >> >> apach >> >> e/camel/component/xslt/XsltRouteAllowStAXTest.java?p2=camel/trunk/cam >> >> el-core >> >> /src/test/java/org/apache/camel/component/xslt/XsltRouteAllowStAXTest >> >> .java&p >> >> 1=camel/trunk/camel-core/src/test/java/org/apache/camel/component/xsl >> >> t/XsltR outeTest.java&r1=1188579&r2=1188642&rev=1188642&view=diff >> >> ====================================================================== >> >> ===== === --- >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteTest.java (original) +++ >> >> camel/trunk/camel-core/src/test/java/org/apache/camel/component/xslt/X >> >> sltRo uteAllowStAXTest.java Tue Oct 25 13:06:49 2011 @@ -16,43 +16,12 >> >> @@ */ >> >> package org.apache.camel.component.xslt; >> >> >> >> -import java.util.List; >> >> - >> >> -import org.apache.camel.ContextTestSupport; >> >> -import org.apache.camel.Exchange; >> >> import org.apache.camel.builder.RouteBuilder; >> >> -import org.apache.camel.component.mock.MockEndpoint; >> >> -import org.apache.camel.impl.JndiRegistry; >> >> >> >> /** >> >> * >> >> */ >> >> -public class XsltRouteTest extends ContextTestSupport { >> >> - >> >> - public void testSendMessageAndHaveItTransformed() throws >> >> Exception { - MockEndpoint endpoint = >> >> getMockEndpoint("mock:result"); - >> >> endpoint.expectedMessageCount(1); >> >> - >> >> - template.sendBody("direct:start", >> >> - "<mail><subject>Hey</subject><body>Hello >> >> world!</body></mail>"); - >> >> - assertMockEndpointsSatisfied(); >> >> - >> >> - List<Exchange> list = endpoint.getReceivedExchanges(); >> >> - Exchange exchange = list.get(0); >> >> - String xml = exchange.getIn().getBody(String.class); >> >> - >> >> - assertNotNull("The transformed XML should not be null", xml); >> >> - assertTrue(xml.indexOf("transformed") > -1); >> >> - // the cheese tag is in the transform.xsl >> >> - assertTrue(xml.indexOf("cheese") > -1); >> >> - assertTrue(xml.indexOf("<subject>Hey</subject>") > -1); >> >> - assertTrue(xml.indexOf("<body>Hello world!</body>") > -1); >> >> - >> >> - TestBean bean = context.getRegistry().lookup("testBean", >> >> TestBean.class); - assertNotNull(bean); >> >> - assertEquals("bean.subject", "Hey", bean.getSubject()); >> >> - } >> >> +public class XsltRouteAllowStAXTest extends XsltRouteTest { >> >> >> >> @Override >> >> protected RouteBuilder createRouteBuilder() throws Exception { >> >> @@ -60,7 +29,7 @@ public class XsltRouteTest extends Conte >> >> @Override >> >> public void configure() throws Exception { >> >> from("direct:start") >> >> - >> >> .to("xslt:org/apache/camel/component/xslt/transform.xsl") + >> >> >> >> .to("xslt:org/apache/camel/component/xslt/transform.xsl?allowStAX=tr >> >> ue") .multicast() >> >> .beanRef("testBean") >> >> .to("mock:result"); >> >> @@ -68,11 +37,4 @@ public class XsltRouteTest extends Conte >> >> }; >> >> } >> >> >> >> - @Override >> >> - protected JndiRegistry createRegistry() throws Exception { >> >> - JndiRegistry jndi = super.createRegistry(); >> >> - jndi.bind("testBean", new TestBean()); >> >> - return jndi; >> >> - } >> >> - >> >> } >> > >> > -- >> > Daniel Kulp >> > dk...@apache.org >> > http://dankulp.com/blog >> > Talend - http://www.talend.com > -- > Daniel Kulp > dk...@apache.org > http://dankulp.com/blog > Talend - http://www.talend.com > -- Claus Ibsen ----------------- FuseSource Email: cib...@fusesource.com Web: http://fusesource.com Twitter: davsclaus, fusenews Blog: http://davsclaus.blogspot.com/ Author of Camel in Action: http://www.manning.com/ibsen/
