From 79e960c244ff119a277bda981b5425be260df0ea Mon Sep 17 00:00:00 2001 From: Joshua Lee Date: Thu, 6 Oct 2016 13:36:16 +0800 Subject: [PATCH] Reduced duplication between ConfigUtil and JsonUserPrefsStorage --- src/main/java/seedu/address/MainApp.java | 3 +- .../address/commons/util/ConfigUtil.java | 45 +------------- .../seedu/address/commons/util/FileUtil.java | 11 +--- .../seedu/address/commons/util/JsonUtil.java | 62 +++++++++++++++++++ .../address/storage/JsonUserPrefsStorage.java | 47 +++----------- .../java/seedu/address/storage/Storage.java | 1 - .../seedu/address/storage/StorageManager.java | 1 - .../seedu/address/storage/XmlAdaptedTag.java | 1 - ...atConfig.json => NotJsonFormatConfig.json} | 0 .../address/commons/util/ConfigUtilTest.java | 16 ++--- .../address/commons/util/FileUtilTest.java | 26 -------- .../address/commons/util/JsonUtilTest.java | 33 ++++++++++ .../address/commons/util/StringUtilTest.java | 1 - .../storage/JsonUserPrefsStorageTest.java | 2 +- 14 files changed, 118 insertions(+), 131 deletions(-) rename src/test/data/ConfigUtilTest/{NotJasonFormatConfig.json => NotJsonFormatConfig.json} (100%) diff --git a/src/main/java/seedu/address/MainApp.java b/src/main/java/seedu/address/MainApp.java index 36dc72a74b7a..6a991eb7fdfb 100644 --- a/src/main/java/seedu/address/MainApp.java +++ b/src/main/java/seedu/address/MainApp.java @@ -10,17 +10,16 @@ import seedu.address.commons.core.Version; import seedu.address.commons.events.ui.ExitAppRequestEvent; import seedu.address.commons.exceptions.DataConversionException; +import seedu.address.commons.util.ConfigUtil; import seedu.address.commons.util.StringUtil; import seedu.address.logic.Logic; import seedu.address.logic.LogicManager; import seedu.address.model.*; -import seedu.address.commons.util.ConfigUtil; import seedu.address.storage.Storage; import seedu.address.storage.StorageManager; import seedu.address.ui.Ui; import seedu.address.ui.UiManager; -import java.io.FileNotFoundException; import java.io.IOException; import java.util.Map; import java.util.Optional; diff --git a/src/main/java/seedu/address/commons/util/ConfigUtil.java b/src/main/java/seedu/address/commons/util/ConfigUtil.java index af42e03df06c..10864187a3c8 100644 --- a/src/main/java/seedu/address/commons/util/ConfigUtil.java +++ b/src/main/java/seedu/address/commons/util/ConfigUtil.java @@ -1,62 +1,23 @@ package seedu.address.commons.util; import seedu.address.commons.core.Config; -import seedu.address.commons.core.LogsCenter; import seedu.address.commons.exceptions.DataConversionException; +import seedu.address.commons.util.JsonUtil; -import java.io.File; import java.io.IOException; import java.util.Optional; -import java.util.logging.Logger; /** * A class for accessing the Config File. */ public class ConfigUtil { - private static final Logger logger = LogsCenter.getLogger(ConfigUtil.class); - - /** - * Returns the Config object from the given file or {@code Optional.empty()} object if the file is not found. - * If any values are missing from the file, default values will be used, as long as the file is a valid json file. - * @param configFilePath cannot be null. - * @throws DataConversionException if the file format is not as expected. - */ public static Optional readConfig(String configFilePath) throws DataConversionException { - - assert configFilePath != null; - - File configFile = new File(configFilePath); - - if (!configFile.exists()) { - logger.info("Config file " + configFile + " not found"); - return Optional.empty(); - } - - Config config; - - try { - config = FileUtil.deserializeObjectFromJsonFile(configFile, Config.class); - } catch (IOException e) { - logger.warning("Error reading from config file " + configFile + ": " + e); - throw new DataConversionException(e); - } - - return Optional.of(config); + return JsonUtil.readJsonFile(configFilePath, Config.class); } - /** - * Saves the Config object to the specified file. - * Overwrites existing file if it exists, creates a new file if it doesn't. - * @param config cannot be null - * @param configFilePath cannot be null - * @throws IOException if there was an error during writing to the file - */ public static void saveConfig(Config config, String configFilePath) throws IOException { - assert config != null; - assert configFilePath != null; - - FileUtil.serializeObjectToJsonFile(new File(configFilePath), config); + JsonUtil.saveJsonFile(config, configFilePath); } } diff --git a/src/main/java/seedu/address/commons/util/FileUtil.java b/src/main/java/seedu/address/commons/util/FileUtil.java index ca8221250de4..3e1eb4d9ff6e 100644 --- a/src/main/java/seedu/address/commons/util/FileUtil.java +++ b/src/main/java/seedu/address/commons/util/FileUtil.java @@ -5,9 +5,10 @@ import java.nio.file.Files; /** - * Writes and reads file + * Writes and reads files */ public class FileUtil { + private static final String CHARSET = "UTF-8"; public static boolean isFileExists(File file) { @@ -84,12 +85,4 @@ public static String getPath(String pathWithForwardSlash) { return pathWithForwardSlash.replace("/", File.separator); } - public static void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException { - FileUtil.writeToFile(jsonFile, JsonUtil.toJsonString(objectToSerialize)); - } - - public static T deserializeObjectFromJsonFile(File jsonFile, Class classOfObjectToDeserialize) - throws IOException { - return JsonUtil.fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize); - } } diff --git a/src/main/java/seedu/address/commons/util/JsonUtil.java b/src/main/java/seedu/address/commons/util/JsonUtil.java index 80b67de5b7e8..b1c3ed3dbc93 100644 --- a/src/main/java/seedu/address/commons/util/JsonUtil.java +++ b/src/main/java/seedu/address/commons/util/JsonUtil.java @@ -10,14 +10,76 @@ import com.fasterxml.jackson.databind.deser.std.FromStringDeserializer; import com.fasterxml.jackson.databind.module.SimpleModule; import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; +import seedu.address.commons.core.LogsCenter; +import seedu.address.commons.exceptions.DataConversionException; +import java.io.File; import java.io.IOException; +import java.util.Optional; import java.util.logging.Level; +import java.util.logging.Logger; /** * Converts a Java object instance to JSON and vice versa */ public class JsonUtil { + + private static final Logger logger = LogsCenter.getLogger(JsonUtil.class); + + static void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException { + FileUtil.writeToFile(jsonFile, toJsonString(objectToSerialize)); + } + + static T deserializeObjectFromJsonFile(File jsonFile, Class classOfObjectToDeserialize) + throws IOException { + return fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize); + } + + /** + * Returns the Json object from the given file or {@code Optional.empty()} object if the file is not found. + * If any values are missing from the file, default values will be used, as long as the file is a valid json file. + * @param filePath cannot be null. + * @param classOfObjectToDeserialize Json file has to correspond to the structure in the class given here. + * @throws DataConversionException if the file format is not as expected. + */ + public static Optional readJsonFile( + String filePath, Class classOfObjectToDeserialize) throws DataConversionException { + + assert filePath != null; + + File file = new File(filePath); + + if (!file.exists()) { + logger.info("Json file " + file + " not found"); + return Optional.empty(); + } + + T jsonFile; + + try { + jsonFile = deserializeObjectFromJsonFile(file, classOfObjectToDeserialize); + } catch (IOException e) { + logger.warning("Error reading from jsonFile file " + file + ": " + e); + throw new DataConversionException(e); + } + + return Optional.of(jsonFile); + } + + /** + * Saves the Json object to the specified file. + * Overwrites existing file if it exists, creates a new file if it doesn't. + * @param jsonFile cannot be null + * @param filePath cannot be null + * @throws IOException if there was an error during writing to the file + */ + public static void saveJsonFile(T jsonFile, String filePath) throws IOException { + assert jsonFile != null; + assert filePath != null; + + serializeObjectToJsonFile(new File(filePath), jsonFile); + } + private static class LevelDeserializer extends FromStringDeserializer { protected LevelDeserializer(Class vc) { diff --git a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java index 1efa8288e4f6..10905d644168 100644 --- a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java +++ b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java @@ -1,21 +1,16 @@ package seedu.address.storage; -import seedu.address.commons.core.LogsCenter; import seedu.address.commons.exceptions.DataConversionException; -import seedu.address.commons.util.FileUtil; +import seedu.address.commons.util.JsonUtil; import seedu.address.model.UserPrefs; -import java.io.File; import java.io.IOException; import java.util.Optional; -import java.util.logging.Logger; /** * A class to access UserPrefs stored in the hard disk as a json file */ -public class JsonUserPrefsStorage implements UserPrefsStorage{ - - private static final Logger logger = LogsCenter.getLogger(JsonUserPrefsStorage.class); +public class JsonUserPrefsStorage implements UserPrefsStorage { private String filePath; @@ -28,46 +23,18 @@ public Optional readUserPrefs() throws DataConversionException, IOExc return readUserPrefs(filePath); } - @Override - public void saveUserPrefs(UserPrefs userPrefs) throws IOException { - saveUserPrefs(userPrefs, filePath); - } - /** * Similar to {@link #readUserPrefs()} * @param prefsFilePath location of the data. Cannot be null. * @throws DataConversionException if the file format is not as expected. */ public Optional readUserPrefs(String prefsFilePath) throws DataConversionException { - assert prefsFilePath != null; - - File prefsFile = new File(prefsFilePath); - - if (!prefsFile.exists()) { - logger.info("Prefs file " + prefsFile + " not found"); - return Optional.empty(); - } - - UserPrefs prefs; - - try { - prefs = FileUtil.deserializeObjectFromJsonFile(prefsFile, UserPrefs.class); - } catch (IOException e) { - logger.warning("Error reading from prefs file " + prefsFile + ": " + e); - throw new DataConversionException(e); - } - - return Optional.of(prefs); + return JsonUtil.readJsonFile(prefsFilePath, UserPrefs.class); } - /** - * Similar to {@link #saveUserPrefs(UserPrefs)} - * @param prefsFilePath location of the data. Cannot be null. - */ - public void saveUserPrefs(UserPrefs userPrefs, String prefsFilePath) throws IOException { - assert userPrefs != null; - assert prefsFilePath != null; - - FileUtil.serializeObjectToJsonFile(new File(prefsFilePath), userPrefs); + @Override + public void saveUserPrefs(UserPrefs userPrefs) throws IOException { + JsonUtil.saveJsonFile(userPrefs, filePath); } + } diff --git a/src/main/java/seedu/address/storage/Storage.java b/src/main/java/seedu/address/storage/Storage.java index 91002a8a821a..1d7f3113e95b 100644 --- a/src/main/java/seedu/address/storage/Storage.java +++ b/src/main/java/seedu/address/storage/Storage.java @@ -6,7 +6,6 @@ import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.UserPrefs; -import java.io.FileNotFoundException; import java.io.IOException; import java.util.Optional; diff --git a/src/main/java/seedu/address/storage/StorageManager.java b/src/main/java/seedu/address/storage/StorageManager.java index ba1f72f15c27..ad09c2defe89 100644 --- a/src/main/java/seedu/address/storage/StorageManager.java +++ b/src/main/java/seedu/address/storage/StorageManager.java @@ -9,7 +9,6 @@ import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.UserPrefs; -import java.io.FileNotFoundException; import java.io.IOException; import java.util.Optional; import java.util.logging.Logger; diff --git a/src/main/java/seedu/address/storage/XmlAdaptedTag.java b/src/main/java/seedu/address/storage/XmlAdaptedTag.java index b9723fafbc67..de199188c7e1 100644 --- a/src/main/java/seedu/address/storage/XmlAdaptedTag.java +++ b/src/main/java/seedu/address/storage/XmlAdaptedTag.java @@ -1,6 +1,5 @@ package seedu.address.storage; -import seedu.address.commons.util.CollectionUtil; import seedu.address.commons.exceptions.IllegalValueException; import seedu.address.model.tag.Tag; diff --git a/src/test/data/ConfigUtilTest/NotJasonFormatConfig.json b/src/test/data/ConfigUtilTest/NotJsonFormatConfig.json similarity index 100% rename from src/test/data/ConfigUtilTest/NotJasonFormatConfig.json rename to src/test/data/ConfigUtilTest/NotJsonFormatConfig.json diff --git a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java index 6699343c4a82..1d4d35d0f864 100644 --- a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java +++ b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java @@ -7,6 +7,8 @@ import org.junit.rules.TemporaryFolder; import seedu.address.commons.core.Config; import seedu.address.commons.exceptions.DataConversionException; +import seedu.address.commons.util.ConfigUtil; +import seedu.address.commons.util.FileUtil; import java.io.File; import java.io.IOException; @@ -38,10 +40,10 @@ public void read_missingFile_emptyResult() throws DataConversionException { } @Test - public void read_notJasonFormat_exceptionThrown() throws DataConversionException { + public void read_notJsonFormat_exceptionThrown() throws DataConversionException { thrown.expect(DataConversionException.class); - read("NotJasonFormatConfig.json"); + read("NotJsonFormatConfig.json"); /* IMPORTANT: Any code below an exception-throwing line (like the one above) will be ignored. * That means you should not have more than one exception test in one method @@ -103,18 +105,18 @@ public void saveConfig_allInOrder_success() throws DataConversionException, IOEx Config original = getTypicalConfig(); String configFilePath = testFolder.getRoot() + File.separator + "TempConfig.json"; - ConfigUtil configStorage = new ConfigUtil(); + ConfigUtil configUtil = new ConfigUtil(); //Try writing when the file doesn't exist - configStorage.saveConfig(original, configFilePath); - Config readBack = configStorage.readConfig(configFilePath).get(); + configUtil.saveConfig(original, configFilePath); + Config readBack = configUtil.readConfig(configFilePath).get(); assertEquals(original, readBack); //Try saving when the file exists original.setAppTitle("Updated Title"); original.setLogLevel(Level.FINE); - configStorage.saveConfig(original, configFilePath); - readBack = configStorage.readConfig(configFilePath).get(); + configUtil.saveConfig(original, configFilePath); + readBack = configUtil.readConfig(configFilePath).get(); assertEquals(original, readBack); } diff --git a/src/test/java/seedu/address/commons/util/FileUtilTest.java b/src/test/java/seedu/address/commons/util/FileUtilTest.java index 8de2621799cf..564a35ada340 100644 --- a/src/test/java/seedu/address/commons/util/FileUtilTest.java +++ b/src/test/java/seedu/address/commons/util/FileUtilTest.java @@ -4,17 +4,12 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import seedu.address.testutil.SerializableTestClass; -import seedu.address.testutil.TestUtil; import java.io.File; -import java.io.IOException; import static org.junit.Assert.assertEquals; public class FileUtilTest { - private static final File SERIALIZATION_FILE = new File(TestUtil.getFilePathInSandboxFolder("serialize.json")); - @Rule public ExpectedException thrown = ExpectedException.none(); @@ -34,25 +29,4 @@ public void getPath(){ FileUtil.getPath("folder"); } - @Test - public void serializeObjectToJsonFile_noExceptionThrown() throws IOException { - SerializableTestClass serializableTestClass = new SerializableTestClass(); - serializableTestClass.setTestValues(); - - FileUtil.serializeObjectToJsonFile(SERIALIZATION_FILE, serializableTestClass); - - assertEquals(FileUtil.readFromFile(SERIALIZATION_FILE), SerializableTestClass.JSON_STRING_REPRESENTATION); - } - - @Test - public void deserializeObjectFromJsonFile_noExceptionThrown() throws IOException { - FileUtil.writeToFile(SERIALIZATION_FILE, SerializableTestClass.JSON_STRING_REPRESENTATION); - - SerializableTestClass serializableTestClass = FileUtil - .deserializeObjectFromJsonFile(SERIALIZATION_FILE, SerializableTestClass.class); - - assertEquals(serializableTestClass.getName(), SerializableTestClass.getNameTestValue()); - assertEquals(serializableTestClass.getListOfLocalDateTimes(), SerializableTestClass.getListTestValues()); - assertEquals(serializableTestClass.getMapOfIntegerToString(), SerializableTestClass.getHashMapTestValues()); - } } diff --git a/src/test/java/seedu/address/commons/util/JsonUtilTest.java b/src/test/java/seedu/address/commons/util/JsonUtilTest.java index fc3902188807..9607283a41c2 100644 --- a/src/test/java/seedu/address/commons/util/JsonUtilTest.java +++ b/src/test/java/seedu/address/commons/util/JsonUtilTest.java @@ -1,10 +1,43 @@ package seedu.address.commons.util; +import org.junit.Test; +import seedu.address.testutil.SerializableTestClass; +import seedu.address.testutil.TestUtil; + +import java.io.File; +import java.io.IOException; + +import static org.junit.Assert.assertEquals; + /** * Tests JSON Read and Write */ public class JsonUtilTest { + private static final File SERIALIZATION_FILE = new File(TestUtil.getFilePathInSandboxFolder("serialize.json")); + + @Test + public void serializeObjectToJsonFile_noExceptionThrown() throws IOException { + SerializableTestClass serializableTestClass = new SerializableTestClass(); + serializableTestClass.setTestValues(); + + JsonUtil.serializeObjectToJsonFile(SERIALIZATION_FILE, serializableTestClass); + + assertEquals(FileUtil.readFromFile(SERIALIZATION_FILE), SerializableTestClass.JSON_STRING_REPRESENTATION); + } + + @Test + public void deserializeObjectFromJsonFile_noExceptionThrown() throws IOException { + FileUtil.writeToFile(SERIALIZATION_FILE, SerializableTestClass.JSON_STRING_REPRESENTATION); + + SerializableTestClass serializableTestClass = JsonUtil + .deserializeObjectFromJsonFile(SERIALIZATION_FILE, SerializableTestClass.class); + + assertEquals(serializableTestClass.getName(), SerializableTestClass.getNameTestValue()); + assertEquals(serializableTestClass.getListOfLocalDateTimes(), SerializableTestClass.getListTestValues()); + assertEquals(serializableTestClass.getMapOfIntegerToString(), SerializableTestClass.getHashMapTestValues()); + } + //TODO: @Test jsonUtil_readJsonStringToObjectInstance_correctObject() //TODO: @Test jsonUtil_writeThenReadObjectToJson_correctObject() diff --git a/src/test/java/seedu/address/commons/util/StringUtilTest.java b/src/test/java/seedu/address/commons/util/StringUtilTest.java index 194dd71d2c3f..7c53c58e462a 100644 --- a/src/test/java/seedu/address/commons/util/StringUtilTest.java +++ b/src/test/java/seedu/address/commons/util/StringUtilTest.java @@ -8,7 +8,6 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; diff --git a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java index 4e87203611be..c0d45080ef4f 100644 --- a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java +++ b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java @@ -43,7 +43,7 @@ public void readUserPrefs_missingFile_emptyResult() throws DataConversionExcepti } @Test - public void readUserPrefs_notJasonFormat_exceptionThrown() throws DataConversionException { + public void readUserPrefs_notJsonFormat_exceptionThrown() throws DataConversionException { thrown.expect(DataConversionException.class); readUserPrefs("NotJsonFormatUserPrefs.json");