Merge pull request #200 from jitwei98/jitwei98/tests-for-v1.4

Write tests for import and export function
CS2113-AY1819S1-W12-1 · Nov 12, 2018 · 1a6a944 · 1a6a944
2 parents 7fcdf59 + d1424a4
commit 1a6a944
Show file tree

Hide file tree

Showing 21 changed files with 677 additions and 68 deletions.
diff --git a/src/main/java/seedu/address/commons/util/FileUtil.java b/src/main/java/seedu/address/commons/util/FileUtil.java
@@ -44,8 +44,10 @@ public static boolean isValidXmlFilename(String filename) {
         }
 
         // Compares the filename extension with the expected xml file extension
-        String filenameExtension = filename.substring(filename.length() - XML_FILE_EXTENSION.length());
-        return filenameExtension.equals(XML_FILE_EXTENSION);
+        int indexOfExtension = filename.length() - XML_FILE_EXTENSION.length();
+        String fileExtension = filename.substring(indexOfExtension);
+        String lowercaseFileExtension = fileExtension.toLowerCase();
+        return lowercaseFileExtension.equals(XML_FILE_EXTENSION);
     }
 
     /**

diff --git a/src/main/java/seedu/address/export/Export.java b/src/main/java/seedu/address/export/Export.java
@@ -3,7 +3,8 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
-import javafx.collections.transformation.FilteredList;
+import javafx.collections.ObservableList;
+import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.model.person.Person;
 
 /**
@@ -14,13 +15,14 @@ public interface Export {
     /**
      * Saves the filteredPersons to the storage.
      * @throws IOException if there was any problem writing to the file.
+     * @throws IllegalValueException if the current addressbook is empty.
      */
-    void saveFilteredAddressBook() throws IOException;
+    void saveFilteredPersons() throws IOException, IllegalValueException;
 
     /**
-     * @see #saveFilteredAddressBook()
+     * @see #saveFilteredPersons()
      */
-    void saveFilteredAddressBook(FilteredList<Person> filteredPersons, Path filePath)
-            throws IOException;
+    void saveFilteredPersons(ObservableList<Person> filteredPersons, Path filePath)
+            throws IOException, IllegalValueException;
 
 }
diff --git a/src/main/java/seedu/address/export/ExportManager.java b/src/main/java/seedu/address/export/ExportManager.java
@@ -6,8 +6,9 @@
 import java.nio.file.Path;
 import java.util.logging.Logger;
 
-import javafx.collections.transformation.FilteredList;
+import javafx.collections.ObservableList;
 import seedu.address.commons.core.LogsCenter;
+import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.commons.util.FileUtil;
 import seedu.address.model.person.Person;
 import seedu.address.storage.XmlFileStorage;
