From edd29e702665d9e1161b922e3551658d95413a2f Mon Sep 17 00:00:00 2001 From: jeb228 Date: Wed, 9 Mar 2011 22:16:33 +0000 Subject: [PATCH] NIHVIVO-2211 Clean up the logic in RequestPolicyList and the classes that call it. --- .../webapp/auth/AuthorizationHelper.java | 25 +----- .../webapp/auth/policy/RequestPolicyList.java | 82 ++++++++++++------- .../webapp/web/jsptags/PropertyEditLinks.java | 12 +-- .../BaseIndividualTemplateModel.java | 2 +- .../individual/EditingPolicyHelper.java | 31 ++----- 5 files changed, 65 insertions(+), 87 deletions(-) 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 591a5d29c..7e5858d99 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/AuthorizationHelper.java @@ -2,16 +2,12 @@ package edu.cornell.mannlib.vitro.webapp.auth; -import javax.servlet.ServletContext; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; 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.RequestPolicyList; -import edu.cornell.mannlib.vitro.webapp.auth.policy.ServletPolicyList; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.Authorization; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface; @@ -42,27 +38,8 @@ public class AuthorizationHelper { } } - /** - * Get the policy from the request, or from the servlet context. - */ private PolicyIface getPolicies() { - ServletContext servletContext = vreq.getSession().getServletContext(); - - PolicyIface policy = RequestPolicyList.getPolicies(vreq); - if (isEmptyPolicy(policy)) { - policy = ServletPolicyList.getPolicies(servletContext); - } - - return policy; - } - - /** - * Is there actually a policy here? - */ - private boolean isEmptyPolicy(PolicyIface policy) { - return policy == null - || (policy instanceof PolicyList && ((PolicyList) policy) - .size() == 0); + return RequestPolicyList.getPolicies(vreq); } private IdentifierBundle getIdentifiers() { diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RequestPolicyList.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RequestPolicyList.java index fdb363be2..cf344e9e0 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RequestPolicyList.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RequestPolicyList.java @@ -2,7 +2,9 @@ package edu.cornell.mannlib.vitro.webapp.auth.policy; +import javax.servlet.ServletContext; import javax.servlet.ServletRequest; +import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -10,36 +12,60 @@ import org.apache.commons.logging.LogFactory; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface; /** - * This is store and get policies with a Request. + * Allow us to store policies in a Request, in addition to those in the + * ServletContext */ -public class RequestPolicyList extends PolicyList{ - public final static String POLICY_LIST = "PolicyList"; - private static final Log log = LogFactory.getLog( RequestPolicyList.class ); +public class RequestPolicyList extends PolicyList { + private static final String ATTRIBUTE_POLICY_ADDITIONS = RequestPolicyList.class + .getName(); + private static final Log log = LogFactory.getLog(RequestPolicyList.class); - @SuppressWarnings("unchecked") - public static PolicyList getPolicies(ServletRequest request){ - PolicyList list = null; - try{ - list = (PolicyList)request.getAttribute(POLICY_LIST); - }catch(ClassCastException cce){ - log.error(POLICY_LIST +" server context attribute was not of type PolicyList"); - } - if( list == null ){ - list = new RequestPolicyList(); - request.setAttribute(POLICY_LIST, list); - } - return list; - } + /** + * Get a copy of the current list of policies. This includes the policies in + * the ServletContext, followed by any stored in the request. This method may + * return an empty list, but it never returns null. + */ + public static PolicyList getPolicies(HttpServletRequest request) { + ServletContext ctx = request.getSession().getServletContext(); - public static void addPolicy(ServletRequest request, PolicyIface policy){ - PolicyList policies = getPolicies(request); - if( !policies.contains(policy) ){ - policies.add(policy); - log.info("Added policy: " + policy.toString()); - }else{ - log.info("Ignored attempt to add redundent policy."); - } - } - + PolicyList list = ServletPolicyList.getPolicies(ctx); + list.addAll(getPoliciesFromRequest(request)); + return list; + } + public static void addPolicy(ServletRequest request, PolicyIface policy) { + PolicyList policies = getPoliciesFromRequest(request); + if (!policies.contains(policy)) { + policies.add(policy); + log.debug("Added policy: " + policy.toString()); + } else { + log.warn("Ignored attempt to add redundent policy."); + } + } + + /** + * Get the current list of policy additions from the request, or create one + * if there is none. This method may return an empty list, but it never + * returns null. + */ + private static PolicyList getPoliciesFromRequest(ServletRequest request) { + if (request == null) { + throw new NullPointerException("request may not be null."); + } + + Object obj = request.getAttribute(ATTRIBUTE_POLICY_ADDITIONS); + if (obj == null) { + obj = new PolicyList(); + request.setAttribute(ATTRIBUTE_POLICY_ADDITIONS, 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; + } } 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 d7860857d..526628983 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 @@ -88,12 +88,8 @@ public class PropertyEditLinks extends TagSupport{ log.error("item passed to tag is null"); return SKIP_BODY; } - //try the policy in the request first, the look for a policy in the servlet context - //request policy takes precedence - PolicyIface policy = RequestPolicyList.getPolicies(pageContext.getRequest()); - if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ - policy = ServletPolicyList.getPolicies( pageContext.getServletContext() ); - } + + PolicyIface policy = RequestPolicyList.getPolicies((HttpServletRequest)pageContext.getRequest()); IdentifierBundle ids = RequestIdentifiers.getIdBundleForRequest(pageContext.getRequest()); @@ -136,7 +132,7 @@ public class PropertyEditLinks extends TagSupport{ if (data == null) { // link to add a new value links = doVitroNsDataProp( subjectUri, predicateUri, policyToAccess(ids, policy, subjectUri, predicateUri), contextPath ); } else { // links to edit or delete an existing value - DataPropertyStatement dps = (DataPropertyStatement) new DataPropertyStatementImpl(subjectUri, predicateUri, data); + DataPropertyStatement dps = new DataPropertyStatementImpl(subjectUri, predicateUri, data); links = doVitroNsDataPropStmt( dps, entity, policyToAccess(ids, policy, dps), contextPath ); } } else if (FrontEndEditingUtils.isVitroNsObjProp(predicateUri)) { @@ -627,7 +623,7 @@ public class PropertyEditLinks extends TagSupport{ return access; } - public enum EditLinkAccess{ MODIFY, DELETE, ADDNEW, INFO, ADMIN }; + public enum EditLinkAccess{ MODIFY, DELETE, ADDNEW, INFO, ADMIN } public class LinkStruct { String href; diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/BaseIndividualTemplateModel.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/BaseIndividualTemplateModel.java index 22c6b1cc3..f12736ddd 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/BaseIndividualTemplateModel.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/web/templatemodels/individual/BaseIndividualTemplateModel.java @@ -41,7 +41,7 @@ public abstract class BaseIndividualTemplateModel extends BaseTemplateModel { // If editing, create a helper object to check requested actions against policies if (isEditable()) { - policyHelper = new EditingPolicyHelper(vreq, getServletContext()); + policyHelper = new EditingPolicyHelper(vreq); } } 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 0b07be840..e06f43c49 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 @@ -2,16 +2,12 @@ package edu.cornell.mannlib.vitro.webapp.web.templatemodels.individual; -import javax.servlet.ServletContext; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; 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.RequestPolicyList; -import edu.cornell.mannlib.vitro.webapp.auth.policy.ServletPolicyList; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.Authorization; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyDecision; import edu.cornell.mannlib.vitro.webapp.auth.policy.ifaces.PolicyIface; @@ -19,37 +15,20 @@ import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.ifaces.RequestedAct import edu.cornell.mannlib.vitro.webapp.controller.VitroRequest; public class EditingPolicyHelper { - private static final Log log = LogFactory.getLog(EditingPolicyHelper.class); - private VitroRequest vreq; - private ServletContext servletContext; - private PolicyIface policy; - private IdentifierBundle ids; + private final PolicyIface policy; + private final IdentifierBundle ids; - protected EditingPolicyHelper(VitroRequest vreq, ServletContext servletContext) { - this.vreq = vreq; - this.servletContext = servletContext; - setPolicy(); - setIds(); + protected EditingPolicyHelper(VitroRequest vreq) { + this.policy = RequestPolicyList.getPolicies(vreq); + this.ids = RequestIdentifiers.getIdBundleForRequest(vreq); } - private void setPolicy() { - policy = RequestPolicyList.getPolicies(vreq); - if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){ - policy = ServletPolicyList.getPolicies( servletContext ); - } - } - - private void setIds() { - ids = RequestIdentifiers.getIdBundleForRequest(vreq); - } - protected boolean isAuthorizedAction(RequestedAction action) { PolicyDecision decision = getPolicyDecision(action); return (decision != null && decision.getAuthorized() == Authorization.AUTHORIZED); } - private PolicyDecision getPolicyDecision(RequestedAction action) { return policy.isAuthorized(ids, action);