modules/enterprise/binding/src/main/java/org/rhq/bindings/client/ResourceClientFactory.java | 33 ++ modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ClassPoolFactory.java | 66 ++++ modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ConfigurationClassBuilder.java | 6 modules/enterprise/binding/src/main/java/org/rhq/bindings/util/InterfaceSimplifier.java | 11 modules/enterprise/server/client-api/src/main/java/org/rhq/enterprise/client/LocalClient.java | 3 modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/client/security/test/ScriptingAPIServerTest.java | 133 ++++++++++ 6 files changed, 237 insertions(+), 15 deletions(-)

New commits: commit fe8952cf9631c2d839f9e37d53f8c1aa1bb04b83 Author: Lukas Krejci lkrejci@redhat.com Date: Fri Mar 29 16:06:45 2013 +0100

Fix classloading issues when defining objects in the std script context.

It seems that the change of the container to AS7 has caused a regression in the CLI alert scripts that were no longer able to properly define the standard context provided to scripts by RHQ.

This is fixed by using a differently configured Javassist's ClassPool during the various class modifications we use to prepare the *Manager objects and when generating a resource proxy in ProxyFactory.

Also, the errors that can happen during the various stages of this setup are now thrown up the chain instead of merely logged so that errors like this are apparent - it is not something the user should be expected to deal with.

diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/client/ResourceClientFactory.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/client/ResourceClientFactory.java index 3bf4729..2e6ce5c 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/client/ResourceClientFactory.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/client/ResourceClientFactory.java @@ -2,6 +2,9 @@ package org.rhq.bindings.client;

import java.io.PrintWriter; import java.lang.reflect.InvocationTargetException; +import java.security.AccessController; +import java.security.PrivilegedAction; +import java.security.PrivilegedExceptionAction; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -26,6 +29,7 @@ import javax.jws.WebParam; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory;

+import org.rhq.bindings.util.ClassPoolFactory; import org.rhq.bindings.util.ConfigurationClassBuilder; import org.rhq.bindings.util.ResourceTypeFingerprint; import org.rhq.core.domain.resource.ResourceCreationDataType; @@ -109,12 +113,16 @@ public class ResourceClientFactory { instantiateMethodHandler(proxy, interfaces, rhqFacade)); } catch (InstantiationException e) { LOG.error("Could not instantiate a ResourceClientProxy, this is a bug.", e); + throw new IllegalStateException("Could not instantiate a ResourceClientProxy, this is a bug.", e); } catch (IllegalAccessException e) { LOG.error("Could not instantiate a ResourceClientProxy, this is a bug.", e); + throw new IllegalStateException("Could not instantiate a ResourceClientProxy, this is a bug.", e); } catch (NoSuchMethodException e) { LOG.error("Could not instantiate a ResourceClientProxy, this is a bug.", e); + throw new IllegalStateException("Could not instantiate a ResourceClientProxy, this is a bug.", e); } catch (InvocationTargetException e) { LOG.error("Could not instantiate a ResourceClientProxy, this is a bug.", e); + throw new IllegalStateException("Could not instantiate a ResourceClientProxy, this is a bug.", e); } return proxied; } @@ -145,7 +153,7 @@ public class ResourceClientFactory { private Class<?> defineCustomInterface(ResourceClientProxy proxy) { try { // define the dynamic class - do not put it in any known rhq package in case our jars are signed (see BZ-794503) - ClassPool pool = ClassPool.getDefault(); + ClassPool pool = ClassPoolFactory.get(); CtClass customClass = pool.makeInterface("org.rhq.bindings.client.dynamic." + ResourceClientProxy.class.getSimpleName() + proxy.fingerprint);

@@ -202,19 +210,34 @@ public class ResourceClientFactory {

ClassLoader cl = Thread.currentThread().getContextClassLoader(); try { - Thread.currentThread().setContextClassLoader(ResourceClientProxy.class.getClassLoader()); + setContextClassLoader(ResourceClientProxy.class.getClassLoader()); return customClass.toClass(); } finally { - Thread.currentThread().setContextClassLoader(cl); + setContextClassLoader(cl); } } catch (NotFoundException e) { LOG.error("Could not create custom interface for resource with id " + proxy.getId(), e); + throw new IllegalStateException("Could not create custom interface for resource with id " + proxy.getId(), e); } catch (CannotCompileException e) { LOG.error("Could not create custom interface for resource with id " + proxy.getId(), e); + throw new IllegalStateException("Could not create custom interface for resource with id " + proxy.getId(), e); } catch (Exception e) { LOG.error("Could not create custom interface for resource with id " + proxy.getId(), e); + throw new IllegalStateException("Could not create custom interface for resource with id " + proxy.getId(), e); + } + } + + private void setContextClassLoader(final ClassLoader cl) { + if (System.getSecurityManager() != null) { + AccessController.doPrivileged(new PrivilegedAction<Void>() { + @Override + public Void run() { + Thread.currentThread().setContextClassLoader(cl); + return null; + } + }); + } else { + Thread.currentThread().setContextClassLoader(cl); } - - return null; } } diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ClassPoolFactory.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ClassPoolFactory.java new file mode 100644 index 0000000..50673e8 --- /dev/null +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ClassPoolFactory.java @@ -0,0 +1,66 @@ +/* + * RHQ Management Platform + * Copyright (C) 2013 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation, and/or the GNU Lesser + * General Public License, version 2.1, also as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License and the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * and the GNU Lesser General Public License along with this program; + * if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.rhq.bindings.util; + +import javassist.ClassClassPath; +import javassist.ClassPool; +import javassist.LoaderClassPath; + +/** + * This class is used to create Javassist's classpools usable in RHQ on both client and server side. + * This only exists to centralize the initialization code for the pools. + * <p> + * This class is similar to how the <code>ClassPool.getDefault()</code> method operates (in that it only ever returns + * a single instance of the pool) but uses a "wider" class path, looking for classes using the context class loader + * and the using the default resource lookup of the <code>Class</code> class, in addition to just the system class path + * as detected by Javassist (which is the only one used in the class pool returned by + * <code>ClassPool.getDefault()</code>). + * <p> + * This is to ensure that Javassist can locate the classes in various classloading "schemes" - the traditional + * application classloader used in the CLI client and the JBoss Modules classloading used on the server. + * + * @author Lukas Krejci + */ +public class ClassPoolFactory { + + private static ClassPool pool; + + private ClassPoolFactory() { + + } + + /** + * @return the singleton class pool instance initialized according the {@link ClassPoolFactory rules}. + */ + public static synchronized ClassPool get() { + if (pool == null) { + pool = new ClassPool(null); + pool.appendClassPath(new LoaderClassPath(Thread.currentThread().getContextClassLoader())); + pool.appendClassPath(new ClassClassPath(ClassPoolFactory.class)); + pool.appendSystemPath(); + } + + return pool; + } +} diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ConfigurationClassBuilder.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ConfigurationClassBuilder.java index b739581..fd72443 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ConfigurationClassBuilder.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/ConfigurationClassBuilder.java @@ -84,7 +84,7 @@ public class ConfigurationClassBuilder { paramType = Double.TYPE; break; } - return ClassPool.getDefault().get(paramType.getName()); + return ClassPoolFactory.get().get(paramType.getName()); }

