From 1031f01ee0821c99642b5dee36d48d98f31b4aa6 Mon Sep 17 00:00:00 2001 From: Scott Battaglia Date: Fri, 22 May 2009 19:37:53 +0000 Subject: [PATCH] NOJIRA updated so that we rely on the filter instead of the listener. --- .../cas/client/proxy/CleanUpListener.java | 88 ------------------- .../cas/client/proxy/CleanUpTimerTask.java | 38 +++----- .../cas/client/util/AbstractCasFilter.java | 12 ++- .../AbstractTicketValidationFilter.java | 4 + ...0ProxyReceivingTicketValidationFilter.java | 57 +++++++----- .../validation/Saml11TicketValidator.java | 3 +- .../cas/client/proxy/CleanUpListenerTest.java | 37 +------- .../client/proxy/CleanUpTimerTaskTest.java | 8 +- ...xyReceivingTicketValidationFilterTest.java | 3 - 9 files changed, 66 insertions(+), 184 deletions(-) delete mode 100644 cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpListener.java diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpListener.java b/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpListener.java deleted file mode 100644 index 01da2c5..0000000 --- a/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpListener.java +++ /dev/null @@ -1,88 +0,0 @@ -package org.jasig.cas.client.proxy; - -import java.util.Timer; -import java.util.TimerTask; - -import javax.servlet.ServletContext; -import javax.servlet.ServletContextEvent; -import javax.servlet.ServletContextListener; - - -/** - * One of two choices for cleaning up {@link ProxyGrantingTicketStorage}. - * You must either configure this listener in web.xml or configure - * {@link CleanUpTimerTask} in Spring xml. Both choices perform the same - * operation, the only difference is one is configured via the Spring - * Framework and the other via web.xml. - *

- * See below for example web.xml configuration. - * See {@link CleanUpTimerTask} for an example Spring xml configuration. - *

- * With this listener configured, a timer will clean up the - * {@link ProxyGrantingTicketStorage storage}. This timer automatically - * shuts down on webapp undeploy, so there will be no classloader leaks - * due to an orphan thread. - *

- * Example web.xml configuration: - * - * - * org.jasig.cas.client.proxy.CleanUpListener - * - * - *

- * The default time between cleanups is 60 seconds, but you can optionally - * customize it by setting a context-param in web.xml. Example config: - * - * - * millisBetweenCleanUps - * - * 45000 - * - * - * - * @author Brad Cupit (brad [at] lsu {dot} edu) - */ -public final class CleanUpListener implements ServletContextListener { - protected static final int DEFAULT_MILLIS_BETWEEN_CLEANUPS = 60 * 1000; - protected static final String MILLIS_BETWEEN_CLEANUPS_INIT_PARAM = "millisBetweenCleanUps"; - private final Timer timer; - private final TimerTask timerTask; - - public CleanUpListener() { - this.timer = new Timer(true); - this.timerTask = new CleanUpTimerTask(); - } - - /** - * for unit test use only - */ - protected CleanUpListener(final Timer timer, TimerTask timerTask ) { - this.timer = timer; - this.timerTask = timerTask; - } - - public void contextInitialized(ServletContextEvent servletContextEvent) { - final long millisBetweenCleanUps = getMillisBetweenCleanups(servletContextEvent.getServletContext()); - final long millisBeforeStart = millisBetweenCleanUps; - - this.timer.schedule(timerTask, millisBeforeStart, millisBetweenCleanUps); - } - - public void contextDestroyed(ServletContextEvent servletContextEvent) { - this.timer.cancel(); - } - - protected long getMillisBetweenCleanups(ServletContext servletContext) { - final String millisBetweenCleanUps = servletContext.getInitParameter(MILLIS_BETWEEN_CLEANUPS_INIT_PARAM); - - if (millisBetweenCleanUps == null) { - return DEFAULT_MILLIS_BETWEEN_CLEANUPS; - } - - try { - return Long.parseLong(millisBetweenCleanUps); - } catch (NumberFormatException exception) { - throw new RuntimeException("The servlet context-param " + MILLIS_BETWEEN_CLEANUPS_INIT_PARAM + " must be a valid number (hint: this is usually set in web.xml)", exception); - } - } -} diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpTimerTask.java b/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpTimerTask.java index 689a600..be50e67 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpTimerTask.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/proxy/CleanUpTimerTask.java @@ -8,37 +8,21 @@ import org.jasig.cas.client.validation.Cas20ProxyReceivingTicketValidationFilter * A {@link TimerTask} implementation which performs the * actual 'cleaning' by calling {@link ProxyGrantingTicketStorage#cleanUp()}. *

- * You must configure either this TimerTask directly in Spring, - * or the {@link CleanUpListener} (which is configured in - * web.xml). Both choices perform the same operation, the only difference is - * one is configured via the Spring Framework and the other via web.xml. - *

- * For an example web.xml configuration, see {@link CleanUpListener}. - * For an example Spring xml configuration, see below: - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * - * + * By default, the {@link org.jasig.cas.client.validation.Cas20ProxyReceivingTicketValidationFilter} configures + * a task that cleans up the {@link org.jasig.cas.client.proxy.ProxyGrantingTicketStorage} associated with it. * * @author Brad Cupit (brad [at] lsu {dot} edu) + * @version $Revision$ $Date$ + * @since 3.1.6 */ public final class CleanUpTimerTask extends TimerTask { + + private final ProxyGrantingTicketStorage proxyGrantingTicketStorage; + + public CleanUpTimerTask(final ProxyGrantingTicketStorage proxyGrantingTicketStorage) { + this.proxyGrantingTicketStorage = proxyGrantingTicketStorage; + } public void run() { - Cas20ProxyReceivingTicketValidationFilter.getProxyGrantingTicketStorage().cleanUp(); + this.proxyGrantingTicketStorage.cleanUp(); } } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java index 1c98d9f..ab486a4 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/util/AbstractCasFilter.java @@ -63,13 +63,18 @@ public abstract class AbstractCasFilter extends AbstractConfigurationFilter { init(); } - /** Controls the ordering of filter initialization and checking by defining a method that runs before the init. */ + /** Controls the ordering of filter initialization and checking by defining a method that runs before the init. + * @param filterConfig the original filter configuration. + * @throws ServletException if there is a problem. + * + */ protected void initInternal(final FilterConfig filterConfig) throws ServletException { // template method } /** - * Initialization method. Called by Filter's init method or by Spring. + * Initialization method. Called by Filter's init method or by Spring. Similar in concept to the InitializingBean interface's + * afterPropertiesSet(); */ public void init() { CommonUtils.assertNotNull(this.artifactParameterName, "artifactParameterName cannot be null."); @@ -77,7 +82,8 @@ public abstract class AbstractCasFilter extends AbstractConfigurationFilter { CommonUtils.assertTrue(CommonUtils.isNotEmpty(this.serverName) || CommonUtils.isNotEmpty(this.service), "serverName or service must be set."); } - public final void destroy() { + // empty implementation as most filters won't need this. + public void destroy() { // nothing to do } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java index e761810..3861f58 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/AbstractTicketValidationFilter.java @@ -183,4 +183,8 @@ public abstract class AbstractTicketValidationFilter extends AbstractCasFilter { public final void setUseSession(final boolean useSession) { this.useSession = useSession; } + + + + } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilter.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilter.java index d306338..36834fa 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilter.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilter.java @@ -6,12 +6,7 @@ package org.jasig.cas.client.validation; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import javax.servlet.FilterChain; import javax.servlet.FilterConfig; @@ -21,48 +16,58 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.jasig.cas.client.proxy.Cas20ProxyRetriever; -import org.jasig.cas.client.proxy.ProxyGrantingTicketStorage; -import org.jasig.cas.client.proxy.ProxyGrantingTicketStorageImpl; -import org.jasig.cas.client.proxy.CleanUpListener; +import org.jasig.cas.client.proxy.*; import org.jasig.cas.client.util.CommonUtils; /** * Creates either a CAS20ProxyTicketValidator or a CAS20ServiceTicketValidator depending on whether any of the * proxy parameters are set. *

- * This filter can also pass additional parameteres to the ticket validator. Any init parameter not included in the + * This filter can also pass additional parameters to the ticket validator. Any init parameter not included in the * reserved list {@link org.jasig.cas.client.validation.Cas20ProxyReceivingTicketValidationFilter#RESERVED_INIT_PARAMS}. * * @author Scott Battaglia + * @author Brad Cupit (brad [at] lsu {dot} edu) * @version $Revision$ $Date$ * @since 3.1 * */ public class Cas20ProxyReceivingTicketValidationFilter extends AbstractTicketValidationFilter { - private static final String[] RESERVED_INIT_PARAMS = new String[] {"proxyReceptorUrl", "acceptAnyProxy", "allowedProxyChains", "casServerUrlPrefix", "proxyCallbackUrl", "renew", "exceptionOnValidationFailure", "redirectAfterValidation", "useSession", "serverName", "service", "artifactParameterName", "serviceParameterName", "encodeServiceUrl"}; + private static final String[] RESERVED_INIT_PARAMS = new String[] {"proxyReceptorUrl", "acceptAnyProxy", "allowedProxyChains", "casServerUrlPrefix", "proxyCallbackUrl", "renew", "exceptionOnValidationFailure", "redirectAfterValidation", "useSession", "serverName", "service", "artifactParameterName", "serviceParameterName", "encodeServiceUrl", "millisBetweenCleanUps"}; + + private static final int DEFAULT_MILLIS_BETWEEN_CLEANUPS = 60 * 1000; /** * The URL to send to the CAS server as the URL that will process proxying requests on the CAS client. */ private String proxyReceptorUrl; + + private Timer timer; + + private TimerTask timerTask; + + private int millisBetweenCleanUps; /** * Storage location of ProxyGrantingTickets and Proxy Ticket IOUs. */ - private static ProxyGrantingTicketStorage proxyGrantingTicketStorage = new ProxyGrantingTicketStorageImpl(); + private ProxyGrantingTicketStorage proxyGrantingTicketStorage = new ProxyGrantingTicketStorageImpl(); protected void initInternal(final FilterConfig filterConfig) throws ServletException { super.initInternal(filterConfig); setProxyReceptorUrl(getPropertyFromInitParams(filterConfig, "proxyReceptorUrl", null)); + log.trace("Setting proxyReceptorUrl parameter: " + this.proxyReceptorUrl); + this.millisBetweenCleanUps = Integer.parseInt(getPropertyFromInitParams(filterConfig, "millisBetweenCleanUps", Integer.toString(DEFAULT_MILLIS_BETWEEN_CLEANUPS))); } public void init() { super.init(); - CommonUtils.assertNotNull(proxyGrantingTicketStorage, "proxyGrantingTicketStorage cannot be null."); + this.timer = new Timer(true); + this.timerTask = new CleanUpTimerTask(this.proxyGrantingTicketStorage); + this.timer.schedule(this.timerTask, this.millisBetweenCleanUps, this.millisBetweenCleanUps); } /** @@ -118,6 +123,11 @@ public class Cas20ProxyReceivingTicketValidationFilter extends AbstractTicketVal return (List) editor.getValue(); } + public void destroy() { + super.destroy(); + this.timer.cancel(); + } + /** * This processes the ProxyReceptor request before the ticket validation code executes. */ @@ -138,17 +148,16 @@ public class Cas20ProxyReceivingTicketValidationFilter extends AbstractTicketVal public final void setProxyReceptorUrl(final String proxyReceptorUrl) { this.proxyReceptorUrl = proxyReceptorUrl; } - - /** - * Static getter so {@link CleanUpListener} has some way of retrieving - * the actual storage. This relies on the fact that this class (the Filter) - * will only be instantiated once per webapp. - */ - public static ProxyGrantingTicketStorage getProxyGrantingTicketStorage() { - return proxyGrantingTicketStorage; - } - public void setProxyGrantingTicketStorage(ProxyGrantingTicketStorage storage) { + public void setProxyGrantingTicketStorage(final ProxyGrantingTicketStorage storage) { proxyGrantingTicketStorage = storage; } + + public void setTimer(final Timer timer) { + this.timer = timer; + } + + public void setTimerTask(final TimerTask timerTask) { + this.timerTask = timerTask; + } } diff --git a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java index 109cf2b..b707b88 100644 --- a/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java +++ b/cas-client-core/src/main/java/org/jasig/cas/client/validation/Saml11TicketValidator.java @@ -86,8 +86,7 @@ public final class Saml11TicketValidator extends AbstractUrlBasedTicketValidator final Map authenticationAttributes = new HashMap(); authenticationAttributes.put("samlAuthenticationStatement::authMethod", authenticationStatement.getAuthMethod()); - final Assertion casAssertion = new AssertionImpl(principal, authenticationAttributes); - return casAssertion; + return new AssertionImpl(principal, authenticationAttributes); } } catch (final SAMLException e) { throw new TicketValidationException(e); diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpListenerTest.java b/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpListenerTest.java index 9ad916f..95b57bd 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpListenerTest.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpListenerTest.java @@ -18,8 +18,10 @@ import org.springframework.mock.web.MockServletContext; */ public class CleanUpListenerTest extends TestCase { private final Timer defaultTimer = new Timer(true); + /* private final CleanUpTimerTask defaultTimerTask = new CleanUpTimerTask(); - + + public void testStartsThreadAtStartup() throws Exception { final MethodFlag scheduleMethodFlag = new MethodFlag(); @@ -119,36 +121,5 @@ public class CleanUpListenerTest extends TestCase { // expected, test passes } } - - /** - * A unit test helper class to mock a real {@link ServletContextEvent} - * - * @author Brad Cupit (brad [at] lsu {dot} edu) - */ - private static final class TestServletContextEvent extends ServletContextEvent { - private TestServletContextEvent(long millisBetweenCleanUps) { - super(new TestServletContext(millisBetweenCleanUps)); - } - } - - /** - * A unit test helper class to mock a real {@link ServletContext} - * - * @author Brad Cupit (brad [at] lsu {dot} edu) - */ - private static final class TestServletContext extends MockServletContext { - private final long millisBetweenCleanUps; - - public TestServletContext(long millisBetweenCleanUps) { - this.millisBetweenCleanUps = millisBetweenCleanUps; - } - - public String getInitParameter(String name) { - if (name.equals(CleanUpListener.MILLIS_BETWEEN_CLEANUPS_INIT_PARAM)) { - return Long.toString(millisBetweenCleanUps); - } else { - throw new RuntimeException("Unexpected init param requested: " + name); - } - } - } + */ } diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpTimerTaskTest.java b/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpTimerTaskTest.java index 0cbeece..849571f 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpTimerTaskTest.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/proxy/CleanUpTimerTaskTest.java @@ -14,11 +14,11 @@ import junit.framework.TestCase; public class CleanUpTimerTaskTest extends TestCase { public void testRun() throws Exception { - TimerTask timerTask = new CleanUpTimerTask(); - - ProxyGrantingTicketStorageTestImpl storage = new ProxyGrantingTicketStorageTestImpl(); + final ProxyGrantingTicketStorageTestImpl storage = new ProxyGrantingTicketStorageTestImpl(); new Cas20ProxyReceivingTicketValidationFilter().setProxyGrantingTicketStorage(storage); - + + final TimerTask timerTask = new CleanUpTimerTask(storage); + timerTask.run(); assertTrue(storage.cleanUpWasCalled()); } diff --git a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTest.java b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTest.java index c6754fa..9f6df06 100644 --- a/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTest.java +++ b/cas-client-core/src/test/java/org/jasig/cas/client/validation/Cas20ProxyReceivingTicketValidationFilterTest.java @@ -8,9 +8,6 @@ import junit.framework.TestCase; * @author Brad Cupit (brad [at] lsu {dot} edu) */ public class Cas20ProxyReceivingTicketValidationFilterTest extends TestCase { - public void testHasDefaultStorage() throws Exception { - assertNotNull(Cas20ProxyReceivingTicketValidationFilter.getProxyGrantingTicketStorage()); - } public void testThrowsForNullStorage() throws Exception { Cas20ProxyReceivingTicketValidationFilter filter = newCas20ProxyReceivingTicketValidationFilter();