From 76d5b913cdbbdb3be1bdbc109abaeb3aae0d42a0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 3 Jan 2017 12:08:38 -0800 Subject: [PATCH 1/5] Polish --- .../autoconfigure/orm/jpa/DatabaseLookup.java | 85 ++++++++++++++ .../orm/jpa/DatabasePlatform.java | 73 ------------ .../autoconfigure/orm/jpa/JpaProperties.java | 24 +--- ...tomHibernateJpaAutoConfigurationTests.java | 13 ++- .../orm/jpa/DatabaseLookupTests.java | 106 ++++++++++++++++++ .../orm/jpa/DatabasePlatformTests.java | 86 -------------- .../boot/jdbc/DatabaseDriver.java | 7 +- 7 files changed, 204 insertions(+), 190 deletions(-) create mode 100644 spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookup.java delete mode 100644 spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatform.java create mode 100644 spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookupTests.java delete mode 100644 spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatformTests.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookup.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookup.java new file mode 100644 index 0000000000..ef4ac3d0b9 --- /dev/null +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookup.java @@ -0,0 +1,85 @@ +/* + * Copyright 2012-2017 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.orm.jpa; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +import javax.sql.DataSource; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + +import org.springframework.boot.jdbc.DatabaseDriver; +import org.springframework.jdbc.support.JdbcUtils; +import org.springframework.jdbc.support.MetaDataAccessException; +import org.springframework.orm.jpa.vendor.Database; + +/** + * Utility to lookup well known {@link Database Databases} from a {@link DataSource}. + * + * @author Eddú Meléndez + * @author Phillip Webb + */ +final class DatabaseLookup { + + private static final Log logger = LogFactory.getLog(DatabaseLookup.class); + + private static final Map LOOKUP; + + static { + Map map = new HashMap(); + map.put(DatabaseDriver.DERBY, Database.DERBY); + map.put(DatabaseDriver.H2, Database.H2); + map.put(DatabaseDriver.HSQLDB, Database.HSQL); + map.put(DatabaseDriver.MYSQL, Database.MYSQL); + map.put(DatabaseDriver.ORACLE, Database.ORACLE); + map.put(DatabaseDriver.POSTGRESQL, Database.POSTGRESQL); + map.put(DatabaseDriver.SQLSERVER, Database.SQL_SERVER); + map.put(DatabaseDriver.DB2, Database.DB2); + map.put(DatabaseDriver.INFORMIX, Database.INFORMIX); + LOOKUP = Collections.unmodifiableMap(map); + } + + private DatabaseLookup() { + } + + /** + * Return the most suitable {@link Database} for the given {@link DataSource}. + * @param dataSource the source {@link DataSource} + * @return the most sutable {@link Database} + */ + public static Database getDatabase(DataSource dataSource) { + if (dataSource == null) { + return Database.DEFAULT; + } + try { + String url = (String) JdbcUtils.extractDatabaseMetaData(dataSource, "getURL"); + DatabaseDriver driver = DatabaseDriver.fromJdbcUrl(url); + Database database = LOOKUP.get(driver); + if (database != null) { + return database; + } + } + catch (MetaDataAccessException ex) { + logger.warn("Unable to determine jdbc url from datasource", ex); + } + return Database.DEFAULT; + } + +} diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatform.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatform.java deleted file mode 100644 index a10a473d5d..0000000000 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatform.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * Copyright 2012-2016 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.autoconfigure.orm.jpa; - -import org.springframework.boot.jdbc.DatabaseDriver; -import org.springframework.orm.jpa.vendor.Database; - -/** - * Mapper between {@link Database} and {@link DatabaseDriver}. - * - * @author Eddú Meléndez - */ -enum DatabasePlatform { - - DB2(Database.DB2, DatabaseDriver.DB2), - - DERBY(Database.DERBY, DatabaseDriver.DERBY), - - H2(Database.H2, DatabaseDriver.H2), - - HSQL(Database.HSQL, DatabaseDriver.HSQLDB), - - INFORMIX(Database.INFORMIX, DatabaseDriver.INFORMIX), - - MYSQL(Database.MYSQL, DatabaseDriver.MYSQL), - - ORACLE(Database.ORACLE, DatabaseDriver.ORACLE), - - POSTGRESQL(Database.POSTGRESQL, DatabaseDriver.POSTGRESQL), - - SQL_SERVER(Database.SQL_SERVER, DatabaseDriver.SQLSERVER); - - private final Database database; - - private final DatabaseDriver driver; - - DatabasePlatform(Database database, DatabaseDriver driver) { - this.database = database; - this.driver = driver; - } - - public Database getDatabase() { - return this.database; - } - - public DatabaseDriver getDriver() { - return this.driver; - } - - public static DatabasePlatform fromDatabaseDriver(DatabaseDriver driver) { - for (DatabasePlatform mapper : values()) { - if (mapper.getDriver() == driver) { - return mapper; - } - } - return null; - } - -} diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java index 148ba6cc01..d5adda332f 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/orm/jpa/JpaProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -21,15 +21,9 @@ import java.util.Map; import javax.sql.DataSource; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import org.springframework.boot.autoconfigure.jdbc.EmbeddedDatabaseConnection; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.NestedConfigurationProperty; -import org.springframework.boot.jdbc.DatabaseDriver; -import org.springframework.jdbc.support.JdbcUtils; -import org.springframework.jdbc.support.MetaDataAccessException; import org.springframework.orm.jpa.vendor.Database; import org.springframework.util.StringUtils; @@ -45,8 +39,6 @@ import org.springframework.util.StringUtils; @ConfigurationProperties(prefix = "spring.jpa") public class JpaProperties { - private static final Log logger = LogFactory.getLog(JpaProperties.class); - /** * Additional native properties to set on the JPA provider. */ @@ -144,19 +136,7 @@ public class JpaProperties { if (this.database != null) { return this.database; } - try { - String jdbcUrl = (String) JdbcUtils.extractDatabaseMetaData(dataSource, - "getURL"); - DatabasePlatform databasePlatform = DatabasePlatform.fromDatabaseDriver( - DatabaseDriver.fromJdbcUrl(jdbcUrl)); - if (databasePlatform != null) { - return databasePlatform.getDatabase(); - } - } - catch (MetaDataAccessException ex) { - logger.warn("Unable to determine jdbc url from datasource", ex); - } - return Database.DEFAULT; + return DatabaseLookup.getDatabase(dataSource); } public static class Hibernate { diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/CustomHibernateJpaAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/CustomHibernateJpaAutoConfigurationTests.java index 208e1e88fb..e0fff71ec8 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/CustomHibernateJpaAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/CustomHibernateJpaAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -41,7 +41,7 @@ import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; -import static org.mockito.BDDMockito.mock; +import static org.mockito.Mockito.mock; /** * Tests for {@link HibernateJpaAutoConfiguration}. @@ -120,7 +120,8 @@ public class CustomHibernateJpaAutoConfigurationTests { PropertyPlaceholderAutoConfiguration.class, HibernateJpaAutoConfiguration.class); this.context.refresh(); - HibernateJpaVendorAdapter bean = this.context.getBean(HibernateJpaVendorAdapter.class); + HibernateJpaVendorAdapter bean = this.context + .getBean(HibernateJpaVendorAdapter.class); Database database = (Database) ReflectionTestUtils.getField(bean, "database"); assertThat(database).isEqualTo(Database.H2); } @@ -139,11 +140,11 @@ public class CustomHibernateJpaAutoConfigurationTests { DataSource dataSource = mock(DataSource.class); try { given(dataSource.getConnection()).willReturn(mock(Connection.class)); - given(dataSource.getConnection().getMetaData()).willReturn( - mock(DatabaseMetaData.class)); + given(dataSource.getConnection().getMetaData()) + .willReturn(mock(DatabaseMetaData.class)); } catch (SQLException e) { - //Do nothing + // Do nothing } return dataSource; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookupTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookupTests.java new file mode 100644 index 0000000000..a3ff71753f --- /dev/null +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabaseLookupTests.java @@ -0,0 +1,106 @@ +/* + * Copyright 2012-2017 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.autoconfigure.orm.jpa; + +import java.sql.Connection; +import java.sql.DatabaseMetaData; + +import javax.sql.DataSource; + +import org.junit.Test; + +import org.springframework.orm.jpa.vendor.Database; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link DatabaseLookup}. + * + * @author Eddú Meléndez + * @author Phillip Webb + */ +public class DatabaseLookupTests { + + @Test + public void getDatabaseWhenDataSourceIsNullShouldReturnDefault() throws Exception { + assertThat(DatabaseLookup.getDatabase(null)).isEqualTo(Database.DEFAULT); + } + + @Test + public void getDatabaseWhenDataSourceIsUnknownShouldReturnDefault() throws Exception { + testGetDatabase("jdbc:idontexist:", Database.DEFAULT); + } + + @Test + public void getDatabaseWhenDerbyShouldReturnDerby() throws Exception { + testGetDatabase("jdbc:derby:", Database.DERBY); + } + + @Test + public void getDatabaseWhenH2ShouldReturnH2() throws Exception { + testGetDatabase("jdbc:h2:", Database.H2); + } + + @Test + public void getDatabaseWhenHsqldbShouldReturnHsqldb() throws Exception { + testGetDatabase("jdbc:hsqldb:", Database.HSQL); + } + + @Test + public void getDatabaseWhenMysqlShouldReturnMysql() throws Exception { + testGetDatabase("jdbc:mysql:", Database.MYSQL); + } + + @Test + public void getDatabaseWhenOracleShouldReturnOracle() throws Exception { + testGetDatabase("jdbc:oracle:", Database.ORACLE); + } + + @Test + public void getDatabaseWhenPostgresShouldReturnPostgres() throws Exception { + testGetDatabase("jdbc:postgresql:", Database.POSTGRESQL); + } + + @Test + public void getDatabaseWhenSqlserverShouldReturnSqlserver() throws Exception { + testGetDatabase("jdbc:sqlserver:", Database.SQL_SERVER); + } + + @Test + public void getDatabaseWhenDb2ShouldReturnDb2() throws Exception { + testGetDatabase("jdbc:db2:", Database.DB2); + } + + @Test + public void getDatabaseWhenInformixShouldReturnInformix() throws Exception { + testGetDatabase("jdbc:informix-sqli:", Database.INFORMIX); + } + + private void testGetDatabase(String url, Database expected) throws Exception { + DataSource dataSource = mock(DataSource.class); + Connection connection = mock(Connection.class); + DatabaseMetaData metaData = mock(DatabaseMetaData.class); + given(dataSource.getConnection()).willReturn(connection); + given(connection.getMetaData()).willReturn(metaData); + given(metaData.getURL()).willReturn(url); + Database database = DatabaseLookup.getDatabase(dataSource); + assertThat(database).isEqualTo(expected); + } + +} diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatformTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatformTests.java deleted file mode 100644 index d91cc6f0b8..0000000000 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/orm/jpa/DatabasePlatformTests.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * Copyright 2012-2016 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 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.autoconfigure.orm.jpa; - -import org.junit.Test; - -import org.springframework.boot.jdbc.DatabaseDriver; -import org.springframework.orm.jpa.vendor.Database; - -import static org.assertj.core.api.Assertions.assertThat; - -/** - * Tests for {@link DatabasePlatform}. - * - * @author Eddú Meléndez - */ -public class DatabasePlatformTests { - - @Test - public void databaseDriverLookups() { - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.DB2)) - .isEqualTo(DatabasePlatform.DB2); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.DERBY)) - .isEqualTo(DatabasePlatform.DERBY); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.H2)) - .isEqualTo(DatabasePlatform.H2); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.HSQLDB)) - .isEqualTo(DatabasePlatform.HSQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.INFORMIX)) - .isEqualTo(DatabasePlatform.INFORMIX); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.MYSQL)) - .isEqualTo(DatabasePlatform.MYSQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.ORACLE)) - .isEqualTo(DatabasePlatform.ORACLE); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.POSTGRESQL)) - .isEqualTo(DatabasePlatform.POSTGRESQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.SQLSERVER)) - .isEqualTo(DatabasePlatform.SQL_SERVER); - } - - @Test - public void databaseLookups() { - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.DB2) - .getDatabase()) - .isEqualTo(Database.DB2); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.DERBY) - .getDatabase()) - .isEqualTo(Database.DERBY); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.H2) - .getDatabase()) - .isEqualTo(Database.H2); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.HSQLDB) - .getDatabase()) - .isEqualTo(Database.HSQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.INFORMIX) - .getDatabase()) - .isEqualTo(Database.INFORMIX); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.MYSQL) - .getDatabase()) - .isEqualTo(Database.MYSQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.ORACLE) - .getDatabase()) - .isEqualTo(Database.ORACLE); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.POSTGRESQL) - .getDatabase()) - .isEqualTo(Database.POSTGRESQL); - assertThat(DatabasePlatform.fromDatabaseDriver(DatabaseDriver.SQLSERVER) - .getDatabase()) - .isEqualTo(Database.SQL_SERVER); - } - -} diff --git a/spring-boot/src/main/java/org/springframework/boot/jdbc/DatabaseDriver.java b/spring-boot/src/main/java/org/springframework/boot/jdbc/DatabaseDriver.java index 27b7548594..e2ea57888e 100644 --- a/spring-boot/src/main/java/org/springframework/boot/jdbc/DatabaseDriver.java +++ b/spring-boot/src/main/java/org/springframework/boot/jdbc/DatabaseDriver.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -72,8 +72,8 @@ public enum DatabaseDriver { /** * Maria DB. */ - MARIADB("MySQL", "org.mariadb.jdbc.Driver", - "org.mariadb.jdbc.MariaDbDataSource", "SELECT 1") { + MARIADB("MySQL", "org.mariadb.jdbc.Driver", "org.mariadb.jdbc.MariaDbDataSource", + "SELECT 1") { @Override public String getId() { @@ -181,6 +181,7 @@ public enum DatabaseDriver { protected Collection getUrlPrefixes() { return Arrays.asList("informix-sqli", "informix-direct"); } + }; private final String productName; From 8a9a61b64ffcb4f9d22f16ea1301a2f48e1107ca Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 3 Jan 2017 14:41:48 -0800 Subject: [PATCH 2/5] Prevent duplicate JmxEndpoint MBean registration Update JmxEndpoint support so that the `@ManagedResource` annotation is no longer required. This prevents both `EndpointMBeanExporter` and the regular `AnnotationMBeanExporter` from both registering the bean. Fixes gh-7813 See gh-6579 --- .../endpoint/jmx/AbstractJmxEndpoint.java | 2 -- .../endpoint/jmx/AuditEventsJmxEndpoint.java | 6 ++++- .../endpoint/jmx/DataEndpointMBean.java | 2 -- .../actuate/endpoint/jmx/EndpointMBean.java | 2 -- .../endpoint/jmx/EndpointMBeanExporter.java | 24 ++++++++++++++++--- .../actuate/endpoint/jmx/JmxEndpoint.java | 6 +++-- .../endpoint/jmx/LoggersEndpointMBean.java | 2 -- .../endpoint/jmx/ShutdownEndpointMBean.java | 2 -- 8 files changed, 30 insertions(+), 16 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AbstractJmxEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AbstractJmxEndpoint.java index ce43333f13..ca913c1dc8 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AbstractJmxEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AbstractJmxEndpoint.java @@ -22,7 +22,6 @@ import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.EndpointProperties; import org.springframework.context.EnvironmentAware; import org.springframework.core.env.Environment; -import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.util.ObjectUtils; /** @@ -33,7 +32,6 @@ import org.springframework.util.ObjectUtils; * @author Phillip Webb * @since 1.5.0 */ -@ManagedResource public abstract class AbstractJmxEndpoint implements JmxEndpoint, EnvironmentAware { private final DataConverter dataConverter; diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AuditEventsJmxEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AuditEventsJmxEndpoint.java index 2c1186d8df..9667abc137 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AuditEventsJmxEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/AuditEventsJmxEndpoint.java @@ -28,6 +28,7 @@ import org.springframework.boot.actuate.audit.AuditEventRepository; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** * {@link JmxEndpoint} for {@link AuditEventRepository}. @@ -72,7 +73,10 @@ public class AuditEventsJmxEndpoint extends AbstractJmxEndpoint { private Date parseDate(String date) { try { - return new SimpleDateFormat(DATE_FORMAT).parse(date); + if (StringUtils.hasLength(date)) { + return new SimpleDateFormat(DATE_FORMAT).parse(date); + } + return null; } catch (ParseException ex) { throw new IllegalArgumentException(ex); diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/DataEndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/DataEndpointMBean.java index 385821119b..36869923de 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/DataEndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/DataEndpointMBean.java @@ -20,7 +20,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.jmx.export.annotation.ManagedAttribute; -import org.springframework.jmx.export.annotation.ManagedResource; /** * Simple wrapper around {@link Endpoint} implementations that provide actuator data of @@ -29,7 +28,6 @@ import org.springframework.jmx.export.annotation.ManagedResource; * @author Christian Dupuis * @author Andy Wilkinson */ -@ManagedResource public class DataEndpointMBean extends EndpointMBean { /** diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java index a04e131fa3..dda22b3d49 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBean.java @@ -20,7 +20,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.jmx.export.annotation.ManagedAttribute; -import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; @@ -35,7 +34,6 @@ import org.springframework.util.ObjectUtils; * @see JmxEndpoint * @see DataEndpointMBean */ -@ManagedResource public abstract class EndpointMBean implements JmxEndpoint { private final DataConverter dataConverter; diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java index df3e4db7f6..a07542da61 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/EndpointMBeanExporter.java @@ -46,9 +46,12 @@ import org.springframework.jmx.export.MBeanExporter; import org.springframework.jmx.export.annotation.AnnotationJmxAttributeSource; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.jmx.export.assembler.MetadataMBeanInfoAssembler; +import org.springframework.jmx.export.metadata.InvalidMetadataException; +import org.springframework.jmx.export.metadata.JmxAttributeSource; import org.springframework.jmx.export.naming.MetadataNamingStrategy; import org.springframework.jmx.export.naming.SelfNaming; import org.springframework.jmx.support.ObjectNameManager; +import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; /** @@ -70,7 +73,7 @@ public class EndpointMBeanExporter extends MBeanExporter private static final Log logger = LogFactory.getLog(EndpointMBeanExporter.class); - private final AnnotationJmxAttributeSource attributeSource = new AnnotationJmxAttributeSource(); + private final AnnotationJmxAttributeSource attributeSource = new EndpointJmxAttributeSource(); private final MetadataMBeanInfoAssembler assembler = new MetadataMBeanInfoAssembler( this.attributeSource); @@ -253,8 +256,8 @@ public class EndpointMBeanExporter extends MBeanExporter if (bean instanceof SelfNaming) { return ((SelfNaming) bean).getObjectName(); } - if (bean instanceof EndpointMBean) { - return getObjectName((EndpointMBean) bean, beanKey); + if (bean instanceof JmxEndpoint) { + return getObjectName((JmxEndpoint) bean, beanKey); } return this.defaultNamingStrategy.getObjectName(bean, beanKey); } @@ -363,4 +366,19 @@ public class EndpointMBeanExporter extends MBeanExporter } } + /** + * {@link JmxAttributeSource} for {@link JmxEndpoint JmxEndpoints}. + */ + private static class EndpointJmxAttributeSource extends AnnotationJmxAttributeSource { + + @Override + public org.springframework.jmx.export.metadata.ManagedResource getManagedResource( + Class beanClass) throws InvalidMetadataException { + Assert.state(super.getManagedResource(beanClass) == null, + "@ManagedResource annotation found on JmxEndpoint " + beanClass); + return new org.springframework.jmx.export.metadata.ManagedResource(); + } + + } + } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpoint.java index 70926afa6e..65a92a0db8 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/JmxEndpoint.java @@ -17,11 +17,13 @@ package org.springframework.boot.actuate.endpoint.jmx; import org.springframework.boot.actuate.endpoint.Endpoint; +import org.springframework.jmx.export.annotation.ManagedResource; /** * A strategy for the JMX layer on top of an {@link Endpoint}. Implementations are allowed - * to use {@code @ManagedAttribute} and the full Spring JMX machinery. Implementations may - * be backed by an actual {@link Endpoint} or may be specifically designed for JMX only. + * to use {@code @ManagedAttribute} and the full Spring JMX machinery but should not use + * the {@link ManagedResource @ManagedResource} annotation. Implementations may be backed + * by an actual {@link Endpoint} or may be specifically designed for JMX only. * * @author Phillip Webb * @since 1.5.0 diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java index b95aea460b..16b8a02efa 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/LoggersEndpointMBean.java @@ -24,7 +24,6 @@ import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; import org.springframework.boot.logging.LogLevel; import org.springframework.jmx.export.annotation.ManagedAttribute; import org.springframework.jmx.export.annotation.ManagedOperation; -import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.util.Assert; /** @@ -33,7 +32,6 @@ import org.springframework.util.Assert; * @author Vedran Pavic * @since 1.5.0 */ -@ManagedResource public class LoggersEndpointMBean extends EndpointMBean { public LoggersEndpointMBean(String beanName, Endpoint endpoint, diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java index d61642f514..acb181da74 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/jmx/ShutdownEndpointMBean.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.ShutdownEndpoint; import org.springframework.jmx.export.annotation.ManagedOperation; -import org.springframework.jmx.export.annotation.ManagedResource; /** * Special endpoint wrapper for {@link ShutdownEndpoint}. @@ -29,7 +28,6 @@ import org.springframework.jmx.export.annotation.ManagedResource; * @author Christian Dupuis * @author Andy Wilkinson */ -@ManagedResource public class ShutdownEndpointMBean extends EndpointMBean { /** From b4b837063214a683fe38bf42150b48da1903af51 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 3 Jan 2017 14:42:30 -0800 Subject: [PATCH 3/5] Fix AuditEventsJmxEndpoint bean name See gh-6579 --- .../autoconfigure/EndpointMBeanExportAutoConfiguration.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfiguration.java index 9f4a72eb98..a8429342c4 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointMBeanExportAutoConfiguration.java @@ -90,7 +90,7 @@ public class EndpointMBeanExportAutoConfiguration { @Bean @ConditionalOnBean(AuditEventRepository.class) @ConditionalOnEnabledEndpoint("auditevents") - public AuditEventsJmxEndpoint abstractEndpointMBean( + public AuditEventsJmxEndpoint auditEventsEndpoint( AuditEventRepository auditEventRepository) { return new AuditEventsJmxEndpoint(this.objectMapper, auditEventRepository); } From 556ce14f2dec5d39b13ba9a483a87baf9c9848b7 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 3 Jan 2017 14:44:08 -0800 Subject: [PATCH 4/5] Fix TestRestTemplate.withBasicAuth interceptors Update `TestRestTemplate` to handle `BasicAuthorizationInterceptor`s correctly. Prior to this commit the `BasicAuthorizationInterceptor` was added directly to the `ClientHttpRequestFactory` rather than to the `RestTemplate`. This meant that it could not easily be removed when `TestRestTemplate.withBasicAuth` was invoked. The `TestRestTemplate` now sets the interceptor on `RestTemplate` directly and relies on the logic in `InterceptingHttpAccessor` to add it to the `ClientHttpRequestFactory`. Fixes gh-7812 --- .../test/web/client/TestRestTemplate.java | 37 ++++++++----------- .../web/client/TestRestTemplateTests.java | 12 +++--- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java b/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java index 7f77844163..4bbfa434f0 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/web/client/TestRestTemplate.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -48,7 +48,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.http.client.ClientHttpRequestInterceptor; import org.springframework.http.client.ClientHttpResponse; import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; -import org.springframework.http.client.InterceptingClientHttpRequestFactory; import org.springframework.http.client.support.BasicAuthorizationInterceptor; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -144,11 +143,19 @@ public class TestRestTemplate { if (username == null) { return; } - List interceptors = Collections - .singletonList( - new BasicAuthorizationInterceptor(username, password)); - restTemplate.setRequestFactory(new InterceptingClientHttpRequestFactory( - restTemplate.getRequestFactory(), interceptors)); + List interceptors = restTemplate.getInterceptors(); + if (interceptors == null) { + interceptors = Collections.emptyList(); + } + interceptors = new ArrayList(interceptors); + Iterator iterator = interceptors.iterator(); + while (iterator.hasNext()) { + if (iterator.next() instanceof BasicAuthorizationInterceptor) { + iterator.remove(); + } + } + interceptors.add(new BasicAuthorizationInterceptor(username, password)); + restTemplate.setInterceptors(interceptors); } /** @@ -985,8 +992,7 @@ public class TestRestTemplate { public TestRestTemplate withBasicAuth(String username, String password) { RestTemplate restTemplate = new RestTemplate(); restTemplate.setMessageConverters(getRestTemplate().getMessageConverters()); - restTemplate.setInterceptors( - removeBasicAuthInterceptorIfPresent(getRestTemplate().getInterceptors())); + restTemplate.setInterceptors(getRestTemplate().getInterceptors()); restTemplate.setRequestFactory(getRestTemplate().getRequestFactory()); restTemplate.setUriTemplateHandler(getRestTemplate().getUriTemplateHandler()); TestRestTemplate testRestTemplate = new TestRestTemplate(restTemplate, username, @@ -996,19 +1002,6 @@ public class TestRestTemplate { return testRestTemplate; } - private List removeBasicAuthInterceptorIfPresent( - List interceptors) { - List updatedInterceptors = new ArrayList( - interceptors); - Iterator iterator = updatedInterceptors.iterator(); - while (iterator.hasNext()) { - if (iterator.next() instanceof BasicAuthorizationInterceptor) { - iterator.remove(); - } - } - return interceptors; - } - /** * Options used to customize the Apache Http Client if it is used. */ diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java index 924e230c27..5b1549df13 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/web/client/TestRestTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -160,16 +160,15 @@ public class TestRestTemplateTests { .isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class); assertThat(basicAuthTemplate.getRestTemplate().getUriTemplateHandler()) .isSameAs(originalTemplate.getRestTemplate().getUriTemplateHandler()); - assertThat(basicAuthTemplate.getRestTemplate().getInterceptors()) - .containsExactlyElementsOf( - originalTemplate.getRestTemplate().getInterceptors()); + assertThat(basicAuthTemplate.getRestTemplate().getInterceptors()).hasSize(1); assertBasicAuthorizationInterceptorCredentials(basicAuthTemplate, "user", "password"); } @Test public void withBasicAuthReplacesBasicAuthInterceptorWhenAlreadyPresent() { - TestRestTemplate original = new TestRestTemplate("foo", "bar"); + TestRestTemplate original = new TestRestTemplate("foo", "bar") + .withBasicAuth("replace", "repalce"); TestRestTemplate basicAuth = original.withBasicAuth("user", "password"); assertThat(basicAuth.getRestTemplate().getMessageConverters()) .containsExactlyElementsOf( @@ -181,8 +180,7 @@ public class TestRestTemplateTests { .isInstanceOf(CustomHttpComponentsClientHttpRequestFactory.class); assertThat(basicAuth.getRestTemplate().getUriTemplateHandler()) .isSameAs(original.getRestTemplate().getUriTemplateHandler()); - assertThat(basicAuth.getRestTemplate().getInterceptors()) - .containsExactlyElementsOf(original.getRestTemplate().getInterceptors()); + assertThat(basicAuth.getRestTemplate().getInterceptors()).hasSize(1); assertBasicAuthorizationInterceptorCredentials(basicAuth, "user", "password"); } From 732011987d714c129543f23e43113380994d86c8 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 3 Jan 2017 15:04:31 -0800 Subject: [PATCH 5/5] Fix ManagementContextConfiguration @Order support Update `ManagementContextConfigurationsImportSelector` to instantiate classes so that order annotations are respected. Fixes gh-7814 --- ...ntContextConfigurationsImportSelector.java | 29 ++++++-- ...textConfigurationsImportSelectorTests.java | 68 +++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelectorTests.java diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelector.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelector.java index a28c479e2b..3372cf8767 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelector.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelector.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2017 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. @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.autoconfigure; import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import org.springframework.beans.factory.BeanClassLoaderAware; import org.springframework.context.annotation.DeferredImportSelector; @@ -27,6 +28,7 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.Order; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.core.type.AnnotationMetadata; +import org.springframework.util.ClassUtils; /** * Selects configuration classes for the management context configuration. Entries are @@ -47,11 +49,26 @@ class ManagementContextConfigurationsImportSelector @Override public String[] selectImports(AnnotationMetadata metadata) { // Find all possible auto configuration classes, filtering duplicates - List factories = new ArrayList( - new LinkedHashSet(SpringFactoriesLoader.loadFactoryNames( - ManagementContextConfiguration.class, this.classLoader))); - AnnotationAwareOrderComparator.sort(factories); - return factories.toArray(new String[0]); + List names = loadFactoryNames(); + Set> classes = new LinkedHashSet>(); + for (String factoryName : names) { + classes.add(ClassUtils.resolveClassName(factoryName, this.classLoader)); + } + return getSortedClassNames(new ArrayList>(classes)); + } + + protected List loadFactoryNames() { + return SpringFactoriesLoader + .loadFactoryNames(ManagementContextConfiguration.class, this.classLoader); + } + + private String[] getSortedClassNames(List> classes) { + AnnotationAwareOrderComparator.sort(classes); + List names = new ArrayList(); + for (Class sourceClass : classes) { + names.add(sourceClass.getName()); + } + return names.toArray(new String[names.size()]); } @Override diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelectorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelectorTests.java new file mode 100644 index 0000000000..41a626db64 --- /dev/null +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/ManagementContextConfigurationsImportSelectorTests.java @@ -0,0 +1,68 @@ +/* + * Copyright 2012-2017 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.actuate.autoconfigure; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Test; + +import org.springframework.core.annotation.Order; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link ManagementContextConfigurationsImportSelector}. + * + * @author Phillip Webb + */ +public class ManagementContextConfigurationsImportSelectorTests { + + @Test + public void selectImportsShouldOrderResult() throws Exception { + String[] imports = new TestManagementContextConfigurationsImportSelector() + .selectImports(null); + assertThat(imports).containsExactly(A.class.getName(), B.class.getName(), + C.class.getName()); + } + + private static class TestManagementContextConfigurationsImportSelector + extends ManagementContextConfigurationsImportSelector { + + @Override + protected List loadFactoryNames() { + return Arrays.asList(C.class.getName(), A.class.getName(), B.class.getName()); + } + + } + + @Order(1) + private static class A { + + } + + @Order(2) + private static class B { + + } + + @Order(3) + private static class C { + + } + +}