Bug 723025 - Operations with parameters only work for DynamicMBeans
Operations with parameters only work for DynamicMBeans
Status: CLOSED CURRENTRELEASE
Product: RHQ Project
Classification: Other
Component: Plugins (Show other bugs)
4.1
Unspecified Unspecified
urgent Severity high (vote)
: ---
: ---
Assigned To: Martin Vecera
Mike Foley
:
Depends On:
Blocks: jon3
  Show dependency treegraph
 
Reported: 2011-07-18 15:10 EDT by tcunning
Modified: 2013-09-03 13:02 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-09-03 13:02:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description tcunning 2011-07-18 15:10:19 EDT
Description of problem:
In MBeanResourceComponent.invokeOperation, the logic which populates the parameter values for operations relies on MBeanOperationalInfo gathered from EMS :

        EmsOperation operation = emsBean.getOperation(name);
        ....
        List<EmsParameter> params = operation.getParameters();
        ....
        PropertySimple ps = parameters.getSimple(param.getName());
        ....
        parameterValues[i] = (ps == null) ? null : ps.getStringValue();

This is great for Dynamic MBeans, but Standard MBeans have no way of generating the MBeanOperationalInfo or the descriptions needed to match the parameters up so that a parameter value can be used in the invocation.    It looks like the result here is that any time you invoke an operation with a parameter upon a standard mbean, you are going to invoke it with null parameters.

What could be done is insert some checks to see if the description is there or not, and if it is not, use the information in the Configuration (parameters) to populate the parameter values.

  
Version-Release number of selected component (if applicable):
I used JON 2.4.1 to see this, from viewing the RHQ code I think it's still present.

How reproducible:
Always


Steps to Reproduce:
I reproduced this using Drools Plugin, however it would be very easy to reproduce this by creating a standard mbean with an operation that takes a string and prints it out and a rhq-plugin.xml which discovers it.
    The Drools plugin is here:

https://github.com/droolsjbpm/droolsjbpm-integration/blob/master/drools-rhq-plugin/src/main/resources/META-INF/rhq-plugin.xml

    You can use this Drools application as a test application (remember to enable JMX remote connectivity, as local connectivity does not seem to work in RHQ):

https://github.com/droolsjbpm/droolsjbpm-integration/blob/master/droolsjbpm-integration-examples/src/main/java/org/drools/examples/broker/BrokerExample.java

    If you use the above, you need to check out the whole project, not just that java file.
Comment 1 Charles Crouch 2011-07-18 16:02:21 EDT
(2:49:52 PM) mazz: I thought JMX can still give you MBeanOperationInfo, even for non-dynamicMBeans?
(2:50:02 PM) mazz: not sure if EMS is broke or what.
(2:50:13 PM) mazz: this is probably a couple days worth of work
Comment 2 Charles Crouch 2011-07-18 16:22:22 EDT
(3:19:57 PM) tcunning1: ccrouch, mazz : i'm not sure you can get it from standard mbeans, i was going off of this - it had a response from heiko, so i figured it was kosher (http://stackoverflow.com/questions/5258603/description-for-standard-mbean)
Comment 3 Ian Springer 2011-08-01 15:14:29 EDT
The StandardMBean class does provide optional hooks for specifying names and descriptions for parameters. See the getParameterName and getDescription methods:

http://download.oracle.com/javase/6/docs/api/javax/management/StandardMBean.html

Tom, I'm not sure how you're creating and registering your mbeans. Is subclassing StandardMBean so you can override getParameterName and getDescription an option?

In any case, I agree the jmx plugin should be flexible enough to support un-named parameters. I'll look into that now.
Comment 4 Ian Springer 2011-08-01 17:23:14 EDT
[master e4fa39a] adds support for operationinfos with un-named parameterinfos. To define such an operation in an RHQ plugin descriptor, use the following syntax:

<operation name="doSomething" displayName="Do Something"
           description="blah blah blah">
   <parameters>
        <c:simple-property name="[0]" displayName="Foo" required="true"/>
        <c:simple-property name="[1]" displayName="Wah" required="false"/>
   </parameters>
   <results>
        <c:list-property name="Results">
            <c:simple-property name="result"/>
        </c:list-property>
   </results>