@@ -19,36 +20,49 @@
  */
 public class ExportManager implements Export {
 
+    private static final String MESSAGE_NOTHING_TO_EXPORT = "There is nothing to export!";
     private static final Logger logger = LogsCenter.getLogger(seedu.address.export.ExportManager.class);
 
-    private FilteredList<Person> filteredPersons;
+    private ObservableList<Person> filteredPersons;
     private Path exportPath;
 
-    public ExportManager(FilteredList<Person> filteredPersons, Path filePath) {
+    public ExportManager(ObservableList<Person> filteredPersons, Path filePath) {
         this.filteredPersons = filteredPersons;
         this.exportPath = filePath;
     }
 
-    public Path getExportPath() {
+    public Path getExportFilePath() {
         return exportPath;
     }
 
+
     // TODO: add header in the interface header, refer to XmlAddressBookStorage
+    /**
+     * Saves the {@code filteredPersons} to the {@code exportPath}.
+     * @throws IOException if there was any problem writing to the file.
+     * @throws IllegalValueException if the current addressbook is empty.
+     */
     @Override
-    public void saveFilteredAddressBook() throws IOException {
-        saveFilteredAddressBook(filteredPersons, exportPath);
+    public void saveFilteredPersons() throws IOException, IllegalValueException {
+        saveFilteredPersons(filteredPersons, exportPath);
     }
 
     /**
-     * Similar to {@link #saveFilteredAddressBook()}
-     *
-     * @param filePath location of the data. Cannot be null
+     * Similar to {@link #saveFilteredPersons()}
+     * @param filteredPersons cannot be null.
+     * @param filePath file path of the data. Cannot be null
      */
     @Override
-    public void saveFilteredAddressBook(FilteredList<Person> filteredPersons, Path filePath) throws IOException {
+    public void saveFilteredPersons(ObservableList<Person> filteredPersons, Path filePath)
+            throws IOException, IllegalValueException {
         requireNonNull(filteredPersons);
         requireNonNull(filePath);
 
+        if (filteredPersons.size() <= 0) {
+            logger.warning("There is nothing to export!");
+            throw new IllegalValueException(MESSAGE_NOTHING_TO_EXPORT);
+        }
+
         if (FileUtil.isFileExists(filePath)) {
             logger.fine("File exists. Overwriting output file: " + filePath.toString());
         } else {

diff --git a/src/main/java/seedu/address/export/Import.java b/src/main/java/seedu/address/export/Import.java
@@ -0,0 +1,30 @@
+package seedu.address.export;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.Optional;
+
+import seedu.address.commons.exceptions.DataConversionException;
+import seedu.address.model.ReadOnlyAddressBook;
+
+//@@author jitwei98
+/**
+ * The API of the Import component.
+ */
+public interface Import {
+
+    /**
+     * Returns the addressBook from the xml file specified.
+     * @throws DataConversionException if the file is not in the correct format.
+     * @throws FileNotFoundException if the file does not exist
+     */
+    Optional<ReadOnlyAddressBook> readAddressBook() throws DataConversionException, IOException;
+
+    /**
+     * Similar to {@link #readAddressBook()}
+     * @param filePath location of the data. Cannot be null
+     */
+    Optional<ReadOnlyAddressBook> readAddressBook(Path filePath) throws DataConversionException, FileNotFoundException;
+
+}
diff --git a/src/main/java/seedu/address/export/ImportManager.java b/src/main/java/seedu/address/export/ImportManager.java
@@ -21,7 +21,7 @@
 /**
  * Manages importing of AddressBook data.
  */
-public class ImportManager {
+public class ImportManager implements Import {
 
     private static final Logger logger = LogsCenter.getLogger(ImportManager.class);
 
@@ -35,17 +35,12 @@ public Path getImportPath() {
         return importPath;
     }
 
-    // @Override
+    @Override
     public Optional<ReadOnlyAddressBook> readAddressBook() throws DataConversionException, IOException {
         return readAddressBook(importPath);
     }
 
-    /**
-     * Similar to {@link #readAddressBook()}
-     *
-     * @param filePath location of the data. Cannot be null
-     * @throws DataConversionException if the file is not in the correct format.
-     */
+    @Override
     public Optional<ReadOnlyAddressBook> readAddressBook(Path filePath) throws DataConversionException,
             FileNotFoundException {
         requireNonNull(filePath);

diff --git a/src/main/java/seedu/address/logic/commands/ExportCommand.java b/src/main/java/seedu/address/logic/commands/ExportCommand.java
@@ -5,6 +5,7 @@
 import java.io.IOException;
 import java.nio.file.Path;
 
+import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.logic.CommandHistory;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
@@ -25,6 +26,7 @@ public class ExportCommand extends Command {
 
     public static final String MESSAGE_EXPORT_SUCCESS = "Exported persons listed to %1$s";
     public static final String MESSAGE_FAILURE = "Export failed!";
+    public static final String MESSAGE_FAILURE_EMPTY_AB = "There is nothing to export!";
 
     private final Path filePath;
 
@@ -42,6 +44,8 @@ public CommandResult execute(Model model, CommandHistory history) throws Command
             model.exportFilteredAddressBook(filePath);
         } catch (IOException e) {
             throw new CommandException(MESSAGE_FAILURE);
+        } catch (IllegalValueException e) {
+            throw new CommandException(MESSAGE_FAILURE_EMPTY_AB);
         }
 
         return new CommandResult(String.format(MESSAGE_EXPORT_SUCCESS, filePath));

diff --git a/src/main/java/seedu/address/logic/commands/ImportCommand.java b/src/main/java/seedu/address/logic/commands/ImportCommand.java
@@ -1,14 +1,18 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.commons.util.FileUtil.isFileExists;
 
 import java.io.IOException;
 import java.nio.file.Path;
 
+import javafx.collections.ObservableList;
 import seedu.address.commons.exceptions.DataConversionException;
 import seedu.address.logic.CommandHistory;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
+import seedu.address.model.ReadOnlyAddressBook;
+import seedu.address.model.person.Person;
 
 //@@author jitwei98
 /**
@@ -26,6 +30,7 @@ public class ImportCommand extends Command {
     public static final String MESSAGE_IMPORT_SUCCESS = "Imported %1$s persons.";
     private static final String MESSAGE_FAILURE = "Import failed! Error: %1$s";
     private static final String MESSAGE_INVALID_LIST_SIZE = "Invalid list size.";
+    private static final String MESSAGE_FILE_NOT_FOUND = "File not found!";
 
     private final Path filePath;
 
@@ -39,7 +44,13 @@ public ImportCommand(Path filePath) {
     public CommandResult execute(Model model, CommandHistory history) throws CommandException {
         requireNonNull(model);
 
-        int initialNumberOfPersons = model.getFilteredPersonList().size();
+        ReadOnlyAddressBook readOnlyAddressBook = model.getAddressBook();
+        ObservableList<Person> personList = readOnlyAddressBook.getPersonList();
+        int initialNumberOfPersons = personList.size();
+
+        if (!isFileExists(filePath)) {
+            throw new CommandException(MESSAGE_FILE_NOT_FOUND);
+        }
 
         // TODO: Write better Exception messages
         try {
@@ -50,9 +61,10 @@ public CommandResult execute(Model model, CommandHistory history) throws Command
             throw new CommandException(String.format(MESSAGE_FAILURE, dce));
         }
 
-        int finalNumberOfPersons = model.getFilteredPersonList().size();
+        int finalNumberOfPersons = personList.size();
         int personsImported = calculateImportedEntries(initialNumberOfPersons, finalNumberOfPersons);
 
+        model.commitAddressBook();
         return new CommandResult(String.format(MESSAGE_IMPORT_SUCCESS, personsImported));
     }
 

diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java
@@ -6,6 +6,7 @@
 
 import javafx.collections.ObservableList;
 import seedu.address.commons.exceptions.DataConversionException;
+import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.model.person.Person;
 import seedu.address.model.tag.Tag;
 import seedu.address.model.todo.Todo;
@@ -96,13 +97,14 @@ public interface Model {
 
     /**
      * Adds all the persons in {@code addressBookImported} to the current address book.
+     * @return hasChanged is true if the addressBook is modified, returns false otherwise.
      */
-    void addPersonsToAddressBook(ReadOnlyAddressBook addressBookToImported);
+    boolean addPersonsToAddressBook(ReadOnlyAddressBook addressBookToImported);
 
     /**
      * Exports the current filtered person list to a xml file at {@code exportFilePath}.
      */
-    void exportFilteredAddressBook(Path exportFilePath) throws IOException;
+    void exportFilteredAddressBook(Path exportFilePath) throws IOException, IllegalValueException;
 
     /**
      * Exports the current address book state to a .csv file.

diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java
@@ -5,6 +5,7 @@
 
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Predicate;
 import java.util.logging.Logger;
 
@@ -15,9 +16,11 @@
 import seedu.address.commons.core.LogsCenter;
 import seedu.address.commons.events.model.AddressBookChangedEvent;
 import seedu.address.commons.exceptions.DataConversionException;
+import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.export.CsvWriter;
 import seedu.address.export.Export;
 import seedu.address.export.ExportManager;
+import seedu.address.export.Import;
 import seedu.address.export.ImportManager;
 import seedu.address.model.person.Person;
 import seedu.address.model.tag.Tag;
@@ -188,29 +191,34 @@ public void deleteTag(Tag tag) {
     //=========== Import/ Export ==============================================================================
     @Override
     public void importPersonsFromAddressBook(Path importFilePath) throws IOException, DataConversionException {
-        ImportManager importManager = new ImportManager(importFilePath);
+        Import importManager = new ImportManager(importFilePath);
+        ReadOnlyAddressBook addressBookImported = importManager.readAddressBook().orElseThrow(IOException::new);
+        boolean hasChanged = addPersonsToAddressBook(addressBookImported);
 
-        // TODO: dont use null in orElse(), use orElseThrow()
-        ReadOnlyAddressBook addressBookImported = importManager.readAddressBook().orElse(null);
-        addPersonsToAddressBook(addressBookImported);
-
-        updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
-        indicateAddressBookChanged();
+        if (hasChanged) {
+            updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+            indicateAddressBookChanged();
+        }
     }
 
     @Override
-    public void addPersonsToAddressBook(ReadOnlyAddressBook addressBookImported) {
+    public boolean addPersonsToAddressBook(ReadOnlyAddressBook addressBookImported) {
         ObservableList<Person> persons = addressBookImported.getPersonList();
+        AtomicBoolean hasChanged = new AtomicBoolean(false);
         persons.forEach((person) -> {
             // TODO: explain why this instead of addPerson() above in developer guide (indicate ab changed at the end)
-            versionedAddressBook.addPerson(person);
+            if (!hasPerson(person)) {
+                hasChanged.set(true);
+                versionedAddressBook.addPerson(person);
+            }
         });
+        return hasChanged.get();
     }
 
     @Override
-    public void exportFilteredAddressBook(Path exportFilePath) throws IOException {
-        Export export = new ExportManager(filteredPersons, exportFilePath);
-        export.saveFilteredAddressBook();
+    public void exportFilteredAddressBook(Path exportFilePath) throws IOException, IllegalValueException {
+        Export export = new ExportManager(getFilteredPersonList(), exportFilePath);
+        export.saveFilteredPersons();
     }
 
     @Override

diff --git a/src/main/java/seedu/address/storage/XmlSerializableAddressBook.java b/src/main/java/seedu/address/storage/XmlSerializableAddressBook.java
@@ -7,7 +7,7 @@
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
-import javafx.collections.transformation.FilteredList;
+import javafx.collections.ObservableList;
 import seedu.address.commons.exceptions.IllegalValueException;
 import seedu.address.model.AddressBook;
 import seedu.address.model.ReadOnlyAddressBook;
@@ -50,7 +50,7 @@ public XmlSerializableAddressBook(ReadOnlyAddressBook src) {
     /**
      * Conversion with filtered Persons instead of the whole address book.
      */
-    public XmlSerializableAddressBook(FilteredList<Person> filteredPersons) {
+    public XmlSerializableAddressBook(ObservableList<Person> filteredPersons) {
         this();
         persons.addAll(filteredPersons.stream().map(XmlAdaptedPerson::new).collect(Collectors.toList()));
     }

diff --git a/src/test/java/seedu/address/commons/util/FileUtilTest.java b/src/test/java/seedu/address/commons/util/FileUtilTest.java
@@ -21,4 +21,25 @@ public void isValidPath() {
         Assert.assertThrows(NullPointerException.class, () -> FileUtil.isValidPath(null));
     }
 
+    @Test
+    public void isValidXmlFilename() {
+        // valid xml filename
+        assertTrue(FileUtil.isValidXmlFilename("someXmlFile.xml"));
+        assertTrue(FileUtil.isValidXmlFilename("anotherXmlFile.xml"));
+        assertTrue(FileUtil.isValidXmlFilename("someXmlFile.XML"));
+
+        // invalid filename, not .xml
+        assertFalse(FileUtil.isValidXmlFilename("someXmlFile.pdf"));
+
+        // invalid filename, empty string
+        assertFalse(FileUtil.isValidXmlFilename(""));
+
+        // invalid filename, no extension
+        assertFalse(FileUtil.isValidXmlFilename("someXmlFile"));
+
+        // null filename -> throws NullPointerException
+        Assert.assertThrows(NullPointerException.class, () -> FileUtil.isValidXmlFilename(null));
+
+    }
+
 }