Sanity checks, thread safety, remove unused grouplist in AvailableClient

5 files changed, 45 insertions, 30 deletions

diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java
index 75e93f3..30cd951 100644
--- a/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java
+++ b/src/main/java/de/bwlehrpool/bwlp_guac/AvailableClient.java
@@ -22,8 +22,6 @@ public class AvailableClient implements Cloneable {
 
 	private static final AtomicLong CON_ID = new AtomicLong();
 
-	public ArrayList<JsonGroup> groupList = new ArrayList<JsonGroup>();
-
 	private final String clientip;
 
 	private String password;
diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java b/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java
index 524ce8d..c737c4f 100644
--- a/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java
+++ b/src/main/java/de/bwlehrpool/bwlp_guac/BwlpAuthenticationProvider.java
@@ -117,10 +117,10 @@ public class BwlpAuthenticationProvider implements AuthenticationProvider {
 			response.groupid = Integer.parseInt(group.get("id").asText());
 			if (response.groupid != 0) {
 				password = group.get("password").asText();
-				correctPassword = ConnectionManager.getGroupPool().get(response.groupid).password;
+				correctPassword = ConnectionManager.getGroup(response.groupid).password;
 			}
 		} catch (Exception e) {
-			LOGGER.info("Error reading group", e);
+			LOGGER.info("Error reading group choice by user, asking again...", e);
 			tryAgain = true;
 		}
 
diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java
index 4bc653b..01bf2fc 100644
--- a/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java
+++ b/src/main/java/de/bwlehrpool/bwlp_guac/ConnectionManager.java
@@ -10,7 +10,14 @@ import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
 
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.HttpsURLConnection;
@@ -37,14 +44,30 @@ public class ConnectionManager {
 
 	private static final Logger LOGGER = LoggerFactory.getLogger(ConnectionManager.class);
 
-	// TODO: Config
 	private static final String SOURCE_URL = SlxConfig.clientListUrl();
 
+	/**
+	 * synchronize on clientPool when accessing
+	 */
 	private static final LinkedHashMap<String, AvailableClient> clientPool = new LinkedHashMap<String, AvailableClient>();
 
+	/**
+	 * synchronize on clientPool when accessing
+	 */
 	private static final LinkedHashMap<Integer, JsonGroup> groupPool = new LinkedHashMap<Integer, JsonGroup>();
 
-	public static LinkedHashMap<Integer, JsonGroup> getGroupPool() { return groupPool; }
+	
+	public static JsonGroup getGroup(int id) {
+		synchronized (clientPool) {
+			return groupPool.get(id);
+		}
+	}
+
+	public static Collection<JsonGroup> getGroupList() {
+		synchronized (clientPool) {
+			return new ArrayList<>(groupPool.values());
+		}
+	}
 	
 	public static WrappedConnection getExistingConnection(String user) {
 		AvailableClient freeClient = null;
@@ -198,15 +221,18 @@ public class ConnectionManager {
 			return;
 		}
 
-		// Temporary
-		JsonGroup[] groups = root.locations;
-
-		if (groups == null) {
-			LOGGER.info("Group list null");
+		if (root.locations == null) {
+			LOGGER.info("META DATA ERROR: Group list null");
+			return;
+		}
+		if (root.clients == null) {
+			LOGGER.info("META DATA ERROR: Client list null");
+			return;
 		}
-		synchronized (groupPool) {
+		
+		synchronized (clientPool) {
 			HashSet<JsonGroup> processedGroups = new HashSet<JsonGroup>();
-			for (JsonGroup gnew : groups) {
+			for (JsonGroup gnew : root.locations) {
 				if (gnew.locationids == null)
 					continue;
 				JsonGroup existing = groupPool.get(gnew.id);
@@ -215,6 +241,7 @@ public class ConnectionManager {
 					groupPool.put(gnew.id, gnew);
 					existing = gnew;
 					redoClientMapping = true;
+					LOGGER.info("Group " + gnew.name + " with pw " + gnew.password);
 				} else {
 					if (!Arrays.equals(existing.locationids, gnew.locationids)) {
 						redoClientMapping = true;
@@ -224,7 +251,7 @@ public class ConnectionManager {
 					existing.password = gnew.password;
 				}
 				if (redoClientMapping) {
-					existing.clientList = new ArrayList<AvailableClient>();
+					existing.clientList = new ConcurrentLinkedQueue<AvailableClient>();
 					for (AvailableClient client : clientPool.values()) {
 						int locationid = client.getLocationid();
 						for (int id : existing.locationids) {
@@ -236,22 +263,15 @@ public class ConnectionManager {
 					}
 				}
 				processedGroups.add(existing);
-				LOGGER.info("Group " + gnew.name + " with pw " + gnew.password);
 			}
 			for (Iterator<JsonGroup> it = groupPool.values().iterator(); it.hasNext();) {
 				JsonGroup g = it.next();
 				if (!processedGroups.contains(g)) {
-					for (AvailableClient client : g.clientList) {
-						client.groupList.remove(g);
-					}
 					it.remove();
 				}
 			}
 		}
 
-		if (root.clients == null) {
-			LOGGER.info("Client list null");
-		}
 		synchronized (clientPool) {
 			for (JsonClient cnew : root.clients) {
 				if (cnew.password == null || cnew.clientip == null)
@@ -266,7 +286,6 @@ public class ConnectionManager {
 						for (int id : group.locationids) {
 							if (id == cnew.locationid) {
 								group.clientList.add(existing);
-								existing.groupList.add(group);
 								break;
 							}
 						}
@@ -278,11 +297,9 @@ public class ConnectionManager {
 							for (int id : group.locationids) {
 								if (id == existing.getLocationid()) {
 									group.clientList.remove(existing);
-									existing.groupList.remove(group);
 								}
 								if (id == cnew.locationid) {
 									group.clientList.add(existing);
-									existing.groupList.add(group);
 								}
 							}
 						}
@@ -295,7 +312,7 @@ public class ConnectionManager {
 				AvailableClient c = it.next();
 				if (c.isTimeout(NOW)) {
 					LOGGER.info("Removing client " + c + " from list");
-					for (JsonGroup group : c.groupList) {
+					for (JsonGroup group : groupPool.values()) {
 						group.clientList.remove(c);
 					}
 					it.remove();
diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java b/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java
index 0e9c27b..ddc57e8 100644
--- a/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java
+++ b/src/main/java/de/bwlehrpool/bwlp_guac/GroupField.java
@@ -16,6 +16,6 @@ public class GroupField extends Field {
 
     @JsonProperty("groups")
     public Collection<JsonGroup> getGroups() throws GuacamoleException {
-        return ConnectionManager.getGroupPool().values();
+        return ConnectionManager.getGroupList();
     }
 }
\ No newline at end of file
diff --git a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java
index c5adf9e..e1aef81 100644
--- a/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java
+++ b/src/main/java/de/bwlehrpool/bwlp_guac/JsonGroup.java
@@ -1,10 +1,10 @@
 package de.bwlehrpool.bwlp_guac;
 
+import java.util.concurrent.ConcurrentLinkedQueue;
+
 import org.codehaus.jackson.annotate.JsonIgnore;
 import org.codehaus.jackson.annotate.JsonProperty;
 
-import java.util.ArrayList;
-
 public class JsonGroup {
 
 	public int id;
@@ -37,6 +37,6 @@ public class JsonGroup {
 	}
 
 	@JsonIgnore
-	public ArrayList<AvailableClient> clientList = new ArrayList<AvailableClient>();
+	public ConcurrentLinkedQueue<AvailableClient> clientList = new ConcurrentLinkedQueue<AvailableClient>();
 
 }