</operation>

Note, the names of the parameter properties are the MBean parameter indexes, rather than MBean parameter names. The index must be enclosed in square brackets, which is what the jmx plugin uses to differentiate an index from a name.

Here's the updated code from MBeanResourceComponent:

    public OperationResult invokeOperation(String name, Configuration parameters, EmsBean emsBean) throws Exception {
        if (emsBean == null) {
            throw new Exception("Can not invoke operation [" + name
                + "], as we can't connect to the MBean - is it down?");
        }

        Map<String, PropertySimple> paramProps = parameters.getSimpleProperties();
        SortedSet<EmsOperation> emsOperations = emsBean.getOperations();
        EmsOperation operation = null;
        // There could be multiple operations with the same name but different parameters. Try to find one that has
        // the same # of parameters as the RHQ operation def.
        for (EmsOperation emsOperation : emsOperations) {
            if (emsOperation.getName().equals(name) && (emsOperation.getParameters().size() == paramProps.size())) {
                operation = emsOperation;
                break;
            }
        }
        if (operation == null) {
            // We couldn't find an operation with the expected name and # of parameters, so as as a last ditch effort,
            // see if there's an operation that at least has the expected name.
            operation = emsBean.getOperation(name);
        }
        if (operation == null) {
            throw new Exception("Operation [" + name + "] not found on MBean [" + emsBean.getBeanName() + "].");
        }

        List<EmsParameter> emsParams = operation.getParameters();
        Map<String, Integer> emsParamIndexesByName = new HashMap<String, Integer>();
        for (int i = 0, emsParamsSize = emsParams.size(); i < emsParamsSize; i++) {
            EmsParameter emsParam = emsParams.get(i);
            if (emsParam.getName() != null) {
                emsParamIndexesByName.put(emsParam.getName(), i);
            }
        }

        Object[] paramValues = new Object[operation.getParameters().size()];

        for (String propName : paramProps.keySet()) {
            Integer paramIndex;
            if (propName.matches("\\[\\d+\\]")) {
                paramIndex = Integer.valueOf(propName.substring(propName.indexOf('[') + 1, propName.indexOf(']')));
                if (paramIndex < 0 || paramIndex >= emsParams.size()) {
                    throw new IllegalStateException("Index [" + paramIndex + "] specified for parameter of operation ["
                            + name + "] on MBean [" + emsBean.getBeanName() + "] is invalid. The MBean operation takes "
                            + emsParams.size() + " parameters.");
                }
            } else {
                paramIndex = emsParamIndexesByName.get(propName);
                if (paramIndex == null) {
                    throw new IllegalStateException("Name [" + propName + "] specified for parameter of operation ["
                            + name + "] on MBean [" + emsBean.getBeanName()
                            + "] is invalid. The MBean operation does not take a parameter by that name.");
                }
            }
            EmsParameter emsParam = emsParams.get(paramIndex);

            PropertySimple paramProp = paramProps.get(propName);
            String emsParamType = emsParam.getType();
            Object paramValue = getPropertyValueAsType(paramProp, emsParamType);
            paramValues[paramIndex] = paramValue;
        }

        Object resultObject = operation.invoke(paramValues);

        boolean hasVoidReturnType = (operation.getReturnType() == null
            || Void.class.getName().equals(operation.getReturnType()) || void.class.getName().equals(
            operation.getReturnType()));

        OperationResult resultToReturn;
        if (resultObject == null && hasVoidReturnType) {
            resultToReturn = null;
        } else {
            // if the returned object is an array, put the elements in a list so we can stringify it later
            if (resultObject != null && resultObject.getClass().isArray()) {
                int len = Array.getLength(resultObject);
                ArrayList<Object> list = new ArrayList<Object>(len);
                for (int index = 0; index < len; index++) {
                    list.add(Array.get(resultObject, index));
                }
                resultObject = list;
            }

            // put the results object in an operation result if it isn't already one
            if (resultObject instanceof OperationResult) {
                resultToReturn = (OperationResult) resultObject;
            } else {
                resultToReturn = new OperationResult(String.valueOf(resultObject));
            }
        }

        return resultToReturn;
    }

    private Object getPropertyValueAsType(PropertySimple propSimple, String typeName) {
        Object value;
        if (typeName.equals(String.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getStringValue();
        } else if (typeName.equals(Boolean.class.getName()) || typeName.equals(boolean.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getBooleanValue();
        } else if (typeName.equals(Integer.class.getName()) || typeName.equals(int.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getIntegerValue();
        } else if (typeName.equals(Long.class.getName()) || typeName.equals(long.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getLongValue();
        } else if (typeName.equals(Float.class.getName()) || typeName.equals(float.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getFloatValue();
        } else if (typeName.equals(Double.class.getName()) || typeName.equals(double.class.getName())) {
            value = (propSimple == null) ? null : propSimple.getDoubleValue();
        } else {
            throw new IllegalStateException("Operation parameter maps to MBean parameter with an unsupported type (" + typeName + ").");
        }
        // TODO GH: Handle rest of types. (I think i have a mapper for this in mc4j
        return value;
    }


Tom, please let me know how this works for you.
Comment 5 Mike Foley 2011-08-10 15:12:48 EDT
verified RHQ 4.1 Beta, as follows:  successfully executed an operation with a parameter, specifically, I executed an operation on the RHQ Agent resource ... sending the parameter "avail" to the agent, and observed the successful completion of this operation.
Comment 6 Ian Springer 2011-08-10 16:30:19 EDT
Mike, we need Tom to verify this with his app, which includes some standard MBeans.

Tom, when you have a chance, can you try this out and see if it does the trick? You should be able to either use a build of RHQ master branch from Hudson (https://hudson.qa.jboss.com/hudson/view/RHQ/job/rhq-master-gwt-locales/lastSuccessfulBuild/) or RHQ 4.1 Beta, which should get released Thurs, 8/11.
Comment 7 Mike Foley 2011-08-10 17:01:23 EDT
Tom ... RHQ Beta can be found here:  https://hudson.qa.jboss.com/hudson/view/RHQ/job/rhq-publish-release/
Comment 8 Mike Foley 2011-09-07 09:16:19 EDT
assigning to tcunning@redhat.com.  please verify.  if you cannot verify now and the issue is no longer important, please adjust the priority and severity.
Comment 10 tcunning 2011-09-09 16:06:13 EDT
It doesn't look fixed to me.    I tried scheduling the operation "getStatsForRule" with a ruleName of "Portfolio action no longer needed".    Worked fine in JConsole.     Tried it in the build of rhq that was specified (rhq-publish-release_N67_Wed Sep 07 15-13-00 EDT 2011.zip) and here is what I'm seeing : 

java.lang.IllegalStateException: Name [ruleName] specified for parameter of operation [getStatsForRule] on MBean [org.drools.kbases:type=Stock Broker,group=Sessions,sessionId=Session-0] is invalid. The MBean operation does not take a parameter by that name.
	at org.rhq.plugins.jmx.MBeanResourceComponent.invokeOperation(MBeanResourceComponent.java:570)
	at org.rhq.plugins.jmx.MBeanResourceComponent.invokeOperation(MBeanResourceComponent.java:518)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at org.rhq.core.pc.inventory.ResourceContainer$ComponentInvocationThread.call(ResourceContainer.java:537)
	at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:303)
	at java.util.concurrent.FutureTask.run(FutureTask.java:138)
	at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
	at java.lang.Thread.run(Thread.java:619)
Comment 11 Ian Springer 2011-09-11 22:31:52 EDT
Did you change the syntax of the value of the operation parameter "name" attribute in your plugin descriptor as I described in comment 4? i.e.:

<c:simple-property name="[0]" ...

rather than:

<c:simple-property name="ruleName" ...

?
Comment 12 Charles Crouch 2011-10-07 16:10:17 EDT
Assigning to Pavel, via Len, to verify if this is still an issue with the latest version of the plugins and JON3
Comment 14 Heiko W. Rupp 2013-09-03 13:02:07 EDT
Bulk closing of old issues that are in VERIFIED state.

Note You need to log in before you can comment on or make changes to this bug.