From 9ef78be7a7eab2b693566ebb20fe48fbe1877ed3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 26 Aug 2015 15:53:29 +0200 Subject: [PATCH] fix privileged plugin ClassLoader configuration rename PluginClassloaderDef#serverExtension to PluginClassloaderDef#privileged to avoid confusion with the ServerExtension class and annotations rename PluginClassloaderDef to PluginClassLoaderDef (capital L) --- ...aderDef.java => PluginClassLoaderDef.java} | 18 ++++++------ .../platform/PluginClassloaderFactory.java | 28 +++++++++++-------- .../org/sonar/core/platform/PluginLoader.java | 28 +++++++++---------- .../PluginClassloaderFactoryTest.java | 20 ++++++------- .../sonar/core/platform/PluginLoaderTest.java | 20 ++++++------- 5 files changed, 60 insertions(+), 54 deletions(-) rename sonar-core/src/main/java/org/sonar/core/platform/{PluginClassloaderDef.java => PluginClassLoaderDef.java} (88%) diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderDef.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginClassLoaderDef.java similarity index 88% rename from sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderDef.java rename to sonar-core/src/main/java/org/sonar/core/platform/PluginClassLoaderDef.java index a78403fa49e..0c6d635b792 100644 --- a/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderDef.java +++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginClassLoaderDef.java @@ -31,9 +31,9 @@ import javax.annotation.Nullable; import org.sonar.classloader.Mask; /** - * Temporary information about the classloader to be created for a plugin (or a group of plugins). + * Temporary information about the classLoader to be created for a plugin (or a group of plugins). */ -class PluginClassloaderDef { +class PluginClassLoaderDef { private final String basePluginKey; private final Map mainClassesByPluginKey = new HashMap<>(); @@ -46,9 +46,9 @@ class PluginClassloaderDef { */ private boolean compatibilityMode = false; - private boolean serverExtension = false; + private boolean privileged = false; - PluginClassloaderDef(String basePluginKey) { + PluginClassLoaderDef(String basePluginKey) { Preconditions.checkArgument(!Strings.isNullOrEmpty(basePluginKey)); this.basePluginKey = basePluginKey; } @@ -95,12 +95,12 @@ class PluginClassloaderDef { this.compatibilityMode = b; } - boolean isServerExtension() { - return serverExtension; + boolean isPrivileged() { + return privileged; } - void setServerExtension(boolean serverExtension) { - this.serverExtension = serverExtension; + void setPrivileged(boolean privileged) { + this.privileged = privileged; } @Override @@ -111,7 +111,7 @@ class PluginClassloaderDef { if (o == null || getClass() != o.getClass()) { return false; } - PluginClassloaderDef that = (PluginClassloaderDef) o; + PluginClassLoaderDef that = (PluginClassLoaderDef) o; return basePluginKey.equals(that.basePluginKey); } diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderFactory.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderFactory.java index 3750e9e925a..66b11875dfe 100644 --- a/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderFactory.java +++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginClassloaderFactory.java @@ -62,14 +62,20 @@ public class PluginClassloaderFactory { /** * Creates as many classloaders as requested by the input parameter. */ - public Map create(Collection defs) { + public Map create(Collection defs) { + ClassLoader baseClassLoader = baseClassLoader(); + ClassloaderBuilder builder = new ClassloaderBuilder(); - builder.newClassloader(API_CLASSLOADER_KEY, baseClassloader()); + builder.newClassloader(API_CLASSLOADER_KEY, baseClassLoader); + builder.setMask(API_CLASSLOADER_KEY, apiMask()); - for (PluginClassloaderDef def : defs) { - builder.setMask(API_CLASSLOADER_KEY, def.isServerExtension() ? new Mask() : apiMask()); + for (PluginClassLoaderDef def : defs) { builder.newClassloader(def.getBasePluginKey()); - builder.setParent(def.getBasePluginKey(), API_CLASSLOADER_KEY, new Mask()); + if (def.isPrivileged()) { + builder.setParent(def.getBasePluginKey(), baseClassLoader, new Mask()); + } else { + builder.setParent(def.getBasePluginKey(), API_CLASSLOADER_KEY, new Mask()); + } builder.setLoadingOrder(def.getBasePluginKey(), def.isSelfFirstStrategy() ? SELF_FIRST : PARENT_FIRST); for (File jar : def.getFiles()) { builder.addURL(def.getBasePluginKey(), fileToUrl(jar)); @@ -86,10 +92,10 @@ public class PluginClassloaderFactory { /** * A plugin can export some resources to other plugins */ - private void exportResources(PluginClassloaderDef def, ClassloaderBuilder builder, Collection allPlugins) { + private void exportResources(PluginClassLoaderDef def, ClassloaderBuilder builder, Collection allPlugins) { // export the resources to all other plugins builder.setExportMask(def.getBasePluginKey(), def.getExportMask()); - for (PluginClassloaderDef other : allPlugins) { + for (PluginClassLoaderDef other : allPlugins) { if (!other.getBasePluginKey().equals(def.getBasePluginKey())) { builder.addSibling(def.getBasePluginKey(), other.getBasePluginKey(), new Mask()); } @@ -99,10 +105,10 @@ public class PluginClassloaderFactory { /** * Builds classloaders and verifies that all of them are correctly defined */ - private Map build(Collection defs, ClassloaderBuilder builder) { - Map result = new HashMap<>(); + private Map build(Collection defs, ClassloaderBuilder builder) { + Map result = new HashMap<>(); Map classloadersByBasePluginKey = builder.build(); - for (PluginClassloaderDef def : defs) { + for (PluginClassLoaderDef def : defs) { ClassLoader classloader = classloadersByBasePluginKey.get(def.getBasePluginKey()); if (classloader == null) { throw new IllegalStateException(String.format("Fail to create classloader for plugin [%s]", def.getBasePluginKey())); @@ -112,7 +118,7 @@ public class PluginClassloaderFactory { return result; } - ClassLoader baseClassloader() { + ClassLoader baseClassLoader() { return getClass().getClassLoader(); } diff --git a/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java b/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java index 3428358e6fd..ea5850f709e 100644 --- a/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java +++ b/sonar-core/src/main/java/org/sonar/core/platform/PluginLoader.java @@ -55,7 +55,7 @@ public class PluginLoader { * Defines the base keys (defined by {@link #basePluginKey(PluginInfo, Map)}) of the plugins which are allowed to * run a full server extensions. */ - private static final Set SYSTEM_EXTENSION_PLUGINS_BASE_KEYS = ImmutableSet.of("views"); + private static final Set PRIVILEGED_PLUGINS_BASE_KEYS = ImmutableSet.of("views"); public static final Version COMPATIBILITY_MODE_MAX_VERSION = Version.create("5.2"); @@ -68,8 +68,8 @@ public class PluginLoader { } public Map load(Map infoByKeys) { - Collection defs = defineClassloaders(infoByKeys); - Map classloaders = classloaderFactory.create(defs); + Collection defs = defineClassloaders(infoByKeys); + Map classloaders = classloaderFactory.create(defs); return instantiatePluginClasses(classloaders); } @@ -78,14 +78,14 @@ public class PluginLoader { * different than number of plugins. */ @VisibleForTesting - Collection defineClassloaders(Map infoByKeys) { - Map classloadersByBasePlugin = new HashMap<>(); + Collection defineClassloaders(Map infoByKeys) { + Map classloadersByBasePlugin = new HashMap<>(); for (PluginInfo info : infoByKeys.values()) { String baseKey = basePluginKey(info, infoByKeys); - PluginClassloaderDef def = classloadersByBasePlugin.get(baseKey); + PluginClassLoaderDef def = classloadersByBasePlugin.get(baseKey); if (def == null) { - def = new PluginClassloaderDef(baseKey); + def = new PluginClassLoaderDef(baseKey); classloadersByBasePlugin.put(baseKey, def); } ExplodedPlugin explodedPlugin = jarExploder.explode(info); @@ -104,7 +104,7 @@ public class PluginLoader { Version minSqVersion = info.getMinimalSqVersion(); boolean compatibilityMode = minSqVersion != null && minSqVersion.compareToIgnoreQualifier(COMPATIBILITY_MODE_MAX_VERSION) < 0; def.setCompatibilityMode(compatibilityMode); - def.setServerExtension(isServerExtension(baseKey)); + def.setPrivileged(isPrivileged(baseKey)); if (compatibilityMode) { Loggers.get(getClass()).debug("API compatibility mode is enabled on plugin {} [{}] " + "(built with API lower than {})", @@ -115,8 +115,8 @@ public class PluginLoader { return classloadersByBasePlugin.values(); } - private static boolean isServerExtension(String basePluginKey) { - return SYSTEM_EXTENSION_PLUGINS_BASE_KEYS.contains(basePluginKey); + private static boolean isPrivileged(String basePluginKey) { + return PRIVILEGED_PLUGINS_BASE_KEYS.contains(basePluginKey); } /** @@ -126,11 +126,11 @@ public class PluginLoader { * @throws IllegalStateException if at least one plugin can't be correctly loaded */ @VisibleForTesting - Map instantiatePluginClasses(Map classloaders) { + Map instantiatePluginClasses(Map classloaders) { // instantiate plugins Map instancesByPluginKey = new HashMap<>(); - for (Map.Entry entry : classloaders.entrySet()) { - PluginClassloaderDef def = entry.getKey(); + for (Map.Entry entry : classloaders.entrySet()) { + PluginClassLoaderDef def = entry.getKey(); ClassLoader classLoader = entry.getValue(); // the same classloader can be used by multiple plugins @@ -154,7 +154,7 @@ public class PluginLoader { public void unload(Collection plugins) { for (Plugin plugin : plugins) { ClassLoader classLoader = plugin.getClass().getClassLoader(); - if (classLoader instanceof Closeable && classLoader != classloaderFactory.baseClassloader()) { + if (classLoader instanceof Closeable && classLoader != classloaderFactory.baseClassLoader()) { try { ((Closeable) classLoader).close(); } catch (Exception e) { diff --git a/sonar-core/src/test/java/org/sonar/core/platform/PluginClassloaderFactoryTest.java b/sonar-core/src/test/java/org/sonar/core/platform/PluginClassloaderFactoryTest.java index 62d41a14a3f..5a4a232167d 100644 --- a/sonar-core/src/test/java/org/sonar/core/platform/PluginClassloaderFactoryTest.java +++ b/sonar-core/src/test/java/org/sonar/core/platform/PluginClassloaderFactoryTest.java @@ -44,8 +44,8 @@ public class PluginClassloaderFactoryTest { @Test public void create_isolated_classloader() { - PluginClassloaderDef def = basePluginDef(); - Map map = factory.create(asList(def)); + PluginClassLoaderDef def = basePluginDef(); + Map map = factory.create(asList(def)); assertThat(map).containsOnlyKeys(def); ClassLoader classLoader = map.get(def); @@ -62,7 +62,7 @@ public class PluginClassloaderFactoryTest { @Test public void create_classloader_compatible_with_with_old_api_dependencies() { - PluginClassloaderDef def = basePluginDef(); + PluginClassLoaderDef def = basePluginDef(); def.setCompatibilityMode(true); ClassLoader classLoader = factory.create(asList(def)).get(def); @@ -76,9 +76,9 @@ public class PluginClassloaderFactoryTest { @Test public void classloader_exports_resources_to_other_classloaders() { - PluginClassloaderDef baseDef = basePluginDef(); - PluginClassloaderDef dependentDef = dependentPluginDef(); - Map map = factory.create(asList(baseDef, dependentDef)); + PluginClassLoaderDef baseDef = basePluginDef(); + PluginClassLoaderDef dependentDef = dependentPluginDef(); + Map map = factory.create(asList(baseDef, dependentDef)); ClassLoader baseClassloader = map.get(baseDef); ClassLoader dependentClassloader = map.get(dependentDef); @@ -92,16 +92,16 @@ public class PluginClassloaderFactoryTest { assertThat(canLoadClass(baseClassloader, BASE_PLUGIN_CLASSNAME)).isTrue(); } - private static PluginClassloaderDef basePluginDef() { - PluginClassloaderDef def = new PluginClassloaderDef(BASE_PLUGIN_KEY); + private static PluginClassLoaderDef basePluginDef() { + PluginClassLoaderDef def = new PluginClassLoaderDef(BASE_PLUGIN_KEY); def.addMainClass(BASE_PLUGIN_KEY, BASE_PLUGIN_CLASSNAME); def.getExportMask().addInclusion("org/sonar/plugins/base/api/"); def.addFiles(asList(fakePluginJar("base-plugin/target/base-plugin-0.1-SNAPSHOT.jar"))); return def; } - private static PluginClassloaderDef dependentPluginDef() { - PluginClassloaderDef def = new PluginClassloaderDef(DEPENDENT_PLUGIN_KEY); + private static PluginClassLoaderDef dependentPluginDef() { + PluginClassLoaderDef def = new PluginClassLoaderDef(DEPENDENT_PLUGIN_KEY); def.addMainClass(DEPENDENT_PLUGIN_KEY, DEPENDENT_PLUGIN_CLASSNAME); def.getExportMask().addInclusion("org/sonar/plugins/dependent/api/"); def.addFiles(asList(fakePluginJar("dependent-plugin/target/dependent-plugin-0.1-SNAPSHOT.jar"))); diff --git a/sonar-core/src/test/java/org/sonar/core/platform/PluginLoaderTest.java b/sonar-core/src/test/java/org/sonar/core/platform/PluginLoaderTest.java index 3e4a2634740..c79cccc04f0 100644 --- a/sonar-core/src/test/java/org/sonar/core/platform/PluginLoaderTest.java +++ b/sonar-core/src/test/java/org/sonar/core/platform/PluginLoaderTest.java @@ -49,7 +49,7 @@ public class PluginLoaderTest { @Test public void instantiate_plugin_entry_point() { - PluginClassloaderDef def = new PluginClassloaderDef("fake"); + PluginClassLoaderDef def = new PluginClassLoaderDef("fake"); def.addMainClass("fake", FakePlugin.class.getName()); Map instances = loader.instantiatePluginClasses(ImmutableMap.of(def, getClass().getClassLoader())); @@ -59,7 +59,7 @@ public class PluginLoaderTest { @Test public void plugin_entry_point_must_be_no_arg_public() { - PluginClassloaderDef def = new PluginClassloaderDef("fake"); + PluginClassLoaderDef def = new PluginClassLoaderDef("fake"); def.addMainClass("fake", IncorrectPlugin.class.getName()); try { @@ -78,10 +78,10 @@ public class PluginLoaderTest { .setMainClass("org.foo.FooPlugin") .setMinimalSqVersion(Version.create("5.2")); - Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", info)); + Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", info)); assertThat(defs).hasSize(1); - PluginClassloaderDef def = defs.iterator().next(); + PluginClassLoaderDef def = defs.iterator().next(); assertThat(def.getBasePluginKey()).isEqualTo("foo"); assertThat(def.isSelfFirstStrategy()).isFalse(); assertThat(def.getFiles()).containsOnly(jarFile); @@ -100,7 +100,7 @@ public class PluginLoaderTest { .setMainClass("org.foo.FooPlugin") .setMinimalSqVersion(Version.create("4.5.2")); - Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", info)); + Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", info)); assertThat(defs.iterator().next().isCompatibilityMode()).isTrue(); } @@ -132,11 +132,11 @@ public class PluginLoaderTest { .setBasePlugin("foo") .setUseChildFirstClassLoader(true); - Collection defs = loader.defineClassloaders(ImmutableMap.of( + Collection defs = loader.defineClassloaders(ImmutableMap.of( base.getKey(), base, extension1.getKey(), extension1, extension2.getKey(), extension2)); assertThat(defs).hasSize(1); - PluginClassloaderDef def = defs.iterator().next(); + PluginClassLoaderDef def = defs.iterator().next(); assertThat(def.getBasePluginKey()).isEqualTo("foo"); assertThat(def.isSelfFirstStrategy()).isFalse(); assertThat(def.getFiles()).containsOnly(baseJarFile, extensionJar1, extensionJar2); @@ -151,9 +151,9 @@ public class PluginLoaderTest { public void plugin_is_recognised_as_server_extension_if_key_is_views_and_extends_no_other_plugin_and_runs_in_compatibility_mode() throws IOException { PluginInfo views = create52PluginInfo("views"); - Collection defs = loader.defineClassloaders(ImmutableMap.of("views", views)); + Collection defs = loader.defineClassloaders(ImmutableMap.of("views", views)); - assertThat(defs.iterator().next().isServerExtension()).isTrue(); + assertThat(defs.iterator().next().isPrivileged()).isTrue(); } @Test @@ -162,7 +162,7 @@ public class PluginLoaderTest { PluginInfo views = create52PluginInfo("views") .setBasePlugin("foo"); - Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", foo, "views", views)); + Collection defs = loader.defineClassloaders(ImmutableMap.of("foo", foo, "views", views)); assertThat(defs).extracting("compatibilityMode").containsOnly(false, false); } -- 2.39.5