Better handling of errors in user and group yml's.

2 files changed, 232 insertions, 178 deletions

diff --git a/EssentialsGroupManager/src/Changelog.txt b/EssentialsGroupManager/src/Changelog.txt
index f699c3f6f..c6c3853fb 100644
--- a/EssentialsGroupManager/src/Changelog.txt
+++ b/EssentialsGroupManager/src/Changelog.txt
@@ -167,4 +167,5 @@ v 2.0:
 	- Don't throw errors when attempting to remove permission attachments (bukkit will have already removed it).

 	- Remove all permission attachments when performing a manload or restart.

 	- Expand 'manwhois' to also list a users subgroups.

-	- Fix a concurrent modification error when removing all attachments.
\ No newline at end of file
+	- Fix a concurrent modification error when removing all attachments.

+	- Better handling of errors in user and group yml's.
\ No newline at end of file
diff --git a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java
index cee2ec0b8..135417068 100644
--- a/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java
+++ b/EssentialsGroupManager/src/org/anjocaido/groupmanager/dataholder/WorldDataHolder.java
@@ -14,6 +14,7 @@ import java.io.UnsupportedEncodingException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.logging.Level;
@@ -459,98 +460,140 @@ public class WorldDataHolder {
         //PROCESS GROUPS FILE
         Map<String, List<String>> inheritance = new HashMap<String, List<String>>();
         try {
+        	/*
+        	 * Fetch all child nodes under the 'groups' entry.
+        	 */
             Map<String, Object> allGroupsNode = (Map<String, Object>) groupsRootDataNode.get("groups");
-            try {
-	            for (String groupKey : allGroupsNode.keySet()) {
-	                Map<String, Object> thisGroupNode = (Map<String, Object>) allGroupsNode.get(groupKey);
-	                Group thisGrp = ph.createGroup(groupKey);
-	                if (thisGrp == null) {
-	                    throw new IllegalArgumentException("I think this Group was declared more than once: " + groupKey + " in file: " + groupsFile.getPath());
-	                }
-	                if (thisGroupNode.get("default") == null) {
-	                    thisGroupNode.put("default", false);
-	                }
-	                if ((Boolean.parseBoolean(thisGroupNode.get("default").toString()))) {
-	                    if (ph.getDefaultGroup() != null) {
-	                        GroupManager.logger.warning("The group " + thisGrp.getName() + " is claiming to be default where" + ph.getDefaultGroup().getName() + " already was.");
-	                        GroupManager.logger.warning("Overriding first request for file: " + groupsFile.getPath());
-	                    }
-	                    ph.setDefaultGroup(thisGrp);
-	                }
-	
-	                //PERMISSIONS NODE
-	                try {
-		                if (thisGroupNode.get("permissions") == null) {
-		                    thisGroupNode.put("permissions", new ArrayList<String>());
+            Iterator<String> groupItr = allGroupsNode.keySet().iterator();
+        	String groupKey;
+        	Integer groupCount = 0;
+        	
+        	/*
+        	 * loop each group entry
+        	 * and read it's data.
+        	 */
+        	while (groupItr.hasNext()) {
+        		try {
+        			groupCount++;
+        			// Attempt to fetch the next group name.
+        			groupKey = groupItr.next();
+        		} catch (Exception e) {
+    				throw new IllegalArgumentException("Invalid node type for group entry (" + groupCount + ") in file: " + groupsFile.getPath());
+    			}
+        		
+        		/*
+        		 * Fetch this groups child nodes
+        		 */
+        		Map<String, Object> thisGroupNode = (Map<String, Object>) allGroupsNode.get(groupKey);
+        		/*
+        		 * Create a new group with this name
+        		 * in the assigned data source.
+        		 */
+                Group thisGrp = ph.createGroup(groupKey);
+                
+                if (thisGrp == null) {
+                    throw new IllegalArgumentException("I think this Group was declared more than once: " + groupKey + " in file: " + groupsFile.getPath());
+                }
+                /*
+                 * If no default node is found set it as false.
+                 */
+                if (thisGroupNode.get("default") == null) {
+                    thisGroupNode.put("default", false);
+                } else if ((Boolean.parseBoolean(thisGroupNode.get("default").toString()))) {
+                	/*
+                	 * Set this as the default group.
+                	 * Warn if some other group has already claimed that position.
+                	 */
+                    if (ph.getDefaultGroup() != null) {
+                        GroupManager.logger.warning("The group " + thisGrp.getName() + " is claiming to be default where" + ph.getDefaultGroup().getName() + " already was.");
+                        GroupManager.logger.warning("Overriding first request for file: " + groupsFile.getPath());
+                    }
+                    ph.setDefaultGroup(thisGrp);
+                }
+
+                //PERMISSIONS NODE
+                try {
+                	/*
+                	 * If no permissions node is found, or it's empty
+                	 * set an empty permission list
+                	 */
+	                if (thisGroupNode.get("permissions") == null) {
+	                    thisGroupNode.put("permissions", new ArrayList<String>());
+	                } else {
+	                	/*
+	                	 * There is a permission list Which seems to hold some data
+	                	 */
+		                if (thisGroupNode.get("permissions") instanceof List) {
+		                	/*
+		                	 * Check each entry and add it as a new permission.
+		                	 */
+		                    for (Object o : ((List) thisGroupNode.get("permissions"))) {
+		                    	try {
+		                    		/*
+		                    		 * Only add this permission if it's not empty.
+		                    		 */
+		                    		if (!thisGroupNode.get("permissions").toString().isEmpty())
+		                    			thisGrp.addPermission(o.toString());
+		                    	} catch (NullPointerException e) {
+		                    		// Ignore this entry as it's null. It can be safely dropped
+		                    	}
+		                    }
+		                } else if (thisGroupNode.get("permissions") instanceof String) {
+		                	/*
+		                	 * Only add this permission if it's not empty.
+		                	 */
+		                	if (!thisGroupNode.get("permissions").toString().isEmpty())
+		                		thisGrp.addPermission((String) thisGroupNode.get("permissions"));
 		                } else {
-			                if (thisGroupNode.get("permissions") instanceof List) {
-			                    for (Object o : ((List) thisGroupNode.get("permissions"))) {
-			                    	try {
-			                    		/*
-			                    		 * Only add this permission if it's not empty.
-			                    		 */
-			                    		if (!thisGroupNode.get("permissions").toString().isEmpty())
-			                    			thisGrp.addPermission(o.toString());
-			                    	} catch (NullPointerException e) {
-			                    		// Ignore this entry as it's null.
-			                    		//throw new IllegalArgumentException("Invalid permission node in group:  " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-			                    	}
-			                    }
-			                } else if (thisGroupNode.get("permissions") instanceof String) {
-			                	/*
-			                	 * Only add this permission if it's not empty.
-			                	 */
-			                	if (!thisGroupNode.get("permissions").toString().isEmpty())
-			                		thisGrp.addPermission((String) thisGroupNode.get("permissions"));
-			                } else {
-			                    throw new IllegalArgumentException("Unknown type of permissions node(Should be String or List<String>) for group:  " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-			                }
-			                thisGrp.sortPermissions();
+		                    throw new IllegalArgumentException("Unknown type of permissions node(Should be String or List<String>) for group:  " + thisGrp.getName() + " in file: " + groupsFile.getPath());
 		                }
-	                } catch (Exception e) {
-	                	throw new IllegalArgumentException("Invalid formatting found in permissions section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-	                }
-	
-	                //INFO NODE
-	                try {
-		                if (thisGroupNode.get("info") instanceof Map) {
-			                Map<String, Object> infoNode = (Map<String, Object>) thisGroupNode.get("info");
-			                if (infoNode != null) {
-			                    thisGrp.setVariables(infoNode);
-			                }
-		                } else
-		                	throw new IllegalArgumentException("Unknown entry found in Info section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-	                } catch (Exception e1) {
-	                	throw new IllegalArgumentException("Invalid formatting found in info section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+		                /*
+		                 * Sort all permissions so they are in the correct order for checking.
+		                 */
+		                thisGrp.sortPermissions();
 	                }
-	                	
-	                //END INFO NODE
-	
-	                try {
-		                if (thisGroupNode.get("inheritance") == null || thisGroupNode.get("inheritance") instanceof List) {
-			                Object inheritNode = thisGroupNode.get("inheritance");
-			                if (inheritNode == null) {
-			                    thisGroupNode.put("inheritance", new ArrayList<String>());
-			                } else if (inheritNode instanceof List) {
-			                    List<String> groupsInh = (List<String>) inheritNode;
-			                    for (String grp : groupsInh) {
-			                        if (inheritance.get(groupKey) == null) {
-			                            List<String> thisInherits = new ArrayList<String>();
-			                            inheritance.put(groupKey, thisInherits);
-			                        }
-			                        inheritance.get(groupKey).add(grp);
-			
-			                    }
-			                }
-		                }else
-		                	throw new IllegalArgumentException("Unknown entry found in inheritance section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-	                } catch (Exception e2) {
-	                	throw new IllegalArgumentException("Invalid formatting found in inheritance section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
-	                }
-	            }
-            } catch (Exception e) {
-				throw new IllegalArgumentException("Invalid node type, or bad indentation in file: " + groupsFile.getPath());
-			}
+                } catch (Exception e) {
+                	throw new IllegalArgumentException("Invalid formatting found in permissions section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+                }
+
+                //INFO NODE
+                try {
+	                if (thisGroupNode.get("info") instanceof Map) {
+		                Map<String, Object> infoNode = (Map<String, Object>) thisGroupNode.get("info");
+		                if (infoNode != null) {
+		                    thisGrp.setVariables(infoNode);
+		                }
+	                } else
+	                	throw new IllegalArgumentException("Unknown entry found in Info section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+                } catch (Exception e1) {
+                	throw new IllegalArgumentException("Invalid formatting found in info section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+                }
+                	
+                //END INFO NODE
+
+                try {
+	                if (thisGroupNode.get("inheritance") == null || thisGroupNode.get("inheritance") instanceof List) {
+		                Object inheritNode = thisGroupNode.get("inheritance");
+		                if (inheritNode == null) {
+		                    thisGroupNode.put("inheritance", new ArrayList<String>());
+		                } else if (inheritNode instanceof List) {
+		                    List<String> groupsInh = (List<String>) inheritNode;
+		                    for (String grp : groupsInh) {
+		                        if (inheritance.get(groupKey) == null) {
+		                            List<String> thisInherits = new ArrayList<String>();
+		                            inheritance.put(groupKey, thisInherits);
+		                        }
+		                        inheritance.get(groupKey).add(grp);
+		
+		                    }
+		                }
+	                }else
+	                	throw new IllegalArgumentException("Unknown entry found in inheritance section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+                } catch (Exception e2) {
+                	throw new IllegalArgumentException("Invalid formatting found in inheritance section for group: " + thisGrp.getName() + " in file: " + groupsFile.getPath());
+                }
+            }
+
         } catch (Exception ex) {
             ex.printStackTrace();
             throw new IllegalArgumentException("Your " + groupsFile.getPath() + " file is invalid. See console for details.");
@@ -614,97 +657,107 @@ public class WorldDataHolder {
         Map<String, Object> allUsersNode = (Map<String, Object>) usersRootDataNode.get("users");
         
         // Load users if the file is NOT empty
-        if (allUsersNode != null)
-        	try {
-		        for (String usersKey : allUsersNode.keySet()) {
-		        	Map<String, Object> thisUserNode = null;
-		        	try {
-		        		thisUserNode = (Map<String, Object>) allUsersNode.get(usersKey);
-		        	} catch (Exception ex) {
-		        		throw new IllegalArgumentException("Bad format found in file: " + usersFile.getPath());
-		            }
-		            User thisUser = ph.createUser(usersKey);
-		            if (thisUser == null) {
-		                throw new IllegalArgumentException("I think this user was declared more than once: " + usersKey + " in file: " + usersFile.getPath());
-		            }
-		            if (thisUserNode.get("permissions") == null) {
-		                thisUserNode.put("permissions", new ArrayList<String>());
-		            } else {
-			            if (thisUserNode.get("permissions") instanceof List) {
-			                for (Object o : ((List) thisUserNode.get("permissions"))) {
-			                	/*
-			            		 * Only add this permission if it's not empty
-			            		 */
-			            		if (!o.toString().isEmpty())
-			            			thisUser.addPermission(o.toString());
-			                }
-			            } else if (thisUserNode.get("permissions") instanceof String) {
-			            	try {
-			            		/*
-			            		 * Only add this permission if it's not empty
-			            		 */
-			            		if (!thisUserNode.get("permissions").toString().isEmpty())
-			            			thisUser.addPermission(thisUserNode.get("permissions").toString());
-			            	} catch (NullPointerException e) {
-			            		// Ignore this entry as it's null.
-			            		//throw new IllegalArgumentException("Invalid permission node for user:  " + thisUser.getName() + " in file: " + UserFile.getPath());
-			            	}
-			            }
-			            thisUser.sortPermissions();
-		            }
-		
-		            //SUBGROUPS LOADING
-		            if (thisUserNode.get("subgroups") == null) {
-		                thisUserNode.put("subgroups", new ArrayList<String>());
-		            }
-		            if (thisUserNode.get("subgroups") instanceof List) {
-		                for (Object o : ((List) thisUserNode.get("subgroups"))) {
-		                    Group subGrp = ph.getGroup(o.toString());
-		                    if (subGrp != null) {
-		                        thisUser.addSubGroup(subGrp);
-		                    } else {
-		                        GroupManager.logger.warning("Subgroup " + o.toString() + " not found for user " + thisUser.getName() + ". Ignoring entry in file: " + usersFile.getPath());
-		                    }
-		                }
-		            } else if (thisUserNode.get("subgroups") instanceof String) {
-		                Group subGrp = ph.getGroup(thisUserNode.get("subgroups").toString());
-		                if (subGrp != null) {
-		                    thisUser.addSubGroup(subGrp);
-		                } else {
-		                    GroupManager.logger.warning("Subgroup " + thisUserNode.get("subgroups").toString() + " not found for user " + thisUser.getName() + ". Ignoring entry in file: " + usersFile.getPath());
-		                }
-		            }
-		
-		
-		            //USER INFO NODE
-		            
-		            //INFO NODE
-	                if (thisUserNode.get("info") instanceof Map) {
-		                Map<String, Object> infoNode = (Map<String, Object>) thisUserNode.get("info");
-		                if (infoNode != null) {
-		                	thisUser.setVariables(infoNode);
-		                }
-	                } else if (thisUserNode.get("info") != null)
-	                	throw new IllegalArgumentException("Unknown entry found in Info section for user: " + thisUser.getName() + " in file: " + usersFile.getPath());
-	                	
-	                //END INFO NODE
-		
-		
-		            if (thisUserNode.get("group") != null) {
-		                Group hisGroup = ph.getGroup(thisUserNode.get("group").toString());
-		                if (hisGroup == null) {
-		                	GroupManager.logger.warning("There is no group " + thisUserNode.get("group").toString() + ", as stated for player " + thisUser.getName() + ": Set to '" + ph.getDefaultGroup().getName() + "' for file: " + usersFile.getPath());
-		                	hisGroup = ph.getDefaultGroup();
-		                    //throw new IllegalArgumentException("There is no group " + thisUserNode.get("group").toString() + ", as stated for player " + thisUser.getName());
+        if (allUsersNode != null) {
+        	
+        	Iterator<String> usersItr = allUsersNode.keySet().iterator();
+        	String usersKey;
+        	Integer userCount = 0;
+        	
+        	while (usersItr.hasNext()) {
+        		try {
+        			userCount++;
+        			// Attempt to fetch the next user name.
+        			usersKey = usersItr.next();
+        		} catch (Exception e) {
+    				throw new IllegalArgumentException("Invalid node type for user entry (" + userCount + ") in file: " + usersFile.getPath());
+    			}
+
+        		Map<String, Object> thisUserNode = null;
+	        	try {
+	        		thisUserNode = (Map<String, Object>) allUsersNode.get(usersKey);
+	        	} catch (Exception ex) {
+	        		throw new IllegalArgumentException("Bad format found in file: " + usersFile.getPath());
+	            }
+	            User thisUser = ph.createUser(usersKey);
+	            if (thisUser == null) {
+	                throw new IllegalArgumentException("I think this user was declared more than once: " + usersKey + " in file: " + usersFile.getPath());
+	            }
+	            if (thisUserNode.get("permissions") == null) {
+	                thisUserNode.put("permissions", new ArrayList<String>());
+	            } else {
+		            if (thisUserNode.get("permissions") instanceof List) {
+		                for (Object o : ((List) thisUserNode.get("permissions"))) {
+		                	/*
+		            		 * Only add this permission if it's not empty
+		            		 */
+		            		if (!o.toString().isEmpty())
+		            			thisUser.addPermission(o.toString());
 		                }
-		                thisUser.setGroup(hisGroup);
-		            } else {
-		                thisUser.setGroup(ph.getDefaultGroup());
+		            } else if (thisUserNode.get("permissions") instanceof String) {
+		            	try {
+		            		/*
+		            		 * Only add this permission if it's not empty
+		            		 */
+		            		if (!thisUserNode.get("permissions").toString().isEmpty())
+		            			thisUser.addPermission(thisUserNode.get("permissions").toString());
+		            	} catch (NullPointerException e) {
+		            		// Ignore this entry as it's null.
+		            		//throw new IllegalArgumentException("Invalid permission node for user:  " + thisUser.getName() + " in file: " + UserFile.getPath());
+		            	}
 		            }
-		        }
-        	} catch (Exception e) {
-				throw new IllegalArgumentException("Invalid node type, or bad indentation in file: " + usersFile.getPath());
-			}
+		            thisUser.sortPermissions();
+	            }
+	
+	            //SUBGROUPS LOADING
+	            if (thisUserNode.get("subgroups") == null) {
+	                thisUserNode.put("subgroups", new ArrayList<String>());
+	            }
+	            if (thisUserNode.get("subgroups") instanceof List) {
+	                for (Object o : ((List) thisUserNode.get("subgroups"))) {
+	                    Group subGrp = ph.getGroup(o.toString());
+	                    if (subGrp != null) {
+	                        thisUser.addSubGroup(subGrp);
+	                    } else {
+	                        GroupManager.logger.warning("Subgroup " + o.toString() + " not found for user " + thisUser.getName() + ". Ignoring entry in file: " + usersFile.getPath());
+	                    }
+	                }
+	            } else if (thisUserNode.get("subgroups") instanceof String) {
+	                Group subGrp = ph.getGroup(thisUserNode.get("subgroups").toString());
+	                if (subGrp != null) {
+	                    thisUser.addSubGroup(subGrp);
+	                } else {
+	                    GroupManager.logger.warning("Subgroup " + thisUserNode.get("subgroups").toString() + " not found for user " + thisUser.getName() + ". Ignoring entry in file: " + usersFile.getPath());
+	                }
+	            }
+	
+	
+	            //USER INFO NODE
+	            
+	            //INFO NODE
+                if (thisUserNode.get("info") instanceof Map) {
+	                Map<String, Object> infoNode = (Map<String, Object>) thisUserNode.get("info");
+	                if (infoNode != null) {
+	                	thisUser.setVariables(infoNode);
+	                }
+                } else if (thisUserNode.get("info") != null)
+                	throw new IllegalArgumentException("Unknown entry found in Info section for user: " + thisUser.getName() + " in file: " + usersFile.getPath());
+                	
+                //END INFO NODE
+	
+	
+	            if (thisUserNode.get("group") != null) {
+	                Group hisGroup = ph.getGroup(thisUserNode.get("group").toString());
+	                if (hisGroup == null) {
+	                	GroupManager.logger.warning("There is no group " + thisUserNode.get("group").toString() + ", as stated for player " + thisUser.getName() + ": Set to '" + ph.getDefaultGroup().getName() + "' for file: " + usersFile.getPath());
+	                	hisGroup = ph.getDefaultGroup();
+	                    //throw new IllegalArgumentException("There is no group " + thisUserNode.get("group").toString() + ", as stated for player " + thisUser.getName());
+	                }
+	                thisUser.setGroup(hisGroup);
+	            } else {
+	                thisUser.setGroup(ph.getDefaultGroup());
+	            }
+	        }
+    	}
         
         ph.removeUsersChangedFlag();
         // Update the LastModified time.