SAK-19419 findbugs static code review for common

git-svn-id: https://source.sakaiproject.org/svn/common/trunk@84140 66ffb92e-73f9-0310-93c1-f5514f145a0a
abdobaz · Nov 2, 2010 · a291482 · a291482
1 parent 188afd5
commit a291482
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 32 deletions.
diff --git a/common/archive-impl/impl/src/java/org/sakaiproject/archive/impl/BasicArchiveService.java b/common/archive-impl/impl/src/java/org/sakaiproject/archive/impl/BasicArchiveService.java
@@ -36,6 +36,7 @@
 import org.apache.commons.logging.LogFactory;
 import org.sakaiproject.archive.api.ArchiveService;
 import org.sakaiproject.authz.api.AuthzGroup;
+import org.sakaiproject.authz.api.AuthzPermissionException;
 import org.sakaiproject.authz.api.GroupNotDefinedException;
 import org.sakaiproject.authz.api.Role;
 import org.sakaiproject.authz.cover.AuthzGroupService;
@@ -79,9 +80,9 @@ public class BasicArchiveService
 	/** A full path and file name to the storage file. */
 	protected String m_storagePath = "/";
 
-	protected static HashMap userIdTrans = new HashMap();
+	protected final static HashMap userIdTrans = new HashMap();
 
-	protected HashSet UsersListAllowImport = new HashSet(); 
+	protected HashSet usersListAllowImport = new HashSet(); 
 
 	// only the resources created by the followinng roles will be imported
 	// role sets are different to different system
@@ -96,7 +97,7 @@ public class BasicArchiveService
 	private String[] new_toolIds = {"sakai.preferences", "sakai.online", "sakai.siteinfo", "sakai.sitesetup", "sakai.discussion"};
 
 	// only these Sakai tools will be imported
-	public String[] SakaiServicesToImport = 
+	public String[] sakaiServicesToImport = 
 		{"AnnouncementService", 
 		"AssignmentService", 
 		"ContentHostingService", 
@@ -581,9 +582,9 @@ else if (system.equalsIgnoreCase(FROM_SAKAI) || system.equalsIgnoreCase(FROM_SAK
 	 */
 	 protected boolean checkSakaiService (String serviceName)
 	 {
-	 	for (int i = 0; i < SakaiServicesToImport.length; i ++)
+	 	for (int i = 0; i < sakaiServicesToImport.length; i ++)
 	 	{
-	 		if (serviceName.endsWith(SakaiServicesToImport[i]))
+	 		if (serviceName.endsWith(sakaiServicesToImport[i]))
 	 		{
 	 			return true;
 	 		}
@@ -738,9 +739,9 @@ else if (element.getTagName().equals(UserDirectoryService.APPLICATION_ID))
 							msg = service.merge(siteId, element, fileName, fromSite, attachmentNames, userIdTrans, new HashSet());
 						else if ((system.equalsIgnoreCase(FROM_SAKAI) || system.equalsIgnoreCase(FROM_SAKAI_2_8))
                                  && (checkSakaiService(serviceName)))
-							msg = service.merge(siteId, element, fileName, fromSite, attachmentNames, new HashMap() /* empty userIdTran map */, UsersListAllowImport);
+							msg = service.merge(siteId, element, fileName, fromSite, attachmentNames, new HashMap() /* empty userIdTran map */, usersListAllowImport);
 						else if (system.equalsIgnoreCase(FROM_CT))
-							msg = service.merge(siteId, element, fileName, fromSite, attachmentNames, new HashMap() /* empty userIdTran map */, UsersListAllowImport);
+							msg = service.merge(siteId, element, fileName, fromSite, attachmentNames, new HashMap() /* empty userIdTran map */, usersListAllowImport);
 
 						results.append(msg);
 					}
@@ -850,9 +851,6 @@ protected void mergeSiteRoles(Element el, String siteId, HashMap useIdTrans) thr
 		}
 
 		List roles = new Vector();
-		List maintainUsers = new Vector();
-		List accessUsers = new Vector();
-
 		// to add this user with this role inito this realm
 		String realmId = "/site/" + siteId;
 		try
@@ -909,8 +907,10 @@ protected void mergeSiteRoles(Element el, String siteId, HashMap useIdTrans) thr
 								AuthzGroupService.save(realmEdit);
 							}
 						} 
-						catch (UserNotDefinedException e) 
-						{
+						catch (UserNotDefinedException e) {
+							M_log.warn("UserNotDefined in mergeSiteRoles: " + userId);
+						} catch (AuthzPermissionException e) {
+							M_log.warn("AuthzPermissionException in mergeSiteRoles", e);
 						}
 					}
 				}
