Hello Taher, Just few nitpicks.
ta...@apache.org writes: > + protected XmlRpcRequest getRequest(final XmlRpcStreamRequestConfig > pConfig, InputStream pStream) > + throws XmlRpcException { > + final XmlRpcRequestParser parser = new XmlRpcRequestParser(pConfig, > getTypeFactory()); > + final XMLReader xr = SAXParsers.newXMLReader(); > + xr.setContentHandler(parser); > + try { > + > xr.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); > + > xr.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", > false); > + > xr.setFeature("http://xml.org/sax/features/external-general-entities", false); > + > xr.setFeature("http://xml.org/sax/features/external-parameter-entities", > false); > + xr.parse(new InputSource(pStream)); > + } catch (SAXException | IOException e) { > + throw new XmlRpcException("Failed to parse / read XML-RPC > request: " + e.getMessage(), e); > + } > + final List<?> params = parser.getParams(); > + return new XmlRpcRequest() { > + public XmlRpcRequestConfig getConfig() { > + return pConfig; > + } > + public String getMethodName() { > + return parser.getMethodName(); > + } > + public int getParameterCount() { > + return params == null ? 0 : params.size(); > + } > + public Object getParameter(int pIndex) { > + return params.get(pIndex); > + } > + }; > + } > + I would recommend against using explicit ‘final’ outside of class fields where it conveys the semantic of immutable objects which is meaningful since object are inherently stateful. In the case of method parameter and local variables I think being explicit about the immutability of the reference is a bit noisy IMHO. I like the citation of Remi Forax [1] where in one of his french version of “Design Pattern Reloaded” presentation [2] said “I want to read code, not to read ‘final’”. :-) besides that, I have found ‘XmlRpcClientRequestImpl’ which is a default implementation of ‘XmlRpcRequest’ which can be used to avoid the anonymous class. Here is a cosmetic *untested* patch applying those two recommendations.
diff --git framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/XmlRpcEventHandler.java framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/XmlRpcEventHandler.java index fa1ae4f9bc..bf08208881 100644 --- framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/XmlRpcEventHandler.java +++ framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/XmlRpcEventHandler.java @@ -29,7 +29,6 @@ import java.io.InputStreamReader; import java.io.OutputStream; import java.io.Writer; import java.util.HashMap; -import java.util.List; import java.util.Locale; import java.util.Map; @@ -53,7 +52,7 @@ import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap; import org.apache.xmlrpc.XmlRpcException; import org.apache.xmlrpc.XmlRpcHandler; import org.apache.xmlrpc.XmlRpcRequest; -import org.apache.xmlrpc.XmlRpcRequestConfig; +import org.apache.xmlrpc.client.XmlRpcClientRequestImpl; import org.apache.xmlrpc.common.ServerStreamConnection; import org.apache.xmlrpc.common.XmlRpcHttpRequestConfig; import org.apache.xmlrpc.common.XmlRpcHttpRequestConfigImpl; @@ -272,10 +271,10 @@ public class XmlRpcEventHandler extends XmlRpcHttpServer implements EventHandler } } - protected XmlRpcRequest getRequest(final XmlRpcStreamRequestConfig pConfig, InputStream pStream) + protected XmlRpcRequest getRequest(XmlRpcStreamRequestConfig pConfig, InputStream pStream) throws XmlRpcException { - final XmlRpcRequestParser parser = new XmlRpcRequestParser(pConfig, getTypeFactory()); - final XMLReader xr = SAXParsers.newXMLReader(); + XmlRpcRequestParser parser = new XmlRpcRequestParser(pConfig, getTypeFactory()); + XMLReader xr = SAXParsers.newXMLReader(); xr.setContentHandler(parser); try { xr.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); @@ -286,21 +285,7 @@ public class XmlRpcEventHandler extends XmlRpcHttpServer implements EventHandler } catch (SAXException | IOException e) { throw new XmlRpcException("Failed to parse / read XML-RPC request: " + e.getMessage(), e); } - final List<?> params = parser.getParams(); - return new XmlRpcRequest() { - public XmlRpcRequestConfig getConfig() { - return pConfig; - } - public String getMethodName() { - return parser.getMethodName(); - } - public int getParameterCount() { - return params == null ? 0 : params.size(); - } - public Object getParameter(int pIndex) { - return params.get(pIndex); - } - }; + return new XmlRpcClientRequestImpl(pConfig, parser.getMethodName(), parser.getParams()); } class ServiceRpcHandler extends AbstractReflectiveHandlerMapping implements XmlRpcHandler {
Feel free to do whatever you want with it. I am not sure to understand what is the purpose of the ‘getRequest’ method so it would be nice if you could provide a docstring for it. Thanks. [1] https://forax.github.io/ [2] https://www.youtube.com/watch?v=aC1wGHDOQic -- Mathieu Lirzin GPG: F2A3 8D7E EB2B 6640 5761 070D 0ADE E100 9460 4D37