From 2a9e6c40eda61905df42d91859b7c162d1b00f1d Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Tue, 26 Jan 2016 11:13:18 -0600 Subject: [PATCH] Fix logic for disabling plugins in CrshAutoConfiguration Plugin disabling logic was broken by e009d3e4. Prior to this change, a plugin would be disabled if it or any of the implemented interfaces in its inheritance hierarchy were configured as being disabled. The offending commit inverted the logic so that the plugin would be enabled if any part of it was NOT configured as being disabled. This commit restores the logic such that the early return happens only in the negative case. Previously, the tests were written as though PluginContext#getPlugin(Class) would consider the specified class against the runtime type of the plugin (not an unreasonable assumption); rather this method considers the broader 'plugin type'. This commit rewrites the test to seek by plugin type and assert the absence of the disabled plugins. Closes gh-5032 --- .../autoconfigure/CrshAutoConfiguration.java | 9 ++++---- .../CrshAutoConfigurationTests.java | 23 +++++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java index 2a1fec58076..487617b3a9f 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -111,6 +111,7 @@ import org.springframework.util.StringUtils; * {@code shell.command_path_patterns} in your application configuration. * * @author Christian Dupuis + * @author Matt Benson * @see ShellProperties */ @Configuration @@ -395,11 +396,11 @@ public class CrshAutoConfiguration { pluginClasses.add(plugin.getClass()); for (Class pluginClass : pluginClasses) { - if (isEnabled(pluginClass)) { - return true; + if (!isEnabled(pluginClass)) { + return false; } } - return false; + return true; } private boolean isEnabled(Class pluginClass) { diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfigurationTests.java index b0fce565e4b..32d487873e2 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfigurationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * 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. @@ -25,11 +25,13 @@ import java.util.UUID; import org.crsh.auth.AuthenticationPlugin; import org.crsh.auth.JaasAuthenticationPlugin; -import org.crsh.lang.impl.groovy.GroovyRepl; +import org.crsh.lang.impl.java.JavaLanguage; +import org.crsh.lang.spi.Language; import org.crsh.plugin.PluginContext; import org.crsh.plugin.PluginLifeCycle; import org.crsh.plugin.ResourceKind; import org.crsh.telnet.term.processor.ProcessorIOHandler; +import org.crsh.telnet.term.spi.TermIOHandler; import org.crsh.vfs.Resource; import org.junit.After; import org.junit.Test; @@ -51,10 +53,13 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.isA; +import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; /** @@ -63,6 +68,7 @@ import static org.junit.Assert.assertTrue; * @author Christian Dupuis * @author Andreas Ahlenstorf * @author Eddú Meléndez + * @author Matt Benson */ @SuppressWarnings({ "rawtypes", "unchecked" }) public class CrshAutoConfigurationTests { @@ -80,15 +86,18 @@ public class CrshAutoConfigurationTests { public void testDisabledPlugins() throws Exception { MockEnvironment env = new MockEnvironment(); env.setProperty("shell.disabled_plugins", - "GroovyREPL, termIOHandler, org.crsh.auth.AuthenticationPlugin"); + "termIOHandler, org.crsh.auth.AuthenticationPlugin, javaLanguage"); load(env); PluginLifeCycle lifeCycle = this.context.getBean(PluginLifeCycle.class); assertNotNull(lifeCycle); - assertNull(lifeCycle.getContext().getPlugin(GroovyRepl.class)); - assertNull(lifeCycle.getContext().getPlugin(ProcessorIOHandler.class)); - assertNull(lifeCycle.getContext().getPlugin(JaasAuthenticationPlugin.class)); + assertThat(lifeCycle.getContext().getPlugins(TermIOHandler.class), + not(hasItem(isA(ProcessorIOHandler.class)))); + assertThat(lifeCycle.getContext().getPlugins(AuthenticationPlugin.class), + not(hasItem(isA(JaasAuthenticationPlugin.class)))); + assertThat(lifeCycle.getContext().getPlugins(Language.class), + not(hasItem(isA(JavaLanguage.class)))); } @Test