From e0bb838259a9ff85fb5286233c76b95826469a5c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 26 Aug 2009 13:42:28 +0000 Subject: [PATCH] MBeanServerFactoryBean returns JDK 1.5 platform MBeanServer for agent id "" (SPR-5909) --- .../jmx/access/MBeanClientInterceptor.java | 1 + .../access/NotificationListenerRegistrar.java | 3 +- .../springframework/jmx/support/JmxUtils.java | 27 ++++++++------ .../jmx/support/MBeanServerFactoryBean.java | 1 + .../support/MBeanServerFactoryBeanTests.java | 37 +++++++++++++------ 5 files changed, 44 insertions(+), 25 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/jmx/access/MBeanClientInterceptor.java b/org.springframework.context/src/main/java/org/springframework/jmx/access/MBeanClientInterceptor.java index 8e2dfee1a12..9d88d0e5988 100644 --- a/org.springframework.context/src/main/java/org/springframework/jmx/access/MBeanClientInterceptor.java +++ b/org.springframework.context/src/main/java/org/springframework/jmx/access/MBeanClientInterceptor.java @@ -161,6 +161,7 @@ public class MBeanClientInterceptor * attempt being made to locate the attendant MBeanServer, unless * the {@link #setServiceUrl "serviceUrl"} property has been set. * @see javax.management.MBeanServerFactory#findMBeanServer(String) + *

Specifying the empty String indicates the platform MBeanServer. */ public void setAgentId(String agentId) { this.agentId = agentId; diff --git a/org.springframework.context/src/main/java/org/springframework/jmx/access/NotificationListenerRegistrar.java b/org.springframework.context/src/main/java/org/springframework/jmx/access/NotificationListenerRegistrar.java index 9feb174f108..9c9f5330d57 100644 --- a/org.springframework.context/src/main/java/org/springframework/jmx/access/NotificationListenerRegistrar.java +++ b/org.springframework.context/src/main/java/org/springframework/jmx/access/NotificationListenerRegistrar.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2008 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -105,6 +105,7 @@ public class NotificationListenerRegistrar extends NotificationListenerHolder * attempt being made to locate the attendant MBeanServer, unless * the {@link #setServiceUrl "serviceUrl"} property has been set. * @see javax.management.MBeanServerFactory#findMBeanServer(String) + *

Specifying the empty String indicates the platform MBeanServer. */ public void setAgentId(String agentId) { this.agentId = agentId; diff --git a/org.springframework.context/src/main/java/org/springframework/jmx/support/JmxUtils.java b/org.springframework.context/src/main/java/org/springframework/jmx/support/JmxUtils.java index 849e1c0c280..159f2b347e6 100644 --- a/org.springframework.context/src/main/java/org/springframework/jmx/support/JmxUtils.java +++ b/org.springframework.context/src/main/java/org/springframework/jmx/support/JmxUtils.java @@ -91,28 +91,31 @@ public abstract class JmxUtils { * MBeanServer can be found. Logs a warning if more than one * MBeanServer found, returning the first one from the list. * @param agentId the agent identifier of the MBeanServer to retrieve. - * If this parameter is null, all registered MBeanServers are - * considered. + * If this parameter is null, all registered MBeanServers are considered. + * If the empty String is given, the platform MBeanServer will be returned. * @return the MBeanServer if found * @throws org.springframework.jmx.MBeanServerNotFoundException * if no MBeanServer could be found * @see javax.management.MBeanServerFactory#findMBeanServer(String) */ public static MBeanServer locateMBeanServer(String agentId) throws MBeanServerNotFoundException { - List servers = MBeanServerFactory.findMBeanServer(agentId); - MBeanServer server = null; - if (servers != null && servers.size() > 0) { - // Check to see if an MBeanServer is registered. - if (servers.size() > 1 && logger.isWarnEnabled()) { - logger.warn("Found more than one MBeanServer instance" + - (agentId != null ? " with agent id [" + agentId + "]" : "") + - ". Returning first from list."); + + // null means any registered server, but "" specifically means the platform server + if (!"".equals(agentId)) { + List servers = MBeanServerFactory.findMBeanServer(agentId); + if (servers != null && servers.size() > 0) { + // Check to see if an MBeanServer is registered. + if (servers.size() > 1 && logger.isWarnEnabled()) { + logger.warn("Found more than one MBeanServer instance" + + (agentId != null ? " with agent id [" + agentId + "]" : "") + + ". Returning first from list."); + } + server = servers.get(0); } - server = (MBeanServer) servers.get(0); } - if (server == null && agentId == null) { + if (server == null && !StringUtils.hasLength(agentId)) { // Attempt to load the PlatformMBeanServer. try { server = ManagementFactory.getPlatformMBeanServer(); diff --git a/org.springframework.context/src/main/java/org/springframework/jmx/support/MBeanServerFactoryBean.java b/org.springframework.context/src/main/java/org/springframework/jmx/support/MBeanServerFactoryBean.java index 84bd891c90a..3f4b1c9ab78 100644 --- a/org.springframework.context/src/main/java/org/springframework/jmx/support/MBeanServerFactoryBean.java +++ b/org.springframework.context/src/main/java/org/springframework/jmx/support/MBeanServerFactoryBean.java @@ -84,6 +84,7 @@ public class MBeanServerFactoryBean implements FactoryBean, Initial * and (importantly) if said MBeanServer cannot be located no * attempt will be made to create a new MBeanServer (and an * MBeanServerNotFoundException will be thrown at resolution time). + *

Specifying the empty String indicates the platform MBeanServer. * @see javax.management.MBeanServerFactory#findMBeanServer(String) */ public void setAgentId(String agentId) { diff --git a/org.springframework.context/src/test/java/org/springframework/jmx/support/MBeanServerFactoryBeanTests.java b/org.springframework.context/src/test/java/org/springframework/jmx/support/MBeanServerFactoryBeanTests.java index c12ecf5f8ec..d2b224dad7b 100644 --- a/org.springframework.context/src/test/java/org/springframework/jmx/support/MBeanServerFactoryBeanTests.java +++ b/org.springframework.context/src/test/java/org/springframework/jmx/support/MBeanServerFactoryBeanTests.java @@ -1,11 +1,11 @@ /* - * Copyright 2002-2005 the original author or authors. + * Copyright 2002-2009 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT @@ -17,7 +17,7 @@ package org.springframework.jmx.support; import java.util.List; - +import java.lang.management.ManagementFactory; import javax.management.MBeanServer; import javax.management.MBeanServerFactory; @@ -25,6 +25,7 @@ import junit.framework.TestCase; /** * @author Rob Harrop + * @author Juergen Hoeller */ public class MBeanServerFactoryBeanTests extends TestCase { @@ -32,7 +33,7 @@ public class MBeanServerFactoryBeanTests extends TestCase { MBeanServerFactoryBean bean = new MBeanServerFactoryBean(); bean.afterPropertiesSet(); try { - MBeanServer server = (MBeanServer) bean.getObject(); + MBeanServer server = bean.getObject(); assertNotNull("The MBeanServer should not be null", server); } finally { @@ -45,7 +46,7 @@ public class MBeanServerFactoryBeanTests extends TestCase { bean.setDefaultDomain("foo"); bean.afterPropertiesSet(); try { - MBeanServer server = (MBeanServer) bean.getObject(); + MBeanServer server = bean.getObject(); assertEquals("The default domain should be foo", "foo", server.getDefaultDomain()); } finally { @@ -60,7 +61,7 @@ public class MBeanServerFactoryBeanTests extends TestCase { bean.setLocateExistingServerIfPossible(true); bean.afterPropertiesSet(); try { - MBeanServer otherServer = (MBeanServer) bean.getObject(); + MBeanServer otherServer = bean.getObject(); assertSame("Existing MBeanServer not located", server, otherServer); } finally { @@ -72,12 +73,24 @@ public class MBeanServerFactoryBeanTests extends TestCase { } } - public void testWithLocateExistingAndNoExistingServer() { + public void testWithLocateExistingAndFallbackToPlatformServer() { MBeanServerFactoryBean bean = new MBeanServerFactoryBean(); bean.setLocateExistingServerIfPossible(true); bean.afterPropertiesSet(); try { - assertNotNull("MBeanServer not created", bean.getObject()); + assertSame(ManagementFactory.getPlatformMBeanServer(), bean.getObject()); + } + finally { + bean.destroy(); + } + } + + public void testWithEmptyAgentIdAndFallbackToPlatformServer() { + MBeanServerFactoryBean bean = new MBeanServerFactoryBean(); + bean.setAgentId(""); + bean.afterPropertiesSet(); + try { + assertSame(ManagementFactory.getPlatformMBeanServer(), bean.getObject()); } finally { bean.destroy(); @@ -98,12 +111,12 @@ public class MBeanServerFactoryBeanTests extends TestCase { bean.afterPropertiesSet(); try { - MBeanServer server = (MBeanServer) bean.getObject(); - List servers = MBeanServerFactory.findMBeanServer(null); + MBeanServer server = bean.getObject(); + List servers = MBeanServerFactory.findMBeanServer(null); boolean found = false; - for (int x = 0; x < servers.size(); x++) { - if (servers.get(x) == server) { + for (MBeanServer candidate : servers) { + if (candidate == server) { found = true; break; }