Ticket #263: Changes in response to code review.

This commit is contained in:
James Muehlner
2013-08-09 10:00:45 -07:00
parent c4e3ff1122
commit 8e35804c85
12 changed files with 43 additions and 50 deletions

View File

@@ -139,7 +139,8 @@ public class ActiveConnectionMap {
* Returns the ID of the connection with the lowest number of current * Returns the ID of the connection with the lowest number of current
* active users, if found. * active users, if found.
* *
* @param connectionIDs * @param connectionIDs The subset of connection IDs to find the least
* used connection within.
* *
* @return The ID of the connection with the lowest number of current * @return The ID of the connection with the lowest number of current
* active users, if found. * active users, if found.
@@ -149,28 +150,28 @@ public class ActiveConnectionMap {
if(connectionIDs.isEmpty()) if(connectionIDs.isEmpty())
return null; return null;
List<Connection> groupConnections = int minUserCount = Integer.MAX_VALUE;
new ArrayList<ActiveConnectionMap.Connection>(); Integer minConnectionID = null;
for(Integer connectionID : connectionIDs) { for(Integer connectionID : connectionIDs) {
Connection connection = activeConnectionMap.get(connectionID); Connection connection = activeConnectionMap.get(connectionID);
// Create the Connection if it does not exist /*
* If the connection is not found in the map, it has not been used,
* and therefore will be count 0.
*/
if(connection == null) { if(connection == null) {
connection = new Connection(connectionID); minUserCount = 0;
activeConnectionMap.put(connectionID, connection); minConnectionID = connectionID;
}
// If this is the least active connection
else if(connection.getCurrentUserCount() < minUserCount) {
minUserCount = connection.getCurrentUserCount();
minConnectionID = connection.getConnectionID();
} }
groupConnections.add(connection);
} }
// Sort the Connections into decending order return minConnectionID;
Collections.sort(groupConnections);
if(!groupConnections.isEmpty())
return groupConnections.get(0).getConnectionID();
return null;
} }
/** /**

View File

@@ -291,9 +291,14 @@ public class ConnectionDirectory implements Directory<String, Connection>{
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, Connection> directory)
throws GuacamoleException { throws GuacamoleException {
if(!(directory instanceof ConnectionDirectory))
throw new GuacamoleException("Directory not from database");
int toConnectionGroupID = ((ConnectionDirectory)directory).parentID;
// Get connection // Get connection
MySQLConnection mySQLConnection = MySQLConnection mySQLConnection =
connectionService.retrieveConnection(identifier, user_id); connectionService.retrieveConnection(identifier, user_id);
@@ -314,15 +319,6 @@ public class ConnectionDirectory implements Directory<String, Connection>{
permissionCheckService.verifyConnectionGroupAccess(this.user_id, permissionCheckService.verifyConnectionGroupAccess(this.user_id,
mySQLConnection.getParentID(), MySQLConstants.CONNECTION_GROUP_UPDATE); mySQLConnection.getParentID(), MySQLConstants.CONNECTION_GROUP_UPDATE);
Integer toConnectionGroupID;
if(groupIdentifier.equals(MySQLConstants.CONNECTION_GROUP_ROOT_IDENTIFIER))
toConnectionGroupID = null;
try {
toConnectionGroupID = Integer.valueOf(groupIdentifier);
} catch(NumberFormatException e) {
throw new GuacamoleException("Invalid connection group identifier.");
}
// Verify permission to use the to connection group for organizational purposes // Verify permission to use the to connection group for organizational purposes
permissionCheckService.verifyConnectionGroupUsageAccess permissionCheckService.verifyConnectionGroupUsageAccess
(toConnectionGroupID, user_id, MySQLConstants.CONNECTION_GROUP_ORGANIZATIONAL); (toConnectionGroupID, user_id, MySQLConstants.CONNECTION_GROUP_ORGANIZATIONAL);

View File

@@ -63,7 +63,7 @@ public class ConnectionGroupDirectory implements Directory<String, ConnectionGro
private int user_id; private int user_id;
/** /**
* The ID of the parent connection for this connection. * The ID of the parent connection group.
*/ */
private Integer parentID; private Integer parentID;
@@ -235,9 +235,14 @@ public class ConnectionGroupDirectory implements Directory<String, ConnectionGro
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, ConnectionGroup> directory)
throws GuacamoleException { throws GuacamoleException {
if(!(directory instanceof ConnectionGroupDirectory))
throw new GuacamoleException("Directory not from database");
int toConnectionGroupID = ((ConnectionGroupDirectory)directory).parentID;
// Get connection // Get connection
MySQLConnectionGroup mySQLConnectionGroup = MySQLConnectionGroup mySQLConnectionGroup =
connectionGroupService.retrieveConnectionGroup(identifier, user_id); connectionGroupService.retrieveConnectionGroup(identifier, user_id);
@@ -258,15 +263,6 @@ public class ConnectionGroupDirectory implements Directory<String, ConnectionGro
permissionCheckService.verifyConnectionGroupAccess(this.user_id, permissionCheckService.verifyConnectionGroupAccess(this.user_id,
mySQLConnectionGroup.getParentID(), MySQLConstants.CONNECTION_GROUP_UPDATE); mySQLConnectionGroup.getParentID(), MySQLConstants.CONNECTION_GROUP_UPDATE);
Integer toConnectionGroupID;
if(groupIdentifier.equals(MySQLConstants.CONNECTION_GROUP_ROOT_IDENTIFIER))
toConnectionGroupID = null;
try {
toConnectionGroupID = Integer.valueOf(groupIdentifier);
} catch(NumberFormatException e) {
throw new GuacamoleException("Invalid connection group identifier.");
}
// Verify permission to use the to connection group for organizational purposes // Verify permission to use the to connection group for organizational purposes
permissionCheckService.verifyConnectionGroupUsageAccess permissionCheckService.verifyConnectionGroupUsageAccess
(toConnectionGroupID, user_id, MySQLConstants.CONNECTION_GROUP_ORGANIZATIONAL); (toConnectionGroupID, user_id, MySQLConstants.CONNECTION_GROUP_ORGANIZATIONAL);

View File

@@ -61,7 +61,7 @@ public class MySQLConnection extends AbstractConnection {
private Integer connectionID; private Integer connectionID;
/** /**
* The ID of the parent connection for this connection. * The ID of the parent connection group for this connection.
*/ */
private Integer parentID; private Integer parentID;

View File

@@ -101,7 +101,7 @@ public class MySQLUserContext implements UserContext {
} }
@Override @Override
public ConnectionGroup getConnectionGroup() throws GuacamoleException { public ConnectionGroup getRootConnectionGroup() throws GuacamoleException {
return rootConnectionGroup; return rootConnectionGroup;
} }

View File

@@ -49,6 +49,7 @@ import net.sourceforge.guacamole.GuacamoleClientException;
import net.sourceforge.guacamole.GuacamoleException; import net.sourceforge.guacamole.GuacamoleException;
import net.sourceforge.guacamole.GuacamoleSecurityException; import net.sourceforge.guacamole.GuacamoleSecurityException;
import net.sourceforge.guacamole.net.auth.Directory; import net.sourceforge.guacamole.net.auth.Directory;
import net.sourceforge.guacamole.net.auth.User;
import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionGroupPermissionMapper; import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionGroupPermissionMapper;
import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionPermissionMapper; import net.sourceforge.guacamole.net.auth.mysql.dao.ConnectionPermissionMapper;
import net.sourceforge.guacamole.net.auth.mysql.dao.SystemPermissionMapper; import net.sourceforge.guacamole.net.auth.mysql.dao.SystemPermissionMapper;
@@ -76,7 +77,7 @@ import org.mybatis.guice.transactional.Transactional;
* A MySQL based implementation of the User Directory. * A MySQL based implementation of the User Directory.
* @author James Muehlner * @author James Muehlner
*/ */
public class UserDirectory implements Directory<String, net.sourceforge.guacamole.net.auth.User> { public class UserDirectory implements Directory<String, User> {
/** /**
* The ID of the user who this user directory belongs to. * The ID of the user who this user directory belongs to.
@@ -712,7 +713,7 @@ public class UserDirectory implements Directory<String, net.sourceforge.guacamol
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, User> groupIdentifier)
throws GuacamoleException { throws GuacamoleException {
throw new GuacamoleSecurityException("Permission denied."); throw new GuacamoleSecurityException("Permission denied.");
} }

View File

@@ -115,16 +115,15 @@ public interface Directory<IdentifierType, ObjectType> {
void remove(IdentifierType identifier) throws GuacamoleException; void remove(IdentifierType identifier) throws GuacamoleException;
/** /**
* Moves the object with the given identifier to the group with the given * Moves the object with the given identifier to the given directory.
* group identifier.
* *
* @param identifier The identifier of the object to remove. * @param identifier The identifier of the object to remove.
* @param groupIdentifier The identifier of the group to move the object to. * @param directory The directory to move the object to.
* *
* @throws GuacamoleException If an error occurs while moving the object, * @throws GuacamoleException If an error occurs while moving the object,
* or if moving object is not allowed. * or if moving object is not allowed.
*/ */
void move(IdentifierType identifier, IdentifierType groupIdentifier) void move(IdentifierType identifier, Directory<IdentifierType, ObjectType> directory)
throws GuacamoleException; throws GuacamoleException;
} }

View File

@@ -81,6 +81,6 @@ public interface UserContext {
* @throws GuacamoleException If an error occurs while creating the * @throws GuacamoleException If an error occurs while creating the
* Directory. * Directory.
*/ */
ConnectionGroup getConnectionGroup() throws GuacamoleException; ConnectionGroup getRootConnectionGroup() throws GuacamoleException;
} }

View File

@@ -110,7 +110,7 @@ public class SimpleConnectionDirectory
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, Connection> directory)
throws GuacamoleException { throws GuacamoleException {
throw new GuacamoleSecurityException("Permission denied."); throw new GuacamoleSecurityException("Permission denied.");
} }

View File

@@ -87,7 +87,7 @@ public class SimpleConnectionGroupDirectory
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, ConnectionGroup> directory)
throws GuacamoleException { throws GuacamoleException {
throw new GuacamoleSecurityException("Permission denied."); throw new GuacamoleSecurityException("Permission denied.");
} }

View File

@@ -124,7 +124,7 @@ public class SimpleUserContext implements UserContext {
} }
@Override @Override
public ConnectionGroup getConnectionGroup() throws GuacamoleException { public ConnectionGroup getRootConnectionGroup() throws GuacamoleException {
return connectionGroup; return connectionGroup;
} }

View File

@@ -101,7 +101,7 @@ public class SimpleUserDirectory implements Directory<String, User> {
} }
@Override @Override
public void move(String identifier, String groupIdentifier) public void move(String identifier, Directory<String, User> directory)
throws GuacamoleException { throws GuacamoleException {
throw new GuacamoleSecurityException("Permission denied."); throw new GuacamoleSecurityException("Permission denied.");
} }