http://cr.openjdk.java.net/~jbachorik/8010285/webrev.04
Hi Jaroslav,
This looks good to me.
I assume you've been running both the java.lang & javax.management
unit tests & JCK (java.lang JCK also has some test cases that
indirectly involve JMX introspection).
Also, maybe you could add a test to check that
the system property which allows to revert to the old
behavior is taken into account?
good work!
-- daniel
On 6/7/13 9:13 AM, Jaroslav Bachorik wrote:
On Thu 06 Jun 2013 06:06:52 PM CEST, shanliang wrote:
Jaroslav Bachorik wrote:
On Thu 06 Jun 2013 05:22:31 PM CEST, shanliang wrote:
Jaroslav,
It is now OK for me about the MBean interface searching in the
Introspector.
Here is my comment on JMX.java:
206 -- 212 you added a call
Introspector.testComplianceMBeanInterface(interfaceClass);
It is better to move this call to:
MBeanServerInvocationHandler.newProxyInstance
because the real job is done in newProxyInstance and it could be
directly called by anyone.
Hm, wouldn't it be better to move the actual logic from
MBeanServerInvocationHandler.newProxyInstance to JMX.newMBeanProxy and
delegate the MBSIH.newProxyInstance back to JMX.newMBeanProxy ? This
way it would be consistent with the way JMX.newMXBeanProxy is written.
I have no opinion here, this is an implementation detail, anyway we
have to keep both JMX.newMBeanProxy and
MBeanServerInvocationHandler.newProxyInstance
http://cr.openjdk.java.net/~jbachorik/8010285/webrev.04
I've moved the newMBeanProxy() logic to JMX to keep it consistent with
JMX.newMXBeanProxy(); MBSIH.newProxyInstance() is just forwarded to
JMX.newMBeanProxy()
-JB-
Shanliang
All others are OK for me.
Thanks.
-JB-
Shanliang
Jaroslav Bachorik wrote:
On Wed 05 Jun 2013 07:54:10 PM CEST, shanliang wrote:
Daniel Fuchs wrote:
On 6/5/13 3:55 PM, Jaroslav Bachorik wrote:
class A extends B { ...}
class B implements AMBean {...}
Yes, I see it now. However, when you check the JMX specification, page
50 onwards, the current implementation does not seem to be correct.
"3. If MyClass is an instance of the DynamicMBean interface, then
MyClassMBean is
ignored. If MyClassMBean is not a public interface, it is not a JMX
manageable
resource. If the MBean is an instance of neither MyClassMBean nor
DynamicMBean, the inheritance tree of MyClass is examined, looking
for the
nearest superclass that implements its own MBean interface.
a. If there is an ancestor called SuperClass that is an instance of
SuperClassMBean, the design patterns are used to derive the
attributes and
operations from SuperClassMBean. In this case, the MBean MyClass then
has the same management interface as the MBean SuperClass. If
SuperClassMBean is not a public interface, it is not a JMX manageable
resource.
b. When there is no superclass with its own MBean interface, MyClass is
not a
Standard MBean."
According to the specification the correct MBean interface for
class A extends B { ...}
class B implements BMBean, AMBean
would be BMBean
Hi Jaroslav,
Given that A is an instance of AMBean I think that according to the
specification the correct interface should be AMBean.
It's true that the JMX Specification does not explicitly speak of this
case - but neither does it forbid it.
My advice would therefore be to clarify the spec on this point,
if that's needed - rather than risking the introduction of
incompatibilities.
-- daniel
Look at the spec 1.4:
------
2. If the MyClass MBean is an instance of a MyClassMBean interface,
then only the methods listed in, or inherited by, the interface are
considered among all the methods of, or inherited by, the MBean. The
design patterns are then used to identify the attributes and
operations from the method names in the MyClassMBean interface and its
ancestors. In other words, MyClass is a standard MBean
------
Here A is an instance of AMBean, according to 2), A is a standard
MBean and AMBean must be taken,
3) specifies the condition as "If the MBean is an instance of neither
MyClassMBean nor DynamicMBean", our example is out of this condition,
so should not apply 3) to our example.
Ok. I've reverted to the original implementation.
http://cr.openjdk.java.net/~jbachorik/8010285/webrev.03/
The whole MBean interface inferring algorithm is a bit of black magic,
though. I mean, checking all the interfaces implemented by all the
classes in the inheritance hierarchy is counter-intuitive. I mean, why
would you do something like :
MyServiceIfc extends ObscureIfc extends ServiceMBean
Service implements MyServiceIfc
and silently supposing that somewhere in the interface inheritance
hierarchy just happen to be a properly named interface and my Service
would become a managed resource. Not mentioning the fact, that the
current implementation will fail to resolve the ServiceMBean as the
MBean interface - it stops checking by ObscureIfc.
It would be much easier for the user if he just specified the MBean
interface alongside the implementation class (... implements ...,
ServiceMBean) cleanly indicating that an object is a managed resource.
But, what you gonna do ... changing the spec would probably open a
whole another can of worms and since nobody is complaining about the
current implementation we can just leave it as it is.
-JB-
Shanliang
and for
class A extends B { ...}
class B implements AMBean {...}
is not defined; neither B or A are manageable resources.
As I said, the jtreg and jck test does not seem to mind which
implementation is used, they all pass happily. I would prefer bringing
the implementation in sync with the specification.
-JB-
