From 3e9c2eaf0f26f69f566308b3aa760672c361dea6 Mon Sep 17 00:00:00 2001 From: zargess Date: Sun, 20 Jul 2025 19:33:01 +0200 Subject: [PATCH 1/4] Remove unused code Compression is not used by anything as has now been removed. --- .../com/fagi/compression/Compression.java | 45 ------------------- 1 file changed, 45 deletions(-) delete mode 100644 shared/src/main/java/com/fagi/compression/Compression.java diff --git a/shared/src/main/java/com/fagi/compression/Compression.java b/shared/src/main/java/com/fagi/compression/Compression.java deleted file mode 100644 index 1a6fad94..00000000 --- a/shared/src/main/java/com/fagi/compression/Compression.java +++ /dev/null @@ -1,45 +0,0 @@ -package com.fagi.compression; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.util.zip.DataFormatException; -import java.util.zip.Deflater; -import java.util.zip.Inflater; - -/** - * Created by Marcus on 05-06-2016. - */ -public class Compression { - public static byte[] compress(byte[] data) throws IOException { - Deflater deflater = new Deflater(); - deflater.setInput(data); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(data.length); - deflater.finish(); - byte[] buffer = new byte[1024]; - while (!deflater.finished()) { - int count = deflater.deflate(buffer); // returns the generated code... index - outputStream.write(buffer, 0, count); - } - outputStream.close(); - byte[] output = outputStream.toByteArray(); - System.out.println("Original: " + data.length); - System.out.println("Compressed: " + output.length); - return output; - } - - public static byte[] decompress(byte[] data) throws IOException, DataFormatException { - Inflater inflater = new Inflater(); - inflater.setInput(data); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(data.length); - byte[] buffer = new byte[1024]; - while (!inflater.finished()) { - int count = inflater.inflate(buffer); - outputStream.write(buffer, 0, count); - } - outputStream.close(); - byte[] output = outputStream.toByteArray(); - System.out.println("Original: " + output.length); - System.out.println("Compressed: " + data.length); - return output; - } -} From 58d0379a065bb356aae6ddd33dfe87f2d618bcf4 Mon Sep 17 00:00:00 2001 From: zargess Date: Sat, 19 Jul 2025 10:19:25 +0200 Subject: [PATCH 2/4] Fagi Logging Framework ## Intro We have now introduced a framework that allows the Fagi code to not care what logging framework is being used. This allows us to very easily support multiple frameworks, or completely remove the usage of a framework, without having to edit too much of the code. ## FagiLoggerFactory This class allows for all other classes in Fagi to create a logger using the registered creation strategy. This allows us to quickly change framework a very central place. This class also handles the configuration of the logging framework used, by calling a configuration strategy. Though it only allows calling the default configuration if no custom configuration has been provided to the framework in use. ## JavaLogger The current logging framework to be used is the one the java standard library provides. ## Logging Config To ensure that the logging format of the client and server are acceptable, even if the user doesn't provide a logging config properties file, we have introduced a default config. This should only be used when there is no file specified in the property: java.util.logging.config.file. The default config makes use of a console logger and a file logger. --- settings.gradle.kts | 6 +- shared/build.gradle.kts | 8 + .../java/com/fagi/logging/FagiLogger.java | 83 ++++++ .../logging/FagiLoggerConfigStrategy.java | 31 ++ .../logging/FagiLoggerCreationStrategy.java | 22 ++ .../com/fagi/logging/FagiLoggerFactory.java | 88 ++++++ .../com/fagi/logging/java/JavaLogger.java | 98 +++++++ .../java/JavaLoggerConfigStrategy.java | 56 ++++ .../java/JavaLoggerCreationStrategy.java | 16 + .../logging/java/JavaLoggerFormatter.java | 60 ++++ .../fagi/logging/FagiLoggerFactoryTest.java | 99 +++++++ .../java/JavaLoggerConfigStrategyTest.java | 275 ++++++++++++++++++ .../java/JavaLoggerCreationStrategyTest.java | 22 ++ .../logging/java/JavaLoggerFormatterTest.java | 37 +++ .../logging/java/TestJavaLoggingHandler.java | 36 +++ .../java/com/fagi/BaseFagiTest.java | 96 ++++++ .../java/com/fagi/logging/TestLogLevel.java | 11 + .../java/com/fagi/logging/TestLogRecord.java | 12 + .../java/com/fagi/logging/TestLogger.java | 130 +++++++++ .../TestLoggerConfigurationStrategy.java | 35 +++ .../logging/TestLoggerCreationStrategy.java | 22 ++ 21 files changed, 1240 insertions(+), 3 deletions(-) create mode 100644 shared/src/main/java/com/fagi/logging/FagiLogger.java create mode 100644 shared/src/main/java/com/fagi/logging/FagiLoggerConfigStrategy.java create mode 100644 shared/src/main/java/com/fagi/logging/FagiLoggerCreationStrategy.java create mode 100644 shared/src/main/java/com/fagi/logging/FagiLoggerFactory.java create mode 100644 shared/src/main/java/com/fagi/logging/java/JavaLogger.java create mode 100644 shared/src/main/java/com/fagi/logging/java/JavaLoggerConfigStrategy.java create mode 100644 shared/src/main/java/com/fagi/logging/java/JavaLoggerCreationStrategy.java create mode 100644 shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java create mode 100644 shared/src/test/java/com/fagi/logging/FagiLoggerFactoryTest.java create mode 100644 shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java create mode 100644 shared/src/test/java/com/fagi/logging/java/JavaLoggerCreationStrategyTest.java create mode 100644 shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java create mode 100644 shared/src/test/java/com/fagi/logging/java/TestJavaLoggingHandler.java create mode 100644 shared/src/testFixtures/java/com/fagi/BaseFagiTest.java create mode 100644 shared/src/testFixtures/java/com/fagi/logging/TestLogLevel.java create mode 100644 shared/src/testFixtures/java/com/fagi/logging/TestLogRecord.java create mode 100644 shared/src/testFixtures/java/com/fagi/logging/TestLogger.java create mode 100644 shared/src/testFixtures/java/com/fagi/logging/TestLoggerConfigurationStrategy.java create mode 100644 shared/src/testFixtures/java/com/fagi/logging/TestLoggerCreationStrategy.java diff --git a/settings.gradle.kts b/settings.gradle.kts index a5cbd95a..d0449477 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -1,5 +1,5 @@ rootProject.name = "fagi" -include("fagiClient") -include("shared") -include("fagiServer") \ No newline at end of file +include(":fagiClient") +include(":fagiServer") +include(":shared") \ No newline at end of file diff --git a/shared/build.gradle.kts b/shared/build.gradle.kts index 95e30793..baf9f80f 100644 --- a/shared/build.gradle.kts +++ b/shared/build.gradle.kts @@ -1,9 +1,17 @@ +plugins { + id("java-test-fixtures") +} + tasks.test { useJUnitPlatform() } dependencies { implementation(libs.gson) + + testFixturesImplementation(libs.junit.jupiter.api) + + testImplementation(testFixtures(project(":shared"))) testImplementation(libs.bundles.junit.base) testImplementation(libs.junit.platform) testImplementation(libs.bundles.mockito) diff --git a/shared/src/main/java/com/fagi/logging/FagiLogger.java b/shared/src/main/java/com/fagi/logging/FagiLogger.java new file mode 100644 index 00000000..fd6d1aad --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/FagiLogger.java @@ -0,0 +1,83 @@ +package com.fagi.logging; + +import java.util.function.Supplier; + +/** + *

+ * A Logger interface used to log messages for the Fagi server and client. + *

+ *

+ * This interface helps hide what logging framework is used, and thus makes it simpler to change framework without having to change a lot of code. + *

+ * + * @author Marcus Haagh + */ +public interface FagiLogger { + /** + * Logs message in DEBUG level + * + * @param messageSupplier a supplier that returns the message to be logged + */ + void debug(Supplier messageSupplier); + + /** + * Logs message in DEBUG level along with the associated throwable + * + * @param throwable the throwable to be logged + * @param messageSupplier a supplier that returns the message to be logged + */ + void debug( + Throwable throwable, + Supplier messageSupplier); + + /** + * Logs message in INFO level + * + * @param messageSupplier a supplier that returns the message to be logged + */ + void info(Supplier messageSupplier); + + /** + * Logs message in INFO level along with the associated throwable + * + * @param throwable the throwable to be logged + * @param messageSupplier a supplier that returns the message to be logged + */ + void info( + Throwable throwable, + Supplier messageSupplier); + + /** + * Logs message in WARNING level + * + * @param messageSupplier a supplier that returns the message to be logged + */ + void warning(Supplier messageSupplier); + + /** + * Logs message in WARNING level along with the associated throwable + * + * @param throwable the throwable to be logged + * @param messageSupplier a supplier that returns the message to be logged + */ + void warning( + Throwable throwable, + Supplier messageSupplier); + + /** + * Logs message in ERROR level + * + * @param messageSupplier a supplier that returns the message to be logged + */ + void error(Supplier messageSupplier); + + /** + * Logs message in ERROR level along with the associated throwable + * + * @param throwable the throwable to be logged + * @param messageSupplier a supplier that returns the message to be logged + */ + void error( + Throwable throwable, + Supplier messageSupplier); +} diff --git a/shared/src/main/java/com/fagi/logging/FagiLoggerConfigStrategy.java b/shared/src/main/java/com/fagi/logging/FagiLoggerConfigStrategy.java new file mode 100644 index 00000000..6edd46b9 --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/FagiLoggerConfigStrategy.java @@ -0,0 +1,31 @@ +package com.fagi.logging; + +import java.nio.file.Path; + +/** + *

+ * This strategy can check if a default configuration should be used and create the default logging configuration. + *

+ *

+ * The strategy must be set in the {@link FagiLoggerFactory} in order to be used. + *

+ * + * @author Marcus Haagh + * @see FagiLoggerFactory + */ +public interface FagiLoggerConfigStrategy { + /** + * Checks if custom configuration has been provided. + * + * @return true if logging configuration has been provided by the user + */ + boolean isCustomConfigurationAvailable(); + + /** + * Sets up the logging in a default manner. This ensures that proper logging is configured, even if the user hasn't + * provided a log config file. + * + * @param logFile the path of the resulting log file. + */ + void setupDefaultConfiguration(Path logFile); +} diff --git a/shared/src/main/java/com/fagi/logging/FagiLoggerCreationStrategy.java b/shared/src/main/java/com/fagi/logging/FagiLoggerCreationStrategy.java new file mode 100644 index 00000000..cd40ae31 --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/FagiLoggerCreationStrategy.java @@ -0,0 +1,22 @@ +package com.fagi.logging; + +/** + *

+ * A strategy to help create a {@link FagiLogger}. + *

+ *

+ * The strategy should hold the details on how to instantiate a new instance of a {@link FagiLogger}. The strategy must be set in the {@link FagiLoggerFactory} in order to be used. + *

+ * + * @author Marcus Haagh + * @see FagiLogger + */ +public interface FagiLoggerCreationStrategy { + /** + * Creates a logger for the given class. + * + * @param tClass the class the new logger should be associated with. + * @return a {@link FagiLogger} + */ + FagiLogger createLogger(Class tClass); +} diff --git a/shared/src/main/java/com/fagi/logging/FagiLoggerFactory.java b/shared/src/main/java/com/fagi/logging/FagiLoggerFactory.java new file mode 100644 index 00000000..ad7ab982 --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/FagiLoggerFactory.java @@ -0,0 +1,88 @@ +package com.fagi.logging; + +import com.fagi.logging.java.JavaLoggerConfigStrategy; +import com.fagi.logging.java.JavaLoggerCreationStrategy; + +import java.nio.file.Path; + +/** + * This class is used to aid in creating and configuring {@link FagiLogger}s. This is to make it simpler to change + * logging frameworks, both during testing and in cases where we want to change the entire framework. + * + * @author Marcus Haagh + * @see FagiLogger + * @see FagiLoggerCreationStrategy + * @see FagiLoggerConfigStrategy + */ +public class FagiLoggerFactory { + /** + * This contains the creation strategy to be used. Should this change, then make sure that the config strategy is + * changed as well. + */ + private static FagiLoggerCreationStrategy loggerCreationStrategy = new JavaLoggerCreationStrategy(); + /** + * This contains the config strategy to be used.Should this change, then make sure that the creation strategy is + * changed as well. + */ + private static FagiLoggerConfigStrategy loggerConfigStrategy = new JavaLoggerConfigStrategy(); + private static FagiLogger LOGGER = createLogger(FagiLoggerFactory.class); + + private FagiLoggerFactory() { + throw new UnsupportedOperationException("Factory class - do not instantiate"); + } + + /** + * Creates a FagiLogger for the given class + * + * @param tClass the class for the logger to be created for + * @return a FagiLogger + */ + public static FagiLogger createLogger(Class tClass) { + return loggerCreationStrategy.createLogger(tClass); + } + + /** + * Determines if custom configuration has been provided by the user. If that's the case, + * then configuration should not be overridden with the default configuration. + * + * @return true if logging configuration has been provided by the user + */ + public static boolean isCustomConfigurationAvailable() { + return loggerConfigStrategy.isCustomConfigurationAvailable(); + } + + /** + *

+ * Sets up the default logging configuration. + *

+ *

+ * Does nothing if {@link FagiLoggerFactory#isCustomConfigurationAvailable()} is true + *

+ * + * @param logFile the file where logs will be stored + */ + public static void setupDefaultConfiguration(Path logFile) { + if (!isCustomConfigurationAvailable()) { + loggerConfigStrategy.setupDefaultConfiguration(logFile); + } else { + LOGGER.debug(() -> "Custom configuration has been provided. No further configuration is made."); + } + } + + public static void setLoggerCreationStrategy(FagiLoggerCreationStrategy strategy) { + loggerCreationStrategy = strategy; + } + + public static void setLoggerConfigStrategy(FagiLoggerConfigStrategy loggerConfigStrategy) { + FagiLoggerFactory.loggerConfigStrategy = loggerConfigStrategy; + } + + /** + * Used for testing as the logger on the class is created before any test class can override the strategies. + * + * @param logger the logger to be used by the factory + */ + static void setLogger(FagiLogger logger) { + LOGGER = logger; + } +} diff --git a/shared/src/main/java/com/fagi/logging/java/JavaLogger.java b/shared/src/main/java/com/fagi/logging/java/JavaLogger.java new file mode 100644 index 00000000..939975bd --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/java/JavaLogger.java @@ -0,0 +1,98 @@ +package com.fagi.logging.java; + +import com.fagi.logging.FagiLogger; + +import java.util.function.Supplier; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * A {@link FagiLogger} implementation that uses {@link Logger} from the Java Standard Library. + * + * @author Marcus Haagh + * @see FagiLogger + * @see Logger + */ +public class JavaLogger implements FagiLogger { + private final Logger logger; + + public JavaLogger(Class tClass) { + this.logger = Logger.getLogger(tClass.getName()); + } + + @Override + public void debug(Supplier messageSupplier) { + logger.log( + Level.FINE, + messageSupplier + ); + } + + @Override + public void debug( + Throwable throwable, + Supplier messageSupplier) { + logger.log( + Level.FINE, + throwable, + messageSupplier + ); + } + + @Override + public void info(Supplier messageSupplier) { + logger.log( + Level.INFO, + messageSupplier + ); + } + + @Override + public void info( + Throwable throwable, + Supplier messageSupplier) { + logger.log( + Level.INFO, + throwable, + messageSupplier + ); + } + + @Override + public void warning(Supplier messageSupplier) { + logger.log( + Level.WARNING, + messageSupplier + ); + } + + @Override + public void warning( + Throwable throwable, + Supplier messageSupplier) { + logger.log( + Level.WARNING, + throwable, + messageSupplier + ); + } + + @Override + public void error(Supplier messageSupplier) { + logger.log( + Level.SEVERE, + messageSupplier + ); + } + + @Override + public void error( + Throwable throwable, + Supplier messageSupplier) { + logger.log( + Level.SEVERE, + throwable, + messageSupplier + ); + } +} diff --git a/shared/src/main/java/com/fagi/logging/java/JavaLoggerConfigStrategy.java b/shared/src/main/java/com/fagi/logging/java/JavaLoggerConfigStrategy.java new file mode 100644 index 00000000..b47cc8f0 --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/java/JavaLoggerConfigStrategy.java @@ -0,0 +1,56 @@ +package com.fagi.logging.java; + +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerConfigStrategy; +import com.fagi.logging.FagiLoggerFactory; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.logging.ConsoleHandler; +import java.util.logging.FileHandler; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * This strategy configures {@link Logger} from the Java Standard Library. + * + * @author Marcus Haagh + */ +public class JavaLoggerConfigStrategy implements FagiLoggerConfigStrategy { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(JavaLoggerConfigStrategy.class); + static final String LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME = "java.util.logging.config.file"; + + @Override + public boolean isCustomConfigurationAvailable() { + return System.getProperty(LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME) != null; + } + + @Override + public void setupDefaultConfiguration(Path logFile) { + var logLevel = Level.INFO; + var rootLogger = Logger.getLogger(""); + rootLogger.setLevel(logLevel); + + for (var handler : rootLogger.getHandlers()) { + rootLogger.removeHandler(handler); + } + + var consoleHandler = new ConsoleHandler(); + consoleHandler.setLevel(logLevel); + consoleHandler.setFormatter(new JavaLoggerFormatter()); + + rootLogger.addHandler(consoleHandler); + + try { + var fileHandler = new FileHandler(logFile.toString()); + fileHandler.setLevel(logLevel); + fileHandler.setFormatter(new JavaLoggerFormatter()); + rootLogger.addHandler(fileHandler); + } catch (IOException e) { + LOGGER.error( + e, + () -> "Failed to setup file handler for logger. Log statements will not be saved in files." + ); + } + } +} diff --git a/shared/src/main/java/com/fagi/logging/java/JavaLoggerCreationStrategy.java b/shared/src/main/java/com/fagi/logging/java/JavaLoggerCreationStrategy.java new file mode 100644 index 00000000..07952da6 --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/java/JavaLoggerCreationStrategy.java @@ -0,0 +1,16 @@ +package com.fagi.logging.java; + +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerCreationStrategy; + +/** + * Creates a {@link java.util.logging.Logger} from the Java Standard Library + * + * @author Marcus Haagh + */ +public class JavaLoggerCreationStrategy implements FagiLoggerCreationStrategy { + @Override + public FagiLogger createLogger(Class tClass) { + return new JavaLogger(tClass); + } +} diff --git a/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java b/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java new file mode 100644 index 00000000..df988ace --- /dev/null +++ b/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java @@ -0,0 +1,60 @@ +package com.fagi.logging.java; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.logging.Formatter; +import java.util.logging.LogRecord; + +/** + *

+ * This class formats logs in the default Fagi format. The format looks as follows: + *

+ *

+ * yyyy-MM-dd HH:mm:ss [LOG LEVEL] full.classpath.for.class - LOG MESSAGE + *

+ *

+ * The following is an example of a log statement: + *

+ *

+ * 2025-07-19 09:42:37 [INFO] com.fagi.server.Server - Starting Server + *

+ *

+ * If there is an exception in the log, the stacktrace will start on the next line. + *

+ *

+ * This should only be used if the user hasn't specified their own logging configuration file. + *

+ * + * @author Marcus Haagh + */ +public class JavaLoggerFormatter extends Formatter { + final SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); + + @Override + public String format(LogRecord record) { + String loggerName = record.getLoggerName(); + + StringBuilder sb = new StringBuilder(); + + sb.append(dateFormat.format(new Date(record.getMillis()))); + + sb.append(String.format( + " [%s] %s - %s%n", + record.getLevel(), + loggerName, + formatMessage(record) + )); + + if (record.getThrown() != null) { + StringWriter sw = new StringWriter(); + record + .getThrown() + .printStackTrace(new PrintWriter(sw)); + sb.append(sw); + } + + return sb.toString(); + } +} diff --git a/shared/src/test/java/com/fagi/logging/FagiLoggerFactoryTest.java b/shared/src/test/java/com/fagi/logging/FagiLoggerFactoryTest.java new file mode 100644 index 00000000..f7d0d6e4 --- /dev/null +++ b/shared/src/test/java/com/fagi/logging/FagiLoggerFactoryTest.java @@ -0,0 +1,99 @@ +package com.fagi.logging; + +import com.fagi.BaseFagiTest; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; +import java.util.List; + +public class FagiLoggerFactoryTest extends BaseFagiTest { + @Test + void testCanCreateLogger() { + FagiLogger logger = FagiLoggerFactory.createLogger(FagiLoggerFactoryTest.class); + + Assertions.assertNotNull(logger); + Assertions.assertInstanceOf( + TestLogger.class, + logger + ); + Assertions.assertEquals( + FagiLoggerFactoryTest.class, + ((TestLogger) logger).getLoggerClass() + ); + } + + @Test + void testConfigurationAvailableWhenStrategyReturnsTrue() { + FagiLoggerFactory.setLoggerConfigStrategy(new TestLoggerConfigurationStrategy(true)); + + Assertions.assertTrue(FagiLoggerFactory.isCustomConfigurationAvailable()); + } + + @Test + void testConfigurationNotAvailableWhenStrategyReturnsFalse() { + FagiLoggerFactory.setLoggerConfigStrategy(new TestLoggerConfigurationStrategy(false)); + + Assertions.assertFalse(FagiLoggerFactory.isCustomConfigurationAvailable()); + } + + @Test + void testFactoryUsesConfigurationStrategyWhenNoCustomConfigurationIsProvided() { + // Creates logger to be used by the factory. This allows us to check the log records later. + FagiLoggerFactory.setLogger(FagiLoggerFactory.createLogger(FagiLoggerFactory.class)); + + TestLoggerConfigurationStrategy loggerConfigStrategy = new TestLoggerConfigurationStrategy(false); + FagiLoggerFactory.setLoggerConfigStrategy(loggerConfigStrategy); + + Path logFile = Path.of("/some/path"); + FagiLoggerFactory.setupDefaultConfiguration(logFile); + + List> testLogRecords = lookupLogRecordsForClass(FagiLoggerFactory.class); + + Assertions.assertAll( + () -> Assertions.assertTrue(testLogRecords.isEmpty()), + () -> Assertions.assertTrue(loggerConfigStrategy.isConfigured()), + () -> Assertions.assertEquals( + logFile, + loggerConfigStrategy.getLogFile() + ) + ); + } + + @Test + void testFactoryDebugLogsWhenItShouldntUseDefaultConfiguration() { + // Creates logger to be used by the factory. This allows us to check the log records later. + FagiLoggerFactory.setLogger(FagiLoggerFactory.createLogger(FagiLoggerFactory.class)); + + TestLoggerConfigurationStrategy loggerConfigStrategy = new TestLoggerConfigurationStrategy(true); + FagiLoggerFactory.setLoggerConfigStrategy(loggerConfigStrategy); + + FagiLoggerFactory.setupDefaultConfiguration(Path.of("/some/path")); + + List> testLogRecords = lookupLogRecordsForClass(FagiLoggerFactory.class); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + TestLogLevel.DEBUG, + logRecord.logLevel() + ), + () -> Assertions.assertEquals( + FagiLoggerFactory.class, + logRecord.loggerClass() + ), + () -> Assertions.assertEquals( + "Custom configuration has been provided. No further configuration is made.", + logRecord.message() + ), + () -> Assertions.assertNull(logRecord.throwable()), + () -> Assertions.assertFalse(loggerConfigStrategy.isConfigured()) + ); + } +} diff --git a/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java b/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java new file mode 100644 index 00000000..cf4a654c --- /dev/null +++ b/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java @@ -0,0 +1,275 @@ +package com.fagi.logging.java; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.List; +import java.util.logging.ConsoleHandler; +import java.util.logging.FileHandler; +import java.util.logging.Handler; +import java.util.logging.Level; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +/** + * Doesn't utilise {@link com.fagi.BaseFagiTest} as we want to test the real logging framework to check if the + * logs work as expected. + */ +class JavaLoggerConfigStrategyTest { + private static Path tempDir; + private final Logger LOGGER = Logger.getLogger(JavaLoggerConfigStrategy.class.getName()); + private final TestJavaLoggingHandler testJavaLoggingHandler = new TestJavaLoggingHandler(); + private final JavaLoggerConfigStrategy strategy = new JavaLoggerConfigStrategy(); + + @BeforeAll + static void setupClass() throws IOException { + tempDir = Path.of("build/test_logs"); + // Deleting the "build/test_logs" folder before executing the test. + // This is done to mitigate Windows locking the log files in such a way that they cannot be + // deleted after running the tests. + tempDir + .toFile() + .delete(); + + // Create the "build/test_logs" folder + Files.createDirectory(tempDir); + } + + @BeforeEach + void setup() { + LOGGER.addHandler(testJavaLoggingHandler); + } + + @Test + void testRootLoggerHasConsoleAndFileHandlersAndBothHasCorrectFormatter() { + strategy.setupDefaultConfiguration(createLogFile()); + + var logger = Logger.getLogger(""); + + Assertions.assertAll( + () -> Assertions.assertEquals( + 2, + logger.getHandlers().length + ), + () -> Assertions.assertTrue(Arrays + .stream(logger.getHandlers()) + .anyMatch(h -> h instanceof ConsoleHandler)), + () -> Assertions.assertTrue(Arrays + .stream(logger.getHandlers()) + .anyMatch(h -> h instanceof FileHandler)), + () -> Assertions.assertTrue(Arrays + .stream(logger.getHandlers()) + .allMatch(h -> h.getFormatter() instanceof JavaLoggerFormatter)) + ); + + cleanHandlers(); + } + + @Test + void testConfigLogsIfFailsToMakeFileHandler() throws IOException { + Path restrictedFile = createLogFile(); + Files.createFile(restrictedFile); + + // Remove read/write permissions on file to force IOException. + restrictedFile + .toFile() + .setReadable( + false, + false + ); + restrictedFile + .toFile() + .setWritable( + false, + false + ); + + strategy.setupDefaultConfiguration(restrictedFile.toAbsolutePath()); + + List logRecords = testJavaLoggingHandler.getLogRecords(); + Assertions.assertAll( + () -> Assertions.assertEquals( + 1, + logRecords.size() + ), + () -> Assertions.assertEquals( + Level.SEVERE, + logRecords + .getFirst() + .getLevel() + ), + () -> Assertions.assertEquals( + "Failed to setup file handler for logger. Log statements will not be saved in files.", + logRecords + .getFirst() + .getMessage() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecords + .getFirst() + .getThrown() + ) + ); + + cleanHandlers(); + } + + @Test + void testShouldNotLogFineRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.fine(() -> "This record should not be recorded."); + + Assertions.assertEquals( + 0, + testJavaLoggingHandler + .getLogRecords() + .size() + ); + + cleanHandlers(); + } + + @Test + void testShouldNotLogFinerRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.finer(() -> "This record should not be recorded."); + + Assertions.assertEquals( + 0, + testJavaLoggingHandler + .getLogRecords() + .size() + ); + + cleanHandlers(); + } + + @Test + void testShouldNotLogFinestRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.finest(() -> "This record should not be recorded."); + + Assertions.assertEquals( + 0, + testJavaLoggingHandler + .getLogRecords() + .size() + ); + + cleanHandlers(); + } + + @Test + void testShouldLogInfoRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.info(() -> "This record should be recorded."); + + Assertions.assertAll(() -> Assertions.assertEquals( + 1, + testJavaLoggingHandler + .getLogRecords() + .size() + )); + + cleanHandlers(); + } + + @Test + void testShouldLogWarningRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.warning(() -> "This record should be recorded."); + + Assertions.assertAll(() -> Assertions.assertEquals( + 1, + testJavaLoggingHandler + .getLogRecords() + .size() + )); + + cleanHandlers(); + } + + @Test + void testShouldLogSevereRecords() { + strategy.setupDefaultConfiguration(createLogFile()); + + LOGGER.severe(() -> "This record should be recorded."); + + Assertions.assertAll(() -> Assertions.assertEquals( + 1, + testJavaLoggingHandler + .getLogRecords() + .size() + )); + + cleanHandlers(); + } + + @Test + void testCustomConfigCheckIsTrueWhenPropertyIsNonNull() { + String sysPropertyOldValue = System.getProperty(JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME); + System.setProperty( + JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME, + "Some non-null value" + ); + + boolean isCustomConfigAvailable = strategy.isCustomConfigurationAvailable(); + + Assertions.assertTrue(isCustomConfigAvailable); + + if (sysPropertyOldValue != null) { + System.setProperty( + JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME, + sysPropertyOldValue + ); + } else { + System.clearProperty(JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME); + } + + cleanHandlers(); + } + + @Test + void testCustomConfigCheckIsFalseWhenPropertyIsNull() { + String sysPropertyOldValue = System.getProperty(JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME); + System.clearProperty(JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME); + + boolean isCustomConfigAvailable = strategy.isCustomConfigurationAvailable(); + + Assertions.assertFalse(isCustomConfigAvailable); + + if (sysPropertyOldValue != null) { + System.setProperty( + JavaLoggerConfigStrategy.LOGGING_CONFIG_FILE_SYSTEM_PROPERTY_NAME, + sysPropertyOldValue + ); + } + + cleanHandlers(); + } + + private static Path createLogFile() { + return tempDir.resolve("test_" + System.currentTimeMillis() + ".log"); + } + + private void cleanHandlers() { + Handler[] handlers = LOGGER.getHandlers(); + for (Handler handler : handlers) { + handler.flush(); + handler.close(); + LOGGER.removeHandler(handler); + } + } +} diff --git a/shared/src/test/java/com/fagi/logging/java/JavaLoggerCreationStrategyTest.java b/shared/src/test/java/com/fagi/logging/java/JavaLoggerCreationStrategyTest.java new file mode 100644 index 00000000..afb71b8f --- /dev/null +++ b/shared/src/test/java/com/fagi/logging/java/JavaLoggerCreationStrategyTest.java @@ -0,0 +1,22 @@ +package com.fagi.logging.java; + +import com.fagi.logging.FagiLogger; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class JavaLoggerCreationStrategyTest { + @Test + void verifyStrategyCreatesJavaLogger() { + var strategy = new JavaLoggerCreationStrategy(); + + FagiLogger logger = strategy.createLogger(JavaLoggerCreationStrategyTest.class); + + Assertions.assertAll( + () -> Assertions.assertNotNull(logger), + () -> Assertions.assertInstanceOf( + JavaLogger.class, + logger + ) + ); + } +} diff --git a/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java b/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java new file mode 100644 index 00000000..2196e652 --- /dev/null +++ b/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java @@ -0,0 +1,37 @@ +package com.fagi.logging.java; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.Date; +import java.util.logging.Level; +import java.util.logging.LogRecord; + +class JavaLoggerFormatterTest { + private final JavaLoggerFormatter formatter = new JavaLoggerFormatter(); + + @Test + void testFormatterShouldGiveCorrectFormattedStrings() { + var logRecord = new LogRecord( + Level.WARNING, + "This is the log message" + ); + logRecord.setLoggerName(JavaLoggerFormatterTest.class.getName()); + logRecord.setThrown(new IOException()); + + String dateTimeString = formatter.dateFormat.format(new Date(logRecord.getMillis())); + + var expectedFormattedLogEntry = dateTimeString + " [" + logRecord.getLevel() + "] " + logRecord.getLoggerName() + " - " + logRecord.getMessage() + System.lineSeparator() + IOException.class.getName(); + + // Normalize line endings such that the test works on different operating systems + String formattedLogRecord = formatter + .format(logRecord) + .replace( + "\n\r", + System.lineSeparator() + ); + + Assertions.assertTrue(formattedLogRecord.startsWith(expectedFormattedLogEntry)); + } +} \ No newline at end of file diff --git a/shared/src/test/java/com/fagi/logging/java/TestJavaLoggingHandler.java b/shared/src/test/java/com/fagi/logging/java/TestJavaLoggingHandler.java new file mode 100644 index 00000000..da094395 --- /dev/null +++ b/shared/src/test/java/com/fagi/logging/java/TestJavaLoggingHandler.java @@ -0,0 +1,36 @@ +package com.fagi.logging.java; + +import java.util.ArrayList; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.LogRecord; + +/** + * This class is used to record all logs sent to a logger + */ +public class TestJavaLoggingHandler extends Handler { + private final List logRecords = new ArrayList<>(); + + @Override + public void publish(LogRecord record) { + logRecords.add(record); + } + + /** + * Does nothing + */ + @Override + public void flush() { + } + + /** + * Does nothing + */ + @Override + public void close() throws SecurityException { + } + + public List getLogRecords() { + return logRecords; + } +} diff --git a/shared/src/testFixtures/java/com/fagi/BaseFagiTest.java b/shared/src/testFixtures/java/com/fagi/BaseFagiTest.java new file mode 100644 index 00000000..2a71407d --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/BaseFagiTest.java @@ -0,0 +1,96 @@ +package com.fagi; + +import com.fagi.logging.FagiLoggerFactory; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; +import com.fagi.logging.TestLogger; +import com.fagi.logging.TestLoggerConfigurationStrategy; +import com.fagi.logging.TestLoggerCreationStrategy; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +/** + *

+ * This abstract class offers utility to make unit tests easier to write. + *

+ *

+ * All unit tests should inherit from this class. + *

+ */ +public abstract class BaseFagiTest { + /** + * All loggers registered during a test class will be stored here. This allows for reading log records + * and clearing log records between tests. + */ + private static final Map, TestLogger> LOGGERS = new HashMap<>(); + + @BeforeAll + static void baseTestClassSetup() { + FagiLoggerFactory.setLoggerCreationStrategy(new TestLoggerCreationStrategy(LOGGERS)); + FagiLoggerFactory.setLoggerConfigStrategy(new TestLoggerConfigurationStrategy(false)); + } + + /** + *

+ * Performs tear down after every test. + *

+ *

+ * This currently does the following: + *

    + *
  • Clears log records from all registered loggers.
  • + *
+ *

+ */ + @AfterEach + void baseTestTearDown() { + for (TestLogger logger : LOGGERS.values()) { + logger.clearRecords(); + } + } + + /** + *

+ * Allows tests to lookup log records from a logger registered on the given Class. + *

+ *

+ * To be used when testing if logs have been made correctly. + *

+ * + * @param tClass the class the logger was registered on. + * @return a list of records made with a logger registered on the given Class. + * @throws AssertionError if no logger has been registered on the given Class. + */ + public List> lookupLogRecordsForClass(Class tClass) { + var logger = LOGGERS.get(tClass); + if (logger != null) { + return logger.getLogRecords(); + } + throw new AssertionError("No logger has been registered for class: " + tClass); + } + + /** + *

+ * Allows tests to lookup log records from a logger registered on the given Class with the given log level. + *

+ *

+ * To be used when testing if logs have been made correctly. + *

+ * + * @param tClass the class the logger was registered on. + * @param logLevel the log level of records to be found. + * @return a list of records made with a logger registered on the given Class at the given level. + * @throws AssertionError if no logger has been registered on the given Class. + */ + public List> lookupLogRecordsForClass( + Class tClass, + TestLogLevel logLevel) { + return lookupLogRecordsForClass(tClass) + .stream() + .filter(logRecord -> logLevel.equals(logRecord.logLevel())) + .toList(); + } +} diff --git a/shared/src/testFixtures/java/com/fagi/logging/TestLogLevel.java b/shared/src/testFixtures/java/com/fagi/logging/TestLogLevel.java new file mode 100644 index 00000000..7e7720b9 --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/logging/TestLogLevel.java @@ -0,0 +1,11 @@ +package com.fagi.logging; + +/** + * A test representation of log levels. + */ +public enum TestLogLevel { + DEBUG, + INFO, + WARNING, + ERROR +} diff --git a/shared/src/testFixtures/java/com/fagi/logging/TestLogRecord.java b/shared/src/testFixtures/java/com/fagi/logging/TestLogRecord.java new file mode 100644 index 00000000..5eb7c4fe --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/logging/TestLogRecord.java @@ -0,0 +1,12 @@ +package com.fagi.logging; + +/** + * A test representation of a log record. Made to be agnostic about the logging framework used by the production code. + * + * @param loggerClass the Class the logger that made the record was registered on. + * @param logLevel the log level the log record was made to. + * @param message the logged message. + * @param throwable the throwable associated with the message. + */ +public record TestLogRecord(Class loggerClass, TestLogLevel logLevel, String message, Throwable throwable) { +} diff --git a/shared/src/testFixtures/java/com/fagi/logging/TestLogger.java b/shared/src/testFixtures/java/com/fagi/logging/TestLogger.java new file mode 100644 index 00000000..497c2bd3 --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/logging/TestLogger.java @@ -0,0 +1,130 @@ +package com.fagi.logging; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; + +/** + *

+ * This class is used to simulate a logger during unit tests, making it easier to test that the correct + * logs were made. + *

+ *

+ * All log records are stored in a list, ready to be read by tests. + *

+ *

+ * Log records should be cleared after every unit test as loggers are stored statically, + * making it possible for records to carry over to other tests. + *

+ */ +public class TestLogger implements FagiLogger { + private final List> logRecords = new ArrayList<>(); + private final Class loggerClass; + + public TestLogger(Class tClass) { + this.loggerClass = tClass; + } + + @Override + public void debug(Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.DEBUG, + messageSupplier.get(), + null + )); + } + + @Override + public void debug( + Throwable throwable, + Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.DEBUG, + messageSupplier.get(), + throwable + )); + } + + @Override + public void info(Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.INFO, + messageSupplier.get(), + null + )); + } + + @Override + public void info( + Throwable throwable, + Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.INFO, + messageSupplier.get(), + throwable + )); + } + + @Override + public void warning(Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.WARNING, + messageSupplier.get(), + null + )); + } + + @Override + public void warning( + Throwable throwable, + Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.WARNING, + messageSupplier.get(), + throwable + )); + } + + @Override + public void error(Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.ERROR, + messageSupplier.get(), + null + )); + } + + @Override + public void error( + Throwable throwable, + Supplier messageSupplier) { + logRecords.add(new TestLogRecord<>( + loggerClass, + TestLogLevel.ERROR, + messageSupplier.get(), + throwable + )); + } + + public List> getLogRecords() { + return logRecords; + } + + public Class getLoggerClass() { + return loggerClass; + } + + /** + * Clears the stored log records. Should be done after every unit test. + */ + public void clearRecords() { + logRecords.clear(); + } +} diff --git a/shared/src/testFixtures/java/com/fagi/logging/TestLoggerConfigurationStrategy.java b/shared/src/testFixtures/java/com/fagi/logging/TestLoggerConfigurationStrategy.java new file mode 100644 index 00000000..451231a2 --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/logging/TestLoggerConfigurationStrategy.java @@ -0,0 +1,35 @@ +package com.fagi.logging; + +import java.nio.file.Path; + +/** + * Used to help test logging + */ +public class TestLoggerConfigurationStrategy implements FagiLoggerConfigStrategy { + private final boolean customConfigurationAvailable; + private boolean configured = false; + private Path logFile; + + public TestLoggerConfigurationStrategy(boolean customConfigurationAvailable) { + this.customConfigurationAvailable = customConfigurationAvailable; + } + + @Override + public boolean isCustomConfigurationAvailable() { + return customConfigurationAvailable; + } + + @Override + public void setupDefaultConfiguration(Path logFile) { + configured = true; + this.logFile = logFile; + } + + public boolean isConfigured() { + return configured; + } + + public Path getLogFile() { + return logFile; + } +} diff --git a/shared/src/testFixtures/java/com/fagi/logging/TestLoggerCreationStrategy.java b/shared/src/testFixtures/java/com/fagi/logging/TestLoggerCreationStrategy.java new file mode 100644 index 00000000..7eea4ab1 --- /dev/null +++ b/shared/src/testFixtures/java/com/fagi/logging/TestLoggerCreationStrategy.java @@ -0,0 +1,22 @@ +package com.fagi.logging; + +import java.util.Map; + +/** + * Test creation strategy that stores created loggers to simulate the same behaviour as normal logger creation + */ +public class TestLoggerCreationStrategy implements FagiLoggerCreationStrategy { + private final Map, TestLogger> loggers; + + public TestLoggerCreationStrategy(Map, TestLogger> loggers) { + this.loggers = loggers; + } + + @Override + public FagiLogger createLogger(Class tClass) { + return loggers.computeIfAbsent( + tClass, + (k) -> new TestLogger<>(tClass) + ); + } +} From 28cccab6797df569907953943bd6c7dd386b9579 Mon Sep 17 00:00:00 2001 From: zargess Date: Thu, 17 Jul 2025 15:29:28 +0200 Subject: [PATCH 3/4] Using fagi logging framework The usage of System.out, System.err and exception.printStackTrace, has been replaced with the Fagi Logging framework. This is to allow for storing logs in files and better granularity in the level of logs. Also deleted the old Logger class that stored logs in a *.fagi file. --- .gitignore | 2 + fagiClient/build.gradle.kts | 2 + .../java/com/fagi/action/items/LoadFXML.java | 10 +- .../action/items/OpenConversationFromID.java | 5 +- .../java/com/fagi/controller/MainScreen.java | 97 +++++-- .../fagi/controller/login/MasterLogin.java | 17 +- .../src/main/java/com/fagi/main/FagiApp.java | 17 +- .../java/com/fagi/network/ChatManager.java | 18 +- .../java/com/fagi/network/Communication.java | 27 +- .../java/com/fagi/network/InputHandler.java | 17 +- .../handlers/DefaultThreadHandler.java | 4 +- .../fagi/network/handlers/GeneralHandler.java | 24 +- .../network/handlers/TextMessageHandler.java | 24 +- .../java/com/fagi/threads/ThreadPool.java | 4 - .../com/fagi/action/items/LoadFXMLTest.java | 3 +- .../com/fagi/guitests/ConversationTests.java | 3 +- .../fagi/guitests/CreatePasswordTests.java | 3 +- .../fagi/guitests/CreateUserNameTests.java | 3 +- .../java/com/fagi/guitests/FriendsTests.java | 3 +- .../com/fagi/guitests/InviteCodeTests.java | 3 +- .../java/com/fagi/guitests/LoginTests.java | 3 +- .../guitests/ReceiveFriendRequestTests.java | 3 +- .../com/fagi/guitests/SearchUserTests.java | 3 +- .../fagi/guitests/SendFriendRequestTests.java | 3 +- .../java/com/fagi/guitests/SignOutTests.java | 3 +- .../com/fagi/network/CommunicationTest.java | 3 +- .../java/com/fagi/util/FontUtilsTest.java | 3 +- fagiServer/build.gradle.kts | 2 + .../java/com/fagi/encryption/Encryption.java | 14 +- .../com/fagi/handler/ConversationHandler.java | 8 +- .../java/com/fagi/handler/InputHandler.java | 7 +- .../src/main/java/com/fagi/main/Main.java | 20 +- .../src/main/java/com/fagi/model/Data.java | 5 +- .../src/main/java/com/fagi/server/Server.java | 22 +- .../java/com/fagi/worker/InputWorker.java | 26 +- .../java/com/fagi/worker/OutputWorker.java | 15 +- .../java/TextMessageIntegrationTests.java | 3 +- .../com/fagi/encryption/EncryptionTests.java | 130 +++++++-- .../fagi/handler/ConversationHandlerTest.java | 49 +++- .../inputhandler/BaseInputHandlerTest.java | 3 +- .../inputhandler/NotValidRequestTests.java | 62 +++-- .../test/java/com/fagi/model/DataTests.java | 39 +-- .../test/java/com/fagi/model/UserTests.java | 3 +- .../ServerConversationHandlerTests.java | 2 +- .../com/fagi/server/ServerLoggingTests.java | 182 ++++++++----- .../java/com/fagi/server/ServerTests.java | 3 +- .../com/fagi/server/ServerWorkerTests.java | 2 +- .../util/{ => running}/NeverRunStrategy.java | 2 +- .../util/{ => running}/RunOnceStrategy.java | 2 +- .../com/fagi/worker/InputWorkerTests.java | 246 ++++++++++-------- .../com/fagi/worker/OutputWorkerTest.java | 196 ++++++++------ .../java/com/fagi/db/LogBasedDatabase.java | 104 ++++---- .../main/java/com/fagi/encryption/AES.java | 22 +- .../java/com/fagi/encryption/KeyStorage.java | 10 +- .../main/java/com/fagi/encryption/RSA.java | 28 +- .../main/java/com/fagi/utility/Checksum.java | 12 +- .../com/fagi/utility/JsonFileOperations.java | 54 +++- .../main/java/com/fagi/utility/Logger.java | 33 --- .../java/com/fagi/utility/NetworkUtility.java | 14 +- .../db/LogBasedDatabaseConcurrencyTests.java | 17 +- .../db/LogBasedDatabaseCorruptionTests.java | 66 +++-- .../db/LogBasedDatabaseIntegrationTests.java | 56 ++-- .../db/LogBasedDatabasePerformanceTests.java | 12 +- .../db/LogBasedDatabaseResourceTests.java | 3 +- .../com/fagi/db/LogBasedDatabaseTests.java | 3 +- 65 files changed, 1179 insertions(+), 605 deletions(-) rename fagiServer/src/test/java/com/fagi/util/{ => running}/NeverRunStrategy.java (90%) rename fagiServer/src/test/java/com/fagi/util/{ => running}/RunOnceStrategy.java (94%) delete mode 100644 shared/src/main/java/com/fagi/utility/Logger.java diff --git a/.gitignore b/.gitignore index 8dfbde68..0fc6d408 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,8 @@ users *.key *.config *.fagi +*.log +*.log.lck .gradle/* **/build/* buildSrc/.gradle \ No newline at end of file diff --git a/fagiClient/build.gradle.kts b/fagiClient/build.gradle.kts index fc5a93e9..2d3851fb 100644 --- a/fagiClient/build.gradle.kts +++ b/fagiClient/build.gradle.kts @@ -18,6 +18,8 @@ javafx { dependencies { implementation(project(":shared")) + testImplementation(testFixtures(project(":shared"))) + testImplementation(libs.bundles.junit.base) testImplementation(libs.bundles.mockito) testImplementation(libs.hamcrest) diff --git a/fagiClient/src/main/java/com/fagi/action/items/LoadFXML.java b/fagiClient/src/main/java/com/fagi/action/items/LoadFXML.java index f8b94c0c..bde3ecb7 100644 --- a/fagiClient/src/main/java/com/fagi/action/items/LoadFXML.java +++ b/fagiClient/src/main/java/com/fagi/action/items/LoadFXML.java @@ -5,7 +5,8 @@ package com.fagi.action.items; import com.fagi.action.Action; -import com.fagi.utility.Logger; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import javafx.fxml.FXMLLoader; import javafx.scene.Parent; @@ -18,6 +19,7 @@ * @author miniwolf */ public record LoadFXML(String resourcePath) implements Action { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(LoadFXML.class); @Override public void execute(Parent parent) { @@ -27,8 +29,10 @@ public void execute(Parent parent) { try { loader.load(); } catch (IOException ioe) { - ioe.printStackTrace(); - Logger.logStackTrace(ioe); + LOGGER.error( + ioe, + () -> "Failed to load the FXML file: " + resourcePath + ); } } } diff --git a/fagiClient/src/main/java/com/fagi/action/items/OpenConversationFromID.java b/fagiClient/src/main/java/com/fagi/action/items/OpenConversationFromID.java index 4bbfcfe3..a4236bb9 100644 --- a/fagiClient/src/main/java/com/fagi/action/items/OpenConversationFromID.java +++ b/fagiClient/src/main/java/com/fagi/action/items/OpenConversationFromID.java @@ -6,6 +6,8 @@ import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; import com.fagi.conversation.GetAllConversationDataRequest; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.network.Communication; import java.util.Optional; @@ -14,6 +16,7 @@ * @author miniwolf */ public record OpenConversationFromID(MainScreen mainScreen) implements Action { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(OpenConversationFromID.class); @Override public void execute(Long id) { Optional optional = mainScreen @@ -58,7 +61,7 @@ private void addConversationToMain( } private void errorHandling(long id) { - System.err.println("OpenConversationFromID: Couldn't find conversation on ID <" + id + ">"); + LOGGER.error(() -> "OpenConversationFromID: Couldn't find conversation on ID <" + id + ">"); throw new RuntimeException(); } } diff --git a/fagiClient/src/main/java/com/fagi/controller/MainScreen.java b/fagiClient/src/main/java/com/fagi/controller/MainScreen.java index c9de5f4a..d8962ed8 100644 --- a/fagiClient/src/main/java/com/fagi/controller/MainScreen.java +++ b/fagiClient/src/main/java/com/fagi/controller/MainScreen.java @@ -16,6 +16,8 @@ import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationFilter; import com.fagi.handler.Search; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.GetFriendListRequest; import com.fagi.model.Logout; import com.fagi.model.conversation.GetConversationsRequest; @@ -30,7 +32,6 @@ import com.fagi.threads.ThreadPool; import com.fagi.uimodel.FriendMapWrapper; import com.fagi.utility.JsonFileOperations; -import com.fagi.utility.Logger; import javafx.application.Platform; import javafx.beans.value.ChangeListener; import javafx.beans.value.ObservableValue; @@ -60,6 +61,7 @@ * TODO: Write description. */ public class MainScreen extends Pane { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(MainScreen.class); @FXML private Pane messages; @FXML private Pane contacts; @FXML private ScrollPane listContent; @@ -139,11 +141,24 @@ public void initCommunication(ThreadPool threadPool) { setupFriendList(); setupContactList(); - messageHandler = new TextMessageHandler(this, communication.getInputDistributor()); - threadPool.startThread(messageHandler.getRunnable(), "MessageHandler"); + messageHandler = new TextMessageHandler( + this, + communication.getInputDistributor() + ); + threadPool.startThread( + messageHandler.getRunnable(), + "MessageHandler" + ); - generalHandler = new GeneralHandlerFactory().construct(this, communication.getInputDistributor(), threadPool); - threadPool.startThread(generalHandler.getRunnable(), "GeneralHandler"); + generalHandler = new GeneralHandlerFactory().construct( + this, + communication.getInputDistributor(), + threadPool + ); + threadPool.startThread( + generalHandler.getRunnable(), + "GeneralHandler" + ); updateConversationListFromServer(conversations); } @@ -161,15 +176,31 @@ private void initialize() { emptyFocusElement = messages; username.setText(usernameString); char cUpper = Character.toUpperCase(usernameString.toCharArray()[0]); - Image tiny = new Image("/style/material-icons/" + cUpper + ".png", 40, 40, true, true); + Image tiny = new Image( + "/style/material-icons/" + cUpper + ".png", + 40, + 40, + true, + true + ); this.tinyIcon.setImage(tiny); - Image large = new Image("/style/material-icons/" + cUpper + ".png", 96, 96, true, true); + Image large = new Image( + "/style/material-icons/" + cUpper + ".png", + 96, + 96, + true, + true + ); this.largeIcon.setImage(large); this.requestFocus(); Scene scene = primaryStage.getScene(); final MainScreen mainScreen = this; - Platform.runLater(() -> search = new Search(searchBox, searchHeader, mainScreen)); + Platform.runLater(() -> search = new Search( + searchBox, + searchHeader, + mainScreen + )); scene .widthProperty() .addListener(new ChangeListener<>() { @@ -183,14 +214,20 @@ public void changed( try { Thread.sleep(1000); } catch (InterruptedException e) { - e.printStackTrace(); - Logger.logStackTrace(e); + LOGGER.info(() -> "Search thread was interrupted."); } finally { - Platform.runLater(() -> search = new Search(searchBox, searchHeader, mainScreen)); + Platform.runLater(() -> search = new Search( + searchBox, + searchHeader, + mainScreen + )); } }; - threadPool.startThread(run, "Search thread"); + threadPool.startThread( + run, + "Search thread" + ); scene .widthProperty() @@ -281,7 +318,10 @@ public void setScrollPaneContent( Platform.runLater(() -> listContent.setContent(parent)); } - listContentMap.put(content, parent); + listContentMap.put( + content, + parent + ); } public void setFriendList(FriendList friendList) { @@ -318,7 +358,7 @@ public void changeMenuStyle(String menu) { currentPane = messages; } default -> { - System.err.println("Mainscreen, changeMenuStyle: " + menu); + LOGGER.error(() -> "Failed to change MainScreen menu into unsupported menu type: " + menu); throw new UnsupportedOperationException(); } } @@ -364,10 +404,16 @@ public Parent getListContent(PaneContent content) { private void updateConversationListFromServer(List conversations) { List filters = conversations .stream() - .map(x -> new ConversationFilter(x.getId(), x.getLastMessageDate())) + .map(x -> new ConversationFilter( + x.getId(), + x.getLastMessageDate() + )) .collect(Collectors.toList()); - communication.sendObject(new GetConversationsRequest(usernameString, filters)); + communication.sendObject(new GetConversationsRequest( + usernameString, + filters + )); } private void setupFriendList() { @@ -376,12 +422,18 @@ private void setupFriendList() { private void setupContactList() { ContentController contactContentController = new ContentController("/view/content/ContentList.fxml"); - setScrollPaneContent(PaneContent.Contacts, contactContentController); + setScrollPaneContent( + PaneContent.Contacts, + contactContentController + ); } private synchronized void setupConversationList() { conversationContentController = new ContentController("/view/content/ContentList.fxml"); - setScrollPaneContent(PaneContent.Messages, conversationContentController); + setScrollPaneContent( + PaneContent.Messages, + conversationContentController + ); messageItems.forEach(MessageItemController::stopTimer); messageItems.clear(); @@ -391,10 +443,11 @@ private synchronized void setupConversationList() { } public Pane createMessageItem(Conversation conversation) { - MessageItemController messageItemController = new MessageItemController(usernameString, - conversation, - new OpenConversationFromID(this), - conversation.getLastMessageDate() + MessageItemController messageItemController = new MessageItemController( + usernameString, + conversation, + new OpenConversationFromID(this), + conversation.getLastMessageDate() ); messageItems.add(messageItemController); return messageItemController; diff --git a/fagiClient/src/main/java/com/fagi/controller/login/MasterLogin.java b/fagiClient/src/main/java/com/fagi/controller/login/MasterLogin.java index e4ff756b..0aed6de4 100644 --- a/fagiClient/src/main/java/com/fagi/controller/login/MasterLogin.java +++ b/fagiClient/src/main/java/com/fagi/controller/login/MasterLogin.java @@ -6,6 +6,8 @@ import com.fagi.controller.utility.Draggable; import com.fagi.enums.LoginState; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.main.FagiApp; import com.fagi.network.ChatManager; import com.fagi.network.Communication; @@ -21,6 +23,7 @@ * class. */ public class MasterLogin { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(MasterLogin.class); private final FagiApp fagiApp; private final Communication communication; private final Draggable draggable; @@ -63,7 +66,10 @@ public void handleQuit() { ChatManager.closeCommunication(); fagiApp.stop(); } catch (Exception ex) { - System.err.println(ex.toString()); + LOGGER.error( + ex, + () -> "Client encountered unexpected exception whilst attempting to shutdown." + ); } } @@ -90,7 +96,7 @@ public void next() { case PASSWORD -> state = LoginState.INVITE_CODE; case INVITE_CODE -> state = LoginState.LOGIN; default -> { - System.out.println(state + " is not known"); + LOGGER.error(() -> state + " is not known"); throw new UnsupportedOperationException(); } } @@ -115,7 +121,7 @@ public void back() { state = LoginState.PASSWORD; break; default: - System.out.println(state + " is not known"); + LOGGER.error(() -> state + " is not known"); throw new UnsupportedOperationException(); } showScreen(state); @@ -140,7 +146,10 @@ private boolean setupController(LoginState screen) { LoginController controller; switch (screen) { case LOGIN: - controller = new LoginScreenController(this, communication); + controller = new LoginScreenController( + this, + communication + ); break; case USERNAME: controller = new CreateUserNameController(this); diff --git a/fagiClient/src/main/java/com/fagi/main/FagiApp.java b/fagiClient/src/main/java/com/fagi/main/FagiApp.java index c2ae7481..ea9489ee 100644 --- a/fagiClient/src/main/java/com/fagi/main/FagiApp.java +++ b/fagiClient/src/main/java/com/fagi/main/FagiApp.java @@ -8,10 +8,11 @@ import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; import com.fagi.encryption.AES; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.network.ChatManager; import com.fagi.network.Communication; import com.fagi.threads.ThreadPool; -import com.fagi.utility.Logger; import javafx.application.Application; import javafx.application.Platform; import javafx.scene.Scene; @@ -20,12 +21,14 @@ import javafx.stage.StageStyle; import java.io.IOException; +import java.nio.file.Path; import java.util.concurrent.atomic.AtomicBoolean; /** * JavaFX application class for handling GUI. */ public class FagiApp extends Application { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(FagiApp.class); private Stage primaryStage; private Scene scene; private ThreadPool threadPool; @@ -36,8 +39,12 @@ public class FagiApp extends Application { * @param args the command line arguments */ public static void main(String[] args) { + if (!FagiLoggerFactory.isCustomConfigurationAvailable()) { + FagiLoggerFactory.setupDefaultConfiguration(Path.of("client.log")); + LOGGER.info(() -> "No log config file specified. Using default log config instead."); + } if (args.length != 0) { - System.out.println("Usage: java LoginScreen"); + LOGGER.info(() -> "Usage: java LoginScreen"); } launch(args); } @@ -80,8 +87,10 @@ private void startCommunication( successfulConnection.set(true); } catch (IOException e) { Platform.runLater(() -> masterLogin.setMessageLabel("Connection refused")); - e.printStackTrace(); - Logger.logStackTrace(e); + LOGGER.error( + e, + () -> "Failed to connect to the server." + ); } }); diff --git a/fagiClient/src/main/java/com/fagi/network/ChatManager.java b/fagiClient/src/main/java/com/fagi/network/ChatManager.java index c8401d37..473fbea0 100644 --- a/fagiClient/src/main/java/com/fagi/network/ChatManager.java +++ b/fagiClient/src/main/java/com/fagi/network/ChatManager.java @@ -4,6 +4,8 @@ * ChatManager.java */ +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.main.FagiApp; import com.fagi.model.CreateUser; import com.fagi.model.InviteCode; @@ -24,6 +26,7 @@ * Handles login requests and responds to and from server. */ public class ChatManager { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(ChatManager.class); private static Communication communication = null; private static FagiApp application; private ServiceLoader communicationLoader; @@ -45,7 +48,7 @@ public static void handleLogout(Logout logout) { communication.sendObject(logout); Response response = communication.getNextResponse(); if (!(response instanceof AllIsWell)) { - System.err.println("Could not log out properly. " + "Shut down and let server handle the response"); + LOGGER.error(() -> "Could not log out properly. Shut down and let server handle the response."); } communication.close(); application.showLoginScreen(); @@ -79,12 +82,16 @@ public static boolean handleCreateUser( } if (!isValidUserName(username)) { - System.out.println(username); + LOGGER.info(() -> "The username " + username + " is not valid."); labelMessage.setText("Username may not contain special symbols"); return false; } - CreateUser createUser = new CreateUser(username, password, new InviteCode(inviteCode)); + CreateUser createUser = new CreateUser( + username, + password, + new InviteCode(inviteCode) + ); communication.sendObject(createUser); Response response = communication.getNextResponse(); @@ -128,7 +135,10 @@ public static void setApplication(FagiApp application) { } public static boolean isValidUserName(String string) { - return Pattern.matches("\\w*", string); + return Pattern.matches( + "\\w*", + string + ); } public static FagiApp getApplication() { diff --git a/fagiClient/src/main/java/com/fagi/network/Communication.java b/fagiClient/src/main/java/com/fagi/network/Communication.java index b39156e2..28b4dbf0 100644 --- a/fagiClient/src/main/java/com/fagi/network/Communication.java +++ b/fagiClient/src/main/java/com/fagi/network/Communication.java @@ -13,12 +13,13 @@ import com.fagi.encryption.EncryptionAlgorithm; import com.fagi.encryption.RSA; import com.fagi.encryption.RSAKey; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.HistoryUpdates; import com.fagi.model.Session; import com.fagi.responses.AllIsWell; import com.fagi.responses.Response; import com.fagi.threads.ThreadPool; -import com.fagi.utility.Logger; import java.io.IOException; import java.io.ObjectInputStream; @@ -32,6 +33,7 @@ * TODO: Add description */ public class Communication { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(Communication.class); private ObjectOutputStream out; private InputHandler inputHandler; private Socket socket; @@ -76,12 +78,11 @@ public void connect( encryption, serverKey ); - } catch (UnknownHostException Uhe) { - System.err.println("c Uhe: " + Uhe); - Logger.logStackTrace(Uhe); - } catch (IOException ioe) { - Logger.logStackTrace(ioe); - throw new IOException("c ioe: " + ioe.toString()); + } catch (UnknownHostException uhe) { + LOGGER.error( + uhe, + () -> "Failed to connect to the following host: " + host + ); } } @@ -123,9 +124,10 @@ public void sendObject(Object obj) { out.writeObject(encryption.encrypt(Conversion.convertToBytes(obj))); out.flush(); } catch (IOException e) { - System.err.println("cso ioe: " + e.toString()); - e.printStackTrace(); - Logger.logStackTrace(e); + LOGGER.error( + e, + () -> "Lost connection to server. Crashing now." + ); System.exit(1); } } @@ -136,7 +138,10 @@ public void close() { try { socket.close(); } catch (IOException ioe) { - System.err.println("cc ioe: " + ioe.toString()); + LOGGER.error( + ioe, + () -> "Failed to close Communication gracefully." + ); } } diff --git a/fagiClient/src/main/java/com/fagi/network/InputHandler.java b/fagiClient/src/main/java/com/fagi/network/InputHandler.java index 234602d6..b30d4bd1 100644 --- a/fagiClient/src/main/java/com/fagi/network/InputHandler.java +++ b/fagiClient/src/main/java/com/fagi/network/InputHandler.java @@ -8,11 +8,12 @@ import com.fagi.conversation.Conversation; import com.fagi.encryption.Conversion; import com.fagi.encryption.EncryptionAlgorithm; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.HistoryUpdates; import com.fagi.model.messages.InGoingMessages; import com.fagi.responses.Response; import com.fagi.threads.ThreadPool; -import com.fagi.utility.Logger; import javafx.application.Platform; import java.io.IOException; @@ -25,6 +26,7 @@ * TODO: Write description */ public class InputHandler implements Runnable { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(InputHandler.class); private final Queue inputs = new LinkedBlockingQueue<>(); private final ObjectInputStream in; private final EncryptionAlgorithm encryption; @@ -53,9 +55,10 @@ public void run() { handleInput(Conversion.convertFromBytes(encryption.decrypt(input))); } catch (IOException ioe) { if (running) { - System.err.println("inputhandler ioe: " + ioe.toString()); - ioe.printStackTrace(); // DEBUG need to terminate before closing socket. - Logger.logStackTrace(ioe); + LOGGER.error( + ioe, + () -> "Failed to receive data from server. Logging user out." + ); } running = false; ChatManager.closeCommunication(); @@ -63,9 +66,11 @@ public void run() { .getApplication() .showLoginScreen()); } catch (ClassNotFoundException cnfe) { + LOGGER.error( + cnfe, + () -> "Failed to parse response data from server." + ); // Shared files are not the same on both side of the server - System.err.println(cnfe.getMessage()); - Logger.logStackTrace(cnfe); // TODO: This will be a bitch when having to update the server // Fix: could be to implement JSON } diff --git a/fagiClient/src/main/java/com/fagi/network/handlers/DefaultThreadHandler.java b/fagiClient/src/main/java/com/fagi/network/handlers/DefaultThreadHandler.java index 81539fff..06984e86 100644 --- a/fagiClient/src/main/java/com/fagi/network/handlers/DefaultThreadHandler.java +++ b/fagiClient/src/main/java/com/fagi/network/handlers/DefaultThreadHandler.java @@ -7,11 +7,13 @@ import com.fagi.network.handlers.container.Container; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; /** * @author miniwolf */ public class DefaultThreadHandler implements Runnable { + private static final Logger LOGGER = Logger.getLogger(DefaultThreadHandler.class.getName()); private Container container; private Handler handler; private AtomicBoolean running = new AtomicBoolean(true); @@ -39,7 +41,7 @@ public void run() { } } catch (InterruptedException e) { running.set(false); - System.out.println("Stopped the thread handler"); + LOGGER.info(() -> "Stopped the thread handler"); } } } diff --git a/fagiClient/src/main/java/com/fagi/network/handlers/GeneralHandler.java b/fagiClient/src/main/java/com/fagi/network/handlers/GeneralHandler.java index d0113198..129ea653 100644 --- a/fagiClient/src/main/java/com/fagi/network/handlers/GeneralHandler.java +++ b/fagiClient/src/main/java/com/fagi/network/handlers/GeneralHandler.java @@ -9,17 +9,22 @@ import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Logger; /** * Created by Marcus on 08-07-2016. */ public class GeneralHandler implements Handler { + private static final Logger LOGGER = Logger.getLogger(GeneralHandler.class.getName()); private final Map> handlers = new ConcurrentHashMap<>(); private final Container container = new DefaultContainer<>(); private final InputDistributor inputDistributor; private final List unhandledObjects = new ArrayList<>(); private final ThreadPool threadPool; - private DefaultThreadHandler runnable = new DefaultThreadHandler<>(container, this); + private DefaultThreadHandler runnable = new DefaultThreadHandler<>( + container, + this + ); public GeneralHandler( InputDistributor inputDistributor, @@ -34,19 +39,28 @@ public void handle(T object) { Handler handler = handlers.get(object.getClass()); if (handler == null) { - System.err.println("Missing handler: " + object.getClass()); + LOGGER.severe(() -> "Missing handler: " + object.getClass()); unhandledObjects.add(object); return; } - threadPool.startThread(() -> handler.handle(object), "GeneralHandler: " + object.getClass()); + threadPool.startThread( + () -> handler.handle(object), + "GeneralHandler: " + object.getClass() + ); } public void registerHandler( Class clazz, Handler handler) { - handlers.put(clazz, handler); - inputDistributor.register(clazz, container); + handlers.put( + clazz, + handler + ); + inputDistributor.register( + clazz, + container + ); } @Override diff --git a/fagiClient/src/main/java/com/fagi/network/handlers/TextMessageHandler.java b/fagiClient/src/main/java/com/fagi/network/handlers/TextMessageHandler.java index d8a573dc..4bd48830 100644 --- a/fagiClient/src/main/java/com/fagi/network/handlers/TextMessageHandler.java +++ b/fagiClient/src/main/java/com/fagi/network/handlers/TextMessageHandler.java @@ -18,20 +18,28 @@ import javafx.application.Platform; import java.util.Optional; +import java.util.logging.Logger; /** * @author miniwolf */ public class TextMessageHandler implements Handler { + private static final Logger LOGGER = Logger.getLogger(TextMessageHandler.class.getName()); private Container container = new DefaultContainer<>(); - private DefaultThreadHandler runnable = new DefaultThreadHandler<>(container, this); + private DefaultThreadHandler runnable = new DefaultThreadHandler<>( + container, + this + ); private final MainScreen mainScreen; public TextMessageHandler( MainScreen mainScreen, InputDistributor inputDistributor) { container.setThread(runnable); - inputDistributor.register(TextMessage.class, container); + inputDistributor.register( + TextMessage.class, + container + ); this.mainScreen = mainScreen; } @@ -45,7 +53,7 @@ public void handle(TextMessage message) { .getConversationID()) .findFirst(); if (first.isEmpty()) { - System.err.println("Server sent a message before it sent the conversation of ID '" + message + LOGGER.severe(() -> "Server sent a message before it sent the conversation of ID '" + message .getMessageInfo() .getConversationID() + "'to the profile."); return; @@ -60,7 +68,10 @@ public void handle(TextMessage message) { conversation.setType(type); mainScreen .getCommunication() - .sendObject(new GetAllConversationDataRequest(mainScreen.getUsername(), conversation.getId())); + .sendObject(new GetAllConversationDataRequest( + mainScreen.getUsername(), + conversation.getId() + )); } if (mainScreen.hasCurrentOpenConversation(conversation)) { @@ -71,7 +82,10 @@ public void handle(TextMessage message) { conversation .data() .addMessage(message); - JsonFileOperations.storeClientConversation(conversation, mainScreen.getUsername()); + JsonFileOperations.storeClientConversation( + conversation, + mainScreen.getUsername() + ); MessageItemController messageItemController = mainScreen .getMessageItems() diff --git a/fagiClient/src/main/java/com/fagi/threads/ThreadPool.java b/fagiClient/src/main/java/com/fagi/threads/ThreadPool.java index 75927256..f504a139 100644 --- a/fagiClient/src/main/java/com/fagi/threads/ThreadPool.java +++ b/fagiClient/src/main/java/com/fagi/threads/ThreadPool.java @@ -42,8 +42,4 @@ public void stopThreads() { threads.forEach(Thread::interrupt); threads.clear(); } - - public void printThreads() { - threads.forEach(thread -> System.out.println("Thread: " + thread.getName() + " alive: " + thread.isAlive())); - } } diff --git a/fagiClient/src/test/java/com/fagi/action/items/LoadFXMLTest.java b/fagiClient/src/test/java/com/fagi/action/items/LoadFXMLTest.java index b483160c..6fb82922 100644 --- a/fagiClient/src/test/java/com/fagi/action/items/LoadFXMLTest.java +++ b/fagiClient/src/test/java/com/fagi/action/items/LoadFXMLTest.java @@ -1,5 +1,6 @@ package com.fagi.action.items; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.conversation.ConversationController; import com.fagi.conversation.Conversation; @@ -14,7 +15,7 @@ * Created by miniwolf on 01-04-2017. */ @ExtendWith(JavaFXThreadingExtension.class) -public class LoadFXMLTest { +public class LoadFXMLTest extends BaseFagiTest { private LoadFXML loadFXML; private ConversationController mock; diff --git a/fagiClient/src/test/java/com/fagi/guitests/ConversationTests.java b/fagiClient/src/test/java/com/fagi/guitests/ConversationTests.java index 27003622..19093e57 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/ConversationTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/ConversationTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -44,7 +45,7 @@ import java.util.List; @ExtendWith(ApplicationExtension.class) -public class ConversationTests { +public class ConversationTests extends BaseFagiTest { private InputHandler inputHandler; private Communication communication; diff --git a/fagiClient/src/test/java/com/fagi/guitests/CreatePasswordTests.java b/fagiClient/src/test/java/com/fagi/guitests/CreatePasswordTests.java index 220c1d1f..51a0239a 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/CreatePasswordTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/CreatePasswordTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; import com.fagi.enums.LoginState; @@ -25,7 +26,7 @@ import org.testfx.matcher.base.NodeMatchers; @ExtendWith(ApplicationExtension.class) -public class CreatePasswordTests { +public class CreatePasswordTests extends BaseFagiTest { private MasterLogin masterLogin; @BeforeAll diff --git a/fagiClient/src/test/java/com/fagi/guitests/CreateUserNameTests.java b/fagiClient/src/test/java/com/fagi/guitests/CreateUserNameTests.java index 604364d8..fc971c1e 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/CreateUserNameTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/CreateUserNameTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; import com.fagi.enums.LoginState; @@ -28,7 +29,7 @@ import org.testfx.matcher.base.NodeMatchers; @ExtendWith(ApplicationExtension.class) -public class CreateUserNameTests { +public class CreateUserNameTests extends BaseFagiTest { private Communication communication; private MasterLogin masterLogin; diff --git a/fagiClient/src/test/java/com/fagi/guitests/FriendsTests.java b/fagiClient/src/test/java/com/fagi/guitests/FriendsTests.java index c1b7615c..aca3a1a8 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/FriendsTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/FriendsTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -44,7 +45,7 @@ import static org.hamcrest.collection.IsIterableContainingInOrder.contains; @ExtendWith(ApplicationExtension.class) -public class FriendsTests { +public class FriendsTests extends BaseFagiTest { private InputHandler inputHandler; private final ThreadPool threadPool = new ThreadPool(); diff --git a/fagiClient/src/test/java/com/fagi/guitests/InviteCodeTests.java b/fagiClient/src/test/java/com/fagi/guitests/InviteCodeTests.java index 1057dbbf..b550d407 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/InviteCodeTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/InviteCodeTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; import com.fagi.enums.LoginState; @@ -26,7 +27,7 @@ import org.testfx.framework.junit5.Start; @ExtendWith(ApplicationExtension.class) -public class InviteCodeTests { +public class InviteCodeTests extends BaseFagiTest { private Communication communication; private MasterLogin masterLogin; diff --git a/fagiClient/src/test/java/com/fagi/guitests/LoginTests.java b/fagiClient/src/test/java/com/fagi/guitests/LoginTests.java index 2dc54ce6..5bae185a 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/LoginTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/LoginTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; import com.fagi.enums.LoginState; @@ -30,7 +31,7 @@ import org.testfx.util.WaitForAsyncUtils; @ExtendWith(ApplicationExtension.class) -public class LoginTests { +public class LoginTests extends BaseFagiTest { private MasterLogin masterLogin; private Communication communication; diff --git a/fagiClient/src/test/java/com/fagi/guitests/ReceiveFriendRequestTests.java b/fagiClient/src/test/java/com/fagi/guitests/ReceiveFriendRequestTests.java index 5322acfb..aad32805 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/ReceiveFriendRequestTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/ReceiveFriendRequestTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -34,7 +35,7 @@ import static com.fagi.helpers.WaitForFXEventsTestHelper.addIngoingMessageToInputHandler; @ExtendWith(ApplicationExtension.class) -public class ReceiveFriendRequestTests { +public class ReceiveFriendRequestTests extends BaseFagiTest { private static final String myUsername = "Test"; private Communication communication; private InputHandler inputHandler; diff --git a/fagiClient/src/test/java/com/fagi/guitests/SearchUserTests.java b/fagiClient/src/test/java/com/fagi/guitests/SearchUserTests.java index 00352239..e72237b1 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/SearchUserTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/SearchUserTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -46,7 +47,7 @@ import static com.fagi.helpers.WaitForFXEventsTestHelper.addIngoingMessageToInputHandler; @ExtendWith(ApplicationExtension.class) -public class SearchUserTests { +public class SearchUserTests extends BaseFagiTest { private Communication communication; private InputHandler inputHandler; private final ThreadPool threadPool = new ThreadPool(); diff --git a/fagiClient/src/test/java/com/fagi/guitests/SendFriendRequestTests.java b/fagiClient/src/test/java/com/fagi/guitests/SendFriendRequestTests.java index 5184cb3a..7a77b46d 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/SendFriendRequestTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/SendFriendRequestTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -35,7 +36,7 @@ import static com.fagi.helpers.WaitForFXEventsTestHelper.addIngoingMessageToInputHandler; @ExtendWith(ApplicationExtension.class) -public class SendFriendRequestTests { +public class SendFriendRequestTests extends BaseFagiTest { private Communication communication; private InputHandler inputHandler; private final ThreadPool threadPool = new ThreadPool(); diff --git a/fagiClient/src/test/java/com/fagi/guitests/SignOutTests.java b/fagiClient/src/test/java/com/fagi/guitests/SignOutTests.java index 6b23d73e..09874402 100644 --- a/fagiClient/src/test/java/com/fagi/guitests/SignOutTests.java +++ b/fagiClient/src/test/java/com/fagi/guitests/SignOutTests.java @@ -1,5 +1,6 @@ package com.fagi.guitests; +import com.fagi.BaseFagiTest; import com.fagi.controller.MainScreen; import com.fagi.controller.login.MasterLogin; import com.fagi.controller.utility.Draggable; @@ -26,7 +27,7 @@ import org.testfx.matcher.base.NodeMatchers; @ExtendWith(ApplicationExtension.class) -public class SignOutTests { +public class SignOutTests extends BaseFagiTest { private final ThreadPool threadPool = new ThreadPool(); @BeforeAll diff --git a/fagiClient/src/test/java/com/fagi/network/CommunicationTest.java b/fagiClient/src/test/java/com/fagi/network/CommunicationTest.java index 9fe8838b..56bb45f5 100644 --- a/fagiClient/src/test/java/com/fagi/network/CommunicationTest.java +++ b/fagiClient/src/test/java/com/fagi/network/CommunicationTest.java @@ -1,5 +1,6 @@ package com.fagi.network; +import com.fagi.BaseFagiTest; import com.fagi.encryption.Conversion; import com.fagi.encryption.EncryptionAlgorithm; import org.junit.jupiter.api.BeforeEach; @@ -13,7 +14,7 @@ * * @author miniwolf */ -public class CommunicationTest { +public class CommunicationTest extends BaseFagiTest { private Communication communication; @BeforeEach diff --git a/fagiClient/src/test/java/com/fagi/util/FontUtilsTest.java b/fagiClient/src/test/java/com/fagi/util/FontUtilsTest.java index ead62759..b1a74e0e 100644 --- a/fagiClient/src/test/java/com/fagi/util/FontUtilsTest.java +++ b/fagiClient/src/test/java/com/fagi/util/FontUtilsTest.java @@ -1,5 +1,6 @@ package com.fagi.util; +import com.fagi.BaseFagiTest; import javafx.scene.text.Font; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -34,7 +35,7 @@ private boolean isWindows() { * @author miniwolf */ @ExtendWith({JavaFXThreadingExtension.class, DisableOnLinuxAndMacCondition.class}) -public class FontUtilsTest { +public class FontUtilsTest extends BaseFagiTest { private static final Font ROBOTO = new Font("Roboto-Regular", 13); @Test diff --git a/fagiServer/build.gradle.kts b/fagiServer/build.gradle.kts index 22655d3c..d5224630 100644 --- a/fagiServer/build.gradle.kts +++ b/fagiServer/build.gradle.kts @@ -12,6 +12,8 @@ tasks.test { dependencies { implementation(project(":shared")) + + testImplementation(testFixtures(project(":shared"))) testImplementation(libs.bundles.junit.base) testImplementation(libs.bundles.mockito) } diff --git a/fagiServer/src/main/java/com/fagi/encryption/Encryption.java b/fagiServer/src/main/java/com/fagi/encryption/Encryption.java index 91bea5e5..fabe5db7 100644 --- a/fagiServer/src/main/java/com/fagi/encryption/Encryption.java +++ b/fagiServer/src/main/java/com/fagi/encryption/Encryption.java @@ -1,5 +1,8 @@ package com.fagi.encryption; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; + import java.io.File; import java.io.IOException; import java.security.KeyPair; @@ -10,6 +13,7 @@ * Created by Marcus on 04-06-2016. */ public class Encryption { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(Encryption.class); private static Encryption instance; private RSA rsa; @@ -24,14 +28,20 @@ public class Encryption { try { KeyStorage.SaveKeyPair(key); } catch (IOException e) { - e.printStackTrace(); + LOGGER.error( + e, + () -> "Could not save RSA key pair." + ); } } else { try { KeyPair key = KeyStorage.LoadKeyPair("RSA"); this.rsa = new RSA(key); } catch (IOException | NoSuchAlgorithmException | InvalidKeySpecException e) { - e.printStackTrace(); + LOGGER.error( + e, + () -> "Could not load RSA key pair." + ); } } diff --git a/fagiServer/src/main/java/com/fagi/handler/ConversationHandler.java b/fagiServer/src/main/java/com/fagi/handler/ConversationHandler.java index a818a73f..4374d4bb 100644 --- a/fagiServer/src/main/java/com/fagi/handler/ConversationHandler.java +++ b/fagiServer/src/main/java/com/fagi/handler/ConversationHandler.java @@ -1,6 +1,8 @@ package com.fagi.handler; import com.fagi.conversation.Conversation; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.Data; import com.fagi.model.messages.message.TextMessage; @@ -10,6 +12,7 @@ * Created by Marcus on 04-07-2016. */ public class ConversationHandler implements Runnable { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(ConversationHandler.class); private final Data data; private final LinkedBlockingQueue queue = new LinkedBlockingQueue<>(); @@ -44,7 +47,10 @@ public void tick() { data.storeConversation(conversation); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); - ie.printStackTrace(); + LOGGER.debug( + ie, + () -> "Interrupted ConversationHandler thread." + ); } } diff --git a/fagiServer/src/main/java/com/fagi/handler/InputHandler.java b/fagiServer/src/main/java/com/fagi/handler/InputHandler.java index 3d09fa62..1b8abfe4 100644 --- a/fagiServer/src/main/java/com/fagi/handler/InputHandler.java +++ b/fagiServer/src/main/java/com/fagi/handler/InputHandler.java @@ -4,6 +4,8 @@ import com.fagi.conversation.ConversationDataUpdate; import com.fagi.conversation.GetAllConversationDataRequest; import com.fagi.encryption.AES; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.CreateUser; import com.fagi.model.Data; import com.fagi.model.DeleteFriend; @@ -50,9 +52,10 @@ import java.util.stream.Collectors; public record InputHandler(InputAgent inputAgent, OutputAgent out, ConversationHandler conversationHandler, Data data) { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(InputHandler.class); public void handleInput(Object input) { if (input == null) { - System.out.println("Input is null. Doing nothing."); + LOGGER.info(() -> "Input is null. Doing nothing."); } else if (input instanceof TextMessage arg) { MessageInfo messageInfo = arg.getMessageInfo(); messageInfo.setTimestamp(new Timestamp(System.currentTimeMillis())); @@ -102,7 +105,7 @@ public void handleInput(Object input) { } else if (input instanceof UserNameAvailableRequest request) { out.addResponse(handleUserNameAvailableRequest(request)); } else { - System.out.println("Unknown handle: " + input.getClass()); + LOGGER.info(() -> "Unknown handle: " + input.getClass()); } } diff --git a/fagiServer/src/main/java/com/fagi/main/Main.java b/fagiServer/src/main/java/com/fagi/main/Main.java index 5021a448..ca402ee5 100644 --- a/fagiServer/src/main/java/com/fagi/main/Main.java +++ b/fagiServer/src/main/java/com/fagi/main/Main.java @@ -4,30 +4,46 @@ * Main.java */ +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.Data; import com.fagi.server.Server; import java.io.IOException; import java.net.ServerSocket; +import java.nio.file.Path; /** * Handling server start. */ class Main { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(Main.class); + public static void main(String[] args) { Data data = new Data(); data.loadUsers(); int port = args.length > 0 ? Integer.parseInt(args[0]) : 4242; - Server server = new Server(port, data); + Server server = new Server( + port, + data + ); + + if (!FagiLoggerFactory.isCustomConfigurationAvailable()) { + FagiLoggerFactory.setupDefaultConfiguration(Path.of("server.log")); + LOGGER.info(() -> "No log config file specified. Using default log config instead."); + } try { var serverSocket = new ServerSocket(port); server.start(serverSocket); } catch (IOException e) { - System.out.println("Error while creating socket, are you sure you can use port " + port + " on you system?"); + LOGGER.error( + e, + () -> "Error while creating socket, are you sure you can use port " + port + " on you system?" + ); } } } \ No newline at end of file diff --git a/fagiServer/src/main/java/com/fagi/model/Data.java b/fagiServer/src/main/java/com/fagi/model/Data.java index 1b5ff75e..4737f195 100644 --- a/fagiServer/src/main/java/com/fagi/model/Data.java +++ b/fagiServer/src/main/java/com/fagi/model/Data.java @@ -6,6 +6,8 @@ import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.responses.AllIsWell; import com.fagi.responses.NoSuchUser; import com.fagi.responses.PasswordError; @@ -31,6 +33,7 @@ * Contains and update information on users. */ public class Data { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(Data.class); private final Map OUTPUT_AGENT_MAP = new ConcurrentHashMap<>(); private final Map INPUT_AGENT_MAP = new ConcurrentHashMap<>(); private final Map registeredUsers = new ConcurrentHashMap<>(); @@ -164,7 +167,7 @@ public void userLogout(String userName) { OUTPUT_AGENT_MAP.remove(userName); INPUT_AGENT_MAP.remove(userName); } else { - System.out.println("Couldn't log " + userName + " out"); + LOGGER.info(() -> "Couldn't log " + userName + " out"); } } diff --git a/fagiServer/src/main/java/com/fagi/server/Server.java b/fagiServer/src/main/java/com/fagi/server/Server.java index 9beda6d7..82a96a74 100644 --- a/fagiServer/src/main/java/com/fagi/server/Server.java +++ b/fagiServer/src/main/java/com/fagi/server/Server.java @@ -10,6 +10,8 @@ import com.fagi.encryption.Encryption; import com.fagi.encryption.RSAKey; import com.fagi.handler.ConversationHandler; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.Data; import com.fagi.model.InviteCodeContainer; import com.fagi.running.CheckFieldRunningStrategy; @@ -30,6 +32,7 @@ import java.util.List; public class Server { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(Server.class); static final String CONFIG_FILE = "config/serverinfo.config"; private final Data data; private IsRunningStrategy isRunningStrategy = new CheckFieldRunningStrategy(); @@ -64,12 +67,15 @@ public Server( data.storeInviteCodes(new InviteCodeContainer(new ArrayList<>())); } } catch (IOException e) { - e.printStackTrace(); + LOGGER.error( + e, + () -> "Could not create server." + ); } } public void start(ServerSocket serverSocket) { - System.out.println("Starting Server"); + LOGGER.info(() -> "Starting Server"); conversationHandlerThread = new Thread(handler); conversationHandlerThread.setDaemon(true); @@ -80,20 +86,26 @@ public void start(ServerSocket serverSocket) { try { workerCreation(serverSocket); } catch (IOException e) { - System.out.println("Error in server loop exception = " + e); + LOGGER.error( + e, + () -> "Error in server loop exception" + ); isRunningStrategy.stop(); } } conversationHandlerThread.interrupt(); - System.out.println("Stopping Server"); + LOGGER.info(() -> "Stopping Server"); if (serverSocket != null) { try { serverSocket.close(); } catch (IOException e) { - e.printStackTrace(); + LOGGER.error( + e, + () -> "Failed to close server socket gracefully." + ); } } } diff --git a/fagiServer/src/main/java/com/fagi/worker/InputWorker.java b/fagiServer/src/main/java/com/fagi/worker/InputWorker.java index e0eab39b..b45b2b4c 100644 --- a/fagiServer/src/main/java/com/fagi/worker/InputWorker.java +++ b/fagiServer/src/main/java/com/fagi/worker/InputWorker.java @@ -9,6 +9,8 @@ import com.fagi.encryption.EncryptionAlgorithm; import com.fagi.handler.ConversationHandler; import com.fagi.handler.InputHandler; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.Data; import java.io.EOFException; @@ -20,6 +22,7 @@ * @author miniwolf */ public class InputWorker extends Worker implements InputAgent { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(InputWorker.class); private final InputHandler inputHandler; private final Data data; private final ObjectInputStream objIn; @@ -35,9 +38,6 @@ public InputWorker( ConversationHandler handler, Data data) { this.data = data; - // TODO: This sysout does not make sense. Should be in the run method or where the thread is started. - // https://trello.com/c/SVazRIgj/58-inputworker-should-not-print-starting-an-input-thread-in-its-constructor - System.out.println("Starting an input thread"); this.objIn = objIn; this.out = out; this.inputHandler = new InputHandler( @@ -50,8 +50,9 @@ public InputWorker( @Override public void run() { + LOGGER.info(() -> "Starting an input thread"); while (isRunningStrategy.isRunning()) { - System.out.println("Running"); + LOGGER.info(() -> "Running"); try { Object input = objIn.readObject(); @@ -64,19 +65,21 @@ public void run() { inputHandler.handleInput(input); } catch (EOFException | SocketException eof) { stop(); - System.out.println("Logging out user " + myUserName); + LOGGER.info(() -> "Logging out user " + myUserName); out.stop(); data.userLogout(myUserName); } catch (Exception e) { stop(); out.stop(); - System.out.println("Something went wrong in a input worker while loop " + e); - e.printStackTrace(); - System.out.println("Logging out user " + myUserName); + LOGGER.error( + e, + () -> "Something went wrong in a input worker while loop." + ); + LOGGER.info(() -> "Logging out user " + myUserName); data.userLogout(myUserName); } } - System.out.println("Closing input"); + LOGGER.info(() -> "Closing input."); } private Object decryptAndConvertToObject(byte[] input) { @@ -89,7 +92,10 @@ private Object decryptAndConvertToObject(byte[] input) { try { return Conversion.convertFromBytes(input); } catch (IOException | ClassNotFoundException e) { - e.printStackTrace(); + LOGGER.error( + e, + () -> "Failed to decrypt or deserialize object." + ); } return null; } diff --git a/fagiServer/src/main/java/com/fagi/worker/OutputWorker.java b/fagiServer/src/main/java/com/fagi/worker/OutputWorker.java index 5203f6e4..c30bbbcf 100644 --- a/fagiServer/src/main/java/com/fagi/worker/OutputWorker.java +++ b/fagiServer/src/main/java/com/fagi/worker/OutputWorker.java @@ -6,6 +6,8 @@ import com.fagi.encryption.AESKey; import com.fagi.encryption.Conversion; import com.fagi.encryption.EncryptionAlgorithm; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.fagi.model.Data; import com.fagi.model.FriendRequest; import com.fagi.model.User; @@ -26,6 +28,7 @@ * @author miniwolf */ public class OutputWorker extends Worker implements OutputAgent { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(OutputWorker.class); private final Queue messages = new ConcurrentLinkedQueue<>(); private final Queue respondObjects = new ConcurrentLinkedQueue<>(); private final Data data; @@ -45,7 +48,7 @@ public OutputWorker( @Override public void run() { while (isRunningStrategy.isRunning()) { - System.out.println("Running"); + LOGGER.info(() -> "Running"); try { sendIncMessages(); sendResponses(); @@ -58,8 +61,10 @@ public void run() { } } catch (IOException | InterruptedException ioe) { stop(); - System.out.println(ioe.toString()); - System.out.println("Logging out user " + myUserName); + LOGGER.error( + ioe, + () -> "Logging out user " + myUserName + ); data.userLogout(myUserName); } } @@ -69,7 +74,7 @@ public void run() { } catch (IOException ignored) { // User logged off we didn't manage to send response } } - System.out.println("Closing output"); + LOGGER.info(() -> "Closing output"); } private void checkForLists() throws IOException { @@ -103,7 +108,7 @@ private void sendIncMessages() throws IOException { } private void send(Object object) throws IOException { - System.out.println("so: " + object.toString()); + LOGGER.debug(() -> "Sending the following object to user " + myUserName + ": " + object.toString()); objOut.writeObject(aes.encrypt(Conversion.convertToBytes(object))); objOut.flush(); } diff --git a/fagiServer/src/test/java/TextMessageIntegrationTests.java b/fagiServer/src/test/java/TextMessageIntegrationTests.java index 9ace37d9..cd3b4665 100644 --- a/fagiServer/src/test/java/TextMessageIntegrationTests.java +++ b/fagiServer/src/test/java/TextMessageIntegrationTests.java @@ -1,3 +1,4 @@ +import com.fagi.BaseFagiTest; import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; import com.fagi.handler.ConversationHandler; @@ -17,7 +18,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; -public class TextMessageIntegrationTests { +public class TextMessageIntegrationTests extends BaseFagiTest { private OutputAgent outputAgent; private ConversationHandler conversationHandler; private Data data; diff --git a/fagiServer/src/test/java/com/fagi/encryption/EncryptionTests.java b/fagiServer/src/test/java/com/fagi/encryption/EncryptionTests.java index e8e2b97a..3228cd10 100644 --- a/fagiServer/src/test/java/com/fagi/encryption/EncryptionTests.java +++ b/fagiServer/src/test/java/com/fagi/encryption/EncryptionTests.java @@ -1,16 +1,19 @@ package com.fagi.encryption; +import com.fagi.BaseFagiTest; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; -import java.io.PrintStream; import java.security.KeyPair; import java.security.NoSuchAlgorithmException; import java.security.spec.InvalidKeySpecException; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; @@ -18,12 +21,11 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -class EncryptionTests { +class EncryptionTests extends BaseFagiTest { private static final String PUBLIC_KEY_PATH = "build/test/data/encryption_test/public.key"; @AfterEach void tearDown() { - System.setErr(System.err); deleteFileAndFolder(PUBLIC_KEY_PATH); deleteFileAndFolder(KeyStorage.PUBLICKEYFILE); deleteFileAndFolder(KeyStorage.PRIVATEKEYFILE); @@ -39,13 +41,33 @@ public void whenLoadKeyPairThrowsIOException_ThenConstructorPrintsStacktrace() t createPublicKeyFile(); - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - new Encryption(PUBLIC_KEY_PATH); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.io.IOException")); + List> logRecords = lookupLogRecordsForClass(Encryption.class); + Assertions.assertAll( + () -> Assertions.assertEquals( + 1, + logRecords.size() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecords + .getFirst() + .logLevel() + ), + () -> Assertions.assertEquals( + "Could not load RSA key pair.", + logRecords + .getFirst() + .message() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecords + .getFirst() + .throwable() + ) + ); } } @@ -58,13 +80,33 @@ public void whenLoadKeyPairThrowsNoSuchAlgorithmException_ThenConstructorPrintsS createPublicKeyFile(); - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - new Encryption(PUBLIC_KEY_PATH); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.security.NoSuchAlgorithmException")); + List> logRecords = lookupLogRecordsForClass(Encryption.class); + Assertions.assertAll( + () -> Assertions.assertEquals( + 1, + logRecords.size() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecords + .getFirst() + .logLevel() + ), + () -> Assertions.assertEquals( + "Could not load RSA key pair.", + logRecords + .getFirst() + .message() + ), + () -> Assertions.assertInstanceOf( + NoSuchAlgorithmException.class, + logRecords + .getFirst() + .throwable() + ) + ); } } @@ -77,13 +119,33 @@ public void whenLoadKeyPairThrowsInvalidKeySpecException_ThenConstructorPrintsSt createPublicKeyFile(); - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - new Encryption(PUBLIC_KEY_PATH); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.security.spec.InvalidKeySpecException")); + List> logRecords = lookupLogRecordsForClass(Encryption.class); + Assertions.assertAll( + () -> Assertions.assertEquals( + 1, + logRecords.size() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecords + .getFirst() + .logLevel() + ), + () -> Assertions.assertEquals( + "Could not load RSA key pair.", + logRecords + .getFirst() + .message() + ), + () -> Assertions.assertInstanceOf( + InvalidKeySpecException.class, + logRecords + .getFirst() + .throwable() + ) + ); } } @@ -94,13 +156,33 @@ void whenSaveKeyPairThrowsIOException_ThenConstructorPrintsStacktrace() { .when(() -> KeyStorage.SaveKeyPair(any())) .thenThrow(new IOException()); - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - new Encryption(PUBLIC_KEY_PATH); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.io.IOException")); + List> logRecords = lookupLogRecordsForClass(Encryption.class); + Assertions.assertAll( + () -> Assertions.assertEquals( + 1, + logRecords.size() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecords + .getFirst() + .logLevel() + ), + () -> Assertions.assertEquals( + "Could not save RSA key pair.", + logRecords + .getFirst() + .message() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecords + .getFirst() + .throwable() + ) + ); } } diff --git a/fagiServer/src/test/java/com/fagi/handler/ConversationHandlerTest.java b/fagiServer/src/test/java/com/fagi/handler/ConversationHandlerTest.java index b17150f7..6e1bd77b 100644 --- a/fagiServer/src/test/java/com/fagi/handler/ConversationHandlerTest.java +++ b/fagiServer/src/test/java/com/fagi/handler/ConversationHandlerTest.java @@ -1,7 +1,10 @@ package com.fagi.handler; +import com.fagi.BaseFagiTest; import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import com.fagi.model.Data; import com.fagi.model.User; import com.fagi.model.messages.message.TextMessage; @@ -10,13 +13,14 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; +import java.util.List; +import java.util.concurrent.TimeUnit; -class ConversationHandlerTest { +class ConversationHandlerTest extends BaseFagiTest { private Data data; private ConversationHandler conversationHandler; @@ -195,12 +199,8 @@ void givenMessageInQueue_WhenTwoOfThreeParticipantsAreOnline_ThenOnlyTheOnlinePa } @Test - void dummy() throws InterruptedException { - var outContent = new ByteArrayOutputStream(); - var errorContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(errorContent)); - System.setOut(new PrintStream(outContent)); - + @Timeout(value = 2, unit = TimeUnit.SECONDS) + void testConversationHandlerLogsMessageWhenInterrupted() throws InterruptedException { var thread = new Thread(conversationHandler); thread.setDaemon(true); @@ -210,13 +210,34 @@ void dummy() throws InterruptedException { thread.interrupt(); + List> logRecords = lookupLogRecordsForClass(ConversationHandler.class); + + while (logRecords.isEmpty()) { + Thread.sleep(100); + logRecords = lookupLogRecordsForClass(ConversationHandler.class); + } + + Assertions.assertEquals( + 1, + logRecords.size() + ); + + TestLogRecord logRecord = logRecords.getFirst(); + Assertions.assertAll( - () -> Assertions.assertTrue(errorContent.toString().contains("java.lang.InterruptedException")), - () -> Assertions.assertFalse(outContent.toString().contains("java.lang.InterruptedException")), + () -> Assertions.assertEquals( + "Interrupted ConversationHandler thread.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.DEBUG, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + InterruptedException.class, + logRecord.throwable() + ), () -> Assertions.assertTrue(thread.isInterrupted()) ); - - System.setErr(System.err); - System.setOut(System.out); } } \ No newline at end of file diff --git a/fagiServer/src/test/java/com/fagi/handler/inputhandler/BaseInputHandlerTest.java b/fagiServer/src/test/java/com/fagi/handler/inputhandler/BaseInputHandlerTest.java index ce56a491..e519c722 100644 --- a/fagiServer/src/test/java/com/fagi/handler/inputhandler/BaseInputHandlerTest.java +++ b/fagiServer/src/test/java/com/fagi/handler/inputhandler/BaseInputHandlerTest.java @@ -1,5 +1,6 @@ package com.fagi.handler.inputhandler; +import com.fagi.BaseFagiTest; import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; import com.fagi.handler.ConversationHandler; @@ -23,7 +24,7 @@ * A base class to make it easy for all {@link InputHandler} tests to have the base set up ready for each test. * Also contains helpful methods to simplify st ups and mocks that are used multiple times. */ -public abstract class BaseInputHandlerTest { +public abstract class BaseInputHandlerTest extends BaseFagiTest { protected OutputAgent outputAgent; protected Data data; protected InputAgent inputAgent; diff --git a/fagiServer/src/test/java/com/fagi/handler/inputhandler/NotValidRequestTests.java b/fagiServer/src/test/java/com/fagi/handler/inputhandler/NotValidRequestTests.java index 3dfc5be8..c3feb210 100644 --- a/fagiServer/src/test/java/com/fagi/handler/inputhandler/NotValidRequestTests.java +++ b/fagiServer/src/test/java/com/fagi/handler/inputhandler/NotValidRequestTests.java @@ -1,28 +1,20 @@ package com.fagi.handler.inputhandler; +import com.fagi.handler.InputHandler; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import com.fagi.util.OutputAgentTestUtil; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; +import java.util.List; import static org.mockito.Mockito.when; public class NotValidRequestTests extends BaseInputHandlerTest { - private final PrintStream standardOut = System.out; - private final ByteArrayOutputStream outputStreamCaptor = new ByteArrayOutputStream(); - void beforeEach() { when(data.getOutputAgent(Mockito.anyString())).thenReturn(outputAgent); - System.setOut(new PrintStream(outputStreamCaptor)); - } - - @AfterEach - public void tearDown() { - System.setOut(standardOut); } @Test @@ -36,14 +28,27 @@ void whenSendingUnknownRequestObject_ShouldGiveNoResponse() { } @Test - void whenSendingUnknownRequestObject_ShouldPrintMessageInSysOut() { + void whenSendingUnknownRequestObject_ShouldLogMessage() { inputHandler.handleInput(new UnknownRequest()); + List> testLogRecords = lookupLogRecordsForClass(InputHandler.class); + Assertions.assertEquals( - "Unknown handle: " + UnknownRequest.class, - outputStreamCaptor - .toString() - .trim() + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Unknown handle: " + UnknownRequest.class, + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.INFO, + logRecord.logLevel() + ) ); } @@ -58,14 +63,27 @@ void givenInputIsNull_ShouldGiveNoResponse() { } @Test - void givenInputIsNull_ShouldPrintMessageInSysOut() { + void givenInputIsNull_ShouldLogMessage() { inputHandler.handleInput(null); + List> testLogRecords = lookupLogRecordsForClass(InputHandler.class); + Assertions.assertEquals( - "Input is null. Doing nothing.", - outputStreamCaptor - .toString() - .trim() + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Input is null. Doing nothing.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.INFO, + logRecord.logLevel() + ) ); } diff --git a/fagiServer/src/test/java/com/fagi/model/DataTests.java b/fagiServer/src/test/java/com/fagi/model/DataTests.java index 39189da7..c899619f 100644 --- a/fagiServer/src/test/java/com/fagi/model/DataTests.java +++ b/fagiServer/src/test/java/com/fagi/model/DataTests.java @@ -1,7 +1,10 @@ package com.fagi.model; +import com.fagi.BaseFagiTest; import com.fagi.conversation.Conversation; import com.fagi.conversation.ConversationType; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import com.fagi.responses.AllIsWell; import com.fagi.responses.NoSuchUser; import com.fagi.responses.PasswordError; @@ -11,21 +14,18 @@ import com.fagi.utility.JsonFileOperations; import com.fagi.worker.InputAgent; import com.fagi.worker.OutputAgent; -import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; import java.util.List; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; -class DataTests { +class DataTests extends BaseFagiTest { private Data data; private User user; private InputAgent inputAgent; @@ -42,11 +42,6 @@ void setup() { ); } - @AfterEach - void tearDown() { - System.setOut(System.out); - } - @Test void userAlreadyOnline_ShouldResultInUserOnlineResponse() { doReturn(true) @@ -529,10 +524,7 @@ void storeInviteCodes_ShouldAttemptToStoreInviteCodesToDisk() { } @Test - void logoutUserWithNotLoggedInUser_ShouldResultInPrintToSysOut() { - var outputStreamCaptor = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outputStreamCaptor)); - + void logoutUserWithNotLoggedInUser_ShouldLogMessage() { var user = new User( "bob", "password" @@ -540,11 +532,24 @@ void logoutUserWithNotLoggedInUser_ShouldResultInPrintToSysOut() { data.userLogout(user.getUserName()); + List> testLogRecords = lookupLogRecordsForClass(Data.class); + Assertions.assertEquals( - "Couldn't log " + user.getUserName() + " out", - outputStreamCaptor - .toString() - .trim() + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Couldn't log " + user.getUserName() + " out", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.INFO, + logRecord.logLevel() + ) ); } diff --git a/fagiServer/src/test/java/com/fagi/model/UserTests.java b/fagiServer/src/test/java/com/fagi/model/UserTests.java index 303e70b4..c0bcdc25 100644 --- a/fagiServer/src/test/java/com/fagi/model/UserTests.java +++ b/fagiServer/src/test/java/com/fagi/model/UserTests.java @@ -1,5 +1,6 @@ package com.fagi.model; +import com.fagi.BaseFagiTest; import com.fagi.model.messages.message.TextMessage; import com.fagi.responses.AllIsWell; import com.fagi.responses.NoSuchUser; @@ -23,7 +24,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.when; -public class UserTests { +public class UserTests extends BaseFagiTest { private Data data; private User user; private User secondUser; diff --git a/fagiServer/src/test/java/com/fagi/server/ServerConversationHandlerTests.java b/fagiServer/src/test/java/com/fagi/server/ServerConversationHandlerTests.java index 69cd97af..4ecbba91 100644 --- a/fagiServer/src/test/java/com/fagi/server/ServerConversationHandlerTests.java +++ b/fagiServer/src/test/java/com/fagi/server/ServerConversationHandlerTests.java @@ -6,7 +6,7 @@ import com.fagi.model.messages.message.TextMessage; import com.fagi.running.IsRunningStrategy; import com.fagi.util.DataTestUtil; -import com.fagi.util.NeverRunStrategy; +import com.fagi.util.running.NeverRunStrategy; import com.fagi.utility.JsonFileOperations; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; diff --git a/fagiServer/src/test/java/com/fagi/server/ServerLoggingTests.java b/fagiServer/src/test/java/com/fagi/server/ServerLoggingTests.java index 89733789..c582d21a 100644 --- a/fagiServer/src/test/java/com/fagi/server/ServerLoggingTests.java +++ b/fagiServer/src/test/java/com/fagi/server/ServerLoggingTests.java @@ -1,30 +1,21 @@ package com.fagi.server; -import com.fagi.util.NeverRunStrategy; -import org.junit.jupiter.api.AfterEach; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; +import com.fagi.util.running.NeverRunStrategy; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.PrintStream; import java.net.ServerSocket; import java.nio.file.Files; +import java.util.List; class ServerLoggingTests extends ServerTests { - @AfterEach - void tearDown() { - System.setOut(System.out); - System.setErr(System.err); - } - @Test - void givenFileWriteGivesIOException_ThenShouldPrintErrorToErrorConsole() { + void givenFileWriteGivesIOException_ThenShouldLogError() { try (var mockedFiles = Mockito.mockStatic(Files.class)) { - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - mockedFiles .when(() -> Files.write( Mockito.any(), @@ -38,18 +29,38 @@ void givenFileWriteGivesIOException_ThenShouldPrintErrorToErrorConsole() { data ); - String consoleOutput = outContent.toString(); - Assertions.assertTrue(consoleOutput.contains("java.io.IOException")); + List> testLogRecords = lookupLogRecordsForClass( + Server.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Could not create server.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ) + ); } } @Test - void givenServerSocketAcceptThrowsIOException_ThenShouldPrintErrorToSysOutButNotSysError() throws IOException { + void givenServerSocketAcceptThrowsIOException_ThenShouldLogError() throws IOException { var serverSocket = Mockito.mock(ServerSocket.class); - var outContent = new ByteArrayOutputStream(); - var outErrorContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - System.setErr(new PrintStream(outErrorContent)); Mockito .when(serverSocket.accept()) @@ -61,30 +72,38 @@ void givenServerSocketAcceptThrowsIOException_ThenShouldPrintErrorToSysOutButNot ); server.start(serverSocket); + List> testLogRecords = lookupLogRecordsForClass( + Server.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + Assertions.assertAll( - () -> Assertions.assertTrue(outContent - .toString() - .contains("Error in server loop exception = ")), - () -> Assertions.assertTrue(outContent - .toString() - .contains("java.io.IOException")), - () -> Assertions.assertFalse(outErrorContent - .toString() - .contains("Error in server loop exception = ")), - () -> Assertions.assertFalse(outErrorContent - .toString() - .contains("java.io.IOException")), + () -> Assertions.assertEquals( + "Error in server loop exception", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ), () -> Assertions.assertFalse(server.isRunning()) ); } @Test - void givenServerSocketCloseThrowsIOException_ThenShouldPrintErrorToSysErrorButNotSysOut() throws IOException { + void givenServerSocketCloseThrowsIOException_ThenShouldBeLogged() throws IOException { var serverSocket = Mockito.mock(ServerSocket.class); - var outContent = new ByteArrayOutputStream(); - var outErrorContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - System.setErr(new PrintStream(outErrorContent)); Mockito .doThrow(new IOException()) @@ -98,22 +117,36 @@ void givenServerSocketCloseThrowsIOException_ThenShouldPrintErrorToSysErrorButNo server.setIsRunningStrategy(new NeverRunStrategy()); server.start(serverSocket); + List> testLogRecords = lookupLogRecordsForClass( + Server.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + Assertions.assertAll( - () -> Assertions.assertFalse(outContent - .toString() - .contains("java.io.IOException")), - () -> Assertions.assertTrue(outErrorContent - .toString() - .contains("java.io.IOException")), - () -> Assertions.assertFalse(server.isRunning()) + () -> Assertions.assertEquals( + "Failed to close server socket gracefully.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ) ); } @Test - void whenServerStarts_ThenStartingServerIsPrintedInSysOut() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - + void whenServerStartsAndThenStops_ThenShouldLogThatServerStartsAndStops() { var server = new Server( serverPort, data @@ -121,25 +154,42 @@ void whenServerStarts_ThenStartingServerIsPrintedInSysOut() { server.setIsRunningStrategy(new NeverRunStrategy()); server.start(null); - Assertions.assertTrue(outContent - .toString() - .contains("Starting Server")); - } - - @Test - void whenServerStops_ThenStoppingServerIsPrintedInSysOut() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); + List> testLogRecords = lookupLogRecordsForClass( + Server.class, + TestLogLevel.INFO + ); - var server = new Server( - serverPort, - data + Assertions.assertEquals( + 2, + testLogRecords.size() ); - server.setIsRunningStrategy(new NeverRunStrategy()); - server.start(null); - Assertions.assertTrue(outContent - .toString() - .contains("Stopping Server")); + Assertions.assertAll( + () -> Assertions.assertEquals( + "Starting Server", + testLogRecords + .getFirst() + .message() + ), + () -> Assertions.assertEquals( + TestLogLevel.INFO, + testLogRecords + .getFirst() + .logLevel() + ), + + () -> Assertions.assertEquals( + "Stopping Server", + testLogRecords + .getLast() + .message() + ), + () -> Assertions.assertEquals( + TestLogLevel.INFO, + testLogRecords + .getLast() + .logLevel() + ) + ); } } diff --git a/fagiServer/src/test/java/com/fagi/server/ServerTests.java b/fagiServer/src/test/java/com/fagi/server/ServerTests.java index 264f5210..9fdb8df9 100644 --- a/fagiServer/src/test/java/com/fagi/server/ServerTests.java +++ b/fagiServer/src/test/java/com/fagi/server/ServerTests.java @@ -1,5 +1,6 @@ package com.fagi.server; +import com.fagi.BaseFagiTest; import com.fagi.model.Data; import com.fagi.utility.JsonFileOperations; import org.junit.jupiter.api.AfterEach; @@ -8,7 +9,7 @@ import java.io.File; -abstract class ServerTests { +abstract class ServerTests extends BaseFagiTest { protected final int serverPort = 4242; protected Data data; diff --git a/fagiServer/src/test/java/com/fagi/server/ServerWorkerTests.java b/fagiServer/src/test/java/com/fagi/server/ServerWorkerTests.java index ced9ef77..be24bd2c 100644 --- a/fagiServer/src/test/java/com/fagi/server/ServerWorkerTests.java +++ b/fagiServer/src/test/java/com/fagi/server/ServerWorkerTests.java @@ -12,7 +12,7 @@ import com.fagi.responses.AllIsWell; import com.fagi.running.IsRunningStrategy; import com.fagi.util.DataTestUtil; -import com.fagi.util.RunOnceStrategy; +import com.fagi.util.running.RunOnceStrategy; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; diff --git a/fagiServer/src/test/java/com/fagi/util/NeverRunStrategy.java b/fagiServer/src/test/java/com/fagi/util/running/NeverRunStrategy.java similarity index 90% rename from fagiServer/src/test/java/com/fagi/util/NeverRunStrategy.java rename to fagiServer/src/test/java/com/fagi/util/running/NeverRunStrategy.java index d1ca6019..f31748b1 100644 --- a/fagiServer/src/test/java/com/fagi/util/NeverRunStrategy.java +++ b/fagiServer/src/test/java/com/fagi/util/running/NeverRunStrategy.java @@ -1,4 +1,4 @@ -package com.fagi.util; +package com.fagi.util.running; import com.fagi.running.IsRunningStrategy; diff --git a/fagiServer/src/test/java/com/fagi/util/RunOnceStrategy.java b/fagiServer/src/test/java/com/fagi/util/running/RunOnceStrategy.java similarity index 94% rename from fagiServer/src/test/java/com/fagi/util/RunOnceStrategy.java rename to fagiServer/src/test/java/com/fagi/util/running/RunOnceStrategy.java index 0640b3c0..2cd7b0c5 100644 --- a/fagiServer/src/test/java/com/fagi/util/RunOnceStrategy.java +++ b/fagiServer/src/test/java/com/fagi/util/running/RunOnceStrategy.java @@ -1,4 +1,4 @@ -package com.fagi.util; +package com.fagi.util.running; import com.fagi.running.IsRunningStrategy; diff --git a/fagiServer/src/test/java/com/fagi/worker/InputWorkerTests.java b/fagiServer/src/test/java/com/fagi/worker/InputWorkerTests.java index c5b3d1b0..37678c37 100644 --- a/fagiServer/src/test/java/com/fagi/worker/InputWorkerTests.java +++ b/fagiServer/src/test/java/com/fagi/worker/InputWorkerTests.java @@ -1,5 +1,6 @@ package com.fagi.worker; +import com.fagi.BaseFagiTest; import com.fagi.encryption.AES; import com.fagi.encryption.AESKey; import com.fagi.encryption.Conversion; @@ -7,36 +8,34 @@ import com.fagi.encryption.RSA; import com.fagi.handler.ConversationHandler; import com.fagi.handler.InputHandler; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import com.fagi.model.Data; import com.fagi.model.Login; import com.fagi.model.Session; import com.fagi.model.UserNameAvailableRequest; import com.fagi.responses.AllIsWell; -import com.fagi.util.NeverRunStrategy; import com.fagi.util.OutputAgentTestUtil; -import com.fagi.util.RunOnceStrategy; -import org.junit.jupiter.api.AfterEach; +import com.fagi.util.running.NeverRunStrategy; +import com.fagi.util.running.RunOnceStrategy; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; -import java.io.ByteArrayOutputStream; import java.io.EOFException; import java.io.IOException; import java.io.ObjectInputStream; -import java.io.PrintStream; import java.net.SocketException; import java.net.SocketTimeoutException; +import java.util.List; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; -public class InputWorkerTests { +public class InputWorkerTests extends BaseFagiTest { private static final byte[] ENCRYPTED_DATA = "encryptedData".getBytes(); private OutputWorker outputWorker; private ConversationHandler conversationHandler; @@ -61,26 +60,19 @@ void setup() throws IOException, ClassNotFoundException { ); } - @AfterEach - void tearDown() { - System.setErr(System.err); - System.setOut(System.out); - } - @Test - void whenInputWorkerIsCreated_ThenStartingThreadMessagesIsPrintedToConsole() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); + void whenInputWorkerIsStarted_ThenStartingThreadMessageIsLogged() { + inputWorker.setIsRunningStrategy(new NeverRunStrategy()); - new InputWorker( - mockObjectInputStream, - outputWorker, - conversationHandler, - data - ); + inputWorker.run(); + + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("Starting an input thread")); + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Starting an input thread"))); } @Test @@ -109,33 +101,37 @@ void givenUsernameSetToCharles_ThenUsernameShouldBeCharles() { } @Test - void givenRunningIsSetToFalse_WhenWorkerIsRunning_ThenShouldNotPrintRunningToConsole() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - + void givenRunningIsSetToFalse_WhenWorkerIsRunning_ThenShouldNotLogRunning() { inputWorker.setIsRunningStrategy(new NeverRunStrategy()); inputWorker.run(); - String consoleOutput = outContent.toString(); - assertFalse(consoleOutput.contains("Running")); + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); + + Assertions.assertFalse(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Running"))); } @Test - void givenRunningIsSetToFalse_WhenWorkerIsRunning_ThenShouldPrintClosingInputToConsole() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - + void givenRunningIsSetToFalse_WhenWorkerIsRunning_ThenShouldLogClosingInput() { inputWorker.setIsRunningStrategy(new NeverRunStrategy()); inputWorker.run(); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("Closing input")); + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Closing input."))); } @Test - void givenRunningIsSetToTrue_WhenWorkerIsRunning_ThenShouldPrintRunningToConsole() { + void givenRunningIsSetToTrue_WhenWorkerIsRunning_ThenShouldLogRunning() { try (var mockedConversion = Mockito.mockStatic(Conversion.class)) { mockedConversion .when(() -> Conversion.convertFromBytes(any())) @@ -144,17 +140,20 @@ void givenRunningIsSetToTrue_WhenWorkerIsRunning_ThenShouldPrintRunningToConsole var mockAes = Mockito.mock(AES.class); when(mockAes.decrypt(any())).thenReturn("decrypted".getBytes()); - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - inputWorker.setIsRunningStrategy(new RunOnceStrategy()); inputWorker.setSessionCreated(true); inputWorker.setAes(mockAes); inputWorker.run(); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("Running")); + + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Running"))); } } @@ -235,7 +234,7 @@ void givenSessionCreated_WhenWorkerReceivesEncryptedObject_TheAESDecryptionIsCal } @Test - void givenConversionFailsToConvertByteArrayToObject_WhenWorkerReceivesEncryptedObject_ThenSystemErrorShouldContainClassNotFoundException() { + void givenConversionFailsToConvertByteArrayToObject_WhenWorkerReceivesEncryptedObject_ThenShouldLogError() { try (var mockedConversion = Mockito.mockStatic(Conversion.class)) { byte[] decryptedInput = "decryptedData".getBytes(); var mockedAES = Mockito.mock(AES.class); @@ -245,23 +244,43 @@ void givenConversionFailsToConvertByteArrayToObject_WhenWorkerReceivesEncryptedO .when(() -> Conversion.convertFromBytes(any())) .thenThrow(new ClassNotFoundException()); - - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - inputWorker.setIsRunningStrategy(new RunOnceStrategy()); inputWorker.setSessionCreated(true); inputWorker.setAes(mockedAES); inputWorker.run(); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.lang.ClassNotFoundException")); + List> testLogRecords = lookupLogRecordsForClass( + InputWorker.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Failed to decrypt or deserialize object.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + ClassNotFoundException.class, + logRecord.throwable() + ) + ); } } @Test - void givenConversionFailsWithIo_WhenWorkerReceivesEncryptedObject_ThenSystemErrorShouldContainIOException() { + void givenConversionFailsWithIo_WhenWorkerReceivesEncryptedObject_ThenShouldLogError() { try (var mockedConversion = Mockito.mockStatic(Conversion.class)) { byte[] decryptedInput = "decryptedData".getBytes(); var mockedAES = Mockito.mock(AES.class); @@ -271,18 +290,38 @@ void givenConversionFailsWithIo_WhenWorkerReceivesEncryptedObject_ThenSystemErro .when(() -> Conversion.convertFromBytes(any())) .thenThrow(new IOException()); - - var outContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(outContent)); - inputWorker.setIsRunningStrategy(new RunOnceStrategy()); inputWorker.setSessionCreated(true); inputWorker.setAes(mockedAES); inputWorker.run(); - String consoleOutput = outContent.toString(); - assertTrue(consoleOutput.contains("java.io.IOException")); + List> testLogRecords = lookupLogRecordsForClass( + InputWorker.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + + Assertions.assertAll( + () -> Assertions.assertEquals( + "Failed to decrypt or deserialize object.", + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ) + ); } } @@ -291,11 +330,6 @@ void givenSocketException_WhenRunning_ThenShouldHandleUserLogoutGracefully() thr var username = "my username"; when(mockObjectInputStream.readObject()).thenThrow(new SocketException()); - var outContent = new ByteArrayOutputStream(); - var errorContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(errorContent)); - System.setOut(new PrintStream(outContent)); - inputWorker.setSessionCreated(true); inputWorker.setUsername(username); @@ -315,15 +349,13 @@ void givenSocketException_WhenRunning_ThenShouldHandleUserLogoutGracefully() thr ) .stop(); - Assertions.assertAll( - () -> Assertions.assertFalse(inputWorker.isRunning()), - () -> Assertions.assertTrue(outContent - .toString() - .contains("Logging out user " + username)), - () -> Assertions.assertFalse(errorContent - .toString() - .contains("java.net.SocketException")) - ); + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Logging out user " + username))); } @Test @@ -331,11 +363,6 @@ void givenEOFException_WhenRunning_ThenShouldHandleUserLogoutGracefully() throws var username = "my username"; when(mockObjectInputStream.readObject()).thenThrow(new EOFException()); - var outContent = new ByteArrayOutputStream(); - var errorContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(errorContent)); - System.setOut(new PrintStream(outContent)); - inputWorker.setSessionCreated(true); inputWorker.setUsername(username); @@ -355,27 +382,20 @@ void givenEOFException_WhenRunning_ThenShouldHandleUserLogoutGracefully() throws ) .stop(); - Assertions.assertAll( - () -> Assertions.assertFalse(inputWorker.isRunning()), - () -> Assertions.assertTrue(outContent - .toString() - .contains("Logging out user " + username)), - () -> Assertions.assertFalse(errorContent - .toString() - .contains("java.io.EOFException")) - ); + List> testLogRecords = lookupLogRecordsForClass(InputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Logging out user " + username))); } @Test - void givenUnexpectedException_WhenRunning_ThenShouldHandleUserLogoutGracefullyButWithStacktrace() throws IOException, ClassNotFoundException { + void givenUnexpectedException_WhenRunning_ThenShouldHandleUserLogoutGracefullyButWithErrorLog() throws IOException, ClassNotFoundException { var username = "my username"; when(mockObjectInputStream.readObject()).thenThrow(new SocketTimeoutException()); - var outContent = new ByteArrayOutputStream(); - var errorContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(errorContent)); - System.setOut(new PrintStream(outContent)); - inputWorker.setSessionCreated(true); inputWorker.setUsername(username); @@ -395,20 +415,38 @@ void givenUnexpectedException_WhenRunning_ThenShouldHandleUserLogoutGracefullyBu ) .stop(); + Assertions.assertTrue(lookupLogRecordsForClass(InputWorker.class) + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Logging out user " + username))); + + + List> testErrorLogRecords = lookupLogRecordsForClass( + InputWorker.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testErrorLogRecords.size() + ); + + TestLogRecord errorLogRecord = testErrorLogRecords.getFirst(); + Assertions.assertAll( - () -> Assertions.assertFalse(inputWorker.isRunning()), - () -> Assertions.assertTrue(outContent - .toString() - .contains("Logging out user " + username)), - () -> Assertions.assertTrue(outContent - .toString() - .contains("Something went wrong in a input worker while loop ")), - () -> Assertions.assertTrue(outContent - .toString() - .contains("java.net.SocketTimeoutException")), - () -> Assertions.assertTrue(errorContent - .toString() - .contains("java.net.SocketTimeoutException")) + () -> Assertions.assertEquals( + "Something went wrong in a input worker while loop.", + errorLogRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + errorLogRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + SocketTimeoutException.class, + errorLogRecord.throwable() + ) ); } diff --git a/fagiServer/src/test/java/com/fagi/worker/OutputWorkerTest.java b/fagiServer/src/test/java/com/fagi/worker/OutputWorkerTest.java index d9e34c8b..66dffd3f 100644 --- a/fagiServer/src/test/java/com/fagi/worker/OutputWorkerTest.java +++ b/fagiServer/src/test/java/com/fagi/worker/OutputWorkerTest.java @@ -1,7 +1,10 @@ package com.fagi.worker; +import com.fagi.BaseFagiTest; import com.fagi.encryption.AES; import com.fagi.encryption.Conversion; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; import com.fagi.model.Data; import com.fagi.model.FriendRequest; import com.fagi.model.User; @@ -12,9 +15,8 @@ import com.fagi.model.messages.message.TextMessage; import com.fagi.responses.AllIsWell; import com.fagi.responses.UserOnline; -import com.fagi.util.NeverRunStrategy; -import com.fagi.util.RunOnceStrategy; -import org.junit.jupiter.api.AfterEach; +import com.fagi.util.running.NeverRunStrategy; +import com.fagi.util.running.RunOnceStrategy; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; @@ -36,8 +38,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.when; -class OutputWorkerTest { +class OutputWorkerTest extends BaseFagiTest { private final ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); + private final String username = "Charles"; private ObjectOutputStream objOut; private Data data; private OutputWorker outputWorker; @@ -62,15 +65,10 @@ void givenOutputWorkerCreated_ThenShouldBeRunning() { } @Nested - class OutputWorkerErrorHandlingTests { - @AfterEach - void tearDown() { - System.setErr(System.err); - System.setOut(System.out); - } + class OutputWorkerErrorHandlingTests extends BaseFagiTest { @Test - void givenWritingObjectGivesIOException_WhenRunningIsFalse_ThenShouldNotWriteToConsole() throws IOException { + void givenWritingObjectGivesIOException_WhenRunningIsFalse_ThenShouldNotLogError() throws IOException { var outContent = new ByteArrayOutputStream(); var errorContent = new ByteArrayOutputStream(); System.setErr(new PrintStream(errorContent)); @@ -84,14 +82,12 @@ void givenWritingObjectGivesIOException_WhenRunningIsFalse_ThenShouldNotWriteToC outputWorker.run(); - Assertions.assertAll( - () -> Assertions.assertFalse(outContent - .toString() - .contains("java.io.IOException")), - () -> Assertions.assertFalse(errorContent - .toString() - .contains("java.io.IOException")) + List> testLogRecords = lookupLogRecordsForClass( + OutputWorker.class, + TestLogLevel.ERROR ); + + Assertions.assertTrue(testLogRecords.isEmpty()); } @Test @@ -107,50 +103,91 @@ void givenWritingObjectsGivesException_WhenRunningIsTrue_ThenRunningShouldBeSetT } @Test - void givenWritingObjectsGivesIOException_WhenRunningIsTrue_ThenShouldWriteToConsoleAsInfo() throws IOException { - var outContent = new ByteArrayOutputStream(); - var errorContent = new ByteArrayOutputStream(); - System.setErr(new PrintStream(errorContent)); - System.setOut(new PrintStream(outContent)); - + void givenWritingObjectsGivesIOException_WhenRunningIsTrue_ThenShouldLogError() throws IOException { doThrow(new IOException()) .when(objOut) .writeObject(any()); outputWorker.addResponse("dummy"); + outputWorker.setUserName(username); outputWorker.run(); + List> testLogRecords = lookupLogRecordsForClass( + OutputWorker.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); + Assertions.assertAll( - () -> Assertions.assertTrue(outContent - .toString() - .contains("java.io.IOException")), - () -> Assertions.assertFalse(errorContent - .toString() - .contains("java.io.IOException")) + () -> Assertions.assertEquals( + "Logging out user " + username, + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ) ); } @Test void givenWritingObjectsGivesException_WhenRunningIsTrue_ThenShouldLogoutUser() throws IOException { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - doThrow(new IOException()) .when(objOut) .writeObject(any()); outputWorker.addResponse("dummy"); - outputWorker.setUserName("bob"); + outputWorker.setUserName(username); outputWorker.run(); - Mockito.verify(data, times(1)).userLogout("bob"); + Mockito + .verify( + data, + times(1) + ) + .userLogout(username); + + List> testLogRecords = lookupLogRecordsForClass( + OutputWorker.class, + TestLogLevel.ERROR + ); + + Assertions.assertEquals( + 1, + testLogRecords.size() + ); + + TestLogRecord logRecord = testLogRecords.getFirst(); - Assertions.assertTrue(outContent.toString().contains("Logging out user bob")); + Assertions.assertAll( + () -> Assertions.assertEquals( + "Logging out user " + username, + logRecord.message() + ), + () -> Assertions.assertEquals( + TestLogLevel.ERROR, + logRecord.logLevel() + ), + () -> Assertions.assertInstanceOf( + IOException.class, + logRecord.throwable() + ) + ); } } @Nested - class OutputWorkerEqualListsTests { + class OutputWorkerEqualListsTests extends BaseFagiTest { @Test void givenBothListsAreNull_WhenCallingEqualLists_ThenShouldReturnTrue() { Assertions.assertTrue(outputWorker.equalLists( @@ -252,7 +289,7 @@ void givenTwoListsHaveDifferentContent_WhenCallingEqualLists_ThenShouldReturnTru } @Nested - class OutputWorkerSendFriendRequestListTests { + class OutputWorkerSendFriendRequestListTests extends BaseFagiTest { @Test void givenNoUsernameInOutputWorker_WhenCheckingFriendRequestList_ThenShouldNotSendFriendRequestList() throws IOException { outputWorker.setIsRunningStrategy(new WorkerRunCalledNTimesStrategy(2)); @@ -463,16 +500,14 @@ void whenCheckingFriendRequestListTwentyTimes_ThenRunShouldTakeAtLeastTwoSeconds } @Nested - class OutputWorkerSendMessagesAndResponsesTests { + class OutputWorkerSendMessagesAndResponsesTests extends BaseFagiTest { @Test void givenMessagesQueueHasTwoMessages_WhenRunningIsTrue_ThenHaveSentTwoObject() throws IOException { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - var user1LoggedInMessage = new UserLoggedIn("bob"); var user2LoggedInMessage = new UserLoggedIn("eve"); outputWorker.setIsRunningStrategy(new RunOnceStrategy()); + outputWorker.setUserName(username); outputWorker.addMessage(user1LoggedInMessage); outputWorker.addMessage(user2LoggedInMessage); @@ -490,6 +525,8 @@ void givenMessagesQueueHasTwoMessages_WhenRunningIsTrue_ThenHaveSentTwoObject() List messageObjects = captor.getAllValues(); + List> testLogRecords = lookupLogRecordsForClass(OutputWorker.class); + Assertions.assertAll( () -> Assertions.assertEquals( 2, @@ -499,16 +536,20 @@ void givenMessagesQueueHasTwoMessages_WhenRunningIsTrue_ThenHaveSentTwoObject() Conversion.convertToBytes(user1LoggedInMessage), messageObjects.getFirst() ), - () -> Assertions.assertTrue(outContent - .toString() - .contains(user1LoggedInMessage.toString())), + () -> Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Sending the following object to user " + username + ": " + user1LoggedInMessage))), () -> Assertions.assertArrayEquals( Conversion.convertToBytes(user2LoggedInMessage), messageObjects.getLast() ), - () -> Assertions.assertTrue(outContent - .toString() - .contains(user2LoggedInMessage.toString())) + () -> Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Sending the following object to user " + username + ": " + user2LoggedInMessage))) ); } @@ -581,45 +622,39 @@ void whenRunningIsFalse_ThenShouldSendAllRespondObjects() throws IOException { } @Nested - class OutputWorkerLoggingTests { - @AfterEach - void tearDown() { - System.setOut(System.out); - } - + class OutputWorkerLoggingTests extends BaseFagiTest { @Test - void whenRunningIsFalse_ThenClosingOutputShouldBePrintedToConsole() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - + void whenRunningIsFalse_ThenClosingOutputShouldBeLogged() { outputWorker.setIsRunningStrategy(new NeverRunStrategy()); outputWorker.run(); - Assertions.assertTrue(outContent - .toString() - .contains("Closing output")); + List> testLogRecords = lookupLogRecordsForClass(OutputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Closing output"))); } @Test - void whenRunningIsTrue_ThenRunningShouldBePrintedToConsole() { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - + void whenRunningIsTrue_ThenRunningShouldBeLogged() { outputWorker.setIsRunningStrategy(new RunOnceStrategy()); outputWorker.run(); - Assertions.assertTrue(outContent - .toString() - .contains("Running")); + List> testLogRecords = lookupLogRecordsForClass(OutputWorker.class); + + Assertions.assertTrue(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals("Running"))); } @Test void givenMessagesQueueHasOneMessage_WhenRunningIsTrue_ThenHaveSentOneObject() throws IOException { - var outContent = new ByteArrayOutputStream(); - System.setOut(new PrintStream(outContent)); - var userLoggedInMessage = new UserLoggedIn("bob"); outputWorker.setIsRunningStrategy(new RunOnceStrategy()); @@ -639,15 +674,18 @@ void givenMessagesQueueHasOneMessage_WhenRunningIsTrue_ThenHaveSentOneObject() t byte[] messageObject = captor.getValue(); - Assertions.assertAll( - () -> Assertions.assertArrayEquals( - Conversion.convertToBytes(userLoggedInMessage), - messageObject - ), - () -> Assertions.assertTrue(outContent - .toString() - .contains(userLoggedInMessage.toString())) + Assertions.assertArrayEquals( + Conversion.convertToBytes(userLoggedInMessage), + messageObject ); + + List> testLogRecords = lookupLogRecordsForClass(OutputWorker.class); + + Assertions.assertFalse(testLogRecords + .stream() + .anyMatch(logRecord -> logRecord + .message() + .equals(userLoggedInMessage.toString()))); } } diff --git a/shared/src/main/java/com/fagi/db/LogBasedDatabase.java b/shared/src/main/java/com/fagi/db/LogBasedDatabase.java index 9c369048..b4aeda95 100644 --- a/shared/src/main/java/com/fagi/db/LogBasedDatabase.java +++ b/shared/src/main/java/com/fagi/db/LogBasedDatabase.java @@ -1,7 +1,16 @@ package com.fagi.db; -import java.io.*; -import java.util.*; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; + +import java.io.BufferedReader; +import java.io.FileReader; +import java.io.FileWriter; +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -70,8 +79,8 @@ *

Error Handling:

*

The database handles various error conditions gracefully:

*
    - *
  • Corrupted Log entries are skipped with warnings to stderr
  • - *
  • Malformed entries are ignored during startup
  • + *
  • Corrupted Log entries are skipped with warnings logged to WARNING
  • + *
  • Malformed entries are ignored during startup and logged to WARNING
  • *
  • I/O errors during writes throw RuntimeException
  • *
* @@ -87,6 +96,7 @@ * @see com.fagi.utility.Checksum */ public class LogBasedDatabase implements AutoCloseable { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(LogBasedDatabase.class); enum DBOperation { PUT, @@ -125,11 +135,11 @@ enum DBOperation { * * @param filePath the path to the database Log file, must not be null * @throws DatabaseInitializeException if the database cannot be initialized due to: - *
    - *
  • I/O errors when creating/opening the Log file
  • - *
  • Permission issue with the file or directory
  • - *
  • Corruption in the existing Log file that prevents loading
  • - *
+ *
    + *
  • I/O errors when creating/opening the Log file
  • + *
  • Permission issue with the file or directory
  • + *
  • Corruption in the existing Log file that prevents loading
  • + *
*/ public LogBasedDatabase(String filePath) throws DatabaseInitializeException { this.logFilePath = filePath; @@ -144,7 +154,10 @@ public LogBasedDatabase(String filePath) throws DatabaseInitializeException { loadFromLog(); } catch (IOException e) { close(); - throw new DatabaseInitializeException("Failed to initialize database", e); + throw new DatabaseInitializeException( + "Failed to initialize database", + e + ); } catch (DatabaseInitializeException e) { close(); throw e; @@ -158,14 +171,14 @@ public LogBasedDatabase(String filePath) throws DatabaseInitializeException { * It bypasses normal file initialization and error handling. The provided * FileWriter must be properly configured for append mode.

* - * @param filePath the path to the database log file, used for identification + * @param filePath the path to the database log file, used for identification * @param logWriter the FileWriter to use for logging operations, must not be null * @throws DatabaseInitializeException if the database cannot be initialized due to: - *
    - *
  • I/O errors when creating/opening the Log file
  • - *
  • Permission issue with the file or directory
  • - *
  • Corruption in the existing Log file that prevents loading
  • - *
+ *
    + *
  • I/O errors when creating/opening the Log file
  • + *
  • Permission issue with the file or directory
  • + *
  • Corruption in the existing Log file that prevents loading
  • + *
*/ public LogBasedDatabase( String filePath, @@ -190,14 +203,12 @@ public LogBasedDatabase( *

Performance: O(1) time complexity. Constant time regardless * of database size.

* - * @param id the record identifier, must not be null - * @param key the field key, must not be null + * @param id the record identifier, must not be null + * @param key the field key, must not be null * @param value the field value, must not be null * @throws IllegalArgumentException if any parameter is null - * @throws DatabaseUpdateException if the write operation fails due to I/O errors - * - * @snippet - *
{@code
+     * @throws DatabaseUpdateException  if the write operation fails due to I/O errors
+     * @snippet 
{@code
      * db.put("user123", "name", "Alice");
      * db.put("user123", "email", "alice@example.com");
      * db.put("user123", "status", "active");
@@ -245,13 +256,11 @@ public void put(
      *
      * 

Performance: O(1) time complexity. Direct hash map lookup.

* - * @param id the record identifier to look up, must not be null + * @param id the record identifier to look up, must not be null * @param key the field key to retrieve, must not be null * @return the field value, or {@code null} if the record or field doesn't exist * @throws IllegalArgumentException if the id or key is null - * - * @snippet - *
{@code
+     * @snippet 
{@code
      * String email = db.get("user123", "email"); // Returns "alice@example.com" or null
      * String phone = db.get("user123", "phone"); // Returns null if field doesn't exist
      * String name = db.get("nonexistent", "name"); // Returns null if record doesn't exist
@@ -288,8 +297,7 @@ public String get(
      * @param id the record identifier to retrieve, must not be null
      * @return a copy of all fields for the record, or {@code null} if the record doesn't exist
      * @throws IllegalArgumentException if id is null
-     * @snippet
-     * 
{@code
+     * @snippet 
{@code
      * Map user = db.get("user123");
      * if (user != null) {
      *     String name = user.get("name");     // "Alice Johnson"
@@ -344,7 +352,8 @@ private void loadFromLog() throws DatabaseInitializeException {
 
                     String[] parts = line.split("\\|");
                     if (parts.length < 5) {
-                        System.err.println("Malformed entry at line " + lineNumber);
+                        int malformedLine = lineNumber;
+                        LOGGER.warning(() -> "Malformed entry at line " + malformedLine);
                         continue;
                     }
 
@@ -358,7 +367,8 @@ private void loadFromLog() throws DatabaseInitializeException {
                     );
 
                     if (!calculateChecksum(originalEntry).equals(storedChecksum)) {
-                        System.err.println("Corruption detected at line " + lineNumber);
+                        int corruptLine = lineNumber;
+                        LOGGER.warning(() -> "Corruption detected at line " + corruptLine);
                         continue;
                     }
 
@@ -405,9 +415,9 @@ private void processLogEntry(String[] parts) {
      * Appends an operation to the Log file with checksum verification.
      *
      * @param operation the operator type (e.g. "PUT")
-     * @param id the record identifier
-     * @param key the field key
-     * @param value the field value
+     * @param id        the record identifier
+     * @param key       the field key
+     * @param value     the field value
      * @throws DatabaseUpdateException if the write operations fails
      */
     private void appendToLog(
@@ -450,14 +460,12 @@ private void appendToLog(
      * 

Performance: O(n) where n is the total number of records. * Performance degrades linearly with database size.

* - * @param fieldName the name of the field to match against, must not be null + * @param fieldName the name of the field to match against, must not be null * @param fieldValue the value to search for, must not be null * @return a List or records (as maps) that contain the specified field-value pair, - * empty List if no matches found, never null + * empty List if no matches found, never null * @throws IllegalArgumentException if fieldName or fieldValue is null - * - * @snippet - *
{@code
+     * @snippet 
{@code
      * // Find all active users
      * List> activeUsers = db.queryByField("status", "active");
      *
@@ -466,7 +474,7 @@ private void appendToLog(
      *
      * // Process results
      * for (Map user : activeUsers) {
-     *     System.out.println("Active user: " + user.get("name"));
+     *     LOGGER.info(() -> "Active user: " + user.get("name"));
      * }
      * }
*/ @@ -499,15 +507,13 @@ public List> queryByField( *

Performance: O(1) time complexity. Direct size lookup.

* * @return the number of records currently stored in the database, never negative - * - * @snippet - *
{@code
+     * @snippet 
{@code
      * int totalUsers = db.count();
-     * System.out.println("Database contains " + totalUsers + " records");
+     * LOGGER.info(() -> "Database contains " + totalUsers + " records");
      *
      * // Check if database is empty
      * if (db.count() == 0) {
-     *     System.out.println("Database is empty");
+     *     LOGGER.info(() -> "Database is empty");
      * }
      * }
*/ @@ -535,12 +541,11 @@ public int count() { * only be called when no other operations are in progress.

* *

Error Handling: I/O errors during close are logged - * to stderr but do not throw exceptions.

+ * to ERROR but do not throw exceptions.

* *

This method is idempotent - calling it multiple times has no additional effect.

* - * @snippet - *
{@code
+     * @snippet 
{@code
      * LogBasedDatabase db = new LogBasedDatabase("myapp.db");
      * try {
      *     // Use database...
@@ -563,7 +568,10 @@ public void close() {
                     logWriter.close();
                 }
             } catch (IOException e) {
-                System.err.println("Error closing database: " + e.getMessage());
+                LOGGER.error(
+                        e,
+                        () -> "Error closing database."
+                );
             }
         }
     }
diff --git a/shared/src/main/java/com/fagi/encryption/AES.java b/shared/src/main/java/com/fagi/encryption/AES.java
index 666198f2..5da51d6b 100644
--- a/shared/src/main/java/com/fagi/encryption/AES.java
+++ b/shared/src/main/java/com/fagi/encryption/AES.java
@@ -1,6 +1,7 @@
 package com.fagi.encryption;
 
-import com.fagi.utility.Logger;
+import com.fagi.logging.FagiLogger;
+import com.fagi.logging.FagiLoggerFactory;
 
 import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
@@ -17,6 +18,7 @@
  * Created by Marcus on 04-06-2016.
  */
 public class AES implements EncryptionAlgorithm {
+    private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(AES.class);
     byte[] iv = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
     IvParameterSpec ivspec = new IvParameterSpec(iv);
     private AESKey key;
@@ -44,8 +46,10 @@ public byte[] encrypt(byte[] msg) {
             cipher.init(Cipher.ENCRYPT_MODE, key.key(), ivspec);
             return cipher.doFinal(msg);
         } catch (NoSuchAlgorithmException | NoSuchPaddingException | BadPaddingException | InvalidKeyException | IllegalBlockSizeException | InvalidAlgorithmParameterException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to encrypt byte array."
+            );
         }
         return null;
     }
@@ -57,8 +61,10 @@ public byte[] decrypt(byte[] cipherText) {
             cipher.init(Cipher.DECRYPT_MODE, key.key(), ivspec);
             return cipher.doFinal(cipherText);
         } catch (NoSuchAlgorithmException | NoSuchPaddingException | BadPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | IllegalBlockSizeException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to decrypt byte array."
+            );
         }
         return null;
     }
@@ -70,8 +76,10 @@ public void generateKey(int keyLength) {
             keygen.init(keyLength);
             this.key = new AESKey(keygen.generateKey());
         } catch (NoSuchAlgorithmException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to generate AES key."
+            );
         }
     }
 
diff --git a/shared/src/main/java/com/fagi/encryption/KeyStorage.java b/shared/src/main/java/com/fagi/encryption/KeyStorage.java
index f51f345d..50c27fcb 100644
--- a/shared/src/main/java/com/fagi/encryption/KeyStorage.java
+++ b/shared/src/main/java/com/fagi/encryption/KeyStorage.java
@@ -1,6 +1,7 @@
 package com.fagi.encryption;
 
-import com.fagi.utility.Logger;
+import com.fagi.logging.FagiLogger;
+import com.fagi.logging.FagiLoggerFactory;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -22,6 +23,7 @@
  * Created by Marcus on 04-06-2016.
  */
 public class KeyStorage {
+    private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(KeyStorage.class);
     private static final String KEYSFOLDER = "config/keys";
     public static final String PUBLICKEYFILE = KEYSFOLDER + "/public.key";
     public static final String PRIVATEKEYFILE = KEYSFOLDER + "/private.key";
@@ -71,8 +73,10 @@ public static KeyPair LoadKeyPair(String algorithm) throws IOException, NoSuchAl
         try {
             fis.read(encodedPrivateKey);
         } catch (IOException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to read encoded private key."
+            );
         }
         fis.close();
 
diff --git a/shared/src/main/java/com/fagi/encryption/RSA.java b/shared/src/main/java/com/fagi/encryption/RSA.java
index 655e7480..031f8a59 100644
--- a/shared/src/main/java/com/fagi/encryption/RSA.java
+++ b/shared/src/main/java/com/fagi/encryption/RSA.java
@@ -1,6 +1,7 @@
 package com.fagi.encryption;
 
-import com.fagi.utility.Logger;
+import com.fagi.logging.FagiLogger;
+import com.fagi.logging.FagiLoggerFactory;
 
 import javax.crypto.BadPaddingException;
 import javax.crypto.Cipher;
@@ -20,6 +21,7 @@
  * Created by Marcus on 30-05-2016.
  */
 public class RSA implements EncryptionAlgorithm {
+    private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(RSA.class);
     private RSAKey key;
     // TODO: Remove this as it is redundant. Trello task: https://trello.com/c/JZynEGUQ
     private PublicKey encryptionKey;
@@ -30,8 +32,10 @@ public RSA() {
             try {
                 key = new RSAKey(KeyStorage.LoadKeyPair("RSA"));
             } catch (IOException | NoSuchAlgorithmException | InvalidKeySpecException e) {
-                e.printStackTrace();
-                Logger.logStackTrace(e);
+                LOGGER.error(
+                        e,
+                        () -> "Failed to load existing RSA key pair."
+                );
             }
         } else {
             generateKey(4096);
@@ -61,8 +65,10 @@ public byte[] encrypt(byte[] msg) {
             cipher.init(Cipher.ENCRYPT_MODE, encryptionKey);
             return cipher.doFinal(msg);
         } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | IllegalBlockSizeException | BadPaddingException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to encrypt byte array."
+            );
         }
         return null;
     }
@@ -78,8 +84,10 @@ public byte[] decrypt(byte[] cipherText) {
             );
             return cipher.doFinal(cipherText);
         } catch (NoSuchAlgorithmException | NoSuchPaddingException | BadPaddingException | IllegalBlockSizeException | InvalidKeyException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to decrypt byte array."
+            );
         }
         return null;
     }
@@ -91,8 +99,10 @@ public void generateKey(int keyLength) {
             keygen.initialize(keyLength);
             key = new RSAKey(keygen.generateKeyPair());
         } catch (NoSuchAlgorithmException e) {
-            e.printStackTrace();
-            Logger.logStackTrace(e);
+            LOGGER.error(
+                    e,
+                    () -> "Failed to generate RSA key."
+            );
         }
     }
 
diff --git a/shared/src/main/java/com/fagi/utility/Checksum.java b/shared/src/main/java/com/fagi/utility/Checksum.java
index a1a35b75..acb64f7e 100644
--- a/shared/src/main/java/com/fagi/utility/Checksum.java
+++ b/shared/src/main/java/com/fagi/utility/Checksum.java
@@ -40,7 +40,7 @@
  * // Calculate checksum for data integrity
  * String data = "PUT|user1|name|John Doe";
  * String checksum = Checksum.calculateChecksum(data);
- * System.out.println("Checksum: " + checksum); // e.g., "a1b2c3d4"
+ * LOGGER.info(() -> "Checksum: " + checksum); // e.g., "a1b2c3d4"
  *
  * // Verify data integrity
  * String receivedData = "PUT|user1|name|John Doe";
@@ -48,9 +48,9 @@
  * String calculatedChecksum = Checksum.calculateChecksum(receivedData);
  *
  * if (calculatedChecksum.equals(receivedChecksum)) {
- *     System.out.println("Data integrity verified");
+ *     LOGGER.info(() -> "Data integrity verified");
  * } else {
- *     System.out.println("Data corruption detected!");
+ *     LOGGER.warning(() -> "Data corruption detected!");
  * }
  * }
* @@ -112,7 +112,7 @@ private Checksum() { * * // Empty string handling * String emptyChecksum = Checksum.calculateChecksum(""); - * System.out.println("Empty string checksum: " + emptyChecksum); + * LOGGER.info(() -> "Empty string checksum: " + emptyChecksum); * * // Database log entry checksumming * String logEntry = "PUT|user123|email|john@example.com"; @@ -203,9 +203,9 @@ public static String calculateChecksum(String data, String encoding) * // Later, verify the data hasn't been corrupted * String retrievedData = "Important data"; * if (Checksum.verifyChecksum(retrievedData, storedChecksum)) { - * System.out.println("Data integrity verified"); + * LOGGER.info(() -> "Data integrity verified"); * } else { - * System.out.println("Data corruption detected!"); + * LOGGER.warning(() -> "Data corruption detected!"); * } * }
*/ diff --git a/shared/src/main/java/com/fagi/utility/JsonFileOperations.java b/shared/src/main/java/com/fagi/utility/JsonFileOperations.java index 4036e18a..e7902643 100644 --- a/shared/src/main/java/com/fagi/utility/JsonFileOperations.java +++ b/shared/src/main/java/com/fagi/utility/JsonFileOperations.java @@ -1,6 +1,8 @@ package com.fagi.utility; import com.fagi.conversation.Conversation; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; import com.google.gson.Gson; import java.io.BufferedReader; @@ -17,6 +19,7 @@ * Created by costa on 09-11-2016. */ public class JsonFileOperations { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(JsonFileOperations.class); public static final String FAGI_EXTENSION = ".fagi"; public static final String CONFIG_FOLDER_PATH = "config/"; public static final String CONVERSATION_FOLDER_PATH = "conversations/"; @@ -40,13 +43,18 @@ public static void storeObjectToFile( Gson gson = new Gson(); - PrintWriter out = new PrintWriter(new FileWriter(folderPath + fileName + FAGI_EXTENSION, false)); + PrintWriter out = new PrintWriter(new FileWriter( + folderPath + fileName + FAGI_EXTENSION, + false + )); out.println(gson.toJson(object)); out.flush(); out.close(); } catch (IOException e) { - e.printStackTrace(); - Logger.logStackTrace(e); + LOGGER.error( + e, + () -> "Failed to write object to file." + ); } } @@ -68,10 +76,15 @@ public static T loadObjectFromFile( Gson gson = new Gson(); - res = gson.fromJson(json.toString(), clazz); + res = gson.fromJson( + json.toString(), + clazz + ); } catch (IOException e) { - e.printStackTrace(); - Logger.logStackTrace(e); + LOGGER.error( + e, + () -> "Failed to load object from file." + ); } return res; } @@ -92,9 +105,12 @@ public static List loadAllObjectsInFolder( } for (File file : files) { - var loadedObj = loadObjectFromFile(file.getAbsolutePath(), clazz); + var loadedObj = loadObjectFromFile( + file.getAbsolutePath(), + clazz + ); if (loadedObj == null) { - System.err.println("Warning: We were about to add null due to this file path: " + file.getAbsolutePath()); + LOGGER.warning(() -> "We were about to add null due to this file path: " + file.getAbsolutePath()); continue; } res.add(loadedObj); @@ -104,7 +120,11 @@ public static List loadAllObjectsInFolder( } public static void storeConversation(Conversation c) { - storeObjectToFile(c, CONVERSATION_FOLDER_PATH, c.getId() + ""); + storeObjectToFile( + c, + CONVERSATION_FOLDER_PATH, + c.getId() + "" + ); } public static void storeClientConversation( @@ -114,14 +134,24 @@ public static void storeClientConversation( if (!clientFolder.exists()) { clientFolder.mkdir(); } - storeObjectToFile(c, username + "/" + CONVERSATION_FOLDER_PATH, c.getId() + ""); + storeObjectToFile( + c, + username + "/" + CONVERSATION_FOLDER_PATH, + c.getId() + "" + ); } public static List loadAllConversations() { - return loadAllObjectsInFolder(CONVERSATION_FOLDER_PATH, Conversation.class); + return loadAllObjectsInFolder( + CONVERSATION_FOLDER_PATH, + Conversation.class + ); } public static List loadAllClientConversations(String username) { - return loadAllObjectsInFolder(username + "/" + CONVERSATION_FOLDER_PATH, Conversation.class); + return loadAllObjectsInFolder( + username + "/" + CONVERSATION_FOLDER_PATH, + Conversation.class + ); } } diff --git a/shared/src/main/java/com/fagi/utility/Logger.java b/shared/src/main/java/com/fagi/utility/Logger.java deleted file mode 100644 index 59db7300..00000000 --- a/shared/src/main/java/com/fagi/utility/Logger.java +++ /dev/null @@ -1,33 +0,0 @@ -package com.fagi.utility; - -import java.io.File; -import java.io.FileWriter; -import java.io.IOException; -import java.io.PrintWriter; - -/** - * Created by costa on 02-04-2017. - */ -public class Logger { - public static final String LOGS_FOLDER_PATH = "logs/"; - - public static void logStackTrace(Exception e) { - long time = System.nanoTime(); - String filePath = LOGS_FOLDER_PATH + "log-error-" + time + JsonFileOperations.FAGI_EXTENSION; - File folder = new File(LOGS_FOLDER_PATH); - File logFile = new File(filePath); - if (!folder.exists()) { - folder.mkdir(); - } - try { - logFile.createNewFile(); - - PrintWriter out = new PrintWriter(new FileWriter(logFile), false); - e.printStackTrace(out); - out.flush(); - out.close(); - } catch (IOException ioe) { - e.printStackTrace(); - } - } -} diff --git a/shared/src/main/java/com/fagi/utility/NetworkUtility.java b/shared/src/main/java/com/fagi/utility/NetworkUtility.java index 80e9daa8..8f17ca1c 100644 --- a/shared/src/main/java/com/fagi/utility/NetworkUtility.java +++ b/shared/src/main/java/com/fagi/utility/NetworkUtility.java @@ -1,5 +1,8 @@ package com.fagi.utility; +import com.fagi.logging.FagiLogger; +import com.fagi.logging.FagiLoggerFactory; + import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -10,6 +13,8 @@ * Used for utility functions related to network */ public class NetworkUtility { + private static final FagiLogger LOGGER = FagiLoggerFactory.createLogger(NetworkUtility.class); + /** * Calls external service to find the public IP of the machine the server is running on * @@ -17,16 +22,21 @@ public class NetworkUtility { */ public static String getExternalIP() { String ip = ""; + String checkIPServiceUrl = "http://checkip.amazonaws.com"; try { URL whatismyip = URI - .create("http://checkip.amazonaws.com") + .create(checkIPServiceUrl) .toURL(); BufferedReader in = new BufferedReader(new InputStreamReader(whatismyip.openStream())); ip = in.readLine(); } catch (IOException e) { - System.err.println("Could not get public it. Cause: " + e); + LOGGER.error( + e, + () -> "Could not get public ip from " + checkIPServiceUrl + ); } + return ip; } } diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabaseConcurrencyTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabaseConcurrencyTests.java index efaa3658..a9cf0857 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabaseConcurrencyTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabaseConcurrencyTests.java @@ -1,14 +1,23 @@ package com.fagi.db; -import org.junit.jupiter.api.*; +import com.fagi.BaseFagiTest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; import java.nio.file.Path; -import java.util.*; -import java.util.concurrent.*; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -public class LogBasedDatabaseConcurrencyTests { +public class LogBasedDatabaseConcurrencyTests extends BaseFagiTest { private LogBasedDatabase db; @BeforeEach diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabaseCorruptionTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabaseCorruptionTests.java index 4744a98a..4b1ba985 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabaseCorruptionTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabaseCorruptionTests.java @@ -1,15 +1,25 @@ package com.fagi.db; -import org.junit.jupiter.api.*; +import com.fagi.BaseFagiTest; +import com.fagi.logging.TestLogLevel; +import com.fagi.logging.TestLogRecord; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import java.io.*; -import java.nio.file.*; -import java.util.*; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardOpenOption; +import java.util.Arrays; +import java.util.List; import static com.fagi.utility.Checksum.calculateChecksum; -public class LogBasedDatabaseCorruptionTests { +public class LogBasedDatabaseCorruptionTests extends BaseFagiTest { private LogBasedDatabase db; private String dbPath; @@ -53,14 +63,27 @@ void testCorruptedChecksum() throws IOException, DatabaseInitializeException, Da corruptedContent ); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - System.setErr(new PrintStream(out)); - db = new LogBasedDatabase(dbPath); + List> logRecords = lookupLogRecordsForClass(LogBasedDatabase.class); + + Assertions.assertEquals( + 1, + logRecords.size() + ); + + Assertions.assertEquals( + "Corruption detected at line 2", + logRecords + .getFirst() + .message() + ); + Assertions.assertEquals( - "Corruption detected at line 2" + System.lineSeparator(), - out.toString() + TestLogLevel.WARNING, + logRecords + .getFirst() + .logLevel() ); Assertions.assertEquals( @@ -133,15 +156,28 @@ void testOperationsAfterRecovery() throws IOException, DatabaseInitializeExcepti StandardOpenOption.APPEND ); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - System.setErr(new PrintStream(out)); - // Reload and continue operations db = new LogBasedDatabase(dbPath); + List> logRecords = lookupLogRecordsForClass(LogBasedDatabase.class); + + Assertions.assertEquals( + 1, + logRecords.size() + ); + + Assertions.assertEquals( + "Malformed entry at line 3", + logRecords + .getFirst() + .message() + ); + Assertions.assertEquals( - "Malformed entry at line 3" + System.lineSeparator(), - out.toString() + TestLogLevel.WARNING, + logRecords + .getFirst() + .logLevel() ); // Should be able to continue normal operations diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabaseIntegrationTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabaseIntegrationTests.java index 11a8fee3..8efc2502 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabaseIntegrationTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabaseIntegrationTests.java @@ -1,19 +1,25 @@ package com.fagi.db; -import org.junit.jupiter.api.*; +import com.fagi.BaseFagiTest; +import com.fagi.logging.TestLogRecord; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mockito; -import java.io.*; -import java.nio.file.*; -import java.util.*; +import java.io.FileWriter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.List; +import java.util.Map; import static com.fagi.utility.Checksum.calculateChecksum; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; -class LogBasedDatabaseIntegrationTests { - +class LogBasedDatabaseIntegrationTests extends BaseFagiTest { @Test void testCloseIOException(@TempDir Path tempDir) throws IOException, DatabaseInitializeException { FileWriter mockWriter = Mockito.mock(FileWriter.class); @@ -21,26 +27,44 @@ void testCloseIOException(@TempDir Path tempDir) throws IOException, DatabaseIni .when(mockWriter) .close(); Path dbPath = tempDir.resolve("test.db"); - var ignored = dbPath.toFile().createNewFile(); + var ignored = dbPath + .toFile() + .createNewFile(); LogBasedDatabase db = new LogBasedDatabase( dbPath.toString(), mockWriter ); - // Capture System.err output to verify error message - ByteArrayOutputStream errOutput = new ByteArrayOutputStream(); - PrintStream originalErr = System.err; - System.setErr(new PrintStream(errOutput)); - db.close(); - String errorOutput = errOutput.toString(); - Assertions.assertTrue(errorOutput.contains("Error closing database")); - Assertions.assertTrue(errorOutput.contains("Disk full")); + List> logRecords = lookupLogRecordsForClass(LogBasedDatabase.class); + + Assertions.assertEquals( + 1, + logRecords.size() + ); + Assertions.assertEquals( + "Error closing database.", + logRecords + .getFirst() + .message() + ); + Assertions.assertInstanceOf( + IOException.class, + logRecords + .getFirst() + .throwable() + ); + Assertions.assertEquals( + "Disk full", + logRecords + .getFirst() + .throwable() + .getMessage() + ); verify(mockWriter).close(); - System.setErr(originalErr); } @Test diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabasePerformanceTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabasePerformanceTests.java index 8a1ac3dc..a8488bcb 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabasePerformanceTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabasePerformanceTests.java @@ -1,17 +1,23 @@ package com.fagi.db; -import org.junit.jupiter.api.*; +import com.fagi.BaseFagiTest; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.nio.file.Path; -import java.util.*; +import java.util.List; +import java.util.Map; +import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -public class LogBasedDatabasePerformanceTests { +public class LogBasedDatabasePerformanceTests extends BaseFagiTest { private LogBasedDatabase db; private String dbPath; diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabaseResourceTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabaseResourceTests.java index 3be62eaa..c9075a4b 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabaseResourceTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabaseResourceTests.java @@ -1,12 +1,13 @@ package com.fagi.db; +import com.fagi.BaseFagiTest; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import java.nio.file.Path; -public class LogBasedDatabaseResourceTests { +public class LogBasedDatabaseResourceTests extends BaseFagiTest { @Test void testMemoryUsage(@TempDir Path tempDir) throws DatabaseInitializeException, DatabaseUpdateException { String dbPath = tempDir diff --git a/shared/src/test/java/com/fagi/db/LogBasedDatabaseTests.java b/shared/src/test/java/com/fagi/db/LogBasedDatabaseTests.java index da31133e..3d843486 100644 --- a/shared/src/test/java/com/fagi/db/LogBasedDatabaseTests.java +++ b/shared/src/test/java/com/fagi/db/LogBasedDatabaseTests.java @@ -1,5 +1,6 @@ package com.fagi.db; +import com.fagi.BaseFagiTest; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -9,7 +10,7 @@ import java.nio.file.Paths; import java.util.Map; -public class LogBasedDatabaseTests { +public class LogBasedDatabaseTests extends BaseFagiTest { private String dbPath; @BeforeEach From 74f9ffbc60d8e15b327f0d57f2e363738ccec7cf Mon Sep 17 00:00:00 2001 From: zargess Date: Sun, 14 Dec 2025 10:41:51 +0100 Subject: [PATCH 4/4] Made framework print more sensible log level names Changed what log level names are printed in console and in files to match the names of the methods of the FagiLogger Fixed deletion of old log files generated by tests --- .../logging/java/JavaLoggerFormatter.java | 14 +++++++++- .../java/JavaLoggerConfigStrategyTest.java | 27 +++++++++++++++---- .../logging/java/JavaLoggerFormatterTest.java | 4 +-- 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java b/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java index df988ace..5fbfb5ee 100644 --- a/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java +++ b/shared/src/main/java/com/fagi/logging/java/JavaLoggerFormatter.java @@ -5,6 +5,7 @@ import java.text.SimpleDateFormat; import java.util.Date; import java.util.logging.Formatter; +import java.util.logging.Level; import java.util.logging.LogRecord; /** @@ -19,6 +20,7 @@ *

*

* 2025-07-19 09:42:37 [INFO] com.fagi.server.Server - Starting Server + * 2025-07-19 09:42:40 [DEBUG] com.fagi.server.Server - Starting Server took 3 ms *

*

* If there is an exception in the log, the stacktrace will start on the next line. @@ -42,7 +44,7 @@ public String format(LogRecord record) { sb.append(String.format( " [%s] %s - %s%n", - record.getLevel(), + transformLogLevel(record.getLevel()), loggerName, formatMessage(record) )); @@ -57,4 +59,14 @@ public String format(LogRecord record) { return sb.toString(); } + + private String transformLogLevel(Level level) { + return switch (level.getName()) { + case "SEVERE" -> "ERROR"; + case "WARNING" -> "WARNING"; + case "INFO" -> "INFO"; + case "FINE" -> "DEBUG"; + default -> level.getName(); + }; + } } diff --git a/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java b/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java index cf4a654c..303c20c2 100644 --- a/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java +++ b/shared/src/test/java/com/fagi/logging/java/JavaLoggerConfigStrategyTest.java @@ -9,6 +9,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.Comparator; import java.util.List; import java.util.logging.ConsoleHandler; import java.util.logging.FileHandler; @@ -16,6 +17,7 @@ import java.util.logging.Level; import java.util.logging.LogRecord; import java.util.logging.Logger; +import java.util.stream.Stream; /** * Doesn't utilise {@link com.fagi.BaseFagiTest} as we want to test the real logging framework to check if the @@ -33,12 +35,27 @@ static void setupClass() throws IOException { // Deleting the "build/test_logs" folder before executing the test. // This is done to mitigate Windows locking the log files in such a way that they cannot be // deleted after running the tests. - tempDir - .toFile() - .delete(); + if (Files.exists(tempDir)) { + try (Stream walk = Files.walk(tempDir)) { + walk + .sorted(Comparator.reverseOrder()) + .forEach(path -> { + try { + Files.delete(path); + } catch (IOException e) { + throw new RuntimeException( + "Failed to delete: " + path, + e + ); + } + }); + } + } - // Create the "build/test_logs" folder - Files.createDirectory(tempDir); + if (!Files.exists(tempDir)) { + // Create the "build/test_logs" folder + Files.createDirectory(tempDir); + } } @BeforeEach diff --git a/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java b/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java index 2196e652..65a73c74 100644 --- a/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java +++ b/shared/src/test/java/com/fagi/logging/java/JavaLoggerFormatterTest.java @@ -14,7 +14,7 @@ class JavaLoggerFormatterTest { @Test void testFormatterShouldGiveCorrectFormattedStrings() { var logRecord = new LogRecord( - Level.WARNING, + Level.FINE, "This is the log message" ); logRecord.setLoggerName(JavaLoggerFormatterTest.class.getName()); @@ -22,7 +22,7 @@ void testFormatterShouldGiveCorrectFormattedStrings() { String dateTimeString = formatter.dateFormat.format(new Date(logRecord.getMillis())); - var expectedFormattedLogEntry = dateTimeString + " [" + logRecord.getLevel() + "] " + logRecord.getLoggerName() + " - " + logRecord.getMessage() + System.lineSeparator() + IOException.class.getName(); + var expectedFormattedLogEntry = dateTimeString + " [DEBUG] " + logRecord.getLoggerName() + " - " + logRecord.getMessage() + System.lineSeparator() + IOException.class.getName(); // Normalize line endings such that the test works on different operating systems String formattedLogRecord = formatter