From 1c9b125800534ff146008e78f3b14fda100316de Mon Sep 17 00:00:00 2001 From: jeb228 Date: Wed, 9 Mar 2011 21:49:38 +0000 Subject: [PATCH] NIHVIVO-2211 Clean up the logic in ServletPolicyList and the classes that call it. --- webapp/config/default.log4j.properties | 1 - .../vitro/webapp/auth/AuthTestController.java | 9 +- .../webapp/auth/AuthorizationHelper.java | 5 - .../webapp/auth/policy/JenaNetidPolicy.java | 4 +- .../vitro/webapp/auth/policy/PolicyList.java | 15 +- .../webapp/auth/policy/ServletPolicyList.java | 183 ++++++++++-------- .../webapp/web/jsptags/PropertyEditLinks.java | 4 - .../individual/EditingPolicyHelper.java | 3 - webapp/web/edit/selfeditcheck.jsp | 2 +- 9 files changed, 113 insertions(+), 113 deletions(-) diff --git a/webapp/config/default.log4j.properties b/webapp/config/default.log4j.properties index e739dbf77..48356bd1b 100644 --- a/webapp/config/default.log4j.properties +++ b/webapp/config/default.log4j.properties @@ -33,7 +33,6 @@ log4j.appender.AllAppender.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss,SSS} log4j.rootLogger=INFO, AllAppender -log4j.logger.edu.cornell.mannlib.vitro.webapp.auth.policy.ServletPolicyList=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.controller.freemarker.BrowseController=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.dao.jena.pellet.PelletListener=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.dao.jena.RDBGraphGenerator=WARN diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthTestController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthTestController.java index 0bba54e34..dc6ccf53b 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthTestController.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthTestController.java @@ -14,6 +14,7 @@ import javax.servlet.http.HttpServletResponse; import edu.cornell.mannlib.vitro.webapp.auth.identifier.IdentifierBundle; import edu.cornell.mannlib.vitro.webapp.auth.identifier.RequestIdentifiers; +import edu.cornell.mannlib.vitro.webapp.auth.policy.PolicyList; import edu.cornell.mannlib.vitro.webapp.auth.policy.ServletPolicyList; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision; import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAction; @@ -59,12 +60,12 @@ public class AuthTestController extends VitroHttpServlet { private void checkAuths(ServletOutputStream out, IdentifierBundle ids, ServletContext servletContext) throws IOException{ - ServletPolicyList policy = ServletPolicyList.getPolicies(servletContext); + PolicyList policy = ServletPolicyList.getPolicies(servletContext); out.println("

Authorization tests:

"); - if( policy == null ) { out.println("No Policy objects found in ServletContext. "); - - } + if (policy.isEmpty()) { + out.println("No Policy objects found in ServletContext. "); + } out.println(""); for(RequestedAction action: actions){ out.println(""); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java index 55fe07223..591a5d29c 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java @@ -51,11 +51,6 @@ public class AuthorizationHelper { PolicyIface policy = RequestPolicyList.getPolicies(vreq); if (isEmptyPolicy(policy)) { policy = ServletPolicyList.getPolicies(servletContext); - if (isEmptyPolicy(policy)) { - log.error("No policy found in request at " - + RequestPolicyList.POLICY_LIST); - policy = new PolicyList(); - } } return policy; diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/JenaNetidPolicy.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/JenaNetidPolicy.java index 13e41965f..c797c1557 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/JenaNetidPolicy.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/JenaNetidPolicy.java @@ -375,9 +375,7 @@ public class JenaNetidPolicy extends DefaultInconclusivePolicy implements Visiti log.error("could not get jenaOntModel from JenaBaseDao, JenaNetidPolicy will not work"); } - JenaNetidPolicy jnip = new JenaNetidPolicy(model); - ServletPolicyList spl = ServletPolicyList.getPolicies(sce.getServletContext()); - spl.add(jnip); + ServletPolicyList.addPolicy(sce.getServletContext(), new JenaNetidPolicy(model)); ActiveIdentifierBundleFactories.addFactory(sce, new SelfEditingIdentifierFactory()); }catch(Exception e){ diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyList.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyList.java index e1749e2fc..d05f04bab 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyList.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/PolicyList.java @@ -3,6 +3,7 @@ package edu.cornell.mannlib.vitro.webapp.auth.policy; import java.util.ArrayList; +import java.util.Collection; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -21,10 +22,8 @@ import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAct * and return the first AUTHORIZED or UNAUTHROIZED decision. INCONCLUSIVE * or null decisions will be ignored and the next policy on the list will * be queried. - * * * @author bdc34 - * */ public class PolicyList extends ArrayList implements PolicyIface{ private static final Log log = LogFactory.getLog(PolicyList.class.getName()); @@ -33,7 +32,12 @@ public class PolicyList extends ArrayList implements PolicyIface{ super(); } - public PolicyDecision isAuthorized(IdentifierBundle whoToAuth, RequestedAction whatToAuth) { + public PolicyList(Collection policies) { + super(policies); + } + + @Override + public PolicyDecision isAuthorized(IdentifierBundle whoToAuth, RequestedAction whatToAuth) { PolicyDecision pd = null; for(PolicyIface policy : this){ try{ @@ -43,12 +47,11 @@ public class PolicyList extends ArrayList implements PolicyIface{ break; if( pd.getAuthorized() == Authorization.UNAUTHORIZED ) break; -// if( pd.getAuthorized() == Authorization.INCONCLUSIVE ) -// continue; + // if( pd.getAuthorized() == Authorization.INCONCLUSIVE ) + // continue; } else{ log.debug("policy " + policy.toString() + " returned a null PolicyDecision"); } - }catch(Throwable th){ log.error("ignoring exception in policy " + policy.toString(), th ); } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/ServletPolicyList.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/ServletPolicyList.java index c6300d9bb..bf5624945 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/ServletPolicyList.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/ServletPolicyList.java @@ -11,94 +11,105 @@ import org.apache.commons.logging.LogFactory; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface; - /** - * This is a PolicyList that can do isAuthorized and stashes a singleton - * in the ServletContext. - * - * The intent of this class is to allow a single point for policies - * in a ServletContext. example: - * - * Authorization canIDoIt = ServletPolicyList.getPolicies( getServletContext() ).isAuthorized( IdBundle, action ); - * - * - * @author bdc34 - * + * This maintains a PolicyList in the ServletContext. As a rule, however, this + * is only used as the basis for the RequestPolicyList. Client code that wants + * to access the current list of policies should look there. */ -public class ServletPolicyList extends PolicyList { - protected static String POLICY_LIST = "policy_list"; - private static final Log log = LogFactory.getLog(ServletPolicyList.class.getName()); +public class ServletPolicyList { + private static final String ATTRIBUTE_POLICY_LIST = ServletPolicyList.class.getName(); + private static final Log log = LogFactory.getLog(ServletPolicyList.class); - /** - * This is for general public use to get a list of policies for the ServletContext. - * @param sc - * @return - */ - @SuppressWarnings("unchecked") - public static ServletPolicyList getPolicies(ServletContext sc){ - ServletPolicyList list = null; - try{ - list = (ServletPolicyList)sc.getAttribute(POLICY_LIST); - }catch(ClassCastException cce){ - log.error(POLICY_LIST +" server context attribute was not of type List"); - } - if( list == null ){ - list = new ServletPolicyList(); - sc.setAttribute(POLICY_LIST, list); - } - return list; - } + /** + * Get a copy of the current list of policies. This method may return an + * empty list, but it never returns null. + */ + public static PolicyList getPolicies(ServletContext sc) { + return new PolicyList(getPolicyList(sc)); + } + + /** + * Add the policy to the end of the list. + */ + public static void addPolicy(ServletContext sc, PolicyIface policy) { + if (policy == null) { + return; + } + + PolicyList policies = getPolicyList(sc); + if (!policies.contains(policy)) { + policies.add(policy); + log.info("Added policy: " + policy.getClass().getSimpleName()); + log.debug("Added policy: " + policy.toString()); + } else { + log.warn("Ignored attempt to add redundant policy."); + } + } + + /** + * Add the policy to the front of the list. It may be moved further down the + * list by other policies that are later added using this method. + */ + public static void addPolicyAtFront(ServletContext sc, PolicyIface policy) { + if (policy == null) { + return; + } + + PolicyList policies = getPolicyList(sc); + if (!policies.contains(policy)) { + policies.add(0, policy); + log.info("Added policy at front: " + policy.getClass().getSimpleName()); + log.debug("Added policy at front: " + policy.toString()); + } else { + log.warn("Ignored attempt to add redundant policy."); + } + } + + /** + * Replace the first instance of this class of policy in the list. If no + * instance is found, add the policy to the end of the list. + */ + public static void replacePolicy(ServletContext sc, PolicyIface policy) { + if (policy == null) { + return; + } + + Class clzz = policy.getClass(); + PolicyList policies = getPolicyList(sc); + ListIterator it = policies.listIterator(); + while (it.hasNext()) { + if (clzz.isAssignableFrom(it.next().getClass())) { + it.set(policy); + return; + } + } + + addPolicy(sc, policy); + } + + /** + * Get the current PolicyList from the context, or create one if there is + * none. This method may return an empty list, but it never returns null. + */ + private static PolicyList getPolicyList(ServletContext ctx) { + if (ctx == null) { + throw new NullPointerException("ctx may not be null."); + } + + Object obj = ctx.getAttribute(ATTRIBUTE_POLICY_LIST); + if (obj == null) { + obj = new PolicyList(); + ctx.setAttribute(ATTRIBUTE_POLICY_LIST, obj); + } + + if (!(obj instanceof PolicyList)) { + throw new IllegalStateException("Expected to find an instance of " + + PolicyList.class.getName() + + " in the context, but found an instance of " + + obj.getClass().getName() + " instead."); + } + + return (PolicyList) obj; + } - public static void addPolicy(ServletContext sc, PolicyIface policy){ - ServletPolicyList policies = getPolicies(sc); - if( !policies.contains(policy) ){ - policies.add(policy); - log.info("Added policy: " + policy.toString()); - }else{ - log.info("Ignored attempt to add redundent policy."); - } - } - - /** - * This adds the policy to the front of the list but it may be moved further down - * the list by other policies that are later added using this method. - */ - public static void addPolicyAtFront(ServletContext sc, PolicyIface policy){ - ServletPolicyList policies = getPolicies(sc); - if( !policies.contains(policy) ){ - policies.add(0,policy); - log.info("Added policy at front of ServletPolicyList: " + policy.toString()); - }else{ - log.info("Ignored attempt to add redundent policy."); - } - } - - /** import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.VisitingPolicyIface; - * Replace first instance of policy found in policy list. If no instance - * is found in list add at end of the list. - * - * @param sc - * @param policy - */ - public static void replacePolicy(ServletContext sc, PolicyIface policy){ - if( sc == null ) - throw new IllegalArgumentException( "replacePolicy() needs a non-null ServletContext"); - if( policy == null ) - return; - Class clzz = policy.getClass(); - - ServletPolicyList spl = ServletPolicyList.getPolicies(sc); - ListIterator it = spl.listIterator(); - boolean replaced = false; - while(it.hasNext()){ - PolicyIface p = (PolicyIface)it.next(); - if( clzz.isAssignableFrom(p.getClass()) ){ - it.set( policy ); - replaced = true; - } - } - if( ! replaced ){ - ServletPolicyList.addPolicy(sc, policy); - } - } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/PropertyEditLinks.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/PropertyEditLinks.java index 65cb99ba7..d7860857d 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/PropertyEditLinks.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/jsptags/PropertyEditLinks.java @@ -93,10 +93,6 @@ public class PropertyEditLinks extends TagSupport{ PolicyIface policy = RequestPolicyList.getPolicies(pageContext.getRequest()); if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ policy = ServletPolicyList.getPolicies( pageContext.getServletContext() ); - if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ - log.error("No policy found in request at " + RequestPolicyList.POLICY_LIST); - return SKIP_BODY; - } } IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(pageContext.getRequest()); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/EditingPolicyHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/EditingPolicyHelper.java index fd91e4380..0b07be840 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/EditingPolicyHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/EditingPolicyHelper.java @@ -38,9 +38,6 @@ public class EditingPolicyHelper { policy = RequestPolicyList.getPolicies(vreq); if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ policy = ServletPolicyList.getPolicies( servletContext ); - if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ - log.error("No policy found in request at " + RequestPolicyList.POLICY_LIST); - } } } diff --git a/webapp/web/edit/selfeditcheck.jsp b/webapp/web/edit/selfeditcheck.jsp index a5ff562e8..c7d277fec 100644 --- a/webapp/web/edit/selfeditcheck.jsp +++ b/webapp/web/edit/selfeditcheck.jsp @@ -28,7 +28,7 @@

Is there a self editing policy in the context?

<% -ServletPolicyList spl = ServletPolicyList.getPolicies(application); +PolicyList spl = ServletPolicyList.getPolicies(application); SelfEditingPolicy sePolicy = null; ListIterator it = spl.listIterator(); String found = "Could not find a SelfEditingPolicy";
"+action.getClass().getName()+"