Merge pull request #25 from BorderTech/refactor-submodules

TaskMaster shutdownNow and CacheHelper closeCache

Author: jonathanaustin

taskmaster-cache-helper/src/main/java/com/github/bordertech/taskmaster/cache/CachingHelperProvider.java

@@ -14,6 +14,11 @@
  */
 public interface CachingHelperProvider {
 
+	/**
+	 * Close and release the cache resources.
+	 */
+	void closeCacheManager();
+
 	/**
 	 * Create a cache with the specified duration.
 	 *

taskmaster-cache-helper/src/main/java/com/github/bordertech/taskmaster/cache/impl/CachingHelperProviderDefault.java

@@ -8,6 +8,7 @@
 import javax.cache.configuration.MutableConfiguration;
 import javax.cache.expiry.AccessedExpiryPolicy;
 import javax.cache.expiry.Duration;
+import javax.cache.spi.CachingProvider;
 
 /**
  * Default CachingHelperProvider implementation using JSR107 provider.
@@ -16,6 +17,14 @@
  */
 public class CachingHelperProviderDefault implements CachingHelperProvider {
 
+	@Override
+	public synchronized void closeCacheManager() {
+		CachingProvider provider = Caching.getCachingProvider();
+		if (provider != null && !provider.getCacheManager().isClosed()) {
+			provider.getCacheManager().close();
+		}
+	}
+
 	@Override
 	public synchronized <K, V> Cache<K, V> getOrCreateCache(final String name, final Class<K> keyClass,
 			final Class<V> valueClass, final Duration duration) {

taskmaster-cache-helper/src/main/java/com/github/bordertech/taskmaster/cache/impl/CachingHelperProviderXmlConfig.java

@@ -38,6 +38,14 @@ public class CachingHelperProviderXmlConfig implements CachingHelperProvider {
 		MANAGER = cachingProvider.getCacheManager(uri, CachingHelperProviderXmlConfig.class.getClassLoader());
 	}
 
+	@Override
+	public synchronized void closeCacheManager() {
+		CachingProvider provider = Caching.getCachingProvider();
+		if (provider != null && !provider.getCacheManager().isClosed()) {
+			provider.getCacheManager().close();
+		}
+	}
+
 	@Override
 	public synchronized <K, V> Cache<K, V> getOrCreateCache(final String name, final Class<K> keyClass,
 			final Class<V> valueClass, final Duration duration) {

taskmaster-cache-helper/src/main/java/com/github/bordertech/taskmaster/cache/servlet/CachingProviderListener.java

@@ -1,7 +1,6 @@
 package com.github.bordertech.taskmaster.cache.servlet;
 
-import javax.cache.Caching;
-import javax.cache.spi.CachingProvider;
+import com.github.bordertech.taskmaster.cache.CachingHelper;
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 
@@ -17,11 +16,7 @@ public void contextInitialized(final ServletContextEvent sce) {
 
 	@Override
 	public void contextDestroyed(final ServletContextEvent sce) {
-		// TODO Include this in CacheHelperProvider Interface
-		CachingProvider provider = Caching.getCachingProvider();
-		if (provider != null && !provider.getCacheManager().isClosed()) {
-			provider.getCacheManager().close();
-		}
+		CachingHelper.getProvider().closeCacheManager();
 	}
 
 }

taskmaster-core/src/main/java/com/github/bordertech/taskmaster/TaskMasterProvider.java

@@ -15,12 +15,18 @@ public interface TaskMasterProvider {
 	 * accepted. Invocation has no additional effect if already shut down.
 	 *
 	 * @throws SecurityException if a security manager exists and shutting down this ExecutorService may manipulate
-	 * threads that the caller is not permitted to modify because it does not hold {@link
-	 *         java.lang.RuntimePermission}<tt>("modifyThread")</tt>, or the security manager's <tt>checkAccess</tt> method
-	 * denies access.
+	 * threads that the caller is not permitted to modify because it does not hold permission
 	 */
 	void shutdown();
 
+	/**
+	 * Attempts to stop all actively executing tasks, halts the processing of waiting tasks.
+	 *
+	 * @throws SecurityException if a security manager exists and shutting down this ExecutorService may manipulate
+	 * threads that the caller is not permitted to modify because it does not hold permission
+	 */
+	void shutdownNow();
+
 	/**
 	 * Submits a Runnable task for execution and returns a Future representing that task. The Future's <tt>get</tt>
 	 * method will return the given result upon successful completion.

taskmaster-core/src/main/java/com/github/bordertech/taskmaster/impl/TaskMasterPoolUtil.java

@@ -1,6 +1,7 @@
 package com.github.bordertech.taskmaster.impl;
 
 import com.github.bordertech.config.Config;
+import com.github.bordertech.taskmaster.exception.TaskMasterException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ArrayBlockingQueue;
@@ -11,12 +12,15 @@
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * TaskMaster ExecutorService thread pool utility.
  */
 public final class TaskMasterPoolUtil {
 
+	private static final Log LOGGER = LogFactory.getLog(TaskMasterPoolUtil.class);
 	private static final Map<String, ExecutorService> THREAD_POOLS = new HashMap<>();
 	private static final String TP_PARAM_PREFIX = "bordertech.taskmaster.pool.";
 	private static final int DEFAULT_MAX_THREADS = 20;
@@ -91,10 +95,20 @@ private static ExecutorService buildPool(final String pool) {
 	/**
 	 * Shutdown the thread pools.
 	 */
-	public static void shutdown() {
-		for (ExecutorService exec : THREAD_POOLS.values()) {
-			// TODO This logic needs to be put into TaskMaster
-			exec.shutdownNow();
+	public static void shutdownNow() {
+		boolean error = false;
+		// Shutdown the threads in the pool
+		for (Map.Entry<String, ExecutorService> pools : THREAD_POOLS.entrySet()) {
+			try {
+				pools.getValue().shutdownNow();
+				LOGGER.info("Shutdown thread pool [" + pools.getKey() + "].");
+			} catch (Exception e) {
+				error = true;
+				LOGGER.error("Could not shutdown thread pool [" + pools.getKey() + "]. " + e.getMessage(), e);
+			}
+		}
+		if (error) {
+			throw new TaskMasterException("Errors occured shutdowning thread pools.");
 		}
 	}
 
@@ -110,14 +124,13 @@ public static synchronized ExecutorService getPool(final String poolName) {
 		if (pool == null) {
 			throw new IllegalStateException("Pool [" + name + "] has not been defined.");
 		}
-		// TODO This logic needs to be put into TaskMaster
 		// Check if terminated (reactivate)
 		if (pool.isTerminated()) {
-			// TODO LOG Warning and maybe control this via flag
 			// Check not interrupted for some reason (maybe server shutting down)
 			if (Thread.currentThread().isInterrupted()) {
 				throw new IllegalStateException("Pool [" + name + "] has terminated and thread is interrupted.");
 			}
+			LOGGER.info("Thread pool [" + name + "] is terminated. Will be built again.");
 			pool = TaskMasterPoolUtil.buildPool(name);
 			THREAD_POOLS.put(name, pool);
 		}

taskmaster-core/src/main/java/com/github/bordertech/taskmaster/impl/TaskMasterProviderExecutorService.java

@@ -1,12 +1,12 @@
 package com.github.bordertech.taskmaster.impl;
 
-import com.github.bordertech.taskmaster.exception.RejectedTaskException;
 import com.github.bordertech.taskmaster.TaskFuture;
+import com.github.bordertech.taskmaster.TaskMasterProvider;
+import com.github.bordertech.taskmaster.exception.RejectedTaskException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.RejectedExecutionException;
 import javax.inject.Singleton;
-import com.github.bordertech.taskmaster.TaskMasterProvider;
 
 /**
  * Handle running tasks via {@link ExecutorService}.
@@ -22,12 +22,10 @@ public void shutdown() {
 		shutdownNow();
 	}
 
-	/**
-	 * TODO This needs to be put in TaskMaster.
-	 */
+	@Override
 	public void shutdownNow() {
 		// Provide an immediate shutdown of threads without running the waiting threads.
-		TaskMasterPoolUtil.shutdown();
+		TaskMasterPoolUtil.shutdownNow();
 	}
 
 	@Override
@@ -37,7 +35,18 @@ public <T> TaskFuture<T> submit(final Runnable task, final T result) throws Reje
 
 	@Override
 	public <T> TaskFuture<T> submit(final Runnable task, final T result, final String pool) throws RejectedTaskException {
-		ExecutorService exec = TaskMasterPoolUtil.getPool(pool);
+		if (task == null) {
+			throw new IllegalArgumentException("Task cannot be null");
+		}
+		if (result == null) {
+			throw new IllegalArgumentException("Result cannot be null");
+		}
+		if (pool == null) {
+			throw new IllegalArgumentException("Pool cannot be null");
+		}
+		// Get the executor
+		ExecutorService exec = getPool(pool);
+		// Submit the task
 		try {
 			Future<T> future = exec.submit(task, result);
 			return new TaskFutureWrapper<>(future);
@@ -46,4 +55,12 @@ public <T> TaskFuture<T> submit(final Runnable task, final T result, final Strin
 		}
 	}
 
+	/**
+	 * @param pool the pool to execute the task in
+	 * @return the ExecutorService for this pool
+	 */
+	protected ExecutorService getPool(final String pool) {
+		return TaskMasterPoolUtil.getPool(pool);
+	}
+
 }