InternalTransporterServices
OK, the new InternalTransporterServices class is checked in and we have a way to customize the MBeanServer, Detector and Registry that the transporter framework uses.
I think a redesign of the way the TransporterServer is created needs to be done.
I'm not crazy about the need for static create methods. It doesn't provide an easy way for someone to subclass the TransporterServer class (e.g. so I can provide my own Connector). I'd prefer a set of constructors that I can then use in my subclass.
Looking at the code, it seems the only reason why we need those static create methods is to support clustering - i.e. before you can "new" the TransporterServer, if clustering is enabled, you have to initialize the internal transporter services.
BTW: the static create methods also starts the connector - not sure if that should be. At the very least, it needs to be javadoc'ed that the fact that not only is the server created, but it is also "start()"ed. I prefer to only have constructors/create methods "create" things and let the caller determine when/if to start() the thing. But, this is minor.
Here's what I propose - this replaces the createTransporterServer static method with the same signature:
/** Constructor */
public TransporterServer(InvokerLocator locator, Object target, String subsys, boolean isClustered)
{
if (isClustered && (InternalTransporterServices.getInstance().getDetector() == null))
{
setupDetector(); // this is the same static method we have currently
}
// below is the code from the original TransporterServer constructor
connector = getConnector(locator);
ServerInvocationHandler handler = new TransporterHandler(target);
connector.addInvocationHandler(subsys.toUpperCase(), handler);
// the original static create method also start()ed the server
// I'm not sure that should be done here, but we can.
// start();
// I would at least provide a constructor parameter "startIt", if true
// we call start(), if false, we do not. I just want to give the opportunity
// for the caller to create a server but not start it. Currently, there is
// no way for you to do that. For those callers that want to do their
// own lifecycle management, they need the flexibility to be able to
// start the server when they want.
}
Now we just replace the other static create methods with analogous constructors:
public TransporterServer(String locatorURI, Object target, String subsys, boolean isClustered)
{
this(new InvokerLocator(locatorURI), target, subsystem, isClustered);
}
public TransporterServer(String locator, Object target, String subsys)
{
this(locator, target, subsystem, false);
}
public TransporterServer(String locatorURI, Object target, String subsys)
{
this(new InvokerLocator(locatorURI), target, subsystem, false);
}
Now we can remove all static create methods. In fact, now, the only static method or data member we have is the static setupDetector() method. This is OK, it can be static since it doesn't need any instance data from any TransporterServer object. But, I would even go so far as recommending that setupDetector is "protected" and not static. This would then let me override that in my subclass too - this would allow my subclass to override setupDetector and customize my detector right there.
I like this much better because it more extensible - I can easily subclass TransporterServer and override getConnector() and setupDetector() to customize it however I want.
Static stuff is just annoying to work with :-)
