SRP: split CorePlugin to EnhancerPlugin and PlayStatusPlugin

* these 2 plugins have totally different responsibilities and priorities * splitting them allows users to easily override/disable/reorder any of them
playframework · May 22, 2017 · 15874d0 · 15874d0
1 parent ffe61bc
commit 15874d0
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 72 deletions.
diff --git a/documentation/manual/main.textile b/documentation/manual/main.textile
@@ -148,7 +148,7 @@ h2. Class enhancement
 
 A Play plug-in (i.e. a subclass of @play.PlayPlugin@) may include ‘enhancers’ that modify application classes at runtime, to add functionality. This is how some of Play’s magic works.
 
-The built-in @play.CorePlugin@ uses enhancers from the @play.classloading.enhancers@ package to dynamically add code to your application’s classes:
+The built-in @play.plugins.EnhancerPlugin@ uses enhancers from the @play.classloading.enhancers@ package to dynamically add code to your application’s classes:
 
 * @ContinuationEnhancer@ - adds continuations support to controller classes
 * @ControllersEnhancer@ - makes controller action methods thread-safe and adds HTTP redirects for method calls

diff --git a/framework/src/play.plugins b/framework/src/play.plugins
@@ -1,4 +1,4 @@
-0:play.CorePlugin
+0:play.plugins.EnhancerPlugin
 1:play.ConfigurationChangeWatcherPlugin
 100:play.data.parsing.TempFilePlugin
 200:play.data.validation.ValidationPlugin
@@ -8,4 +8,5 @@
 500:play.i18n.MessagesPlugin
 600:play.libs.WS
 700:play.jobs.JobsPlugin
-100000:play.plugins.ConfigurablePluginDisablingPlugin
+100000:play.plugins.ConfigurablePluginDisablingPlugin
+100100:play.plugins.PlayStatusPlugin
diff --git a/framework/src/play/plugins/EnhancerPlugin.java b/framework/src/play/plugins/EnhancerPlugin.java
@@ -0,0 +1,34 @@
+package play.plugins;
+
+import play.Logger;
+import play.PlayPlugin;
+import play.classloading.ApplicationClasses.ApplicationClass;
+import play.classloading.enhancers.*;
+import play.exceptions.UnexpectedException;
+
+/**
+ * Plugin used for core tasks
+ */
+public class EnhancerPlugin extends PlayPlugin {
+
+    protected Enhancer[] defaultEnhancers() {
+        return new Enhancer[] { new PropertiesEnhancer(), new ContinuationEnhancer(), new SigEnhancer(), new ControllersEnhancer(),
+                new MailerEnhancer(), new LocalvariablesNamesEnhancer() };
+    }
+
+    @Override
+    public void enhance(ApplicationClass applicationClass) {
+        for (Enhancer enhancer : defaultEnhancers()) {
+            try {
+                long start = System.currentTimeMillis();
+                enhancer.enhanceThisClass(applicationClass);
+                if (Logger.isTraceEnabled()) {
+                    Logger.trace("%sms to apply %s to %s", System.currentTimeMillis() - start, enhancer.getClass().getSimpleName(),
+                            applicationClass.name);
+                }
+            } catch (Exception e) {
+                throw new UnexpectedException("While applying " + enhancer + " on " + applicationClass.name, e);
+            }
+        }
+    }
+}
diff --git a/framework/src/play/CorePlugin.java → ...rk/src/play/plugins/PlayStatusPlugin.java b/framework/src/play/CorePlugin.java → ...rk/src/play/plugins/PlayStatusPlugin.java
@@ -1,44 +1,29 @@
-package play;
-
-import static java.util.Arrays.asList;
-
-import java.io.PrintWriter;
-import java.io.StringWriter;
-import java.text.SimpleDateFormat;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Date;
-import java.util.List;
-
-import org.apache.commons.lang.StringUtils;
+package play.plugins;
 
 import com.google.gson.JsonArray;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonPrimitive;
 import com.jamonapi.Monitor;
 import com.jamonapi.MonitorFactory;
 import com.jamonapi.utils.Misc;
