David,

This change seems to be causing or surfacing some failures in the TCK. Can you check out the geronimo tck list and lend your expert advice as to what might be going on?

Thanks,
Joe


[EMAIL PROTECTED] wrote:
Author: dblevins
Date: Sat Sep 15 23:23:54 2007
New Revision: 576045

URL: http://svn.apache.org/viewvc?rev=576045&view=rev
Log:
- Fixed the IvmContext to properly throw NameAlreadyBoundException
rather than allowing the bind to cover a previous binding.

- Severely fixed up the information, recovery, and flexibility
associated with jndi binding and collisions.

- Added a new system property openejb.jndiname.failoncollision which
defaults to true, when set to false attempting to bind an object to
jndi with a name that already exists results in logging an error but
the app still deploys


Modified:
    
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
    
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/JndiBuilder.java
    
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/cmp/CmpContainer.java
    
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/ivm/naming/NameNode.java
    
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/stateless/StatelessInstanceManager.java
    
openejb/trunk/openejb3/container/openejb-core/src/main/resources/embedded.logging.properties
    
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/IvmContextTest.java

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
 Sat Sep 15 23:23:54 2007
@@ -530,6 +530,7 @@
     }
private void destroyApplication(AppInfo appInfo) throws UndeployException {
+        logger.info("Undeploying app: "+appInfo.jarPath);
         Context globalContext = containerSystem.getJNDIContext();
         UndeployException undeployException = new UndeployException("Failed 
undeploying application: id=" + appInfo.jarPath);
@@ -563,7 +564,7 @@
             }
JndiBuilder.Bindings bindings = deployment.get(JndiBuilder.Bindings.class);
-            for (String name : bindings.getBindings()) {
+            if (bindings != null) for (String name : bindings.getBindings()) {
                 try {
                     globalContext.unbind(name);
                 } catch (Throwable t) {

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/JndiBuilder.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/JndiBuilder.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/JndiBuilder.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/JndiBuilder.java
 Sat Sep 15 23:23:54 2007
@@ -26,9 +26,9 @@
import org.apache.openejb.DeploymentInfo;
 import org.apache.openejb.InterfaceType;
+import org.apache.openejb.spi.ContainerSystem;
 import org.apache.openejb.util.LogCategory;
 import org.apache.openejb.util.Logger;
-import org.apache.openejb.util.Classes;
 import org.apache.openejb.loader.SystemInstance;
 import org.apache.openejb.core.CoreDeploymentInfo;
 import org.apache.openejb.core.ivm.naming.BusinessLocalReference;
@@ -54,9 +54,13 @@
private final Context context;
     private static final String JNDINAME_STRATEGY_CLASS = 
"openejb.jndiname.strategy.class";
+    private static final String JNDINAME_FAILONCOLLISION = 
"openejb.jndiname.failoncollision";
+    private final boolean failOnCollision;
public JndiBuilder(Context context) {
         this.context = context;
+        String property = SystemInstance.get().getProperty(JNDINAME_FAILONCOLLISION, 
"true");
+        failOnCollision = "true".equalsIgnoreCase(property);
     }
public void build(EjbJarInfo ejbJar, HashMap<String, DeploymentInfo> deployments) {
@@ -265,11 +269,8 @@
                 String internalName = "openejb/Deployment/" + 
deployment.getDeploymentID() + "/" + interfce.getName();
                 bind(internalName, ref, bindings, beanInfo);
- try {
-                    String externalName = "openejb/ejb/" + 
strategy.getName(deployment, interfce, JndiNameStrategy.Interface.BUSINESS_LOCAL);
-                    bind(externalName, ref, bindings, beanInfo);
-                } catch (NamingException dontCareJustYet) {
-                }
+                String externalName = "openejb/ejb/" + 
strategy.getName(deployment, interfce, JndiNameStrategy.Interface.BUSINESS_LOCAL);
+                bind(externalName, ref, bindings, beanInfo);
             }
         } catch (NamingException e) {
             throw new RuntimeException("Unable to bind business local interface for 
deployment " + id, e);
@@ -289,11 +290,8 @@
                 String internalName = "openejb/Deployment/" + 
deployment.getDeploymentID() + "/" + interfce.getName();
                 bind(internalName, ref, bindings, beanInfo);
- try {
-                    String externalName = "openejb/ejb/" + 
strategy.getName(deployment, interfce, JndiNameStrategy.Interface.BUSINESS_REMOTE);
-                    bind(externalName, ref, bindings, beanInfo);
-                } catch (NamingException dontCareJustYet) {
-                }
+                String externalName = "openejb/ejb/" + 
strategy.getName(deployment, interfce, JndiNameStrategy.Interface.BUSINESS_REMOTE);
+                bind(externalName, ref, bindings, beanInfo);
             }
         } catch (NamingException e) {
             throw new RuntimeException("Unable to bind business remote deployment 
in jndi.", e);
@@ -330,9 +328,17 @@
                 context.bind(name, ref);
                 bindings.add(name);
                 beanInfo.jndiNames.add(externalName);
-                logger.info("Jndi(name=" + externalName +")");
+                logger.info("Jndi(name=" + externalName +") bound to 
Ejb(deployment-id="+beanInfo.ejbDeploymentId+")");
             } catch (NameAlreadyBoundException e) {
-                logger.error("Jndi name already in use: could not be bound; it may be taken 
by another ejb.  Jndi(name=" + name +")");
+                DeploymentInfo deployment = findNameOwner(name);
+                if (deployment != null){
+                    logger.error("Jndi(name=" + externalName +") cannot be bound to 
Ejb(deployment-id="+beanInfo.ejbDeploymentId+").  Name already taken by 
Ejb(deployment-id="+deployment.getDeploymentID()+")");
+                } else {
+                    logger.error("Jndi(name=" + externalName +") cannot be bound to 
Ejb(deployment-id="+beanInfo.ejbDeploymentId+").  Name already taken by another object in the 
system.");
+                }
+                // Construct a new exception as the IvmContext doesn't include
+                // the name in the exception that it throws
+                if (failOnCollision) throw new 
NameAlreadyBoundException(externalName);
             }
         } else {
             try {
@@ -340,16 +346,27 @@
                 bindings.add(name);
             } catch (NameAlreadyBoundException e) {
                 logger.error("Jndi name could not be bound; it may be taken by another ejb.  
Jndi(name=" + name +")");
-                throw e;
+                // Construct a new exception as the IvmContext doesn't include
+                // the name in the exception that it throws
+                throw new NameAlreadyBoundException(name);
             }
         }
} - private static List<Class> asList(Class interfce) {
-        List<Class> list = new ArrayList<Class>();
-        list.add(interfce);
-        return list;
+    /**
+     * This may not be that performant, but it's certain to be faster than the
+     * user having to track down which deployment is using a particular jndi 
name
+     * @param name
+     * @return .
+     */
+    private DeploymentInfo findNameOwner(String name) {
+        ContainerSystem containerSystem = 
SystemInstance.get().getComponent(ContainerSystem.class);
+        for (DeploymentInfo deploymentInfo : containerSystem.deployments()) {
+            Bindings bindings = deploymentInfo.get(Bindings.class);
+            if (bindings != null && bindings.getBindings().contains(name)) 
return deploymentInfo;
+        }
+        return null;
     }
protected static final class Bindings {

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/cmp/CmpContainer.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/cmp/CmpContainer.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/cmp/CmpContainer.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/cmp/CmpContainer.java
 Sat Sep 15 23:23:54 2007
@@ -208,11 +208,13 @@
             }
CmpEngine cmpEngine = (CmpEngine) deploymentInfo.getContainerData();
-            cmpEngine.undeploy(deploymentInfo);
+            if (cmpEngine != null){
+                cmpEngine.undeploy(deploymentInfo);
- if (cmpEngine.isEmpty()){
-                cmpEngines.remove(deploymentInfo.getJarPath());
-                cmpEngines.remove(deploymentInfo.getClassLoader());
+                if (cmpEngine.isEmpty()){
+                    cmpEngines.remove(deploymentInfo.getJarPath());
+                    cmpEngines.remove(deploymentInfo.getClassLoader());
+                }
             }
             deploymentInfo.setContainer(null);
             deploymentInfo.setContainerData(null);

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/ivm/naming/NameNode.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/ivm/naming/NameNode.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/ivm/naming/NameNode.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/ivm/naming/NameNode.java
 Sat Sep 15 23:23:54 2007
@@ -86,6 +86,9 @@
                 if (subTree != null) {
                     throw new javax.naming.NameAlreadyBoundException();
                 }
+                if (myObject != null){
+                    throw new javax.naming.NameAlreadyBoundException();
+                }
                 unbound = false;
                 myObject = obj;// bind the object to this node
             }

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/stateless/StatelessInstanceManager.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/stateless/StatelessInstanceManager.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/stateless/StatelessInstanceManager.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/core/stateless/StatelessInstanceManager.java
 Sat Sep 15 23:23:54 2007
@@ -288,6 +288,7 @@
public void undeploy(CoreDeploymentInfo deploymentInfo) {
         Data data = (Data) deploymentInfo.getContainerData();
+        if (data == null) return;
         Stack pool = data.getPool();
         //TODO ejbRemove on each bean in pool.
         //clean pool

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/main/resources/embedded.logging.properties
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/resources/embedded.logging.properties?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/main/resources/embedded.logging.properties
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/main/resources/embedded.logging.properties
 Sat Sep 15 23:23:54 2007
@@ -51,6 +51,7 @@
 log4j.category.OpenEJB             = warn
 log4j.category.OpenEJB.server      = info
 log4j.category.OpenEJB.startup     = info
+log4j.category.OpenEJB.startup.config = info
 log4j.category.OpenEJB.hsql        = info
 log4j.category.CORBA-Adapter       = debug
 log4j.category.Transaction         = warn

Modified: 
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/IvmContextTest.java
URL: 
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/IvmContextTest.java?rev=576045&r1=576044&r2=576045&view=diff
==============================================================================
--- 
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/IvmContextTest.java
 (original)
+++ 
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/core/ivm/naming/IvmContextTest.java
 Sat Sep 15 23:23:54 2007
@@ -23,6 +23,7 @@
 import org.apache.openejb.util.Debug;
import javax.naming.*;
+import javax.naming.NamingException;
/**
  * @version $Rev$ $Date$
@@ -126,6 +127,19 @@
Map<String, Object> map = Debug.contextToMap(context);
         assertFalse("name should not appear in bindings list", 
map.containsKey("veggies/tomato/roma"));
+    }
+
+    public void testAlreadyBound() throws Exception {
+
+        IvmContext context = new IvmContext();
+        context.bind("root/number", 2);
+        try {
+            context.bind("root/number", 3);
+            fail("A NameAlreadyBoundException should have been thrown");
+        } catch (NameAlreadyBoundException e) {
+            // pass
+        }
+
     }
public void test() throws Exception {



Reply via email to