Repository: tomee Updated Branches: refs/heads/tomee-1.7.x a2bbedf12 -> 4e9444c58
Add test, and clean up resources when container is shut down Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/4e9444c5 Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/4e9444c5 Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/4e9444c5 Branch: refs/heads/tomee-1.7.x Commit: 4e9444c5848b9e89fafcb7ba97f2600a4b241634 Parents: a2bbedf Author: Jonathan Gallimore <[email protected]> Authored: Tue Feb 9 14:36:39 2016 +0000 Committer: Jonathan Gallimore <[email protected]> Committed: Tue Feb 9 14:36:39 2016 +0000 ---------------------------------------------------------------------- .../openejb/assembler/classic/Assembler.java | 34 +++-- .../openejb/resource/BadResourceTest.java | 129 +++++++++++++++++++ 2 files changed, 154 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/4e9444c5/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java index 91b4f7a..ba37a1c 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java @@ -1664,7 +1664,7 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A } catch (final NamingException ignored) { // no resource adapters were created } - destroyResourceTree(namingEnumeration); + destroyResourceTree("openejb/Resource", namingEnumeration); try { containerSystem.getJNDIContext().unbind("java:global"); @@ -1683,17 +1683,27 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A } } - private Collection<DestroyingResource> destroyResourceTree(final NamingEnumeration<Binding> namingEnumeration) { + //TODO: add Jndi name to DestroyingResource + private Collection<DestroyingResource> destroyResourceTree(final String name, NamingEnumeration<Binding> namingEnumeration) { final List<DestroyingResource> resources = new LinkedList<DestroyingResource>(); while (namingEnumeration != null && namingEnumeration.hasMoreElements()) { final Binding binding = namingEnumeration.nextElement(); + final String boundName = name + "/" + binding.getName(); final Object object = binding.getObject(); if (Context.class.isInstance(object)) { try { - resources.addAll(destroyResourceTree(Context.class.cast(object).listBindings(""))); - } catch (final Exception ignored) { - // no-op + NamingEnumeration<Binding> bindings = Context.class.cast(object).listBindings(""); + resources.addAll(destroyResourceTree(boundName, bindings)); + } catch (final NamingException e) { + logger.error("Error removing bindings from " + boundName, e); } + } else if (LazyResource.class.isInstance(object)) { + removeResourceInfo(boundName); + try { + containerSystem.getJNDIContext().unbind(boundName); + } catch (NamingException e) { + logger.error("Error unbinding " + boundName, e); + } } else { resources.add(new DestroyingResource(binding.getName(), binding.getClassName(), object)); } @@ -1792,7 +1802,7 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A final Iterator<ResourceInfo> iterator = configuration.facilities.resources.iterator(); while (iterator.hasNext()) { final ResourceInfo info = iterator.next(); - if (name.equals(info.id)) { + if (name.equals(OPENEJB_RESOURCE_JNDI_PREFIX + info.id)) { iterator.remove(); break; } @@ -2210,15 +2220,21 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A // if we catch a NamingException, check to see if the resource is a LaztObjectReference that has not been initialized correctly final String ctx = name.substring(0, name.lastIndexOf("/")); final String objName = name.substring(ctx.length() + 1); + final NamingEnumeration<Binding> bindings = globalContext.listBindings(ctx); while (bindings.hasMoreElements()) { final Binding binding = bindings.nextElement(); - if (!binding.getName().equals(objName)) continue; - if (!LazyObjectReference.class.isInstance(binding.getObject())) continue; + if (!binding.getName().equals(objName)) { + continue; + } + + if (!LazyObjectReference.class.isInstance(binding.getObject())) { + continue; + } final LazyObjectReference<?> ref = LazyObjectReference.class.cast(binding.getObject()); if (! ref.isInitialized()) { - globalContext.unbind(name); + globalContext.unbind(name); removeResourceInfo(name); return; } http://git-wip-us.apache.org/repos/asf/tomee/blob/4e9444c5/container/openejb-core/src/test/java/org/apache/openejb/resource/BadResourceTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/BadResourceTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/BadResourceTest.java new file mode 100644 index 0000000..04662a8 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/BadResourceTest.java @@ -0,0 +1,129 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.openejb.resource; + +import org.apache.openejb.jee.WebApp; +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.testing.Configuration; +import org.apache.openejb.testing.Module; +import org.apache.openejb.testing.SimpleLog; +import org.apache.openejb.testng.PropertiesBuilder; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.Collection; +import java.util.LinkedList; +import java.util.Properties; + +import javax.naming.Binding; +import javax.naming.Context; +import javax.naming.InitialContext; +import javax.naming.NameNotFoundException; +import javax.naming.NamingEnumeration; + +@SimpleLog +@RunWith(ApplicationComposer.class) +public class BadResourceTest { + + @Configuration + public Properties configuration() { + + return new PropertiesBuilder() + .p("Resource1", "new://Resource?class-name=org.apache.openejb.resource.BadResourceTest$MyFactory&factory-name=create") + .p("Resource1.name", "Resource1") + .p("Resource1.Lazy", "true") + .p("Resource2", "new://Resource?class-name=org.apache.openejb.resource.BadResourceTest$MyFactory&factory-name=create") + .p("Resource2.name", "Resource2") + .p("Resource2.Lazy", "true") + .build(); + } + + @Module + public WebApp webApp() { + return new WebApp(); + } + + private static final Collection<Runnable> POST_CONTAINER_VALIDATIONS = new LinkedList<Runnable>(); + + @AfterClass + public static void lastValidations() { // late to make the test failing (ie junit report will be broken) but better than destroying eagerly the resource + for (final Runnable runnable : POST_CONTAINER_VALIDATIONS) { + runnable.run(); + } + POST_CONTAINER_VALIDATIONS.clear(); + } + + @Test + public void ensureCleanup() { + POST_CONTAINER_VALIDATIONS.add(new Runnable() { + @Override + public void run() { + check("openejb/Resource/Resource1"); + check("openejb/Resource/Resource2"); + } + + public void check(final String jndiName) { + try { + InitialContext ctx = new InitialContext(); + ctx.lookup(jndiName); + Assert.fail("Resource not cleaned up"); + } catch (NameNotFoundException e) { + // ignore, we expect this exception + } catch (Throwable e) { + Assert.fail(e.getMessage()); + } + } + }); + } + + public static class MyResource { + private String name; + + public MyResource(String name) { + super(); + this.name = name; + } + + public MyResource() { + super(); + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + public static class MyFactory { + private Properties properties; + + public Object create() { + final String name = (String) properties.remove("name"); + + if (name.equals("Resource1")) { + throw new RuntimeException("Not creating this resource"); + } + + return new MyResource(name); + } + } +}
