NIHVIVO-2211 Clean up the logic in RequestPolicyList and the classes that call it.

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();
+        return RequestPolicyList.getPolicies(vreq);
-
-        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);
     }
 
     private IdentifierBundle getIdentifiers() {

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 class RequestPolicyList extends PolicyList {
-    public final static String POLICY_LIST = "PolicyList";
+	private static final String ATTRIBUTE_POLICY_ADDITIONS = RequestPolicyList.class
-    private static final Log log = LogFactory.getLog( RequestPolicyList.class );
+			.getName();
+	private static final Log log = LogFactory.getLog(RequestPolicyList.class);
 
-    @SuppressWarnings("unchecked")
+	/**
-    public static PolicyList getPolicies(ServletRequest request){
+	 * Get a copy of the current list of policies. This includes the policies in
-        PolicyList list  = null;
+	 * the ServletContext, followed by any stored in the request. This method may
-        try{
+	 * return an empty list, but it never returns null.
-            list = (PolicyList)request.getAttribute(POLICY_LIST);
+	 */
-        }catch(ClassCastException cce){
+	public static PolicyList getPolicies(HttpServletRequest request) {
-            log.error(POLICY_LIST +" server context attribute was not of type PolicyList");
+		ServletContext ctx = request.getSession().getServletContext();
-        }
-        if( list == null ){
-            list = new RequestPolicyList();
-            request.setAttribute(POLICY_LIST, list);
-        }
-        return list;
-    }
 
-    public static void addPolicy(ServletRequest request, PolicyIface policy){
+		PolicyList list = ServletPolicyList.getPolicies(ctx);
-        PolicyList policies = getPolicies(request);
+		list.addAll(getPoliciesFromRequest(request));
-        if( !policies.contains(policy) ){
+		return list;
-            policies.add(policy);
+	}
-            log.info("Added policy: " + policy.toString());
-        }else{
-            log.info("Ignored attempt to add redundent policy.");
-        }
-    }
-    
 
+	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;
+	}
 }

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 <edLnk> 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((HttpServletRequest)pageContext.getRequest());
-        PolicyIface policy = RequestPolicyList.getPolicies(pageContext.getRequest());
-        if( policy == null || ( policy instanceof PolicyList && ((PolicyList)policy).size() == 0 )){
-            policy = ServletPolicyList.getPolicies( pageContext.getServletContext() );
-        }              
         
 		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;

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);
         } 
     }
     

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 final PolicyIface policy;
-    private ServletContext servletContext;
+    private final IdentifierBundle ids;
-    private PolicyIface policy;
-    private IdentifierBundle ids;
     
-    protected EditingPolicyHelper(VitroRequest vreq, ServletContext servletContext) {
+    protected EditingPolicyHelper(VitroRequest vreq) {
-        this.vreq = vreq;
+        this.policy = RequestPolicyList.getPolicies(vreq);
-        this.servletContext = servletContext;
+        this.ids = RequestIdentifiers.getIdBundleForRequest(vreq);
-        setPolicy();
-        setIds();
     }
     
-    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);