On Apr 3, 2009, at 7:09 PM, David Jencks wrote:

I got annoyed that the global ejb bindings from openejb didn't show up in the geronimo global jndi tree so thought I'd try to fix it.

My first and possibly ill-advised strategy was to use the same flag we use to decide what jndi implementation to use for the java:comp context to decide what to use for the global context. After a little bit of struggle I think I have this working OK in geronimo.

While doing this I discovered that the openejb jndi implementation does a couple of odd things:

1. creates subcontexts on the fly when you bind something. I don't think this is spec compliant but I have no particular objection to it -- it certainly is handy and I can't see how it can create problems or confusion.

Agreed. As a result, we really don't need any of the calls to createSubcontext() when they are followed by a bind using the name and should shave those down too.

2. strips off name prefixes such as java: and openejb: I'm not too sure what the purpose of this is but I think it may be leading to sloppy and very confusing code in the server. For instance when the global context is set up we see:

The "chopping" code in the lookup matches the behavior in the InitialContext.lookup() implementation which will catch all prefixed lookups and direct then to an essentially static context implementation designated to handle the prefix. Merely striving for some consistency with that as we've had complaints that sometimes "prefix:foo" works and sometimes it doesn't. Many reasons for that of course, but one of them has been because of the difference between 'new InitialContext().lookup("openejb:foo")' and 'new InitialContext().lookup("").lookup("openejb:foo")' [cast to (Context) omitted for simplicity]

For the moment though the bind, rebind and list methods don't match that updated behavior and still have, IMO, bugs in their chopping logic.

           jndiRootContext.createSubcontext("java:openejb/ejb");
           jndiRootContext.createSubcontext("java:openejb/client");
jndiRootContext.createSubcontext("java:openejb/ Deployment");

These lines are old and should be deleted as you point out.

           jndiRootContext.bind("openejb/ejb/.", "");
           jndiRootContext.bind("openejb/client/.", "");
           jndiRootContext.bind("openejb/Deployment/.", "");

Agreed. Chopping the "java:" is a good thing.

I have no objection to the jndi implementation modifying names to make it easier for users to find stuff -- so for user convenience I think stripping off java: and other prefixes is just fine. However I think the server will be a lot less confusing if it does not rely on this when binding stuff.

Agreed, especially for the reason that I mention about "chopping" in the binding not yet matching the updated chopping logic in the lookup(). The old way just chops it off and binds to that exact context instance, not from the root. So the fact that it works is just coincidence. The change is only two weeks old and I definitely intended to come back and fix that when I got the chance. Removing the "java:" on binds will be a great step in the right direction.

So, I'd like to propose that we consider making the jndi provider more pluggable, and testing everything with a provider that does not strip off prefixes (such as xbean)

I've created https://issues.apache.org/jira/browse/OPENEJB-1014 and attached my current patch in case anyone wants to take a look. If people think going in this direction is a good idea I'd like to see if there's a cleaner way to supply the jndi contexts, perhaps by injecting something.

As long as the context being plugged in supports the extra bits we need, than I'm fine with it as long as it doesn't add a lot of complexity. The argument for or against it is equally unimportant: if openejb is running in geronimo, and one Context implementation is as good as another, why not use the geronimo one; if openejb is running in geronimo, and one Context implementation is as good as another, why does it mater if openejb uses openejb's implementation. But as I say, as long as there's no objections to OpenEJB using it's extra bits, I have no objection to another implementation being plugged in that supports those extra bits.

One last "extra bit" that we do is the inverse of the "auto subcontext adding" is "auto empty-subcontext pruning" via the IvmContext.prune() method we use to prune the section of the OpenEJB internal JNDI tree that holds the EJB refs (openejb/Deployment and openejb/ejb). I don't recall if it was a TCK issue or an issue on the G user list, but I added that pruning to get around issues relating to undeployment of an app leaving behind empty subcontexts that can result in inability to deploy apps that might want to use that same name as a non-context. Happens more frequently with longer deployment ids (i.e. appName/ moduleName/ejbName/interfaceClass).

In regards to the patch, for style lets move the inner class out (maybe to a class called XBeanContext) so we can keep the CoreContainerSystem class small and focused.

btw dblevins mentioned that the tomcat integration seemed to be having some jndi problems.

I fixed those later Thursday.  It should run now.

I tried running the tests with my modified openejb and did not see anything that looked like a jndi problem -- all the tests I could identify as relating to tomcat passed. The remote tests had this error: java.lang.IllegalStateException: Cannot find client/tools/ DatabaseHome: javax.naming.NameNotFoundException /client/tools/ DatabaseHome does not exist in the system. Check that the app was successfully deployed. which appeared to relate somehow with hsqldb. I didn't investigate this.

Either Tomcat didn't start or the openejb.war didn't start. Check the logs for port conflicts or something similar.

-David

Reply via email to