From 6d8c91f00b6d145c1300afe766fec22f146d8bc7 Mon Sep 17 00:00:00 2001 From: Martin Choo Date: Fri, 9 Sep 2016 12:52:20 +0800 Subject: [PATCH] [26] Use util.logging's logger and update logging documentation --- README.md | 4 + build.gradle | 8 +- docs/addressbook/Configuration.md | 33 +------ docs/addressbook/Logging.md | 46 ++++----- src/main/java/seedu/address/MainApp.java | 6 +- .../seedu/address/browser/BrowserManager.java | 5 +- .../java/seedu/address/commons/JsonUtil.java | 9 +- .../seedu/address/controller/HelpWindow.java | 7 +- .../seedu/address/controller/MainWindow.java | 11 ++- .../address/controller/PersonListPanel.java | 8 +- .../java/seedu/address/controller/Ui.java | 10 +- .../seedu/address/events/EventManager.java | 11 ++- .../seedu/address/model/ModelManager.java | 8 +- .../seedu/address/storage/StorageManager.java | 36 +++---- .../java/seedu/address/util/AppLogger.java | 56 ----------- src/main/java/seedu/address/util/Config.java | 14 +-- .../seedu/address/util/LoggerManager.java | 94 ++++++++----------- .../address/util/ManifestFileReader.java | 7 +- src/main/resources/log4j2.json | 72 -------------- .../java/guitests/guihandles/GuiHandle.java | 10 +- 20 files changed, 136 insertions(+), 319 deletions(-) delete mode 100644 src/main/java/seedu/address/util/AppLogger.java delete mode 100644 src/main/resources/log4j2.json diff --git a/README.md b/README.md index a28b7ceb7be..b2700411181 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,6 @@ # addressbook-level4 Address Book sample application (Level 4) + +- [Gradle](docs/devops/gradle/Introduction to Gradle.md) +- [Configuration](docs/addressbook/Configuration.md) +- [Logging](docs/addressbook/Logging.md) \ No newline at end of file diff --git a/build.gradle b/build.gradle index 99b65ef5426..32c2c51f644 100644 --- a/build.gradle +++ b/build.gradle @@ -7,7 +7,7 @@ plugins { id "com.github.kt3k.coveralls" version "2.4.0" - id 'com.github.johnrengelman.shadow' version '1.2.3' + id "com.github.johnrengelman.shadow" version '1.2.3' } allprojects { @@ -44,12 +44,9 @@ allprojects { guavaVersion = '19.0' jacksonVersion = '2.7.0' jacksonDataTypeVersion = '2.7.4' - jKeyMasterVersion = '1.2' junitVersion = '4.12' - log4jVersion = '2.6' testFxVersion = '3.1.0' monocleVersion = '1.8.0_20' - slf4jSimpleVersion = '1.6.4' libDir = 'lib' } @@ -63,9 +60,6 @@ allprojects { } dependencies { - compile "org.apache.logging.log4j:log4j-api:$log4jVersion" - compile "org.apache.logging.log4j:log4j-core:$log4jVersion" - compile "org.slf4j:slf4j-simple:$slf4jSimpleVersion" // Required to suppress warning, for jkeymaster, see http://www.slf4j.org/codes.html#StaticLoggerBinder compile "org.controlsfx:controlsfx:$controlsFxVersion" compile "com.fasterxml.jackson.core:jackson-databind:$jacksonVersion" compile "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jacksonDataTypeVersion" diff --git a/docs/addressbook/Configuration.md b/docs/addressbook/Configuration.md index 0f87979bb1b..05c8b9be20e 100644 --- a/docs/addressbook/Configuration.md +++ b/docs/addressbook/Configuration.md @@ -2,40 +2,15 @@ Certain properties of the application can be customized through the configuration file (default: `config.json`): +- Application title - Logging level -- Update interval -- Number/Type of browser(s) used - Local Data file -- Cloud data file +- Preferences file - Name of addressbook -- Application title - -Most of the variable names are rather straightforward. However, the logging section is slightly more complex and will be elaborated on. - -Behaviours to note: -- The application creates a config file with default values if no existing config file can be found. -- Configuration is reset to default if file cannot be read (including if the file contains unexpected or has missing parameters) - -# Logging -There are 2 configuration variables meant for logging: -- `currentLogLevel` - - Specifies the application-wide logging level -- `specialLogLevel` - - Adding `className` : `loggingLevel` pairs to this variable will impose a special logging level for that class (priority over the application-wide logging level) -Here is a typical example of the logging parameters in `config.json`: -``` -... - "currentLogLevel" : "DEBUG", - "specialLogLevels" : { - "ModelManager" : "TRACE", - "SyncManager" : "TRACE" - }, -... -``` -Such a configuration cause the loggers to log messages at the `DEBUG` level throughout the application, except `ModelManager`'s and `SyncManager`'s loggers which will log messages at the `TRACE` level. +Variable names in this application are meant to be straightforward, to allow students to understand its idea more easily. -**Note that these settings do not apply to most of `Config`, and also `StorageManager`'s code to read the configuration file since they are not yet in effect. Therefore the logging will usually be at the default level for `Config` and for the affected parts of `StorageManager`. This default logging level is specified in `resources/log4j2.json`.** +**Note that setting of custom the logging level is done only after reading of the configuration file, thus any logging done before it will be at the default `INFO` level** # Introducing new configuration variables (For developers) diff --git a/docs/addressbook/Logging.md b/docs/addressbook/Logging.md index d8935564121..1c11ef7f513 100644 --- a/docs/addressbook/Logging.md +++ b/docs/addressbook/Logging.md @@ -1,29 +1,20 @@ # Logging -We are using [log4j2](http://logging.apache.org/log4j/2.x/) as our logger, and `AppLogger` is used as a wraparound of the logger to control the logging levels programmatically according to our requirements. +We are using `java.util.logging.Logger` as our logger, and `LoggerManager` is used to manage the logging levels of loggers and handlers (for output of log messages) **Note to developers** -- The logging level can be controlled using the `loggingLevel` setting in the configuration file (See [Configuration](../docs/Configuration.md)). -- The `AppLogger` for a class can be obtained using `LoggerManager.getLogger(Class)` which will log messages according to `src/main/resources/log4j2.json`'s specified patterns -- Currently log messages are output through 3 methods: `Console`, `.log` and `.csv`. Since the CSV file is using `;` as its delimiter, developers should avoid using `;` which may lead to incorrect interpretation of log data. - - -**log4j2 information** -- Supposedly optimized version of `LogBack` -- Can also act as an API for other logger implementations such as `slf4j` - - `log4j2` has its internal implementation, but can be substituted as needed - - Our application uses the default internal implementation +- The logging level can be controlled using the `currentLogLevel` setting in the configuration file (See [Configuration](../docs/Configuration.md)). +- The `Logger` for a class can be obtained using `LoggerManager.getLogger(Class)` which will log messages according to the specified logging level +- Currently log messages are output through: `Console` and `.log` # Guidelines ## Logging Levels -- FATAL +- SEVERE - Critical use case affected, which may possibly cause the termination of the application -- ERROR **(NOT USED)** - -- WARN +- WARNING - Can continue, but with caution - INFO @@ -31,21 +22,21 @@ We are using [log4j2](http://logging.apache.org/log4j/2.x/) as our logger, and ` - e.g. update to local model/request sent to cloud - Information that the layman user can understand -- DEBUG +- CONFIG + - Usually used to output information regarding the system's configuration + +- FINE - Used for superficial debugging purposes to pinpoint components that the fault/bug is likely to arise from - Should include more detailed information as compared to `INFO` i.e. log useful information! - e.g. print the actual list instead of just its size -- TRACE - - Everywhere, e.g. entry and exit of ALL methods, including the parameters being given/returned - - Will not be using log4j2 for this, will explore AOP at a future date +- FINER/FINEST + - Almost everywhere, to allow more specific pinpointing of errors in methods ## General - Possible guidelines to adhere to: - Include version of code being run in EVERY file - - Don't manually concatenate strings, which may worsen performance - - Instead, use log4j2's `{}` to log parameters - `Catch` blocks that re-throw the exception should not log the exception as this may lead to repeated logging of the same exception - Avoid `NullPointerException`s, which may occur if we try to log some return value of an object's method, when the object itself may be null - Ensure that the object being printed has `toString()` implemented, if not the printout will be uninformative such as `Object@31942` @@ -54,3 +45,16 @@ We are using [log4j2](http://logging.apache.org/log4j/2.x/) as our logger, and ` - [Apache Commons Logging guide](http://commons.apache.org/proper/commons-logging/guide.html#Message_PrioritiesLevels) - [10 Tips for Proper Application Logging](https://www.javacodegeeks.com/2011/01/10-tips-proper-application-logging.html) - [Base 22 Java Logging Standards and Guidelines](https://wiki.base22.com/display/btg/Java+Logging+Standards+and+Guidelines) + +# Technical Information +`Logger`s create the log records, and `Handler`s publish these log records to some output destination +- We can specify the log level for both the `Logger`s and the `Handler`s +- By default, `java.util.logging.Logger` has a root logger of level `INFO` which has a `ConsoleHandler` of level `INFO` attached. + +## LoggerManager + - Named `Logger`s can be obtained from this class + - These loggers will output messages to the console and a `.log` file by default, at the `INFO` level + - Can be initialized with a `Config` object which contains a custom logging level + - Loggers obtained *AFTER* this initialization will have their logging level changed + - Logging levels for existing loggers will only be updated if the logger with the same name is requested again from `LoggerManager` + \ No newline at end of file diff --git a/src/main/java/seedu/address/MainApp.java b/src/main/java/seedu/address/MainApp.java index 3be1a22f57f..e67481e75b3 100644 --- a/src/main/java/seedu/address/MainApp.java +++ b/src/main/java/seedu/address/MainApp.java @@ -11,17 +11,17 @@ import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; import seedu.address.storage.StorageManager; -import seedu.address.util.AppLogger; import seedu.address.util.Config; import seedu.address.util.LoggerManager; import java.util.Map; +import java.util.logging.Logger; /** * The main entry point to the application. */ public class MainApp extends Application { - private static final AppLogger logger = LoggerManager.getLogger(MainApp.class); + private static final Logger logger = LoggerManager.getLogger(MainApp.class); private static final int VERSION_MAJOR = 1; private static final int VERSION_MINOR = 6; @@ -88,7 +88,7 @@ protected ModelManager initModelManager(Config config) { @Override public void start(Stage primaryStage) { - logger.info("Starting application: {}", MainApp.VERSION); + logger.info("Starting application: " + MainApp.VERSION); ui.start(primaryStage, this); storageManager.start(); } diff --git a/src/main/java/seedu/address/browser/BrowserManager.java b/src/main/java/seedu/address/browser/BrowserManager.java index cd4ea81fe3e..c36365d8a8d 100644 --- a/src/main/java/seedu/address/browser/BrowserManager.java +++ b/src/main/java/seedu/address/browser/BrowserManager.java @@ -4,9 +4,10 @@ import javafx.scene.web.WebView; import seedu.address.commons.FxViewUtil; import seedu.address.model.person.ReadOnlyPerson; -import seedu.address.util.AppLogger; import seedu.address.util.LoggerManager; +import java.util.logging.Logger; + /** * Manages the AddressBook browser. * To begin using this class: call start(). @@ -15,7 +16,7 @@ public class BrowserManager { private static final String GITHUB_ROOT_URL = "https://github.com/"; private static final String INVALID_GITHUB_USERNAME_MESSAGE = "Unparsable GitHub Username."; - private static AppLogger logger = LoggerManager.getLogger(BrowserManager.class); + private static Logger logger = LoggerManager.getLogger(BrowserManager.class); private WebView browser; public BrowserManager() { diff --git a/src/main/java/seedu/address/commons/JsonUtil.java b/src/main/java/seedu/address/commons/JsonUtil.java index 0c3259ed8a0..347888e5804 100644 --- a/src/main/java/seedu/address/commons/JsonUtil.java +++ b/src/main/java/seedu/address/commons/JsonUtil.java @@ -11,11 +11,11 @@ import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; import com.fasterxml.jackson.databind.type.TypeFactory; -import org.apache.logging.log4j.Level; import java.io.IOException; import java.util.HashMap; import java.util.List; +import java.util.logging.Level; /** * Converts a Java object instance to JSON and vice versa @@ -41,12 +41,7 @@ protected Level _deserialize(String value, DeserializationContext ctxt) throws I * @return */ private Level getLoggingLevel(String loggingLevelString) { - for (Level level : Level.values()) { - if (level.toString().equals(loggingLevelString)) { - return level; - } - } - return null; + return Level.parse(loggingLevelString); } @Override diff --git a/src/main/java/seedu/address/controller/HelpWindow.java b/src/main/java/seedu/address/controller/HelpWindow.java index f97d3a39b4c..39d01079700 100644 --- a/src/main/java/seedu/address/controller/HelpWindow.java +++ b/src/main/java/seedu/address/controller/HelpWindow.java @@ -8,15 +8,16 @@ import javafx.stage.Stage; import seedu.address.MainApp; import seedu.address.commons.FxViewUtil; -import seedu.address.util.AppLogger; import seedu.address.util.LoggerManager; +import java.util.logging.Logger; + /** * Controller for a help page */ public class HelpWindow extends BaseUiPart { - private static final AppLogger logger = LoggerManager.getLogger(HelpWindow.class); + private static final Logger logger = LoggerManager.getLogger(HelpWindow.class); private static final String ICON = "/images/help_icon.png"; public static final String FXML = "HelpWindow.fxml"; public static final String TITLE = "Help"; @@ -26,7 +27,7 @@ public class HelpWindow extends BaseUiPart { private Stage dialogStage; public static HelpWindow load(Stage primaryStage) { - logger.debug("Showing help page about the application."); + logger.fine("Showing help page about the application."); HelpWindow helpWindow = UiPartLoader.loadUiPart(primaryStage, new HelpWindow()); helpWindow.configure(); return helpWindow; diff --git a/src/main/java/seedu/address/controller/MainWindow.java b/src/main/java/seedu/address/controller/MainWindow.java index c498132898e..7c4fb23dbea 100644 --- a/src/main/java/seedu/address/controller/MainWindow.java +++ b/src/main/java/seedu/address/controller/MainWindow.java @@ -21,17 +21,18 @@ import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; import seedu.address.parser.Parser; -import seedu.address.util.AppLogger; import seedu.address.util.Config; import seedu.address.util.GuiSettings; import seedu.address.util.LoggerManager; +import java.util.logging.Logger; + /** * The Main Window. Provides the basic application layout containing * a menu bar and space where other JavaFX elements can be placed. */ public class MainWindow extends BaseUiPart { - private static AppLogger logger = LoggerManager.getLogger(MainWindow.class); + private static Logger logger = LoggerManager.getLogger(MainWindow.class); private static final String ICON = "/images/address_book_32.png"; private static final String FXML = "MainWindow.fxml"; private static final String HEADER_STATUSBAR_PLACEHOLDER_FIELD_ID = "#headerStatusbarPlaceholder"; @@ -87,7 +88,7 @@ public String getFxmlPath() { public static MainWindow load(Stage primaryStage, Config config, UserPrefs prefs, Ui ui, ModelManager modelManager, BrowserManager browserManager) { - logger.debug("Initializing main window."); + logger.fine("Initializing main window."); MainWindow mainWindow = UiPartLoader.loadUiPart(primaryStage, new MainWindow()); mainWindow.configure(config.getAppTitle(), config.getAddressBookName(), prefs, ui, modelManager, browserManager); @@ -209,7 +210,7 @@ private void handleCommandInputChanged() { logger.info("Result: " + command.getClass().getSimpleName()); logger.info("Result: " + result.feedbackToUser); - logger.debug("Invalid command: {}", commandInput.getText()); + logger.fine("Invalid command: " + commandInput.getText()); } @FXML @@ -227,7 +228,7 @@ public void show() { */ @FXML private void handleAbout() {//TODO: refactor to be similar to handleHelp and remove the dependency to ui - logger.debug("Showing information about the application."); + logger.fine("Showing information about the application."); ui.showAlertDialogAndWait(AlertType.INFORMATION, "AddressApp", "About", "Version " + MainApp.VERSION.toString() + "\nSome code adapted from http://code.makery.ch"); } diff --git a/src/main/java/seedu/address/controller/PersonListPanel.java b/src/main/java/seedu/address/controller/PersonListPanel.java index ada1ca2c693..4a4d70367ef 100644 --- a/src/main/java/seedu/address/controller/PersonListPanel.java +++ b/src/main/java/seedu/address/controller/PersonListPanel.java @@ -14,11 +14,11 @@ import seedu.address.model.ModelManager; import seedu.address.model.person.ReadOnlyPerson; import seedu.address.ui.PersonListViewCell; -import seedu.address.util.AppLogger; import seedu.address.util.LoggerManager; import java.util.ArrayList; import java.util.List; +import java.util.logging.Logger; /** * Dialog to view the list of persons and their details @@ -26,7 +26,7 @@ * setConnections should be set before showing stage */ public class PersonListPanel extends BaseUiPart { - private static AppLogger logger = LoggerManager.getLogger(PersonListPanel.class); + private static Logger logger = LoggerManager.getLogger(PersonListPanel.class); public static final String FXML = "PersonListPanel.fxml"; private VBox panel; private AnchorPane placeHolderPane; @@ -40,7 +40,7 @@ public PersonListPanel() { public static PersonListPanel load(Stage primaryStage, AnchorPane personListPlaceholder, ModelManager modelManager, BrowserManager browserManager) { - logger.debug("Loading person list panel."); + logger.fine("Loading person list panel."); PersonListPanel personListPanel = UiPartLoader.loadUiPart(primaryStage, personListPlaceholder, new PersonListPanel()); personListPanel.configure(modelManager, browserManager, @@ -99,7 +99,7 @@ private void setupListviewSelectionModelSettings() { private void loadGithubProfilePageWhenPersonIsSelected(BrowserManager browserManager) { personListView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { if (newValue != null) { - logger.debug("Person in list view clicked. Loading GitHub profile page: '{}'", newValue); + logger.fine("Person in list view clicked. Loading GitHub profile page: '" + newValue + "'"); browserManager.loadPersonPage(newValue); } }); diff --git a/src/main/java/seedu/address/controller/Ui.java b/src/main/java/seedu/address/controller/Ui.java index d6e0f3a296f..faa706c9523 100644 --- a/src/main/java/seedu/address/controller/Ui.java +++ b/src/main/java/seedu/address/controller/Ui.java @@ -20,20 +20,20 @@ import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; import seedu.address.model.person.ReadOnlyPerson; -import seedu.address.util.AppLogger; import seedu.address.util.Config; import seedu.address.util.LoggerManager; import java.io.File; import java.io.IOException; import java.util.List; +import java.util.logging.Logger; /** * The controller that creates the other controllers */ public class Ui { public static final String DIALOG_TITLE_TAG_LIST = "List of Tags"; - private static final AppLogger logger = LoggerManager.getLogger(Ui.class); + private static final Logger logger = LoggerManager.getLogger(Ui.class); private static final String FXML_HELP = "/view/HelpWindow.fxml"; private static final String ICON_APPLICATION = "/images/address_book_32.png"; private static final String ICON_HELP = "/images/help_icon.png"; @@ -94,7 +94,7 @@ private Node loadLoader(FXMLLoader loader, String errorMsg) { try { return loader.load(); } catch (IOException e) { - logger.fatal(errorMsg + ": {}", e); + logger.severe(errorMsg + ": " + e); showFatalErrorDialogAndShutdown("FXML Load Error", errorMsg, "IOException when trying to load ", loader.getLocation().toExternalForm()); return null; @@ -120,7 +120,7 @@ private Stage loadDialogStage(String value, Stage primaryStage, Scene scene) { * Opens a dialog to show the help page */ public void showHelpPage() { - logger.debug("Loading help page."); + logger.fine("Loading help page."); final String fxmlResourcePath = FXML_HELP; // Load the fxml file and create a new stage for the popup dialog. FXMLLoader loader = loadFxml(fxmlResourcePath); @@ -199,7 +199,7 @@ private void showFatalErrorDialogAndShutdown(String title, String headerText, St public void showFatalErrorDialogAndShutdown(String title, Throwable e) { //TODO: Do a more detailed error reporting e.g. stack trace - logger.fatal(title + " " + e.getMessage()); + logger.severe(title + " " + e.getMessage()); showAlertDialogAndWait(Alert.AlertType.ERROR, title, e.getMessage(), e.toString()); Platform.exit(); System.exit(1); diff --git a/src/main/java/seedu/address/events/EventManager.java b/src/main/java/seedu/address/events/EventManager.java index f686f439fe8..31e3f628a05 100644 --- a/src/main/java/seedu/address/events/EventManager.java +++ b/src/main/java/seedu/address/events/EventManager.java @@ -1,14 +1,15 @@ package seedu.address.events; import com.google.common.eventbus.EventBus; -import seedu.address.util.AppLogger; import seedu.address.util.LoggerManager; +import java.util.logging.Logger; + /** * Manages the event dispatching of the app. */ public class EventManager { - private static final AppLogger logger = LoggerManager.getLogger(EventManager.class); + private static final Logger logger = LoggerManager.getLogger(EventManager.class); private final EventBus eventBus; private static EventManager instance; @@ -39,7 +40,7 @@ public EventManager registerHandler(Object handler) { * @return */ public EventManager post(E event) { - logger.infoEvent(event); + logger.info(event.getClass().getName() + ": " + event.toString()); return postEvent(event); } @@ -49,14 +50,14 @@ private EventManager postEvent(E event) { } /** - * Similar to {@link #post} event, but logs at debug level. + * Similar to {@link #post} event, but logs at fine level. * To be used for less important events. * @param event * @param * @return */ public EventManager postPotentialEvent(E event) { - logger.debugEvent(event); + logger.fine(event.getClass().getName() + ": " + event.toString()); return postEvent(event); } diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index c04bd60f0d1..fa8339349a1 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -12,19 +12,19 @@ import seedu.address.model.person.UniquePersonList; import seedu.address.model.person.UniquePersonList.PersonNotFoundException; import seedu.address.parser.expr.Expr; -import seedu.address.util.AppLogger; import seedu.address.util.Config; import seedu.address.util.LoggerManager; import seedu.address.util.collections.UnmodifiableObservableList; import java.util.List; +import java.util.logging.Logger; /** * Represents the in-memory model of the address book data. * All changes to any model should be synchronized. */ public class ModelManager extends ComponentManager implements ReadOnlyAddressBook { - private static final AppLogger logger = LoggerManager.getLogger(ModelManager.class); + private static final Logger logger = LoggerManager.getLogger(ModelManager.class); private final AddressBook backingModel; private final FilteredList filteredPersons; @@ -38,10 +38,10 @@ public class ModelManager extends ComponentManager implements ReadOnlyAddressBoo public ModelManager(AddressBook src, Config config) { super(); if (src == null) { - logger.fatal("Attempted to initialize with a null AddressBook"); + logger.severe("Attempted to initialize with a null AddressBook"); assert false; } - logger.debug("Initializing with address book: {}", src); + logger.fine("Initializing with address book: " + src); backingModel = new AddressBook(src); filteredPersons = new FilteredList<>(backingModel.getPersons()); diff --git a/src/main/java/seedu/address/storage/StorageManager.java b/src/main/java/seedu/address/storage/StorageManager.java index edd5b45fd79..21d3fe27e96 100644 --- a/src/main/java/seedu/address/storage/StorageManager.java +++ b/src/main/java/seedu/address/storage/StorageManager.java @@ -8,7 +8,6 @@ import seedu.address.main.ComponentManager; import seedu.address.model.UserPrefs; import seedu.address.model.datatypes.ReadOnlyAddressBook; -import seedu.address.util.AppLogger; import seedu.address.util.Config; import seedu.address.util.LoggerManager; @@ -17,6 +16,7 @@ import java.io.IOException; import java.util.function.Consumer; import java.util.function.Supplier; +import java.util.logging.Logger; /** * Manages storage of addressbook data in local disk. @@ -24,7 +24,7 @@ */ public class StorageManager extends ComponentManager { - private static final AppLogger logger = LoggerManager.getLogger(StorageManager.class); + private static final Logger logger = LoggerManager.getLogger(StorageManager.class); private static final String DEFAULT_CONFIG_FILE = "config.json"; private final Consumer loadedDataCallback; private final Supplier defaultDataSupplier; @@ -53,10 +53,10 @@ public static Config getConfig(String configFilePath) { Config config; if (configFile.exists()) { - logger.info("Config file {} found, attempting to read.", configFile); + logger.info("Config file " + configFile + " found, attempting to read."); config = readFromConfigFile(configFile); } else { - logger.info("Config file {} not found, using default config.", configFile); + logger.info("Config file " + configFile + " not found, using default config."); config = new Config(); } // Recreate the file so that any missing fields will be restored @@ -73,7 +73,7 @@ private static void createAndWriteToConfigFile(File configFile, Config config) { try { FileUtil.serializeObjectToJsonFile(configFile, config); } catch (IOException e) { - logger.warn("Error writing to config file {}.", configFile); + logger.warning("Error writing to config file " + configFile); } } @@ -90,7 +90,7 @@ private static boolean deleteConfigFileIfExists(File configFile) { FileUtil.deleteFile(configFile); return true; } catch (IOException e) { - logger.warn("Error removing previous config file {}.", configFile); + logger.warning("Error removing previous config file " + configFile); return false; } } @@ -105,7 +105,7 @@ private static Config readFromConfigFile(File configFile) { try { return FileUtil.deserializeObjectFromJsonFile(configFile, Config.class); } catch (IOException e) { - logger.warn("Error reading from config file {}: {}", configFile, e); + logger.warning("Error reading from config file " + configFile + ": " + e); return new Config(); } } @@ -117,7 +117,7 @@ private static Config readFromConfigFile(File configFile) { @Subscribe public void handleLoadDataRequestEvent(LoadDataRequestEvent ldre) { File dataFile = ldre.file; - logger.info("Handling load data request received: {}", dataFile); + logger.info("Handling load data request received: " + dataFile); loadDataFile(dataFile); } @@ -135,7 +135,7 @@ public void handleLocalModelChangedEvent(LocalModelChangedEvent lmce) { */ @Subscribe public void handleSaveDataRequestEvent(SaveDataRequestEvent sdre) { - logger.info("Save data request received: {}", sdre.data); + logger.info("Save data request received: " + sdre.data); saveDataToFile(sdre.file, sdre.data); } @@ -166,7 +166,7 @@ public static void saveAddressBook(File file, ReadOnlyAddressBook data) throws I */ @Subscribe public void handleSavePrefsRequestEvent(SavePrefsRequestEvent spre) { - logger.info("Save prefs request received: {}", spre.prefs); + logger.info("Save prefs request received: " + spre.prefs); savePrefsToFile(spre.prefs); } @@ -189,10 +189,10 @@ public static UserPrefs getUserPrefs(File prefsFile) { } try { - logger.debug("Attempting to load prefs from file: {}", prefsFile); + logger.fine("Attempting to load prefs from file: " + prefsFile); prefs = FileUtil.deserializeObjectFromJsonFile(prefsFile, UserPrefs.class); } catch (IOException e) { - logger.debug("Error loading prefs from file: {}", e); + logger.fine("Error loading prefs from file: " + e); } return prefs; @@ -210,11 +210,11 @@ protected void initializeDataFile(File dataFile) { try { loadDataFromFile(dataFile); } catch (FileNotFoundException e) { - logger.debug("File {} not found, attempting to create file with default data", dataFile); + logger.fine("File " + dataFile + " not found, attempting to create file with default data"); try { saveAddressBook(saveFile, defaultDataSupplier.get()); } catch (DataConversionException | IOException e1) { - logger.fatal("Unable to initialize local data file with default data."); + logger.severe("Unable to initialize local data file with default data."); assert false : "Unable to initialize local data file with default data."; } } @@ -224,23 +224,23 @@ protected void loadDataFile(File dataFile) { try { loadDataFromFile(dataFile); } catch (FileNotFoundException e) { - logger.debug("File not found: {}", dataFile); + logger.fine("File not found: " + dataFile); raise(new FileOpeningExceptionEvent(e, dataFile)); } } protected void loadDataFromFile(File dataFile) throws FileNotFoundException { try { - logger.debug("Attempting to load data from file: {}", dataFile); + logger.fine("Attempting to load data from file: " + dataFile); loadedDataCallback.accept(getData()); } catch (DataConversionException e) { - logger.debug("Error loading data from file: {}", e); + logger.fine("Error loading data from file: " + e); raise(new FileOpeningExceptionEvent(e, dataFile)); } } public ReadOnlyAddressBook getData() throws FileNotFoundException, DataConversionException { - logger.debug("Attempting to read data from file: {}", saveFile); + logger.fine("Attempting to read data from file: " + saveFile); return XmlFileStorage.loadDataFromSaveFile(saveFile); } } diff --git a/src/main/java/seedu/address/util/AppLogger.java b/src/main/java/seedu/address/util/AppLogger.java deleted file mode 100644 index d92b7f9182a..00000000000 --- a/src/main/java/seedu/address/util/AppLogger.java +++ /dev/null @@ -1,56 +0,0 @@ -package seedu.address.util; - -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.Logger; -import seedu.address.events.BaseEvent; - -public class AppLogger { - private Logger logger; - - public AppLogger(Logger logger) { - this.logger = logger; - } - - public void info(String message, Object... params) { - logger.info(message, params); - } - - public void debug(String message, Object... params) { - logger.debug(message, params); - } - - public void warn(String message, Object... params) { - logger.warn(message, params); - - } - - public void fatal(String message, Object... params) { - logger.fatal(message, params); - } - - public void throwing(T throwable) { - logger.throwing(Level.DEBUG, throwable); - } - - public void catching(T throwable) { - logger.catching(Level.DEBUG, throwable); - } - - public void debugEvent(T event) { - debugEvent("{}: {}", event.getClass().getSimpleName(), event.toString()); - } - - public void infoEvent(BaseEvent event) { - infoEvent("{}: {}", event.getClass().getSimpleName(), event.toString()); - } - - // this method is required since debug(message, obj, obj) seems to be problematic - private void debugEvent(String message, Object... params) { - logger.debug(message, params); - } - - // this method is required since info(message, obj, obj) seems to be problematic - private void infoEvent(String message, Object... params) { - logger.info(message, params); - } -} diff --git a/src/main/java/seedu/address/util/Config.java b/src/main/java/seedu/address/util/Config.java index 6e9530dbf67..0a0aff84273 100644 --- a/src/main/java/seedu/address/util/Config.java +++ b/src/main/java/seedu/address/util/Config.java @@ -1,9 +1,7 @@ package seedu.address.util; -import org.apache.logging.log4j.Level; - import java.io.File; -import java.util.HashMap; +import java.util.logging.Level; /** * Config values used by the app @@ -11,7 +9,6 @@ public class Config { // Default values private static final Level DEFAULT_LOGGING_LEVEL = Level.INFO; - private static final HashMap DEFAULT_SPECIAL_LOG_LEVELS = new HashMap<>(); private static final String DEFAULT_LOCAL_DATA_FILE_PATH = "data/addressbook.xml"; private static final String DEFAULT_ADDRESS_BOOK_NAME = "MyAddressBook"; @@ -19,7 +16,6 @@ public class Config { private String appTitle = "Address App"; // Customizable through config file private Level currentLogLevel = DEFAULT_LOGGING_LEVEL; - private HashMap specialLogLevels = DEFAULT_SPECIAL_LOG_LEVELS; private File prefsFileLocation = new File("preferences.json"); //Default user preferences file private String localDataFilePath = DEFAULT_LOCAL_DATA_FILE_PATH; private String addressBookName = DEFAULT_ADDRESS_BOOK_NAME; @@ -44,14 +40,6 @@ public void setCurrentLogLevel(Level currentLogLevel) { this.currentLogLevel = currentLogLevel; } - public HashMap getSpecialLogLevels() { - return specialLogLevels; - } - - public void setSpecialLogLevels(HashMap specialLogLevels) { - this.specialLogLevels = specialLogLevels; - } - public File getPrefsFileLocation() { return prefsFileLocation; } diff --git a/src/main/java/seedu/address/util/LoggerManager.java b/src/main/java/seedu/address/util/LoggerManager.java index 7aea5d83a76..5f78c0e07b6 100644 --- a/src/main/java/seedu/address/util/LoggerManager.java +++ b/src/main/java/seedu/address/util/LoggerManager.java @@ -1,81 +1,61 @@ package seedu.address.util; -import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.core.LoggerContext; -import org.apache.logging.log4j.core.config.AbstractConfiguration; -import org.apache.logging.log4j.core.config.LoggerConfig; - -import java.util.HashMap; +import java.io.IOException; +import java.util.logging.*; +/** + * Configures and manages loggers and handlers, including their logging level + */ public class LoggerManager { - private static final AppLogger logger = LoggerManager.getLogger(LoggerManager.class); private static Level currentLogLevel = Level.INFO; - private static HashMap specialLogLevels = new HashMap<>(); + private static final Logger logger = LoggerManager.getLogger(LoggerManager.class); + private static final String LOG_FILE = "addressbook.log"; + private static FileHandler fileHandler; + private static ConsoleHandler consoleHandler; public static void init(Config config) { currentLogLevel = config.getCurrentLogLevel(); - specialLogLevels = config.getSpecialLogLevels(); - - logger.info("currentLogLevel: {}", currentLogLevel); - logger.info("specialLogLevels: {}", specialLogLevels); - - LoggerContext loggerContext = getLoggerContext(); - AbstractConfiguration loggersConfig = getLoggersConfig(loggerContext); - updateExistingLoggersLevel(loggersConfig); - loggerContext.updateLoggers(loggersConfig); + logger.info("currentLogLevel: " + currentLogLevel); } - public static AppLogger getLogger(String className, Level loggingLevel) { - setClassLoggingLevel(className, loggingLevel); - return new AppLogger(LogManager.getLogger(className)); - } + public static Logger getLogger(String className) { + Logger logger = Logger.getLogger(className); + logger.setUseParentHandlers(false); - public static AppLogger getLogger(String className) { - Level loggingLevelToSet = determineLoggingLevelToSet(className); - setClassLoggingLevel(className, loggingLevelToSet); - return new AppLogger(LogManager.getLogger(className)); - } + addConsoleHandler(logger); + addFileHandler(logger); - public static AppLogger getLogger(Class clazz) { - if (clazz == null) return new AppLogger(LogManager.getRootLogger()); - return getLogger(clazz.getSimpleName()); + return Logger.getLogger(className); } - private static LoggerContext getLoggerContext() { - return (LoggerContext) LogManager.getContext(false); + private static void addConsoleHandler(Logger logger) { + if (consoleHandler == null) consoleHandler = createConsoleHandler(); + logger.addHandler(consoleHandler); } - private static AbstractConfiguration getLoggersConfig(LoggerContext loggerContext) { - return (AbstractConfiguration) loggerContext.getConfiguration(); - } - - private static void updateExistingLoggersLevel(AbstractConfiguration absConfig) { - absConfig.getLoggers().forEach((loggerName, loggerConfig) -> { - loggerConfig.setLevel(determineLoggingLevelToSet(loggerName)); - }); + private static void addFileHandler(Logger logger) { + try { + if (fileHandler == null) fileHandler = createFileHandler(); + logger.addHandler(fileHandler); + } catch (IOException e) { + logger.warning("Error adding file handler for logger."); + } } - private static void setClassLoggingLevel(String className, Level loggingLevel) { - LoggerContext loggerContext = getLoggerContext(); - AbstractConfiguration loggersConfig = getLoggersConfig(loggerContext); - setLoggingLevel(loggersConfig, className, loggingLevel); - loggerContext.updateLoggers(loggersConfig); + private static FileHandler createFileHandler() throws IOException { + FileHandler fileHandler = new FileHandler(LOG_FILE); + fileHandler.setFormatter(new SimpleFormatter()); + return fileHandler; } - private static Level determineLoggingLevelToSet(String className) { - if (specialLogLevels != null && specialLogLevels.containsKey(className)) { - return specialLogLevels.get(className); - } - return currentLogLevel; + private static ConsoleHandler createConsoleHandler() { + ConsoleHandler consoleHandler = new ConsoleHandler(); + consoleHandler.setLevel(currentLogLevel); + return consoleHandler; } - private static void setLoggingLevel(AbstractConfiguration config, String className, Level loggingLevel) { - if (config.getLogger(className) != null) { - config.getLogger(className).setLevel(loggingLevel); - return; - } - - config.addLogger(className, new LoggerConfig(className, loggingLevel, true)); + public static Logger getLogger(Class clazz) { + if (clazz == null) return Logger.getLogger(""); + return getLogger(clazz.getSimpleName()); } } diff --git a/src/main/java/seedu/address/util/ManifestFileReader.java b/src/main/java/seedu/address/util/ManifestFileReader.java index 31b5e6d112c..e58a57735f5 100644 --- a/src/main/java/seedu/address/util/ManifestFileReader.java +++ b/src/main/java/seedu/address/util/ManifestFileReader.java @@ -10,12 +10,13 @@ import java.util.Optional; import java.util.jar.Attributes; import java.util.jar.Manifest; +import java.util.logging.Logger; /** * Reads values in manifest file */ public class ManifestFileReader { - private static final AppLogger logger = LoggerManager.getLogger(ManifestFileReader.class); + private static final Logger logger = LoggerManager.getLogger(ManifestFileReader.class); private static String getResourcePath() { Class mainAppClass = MainApp.class; @@ -47,7 +48,7 @@ private static Optional getManifest() { try { manifest = new Manifest(new URL(manifestPath).openStream()); } catch (IOException e) { - logger.debug("Manifest can't be read, most probably not run from JAR", e); + logger.fine("Manifest can't be read, most probably not run from JAR: " + e); return Optional.empty(); } @@ -61,7 +62,7 @@ public static Optional> getLibrariesInClasspathFromManifest() { Optional manifest = getManifest(); if (!manifest.isPresent()) { - logger.debug("Manifest is not present"); + logger.fine("Manifest is not present"); return Optional.of(new ArrayList<>()); } diff --git a/src/main/resources/log4j2.json b/src/main/resources/log4j2.json deleted file mode 100644 index e168a3bff6f..00000000000 --- a/src/main/resources/log4j2.json +++ /dev/null @@ -1,72 +0,0 @@ -{ - "configuration": { - "status": "info", - "properties": { - "property": { - "name": "filename", - "value": "addressbook" - } - }, - "ThresholdFilter": { - "level": "trace" - }, - "appenders": { - "Console": { - "name": "STDOUT", - "PatternLayout": { - "pattern": "%-35style{[%thread]}{magenta}| %highlight{%level{length=1}} %style{%d{HH:mm:ss.SSS}}{yellow} %30.-30style{(%logger{0})}{gray} %message%n" - } - }, - "RollingFile": [ - { - "name": "RollingFile", - "fileName": "logs/${filename}.log", - "filePattern": "logs/${filename}-%i.log", - "PatternLayout": { - "pattern": "[%thread] %level{length=1} %d{HH:mm:ss.SSS} (%logger{0}) %message%n" - }, - "Policies": { - "SizeBasedTriggeringPolicy": { - "size": "5 MB" - } - }, - "DefaultRolloverStrategy": { - "max": "5" - } - }, - { - "name": "RollingCsvFile", - "fileName": "logs/${filename}.csv", - "filePattern": "logs/${filename}-%i.csv", - "PatternLayout": { - "pattern": "%thread;%level;%d{HH:mm:ss.SSS}{GMT+8};%logger{0};%message%n" - }, - "Policies": { - "SizeBasedTriggeringPolicy": { - "size": "5 MB" - } - }, - "DefaultRolloverStrategy": { - "max": "5" - } - } - ] - }, - "loggers": { - "root": { - "level": "info", - "AppenderRef": [ - { - "ref": "STDOUT" - }, - { - "ref": "RollingFile" - }, - { - "ref": "RollingCsvFile" - } - ] - } - } - } -} \ No newline at end of file diff --git a/src/test/java/guitests/guihandles/GuiHandle.java b/src/test/java/guitests/guihandles/GuiHandle.java index 05eef7c4963..8b5144feeb6 100644 --- a/src/test/java/guitests/guihandles/GuiHandle.java +++ b/src/test/java/guitests/guihandles/GuiHandle.java @@ -9,10 +9,10 @@ import javafx.stage.Window; import seedu.address.TestApp; import seedu.address.model.person.Person; -import seedu.address.util.AppLogger; import seedu.address.util.LoggerManager; import java.lang.reflect.Constructor; +import java.util.logging.Logger; /** * Base class for all GUI Handles used in testing. @@ -22,7 +22,7 @@ public class GuiHandle { protected final Stage primaryStage; protected final String stageTitle; - private final AppLogger logger = LoggerManager.getLogger(this.getClass()); + private final Logger logger = LoggerManager.getLogger(this.getClass()); public GuiHandle(GuiRobot guiRobot, Stage primaryStage, String stageTitle) { this.guiRobot = guiRobot; @@ -49,19 +49,19 @@ public void write(String textToWrite) { } public void focusOnWindow(String stageTitle) { - logger.info("Focusing {}", stageTitle); + logger.info("Focusing " + stageTitle); java.util.Optional window = guiRobot.listTargetWindows() .stream() .filter(w -> w instanceof Stage && ((Stage) w).getTitle().equals(stageTitle)).findAny(); if (!window.isPresent()) { - logger.warn("Can't find stage {}, Therefore, aborting focusing", stageTitle); + logger.warning("Can't find stage " + stageTitle + ", Therefore, aborting focusing"); return; } guiRobot.targetWindow(window.get()); guiRobot.interact(() -> window.get().requestFocus()); - logger.info("Finishing focus {}", stageTitle); + logger.info("Finishing focus " + stageTitle); } protected Node getNode(String query) {