From 4af70f184016436338a6e168d272d035d1510dca Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 16 Mar 2015 16:25:42 -0700 Subject: [PATCH] Polish --- .../HealthIndicatorAutoConfiguration.java | 37 +++++-------------- .../boot/actuate/endpoint/HealthEndpoint.java | 4 +- .../actuate/health/JmsHealthIndicator.java | 12 +++--- .../actuate/health/MailHealthIndicator.java | 20 ++++------ ...HealthIndicatorAutoConfigurationTests.java | 28 +++++++------- .../health/JmsHealthIndicatorTests.java | 20 +++++----- .../health/MailHealthIndicatorTests.java | 30 +++++++-------- .../mail/MailSenderAutoConfiguration.java | 3 +- .../sendgrid/SendGridAutoConfiguration.java | 9 +---- .../SendGridAutoConfigurationTests.java | 5 ++- 10 files changed, 68 insertions(+), 100 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java index 52d46b1966..192be3221e 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfiguration.java @@ -19,11 +19,11 @@ package org.springframework.boot.actuate.autoconfigure; import java.util.Collection; import java.util.Collections; import java.util.Map; + import javax.jms.ConnectionFactory; import javax.sql.DataSource; import org.apache.solr.client.solrj.SolrServer; - import org.springframework.amqp.rabbit.core.RabbitTemplate; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.health.ApplicationHealthIndicator; @@ -77,7 +77,7 @@ import org.springframework.mail.javamail.JavaMailSenderImpl; @AutoConfigureAfter({ DataSourceAutoConfiguration.class, MongoAutoConfiguration.class, MongoDataAutoConfiguration.class, RedisAutoConfiguration.class, RabbitAutoConfiguration.class, SolrAutoConfiguration.class, - MailSenderAutoConfiguration.class, JmsAutoConfiguration.class}) + MailSenderAutoConfiguration.class, JmsAutoConfiguration.class }) @EnableConfigurationProperties({ HealthIndicatorAutoConfigurationProperties.class }) public class HealthIndicatorAutoConfiguration { @@ -165,7 +165,6 @@ public class HealthIndicatorAutoConfiguration { return new MongoHealthIndicator(this.mongoTemplates.values().iterator() .next()); } - CompositeHealthIndicator composite = new CompositeHealthIndicator( this.healthAggregator); for (Map.Entry entry : this.mongoTemplates.entrySet()) { @@ -224,7 +223,6 @@ public class HealthIndicatorAutoConfiguration { return new RabbitHealthIndicator(this.rabbitTemplates.values().iterator() .next()); } - CompositeHealthIndicator composite = new CompositeHealthIndicator( this.healthAggregator); for (Map.Entry entry : this.rabbitTemplates @@ -254,7 +252,6 @@ public class HealthIndicatorAutoConfiguration { return new SolrHealthIndicator(this.solrServers.entrySet().iterator() .next().getValue()); } - CompositeHealthIndicator composite = new CompositeHealthIndicator( this.healthAggregator); for (Map.Entry entry : this.solrServers.entrySet()) { @@ -298,25 +295,18 @@ public class HealthIndicatorAutoConfiguration { @ConditionalOnMissingBean(name = "mailHealthIndicator") public HealthIndicator mailHealthIndicator() { if (this.mailSenders.size() == 1) { - JavaMailSenderImpl mailSender = this.mailSenders.values().iterator() - .next(); - return createMailHealthIndicator(mailSender); + return new MailHealthIndicator(this.mailSenders.values().iterator() + .next()); } CompositeHealthIndicator composite = new CompositeHealthIndicator( this.healthAggregator); for (Map.Entry entry : this.mailSenders .entrySet()) { - String name = entry.getKey(); - JavaMailSenderImpl mailSender = entry.getValue(); - composite.addHealthIndicator(name, createMailHealthIndicator(mailSender)); + composite.addHealthIndicator(entry.getKey(), new MailHealthIndicator( + entry.getValue())); } return composite; } - - private MailHealthIndicator createMailHealthIndicator( - JavaMailSenderImpl mailSender) { - return new MailHealthIndicator(mailSender); - } } @Configuration @@ -334,25 +324,18 @@ public class HealthIndicatorAutoConfiguration { @ConditionalOnMissingBean(name = "jmsHealthIndicator") public HealthIndicator jmsHealthIndicator() { if (this.connectionFactories.size() == 1) { - ConnectionFactory connectionFactory = this.connectionFactories.values() - .iterator().next(); - return createJmsHealthIndicator(connectionFactory); + return new JmsHealthIndicator(this.connectionFactories.values() + .iterator().next()); } CompositeHealthIndicator composite = new CompositeHealthIndicator( this.healthAggregator); for (Map.Entry entry : this.connectionFactories .entrySet()) { - String name = entry.getKey(); - ConnectionFactory connectionFactory = entry.getValue(); - composite.addHealthIndicator(name, createJmsHealthIndicator(connectionFactory)); + composite.addHealthIndicator(entry.getKey(), + new JmsHealthIndicator(entry.getValue())); } return composite; } - - private JmsHealthIndicator createJmsHealthIndicator( - ConnectionFactory connectionFactory) { - return new JmsHealthIndicator(connectionFactory); - } } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java index b057f3fcb1..a48bb894a6 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/endpoint/HealthEndpoint.java @@ -54,8 +54,8 @@ public class HealthEndpoint extends AbstractEndpoint { Assert.notNull(healthIndicators, "HealthIndicators must not be null"); CompositeHealthIndicator healthIndicator = new CompositeHealthIndicator( healthAggregator); - for (Map.Entry h : healthIndicators.entrySet()) { - healthIndicator.addHealthIndicator(getKey(h.getKey()), h.getValue()); + for (Map.Entry entry : healthIndicators.entrySet()) { + healthIndicator.addHealthIndicator(getKey(entry.getKey()), entry.getValue()); } this.healthIndicator = healthIndicator; } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/JmsHealthIndicator.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/JmsHealthIndicator.java index 5f31083423..0ca4c3adf1 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/JmsHealthIndicator.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/JmsHealthIndicator.java @@ -35,15 +35,13 @@ public class JmsHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Health.Builder builder) throws Exception { - Connection conToClose = null; + Connection connection = this.connectionFactory.createConnection(); try { - conToClose = this.connectionFactory.createConnection(); builder.up().withDetail("provider", - conToClose.getMetaData().getJMSProviderName()); - } finally { - if (conToClose != null) { - conToClose.close(); - } + connection.getMetaData().getJMSProviderName()); + } + finally { + connection.close(); } } diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/MailHealthIndicator.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/MailHealthIndicator.java index c6d42f42ee..e1c9c6fcae 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/MailHealthIndicator.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/MailHealthIndicator.java @@ -40,22 +40,17 @@ public class MailHealthIndicator extends AbstractHealthIndicator { @Override protected void doHealthCheck(Builder builder) throws Exception { - String location = String.format("%s:%d", this.mailSender.getHost(), this.mailSender.getPort()); - builder.withDetail("location", location); - - Transport transport = null; + builder.withDetail("location", + this.mailSender.getHost() + ":" + this.mailSender.getPort()); + Transport transport = connectTransport(); try { - transport = connectTransport(); builder.up(); } finally { - if (transport != null) { - transport.close(); - } + transport.close(); } } - // Copy-paste from JavaMailSenderImpl - see SPR-12799 private Transport connectTransport() throws MessagingException { String username = this.mailSender.getUsername(); String password = this.mailSender.getPassword(); @@ -65,7 +60,6 @@ public class MailHealthIndicator extends AbstractHealthIndicator { password = null; } } - Transport transport = getTransport(this.mailSender.getSession()); transport.connect(this.mailSender.getHost(), this.mailSender.getPort(), username, password); @@ -76,9 +70,9 @@ public class MailHealthIndicator extends AbstractHealthIndicator { String protocol = this.mailSender.getProtocol(); if (protocol == null) { protocol = session.getProperty("mail.transport.protocol"); - if (protocol == null) { - protocol = JavaMailSenderImpl.DEFAULT_PROTOCOL; - } + } + if (protocol == null) { + protocol = JavaMailSenderImpl.DEFAULT_PROTOCOL; } return session.getTransport(protocol); } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationTests.java index f3a0ee6056..251214a9aa 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/HealthIndicatorAutoConfigurationTests.java @@ -295,20 +295,6 @@ public class HealthIndicatorAutoConfigurationTests { .getClass()); } - @Configuration - @EnableConfigurationProperties - protected static class DataSourceConfig { - - @Bean - @ConfigurationProperties(prefix = DataSourceProperties.PREFIX) - public DataSource dataSource() { - return DataSourceBuilder.create() - .driverClassName("org.hsqldb.jdbc.JDBCDriver") - .url("jdbc:hsqldb:mem:test").username("sa").build(); - } - - } - @Test public void mailHealthIndicator() { this.context = new AnnotationConfigApplicationContext(); @@ -376,4 +362,18 @@ public class HealthIndicatorAutoConfigurationTests { .getClass()); } + @Configuration + @EnableConfigurationProperties + protected static class DataSourceConfig { + + @Bean + @ConfigurationProperties(prefix = DataSourceProperties.PREFIX) + public DataSource dataSource() { + return DataSourceBuilder.create() + .driverClassName("org.hsqldb.jdbc.JDBCDriver") + .url("jdbc:hsqldb:mem:test").username("sa").build(); + } + + } + } diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java index 1fabdbfc0e..3eb90a59d7 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/JmsHealthIndicatorTests.java @@ -24,10 +24,10 @@ import javax.jms.JMSException; import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; /** * Tests for {@link JmsHealthIndicator}. @@ -39,12 +39,11 @@ public class JmsHealthIndicatorTests { @Test public void jmsBrokerIsUp() throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); - when(connectionMetaData.getJMSProviderName()).thenReturn("JMS test provider"); + given(connectionMetaData.getJMSProviderName()).willReturn("JMS test provider"); Connection connection = mock(Connection.class); - when(connection.getMetaData()).thenReturn(connectionMetaData); + given(connection.getMetaData()).willReturn(connectionMetaData); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); - when(connectionFactory.createConnection()).thenReturn(connection); - + given(connectionFactory.createConnection()).willReturn(connection); JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertEquals(Status.UP, health.getStatus()); @@ -55,7 +54,8 @@ public class JmsHealthIndicatorTests { @Test public void jmsBrokerIsDown() throws JMSException { ConnectionFactory connectionFactory = mock(ConnectionFactory.class); - when(connectionFactory.createConnection()).thenThrow(new JMSException("test", "123")); + given(connectionFactory.createConnection()).willThrow( + new JMSException("test", "123")); JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertEquals(Status.DOWN, health.getStatus()); @@ -65,12 +65,12 @@ public class JmsHealthIndicatorTests { @Test public void jmsBrokerCouldNotRetrieveProviderMetadata() throws JMSException { ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class); - when(connectionMetaData.getJMSProviderName()).thenThrow(new JMSException("test", "123")); + given(connectionMetaData.getJMSProviderName()).willThrow( + new JMSException("test", "123")); Connection connection = mock(Connection.class); - when(connection.getMetaData()).thenReturn(connectionMetaData); + given(connection.getMetaData()).willReturn(connectionMetaData); ConnectionFactory connectionFactory = mock(ConnectionFactory.class); - when(connectionFactory.createConnection()).thenReturn(connection); - + given(connectionFactory.createConnection()).willReturn(connection); JmsHealthIndicator indicator = new JmsHealthIndicator(connectionFactory); Health health = indicator.health(); assertEquals(Status.DOWN, health.getStatus()); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/MailHealthIndicatorTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/MailHealthIndicatorTests.java index 2c4a3aa2fb..b7848c82cc 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/MailHealthIndicatorTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/health/MailHealthIndicatorTests.java @@ -32,8 +32,8 @@ import org.junit.Test; import org.springframework.mail.javamail.JavaMailSenderImpl; import static org.junit.Assert.assertEquals; +import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; /** * Tests for {@link MailHealthIndicator}. @@ -43,6 +43,7 @@ import static org.mockito.Mockito.when; public class MailHealthIndicatorTests { private JavaMailSenderImpl mailSender; + private MailHealthIndicator indicator; @Before @@ -50,52 +51,43 @@ public class MailHealthIndicatorTests { Session session = Session.getDefaultInstance(new Properties()); session.addProvider(new Provider(Type.TRANSPORT, "success", SuccessTransport.class.getName(), "Test", "1.0.0")); - session.addProvider(new Provider(Type.TRANSPORT, "fail", FailTransport.class .getName(), "Test", "1.0.0")); - session.addProvider(new Provider(Type.TRANSPORT, "failOnClose", FailOnCloseTransport.class.getName(), "Test", "1.0.0")); - this.mailSender = mock(JavaMailSenderImpl.class); - when(this.mailSender.getHost()).thenReturn("smtp.acme.org"); - when(this.mailSender.getPort()).thenReturn(25); - when(this.mailSender.getSession()).thenReturn(session); - + given(this.mailSender.getHost()).willReturn("smtp.acme.org"); + given(this.mailSender.getPort()).willReturn(25); + given(this.mailSender.getSession()).willReturn(session); this.indicator = new MailHealthIndicator(this.mailSender); } @Test public void smtpIsUp() { - when(this.mailSender.getProtocol()).thenReturn("success"); - + given(this.mailSender.getProtocol()).willReturn("success"); Health health = this.indicator.health(); - assertEquals(Status.UP, health.getStatus()); assertEquals("smtp.acme.org:25", health.getDetails().get("location")); } @Test public void smtpIsDown() { - when(this.mailSender.getProtocol()).thenReturn("fail"); - + given(this.mailSender.getProtocol()).willReturn("fail"); Health health = this.indicator.health(); - assertEquals(Status.DOWN, health.getStatus()); assertEquals("smtp.acme.org:25", health.getDetails().get("location")); } @Test public void unexpectedExceptionOnClose() { - when(this.mailSender.getProtocol()).thenReturn("failOnClose"); - + given(this.mailSender.getProtocol()).willReturn("failOnClose"); Health health = this.indicator.health(); - assertEquals(Status.DOWN, health.getStatus()); assertEquals("smtp.acme.org:25", health.getDetails().get("location")); } public static class SuccessTransport extends Transport { + public SuccessTransport(Session session, URLName urlname) { super(session, urlname); } @@ -113,6 +105,7 @@ public class MailHealthIndicatorTests { } public static class FailTransport extends SuccessTransport { + public FailTransport(Session session, URLName urlname) { super(session, urlname); } @@ -122,9 +115,11 @@ public class MailHealthIndicatorTests { String password) throws MessagingException { throw new MessagingException("fail on connect"); } + } public static class FailOnCloseTransport extends SuccessTransport { + public FailOnCloseTransport(Session session, URLName urlname) { super(session, urlname); } @@ -133,6 +128,7 @@ public class MailHealthIndicatorTests { public synchronized void close() throws MessagingException { throw new MessagingException("fail on close"); } + } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/mail/MailSenderAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/mail/MailSenderAutoConfiguration.java index 44aadecd34..bc9ad71139 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/mail/MailSenderAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/mail/MailSenderAutoConfiguration.java @@ -37,7 +37,6 @@ import org.springframework.mail.javamail.JavaMailSenderImpl; * * @author Oliver Gierke * @author Stephane Nicoll - * @author Johannes Stelzer * @since 1.2.0 */ @Configuration @@ -48,7 +47,7 @@ import org.springframework.mail.javamail.JavaMailSenderImpl; public class MailSenderAutoConfiguration { @Autowired - MailProperties properties; + private MailProperties properties; @Bean public JavaMailSenderImpl mailSender() { diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfiguration.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfiguration.java index bdc8c5958c..f41377e7f1 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfiguration.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfiguration.java @@ -17,7 +17,6 @@ package org.springframework.boot.autoconfigure.sendgrid; import org.apache.http.HttpHost; -import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -50,16 +49,12 @@ public class SendGridAutoConfiguration { public SendGrid sendGrid() { SendGrid sendGrid = new SendGrid(this.properties.getUsername(), this.properties.getPassword()); - if (this.properties.isProxyConfigured()) { HttpHost proxy = new HttpHost(this.properties.getProxy().getHost(), this.properties.getProxy().getPort()); - CloseableHttpClient http = HttpClientBuilder.create().setProxy(proxy) - .setUserAgent("sendgrid/" + sendGrid.getVersion() + ";java").build(); - - sendGrid.setClient(http); + sendGrid.setClient(HttpClientBuilder.create().setProxy(proxy) + .setUserAgent("sendgrid/" + sendGrid.getVersion() + ";java").build()); } - return sendGrid; } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfigurationTests.java index 5ae23bea0f..28c32092ef 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/sendgrid/SendGridAutoConfigurationTests.java @@ -40,6 +40,7 @@ import static org.junit.Assert.assertThat; * @author Maciej Walkowiak */ public class SendGridAutoConfigurationTests { + private AnnotationConfigApplicationContext context; @After @@ -105,5 +106,7 @@ public class SendGridAutoConfigurationTests { SendGrid sendGrid() { return new SendGrid("manual-user", "manual-secret"); } + } -} \ No newline at end of file + +}