public static CtClass translateConfiguration(ConfigurationDefinition def) throws NotFoundException { @@ -96,7 +96,7 @@ public class ConfigurationClassBuilder { } else {

// TODO GH: Build a custom type? - return ClassPool.getDefault().get(Configuration.class.getName()); + return ClassPoolFactory.get().get(Configuration.class.getName()); } }

@@ -127,7 +127,7 @@ public class ConfigurationClassBuilder {

CtClass expectedReturn = translateConfiguration(resultsConfigurationDefinition);

- if (expectedReturn.equals(ClassPool.getDefault().get(Configuration.class.getName()))) { + if (expectedReturn.equals(ClassPoolFactory.get().get(Configuration.class.getName()))) { return result; } else { //bail on translation if Configuration passed in is null diff --git a/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/InterfaceSimplifier.java b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/InterfaceSimplifier.java index 8ce7bc6..1e13179 100644 --- a/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/InterfaceSimplifier.java +++ b/modules/enterprise/binding/src/main/java/org/rhq/bindings/util/InterfaceSimplifier.java @@ -143,7 +143,7 @@ public class InterfaceSimplifier {

public static Class<?> simplify(Class<?> intf) { try { - ClassPool classPool = ClassPool.getDefault(); + ClassPool classPool = ClassPoolFactory.get();

String simplifiedName = getSimplifiedName(intf); LOG.debug("Simplifying " + intf + " (simplified interface name: " + simplifiedName + ")..."); @@ -275,12 +275,11 @@ public class InterfaceSimplifier {

return newClass.toClass();

- } catch (NotFoundException e) { - LOG.debug("Failed to simplify " + intf + " - cause: " + e); - } catch (CannotCompileException e) { - LOG.error("Failed to simplify " + intf + ".", e); + } catch (Exception e) { + String msg = "Failed to simplify " + intf + "."; + LOG.error(msg, e); + throw new IllegalStateException(msg, e); } - return intf; }

private static String getSimplifiedName(Class<?> interfaceClass) { diff --git a/modules/enterprise/server/client-api/src/main/java/org/rhq/enterprise/client/LocalClient.java b/modules/enterprise/server/client-api/src/main/java/org/rhq/enterprise/client/LocalClient.java index a918389..df0eab4 100644 --- a/modules/enterprise/server/client-api/src/main/java/org/rhq/enterprise/client/LocalClient.java +++ b/modules/enterprise/server/client-api/src/main/java/org/rhq/enterprise/client/LocalClient.java @@ -87,7 +87,8 @@ public class LocalClient implements RhqFacade {

managers.put(manager, proxy); } catch (Throwable e) { - LOG.error("Failed to load manager " + manager + " due to missing class.", e); + LOG.error("Failed to load manager " + manager + ".", e); + throw new IllegalStateException("Failed to load manager " + manager + ".", e); } } } diff --git a/modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/client/security/test/ScriptingAPIServerTest.java b/modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/client/security/test/ScriptingAPIServerTest.java new file mode 100644 index 0000000..0195603 --- /dev/null +++ b/modules/enterprise/server/itests-2/src/test/java/org/rhq/enterprise/client/security/test/ScriptingAPIServerTest.java @@ -0,0 +1,133 @@ +/* + * RHQ Management Platform + * Copyright (C) 2013 Red Hat, Inc. + * All rights reserved. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License, version 2, as + * published by the Free Software Foundation, and/or the GNU Lesser + * General Public License, version 2.1, also as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License and the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * and the GNU Lesser General Public License along with this program; + * if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.rhq.enterprise.client.security.test; + +import java.util.UUID; + +import javax.script.ScriptEngine; + +import org.testng.annotations.Test; + +import org.rhq.bindings.client.RhqManager; +import org.rhq.bindings.util.SimplifiedClass; +import org.rhq.core.domain.auth.Subject; +import org.rhq.core.domain.configuration.definition.ConfigurationDefinition; +import org.rhq.core.domain.configuration.definition.PropertyDefinitionSimple; +import org.rhq.core.domain.configuration.definition.PropertySimpleType; +import org.rhq.core.domain.measurement.DataType; +import org.rhq.core.domain.measurement.DisplayType; +import org.rhq.core.domain.measurement.MeasurementCategory; +import org.rhq.core.domain.measurement.MeasurementDefinition; +import org.rhq.core.domain.measurement.MeasurementUnits; +import org.rhq.core.domain.operation.OperationDefinition; +import org.rhq.core.domain.resource.Resource; +import org.rhq.core.domain.resource.ResourceCategory; +import org.rhq.core.domain.resource.ResourceType; +import org.rhq.enterprise.client.ScriptableAbstractEJB3Test; +import org.rhq.enterprise.server.test.TransactionCallback; +import org.rhq.enterprise.server.util.LookupUtil; + +import static org.testng.Assert.assertNotEquals; + +/** + * @author Lukas Krejci + */ +public class ScriptingAPIServerTest extends ScriptableAbstractEJB3Test { + + @Test + public void testProxyFactoryCorrectlyDefinesMethodsAndProperties() throws Exception { + executeInTransaction(new TransactionCallback() { + @Override + public void execute() throws Exception { + ResourceType rt = createResourceType(); + Resource r = createResource(rt); + + Subject overlord = LookupUtil.getSubjectManager().getOverlord(); + ScriptEngine engine = getEngine(overlord); + + engine.eval("var r = ProxyFactory.getResource(" + r.getId() + ");"); + + String detectedMeasurementType = (String) engine.eval("typeof(r.measurement);"); + String detectedOperationType = (String) engine.eval("typeof(r.operation);"); + + assertEquals("object", detectedMeasurementType); + assertEquals("function", detectedOperationType); + } + }); + } + + @Test + public void testManagersHaveSimplifiedSignatures() throws Exception { + Subject overlord = LookupUtil.getSubjectManager().getOverlord(); + ScriptEngine engine = getEngine(overlord); + + //we know that interface simplification worked if the full class name is different from the original + //Here, we want to test the simplification worked within the classloading environment of the RHQ server. + //Simplification itself is unit tested separately. + + for (RhqManager m : RhqManager.values()) { + String name = m.name(); + Object scriptedManager = engine.eval(name); + assertNotNull(scriptedManager); + + Class<?> implementedIface = scriptedManager.getClass().getInterfaces()[0]; + + assertNotEquals(m.remote(), implementedIface); + assertNotNull(implementedIface.getAnnotation(SimplifiedClass.class)); + } + } + + private ResourceType createResourceType() { + ResourceType resourceType = new ResourceType("ScriptingAPITestType", "dummy", ResourceCategory.PLATFORM, null); + + MeasurementDefinition measurement = new MeasurementDefinition("measurement", MeasurementCategory.PERFORMANCE, + MeasurementUnits.BYTES, + DataType.MEASUREMENT, true, 1, DisplayType.DETAIL); + measurement.setDisplayName("measurement"); + + resourceType.addMetricDefinition(measurement); + + ConfigurationDefinition params = new ConfigurationDefinition("dummy", null); + params.put(new PropertyDefinitionSimple("parameter", null, true, PropertySimpleType.BOOLEAN)); + + OperationDefinition operation = new OperationDefinition(resourceType, "operation"); + operation.setDisplayName("operation"); + operation.setParametersConfigurationDefinition(params); + + resourceType.addOperationDefinition(operation); + + getEntityManager().persist(resourceType); + + return resourceType; + } + + private Resource createResource(ResourceType resourceType) { + Resource resource = new Resource("key", "resource", resourceType); + resource.setUuid(UUID.randomUUID().toString()); + + getEntityManager().persist(resource); + + return resource; + } +}