Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Reduce duplication between ConfigUtil and JsonUserPrefsStorage #95 #147

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/java/seedu/address/MainApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
45 changes: 3 additions & 42 deletions src/main/java/seedu/address/commons/util/ConfigUtil.java
Original file line number Diff line number Diff line change
@@ -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<Config> 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);
}

}
11 changes: 2 additions & 9 deletions src/main/java/seedu/address/commons/util/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -84,12 +85,4 @@ public static String getPath(String pathWithForwardSlash) {
return pathWithForwardSlash.replace("/", File.separator);
}

public static <T> void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException {
FileUtil.writeToFile(jsonFile, JsonUtil.toJsonString(objectToSerialize));
}

public static <T> T deserializeObjectFromJsonFile(File jsonFile, Class<T> classOfObjectToDeserialize)
throws IOException {
return JsonUtil.fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize);
}
}
62 changes: 62 additions & 0 deletions src/main/java/seedu/address/commons/util/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 <T> void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException {
FileUtil.writeToFile(jsonFile, toJsonString(objectToSerialize));
}

static <T> T deserializeObjectFromJsonFile(File jsonFile, Class<T> classOfObjectToDeserialize)
throws IOException {
return fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize);
}
Copy link
Contributor

@damithc damithc Oct 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these two methods be private? or at least package-private?
If they are used only once each, you can inline them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't be private as JsonUtilTest uses them.


/**
* 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 <T> Optional<T> readJsonFile(
String filePath, Class<T> 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 <T> 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<Level> {

protected LevelDeserializer(Class<?> vc) {
Expand Down
47 changes: 7 additions & 40 deletions src/main/java/seedu/address/storage/JsonUserPrefsStorage.java
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -28,46 +23,18 @@ public Optional<UserPrefs> 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<UserPrefs> readUserPrefs(String prefsFilePath) throws DataConversionException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header comment got deleted?

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method doesn't have a header comment, then the parent method should have one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the comments are in the interface UserPrefsStorage for that method.

public void saveUserPrefs(UserPrefs userPrefs) throws IOException {
JsonUtil.saveJsonFile(userPrefs, filePath);
}

}
1 change: 0 additions & 1 deletion src/main/java/seedu/address/storage/Storage.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion src/main/java/seedu/address/storage/StorageManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion src/main/java/seedu/address/storage/XmlAdaptedTag.java
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
16 changes: 9 additions & 7 deletions src/test/java/seedu/address/commons/util/ConfigUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
26 changes: 0 additions & 26 deletions src/test/java/seedu/address/commons/util/FileUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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());
}
}
Loading