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