I started looking through this also -- it looks very nice. However, I
have an ulterior motive :-) If I understand the stub/skeleton
generation code correctly, it does not currently handle overloaded
methods properly, it just assumes the IDL operation name is the same as
the java method name. Do you have just sitting there waiting to submit
or are you planning to write soon code to translate java names to IDL
operation names taking account of overloaded methods?
Many thanks,
david jencks
On Jan 24, 2005, at 7:28 PM, Mark wrote:
Thanks for the review. I will go through your comments one-by-one
over the next few days.
A few things to keep in mind:
1. The rmi/iiop code is fairly solid (with my fingers crossed). I
hope to get in a bunch of test cases that following the Geronimo
design.
2. The stub/skeleton generation is farily *dirty* it. You can
convience it to work, code generation style/speed were not my primary
objective. The rmi-iiop was. So I fully anticipate this to change in
the near future.
3. The original code was stand alone, hence there are a few other
interop.* packages. Once I learn more about Geronimo, I anticipate a
much neater/tighter integration. Of course, I think I will have lots
of help from Alan. :-)
4. I don't mind making the code or style changes to conform tothe
Apache project. The current style is a required habit for other
projects that I work on. :-)
I've added a few quick answers below see MD>>
( I only made it 1/2 way through this email... )
Expect that I will be submitting lots more changes to Alan over the
comming days.
Thanks
Mark
Dain Sundstrom wrote:
Mark,
Wow! This is very impressive work. I spent some time reading over
this (well the first half... the thing is huge). Most of the
comments, I have are just my curiosity and don't really need to be
fixed. Other are notes on differences between this code an
geronimo, and I these cases I think we should change current
geronimo or this new code so we don't have multiple ways to do the
same thing (BTW, I generally don't have a preference on which code
we change unless one is broken :) Then there are the few coding
convention things, and finally there are a few nit picky things :)
Thanks for the great work,
-dain
On Jan 23, 2005, at 11:33 PM, [EMAIL PROTECTED] wrote:
Added: geronimo/trunk/modules/interop/src/idl/CosNaming.idl
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
idl/ CosNaming.idl?view=auto&rev=126264
Added: geronimo/trunk/modules/interop/src/idl/GIOP.idl
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
idl/ GIOP.idl?view=auto&rev=126264
Added: geronimo/trunk/modules/interop/src/idl/IIOP.idl
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
idl/ IIOP.idl?view=auto&rev=126264
Added: geronimo/trunk/modules/interop/src/idl/IOP.idl
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
idl/ IOP.idl?view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/idl/org-apache-geronimo-interop-
rmi-iiop.idl
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
idl/ org-apache-geronimo-interop-rmi-iiop.idl?view=auto&rev=126264
The idl files need Apache license headers... maybe in the comments
that are copied into the generated code.
MD>> If these are taken from the www.omg.org website, should we
include the apache headers?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
CheckedException.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/CheckedException.java?
view=auto&rev=126264
+/**
+ * * A wrapper that allows checked exceptions to be thrown as
unchecked.
+ */
+public class CheckedException extends RuntimeException {
+ public CheckedException(Throwable cause) {
+ super(cause);
+ }
+}
How is this used? Normally in Geronimo we have our invocation chains
(interceptor stacks) throw Throwable, and we just let exceptions
flow up. In other cases, we divide exceptions into System and
Application. We let the System exceptions flow up and the
Application exceptions are wrapped in the results object since they
are treated as a normal return for ejbs. Anyway, I'm just curious.
If it falls into one of the basic scenarios we already have a
preferred solution for, I'd like to either switch the exiting code
to follow the new pattern or switch the new iiop code to follow the
current pattern.
MD>> Its most likely that this class will go away soon.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
CosNaming/iiop_stubs/NamingContext_Stub.1.txt
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/
NamingContext_Stub.1.txt?view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
CosNaming/iiop_stubs/NamingContext_Stub.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/CosNaming/iiop_stubs/
NamingContext_Stub.java?view=auto&rev=126264
Are these generated stubs based on the IDL above? Normally, we don't
check in generated code, but I bet these almost never change.
MD>> The name service stubs/skels are generated. Right now, due to a
couple of issues, I have hand modified them and simply checked them
in. This will need to change. All other generated files go under
interop/target/src
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
InteropGBean.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/InteropGBean.java?view=auto&rev=126264
=====================================================================
== =======
--- (empty file)
+++
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
InteropGBean.java Sun Jan 23 23:33:10 2005
+/**
+ * A GBean that provides an example interop
+ *
+ * @version $Rev: $ $Date: $
+ */
+public class InteropGBean implements GBeanLifecycle {
Is this test code? If it is an example we want to ship with the
server, maybe we should put it in an example package or rename it
ExampleInteropGBean.
MD>> This GBean was the interop/corba container loader. maybe a
better name is in order. I haven't had a chance to flesh out the
details for the Geronimo integration. I'd like to see the interop
container be loaded by the InteropGBean. The InteropGBean could be a
seperate plan or as part of the j2ee-server plan. For the J2EE 1.4
configuration, the interop container would be enabled, but if a
customer didn't care about the interop container they could remove the
plan from the server configuration.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
SystemException.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/SystemException.java?
view=auto&rev=126264
Same comment as above for CheckedException.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
adapter/Adapter.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/adapter/Adapter.java?
view=auto&rev=126264
=====================================================================
== =======
+public class Adapter {
+ //
+ // Public Accessible Properties
+ //
I don't think we need comments like this... it is self explanatory.
MD>> Default template code .. Comments can be taken out. :-)
+ protected String _bindName;
+ protected String _remoteClassName;
+ protected String _remoteInterfaceName;
+ protected Vector _idVector;
+ protected boolean _shared;
+ protected ClassLoader _cl;
+ protected RemoteInterface _ri;
Our naming convention doesn't use lead underscores. Also we tend not
to use protected fields. Maybe there is a subclass or some reason.
MD>> I have noticed that. Will be changed.
+ //
+ // Internal Properrties
+ //
+
+ protected Object _sharedObject;
+ protected HashMap _objects;
+ protected Class _remoteClassClass;
+ protected Class _remoteInterfaceClass;
+
+ public Adapter() {
+ _objects = new HashMap();
+ _idVector = new Vector();
+ }
+
+ /*
+ * BindName is the name that will be registered with the INS
(Inter-operable Name Service)
+ */
+ public String getBindName() {
+ return _bindName;
+ }
+
+ public void setBindName(String bindName) {
+ _bindName = bindName;
+ }
+
+ /*
+ * Is this a shared component? If so this will invoke the
getInstance method on
+ * the component ...
+ */
+ public boolean isShared() {
+ return _shared;
+ }
+
+ public void setShared(boolean shared) {
+ _shared = shared;
+ }
+
+ /*
+ * The classloader that will load any dependancies of the
adapter or corba skel interfaces.
+ * Its should be set by the ejb container
+ */
+ public ClassLoader getClassLoader() {
+ return _cl;
+ }
+
+ public void setClassLoader(ClassLoader cl) {
+ _cl = cl;
+ }
Can we change these to be constructor injected and make the fields
final?
MD>> Its possible, I have to discuss this more with Alan .
+ /*
+ * This is the name of the remote class that implements the
remote interface.
+ *
+ * This is only used if this adapter is going to directly
invoke an object. For the
+ * EJB Container, the adapter will pass through the method
invocations.
+ */
+ public String getRemoteClassName() {
+ return _remoteClassName;
+ }
+
+ public void setRemoteClassName(String rcName) {
+ _remoteClassName = rcName;
+ }
+
+ /*
+ * The remote interface name for the remote object. This will
most likely be the name
+ * of the EJB's RemoteInterface and RemoteHomeInterface
+ *
+ * The stub/skel generator will use this interface name.
+ */
+ public String getRemoteInterfaceName() {
+ return _remoteInterfaceName;
+ }
+
+ public void setRemoteInterfaceName(String riName) {
+ _remoteInterfaceName = riName;
+ }
+
+ /*
+ * A list of public IDs that the remote object implements:
+ *
+ * IDL:....:1.0
+ * RMI:....:X:Y
+ */
+ public Vector getIds() {
+ return _idVector;
+ }
+
+ public void addId(String id) {
+ _idVector.add(id);
+ }
+
+ public void removeId(String id) {
+ _idVector.remove(id);
+ }
+
+ /*
+ * Return the skeleton implemention for the remote interface.
This interface has the
+ * invoke method to handle the rmi/iiop messages.
+ */
+ public RemoteInterface getRemoteInterface() {
+ if (_ri == null) {
+ synchronized (this) {
+ String riName = _remoteInterfaceName + "_Skeleton";
+ _remoteInterfaceClass = loadClass(riName);
+
+ try {
+ _ri = (RemoteInterface)
_remoteInterfaceClass.newInstance();
+ } catch (InstantiationException e) {
+ e.printStackTrace(); //To change body of catch
statement use File | Settings | File Templates.
+ } catch (IllegalAccessException e) {
+ e.printStackTrace(); //To change body of catch
statement use File | Settings | File Templates.
+ }
+ }
+ }
+
+ return _ri;
+ }
This is double check locking. The whole method needs to be
synchronized.
MD>> The double check code is wrong and I need to change it. Does
the method require sync? Maybe. I recently came accross a strategy
for multiprocessor locking that uses a transient int as a memory
barrier. Probably best if I don't attempt to explain it. We can
optimize our locking. The template that I came accross is as follows:
private volatile boolean _evaluated = false;
private Object _value;
/**
** Sub-classes should override this method to get the object's
value
** when it is first needed.
**/
public abstract Object evaluate();
public final Object getValue()
{
if (_volatileIsEffectiveMemoryBarrier)
{
if (! _evaluated)
{
synchronized (this)
{
if (! _evaluated)
{
_value = evaluate();
_evaluated = true;
}
}
}
}
else
{
synchronized (this)
{
if (! _evaluated)
{
_value = evaluate();
_evaluated = true;
}
}
}
return _value;
}
+ /*
+ * Get an object instance to invoke based on the object key.
+ *
+ * The objectKey could probably be passed to the EJB container
so that the
+ * container can directly invoke the ejb object as required.
+ */
+ public Object getInstance(byte[] objectKey) {
+ String key = new String(objectKey);
+ return getInstance(key);
+ }
+
+ public Object getInstance(String key) {
+ Object o = _objects.get(key);
+
+ if (o == null) {
+ o = newInstance(key);
+ }
+
+ return o;
+ }
+
+ public Object newInstance(byte[] objectKey) {
+ String key = new String(objectKey);
+ return newInstance(key);
+ }
+
+ public Object newInstance(String key) {
+ Object o = null;
+
+ if (_remoteClassClass == null) {
+ synchronized (this) {
+ _remoteClassClass = loadClass(_remoteClassName);
+ }
+
+ try {
+ if (_shared) {
+ synchronized (this) {
+ Method m =
_remoteClassClass.getMethod("getInstance", (Class[]) null);
+ o = m.invoke(_remoteClassClass, (Object[])
null);
+
+ if (o != null) {
+ _objects.put(key, o);
+ }
+ }
+ } else {
+ o = _remoteClassClass.newInstance();
+ _objects.put(key, o);
+ }
+ } catch (InstantiationException e) {
+ e.printStackTrace(); //To change body of catch
statement use File | Settings | File Templates.
+ } catch (IllegalAccessException e) {
+ e.printStackTrace(); //To change body of catch
statement use File | Settings | File Templates.
+ } catch (NoSuchMethodException e) {
+ e.printStackTrace();
+ } catch (InvocationTargetException e) {
+ e.printStackTrace();
+ }
+ }
+
+ return o;
+ }
Again there is double check locking. Also, if this is called in the
critical path, I suggest we use CGLIB instead of reflection for
construction as it is way faster.
MD>> I can look into CGLIB.
+ /*
+ * Invoke method from the IIOP Message Handler. The adapter is
bound to the INS name service.
+ * When an RMI/IIOP message is processed by the server, the
message handler will perform a lookup
+ * on the name service to get the Adapter, then the invocation
will be passed to the adapter
+ * The adapter will obtain the object key and then determine
which object instance to pass the
+ * invocation to.
+ */
This comment is missing the double star at the top to signify javadoc.
+ public void invoke(java.lang.String methodName, byte[]
objectKey, org.apache.geronimo.interop.rmi.iiop.ObjectInputStream
input, org.apache.geronimo.interop.rmi.iiop.ObjectOutputStream
output) {
+ RemoteInterface skeleton = getRemoteInterface();
+ Object instance = getInstance(objectKey);
+
+ if (instance != null) {
+ skeleton.$invoke(methodName, objectKey, instance,
input, output);
+ } else {
+ throw new org.omg.CORBA.OBJECT_NOT_EXIST(new
String(objectKey));
+ }
+ }
+
+ /*
+ * Helper function to load a class. This uses classloader for
the adapter.
+ */
+ protected Class loadClass(String name) {
+ Class c = null;
+
+ try {
+ c = _cl.loadClass(name);
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+
+ return c;
+ }
+}
I think we should be at least logging the error or actually letting
the ClassNotFoundException through. We also have a ClassLoading
class which you may want to use as it handles some of the more weird
class name formats.
MD >> Will change with better Geronimo integration.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
adapter/AdapterManager.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/adapter/AdapterManager.java?
view=auto&rev=126264
=====================================================================
== =======
--- (empty file)
+++
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
adapter/AdapterManager.java Sun Jan 23 23:33:10 2005
@@ -0,0 +1,43 @@
+/**
+ *
+ * Copyright 2004-2005 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
software
+ * distributed under the License is distributed on an "AS IS"
BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ *
+ * See the License for the specific language governing permissions
and
+ * limitations under the License.
+ */
+package org.apache.geronimo.interop.adapter;
+
+import java.util.Hashtable;
+
+
+public class AdapterManager {
+ protected Hashtable _adapters;
+ protected static AdapterManager _me = new AdapterManager();
Underscores and protected...
MD>> Will do.
+ protected AdapterManager() {
+ _adapters = new Hashtable();
+ }
+
+ public static AdapterManager getInstance() {
+ return _me;
+ }
This is a bad idea. Instead we should have a manger gbean and the
singletonness would be handled by there being only one GBean instance
registered.
MD>> Sure, this will change with the introduction of GBeans, better
integration...
+ public void registerAdapter(Adapter a) {
+
+ _adapters.put(a.getBindName(), a);
+ }
+
+ public Adapter getAdapter(String objectName) {
+ return (Adapter) _adapters.get(objectName);
+ }
I prefer HashMap and synchronizing the methods that access the Map,
but this works in this case.
MD>> Hashtables are going to be dead soon. ;-)
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
client/InitialContextFactory.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/client/InitialContextFactory.java?
view=auto&rev=126264
=====================================================================
== =======
+public class InitialContextFactory
+ implements javax.naming.spi.InitialContextFactory {
+ private HashMap _startMap = new HashMap();
+
+ public Context getInitialContext(java.util.Hashtable env)
throws NamingException {
+ return
org.apache.geronimo.interop.rmi.iiop.client.ClientNamingContext.getIn
st ance(env);
+ }
+}
Is the _startMap variable used anywhere?
MD>> I'd like to remove any naming in the interop and just use the
naming provided in Geronimo.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
client/InitialContextFactoryBuilder.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/client/
InitialContextFactoryBuilder.java? view=auto&rev=126264
=====================================================================
== =======
+public class InitialContextFactoryBuilder
+ implements javax.naming.spi.InitialContextFactoryBuilder {
+ public javax.naming.spi.InitialContextFactory
createInitialContextFactory(Hashtable env) {
+ return new InitialContextFactory();
+ }
+}
What is this for? Just curious...
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
generator/GenException.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/generator/GenException.java?
view=auto&rev=126264
=====================================================================
== =======
+/**
+ * User: Mark
+ * Date: Dec 21, 2004
+ * Time: 3:49:45 PM
+ */
Oops :)
+public class GenException
+ extends Exception {
+ public GenException() {
+ super();
+ }
+
+ public GenException(String message) {
+ super(message);
+ }
+
+ public GenException(Throwable cause) {
+ super(cause);
+ }
+
+ public GenException(String message, Throwable cause) {
+ super(message, cause);
+ }
+}
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
generator/GenOptions.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/generator/GenOptions.java?
view=auto&rev=126264
=====================================================================
== =======
+public class GenOptions {
+ protected String _genDir = "./";
+ protected boolean _overwrite = false;
+ protected boolean _verbose = false;
+
+ public GenOptions() {
+ }
+
+ public GenOptions(String genDir, boolean overwrite, boolean
verbose) {
+ _genDir = genDir;
+ _overwrite = overwrite;
+ _verbose = verbose;
+ }
+
+ public String getGenDir() {
+ return _genDir;
+ }
+
+ public void setGenDir(String genDir) {
+ _genDir = genDir;
+ }
+
+ public boolean isOverwrite() {
+ return _overwrite;
+ }
+
+ public void setOverwrite(boolean overwrite) {
+ _overwrite = overwrite;
+ }
+
+ public boolean isVerbose() {
+ return _verbose;
+ }
+
+ public void setVerbose(boolean verbose) {
+ _verbose = verbose;
+ }
+
+}
Do we need both the constructor args and the setters? If I set a
variable is whatever is using this class going to adapt to the
recently changed variable?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
naming/NameService.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/naming/NameService.java?
view=auto&rev=126264
=====================================================================
== =======
--- (empty file)
+++
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
naming/NameService.java Sun Jan 23 23:33:10 2005
@@ -0,0 +1,66 @@
+/**
+ *
+ * Copyright 2004-2005 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
software
+ * distributed under the License is distributed on an "AS IS"
BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ *
+ * See the License for the specific language governing permissions
and
+ * limitations under the License.
+ */
+package org.apache.geronimo.interop.naming;
+
+import java.util.HashMap;
+import javax.naming.NamingException;
+
+import org.apache.geronimo.interop.adapter.Adapter;
+
+
+public class NameService {
+ protected static NameService _ns = null;
protected and underscore
+ public static NameService getInstance() {
+ if (_ns == null) {
+ synchronized (NameService.class) {
+ if (_ns == null) {
+ _ns = new NameService();
+ _ns.init();
+ }
+ }
+ }
+
+ return _ns;
+ }
Double check locking
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
naming/NamingContext.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/naming/NamingContext.java?
view=auto&rev=126264
=====================================================================
== =======
--- (empty file)
+++
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
naming/NamingContext.java Sun Jan 23 23:33:10 2005
@@ -0,0 +1,150 @@
+/**
+ *
+ * Copyright 2004-2005 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
software
+ * distributed under the License is distributed on an "AS IS"
BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ *
+ * See the License for the specific language governing permissions
and
+ * limitations under the License.
+ */
+package org.apache.geronimo.interop.naming;
+
+import java.util.HashMap;
+import javax.naming.NameNotFoundException;
+import javax.naming.NamingException;
+
+import org.apache.geronimo.interop.adapter.Adapter;
+
+
+public class NamingContext {
+ public static final NamingContext getInstance(Class baseClass) {
+ NamingContext context;
+ synchronized (_contextMap) {
+ context = (NamingContext) _contextMap.get(baseClass);
+ if (context == null) {
+ context = new NamingContext();
+ _contextMap.put(baseClass, context);
+ context.init(baseClass);
+ }
+ }
+ return context;
+ }
+
+ private static ThreadLocal _current = new ThreadLocal();
Should this be an InheritableThreadLocal?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/BooleanProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/BooleanProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/ByteProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/ByteProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/DoubleProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/DoubleProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/FloatProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/FloatProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/IntProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/IntProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/LongProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/LongProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/ShortProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/ShortProperty.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/StringProperty.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/StringProperty.java?
view=auto&rev=126264
How are these used?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
properties/SystemProperties.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/properties/SystemProperties.java?
view=auto&rev=126264
Is this a bridge to java.lang.System properties? If so, do we really
want to support system properties?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
repository/Repository.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/repository/Repository.java?
view=auto&rev=126264
=====================================================================
== =======
+public class Repository {
+ // ??
+}
Say what? :)
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/IiopVersion.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/rmi/iiop/IiopVersion.java?
view=auto&rev=126264
=====================================================================
== =======
+public class IiopVersion {
+}
What's this for?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/ListenerInfo.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/rmi/iiop/ListenerInfo.java?
view=auto&rev=126264
+public class ListenerInfo {
+ public int protocol;
+
+ public String host;
+
+ public int port;
+}
Do these have to be public fields? Normally we use, private fields
with getters and setters.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/NameServiceOperations_Skeleton.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/rmi/iiop/
NameServiceOperations_Skeleton.java?view=auto&rev=126264
Do we want the Skeletons checked in?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/ObjectKey.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/rmi/iiop/ObjectKey.java?
view=auto&rev=126264
=====================================================================
== =======
+public class ObjectKey {
+ public static final int TYPE_MANAGER = 'M';
+
+ public static final int TYPE_SESSION = 'S';
+
+ public int type;
+ public String username = "";
+ public String password = "";
+ public String component = "";
+ public String sessionID = "";
public fields?
+ public void checkPassword() {
+ User.getInstance(username).login(password);
+ }
Shouldn't this go though Geronimo security?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/ObjectRef.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/rmi/iiop/ObjectRef.java?
view=auto&rev=126264
=====================================================================
== =======
+ public int $getIiopVersion() {
+ return _iiopVersion;
+ }
Why do all the methods in this class start with a dollar sign?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/client/ClientNamingContext.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/rmi/iiop/client/
ClientNamingContext.java? view=auto&rev=126264
We should consider merging this with the Geronimo naming stuff... not
a task for today, but (much) later on :)
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/compiler/StubCompiler.txt
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/rmi/iiop/compiler/StubCompiler.txt?
view=auto&rev=126264
Why is this a text file?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
rmi/iiop/server/SocketListener.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/rmi/iiop/server/SocketListener.java?
view=auto&rev=126264
=====================================================================
== =======
+public class SocketListener extends Thread {
+ public static SocketListener getInstance() {
+ return new SocketListener();
+ }
Why is there a static factory?
+ // private data
+
+ private String _name;
+
+ private String _host;
+
+ private int _port;
+
+ private int _listenBacklog;
+
+ private java.net.ServerSocket _listener;
+
+ // internal methods
+
+ protected void init() {
+ _host = "localhost";
+ _port = 2000;
+ _listenBacklog = 10;
+ setDaemon(true);
+ }
+
+ // public methods
+
+ public void setHost(String host) {
+ _host = host;
+ }
+
+ public void setPort(int port) {
+ _port = port;
+ }
+
+ public void setListenBacklog(int backlog) {
+ _listenBacklog = backlog;
+ }
+
+ public void run() {
+ String iiopURL = "iiop://" + _host + ":" + _port;
+ ListenerInfo listenerInfo = new ListenerInfo();
+ listenerInfo.protocol = Protocol.IIOP; // TODO: other
protocols (IIOPS etc.)
+ listenerInfo.host = _host;
+ listenerInfo.port = _port;
+ try {
+ InetAddress addr = InetAddress.getByName(_host);
+ _listener = new java.net.ServerSocket(_port,
_listenBacklog, addr);
+ } catch (Exception ex) {
+ System.out.println("SocketListener: Error creating
server socket.");
+ ex.printStackTrace();
+ try {
+ Socket socket = new Socket(_host, _port);
+ socket.close();
+ System.out.println("SocketListener: Error server
already running: " + iiopURL);
+ ex.printStackTrace();
+ } catch (Exception ignore) {
+ }
+ return;
+ }
+ new CheckConnect().start();
+ for (; ;) {
+ java.net.Socket socket;
+ try {
+ socket = _listener.accept();
+ } catch (Exception ex) {
+ throw new SystemException(ex); // TODO: log error
message
+ }
+ MessageHandler.getInstance(listenerInfo,
socket).start();
+ }
+ }
+
+ private class CheckConnect extends Thread {
+ public void run() {
+ try {
+ Socket socket = new Socket(_host, _port);
+ socket.close();
+ if (!SystemProperties.quiet()) {
+
System.out.println(formatAcceptingIiopConnections());
+ }
+ } catch (Exception ex) {
+ warnConnectFailed(_host, _port);
+ }
+ }
+ }
+
+ // format methods
+
+ protected String formatAcceptingIiopConnections() {
+ String msg =
"SocketListener.formatAcceptingIiopConnection(): ";
+ return msg;
+ }
+
+ // log methods
+
+ protected void warnConnectFailed(String host, int port) {
+ System.out.println("SocketListener.warnConnectFailed():
host = " + host + ", port = " + port);
+ }
+}
I suggest, you either require all state data such as host and port be
passed in via the constructor, or make the setters throw an "already
running" exception. Otherwise people mistakenly think the can
change the port.
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
security/Role.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/security/Role.java?view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
security/SimpleSubject.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/security/SimpleSubject.java?
view=auto&rev=126264
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
security/User.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/security/User.java?view=auto&rev=126264
What is the plan for these with regards to the Geronimo security
module?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
util/SystemUtil.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/
org/apache/geronimo/interop/util/SystemUtil.java?
view=auto&rev=126264
=====================================================================
== =======
+public class SystemUtil {
+ // properties
+
+ public static final StringProperty vmVersionProperty =
+ new StringProperty(SystemProperties.class,
"java.vm.version");
+
+ // private data
+
+ private static String _vmVersion =
vmVersionProperty.getString();
+
+ private static boolean _isJDK13 = _vmVersion.startsWith("1.3")
+ ||
_vmVersion.startsWith("CrE-ME V4.00");
+
+ private static boolean _isJDK14 = _vmVersion.startsWith("1.4");
+
+ // public methods
+
+ public static String getExecutableSuffix() {
+ return isWindows() ? ".exe" : "";
+ }
+
+ public static String getShellScriptSuffix() {
+ return isWindows() ? ".bat" : ".sh";
+ }
+
+ public static boolean isJDK13() {
+ return _isJDK13;
+ }
+
+ public static boolean isJDK14() {
+ return _isJDK14;
+ }
+
+ public static boolean isWindows() {
+ return java.io.File.separatorChar == '\\';
+ }
+}
How is this used?
Added:
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
util/ThreadContext.java
Url:
http://svn.apache.org/viewcvs/geronimo/trunk/modules/interop/src/
java/ org/apache/geronimo/interop/util/ThreadContext.java?
view=auto&rev=126264
=====================================================================
== =======
--- (empty file)
+++
geronimo/trunk/modules/interop/src/java/org/apache/geronimo/interop/
util/ThreadContext.java Sun Jan 23 23:33:10 2005
@@ -0,0 +1,135 @@
+/**
+ *
+ * Copyright 2004-2005 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
software
+ * distributed under the License is distributed on an "AS IS"
BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
+ *
+ * See the License for the specific language governing permissions
and
+ * limitations under the License.
+ */
+package org.apache.geronimo.interop.util;
+
+import java.util.HashMap;
+
+import org.apache.geronimo.interop.SystemException;
+
+
+public abstract class ThreadContext {
+ private static HashMap _primTypes;
+
+ private static ThreadLocal _defaultRmiHost = new ThreadLocal();
+
+ private static ThreadLocal _defaultRmiPort = new ThreadLocal();
+
+ static {
+ _primTypes = new HashMap();
+ _primTypes.put("boolean", boolean.class);
+ _primTypes.put("char", char.class);
+ _primTypes.put("byte", byte.class);
+ _primTypes.put("short", short.class);
+ _primTypes.put("int", int.class);
+ _primTypes.put("long", long.class);
+ _primTypes.put("float", float.class);
+ _primTypes.put("double", double.class);
+ _primTypes.put("boolean[]", boolean[].class);
+ _primTypes.put("char[]", char[].class);
+ _primTypes.put("byte[]", byte[].class);
+ _primTypes.put("short[]", short[].class);
+ _primTypes.put("int[]", int[].class);
+ _primTypes.put("long[]", long[].class);
+ _primTypes.put("float[]", float[].class);
+ _primTypes.put("double[]", double[].class);
+ }
+
+ public static String getDefaultRmiHost() {
+ String host = (String) _defaultRmiHost.get();
+ if (host == null) {
+ host = "0";
+ }
+ return host;
+ }
+
+ public static int getDefaultRmiPort() {
+ Integer port = (Integer) _defaultRmiPort.get();
+ if (port == null) {
+ port = IntegerCache.get(0);
+ }
+ return port.intValue();
+ }
+
+ public static Class loadClass(String className) {
+ Class t = (Class) _primTypes.get(className);
+ if (t != null) {
+ return t;
+ }
+ try {
+ ClassLoader loader =
Thread.currentThread().getContextClassLoader();
+ if (loader == null) {
+ return Class.forName(className);
+ } else {
+ return loader.loadClass(className);
+ }
+ } catch (RuntimeException ex) {
+ throw (RuntimeException) ex;
+ } catch (Exception ex) {
+ throw new SystemException(ex);
+ }
+ }
This is unlikely to work in Geronimo as we almost never set context
class loader. Normally we try to explicitly pass around the
classloader.
+
+ public static Class loadClass(String className, Class
parentClass) {
+ if (parentClass == null) {
+ return loadClass(className);
+ }
+ Class t = (Class) _primTypes.get(className);
+ if (t != null) {
+ return t;
+ }
+ try {
+ ClassLoader loader = parentClass.getClassLoader();
+ if (loader == null) {
+ return loadClass(className);
+ } else {
+ return loader.loadClass(className);
+ }
+ } catch (RuntimeException ex) {
+ throw (RuntimeException) ex;
+ } catch (Exception ex) {
+ throw new SystemException(ex);
+ }
+ }
+
+ public static Class loadClassOrReturnNullIfNotFound(String
className) {
+ try {
+ return loadClass(className);
+ } catch (RuntimeException ex) {
+ return null;
+ }
+ }
+
+ public static Class loadClassOrReturnNullIfNotFound(String
className, Class parentClass) {
+ if (parentClass == null) {
+ return loadClassOrReturnNullIfNotFound(className);
+ }
+ try {
+ return loadClass(className, parentClass);
+ } catch (RuntimeException ex) {
+ return null;
+ }
+ }
Most of this class loader stuff is handled by the class ClassLoading
which handles some of the more weird stuff.
.
