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
This commit is contained in:
Matt Benson 2016-01-26 11:13:18 -06:00 committed by Andy Wilkinson
parent aa17599675
commit 2a9e6c40ed
2 changed files with 21 additions and 11 deletions

View File

@ -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) {

View File

@ -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