ServiceControllerContext.target is null via @JMX

[alesj]

While figuring out what's the problem with ServiceSupportMBean,
I saw that ServiceControllerContext's target is null
if mbean is registered via @JMX on plain MC pojo.

So things like this don't get what they expect:

<deployment xmlns="urn:jboss:bean-deployer:2.0">

 <bean name="XMLLoginConfig" class="org.jboss.demos.models.old.XMLLoginConfig"/>

 <bean name="SecurityConfig" class="org.jboss.demos.models.old.SecurityConfig">
 <property name="defaultLoginConfig"><inject bean="XMLLoginConfig"/></property>
 </bean>

 <bean name="SecurityChecker" class="org.jboss.demos.models.old.Checker">
 <property name="loginConfig"><inject bean="jboss.security:service=XMLLoginConfig"/></property>
 <property name="securityConfig"><inject bean="jboss.security:service=SecurityConfig"/></property>
 </bean>

</deployment>

Where XMLLoginConfig has

@JMX(name="jboss.security:service=XMLLoginConfig", exposedInterface=XMLLoginConfigMBean.class)
public class XMLLoginConfig extends ServiceMBeanSupport implements XMLLoginConfigMBean

and SecurityConfig

@JMX(name = "jboss.security:service=SecurityConfig", exposedInterface = SecurityConfigMBean.class)
public class SecurityConfig extends ServiceMBeanSupport implements SecurityConfigMBean

And then SecurityChecker was getting nulls injected at Configure:

public class Checker
{
 private boolean loginConfigSet;
 private boolean securityConfigSet;

 public void setLoginConfig(XMLLoginConfig loginConfig)
 {
 this.loginConfigSet = true;
 }

 public void setSecurityConfig(SecurityConfig securityConfig)
 {
 this.securityConfigSet = true;
 }

 public void start()
 {
 if (loginConfigSet == false)
 throw new IllegalArgumentException("Login config set not called");
 if (securityConfigSet == false)
 throw new IllegalArgumentException("Security config set not called");
 }
}

Is this expected?

Since looking at the system-jmx code,
the ServiceControllerContextActions::getLifecycleOnly
doesn't seem to have the code to actually set the existing MC pojo
target on the coresponding ServiceControllerContext.

[alesj]

This would probably do the trick.
Legit enough?

Index: src/main/org/jboss/system/ServiceController.java
===================================================================
--- src/main/org/jboss/system/ServiceController.java (revision 76752)
+++ src/main/org/jboss/system/ServiceController.java (working copy)
@@ -327,12 +327,12 @@
 register(serviceName, depends, true);
 }

- public void register(ObjectName serviceName, Collection<ObjectName> depends, boolean includeLifecycle) throws Excep
+ public ServiceControllerContext register(ObjectName serviceName, Collection<ObjectName> depends, boolean includeLife
 {
 if (serviceName == null)
 {
 log.warn("Ignoring request to register null service: ", new Exception("STACKTRACE"));
- return;
+ return null;
 }

 log.debug("Registering service " + serviceName);
@@ -348,13 +348,14 @@
 {
 doInstall(controller, context);
 doChange(controller, context, ControllerState.CONFIGURED, "configure");
+ return context;
 }
 catch (Throwable t)
 {
 // Something went wrong
 safelyRemoveAnyRegisteredContext(context);

- DeploymentException.rethrowAsDeploymentException("Error during register: " + serviceName, t);
+ throw DeploymentException.rethrowAsDeploymentException("Error during register: " + serviceName, t);
 }
 }

Index: src/main/org/jboss/system/microcontainer/jmx/ServiceControllerRegistrationLifecycleCallback.java
===================================================================
--- src/main/org/jboss/system/microcontainer/jmx/ServiceControllerRegistrationLifecycleCallback.java (revision 76752)
+++ src/main/org/jboss/system/microcontainer/jmx/ServiceControllerRegistrationLifecycleCallback.java (working copy)
@@ -23,7 +23,6 @@

 import java.util.HashMap;
 import java.util.Map;
-
 import javax.management.MBeanServer;
 import javax.management.ObjectName;
 import javax.management.StandardMBean;
@@ -34,6 +33,7 @@
 import org.jboss.dependency.spi.dispatch.InvokeDispatchContext;
 import org.jboss.logging.Logger;
 import org.jboss.mx.server.ServerConstants;
+import org.jboss.system.microcontainer.ServiceControllerContext;

 /**
 * ServiceControllerLifecycleCallback.
@@ -102,7 +102,9 @@
 {
 // Don't include the lifecycle callouts unless we know the MBean implementation
 // wants them and supports "double invocation"
- getServiceController().register(objectName, null, false);
+ ServiceControllerContext scc = getServiceController().register(objectName, null, false);
+ if (scc != null)
+ scc.setTarget(context.getTarget());
 }
 catch (Exception e)
 {

[adrian.brock]

Yes. I hadn't considered that somebody would expose their POJO via
@JMX and then expect POJO injection to work using the JMX name
instead of the POJO name. ;-)

Can you see if there is an easy way to issue a warning for this case?
It looks like bad practice to me. I also think it is shows some confusion.

It should work with the current configuration of JBossAS,
but we may in the future come up with a JBossAS config where there is
no JMX exposed.

Which means @JMX won't map to an aspect that registers pojos in the MBeanServer
so there will be no ServiceControllerContext for the jmx name, just the POJO
ControllerContext.

[alesj]

Can you see if there is an easy way to issue a warning for this case?

Hmmm, I doubt it.
Unless there is some flag on (Service)ControllerContext saying that we actually have a 'better' ControllerContext to access.

Since the only things we do,
are parse name from inject element - no knowlegde about ObjectName -,
set this String name as underlying value to AbstractDependencyValueMetaData,
then at ADVMD.getValue(TypeInfo, ClassLoader) do lookup by name (underlying value),
which matches our mbean's ObjectName::getCanonicalName.
And to get the actual bean, we just call ControllerContext::getTarget,
not caring where it comes from.

As you can see, it's your fault due to such ControllerContext abstraction. :-)

[alesj]

"alesj" wrote:
This would probably do the trick.
Legit enough?

Yup, this works - tested on my demo.
Commit?

[adrian.brock]

"alesj" wrote:

"alesj" wrote:
This would probably do the trick.
Legit enough?

Yup, this works - tested on my demo.
Commit?

JFDI - I'll complain about it later when I have more time to understand the problem.
P.S. Since you're now project lead, it should be me asking you. ;-)

[alesj]

P.S. Since you're now project lead, it should be me asking you. ;-)

Haven't seen this officially yet, ;-)
so I though the handover wasn't final yet,
hence still asking you. :-)

[alesj]

JFDI - I'll complain about it later when I have more time to understand the problem.

JFDone:
https://jira.jboss.org/jira/browse/JBAS-5843