Skip to content

Commit 2a9e6c4

Browse files
mbensonwilkinsona
authored andcommitted
Fix logic for disabling plugins in CrshAutoConfiguration
Plugin disabling logic was broken by e009d3e. 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
1 parent aa17599 commit 2a9e6c4

File tree

2 files changed

+21
-11
lines changed

2 files changed

+21
-11
lines changed

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfiguration.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2015 the original author or authors.
2+
* Copyright 2012-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -111,6 +111,7 @@
111111
* {@code shell.command_path_patterns} in your application configuration.
112112
*
113113
* @author Christian Dupuis
114+
* @author Matt Benson
114115
* @see ShellProperties
115116
*/
116117
@Configuration
@@ -395,11 +396,11 @@ protected boolean isEnabled(CRaSHPlugin<?> plugin) {
395396
pluginClasses.add(plugin.getClass());
396397

397398
for (Class<?> pluginClass : pluginClasses) {
398-
if (isEnabled(pluginClass)) {
399-
return true;
399+
if (!isEnabled(pluginClass)) {
400+
return false;
400401
}
401402
}
402-
return false;
403+
return true;
403404
}
404405

405406
private boolean isEnabled(Class<?> pluginClass) {

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/CrshAutoConfigurationTests.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2015 the original author or authors.
2+
* Copyright 2012-2016 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,11 +25,13 @@
2525

2626
import org.crsh.auth.AuthenticationPlugin;
2727
import org.crsh.auth.JaasAuthenticationPlugin;
28-
import org.crsh.lang.impl.groovy.GroovyRepl;
28+
import org.crsh.lang.impl.java.JavaLanguage;
29+
import org.crsh.lang.spi.Language;
2930
import org.crsh.plugin.PluginContext;
3031
import org.crsh.plugin.PluginLifeCycle;
3132
import org.crsh.plugin.ResourceKind;
3233
import org.crsh.telnet.term.processor.ProcessorIOHandler;
34+
import org.crsh.telnet.term.spi.TermIOHandler;
3335
import org.crsh.vfs.Resource;
3436
import org.junit.After;
3537
import org.junit.Test;
@@ -51,10 +53,13 @@
5153
import org.springframework.security.core.authority.SimpleGrantedAuthority;
5254
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
5355

56+
import static org.hamcrest.CoreMatchers.hasItem;
57+
import static org.hamcrest.CoreMatchers.isA;
58+
import static org.hamcrest.CoreMatchers.not;
5459
import static org.junit.Assert.assertEquals;
5560
import static org.junit.Assert.assertFalse;
5661
import static org.junit.Assert.assertNotNull;
57-
import static org.junit.Assert.assertNull;
62+
import static org.junit.Assert.assertThat;
5863
import static org.junit.Assert.assertTrue;
5964

6065
/**
@@ -63,6 +68,7 @@
6368
* @author Christian Dupuis
6469
* @author Andreas Ahlenstorf
6570
* @author Eddú Meléndez
71+
* @author Matt Benson
6672
*/
6773
@SuppressWarnings({ "rawtypes", "unchecked" })
6874
public class CrshAutoConfigurationTests {
@@ -80,15 +86,18 @@ public void close() {
8086
public void testDisabledPlugins() throws Exception {
8187
MockEnvironment env = new MockEnvironment();
8288
env.setProperty("shell.disabled_plugins",
83-
"GroovyREPL, termIOHandler, org.crsh.auth.AuthenticationPlugin");
89+
"termIOHandler, org.crsh.auth.AuthenticationPlugin, javaLanguage");
8490
load(env);
8591

8692
PluginLifeCycle lifeCycle = this.context.getBean(PluginLifeCycle.class);
8793
assertNotNull(lifeCycle);
8894

89-
assertNull(lifeCycle.getContext().getPlugin(GroovyRepl.class));
90-
assertNull(lifeCycle.getContext().getPlugin(ProcessorIOHandler.class));
91-
assertNull(lifeCycle.getContext().getPlugin(JaasAuthenticationPlugin.class));
95+
assertThat(lifeCycle.getContext().getPlugins(TermIOHandler.class),
96+
not(hasItem(isA(ProcessorIOHandler.class))));
97+
assertThat(lifeCycle.getContext().getPlugins(AuthenticationPlugin.class),
98+
not(hasItem(isA(JaasAuthenticationPlugin.class))));
99+
assertThat(lifeCycle.getContext().getPlugins(Language.class),
100+
not(hasItem(isA(JavaLanguage.class))));
92101
}
93102

94103
@Test

0 commit comments

Comments
 (0)