@@ -933,14 +933,14 @@ protected void mergeSiteRoles(Element el, String siteId, HashMap useIdTrans) thr
 						String userId = element3.getAttribute("userId");
 
 						// this user has a qualified role, his/her resource will be imported
-						UsersListAllowImport.add(userId);
+						usersListAllowImport.add(userId);
 					}
 				} // if - elseif - elseif
 			} // for
 		}
-		catch(Exception err)
+		catch(GroupNotDefinedException err)
 		{
-			M_log.warn("()mergeSiteRoles realm edit exception caught" + realmId);
+			M_log.warn("()mergeSiteRoles realm edit exception caught GroupNotDefinedException " + realmId);
 		}
 		return;
 
@@ -1144,16 +1144,7 @@ else if (fromCTools)
 	*/
 
 	protected void mergeSite(String siteId, String fromSiteId, Element element, HashMap useIdTrans, String creatorId)
-	{
-		String source = "";
-
-		Node parent = element.getParentNode();
-		if (parent.getNodeType() == Node.ELEMENT_NODE)
-		{
-			Element parentEl = (Element)parent;
-			source = parentEl.getAttribute("system");
-		}
-
+	{		
 		NodeList children = element.getChildNodes();
 		final int length = children.getLength();
 		for(int i = 0; i < length; i++)

diff --git a/...n/archive-impl/impl/src/java/org/sakaiproject/archive/impl/ImportMetadataServiceImpl.java b/...n/archive-impl/impl/src/java/org/sakaiproject/archive/impl/ImportMetadataServiceImpl.java
@@ -175,7 +175,6 @@ public ImportMetadata getImportMapById(String id)
         Element mappingElement = (Element) mapping;
         if (mappingElement.getTagName().equals(MAPPINGS))
         {
-          List maps = new ArrayList();
           NodeList mapNode = mappingElement.getChildNodes();
           final int mapLength = mapNode.getLength();
           for (int j = 0; j < mapLength; j++)

diff --git a/common/privacy-comp-impl/src/java/org/sakaiproject/component/privacy/PrivacyManagerImpl.java b/common/privacy-comp-impl/src/java/org/sakaiproject/component/privacy/PrivacyManagerImpl.java
@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.Iterator;
 import java.util.HashSet;
@@ -322,7 +323,7 @@ public Map getViewableState(String contextId, String recordType)
   			for(int i=0; i<returnedList.size(); i++)
   			{
   				pr = (PrivacyRecordImpl)returnedList.get(i);
-  				returnMap.put(pr.getUserId(), new Boolean(pr.getViewable()));
+  				returnMap.put(pr.getUserId(), Boolean.valueOf(pr.getViewable()));
   			}
   			return returnMap;
   		}
@@ -391,12 +392,11 @@ public void setViewableState(String contextId, Map userViewableState, String rec
 		}
 
 		Set keySet = userViewableState.keySet();
-		Iterator iter = keySet.iterator();
-		while(iter.hasNext())
+		for(Iterator<Entry<String, Boolean>> mapIter = keySet.iterator(); mapIter.hasNext();)
 		{
-			String userId = (String)iter.next();
-			Boolean viewable = (Boolean) userViewableState.get(userId);
-			setViewableState(contextId, userId, viewable, recordType);
+			Entry<String, Boolean> entry = mapIter.next();
+			Boolean viewable = (Boolean) entry.getValue();
+			setViewableState(contextId, entry.getKey(), viewable, recordType);
 		}
 	}