From 75fdb9bb6a07290ab5d4b277012b2d2f4eb6ecfe Mon Sep 17 00:00:00 2001 From: Vivek Date: Sat, 21 Apr 2018 00:15:41 +0800 Subject: [PATCH 1/6] Replace use of java.io.File with java.nio.file.Path There are many usages of java.io.File throughout the codebase. Since Path is meant to replace File as mentioned in the Oracle Documentation on Legacy File I/O Code, let's convert all usages of File in the codebase to Path. Oracle Documentation on Legacy File I/O Code: https://docs.oracle.com/javase/tutorial/essential/io/legacy.html --- .../seedu/address/commons/util/FileUtil.java | 35 ++++++++++--------- .../seedu/address/commons/util/JsonUtil.java | 14 ++++---- .../seedu/address/commons/util/XmlUtil.java | 17 ++++----- .../storage/XmlAddressBookStorage.java | 12 ++++--- .../seedu/address/storage/XmlFileStorage.java | 6 ++-- src/test/java/seedu/address/TestApp.java | 5 +-- .../address/commons/util/JsonUtilTest.java | 5 +-- .../address/commons/util/XmlUtilTest.java | 19 +++++----- .../XmlSerializableAddressBookTest.java | 7 ++-- .../java/seedu/address/testutil/TestUtil.java | 4 +-- .../seedu/address/ui/PersonListPanelTest.java | 11 +++--- 11 files changed, 74 insertions(+), 61 deletions(-) diff --git a/src/main/java/seedu/address/commons/util/FileUtil.java b/src/main/java/seedu/address/commons/util/FileUtil.java index 73575030d7dd..4048c8feb800 100644 --- a/src/main/java/seedu/address/commons/util/FileUtil.java +++ b/src/main/java/seedu/address/commons/util/FileUtil.java @@ -5,6 +5,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; /** * Writes and reads files @@ -13,15 +14,15 @@ public class FileUtil { private static final String CHARSET = "UTF-8"; - public static boolean isFileExists(File file) { - return file.exists() && file.isFile(); + public static boolean isFileExists(Path file) { + return Files.exists(file) && Files.isRegularFile(file); } /** * Creates a file if it does not exist along with its missing parent directories. * @throws IOException if the file or directory cannot be created. */ - public static void createIfMissing(File file) throws IOException { + public static void createIfMissing(Path file) throws IOException { if (!isFileExists(file)) { createFile(file); } @@ -32,14 +33,15 @@ public static void createIfMissing(File file) throws IOException { * * @return true if file is created, false if file already exists */ - public static boolean createFile(File file) throws IOException { - if (file.exists()) { + public static boolean createFile(Path file) throws IOException { + if (Files.exists(file)) { return false; } createParentDirsOfFile(file); - return file.createNewFile(); + Files.createFile(file); + return true; } /** @@ -48,36 +50,37 @@ public static boolean createFile(File file) throws IOException { * @param dir the directory to be created; assumed not null * @throws IOException if the directory or a parent directory cannot be created */ - public static void createDirs(File dir) throws IOException { - if (!dir.exists() && !dir.mkdirs()) { - throw new IOException("Failed to make directories of " + dir.getName()); + public static void createDirs(Path dir) throws IOException { + if (Files.exists(dir)) { + return; } + Files.createDirectories(dir); } /** * Creates parent directories of file if it has a parent directory */ - public static void createParentDirsOfFile(File file) throws IOException { - File parentDir = file.getParentFile(); + public static void createParentDirsOfFile(Path file) throws IOException { + Path parentDir = file.getParent(); if (parentDir != null) { - createDirs(parentDir); + Files.createDirectories(parentDir); } } /** * Assumes file exists */ - public static String readFromFile(File file) throws IOException { - return new String(Files.readAllBytes(file.toPath()), CHARSET); + public static String readFromFile(Path file) throws IOException { + return new String(Files.readAllBytes(file), CHARSET); } /** * Writes given string to a file. * Will create the file if it does not exist yet. */ - public static void writeToFile(File file, String content) throws IOException { - Files.write(file.toPath(), content.getBytes(CHARSET)); + public static void writeToFile(Path file, String content) throws IOException { + Files.write(file, content.getBytes(CHARSET)); } /** diff --git a/src/main/java/seedu/address/commons/util/JsonUtil.java b/src/main/java/seedu/address/commons/util/JsonUtil.java index 1c629b0d4a16..e1dc6cb25bc9 100644 --- a/src/main/java/seedu/address/commons/util/JsonUtil.java +++ b/src/main/java/seedu/address/commons/util/JsonUtil.java @@ -2,8 +2,10 @@ import static java.util.Objects.requireNonNull; -import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -38,11 +40,11 @@ public class JsonUtil { .addSerializer(Level.class, new ToStringSerializer()) .addDeserializer(Level.class, new LevelDeserializer(Level.class))); - static void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException { + static void serializeObjectToJsonFile(Path jsonFile, T objectToSerialize) throws IOException { FileUtil.writeToFile(jsonFile, toJsonString(objectToSerialize)); } - static T deserializeObjectFromJsonFile(File jsonFile, Class classOfObjectToDeserialize) + static T deserializeObjectFromJsonFile(Path jsonFile, Class classOfObjectToDeserialize) throws IOException { return fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize); } @@ -57,9 +59,9 @@ static T deserializeObjectFromJsonFile(File jsonFile, Class classOfObject public static Optional readJsonFile( String filePath, Class classOfObjectToDeserialize) throws DataConversionException { requireNonNull(filePath); - File file = new File(filePath); + Path file = Paths.get(filePath); - if (!file.exists()) { + if (!Files.exists(file)) { logger.info("Json file " + file + " not found"); return Optional.empty(); } @@ -87,7 +89,7 @@ public static void saveJsonFile(T jsonFile, String filePath) throws IOExcept requireNonNull(filePath); requireNonNull(jsonFile); - serializeObjectToJsonFile(new File(filePath), jsonFile); + serializeObjectToJsonFile(Paths.get(filePath), jsonFile); } diff --git a/src/main/java/seedu/address/commons/util/XmlUtil.java b/src/main/java/seedu/address/commons/util/XmlUtil.java index 5f61738627cc..a78cd15b7f0c 100644 --- a/src/main/java/seedu/address/commons/util/XmlUtil.java +++ b/src/main/java/seedu/address/commons/util/XmlUtil.java @@ -2,8 +2,9 @@ import static java.util.Objects.requireNonNull; -import java.io.File; import java.io.FileNotFoundException; +import java.nio.file.Files; +import java.nio.file.Path; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; @@ -26,20 +27,20 @@ public class XmlUtil { * @throws JAXBException Thrown if the file is empty or does not have the correct format. */ @SuppressWarnings("unchecked") - public static T getDataFromFile(File file, Class classToConvert) + public static T getDataFromFile(Path file, Class classToConvert) throws FileNotFoundException, JAXBException { requireNonNull(file); requireNonNull(classToConvert); if (!FileUtil.isFileExists(file)) { - throw new FileNotFoundException("File not found : " + file.getAbsolutePath()); + throw new FileNotFoundException("File not found : " + file.toAbsolutePath()); } JAXBContext context = JAXBContext.newInstance(classToConvert); Unmarshaller um = context.createUnmarshaller(); - return ((T) um.unmarshal(file)); + return ((T) um.unmarshal(file.toFile())); } /** @@ -51,20 +52,20 @@ public static T getDataFromFile(File file, Class classToConvert) * @throws JAXBException Thrown if there is an error during converting the data * into xml and writing to the file. */ - public static void saveDataToFile(File file, T data) throws FileNotFoundException, JAXBException { + public static void saveDataToFile(Path file, T data) throws FileNotFoundException, JAXBException { requireNonNull(file); requireNonNull(data); - if (!file.exists()) { - throw new FileNotFoundException("File not found : " + file.getAbsolutePath()); + if (!Files.exists(file)) { + throw new FileNotFoundException("File not found : " + file.toAbsolutePath()); } JAXBContext context = JAXBContext.newInstance(data.getClass()); Marshaller m = context.createMarshaller(); m.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, true); - m.marshal(data, file); + m.marshal(data, file.toFile()); } } diff --git a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java b/src/main/java/seedu/address/storage/XmlAddressBookStorage.java index c77ebe67435c..6846c7a962da 100644 --- a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java +++ b/src/main/java/seedu/address/storage/XmlAddressBookStorage.java @@ -2,9 +2,11 @@ import static java.util.Objects.requireNonNull; -import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Optional; import java.util.logging.Logger; @@ -45,14 +47,14 @@ public Optional readAddressBook(String filePath) throws Dat FileNotFoundException { requireNonNull(filePath); - File addressBookFile = new File(filePath); + Path addressBookFile = Paths.get(filePath); - if (!addressBookFile.exists()) { + if (!Files.exists(addressBookFile)) { logger.info("AddressBook file " + addressBookFile + " not found"); return Optional.empty(); } - XmlSerializableAddressBook xmlAddressBook = XmlFileStorage.loadDataFromSaveFile(new File(filePath)); + XmlSerializableAddressBook xmlAddressBook = XmlFileStorage.loadDataFromSaveFile(Paths.get(filePath)); try { return Optional.of(xmlAddressBook.toModelType()); } catch (IllegalValueException ive) { @@ -74,7 +76,7 @@ public void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) th requireNonNull(addressBook); requireNonNull(filePath); - File file = new File(filePath); + Path file = Paths.get(filePath); FileUtil.createIfMissing(file); XmlFileStorage.saveDataToFile(file, new XmlSerializableAddressBook(addressBook)); } diff --git a/src/main/java/seedu/address/storage/XmlFileStorage.java b/src/main/java/seedu/address/storage/XmlFileStorage.java index 289fcb63038e..abc51e66197f 100644 --- a/src/main/java/seedu/address/storage/XmlFileStorage.java +++ b/src/main/java/seedu/address/storage/XmlFileStorage.java @@ -1,7 +1,7 @@ package seedu.address.storage; -import java.io.File; import java.io.FileNotFoundException; +import java.nio.file.Path; import javax.xml.bind.JAXBException; @@ -15,7 +15,7 @@ public class XmlFileStorage { /** * Saves the given addressbook data to the specified file. */ - public static void saveDataToFile(File file, XmlSerializableAddressBook addressBook) + public static void saveDataToFile(Path file, XmlSerializableAddressBook addressBook) throws FileNotFoundException { try { XmlUtil.saveDataToFile(file, addressBook); @@ -27,7 +27,7 @@ public static void saveDataToFile(File file, XmlSerializableAddressBook addressB /** * Returns address book in the file or an empty address book */ - public static XmlSerializableAddressBook loadDataFromSaveFile(File file) throws DataConversionException, + public static XmlSerializableAddressBook loadDataFromSaveFile(Path file) throws DataConversionException, FileNotFoundException { try { return XmlUtil.getDataFromFile(file, XmlSerializableAddressBook.class); diff --git a/src/test/java/seedu/address/TestApp.java b/src/test/java/seedu/address/TestApp.java index 5b32c8faec9d..6e33de7f2112 100644 --- a/src/test/java/seedu/address/TestApp.java +++ b/src/test/java/seedu/address/TestApp.java @@ -1,7 +1,8 @@ package seedu.address; -import java.io.File; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.function.Supplier; import javafx.stage.Screen; @@ -113,7 +114,7 @@ public static void main(String[] args) { */ private void createDataFileWithData(T data, String filePath) { try { - File saveFileForTesting = new File(filePath); + Path saveFileForTesting = Paths.get(filePath); FileUtil.createIfMissing(saveFileForTesting); XmlUtil.saveDataToFile(saveFileForTesting, data); } catch (Exception e) { diff --git a/src/test/java/seedu/address/commons/util/JsonUtilTest.java b/src/test/java/seedu/address/commons/util/JsonUtilTest.java index 8da024a76667..778cc7deabd3 100644 --- a/src/test/java/seedu/address/commons/util/JsonUtilTest.java +++ b/src/test/java/seedu/address/commons/util/JsonUtilTest.java @@ -2,8 +2,9 @@ import static org.junit.Assert.assertEquals; -import java.io.File; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Test; @@ -15,7 +16,7 @@ */ public class JsonUtilTest { - private static final File SERIALIZATION_FILE = new File(TestUtil.getFilePathInSandboxFolder("serialize.json")); + private static final Path SERIALIZATION_FILE = Paths.get(TestUtil.getFilePathInSandboxFolder("serialize.json")); @Test public void serializeObjectToJsonFile_noExceptionThrown() throws IOException { diff --git a/src/test/java/seedu/address/commons/util/XmlUtilTest.java b/src/test/java/seedu/address/commons/util/XmlUtilTest.java index 6c9f7fef313e..74d1e8a7eef9 100644 --- a/src/test/java/seedu/address/commons/util/XmlUtilTest.java +++ b/src/test/java/seedu/address/commons/util/XmlUtilTest.java @@ -2,8 +2,9 @@ import static org.junit.Assert.assertEquals; -import java.io.File; import java.io.FileNotFoundException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Collections; import java.util.List; @@ -25,13 +26,13 @@ public class XmlUtilTest { private static final String TEST_DATA_FOLDER = FileUtil.getPath("src/test/data/XmlUtilTest/"); - private static final File EMPTY_FILE = new File(TEST_DATA_FOLDER + "empty.xml"); - private static final File MISSING_FILE = new File(TEST_DATA_FOLDER + "missing.xml"); - private static final File VALID_FILE = new File(TEST_DATA_FOLDER + "validAddressBook.xml"); - private static final File MISSING_PERSON_FIELD_FILE = new File(TEST_DATA_FOLDER + "missingPersonField.xml"); - private static final File INVALID_PERSON_FIELD_FILE = new File(TEST_DATA_FOLDER + "invalidPersonField.xml"); - private static final File VALID_PERSON_FILE = new File(TEST_DATA_FOLDER + "validPerson.xml"); - private static final File TEMP_FILE = new File(TestUtil.getFilePathInSandboxFolder("tempAddressBook.xml")); + private static final Path EMPTY_FILE = Paths.get(TEST_DATA_FOLDER + "empty.xml"); + private static final Path MISSING_FILE = Paths.get(TEST_DATA_FOLDER + "missing.xml"); + private static final Path VALID_FILE = Paths.get(TEST_DATA_FOLDER + "validAddressBook.xml"); + private static final Path MISSING_PERSON_FIELD_FILE = Paths.get(TEST_DATA_FOLDER + "missingPersonField.xml"); + private static final Path INVALID_PERSON_FIELD_FILE = Paths.get(TEST_DATA_FOLDER + "invalidPersonField.xml"); + private static final Path VALID_PERSON_FILE = Paths.get(TEST_DATA_FOLDER + "validPerson.xml"); + private static final Path TEMP_FILE = Paths.get(TestUtil.getFilePathInSandboxFolder("tempAddressBook.xml")); private static final String INVALID_PHONE = "9482asf424"; @@ -121,7 +122,7 @@ public void saveDataToFile_missingFile_fileNotFoundException() throws Exception @Test public void saveDataToFile_validFile_dataSaved() throws Exception { - TEMP_FILE.createNewFile(); + FileUtil.createFile(TEMP_FILE); XmlSerializableAddressBook dataToWrite = new XmlSerializableAddressBook(new AddressBook()); XmlUtil.saveDataToFile(TEMP_FILE, dataToWrite); XmlSerializableAddressBook dataFromFile = XmlUtil.getDataFromFile(TEMP_FILE, XmlSerializableAddressBook.class); diff --git a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java b/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java index 00e4bf68ea0c..3e9049dfe793 100644 --- a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java +++ b/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java @@ -2,7 +2,8 @@ import static org.junit.Assert.assertEquals; -import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Rule; import org.junit.Test; @@ -17,8 +18,8 @@ public class XmlSerializableAddressBookTest { private static final String TEST_DATA_FOLDER = FileUtil.getPath("src/test/data/XmlSerializableAddressBookTest/"); - private static final File TYPICAL_PERSONS_FILE = new File(TEST_DATA_FOLDER + "typicalPersonsAddressBook.xml"); - private static final File INVALID_PERSON_FILE = new File(TEST_DATA_FOLDER + "invalidPersonAddressBook.xml"); + private static final Path TYPICAL_PERSONS_FILE = Paths.get(TEST_DATA_FOLDER + "typicalPersonsAddressBook.xml"); + private static final Path INVALID_PERSON_FILE = Paths.get(TEST_DATA_FOLDER + "invalidPersonAddressBook.xml"); @Rule public ExpectedException thrown = ExpectedException.none(); diff --git a/src/test/java/seedu/address/testutil/TestUtil.java b/src/test/java/seedu/address/testutil/TestUtil.java index c1458738c054..dcde0256c8df 100644 --- a/src/test/java/seedu/address/testutil/TestUtil.java +++ b/src/test/java/seedu/address/testutil/TestUtil.java @@ -1,7 +1,7 @@ package seedu.address.testutil; -import java.io.File; import java.io.IOException; +import java.nio.file.Paths; import seedu.address.commons.core.index.Index; import seedu.address.commons.util.FileUtil; @@ -24,7 +24,7 @@ public class TestUtil { */ public static String getFilePathInSandboxFolder(String fileName) { try { - FileUtil.createDirs(new File(SANDBOX_FOLDER)); + FileUtil.createDirs(Paths.get(SANDBOX_FOLDER)); } catch (IOException e) { throw new RuntimeException(e); } diff --git a/src/test/java/seedu/address/ui/PersonListPanelTest.java b/src/test/java/seedu/address/ui/PersonListPanelTest.java index d21de7e05662..02d866064123 100644 --- a/src/test/java/seedu/address/ui/PersonListPanelTest.java +++ b/src/test/java/seedu/address/ui/PersonListPanelTest.java @@ -9,7 +9,8 @@ import static seedu.address.ui.testutil.GuiTestAssert.assertCardDisplaysPerson; import static seedu.address.ui.testutil.GuiTestAssert.assertCardEquals; -import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Test; @@ -79,7 +80,7 @@ public void performanceTest() throws Exception { * {@code PersonListPanel}. */ private ObservableList createBackingList(int personCount) throws Exception { - File xmlFile = createXmlFileWithPersons(personCount); + Path xmlFile = createXmlFileWithPersons(personCount); XmlSerializableAddressBook xmlAddressBook = XmlUtil.getDataFromFile(xmlFile, XmlSerializableAddressBook.class); return FXCollections.observableArrayList(xmlAddressBook.toModelType().getPersonList()); @@ -88,7 +89,7 @@ private ObservableList createBackingList(int personCount) throws Excepti /** * Returns a .xml file containing {@code personCount} persons. This file will be deleted when the JVM terminates. */ - private File createXmlFileWithPersons(int personCount) throws Exception { + private Path createXmlFileWithPersons(int personCount) throws Exception { StringBuilder builder = new StringBuilder(); builder.append("\n"); builder.append("\n"); @@ -102,10 +103,10 @@ private File createXmlFileWithPersons(int personCount) throws Exception { } builder.append("\n"); - File manyPersonsFile = new File(TEST_DATA_FOLDER + "manyPersons.xml"); + Path manyPersonsFile = Paths.get(TEST_DATA_FOLDER + "manyPersons.xml"); FileUtil.createFile(manyPersonsFile); FileUtil.writeToFile(manyPersonsFile, builder.toString()); - manyPersonsFile.deleteOnExit(); + manyPersonsFile.toFile().deleteOnExit(); return manyPersonsFile; } From 0bd4b85c23c1eb5a8ae54325f69144636b26a205 Mon Sep 17 00:00:00 2001 From: Vivek Date: Sat, 21 Apr 2018 00:20:04 +0800 Subject: [PATCH 2/6] MainApp: extract handling of application parameters into AppParameters The handling of application parameters is done in the MainApp. This decreases cohesiveness as handling and parsing of application parameters should be done separately and not in the MainApp as command-line parsing is at a much lower level compared to the high level setup of the application architecture. This also allows the parsing of application parameters to be tested independently. Let's extract the handling of application parameters from MainApp into the AppParameters for future parsing of application parameters. --- .../java/seedu/address/AppParameters.java | 54 +++++++++++++++++++ src/main/java/seedu/address/MainApp.java | 9 +--- .../java/seedu/address/AppParametersTest.java | 50 +++++++++++++++++ 3 files changed, 106 insertions(+), 7 deletions(-) create mode 100644 src/main/java/seedu/address/AppParameters.java create mode 100644 src/test/java/seedu/address/AppParametersTest.java diff --git a/src/main/java/seedu/address/AppParameters.java b/src/main/java/seedu/address/AppParameters.java new file mode 100644 index 000000000000..d9ae2dc96e8b --- /dev/null +++ b/src/main/java/seedu/address/AppParameters.java @@ -0,0 +1,54 @@ +package seedu.address; + +import java.util.Map; +import java.util.Objects; + +import javafx.application.Application; + +/** + * Represents the parsed command-line parameters given to the application. + */ +public class AppParameters { + + private String configPath; + + public String getConfigPath() { + return configPath; + } + + public void setConfigPath(String configPath) { + this.configPath = configPath; + } + + /** + * Parses the application command-line parameters. + */ + public static AppParameters parse(Application.Parameters parameters) { + AppParameters appParameters = new AppParameters(); + Map namedParameters = parameters.getNamed(); + + String configPathParameter = namedParameters.get("config"); + appParameters.setConfigPath(configPathParameter); + + return appParameters; + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + + if (!(other instanceof AppParameters)) { + return false; + } + + AppParameters otherAppParameters = (AppParameters) other; + return Objects.equals(getConfigPath(), otherAppParameters.getConfigPath()); + } + + @Override + public int hashCode() { + return configPath.hashCode(); + } +} diff --git a/src/main/java/seedu/address/MainApp.java b/src/main/java/seedu/address/MainApp.java index fa0800d55cb9..11685f86f5a3 100644 --- a/src/main/java/seedu/address/MainApp.java +++ b/src/main/java/seedu/address/MainApp.java @@ -1,7 +1,6 @@ package seedu.address; import java.io.IOException; -import java.util.Map; import java.util.Optional; import java.util.logging.Logger; @@ -57,7 +56,8 @@ public void init() throws Exception { logger.info("=============================[ Initializing AddressBook ]==========================="); super.init(); - config = initConfig(getApplicationParameter("config")); + AppParameters appParameters = AppParameters.parse(getParameters()); + config = initConfig(appParameters.getConfigPath()); UserPrefsStorage userPrefsStorage = new JsonUserPrefsStorage(config.getUserPrefsFilePath()); userPrefs = initPrefs(userPrefsStorage); @@ -75,11 +75,6 @@ public void init() throws Exception { initEventsCenter(); } - private String getApplicationParameter(String parameterName) { - Map applicationParameters = getParameters().getNamed(); - return applicationParameters.get(parameterName); - } - /** * Returns a {@code ModelManager} with the data from {@code storage}'s address book and {@code userPrefs}.
* The data from the sample address book will be used instead if {@code storage}'s address book is not found, diff --git a/src/test/java/seedu/address/AppParametersTest.java b/src/test/java/seedu/address/AppParametersTest.java new file mode 100644 index 000000000000..a957a35378e7 --- /dev/null +++ b/src/test/java/seedu/address/AppParametersTest.java @@ -0,0 +1,50 @@ +package seedu.address; + +import static org.junit.Assert.assertEquals; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Test; + +import javafx.application.Application; + +public class AppParametersTest { + + private final ParametersStub parametersStub = new ParametersStub(); + private final AppParameters expected = new AppParameters(); + + @Test + public void parse_validConfigPath_success() { + parametersStub.namedParameters.put("config", "config.json"); + expected.setConfigPath("config.json"); + assertEquals(expected, AppParameters.parse(parametersStub)); + } + + @Test + public void parse_nullConfigPath_success() { + parametersStub.namedParameters.put("config", null); + assertEquals(expected, AppParameters.parse(parametersStub)); + } + + private static class ParametersStub extends Application.Parameters { + private Map namedParameters = new HashMap<>(); + + @Override + public List getRaw() { + throw new AssertionError("should not be called"); + } + + @Override + public List getUnnamed() { + throw new AssertionError("should not be called"); + } + + @Override + public Map getNamed() { + return Collections.unmodifiableMap(namedParameters); + } + } +} From 9eac635d9da1a71c81587ba66cab1911977a863b Mon Sep 17 00:00:00 2001 From: Vivek Date: Sat, 21 Apr 2018 00:24:27 +0800 Subject: [PATCH 3/6] Refactor codebase to use java.nio.file for file/path handling String operations are used for the construction of file paths. Also, many APIs in the codebase take in String arguments as file paths. This method of file/path handling is error prone as creating paths using string concatenation can lead to the creation of invalid paths. Furthermore, checking if two paths resolve to the same file using string equality is incorrect as it only tests if the strings are equal. Let's refactor the codebase to use java.nio.file for file/path handling as it provides a safe and platform-independent way to perform operations on paths and properly separate the concerns of parsing/stringifying paths and manipulating paths. More information on why String operations are error-prone: http://twistedoakstudios.com/blog/Post4872_dont-treat-paths-like-strings --- .../java/seedu/address/AppParameters.java | 18 ++++++++++--- src/main/java/seedu/address/MainApp.java | 7 ++--- .../seedu/address/commons/core/Config.java | 10 ++++--- .../address/commons/util/ConfigUtil.java | 5 ++-- .../seedu/address/commons/util/FileUtil.java | 16 ++++++++++++ .../seedu/address/commons/util/JsonUtil.java | 16 +++++------- .../java/seedu/address/model/UserPrefs.java | 8 +++--- .../address/storage/AddressBookStorage.java | 7 ++--- .../address/storage/JsonUserPrefsStorage.java | 9 ++++--- .../java/seedu/address/storage/Storage.java | 3 ++- .../seedu/address/storage/StorageManager.java | 9 ++++--- .../address/storage/UserPrefsStorage.java | 3 ++- .../storage/XmlAddressBookStorage.java | 26 ++++++++----------- .../seedu/address/ui/StatusBarFooter.java | 6 +++-- .../java/seedu/address/AppParametersTest.java | 10 ++++++- src/test/java/seedu/address/TestApp.java | 20 +++++++------- .../address/commons/util/ConfigUtilTest.java | 17 ++++++------ .../address/commons/util/FileUtilTest.java | 16 ++++++++++++ .../address/commons/util/JsonUtilTest.java | 3 +-- .../address/commons/util/XmlUtilTest.java | 16 ++++++------ .../storage/JsonUserPrefsStorageTest.java | 16 ++++++------ .../address/storage/StorageManagerTest.java | 14 +++++----- .../storage/XmlAddressBookStorageTest.java | 16 +++++++----- .../XmlSerializableAddressBookTest.java | 7 +++-- .../java/seedu/address/testutil/TestUtil.java | 13 +++++----- .../seedu/address/ui/PersonListPanelTest.java | 2 +- .../seedu/address/ui/StatusBarFooterTest.java | 10 ++++--- .../systemtests/AddressBookSystemTest.java | 7 +++-- src/test/java/systemtests/SampleDataTest.java | 10 +++---- .../systemtests/SystemTestSetupHelper.java | 3 ++- 30 files changed, 194 insertions(+), 129 deletions(-) diff --git a/src/main/java/seedu/address/AppParameters.java b/src/main/java/seedu/address/AppParameters.java index d9ae2dc96e8b..ab552c398f3d 100644 --- a/src/main/java/seedu/address/AppParameters.java +++ b/src/main/java/seedu/address/AppParameters.java @@ -1,22 +1,28 @@ package seedu.address; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Map; import java.util.Objects; +import java.util.logging.Logger; import javafx.application.Application; +import seedu.address.commons.core.LogsCenter; +import seedu.address.commons.util.FileUtil; /** * Represents the parsed command-line parameters given to the application. */ public class AppParameters { + private static final Logger logger = LogsCenter.getLogger(AppParameters.class); - private String configPath; + private Path configPath; - public String getConfigPath() { + public Path getConfigPath() { return configPath; } - public void setConfigPath(String configPath) { + public void setConfigPath(Path configPath) { this.configPath = configPath; } @@ -28,7 +34,11 @@ public static AppParameters parse(Application.Parameters parameters) { Map namedParameters = parameters.getNamed(); String configPathParameter = namedParameters.get("config"); - appParameters.setConfigPath(configPathParameter); + if (configPathParameter != null && !FileUtil.isValidPath(configPathParameter)) { + logger.warning("Invalid config path " + configPathParameter + ". Using default config path."); + configPathParameter = null; + } + appParameters.setConfigPath(configPathParameter != null ? Paths.get(configPathParameter) : null); return appParameters; } diff --git a/src/main/java/seedu/address/MainApp.java b/src/main/java/seedu/address/MainApp.java index 11685f86f5a3..8d1838c11bec 100644 --- a/src/main/java/seedu/address/MainApp.java +++ b/src/main/java/seedu/address/MainApp.java @@ -1,6 +1,7 @@ package seedu.address; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import java.util.logging.Logger; @@ -109,9 +110,9 @@ private void initLogging(Config config) { * The default file path {@code Config#DEFAULT_CONFIG_FILE} will be used instead * if {@code configFilePath} is null. */ - protected Config initConfig(String configFilePath) { + protected Config initConfig(Path configFilePath) { Config initializedConfig; - String configFilePathUsed; + Path configFilePathUsed; configFilePathUsed = Config.DEFAULT_CONFIG_FILE; @@ -146,7 +147,7 @@ protected Config initConfig(String configFilePath) { * reading from the file. */ protected UserPrefs initPrefs(UserPrefsStorage storage) { - String prefsFilePath = storage.getUserPrefsFilePath(); + Path prefsFilePath = storage.getUserPrefsFilePath(); logger.info("Using prefs file : " + prefsFilePath); UserPrefs initializedPrefs; diff --git a/src/main/java/seedu/address/commons/core/Config.java b/src/main/java/seedu/address/commons/core/Config.java index 8f4d737d0e24..e978d621e086 100644 --- a/src/main/java/seedu/address/commons/core/Config.java +++ b/src/main/java/seedu/address/commons/core/Config.java @@ -1,5 +1,7 @@ package seedu.address.commons.core; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Objects; import java.util.logging.Level; @@ -8,12 +10,12 @@ */ public class Config { - public static final String DEFAULT_CONFIG_FILE = "config.json"; + public static final Path DEFAULT_CONFIG_FILE = Paths.get("config.json"); // Config values customizable through config file private String appTitle = "Address App"; private Level logLevel = Level.INFO; - private String userPrefsFilePath = "preferences.json"; + private Path userPrefsFilePath = Paths.get("preferences.json"); public String getAppTitle() { return appTitle; @@ -31,11 +33,11 @@ public void setLogLevel(Level logLevel) { this.logLevel = logLevel; } - public String getUserPrefsFilePath() { + public Path getUserPrefsFilePath() { return userPrefsFilePath; } - public void setUserPrefsFilePath(String userPrefsFilePath) { + public void setUserPrefsFilePath(Path userPrefsFilePath) { this.userPrefsFilePath = userPrefsFilePath; } diff --git a/src/main/java/seedu/address/commons/util/ConfigUtil.java b/src/main/java/seedu/address/commons/util/ConfigUtil.java index 7eb770dba3e3..f7f8a2bd44c0 100644 --- a/src/main/java/seedu/address/commons/util/ConfigUtil.java +++ b/src/main/java/seedu/address/commons/util/ConfigUtil.java @@ -1,6 +1,7 @@ package seedu.address.commons.util; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import seedu.address.commons.core.Config; @@ -11,11 +12,11 @@ */ public class ConfigUtil { - public static Optional readConfig(String configFilePath) throws DataConversionException { + public static Optional readConfig(Path configFilePath) throws DataConversionException { return JsonUtil.readJsonFile(configFilePath, Config.class); } - public static void saveConfig(Config config, String configFilePath) throws IOException { + public static void saveConfig(Config config, Path configFilePath) throws IOException { 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 4048c8feb800..e2433413483e 100644 --- a/src/main/java/seedu/address/commons/util/FileUtil.java +++ b/src/main/java/seedu/address/commons/util/FileUtil.java @@ -5,7 +5,9 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.nio.file.Path; +import java.nio.file.Paths; /** * Writes and reads files @@ -18,6 +20,20 @@ public static boolean isFileExists(Path file) { return Files.exists(file) && Files.isRegularFile(file); } + /** + * Returns true if {@code path} can be converted into a {@code Path} via {@link Paths#get(String)}, + * otherwise returns false. + * @param path A string representing the file path. Cannot be null. + */ + public static boolean isValidPath(String path) { + try { + Paths.get(path); + } catch (InvalidPathException ipe) { + return false; + } + return true; + } + /** * Creates a file if it does not exist along with its missing parent directories. * @throws IOException if the file or directory cannot be created. diff --git a/src/main/java/seedu/address/commons/util/JsonUtil.java b/src/main/java/seedu/address/commons/util/JsonUtil.java index e1dc6cb25bc9..0b13f5180370 100644 --- a/src/main/java/seedu/address/commons/util/JsonUtil.java +++ b/src/main/java/seedu/address/commons/util/JsonUtil.java @@ -5,7 +5,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; @@ -57,21 +56,20 @@ static T deserializeObjectFromJsonFile(Path jsonFile, Class classOfObject * @throws DataConversionException if the file format is not as expected. */ public static Optional readJsonFile( - String filePath, Class classOfObjectToDeserialize) throws DataConversionException { + Path filePath, Class classOfObjectToDeserialize) throws DataConversionException { requireNonNull(filePath); - Path file = Paths.get(filePath); - if (!Files.exists(file)) { - logger.info("Json file " + file + " not found"); + if (!Files.exists(filePath)) { + logger.info("Json file " + filePath + " not found"); return Optional.empty(); } T jsonFile; try { - jsonFile = deserializeObjectFromJsonFile(file, classOfObjectToDeserialize); + jsonFile = deserializeObjectFromJsonFile(filePath, classOfObjectToDeserialize); } catch (IOException e) { - logger.warning("Error reading from jsonFile file " + file + ": " + e); + logger.warning("Error reading from jsonFile file " + filePath + ": " + e); throw new DataConversionException(e); } @@ -85,11 +83,11 @@ public static Optional readJsonFile( * @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 { + public static void saveJsonFile(T jsonFile, Path filePath) throws IOException { requireNonNull(filePath); requireNonNull(jsonFile); - serializeObjectToJsonFile(Paths.get(filePath), jsonFile); + serializeObjectToJsonFile(filePath, jsonFile); } diff --git a/src/main/java/seedu/address/model/UserPrefs.java b/src/main/java/seedu/address/model/UserPrefs.java index 8c8a071876eb..362b38400da3 100644 --- a/src/main/java/seedu/address/model/UserPrefs.java +++ b/src/main/java/seedu/address/model/UserPrefs.java @@ -1,5 +1,7 @@ package seedu.address.model; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Objects; import seedu.address.commons.core.GuiSettings; @@ -10,7 +12,7 @@ public class UserPrefs { private GuiSettings guiSettings; - private String addressBookFilePath = "data/addressbook.xml"; + private Path addressBookFilePath = Paths.get("data" , "addressbook.xml"); private String addressBookName = "MyAddressBook"; public UserPrefs() { @@ -29,11 +31,11 @@ public void setGuiSettings(double width, double height, int x, int y) { guiSettings = new GuiSettings(width, height, x, y); } - public String getAddressBookFilePath() { + public Path getAddressBookFilePath() { return addressBookFilePath; } - public void setAddressBookFilePath(String addressBookFilePath) { + public void setAddressBookFilePath(Path addressBookFilePath) { this.addressBookFilePath = addressBookFilePath; } diff --git a/src/main/java/seedu/address/storage/AddressBookStorage.java b/src/main/java/seedu/address/storage/AddressBookStorage.java index cf5b527c063a..4599182b3f92 100644 --- a/src/main/java/seedu/address/storage/AddressBookStorage.java +++ b/src/main/java/seedu/address/storage/AddressBookStorage.java @@ -1,6 +1,7 @@ package seedu.address.storage; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import seedu.address.commons.exceptions.DataConversionException; @@ -14,7 +15,7 @@ public interface AddressBookStorage { /** * Returns the file path of the data file. */ - String getAddressBookFilePath(); + Path getAddressBookFilePath(); /** * Returns AddressBook data as a {@link ReadOnlyAddressBook}. @@ -27,7 +28,7 @@ public interface AddressBookStorage { /** * @see #getAddressBookFilePath() */ - Optional readAddressBook(String filePath) throws DataConversionException, IOException; + Optional readAddressBook(Path filePath) throws DataConversionException, IOException; /** * Saves the given {@link ReadOnlyAddressBook} to the storage. @@ -39,6 +40,6 @@ public interface AddressBookStorage { /** * @see #saveAddressBook(ReadOnlyAddressBook) */ - void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) throws IOException; + void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException; } diff --git a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java index 4f41aff81251..498fe3c4e9a0 100644 --- a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java +++ b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java @@ -1,6 +1,7 @@ package seedu.address.storage; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import seedu.address.commons.exceptions.DataConversionException; @@ -12,14 +13,14 @@ */ public class JsonUserPrefsStorage implements UserPrefsStorage { - private String filePath; + private Path filePath; - public JsonUserPrefsStorage(String filePath) { + public JsonUserPrefsStorage(Path filePath) { this.filePath = filePath; } @Override - public String getUserPrefsFilePath() { + public Path getUserPrefsFilePath() { return filePath; } @@ -33,7 +34,7 @@ public Optional readUserPrefs() throws DataConversionException, IOExc * @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 { + public Optional readUserPrefs(Path prefsFilePath) throws DataConversionException { return JsonUtil.readJsonFile(prefsFilePath, UserPrefs.class); } diff --git a/src/main/java/seedu/address/storage/Storage.java b/src/main/java/seedu/address/storage/Storage.java index c0881a5a6483..28791127999b 100644 --- a/src/main/java/seedu/address/storage/Storage.java +++ b/src/main/java/seedu/address/storage/Storage.java @@ -1,6 +1,7 @@ package seedu.address.storage; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import seedu.address.commons.events.model.AddressBookChangedEvent; @@ -21,7 +22,7 @@ public interface Storage extends AddressBookStorage, UserPrefsStorage { void saveUserPrefs(UserPrefs userPrefs) throws IOException; @Override - String getAddressBookFilePath(); + Path getAddressBookFilePath(); @Override Optional readAddressBook() throws DataConversionException, IOException; diff --git a/src/main/java/seedu/address/storage/StorageManager.java b/src/main/java/seedu/address/storage/StorageManager.java index 53967b391a5a..b0df908a76a7 100644 --- a/src/main/java/seedu/address/storage/StorageManager.java +++ b/src/main/java/seedu/address/storage/StorageManager.java @@ -1,6 +1,7 @@ package seedu.address.storage; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import java.util.logging.Logger; @@ -33,7 +34,7 @@ public StorageManager(AddressBookStorage addressBookStorage, UserPrefsStorage us // ================ UserPrefs methods ============================== @Override - public String getUserPrefsFilePath() { + public Path getUserPrefsFilePath() { return userPrefsStorage.getUserPrefsFilePath(); } @@ -51,7 +52,7 @@ public void saveUserPrefs(UserPrefs userPrefs) throws IOException { // ================ AddressBook methods ============================== @Override - public String getAddressBookFilePath() { + public Path getAddressBookFilePath() { return addressBookStorage.getAddressBookFilePath(); } @@ -61,7 +62,7 @@ public Optional readAddressBook() throws DataConversionExce } @Override - public Optional readAddressBook(String filePath) throws DataConversionException, IOException { + public Optional readAddressBook(Path filePath) throws DataConversionException, IOException { logger.fine("Attempting to read data from file: " + filePath); return addressBookStorage.readAddressBook(filePath); } @@ -72,7 +73,7 @@ public void saveAddressBook(ReadOnlyAddressBook addressBook) throws IOException } @Override - public void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) throws IOException { + public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { logger.fine("Attempting to write to data file: " + filePath); addressBookStorage.saveAddressBook(addressBook, filePath); } diff --git a/src/main/java/seedu/address/storage/UserPrefsStorage.java b/src/main/java/seedu/address/storage/UserPrefsStorage.java index 146477fad976..877b0ee5c4f0 100644 --- a/src/main/java/seedu/address/storage/UserPrefsStorage.java +++ b/src/main/java/seedu/address/storage/UserPrefsStorage.java @@ -1,6 +1,7 @@ package seedu.address.storage; import java.io.IOException; +import java.nio.file.Path; import java.util.Optional; import seedu.address.commons.exceptions.DataConversionException; @@ -14,7 +15,7 @@ public interface UserPrefsStorage { /** * Returns the file path of the UserPrefs data file. */ - String getUserPrefsFilePath(); + Path getUserPrefsFilePath(); /** * Returns UserPrefs data from storage. diff --git a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java b/src/main/java/seedu/address/storage/XmlAddressBookStorage.java index 6846c7a962da..8af60be0dc5d 100644 --- a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java +++ b/src/main/java/seedu/address/storage/XmlAddressBookStorage.java @@ -6,7 +6,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Optional; import java.util.logging.Logger; @@ -23,13 +22,13 @@ public class XmlAddressBookStorage implements AddressBookStorage { private static final Logger logger = LogsCenter.getLogger(XmlAddressBookStorage.class); - private String filePath; + private Path filePath; - public XmlAddressBookStorage(String filePath) { + public XmlAddressBookStorage(Path filePath) { this.filePath = filePath; } - public String getAddressBookFilePath() { + public Path getAddressBookFilePath() { return filePath; } @@ -43,22 +42,20 @@ public Optional readAddressBook() throws DataConversionExce * @param filePath location of the data. Cannot be null * @throws DataConversionException if the file is not in the correct format. */ - public Optional readAddressBook(String filePath) throws DataConversionException, + public Optional readAddressBook(Path filePath) throws DataConversionException, FileNotFoundException { requireNonNull(filePath); - Path addressBookFile = Paths.get(filePath); - - if (!Files.exists(addressBookFile)) { - logger.info("AddressBook file " + addressBookFile + " not found"); + if (!Files.exists(filePath)) { + logger.info("AddressBook file " + filePath + " not found"); return Optional.empty(); } - XmlSerializableAddressBook xmlAddressBook = XmlFileStorage.loadDataFromSaveFile(Paths.get(filePath)); + XmlSerializableAddressBook xmlAddressBook = XmlFileStorage.loadDataFromSaveFile(filePath); try { return Optional.of(xmlAddressBook.toModelType()); } catch (IllegalValueException ive) { - logger.info("Illegal values found in " + addressBookFile + ": " + ive.getMessage()); + logger.info("Illegal values found in " + filePath + ": " + ive.getMessage()); throw new DataConversionException(ive); } } @@ -72,13 +69,12 @@ public void saveAddressBook(ReadOnlyAddressBook addressBook) throws IOException * Similar to {@link #saveAddressBook(ReadOnlyAddressBook)} * @param filePath location of the data. Cannot be null */ - public void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) throws IOException { + public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { requireNonNull(addressBook); requireNonNull(filePath); - Path file = Paths.get(filePath); - FileUtil.createIfMissing(file); - XmlFileStorage.saveDataToFile(file, new XmlSerializableAddressBook(addressBook)); + FileUtil.createIfMissing(filePath); + XmlFileStorage.saveDataToFile(filePath, new XmlSerializableAddressBook(addressBook)); } } diff --git a/src/main/java/seedu/address/ui/StatusBarFooter.java b/src/main/java/seedu/address/ui/StatusBarFooter.java index 06fb7e50c935..7fcf47864caa 100644 --- a/src/main/java/seedu/address/ui/StatusBarFooter.java +++ b/src/main/java/seedu/address/ui/StatusBarFooter.java @@ -1,5 +1,7 @@ package seedu.address.ui; +import java.nio.file.Path; +import java.nio.file.Paths; import java.time.Clock; import java.util.Date; import java.util.logging.Logger; @@ -42,10 +44,10 @@ public class StatusBarFooter extends UiPart { private StatusBar saveLocationStatus; - public StatusBarFooter(String saveLocation) { + public StatusBarFooter(Path saveLocation) { super(FXML); setSyncStatus(SYNC_STATUS_INITIAL); - setSaveLocation("./" + saveLocation); + setSaveLocation(Paths.get(".").resolve(saveLocation).toString()); registerAsAnEventHandler(this); } diff --git a/src/test/java/seedu/address/AppParametersTest.java b/src/test/java/seedu/address/AppParametersTest.java index a957a35378e7..e6ef73d83b50 100644 --- a/src/test/java/seedu/address/AppParametersTest.java +++ b/src/test/java/seedu/address/AppParametersTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; +import java.nio.file.Paths; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -19,7 +20,7 @@ public class AppParametersTest { @Test public void parse_validConfigPath_success() { parametersStub.namedParameters.put("config", "config.json"); - expected.setConfigPath("config.json"); + expected.setConfigPath(Paths.get("config.json")); assertEquals(expected, AppParameters.parse(parametersStub)); } @@ -29,6 +30,13 @@ public void parse_nullConfigPath_success() { assertEquals(expected, AppParameters.parse(parametersStub)); } + @Test + public void parse_invalidConfigPath_success() { + parametersStub.namedParameters.put("config", "a\0"); + expected.setConfigPath(null); + assertEquals(expected, AppParameters.parse(parametersStub)); + } + private static class ParametersStub extends Application.Parameters { private Map namedParameters = new HashMap<>(); diff --git a/src/test/java/seedu/address/TestApp.java b/src/test/java/seedu/address/TestApp.java index 6e33de7f2112..f03b4562a362 100644 --- a/src/test/java/seedu/address/TestApp.java +++ b/src/test/java/seedu/address/TestApp.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.function.Supplier; import javafx.stage.Screen; @@ -28,19 +27,19 @@ */ public class TestApp extends MainApp { - public static final String SAVE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("sampleData.xml"); + public static final Path SAVE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("sampleData.xml"); public static final String APP_TITLE = "Test App"; - protected static final String DEFAULT_PREF_FILE_LOCATION_FOR_TESTING = + protected static final Path DEFAULT_PREF_FILE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("pref_testing.json"); protected static final String ADDRESS_BOOK_NAME = "Test"; protected Supplier initialDataSupplier = () -> null; - protected String saveFileLocation = SAVE_LOCATION_FOR_TESTING; + protected Path saveFileLocation = SAVE_LOCATION_FOR_TESTING; public TestApp() { } - public TestApp(Supplier initialDataSupplier, String saveFileLocation) { + public TestApp(Supplier initialDataSupplier, Path saveFileLocation) { super(); this.initialDataSupplier = initialDataSupplier; this.saveFileLocation = saveFileLocation; @@ -53,7 +52,7 @@ public TestApp(Supplier initialDataSupplier, String saveFil } @Override - protected Config initConfig(String configFilePath) { + protected Config initConfig(Path configFilePath) { Config config = super.initConfig(configFilePath); config.setAppTitle(APP_TITLE); config.setUserPrefsFilePath(DEFAULT_PREF_FILE_LOCATION_FOR_TESTING); @@ -87,7 +86,7 @@ public AddressBook readStorageAddressBook() { /** * Returns the file path of the storage file. */ - public String getStorageSaveLocation() { + public Path getStorageSaveLocation() { return storage.getAddressBookFilePath(); } @@ -112,11 +111,10 @@ public static void main(String[] args) { /** * Creates an XML file at the {@code filePath} with the {@code data}. */ - private void createDataFileWithData(T data, String filePath) { + private void createDataFileWithData(T data, Path filePath) { try { - Path saveFileForTesting = Paths.get(filePath); - FileUtil.createIfMissing(saveFileForTesting); - XmlUtil.saveDataToFile(saveFileForTesting, data); + FileUtil.createIfMissing(filePath); + XmlUtil.saveDataToFile(filePath, data); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java index 6c8ab6cb5891..163b5d96bd23 100644 --- a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java +++ b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java @@ -3,8 +3,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import java.io.File; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Optional; import java.util.logging.Level; @@ -18,7 +19,7 @@ public class ConfigUtilTest { - private static final String TEST_DATA_FOLDER = FileUtil.getPath("./src/test/data/ConfigUtilTest/"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "ConfigUtilTest"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -75,12 +76,12 @@ private Config getTypicalConfig() { Config config = new Config(); config.setAppTitle("Typical App Title"); config.setLogLevel(Level.INFO); - config.setUserPrefsFilePath("C:\\preferences.json"); + config.setUserPrefsFilePath(Paths.get("C:\\preferences.json")); return config; } private Optional read(String configFileInTestDataFolder) throws DataConversionException { - String configFilePath = addToTestDataPathIfNotNull(configFileInTestDataFolder); + Path configFilePath = addToTestDataPathIfNotNull(configFileInTestDataFolder); return ConfigUtil.readConfig(configFilePath); } @@ -100,7 +101,7 @@ public void save_nullFile_throwsNullPointerException() throws IOException { public void saveConfig_allInOrder_success() throws DataConversionException, IOException { Config original = getTypicalConfig(); - String configFilePath = testFolder.getRoot() + File.separator + "TempConfig.json"; + Path configFilePath = testFolder.getRoot().toPath().resolve("TempConfig.json"); //Try writing when the file doesn't exist ConfigUtil.saveConfig(original, configFilePath); @@ -116,13 +117,13 @@ public void saveConfig_allInOrder_success() throws DataConversionException, IOEx } private void save(Config config, String configFileInTestDataFolder) throws IOException { - String configFilePath = addToTestDataPathIfNotNull(configFileInTestDataFolder); + Path configFilePath = addToTestDataPathIfNotNull(configFileInTestDataFolder); ConfigUtil.saveConfig(config, configFilePath); } - private String addToTestDataPathIfNotNull(String configFileInTestDataFolder) { + private Path addToTestDataPathIfNotNull(String configFileInTestDataFolder) { return configFileInTestDataFolder != null - ? TEST_DATA_FOLDER + configFileInTestDataFolder + ? TEST_DATA_FOLDER.resolve(configFileInTestDataFolder) : null; } diff --git a/src/test/java/seedu/address/commons/util/FileUtilTest.java b/src/test/java/seedu/address/commons/util/FileUtilTest.java index aa7f551b6687..f4dab9356c9a 100644 --- a/src/test/java/seedu/address/commons/util/FileUtilTest.java +++ b/src/test/java/seedu/address/commons/util/FileUtilTest.java @@ -1,6 +1,8 @@ package seedu.address.commons.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.File; @@ -8,6 +10,8 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import seedu.address.testutil.Assert; + public class FileUtilTest { @Rule @@ -28,4 +32,16 @@ public void getPath() { FileUtil.getPath("folder"); } + @Test + public void isValidPath() { + // valid path + assertTrue(FileUtil.isValidPath("valid/file/path")); + + // invalid path + assertFalse(FileUtil.isValidPath("a\0")); + + // null path -> throws NullPointerException + Assert.assertThrows(NullPointerException.class, () -> FileUtil.isValidPath(null)); + } + } diff --git a/src/test/java/seedu/address/commons/util/JsonUtilTest.java b/src/test/java/seedu/address/commons/util/JsonUtilTest.java index 778cc7deabd3..847b40ad181a 100644 --- a/src/test/java/seedu/address/commons/util/JsonUtilTest.java +++ b/src/test/java/seedu/address/commons/util/JsonUtilTest.java @@ -4,7 +4,6 @@ import java.io.IOException; import java.nio.file.Path; -import java.nio.file.Paths; import org.junit.Test; @@ -16,7 +15,7 @@ */ public class JsonUtilTest { - private static final Path SERIALIZATION_FILE = Paths.get(TestUtil.getFilePathInSandboxFolder("serialize.json")); + private static final Path SERIALIZATION_FILE = TestUtil.getFilePathInSandboxFolder("serialize.json"); @Test public void serializeObjectToJsonFile_noExceptionThrown() throws IOException { diff --git a/src/test/java/seedu/address/commons/util/XmlUtilTest.java b/src/test/java/seedu/address/commons/util/XmlUtilTest.java index 74d1e8a7eef9..f409f974a233 100644 --- a/src/test/java/seedu/address/commons/util/XmlUtilTest.java +++ b/src/test/java/seedu/address/commons/util/XmlUtilTest.java @@ -25,14 +25,14 @@ public class XmlUtilTest { - private static final String TEST_DATA_FOLDER = FileUtil.getPath("src/test/data/XmlUtilTest/"); - private static final Path EMPTY_FILE = Paths.get(TEST_DATA_FOLDER + "empty.xml"); - private static final Path MISSING_FILE = Paths.get(TEST_DATA_FOLDER + "missing.xml"); - private static final Path VALID_FILE = Paths.get(TEST_DATA_FOLDER + "validAddressBook.xml"); - private static final Path MISSING_PERSON_FIELD_FILE = Paths.get(TEST_DATA_FOLDER + "missingPersonField.xml"); - private static final Path INVALID_PERSON_FIELD_FILE = Paths.get(TEST_DATA_FOLDER + "invalidPersonField.xml"); - private static final Path VALID_PERSON_FILE = Paths.get(TEST_DATA_FOLDER + "validPerson.xml"); - private static final Path TEMP_FILE = Paths.get(TestUtil.getFilePathInSandboxFolder("tempAddressBook.xml")); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "XmlUtilTest"); + private static final Path EMPTY_FILE = TEST_DATA_FOLDER.resolve("empty.xml"); + private static final Path MISSING_FILE = TEST_DATA_FOLDER.resolve("missing.xml"); + private static final Path VALID_FILE = TEST_DATA_FOLDER.resolve("validAddressBook.xml"); + private static final Path MISSING_PERSON_FIELD_FILE = TEST_DATA_FOLDER.resolve("missingPersonField.xml"); + private static final Path INVALID_PERSON_FIELD_FILE = TEST_DATA_FOLDER.resolve("invalidPersonField.xml"); + private static final Path VALID_PERSON_FILE = TEST_DATA_FOLDER.resolve("validPerson.xml"); + private static final Path TEMP_FILE = TestUtil.getFilePathInSandboxFolder("tempAddressBook.xml"); private static final String INVALID_PHONE = "9482asf424"; diff --git a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java index a6854c246da9..b150ceeb50be 100644 --- a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java +++ b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java @@ -3,8 +3,9 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import java.io.File; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Optional; import org.junit.Rule; @@ -13,12 +14,11 @@ import org.junit.rules.TemporaryFolder; import seedu.address.commons.exceptions.DataConversionException; -import seedu.address.commons.util.FileUtil; import seedu.address.model.UserPrefs; public class JsonUserPrefsStorageTest { - private static final String TEST_DATA_FOLDER = FileUtil.getPath("./src/test/data/JsonUserPrefsStorageTest/"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "JsonUserPrefsStorageTest"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -33,7 +33,7 @@ public void readUserPrefs_nullFilePath_throwsNullPointerException() throws DataC } private Optional readUserPrefs(String userPrefsFileInTestDataFolder) throws DataConversionException { - String prefsFilePath = addToTestDataPathIfNotNull(userPrefsFileInTestDataFolder); + Path prefsFilePath = addToTestDataPathIfNotNull(userPrefsFileInTestDataFolder); return new JsonUserPrefsStorage(prefsFilePath).readUserPrefs(prefsFilePath); } @@ -52,9 +52,9 @@ public void readUserPrefs_notJsonFormat_exceptionThrown() throws DataConversionE */ } - private String addToTestDataPathIfNotNull(String userPrefsFileInTestDataFolder) { + private Path addToTestDataPathIfNotNull(String userPrefsFileInTestDataFolder) { return userPrefsFileInTestDataFolder != null - ? TEST_DATA_FOLDER + userPrefsFileInTestDataFolder + ? TEST_DATA_FOLDER.resolve(userPrefsFileInTestDataFolder) : null; } @@ -82,7 +82,7 @@ public void readUserPrefs_extraValuesInFile_extraValuesIgnored() throws DataConv private UserPrefs getTypicalUserPrefs() { UserPrefs userPrefs = new UserPrefs(); userPrefs.setGuiSettings(1000, 500, 300, 100); - userPrefs.setAddressBookFilePath("addressbook.xml"); + userPrefs.setAddressBookFilePath(Paths.get("addressbook.xml")); userPrefs.setAddressBookName("TypicalAddressBookName"); return userPrefs; } @@ -117,7 +117,7 @@ public void saveUserPrefs_allInOrder_success() throws DataConversionException, I UserPrefs original = new UserPrefs(); original.setGuiSettings(1200, 200, 0, 2); - String pefsFilePath = testFolder.getRoot() + File.separator + "TempPrefs.json"; + Path pefsFilePath = testFolder.getRoot().toPath().resolve("TempPrefs.json"); JsonUserPrefsStorage jsonUserPrefsStorage = new JsonUserPrefsStorage(pefsFilePath); //Try writing when the file doesn't exist diff --git a/src/test/java/seedu/address/storage/StorageManagerTest.java b/src/test/java/seedu/address/storage/StorageManagerTest.java index 419021d459f6..7436e09cf486 100644 --- a/src/test/java/seedu/address/storage/StorageManagerTest.java +++ b/src/test/java/seedu/address/storage/StorageManagerTest.java @@ -6,6 +6,8 @@ import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Before; import org.junit.Rule; @@ -35,8 +37,8 @@ public void setUp() { storageManager = new StorageManager(addressBookStorage, userPrefsStorage); } - private String getTempFilePath(String fileName) { - return testFolder.getRoot().getPath() + fileName; + private Path getTempFilePath(String fileName) { + return testFolder.getRoot().toPath().resolve(fileName); } @@ -75,8 +77,8 @@ public void getAddressBookFilePath() { @Test public void handleAddressBookChangedEvent_exceptionThrown_eventRaised() { // Create a StorageManager while injecting a stub that throws an exception when the save method is called - Storage storage = new StorageManager(new XmlAddressBookStorageExceptionThrowingStub("dummy"), - new JsonUserPrefsStorage("dummy")); + Storage storage = new StorageManager(new XmlAddressBookStorageExceptionThrowingStub(Paths.get("dummy")), + new JsonUserPrefsStorage(Paths.get("dummy"))); storage.handleAddressBookChangedEvent(new AddressBookChangedEvent(new AddressBook())); assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof DataSavingExceptionEvent); } @@ -87,12 +89,12 @@ public void handleAddressBookChangedEvent_exceptionThrown_eventRaised() { */ class XmlAddressBookStorageExceptionThrowingStub extends XmlAddressBookStorage { - public XmlAddressBookStorageExceptionThrowingStub(String filePath) { + public XmlAddressBookStorageExceptionThrowingStub(Path filePath) { super(filePath); } @Override - public void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) throws IOException { + public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { throw new IOException("dummy exception"); } } diff --git a/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java b/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java index 1bf3765cfba9..7ed10523477b 100644 --- a/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java +++ b/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java @@ -8,6 +8,8 @@ import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook; import java.io.IOException; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Rule; import org.junit.Test; @@ -15,12 +17,11 @@ import org.junit.rules.TemporaryFolder; import seedu.address.commons.exceptions.DataConversionException; -import seedu.address.commons.util.FileUtil; import seedu.address.model.AddressBook; import seedu.address.model.ReadOnlyAddressBook; public class XmlAddressBookStorageTest { - private static final String TEST_DATA_FOLDER = FileUtil.getPath("./src/test/data/XmlAddressBookStorageTest/"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "XmlAddressBookStorageTest"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -35,12 +36,12 @@ public void readAddressBook_nullFilePath_throwsNullPointerException() throws Exc } private java.util.Optional readAddressBook(String filePath) throws Exception { - return new XmlAddressBookStorage(filePath).readAddressBook(addToTestDataPathIfNotNull(filePath)); + return new XmlAddressBookStorage(Paths.get(filePath)).readAddressBook(addToTestDataPathIfNotNull(filePath)); } - private String addToTestDataPathIfNotNull(String prefsFileInTestDataFolder) { + private Path addToTestDataPathIfNotNull(String prefsFileInTestDataFolder) { return prefsFileInTestDataFolder != null - ? TEST_DATA_FOLDER + prefsFileInTestDataFolder + ? TEST_DATA_FOLDER.resolve(prefsFileInTestDataFolder) : null; } @@ -74,7 +75,7 @@ public void readAddressBook_invalidAndValidPersonAddressBook_throwDataConversion @Test public void readAndSaveAddressBook_allInOrder_success() throws Exception { - String filePath = testFolder.getRoot().getPath() + "TempAddressBook.xml"; + Path filePath = testFolder.getRoot().toPath().resolve("TempAddressBook.xml"); AddressBook original = getTypicalAddressBook(); XmlAddressBookStorage xmlAddressBookStorage = new XmlAddressBookStorage(filePath); @@ -109,7 +110,8 @@ public void saveAddressBook_nullAddressBook_throwsNullPointerException() { */ private void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) { try { - new XmlAddressBookStorage(filePath).saveAddressBook(addressBook, addToTestDataPathIfNotNull(filePath)); + new XmlAddressBookStorage(Paths.get(filePath)) + .saveAddressBook(addressBook, addToTestDataPathIfNotNull(filePath)); } catch (IOException ioe) { throw new AssertionError("There should not be an error writing to the file.", ioe); } diff --git a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java b/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java index 3e9049dfe793..0919ed16a38c 100644 --- a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java +++ b/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java @@ -10,16 +10,15 @@ import org.junit.rules.ExpectedException; import seedu.address.commons.exceptions.IllegalValueException; -import seedu.address.commons.util.FileUtil; import seedu.address.commons.util.XmlUtil; import seedu.address.model.AddressBook; import seedu.address.testutil.TypicalPersons; public class XmlSerializableAddressBookTest { - private static final String TEST_DATA_FOLDER = FileUtil.getPath("src/test/data/XmlSerializableAddressBookTest/"); - private static final Path TYPICAL_PERSONS_FILE = Paths.get(TEST_DATA_FOLDER + "typicalPersonsAddressBook.xml"); - private static final Path INVALID_PERSON_FILE = Paths.get(TEST_DATA_FOLDER + "invalidPersonAddressBook.xml"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "XmlSerializableAddressBookTest"); + private static final Path TYPICAL_PERSONS_FILE = TEST_DATA_FOLDER.resolve("typicalPersonsAddressBook.xml"); + private static final Path INVALID_PERSON_FILE = TEST_DATA_FOLDER.resolve("invalidPersonAddressBook.xml"); @Rule public ExpectedException thrown = ExpectedException.none(); diff --git a/src/test/java/seedu/address/testutil/TestUtil.java b/src/test/java/seedu/address/testutil/TestUtil.java index dcde0256c8df..b9a00029924a 100644 --- a/src/test/java/seedu/address/testutil/TestUtil.java +++ b/src/test/java/seedu/address/testutil/TestUtil.java @@ -1,10 +1,11 @@ package seedu.address.testutil; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.Paths; import seedu.address.commons.core.index.Index; -import seedu.address.commons.util.FileUtil; import seedu.address.model.Model; import seedu.address.model.person.Person; @@ -16,19 +17,19 @@ public class TestUtil { /** * Folder used for temp files created during testing. Ignored by Git. */ - private static final String SANDBOX_FOLDER = FileUtil.getPath("./src/test/data/sandbox/"); + private static final Path SANDBOX_FOLDER = Paths.get("src", "test", "data", "sandbox"); /** - * Appends {@code fileName} to the sandbox folder path and returns the resulting string. + * Appends {@code fileName} to the sandbox folder path and returns the resulting path. * Creates the sandbox folder if it doesn't exist. */ - public static String getFilePathInSandboxFolder(String fileName) { + public static Path getFilePathInSandboxFolder(String fileName) { try { - FileUtil.createDirs(Paths.get(SANDBOX_FOLDER)); + Files.createDirectories(SANDBOX_FOLDER); } catch (IOException e) { throw new RuntimeException(e); } - return SANDBOX_FOLDER + fileName; + return SANDBOX_FOLDER.resolve(fileName); } /** diff --git a/src/test/java/seedu/address/ui/PersonListPanelTest.java b/src/test/java/seedu/address/ui/PersonListPanelTest.java index 02d866064123..24c015565eb3 100644 --- a/src/test/java/seedu/address/ui/PersonListPanelTest.java +++ b/src/test/java/seedu/address/ui/PersonListPanelTest.java @@ -30,7 +30,7 @@ public class PersonListPanelTest extends GuiUnitTest { private static final JumpToListRequestEvent JUMP_TO_SECOND_EVENT = new JumpToListRequestEvent(INDEX_SECOND_PERSON); - private static final String TEST_DATA_FOLDER = FileUtil.getPath("src/test/data/sandbox/"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "sandbox"); private static final long CARD_CREATION_AND_DELETION_TIMEOUT = 2500; diff --git a/src/test/java/seedu/address/ui/StatusBarFooterTest.java b/src/test/java/seedu/address/ui/StatusBarFooterTest.java index 0c75a73b1864..c7d21684a472 100644 --- a/src/test/java/seedu/address/ui/StatusBarFooterTest.java +++ b/src/test/java/seedu/address/ui/StatusBarFooterTest.java @@ -5,6 +5,8 @@ import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_INITIAL; import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_UPDATED; +import java.nio.file.Path; +import java.nio.file.Paths; import java.time.Clock; import java.time.Instant; import java.time.ZoneId; @@ -21,8 +23,8 @@ public class StatusBarFooterTest extends GuiUnitTest { - private static final String STUB_SAVE_LOCATION = "Stub"; - private static final String RELATIVE_PATH = "./"; + private static final Path STUB_SAVE_LOCATION = Paths.get("Stub"); + private static final Path RELATIVE_PATH = Paths.get("."); private static final AddressBookChangedEvent EVENT_STUB = new AddressBookChangedEvent(new AddressBook()); @@ -54,11 +56,11 @@ public void setUp() { @Test public void display() { // initial state - assertStatusBarContent(RELATIVE_PATH + STUB_SAVE_LOCATION, SYNC_STATUS_INITIAL); + assertStatusBarContent(RELATIVE_PATH.resolve(STUB_SAVE_LOCATION).toString(), SYNC_STATUS_INITIAL); // after address book is updated postNow(EVENT_STUB); - assertStatusBarContent(RELATIVE_PATH + STUB_SAVE_LOCATION, + assertStatusBarContent(RELATIVE_PATH.resolve(STUB_SAVE_LOCATION).toString(), String.format(SYNC_STATUS_UPDATED, new Date(injectedClock.millis()).toString())); } diff --git a/src/test/java/systemtests/AddressBookSystemTest.java b/src/test/java/systemtests/AddressBookSystemTest.java index 7b132f563e42..5c2c2f88b38d 100644 --- a/src/test/java/systemtests/AddressBookSystemTest.java +++ b/src/test/java/systemtests/AddressBookSystemTest.java @@ -12,6 +12,8 @@ import java.net.MalformedURLException; import java.net.URL; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Arrays; import java.util.Date; import java.util.List; @@ -89,7 +91,7 @@ protected AddressBook getInitialData() { /** * Returns the directory of the data file. */ - protected String getDataFileLocation() { + protected Path getDataFileLocation() { return TestApp.SAVE_LOCATION_FOR_TESTING; } @@ -278,7 +280,8 @@ private void assertApplicationStartingStateIsCorrect() { assertEquals("", getResultDisplay().getText()); assertListMatching(getPersonListPanel(), getModel().getFilteredPersonList()); assertEquals(MainApp.class.getResource(FXML_FILE_FOLDER + DEFAULT_PAGE), getBrowserPanel().getLoadedUrl()); - assertEquals("./" + testApp.getStorageSaveLocation(), getStatusBarFooter().getSaveLocation()); + assertEquals(Paths.get(".").resolve(testApp.getStorageSaveLocation()).toString(), + getStatusBarFooter().getSaveLocation()); assertEquals(SYNC_STATUS_INITIAL, getStatusBarFooter().getSyncStatus()); } catch (Exception e) { throw new AssertionError("Starting state is wrong.", e); diff --git a/src/test/java/systemtests/SampleDataTest.java b/src/test/java/systemtests/SampleDataTest.java index 81a32ce9487b..6a1cfac1ea75 100644 --- a/src/test/java/systemtests/SampleDataTest.java +++ b/src/test/java/systemtests/SampleDataTest.java @@ -4,7 +4,7 @@ import java.io.IOException; import java.nio.file.Files; -import java.nio.file.Paths; +import java.nio.file.Path; import org.junit.Test; @@ -26,8 +26,8 @@ protected AddressBook getInitialData() { * Returns a non-existent file location to force test app to load sample data. */ @Override - protected String getDataFileLocation() { - String filePath = TestUtil.getFilePathInSandboxFolder("SomeFileThatDoesNotExist1234567890.xml"); + protected Path getDataFileLocation() { + Path filePath = TestUtil.getFilePathInSandboxFolder("SomeFileThatDoesNotExist1234567890.xml"); deleteFileIfExists(filePath); return filePath; } @@ -35,9 +35,9 @@ protected String getDataFileLocation() { /** * Deletes the file at {@code filePath} if it exists. */ - private void deleteFileIfExists(String filePath) { + private void deleteFileIfExists(Path filePath) { try { - Files.deleteIfExists(Paths.get(filePath)); + Files.deleteIfExists(filePath); } catch (IOException ioe) { throw new AssertionError(ioe); } diff --git a/src/test/java/systemtests/SystemTestSetupHelper.java b/src/test/java/systemtests/SystemTestSetupHelper.java index bc9773f954ba..2f2928cf2726 100644 --- a/src/test/java/systemtests/SystemTestSetupHelper.java +++ b/src/test/java/systemtests/SystemTestSetupHelper.java @@ -1,5 +1,6 @@ package systemtests; +import java.nio.file.Path; import java.util.concurrent.TimeoutException; import java.util.function.Supplier; @@ -20,7 +21,7 @@ public class SystemTestSetupHelper { /** * Sets up a new {@code TestApp} and returns it. */ - public TestApp setupApplication(Supplier addressBook, String saveFileLocation) { + public TestApp setupApplication(Supplier addressBook, Path saveFileLocation) { try { FxToolkit.registerStage(Stage::new); FxToolkit.setupApplication(() -> testApp = new TestApp(addressBook, saveFileLocation)); From c539921c210e43412c6df3ddeda8400c9dc1070e Mon Sep 17 00:00:00 2001 From: Unknown Date: Sun, 15 Apr 2018 02:03:32 +0800 Subject: [PATCH 4/6] FileUtil: use void for the return type of FileUtil#createFile(Path) The return value of FileUtil#createFile(Path) is unused. For code quality purposes, methods with unused return types should have void as a return type instead. Let's use void for the return type of FileUtil#createFile(Path). --- src/main/java/seedu/address/commons/util/FileUtil.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/seedu/address/commons/util/FileUtil.java b/src/main/java/seedu/address/commons/util/FileUtil.java index e2433413483e..c49e7d7959f5 100644 --- a/src/main/java/seedu/address/commons/util/FileUtil.java +++ b/src/main/java/seedu/address/commons/util/FileUtil.java @@ -45,19 +45,16 @@ public static void createIfMissing(Path file) throws IOException { } /** - * Creates a file if it does not exist along with its missing parent directories - * - * @return true if file is created, false if file already exists + * Creates a file if it does not exist along with its missing parent directories. */ - public static boolean createFile(Path file) throws IOException { + public static void createFile(Path file) throws IOException { if (Files.exists(file)) { - return false; + return; } createParentDirsOfFile(file); Files.createFile(file); - return true; } /** From 2afeb5eb35747dff517ed96eb24e12f51aec7990 Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 12 Apr 2018 23:12:48 +0800 Subject: [PATCH 5/6] FileUtil: remove unused methods After refactoring the codebase to use java.nio.file for file/path handling, several methods became unused. Following that, FileUtilTest would be testing unused methods. For code quality purposes, unused methods and tests should be removed. Let's remove unused methods in FileUtil and FileUtilTest. --- .../seedu/address/commons/util/FileUtil.java | 26 ------------------- .../address/commons/util/FileUtilTest.java | 23 ---------------- 2 files changed, 49 deletions(-) diff --git a/src/main/java/seedu/address/commons/util/FileUtil.java b/src/main/java/seedu/address/commons/util/FileUtil.java index c49e7d7959f5..b1e2767cdd92 100644 --- a/src/main/java/seedu/address/commons/util/FileUtil.java +++ b/src/main/java/seedu/address/commons/util/FileUtil.java @@ -1,8 +1,5 @@ package seedu.address.commons.util; -import static seedu.address.commons.util.AppUtil.checkArgument; - -import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.InvalidPathException; @@ -57,19 +54,6 @@ public static void createFile(Path file) throws IOException { Files.createFile(file); } - /** - * Creates the given directory along with its parent directories - * - * @param dir the directory to be created; assumed not null - * @throws IOException if the directory or a parent directory cannot be created - */ - public static void createDirs(Path dir) throws IOException { - if (Files.exists(dir)) { - return; - } - Files.createDirectories(dir); - } - /** * Creates parent directories of file if it has a parent directory */ @@ -96,14 +80,4 @@ public static void writeToFile(Path file, String content) throws IOException { Files.write(file, content.getBytes(CHARSET)); } - /** - * Converts a string to a platform-specific file path - * @param pathWithForwardSlash A String representing a file path but using '/' as the separator - * @return {@code pathWithForwardSlash} but '/' replaced with {@code File.separator} - */ - public static String getPath(String pathWithForwardSlash) { - checkArgument(pathWithForwardSlash.contains("/")); - return pathWithForwardSlash.replace("/", File.separator); - } - } diff --git a/src/test/java/seedu/address/commons/util/FileUtilTest.java b/src/test/java/seedu/address/commons/util/FileUtilTest.java index f4dab9356c9a..4b30ea91ffcb 100644 --- a/src/test/java/seedu/address/commons/util/FileUtilTest.java +++ b/src/test/java/seedu/address/commons/util/FileUtilTest.java @@ -1,37 +1,14 @@ package seedu.address.commons.util; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import java.io.File; - -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import seedu.address.testutil.Assert; public class FileUtilTest { - @Rule - public ExpectedException thrown = ExpectedException.none(); - - @Test - public void getPath() { - - // valid case - assertEquals("folder" + File.separator + "sub-folder", FileUtil.getPath("folder/sub-folder")); - - // null parameter -> throws NullPointerException - thrown.expect(NullPointerException.class); - FileUtil.getPath(null); - - // no forwards slash -> assertion failure - thrown.expect(AssertionError.class); - FileUtil.getPath("folder"); - } - @Test public void isValidPath() { // valid path From 50771f42d4e126bddfe40b2711a0d0e729a467ab Mon Sep 17 00:00:00 2001 From: Unknown Date: Thu, 12 Apr 2018 23:51:33 +0800 Subject: [PATCH 6/6] ConfigUtilTest: use platform-independent path for preferences ConfigUtilTest#getTypicalConfig() uses a path for the user preference file that is only meant for the Windows operating system. This should not be the case as AddressBook is meant to run on all platforms. Let's update ConfigUtilTest#getTypicalConfig() to use a platform-independent path for the user preference file. Following this, ExtraValuesConfig.json and TypicalConfig.json should be updated to reflect this change as well. --- src/test/data/ConfigUtilTest/ExtraValuesConfig.json | 2 +- src/test/data/ConfigUtilTest/TypicalConfig.json | 2 +- src/test/java/seedu/address/commons/util/ConfigUtilTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/data/ConfigUtilTest/ExtraValuesConfig.json b/src/test/data/ConfigUtilTest/ExtraValuesConfig.json index 7e5fa599efdc..413273f7b26f 100644 --- a/src/test/data/ConfigUtilTest/ExtraValuesConfig.json +++ b/src/test/data/ConfigUtilTest/ExtraValuesConfig.json @@ -1,6 +1,6 @@ { "appTitle" : "Typical App Title", "logLevel" : "INFO", - "userPrefsFilePath" : "C:\\preferences.json", + "userPrefsFilePath" : "preferences.json", "extra" : "extra value" } diff --git a/src/test/data/ConfigUtilTest/TypicalConfig.json b/src/test/data/ConfigUtilTest/TypicalConfig.json index 6d035659d7cc..fbf982090fef 100644 --- a/src/test/data/ConfigUtilTest/TypicalConfig.json +++ b/src/test/data/ConfigUtilTest/TypicalConfig.json @@ -1,5 +1,5 @@ { "appTitle" : "Typical App Title", "logLevel" : "INFO", - "userPrefsFilePath" : "C:\\preferences.json" + "userPrefsFilePath" : "preferences.json" } diff --git a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java index 163b5d96bd23..2014aa3a8eda 100644 --- a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java +++ b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java @@ -76,7 +76,7 @@ private Config getTypicalConfig() { Config config = new Config(); config.setAppTitle("Typical App Title"); config.setLogLevel(Level.INFO); - config.setUserPrefsFilePath(Paths.get("C:\\preferences.json")); + config.setUserPrefsFilePath(Paths.get("preferences.json")); return config; }