Re: Pluggable WSIFServiceFactories
Paul Russell <[email protected]>
| Newsgroups | gmane.comp.apache.webservices.wsif.devel |
|---|---|
| Message-ID | <[email protected]> |
Having taken this to work and tested it, I hit a couple of problems. That'll teach me to preempt myself! Firstly, because when I did this first time around, I built the .jar from inside WebSphere Studio, the wsif.properties file was missing. This meant that despite the WSIFProperties implementation only loading the first properties file it finds, the initial implementation worked. Because I used the .jar I built at home today, the wsif.properties file in my application wasn't loaded (wsif.jar is in my application server's 'lib' directory, and therefore the default properties are higher in the ClassLoader tree than the one in my application). I've made two changes as a result of this: * I've modified the included wsif.properties to remove the wsif.servicefactory property, since it points to a factory which isn't in the core distribution. * I've now made a change to WSIFProperties so that it merges the changes from all the wsif.properties files it finds in the classpath. For example, if you have a context ClassLoader with a wsif.properties file, any properties in this file which have the same key as one in the default file will replace the default value. Can anyone think of any security implications of this? I'm aware that this doesn't quite square with the Java2 ClassLoading pattern. At the moment, the WSIFProperties implementation only has a single cache of properties values, which means that there is likely to be a problem if WSIF is used in multiple applications in the same server. I'm going to make an additional change to make WSIFProperties cache values per context ClassLoader, which should sort this out, and keep performance high. I'm aware that these changes are rather more substantial than the one I originally proposed, so I totally understand if you'd rather stay away from this patch. Let me know what you think. Paul On 27 Jan 2004, at 23:26, Paul Russell wrote: > Nirmal, > > Thanks for your reply. Attached is a patch for this. I'm quite new to > WSIF, so I'm not clear how you deal with patches (I know that other > projects use Jira or similar). I couldn't find anywhere other than > this mailing lists to put this patch, so I've attached it here; > apologies if this isn't the right forum! > > <WSIFServiceFactorySupport.diff> > > Couple of things to bear in mind about this: > * I've made minor modifications (mainly around logging etc) since I > made the initial change. I'm working on this from home (can't get to > CVS to do a diff at work), and the only place I can test this properly > is at work. I'll make sure this happens tomorrow, and will e-mail to > confirm this passed okay. Probably best not to commit the change until > this is complete. > * I tried to run the automated unit tests, but they failed to compile. > Is this expected, or have I broken something? > * I noticed while I was making this change that the WSIFProperties > class caches the Properties instance once it has been built. While I > can totally see why this a good thing from a performance point of > view, I can see this causing problems if you try and use WSIF in an > environment where there are multiple context class loaders, all of > which could potentially want to call WSIF. I think this may apply to > the work I'm doing, although I'm not far enough down the line to be > sure yet. I'll let you know if this turns out to be a problem. If it > does turn out to be a problem, I can't see any reason why we can't > cache the properties instance in a Map keyed against the context > classloader instance. > > Thanks, let me know if the patch is any use or not... > > > Paul > > On 27 Jan 2004, at 15:23, Nirmal Mukhi wrote: > >> >> Hi, >> >> Introduction of custom service factories was taken out for efficiency >> reasons at one point, but I support it as being a good thing, since I >> think the use far outweighs the reduction in performance. Why don't >> you submit the patch so the committers can vote on it? Hopefully it >> will pass that vote and if so I'll commit it to the codebase. >> >> Thanks, >> Nirmal. >> > -- > Paul Russell > [email protected] > > -- Paul Russell [email protected]
WSIFServiceFactorySupport.diff
(application/octet-stream, 9 KB)
Index: java/src/wsif.properties
===================================================================
RCS file: /home/cvspublic/ws-wsif/java/src/wsif.properties,v
retrieving revision 1.7
diff -u -r1.7 wsif.properties
--- java/src/wsif.properties 9 May 2003 12:54:13 -0000 1.7
+++ java/src/wsif.properties 28 Jan 2004 18:47:37 -0000
@@ -26,7 +26,9 @@
# maximum number of seconds to wait for a response to an async request.
# if not defined on invalid defaults to no timeout
wsif.asyncrequest.timeout=60
-wsif.servicefactory=customfactory.client.CustomServiceFactoryImpl
+
+# custom WSIFServiceFactory implementation.
+# wsif.servicefactory=customfactory.client.CustomServiceFactoryImpl
# whether WSIF supports unreferenced attachments. Either on or off. Default is off.
wsif.unreferencedattachments=off
Index: java/src/org/apache/wsif/WSIFServiceFactory.java
===================================================================
RCS file: /home/cvspublic/ws-wsif/java/src/org/apache/wsif/WSIFServiceFactory.java,v
retrieving revision 1.12
diff -u -r1.12 WSIFServiceFactory.java
--- java/src/org/apache/wsif/WSIFServiceFactory.java 6 Mar 2003 15:54:40 -0000 1.12
+++ java/src/org/apache/wsif/WSIFServiceFactory.java 28 Jan 2004 18:47:38 -0000
@@ -63,7 +63,9 @@
import javax.wsdl.Service;
import org.apache.wsif.base.WSIFServiceFactoryImpl;
+import org.apache.wsif.logging.MessageLogger;
import org.apache.wsif.logging.Trc;
+import org.apache.wsif.util.WSIFProperties;
import org.apache.wsif.util.WSIFUtils;
/**
@@ -75,6 +77,8 @@
*/
public abstract class WSIFServiceFactory {
+ private static final String SERVICE_FACTORY_PROPERTY = "wsif.servicefactory";
+
/**
* Creates a new instance of an implementation the abstract
* WSIFServiceFactory class.
@@ -82,7 +86,28 @@
public static WSIFServiceFactory newInstance() {
Trc.entry(null);
- WSIFServiceFactoryImpl wsf = new WSIFServiceFactoryImpl();
+ WSIFServiceFactory wsf;
+
+ String serviceFactoryName = WSIFProperties.getProperty(SERVICE_FACTORY_PROPERTY);
+ if ( serviceFactoryName == null ) {
+ wsf = new WSIFServiceFactoryImpl();
+ } else {
+ try {
+ wsf = (WSIFServiceFactory)Thread.currentThread().getContextClassLoader().loadClass(serviceFactoryName).newInstance();
+ } catch (InstantiationException e) {
+ Trc.exception(e);
+ MessageLogger.log("WSIF.0012W",new String[] {serviceFactoryName,e.getClass().getName(),e.getMessage()});
+ wsf = new WSIFServiceFactoryImpl();
+ } catch (IllegalAccessException e) {
+ Trc.exception(e);
+ MessageLogger.log("WSIF.0012W",new String[] {serviceFactoryName,e.getClass().getName(),e.getMessage()});
+ wsf = new WSIFServiceFactoryImpl();
+ } catch (ClassNotFoundException e) {
+ Trc.exception(e);
+ MessageLogger.log("WSIF.0012W",new String[] {serviceFactoryName,e.getClass().getName(),e.getMessage()});
+ wsf = new WSIFServiceFactoryImpl();
+ }
+ }
// Create the simple types map for use by other WSIF classes
WSIFUtils.createSimpleTypesMap();
Index: java/src/org/apache/wsif/catalog/Messages.properties
===================================================================
RCS file: /home/cvspublic/ws-wsif/java/src/org/apache/wsif/catalog/Messages.properties,v
retrieving revision 1.7
diff -u -r1.7 Messages.properties
--- java/src/org/apache/wsif/catalog/Messages.properties 7 Dec 2002 12:33:57 -0000 1.7
+++ java/src/org/apache/wsif/catalog/Messages.properties 28 Jan 2004 18:47:38 -0000
@@ -63,3 +63,29 @@
# Parameter(s): {0} the user's preferred port
# Description: The preferred port set by the user on org.apache.wsif.WSIFService is not available
# User Action: None unless this message appears for long periods of time in which case the user might want to pick a different port as their preferred port
+
+WSIF.0012W=WSIF0012W: Could not create instance of custom WSIFServiceFactory ''{0}'', using default instead. This was due to a ''{1}'': ''{2}''
+# Parameter(s): {0} the class name of the custom factory.
+# {1} the class name of the exception causing the failure.
+# {2} the text of the message from the exception.
+# Description: Failed to instantiate the custom WSIFServiceFactory for some reason.
+# User Action: This could indicate the the class was not found, or not accessible. The user should check that
+# the class name is correct, that the class is available from the calling thread's context classloader
+# and that the class is publicly accessible. Additionally, the class must have a default (no argument)
+# constructor, which must also be publicly accessible.
+
+WSIF.0013W=WSIF0013W: Could not load wsif.properties file ''{0}'', continuing to load from other files. This was due to a ''{1}'': {2}
+# Parameter(s): {0} the URL of the properties file that failed to load.
+# {1} the class name of the exception caught.
+# {2} the text of the message from the exception.
+# Description: Failed to load a wsif.properties file.
+# User Action: This indicates that although a classloader reported the existance of a properties file, it could not
+# be loaded into the properties instance. This might be caused be a permissions problem, or some
+# kind of low level filesystem problem.
+
+WSIF.0014W=WSIF0014W: Could not load wsif.properties files from the context classloader, continuing with no properties loaded. This was due to a ''{0}'': ''{1}''
+# Parameter(s): {0} the class name of the exception caught.
+# {1} the text of the message from the exception.
+# Description: Failed to load the wsif.properties files.
+# User Action: This indicates that the classloader had IO problems while attempting to find a list of wsif.properties files.
+# This probably indicates that one of the entries in the classpath is corrupt in some manner.
Index: java/src/org/apache/wsif/util/WSIFProperties.java
===================================================================
RCS file: /home/cvspublic/ws-wsif/java/src/org/apache/wsif/util/WSIFProperties.java,v
retrieving revision 1.6
diff -u -r1.6 WSIFProperties.java
--- java/src/org/apache/wsif/util/WSIFProperties.java 12 Mar 2003 11:42:25 -0000 1.6
+++ java/src/org/apache/wsif/util/WSIFProperties.java 28 Jan 2004 18:47:40 -0000
@@ -57,12 +57,15 @@
package org.apache.wsif.util;
-import java.io.InputStream;
+import java.io.IOException;
+import java.net.URL;
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.Enumeration;
import java.util.Properties;
import org.apache.wsif.WSIFConstants;
+import org.apache.wsif.logging.MessageLogger;
import org.apache.wsif.logging.Trc;
/**
@@ -88,17 +91,18 @@
properties =
(Properties) AccessController.doPrivileged(new PrivilegedAction() {
public Object run() {
- InputStream in = (Thread.currentThread().getContextClassLoader())
- .getResourceAsStream(WSIFConstants.WSIF_PROPERTIES);
-
- Properties p2 = new Properties();
+ ClassLoader lContextCL = Thread.currentThread().getContextClassLoader();
+ Enumeration lPropertyFileURLs;
try {
- p2.load(in);
- } catch (Exception ignored) {
- Trc.exception(ignored);
+ lPropertyFileURLs =
+ lContextCL.getResources(WSIFConstants.WSIF_PROPERTIES);
+ } catch (IOException e) {
+ Trc.exception(e);
+ MessageLogger.log("WSIF.0014W", e.getClass().getName(),e.getMessage());
return null;
}
- return p2;
+
+ return buildPropertiesFromURLList(lPropertyFileURLs);
}
});
}
@@ -111,6 +115,23 @@
String s = properties.getProperty(property);
Trc.exit(s);
return s;
+ }
+
+ private static Properties buildPropertiesFromURLList(Enumeration pPropertyFileURLs) {
+ Trc.entry(null,pPropertyFileURLs);
+ Properties p2 = new Properties();
+ while (pPropertyFileURLs.hasMoreElements()) {
+ URL lResourceURL = (URL) pPropertyFileURLs.nextElement();
+ try {
+ Trc.event(null,"Loading wsif.properties from URL ",lResourceURL.toString());
+ p2.load(lResourceURL.openStream());
+ } catch (IOException e) {
+ Trc.exception(e);
+ MessageLogger.log("WSIF.0013W",new String[] {lResourceURL.toString(),e.getClass().getName(),e.getMessage()});
+ }
+ }
+ Trc.exit(p2);
+ return p2;
}
/**