* Better support for jobs with multiple annotations, e.g. @OnApplicationStart and @every

* Previously these jobs would be displayed twice in application status.
* refactor(JobPlugin):Fixed some compiler warnings
* refactor(JobPlugin): Create executor and scheduledJobs at the same time to prevent unexpected NPEs during startup
* refactor(JobPlugin):Be consistent with how jobs are displayed in application status: use toString() in both 'Scheduled jobs' and 'Waiting jobs'
* refactor(JobPlugin):Make JobsPlugin aware of Jobs scheduled programmatically with Job.every() and display them in application status
* refactor(JobPlugin): Use Job.toString() also in monitor name, for consistency with Scheduled Jobs and Waiting Jobs

Conflicts:
	framework/src/play/jobs/Job.java

Author: angryziber

framework/src/play/jobs/Job.java

@@ -140,6 +140,7 @@ public void every(String delay) {
      */
     public void every(int seconds) {
         JobsPlugin.executor.scheduleWithFixedDelay(this, seconds, seconds, TimeUnit.SECONDS);
+        JobsPlugin.scheduledJobs.add(this);
     }
 
     // Customize Invocation
@@ -177,7 +178,7 @@ public V call() {
                 try {
                     lastException = null;
                     lastRun = System.currentTimeMillis();
-                    monitor = MonitorFactory.start(getClass().getName()+".doJob()");
+                    monitor = MonitorFactory.start(this + ".doJob()");
                     result = doJobWithResult();
                     monitor.stop();
                     monitor = null;

framework/src/play/jobs/JobsPlugin.java

@@ -28,9 +28,9 @@
 
 public class JobsPlugin extends PlayPlugin {
 
-    public static ScheduledThreadPoolExecutor executor = null;
-    public static List<Job> scheduledJobs = null;
-    private static ThreadLocal<List<Callable<? extends Object>>> afterInvocationActions = new ThreadLocal<List<Callable<? extends Object>>>();
+    public static ScheduledThreadPoolExecutor executor;
+    public static List<Job> scheduledJobs;
+    private static ThreadLocal<List<Callable<?>>> afterInvocationActions = new ThreadLocal<List<Callable<?>>>();
 
     @Override
     public String getStatus() {
@@ -54,7 +54,7 @@ public String getStatus() {
             out.println("Scheduled jobs (" + scheduledJobs.size() + "):");
             out.println("~~~~~~~~~~~~~~~~~~~~~~~~~~");
             for (Job job : scheduledJobs) {
-                out.print(job.getClass().getName());
+                out.print(job);
                 if (job.getClass().isAnnotationPresent(OnApplicationStart.class)
                         && !(job.getClass().isAnnotationPresent(On.class) || job.getClass().isAnnotationPresent(Every.class))) {
                     OnApplicationStart appStartAnnotation = job.getClass().getAnnotation(OnApplicationStart.class);
@@ -89,10 +89,9 @@ public String getStatus() {
             out.println();
             out.println("Waiting jobs:");
             out.println("~~~~~~~~~~~~~~~~~~~~~~~~~~~");
-            ScheduledFuture[] q = executor.getQueue().toArray(new ScheduledFuture[0]);
+            ScheduledFuture[] q = executor.getQueue().toArray(new ScheduledFuture[executor.getQueue().size()]);
 
-            for (int i = 0; i < q.length; i++) {
-                ScheduledFuture task = q[i];
+            for (ScheduledFuture task : q) {
                 out.println(Java.extractUnderlyingCallable((FutureTask<?>) task) + " will run in " + task.getDelay(TimeUnit.SECONDS)
                         + " seconds");
             }
@@ -108,7 +107,6 @@ public void afterApplicationStart() {
                 jobs.add(clazz);
             }
         }
-        scheduledJobs = new ArrayList<Job>();
         for (final Class<?> clazz : jobs) {
             // @OnApplicationStart
             if (clazz.isAnnotationPresent(OnApplicationStart.class)) {
@@ -117,8 +115,7 @@ public void afterApplicationStart() {
                 if (!appStartAnnotation.async()) {
                     // run job sync
                     try {
-                        Job<?> job = ((Job<?>) clazz.newInstance());
-                        scheduledJobs.add(job);
+                        Job<?> job = createJob(clazz);
                         job.run();
                         if (job.wasError) {
                             if (job.lastException != null) {
@@ -139,8 +136,7 @@ public void afterApplicationStart() {
                 } else {
                     // run job async
                     try {
-                        Job<?> job = ((Job<?>) clazz.newInstance());
-                        scheduledJobs.add(job);
+                        Job<?> job = createJob(clazz);
                         // start running job now in the background
                         @SuppressWarnings("unchecked")
                         Callable<Job> callable = (Callable<Job>) job;
@@ -156,8 +152,7 @@ public void afterApplicationStart() {
             // @On
             if (clazz.isAnnotationPresent(On.class)) {
                 try {
-                    Job<?> job = ((Job<?>) clazz.newInstance());
-                    scheduledJobs.add(job);
+                    Job<?> job = createJob(clazz);
                     scheduleForCRON(job);
                 } catch (InstantiationException ex) {
                     throw new UnexpectedException("Cannot instanciate Job " + clazz.getName());
@@ -168,8 +163,7 @@ public void afterApplicationStart() {
             // @Every
             if (clazz.isAnnotationPresent(Every.class)) {
                 try {
-                    Job job = (Job) clazz.newInstance();
-                    scheduledJobs.add(job);
+                    Job job = createJob(clazz);
                     String value = job.getClass().getAnnotation(Every.class).value();
                     if (value.startsWith("cron.")) {
                         value = Play.configuration.getProperty(value);
@@ -187,10 +181,17 @@ public void afterApplicationStart() {
         }
     }
 
+    private Job<?> createJob(Class<?> clazz) throws InstantiationException, IllegalAccessException {
+        Job<?> job = (Job<?>) clazz.newInstance();
+        scheduledJobs.add(job);
+        return job;
+    }
+
     @Override
     public void onApplicationStart() {
         int core = Integer.parseInt(Play.configuration.getProperty("play.jobs.pool", "10"));
         executor = new ScheduledThreadPoolExecutor(core, new PThreadFactory("jobs"), new ThreadPoolExecutor.AbortPolicy());
+        scheduledJobs = new ArrayList<Job>();
     }
 
     public static <V> void scheduleForCRON(Job<V> job) {
@@ -202,7 +203,7 @@ public static <V> void scheduleForCRON(Job<V> job) {
             cron = Play.configuration.getProperty(cron);
         }
         cron = Expression.evaluate(cron, cron).toString();
-        if (cron == null || "".equals(cron) || "never".equalsIgnoreCase(cron)) {
+        if (cron == null || cron.isEmpty() || "never".equalsIgnoreCase(cron)) {
             Logger.info("Skipping job %s, cron expression is not defined", job.getClass().getName());
             return;
         }
@@ -212,8 +213,8 @@ public static <V> void scheduleForCRON(Job<V> job) {
             CronExpression cronExp = new CronExpression(cron);
             Date nextDate = cronExp.getNextValidTimeAfter(now);
             if (nextDate == null) {
-                Logger.warn("The cron expression for job %s doesn't have any match in the future, will never be executed", job.getClass()
-                        .getName());
+                Logger.warn("The cron expression for job %s doesn't have any match in the future, will never be executed",
+                        job.getClass().getName());
                 return;
             }
             if (nextDate.equals(job.nextPlannedExecution)) {
@@ -240,8 +241,7 @@ public void onApplicationStop() {
             // @OnApplicationStop
             if (clazz.isAnnotationPresent(OnApplicationStop.class)) {
                 try {
-                    Job<?> job = ((Job<?>) clazz.newInstance());
-                    scheduledJobs.add(job);
+                    Job<?> job = createJob(clazz);
                     job.run();
                     if (job.wasError) {
                         if (job.lastException != null) {
@@ -268,20 +268,20 @@ public void onApplicationStop() {
 
     @Override
     public void beforeInvocation() {
-        afterInvocationActions.set(new LinkedList<Callable<? extends Object>>());
+        afterInvocationActions.set(new LinkedList<Callable<?>>());
     }
 
     @Override
     public void afterInvocation() {
-        List<Callable<? extends Object>> currentActions = afterInvocationActions.get();
+        List<Callable<?>> currentActions = afterInvocationActions.get();
         afterInvocationActions.set(null);
-        for (Callable<? extends Object> callable : currentActions) {
-            JobsPlugin.executor.submit(callable);
+        for (Callable<?> callable : currentActions) {
+            executor.submit(callable);
         }
     }
 
     // default visibility, because we want to use this only from Job.java
-    static void addAfterRequestAction(Callable<? extends Object> c) {
+    static void addAfterRequestAction(Callable<?> c) {
         if (Request.current() == null) {
             throw new IllegalStateException("After request actions can be added only from threads that serve requests!");
         }