Address code review comments

From: - Vitro: vivo-project#80 - VIVO: vivo-project/VIVO#85 Resolves: https://github.com/vivo-project/Vitro/pull/80/files#r227428235 https://github.com/vivo-project/Vitro/pull/80/files#r227892906 https://github.com/vivo-project/Vitro/pull/80/files#r227905400 https://github.com/vivo-project/Vitro/pull/80/files#r227907884 https://github.com/vivo-project/Vitro/pull/80/files#r227909775 https://github.com/vivo-project/Vitro/pull/80/files#r227913953 https://github.com/vivo-project/Vitro/pull/80/files#r227913145 https://github.com/vivo-project/Vitro/pull/80/files#r227913209 https://github.com/vivo-project/Vitro/pull/80/files#r227914673 https://github.com/vivo-project/Vitro/pull/80/files#r227915444 https://github.com/vivo-project/Vitro/pull/80/files#r227960329 Not addresses: https://github.com/vivo-project/Vitro/pull/80/files#r227429141 https://github.com/vivo-project/Vitro/pull/80/files#r227434076
awoods · Nov 20, 2018 · 64b0a8c · 64b0a8c
1 parent 5624222
commit 64b0a8c
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 86 deletions.
diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/EntityPermission.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/EntityPermission.java
@@ -52,7 +52,6 @@ protected EntityPermission(String uri) {
      * Instance fields for each EntityPermission
      */
     private final Set<PropertyDao.FullPropertyKey> authorizedKeys = new HashSet<>();
-    private final Set<String> authorizedResources = new HashSet<>();
     private boolean limitToRelatedUser = false;
 
     public static List<EntityPermission> getAllInstances(ContextModelAccess models) {
@@ -72,7 +71,7 @@ public static List<EntityPermission> getAllInstances(ContextModelAccess models)
             updateAllPermissions();
         }
 
-        return new ArrayList<EntityPermission>(allInstances.values());
+        return new ArrayList<>(allInstances.values());
     }
 
     private static void getAllInstances(Class<? extends EntityPermission> clazz) {
@@ -176,12 +175,6 @@ private void update(Map<String, PropertyDao.FullPropertyKey> propertyKeyMap) {
             authorizedKeys.clear();
             authorizedKeys.addAll(newKeys);
         }
-
-        // replace authorized resources
-        synchronized (authorizedResources) {
-            authorizedResources.clear();
-            authorizedResources.addAll(newResources);
-        }
     }
 
     private void updateFor(Property p) {
@@ -206,17 +199,10 @@ private void updateFor(Property p) {
                     authorizedKeys.add(key);
                 }
 
-                synchronized (authorizedResources) {
-                    authorizedResources.add(uri);
-                }
             } else {
                 synchronized (authorizedKeys) {
                     authorizedKeys.remove(key);
                 }
-
-                synchronized (authorizedResources) {
-                    authorizedResources.remove(uri);
-                }
             }
         } finally {
             accountsModel.leaveCriticalSection();

diff --git a/...c/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/EntityUpdatePermission.java b/...c/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/EntityUpdatePermission.java
@@ -21,14 +21,14 @@ public class EntityUpdatePermission extends EntityPermission {
     private static final Log log = LogFactory.getLog(EntityUpdatePermission.class);
 
     private static final Collection<String> PROHIBITED_NAMESPACES = Arrays
-            .asList(new String[] { VitroVocabulary.vitroURI, "" });
+            .asList(VitroVocabulary.vitroURI, "" );
 
     private static final Collection<String> PERMITTED_EXCEPTIONS = Arrays
-            .asList(new String[] { VitroVocabulary.MONIKER,
+            .asList(VitroVocabulary.MONIKER,
                     VitroVocabulary.MODTIME, VitroVocabulary.IND_MAIN_IMAGE,
                     VitroVocabulary.LINK, VitroVocabulary.PRIMARY_LINK,
                     VitroVocabulary.ADDITIONAL_LINK,
-                    VitroVocabulary.LINK_ANCHOR, VitroVocabulary.LINK_URL });
+                    VitroVocabulary.LINK_ANCHOR, VitroVocabulary.LINK_URL );
 
     public EntityUpdatePermission(String uri) {
         super(uri);
@@ -65,7 +65,7 @@ public boolean isAuthorized(List<String> personUris, RequestedAction whatToAuth)
                 isAuthorized = isAuthorizedFor((AbstractPropertyStatementAction) whatToAuth, personUris);
             }
         } else if (whatToAuth instanceof AbstractResourceAction) {
-            String subjectUri = ((AbstractObjectPropertyStatementAction)whatToAuth).getSubjectUri();
+            String subjectUri = ((AbstractResourceAction)whatToAuth).getSubjectUri();
             isAuthorized = isModifiable(subjectUri);
         }
 

diff --git a/api/src/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/Permission.java b/api/src/main/java/edu/cornell/mannlib/vitro/webapp/auth/permissions/Permission.java
@@ -66,16 +66,4 @@ public String toString() {
 		return this.getClass().getSimpleName() + "['" + uri + "']";
 	}
 
-	/**
-	 * A concrete Permission instance that authorizes nothing.
-	 */
-	static Permission NOT_AUTHORIZED = new Permission("java:"
-			+ Permission.class.getName() + "#NOT_AUTHORIZED") {
-
-		@Override
-		public boolean isAuthorized(List<String> personUris, RequestedAction whatToAuth) {
-			return false;
-		}
-
-	};
 }
