Definitely the code that removes 'java:openejb' and replaces it with
'openejb' can go in now.
I like the JndiFactory with the createComponentContext() and
getRootContext() methods. I don't like how far through the code the
JndiFactory is spread because of the bind() method. Obviously all the
functionality can be done in the JNDI impl or a wrapped Context
returned from the factory. I don't think the jndiFactory.bind()
communicates anything that can't be done much better with javadoc on
the JndiFactory class and seeing something with the name 'Factory'
passed around like it's a Context is not too pretty.
As far as the way the JndiFactory is created, I'd much prefer a system
flag that points to the factory impl class name rather than have it
passed in various constructors. Magic methods and constructors that
aren't used in the main code are so easily broken and I think make the
integration ultimately more fragile. On that note, having the xbean
factory in the OpenEJB code would be preferable as it would be easier
to ensure it doesn't break and someone could theoretically use the
xbean impl even if they weren't using Geronimo.
More than happy to make these changes myself if you prefer; have no
problems putting time in with the suggestions.
Not sure what the strangeness is with the JndiRequestHandler code.
There's no code in there that uses startsWith("openejb/Deployment").
What kind of problem did you find?
-David
On Jul 6, 2009, at 1:19 AM, David Jencks wrote:
I've created a better patch for OPENEJB-1014 and if there are no
objections would like to apply it. What it does is mostly:
-- use a factory to create the jndi contexts, rather than the
existing ivm/xbean flag. The factory also handles binding to the
global context so that explicit subcontext creation by the client is
not required.
-- removes all the java: prefixes from global jndi context access I
could find. These are stripped by the ivm context implementation
anyway, so I think not using them in openejb code is clearer.
-- AFAICT JndiRequestHandler is currently doing something weird:
deploymentsJndiTree = (Context)
containerSystem.getJNDIFactory().getRootContext().lookup("openejb/
Deployment");
....
if (name.startsWith("openejb/Deployment") {
deploymentsJndiTree.lookup(name);
}
this isnt' the exact code -- there might be a different problem
since the "startsWith" is actually looking at a moduleId which
appears to be a prefix of name -- but this meaning of
deploymentsJndiTree doesn't seem to work with xbean naming in
geronimo.
I think the resulting code is clearer in openejb and makes the
geronimo integration simpler.
thanks
david jencks
On Apr 3, 2009, at 6: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.
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:
jndiRootContext.createSubcontext("java:openejb/ejb");
jndiRootContext.createSubcontext("java:openejb/client");
jndiRootContext.createSubcontext("java:openejb/
Deployment");
jndiRootContext.bind("openejb/ejb/.", "");
jndiRootContext.bind("openejb/client/.", "");
jndiRootContext.bind("openejb/Deployment/.", "");
I'm not sure the createSubcontext calls are actually necessary due
to (1) but I think it is far from obvious that binding at openejb/
ejb/. occurs in the subcontext just created at java:openejb/ejb.
Similarly pretty much everything is bound into the root context
under "java:" which is stripped off before actually being bound.
At least 90% of the changes I made to get the alternate jndi
provider working were removing the java: from the global names.
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.
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.
Thoughts?
btw dblevins mentioned that the tomcat integration seemed to be
having some jndi problems. 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.
thanks
david jencks
