You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by rm...@apache.org on 2015/04/17 20:06:32 UTC

tomee git commit: 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

Repository: tomee
Updated Branches:
  refs/heads/master 024ce1a9c -> 62bc5d06e


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


Project: http://git-wip-us.apache.org/repos/asf/tomee/repo
Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/62bc5d06
Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/62bc5d06
Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/62bc5d06

Branch: refs/heads/master
Commit: 62bc5d06ea727224e436a7c284ca09e492b262ff
Parents: 024ce1a
Author: Romain Manni-Bucau <rm...@apache.org>
Authored: Fri Apr 17 20:06:16 2015 +0200
Committer: Romain Manni-Bucau <rm...@apache.org>
Committed: Fri Apr 17 20:06:16 2015 +0200

----------------------------------------------------------------------
 .../openejb/assembler/classic/Assembler.java    | 171 ++++++++++++-------
 .../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, 122 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tomee/blob/62bc5d06/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 2750fa8..77d7fd6 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
@@ -157,14 +157,50 @@ 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;
 
+import java.io.ByteArrayInputStream;
+import java.io.Externalizable;
+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;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Properties;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.Callable;
+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 javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.annotation.Resource;
@@ -203,40 +239,6 @@ import javax.validation.ValidationException;
 import javax.validation.Validator;
 import javax.validation.ValidatorFactory;
 
-import java.io.ByteArrayInputStream;
-import java.io.Externalizable;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InvalidObjectException;
-import java.lang.instrument.ClassFileTransformer;
-import java.lang.instrument.Instrumentation;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Proxy;
-import java.net.MalformedURLException;
-import java.net.URISyntaxException;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.Properties;
-import java.util.Set;
-import java.util.TreeMap;
-import java.util.concurrent.Callable;
-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.locks.ReentrantLock;
-
 import static org.apache.openejb.util.Classes.ancestors;
 
 @SuppressWarnings({"UnusedDeclaration", "UnqualifiedFieldAccess", "UnqualifiedMethodAccess"})
@@ -1114,6 +1116,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 {
@@ -1121,6 +1127,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);
@@ -1146,19 +1153,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());
@@ -1166,20 +1175,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);
@@ -1191,6 +1210,10 @@ 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 {
         // used lazily by JaxWsServiceObjectFactory so merge both to keep same config
         // note: we could do the same for resources
@@ -2622,7 +2645,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<>();
+        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")) {
@@ -2667,13 +2708,15 @@ public class Assembler extends AssemblerTool implements org.apache.openejb.spi.A
 
         Object service = serviceRecipe.create(loader);
         if (customLoader) {
-            final Collection<Class<?>> apis = new ArrayList<Class<?>>(Arrays.asList(service.getClass().getInterfaces()));
+            final Collection<Class<?>> apis = new ArrayList<>(Arrays.asList(service.getClass().getInterfaces()));
 
             if (apis.size() - (apis.contains(Serializable.class) ? 1 : 0) - (apis.contains(Externalizable.class) ? 1 : 0) > 0) {
                 service = Proxy.newProxyInstance(loader, apis.toArray(new Class<?>[apis.size()]), new ClassLoaderAwareHandler(null, service, loader));
             } // 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;
@@ -2835,7 +2878,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;
     }
@@ -3071,7 +3116,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
@@ -3118,10 +3163,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/62bc5d06/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/62bc5d06/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/62bc5d06/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/62bc5d06/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 + ".")) {