diff --git a/...du/cornell/mannlib/vitro/webapp/auth/policy/specialrelationships/RelationshipChecker.java b/...du/cornell/mannlib/vitro/webapp/auth/policy/specialrelationships/RelationshipChecker.java
@@ -39,15 +39,12 @@ public abstract class RelationshipChecker {
 	protected static final String URI_INHERES_IN = NS_OBO + "RO_0000052";
 	protected static final String URI_REALIZES = NS_OBO + "BFO_0000055";
 
-	public RelationshipChecker() {
-	}
-
 	public abstract boolean isRelated(OntModel ontModel, List<String> fromUris, List<String> toUris);
 
 	/**
 	 * Are there any URIs that appear in both of these lists?
 	 */
-	public boolean anyUrisInCommon(OntModel ontModel, List<String> list1, List<String> list2) {
+	protected boolean anyUrisInCommon(List<String> list1, List<String> list2) {
 		List<String> urisInCommon = new ArrayList<String>(list1);
 		urisInCommon.retainAll(list2);
 		return !urisInCommon.isEmpty();
@@ -57,7 +54,7 @@ public boolean anyUrisInCommon(OntModel ontModel, List<String> list1, List<Strin
 	 * Is this resource a member of this type? That is, is there an statement of
 	 * the form: {@code <resourceUri> rdfs:type <typeUri> }
 	 */
-	public boolean isResourceOfType(OntModel ontModel, String resourceUri, String typeUri) {
+	protected boolean isResourceOfType(OntModel ontModel, String resourceUri, String typeUri) {
 		Selector selector = createSelector(ontModel, resourceUri,
 				VitroVocabulary.RDF_TYPE, typeUri);
 
@@ -89,7 +86,7 @@ public boolean isResourceOfType(OntModel ontModel, String resourceUri, String ty
 	 * 
 	 * May return an empty list, but never returns null.
 	 */
-	public List<String> getObjectsOfProperty(OntModel ontModel, String resourceUri,
+	protected List<String> getObjectsOfProperty(OntModel ontModel, String resourceUri,
 			String propertyUri) {
 		List<String> list = new ArrayList<String>();
 
@@ -113,46 +110,6 @@ public List<String> getObjectsOfProperty(OntModel ontModel, String resourceUri,
 		}
 	}
 
-	/**
-	 * Get a list of the object URIs that satisfy these statements:
-	 * 
-	 * {@code <resourceUri> <linkUri> <contextNodeUri> }
-	 * 
-	 * {@code <contextNodeUri> <propertyUri> <objectUri> }
-	 * 
-	 * May return an empty list, but never returns null.
-	 */
-	public List<String> getObjectsOfLinkedProperty(OntModel ontModel, String resourceUri,
-			String linkUri, String propertyUri) {
-		List<String> list = new ArrayList<String>();
-
-		Selector selector = createSelector(ontModel, resourceUri, linkUri, null);
-
-		StmtIterator stmts = null;
-
-		ontModel.enterCriticalSection(Lock.READ);
-		try {
-			stmts = ontModel.listStatements(selector);
-			while (stmts.hasNext()) {
-				RDFNode contextNode = stmts.next().getObject();
-				if (contextNode.isResource()) {
-					log.debug("found context node for '" + resourceUri + "': "
-							+ contextNode);
-					list.addAll(getObjectsOfProperty(ontModel, contextNode.asResource()
-							.getURI(), propertyUri));
-				}
-			}
-			log.debug("Objects of linked properties '" + linkUri + "' ==> '"
-					+ propertyUri + "' on '" + resourceUri + "': " + list);
-			return list;
-		} finally {
-			if (stmts != null) {
-				stmts.close();
-			}
-			ontModel.leaveCriticalSection();
-		}
-	}
-
 	/**
 	 * Get a list of URIs for object that link to the specified resource, by
 	 * means of the specified properties, through a linking node of the
@@ -166,7 +123,7 @@ public List<String> getObjectsOfLinkedProperty(OntModel ontModel, String resourc
 	 * 
 	 * {@code <linkNodeUri> <property2Uri> <objectUri> }
 	 */
-	public List<String> getObjectsThroughLinkingNode(OntModel ontModel, String resourceUri,
+	protected List<String> getObjectsThroughLinkingNode(OntModel ontModel, String resourceUri,
 			String property1Uri, String linkNodeTypeUri, String property2Uri) {
 		List<String> list = new ArrayList<String>();
 
@@ -179,14 +136,14 @@ public List<String> getObjectsThroughLinkingNode(OntModel ontModel, String resou
 		return list;
 	}
 
-	public Selector createSelector(OntModel ontModel, String subjectUri, String predicateUri,
+	private Selector createSelector(OntModel ontModel, String subjectUri, String predicateUri,
 			String objectUri) {
 		Resource subject = (subjectUri == null) ? null : ontModel
 				.getResource(subjectUri);
 		return createSelector(ontModel, subject, predicateUri, objectUri);
 	}
 
-	public Selector createSelector(OntModel ontModel, Resource subject, String predicateUri,
+	private Selector createSelector(OntModel ontModel, Resource subject, String predicateUri,
 			String objectUri) {
 		Property predicate = (predicateUri == null) ? null : ontModel
 				.getProperty(predicateUri);

diff --git a/...ava/edu/cornell/mannlib/vitro/webapp/dao/filtering/filters/FilterByDisplayPermission.java b/...ava/edu/cornell/mannlib/vitro/webapp/dao/filtering/filters/FilterByDisplayPermission.java
@@ -5,22 +5,16 @@
 import static edu.cornell.mannlib.vitro.webapp.auth.requestedAction.RequestedAction.SOME_URI;
 
 import javax.servlet.ServletContext;
-import javax.servlet.http.HttpServletRequest;
 
-import edu.cornell.mannlib.vitro.webapp.auth.identifier.common.HasPermissionSet;
 import edu.cornell.mannlib.vitro.webapp.auth.permissions.EntityDisplayPermission;
 import edu.cornell.mannlib.vitro.webapp.beans.*;
-import edu.cornell.mannlib.vitro.webapp.dao.UserAccountsDao;
 import edu.cornell.mannlib.vitro.webapp.dao.WebappDaoFactory;
 import edu.cornell.mannlib.vitro.webapp.modelaccess.ModelAccess;
 import net.sf.jga.fn.UnaryFunctor;
 
 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.identifier.common.HasPermission;
 import edu.cornell.mannlib.vitro.webapp.auth.permissions.Permission;
 import edu.cornell.mannlib.vitro.webapp.auth.permissions.PermissionRegistry;
 import edu.cornell.mannlib.vitro.webapp.auth.requestedAction.RequestedAction;