cleaning up JMXBeanCreator and its Alternative to remove properly used
properties + logging in time unused properties + storing unset properties in
ResourceInfo since it can be super uselful for runtime investigation
Conflicts:
container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/Assembler.java
Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/78f29048
Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/78f29048
Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/78f29048
Branch: refs/heads/tomee-1.7.x
Commit: 78f29048f921cfae2bf8f4bba1c2654747ceb683
Parents: cd4de99
Author: Romain Manni-Bucau <rmannibu...@apache.org>
Authored: Fri Apr 17 20:06:16 2015 +0200
Committer: Jonathan Gallimore <j...@jrg.me.uk>
Committed: Mon Apr 20 13:31:07 2015 +0100
----------------------------------------------------------------------
.../openejb/assembler/classic/Assembler.java | 106 ++++++++++++++-----
.../openejb/assembler/classic/ResourceInfo.java | 2 +-
.../openejb/assembler/classic/ServiceInfo.java | 3 +-
.../resource/jmx/factory/JMXBeanCreator.java | 5 +-
.../resource/jmx/resources/Alternative.java | 11 +-
5 files changed, 91 insertions(+), 36 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tomee/blob/78f29048/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 66f1920..9e7e0a3 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
@@ -150,6 +150,7 @@ import org.apache.xbean.finder.ClassLoaders;
import org.apache.xbean.finder.ResourceFinder;
import org.apache.xbean.finder.UrlSet;
import org.apache.xbean.finder.archive.ClassesArchive;
+import org.apache.xbean.recipe.ConstructionException;
import org.apache.xbean.recipe.ObjectRecipe;
import org.apache.xbean.recipe.Option;
import org.apache.xbean.recipe.UnsetPropertiesRecipe;
@@ -192,15 +193,20 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InvalidObjectException;
+import java.io.ObjectStreamException;
+import java.io.Serializable;
import java.lang.instrument.ClassFileTransformer;
import java.lang.instrument.Instrumentation;
import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.lang.reflect.Type;
import java.net.MalformedURLException;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
@@ -218,6 +224,7 @@ import java.util.concurrent.Executor;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.ReentrantLock;
import static org.apache.openejb.util.Classes.ancestors;
@@ -968,6 +975,10 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
final List<ResourceInfo> resourceList =
config.facilities.resources;
for (final ResourceInfo resourceInfo : resourceList) {
+ if (isTemplatizedResource(resourceInfo)) {
+ continue;
+ }
+
try {
Class<?> clazz;
try {
@@ -975,6 +986,7 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
} catch (final ClassNotFoundException cnfe) { // custom
classpath
clazz =
containerSystemContext.lookup(OPENEJB_RESOURCE_JNDI_PREFIX +
resourceInfo.id).getClass();
}
+
final AnnotationFinder finder = new AnnotationFinder(new
ClassesArchive(ancestors(clazz)));
final List<Method> postConstructs =
finder.findAnnotatedMethods(PostConstruct.class);
final List<Method> preDestroys =
finder.findAnnotatedMethods(PreDestroy.class);
@@ -1000,19 +1012,21 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
}
}
- if (resourceInfo.postConstruct != null) {
- final Method p =
clazz.getDeclaredMethod(resourceInfo.postConstruct);
- if (!p.isAccessible()) {
- SetAccessible.on(p);
+ if (!"none".equals(resourceInfo.postConstruct)) {
+ if (resourceInfo.postConstruct != null) {
+ final Method p =
clazz.getDeclaredMethod(resourceInfo.postConstruct);
+ if (!p.isAccessible()) {
+ SetAccessible.on(p);
+ }
+ p.invoke(resource);
}
- p.invoke(resource);
- }
- for (final Method m : postConstructs) {
- if (!m.isAccessible()) {
- SetAccessible.on(m);
+ for (final Method m : postConstructs) {
+ if (!m.isAccessible()) {
+ SetAccessible.on(m);
+ }
+ m.invoke(resource);
}
- m.invoke(resource);
}
} catch (final Exception e) {
logger.fatal("Error calling @PostConstruct method
on " + resource.getClass().getName());
@@ -1020,20 +1034,30 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
}
}
- if (resourceInfo.preDestroy != null) {
- final Method p =
clazz.getDeclaredMethod(resourceInfo.preDestroy);
- if (!p.isAccessible()) {
- SetAccessible.on(p);
+ if (!"none".equals(resourceInfo.preDestroy)) {
+ if (resourceInfo.preDestroy != null) {
+ final Method p =
clazz.getDeclaredMethod(resourceInfo.preDestroy);
+ if (!p.isAccessible()) {
+ SetAccessible.on(p);
+ }
+ preDestroys.add(p);
+ }
+
+ if (!preDestroys.isEmpty() || creationalContext !=
null) {
+ final String name = OPENEJB_RESOURCE_JNDI_PREFIX +
resourceInfo.id;
+ if (originalResource == null) {
+ originalResource =
containerSystemContext.lookup(name);
+ }
+ this.bindResource(resourceInfo.id, new
ResourceInstance(name, originalResource, preDestroys, creationalContext), true);
}
- preDestroys.add(p);
}
- if (!preDestroys.isEmpty() || creationalContext != null) {
- final String name = OPENEJB_RESOURCE_JNDI_PREFIX +
resourceInfo.id;
- if (originalResource == null) {
- originalResource =
containerSystemContext.lookup(name);
+ // log unused now for these resources now we built the
resource completely and @PostConstruct can have used injected properties
+ if (resourceInfo.unsetProperties != null) {
+ final Set<String> unsetKeys =
resourceInfo.unsetProperties.stringPropertyNames();
+ for (final String key : unsetKeys) { // don't use
keySet to auto filter txMgr for instance and not real properties!
+ unusedProperty(resourceInfo.id, logger, key);
}
- this.bindResource(resourceInfo.id, new
ResourceInstance(name, originalResource, preDestroys, creationalContext), true);
}
} catch (final Exception e) {
logger.fatal("Error calling @PostConstruct method on " +
resourceInfo.id);
@@ -1045,7 +1069,9 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
}
}
-
+ private static boolean isTemplatizedResource(final ResourceInfo
resourceInfo) { // ~ container resource even if not 100% right
+ return resourceInfo.className == null ||
resourceInfo.className.isEmpty();
+ }
public static void mergeServices(final AppInfo appInfo) throws
URISyntaxException {
for (final ServiceInfo si : appInfo.services) { // used lazily by
JaxWsServiceObjectFactory, we could do the same for resources
if (!appInfo.properties.containsKey(si.id)) {
@@ -2463,7 +2489,25 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
}
serviceInfo.properties.remove("SkipImplicitAttributes");
- serviceRecipe.setProperty("properties", new UnsetPropertiesRecipe());
+ final AtomicReference<Properties> injectedProperties = new
AtomicReference<Properties>();
+ serviceRecipe.setProperty("properties", new UnsetPropertiesRecipe() {
+ @Override
+ protected Object internalCreate(final Type expectedType, final
boolean lazyRefAllowed) throws ConstructionException {
+ final Map<String, Object> original =
serviceRecipe.getUnsetProperties();
+ final Properties properties = new SuperProperties() {
+ @Override
+ public Object remove(final Object key) { // avoid to log
them then
+ original.remove(key);
+ return super.remove(key);
+ }
+ }.caseInsensitive(true); // keep our nice case insensitive
feature
+ for (final Map.Entry<String, Object> entry :
original.entrySet()) {
+ properties.put(entry.getKey(), entry.getValue());
+ }
+ injectedProperties.set(properties);
+ return properties;
+ }
+ });
final Properties props =
PropertyPlaceHolderHelper.holds(serviceInfo.properties);
if (serviceInfo.properties.containsKey("Definition")) {
@@ -2515,6 +2559,8 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
} // else proxy would be useless
}
+ serviceInfo.unsetProperties = injectedProperties.get();
+
// Java Connector spec ResourceAdapters and ManagedConnectionFactories
need special activation
if (service instanceof ResourceAdapter) {
final ResourceAdapter resourceAdapter = (ResourceAdapter) service;
@@ -2676,7 +2722,9 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
}
}
} else if (!Properties.class.isInstance(service)) {
- logUnusedProperties(serviceRecipe, serviceInfo);
+ if (serviceInfo.unsetProperties == null ||
isTemplatizedResource(serviceInfo)) {
+ logUnusedProperties(serviceRecipe, serviceInfo);
+ } // else wait post construct
}
return service;
}
@@ -2912,7 +2960,7 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
logUnusedProperties(unsetProperties, info);
}
- private static void logUnusedProperties(final Map<String, Object>
unsetProperties, final ServiceInfo info) {
+ private static void logUnusedProperties(final Map<String, ?>
unsetProperties, final ServiceInfo info) {
Logger logger = null;
for (final String property : unsetProperties.keySet()) {
//TODO: DMB: Make more robust later
@@ -2956,10 +3004,18 @@ public class Assembler extends AssemblerTool implements
org.apache.openejb.spi.A
if (logger == null) {
logger =
SystemInstance.get().getComponent(Assembler.class).logger;
}
- logger.getChildLogger("service").warning("unusedProperty",
property, info.id);
+ unusedProperty(info.id, logger, property);
}
}
+ private static void unusedProperty(final String id, final Logger
parentLogger, final String property) {
+ parentLogger.getChildLogger("service").warning("unusedProperty",
property, id);
+ }
+
+ private static void unusedProperty(final String id, final String property)
{
+ unusedProperty(id,
SystemInstance.get().getComponent(Assembler.class).logger, property);
+ }
+
public static ObjectRecipe prepareRecipe(final ServiceInfo info) {
final String[] constructorArgs = info.constructorArgs.toArray(new
String[info.constructorArgs.size()]);
final ObjectRecipe serviceRecipe = new ObjectRecipe(info.className,
info.factoryMethod, constructorArgs, null);
http://git-wip-us.apache.org/repos/asf/tomee/blob/78f29048/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ResourceInfo.java
----------------------------------------------------------------------
diff --git
a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ResourceInfo.java
b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ResourceInfo.java
index c24ae24..d7ed93d 100644
---
a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ResourceInfo.java
+++
b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ResourceInfo.java
@@ -25,5 +25,5 @@ public class ResourceInfo extends ServiceInfo {
public String postConstruct;
public String preDestroy;
public String originAppName; // if define by an app
- public List<String> aliases = new ArrayList<String>();
+ public List<String> aliases = new ArrayList<>();
}
http://git-wip-us.apache.org/repos/asf/tomee/blob/78f29048/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ServiceInfo.java
----------------------------------------------------------------------
diff --git
a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ServiceInfo.java
b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ServiceInfo.java
index 86d7ada..bd55d12 100644
---
a/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ServiceInfo.java
+++
b/container/openejb-core/src/main/java/org/apache/openejb/assembler/classic/ServiceInfo.java
@@ -33,7 +33,8 @@ public class ServiceInfo extends InfoObject {
public String codebase;
public URI[] classpath;
public Properties properties;
- public final List<String> constructorArgs = new ArrayList<String>();
+ public final List<String> constructorArgs = new ArrayList<>();
+ public Properties unsetProperties; // keep it in the model to be able to
investigate it dumping Infos
/**
* Optional *
http://git-wip-us.apache.org/repos/asf/tomee/blob/78f29048/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/factory/JMXBeanCreator.java
----------------------------------------------------------------------
diff --git
a/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/factory/JMXBeanCreator.java
b/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/factory/JMXBeanCreator.java
index 14a21a6..2571fc4 100644
---
a/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/factory/JMXBeanCreator.java
+++
b/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/factory/JMXBeanCreator.java
@@ -65,9 +65,8 @@ public class JMXBeanCreator {
final T instance = (T) cls.newInstance();
final StandardMBean mBean = new StandardMBean(instance, ifaceCls);
- for (Object property : properties.keySet()) {
- String attributeName = (String) property;
- final Object value = properties.getProperty(attributeName);
+ for (String attributeName : properties.stringPropertyNames()) {
+ final Object value = properties.remove(attributeName);
if (prefix != null) {
if (! attributeName.startsWith(prefix + ".")) {
http://git-wip-us.apache.org/repos/asf/tomee/blob/78f29048/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/resources/Alternative.java
----------------------------------------------------------------------
diff --git
a/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/resources/Alternative.java
b/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/resources/Alternative.java
index 20799da..00adecc 100644
---
a/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/resources/Alternative.java
+++
b/examples/resources-jmx-example/resources-jmx-ejb/src/main/java/org/superbiz/resource/jmx/resources/Alternative.java
@@ -80,9 +80,9 @@ public class Alternative implements AlternativeMBean {
@PostConstruct
public <T> void postConstruct() throws MBeanRegistrationException {
- final String name = properties.getProperty("name");
- final String iface = properties.getProperty("interface");
- final String prefix = properties.getProperty("prefix");
+ final String name = properties.remove("name").toString();
+ final String iface = properties.remove("interface").toString();
+ final String prefix = properties.remove("prefix").toString();
requireNotNull(name);
requireNotNull(iface);
@@ -91,9 +91,8 @@ public class Alternative implements AlternativeMBean {
final Class<T> ifaceCls = (Class<T>) Class.forName(iface, true,
Thread.currentThread().getContextClassLoader());
final StandardMBean mBean = new StandardMBean((T) this, ifaceCls);
- for (Object property : properties.keySet()) {
- String attributeName = (String) property;
- final Object value = properties.getProperty(attributeName);
+ for (String attributeName : properties.stringPropertyNames()) {
+ final Object value = properties.remove(attributeName);
if (prefix != null) {
if (! attributeName.startsWith(prefix + ".")) {
