Apply suggestions from code review
This commit is contained in:
		
							
								
								
									
										2
									
								
								pom.xml
									
									
									
									
									
								
							
							
						
						
									
										2
									
								
								pom.xml
									
									
									
									
									
								
							| @@ -28,7 +28,7 @@ | |||||||
| 		<dependency> | 		<dependency> | ||||||
| 			<groupId>com.github.informatik-ag-ngl</groupId> | 			<groupId>com.github.informatik-ag-ngl</groupId> | ||||||
| 			<artifactId>envoy-common</artifactId> | 			<artifactId>envoy-common</artifactId> | ||||||
| 			<version>f~compatibility_verification-SNAPSHOT</version> | 			<version>develop-SNAPSHOT</version> | ||||||
| 		</dependency> | 		</dependency> | ||||||
| 		<dependency> | 		<dependency> | ||||||
| 			<groupId>com.github.informatik-ag-ngl</groupId> | 			<groupId>com.github.informatik-ag-ngl</groupId> | ||||||
|   | |||||||
							
								
								
									
										94
									
								
								src/main/java/enovy/server/util/VersionUtils.java
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										94
									
								
								src/main/java/enovy/server/util/VersionUtils.java
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,94 @@ | |||||||
|  | package enovy.server.util; | ||||||
|  |  | ||||||
|  | import java.util.regex.Pattern; | ||||||
|  |  | ||||||
|  | /** | ||||||
|  |  * Implements a comparison algorithm between Envoy versions and defines minimal | ||||||
|  |  * and maximal client versions compatible with this server. | ||||||
|  |  * <p> | ||||||
|  |  * Project: <strong>envoy-server-standalone</strong><br> | ||||||
|  |  * File: <strong>VersionUtils.java</strong><br> | ||||||
|  |  * Created: <strong>23.06.2020</strong><br> | ||||||
|  |  *  | ||||||
|  |  * @author KSKE | ||||||
|  |  * @since Envoy Server Standalone v0.1-beta | ||||||
|  |  */ | ||||||
|  | public class VersionUtils { | ||||||
|  |  | ||||||
|  | 	/** | ||||||
|  | 	 * The minimal client version compatible with this server. | ||||||
|  | 	 *  | ||||||
|  | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
|  | 	 */ | ||||||
|  | 	public static final String MIN_CLIENT_VERSION = "0.1-beta"; | ||||||
|  |  | ||||||
|  | 	/** | ||||||
|  | 	 * The maximal client version compatible with this server. | ||||||
|  | 	 *  | ||||||
|  | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
|  | 	 */ | ||||||
|  | 	public static final String MAX_CLIENT_VERSION = "0.1-beta"; | ||||||
|  |  | ||||||
|  | 	private static final Pattern versionPattern = Pattern.compile("(?<major>\\d).(?<minor>\\d)(?:-(?<suffix>\\w+))?"); | ||||||
|  |  | ||||||
|  | 	private VersionUtils() {} | ||||||
|  |  | ||||||
|  | 	/** | ||||||
|  | 	 * Parses an Envoy Client version string and checks whether that version is | ||||||
|  | 	 * compatible with this server. | ||||||
|  | 	 *  | ||||||
|  | 	 * @param version the version string to parse | ||||||
|  | 	 * @return {@code true} if the given version is compatible with this server | ||||||
|  | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
|  | 	 */ | ||||||
|  | 	public static boolean verifyCompatibility(String version) { | ||||||
|  | 		final var currentMatcher = versionPattern.matcher(version); | ||||||
|  |  | ||||||
|  | 		if (!currentMatcher.matches()) return false; | ||||||
|  |  | ||||||
|  | 		final var	minMatcher	= versionPattern.matcher(MIN_CLIENT_VERSION); | ||||||
|  | 		final var	maxMatcher	= versionPattern.matcher(MAX_CLIENT_VERSION); | ||||||
|  |  | ||||||
|  | 		if (!minMatcher.matches() || !maxMatcher.matches()) throw new RuntimeException("Invalid min or max client version configured!"); | ||||||
|  |  | ||||||
|  | 		// Compare suffixes | ||||||
|  | 		{ | ||||||
|  | 			final var	currentSuffix	= convertSuffix(currentMatcher.group("suffix")); | ||||||
|  | 			final var	minSuffix		= convertSuffix(minMatcher.group("suffix")); | ||||||
|  | 			final var	maxSuffix		= convertSuffix(maxMatcher.group("suffix")); | ||||||
|  |  | ||||||
|  | 			if (currentSuffix < minSuffix || currentSuffix > maxSuffix) return false; | ||||||
|  | 		} | ||||||
|  |  | ||||||
|  | 		// Compare major | ||||||
|  | 		{ | ||||||
|  | 			final var	currentMajor	= Integer.parseInt(currentMatcher.group("major")); | ||||||
|  | 			final var	minMajor		= Integer.parseInt(minMatcher.group("major")); | ||||||
|  | 			final var	maxMajor		= Integer.parseInt(maxMatcher.group("major")); | ||||||
|  |  | ||||||
|  | 			if (currentMajor < minMajor || currentMajor > maxMajor) return false; | ||||||
|  | 		} | ||||||
|  |  | ||||||
|  | 		// Compare minor | ||||||
|  | 		{ | ||||||
|  | 			final var	currentMinor	= Integer.parseInt(currentMatcher.group("minor")); | ||||||
|  | 			final var	minMinor		= Integer.parseInt(minMatcher.group("minor")); | ||||||
|  | 			final var	maxMinor		= Integer.parseInt(maxMatcher.group("minor")); | ||||||
|  |  | ||||||
|  | 			if (currentMinor < minMinor || currentMinor > maxMinor) return false; | ||||||
|  | 		} | ||||||
|  |  | ||||||
|  | 		return true; | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	private static int convertSuffix(String suffix) { | ||||||
|  | 		switch (suffix == null ? "" : suffix) { | ||||||
|  | 			case "alpha": | ||||||
|  | 				return 0; | ||||||
|  | 			case "beta": | ||||||
|  | 				return 1; | ||||||
|  | 			default: | ||||||
|  | 				return 2; | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | } | ||||||
							
								
								
									
										11
									
								
								src/main/java/enovy/server/util/package-info.java
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										11
									
								
								src/main/java/enovy/server/util/package-info.java
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,11 @@ | |||||||
|  | /** | ||||||
|  |  * This package contains utility classes used in Envoy Server. | ||||||
|  |  * <p> | ||||||
|  |  * Project: <strong>envoy-server-standalone</strong><br> | ||||||
|  |  * File: <strong>package-info.java</strong><br> | ||||||
|  |  * Created: <strong>23.06.2020</strong><br> | ||||||
|  |  *  | ||||||
|  |  * @author Kai S. K. Engelbart | ||||||
|  |  * @since Envoy Server Standalone v0.1-beta | ||||||
|  |  */ | ||||||
|  | package enovy.server.util; | ||||||
| @@ -29,20 +29,6 @@ import envoy.util.EnvoyLog; | |||||||
|  */ |  */ | ||||||
| public class Startup { | public class Startup { | ||||||
|  |  | ||||||
| 	/** |  | ||||||
| 	 * The minimal client version compatible with this server. |  | ||||||
| 	 *  |  | ||||||
| 	 * @since Envoy Server Standalone v0.1-beta |  | ||||||
| 	 */ |  | ||||||
| 	public static final String	MIN_CLIENT_VERSION	= "0.1-beta"; |  | ||||||
|  |  | ||||||
| 	/** |  | ||||||
| 	 * The maximal client version compatible with this server. |  | ||||||
| 	 *  |  | ||||||
| 	 * @since Envoy Server Standalone v0.1-beta |  | ||||||
| 	 */ |  | ||||||
| 	public static final String	MAX_CLIENT_VERSION	= "0.1-beta"; |  | ||||||
|  |  | ||||||
| 	/** | 	/** | ||||||
| 	 * Initializes the logger with a new config instance. | 	 * Initializes the logger with a new config instance. | ||||||
| 	 *  | 	 *  | ||||||
|   | |||||||
| @@ -1,5 +1,7 @@ | |||||||
| package envoy.server.data; | package envoy.server.data; | ||||||
|  |  | ||||||
|  | import static envoy.data.Message.MessageStatus.*; | ||||||
|  |  | ||||||
| import java.time.LocalDateTime; | import java.time.LocalDateTime; | ||||||
|  |  | ||||||
| import javax.persistence.*; | import javax.persistence.*; | ||||||
| @@ -38,7 +40,7 @@ public class Message { | |||||||
| 	 * @since Envoy Server Standalone v0.1-beta | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
| 	 */ | 	 */ | ||||||
| 	public static final String getPending = "Message.getPending"; | 	public static final String getPending = "Message.getPending"; | ||||||
| 	 |  | ||||||
| 	@Id | 	@Id | ||||||
| 	protected long id; | 	protected long id; | ||||||
|  |  | ||||||
| @@ -111,9 +113,26 @@ public class Message { | |||||||
| 		return message; | 		return message; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | 	/** | ||||||
|  | 	 * Sets the message status to {@link MessageStatus#RECEIVED} and sets the | ||||||
|  | 	 * current time stamp as the received date. | ||||||
|  | 	 *  | ||||||
|  | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
|  | 	 */ | ||||||
| 	public void received() { | 	public void received() { | ||||||
| 		receivedDate	= LocalDateTime.now(); | 		receivedDate	= LocalDateTime.now(); | ||||||
| 		status			= MessageStatus.RECEIVED; | 		status			= RECEIVED; | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	/** | ||||||
|  | 	 * Sets the message status to {@link MessageStatus#READ} and sets the | ||||||
|  | 	 * current time stamp as the read date. | ||||||
|  | 	 *  | ||||||
|  | 	 * @since Envoy Server Standalone v0.1-beta | ||||||
|  | 	 */ | ||||||
|  | 	public void read() { | ||||||
|  | 		readDate	= LocalDateTime.now(); | ||||||
|  | 		status		= READ; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	/** | 	/** | ||||||
|   | |||||||
| @@ -8,15 +8,14 @@ import java.time.LocalDateTime; | |||||||
| import java.util.Arrays; | import java.util.Arrays; | ||||||
| import java.util.HashSet; | import java.util.HashSet; | ||||||
| import java.util.logging.Logger; | import java.util.logging.Logger; | ||||||
| import java.util.regex.Pattern; |  | ||||||
|  |  | ||||||
| import javax.persistence.NoResultException; | import javax.persistence.NoResultException; | ||||||
|  |  | ||||||
|  | import enovy.server.util.VersionUtils; | ||||||
| import envoy.data.LoginCredentials; | import envoy.data.LoginCredentials; | ||||||
| import envoy.data.Message.MessageStatus; | import envoy.data.Message.MessageStatus; | ||||||
| import envoy.event.HandshakeRejection; | import envoy.event.HandshakeRejection; | ||||||
| import envoy.event.MessageStatusChange; | import envoy.event.MessageStatusChange; | ||||||
| import envoy.server.Startup; |  | ||||||
| import envoy.server.data.PersistenceManager; | import envoy.server.data.PersistenceManager; | ||||||
| import envoy.server.data.User; | import envoy.server.data.User; | ||||||
| import envoy.server.net.ConnectionManager; | import envoy.server.net.ConnectionManager; | ||||||
| @@ -40,7 +39,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred | |||||||
| 	private final ConnectionManager		connectionManager	= ConnectionManager.getInstance(); | 	private final ConnectionManager		connectionManager	= ConnectionManager.getInstance(); | ||||||
|  |  | ||||||
| 	private static final Logger		logger			= EnvoyLog.getLogger(LoginCredentialProcessor.class); | 	private static final Logger		logger			= EnvoyLog.getLogger(LoginCredentialProcessor.class); | ||||||
| 	private static final Pattern	versionPattern	= Pattern.compile("(?<major>\\d).(?<minor>\\d)(?:-(?<suffix>\\w+))?"); |  | ||||||
|  |  | ||||||
| 	@Override | 	@Override | ||||||
| 	public void process(LoginCredentials credentials, long socketID, ObjectWriteProxy writeProxy) throws IOException { | 	public void process(LoginCredentials credentials, long socketID, ObjectWriteProxy writeProxy) throws IOException { | ||||||
| @@ -48,10 +46,9 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred | |||||||
| 		// Cache this write proxy for user-independant notifications | 		// Cache this write proxy for user-independant notifications | ||||||
| 		UserStatusChangeProcessor.setWriteProxy(writeProxy); | 		UserStatusChangeProcessor.setWriteProxy(writeProxy); | ||||||
|  |  | ||||||
| 		// TODO: Verify compatibility | 		if (!VersionUtils.verifyCompatibility(credentials.getClientVersion())) { | ||||||
| 		if (!verifyCompatibility(credentials.getClientVersion())) { |  | ||||||
| 			logger.info("The client has the wrong version."); | 			logger.info("The client has the wrong version."); | ||||||
| 			writeProxy.write(socketID, new HandshakeRejection("Wrong version")); | 			writeProxy.write(socketID, new HandshakeRejection(WRONG_VERSION)); | ||||||
| 			return; | 			return; | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
| @@ -85,7 +82,7 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred | |||||||
|  |  | ||||||
| 				// This code only gets executed if this user already exists | 				// This code only gets executed if this user already exists | ||||||
| 				logger.info("The requested user already exists."); | 				logger.info("The requested user already exists."); | ||||||
| 				writeProxy.write(socketID, new HandshakeRejection(INTERNAL_ERROR)); | 				writeProxy.write(socketID, new HandshakeRejection(USERNAME_TAKEN)); | ||||||
| 				return; | 				return; | ||||||
| 			} catch (NoResultException e) { | 			} catch (NoResultException e) { | ||||||
| 				// Creation of a new user | 				// Creation of a new user | ||||||
| @@ -101,7 +98,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred | |||||||
| 		} | 		} | ||||||
|  |  | ||||||
| 		logger.info(user + " successfully authenticated."); | 		logger.info(user + " successfully authenticated."); | ||||||
|  |  | ||||||
| 		connectionManager.registerUser(user.getID(), socketID); | 		connectionManager.registerUser(user.getID(), socketID); | ||||||
|  |  | ||||||
| 		// Change status and notify contacts about it | 		// Change status and notify contacts about it | ||||||
| @@ -125,57 +121,6 @@ public final class LoginCredentialProcessor implements ObjectProcessor<LoginCred | |||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	private boolean verifyCompatibility(String version) { |  | ||||||
| 		final var currentMatcher = versionPattern.matcher(version); |  | ||||||
|  |  | ||||||
| 		if (!currentMatcher.matches()) return false; |  | ||||||
|  |  | ||||||
| 		final var	minMatcher	= versionPattern.matcher(Startup.MIN_CLIENT_VERSION); |  | ||||||
| 		final var	maxMatcher	= versionPattern.matcher(Startup.MAX_CLIENT_VERSION); |  | ||||||
|  |  | ||||||
| 		if (!minMatcher.matches() || !maxMatcher.matches()) throw new RuntimeException("Invalid min or max client version configured!"); |  | ||||||
|  |  | ||||||
| 		// Compare suffixes |  | ||||||
| 		{ |  | ||||||
| 			final var	currentSuffix	= convertSuffix(currentMatcher.group("suffix")); |  | ||||||
| 			final var	minSuffix		= convertSuffix(minMatcher.group("suffix")); |  | ||||||
| 			final var	maxSuffix		= convertSuffix(maxMatcher.group("suffix")); |  | ||||||
|  |  | ||||||
| 			if (currentSuffix < minSuffix || currentSuffix > maxSuffix) return false; |  | ||||||
| 		} |  | ||||||
|  |  | ||||||
| 		// Compare major |  | ||||||
| 		{ |  | ||||||
| 			final var	currentMajor	= Integer.parseInt(currentMatcher.group("major")); |  | ||||||
| 			final var	minMajor		= Integer.parseInt(minMatcher.group("major")); |  | ||||||
| 			final var	maxMajor		= Integer.parseInt(maxMatcher.group("major")); |  | ||||||
|  |  | ||||||
| 			if (currentMajor < minMajor || currentMajor > maxMajor) return false; |  | ||||||
| 		} |  | ||||||
|  |  | ||||||
| 		// Compare minor |  | ||||||
| 		{ |  | ||||||
| 			final var	currentMinor	= Integer.parseInt(currentMatcher.group("minor")); |  | ||||||
| 			final var	minMinor		= Integer.parseInt(minMatcher.group("minor")); |  | ||||||
| 			final var	maxMinor		= Integer.parseInt(maxMatcher.group("minor")); |  | ||||||
|  |  | ||||||
| 			if (currentMinor < minMinor || currentMinor > maxMinor) return false; |  | ||||||
| 		} |  | ||||||
|  |  | ||||||
| 		return true; |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	private int convertSuffix(String suffix) { |  | ||||||
| 		switch (suffix == null ? "" : suffix) { |  | ||||||
| 			case "alpha": |  | ||||||
| 				return 0; |  | ||||||
| 			case "beta": |  | ||||||
| 				return 1; |  | ||||||
| 			default: |  | ||||||
| 				return 2; |  | ||||||
| 		} |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	@Override | 	@Override | ||||||
| 	public Class<LoginCredentials> getInputClass() { return LoginCredentials.class; } | 	public Class<LoginCredentials> getInputClass() { return LoginCredentials.class; } | ||||||
| } | } | ||||||
|   | |||||||
| @@ -35,7 +35,7 @@ public class MessageProcessor implements ObjectProcessor<Message> { | |||||||
| 			return; | 			return; | ||||||
| 		} | 		} | ||||||
| 		message.nextStatus(); | 		message.nextStatus(); | ||||||
| 		ConnectionManager	connectionManager	= ConnectionManager.getInstance(); | 		ConnectionManager connectionManager = ConnectionManager.getInstance(); | ||||||
|  |  | ||||||
| 		sendToUser(connectionManager, message, writeProxy); | 		sendToUser(connectionManager, message, writeProxy); | ||||||
| 		try { | 		try { | ||||||
|   | |||||||
| @@ -28,8 +28,7 @@ public class MessageStatusChangeProcessor implements ObjectProcessor<MessageStat | |||||||
| 		if (statusChange.get() != MessageStatus.READ) throw new IOException(new EnvoyException(statusChange + " has an invalid status")); | 		if (statusChange.get() != MessageStatus.READ) throw new IOException(new EnvoyException(statusChange + " has an invalid status")); | ||||||
|  |  | ||||||
| 		final var msg = persistenceManager.getMessageByID(statusChange.getID()); | 		final var msg = persistenceManager.getMessageByID(statusChange.getID()); | ||||||
| 		msg.setStatus(statusChange.get()); | 		msg.read(); | ||||||
| 		msg.setReadDate(statusChange.getDate()); |  | ||||||
| 		persistenceManager.updateMessage(msg); | 		persistenceManager.updateMessage(msg); | ||||||
|  |  | ||||||
| 		// Notifies the sender of the message about the status-update to READ | 		// Notifies the sender of the message about the status-update to READ | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user