From dcb358d4c8b1dac7492e474d708cb8c257f73b5e Mon Sep 17 00:00:00 2001 From: jeb228 Date: Fri, 5 Nov 2010 15:26:22 +0000 Subject: [PATCH] NIHVIVO-1207 Make LoginProcessBean more encapsulated. --- .../webapp/controller/edit/Authenticate.java | 30 +++---- .../vitro/webapp/controller/edit/Login.java | 2 +- .../controller/login/LoginProcessBean.java | 84 ++++++++++++++++--- .../controller/login/LoginTemplateHelper.java | 24 +----- .../controller/edit/AuthenticateTest.java | 26 +++--- 5 files changed, 103 insertions(+), 63 deletions(-) diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java index 28c599c7f..4cf491cd7 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java @@ -130,7 +130,7 @@ public class Authenticate extends FreemarkerHttpServlet { String username = request.getParameter(PARAMETER_USERNAME); String password = request.getParameter(PARAMETER_PASSWORD); - LoginProcessBean bean = getLoginProcessBean(request); + LoginProcessBean bean = LoginProcessBean.getBean(request); bean.clearMessage(); log.trace("username=" + username + ", password=" + password + ", bean=" + bean); @@ -170,7 +170,7 @@ public class Authenticate extends FreemarkerHttpServlet { private void whatNextForThisGuy(HttpServletRequest request, User user) { if (user.getLoginCount() == 0) { log.debug("Forcing first-time password change"); - LoginProcessBean bean = getLoginProcessBean(request); + LoginProcessBean bean = LoginProcessBean.getBean(request); bean.setState(State.FORCED_PASSWORD_CHANGE); } else { recordLoginInfo(request, user.getUsername()); @@ -191,7 +191,7 @@ public class Authenticate extends FreemarkerHttpServlet { * If they want to cancel the login, let them. */ private void recordLoginCancelled(HttpServletRequest request) { - getLoginProcessBean(request).setState(State.CANCELLED); + LoginProcessBean.getBean(request).setState(State.CANCELLED); } /** @@ -200,7 +200,7 @@ public class Authenticate extends FreemarkerHttpServlet { private User checkChangeProgress(HttpServletRequest request) { String newPassword = request.getParameter(PARAMETER_NEW_PASSWORD); String confirm = request.getParameter(PARAMETER_CONFIRM_PASSWORD); - LoginProcessBean bean = getLoginProcessBean(request); + LoginProcessBean bean = LoginProcessBean.getBean(request); bean.clearMessage(); log.trace("newPassword=" + newPassword + ", confirm=" + confirm + ", bean=" + bean); @@ -261,8 +261,7 @@ public class Authenticate extends FreemarkerHttpServlet { getAuthenticator(request).setLoggedIn(user); // Remove the login process info from the session. - request.getSession() - .removeAttribute(LoginProcessBean.SESSION_ATTRIBUTE); + LoginProcessBean.removeBean(request); } /** @@ -280,10 +279,8 @@ public class Authenticate extends FreemarkerHttpServlet { */ private void redirectCancellingUser(HttpServletRequest request, HttpServletResponse response) throws IOException { - request.getSession() - .removeAttribute(LoginProcessBean.SESSION_ATTRIBUTE); - log.debug("User cancelled the login. Redirect to site admin page."); + LoginProcessBean.removeBean(request); response.sendRedirect(getHomeUrl(request)); } @@ -384,15 +381,15 @@ public class Authenticate extends FreemarkerHttpServlet { return State.LOGGED_IN; } - if (session.getAttribute(LoginProcessBean.SESSION_ATTRIBUTE) == null) { + if (LoginProcessBean.isBean(request)) { + State state = LoginProcessBean.getBean(request).getState(); + log.debug("state from LoginProcessBean is " + state); + return state; + } else { log.debug("no LoginSessionBean, no LoginProcessBean: " + "current state is NOWHERE"); return State.NOWHERE; } - - State state = getLoginProcessBean(request).getState(); - log.debug("state from LoginProcessBean is " + state); - return state; } /** @@ -432,11 +429,6 @@ public class Authenticate extends FreemarkerHttpServlet { return request.getContextPath(); } - /** Where do we stand in the login process? */ - private LoginProcessBean getLoginProcessBean(HttpServletRequest request) { - return LoginProcessBean.getBeanFromSession(request); - } - // ---------------------------------------------------------------------- // Public utility methods. // ---------------------------------------------------------------------- diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Login.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Login.java index 9b58b43d6..a71a67c0f 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Login.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Login.java @@ -53,7 +53,7 @@ public class Login extends HttpServlet { * Otherwise, set up as if they had filled in the login form, and send * them to authenticate it. */ - LoginProcessBean bean = LoginProcessBean.getBeanFromSession(request); + LoginProcessBean bean = LoginProcessBean.getBean(request); bean.setState(LoginProcessBean.State.LOGGING_IN); request.getRequestDispatcher(Controllers.AUTHENTICATE).forward(request, response); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginProcessBean.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginProcessBean.java index 34a47b4ba..693943425 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginProcessBean.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginProcessBean.java @@ -8,31 +8,91 @@ import java.util.Arrays; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + /** * Where are we in the process of logging on? What message should we show to the * user? */ public class LoginProcessBean { + private static final Log log = LogFactory.getLog(LoginProcessBean.class); + private static Object[] NO_ARGUMENTS = new Object[0]; - public static final String SESSION_ATTRIBUTE = LoginProcessBean.class + private static final String SESSION_ATTRIBUTE = LoginProcessBean.class .getName(); + // ---------------------------------------------------------------------- + // static methods + // ---------------------------------------------------------------------- + /** - * Get the login process bean from the session. If there is none, create + * Is there currently a login process bean in the session? + */ + public static boolean isBean(HttpServletRequest request) { + return (null != getBeanFromSession(request)); + } + + /** + * Get the login process bean from the session. If there is no bean, create * one. */ - public static LoginProcessBean getBeanFromSession(HttpServletRequest request) { - HttpSession session = request.getSession(); - LoginProcessBean bean = (LoginProcessBean) session - .getAttribute(SESSION_ATTRIBUTE); - if (bean == null) { - bean = new LoginProcessBean(); - session.setAttribute(SESSION_ATTRIBUTE, bean); + public static LoginProcessBean getBean(HttpServletRequest request) { + if (isBean(request)) { + return getBeanFromSession(request); + } else { + setBean(request, new LoginProcessBean()); + return getBeanFromSession(request); } - return bean; } + /** + * Store this login process bean in the session. + */ + public static void setBean(HttpServletRequest request, LoginProcessBean bean) { + HttpSession session = request.getSession(); + session.setAttribute(SESSION_ATTRIBUTE, bean); + } + + /** + * Remove the login process bean from the session. If there is no bean, do + * nothing. + */ + public static void removeBean(HttpServletRequest request) { + if (isBean(request)) { + request.getSession().removeAttribute(SESSION_ATTRIBUTE); + } + } + + /** + * Get the bean from the session, or null if there is no bean. + */ + private static LoginProcessBean getBeanFromSession( + HttpServletRequest request) { + HttpSession session = request.getSession(false); + if (session == null) { + return null; + } + + Object bean = session.getAttribute(SESSION_ATTRIBUTE); + if (bean == null) { + return null; + } + + if (!(bean instanceof LoginProcessBean)) { + log.warn("Tried to get login process bean, but found an instance of " + + bean.getClass().getName() + ": " + bean); + return null; + } + + return (LoginProcessBean) bean; + } + + // ---------------------------------------------------------------------- + // helper classes + // ---------------------------------------------------------------------- + public enum State { NOWHERE, LOGGING_IN, FORCED_PASSWORD_CHANGE, CANCELLED, LOGGED_IN } @@ -89,6 +149,10 @@ public class LoginProcessBean { } } + // ---------------------------------------------------------------------- + // the bean + // ---------------------------------------------------------------------- + /** Where are we in the process? */ private State currentState = State.NOWHERE; diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginTemplateHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginTemplateHelper.java index 83c2f5c43..0a98f48aa 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginTemplateHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/login/LoginTemplateHelper.java @@ -7,7 +7,6 @@ import java.util.HashMap; import java.util.Map; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -101,7 +100,7 @@ public class LoginTemplateHelper extends LoginTemplateHelperBase { */ private TemplateResponseValues showLoginScreen(VitroRequest vreq) throws IOException { - LoginProcessBean bean = getLoginProcessBean(vreq); + LoginProcessBean bean = LoginProcessBean.getBean(vreq); bean.setState(State.LOGGING_IN); log.trace("Going to login screen: " + bean); @@ -126,7 +125,7 @@ public class LoginTemplateHelper extends LoginTemplateHelperBase { * change it (unless they cancel out). */ private TemplateResponseValues showPasswordChangeScreen(VitroRequest vreq) { - LoginProcessBean bean = getLoginProcessBean(vreq); + LoginProcessBean bean = LoginProcessBean.getBean(vreq); bean.setState(State.FORCED_PASSWORD_CHANGE); log.trace("Going to password change screen: " + bean); @@ -173,27 +172,10 @@ public class LoginTemplateHelper extends LoginTemplateHelperBase { if (LoginStatusBean.getBean(request).isLoggedIn()) { return State.LOGGED_IN; } else { - return getLoginProcessBean(request).getState(); + return LoginProcessBean.getBean(request).getState(); } } - /** - * How is the login process coming along? - */ - private LoginProcessBean getLoginProcessBean(HttpServletRequest request) { - HttpSession session = request.getSession(); - - LoginProcessBean bean = (LoginProcessBean) session - .getAttribute(LoginProcessBean.SESSION_ATTRIBUTE); - - if (bean == null) { - bean = new LoginProcessBean(); - session.setAttribute(LoginProcessBean.SESSION_ATTRIBUTE, bean); - } - - return bean; - } - /** What's the URL for this servlet? */ private String getAuthenticateUrl(HttpServletRequest request) { String contextPath = request.getContextPath(); diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java index e7091ae86..4d5fd0f98 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java @@ -5,9 +5,8 @@ package edu.cornell.mannlib.vitro.webapp.controller.edit; import static edu.cornell.mannlib.vitro.webapp.controller.login.LoginProcessBean.State.FORCED_PASSWORD_CHANGE; import static edu.cornell.mannlib.vitro.webapp.controller.login.LoginProcessBean.State.LOGGING_IN; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.fail; -import java.lang.reflect.Field; import java.net.URL; import java.util.Arrays; import java.util.Date; @@ -28,7 +27,6 @@ import edu.cornell.mannlib.vedit.beans.LoginStatusBean; import edu.cornell.mannlib.vitro.testing.AbstractTestClass; import edu.cornell.mannlib.vitro.webapp.beans.User; import edu.cornell.mannlib.vitro.webapp.controller.Controllers; -import edu.cornell.mannlib.vitro.webapp.controller.authenticate.Authenticator; import edu.cornell.mannlib.vitro.webapp.controller.authenticate.AuthenticatorStub; import edu.cornell.mannlib.vitro.webapp.controller.login.LoginProcessBean; import edu.cornell.mannlib.vitro.webapp.controller.login.LoginProcessBean.State; @@ -331,14 +329,14 @@ public class AuthenticateTest extends AbstractTestClass { private void setProcessBean(State state) { LoginProcessBean processBean = new LoginProcessBean(); processBean.setState(state); - session.setAttribute(LoginProcessBean.SESSION_ATTRIBUTE, processBean); + LoginProcessBean.setBean(request, processBean); } private void setProcessBean(State state, String username) { LoginProcessBean processBean = new LoginProcessBean(); processBean.setState(state); processBean.setUsername(username); - session.setAttribute(LoginProcessBean.SESSION_ATTRIBUTE, processBean); + LoginProcessBean.setBean(request, processBean); } private void setLoginNameAndPassword(String loginName, String password) { @@ -363,15 +361,18 @@ public class AuthenticateTest extends AbstractTestClass { } private void assertNoProcessBean() { - assertEquals("null process bean", null, - session.getAttribute(LoginProcessBean.SESSION_ATTRIBUTE)); + if (LoginProcessBean.isBean(request)) { + fail("Process bean: expected , but was <" + + LoginProcessBean.getBean(request) + ">"); + } } private void assertExpectedProcessBean(State state, String username, String infoMessage, String errorMessage) { - LoginProcessBean bean = (LoginProcessBean) session - .getAttribute(LoginProcessBean.SESSION_ATTRIBUTE); - assertNotNull("login process bean", bean); + if (!LoginProcessBean.isBean(request)) { + fail("login process bean is null"); + } + LoginProcessBean bean = LoginProcessBean.getBean(request); assertEquals("state", state, bean.getState()); assertEquals("info message", infoMessage, bean.getInfoMessage()); assertEquals("error message", errorMessage, bean.getErrorMessage()); @@ -419,9 +420,10 @@ public class AuthenticateTest extends AbstractTestClass { @SuppressWarnings("unused") private void showBeans() { - LoginProcessBean processBean = (LoginProcessBean) session - .getAttribute(LoginProcessBean.SESSION_ATTRIBUTE); + LoginProcessBean processBean = (LoginProcessBean.isBean(request)) ? LoginProcessBean + .getBean(request) : null; System.out.println("LoginProcessBean=" + processBean); + LoginStatusBean statusBean = (LoginStatusBean) session .getAttribute("loginStatus"); System.out.println("LoginStatusBean=" + statusBean);