-
+import org.apache.commons.lang.StringUtils;
+import play.Invoker;
+import play.Logger;
+import play.Play;
 import play.Play.Mode;
-import play.classloading.ApplicationClasses.ApplicationClass;
-import play.classloading.enhancers.ContinuationEnhancer;
-import play.classloading.enhancers.ControllersEnhancer;
-import play.classloading.enhancers.Enhancer;
-import play.classloading.enhancers.LocalvariablesNamesEnhancer;
-import play.classloading.enhancers.MailerEnhancer;
-import play.classloading.enhancers.PropertiesEnhancer;
-import play.classloading.enhancers.SigEnhancer;
-import play.exceptions.UnexpectedException;
-import play.libs.Crypto;
+import play.PlayPlugin;
 import play.mvc.Http.Header;
 import play.mvc.Http.Request;
 import play.mvc.Http.Response;
 
-/**
- * Plugin used for core tasks
- */
-public class CorePlugin extends PlayPlugin {
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.text.SimpleDateFormat;
+import java.util.*;
+
+import static java.util.Arrays.asList;
+
+public class PlayStatusPlugin extends PlayPlugin {
 
     /**
      * Get the application status
@@ -47,7 +32,7 @@ public class CorePlugin extends PlayPlugin {
      *            true if the status should be return in JSON
      * @return application status
      */
-    public static String computeApplicationStatus(boolean json) {
+    public String computeApplicationStatus(boolean json) {
         if (json) {
             JsonObject o = new JsonObject();
             for (PlayPlugin plugin : Play.pluginCollection.getEnabledPlugins()) {
@@ -184,12 +169,7 @@ public String getStatus() {
             out.println("Monitors:");
             out.println("~~~~~~~~");
             List<Monitor> monitors = new ArrayList<>(asList(MonitorFactory.getRootMonitor().getMonitors()));
-            Collections.sort(monitors, new Comparator<Monitor>() {
-                @Override
-                public int compare(Monitor m1, Monitor m2) {
-                    return Double.compare(m2.getTotal(), m1.getTotal());
-                }
-            });
+            monitors.sort((m1, m2) -> Double.compare(m2.getTotal(), m1.getTotal()));
             int lm = 10;
             for (Monitor monitor : monitors) {
                 if (monitor.getLabel().length() > lm) {
@@ -269,7 +249,7 @@ public JsonObject getJsonStatus() {
     /**
      * Recursively visit all JVM threads
      */
-    static void visit(PrintWriter out, ThreadGroup group, int level) {
+    private void visit(PrintWriter out, ThreadGroup group, int level) {
         // Get threads in `group'
         int numThreads = group.activeCount();
         Thread[] threads = new Thread[numThreads * 2];
@@ -296,32 +276,11 @@ static void visit(PrintWriter out, ThreadGroup group, int level) {
     /**
      * Retrieve the JVM root thread group.
      */
-    static ThreadGroup getRootThread() {
+    private ThreadGroup getRootThread() {
         ThreadGroup root = Thread.currentThread().getThreadGroup().getParent();
         while (root.getParent() != null) {
             root = root.getParent();
         }
         return root;
     }
-
-    protected Enhancer[] defaultEnhancers() {
-        return new Enhancer[] { new PropertiesEnhancer(), new ContinuationEnhancer(), new SigEnhancer(), new ControllersEnhancer(),
-                new MailerEnhancer(), new LocalvariablesNamesEnhancer() };
-    }
-
-    @Override
-    public void enhance(ApplicationClass applicationClass) throws Exception {
-        for (Enhancer enhancer : defaultEnhancers()) {
-            try {
-                long start = System.currentTimeMillis();
-                enhancer.enhanceThisClass(applicationClass);
-                if (Logger.isTraceEnabled()) {
-                    Logger.trace("%sms to apply %s to %s", System.currentTimeMillis() - start, enhancer.getClass().getSimpleName(),
-                            applicationClass.name);
-                }
-            } catch (Exception e) {
-                throw new UnexpectedException("While applying " + enhancer + " on " + applicationClass.name, e);
-            }
-        }
-    }
 }
diff --git a/framework/test-src/play/plugins/PluginCollectionTest.java b/framework/test-src/play/plugins/PluginCollectionTest.java
@@ -39,7 +39,7 @@ public void verifyLoading() {
 
         //the following plugin-list should match the list in the file 'play.plugins'
         assertThat(pc.getEnabledPlugins()).containsExactly(
-                pc.getPluginInstance(CorePlugin.class),
+                pc.getPluginInstance(EnhancerPlugin.class),
                 pc.getPluginInstance(ConfigurationChangeWatcherPlugin.class),
                 pc.getPluginInstance(TempFilePlugin.class),
                 pc.getPluginInstance(ValidationPlugin.class),
@@ -49,7 +49,8 @@ public void verifyLoading() {
                 pc.getPluginInstance(MessagesPlugin.class),
                 pc.getPluginInstance(WS.class),
                 pc.getPluginInstance(JobsPlugin.class),
-                pc.getPluginInstance(ConfigurablePluginDisablingPlugin.class));
+                pc.getPluginInstance(ConfigurablePluginDisablingPlugin.class),
+                pc.getPluginInstance(PlayStatusPlugin.class));
     }
 
     @Test
@@ -67,11 +68,11 @@ protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {
 
         pc.loadPlugins();
 
-        CorePlugin corePlugin_first_instance = pc.getPluginInstance(CorePlugin.class);
+        EnhancerPlugin enhancerPlugin_first_instance = pc.getPluginInstance(EnhancerPlugin.class);
         TestPlugin testPlugin_first_instance = pc.getPluginInstance(TestPlugin.class);
 
         assertThat(pc.getAllPlugins()).containsExactly(
-                corePlugin_first_instance,
+            enhancerPlugin_first_instance,
                 testPlugin_first_instance);
 
     }
@@ -91,22 +92,22 @@ protected boolean isLoadedByApplicationClassloader(PlayPlugin plugin) {
 
         pc.loadPlugins();
 
-        CorePlugin corePlugin_first_instance = pc.getPluginInstance(CorePlugin.class);
+        EnhancerPlugin enhancerPlugin_first_instance = pc.getPluginInstance(EnhancerPlugin.class);
         TestPlugin testPlugin_first_instance = pc.getPluginInstance(TestPlugin.class);
 
         //the following plugin-list should match the list in the file 'play.plugins'
         assertThat(pc.getEnabledPlugins()).containsExactly(
-                corePlugin_first_instance,
+            enhancerPlugin_first_instance,
                 testPlugin_first_instance);
         assertThat(pc.getAllPlugins()).containsExactly(
-                corePlugin_first_instance,
+            enhancerPlugin_first_instance,
                 testPlugin_first_instance);
 
         pc.reloadApplicationPlugins();
 
         TestPlugin testPlugin_second_instance = pc.getPluginInstance(TestPlugin.class);
 
-        assertThat(pc.getPluginInstance(CorePlugin.class)).isEqualTo( corePlugin_first_instance);
+        assertThat(pc.getPluginInstance(EnhancerPlugin.class)).isEqualTo(enhancerPlugin_first_instance);
         assertThat(testPlugin_second_instance).isNotEqualTo( testPlugin_first_instance);
 
     }

diff --git a/framework/test-src/play/plugins/custom-play-with-blank-lines.plugins b/framework/test-src/play/plugins/custom-play-with-blank-lines.plugins
@@ -1,4 +1,4 @@
-0:play.CorePlugin
+0:play.plugins.EnhancerPlugin
 
 1:play.plugins.TestPlugin
 
diff --git a/framework/test-src/play/plugins/custom-play.plugins b/framework/test-src/play/plugins/custom-play.plugins
@@ -1,2 +1,2 @@
-0:play.CorePlugin
+0:play.plugins.EnhancerPlugin
 1:play.plugins.TestPlugin