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

Commit

Permalink
[#750] Use java.nio.file for file/path handling (#866)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yamgent authored Apr 22, 2018
2 parents c8f40d8 + 50771f4 commit e968a29
Show file tree
Hide file tree
Showing 34 changed files with 340 additions and 215 deletions.
64 changes: 64 additions & 0 deletions src/main/java/seedu/address/AppParameters.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
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 Path configPath;

public Path getConfigPath() {
return configPath;
}

public void setConfigPath(Path configPath) {
this.configPath = configPath;
}

/**
* Parses the application command-line parameters.
*/
public static AppParameters parse(Application.Parameters parameters) {
AppParameters appParameters = new AppParameters();
Map<String, String> namedParameters = parameters.getNamed();

String configPathParameter = namedParameters.get("config");
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;
}

@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();
}
}
16 changes: 6 additions & 10 deletions src/main/java/seedu/address/MainApp.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package seedu.address;

import java.io.IOException;
import java.util.Map;
import java.nio.file.Path;
import java.util.Optional;
import java.util.logging.Logger;

Expand Down Expand Up @@ -57,7 +57,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);
Expand All @@ -75,11 +76,6 @@ public void init() throws Exception {
initEventsCenter();
}

private String getApplicationParameter(String parameterName) {
Map<String, String> applicationParameters = getParameters().getNamed();
return applicationParameters.get(parameterName);
}

/**
* Returns a {@code ModelManager} with the data from {@code storage}'s address book and {@code userPrefs}. <br>
* The data from the sample address book will be used instead if {@code storage}'s address book is not found,
Expand Down Expand Up @@ -114,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;

Expand Down Expand Up @@ -151,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;
Expand Down
10 changes: 6 additions & 4 deletions src/main/java/seedu/address/commons/core/Config.java
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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;
Expand All @@ -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;
}

Expand Down
5 changes: 3 additions & 2 deletions src/main/java/seedu/address/commons/util/ConfigUtil.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -11,11 +12,11 @@
*/
public class ConfigUtil {

public static Optional<Config> readConfig(String configFilePath) throws DataConversionException {
public static Optional<Config> 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);
}

Expand Down
74 changes: 32 additions & 42 deletions src/main/java/seedu/address/commons/util/FileUtil.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
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;
import java.nio.file.Path;
import java.nio.file.Paths;

/**
* Writes and reads files
Expand All @@ -13,81 +13,71 @@ 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);
}

/**
* 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.
*/
public static void createIfMissing(File file) throws IOException {
public static void createIfMissing(Path file) throws IOException {
if (!isFileExists(file)) {
createFile(file);
}
}

/**
* 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(File file) throws IOException {
if (file.exists()) {
return false;
public static void createFile(Path file) throws IOException {
if (Files.exists(file)) {
return;
}

createParentDirsOfFile(file);

return file.createNewFile();
}

/**
* 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(File dir) throws IOException {
if (!dir.exists() && !dir.mkdirs()) {
throw new IOException("Failed to make directories of " + dir.getName());
}
Files.createFile(file);
}

/**
* 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));
}

/**
* 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);
public static void writeToFile(Path file, String content) throws IOException {
Files.write(file, content.getBytes(CHARSET));
}

}
22 changes: 11 additions & 11 deletions src/main/java/seedu/address/commons/util/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

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.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -38,11 +39,11 @@ public class JsonUtil {
.addSerializer(Level.class, new ToStringSerializer())
.addDeserializer(Level.class, new LevelDeserializer(Level.class)));

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

static <T> T deserializeObjectFromJsonFile(File jsonFile, Class<T> classOfObjectToDeserialize)
static <T> T deserializeObjectFromJsonFile(Path jsonFile, Class<T> classOfObjectToDeserialize)
throws IOException {
return fromJsonString(FileUtil.readFromFile(jsonFile), classOfObjectToDeserialize);
}
Expand All @@ -55,21 +56,20 @@ static <T> T deserializeObjectFromJsonFile(File jsonFile, Class<T> classOfObject
* @throws DataConversionException if the file format is not as expected.
*/
public static <T> Optional<T> readJsonFile(
String filePath, Class<T> classOfObjectToDeserialize) throws DataConversionException {
Path filePath, Class<T> classOfObjectToDeserialize) throws DataConversionException {
requireNonNull(filePath);
File file = new File(filePath);

if (!file.exists()) {
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);
}

Expand All @@ -83,11 +83,11 @@ public static <T> Optional<T> readJsonFile(
* @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 {
public static <T> void saveJsonFile(T jsonFile, Path filePath) throws IOException {
requireNonNull(filePath);
requireNonNull(jsonFile);

serializeObjectToJsonFile(new File(filePath), jsonFile);
serializeObjectToJsonFile(filePath, jsonFile);
}


Expand Down
Loading

0 comments on commit e968a29

Please sign in to comment.