Skip to content

Commit

Permalink
Improve error message if no write permission
Browse files Browse the repository at this point in the history
Insufficient permission to write to the data file will show up as
AccessDeniedException in the error message.
The application should be able to describe the error to the user rather
than just showing the name of the Exception.

Let's
- make the error message more descriptive by informing
the user that there are insufficient permissions for the file for
AccesssDeniedExceptions.

- Also include a better general purpose description for other forms
of IO related exceptions
  • Loading branch information
Eclipse-Dominator authored and damithc committed Jul 13, 2023
1 parent d98c36e commit c07fb9e
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 28 deletions.
11 changes: 9 additions & 2 deletions src/main/java/seedu/address/logic/LogicManager.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package seedu.address.logic;

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

Expand All @@ -21,7 +22,11 @@
* The main LogicManager of the app.
*/
public class LogicManager implements Logic {
public static final String FILE_OPS_ERROR_MESSAGE = "Could not save data to file: ";
public static final String FILE_OPS_ERROR_FORMAT = "Could not save data due to the following error: %s";

public static final String FILE_OPS_PERMISSION_ERROR_FORMAT =
"Could not save data to file %s due to insufficient permissions to write to the file or the folder.";

private final Logger logger = LogsCenter.getLogger(LogicManager.class);

private final Model model;
Expand All @@ -47,8 +52,10 @@ public CommandResult execute(String commandText) throws CommandException, ParseE

try {
storage.saveAddressBook(model.getAddressBook());
} catch (AccessDeniedException e) {
throw new CommandException(String.format(FILE_OPS_PERMISSION_ERROR_FORMAT, e.getMessage()), e);
} catch (IOException ioe) {
throw new CommandException(FILE_OPS_ERROR_MESSAGE + ioe, ioe);
throw new CommandException(String.format(FILE_OPS_ERROR_FORMAT, ioe.getMessage()), ioe);
}

return commandResult;
Expand Down
65 changes: 39 additions & 26 deletions src/test/java/seedu/address/logic/LogicManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static seedu.address.testutil.TypicalPersons.AMY;

import java.io.IOException;
import java.nio.file.AccessDeniedException;
import java.nio.file.Path;

import org.junit.jupiter.api.BeforeEach;
Expand All @@ -33,7 +34,8 @@
import seedu.address.testutil.PersonBuilder;

public class LogicManagerTest {
private static final IOException DUMMY_IO_EXCEPTION = new IOException("dummy exception");
private static final IOException DUMMY_IO_EXCEPTION = new IOException("dummy IO exception");
private static final IOException DUMMY_AD_EXCEPTION = new AccessDeniedException("dummy access denied exception");

@TempDir
public Path temporaryFolder;
Expand Down Expand Up @@ -70,22 +72,14 @@ public void execute_validCommand_success() throws Exception {

@Test
public void execute_storageThrowsIoException_throwsCommandException() {
// Setup LogicManager with JsonAddressBookIoExceptionThrowingStub
JsonAddressBookStorage addressBookStorage =
new JsonAddressBookIoExceptionThrowingStub(temporaryFolder.resolve("ioExceptionAddressBook.json"));
JsonUserPrefsStorage userPrefsStorage =
new JsonUserPrefsStorage(temporaryFolder.resolve("ioExceptionUserPrefs.json"));
StorageManager storage = new StorageManager(addressBookStorage, userPrefsStorage);
logic = new LogicManager(model, storage);
assertCommandFailureForExceptionFromStorage(DUMMY_IO_EXCEPTION, String.format(
LogicManager.FILE_OPS_ERROR_FORMAT, DUMMY_IO_EXCEPTION.getMessage()));
}

// Execute add command
String addCommand = AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY
+ ADDRESS_DESC_AMY;
Person expectedPerson = new PersonBuilder(AMY).withTags().build();
ModelManager expectedModel = new ModelManager();
expectedModel.addPerson(expectedPerson);
String expectedMessage = LogicManager.FILE_OPS_ERROR_MESSAGE + DUMMY_IO_EXCEPTION;
assertCommandFailure(addCommand, CommandException.class, expectedMessage, expectedModel);
@Test
public void execute_storageThrowsAdException_throwsCommandException() {
assertCommandFailureForExceptionFromStorage(DUMMY_AD_EXCEPTION, String.format(
LogicManager.FILE_OPS_PERMISSION_ERROR_FORMAT, DUMMY_AD_EXCEPTION.getMessage()));
}

@Test
Expand Down Expand Up @@ -147,16 +141,35 @@ private void assertCommandFailure(String inputCommand, Class<? extends Throwable
}

/**
* A stub class to throw an {@code IOException} when the save method is called.
* Tests the Logic component's handling of an {@code IOException} thrown by the Storage component.
*
* @param e the exception to be thrown by the Storage component
* @param expectedMessage the message expected inside exception thrown by the Logic component
*/
private static class JsonAddressBookIoExceptionThrowingStub extends JsonAddressBookStorage {
private JsonAddressBookIoExceptionThrowingStub(Path filePath) {
super(filePath);
}

@Override
public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException {
throw DUMMY_IO_EXCEPTION;
}
private void assertCommandFailureForExceptionFromStorage(IOException e, String expectedMessage) {
Path prefPath = temporaryFolder.resolve("ExceptionUserPrefs.json");

// Inject LogicManager with an AddressBookStorage that throws the IOException e when saving
JsonAddressBookStorage addressBookStorage = new JsonAddressBookStorage(prefPath) {
@Override
public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath)
throws IOException {
throw e;
}
};

JsonUserPrefsStorage userPrefsStorage =
new JsonUserPrefsStorage(temporaryFolder.resolve("ExceptionUserPrefs.json"));
StorageManager storage = new StorageManager(addressBookStorage, userPrefsStorage);

logic = new LogicManager(model, storage);

// Triggers the saveAddressBook method by executing an add command
String addCommand = AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY
+ EMAIL_DESC_AMY + ADDRESS_DESC_AMY;
Person expectedPerson = new PersonBuilder(AMY).withTags().build();
ModelManager expectedModel = new ModelManager();
expectedModel.addPerson(expectedPerson);
assertCommandFailure(addCommand, CommandException.class, expectedMessage, expectedModel);
}
}

0 comments on commit c07fb9e

Please sign in to comment.