diff --git a/.travis.yml b/.travis.yml index 1ffe1f2a5c77..c9af1c26c73a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,11 @@ language: java matrix: include: - - jdk: oraclejdk9 + - jdk: oraclejdk8 script: >- ./config/travis/run-checks.sh && - travis_retry ./gradlew clean checkstyleMain checkstyleTest headless allTests coverage coveralls asciidoctor copyDummySearchPage + ./gradlew clean checkstyleMain checkstyleTest headless allTests coverage coveralls asciidoctor deploy: skip_cleanup: true @@ -17,7 +17,7 @@ deploy: addons: apt: packages: - - oracle-java9-installer + - oracle-java8-installer before_cache: - rm -f $HOME/.gradle/caches/modules-2/modules-2.lock diff --git a/README.adoc b/README.adoc index 142ae1b17fbd..4bbb540cb592 100644 --- a/README.adoc +++ b/README.adoc @@ -35,7 +35,7 @@ endif::[] * Some parts of this sample application were inspired by the excellent http://code.makery.ch/library/javafx-8-tutorial/[Java FX tutorial] by _Marco Jakob_. -* Libraries used: https://github.com/TestFX/TestFX[TextFX], https://bitbucket.org/controlsfx/controlsfx/[ControlsFX], https://github.com/FasterXML/jackson[Jackson], https://github.com/google/guava[Guava], https://github.com/junit-team/junit5[JUnit5] +* Libraries used: https://github.com/TestFX/TestFX[TestFX], https://github.com/FasterXML/jackson[Jackson], https://github.com/junit-team/junit5[JUnit5] == Licence : link:LICENSE[MIT] diff --git a/appveyor.yml b/appveyor.yml index 03b94b033ffd..62c6b2d56607 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,10 +8,10 @@ build_script: - gradlew.bat --no-daemon assemble checkstyleMain checkstyleTest test_script: - - appveyor-retry gradlew.bat --no-daemon headless allTests + - gradlew.bat --no-daemon headless allTests environment: - JAVA_HOME: C:\Program Files\Java\jdk9 # Use 64-bit Java + JAVA_HOME: C:\Program Files\Java\jdk1.8.0 # Use 64-bit Java # Files/folders to preserve between builds to speed them up cache: diff --git a/build.gradle b/build.gradle index f8e614f8b49b..7fd1d35ff987 100644 --- a/build.gradle +++ b/build.gradle @@ -14,24 +14,11 @@ plugins { id 'application' } -if (JavaVersion.current() == JavaVersion.VERSION_1_10 - && System.getProperty('os.name').startsWith('Windows')) { - logger.warn('''\ - ============================================================================== - *********************************** WARNING ********************************** - ============================================================================== - You seem to be running Gradle with JDK 10 on Windows. - JDK 10 on Windows will fail to run tests in headless mode due to a JavaFX bug. - You are highly recommended to use JDK 9! - ============================================================================== - '''.stripIndent()) -} - // Specifies the entry point of the application mainClassName = 'seedu.address.MainApp' -sourceCompatibility = JavaVersion.VERSION_1_9 -targetCompatibility = JavaVersion.VERSION_1_9 +sourceCompatibility = JavaVersion.VERSION_1_8 +targetCompatibility = JavaVersion.VERSION_1_8 repositories { mavenCentral() @@ -58,25 +45,16 @@ dependencies { String testFxVersion = '4.0.12-alpha' String jUnitVersion = '5.1.0' - implementation group: 'org.controlsfx', name: 'controlsfx', version: '8.40.11' implementation group: 'com.fasterxml.jackson.core', name: 'jackson-databind', version: '2.7.0' implementation group: 'com.fasterxml.jackson.datatype', name: 'jackson-datatype-jsr310', version: '2.7.4' - implementation group: 'com.google.guava', name: 'guava', version: '19.0' - implementation group: 'javax.xml.bind', name: 'jaxb-api', version: '2.2.8' - implementation group: 'com.sun.xml.bind', name: 'jaxb-impl', version: '2.3.0' - implementation group: 'com.sun.xml.bind', name: 'jaxb-core', version: '2.3.0' - implementation group: 'javax.activation', name: 'activation', version: '1.1.1' testImplementation group: 'junit', name: 'junit', version: '4.12' - testImplementation group: 'org.testfx', name: 'testfx-core', version: testFxVersion, { - exclude group: 'org.testfx', module: 'testfx-internal-java8' - } + testImplementation group: 'org.testfx', name: 'testfx-core', version: testFxVersion testImplementation group: 'org.testfx', name: 'testfx-junit', version: testFxVersion testImplementation group: 'org.junit.jupiter', name: 'junit-jupiter-api', version: jUnitVersion - testRuntimeOnly group: 'org.testfx', name: 'testfx-internal-java9', version: testFxVersion - testRuntimeOnly group: 'org.testfx', name: 'openjfx-monocle', version: 'jdk-9+181' + testRuntimeOnly group: 'org.testfx', name: 'openjfx-monocle', version: '8u76-b04' testRuntimeOnly group:'org.junit.vintage', name:'junit-vintage-engine', version: jUnitVersion testRuntimeOnly group: 'org.junit.jupiter', name: 'junit-jupiter-engine', version: jUnitVersion } @@ -236,11 +214,6 @@ task deployOfflineDocs(type: Copy) { } } -task copyDummySearchPage(type: Copy) { - from 'docs/DummySearchPage.html' - into "${buildDir}/docs/html5" -} - deployOfflineDocs.dependsOn asciidoctor processResources.dependsOn deployOfflineDocs diff --git a/config/checkstyle/checkstyle.xml b/config/checkstyle/checkstyle.xml index 6a5341b6f4c7..a9671a144efd 100644 --- a/config/checkstyle/checkstyle.xml +++ b/config/checkstyle/checkstyle.xml @@ -356,6 +356,25 @@ + + + + + + diff --git a/docs/DeveloperGuide.adoc b/docs/DeveloperGuide.adoc index ea58481e4740..c5d5efc8dd1d 100644 --- a/docs/DeveloperGuide.adoc +++ b/docs/DeveloperGuide.adoc @@ -20,12 +20,12 @@ By: `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT` === Prerequisites -. *JDK `9`* or later +. *JDK `8`* (revision `1.8.0_201` or later) ++ +[NOTE] +Only JDK 8 is supported. + +This app will not work with later major JDK releases such as JDK 9, 10, 11, etc. + -[WARNING] -JDK `10` on Windows will fail to run tests in <> due to a https://github.com/javafxports/openjdk-jfx/issues/66[JavaFX bug]. -Windows developers are highly recommended to use JDK `9`. - . *IntelliJ* IDE + [NOTE] @@ -116,9 +116,9 @@ The `.pptx` files used to create diagrams in this document can be found in the l * At app launch: Initializes the components in the correct sequence, and connects them up with each other. * At shut down: Shuts down the components and invokes cleanup method where necessary. -<> represents a collection of classes used by multiple other components. Two of those classes play important roles at the architecture level. +<> represents a collection of classes used by multiple other components. +The following class plays an important role at the architecture level: -* `EventsCenter` : This class (written using https://github.com/google/guava/wiki/EventBusExplained[Google's Event Bus library]) is used by components to communicate with other components using events (i.e. a form of _Event Driven_ design) * `LogsCenter` : Used by many classes to write log messages to the App's log file. The rest of the App consists of four components. @@ -139,24 +139,13 @@ For example, the `Logic` component (see the class diagram given below) defines i image::LogicClassDiagram.png[width="800"] [discrete] -==== Events-Driven nature of the design +==== How the architecture components interact with each other -The _Sequence Diagram_ below shows how the components interact for the scenario where the user issues the command `delete 1`. +The _Sequence Diagram_ below shows how the components interact with each other for the scenario where the user issues the command `delete 1`. -.Component interactions for `delete 1` command (part 1) +.Component interactions for `delete 1` command image::SDforDeletePerson.png[width="800"] -[NOTE] -Note how the `Model` simply raises a `AddressBookChangedEvent` when the Address Book data are changed, instead of asking the `Storage` to save the updates to the hard disk. - -The diagram below shows how the `EventsCenter` reacts to that event, which eventually results in the updates being saved to the hard disk and the status bar of the UI being updated to reflect the 'Last Updated' time. - -.Component interactions for `delete 1` command (part 2) -image::SDforDeletePersonEventHandling.png[width="800"] - -[NOTE] -Note how the event is propagated through the `EventsCenter` to the `Storage` and `UI` without `Model` having to be coupled to either of them. This is an example of how this Event Driven approach helps us reduce direct coupling between components. - The sections below give more details of each component. [[Design-Ui]] @@ -174,8 +163,7 @@ The `UI` component uses JavaFx UI framework. The layout of these UI parts are de The `UI` component, * Executes user commands using the `Logic` component. -* Binds itself to some data in the `Model` so that the UI can auto-update when data in the `Model` change. -* Responds to events raised from various parts of the App and updates the UI accordingly. +* Listens for changes to `Model` data so that the UI can be updated with the modified data. [[Design-Logic]] === Logic component @@ -189,8 +177,9 @@ link:{repoURL}/src/main/java/seedu/address/logic/Logic.java[`Logic.java`] . `Logic` uses the `AddressBookParser` class to parse the user command. . This results in a `Command` object which is executed by the `LogicManager`. -. The command execution can affect the `Model` (e.g. adding a person) and/or raise events. +. The command execution can affect the `Model` (e.g. adding a person). . The result of the command execution is encapsulated as a `CommandResult` object which is passed back to the `Ui`. +. In addition, the `CommandResult` object can also instruct the `Ui` to perform certain actions, such as displaying help to the user. Given below is the Sequence Diagram for interactions within the `Logic` component for the `execute("delete 1")` API call. @@ -228,7 +217,7 @@ image::StorageClassDiagram.png[width="800"] The `Storage` component, * can save `UserPref` objects in json format and read it back. -* can save the Address Book data in xml format and read it back. +* can save the Address Book data in json format and read it back. [[Design-Commons]] === Common classes @@ -344,7 +333,7 @@ We are using `java.util.logging` package for logging. The `LogsCenter` class is [[Implementation-Configuration]] === Configuration -Certain properties of the application can be controlled (e.g App name, logging level) through the configuration file (default: `config.json`). +Certain properties of the application can be controlled (e.g user prefs file location, logging level) through the configuration file (default: `config.json`). == Documentation @@ -528,9 +517,11 @@ Here are the steps to create a new release. === Managing Dependencies -A project often depends on third-party libraries. For example, Address Book depends on the http://wiki.fasterxml.com/JacksonHome[Jackson library] for XML parsing. Managing these _dependencies_ can be automated using Gradle. For example, Gradle can download the dependencies automatically, which is better than these alternatives. + -a. Include those libraries in the repo (this bloats the repo size) + -b. Require developers to download those libraries manually (this creates extra work for developers) +A project often depends on third-party libraries. For example, Address Book depends on the https://github.com/FasterXML/jackson[Jackson library] for JSON parsing. Managing these _dependencies_ can be automated using Gradle. For example, Gradle can download the dependencies automatically, which is better than these alternatives: + +[loweralpha] +. Include those libraries in the repo (this bloats the repo size) +. Require developers to download those libraries manually (this creates extra work for developers) [[GetStartedProgramming]] [appendix] @@ -678,7 +669,7 @@ Do take a look at <> before attempting to modify the `Storage` c **** * Hint ** Add the API method in link:{repoURL}/src/main/java/seedu/address/storage/AddressBookStorage.java[`AddressBookStorage`] interface. -** Implement the logic in link:{repoURL}/src/main/java/seedu/address/storage/StorageManager.java[`StorageManager`] and link:{repoURL}/src/main/java/seedu/address/storage/XmlAddressBookStorage.java[`XmlAddressBookStorage`] class. +** Implement the logic in link:{repoURL}/src/main/java/seedu/address/storage/StorageManager.java[`StorageManager`] and link:{repoURL}/src/main/java/seedu/address/storage/JsonAddressBookStorage.java[`JsonAddressBookStorage`] class. * Solution ** See this https://github.com/se-edu/addressbook-level4/pull/594[PR] for the full solution. **** @@ -763,18 +754,18 @@ Now we have the `Remark` class, we need to actually use it inside link:{repoURL} . Add `getRemark()` in link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`]. . You may assume that the user will not be able to use the `add` and `edit` commands to modify the remarks field (i.e. the person will be created without a remark). -. Modify link:{repoURL}/src/main/java/seedu/address/model/util/SampleDataUtil.java/[`SampleDataUtil`] to add remarks for the sample data (delete your `addressBook.xml` so that the application will load the sample data when you launch it.) +. Modify link:{repoURL}/src/main/java/seedu/address/model/util/SampleDataUtil.java/[`SampleDataUtil`] to add remarks for the sample data (delete your `data/addressbook.json` so that the application will load the sample data when you launch it.) -===== [Step 6] Storage: Add `Remark` field to `XmlAdaptedPerson` class -We now have `Remark` s for `Person` s, but they will be gone when we exit the application. Let's modify link:{repoURL}/src/main/java/seedu/address/storage/XmlAdaptedPerson.java[`XmlAdaptedPerson`] to include a `Remark` field so that it will be saved. +===== [Step 6] Storage: Add `Remark` field to `JsonAdaptedPerson` class +We now have `Remark` s for `Person` s, but they will be gone when we exit the application. Let's modify link:{repoURL}/src/main/java/seedu/address/storage/JsonAdaptedPerson.java[`JsonAdaptedPerson`] to include a `Remark` field so that it will be saved. **Main:** -. Add a new Xml field for `Remark`. +. Add a new JSON field for `Remark`. **Tests:** -. Fix `invalidAndValidPersonAddressBook.xml`, `typicalPersonsAddressBook.xml`, `validAddressBook.xml` etc., such that the XML tests will not fail due to a missing `` element. +. Fix `invalidAndValidPersonAddressBook.json`, `typicalPersonsAddressBook.json`, `validAddressBook.json` etc., such that the JSON tests will not fail due to a missing `remark` field. ===== [Step 6b] Test: Add withRemark() for `PersonBuilder` Since `Person` can now have a `Remark`, we should add a helper method to link:{repoURL}/src/test/java/seedu/address/testutil/PersonBuilder.java[`PersonBuilder`], so that users are able to create remarks when building a link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`]. @@ -882,7 +873,7 @@ _{More to be added}_ [appendix] == Non Functional Requirements -. Should work on any <> as long as it has Java `9` or higher installed. +. Should work on any <> as long as it has Java `8` (revision `1.8.0_201` or higher) installed. . Should be able to hold up to 1000 persons without a noticeable sluggishness in performance for typical usage. . A user with above average typing speed for regular English text (i.e. not code, not system admin commands) should be able to accomplish most of the tasks faster using commands than using the mouse. diff --git a/docs/DummySearchPage.html b/docs/DummySearchPage.html deleted file mode 100644 index 1607d4c57291..000000000000 --- a/docs/DummySearchPage.html +++ /dev/null @@ -1,32 +0,0 @@ - - - - - Dummy Search Page - - - - Hi : This is a placeholder page for se-edu/addressbook-level4.
- You may update the code to load a page from a real service (e.g., Google search).
- This dummy page is used here because, given the high number of forks of this repo, loading a page from a real third-party service by default can result in that service taking counter-measures (e.g., redirecting to captcha pages) due to the high number of rapid requests received from a single IP.
- When you have made the change, please remove: -
    -
  1. This file (docs/DummySearchPage.html).
    2. -
  2. Task copyDummySearchPage in both build.gradle and .travis.yml.
    3. -
- - diff --git a/docs/LearningOutcomes.adoc b/docs/LearningOutcomes.adoc index 83cda0927226..2837c28b72c7 100644 --- a/docs/LearningOutcomes.adoc +++ b/docs/LearningOutcomes.adoc @@ -28,10 +28,12 @@ Note how the <> describes the high == Use Event-Driven Programming `[LO-EventDriven]` -Note how the <> uses events to communicate with components without needing a direct coupling. Also note how the link:{repoURL}/src/main/java/seedu/address/commons/core/index/EventsCenter.java[`EventsCenter.java`] acts as an event dispatcher to facilitate communication between event creators and event consumers. +The JavaFX framework, which this code base uses for its UI, uses _events_ to communicate user input (such as mouse movement and button presses) to interested _event consumers_. +The JavaFX event loop acts as an _event dispatcher_ to transmit events to event consumers. *Resources* +* https://docs.oracle.com/javase/8/javafx/events-tutorial/events.htm[JavaFX: Handling Events] * https://se-edu.github.io/se-book/architecture/architecturalStyles/eventDriven/[se-edu/se-book: Design: Architecture: Architecture Styles: Event-Driven Architectural Style] ''' @@ -109,7 +111,7 @@ Note <> used by this code base employs the Observer pattern. For example, objects that are interested in events need to have the `@Subscribe` annotation in the class (this is similar to implementing an `\<>` interface) and register with the `EventsCenter`. When something noteworthy happens, an event is raised and the `EventsCenter` notifies all relevant subscribers. Unlike in the Observer pattern in which the `\<>` class is notifying all `\<>` objects, here the `\<>` classes simply raises an event and the `EventsCenter` takes care of the notifications. +* *Observer Pattern*: The code base uses JavaFX's https://docs.oracle.com/javase/8/javafx/api/javafx/beans/value/ObservableValue.html[`ObservableValue`] interface to allow objects to watch for changes in data. +This interface is an implementation of the Observer pattern. +Objects that are interested in changes to an `ObservableValue` can register a listener via `ObservableValue#addListener(...)`. +When the value represented by the `ObservableValue` changes, it will notify all registered listeners. * *MVC Pattern* : ** The 'View' part of the application is mostly in the `.fxml` files in the `src/main/resources/view` folder. ** `Model` component contains the 'Model'. However, note that it is possible to view the `Logic` as the model because it hides the `Model` behind it and the view has to go through the `Logic` to access the `Model`. diff --git a/docs/UserGuide.adoc b/docs/UserGuide.adoc index 7e0070e12f49..eba4fb3bccd9 100644 --- a/docs/UserGuide.adoc +++ b/docs/UserGuide.adoc @@ -22,7 +22,12 @@ AddressBook Level 4 (AB4) is for those who *prefer to use a desktop app for mana == Quick Start -. Ensure you have Java version `9` or later installed in your Computer. +. Ensure you have Java `8` (revision `1.8.0_201` or later) installed in your Computer. ++ +[NOTE] +Only Java 8 is supported. + +This app will not work with later major Java releases such as Java 9, 10, 11, etc. ++ . Download the latest `addressbook.jar` link:{repoURL}/releases[here]. . Copy the file to the folder you want to use as the home folder for your Address Book. . Double-click the file to start the app. The GUI should appear in a few seconds. diff --git a/docs/UsingAppVeyor.adoc b/docs/UsingAppVeyor.adoc index ba15f7113fc5..50d21fff39d3 100644 --- a/docs/UsingAppVeyor.adoc +++ b/docs/UsingAppVeyor.adoc @@ -7,10 +7,7 @@ https://www.appveyor.com/[AppVeyor] is a _Continuous Integration_ platform for G AppVeyor can run the project's tests automatically whenever new code is pushed to the repo. This ensures that existing functionality and features have not been broken on Windows by the changes. -The current AppVeyor setup performs the following things whenever someone pushes code to the repo: - -* Runs the `gradlew.bat headless allTests` command. -* Automatically retries the build up to 3 times if a task fails. +The current AppVeyor setup runs `gradlew.bat headless allTests` whenever someone pushes code to the repo. If you would like to customize your AppVeyor build further, you can learn more about AppVeyor from the https://www.appveyor.com/docs/[AppVeyor Documentation]. diff --git a/docs/UsingGradle.adoc b/docs/UsingGradle.adoc index d1be2f3b7c3a..e8bef729afd8 100644 --- a/docs/UsingGradle.adoc +++ b/docs/UsingGradle.adoc @@ -6,7 +6,6 @@ ifdef::env-github[] :tip-caption: :bulb: :note-caption: :information_source: -:warning-caption: :warning: endif::[] https://gradle.org/[Gradle] is a build automation tool. It can automate build-related tasks such as @@ -77,7 +76,6 @@ Runs the code style check for the test code base The set of code style rules implemented can be found in `config/checkstyle/checkstyle.xml`. To enable _exceptions_ to code styles, add in the comment `//CODESTYLE.OFF: RuleName` at the start of the section and `//CODESTYLE.ON: RuleName` at the end of the section. -[[Running-Tests]] == Running Tests * **`allTests`** + @@ -89,10 +87,6 @@ Runs all non-GUI tests in the `seedu.address` package * **`headless`** + Sets the test mode as _headless_. The mode is effective for that Gradle run only so it should be combined with other test tasks. -+ -[WARNING] -JDK `10` on Windows will fail to run tests in headless mode due to a https://github.com/javafxports/openjdk-jfx/issues/66[JavaFX bug]. -Windows developers are highly recommended to use JDK `9`. Here are some examples: diff --git a/docs/UsingTravis.adoc b/docs/UsingTravis.adoc index 8418759fa5d3..0e383636f7b3 100644 --- a/docs/UsingTravis.adoc +++ b/docs/UsingTravis.adoc @@ -10,7 +10,6 @@ Travis CI can run the projects' tests automatically whenever new code is pushed The current Travis CI set up performs the following things whenever someone push code to the repo: * Runs the `./gradlew clean headless allTests coverage coveralls -i` command (see <> for more details on what this command means). -* Automatically retries the build up to 3 times if a task fails. * Renders documentation from asciidoc to html and automatically publishes them using GitHub Pages. * Runs additional link:#repository-wide-checks[repository-wide checks]. diff --git a/docs/diagrams/ArchitectureDiagram.pptx b/docs/diagrams/ArchitectureDiagram.pptx index b0e5a9d0ff55..13051e815c46 100644 Binary files a/docs/diagrams/ArchitectureDiagram.pptx and b/docs/diagrams/ArchitectureDiagram.pptx differ diff --git a/docs/diagrams/LogicComponentSequenceDiagram.pptx b/docs/diagrams/LogicComponentSequenceDiagram.pptx index c5b6d5fad6e3..410277bbd866 100644 Binary files a/docs/diagrams/LogicComponentSequenceDiagram.pptx and b/docs/diagrams/LogicComponentSequenceDiagram.pptx differ diff --git a/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx b/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx index d0561dfd305a..a0b23659eb29 100644 Binary files a/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx and b/docs/diagrams/ModelComponentClassBetterOopDiagram.pptx differ diff --git a/docs/diagrams/ModelComponentClassDiagram.pptx b/docs/diagrams/ModelComponentClassDiagram.pptx index 3c976908eaa7..dc0e4ac5ea66 100644 Binary files a/docs/diagrams/ModelComponentClassDiagram.pptx and b/docs/diagrams/ModelComponentClassDiagram.pptx differ diff --git a/docs/diagrams/StorageComponentClassDiagram.pptx b/docs/diagrams/StorageComponentClassDiagram.pptx index be29a9de7ca6..4afecd63e210 100644 Binary files a/docs/diagrams/StorageComponentClassDiagram.pptx and b/docs/diagrams/StorageComponentClassDiagram.pptx differ diff --git a/docs/diagrams/UiComponentClassDiagram.pptx b/docs/diagrams/UiComponentClassDiagram.pptx index 384d0a00e6ea..ab325e889f70 100644 Binary files a/docs/diagrams/UiComponentClassDiagram.pptx and b/docs/diagrams/UiComponentClassDiagram.pptx differ diff --git a/docs/images/Architecture.png b/docs/images/Architecture.png index bdc789000f77..d9215a274f93 100644 Binary files a/docs/images/Architecture.png and b/docs/images/Architecture.png differ diff --git a/docs/images/ModelClassBetterOopDiagram.png b/docs/images/ModelClassBetterOopDiagram.png index 9ba8eb5e31d0..b7df3a1c02b4 100644 Binary files a/docs/images/ModelClassBetterOopDiagram.png and b/docs/images/ModelClassBetterOopDiagram.png differ diff --git a/docs/images/ModelClassDiagram.png b/docs/images/ModelClassDiagram.png index 9fb19078b859..4961edd74e76 100644 Binary files a/docs/images/ModelClassDiagram.png and b/docs/images/ModelClassDiagram.png differ diff --git a/docs/images/SDforDeletePerson.png b/docs/images/SDforDeletePerson.png index 1e836f10dcd8..ae171fda7622 100644 Binary files a/docs/images/SDforDeletePerson.png and b/docs/images/SDforDeletePerson.png differ diff --git a/docs/images/SDforDeletePersonEventHandling.png b/docs/images/SDforDeletePersonEventHandling.png deleted file mode 100644 index ecec0805d32c..000000000000 Binary files a/docs/images/SDforDeletePersonEventHandling.png and /dev/null differ diff --git a/docs/images/StorageClassDiagram.png b/docs/images/StorageClassDiagram.png index 7a4cd2700cbf..e5527ecac459 100644 Binary files a/docs/images/StorageClassDiagram.png and b/docs/images/StorageClassDiagram.png differ diff --git a/docs/images/UiClassDiagram.png b/docs/images/UiClassDiagram.png index 369469ef176e..5f3847621e07 100644 Binary files a/docs/images/UiClassDiagram.png and b/docs/images/UiClassDiagram.png differ diff --git a/src/main/java/seedu/address/MainApp.java b/src/main/java/seedu/address/MainApp.java index ecdd043a4f81..a92d4d5d71f0 100644 --- a/src/main/java/seedu/address/MainApp.java +++ b/src/main/java/seedu/address/MainApp.java @@ -5,16 +5,11 @@ import java.util.Optional; import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - import javafx.application.Application; -import javafx.application.Platform; import javafx.stage.Stage; import seedu.address.commons.core.Config; -import seedu.address.commons.core.EventsCenter; import seedu.address.commons.core.LogsCenter; import seedu.address.commons.core.Version; -import seedu.address.commons.events.ui.ExitAppRequestEvent; import seedu.address.commons.exceptions.DataConversionException; import seedu.address.commons.util.ConfigUtil; import seedu.address.commons.util.StringUtil; @@ -24,14 +19,15 @@ import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.UserPrefs; import seedu.address.model.util.SampleDataUtil; import seedu.address.storage.AddressBookStorage; +import seedu.address.storage.JsonAddressBookStorage; import seedu.address.storage.JsonUserPrefsStorage; import seedu.address.storage.Storage; import seedu.address.storage.StorageManager; import seedu.address.storage.UserPrefsStorage; -import seedu.address.storage.XmlAddressBookStorage; import seedu.address.ui.Ui; import seedu.address.ui.UiManager; @@ -49,8 +45,6 @@ public class MainApp extends Application { protected Storage storage; protected Model model; protected Config config; - protected UserPrefs userPrefs; - @Override public void init() throws Exception { @@ -61,19 +55,17 @@ public void init() throws Exception { config = initConfig(appParameters.getConfigPath()); UserPrefsStorage userPrefsStorage = new JsonUserPrefsStorage(config.getUserPrefsFilePath()); - userPrefs = initPrefs(userPrefsStorage); - AddressBookStorage addressBookStorage = new XmlAddressBookStorage(userPrefs.getAddressBookFilePath()); + UserPrefs userPrefs = initPrefs(userPrefsStorage); + AddressBookStorage addressBookStorage = new JsonAddressBookStorage(userPrefs.getAddressBookFilePath()); storage = new StorageManager(addressBookStorage, userPrefsStorage); initLogging(config); model = initModelManager(storage, userPrefs); - logic = new LogicManager(model); + logic = new LogicManager(model, storage); - ui = new UiManager(logic, config, userPrefs); - - initEventsCenter(); + ui = new UiManager(logic); } /** @@ -81,7 +73,7 @@ public void init() throws Exception { * The data from the sample address book will be used instead if {@code storage}'s address book is not found, * or an empty address book will be used instead if errors occur when reading {@code storage}'s address book. */ - private Model initModelManager(Storage storage, UserPrefs userPrefs) { + private Model initModelManager(Storage storage, ReadOnlyUserPrefs userPrefs) { Optional addressBookOptional; ReadOnlyAddressBook initialData; try { @@ -173,10 +165,6 @@ protected UserPrefs initPrefs(UserPrefsStorage storage) { return initializedPrefs; } - private void initEventsCenter() { - EventsCenter.getInstance().registerHandler(this); - } - @Override public void start(Stage primaryStage) { logger.info("Starting AddressBook " + MainApp.VERSION); @@ -186,20 +174,11 @@ public void start(Stage primaryStage) { @Override public void stop() { logger.info("============================ [ Stopping Address Book ] ============================="); - ui.stop(); try { - storage.saveUserPrefs(userPrefs); + storage.saveUserPrefs(model.getUserPrefs()); } catch (IOException e) { logger.severe("Failed to save preferences " + StringUtil.getDetails(e)); } - Platform.exit(); - System.exit(0); - } - - @Subscribe - public void handleExitAppRequestEvent(ExitAppRequestEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - stop(); } public static void main(String[] args) { diff --git a/src/main/java/seedu/address/commons/core/ComponentManager.java b/src/main/java/seedu/address/commons/core/ComponentManager.java deleted file mode 100644 index 05a400773ae8..000000000000 --- a/src/main/java/seedu/address/commons/core/ComponentManager.java +++ /dev/null @@ -1,28 +0,0 @@ -package seedu.address.commons.core; - -import seedu.address.commons.events.BaseEvent; - -/** - * Base class for *Manager classes - * - * Registers the class' event handlers in eventsCenter - */ -public abstract class ComponentManager { - protected EventsCenter eventsCenter; - - /** - * Uses default {@link EventsCenter} - */ - public ComponentManager() { - this(EventsCenter.getInstance()); - } - - public ComponentManager(EventsCenter eventsCenter) { - this.eventsCenter = eventsCenter; - eventsCenter.registerHandler(this); - } - - protected void raise(BaseEvent event) { - eventsCenter.post(event); - } -} diff --git a/src/main/java/seedu/address/commons/core/Config.java b/src/main/java/seedu/address/commons/core/Config.java index e978d621e086..911457455217 100644 --- a/src/main/java/seedu/address/commons/core/Config.java +++ b/src/main/java/seedu/address/commons/core/Config.java @@ -13,18 +13,9 @@ public class Config { 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 Path userPrefsFilePath = Paths.get("preferences.json"); - public String getAppTitle() { - return appTitle; - } - - public void setAppTitle(String appTitle) { - this.appTitle = appTitle; - } - public Level getLogLevel() { return logLevel; } @@ -52,21 +43,19 @@ public boolean equals(Object other) { Config o = (Config) other; - return Objects.equals(appTitle, o.appTitle) - && Objects.equals(logLevel, o.logLevel) + return Objects.equals(logLevel, o.logLevel) && Objects.equals(userPrefsFilePath, o.userPrefsFilePath); } @Override public int hashCode() { - return Objects.hash(appTitle, logLevel, userPrefsFilePath); + return Objects.hash(logLevel, userPrefsFilePath); } @Override public String toString() { StringBuilder sb = new StringBuilder(); - sb.append("App title : " + appTitle); - sb.append("\nCurrent log level : " + logLevel); + sb.append("Current log level : " + logLevel); sb.append("\nPreference file Location : " + userPrefsFilePath); return sb.toString(); } diff --git a/src/main/java/seedu/address/commons/core/EventsCenter.java b/src/main/java/seedu/address/commons/core/EventsCenter.java deleted file mode 100644 index 799b976f7eb7..000000000000 --- a/src/main/java/seedu/address/commons/core/EventsCenter.java +++ /dev/null @@ -1,45 +0,0 @@ -package seedu.address.commons.core; - -import java.util.logging.Logger; - -import com.google.common.eventbus.EventBus; - -import seedu.address.commons.events.BaseEvent; - -/** - * Manages the event dispatching of the app. - */ -public class EventsCenter { - private static final Logger logger = LogsCenter.getLogger(EventsCenter.class); - private static EventsCenter instance; - private final EventBus eventBus; - - private EventsCenter() { - eventBus = new EventBus(); - } - - public static EventsCenter getInstance() { - if (instance == null) { - instance = new EventsCenter(); - } - return instance; - } - - public static void clearSubscribers() { - instance = null; - } - - public void registerHandler(Object handler) { - eventBus.register(handler); - } - - /** - * Posts an event to the event bus. - */ - public EventsCenter post(E event) { - logger.info("------[Event Posted] " + event.getClass().getCanonicalName() + ": " + event.toString()); - eventBus.post(event); - return this; - } - -} diff --git a/src/main/java/seedu/address/commons/core/GuiSettings.java b/src/main/java/seedu/address/commons/core/GuiSettings.java index ed578f1a9bb6..5ace559ad156 100644 --- a/src/main/java/seedu/address/commons/core/GuiSettings.java +++ b/src/main/java/seedu/address/commons/core/GuiSettings.java @@ -6,15 +6,16 @@ /** * A Serializable class that contains the GUI settings. + * Guarantees: immutable. */ public class GuiSettings implements Serializable { private static final double DEFAULT_HEIGHT = 600; private static final double DEFAULT_WIDTH = 740; - private Double windowWidth; - private Double windowHeight; - private Point windowCoordinates; + private final double windowWidth; + private final double windowHeight; + private final Point windowCoordinates; public GuiSettings() { windowWidth = DEFAULT_WIDTH; @@ -22,22 +23,22 @@ public GuiSettings() { windowCoordinates = null; // null represent no coordinates } - public GuiSettings(Double windowWidth, Double windowHeight, int xPosition, int yPosition) { + public GuiSettings(double windowWidth, double windowHeight, int xPosition, int yPosition) { this.windowWidth = windowWidth; this.windowHeight = windowHeight; windowCoordinates = new Point(xPosition, yPosition); } - public Double getWindowWidth() { + public double getWindowWidth() { return windowWidth; } - public Double getWindowHeight() { + public double getWindowHeight() { return windowHeight; } public Point getWindowCoordinates() { - return windowCoordinates; + return windowCoordinates != null ? new Point(windowCoordinates) : null; } @Override @@ -51,10 +52,9 @@ public boolean equals(Object other) { GuiSettings o = (GuiSettings) other; - return Objects.equals(windowWidth, o.windowWidth) - && Objects.equals(windowHeight, o.windowHeight) - && Objects.equals(windowCoordinates.x, o.windowCoordinates.x) - && Objects.equals(windowCoordinates.y, o.windowCoordinates.y); + return windowWidth == o.windowWidth + && windowHeight == o.windowHeight + && Objects.equals(windowCoordinates, o.windowCoordinates); } @Override diff --git a/src/main/java/seedu/address/commons/core/LogsCenter.java b/src/main/java/seedu/address/commons/core/LogsCenter.java index 5316a1d87d3e..431e7185e762 100644 --- a/src/main/java/seedu/address/commons/core/LogsCenter.java +++ b/src/main/java/seedu/address/commons/core/LogsCenter.java @@ -8,8 +8,6 @@ import java.util.logging.Logger; import java.util.logging.SimpleFormatter; -import seedu.address.commons.events.BaseEvent; - /** * Configures and manages loggers and handlers, including their logging level * Named {@link Logger}s can be obtained from this class
@@ -111,18 +109,4 @@ private static ConsoleHandler createConsoleHandler() { consoleHandler.setLevel(currentLogLevel); return consoleHandler; } - - /** - * Decorates the given string to create a log message suitable for logging event handling methods. - */ - public static String getEventHandlingLogMessage(BaseEvent e, String message) { - return "---[Event handled][" + e + "]" + message; - } - - /** - * @see #getEventHandlingLogMessage(BaseEvent, String) - */ - public static String getEventHandlingLogMessage(BaseEvent e) { - return getEventHandlingLogMessage(e, ""); - } } diff --git a/src/main/java/seedu/address/commons/events/BaseEvent.java b/src/main/java/seedu/address/commons/events/BaseEvent.java deleted file mode 100644 index 85e71cbb6b62..000000000000 --- a/src/main/java/seedu/address/commons/events/BaseEvent.java +++ /dev/null @@ -1,16 +0,0 @@ -package seedu.address.commons.events; - -/** - * The base class for all event classes. - */ -public abstract class BaseEvent { - - /** - * All Events should have a clear unambiguous custom toString message so that feedback message creation - * stays consistent and reusable. - * - * For example, the event manager post method will call any posted event's toString and print it in the console. - */ - public abstract String toString(); - -} diff --git a/src/main/java/seedu/address/commons/events/model/AddressBookChangedEvent.java b/src/main/java/seedu/address/commons/events/model/AddressBookChangedEvent.java deleted file mode 100644 index b72ad4740e5a..000000000000 --- a/src/main/java/seedu/address/commons/events/model/AddressBookChangedEvent.java +++ /dev/null @@ -1,19 +0,0 @@ -package seedu.address.commons.events.model; - -import seedu.address.commons.events.BaseEvent; -import seedu.address.model.ReadOnlyAddressBook; - -/** Indicates the AddressBook in the model has changed*/ -public class AddressBookChangedEvent extends BaseEvent { - - public final ReadOnlyAddressBook data; - - public AddressBookChangedEvent(ReadOnlyAddressBook data) { - this.data = data; - } - - @Override - public String toString() { - return "number of persons " + data.getPersonList().size(); - } -} diff --git a/src/main/java/seedu/address/commons/events/storage/DataSavingExceptionEvent.java b/src/main/java/seedu/address/commons/events/storage/DataSavingExceptionEvent.java deleted file mode 100644 index 7096107d8adf..000000000000 --- a/src/main/java/seedu/address/commons/events/storage/DataSavingExceptionEvent.java +++ /dev/null @@ -1,21 +0,0 @@ -package seedu.address.commons.events.storage; - -import seedu.address.commons.events.BaseEvent; - -/** - * Indicates an exception during a file saving - */ -public class DataSavingExceptionEvent extends BaseEvent { - - public final Exception exception; - - public DataSavingExceptionEvent(Exception exception) { - this.exception = exception; - } - - @Override - public String toString() { - return exception.toString(); - } - -} diff --git a/src/main/java/seedu/address/commons/events/ui/ExitAppRequestEvent.java b/src/main/java/seedu/address/commons/events/ui/ExitAppRequestEvent.java deleted file mode 100644 index a280bc7b5569..000000000000 --- a/src/main/java/seedu/address/commons/events/ui/ExitAppRequestEvent.java +++ /dev/null @@ -1,14 +0,0 @@ -package seedu.address.commons.events.ui; - -import seedu.address.commons.events.BaseEvent; - -/** - * Indicates a request for App termination - */ -public class ExitAppRequestEvent extends BaseEvent { - - @Override - public String toString() { - return getClass().getSimpleName(); - } -} diff --git a/src/main/java/seedu/address/commons/events/ui/JumpToListRequestEvent.java b/src/main/java/seedu/address/commons/events/ui/JumpToListRequestEvent.java deleted file mode 100644 index a890f8b47350..000000000000 --- a/src/main/java/seedu/address/commons/events/ui/JumpToListRequestEvent.java +++ /dev/null @@ -1,22 +0,0 @@ -package seedu.address.commons.events.ui; - -import seedu.address.commons.core.index.Index; -import seedu.address.commons.events.BaseEvent; - -/** - * Indicates a request to jump to the list of persons - */ -public class JumpToListRequestEvent extends BaseEvent { - - public final int targetIndex; - - public JumpToListRequestEvent(Index targetIndex) { - this.targetIndex = targetIndex.getZeroBased(); - } - - @Override - public String toString() { - return getClass().getSimpleName(); - } - -} diff --git a/src/main/java/seedu/address/commons/events/ui/NewResultAvailableEvent.java b/src/main/java/seedu/address/commons/events/ui/NewResultAvailableEvent.java deleted file mode 100644 index a5e8b2e13883..000000000000 --- a/src/main/java/seedu/address/commons/events/ui/NewResultAvailableEvent.java +++ /dev/null @@ -1,21 +0,0 @@ -package seedu.address.commons.events.ui; - -import seedu.address.commons.events.BaseEvent; - -/** - * Indicates that a new result is available. - */ -public class NewResultAvailableEvent extends BaseEvent { - - public final String message; - - public NewResultAvailableEvent(String message) { - this.message = message; - } - - @Override - public String toString() { - return getClass().getSimpleName(); - } - -} diff --git a/src/main/java/seedu/address/commons/events/ui/PersonPanelSelectionChangedEvent.java b/src/main/java/seedu/address/commons/events/ui/PersonPanelSelectionChangedEvent.java deleted file mode 100644 index c5c8b9ce90ed..000000000000 --- a/src/main/java/seedu/address/commons/events/ui/PersonPanelSelectionChangedEvent.java +++ /dev/null @@ -1,26 +0,0 @@ -package seedu.address.commons.events.ui; - -import seedu.address.commons.events.BaseEvent; -import seedu.address.model.person.Person; - -/** - * Represents a selection change in the Person List Panel - */ -public class PersonPanelSelectionChangedEvent extends BaseEvent { - - - private final Person newSelection; - - public PersonPanelSelectionChangedEvent(Person newSelection) { - this.newSelection = newSelection; - } - - @Override - public String toString() { - return getClass().getSimpleName(); - } - - public Person getNewSelection() { - return newSelection; - } -} diff --git a/src/main/java/seedu/address/commons/events/ui/ShowHelpRequestEvent.java b/src/main/java/seedu/address/commons/events/ui/ShowHelpRequestEvent.java deleted file mode 100644 index 87d5c70a1b79..000000000000 --- a/src/main/java/seedu/address/commons/events/ui/ShowHelpRequestEvent.java +++ /dev/null @@ -1,15 +0,0 @@ -package seedu.address.commons.events.ui; - -import seedu.address.commons.events.BaseEvent; - -/** - * An event requesting to view the help page. - */ -public class ShowHelpRequestEvent extends BaseEvent { - - @Override - public String toString() { - return getClass().getSimpleName(); - } - -} diff --git a/src/main/java/seedu/address/commons/util/InvalidationListenerManager.java b/src/main/java/seedu/address/commons/util/InvalidationListenerManager.java new file mode 100644 index 000000000000..70165336db6d --- /dev/null +++ b/src/main/java/seedu/address/commons/util/InvalidationListenerManager.java @@ -0,0 +1,52 @@ +package seedu.address.commons.util; + +import static java.util.Objects.requireNonNull; + +import java.util.ArrayList; + +import javafx.beans.InvalidationListener; +import javafx.beans.Observable; + +/** + * Manages a list of {@link InvalidationListener}. + */ +public class InvalidationListenerManager { + private final ArrayList listeners = new ArrayList<>(); + + /** + * Calls {@link InvalidationListener#invalidated(Observable)} on all added listeners. + * Any modifications to the listeners list during the invocation of this method + * will only take effect on the next invocation of this method. + * + * @param observable The {@code Observable} that became invalid. + */ + public void callListeners(Observable observable) { + // Make a copy of listeners such that any modifications to the listeners list during + // the invocation of this method will only take effect on the next invocation of this method. + ArrayList listenersCopy = new ArrayList<>(listeners); + + for (InvalidationListener listener : listenersCopy) { + listener.invalidated(observable); + } + } + + /** + * Adds {@code listener} to the list of listeners. + * If the same listener is added more that once, then it will be notified more than once. + */ + public void addListener(InvalidationListener listener) { + requireNonNull(listener); + listeners.add(listener); + } + + /** + * Removes {@code listener} from the list of listeners. + * If the given listener was not previously added, then this method call is a no-op. + * If the given listener was added more than once, then only the first occurrence in the list will be removed. + */ + public void removeListener(InvalidationListener listener) { + requireNonNull(listener); + listeners.remove(listener); + } + +} diff --git a/src/main/java/seedu/address/commons/util/JsonUtil.java b/src/main/java/seedu/address/commons/util/JsonUtil.java index 8ecd614f550a..8ef609f055df 100644 --- a/src/main/java/seedu/address/commons/util/JsonUtil.java +++ b/src/main/java/seedu/address/commons/util/JsonUtil.java @@ -60,7 +60,7 @@ public static Optional readJsonFile( requireNonNull(filePath); if (!Files.exists(filePath)) { - logger.info("Json file " + filePath + " not found"); + logger.info("Json file " + filePath + " not found"); return Optional.empty(); } diff --git a/src/main/java/seedu/address/commons/util/XmlUtil.java b/src/main/java/seedu/address/commons/util/XmlUtil.java deleted file mode 100644 index a78cd15b7f0c..000000000000 --- a/src/main/java/seedu/address/commons/util/XmlUtil.java +++ /dev/null @@ -1,71 +0,0 @@ -package seedu.address.commons.util; - -import static java.util.Objects.requireNonNull; - -import java.io.FileNotFoundException; -import java.nio.file.Files; -import java.nio.file.Path; - -import javax.xml.bind.JAXBContext; -import javax.xml.bind.JAXBException; -import javax.xml.bind.Marshaller; -import javax.xml.bind.Unmarshaller; - -/** - * Helps with reading from and writing to XML files. - */ -public class XmlUtil { - - /** - * Returns the xml data in the file as an object of the specified type. - * - * @param file Points to a valid xml file containing data that match the {@code classToConvert}. - * Cannot be null. - * @param classToConvert The class corresponding to the xml data. - * Cannot be null. - * @throws FileNotFoundException Thrown if the file is missing. - * @throws JAXBException Thrown if the file is empty or does not have the correct format. - */ - @SuppressWarnings("unchecked") - 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.toAbsolutePath()); - } - - JAXBContext context = JAXBContext.newInstance(classToConvert); - Unmarshaller um = context.createUnmarshaller(); - - return ((T) um.unmarshal(file.toFile())); - } - - /** - * Saves the data in the file in xml format. - * - * @param file Points to a valid xml file containing data that match the {@code classToConvert}. - * Cannot be null. - * @throws FileNotFoundException Thrown if the file is missing. - * @throws JAXBException Thrown if there is an error during converting the data - * into xml and writing to the file. - */ - public static void saveDataToFile(Path file, T data) throws FileNotFoundException, JAXBException { - - requireNonNull(file); - requireNonNull(data); - - 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.toFile()); - } - -} diff --git a/src/main/java/seedu/address/logic/CommandHistory.java b/src/main/java/seedu/address/logic/CommandHistory.java index 39bca9b8df57..404675e43811 100644 --- a/src/main/java/seedu/address/logic/CommandHistory.java +++ b/src/main/java/seedu/address/logic/CommandHistory.java @@ -2,21 +2,21 @@ import static java.util.Objects.requireNonNull; -import java.util.LinkedList; -import java.util.List; +import javafx.collections.FXCollections; +import javafx.collections.ObservableList; /** * Stores the history of commands executed. */ public class CommandHistory { - private LinkedList userInputHistory; + private final ObservableList userInputHistory = FXCollections.observableArrayList(); + private final ObservableList unmodifiableUserInputHistory = + FXCollections.unmodifiableObservableList(userInputHistory); - public CommandHistory() { - userInputHistory = new LinkedList<>(); - } + public CommandHistory() {} public CommandHistory(CommandHistory commandHistory) { - userInputHistory = new LinkedList<>(commandHistory.userInputHistory); + userInputHistory.addAll(commandHistory.userInputHistory); } /** @@ -28,10 +28,10 @@ public void add(String userInput) { } /** - * Returns a defensive copy of {@code userInputHistory}. + * Returns an unmodifiable view of {@code userInputHistory}. */ - public List getHistory() { - return new LinkedList<>(userInputHistory); + public ObservableList getHistory() { + return unmodifiableUserInputHistory; } @Override diff --git a/src/main/java/seedu/address/logic/Logic.java b/src/main/java/seedu/address/logic/Logic.java index 8b34b862039a..60369e2074e4 100644 --- a/src/main/java/seedu/address/logic/Logic.java +++ b/src/main/java/seedu/address/logic/Logic.java @@ -1,9 +1,14 @@ package seedu.address.logic; +import java.nio.file.Path; + +import javafx.beans.property.ReadOnlyProperty; import javafx.collections.ObservableList; +import seedu.address.commons.core.GuiSettings; import seedu.address.logic.commands.CommandResult; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.logic.parser.exceptions.ParseException; +import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.person.Person; /** @@ -19,9 +24,49 @@ public interface Logic { */ CommandResult execute(String commandText) throws CommandException, ParseException; + /** + * Returns the AddressBook. + * + * @see seedu.address.model.Model#getAddressBook() + */ + ReadOnlyAddressBook getAddressBook(); + /** Returns an unmodifiable view of the filtered list of persons */ ObservableList getFilteredPersonList(); - /** Returns the list of input entered by the user, encapsulated in a {@code ListElementPointer} object */ - ListElementPointer getHistorySnapshot(); + /** + * Returns an unmodifiable view of the list of commands entered by the user. + * The list is ordered from the least recent command to the most recent command. + */ + ObservableList getHistory(); + + /** + * Returns the user prefs' address book file path. + */ + Path getAddressBookFilePath(); + + /** + * Returns the user prefs' GUI settings. + */ + GuiSettings getGuiSettings(); + + /** + * Set the user prefs' GUI settings. + */ + void setGuiSettings(GuiSettings guiSettings); + + /** + * Selected person in the filtered person list. + * null if no person is selected. + * + * @see seedu.address.model.Model#selectedPersonProperty() + */ + ReadOnlyProperty selectedPersonProperty(); + + /** + * Sets the selected person in the filtered person list. + * + * @see seedu.address.model.Model#setSelectedPerson(Person) + */ + void setSelectedPerson(Person person); } diff --git a/src/main/java/seedu/address/logic/LogicManager.java b/src/main/java/seedu/address/logic/LogicManager.java index 9aff86fc33dc..5cb24a617beb 100644 --- a/src/main/java/seedu/address/logic/LogicManager.java +++ b/src/main/java/seedu/address/logic/LogicManager.java @@ -1,9 +1,12 @@ package seedu.address.logic; +import java.io.IOException; +import java.nio.file.Path; import java.util.logging.Logger; +import javafx.beans.property.ReadOnlyProperty; import javafx.collections.ObservableList; -import seedu.address.commons.core.ComponentManager; +import seedu.address.commons.core.GuiSettings; import seedu.address.commons.core.LogsCenter; import seedu.address.logic.commands.Command; import seedu.address.logic.commands.CommandResult; @@ -11,33 +14,61 @@ import seedu.address.logic.parser.AddressBookParser; import seedu.address.logic.parser.exceptions.ParseException; import seedu.address.model.Model; +import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.person.Person; +import seedu.address.storage.Storage; /** * The main LogicManager of the app. */ -public class LogicManager extends ComponentManager implements Logic { +public class LogicManager implements Logic { + public static final String FILE_OPS_ERROR_MESSAGE = "Could not save data to file: "; private final Logger logger = LogsCenter.getLogger(LogicManager.class); private final Model model; + private final Storage storage; private final CommandHistory history; private final AddressBookParser addressBookParser; + private boolean addressBookModified; - public LogicManager(Model model) { + public LogicManager(Model model, Storage storage) { this.model = model; + this.storage = storage; history = new CommandHistory(); addressBookParser = new AddressBookParser(); + + // Set addressBookModified to true whenever the models' address book is modified. + model.getAddressBook().addListener(observable -> addressBookModified = true); } @Override public CommandResult execute(String commandText) throws CommandException, ParseException { logger.info("----------------[USER COMMAND][" + commandText + "]"); + addressBookModified = false; + + CommandResult commandResult; try { Command command = addressBookParser.parseCommand(commandText); - return command.execute(model, history); + commandResult = command.execute(model, history); } finally { history.add(commandText); } + + if (addressBookModified) { + logger.info("Address book modified, saving to file."); + try { + storage.saveAddressBook(model.getAddressBook()); + } catch (IOException ioe) { + throw new CommandException(FILE_OPS_ERROR_MESSAGE + ioe, ioe); + } + } + + return commandResult; + } + + @Override + public ReadOnlyAddressBook getAddressBook() { + return model.getAddressBook(); } @Override @@ -46,7 +77,32 @@ public ObservableList getFilteredPersonList() { } @Override - public ListElementPointer getHistorySnapshot() { - return new ListElementPointer(history.getHistory()); + public ObservableList getHistory() { + return history.getHistory(); + } + + @Override + public Path getAddressBookFilePath() { + return model.getAddressBookFilePath(); + } + + @Override + public GuiSettings getGuiSettings() { + return model.getGuiSettings(); + } + + @Override + public void setGuiSettings(GuiSettings guiSettings) { + model.setGuiSettings(guiSettings); + } + + @Override + public ReadOnlyProperty selectedPersonProperty() { + return model.selectedPersonProperty(); + } + + @Override + public void setSelectedPerson(Person person) { + model.setSelectedPerson(person); } } diff --git a/src/main/java/seedu/address/logic/commands/ClearCommand.java b/src/main/java/seedu/address/logic/commands/ClearCommand.java index 1f85bcfe85a8..a22219ad76ad 100644 --- a/src/main/java/seedu/address/logic/commands/ClearCommand.java +++ b/src/main/java/seedu/address/logic/commands/ClearCommand.java @@ -18,7 +18,7 @@ public class ClearCommand extends Command { @Override public CommandResult execute(Model model, CommandHistory history) { requireNonNull(model); - model.resetData(new AddressBook()); + model.setAddressBook(new AddressBook()); model.commitAddressBook(); return new CommandResult(MESSAGE_SUCCESS); } diff --git a/src/main/java/seedu/address/logic/commands/CommandResult.java b/src/main/java/seedu/address/logic/commands/CommandResult.java index abdc267a2c44..92f900b7916d 100644 --- a/src/main/java/seedu/address/logic/commands/CommandResult.java +++ b/src/main/java/seedu/address/logic/commands/CommandResult.java @@ -2,15 +2,70 @@ import static java.util.Objects.requireNonNull; +import java.util.Objects; + /** * Represents the result of a command execution. */ public class CommandResult { - public final String feedbackToUser; + private final String feedbackToUser; - public CommandResult(String feedbackToUser) { + /** Help information should be shown to the user. */ + private final boolean showHelp; + + /** The application should exit. */ + private final boolean exit; + + /** + * Constructs a {@code CommandResult} with the specified fields. + */ + public CommandResult(String feedbackToUser, boolean showHelp, boolean exit) { this.feedbackToUser = requireNonNull(feedbackToUser); + this.showHelp = showHelp; + this.exit = exit; + } + + /** + * Constructs a {@code CommandResult} with the specified {@code feedbackToUser}, + * and other fields set to their default value. + */ + public CommandResult(String feedbackToUser) { + this(feedbackToUser, false, false); + } + + public String getFeedbackToUser() { + return feedbackToUser; + } + + public boolean isShowHelp() { + return showHelp; + } + + public boolean isExit() { + return exit; + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + + // instanceof handles nulls + if (!(other instanceof CommandResult)) { + return false; + } + + CommandResult otherCommandResult = (CommandResult) other; + return feedbackToUser.equals(otherCommandResult.feedbackToUser) + && showHelp == otherCommandResult.showHelp + && exit == otherCommandResult.exit; + } + + @Override + public int hashCode() { + return Objects.hash(feedbackToUser, showHelp, exit); } } diff --git a/src/main/java/seedu/address/logic/commands/EditCommand.java b/src/main/java/seedu/address/logic/commands/EditCommand.java index dc782d8e230f..952a9e7e7f2b 100644 --- a/src/main/java/seedu/address/logic/commands/EditCommand.java +++ b/src/main/java/seedu/address/logic/commands/EditCommand.java @@ -82,7 +82,7 @@ public CommandResult execute(Model model, CommandHistory history) throws Command throw new CommandException(MESSAGE_DUPLICATE_PERSON); } - model.updatePerson(personToEdit, editedPerson); + model.setPerson(personToEdit, editedPerson); model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); model.commitAddressBook(); return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, editedPerson)); diff --git a/src/main/java/seedu/address/logic/commands/ExitCommand.java b/src/main/java/seedu/address/logic/commands/ExitCommand.java index e848fa918964..2240a3e4be1f 100644 --- a/src/main/java/seedu/address/logic/commands/ExitCommand.java +++ b/src/main/java/seedu/address/logic/commands/ExitCommand.java @@ -1,7 +1,5 @@ package seedu.address.logic.commands; -import seedu.address.commons.core.EventsCenter; -import seedu.address.commons.events.ui.ExitAppRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; @@ -16,8 +14,7 @@ public class ExitCommand extends Command { @Override public CommandResult execute(Model model, CommandHistory history) { - EventsCenter.getInstance().post(new ExitAppRequestEvent()); - return new CommandResult(MESSAGE_EXIT_ACKNOWLEDGEMENT); + return new CommandResult(MESSAGE_EXIT_ACKNOWLEDGEMENT, false, true); } } diff --git a/src/main/java/seedu/address/logic/commands/HelpCommand.java b/src/main/java/seedu/address/logic/commands/HelpCommand.java index 66305e95d8f2..f0ef78dddded 100644 --- a/src/main/java/seedu/address/logic/commands/HelpCommand.java +++ b/src/main/java/seedu/address/logic/commands/HelpCommand.java @@ -1,7 +1,5 @@ package seedu.address.logic.commands; -import seedu.address.commons.core.EventsCenter; -import seedu.address.commons.events.ui.ShowHelpRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; @@ -19,7 +17,6 @@ public class HelpCommand extends Command { @Override public CommandResult execute(Model model, CommandHistory history) { - EventsCenter.getInstance().post(new ShowHelpRequestEvent()); - return new CommandResult(SHOWING_HELP_MESSAGE); + return new CommandResult(SHOWING_HELP_MESSAGE, true, false); } } diff --git a/src/main/java/seedu/address/logic/commands/HistoryCommand.java b/src/main/java/seedu/address/logic/commands/HistoryCommand.java index f1541fb57f20..dc3de1aad55e 100644 --- a/src/main/java/seedu/address/logic/commands/HistoryCommand.java +++ b/src/main/java/seedu/address/logic/commands/HistoryCommand.java @@ -2,8 +2,8 @@ import static java.util.Objects.requireNonNull; +import java.util.ArrayList; import java.util.Collections; -import java.util.List; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; @@ -20,7 +20,7 @@ public class HistoryCommand extends Command { @Override public CommandResult execute(Model model, CommandHistory history) { requireNonNull(history); - List previousCommands = history.getHistory(); + ArrayList previousCommands = new ArrayList<>(history.getHistory()); if (previousCommands.isEmpty()) { return new CommandResult(MESSAGE_NO_HISTORY); diff --git a/src/main/java/seedu/address/logic/commands/SelectCommand.java b/src/main/java/seedu/address/logic/commands/SelectCommand.java index f5e8c1a8722e..baa3c1f30bb4 100644 --- a/src/main/java/seedu/address/logic/commands/SelectCommand.java +++ b/src/main/java/seedu/address/logic/commands/SelectCommand.java @@ -4,10 +4,8 @@ import java.util.List; -import seedu.address.commons.core.EventsCenter; import seedu.address.commons.core.Messages; import seedu.address.commons.core.index.Index; -import seedu.address.commons.events.ui.JumpToListRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.model.Model; @@ -43,7 +41,7 @@ public CommandResult execute(Model model, CommandHistory history) throws Command throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); } - EventsCenter.getInstance().post(new JumpToListRequestEvent(targetIndex)); + model.setSelectedPerson(filteredPersonList.get(targetIndex.getZeroBased())); return new CommandResult(String.format(MESSAGE_SELECT_PERSON_SUCCESS, targetIndex.getOneBased())); } diff --git a/src/main/java/seedu/address/logic/parser/ParserUtil.java b/src/main/java/seedu/address/logic/parser/ParserUtil.java index 76daf40807e2..b117acb9c55b 100644 --- a/src/main/java/seedu/address/logic/parser/ParserUtil.java +++ b/src/main/java/seedu/address/logic/parser/ParserUtil.java @@ -45,7 +45,7 @@ public static Name parseName(String name) throws ParseException { requireNonNull(name); String trimmedName = name.trim(); if (!Name.isValidName(trimmedName)) { - throw new ParseException(Name.MESSAGE_NAME_CONSTRAINTS); + throw new ParseException(Name.MESSAGE_CONSTRAINTS); } return new Name(trimmedName); } @@ -60,7 +60,7 @@ public static Phone parsePhone(String phone) throws ParseException { requireNonNull(phone); String trimmedPhone = phone.trim(); if (!Phone.isValidPhone(trimmedPhone)) { - throw new ParseException(Phone.MESSAGE_PHONE_CONSTRAINTS); + throw new ParseException(Phone.MESSAGE_CONSTRAINTS); } return new Phone(trimmedPhone); } @@ -75,7 +75,7 @@ public static Address parseAddress(String address) throws ParseException { requireNonNull(address); String trimmedAddress = address.trim(); if (!Address.isValidAddress(trimmedAddress)) { - throw new ParseException(Address.MESSAGE_ADDRESS_CONSTRAINTS); + throw new ParseException(Address.MESSAGE_CONSTRAINTS); } return new Address(trimmedAddress); } @@ -90,7 +90,7 @@ public static Email parseEmail(String email) throws ParseException { requireNonNull(email); String trimmedEmail = email.trim(); if (!Email.isValidEmail(trimmedEmail)) { - throw new ParseException(Email.MESSAGE_EMAIL_CONSTRAINTS); + throw new ParseException(Email.MESSAGE_CONSTRAINTS); } return new Email(trimmedEmail); } @@ -105,7 +105,7 @@ public static Tag parseTag(String tag) throws ParseException { requireNonNull(tag); String trimmedTag = tag.trim(); if (!Tag.isValidTagName(trimmedTag)) { - throw new ParseException(Tag.MESSAGE_TAG_CONSTRAINTS); + throw new ParseException(Tag.MESSAGE_CONSTRAINTS); } return new Tag(trimmedTag); } diff --git a/src/main/java/seedu/address/model/AddressBook.java b/src/main/java/seedu/address/model/AddressBook.java index 7f85c8b9258b..30557cf81ee7 100644 --- a/src/main/java/seedu/address/model/AddressBook.java +++ b/src/main/java/seedu/address/model/AddressBook.java @@ -4,7 +4,9 @@ import java.util.List; +import javafx.beans.InvalidationListener; import javafx.collections.ObservableList; +import seedu.address.commons.util.InvalidationListenerManager; import seedu.address.model.person.Person; import seedu.address.model.person.UniquePersonList; @@ -15,6 +17,7 @@ public class AddressBook implements ReadOnlyAddressBook { private final UniquePersonList persons; + private final InvalidationListenerManager invalidationListenerManager = new InvalidationListenerManager(); /* * The 'unusual' code block below is an non-static initialization block, sometimes used to avoid duplication @@ -45,6 +48,7 @@ public AddressBook(ReadOnlyAddressBook toBeCopied) { */ public void setPersons(List persons) { this.persons.setPersons(persons); + indicateModified(); } /** @@ -72,6 +76,7 @@ public boolean hasPerson(Person person) { */ public void addPerson(Person p) { persons.add(p); + indicateModified(); } /** @@ -79,10 +84,11 @@ public void addPerson(Person p) { * {@code target} must exist in the address book. * The person identity of {@code editedPerson} must not be the same as another existing person in the address book. */ - public void updatePerson(Person target, Person editedPerson) { + public void setPerson(Person target, Person editedPerson) { requireNonNull(editedPerson); persons.setPerson(target, editedPerson); + indicateModified(); } /** @@ -91,6 +97,24 @@ public void updatePerson(Person target, Person editedPerson) { */ public void removePerson(Person key) { persons.remove(key); + indicateModified(); + } + + @Override + public void addListener(InvalidationListener listener) { + invalidationListenerManager.addListener(listener); + } + + @Override + public void removeListener(InvalidationListener listener) { + invalidationListenerManager.removeListener(listener); + } + + /** + * Notifies listeners that the address book has been modified. + */ + protected void indicateModified() { + invalidationListenerManager.callListeners(this); } //// util methods diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java index ac4521f33199..e857533821b6 100644 --- a/src/main/java/seedu/address/model/Model.java +++ b/src/main/java/seedu/address/model/Model.java @@ -1,8 +1,11 @@ package seedu.address.model; +import java.nio.file.Path; import java.util.function.Predicate; +import javafx.beans.property.ReadOnlyProperty; import javafx.collections.ObservableList; +import seedu.address.commons.core.GuiSettings; import seedu.address.model.person.Person; /** @@ -12,8 +15,40 @@ public interface Model { /** {@code Predicate} that always evaluate to true */ Predicate PREDICATE_SHOW_ALL_PERSONS = unused -> true; - /** Clears existing backing model and replaces with the provided new data. */ - void resetData(ReadOnlyAddressBook newData); + /** + * Replaces user prefs data with the data in {@code userPrefs}. + */ + void setUserPrefs(ReadOnlyUserPrefs userPrefs); + + /** + * Returns the user prefs. + */ + ReadOnlyUserPrefs getUserPrefs(); + + /** + * Returns the user prefs' GUI settings. + */ + GuiSettings getGuiSettings(); + + /** + * Sets the user prefs' GUI settings. + */ + void setGuiSettings(GuiSettings guiSettings); + + /** + * Returns the user prefs' address book file path. + */ + Path getAddressBookFilePath(); + + /** + * Sets the user prefs' address book file path. + */ + void setAddressBookFilePath(Path addressBookFilePath); + + /** + * Replaces address book data with the data in {@code addressBook}. + */ + void setAddressBook(ReadOnlyAddressBook addressBook); /** Returns the AddressBook */ ReadOnlyAddressBook getAddressBook(); @@ -40,7 +75,7 @@ public interface Model { * {@code target} must exist in the address book. * The person identity of {@code editedPerson} must not be the same as another existing person in the address book. */ - void updatePerson(Person target, Person editedPerson); + void setPerson(Person target, Person editedPerson); /** Returns an unmodifiable view of the filtered person list */ ObservableList getFilteredPersonList(); @@ -75,4 +110,21 @@ public interface Model { * Saves the current address book state for undo/redo. */ void commitAddressBook(); + + /** + * Selected person in the filtered person list. + * null if no person is selected. + */ + ReadOnlyProperty selectedPersonProperty(); + + /** + * Returns the selected person in the filtered person list. + * null if no person is selected. + */ + Person getSelectedPerson(); + + /** + * Sets the selected person in the filtered person list. + */ + void setSelectedPerson(Person person); } diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java index a664602ef5b1..b56806232814 100644 --- a/src/main/java/seedu/address/model/ModelManager.java +++ b/src/main/java/seedu/address/model/ModelManager.java @@ -3,57 +3,96 @@ import static java.util.Objects.requireNonNull; import static seedu.address.commons.util.CollectionUtil.requireAllNonNull; +import java.nio.file.Path; +import java.util.Objects; import java.util.function.Predicate; import java.util.logging.Logger; -import javafx.collections.FXCollections; +import javafx.beans.property.ReadOnlyProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; import javafx.collections.transformation.FilteredList; -import seedu.address.commons.core.ComponentManager; +import seedu.address.commons.core.GuiSettings; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.model.AddressBookChangedEvent; import seedu.address.model.person.Person; +import seedu.address.model.person.exceptions.PersonNotFoundException; /** * Represents the in-memory model of the address book data. */ -public class ModelManager extends ComponentManager implements Model { +public class ModelManager implements Model { private static final Logger logger = LogsCenter.getLogger(ModelManager.class); private final VersionedAddressBook versionedAddressBook; + private final UserPrefs userPrefs; private final FilteredList filteredPersons; + private final SimpleObjectProperty selectedPerson = new SimpleObjectProperty<>(); /** * Initializes a ModelManager with the given addressBook and userPrefs. */ - public ModelManager(ReadOnlyAddressBook addressBook, UserPrefs userPrefs) { + public ModelManager(ReadOnlyAddressBook addressBook, ReadOnlyUserPrefs userPrefs) { super(); requireAllNonNull(addressBook, userPrefs); logger.fine("Initializing with address book: " + addressBook + " and user prefs " + userPrefs); versionedAddressBook = new VersionedAddressBook(addressBook); + this.userPrefs = new UserPrefs(userPrefs); filteredPersons = new FilteredList<>(versionedAddressBook.getPersonList()); + filteredPersons.addListener(this::ensureSelectedPersonIsValid); } public ModelManager() { this(new AddressBook(), new UserPrefs()); } + //=========== UserPrefs ================================================================================== + @Override - public void resetData(ReadOnlyAddressBook newData) { - versionedAddressBook.resetData(newData); - indicateAddressBookChanged(); + public void setUserPrefs(ReadOnlyUserPrefs userPrefs) { + requireNonNull(userPrefs); + this.userPrefs.resetData(userPrefs); } @Override - public ReadOnlyAddressBook getAddressBook() { - return versionedAddressBook; + public ReadOnlyUserPrefs getUserPrefs() { + return userPrefs; + } + + @Override + public GuiSettings getGuiSettings() { + return userPrefs.getGuiSettings(); + } + + @Override + public void setGuiSettings(GuiSettings guiSettings) { + requireNonNull(guiSettings); + userPrefs.setGuiSettings(guiSettings); + } + + @Override + public Path getAddressBookFilePath() { + return userPrefs.getAddressBookFilePath(); + } + + @Override + public void setAddressBookFilePath(Path addressBookFilePath) { + requireNonNull(addressBookFilePath); + userPrefs.setAddressBookFilePath(addressBookFilePath); + } + + //=========== AddressBook ================================================================================ + + @Override + public void setAddressBook(ReadOnlyAddressBook addressBook) { + versionedAddressBook.resetData(addressBook); } - /** Raises an event to indicate the model has changed */ - private void indicateAddressBookChanged() { - raise(new AddressBookChangedEvent(versionedAddressBook)); + @Override + public ReadOnlyAddressBook getAddressBook() { + return versionedAddressBook; } @Override @@ -65,22 +104,19 @@ public boolean hasPerson(Person person) { @Override public void deletePerson(Person target) { versionedAddressBook.removePerson(target); - indicateAddressBookChanged(); } @Override public void addPerson(Person person) { versionedAddressBook.addPerson(person); updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); - indicateAddressBookChanged(); } @Override - public void updatePerson(Person target, Person editedPerson) { + public void setPerson(Person target, Person editedPerson) { requireAllNonNull(target, editedPerson); - versionedAddressBook.updatePerson(target, editedPerson); - indicateAddressBookChanged(); + versionedAddressBook.setPerson(target, editedPerson); } //=========== Filtered Person List Accessors ============================================================= @@ -91,7 +127,7 @@ public void updatePerson(Person target, Person editedPerson) { */ @Override public ObservableList getFilteredPersonList() { - return FXCollections.unmodifiableObservableList(filteredPersons); + return filteredPersons; } @Override @@ -115,13 +151,11 @@ public boolean canRedoAddressBook() { @Override public void undoAddressBook() { versionedAddressBook.undo(); - indicateAddressBookChanged(); } @Override public void redoAddressBook() { versionedAddressBook.redo(); - indicateAddressBookChanged(); } @Override @@ -129,6 +163,55 @@ public void commitAddressBook() { versionedAddressBook.commit(); } + //=========== Selected person =========================================================================== + + @Override + public ReadOnlyProperty selectedPersonProperty() { + return selectedPerson; + } + + @Override + public Person getSelectedPerson() { + return selectedPerson.getValue(); + } + + @Override + public void setSelectedPerson(Person person) { + if (person != null && !filteredPersons.contains(person)) { + throw new PersonNotFoundException(); + } + selectedPerson.setValue(person); + } + + /** + * Ensures {@code selectedPerson} is a valid person in {@code filteredPersons}. + */ + private void ensureSelectedPersonIsValid(ListChangeListener.Change change) { + while (change.next()) { + if (selectedPerson.getValue() == null) { + // null is always a valid selected person, so we do not need to check that it is valid anymore. + return; + } + + boolean wasSelectedPersonReplaced = change.wasReplaced() && change.getAddedSize() == change.getRemovedSize() + && change.getRemoved().contains(selectedPerson.getValue()); + if (wasSelectedPersonReplaced) { + // Update selectedPerson to its new value. + int index = change.getRemoved().indexOf(selectedPerson.getValue()); + selectedPerson.setValue(change.getAddedSubList().get(index)); + continue; + } + + boolean wasSelectedPersonRemoved = change.getRemoved().stream() + .anyMatch(removedPerson -> selectedPerson.getValue().isSamePerson(removedPerson)); + if (wasSelectedPersonRemoved) { + // Select the person that came before it in the list, + // or clear the selection if there is no such person. + selectedPerson.setValue(change.getFrom() > 0 ? change.getList().get(change.getFrom() - 1) : null); + } + } + } + @Override public boolean equals(Object obj) { // short circuit if same object @@ -144,7 +227,9 @@ public boolean equals(Object obj) { // state check ModelManager other = (ModelManager) obj; return versionedAddressBook.equals(other.versionedAddressBook) - && filteredPersons.equals(other.filteredPersons); + && userPrefs.equals(other.userPrefs) + && filteredPersons.equals(other.filteredPersons) + && Objects.equals(selectedPerson.get(), other.selectedPerson.get()); } } diff --git a/src/main/java/seedu/address/model/ReadOnlyAddressBook.java b/src/main/java/seedu/address/model/ReadOnlyAddressBook.java index 6ddc2cd9a290..6a301434b33b 100644 --- a/src/main/java/seedu/address/model/ReadOnlyAddressBook.java +++ b/src/main/java/seedu/address/model/ReadOnlyAddressBook.java @@ -1,12 +1,13 @@ package seedu.address.model; +import javafx.beans.Observable; import javafx.collections.ObservableList; import seedu.address.model.person.Person; /** * Unmodifiable view of an address book */ -public interface ReadOnlyAddressBook { +public interface ReadOnlyAddressBook extends Observable { /** * Returns an unmodifiable view of the persons list. diff --git a/src/main/java/seedu/address/model/ReadOnlyUserPrefs.java b/src/main/java/seedu/address/model/ReadOnlyUserPrefs.java new file mode 100644 index 000000000000..befd58a4c739 --- /dev/null +++ b/src/main/java/seedu/address/model/ReadOnlyUserPrefs.java @@ -0,0 +1,16 @@ +package seedu.address.model; + +import java.nio.file.Path; + +import seedu.address.commons.core.GuiSettings; + +/** + * Unmodifiable view of user prefs. + */ +public interface ReadOnlyUserPrefs { + + GuiSettings getGuiSettings(); + + Path getAddressBookFilePath(); + +} diff --git a/src/main/java/seedu/address/model/UserPrefs.java b/src/main/java/seedu/address/model/UserPrefs.java index 980b2b388852..25a5fd6eab9e 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 static java.util.Objects.requireNonNull; + import java.nio.file.Path; import java.nio.file.Paths; import java.util.Objects; @@ -9,32 +11,48 @@ /** * Represents User's preferences. */ -public class UserPrefs { - - private GuiSettings guiSettings; - private Path addressBookFilePath = Paths.get("data" , "addressbook.xml"); +public class UserPrefs implements ReadOnlyUserPrefs { + + private GuiSettings guiSettings = new GuiSettings(); + private Path addressBookFilePath = Paths.get("data" , "addressbook.json"); + + /** + * Creates a {@code UserPrefs} with default values. + */ + public UserPrefs() {} + + /** + * Creates a {@code UserPrefs} with the prefs in {@code userPrefs}. + */ + public UserPrefs(ReadOnlyUserPrefs userPrefs) { + this(); + resetData(userPrefs); + } - public UserPrefs() { - setGuiSettings(500, 500, 0, 0); + /** + * Resets the existing data of this {@code UserPrefs} with {@code newUserPrefs}. + */ + public void resetData(ReadOnlyUserPrefs newUserPrefs) { + requireNonNull(newUserPrefs); + setGuiSettings(newUserPrefs.getGuiSettings()); + setAddressBookFilePath(newUserPrefs.getAddressBookFilePath()); } public GuiSettings getGuiSettings() { - return guiSettings == null ? new GuiSettings() : guiSettings; + return guiSettings; } - public void updateLastUsedGuiSetting(GuiSettings guiSettings) { + public void setGuiSettings(GuiSettings guiSettings) { + requireNonNull(guiSettings); this.guiSettings = guiSettings; } - public void setGuiSettings(double width, double height, int x, int y) { - guiSettings = new GuiSettings(width, height, x, y); - } - public Path getAddressBookFilePath() { return addressBookFilePath; } public void setAddressBookFilePath(Path addressBookFilePath) { + requireNonNull(addressBookFilePath); this.addressBookFilePath = addressBookFilePath; } @@ -49,8 +67,8 @@ public boolean equals(Object other) { UserPrefs o = (UserPrefs) other; - return Objects.equals(guiSettings, o.guiSettings) - && Objects.equals(addressBookFilePath, o.addressBookFilePath); + return guiSettings.equals(o.guiSettings) + && addressBookFilePath.equals(o.addressBookFilePath); } @Override @@ -61,7 +79,7 @@ public int hashCode() { @Override public String toString() { StringBuilder sb = new StringBuilder(); - sb.append("Gui Settings : " + guiSettings.toString()); + sb.append("Gui Settings : " + guiSettings); sb.append("\nLocal data file location : " + addressBookFilePath); return sb.toString(); } diff --git a/src/main/java/seedu/address/model/VersionedAddressBook.java b/src/main/java/seedu/address/model/VersionedAddressBook.java index 227a335045d7..e17a9e3ba4ab 100644 --- a/src/main/java/seedu/address/model/VersionedAddressBook.java +++ b/src/main/java/seedu/address/model/VersionedAddressBook.java @@ -27,6 +27,7 @@ public void commit() { removeStatesAfterCurrentPointer(); addressBookStateList.add(new AddressBook(this)); currentStatePointer++; + indicateModified(); } private void removeStatesAfterCurrentPointer() { diff --git a/src/main/java/seedu/address/model/person/Address.java b/src/main/java/seedu/address/model/person/Address.java index a1409233ceb9..60472ca22a09 100644 --- a/src/main/java/seedu/address/model/person/Address.java +++ b/src/main/java/seedu/address/model/person/Address.java @@ -9,14 +9,13 @@ */ public class Address { - public static final String MESSAGE_ADDRESS_CONSTRAINTS = - "Addresses can take any values, and it should not be blank"; + public static final String MESSAGE_CONSTRAINTS = "Addresses can take any values, and it should not be blank"; /* * The first character of the address must not be a whitespace, * otherwise " " (a blank string) becomes a valid input. */ - public static final String ADDRESS_VALIDATION_REGEX = "[^\\s].*"; + public static final String VALIDATION_REGEX = "[^\\s].*"; public final String value; @@ -27,7 +26,7 @@ public class Address { */ public Address(String address) { requireNonNull(address); - checkArgument(isValidAddress(address), MESSAGE_ADDRESS_CONSTRAINTS); + checkArgument(isValidAddress(address), MESSAGE_CONSTRAINTS); value = address; } @@ -35,7 +34,7 @@ public Address(String address) { * Returns true if a given string is a valid email. */ public static boolean isValidAddress(String test) { - return test.matches(ADDRESS_VALIDATION_REGEX); + return test.matches(VALIDATION_REGEX); } @Override diff --git a/src/main/java/seedu/address/model/person/Email.java b/src/main/java/seedu/address/model/person/Email.java index 4a8283a20002..a5bbe0b6a5fc 100644 --- a/src/main/java/seedu/address/model/person/Email.java +++ b/src/main/java/seedu/address/model/person/Email.java @@ -9,8 +9,8 @@ */ public class Email { - private static final String SPECIAL_CHARACTERS = "!#$%&'*+/=?`{|}~^.-"; - public static final String MESSAGE_EMAIL_CONSTRAINTS = "Emails should be of the format local-part@domain " + private static final String SPECIAL_CHARACTERS = "!#$%&'*+/=?`{|}~^.-"; + public static final String MESSAGE_CONSTRAINTS = "Emails should be of the format local-part@domain " + "and adhere to the following constraints:\n" + "1. The local-part should only contain alphanumeric characters and these special characters, excluding " + "the parentheses, (" + SPECIAL_CHARACTERS + ") .\n" @@ -24,7 +24,7 @@ public class Email { private static final String DOMAIN_FIRST_CHARACTER_REGEX = "[^\\W_]"; // alphanumeric characters except underscore private static final String DOMAIN_MIDDLE_REGEX = "[a-zA-Z0-9.-]*"; // alphanumeric, period and hyphen private static final String DOMAIN_LAST_CHARACTER_REGEX = "[^\\W_]$"; - public static final String EMAIL_VALIDATION_REGEX = LOCAL_PART_REGEX + "@" + public static final String VALIDATION_REGEX = LOCAL_PART_REGEX + "@" + DOMAIN_FIRST_CHARACTER_REGEX + DOMAIN_MIDDLE_REGEX + DOMAIN_LAST_CHARACTER_REGEX; public final String value; @@ -36,7 +36,7 @@ public class Email { */ public Email(String email) { requireNonNull(email); - checkArgument(isValidEmail(email), MESSAGE_EMAIL_CONSTRAINTS); + checkArgument(isValidEmail(email), MESSAGE_CONSTRAINTS); value = email; } @@ -44,7 +44,7 @@ public Email(String email) { * Returns if a given string is a valid email. */ public static boolean isValidEmail(String test) { - return test.matches(EMAIL_VALIDATION_REGEX); + return test.matches(VALIDATION_REGEX); } @Override diff --git a/src/main/java/seedu/address/model/person/Name.java b/src/main/java/seedu/address/model/person/Name.java index 9982393dabb5..79244d71cf73 100644 --- a/src/main/java/seedu/address/model/person/Name.java +++ b/src/main/java/seedu/address/model/person/Name.java @@ -9,14 +9,14 @@ */ public class Name { - public static final String MESSAGE_NAME_CONSTRAINTS = + public static final String MESSAGE_CONSTRAINTS = "Names should only contain alphanumeric characters and spaces, and it should not be blank"; /* * The first character of the address must not be a whitespace, * otherwise " " (a blank string) becomes a valid input. */ - public static final String NAME_VALIDATION_REGEX = "[\\p{Alnum}][\\p{Alnum} ]*"; + public static final String VALIDATION_REGEX = "[\\p{Alnum}][\\p{Alnum} ]*"; public final String fullName; @@ -27,7 +27,7 @@ public class Name { */ public Name(String name) { requireNonNull(name); - checkArgument(isValidName(name), MESSAGE_NAME_CONSTRAINTS); + checkArgument(isValidName(name), MESSAGE_CONSTRAINTS); fullName = name; } @@ -35,7 +35,7 @@ public Name(String name) { * Returns true if a given string is a valid name. */ public static boolean isValidName(String test) { - return test.matches(NAME_VALIDATION_REGEX); + return test.matches(VALIDATION_REGEX); } diff --git a/src/main/java/seedu/address/model/person/Phone.java b/src/main/java/seedu/address/model/person/Phone.java index a22e51653835..872c76b382fd 100644 --- a/src/main/java/seedu/address/model/person/Phone.java +++ b/src/main/java/seedu/address/model/person/Phone.java @@ -10,9 +10,9 @@ public class Phone { - public static final String MESSAGE_PHONE_CONSTRAINTS = + public static final String MESSAGE_CONSTRAINTS = "Phone numbers should only contain numbers, and it should be at least 3 digits long"; - public static final String PHONE_VALIDATION_REGEX = "\\d{3,}"; + public static final String VALIDATION_REGEX = "\\d{3,}"; public final String value; /** @@ -22,7 +22,7 @@ public class Phone { */ public Phone(String phone) { requireNonNull(phone); - checkArgument(isValidPhone(phone), MESSAGE_PHONE_CONSTRAINTS); + checkArgument(isValidPhone(phone), MESSAGE_CONSTRAINTS); value = phone; } @@ -30,7 +30,7 @@ public Phone(String phone) { * Returns true if a given string is a valid phone number. */ public static boolean isValidPhone(String test) { - return test.matches(PHONE_VALIDATION_REGEX); + return test.matches(VALIDATION_REGEX); } @Override diff --git a/src/main/java/seedu/address/model/person/UniquePersonList.java b/src/main/java/seedu/address/model/person/UniquePersonList.java index 5856aa42e6b5..0fee4fe57e6b 100644 --- a/src/main/java/seedu/address/model/person/UniquePersonList.java +++ b/src/main/java/seedu/address/model/person/UniquePersonList.java @@ -25,6 +25,8 @@ public class UniquePersonList implements Iterable { private final ObservableList internalList = FXCollections.observableArrayList(); + private final ObservableList internalUnmodifiableList = + FXCollections.unmodifiableObservableList(internalList); /** * Returns true if the list contains an equivalent person as the given argument. @@ -99,7 +101,7 @@ public void setPersons(List persons) { * Returns the backing list as an unmodifiable {@code ObservableList}. */ public ObservableList asUnmodifiableObservableList() { - return FXCollections.unmodifiableObservableList(internalList); + return internalUnmodifiableList; } @Override diff --git a/src/main/java/seedu/address/model/tag/Tag.java b/src/main/java/seedu/address/model/tag/Tag.java index 8cdff2773ac9..b0ea7e7dad7f 100644 --- a/src/main/java/seedu/address/model/tag/Tag.java +++ b/src/main/java/seedu/address/model/tag/Tag.java @@ -9,8 +9,8 @@ */ public class Tag { - public static final String MESSAGE_TAG_CONSTRAINTS = "Tags names should be alphanumeric"; - public static final String TAG_VALIDATION_REGEX = "\\p{Alnum}+"; + public static final String MESSAGE_CONSTRAINTS = "Tags names should be alphanumeric"; + public static final String VALIDATION_REGEX = "\\p{Alnum}+"; public final String tagName; @@ -21,7 +21,7 @@ public class Tag { */ public Tag(String tagName) { requireNonNull(tagName); - checkArgument(isValidTagName(tagName), MESSAGE_TAG_CONSTRAINTS); + checkArgument(isValidTagName(tagName), MESSAGE_CONSTRAINTS); this.tagName = tagName; } @@ -29,7 +29,7 @@ public Tag(String tagName) { * Returns true if a given string is a valid tag name. */ public static boolean isValidTagName(String test) { - return test.matches(TAG_VALIDATION_REGEX); + return test.matches(VALIDATION_REGEX); } @Override diff --git a/src/main/java/seedu/address/storage/XmlAdaptedPerson.java b/src/main/java/seedu/address/storage/JsonAdaptedPerson.java similarity index 52% rename from src/main/java/seedu/address/storage/XmlAdaptedPerson.java rename to src/main/java/seedu/address/storage/JsonAdaptedPerson.java index c03785e5700f..a6321cec2eac 100644 --- a/src/main/java/seedu/address/storage/XmlAdaptedPerson.java +++ b/src/main/java/seedu/address/storage/JsonAdaptedPerson.java @@ -3,11 +3,11 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; -import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; -import javax.xml.bind.annotation.XmlElement; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; import seedu.address.commons.exceptions.IllegalValueException; import seedu.address.model.person.Address; @@ -18,66 +18,55 @@ import seedu.address.model.tag.Tag; /** - * JAXB-friendly version of the Person. + * Jackson-friendly version of {@link Person}. */ -public class XmlAdaptedPerson { +class JsonAdaptedPerson { public static final String MISSING_FIELD_MESSAGE_FORMAT = "Person's %s field is missing!"; - @XmlElement(required = true) - private String name; - @XmlElement(required = true) - private String phone; - @XmlElement(required = true) - private String email; - @XmlElement(required = true) - private String address; - - @XmlElement - private List tagged = new ArrayList<>(); + private final String name; + private final String phone; + private final String email; + private final String address; + private final List tagged = new ArrayList<>(); /** - * Constructs an XmlAdaptedPerson. - * This is the no-arg constructor that is required by JAXB. + * Constructs a {@code JsonAdaptedPerson} with the given person details. */ - public XmlAdaptedPerson() {} - - /** - * Constructs an {@code XmlAdaptedPerson} with the given person details. - */ - public XmlAdaptedPerson(String name, String phone, String email, String address, List tagged) { + @JsonCreator + public JsonAdaptedPerson(@JsonProperty("name") String name, @JsonProperty("phone") String phone, + @JsonProperty("email") String email, @JsonProperty("address") String address, + @JsonProperty("tagged") List tagged) { this.name = name; this.phone = phone; this.email = email; this.address = address; if (tagged != null) { - this.tagged = new ArrayList<>(tagged); + this.tagged.addAll(tagged); } } /** - * Converts a given Person into this class for JAXB use. - * - * @param source future changes to this will not affect the created XmlAdaptedPerson + * Converts a given {@code Person} into this class for Jackson use. */ - public XmlAdaptedPerson(Person source) { + public JsonAdaptedPerson(Person source) { name = source.getName().fullName; phone = source.getPhone().value; email = source.getEmail().value; address = source.getAddress().value; - tagged = source.getTags().stream() - .map(XmlAdaptedTag::new) - .collect(Collectors.toList()); + tagged.addAll(source.getTags().stream() + .map(JsonAdaptedTag::new) + .collect(Collectors.toList())); } /** - * Converts this jaxb-friendly adapted person object into the model's Person object. + * Converts this Jackson-friendly adapted person object into the model's {@code Person} object. * - * @throws IllegalValueException if there were any data constraints violated in the adapted person + * @throws IllegalValueException if there were any data constraints violated in the adapted person. */ public Person toModelType() throws IllegalValueException { final List personTags = new ArrayList<>(); - for (XmlAdaptedTag tag : tagged) { + for (JsonAdaptedTag tag : tagged) { personTags.add(tag.toModelType()); } @@ -85,7 +74,7 @@ public Person toModelType() throws IllegalValueException { throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Name.class.getSimpleName())); } if (!Name.isValidName(name)) { - throw new IllegalValueException(Name.MESSAGE_NAME_CONSTRAINTS); + throw new IllegalValueException(Name.MESSAGE_CONSTRAINTS); } final Name modelName = new Name(name); @@ -93,7 +82,7 @@ public Person toModelType() throws IllegalValueException { throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Phone.class.getSimpleName())); } if (!Phone.isValidPhone(phone)) { - throw new IllegalValueException(Phone.MESSAGE_PHONE_CONSTRAINTS); + throw new IllegalValueException(Phone.MESSAGE_CONSTRAINTS); } final Phone modelPhone = new Phone(phone); @@ -101,7 +90,7 @@ public Person toModelType() throws IllegalValueException { throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Email.class.getSimpleName())); } if (!Email.isValidEmail(email)) { - throw new IllegalValueException(Email.MESSAGE_EMAIL_CONSTRAINTS); + throw new IllegalValueException(Email.MESSAGE_CONSTRAINTS); } final Email modelEmail = new Email(email); @@ -109,7 +98,7 @@ public Person toModelType() throws IllegalValueException { throw new IllegalValueException(String.format(MISSING_FIELD_MESSAGE_FORMAT, Address.class.getSimpleName())); } if (!Address.isValidAddress(address)) { - throw new IllegalValueException(Address.MESSAGE_ADDRESS_CONSTRAINTS); + throw new IllegalValueException(Address.MESSAGE_CONSTRAINTS); } final Address modelAddress = new Address(address); @@ -117,21 +106,4 @@ public Person toModelType() throws IllegalValueException { return new Person(modelName, modelPhone, modelEmail, modelAddress, modelTags); } - @Override - public boolean equals(Object other) { - if (other == this) { - return true; - } - - if (!(other instanceof XmlAdaptedPerson)) { - return false; - } - - XmlAdaptedPerson otherPerson = (XmlAdaptedPerson) other; - return Objects.equals(name, otherPerson.name) - && Objects.equals(phone, otherPerson.phone) - && Objects.equals(email, otherPerson.email) - && Objects.equals(address, otherPerson.address) - && tagged.equals(otherPerson.tagged); - } } diff --git a/src/main/java/seedu/address/storage/JsonAdaptedTag.java b/src/main/java/seedu/address/storage/JsonAdaptedTag.java new file mode 100644 index 000000000000..0df22bdb7546 --- /dev/null +++ b/src/main/java/seedu/address/storage/JsonAdaptedTag.java @@ -0,0 +1,48 @@ +package seedu.address.storage; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; + +import seedu.address.commons.exceptions.IllegalValueException; +import seedu.address.model.tag.Tag; + +/** + * Jackson-friendly version of {@link Tag}. + */ +class JsonAdaptedTag { + + private final String tagName; + + /** + * Constructs a {@code JsonAdaptedTag} with the given {@code tagName}. + */ + @JsonCreator + public JsonAdaptedTag(String tagName) { + this.tagName = tagName; + } + + /** + * Converts a given {@code Tag} into this class for Jackson use. + */ + public JsonAdaptedTag(Tag source) { + tagName = source.tagName; + } + + @JsonValue + public String getTagName() { + return tagName; + } + + /** + * Converts this Jackson-friendly adapted tag object into the model's {@code Tag} object. + * + * @throws IllegalValueException if there were any data constraints violated in the adapted tag. + */ + public Tag toModelType() throws IllegalValueException { + if (!Tag.isValidTagName(tagName)) { + throw new IllegalValueException(Tag.MESSAGE_CONSTRAINTS); + } + return new Tag(tagName); + } + +} diff --git a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java b/src/main/java/seedu/address/storage/JsonAddressBookStorage.java similarity index 59% rename from src/main/java/seedu/address/storage/XmlAddressBookStorage.java rename to src/main/java/seedu/address/storage/JsonAddressBookStorage.java index 8af60be0dc5d..dfab9daaa0d3 100644 --- a/src/main/java/seedu/address/storage/XmlAddressBookStorage.java +++ b/src/main/java/seedu/address/storage/JsonAddressBookStorage.java @@ -2,9 +2,7 @@ import static java.util.Objects.requireNonNull; -import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; import java.util.logging.Logger; @@ -13,18 +11,19 @@ import seedu.address.commons.exceptions.DataConversionException; import seedu.address.commons.exceptions.IllegalValueException; import seedu.address.commons.util.FileUtil; +import seedu.address.commons.util.JsonUtil; import seedu.address.model.ReadOnlyAddressBook; /** - * A class to access AddressBook data stored as an xml file on the hard disk. + * A class to access AddressBook data stored as a json file on the hard disk. */ -public class XmlAddressBookStorage implements AddressBookStorage { +public class JsonAddressBookStorage implements AddressBookStorage { - private static final Logger logger = LogsCenter.getLogger(XmlAddressBookStorage.class); + private static final Logger logger = LogsCenter.getLogger(JsonAddressBookStorage.class); private Path filePath; - public XmlAddressBookStorage(Path filePath) { + public JsonAddressBookStorage(Path filePath) { this.filePath = filePath; } @@ -33,27 +32,27 @@ public Path getAddressBookFilePath() { } @Override - public Optional readAddressBook() throws DataConversionException, IOException { + public Optional readAddressBook() throws DataConversionException { return readAddressBook(filePath); } /** - * Similar to {@link #readAddressBook()} - * @param filePath location of the data. Cannot be null + * Similar to {@link #readAddressBook()}. + * + * @param filePath location of the data. Cannot be null. * @throws DataConversionException if the file is not in the correct format. */ - public Optional readAddressBook(Path filePath) throws DataConversionException, - FileNotFoundException { + public Optional readAddressBook(Path filePath) throws DataConversionException { requireNonNull(filePath); - if (!Files.exists(filePath)) { - logger.info("AddressBook file " + filePath + " not found"); + Optional jsonAddressBook = JsonUtil.readJsonFile( + filePath, JsonSerializableAddressBook.class); + if (!jsonAddressBook.isPresent()) { return Optional.empty(); } - XmlSerializableAddressBook xmlAddressBook = XmlFileStorage.loadDataFromSaveFile(filePath); try { - return Optional.of(xmlAddressBook.toModelType()); + return Optional.of(jsonAddressBook.get().toModelType()); } catch (IllegalValueException ive) { logger.info("Illegal values found in " + filePath + ": " + ive.getMessage()); throw new DataConversionException(ive); @@ -66,15 +65,16 @@ public void saveAddressBook(ReadOnlyAddressBook addressBook) throws IOException } /** - * Similar to {@link #saveAddressBook(ReadOnlyAddressBook)} - * @param filePath location of the data. Cannot be null + * Similar to {@link #saveAddressBook(ReadOnlyAddressBook)}. + * + * @param filePath location of the data. Cannot be null. */ public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { requireNonNull(addressBook); requireNonNull(filePath); FileUtil.createIfMissing(filePath); - XmlFileStorage.saveDataToFile(filePath, new XmlSerializableAddressBook(addressBook)); + JsonUtil.saveJsonFile(new JsonSerializableAddressBook(addressBook), filePath); } } diff --git a/src/main/java/seedu/address/storage/JsonSerializableAddressBook.java b/src/main/java/seedu/address/storage/JsonSerializableAddressBook.java new file mode 100644 index 000000000000..5efd834091d4 --- /dev/null +++ b/src/main/java/seedu/address/storage/JsonSerializableAddressBook.java @@ -0,0 +1,60 @@ +package seedu.address.storage; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonRootName; + +import seedu.address.commons.exceptions.IllegalValueException; +import seedu.address.model.AddressBook; +import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.person.Person; + +/** + * An Immutable AddressBook that is serializable to JSON format. + */ +@JsonRootName(value = "addressbook") +class JsonSerializableAddressBook { + + public static final String MESSAGE_DUPLICATE_PERSON = "Persons list contains duplicate person(s)."; + + private final List persons = new ArrayList<>(); + + /** + * Constructs a {@code JsonSerializableAddressBook} with the given persons. + */ + @JsonCreator + public JsonSerializableAddressBook(@JsonProperty("persons") List persons) { + this.persons.addAll(persons); + } + + /** + * Converts a given {@code ReadOnlyAddressBook} into this class for Jackson use. + * + * @param source future changes to this will not affect the created {@code JsonSerializableAddressBook}. + */ + public JsonSerializableAddressBook(ReadOnlyAddressBook source) { + persons.addAll(source.getPersonList().stream().map(JsonAdaptedPerson::new).collect(Collectors.toList())); + } + + /** + * Converts this address book into the model's {@code AddressBook} object. + * + * @throws IllegalValueException if there were any data constraints violated. + */ + public AddressBook toModelType() throws IllegalValueException { + AddressBook addressBook = new AddressBook(); + for (JsonAdaptedPerson jsonAdaptedPerson : persons) { + Person person = jsonAdaptedPerson.toModelType(); + if (addressBook.hasPerson(person)) { + throw new IllegalValueException(MESSAGE_DUPLICATE_PERSON); + } + addressBook.addPerson(person); + } + return addressBook; + } + +} diff --git a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java index 2ab927023cc4..bc2bbad84aa3 100644 --- a/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java +++ b/src/main/java/seedu/address/storage/JsonUserPrefsStorage.java @@ -6,6 +6,7 @@ import seedu.address.commons.exceptions.DataConversionException; import seedu.address.commons.util.JsonUtil; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.UserPrefs; /** @@ -39,7 +40,7 @@ public Optional readUserPrefs(Path prefsFilePath) throws DataConversi } @Override - public void saveUserPrefs(UserPrefs userPrefs) throws IOException { + public void saveUserPrefs(ReadOnlyUserPrefs userPrefs) throws IOException { JsonUtil.saveJsonFile(userPrefs, filePath); } diff --git a/src/main/java/seedu/address/storage/Storage.java b/src/main/java/seedu/address/storage/Storage.java index 28791127999b..beda8bd9f11b 100644 --- a/src/main/java/seedu/address/storage/Storage.java +++ b/src/main/java/seedu/address/storage/Storage.java @@ -4,10 +4,9 @@ import java.nio.file.Path; import java.util.Optional; -import seedu.address.commons.events.model.AddressBookChangedEvent; -import seedu.address.commons.events.storage.DataSavingExceptionEvent; import seedu.address.commons.exceptions.DataConversionException; import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.UserPrefs; /** @@ -19,7 +18,7 @@ public interface Storage extends AddressBookStorage, UserPrefsStorage { Optional readUserPrefs() throws DataConversionException, IOException; @Override - void saveUserPrefs(UserPrefs userPrefs) throws IOException; + void saveUserPrefs(ReadOnlyUserPrefs userPrefs) throws IOException; @Override Path getAddressBookFilePath(); @@ -30,10 +29,4 @@ public interface Storage extends AddressBookStorage, UserPrefsStorage { @Override void saveAddressBook(ReadOnlyAddressBook addressBook) throws IOException; - /** - * Saves the current version of the Address Book to the hard disk. - * Creates the data file if it is missing. - * Raises {@link DataSavingExceptionEvent} if there was an error during saving. - */ - void handleAddressBookChangedEvent(AddressBookChangedEvent abce); } diff --git a/src/main/java/seedu/address/storage/StorageManager.java b/src/main/java/seedu/address/storage/StorageManager.java index b0df908a76a7..e4f452b6cbf4 100644 --- a/src/main/java/seedu/address/storage/StorageManager.java +++ b/src/main/java/seedu/address/storage/StorageManager.java @@ -5,20 +5,16 @@ import java.util.Optional; import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - -import seedu.address.commons.core.ComponentManager; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.model.AddressBookChangedEvent; -import seedu.address.commons.events.storage.DataSavingExceptionEvent; import seedu.address.commons.exceptions.DataConversionException; import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.UserPrefs; /** * Manages storage of AddressBook data in local storage. */ -public class StorageManager extends ComponentManager implements Storage { +public class StorageManager implements Storage { private static final Logger logger = LogsCenter.getLogger(StorageManager.class); private AddressBookStorage addressBookStorage; @@ -44,7 +40,7 @@ public Optional readUserPrefs() throws DataConversionException, IOExc } @Override - public void saveUserPrefs(UserPrefs userPrefs) throws IOException { + public void saveUserPrefs(ReadOnlyUserPrefs userPrefs) throws IOException { userPrefsStorage.saveUserPrefs(userPrefs); } @@ -78,16 +74,4 @@ public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) thro addressBookStorage.saveAddressBook(addressBook, filePath); } - - @Override - @Subscribe - public void handleAddressBookChangedEvent(AddressBookChangedEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event, "Local data changed, saving to file")); - try { - saveAddressBook(event.data); - } catch (IOException e) { - raise(new DataSavingExceptionEvent(e)); - } - } - } diff --git a/src/main/java/seedu/address/storage/UserPrefsStorage.java b/src/main/java/seedu/address/storage/UserPrefsStorage.java index 877b0ee5c4f0..29eef178dbc4 100644 --- a/src/main/java/seedu/address/storage/UserPrefsStorage.java +++ b/src/main/java/seedu/address/storage/UserPrefsStorage.java @@ -5,6 +5,7 @@ import java.util.Optional; import seedu.address.commons.exceptions.DataConversionException; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.UserPrefs; /** @@ -26,10 +27,10 @@ public interface UserPrefsStorage { Optional readUserPrefs() throws DataConversionException, IOException; /** - * Saves the given {@link seedu.address.model.UserPrefs} to the storage. + * Saves the given {@link seedu.address.model.ReadOnlyUserPrefs} to the storage. * @param userPrefs cannot be null. * @throws IOException if there was any problem writing to the file. */ - void saveUserPrefs(UserPrefs userPrefs) throws IOException; + void saveUserPrefs(ReadOnlyUserPrefs userPrefs) throws IOException; } diff --git a/src/main/java/seedu/address/storage/XmlAdaptedTag.java b/src/main/java/seedu/address/storage/XmlAdaptedTag.java deleted file mode 100644 index d3e2d8be9c4f..000000000000 --- a/src/main/java/seedu/address/storage/XmlAdaptedTag.java +++ /dev/null @@ -1,62 +0,0 @@ -package seedu.address.storage; - -import javax.xml.bind.annotation.XmlValue; - -import seedu.address.commons.exceptions.IllegalValueException; -import seedu.address.model.tag.Tag; - -/** - * JAXB-friendly adapted version of the Tag. - */ -public class XmlAdaptedTag { - - @XmlValue - private String tagName; - - /** - * Constructs an XmlAdaptedTag. - * This is the no-arg constructor that is required by JAXB. - */ - public XmlAdaptedTag() {} - - /** - * Constructs a {@code XmlAdaptedTag} with the given {@code tagName}. - */ - public XmlAdaptedTag(String tagName) { - this.tagName = tagName; - } - - /** - * Converts a given Tag into this class for JAXB use. - * - * @param source future changes to this will not affect the created - */ - public XmlAdaptedTag(Tag source) { - tagName = source.tagName; - } - - /** - * Converts this jaxb-friendly adapted tag object into the model's Tag object. - * - * @throws IllegalValueException if there were any data constraints violated in the adapted person - */ - public Tag toModelType() throws IllegalValueException { - if (!Tag.isValidTagName(tagName)) { - throw new IllegalValueException(Tag.MESSAGE_TAG_CONSTRAINTS); - } - return new Tag(tagName); - } - - @Override - public boolean equals(Object other) { - if (other == this) { - return true; - } - - if (!(other instanceof XmlAdaptedTag)) { - return false; - } - - return tagName.equals(((XmlAdaptedTag) other).tagName); - } -} diff --git a/src/main/java/seedu/address/storage/XmlFileStorage.java b/src/main/java/seedu/address/storage/XmlFileStorage.java deleted file mode 100644 index d8f65dc036ab..000000000000 --- a/src/main/java/seedu/address/storage/XmlFileStorage.java +++ /dev/null @@ -1,39 +0,0 @@ -package seedu.address.storage; - -import java.io.FileNotFoundException; -import java.nio.file.Path; - -import javax.xml.bind.JAXBException; - -import seedu.address.commons.exceptions.DataConversionException; -import seedu.address.commons.util.XmlUtil; - -/** - * Stores addressbook data in an XML file - */ -public class XmlFileStorage { - /** - * Saves the given addressbook data to the specified file. - */ - public static void saveDataToFile(Path file, XmlSerializableAddressBook addressBook) - throws FileNotFoundException { - try { - XmlUtil.saveDataToFile(file, addressBook); - } catch (JAXBException e) { - throw new AssertionError("Unexpected exception " + e.getMessage(), e); - } - } - - /** - * Returns address book in the file or an empty address book - */ - public static XmlSerializableAddressBook loadDataFromSaveFile(Path file) throws DataConversionException, - FileNotFoundException { - try { - return XmlUtil.getDataFromFile(file, XmlSerializableAddressBook.class); - } catch (JAXBException e) { - throw new DataConversionException(e); - } - } - -} diff --git a/src/main/java/seedu/address/storage/XmlSerializableAddressBook.java b/src/main/java/seedu/address/storage/XmlSerializableAddressBook.java deleted file mode 100644 index b85fa4a8f07e..000000000000 --- a/src/main/java/seedu/address/storage/XmlSerializableAddressBook.java +++ /dev/null @@ -1,71 +0,0 @@ -package seedu.address.storage; - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - -import javax.xml.bind.annotation.XmlElement; -import javax.xml.bind.annotation.XmlRootElement; - -import seedu.address.commons.exceptions.IllegalValueException; -import seedu.address.model.AddressBook; -import seedu.address.model.ReadOnlyAddressBook; -import seedu.address.model.person.Person; - -/** - * An Immutable AddressBook that is serializable to XML format - */ -@XmlRootElement(name = "addressbook") -public class XmlSerializableAddressBook { - - public static final String MESSAGE_DUPLICATE_PERSON = "Persons list contains duplicate person(s)."; - - @XmlElement - private List persons; - - /** - * Creates an empty XmlSerializableAddressBook. - * This empty constructor is required for marshalling. - */ - public XmlSerializableAddressBook() { - persons = new ArrayList<>(); - } - - /** - * Conversion - */ - public XmlSerializableAddressBook(ReadOnlyAddressBook src) { - this(); - persons.addAll(src.getPersonList().stream().map(XmlAdaptedPerson::new).collect(Collectors.toList())); - } - - /** - * Converts this addressbook into the model's {@code AddressBook} object. - * - * @throws IllegalValueException if there were any data constraints violated or duplicates in the - * {@code XmlAdaptedPerson}. - */ - public AddressBook toModelType() throws IllegalValueException { - AddressBook addressBook = new AddressBook(); - for (XmlAdaptedPerson p : persons) { - Person person = p.toModelType(); - if (addressBook.hasPerson(person)) { - throw new IllegalValueException(MESSAGE_DUPLICATE_PERSON); - } - addressBook.addPerson(person); - } - return addressBook; - } - - @Override - public boolean equals(Object other) { - if (other == this) { - return true; - } - - if (!(other instanceof XmlSerializableAddressBook)) { - return false; - } - return persons.equals(((XmlSerializableAddressBook) other).persons); - } -} diff --git a/src/main/java/seedu/address/ui/BrowserPanel.java b/src/main/java/seedu/address/ui/BrowserPanel.java index b43de90a2b9f..53876e01c8d1 100644 --- a/src/main/java/seedu/address/ui/BrowserPanel.java +++ b/src/main/java/seedu/address/ui/BrowserPanel.java @@ -1,18 +1,18 @@ package seedu.address.ui; +import static java.util.Objects.requireNonNull; + import java.net.URL; import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - import javafx.application.Platform; +import javafx.beans.value.ObservableValue; import javafx.event.Event; import javafx.fxml.FXML; import javafx.scene.layout.Region; import javafx.scene.web.WebView; import seedu.address.MainApp; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.ui.PersonPanelSelectionChangedEvent; import seedu.address.model.person.Person; /** @@ -20,9 +20,9 @@ */ public class BrowserPanel extends UiPart { - public static final String DEFAULT_PAGE = "default.html"; - public static final String SEARCH_PAGE_URL = - "https://se-edu.github.io/addressbook-level4/DummySearchPage.html?name="; + public static final URL DEFAULT_PAGE = + requireNonNull(MainApp.class.getResource(FXML_FILE_FOLDER + "default.html")); + public static final String SEARCH_PAGE_URL = "https://se-education.org/dummy-search-page/?name="; private static final String FXML = "BrowserPanel.fxml"; @@ -31,14 +31,22 @@ public class BrowserPanel extends UiPart { @FXML private WebView browser; - public BrowserPanel() { + public BrowserPanel(ObservableValue selectedPerson) { super(FXML); // To prevent triggering events for typing inside the loaded Web page. getRoot().setOnKeyPressed(Event::consume); + // Load person page when selected person changes. + selectedPerson.addListener((observable, oldValue, newValue) -> { + if (newValue == null) { + loadDefaultPage(); + return; + } + loadPersonPage(newValue); + }); + loadDefaultPage(); - registerAsAnEventHandler(this); } private void loadPersonPage(Person person) { @@ -53,20 +61,7 @@ public void loadPage(String url) { * Loads a default HTML file with a background that matches the general theme. */ private void loadDefaultPage() { - URL defaultPage = MainApp.class.getResource(FXML_FILE_FOLDER + DEFAULT_PAGE); - loadPage(defaultPage.toExternalForm()); - } - - /** - * Frees resources allocated to the browser. - */ - public void freeResources() { - browser = null; + loadPage(DEFAULT_PAGE.toExternalForm()); } - @Subscribe - private void handlePersonPanelSelectionChangedEvent(PersonPanelSelectionChangedEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - loadPersonPage(event.getNewSelection()); - } } diff --git a/src/main/java/seedu/address/ui/CommandBox.java b/src/main/java/seedu/address/ui/CommandBox.java index 3d7aaded5640..bf09f3dcbea6 100644 --- a/src/main/java/seedu/address/ui/CommandBox.java +++ b/src/main/java/seedu/address/ui/CommandBox.java @@ -1,16 +1,12 @@ package seedu.address.ui; -import java.util.logging.Logger; +import java.util.List; import javafx.collections.ObservableList; import javafx.fxml.FXML; import javafx.scene.control.TextField; import javafx.scene.input.KeyEvent; import javafx.scene.layout.Region; -import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.ui.NewResultAvailableEvent; -import seedu.address.logic.ListElementPointer; -import seedu.address.logic.Logic; import seedu.address.logic.commands.CommandResult; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.logic.parser.exceptions.ParseException; @@ -23,19 +19,20 @@ public class CommandBox extends UiPart { public static final String ERROR_STYLE_CLASS = "error"; private static final String FXML = "CommandBox.fxml"; - private final Logger logger = LogsCenter.getLogger(CommandBox.class); - private final Logic logic; + private final CommandExecutor commandExecutor; + private final List history; private ListElementPointer historySnapshot; @FXML private TextField commandTextField; - public CommandBox(Logic logic) { + public CommandBox(CommandExecutor commandExecutor, List history) { super(FXML); - this.logic = logic; + this.commandExecutor = commandExecutor; + this.history = history; // calls #setStyleToDefault() whenever there is a change to the text of the command box. commandTextField.textProperty().addListener((unused1, unused2, unused3) -> setStyleToDefault()); - historySnapshot = logic.getHistorySnapshot(); + historySnapshot = new ListElementPointer(history); } /** @@ -101,20 +98,13 @@ private void replaceText(String text) { @FXML private void handleCommandEntered() { try { - CommandResult commandResult = logic.execute(commandTextField.getText()); + commandExecutor.execute(commandTextField.getText()); initHistory(); historySnapshot.next(); - // process result of the command commandTextField.setText(""); - logger.info("Result: " + commandResult.feedbackToUser); - raise(new NewResultAvailableEvent(commandResult.feedbackToUser)); - } catch (CommandException | ParseException e) { initHistory(); - // handle command failure setStyleToIndicateCommandFailure(); - logger.info("Invalid command: " + commandTextField.getText()); - raise(new NewResultAvailableEvent(e.getMessage())); } } @@ -122,7 +112,7 @@ private void handleCommandEntered() { * Initializes the history snapshot. */ private void initHistory() { - historySnapshot = logic.getHistorySnapshot(); + historySnapshot = new ListElementPointer(history); // add an empty string to represent the most-recent end of historySnapshot, to be shown to // the user if she tries to navigate past the most-recent end of the historySnapshot. historySnapshot.add(""); @@ -148,4 +138,17 @@ private void setStyleToIndicateCommandFailure() { styleClass.add(ERROR_STYLE_CLASS); } + /** + * Represents a function that can execute commands. + */ + @FunctionalInterface + public interface CommandExecutor { + /** + * Executes the command and returns the result. + * + * @see seedu.address.logic.Logic#execute(String) + */ + CommandResult execute(String commandText) throws CommandException, ParseException; + } + } diff --git a/src/main/java/seedu/address/ui/HelpWindow.java b/src/main/java/seedu/address/ui/HelpWindow.java index 19de515ec8f0..22606d102f83 100644 --- a/src/main/java/seedu/address/ui/HelpWindow.java +++ b/src/main/java/seedu/address/ui/HelpWindow.java @@ -69,6 +69,13 @@ public boolean isShowing() { return getRoot().isShowing(); } + /** + * Hides the help window. + */ + public void hide() { + getRoot().hide(); + } + /** * Focuses on the help window. */ diff --git a/src/main/java/seedu/address/logic/ListElementPointer.java b/src/main/java/seedu/address/ui/ListElementPointer.java similarity index 99% rename from src/main/java/seedu/address/logic/ListElementPointer.java rename to src/main/java/seedu/address/ui/ListElementPointer.java index ca4085d98a11..54db5ae7efee 100644 --- a/src/main/java/seedu/address/logic/ListElementPointer.java +++ b/src/main/java/seedu/address/ui/ListElementPointer.java @@ -1,4 +1,4 @@ -package seedu.address.logic; +package seedu.address.ui; import java.util.ArrayList; import java.util.List; diff --git a/src/main/java/seedu/address/ui/MainWindow.java b/src/main/java/seedu/address/ui/MainWindow.java index 0e361a4d7baf..ac165736001d 100644 --- a/src/main/java/seedu/address/ui/MainWindow.java +++ b/src/main/java/seedu/address/ui/MainWindow.java @@ -2,8 +2,6 @@ import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - import javafx.event.ActionEvent; import javafx.fxml.FXML; import javafx.scene.control.MenuItem; @@ -12,13 +10,12 @@ import javafx.scene.input.KeyEvent; import javafx.scene.layout.StackPane; import javafx.stage.Stage; -import seedu.address.commons.core.Config; import seedu.address.commons.core.GuiSettings; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.ui.ExitAppRequestEvent; -import seedu.address.commons.events.ui.ShowHelpRequestEvent; import seedu.address.logic.Logic; -import seedu.address.model.UserPrefs; +import seedu.address.logic.commands.CommandResult; +import seedu.address.logic.commands.exceptions.CommandException; +import seedu.address.logic.parser.exceptions.ParseException; /** * The Main Window. Provides the basic application layout containing @@ -36,8 +33,7 @@ public class MainWindow extends UiPart { // Independent Ui parts residing in this Ui container private BrowserPanel browserPanel; private PersonListPanel personListPanel; - private Config config; - private UserPrefs prefs; + private ResultDisplay resultDisplay; private HelpWindow helpWindow; @FXML @@ -58,21 +54,17 @@ public class MainWindow extends UiPart { @FXML private StackPane statusbarPlaceholder; - public MainWindow(Stage primaryStage, Config config, UserPrefs prefs, Logic logic) { + public MainWindow(Stage primaryStage, Logic logic) { super(FXML, primaryStage); // Set dependencies this.primaryStage = primaryStage; this.logic = logic; - this.config = config; - this.prefs = prefs; // Configure the UI - setTitle(config.getAppTitle()); - setWindowDefaultSize(prefs); + setWindowDefaultSize(logic.getGuiSettings()); setAccelerators(); - registerAsAnEventHandler(this); helpWindow = new HelpWindow(); } @@ -119,50 +111,35 @@ private void setAccelerator(MenuItem menuItem, KeyCombination keyCombination) { * Fills up all the placeholders of this window. */ void fillInnerParts() { - browserPanel = new BrowserPanel(); + browserPanel = new BrowserPanel(logic.selectedPersonProperty()); browserPlaceholder.getChildren().add(browserPanel.getRoot()); - personListPanel = new PersonListPanel(logic.getFilteredPersonList()); + personListPanel = new PersonListPanel(logic.getFilteredPersonList(), logic.selectedPersonProperty(), + logic::setSelectedPerson); personListPanelPlaceholder.getChildren().add(personListPanel.getRoot()); - ResultDisplay resultDisplay = new ResultDisplay(); + resultDisplay = new ResultDisplay(); resultDisplayPlaceholder.getChildren().add(resultDisplay.getRoot()); - StatusBarFooter statusBarFooter = new StatusBarFooter(prefs.getAddressBookFilePath()); + StatusBarFooter statusBarFooter = new StatusBarFooter(logic.getAddressBookFilePath(), logic.getAddressBook()); statusbarPlaceholder.getChildren().add(statusBarFooter.getRoot()); - CommandBox commandBox = new CommandBox(logic); + CommandBox commandBox = new CommandBox(this::executeCommand, logic.getHistory()); commandBoxPlaceholder.getChildren().add(commandBox.getRoot()); } - void hide() { - primaryStage.hide(); - } - - private void setTitle(String appTitle) { - primaryStage.setTitle(appTitle); - } - /** - * Sets the default size based on user preferences. + * Sets the default size based on {@code guiSettings}. */ - private void setWindowDefaultSize(UserPrefs prefs) { - primaryStage.setHeight(prefs.getGuiSettings().getWindowHeight()); - primaryStage.setWidth(prefs.getGuiSettings().getWindowWidth()); - if (prefs.getGuiSettings().getWindowCoordinates() != null) { - primaryStage.setX(prefs.getGuiSettings().getWindowCoordinates().getX()); - primaryStage.setY(prefs.getGuiSettings().getWindowCoordinates().getY()); + private void setWindowDefaultSize(GuiSettings guiSettings) { + primaryStage.setHeight(guiSettings.getWindowHeight()); + primaryStage.setWidth(guiSettings.getWindowWidth()); + if (guiSettings.getWindowCoordinates() != null) { + primaryStage.setX(guiSettings.getWindowCoordinates().getX()); + primaryStage.setY(guiSettings.getWindowCoordinates().getY()); } } - /** - * Returns the current size and the position of the main Window. - */ - GuiSettings getCurrentGuiSetting() { - return new GuiSettings(primaryStage.getWidth(), primaryStage.getHeight(), - (int) primaryStage.getX(), (int) primaryStage.getY()); - } - /** * Opens the help window or focuses on it if it's already opened. */ @@ -184,20 +161,41 @@ void show() { */ @FXML private void handleExit() { - raise(new ExitAppRequestEvent()); + GuiSettings guiSettings = new GuiSettings(primaryStage.getWidth(), primaryStage.getHeight(), + (int) primaryStage.getX(), (int) primaryStage.getY()); + logic.setGuiSettings(guiSettings); + helpWindow.hide(); + primaryStage.hide(); } public PersonListPanel getPersonListPanel() { return personListPanel; } - void releaseResources() { - browserPanel.freeResources(); - } + /** + * Executes the command and returns the result. + * + * @see seedu.address.logic.Logic#execute(String) + */ + private CommandResult executeCommand(String commandText) throws CommandException, ParseException { + try { + CommandResult commandResult = logic.execute(commandText); + logger.info("Result: " + commandResult.getFeedbackToUser()); + resultDisplay.setFeedbackToUser(commandResult.getFeedbackToUser()); + + if (commandResult.isShowHelp()) { + handleHelp(); + } - @Subscribe - private void handleShowHelpEvent(ShowHelpRequestEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - handleHelp(); + if (commandResult.isExit()) { + handleExit(); + } + + return commandResult; + } catch (CommandException | ParseException e) { + logger.info("Invalid command: " + commandText); + resultDisplay.setFeedbackToUser(e.getMessage()); + throw e; + } } } diff --git a/src/main/java/seedu/address/ui/PersonCard.java b/src/main/java/seedu/address/ui/PersonCard.java index f6727ea83abd..0684b088868a 100644 --- a/src/main/java/seedu/address/ui/PersonCard.java +++ b/src/main/java/seedu/address/ui/PersonCard.java @@ -1,5 +1,7 @@ package seedu.address.ui; +import java.util.Comparator; + import javafx.fxml.FXML; import javafx.scene.control.Label; import javafx.scene.layout.FlowPane; @@ -47,7 +49,9 @@ public PersonCard(Person person, int displayedIndex) { phone.setText(person.getPhone().value); address.setText(person.getAddress().value); email.setText(person.getEmail().value); - person.getTags().forEach(tag -> tags.getChildren().add(new Label(tag.tagName))); + person.getTags().stream() + .sorted(Comparator.comparing(tag -> tag.tagName)) + .forEach(tag -> tags.getChildren().add(new Label(tag.tagName))); } @Override diff --git a/src/main/java/seedu/address/ui/PersonListPanel.java b/src/main/java/seedu/address/ui/PersonListPanel.java index 80080adb4305..5ca3fa4fc671 100644 --- a/src/main/java/seedu/address/ui/PersonListPanel.java +++ b/src/main/java/seedu/address/ui/PersonListPanel.java @@ -1,18 +1,16 @@ package seedu.address.ui; +import java.util.Objects; +import java.util.function.Consumer; import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - -import javafx.application.Platform; +import javafx.beans.value.ObservableValue; import javafx.collections.ObservableList; import javafx.fxml.FXML; import javafx.scene.control.ListCell; import javafx.scene.control.ListView; import javafx.scene.layout.Region; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.ui.JumpToListRequestEvent; -import seedu.address.commons.events.ui.PersonPanelSelectionChangedEvent; import seedu.address.model.person.Person; /** @@ -25,44 +23,34 @@ public class PersonListPanel extends UiPart { @FXML private ListView personListView; - public PersonListPanel(ObservableList personList) { + public PersonListPanel(ObservableList personList, ObservableValue selectedPerson, + Consumer onSelectedPersonChange) { super(FXML); - setConnections(personList); - registerAsAnEventHandler(this); - } - - private void setConnections(ObservableList personList) { personListView.setItems(personList); personListView.setCellFactory(listView -> new PersonListViewCell()); - setEventHandlerForSelectionChangeEvent(); - } + personListView.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> { + logger.fine("Selection in person list panel changed to : '" + newValue + "'"); + onSelectedPersonChange.accept(newValue); + }); + selectedPerson.addListener((observable, oldValue, newValue) -> { + logger.fine("Selected person changed to: " + newValue); - private void setEventHandlerForSelectionChangeEvent() { - personListView.getSelectionModel().selectedItemProperty() - .addListener((observable, oldValue, newValue) -> { - if (newValue != null) { - logger.fine("Selection in person list panel changed to : '" + newValue + "'"); - raise(new PersonPanelSelectionChangedEvent(newValue)); - } - }); - } + // Don't modify selection if we are already selecting the selected person, + // otherwise we would have an infinite loop. + if (Objects.equals(personListView.getSelectionModel().getSelectedItem(), newValue)) { + return; + } - /** - * Scrolls to the {@code PersonCard} at the {@code index} and selects it. - */ - private void scrollTo(int index) { - Platform.runLater(() -> { - personListView.scrollTo(index); - personListView.getSelectionModel().clearAndSelect(index); + if (newValue == null) { + personListView.getSelectionModel().clearSelection(); + } else { + int index = personListView.getItems().indexOf(newValue); + personListView.scrollTo(index); + personListView.getSelectionModel().clearAndSelect(index); + } }); } - @Subscribe - private void handleJumpToListRequestEvent(JumpToListRequestEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - scrollTo(event.targetIndex); - } - /** * Custom {@code ListCell} that displays the graphics of a {@code Person} using a {@code PersonCard}. */ diff --git a/src/main/java/seedu/address/ui/ResultDisplay.java b/src/main/java/seedu/address/ui/ResultDisplay.java index d05536bbee96..7d98e84eedf0 100644 --- a/src/main/java/seedu/address/ui/ResultDisplay.java +++ b/src/main/java/seedu/address/ui/ResultDisplay.java @@ -1,41 +1,28 @@ package seedu.address.ui; -import java.util.logging.Logger; +import static java.util.Objects.requireNonNull; -import com.google.common.eventbus.Subscribe; - -import javafx.application.Platform; -import javafx.beans.property.SimpleStringProperty; -import javafx.beans.property.StringProperty; import javafx.fxml.FXML; import javafx.scene.control.TextArea; import javafx.scene.layout.Region; -import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.ui.NewResultAvailableEvent; /** * A ui for the status bar that is displayed at the header of the application. */ public class ResultDisplay extends UiPart { - private static final Logger logger = LogsCenter.getLogger(ResultDisplay.class); private static final String FXML = "ResultDisplay.fxml"; - private final StringProperty displayed = new SimpleStringProperty(""); - @FXML private TextArea resultDisplay; public ResultDisplay() { super(FXML); - resultDisplay.textProperty().bind(displayed); - registerAsAnEventHandler(this); } - @Subscribe - private void handleNewResultAvailableEvent(NewResultAvailableEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - Platform.runLater(() -> displayed.setValue(event.message)); + public void setFeedbackToUser(String feedbackToUser) { + requireNonNull(feedbackToUser); + resultDisplay.setText(feedbackToUser); } } diff --git a/src/main/java/seedu/address/ui/StatusBarFooter.java b/src/main/java/seedu/address/ui/StatusBarFooter.java index f6ba29502422..b22e1f525256 100644 --- a/src/main/java/seedu/address/ui/StatusBarFooter.java +++ b/src/main/java/seedu/address/ui/StatusBarFooter.java @@ -4,17 +4,11 @@ import java.nio.file.Paths; import java.time.Clock; import java.util.Date; -import java.util.logging.Logger; -import org.controlsfx.control.StatusBar; - -import com.google.common.eventbus.Subscribe; - -import javafx.application.Platform; import javafx.fxml.FXML; +import javafx.scene.control.Label; import javafx.scene.layout.Region; -import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.model.AddressBookChangedEvent; +import seedu.address.model.ReadOnlyAddressBook; /** * A ui for the status bar that is displayed at the footer of the application. @@ -34,21 +28,19 @@ public class StatusBarFooter extends UiPart { */ private static Clock clock = Clock.systemDefaultZone(); - private static final Logger logger = LogsCenter.getLogger(StatusBarFooter.class); - private static final String FXML = "StatusBarFooter.fxml"; @FXML - private StatusBar syncStatus; + private Label syncStatus; @FXML - private StatusBar saveLocationStatus; + private Label saveLocationStatus; - public StatusBarFooter(Path saveLocation) { + public StatusBarFooter(Path saveLocation, ReadOnlyAddressBook addressBook) { super(FXML); - setSyncStatus(SYNC_STATUS_INITIAL); - setSaveLocation(Paths.get(".").resolve(saveLocation).toString()); - registerAsAnEventHandler(this); + addressBook.addListener(observable -> updateSyncStatus()); + syncStatus.setText(SYNC_STATUS_INITIAL); + saveLocationStatus.setText(Paths.get(".").resolve(saveLocation).toString()); } /** @@ -65,19 +57,13 @@ public static Clock getClock() { return clock; } - private void setSaveLocation(String location) { - Platform.runLater(() -> saveLocationStatus.setText(location)); - } - - private void setSyncStatus(String status) { - Platform.runLater(() -> syncStatus.setText(status)); - } - - @Subscribe - public void handleAddressBookChangedEvent(AddressBookChangedEvent abce) { + /** + * Updates "last updated" status to the current time. + */ + private void updateSyncStatus() { long now = clock.millis(); String lastUpdated = new Date(now).toString(); - logger.info(LogsCenter.getEventHandlingLogMessage(abce, "Setting last updated status to " + lastUpdated)); - setSyncStatus(String.format(SYNC_STATUS_UPDATED, lastUpdated)); + syncStatus.setText(String.format(SYNC_STATUS_UPDATED, lastUpdated)); } + } diff --git a/src/main/java/seedu/address/ui/Ui.java b/src/main/java/seedu/address/ui/Ui.java index e6a67fe8c027..17aa0b494fe3 100644 --- a/src/main/java/seedu/address/ui/Ui.java +++ b/src/main/java/seedu/address/ui/Ui.java @@ -10,7 +10,4 @@ public interface Ui { /** Starts the UI (and the App). */ void start(Stage primaryStage); - /** Stops the UI. */ - void stop(); - } diff --git a/src/main/java/seedu/address/ui/UiManager.java b/src/main/java/seedu/address/ui/UiManager.java index 3fd3c17be156..876621d79b94 100644 --- a/src/main/java/seedu/address/ui/UiManager.java +++ b/src/main/java/seedu/address/ui/UiManager.java @@ -2,46 +2,32 @@ import java.util.logging.Logger; -import com.google.common.eventbus.Subscribe; - import javafx.application.Platform; import javafx.scene.control.Alert; import javafx.scene.control.Alert.AlertType; import javafx.scene.image.Image; import javafx.stage.Stage; import seedu.address.MainApp; -import seedu.address.commons.core.ComponentManager; -import seedu.address.commons.core.Config; import seedu.address.commons.core.LogsCenter; -import seedu.address.commons.events.storage.DataSavingExceptionEvent; import seedu.address.commons.util.StringUtil; import seedu.address.logic.Logic; -import seedu.address.model.UserPrefs; /** * The manager of the UI component. */ -public class UiManager extends ComponentManager implements Ui { +public class UiManager implements Ui { public static final String ALERT_DIALOG_PANE_FIELD_ID = "alertDialogPane"; - public static final String FILE_OPS_ERROR_DIALOG_STAGE_TITLE = "File Op Error"; - public static final String FILE_OPS_ERROR_DIALOG_HEADER_MESSAGE = "Could not save data"; - public static final String FILE_OPS_ERROR_DIALOG_CONTENT_MESSAGE = "Could not save data to file"; - private static final Logger logger = LogsCenter.getLogger(UiManager.class); private static final String ICON_APPLICATION = "/images/address_book_32.png"; private Logic logic; - private Config config; - private UserPrefs prefs; private MainWindow mainWindow; - public UiManager(Logic logic, Config config, UserPrefs prefs) { + public UiManager(Logic logic) { super(); this.logic = logic; - this.config = config; - this.prefs = prefs; } @Override @@ -52,7 +38,7 @@ public void start(Stage primaryStage) { primaryStage.getIcons().add(getImage(ICON_APPLICATION)); try { - mainWindow = new MainWindow(primaryStage, config, prefs, logic); + mainWindow = new MainWindow(primaryStage, logic); mainWindow.show(); //This should be called before creating other UI parts mainWindow.fillInnerParts(); @@ -62,18 +48,6 @@ public void start(Stage primaryStage) { } } - @Override - public void stop() { - prefs.updateLastUsedGuiSetting(mainWindow.getCurrentGuiSetting()); - mainWindow.hide(); - mainWindow.releaseResources(); - } - - private void showFileOperationAlertAndWait(String description, String details, Throwable cause) { - final String content = details + ":\n" + cause.toString(); - showAlertDialogAndWait(AlertType.ERROR, FILE_OPS_ERROR_DIALOG_STAGE_TITLE, description, content); - } - private Image getImage(String imagePath) { return new Image(MainApp.class.getResourceAsStream(imagePath)); } @@ -109,12 +83,4 @@ private void showFatalErrorDialogAndShutdown(String title, Throwable e) { System.exit(1); } - //==================== Event Handling Code =============================================================== - - @Subscribe - private void handleDataSavingExceptionEvent(DataSavingExceptionEvent event) { - logger.info(LogsCenter.getEventHandlingLogMessage(event)); - showFileOperationAlertAndWait(FILE_OPS_ERROR_DIALOG_HEADER_MESSAGE, FILE_OPS_ERROR_DIALOG_CONTENT_MESSAGE, - event.exception); - } } diff --git a/src/main/java/seedu/address/ui/UiPart.java b/src/main/java/seedu/address/ui/UiPart.java index 5c237e57154b..fc820e01a9c3 100644 --- a/src/main/java/seedu/address/ui/UiPart.java +++ b/src/main/java/seedu/address/ui/UiPart.java @@ -7,8 +7,6 @@ import javafx.fxml.FXMLLoader; import seedu.address.MainApp; -import seedu.address.commons.core.EventsCenter; -import seedu.address.commons.events.BaseEvent; /** * Represents a distinct part of the UI. e.g. Windows, dialogs, panels, status bars, etc. @@ -60,22 +58,6 @@ public T getRoot() { return fxmlLoader.getRoot(); } - /** - * Raises the event via {@link EventsCenter#post(BaseEvent)} - * @param event - */ - protected void raise(BaseEvent event) { - EventsCenter.getInstance().post(event); - } - - /** - * Registers the object as an event handler at the {@link EventsCenter} - * @param handler usually {@code this} - */ - protected void registerAsAnEventHandler(Object handler) { - EventsCenter.getInstance().registerHandler(handler); - } - /** * Loads the object hierarchy from a FXML document. * @param location Location of the FXML document. diff --git a/src/main/resources/view/DarkTheme.css b/src/main/resources/view/DarkTheme.css index c8941ea18263..36e6b001cd8d 100644 --- a/src/main/resources/view/DarkTheme.css +++ b/src/main/resources/view/DarkTheme.css @@ -143,8 +143,7 @@ } .status-bar { - -fx-background-color: derive(#1d1d1d, 20%); - -fx-text-fill: black; + -fx-background-color: derive(#1d1d1d, 30%); } .result-display { @@ -161,6 +160,8 @@ .status-bar .label { -fx-font-family: "Segoe UI Light"; -fx-text-fill: white; + -fx-padding: 4px; + -fx-pref-height: 30px; } .status-bar-with-border { diff --git a/src/main/resources/view/MainWindow.fxml b/src/main/resources/view/MainWindow.fxml index daf386d8f5b8..601ddabf2b5e 100644 --- a/src/main/resources/view/MainWindow.fxml +++ b/src/main/resources/view/MainWindow.fxml @@ -12,7 +12,7 @@ + title="Address App" minWidth="450" minHeight="600" onCloseRequest="#handleExit"> diff --git a/src/main/resources/view/StatusBarFooter.fxml b/src/main/resources/view/StatusBarFooter.fxml index 041e1ff9004f..1272a7fb8ea5 100644 --- a/src/main/resources/view/StatusBarFooter.fxml +++ b/src/main/resources/view/StatusBarFooter.fxml @@ -1,14 +1,14 @@ - + - + - - + + - - + diff --git a/src/test/data/ConfigUtilTest/ExtraValuesConfig.json b/src/test/data/ConfigUtilTest/ExtraValuesConfig.json index 413273f7b26f..f5a2b3d02938 100644 --- a/src/test/data/ConfigUtilTest/ExtraValuesConfig.json +++ b/src/test/data/ConfigUtilTest/ExtraValuesConfig.json @@ -1,5 +1,4 @@ { - "appTitle" : "Typical App Title", "logLevel" : "INFO", "userPrefsFilePath" : "preferences.json", "extra" : "extra value" diff --git a/src/test/data/ConfigUtilTest/TypicalConfig.json b/src/test/data/ConfigUtilTest/TypicalConfig.json index fbf982090fef..a0edc78a820a 100644 --- a/src/test/data/ConfigUtilTest/TypicalConfig.json +++ b/src/test/data/ConfigUtilTest/TypicalConfig.json @@ -1,5 +1,4 @@ { - "appTitle" : "Typical App Title", "logLevel" : "INFO", "userPrefsFilePath" : "preferences.json" } diff --git a/src/test/data/JsonAddressBookStorageTest/invalidAndValidPersonAddressBook.json b/src/test/data/JsonAddressBookStorageTest/invalidAndValidPersonAddressBook.json new file mode 100644 index 000000000000..6a4d2b7181c3 --- /dev/null +++ b/src/test/data/JsonAddressBookStorageTest/invalidAndValidPersonAddressBook.json @@ -0,0 +1,13 @@ +{ + "persons": [ { + "name": "Valid Person", + "phone": "9482424", + "email": "hans@example.com", + "address": "4th street" + }, { + "name": "Person With Invalid Phone Field", + "phone": "948asdf2424", + "email": "hans@example.com", + "address": "4th street" + } ] +} diff --git a/src/test/data/JsonAddressBookStorageTest/invalidPersonAddressBook.json b/src/test/data/JsonAddressBookStorageTest/invalidPersonAddressBook.json new file mode 100644 index 000000000000..ccd21f7d1a93 --- /dev/null +++ b/src/test/data/JsonAddressBookStorageTest/invalidPersonAddressBook.json @@ -0,0 +1,8 @@ +{ + "persons": [ { + "name": "Person with invalid name field: Ha!ns Mu@ster", + "phone": "9482424", + "email": "hans@example.com", + "address": "4th street" + } ] +} diff --git a/src/test/data/JsonAddressBookStorageTest/notJsonFormatAddressBook.json b/src/test/data/JsonAddressBookStorageTest/notJsonFormatAddressBook.json new file mode 100644 index 000000000000..a1097343b5d7 --- /dev/null +++ b/src/test/data/JsonAddressBookStorageTest/notJsonFormatAddressBook.json @@ -0,0 +1 @@ +not json format! diff --git a/src/test/data/JsonSerializableAddressBookTest/duplicatePersonAddressBook.json b/src/test/data/JsonSerializableAddressBookTest/duplicatePersonAddressBook.json new file mode 100644 index 000000000000..48831cc76744 --- /dev/null +++ b/src/test/data/JsonSerializableAddressBookTest/duplicatePersonAddressBook.json @@ -0,0 +1,14 @@ +{ + "persons": [ { + "name": "Alice Pauline", + "phone": "94351253", + "email": "alice@example.com", + "address": "123, Jurong West Ave 6, #08-111", + "tagged": [ "friends" ] + }, { + "name": "Alice Pauline", + "phone": "94351253", + "email": "pauline@example.com", + "address": "4th street" + } ] +} diff --git a/src/test/data/JsonSerializableAddressBookTest/invalidPersonAddressBook.json b/src/test/data/JsonSerializableAddressBookTest/invalidPersonAddressBook.json new file mode 100644 index 000000000000..ad3f135ae428 --- /dev/null +++ b/src/test/data/JsonSerializableAddressBookTest/invalidPersonAddressBook.json @@ -0,0 +1,8 @@ +{ + "persons": [ { + "name": "Hans Muster", + "phone": "9482424", + "email": "invalid@email!3e", + "address": "4th street" + } ] +} diff --git a/src/test/data/JsonSerializableAddressBookTest/typicalPersonsAddressBook.json b/src/test/data/JsonSerializableAddressBookTest/typicalPersonsAddressBook.json new file mode 100644 index 000000000000..f10eddee12ed --- /dev/null +++ b/src/test/data/JsonSerializableAddressBookTest/typicalPersonsAddressBook.json @@ -0,0 +1,46 @@ +{ + "_comment": "AddressBook save file which contains the same Person values as in TypicalPersons#getTypicalAddressBook()", + "persons" : [ { + "name" : "Alice Pauline", + "phone" : "94351253", + "email" : "alice@example.com", + "address" : "123, Jurong West Ave 6, #08-111", + "tagged" : [ "friends" ] + }, { + "name" : "Benson Meier", + "phone" : "98765432", + "email" : "johnd@example.com", + "address" : "311, Clementi Ave 2, #02-25", + "tagged" : [ "owesMoney", "friends" ] + }, { + "name" : "Carl Kurz", + "phone" : "95352563", + "email" : "heinz@example.com", + "address" : "wall street", + "tagged" : [ ] + }, { + "name" : "Daniel Meier", + "phone" : "87652533", + "email" : "cornelia@example.com", + "address" : "10th street", + "tagged" : [ "friends" ] + }, { + "name" : "Elle Meyer", + "phone" : "9482224", + "email" : "werner@example.com", + "address" : "michegan ave", + "tagged" : [ ] + }, { + "name" : "Fiona Kunz", + "phone" : "9482427", + "email" : "lydia@example.com", + "address" : "little tokyo", + "tagged" : [ ] + }, { + "name" : "George Best", + "phone" : "9482442", + "email" : "anna@example.com", + "address" : "4th street", + "tagged" : [ ] + } ] +} diff --git a/src/test/data/JsonUserPrefsStorageTest/ExtraValuesUserPref.json b/src/test/data/JsonUserPrefsStorageTest/ExtraValuesUserPref.json index a312cecf8bad..1037548a9cde 100644 --- a/src/test/data/JsonUserPrefsStorageTest/ExtraValuesUserPref.json +++ b/src/test/data/JsonUserPrefsStorageTest/ExtraValuesUserPref.json @@ -9,5 +9,5 @@ "z" : 99 } }, - "addressBookFilePath" : "addressbook.xml" + "addressBookFilePath" : "addressbook.json" } diff --git a/src/test/data/JsonUserPrefsStorageTest/TypicalUserPref.json b/src/test/data/JsonUserPrefsStorageTest/TypicalUserPref.json index 412dbd7cac65..b819bed900a1 100644 --- a/src/test/data/JsonUserPrefsStorageTest/TypicalUserPref.json +++ b/src/test/data/JsonUserPrefsStorageTest/TypicalUserPref.json @@ -7,5 +7,5 @@ "y" : 100 } }, - "addressBookFilePath" : "addressbook.xml" + "addressBookFilePath" : "addressbook.json" } diff --git a/src/test/data/XmlAddressBookStorageTest/NotXmlFormatAddressBook.xml b/src/test/data/XmlAddressBookStorageTest/NotXmlFormatAddressBook.xml deleted file mode 100644 index 35efc0f50820..000000000000 --- a/src/test/data/XmlAddressBookStorageTest/NotXmlFormatAddressBook.xml +++ /dev/null @@ -1 +0,0 @@ -not xml format! diff --git a/src/test/data/XmlAddressBookStorageTest/invalidAndValidPersonAddressBook.xml b/src/test/data/XmlAddressBookStorageTest/invalidAndValidPersonAddressBook.xml deleted file mode 100644 index 41e411568a5f..000000000000 --- a/src/test/data/XmlAddressBookStorageTest/invalidAndValidPersonAddressBook.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - Hans Muster - 9482424 - hans@example.com -
4th street
- - - - Hans Muster - 948asdf2424 - hans@example.com -
4th street
- - diff --git a/src/test/data/XmlAddressBookStorageTest/invalidPersonAddressBook.xml b/src/test/data/XmlAddressBookStorageTest/invalidPersonAddressBook.xml deleted file mode 100644 index cfa128e72828..000000000000 --- a/src/test/data/XmlAddressBookStorageTest/invalidPersonAddressBook.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - Ha!ns Mu@ster - 9482424 - hans@example.com -
4th street
- - diff --git a/src/test/data/XmlSerializableAddressBookTest/duplicatePersonAddressBook.xml b/src/test/data/XmlSerializableAddressBookTest/duplicatePersonAddressBook.xml deleted file mode 100644 index ac02230263d3..000000000000 --- a/src/test/data/XmlSerializableAddressBookTest/duplicatePersonAddressBook.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - - - Alice Pauline - 94351253 - alice@example.com -
123, Jurong West Ave 6, #08-111
- friends - - - - - Alice Pauline - 94351253 - pauline@example.com -
4th street
- - - diff --git a/src/test/data/XmlSerializableAddressBookTest/invalidPersonAddressBook.xml b/src/test/data/XmlSerializableAddressBookTest/invalidPersonAddressBook.xml deleted file mode 100644 index 13d5b1cb1c8a..000000000000 --- a/src/test/data/XmlSerializableAddressBookTest/invalidPersonAddressBook.xml +++ /dev/null @@ -1,10 +0,0 @@ - - - - - Hans Muster - 9482424 - hans@exam!32ple -
4th street
- - diff --git a/src/test/data/XmlSerializableAddressBookTest/typicalPersonsAddressBook.xml b/src/test/data/XmlSerializableAddressBookTest/typicalPersonsAddressBook.xml deleted file mode 100644 index d812b05e32bb..000000000000 --- a/src/test/data/XmlSerializableAddressBookTest/typicalPersonsAddressBook.xml +++ /dev/null @@ -1,50 +0,0 @@ - - - - - Alice Pauline - 94351253 - alice@example.com -
123, Jurong West Ave 6, #08-111
- friends - - - Benson Meier - 98765432 - johnd@example.com -
311, Clementi Ave 2, #02-25
- owesMoney - friends - - - Carl Kurz - 95352563 - heinz@example.com -
wall street
- - - Daniel Meier - 87652533 - cornelia@example.com -
10th street
- friends - - - Elle Meyer - 9482224 - werner@example.com -
michegan ave
- - - Fiona Kunz - 9482427 - lydia@example.com -
little tokyo
- - - George Best - 9482442 - anna@example.com -
4th street
- - diff --git a/src/test/data/XmlUtilTest/empty.xml b/src/test/data/XmlUtilTest/empty.xml deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/src/test/data/XmlUtilTest/invalidPersonField.xml b/src/test/data/XmlUtilTest/invalidPersonField.xml deleted file mode 100644 index ba49c971e884..000000000000 --- a/src/test/data/XmlUtilTest/invalidPersonField.xml +++ /dev/null @@ -1,9 +0,0 @@ - - - - Hans Muster - 9482asf424 - hans@example -
4th street
- friends - diff --git a/src/test/data/XmlUtilTest/missingPersonField.xml b/src/test/data/XmlUtilTest/missingPersonField.xml deleted file mode 100644 index c0da5c86d080..000000000000 --- a/src/test/data/XmlUtilTest/missingPersonField.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - 9482424 - hans@example -
4th street
- friends - diff --git a/src/test/data/XmlUtilTest/tempAddressBook.xml b/src/test/data/XmlUtilTest/tempAddressBook.xml deleted file mode 100644 index 4773cf598f4b..000000000000 --- a/src/test/data/XmlUtilTest/tempAddressBook.xml +++ /dev/null @@ -1,12 +0,0 @@ - - - - 1 - John - Doe - - - - - - diff --git a/src/test/data/XmlUtilTest/validAddressBook.xml b/src/test/data/XmlUtilTest/validAddressBook.xml deleted file mode 100644 index 6265778674d3..000000000000 --- a/src/test/data/XmlUtilTest/validAddressBook.xml +++ /dev/null @@ -1,57 +0,0 @@ - - - - Hans Muster - 9482424 - hans@example.com -
4th street
- - - Ruth Mueller - 87249245 - ruth@example.com -
81th street
- - - Heinz Kurz - 95352563 - heinz@example.com -
wall street
- - - Cornelia Meier - 87652533 - cornelia@example.com -
10th street
- - - Werner Meyer - 9482224 - werner@example.com -
michegan ave
- - - Lydia Kunz - 9482427 - lydia@example.com -
little tokyo
- - - Anna Best - 9482442 - anna@example.com -
4th street
- - - Stefan Meier - 8482424 - stefan@example.com -
little india
- - - Martin Mueller - 8482131 - hans@example.com -
chicago ave
- - diff --git a/src/test/data/XmlUtilTest/validPerson.xml b/src/test/data/XmlUtilTest/validPerson.xml deleted file mode 100644 index c029008d54f4..000000000000 --- a/src/test/data/XmlUtilTest/validPerson.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - Hans Muster - 9482424 - hans@example -
4th street
- friends - diff --git a/src/test/java/guitests/GuiRobot.java b/src/test/java/guitests/GuiRobot.java index f4e0700b22f7..7f3031fbef16 100644 --- a/src/test/java/guitests/GuiRobot.java +++ b/src/test/java/guitests/GuiRobot.java @@ -103,7 +103,7 @@ public int getNumberOfWindowsShown(String stageTitle) { */ public Stage getStage(String stageTitle) { Optional targetStage = listTargetWindows().stream() - .filter(Stage.class::isInstance) // checks that the window is of type Stage + .filter(Stage.class::isInstance) // checks that the window is of type Stage .map(Stage.class::cast) .filter(stage -> stage.getTitle().equals(stageTitle)) .findFirst(); diff --git a/src/test/java/guitests/guihandles/PersonCardHandle.java b/src/test/java/guitests/guihandles/PersonCardHandle.java index 1789735e49a8..1aa604a58811 100644 --- a/src/test/java/guitests/guihandles/PersonCardHandle.java +++ b/src/test/java/guitests/guihandles/PersonCardHandle.java @@ -3,8 +3,6 @@ import java.util.List; import java.util.stream.Collectors; -import com.google.common.collect.ImmutableMultiset; - import javafx.scene.Node; import javafx.scene.control.Label; import javafx.scene.layout.Region; @@ -80,8 +78,9 @@ public boolean equals(Person person) { && getAddress().equals(person.getAddress().value) && getPhone().equals(person.getPhone().value) && getEmail().equals(person.getEmail().value) - && ImmutableMultiset.copyOf(getTags()).equals(ImmutableMultiset.copyOf(person.getTags().stream() + && getTags().equals(person.getTags().stream() .map(tag -> tag.tagName) - .collect(Collectors.toList()))); + .sorted() + .collect(Collectors.toList())); } } diff --git a/src/test/java/guitests/guihandles/StatusBarFooterHandle.java b/src/test/java/guitests/guihandles/StatusBarFooterHandle.java index 33c5d1d788b8..06e97755ac1b 100644 --- a/src/test/java/guitests/guihandles/StatusBarFooterHandle.java +++ b/src/test/java/guitests/guihandles/StatusBarFooterHandle.java @@ -1,8 +1,7 @@ package guitests.guihandles; -import org.controlsfx.control.StatusBar; - import javafx.scene.Node; +import javafx.scene.control.Labeled; /** * A handle for the {@code StatusBarFooter} at the footer of the application. @@ -13,8 +12,8 @@ public class StatusBarFooterHandle extends NodeHandle { private static final String SYNC_STATUS_ID = "#syncStatus"; private static final String SAVE_LOCATION_STATUS_ID = "#saveLocationStatus"; - private final StatusBar syncStatusNode; - private final StatusBar saveLocationNode; + private final Labeled syncStatusNode; + private final Labeled saveLocationNode; private String lastRememberedSyncStatus; private String lastRememberedSaveLocation; diff --git a/src/test/java/seedu/address/TestApp.java b/src/test/java/seedu/address/TestApp.java index 74fe55a2e430..3d510937d0cb 100644 --- a/src/test/java/seedu/address/TestApp.java +++ b/src/test/java/seedu/address/TestApp.java @@ -9,15 +9,13 @@ import seedu.address.commons.core.Config; import seedu.address.commons.core.GuiSettings; import seedu.address.commons.exceptions.DataConversionException; -import seedu.address.commons.util.FileUtil; -import seedu.address.commons.util.XmlUtil; import seedu.address.model.AddressBook; import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.UserPrefs; +import seedu.address.storage.JsonAddressBookStorage; import seedu.address.storage.UserPrefsStorage; -import seedu.address.storage.XmlSerializableAddressBook; import seedu.address.testutil.TestUtil; import systemtests.ModelHelper; @@ -27,8 +25,7 @@ */ public class TestApp extends MainApp { - public static final Path SAVE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("sampleData.xml"); - public static final String APP_TITLE = "Test App"; + public static final Path SAVE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("sampleData.json"); protected static final Path DEFAULT_PREF_FILE_LOCATION_FOR_TESTING = TestUtil.getFilePathInSandboxFolder("pref_testing.json"); @@ -45,15 +42,18 @@ public TestApp(Supplier initialDataSupplier, Path saveFileL // If some initial local data has been provided, write those to the file if (initialDataSupplier.get() != null) { - createDataFileWithData(new XmlSerializableAddressBook(this.initialDataSupplier.get()), - this.saveFileLocation); + JsonAddressBookStorage jsonAddressBookStorage = new JsonAddressBookStorage(saveFileLocation); + try { + jsonAddressBookStorage.saveAddressBook(initialDataSupplier.get()); + } catch (IOException ioe) { + throw new AssertionError(ioe); + } } } @Override protected Config initConfig(Path configFilePath) { Config config = super.initConfig(configFilePath); - config.setAppTitle(APP_TITLE); config.setUserPrefsFilePath(DEFAULT_PREF_FILE_LOCATION_FOR_TESTING); return config; } @@ -63,7 +63,7 @@ protected UserPrefs initPrefs(UserPrefsStorage storage) { UserPrefs userPrefs = super.initPrefs(storage); double x = Screen.getPrimary().getVisualBounds().getMinX(); double y = Screen.getPrimary().getVisualBounds().getMinY(); - userPrefs.updateLastUsedGuiSetting(new GuiSettings(600.0, 600.0, (int) x, (int) y)); + userPrefs.setGuiSettings(new GuiSettings(600.0, 600.0, (int) x, (int) y)); userPrefs.setAddressBookFilePath(saveFileLocation); return userPrefs; } @@ -106,15 +106,4 @@ public static void main(String[] args) { launch(args); } - /** - * Creates an XML file at the {@code filePath} with the {@code data}. - */ - private void createDataFileWithData(T data, Path filePath) { - try { - FileUtil.createIfMissing(filePath); - XmlUtil.saveDataToFile(filePath, data); - } catch (Exception e) { - throw new RuntimeException(e); - } - } } diff --git a/src/test/java/seedu/address/commons/core/ConfigTest.java b/src/test/java/seedu/address/commons/core/ConfigTest.java index cc4bf567cb44..a9e7ab09648c 100644 --- a/src/test/java/seedu/address/commons/core/ConfigTest.java +++ b/src/test/java/seedu/address/commons/core/ConfigTest.java @@ -14,8 +14,7 @@ public class ConfigTest { @Test public void toString_defaultObject_stringReturned() { - String defaultConfigAsString = "App title : Address App\n" - + "Current log level : INFO\n" + String defaultConfigAsString = "Current log level : INFO\n" + "Preference file Location : preferences.json"; assertEquals(defaultConfigAsString, new Config().toString()); diff --git a/src/test/java/seedu/address/commons/core/VersionTest.java b/src/test/java/seedu/address/commons/core/VersionTest.java index c3014d633d27..7601efa25893 100644 --- a/src/test/java/seedu/address/commons/core/VersionTest.java +++ b/src/test/java/seedu/address/commons/core/VersionTest.java @@ -56,56 +56,56 @@ public void versionComparable_validVersion_compareToIsCorrect() { // Tests equality one = new Version(0, 0, 0, true); - another = new Version(0, 0, 0, true); + another = new Version(0, 0, 0, true); assertTrue(one.compareTo(another) == 0); one = new Version(11, 12, 13, false); - another = new Version(11, 12, 13, false); + another = new Version(11, 12, 13, false); assertTrue(one.compareTo(another) == 0); // Tests different patch one = new Version(0, 0, 5, false); - another = new Version(0, 0, 0, false); + another = new Version(0, 0, 0, false); assertTrue(one.compareTo(another) > 0); // Tests different minor one = new Version(0, 0, 0, false); - another = new Version(0, 5, 0, false); + another = new Version(0, 5, 0, false); assertTrue(one.compareTo(another) < 0); // Tests different major one = new Version(10, 0, 0, true); - another = new Version(0, 0, 0, true); + another = new Version(0, 0, 0, true); assertTrue(one.compareTo(another) > 0); // Tests high major vs low minor one = new Version(10, 0, 0, true); - another = new Version(0, 1, 0, true); + another = new Version(0, 1, 0, true); assertTrue(one.compareTo(another) > 0); // Tests high patch vs low minor one = new Version(0, 0, 10, false); - another = new Version(0, 1, 0, false); + another = new Version(0, 1, 0, false); assertTrue(one.compareTo(another) < 0); // Tests same major minor different patch one = new Version(2, 15, 0, false); - another = new Version(2, 15, 5, false); + another = new Version(2, 15, 5, false); assertTrue(one.compareTo(another) < 0); // Tests early access vs not early access on same version number one = new Version(2, 15, 0, true); - another = new Version(2, 15, 0, false); + another = new Version(2, 15, 0, false); assertTrue(one.compareTo(another) < 0); // Tests early access lower version vs not early access higher version compare by version number first one = new Version(2, 15, 0, true); - another = new Version(2, 15, 5, false); + another = new Version(2, 15, 5, false); assertTrue(one.compareTo(another) < 0); // Tests early access higher version vs not early access lower version compare by version number first one = new Version(2, 15, 0, false); - another = new Version(2, 15, 5, true); + another = new Version(2, 15, 5, true); assertTrue(one.compareTo(another) < 0); } @@ -124,11 +124,11 @@ public void versionComparable_validVersion_equalIsCorrect() { Version another; one = new Version(0, 0, 0, false); - another = new Version(0, 0, 0, false); + another = new Version(0, 0, 0, false); assertTrue(one.equals(another)); one = new Version(100, 191, 275, true); - another = new Version(100, 191, 275, true); + another = new Version(100, 191, 275, true); assertTrue(one.equals(another)); } diff --git a/src/test/java/seedu/address/commons/util/AppUtilTest.java b/src/test/java/seedu/address/commons/util/AppUtilTest.java index 8a6fe5fcb7d6..b81493f4784b 100644 --- a/src/test/java/seedu/address/commons/util/AppUtilTest.java +++ b/src/test/java/seedu/address/commons/util/AppUtilTest.java @@ -42,6 +42,6 @@ public void checkArgument_falseWithErrorMessage_throwsIllegalArgumentException() String errorMessage = "error message"; thrown.expect(IllegalArgumentException.class); thrown.expectMessage(errorMessage); - AppUtil.checkArgument(false, errorMessage); + AppUtil.checkArgument(false, errorMessage); } } diff --git a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java index 2014aa3a8eda..fdd4ff86d09f 100644 --- a/src/test/java/seedu/address/commons/util/ConfigUtilTest.java +++ b/src/test/java/seedu/address/commons/util/ConfigUtilTest.java @@ -74,7 +74,6 @@ public void read_extraValuesInFile_extraValuesIgnored() throws DataConversionExc private Config getTypicalConfig() { Config config = new Config(); - config.setAppTitle("Typical App Title"); config.setLogLevel(Level.INFO); config.setUserPrefsFilePath(Paths.get("preferences.json")); return config; @@ -109,7 +108,6 @@ public void saveConfig_allInOrder_success() throws DataConversionException, IOEx assertEquals(original, readBack); //Try saving when the file exists - original.setAppTitle("Updated Title"); original.setLogLevel(Level.FINE); ConfigUtil.saveConfig(original, configFilePath); readBack = ConfigUtil.readConfig(configFilePath).get(); diff --git a/src/test/java/seedu/address/commons/util/InvalidationListenerManagerTest.java b/src/test/java/seedu/address/commons/util/InvalidationListenerManagerTest.java new file mode 100644 index 000000000000..e221cce8b649 --- /dev/null +++ b/src/test/java/seedu/address/commons/util/InvalidationListenerManagerTest.java @@ -0,0 +1,72 @@ +package seedu.address.commons.util; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +import javafx.beans.InvalidationListener; +import javafx.beans.property.SimpleObjectProperty; + +public class InvalidationListenerManagerTest { + private final SimpleObjectProperty dummyObservable = new SimpleObjectProperty<>(); + private final InvalidationListenerManager invalidationListenerManager = new InvalidationListenerManager(); + private int counter; + + @Test + public void addListener_sameListenerOnce_listenerAdded() { + invalidationListenerManager.addListener(observable -> { + assertEquals(dummyObservable, observable); + counter++; + }); + invalidationListenerManager.callListeners(dummyObservable); + assertEquals(1, counter); + } + + @Test + public void addListener_sameListenerTwice_listenerAddedTwice() { + InvalidationListener listener = observable -> counter++; + invalidationListenerManager.addListener(listener); + invalidationListenerManager.addListener(listener); + invalidationListenerManager.callListeners(dummyObservable); + assertEquals(2, counter); + } + + @Test + public void addListener_listenersBeingCalled_listenerNotCalled() { + InvalidationListener listener1 = observable -> { + throw new AssertionError("should not be called"); + }; + InvalidationListener listener2 = observable -> invalidationListenerManager.addListener(listener1); + invalidationListenerManager.addListener(listener2); + invalidationListenerManager.callListeners(dummyObservable); + } + + @Test + public void removeListener_singleListenerAdded_listenerRemoved() { + InvalidationListener listener = observable -> counter++; + invalidationListenerManager.addListener(listener); + invalidationListenerManager.removeListener(listener); + invalidationListenerManager.callListeners(dummyObservable); + assertEquals(0, counter); + } + + @Test + public void removeListener_sameListenerAddedTwice_firstListenerRemoved() { + InvalidationListener listener = observable -> counter++; + invalidationListenerManager.addListener(listener); + invalidationListenerManager.addListener(listener); + invalidationListenerManager.removeListener(listener); + invalidationListenerManager.callListeners(dummyObservable); + assertEquals(1, counter); + } + + @Test + public void removeListener_listenersBeingCalled_listenerStillCalled() { + InvalidationListener listener1 = observable -> counter++; + InvalidationListener listener2 = observable -> invalidationListenerManager.removeListener(listener1); + invalidationListenerManager.addListener(listener2); + invalidationListenerManager.addListener(listener1); + invalidationListenerManager.callListeners(dummyObservable); + assertEquals(1, counter); + } +} diff --git a/src/test/java/seedu/address/commons/util/StringUtilTest.java b/src/test/java/seedu/address/commons/util/StringUtilTest.java index 168bdb93c0d3..c0bbca33e188 100644 --- a/src/test/java/seedu/address/commons/util/StringUtilTest.java +++ b/src/test/java/seedu/address/commons/util/StringUtilTest.java @@ -42,7 +42,7 @@ public void isUnsignedPositiveInteger() { // EP: numbers with white space assertFalse(StringUtil.isNonZeroUnsignedInteger(" 10 ")); // Leading/trailing spaces - assertFalse(StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle + assertFalse(StringUtil.isNonZeroUnsignedInteger("1 0")); // Spaces in the middle // EP: number larger than Integer.MAX_VALUE assertFalse(StringUtil.isNonZeroUnsignedInteger(Long.toString(Integer.MAX_VALUE + 1))); diff --git a/src/test/java/seedu/address/commons/util/XmlUtilTest.java b/src/test/java/seedu/address/commons/util/XmlUtilTest.java deleted file mode 100644 index f409f974a233..000000000000 --- a/src/test/java/seedu/address/commons/util/XmlUtilTest.java +++ /dev/null @@ -1,146 +0,0 @@ -package seedu.address.commons.util; - -import static org.junit.Assert.assertEquals; - -import java.io.FileNotFoundException; -import java.nio.file.Path; -import java.nio.file.Paths; -import java.util.Collections; -import java.util.List; - -import javax.xml.bind.JAXBException; -import javax.xml.bind.annotation.XmlRootElement; - -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; - -import seedu.address.model.AddressBook; -import seedu.address.storage.XmlAdaptedPerson; -import seedu.address.storage.XmlAdaptedTag; -import seedu.address.storage.XmlSerializableAddressBook; -import seedu.address.testutil.AddressBookBuilder; -import seedu.address.testutil.PersonBuilder; -import seedu.address.testutil.TestUtil; - -public class XmlUtilTest { - - 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"; - - private static final String VALID_NAME = "Hans Muster"; - private static final String VALID_PHONE = "9482424"; - private static final String VALID_EMAIL = "hans@example"; - private static final String VALID_ADDRESS = "4th street"; - private static final List VALID_TAGS = Collections.singletonList(new XmlAdaptedTag("friends")); - - @Rule - public ExpectedException thrown = ExpectedException.none(); - - @Test - public void getDataFromFile_nullFile_throwsNullPointerException() throws Exception { - thrown.expect(NullPointerException.class); - XmlUtil.getDataFromFile(null, AddressBook.class); - } - - @Test - public void getDataFromFile_nullClass_throwsNullPointerException() throws Exception { - thrown.expect(NullPointerException.class); - XmlUtil.getDataFromFile(VALID_FILE, null); - } - - @Test - public void getDataFromFile_missingFile_fileNotFoundException() throws Exception { - thrown.expect(FileNotFoundException.class); - XmlUtil.getDataFromFile(MISSING_FILE, AddressBook.class); - } - - @Test - public void getDataFromFile_emptyFile_dataFormatMismatchException() throws Exception { - thrown.expect(JAXBException.class); - XmlUtil.getDataFromFile(EMPTY_FILE, AddressBook.class); - } - - @Test - public void getDataFromFile_validFile_validResult() throws Exception { - AddressBook dataFromFile = XmlUtil.getDataFromFile(VALID_FILE, XmlSerializableAddressBook.class).toModelType(); - assertEquals(9, dataFromFile.getPersonList().size()); - } - - @Test - public void xmlAdaptedPersonFromFile_fileWithMissingPersonField_validResult() throws Exception { - XmlAdaptedPerson actualPerson = XmlUtil.getDataFromFile( - MISSING_PERSON_FIELD_FILE, XmlAdaptedPersonWithRootElement.class); - XmlAdaptedPerson expectedPerson = new XmlAdaptedPerson( - null, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - assertEquals(expectedPerson, actualPerson); - } - - @Test - public void xmlAdaptedPersonFromFile_fileWithInvalidPersonField_validResult() throws Exception { - XmlAdaptedPerson actualPerson = XmlUtil.getDataFromFile( - INVALID_PERSON_FIELD_FILE, XmlAdaptedPersonWithRootElement.class); - XmlAdaptedPerson expectedPerson = new XmlAdaptedPerson( - VALID_NAME, INVALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - assertEquals(expectedPerson, actualPerson); - } - - @Test - public void xmlAdaptedPersonFromFile_fileWithValidPerson_validResult() throws Exception { - XmlAdaptedPerson actualPerson = XmlUtil.getDataFromFile( - VALID_PERSON_FILE, XmlAdaptedPersonWithRootElement.class); - XmlAdaptedPerson expectedPerson = new XmlAdaptedPerson( - VALID_NAME, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - assertEquals(expectedPerson, actualPerson); - } - - @Test - public void saveDataToFile_nullFile_throwsNullPointerException() throws Exception { - thrown.expect(NullPointerException.class); - XmlUtil.saveDataToFile(null, new AddressBook()); - } - - @Test - public void saveDataToFile_nullClass_throwsNullPointerException() throws Exception { - thrown.expect(NullPointerException.class); - XmlUtil.saveDataToFile(VALID_FILE, null); - } - - @Test - public void saveDataToFile_missingFile_fileNotFoundException() throws Exception { - thrown.expect(FileNotFoundException.class); - XmlUtil.saveDataToFile(MISSING_FILE, new AddressBook()); - } - - @Test - public void saveDataToFile_validFile_dataSaved() throws Exception { - FileUtil.createFile(TEMP_FILE); - XmlSerializableAddressBook dataToWrite = new XmlSerializableAddressBook(new AddressBook()); - XmlUtil.saveDataToFile(TEMP_FILE, dataToWrite); - XmlSerializableAddressBook dataFromFile = XmlUtil.getDataFromFile(TEMP_FILE, XmlSerializableAddressBook.class); - assertEquals(dataToWrite, dataFromFile); - - AddressBookBuilder builder = new AddressBookBuilder(new AddressBook()); - dataToWrite = new XmlSerializableAddressBook( - builder.withPerson(new PersonBuilder().build()).build()); - - XmlUtil.saveDataToFile(TEMP_FILE, dataToWrite); - dataFromFile = XmlUtil.getDataFromFile(TEMP_FILE, XmlSerializableAddressBook.class); - assertEquals(dataToWrite, dataFromFile); - } - - /** - * Test class annotated with {@code XmlRootElement} to allow unmarshalling of .xml data to {@code XmlAdaptedPerson} - * objects. - */ - @XmlRootElement(name = "person") - private static class XmlAdaptedPersonWithRootElement extends XmlAdaptedPerson {} -} diff --git a/src/test/java/seedu/address/logic/LogicManagerTest.java b/src/test/java/seedu/address/logic/LogicManagerTest.java index 954c1fb04388..56eb8d633185 100644 --- a/src/test/java/seedu/address/logic/LogicManagerTest.java +++ b/src/test/java/seedu/address/logic/LogicManagerTest.java @@ -3,11 +3,22 @@ import static org.junit.Assert.assertEquals; import static seedu.address.commons.core.Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX; import static seedu.address.commons.core.Messages.MESSAGE_UNKNOWN_COMMAND; +import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY; +import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY; +import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_AMY; +import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_AMY; +import static seedu.address.testutil.TypicalPersons.AMY; +import java.io.IOException; +import java.nio.file.Path; + +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; +import seedu.address.logic.commands.AddCommand; import seedu.address.logic.commands.CommandResult; import seedu.address.logic.commands.HistoryCommand; import seedu.address.logic.commands.ListCommand; @@ -15,15 +26,34 @@ import seedu.address.logic.parser.exceptions.ParseException; import seedu.address.model.Model; import seedu.address.model.ModelManager; +import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.UserPrefs; +import seedu.address.model.person.Person; +import seedu.address.storage.JsonAddressBookStorage; +import seedu.address.storage.JsonUserPrefsStorage; +import seedu.address.storage.StorageManager; +import seedu.address.testutil.PersonBuilder; public class LogicManagerTest { + private static final IOException DUMMY_IO_EXCEPTION = new IOException("dummy exception"); + @Rule public ExpectedException thrown = ExpectedException.none(); + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + private Model model = new ModelManager(); - private Logic logic = new LogicManager(model); + private Logic logic; + + @Before + public void setUp() throws Exception { + JsonAddressBookStorage addressBookStorage = new JsonAddressBookStorage(temporaryFolder.newFile().toPath()); + JsonUserPrefsStorage userPrefsStorage = new JsonUserPrefsStorage(temporaryFolder.newFile().toPath()); + StorageManager storage = new StorageManager(addressBookStorage, userPrefsStorage); + logic = new LogicManager(model, storage); + } @Test public void execute_invalidCommandFormat_throwsParseException() { @@ -46,6 +76,27 @@ public void execute_validCommand_success() { assertHistoryCorrect(listCommand); } + @Test + public void execute_storageThrowsIoException_throwsCommandException() throws Exception { + // Setup LogicManager with JsonAddressBookIoExceptionThrowingStub + JsonAddressBookStorage addressBookStorage = + new JsonAddressBookIoExceptionThrowingStub(temporaryFolder.newFile().toPath()); + JsonUserPrefsStorage userPrefsStorage = new JsonUserPrefsStorage(temporaryFolder.newFile().toPath()); + StorageManager storage = new StorageManager(addressBookStorage, userPrefsStorage); + logic = new LogicManager(model, storage); + + // 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); + expectedModel.commitAddressBook(); + String expectedMessage = LogicManager.FILE_OPS_ERROR_MESSAGE + DUMMY_IO_EXCEPTION; + assertCommandBehavior(CommandException.class, addCommand, expectedMessage, expectedModel); + assertHistoryCorrect(addCommand); + } + @Test public void getFilteredPersonList_modifyList_throwsUnsupportedOperationException() { thrown.expect(UnsupportedOperationException.class); @@ -98,7 +149,7 @@ private void assertCommandBehavior(Class expectedException, String inputComma try { CommandResult result = logic.execute(inputCommand); assertEquals(expectedException, null); - assertEquals(expectedMessage, result.feedbackToUser); + assertEquals(expectedMessage, result.getFeedbackToUser()); } catch (CommandException | ParseException e) { assertEquals(expectedException, e.getClass()); assertEquals(expectedMessage, e.getMessage()); @@ -116,9 +167,23 @@ private void assertHistoryCorrect(String... expectedCommands) { CommandResult result = logic.execute(HistoryCommand.COMMAND_WORD); String expectedMessage = String.format( HistoryCommand.MESSAGE_SUCCESS, String.join("\n", expectedCommands)); - assertEquals(expectedMessage, result.feedbackToUser); + assertEquals(expectedMessage, result.getFeedbackToUser()); } catch (ParseException | CommandException e) { throw new AssertionError("Parsing and execution of HistoryCommand.COMMAND_WORD should succeed.", e); } } + + /** + * A stub class to throw an {@code IOException} when the save method is called. + */ + 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; + } + } } diff --git a/src/test/java/seedu/address/logic/commands/AddCommandTest.java b/src/test/java/seedu/address/logic/commands/AddCommandTest.java index c7077829f434..f120a897779b 100644 --- a/src/test/java/seedu/address/logic/commands/AddCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/AddCommandTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Arrays; import java.util.function.Predicate; @@ -13,12 +14,15 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import javafx.beans.property.ReadOnlyProperty; import javafx.collections.ObservableList; +import seedu.address.commons.core.GuiSettings; import seedu.address.logic.CommandHistory; import seedu.address.logic.commands.exceptions.CommandException; import seedu.address.model.AddressBook; import seedu.address.model.Model; import seedu.address.model.ReadOnlyAddressBook; +import seedu.address.model.ReadOnlyUserPrefs; import seedu.address.model.person.Person; import seedu.address.testutil.PersonBuilder; @@ -44,7 +48,7 @@ public void execute_personAcceptedByModel_addSuccessful() throws Exception { CommandResult commandResult = new AddCommand(validPerson).execute(modelStub, commandHistory); - assertEquals(String.format(AddCommand.MESSAGE_SUCCESS, validPerson), commandResult.feedbackToUser); + assertEquals(String.format(AddCommand.MESSAGE_SUCCESS, validPerson), commandResult.getFeedbackToUser()); assertEquals(Arrays.asList(validPerson), modelStub.personsAdded); assertEquals(EMPTY_COMMAND_HISTORY, commandHistory); } @@ -88,13 +92,43 @@ public void equals() { * A default model stub that have all of the methods failing. */ private class ModelStub implements Model { + @Override + public void setUserPrefs(ReadOnlyUserPrefs userPrefs) { + throw new AssertionError("This method should not be called."); + } + + @Override + public ReadOnlyUserPrefs getUserPrefs() { + throw new AssertionError("This method should not be called."); + } + + @Override + public GuiSettings getGuiSettings() { + throw new AssertionError("This method should not be called."); + } + + @Override + public void setGuiSettings(GuiSettings guiSettings) { + throw new AssertionError("This method should not be called."); + } + + @Override + public Path getAddressBookFilePath() { + throw new AssertionError("This method should not be called."); + } + + @Override + public void setAddressBookFilePath(Path addressBookFilePath) { + throw new AssertionError("This method should not be called."); + } + @Override public void addPerson(Person person) { throw new AssertionError("This method should not be called."); } @Override - public void resetData(ReadOnlyAddressBook newData) { + public void setAddressBook(ReadOnlyAddressBook newData) { throw new AssertionError("This method should not be called."); } @@ -114,7 +148,7 @@ public void deletePerson(Person target) { } @Override - public void updatePerson(Person target, Person editedPerson) { + public void setPerson(Person target, Person editedPerson) { throw new AssertionError("This method should not be called."); } @@ -152,6 +186,21 @@ public void redoAddressBook() { public void commitAddressBook() { throw new AssertionError("This method should not be called."); } + + @Override + public ReadOnlyProperty selectedPersonProperty() { + throw new AssertionError("This method should not be called."); + } + + @Override + public Person getSelectedPerson() { + throw new AssertionError("This method should not be called."); + } + + @Override + public void setSelectedPerson(Person person) { + throw new AssertionError("This method should not be called."); + } } /** diff --git a/src/test/java/seedu/address/logic/commands/ClearCommandTest.java b/src/test/java/seedu/address/logic/commands/ClearCommandTest.java index db2c7263d25d..9255d74e5c6b 100644 --- a/src/test/java/seedu/address/logic/commands/ClearCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/ClearCommandTest.java @@ -28,7 +28,7 @@ public void execute_emptyAddressBook_success() { public void execute_nonEmptyAddressBook_success() { Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs()); Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs()); - expectedModel.resetData(new AddressBook()); + expectedModel.setAddressBook(new AddressBook()); expectedModel.commitAddressBook(); assertCommandSuccess(new ClearCommand(), model, commandHistory, ClearCommand.MESSAGE_SUCCESS, expectedModel); diff --git a/src/test/java/seedu/address/logic/commands/CommandResultTest.java b/src/test/java/seedu/address/logic/commands/CommandResultTest.java new file mode 100644 index 000000000000..fc8d80228100 --- /dev/null +++ b/src/test/java/seedu/address/logic/commands/CommandResultTest.java @@ -0,0 +1,54 @@ +package seedu.address.logic.commands; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class CommandResultTest { + @Test + public void equals() { + CommandResult commandResult = new CommandResult("feedback"); + + // same values -> returns true + assertTrue(commandResult.equals(new CommandResult("feedback"))); + assertTrue(commandResult.equals(new CommandResult("feedback", false, false))); + + // same object -> returns true + assertTrue(commandResult.equals(commandResult)); + + // null -> returns false + assertFalse(commandResult.equals(null)); + + // different types -> returns false + assertFalse(commandResult.equals(0.5f)); + + // different feedbackToUser value -> returns false + assertFalse(commandResult.equals(new CommandResult("different"))); + + // different showHelp value -> returns false + assertFalse(commandResult.equals(new CommandResult("feedback", true, false))); + + // different exit value -> returns false + assertFalse(commandResult.equals(new CommandResult("feedback", false, true))); + } + + @Test + public void hashcode() { + CommandResult commandResult = new CommandResult("feedback"); + + // same values -> returns same hashcode + assertEquals(commandResult.hashCode(), new CommandResult("feedback").hashCode()); + + // different feedbackToUser value -> returns different hashcode + assertNotEquals(commandResult.hashCode(), new CommandResult("different").hashCode()); + + // different showHelp value -> returns different hashcode + assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", true, false).hashCode()); + + // different exit value -> returns different hashcode + assertNotEquals(commandResult.hashCode(), new CommandResult("feedback", false, true).hashCode()); + } +} diff --git a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java index bf861fcb36c3..eb87ddcfbcf3 100644 --- a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java +++ b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java @@ -71,16 +71,16 @@ public class CommandTestUtil { /** * Executes the given {@code command}, confirms that
- * - the result message matches {@code expectedMessage}
+ * - the returned {@link CommandResult} matches {@code expectedCommandResult}
* - the {@code actualModel} matches {@code expectedModel}
* - the {@code actualCommandHistory} remains unchanged. */ public static void assertCommandSuccess(Command command, Model actualModel, CommandHistory actualCommandHistory, - String expectedMessage, Model expectedModel) { + CommandResult expectedCommandResult, Model expectedModel) { CommandHistory expectedCommandHistory = new CommandHistory(actualCommandHistory); try { CommandResult result = command.execute(actualModel, actualCommandHistory); - assertEquals(expectedMessage, result.feedbackToUser); + assertEquals(expectedCommandResult, result); assertEquals(expectedModel, actualModel); assertEquals(expectedCommandHistory, actualCommandHistory); } catch (CommandException ce) { @@ -88,11 +88,21 @@ public static void assertCommandSuccess(Command command, Model actualModel, Comm } } + /** + * Convenience wrapper to {@link #assertCommandSuccess(Command, Model, CommandHistory, CommandResult, Model)} + * that takes a string {@code expectedMessage}. + */ + public static void assertCommandSuccess(Command command, Model actualModel, CommandHistory actualCommandHistory, + String expectedMessage, Model expectedModel) { + CommandResult expectedCommandResult = new CommandResult(expectedMessage); + assertCommandSuccess(command, actualModel, actualCommandHistory, expectedCommandResult, expectedModel); + } + /** * Executes the given {@code command}, confirms that
* - a {@code CommandException} is thrown
* - the CommandException message matches {@code expectedMessage}
- * - the address book and the filtered person list in the {@code actualModel} remain unchanged
+ * - the address book, filtered person list and selected person in {@code actualModel} remain unchanged
* - {@code actualCommandHistory} remains unchanged. */ public static void assertCommandFailure(Command command, Model actualModel, CommandHistory actualCommandHistory, @@ -101,6 +111,7 @@ public static void assertCommandFailure(Command command, Model actualModel, Comm // only do so by copying its components. AddressBook expectedAddressBook = new AddressBook(actualModel.getAddressBook()); List expectedFilteredList = new ArrayList<>(actualModel.getFilteredPersonList()); + Person expectedSelectedPerson = actualModel.getSelectedPerson(); CommandHistory expectedCommandHistory = new CommandHistory(actualCommandHistory); @@ -111,6 +122,7 @@ public static void assertCommandFailure(Command command, Model actualModel, Comm assertEquals(expectedMessage, e.getMessage()); assertEquals(expectedAddressBook, actualModel.getAddressBook()); assertEquals(expectedFilteredList, actualModel.getFilteredPersonList()); + assertEquals(expectedSelectedPerson, actualModel.getSelectedPerson()); assertEquals(expectedCommandHistory, actualCommandHistory); } } diff --git a/src/test/java/seedu/address/logic/commands/EditCommandTest.java b/src/test/java/seedu/address/logic/commands/EditCommandTest.java index d8aa24426698..274ccceea15f 100644 --- a/src/test/java/seedu/address/logic/commands/EditCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/EditCommandTest.java @@ -46,7 +46,7 @@ public void execute_allFieldsSpecifiedUnfilteredList_success() { String expectedMessage = String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson); Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs()); - expectedModel.updatePerson(model.getFilteredPersonList().get(0), editedPerson); + expectedModel.setPerson(model.getFilteredPersonList().get(0), editedPerson); expectedModel.commitAddressBook(); assertCommandSuccess(editCommand, model, commandHistory, expectedMessage, expectedModel); @@ -68,7 +68,7 @@ public void execute_someFieldsSpecifiedUnfilteredList_success() { String expectedMessage = String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson); Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs()); - expectedModel.updatePerson(lastPerson, editedPerson); + expectedModel.setPerson(lastPerson, editedPerson); expectedModel.commitAddressBook(); assertCommandSuccess(editCommand, model, commandHistory, expectedMessage, expectedModel); @@ -99,7 +99,7 @@ public void execute_filteredList_success() { String expectedMessage = String.format(EditCommand.MESSAGE_EDIT_PERSON_SUCCESS, editedPerson); Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs()); - expectedModel.updatePerson(model.getFilteredPersonList().get(0), editedPerson); + expectedModel.setPerson(model.getFilteredPersonList().get(0), editedPerson); expectedModel.commitAddressBook(); assertCommandSuccess(editCommand, model, commandHistory, expectedMessage, expectedModel); @@ -159,7 +159,7 @@ public void executeUndoRedo_validIndexUnfilteredList_success() throws Exception EditPersonDescriptor descriptor = new EditPersonDescriptorBuilder(editedPerson).build(); EditCommand editCommand = new EditCommand(INDEX_FIRST_PERSON, descriptor); Model expectedModel = new ModelManager(new AddressBook(model.getAddressBook()), new UserPrefs()); - expectedModel.updatePerson(personToEdit, editedPerson); + expectedModel.setPerson(personToEdit, editedPerson); expectedModel.commitAddressBook(); // edit -> first person edited @@ -204,7 +204,7 @@ public void executeUndoRedo_validIndexFilteredList_samePersonEdited() throws Exc showPersonAtIndex(model, INDEX_SECOND_PERSON); Person personToEdit = model.getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()); - expectedModel.updatePerson(personToEdit, editedPerson); + expectedModel.setPerson(personToEdit, editedPerson); expectedModel.commitAddressBook(); // edit -> edits second person in unfiltered person list / first person in filtered person list diff --git a/src/test/java/seedu/address/logic/commands/ExitCommandTest.java b/src/test/java/seedu/address/logic/commands/ExitCommandTest.java index 60c52950ede2..223e5a362b05 100644 --- a/src/test/java/seedu/address/logic/commands/ExitCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/ExitCommandTest.java @@ -1,30 +1,22 @@ package seedu.address.logic.commands; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess; import static seedu.address.logic.commands.ExitCommand.MESSAGE_EXIT_ACKNOWLEDGEMENT; -import org.junit.Rule; import org.junit.Test; -import seedu.address.commons.events.ui.ExitAppRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; import seedu.address.model.ModelManager; -import seedu.address.ui.testutil.EventsCollectorRule; public class ExitCommandTest { - @Rule - public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule(); - private Model model = new ModelManager(); + private Model expectedModel = new ModelManager(); private CommandHistory commandHistory = new CommandHistory(); @Test public void execute_exit_success() { - CommandResult result = new ExitCommand().execute(model, commandHistory); - assertEquals(MESSAGE_EXIT_ACKNOWLEDGEMENT, result.feedbackToUser); - assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof ExitAppRequestEvent); - assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1); + CommandResult expectedCommandResult = new CommandResult(MESSAGE_EXIT_ACKNOWLEDGEMENT, false, true); + assertCommandSuccess(new ExitCommand(), model, commandHistory, expectedCommandResult, expectedModel); } } diff --git a/src/test/java/seedu/address/logic/commands/HelpCommandTest.java b/src/test/java/seedu/address/logic/commands/HelpCommandTest.java index 6a1e34bf476f..9012e9f18e37 100644 --- a/src/test/java/seedu/address/logic/commands/HelpCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/HelpCommandTest.java @@ -1,30 +1,22 @@ package seedu.address.logic.commands; -import static org.junit.Assert.assertTrue; import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess; import static seedu.address.logic.commands.HelpCommand.SHOWING_HELP_MESSAGE; -import org.junit.Rule; import org.junit.Test; -import seedu.address.commons.events.ui.ShowHelpRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; import seedu.address.model.ModelManager; -import seedu.address.ui.testutil.EventsCollectorRule; public class HelpCommandTest { - @Rule - public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule(); - private Model model = new ModelManager(); private Model expectedModel = new ModelManager(); private CommandHistory commandHistory = new CommandHistory(); @Test public void execute_help_success() { - assertCommandSuccess(new HelpCommand(), model, commandHistory, SHOWING_HELP_MESSAGE, expectedModel); - assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof ShowHelpRequestEvent); - assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1); + CommandResult expectedCommandResult = new CommandResult(SHOWING_HELP_MESSAGE, true, false); + assertCommandSuccess(new HelpCommand(), model, commandHistory, expectedCommandResult, expectedModel); } } diff --git a/src/test/java/seedu/address/logic/commands/SelectCommandTest.java b/src/test/java/seedu/address/logic/commands/SelectCommandTest.java index d345189362da..f6be591a4e67 100644 --- a/src/test/java/seedu/address/logic/commands/SelectCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/SelectCommandTest.java @@ -1,6 +1,5 @@ package seedu.address.logic.commands; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static seedu.address.logic.commands.CommandTestUtil.assertCommandFailure; @@ -11,25 +10,19 @@ import static seedu.address.testutil.TypicalIndexes.INDEX_THIRD_PERSON; import static seedu.address.testutil.TypicalPersons.getTypicalAddressBook; -import org.junit.Rule; import org.junit.Test; import seedu.address.commons.core.Messages; import seedu.address.commons.core.index.Index; -import seedu.address.commons.events.ui.JumpToListRequestEvent; import seedu.address.logic.CommandHistory; import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.UserPrefs; -import seedu.address.ui.testutil.EventsCollectorRule; /** * Contains integration tests (interaction with the Model) for {@code SelectCommand}. */ public class SelectCommandTest { - @Rule - public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule(); - private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs()); private Model expectedModel = new ModelManager(getTypicalAddressBook(), new UserPrefs()); private CommandHistory commandHistory = new CommandHistory(); @@ -93,17 +86,15 @@ public void equals() { } /** - * Executes a {@code SelectCommand} with the given {@code index}, and checks that {@code JumpToListRequestEvent} - * is raised with the correct index. + * Executes a {@code SelectCommand} with the given {@code index}, + * and checks that the model's selected person is set to the person at {@code index} in the filtered person list. */ private void assertExecutionSuccess(Index index) { SelectCommand selectCommand = new SelectCommand(index); String expectedMessage = String.format(SelectCommand.MESSAGE_SELECT_PERSON_SUCCESS, index.getOneBased()); + expectedModel.setSelectedPerson(model.getFilteredPersonList().get(index.getZeroBased())); assertCommandSuccess(selectCommand, model, commandHistory, expectedMessage, expectedModel); - - JumpToListRequestEvent lastEvent = (JumpToListRequestEvent) eventsCollectorRule.eventsCollector.getMostRecent(); - assertEquals(index, Index.fromZeroBased(lastEvent.targetIndex)); } /** @@ -113,6 +104,5 @@ private void assertExecutionSuccess(Index index) { private void assertExecutionFailure(Index index, String expectedMessage) { SelectCommand selectCommand = new SelectCommand(index); assertCommandFailure(selectCommand, model, commandHistory, expectedMessage); - assertTrue(eventsCollectorRule.eventsCollector.isEmpty()); } } diff --git a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java index c770e86482e6..f0d56909fd24 100644 --- a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java @@ -111,27 +111,27 @@ public void parse_compulsoryFieldMissing_failure() { public void parse_invalidValue_failure() { // invalid name assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Name.MESSAGE_NAME_CONSTRAINTS); + + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Name.MESSAGE_CONSTRAINTS); // invalid phone assertParseFailure(parser, NAME_DESC_BOB + INVALID_PHONE_DESC + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Phone.MESSAGE_PHONE_CONSTRAINTS); + + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Phone.MESSAGE_CONSTRAINTS); // invalid email assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + INVALID_EMAIL_DESC + ADDRESS_DESC_BOB - + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Email.MESSAGE_EMAIL_CONSTRAINTS); + + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Email.MESSAGE_CONSTRAINTS); // invalid address assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC - + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Address.MESSAGE_ADDRESS_CONSTRAINTS); + + TAG_DESC_HUSBAND + TAG_DESC_FRIEND, Address.MESSAGE_CONSTRAINTS); // invalid tag assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INVALID_TAG_DESC + VALID_TAG_FRIEND, Tag.MESSAGE_TAG_CONSTRAINTS); + + INVALID_TAG_DESC + VALID_TAG_FRIEND, Tag.MESSAGE_CONSTRAINTS); // two invalid values, only first invalid value reported assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC, - Name.MESSAGE_NAME_CONSTRAINTS); + Name.MESSAGE_CONSTRAINTS); // non-empty preamble assertParseFailure(parser, PREAMBLE_NON_EMPTY + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB diff --git a/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java b/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java index 24c138b41a7f..24c54079db88 100644 --- a/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java @@ -81,28 +81,28 @@ public void parse_invalidPreamble_failure() { @Test public void parse_invalidValue_failure() { - assertParseFailure(parser, "1" + INVALID_NAME_DESC, Name.MESSAGE_NAME_CONSTRAINTS); // invalid name - assertParseFailure(parser, "1" + INVALID_PHONE_DESC, Phone.MESSAGE_PHONE_CONSTRAINTS); // invalid phone - assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, Email.MESSAGE_EMAIL_CONSTRAINTS); // invalid email - assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, Address.MESSAGE_ADDRESS_CONSTRAINTS); // invalid address - assertParseFailure(parser, "1" + INVALID_TAG_DESC, Tag.MESSAGE_TAG_CONSTRAINTS); // invalid tag + assertParseFailure(parser, "1" + INVALID_NAME_DESC, Name.MESSAGE_CONSTRAINTS); // invalid name + assertParseFailure(parser, "1" + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS); // invalid phone + assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, Email.MESSAGE_CONSTRAINTS); // invalid email + assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, Address.MESSAGE_CONSTRAINTS); // invalid address + assertParseFailure(parser, "1" + INVALID_TAG_DESC, Tag.MESSAGE_CONSTRAINTS); // invalid tag // invalid phone followed by valid email - assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, Phone.MESSAGE_PHONE_CONSTRAINTS); + assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, Phone.MESSAGE_CONSTRAINTS); // valid phone followed by invalid phone. The test case for invalid phone followed by valid phone // is tested at {@code parse_invalidValueFollowedByValidValue_success()} - assertParseFailure(parser, "1" + PHONE_DESC_BOB + INVALID_PHONE_DESC, Phone.MESSAGE_PHONE_CONSTRAINTS); + assertParseFailure(parser, "1" + PHONE_DESC_BOB + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS); // while parsing {@code PREFIX_TAG} alone will reset the tags of the {@code Person} being edited, // parsing it together with a valid tag results in error - assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_DESC_HUSBAND + TAG_EMPTY, Tag.MESSAGE_TAG_CONSTRAINTS); - assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_EMPTY + TAG_DESC_HUSBAND, Tag.MESSAGE_TAG_CONSTRAINTS); - assertParseFailure(parser, "1" + TAG_EMPTY + TAG_DESC_FRIEND + TAG_DESC_HUSBAND, Tag.MESSAGE_TAG_CONSTRAINTS); + assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_DESC_HUSBAND + TAG_EMPTY, Tag.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + TAG_DESC_FRIEND + TAG_EMPTY + TAG_DESC_HUSBAND, Tag.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + TAG_EMPTY + TAG_DESC_FRIEND + TAG_DESC_HUSBAND, Tag.MESSAGE_CONSTRAINTS); // multiple invalid values, but only the first invalid value is captured assertParseFailure(parser, "1" + INVALID_NAME_DESC + INVALID_EMAIL_DESC + VALID_ADDRESS_AMY + VALID_PHONE_AMY, - Name.MESSAGE_NAME_CONSTRAINTS); + Name.MESSAGE_CONSTRAINTS); } @Test @@ -168,7 +168,7 @@ public void parse_oneFieldSpecified_success() { @Test public void parse_multipleRepeatedFields_acceptsLast() { Index targetIndex = INDEX_FIRST_PERSON; - String userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY + String userInput = targetIndex.getOneBased() + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY + TAG_DESC_FRIEND + PHONE_DESC_AMY + ADDRESS_DESC_AMY + EMAIL_DESC_AMY + TAG_DESC_FRIEND + PHONE_DESC_BOB + ADDRESS_DESC_BOB + EMAIL_DESC_BOB + TAG_DESC_HUSBAND; diff --git a/src/test/java/seedu/address/model/AddressBookTest.java b/src/test/java/seedu/address/model/AddressBookTest.java index 0d33cff49ab1..433147e1534a 100644 --- a/src/test/java/seedu/address/model/AddressBookTest.java +++ b/src/test/java/seedu/address/model/AddressBookTest.java @@ -17,6 +17,8 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import javafx.beans.InvalidationListener; +import javafx.beans.property.SimpleIntegerProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; import seedu.address.model.person.Person; @@ -91,6 +93,25 @@ public void getPersonList_modifyList_throwsUnsupportedOperationException() { addressBook.getPersonList().remove(0); } + @Test + public void addListener_withInvalidationListener_listenerAdded() { + SimpleIntegerProperty counter = new SimpleIntegerProperty(); + InvalidationListener listener = observable -> counter.set(counter.get() + 1); + addressBook.addListener(listener); + addressBook.addPerson(ALICE); + assertEquals(1, counter.get()); + } + + @Test + public void removeListener_withInvalidationListener_listenerRemoved() { + SimpleIntegerProperty counter = new SimpleIntegerProperty(); + InvalidationListener listener = observable -> counter.set(counter.get() + 1); + addressBook.addListener(listener); + addressBook.removeListener(listener); + addressBook.addPerson(ALICE); + assertEquals(0, counter.get()); + } + /** * A stub ReadOnlyAddressBook whose persons list can violate interface constraints. */ @@ -105,6 +126,16 @@ private static class AddressBookStub implements ReadOnlyAddressBook { public ObservableList getPersonList() { return persons; } + + @Override + public void addListener(InvalidationListener listener) { + throw new AssertionError("This method should not be called."); + } + + @Override + public void removeListener(InvalidationListener listener) { + throw new AssertionError("This method should not be called."); + } } } diff --git a/src/test/java/seedu/address/model/ModelManagerTest.java b/src/test/java/seedu/address/model/ModelManagerTest.java index 7eab39d5de43..e865629884d5 100644 --- a/src/test/java/seedu/address/model/ModelManagerTest.java +++ b/src/test/java/seedu/address/model/ModelManagerTest.java @@ -1,20 +1,29 @@ package seedu.address.model; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static seedu.address.logic.commands.CommandTestUtil.VALID_EMAIL_BOB; import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS; import static seedu.address.testutil.TypicalPersons.ALICE; import static seedu.address.testutil.TypicalPersons.BENSON; +import static seedu.address.testutil.TypicalPersons.BOB; +import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; +import java.util.Collections; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import seedu.address.commons.core.GuiSettings; import seedu.address.model.person.NameContainsKeywordsPredicate; +import seedu.address.model.person.Person; +import seedu.address.model.person.exceptions.PersonNotFoundException; import seedu.address.testutil.AddressBookBuilder; +import seedu.address.testutil.PersonBuilder; public class ModelManagerTest { @Rule @@ -22,6 +31,60 @@ public class ModelManagerTest { private ModelManager modelManager = new ModelManager(); + @Test + public void constructor() { + assertEquals(new UserPrefs(), modelManager.getUserPrefs()); + assertEquals(new GuiSettings(), modelManager.getGuiSettings()); + assertEquals(new AddressBook(), new AddressBook(modelManager.getAddressBook())); + assertEquals(null, modelManager.getSelectedPerson()); + } + + @Test + public void setUserPrefs_nullUserPrefs_throwsNullPointerException() { + thrown.expect(NullPointerException.class); + modelManager.setUserPrefs(null); + } + + @Test + public void setUserPrefs_validUserPrefs_copiesUserPrefs() { + UserPrefs userPrefs = new UserPrefs(); + userPrefs.setAddressBookFilePath(Paths.get("address/book/file/path")); + userPrefs.setGuiSettings(new GuiSettings(1, 2, 3, 4)); + modelManager.setUserPrefs(userPrefs); + assertEquals(userPrefs, modelManager.getUserPrefs()); + + // Modifying userPrefs should not modify modelManager's userPrefs + UserPrefs oldUserPrefs = new UserPrefs(userPrefs); + userPrefs.setAddressBookFilePath(Paths.get("new/address/book/file/path")); + assertEquals(oldUserPrefs, modelManager.getUserPrefs()); + } + + @Test + public void setGuiSettings_nullGuiSettings_throwsNullPointerException() { + thrown.expect(NullPointerException.class); + modelManager.setGuiSettings(null); + } + + @Test + public void setGuiSettings_validGuiSettings_setsGuiSettings() { + GuiSettings guiSettings = new GuiSettings(1, 2, 3, 4); + modelManager.setGuiSettings(guiSettings); + assertEquals(guiSettings, modelManager.getGuiSettings()); + } + + @Test + public void setAddressBookFilePath_nullPath_throwsNullPointerException() { + thrown.expect(NullPointerException.class); + modelManager.setAddressBookFilePath(null); + } + + @Test + public void setAddressBookFilePath_validPath_setsAddressBookFilePath() { + Path path = Paths.get("address/book/file/path"); + modelManager.setAddressBookFilePath(path); + assertEquals(path, modelManager.getAddressBookFilePath()); + } + @Test public void hasPerson_nullPerson_throwsNullPointerException() { thrown.expect(NullPointerException.class); @@ -39,12 +102,53 @@ public void hasPerson_personInAddressBook_returnsTrue() { assertTrue(modelManager.hasPerson(ALICE)); } + @Test + public void deletePerson_personIsSelectedAndFirstPersonInFilteredPersonList_selectionCleared() { + modelManager.addPerson(ALICE); + modelManager.setSelectedPerson(ALICE); + modelManager.deletePerson(ALICE); + assertEquals(null, modelManager.getSelectedPerson()); + } + + @Test + public void deletePerson_personIsSelectedAndSecondPersonInFilteredPersonList_firstPersonSelected() { + modelManager.addPerson(ALICE); + modelManager.addPerson(BOB); + assertEquals(Arrays.asList(ALICE, BOB), modelManager.getFilteredPersonList()); + modelManager.setSelectedPerson(BOB); + modelManager.deletePerson(BOB); + assertEquals(ALICE, modelManager.getSelectedPerson()); + } + + @Test + public void setPerson_personIsSelected_selectedPersonUpdated() { + modelManager.addPerson(ALICE); + modelManager.setSelectedPerson(ALICE); + Person updatedAlice = new PersonBuilder(ALICE).withEmail(VALID_EMAIL_BOB).build(); + modelManager.setPerson(ALICE, updatedAlice); + assertEquals(updatedAlice, modelManager.getSelectedPerson()); + } + @Test public void getFilteredPersonList_modifyList_throwsUnsupportedOperationException() { thrown.expect(UnsupportedOperationException.class); modelManager.getFilteredPersonList().remove(0); } + @Test + public void setSelectedPerson_personNotInFilteredPersonList_throwsPersonNotFoundException() { + thrown.expect(PersonNotFoundException.class); + modelManager.setSelectedPerson(ALICE); + } + + @Test + public void setSelectedPerson_personInFilteredPersonList_setsSelectedPerson() { + modelManager.addPerson(ALICE); + assertEquals(Collections.singletonList(ALICE), modelManager.getFilteredPersonList()); + modelManager.setSelectedPerson(ALICE); + assertEquals(ALICE, modelManager.getSelectedPerson()); + } + @Test public void equals() { AddressBook addressBook = new AddressBookBuilder().withPerson(ALICE).withPerson(BENSON).build(); @@ -76,9 +180,9 @@ public void equals() { // resets modelManager to initial state for upcoming tests modelManager.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); - // different userPrefs -> returns true + // different userPrefs -> returns false UserPrefs differentUserPrefs = new UserPrefs(); differentUserPrefs.setAddressBookFilePath(Paths.get("differentFilePath")); - assertTrue(modelManager.equals(new ModelManager(addressBook, differentUserPrefs))); + assertFalse(modelManager.equals(new ModelManager(addressBook, differentUserPrefs))); } } diff --git a/src/test/java/seedu/address/model/UserPrefsTest.java b/src/test/java/seedu/address/model/UserPrefsTest.java new file mode 100644 index 000000000000..20da6c4050d9 --- /dev/null +++ b/src/test/java/seedu/address/model/UserPrefsTest.java @@ -0,0 +1,21 @@ +package seedu.address.model; + +import org.junit.Test; + +import seedu.address.testutil.Assert; + +public class UserPrefsTest { + + @Test + public void setGuiSettings_nullGuiSettings_throwsNullPointerException() { + UserPrefs userPref = new UserPrefs(); + Assert.assertThrows(NullPointerException.class, () -> userPref.setGuiSettings(null)); + } + + @Test + public void setAddressBookFilePath_nullPath_throwsNullPointerException() { + UserPrefs userPrefs = new UserPrefs(); + Assert.assertThrows(NullPointerException.class, () -> userPrefs.setAddressBookFilePath(null)); + } + +} diff --git a/src/test/java/seedu/address/model/person/EmailTest.java b/src/test/java/seedu/address/model/person/EmailTest.java index 10187e8eb884..ecbd0ee50208 100644 --- a/src/test/java/seedu/address/model/person/EmailTest.java +++ b/src/test/java/seedu/address/model/person/EmailTest.java @@ -51,12 +51,12 @@ public void isValidEmail() { // valid email assertTrue(Email.isValidEmail("PeterJack_1190@example.com")); - assertTrue(Email.isValidEmail("a@bc")); // minimal - assertTrue(Email.isValidEmail("test@localhost")); // alphabets only + assertTrue(Email.isValidEmail("a@bc")); // minimal + assertTrue(Email.isValidEmail("test@localhost")); // alphabets only assertTrue(Email.isValidEmail("!#$%&'*+/=?`{|}~^.-@example.org")); // special characters local part - assertTrue(Email.isValidEmail("123@145")); // numeric local part and domain name + assertTrue(Email.isValidEmail("123@145")); // numeric local part and domain name assertTrue(Email.isValidEmail("a1+be!@example1.com")); // mixture of alphanumeric and special characters - assertTrue(Email.isValidEmail("peter_jack@very-very-very-long-example.com")); // long domain name - assertTrue(Email.isValidEmail("if.you.dream.it_you.can.do.it@example.com")); // long local part + assertTrue(Email.isValidEmail("peter_jack@very-very-very-long-example.com")); // long domain name + assertTrue(Email.isValidEmail("if.you.dream.it_you.can.do.it@example.com")); // long local part } } diff --git a/src/test/java/seedu/address/storage/XmlAdaptedPersonTest.java b/src/test/java/seedu/address/storage/JsonAdaptedPersonTest.java similarity index 64% rename from src/test/java/seedu/address/storage/XmlAdaptedPersonTest.java rename to src/test/java/seedu/address/storage/JsonAdaptedPersonTest.java index c3c91a5c27a7..c21a37283d81 100644 --- a/src/test/java/seedu/address/storage/XmlAdaptedPersonTest.java +++ b/src/test/java/seedu/address/storage/JsonAdaptedPersonTest.java @@ -1,7 +1,7 @@ package seedu.address.storage; import static org.junit.Assert.assertEquals; -import static seedu.address.storage.XmlAdaptedPerson.MISSING_FIELD_MESSAGE_FORMAT; +import static seedu.address.storage.JsonAdaptedPerson.MISSING_FIELD_MESSAGE_FORMAT; import static seedu.address.testutil.TypicalPersons.BENSON; import java.util.ArrayList; @@ -17,7 +17,7 @@ import seedu.address.model.person.Phone; import seedu.address.testutil.Assert; -public class XmlAdaptedPersonTest { +public class JsonAdaptedPersonTest { private static final String INVALID_NAME = "R@chel"; private static final String INVALID_PHONE = "+651234"; private static final String INVALID_ADDRESS = " "; @@ -28,82 +28,82 @@ public class XmlAdaptedPersonTest { private static final String VALID_PHONE = BENSON.getPhone().toString(); private static final String VALID_EMAIL = BENSON.getEmail().toString(); private static final String VALID_ADDRESS = BENSON.getAddress().toString(); - private static final List VALID_TAGS = BENSON.getTags().stream() - .map(XmlAdaptedTag::new) + private static final List VALID_TAGS = BENSON.getTags().stream() + .map(JsonAdaptedTag::new) .collect(Collectors.toList()); @Test public void toModelType_validPersonDetails_returnsPerson() throws Exception { - XmlAdaptedPerson person = new XmlAdaptedPerson(BENSON); + JsonAdaptedPerson person = new JsonAdaptedPerson(BENSON); assertEquals(BENSON, person.toModelType()); } @Test public void toModelType_invalidName_throwsIllegalValueException() { - XmlAdaptedPerson person = - new XmlAdaptedPerson(INVALID_NAME, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - String expectedMessage = Name.MESSAGE_NAME_CONSTRAINTS; + JsonAdaptedPerson person = + new JsonAdaptedPerson(INVALID_NAME, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); + String expectedMessage = Name.MESSAGE_CONSTRAINTS; Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_nullName_throwsIllegalValueException() { - XmlAdaptedPerson person = new XmlAdaptedPerson(null, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); + JsonAdaptedPerson person = new JsonAdaptedPerson(null, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); String expectedMessage = String.format(MISSING_FIELD_MESSAGE_FORMAT, Name.class.getSimpleName()); Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_invalidPhone_throwsIllegalValueException() { - XmlAdaptedPerson person = - new XmlAdaptedPerson(VALID_NAME, INVALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - String expectedMessage = Phone.MESSAGE_PHONE_CONSTRAINTS; + JsonAdaptedPerson person = + new JsonAdaptedPerson(VALID_NAME, INVALID_PHONE, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); + String expectedMessage = Phone.MESSAGE_CONSTRAINTS; Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_nullPhone_throwsIllegalValueException() { - XmlAdaptedPerson person = new XmlAdaptedPerson(VALID_NAME, null, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); + JsonAdaptedPerson person = new JsonAdaptedPerson(VALID_NAME, null, VALID_EMAIL, VALID_ADDRESS, VALID_TAGS); String expectedMessage = String.format(MISSING_FIELD_MESSAGE_FORMAT, Phone.class.getSimpleName()); Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_invalidEmail_throwsIllegalValueException() { - XmlAdaptedPerson person = - new XmlAdaptedPerson(VALID_NAME, VALID_PHONE, INVALID_EMAIL, VALID_ADDRESS, VALID_TAGS); - String expectedMessage = Email.MESSAGE_EMAIL_CONSTRAINTS; + JsonAdaptedPerson person = + new JsonAdaptedPerson(VALID_NAME, VALID_PHONE, INVALID_EMAIL, VALID_ADDRESS, VALID_TAGS); + String expectedMessage = Email.MESSAGE_CONSTRAINTS; Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_nullEmail_throwsIllegalValueException() { - XmlAdaptedPerson person = new XmlAdaptedPerson(VALID_NAME, VALID_PHONE, null, VALID_ADDRESS, VALID_TAGS); + JsonAdaptedPerson person = new JsonAdaptedPerson(VALID_NAME, VALID_PHONE, null, VALID_ADDRESS, VALID_TAGS); String expectedMessage = String.format(MISSING_FIELD_MESSAGE_FORMAT, Email.class.getSimpleName()); Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_invalidAddress_throwsIllegalValueException() { - XmlAdaptedPerson person = - new XmlAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, INVALID_ADDRESS, VALID_TAGS); - String expectedMessage = Address.MESSAGE_ADDRESS_CONSTRAINTS; + JsonAdaptedPerson person = + new JsonAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, INVALID_ADDRESS, VALID_TAGS); + String expectedMessage = Address.MESSAGE_CONSTRAINTS; Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_nullAddress_throwsIllegalValueException() { - XmlAdaptedPerson person = new XmlAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, null, VALID_TAGS); + JsonAdaptedPerson person = new JsonAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, null, VALID_TAGS); String expectedMessage = String.format(MISSING_FIELD_MESSAGE_FORMAT, Address.class.getSimpleName()); Assert.assertThrows(IllegalValueException.class, expectedMessage, person::toModelType); } @Test public void toModelType_invalidTags_throwsIllegalValueException() { - List invalidTags = new ArrayList<>(VALID_TAGS); - invalidTags.add(new XmlAdaptedTag(INVALID_TAG)); - XmlAdaptedPerson person = - new XmlAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, invalidTags); + List invalidTags = new ArrayList<>(VALID_TAGS); + invalidTags.add(new JsonAdaptedTag(INVALID_TAG)); + JsonAdaptedPerson person = + new JsonAdaptedPerson(VALID_NAME, VALID_PHONE, VALID_EMAIL, VALID_ADDRESS, invalidTags); Assert.assertThrows(IllegalValueException.class, person::toModelType); } diff --git a/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java b/src/test/java/seedu/address/storage/JsonAddressBookStorageTest.java similarity index 68% rename from src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java rename to src/test/java/seedu/address/storage/JsonAddressBookStorageTest.java index 484bd43cc0fd..6c18ecd0edd9 100644 --- a/src/test/java/seedu/address/storage/XmlAddressBookStorageTest.java +++ b/src/test/java/seedu/address/storage/JsonAddressBookStorageTest.java @@ -20,8 +20,8 @@ import seedu.address.model.AddressBook; import seedu.address.model.ReadOnlyAddressBook; -public class XmlAddressBookStorageTest { - private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "XmlAddressBookStorageTest"); +public class JsonAddressBookStorageTest { + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "JsonAddressBookStorageTest"); @Rule public ExpectedException thrown = ExpectedException.none(); @@ -36,7 +36,7 @@ public void readAddressBook_nullFilePath_throwsNullPointerException() throws Exc } private java.util.Optional readAddressBook(String filePath) throws Exception { - return new XmlAddressBookStorage(Paths.get(filePath)).readAddressBook(addToTestDataPathIfNotNull(filePath)); + return new JsonAddressBookStorage(Paths.get(filePath)).readAddressBook(addToTestDataPathIfNotNull(filePath)); } private Path addToTestDataPathIfNotNull(String prefsFileInTestDataFolder) { @@ -47,54 +47,53 @@ private Path addToTestDataPathIfNotNull(String prefsFileInTestDataFolder) { @Test public void read_missingFile_emptyResult() throws Exception { - assertFalse(readAddressBook("NonExistentFile.xml").isPresent()); + assertFalse(readAddressBook("NonExistentFile.json").isPresent()); } @Test - public void read_notXmlFormat_exceptionThrown() throws Exception { + public void read_notJsonFormat_exceptionThrown() throws Exception { thrown.expect(DataConversionException.class); - readAddressBook("NotXmlFormatAddressBook.xml"); + readAddressBook("notJsonFormatAddressBook.json"); - /* IMPORTANT: Any code below an exception-throwing line (like the one above) will be ignored. - * That means you should not have more than one exception test in one method - */ + // IMPORTANT: Any code below an exception-throwing line (like the one above) will be ignored. + // That means you should not have more than one exception test in one method } @Test public void readAddressBook_invalidPersonAddressBook_throwDataConversionException() throws Exception { thrown.expect(DataConversionException.class); - readAddressBook("invalidPersonAddressBook.xml"); + readAddressBook("invalidPersonAddressBook.json"); } @Test public void readAddressBook_invalidAndValidPersonAddressBook_throwDataConversionException() throws Exception { thrown.expect(DataConversionException.class); - readAddressBook("invalidAndValidPersonAddressBook.xml"); + readAddressBook("invalidAndValidPersonAddressBook.json"); } @Test public void readAndSaveAddressBook_allInOrder_success() throws Exception { - Path filePath = testFolder.getRoot().toPath().resolve("TempAddressBook.xml"); + Path filePath = testFolder.getRoot().toPath().resolve("TempAddressBook.json"); AddressBook original = getTypicalAddressBook(); - XmlAddressBookStorage xmlAddressBookStorage = new XmlAddressBookStorage(filePath); + JsonAddressBookStorage jsonAddressBookStorage = new JsonAddressBookStorage(filePath); - //Save in new file and read back - xmlAddressBookStorage.saveAddressBook(original, filePath); - ReadOnlyAddressBook readBack = xmlAddressBookStorage.readAddressBook(filePath).get(); + // Save in new file and read back + jsonAddressBookStorage.saveAddressBook(original, filePath); + ReadOnlyAddressBook readBack = jsonAddressBookStorage.readAddressBook(filePath).get(); assertEquals(original, new AddressBook(readBack)); - //Modify data, overwrite exiting file, and read back + // Modify data, overwrite exiting file, and read back original.addPerson(HOON); original.removePerson(ALICE); - xmlAddressBookStorage.saveAddressBook(original, filePath); - readBack = xmlAddressBookStorage.readAddressBook(filePath).get(); + jsonAddressBookStorage.saveAddressBook(original, filePath); + readBack = jsonAddressBookStorage.readAddressBook(filePath).get(); assertEquals(original, new AddressBook(readBack)); - //Save and read without specifying file path + // Save and read without specifying file path original.addPerson(IDA); - xmlAddressBookStorage.saveAddressBook(original); //file path not specified - readBack = xmlAddressBookStorage.readAddressBook().get(); //file path not specified + jsonAddressBookStorage.saveAddressBook(original); // file path not specified + readBack = jsonAddressBookStorage.readAddressBook().get(); // file path not specified assertEquals(original, new AddressBook(readBack)); } @@ -102,7 +101,7 @@ public void readAndSaveAddressBook_allInOrder_success() throws Exception { @Test public void saveAddressBook_nullAddressBook_throwsNullPointerException() { thrown.expect(NullPointerException.class); - saveAddressBook(null, "SomeFile.xml"); + saveAddressBook(null, "SomeFile.json"); } /** @@ -110,7 +109,7 @@ public void saveAddressBook_nullAddressBook_throwsNullPointerException() { */ private void saveAddressBook(ReadOnlyAddressBook addressBook, String filePath) { try { - new XmlAddressBookStorage(Paths.get(filePath)) + new JsonAddressBookStorage(Paths.get(filePath)) .saveAddressBook(addressBook, addToTestDataPathIfNotNull(filePath)); } catch (IOException ioe) { throw new AssertionError("There should not be an error writing to the file.", ioe); @@ -122,6 +121,4 @@ public void saveAddressBook_nullFilePath_throwsNullPointerException() { thrown.expect(NullPointerException.class); saveAddressBook(new AddressBook(), null); } - - } diff --git a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java b/src/test/java/seedu/address/storage/JsonSerializableAddressBookTest.java similarity index 63% rename from src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java rename to src/test/java/seedu/address/storage/JsonSerializableAddressBookTest.java index 55a4f10957b3..9bd56c0e6c5a 100644 --- a/src/test/java/seedu/address/storage/XmlSerializableAddressBookTest.java +++ b/src/test/java/seedu/address/storage/JsonSerializableAddressBookTest.java @@ -10,24 +10,24 @@ import org.junit.rules.ExpectedException; import seedu.address.commons.exceptions.IllegalValueException; -import seedu.address.commons.util.XmlUtil; +import seedu.address.commons.util.JsonUtil; import seedu.address.model.AddressBook; import seedu.address.testutil.TypicalPersons; -public class XmlSerializableAddressBookTest { +public class JsonSerializableAddressBookTest { - 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"); - private static final Path DUPLICATE_PERSON_FILE = TEST_DATA_FOLDER.resolve("duplicatePersonAddressBook.xml"); + private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "JsonSerializableAddressBookTest"); + private static final Path TYPICAL_PERSONS_FILE = TEST_DATA_FOLDER.resolve("typicalPersonsAddressBook.json"); + private static final Path INVALID_PERSON_FILE = TEST_DATA_FOLDER.resolve("invalidPersonAddressBook.json"); + private static final Path DUPLICATE_PERSON_FILE = TEST_DATA_FOLDER.resolve("duplicatePersonAddressBook.json"); @Rule public ExpectedException thrown = ExpectedException.none(); @Test public void toModelType_typicalPersonsFile_success() throws Exception { - XmlSerializableAddressBook dataFromFile = XmlUtil.getDataFromFile(TYPICAL_PERSONS_FILE, - XmlSerializableAddressBook.class); + JsonSerializableAddressBook dataFromFile = JsonUtil.readJsonFile(TYPICAL_PERSONS_FILE, + JsonSerializableAddressBook.class).get(); AddressBook addressBookFromFile = dataFromFile.toModelType(); AddressBook typicalPersonsAddressBook = TypicalPersons.getTypicalAddressBook(); assertEquals(addressBookFromFile, typicalPersonsAddressBook); @@ -35,18 +35,18 @@ public void toModelType_typicalPersonsFile_success() throws Exception { @Test public void toModelType_invalidPersonFile_throwsIllegalValueException() throws Exception { - XmlSerializableAddressBook dataFromFile = XmlUtil.getDataFromFile(INVALID_PERSON_FILE, - XmlSerializableAddressBook.class); + JsonSerializableAddressBook dataFromFile = JsonUtil.readJsonFile(INVALID_PERSON_FILE, + JsonSerializableAddressBook.class).get(); thrown.expect(IllegalValueException.class); dataFromFile.toModelType(); } @Test public void toModelType_duplicatePersons_throwsIllegalValueException() throws Exception { - XmlSerializableAddressBook dataFromFile = XmlUtil.getDataFromFile(DUPLICATE_PERSON_FILE, - XmlSerializableAddressBook.class); + JsonSerializableAddressBook dataFromFile = JsonUtil.readJsonFile(DUPLICATE_PERSON_FILE, + JsonSerializableAddressBook.class).get(); thrown.expect(IllegalValueException.class); - thrown.expectMessage(XmlSerializableAddressBook.MESSAGE_DUPLICATE_PERSON); + thrown.expectMessage(JsonSerializableAddressBook.MESSAGE_DUPLICATE_PERSON); dataFromFile.toModelType(); } diff --git a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java index 50334e4ad45d..5d2699cc889a 100644 --- a/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java +++ b/src/test/java/seedu/address/storage/JsonUserPrefsStorageTest.java @@ -13,6 +13,7 @@ import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; +import seedu.address.commons.core.GuiSettings; import seedu.address.commons.exceptions.DataConversionException; import seedu.address.model.UserPrefs; @@ -81,8 +82,8 @@ public void readUserPrefs_extraValuesInFile_extraValuesIgnored() throws DataConv private UserPrefs getTypicalUserPrefs() { UserPrefs userPrefs = new UserPrefs(); - userPrefs.setGuiSettings(1000, 500, 300, 100); - userPrefs.setAddressBookFilePath(Paths.get("addressbook.xml")); + userPrefs.setGuiSettings(new GuiSettings(1000, 500, 300, 100)); + userPrefs.setAddressBookFilePath(Paths.get("addressbook.json")); return userPrefs; } @@ -114,7 +115,7 @@ private void saveUserPrefs(UserPrefs userPrefs, String prefsFileInTestDataFolder public void saveUserPrefs_allInOrder_success() throws DataConversionException, IOException { UserPrefs original = new UserPrefs(); - original.setGuiSettings(1200, 200, 0, 2); + original.setGuiSettings(new GuiSettings(1200, 200, 0, 2)); Path pefsFilePath = testFolder.getRoot().toPath().resolve("TempPrefs.json"); JsonUserPrefsStorage jsonUserPrefsStorage = new JsonUserPrefsStorage(pefsFilePath); @@ -125,7 +126,7 @@ public void saveUserPrefs_allInOrder_success() throws DataConversionException, I assertEquals(original, readBack); //Try saving when the file exists - original.setGuiSettings(5, 5, 5, 5); + original.setGuiSettings(new GuiSettings(5, 5, 5, 5)); jsonUserPrefsStorage.saveUserPrefs(original); readBack = jsonUserPrefsStorage.readUserPrefs().get(); assertEquals(original, readBack); diff --git a/src/test/java/seedu/address/storage/StorageManagerTest.java b/src/test/java/seedu/address/storage/StorageManagerTest.java index 215ce2bcb0e1..b56a68ded0b3 100644 --- a/src/test/java/seedu/address/storage/StorageManagerTest.java +++ b/src/test/java/seedu/address/storage/StorageManagerTest.java @@ -2,37 +2,30 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; 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; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import seedu.address.commons.events.model.AddressBookChangedEvent; -import seedu.address.commons.events.storage.DataSavingExceptionEvent; +import seedu.address.commons.core.GuiSettings; import seedu.address.model.AddressBook; import seedu.address.model.ReadOnlyAddressBook; import seedu.address.model.UserPrefs; -import seedu.address.ui.testutil.EventsCollectorRule; public class StorageManagerTest { @Rule public TemporaryFolder testFolder = new TemporaryFolder(); - @Rule - public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule(); private StorageManager storageManager; @Before public void setUp() { - XmlAddressBookStorage addressBookStorage = new XmlAddressBookStorage(getTempFilePath("ab")); + JsonAddressBookStorage addressBookStorage = new JsonAddressBookStorage(getTempFilePath("ab")); JsonUserPrefsStorage userPrefsStorage = new JsonUserPrefsStorage(getTempFilePath("prefs")); storageManager = new StorageManager(addressBookStorage, userPrefsStorage); } @@ -50,7 +43,7 @@ public void prefsReadSave() throws Exception { * More extensive testing of UserPref saving/reading is done in {@link JsonUserPrefsStorageTest} class. */ UserPrefs original = new UserPrefs(); - original.setGuiSettings(300, 600, 4, 6); + original.setGuiSettings(new GuiSettings(300, 600, 4, 6)); storageManager.saveUserPrefs(original); UserPrefs retrieved = storageManager.readUserPrefs().get(); assertEquals(original, retrieved); @@ -60,8 +53,8 @@ public void prefsReadSave() throws Exception { public void addressBookReadSave() throws Exception { /* * Note: This is an integration test that verifies the StorageManager is properly wired to the - * {@link XmlAddressBookStorage} class. - * More extensive testing of UserPref saving/reading is done in {@link XmlAddressBookStorageTest} class. + * {@link JsonAddressBookStorage} class. + * More extensive testing of UserPref saving/reading is done in {@link JsonAddressBookStorageTest} class. */ AddressBook original = getTypicalAddressBook(); storageManager.saveAddressBook(original); @@ -74,30 +67,4 @@ public void getAddressBookFilePath() { assertNotNull(storageManager.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(Paths.get("dummy")), - new JsonUserPrefsStorage(Paths.get("dummy"))); - storage.handleAddressBookChangedEvent(new AddressBookChangedEvent(new AddressBook())); - assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof DataSavingExceptionEvent); - } - - - /** - * A Stub class to throw an exception when the save method is called - */ - class XmlAddressBookStorageExceptionThrowingStub extends XmlAddressBookStorage { - - public XmlAddressBookStorageExceptionThrowingStub(Path filePath) { - super(filePath); - } - - @Override - public void saveAddressBook(ReadOnlyAddressBook addressBook, Path filePath) throws IOException { - throw new IOException("dummy exception"); - } - } - - } diff --git a/src/test/java/seedu/address/testutil/EventsUtil.java b/src/test/java/seedu/address/testutil/EventsUtil.java deleted file mode 100644 index 4ef8f28df156..000000000000 --- a/src/test/java/seedu/address/testutil/EventsUtil.java +++ /dev/null @@ -1,26 +0,0 @@ -package seedu.address.testutil; - -import guitests.GuiRobot; -import javafx.application.Platform; -import seedu.address.commons.core.EventsCenter; -import seedu.address.commons.events.BaseEvent; - -/** - * Helper methods related to events. - */ -public class EventsUtil { - /** - * Posts {@code event} to all registered subscribers. This method will return successfully after the {@code event} - * has been posted to all subscribers. - */ - public static void postNow(BaseEvent event) { - new GuiRobot().interact(() -> EventsCenter.getInstance().post(event)); - } - - /** - * Posts {@code event} to all registered subscribers at some unspecified time in the future. - */ - public static void postLater(BaseEvent event) { - Platform.runLater(() -> EventsCenter.getInstance().post(event)); - } -} diff --git a/src/test/java/seedu/address/ui/BrowserPanelTest.java b/src/test/java/seedu/address/ui/BrowserPanelTest.java index 5fe6d4f8767f..b678957f6414 100644 --- a/src/test/java/seedu/address/ui/BrowserPanelTest.java +++ b/src/test/java/seedu/address/ui/BrowserPanelTest.java @@ -2,10 +2,7 @@ import static guitests.guihandles.WebViewUtil.waitUntilBrowserLoaded; import static org.junit.Assert.assertEquals; -import static seedu.address.testutil.EventsUtil.postNow; import static seedu.address.testutil.TypicalPersons.ALICE; -import static seedu.address.ui.BrowserPanel.DEFAULT_PAGE; -import static seedu.address.ui.UiPart.FXML_FILE_FOLDER; import java.net.URL; @@ -13,20 +10,17 @@ import org.junit.Test; import guitests.guihandles.BrowserPanelHandle; -import seedu.address.MainApp; -import seedu.address.commons.events.ui.PersonPanelSelectionChangedEvent; +import javafx.beans.property.SimpleObjectProperty; +import seedu.address.model.person.Person; public class BrowserPanelTest extends GuiUnitTest { - private PersonPanelSelectionChangedEvent selectionChangedEventStub; - + private SimpleObjectProperty selectedPerson = new SimpleObjectProperty<>(); private BrowserPanel browserPanel; private BrowserPanelHandle browserPanelHandle; @Before public void setUp() { - selectionChangedEventStub = new PersonPanelSelectionChangedEvent(ALICE); - - guiRobot.interact(() -> browserPanel = new BrowserPanel()); + guiRobot.interact(() -> browserPanel = new BrowserPanel(selectedPerson)); uiPartRule.setUiPart(browserPanel); browserPanelHandle = new BrowserPanelHandle(browserPanel.getRoot()); @@ -35,11 +29,10 @@ public void setUp() { @Test public void display() throws Exception { // default web page - URL expectedDefaultPageUrl = MainApp.class.getResource(FXML_FILE_FOLDER + DEFAULT_PAGE); - assertEquals(expectedDefaultPageUrl, browserPanelHandle.getLoadedUrl()); + assertEquals(BrowserPanel.DEFAULT_PAGE, browserPanelHandle.getLoadedUrl()); // associated web page of a person - postNow(selectionChangedEventStub); + guiRobot.interact(() -> selectedPerson.set(ALICE)); URL expectedPersonUrl = new URL(BrowserPanel.SEARCH_PAGE_URL + ALICE.getName().fullName.replaceAll(" ", "%20")); waitUntilBrowserLoaded(browserPanelHandle); diff --git a/src/test/java/seedu/address/ui/CommandBoxTest.java b/src/test/java/seedu/address/ui/CommandBoxTest.java index f72304570a7a..b58d1d7eba2f 100644 --- a/src/test/java/seedu/address/ui/CommandBoxTest.java +++ b/src/test/java/seedu/address/ui/CommandBoxTest.java @@ -9,11 +9,9 @@ import guitests.guihandles.CommandBoxHandle; import javafx.scene.input.KeyCode; -import seedu.address.logic.Logic; -import seedu.address.logic.LogicManager; +import seedu.address.logic.commands.CommandResult; import seedu.address.logic.commands.ListCommand; -import seedu.address.model.Model; -import seedu.address.model.ModelManager; +import seedu.address.logic.commands.exceptions.CommandException; public class CommandBoxTest extends GuiUnitTest { @@ -22,15 +20,19 @@ public class CommandBoxTest extends GuiUnitTest { private ArrayList defaultStyleOfCommandBox; private ArrayList errorStyleOfCommandBox; + private final ArrayList history = new ArrayList<>(); private CommandBoxHandle commandBoxHandle; @Before public void setUp() { - Model model = new ModelManager(); - Logic logic = new LogicManager(model); - - CommandBox commandBox = new CommandBox(logic); + CommandBox commandBox = new CommandBox(commandText -> { + history.add(commandText); + if (commandText.equals(COMMAND_THAT_SUCCEEDS)) { + return new CommandResult("Command successful"); + } + throw new CommandException("Command failed"); + }, history); commandBoxHandle = new CommandBoxHandle(getChildNode(commandBox.getRoot(), CommandBoxHandle.COMMAND_INPUT_FIELD_ID)); uiPartRule.setUiPart(commandBox); diff --git a/src/test/java/seedu/address/ui/GuiUnitTest.java b/src/test/java/seedu/address/ui/GuiUnitTest.java index f1ff0656725a..d163c5748337 100644 --- a/src/test/java/seedu/address/ui/GuiUnitTest.java +++ b/src/test/java/seedu/address/ui/GuiUnitTest.java @@ -2,13 +2,11 @@ import java.util.Optional; -import org.junit.After; import org.junit.Rule; import guitests.GuiRobot; import guitests.guihandles.exceptions.NodeNotFoundException; import javafx.scene.Node; -import seedu.address.commons.core.EventsCenter; import seedu.address.ui.testutil.UiPartRule; /** @@ -20,11 +18,6 @@ public abstract class GuiUnitTest { protected final GuiRobot guiRobot = new GuiRobot(); - @After - public void tearDown() { - EventsCenter.clearSubscribers(); - } - /** * Retrieves the {@code query} node owned by the {@code rootNode}. * diff --git a/src/test/java/seedu/address/ui/HelpWindowTest.java b/src/test/java/seedu/address/ui/HelpWindowTest.java index 20d721cd0e45..da285ab5b2ba 100644 --- a/src/test/java/seedu/address/ui/HelpWindowTest.java +++ b/src/test/java/seedu/address/ui/HelpWindowTest.java @@ -60,7 +60,11 @@ public void focus_helpWindowNotFocused_focused() throws Exception { guiRobot.interact(helpWindow::show); // Focus on another stage to remove focus from the helpWindow - FxToolkit.setupStage(Stage::requestFocus); + guiRobot.interact(() -> { + Stage temporaryStage = new Stage(); + temporaryStage.show(); + temporaryStage.requestFocus(); + }); assertFalse(helpWindow.getRoot().isFocused()); guiRobot.interact(helpWindow::focus); diff --git a/src/test/java/seedu/address/logic/ListElementPointerTest.java b/src/test/java/seedu/address/ui/ListElementPointerTest.java similarity index 99% rename from src/test/java/seedu/address/logic/ListElementPointerTest.java rename to src/test/java/seedu/address/ui/ListElementPointerTest.java index 21ac5872f3c2..0557e1d94d6a 100644 --- a/src/test/java/seedu/address/logic/ListElementPointerTest.java +++ b/src/test/java/seedu/address/ui/ListElementPointerTest.java @@ -1,4 +1,4 @@ -package seedu.address.logic; +package seedu.address.ui; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; diff --git a/src/test/java/seedu/address/ui/MainWindowCloseTest.java b/src/test/java/seedu/address/ui/MainWindowCloseTest.java index 6321f255b048..e63967f1b926 100644 --- a/src/test/java/seedu/address/ui/MainWindowCloseTest.java +++ b/src/test/java/seedu/address/ui/MainWindowCloseTest.java @@ -1,28 +1,32 @@ package seedu.address.ui; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.util.Collections; + import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.testfx.api.FxToolkit; +import guitests.guihandles.HelpWindowHandle; import guitests.guihandles.StageHandle; import javafx.stage.Stage; import javafx.stage.WindowEvent; -import seedu.address.commons.core.Config; -import seedu.address.commons.events.ui.ExitAppRequestEvent; import seedu.address.logic.LogicManager; import seedu.address.model.ModelManager; -import seedu.address.model.UserPrefs; -import seedu.address.ui.testutil.EventsCollectorRule; +import seedu.address.storage.JsonAddressBookStorage; +import seedu.address.storage.JsonUserPrefsStorage; +import seedu.address.storage.StorageManager; /** * Contains tests for closing of the {@code MainWindow}. */ public class MainWindowCloseTest extends GuiUnitTest { @Rule - public final EventsCollectorRule eventsCollectorRule = new EventsCollectorRule(); + public final TemporaryFolder temporaryFolder = new TemporaryFolder(); private MainWindow mainWindow; private EmptyMainWindowHandle mainWindowHandle; @@ -30,29 +34,32 @@ public class MainWindowCloseTest extends GuiUnitTest { @Before public void setUp() throws Exception { + JsonAddressBookStorage jsonAddressBookStorage = new JsonAddressBookStorage(temporaryFolder.newFile().toPath()); + JsonUserPrefsStorage jsonUserPrefsStorage = new JsonUserPrefsStorage(temporaryFolder.newFile().toPath()); + StorageManager storageManager = new StorageManager(jsonAddressBookStorage, jsonUserPrefsStorage); FxToolkit.setupStage(stage -> { this.stage = stage; - mainWindow = new MainWindow(stage, new Config(), new UserPrefs(), new LogicManager(new ModelManager())); + mainWindow = new MainWindow(stage, new LogicManager(new ModelManager(), storageManager)); mainWindowHandle = new EmptyMainWindowHandle(stage); - - stage.setScene(mainWindow.getRoot().getScene()); mainWindowHandle.focus(); }); FxToolkit.showStage(); } @Test - public void close_menuBarExitButton_exitAppRequestEventPosted() { + public void close_menuBarExitButton_allWindowsClosed() { mainWindowHandle.clickOnMenuExitButton(); - assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof ExitAppRequestEvent); - assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1); + // The application will exit when all windows are closed. + assertEquals(Collections.emptyList(), guiRobot.listWindows()); } @Test public void close_externalRequest_exitAppRequestEventPosted() { + mainWindowHandle.clickOnMenuHelpButton(); + assertTrue(HelpWindowHandle.isWindowPresent()); mainWindowHandle.closeMainWindowExternally(); - assertTrue(eventsCollectorRule.eventsCollector.getMostRecent() instanceof ExitAppRequestEvent); - assertTrue(eventsCollectorRule.eventsCollector.getSize() == 1); + // The application will exit when all windows are closed. + assertEquals(Collections.emptyList(), guiRobot.listWindows()); } /** @@ -79,5 +86,13 @@ private void clickOnMenuExitButton() { private void closeMainWindowExternally() { guiRobot.interact(() -> stage.fireEvent(new WindowEvent(stage, WindowEvent.WINDOW_CLOSE_REQUEST))); } + + /** + * Opens the {@code HelpWindow} by clicking on the menu bar's help button. + */ + private void clickOnMenuHelpButton() { + guiRobot.clickOn("Help"); + guiRobot.clickOn("F1"); + } } } diff --git a/src/test/java/seedu/address/ui/PersonListPanelTest.java b/src/test/java/seedu/address/ui/PersonListPanelTest.java index 24c015565eb3..bac9d345f62d 100644 --- a/src/test/java/seedu/address/ui/PersonListPanelTest.java +++ b/src/test/java/seedu/address/ui/PersonListPanelTest.java @@ -3,37 +3,33 @@ import static java.time.Duration.ofMillis; import static org.junit.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively; -import static seedu.address.testutil.EventsUtil.postNow; import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_PERSON; import static seedu.address.testutil.TypicalPersons.getTypicalPersons; import static seedu.address.ui.testutil.GuiTestAssert.assertCardDisplaysPerson; import static seedu.address.ui.testutil.GuiTestAssert.assertCardEquals; -import java.nio.file.Path; -import java.nio.file.Paths; +import java.util.Collections; import org.junit.Test; import guitests.guihandles.PersonCardHandle; import guitests.guihandles.PersonListPanelHandle; +import javafx.beans.property.SimpleObjectProperty; import javafx.collections.FXCollections; import javafx.collections.ObservableList; -import seedu.address.commons.events.ui.JumpToListRequestEvent; -import seedu.address.commons.util.FileUtil; -import seedu.address.commons.util.XmlUtil; +import seedu.address.model.person.Address; +import seedu.address.model.person.Email; +import seedu.address.model.person.Name; import seedu.address.model.person.Person; -import seedu.address.storage.XmlSerializableAddressBook; +import seedu.address.model.person.Phone; public class PersonListPanelTest extends GuiUnitTest { private static final ObservableList TYPICAL_PERSONS = FXCollections.observableList(getTypicalPersons()); - private static final JumpToListRequestEvent JUMP_TO_SECOND_EVENT = new JumpToListRequestEvent(INDEX_SECOND_PERSON); - - private static final Path TEST_DATA_FOLDER = Paths.get("src", "test", "data", "sandbox"); - private static final long CARD_CREATION_AND_DELETION_TIMEOUT = 2500; + private final SimpleObjectProperty selectedPerson = new SimpleObjectProperty<>(); private PersonListPanelHandle personListPanelHandle; @Test @@ -51,9 +47,10 @@ public void display() { } @Test - public void handleJumpToListRequestEvent() { + public void selection_modelSelectedPersonChanged_selectionChanges() { initUi(TYPICAL_PERSONS); - postNow(JUMP_TO_SECOND_EVENT); + Person secondPerson = TYPICAL_PERSONS.get(INDEX_SECOND_PERSON.getZeroBased()); + guiRobot.interact(() -> selectedPerson.set(secondPerson)); guiRobot.pauseForHuman(); PersonCardHandle expectedPerson = personListPanelHandle.getPersonCardHandle(INDEX_SECOND_PERSON.getZeroBased()); @@ -66,7 +63,7 @@ public void handleJumpToListRequestEvent() { * {@code CARD_CREATION_AND_DELETION_TIMEOUT} milliseconds to execute. */ @Test - public void performanceTest() throws Exception { + public void performanceTest() { ObservableList backingList = createBackingList(10000); assertTimeoutPreemptively(ofMillis(CARD_CREATION_AND_DELETION_TIMEOUT), () -> { @@ -79,35 +76,17 @@ public void performanceTest() throws Exception { * Returns a list of persons containing {@code personCount} persons that is used to populate the * {@code PersonListPanel}. */ - private ObservableList createBackingList(int personCount) throws Exception { - Path xmlFile = createXmlFileWithPersons(personCount); - XmlSerializableAddressBook xmlAddressBook = - XmlUtil.getDataFromFile(xmlFile, XmlSerializableAddressBook.class); - return FXCollections.observableArrayList(xmlAddressBook.toModelType().getPersonList()); - } - - /** - * Returns a .xml file containing {@code personCount} persons. This file will be deleted when the JVM terminates. - */ - private Path createXmlFileWithPersons(int personCount) throws Exception { - StringBuilder builder = new StringBuilder(); - builder.append("\n"); - builder.append("\n"); + private ObservableList createBackingList(int personCount) { + ObservableList backingList = FXCollections.observableArrayList(); for (int i = 0; i < personCount; i++) { - builder.append("\n"); - builder.append("").append(i).append("a\n"); - builder.append("000\n"); - builder.append("a@aa\n"); - builder.append("
a
\n"); - builder.append("\n"); + Name name = new Name(i + "a"); + Phone phone = new Phone("000"); + Email email = new Email("a@aa"); + Address address = new Address("a"); + Person person = new Person(name, phone, email, address, Collections.emptySet()); + backingList.add(person); } - builder.append("\n"); - - Path manyPersonsFile = Paths.get(TEST_DATA_FOLDER + "manyPersons.xml"); - FileUtil.createFile(manyPersonsFile); - FileUtil.writeToFile(manyPersonsFile, builder.toString()); - manyPersonsFile.toFile().deleteOnExit(); - return manyPersonsFile; + return backingList; } /** @@ -115,7 +94,8 @@ private Path createXmlFileWithPersons(int personCount) throws Exception { * Also shows the {@code Stage} that displays only {@code PersonListPanel}. */ private void initUi(ObservableList backingList) { - PersonListPanel personListPanel = new PersonListPanel(backingList); + PersonListPanel personListPanel = + new PersonListPanel(backingList, selectedPerson, selectedPerson::set); uiPartRule.setUiPart(personListPanel); personListPanelHandle = new PersonListPanelHandle(getChildNode(personListPanel.getRoot(), diff --git a/src/test/java/seedu/address/ui/ResultDisplayTest.java b/src/test/java/seedu/address/ui/ResultDisplayTest.java index acea62615ff4..23a47b651881 100644 --- a/src/test/java/seedu/address/ui/ResultDisplayTest.java +++ b/src/test/java/seedu/address/ui/ResultDisplayTest.java @@ -1,23 +1,20 @@ package seedu.address.ui; import static org.junit.Assert.assertEquals; -import static seedu.address.testutil.EventsUtil.postNow; import org.junit.Before; import org.junit.Test; import guitests.guihandles.ResultDisplayHandle; -import seedu.address.commons.events.ui.NewResultAvailableEvent; public class ResultDisplayTest extends GuiUnitTest { - private static final NewResultAvailableEvent NEW_RESULT_EVENT_STUB = new NewResultAvailableEvent("Stub"); - + private ResultDisplay resultDisplay; private ResultDisplayHandle resultDisplayHandle; @Before public void setUp() { - ResultDisplay resultDisplay = new ResultDisplay(); + resultDisplay = new ResultDisplay(); uiPartRule.setUiPart(resultDisplay); resultDisplayHandle = new ResultDisplayHandle(getChildNode(resultDisplay.getRoot(), @@ -31,8 +28,8 @@ public void display() { assertEquals("", resultDisplayHandle.getText()); // new result received - postNow(NEW_RESULT_EVENT_STUB); + guiRobot.interact(() -> resultDisplay.setFeedbackToUser("Dummy feedback to user")); guiRobot.pauseForHuman(); - assertEquals(NEW_RESULT_EVENT_STUB.message, resultDisplayHandle.getText()); + assertEquals("Dummy feedback to user", resultDisplayHandle.getText()); } } diff --git a/src/test/java/seedu/address/ui/StatusBarFooterTest.java b/src/test/java/seedu/address/ui/StatusBarFooterTest.java index c7d21684a472..f044599b286a 100644 --- a/src/test/java/seedu/address/ui/StatusBarFooterTest.java +++ b/src/test/java/seedu/address/ui/StatusBarFooterTest.java @@ -1,7 +1,7 @@ package seedu.address.ui; import static org.junit.Assert.assertEquals; -import static seedu.address.testutil.EventsUtil.postNow; +import static seedu.address.testutil.TypicalPersons.ALICE; import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_INITIAL; import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_UPDATED; @@ -18,7 +18,6 @@ import org.junit.Test; import guitests.guihandles.StatusBarFooterHandle; -import seedu.address.commons.events.model.AddressBookChangedEvent; import seedu.address.model.AddressBook; public class StatusBarFooterTest extends GuiUnitTest { @@ -26,12 +25,11 @@ public class StatusBarFooterTest extends GuiUnitTest { 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()); - private static final Clock originalClock = StatusBarFooter.getClock(); private static final Clock injectedClock = Clock.fixed(Instant.now(), ZoneId.systemDefault()); private StatusBarFooterHandle statusBarFooterHandle; + private final AddressBook addressBook = new AddressBook(); @BeforeClass public static void setUpBeforeClass() { @@ -47,7 +45,7 @@ public static void tearDownAfterClass() { @Before public void setUp() { - StatusBarFooter statusBarFooter = new StatusBarFooter(STUB_SAVE_LOCATION); + StatusBarFooter statusBarFooter = new StatusBarFooter(STUB_SAVE_LOCATION, addressBook); uiPartRule.setUiPart(statusBarFooter); statusBarFooterHandle = new StatusBarFooterHandle(statusBarFooter.getRoot()); @@ -59,7 +57,7 @@ public void display() { assertStatusBarContent(RELATIVE_PATH.resolve(STUB_SAVE_LOCATION).toString(), SYNC_STATUS_INITIAL); // after address book is updated - postNow(EVENT_STUB); + guiRobot.interact(() -> addressBook.addPerson(ALICE)); assertStatusBarContent(RELATIVE_PATH.resolve(STUB_SAVE_LOCATION).toString(), String.format(SYNC_STATUS_UPDATED, new Date(injectedClock.millis()).toString())); } diff --git a/src/test/java/seedu/address/ui/testutil/EventsCollectorRule.java b/src/test/java/seedu/address/ui/testutil/EventsCollectorRule.java deleted file mode 100644 index 2985a5e60eb1..000000000000 --- a/src/test/java/seedu/address/ui/testutil/EventsCollectorRule.java +++ /dev/null @@ -1,86 +0,0 @@ -package seedu.address.ui.testutil; - -import java.util.ArrayList; -import java.util.List; - -import org.junit.rules.TestRule; -import org.junit.runner.Description; -import org.junit.runners.model.Statement; - -import com.google.common.eventbus.Subscribe; - -import seedu.address.commons.core.EventsCenter; -import seedu.address.commons.events.BaseEvent; - -/** - * Sets up an {@code EventsCollector} and tears it down after each test. - */ -public class EventsCollectorRule implements TestRule { - public final EventsCollector eventsCollector = new EventsCollector(); - - protected void after() { - EventsCenter.clearSubscribers(); - } - - @Override - public Statement apply(final Statement base, final Description description) { - return new Statement() { - @Override - public void evaluate() throws Throwable { - try { - base.evaluate(); - } finally { - after(); - } - } - }; - } - - /** - * A class that collects events raised by other classes. - */ - public class EventsCollector { - private List events = new ArrayList(); - - public EventsCollector() { - EventsCenter.getInstance().registerHandler(this); - } - - /** - * Collects any event raised by any class - */ - @Subscribe - public void collectEvent(BaseEvent event) { - events.add(event); - } - - /** - * Removes collected events from the collected list - */ - public void reset() { - events.clear(); - } - - public int getSize() { - return events.size(); - } - - /** - * Returns the most recent event collected - */ - public BaseEvent getMostRecent() { - if (events.isEmpty()) { - return null; - } - - return events.get(events.size() - 1); - } - - /** - * Returns true if the collector did not receive any events - */ - public boolean isEmpty() { - return events.isEmpty(); - } - } -} diff --git a/src/test/java/seedu/address/ui/testutil/GuiTestAssert.java b/src/test/java/seedu/address/ui/testutil/GuiTestAssert.java index 6d896fc24e79..07618ae9dd3d 100644 --- a/src/test/java/seedu/address/ui/testutil/GuiTestAssert.java +++ b/src/test/java/seedu/address/ui/testutil/GuiTestAssert.java @@ -34,7 +34,7 @@ public static void assertCardDisplaysPerson(Person expectedPerson, PersonCardHan assertEquals(expectedPerson.getPhone().value, actualCard.getPhone()); assertEquals(expectedPerson.getEmail().value, actualCard.getEmail()); assertEquals(expectedPerson.getAddress().value, actualCard.getAddress()); - assertEquals(expectedPerson.getTags().stream().map(tag -> tag.tagName).collect(Collectors.toList()), + assertEquals(expectedPerson.getTags().stream().map(tag -> tag.tagName).sorted().collect(Collectors.toList()), actualCard.getTags()); } diff --git a/src/test/java/systemtests/AddCommandSystemTest.java b/src/test/java/systemtests/AddCommandSystemTest.java index c656bca6f0fd..86a8a2dd04a7 100644 --- a/src/test/java/systemtests/AddCommandSystemTest.java +++ b/src/test/java/systemtests/AddCommandSystemTest.java @@ -158,24 +158,24 @@ public void add() { /* Case: invalid name -> rejected */ command = AddCommand.COMMAND_WORD + INVALID_NAME_DESC + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY; - assertCommandFailure(command, Name.MESSAGE_NAME_CONSTRAINTS); + assertCommandFailure(command, Name.MESSAGE_CONSTRAINTS); /* Case: invalid phone -> rejected */ command = AddCommand.COMMAND_WORD + NAME_DESC_AMY + INVALID_PHONE_DESC + EMAIL_DESC_AMY + ADDRESS_DESC_AMY; - assertCommandFailure(command, Phone.MESSAGE_PHONE_CONSTRAINTS); + assertCommandFailure(command, Phone.MESSAGE_CONSTRAINTS); /* Case: invalid email -> rejected */ command = AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY + INVALID_EMAIL_DESC + ADDRESS_DESC_AMY; - assertCommandFailure(command, Email.MESSAGE_EMAIL_CONSTRAINTS); + assertCommandFailure(command, Email.MESSAGE_CONSTRAINTS); /* Case: invalid address -> rejected */ command = AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY + INVALID_ADDRESS_DESC; - assertCommandFailure(command, Address.MESSAGE_ADDRESS_CONSTRAINTS); + assertCommandFailure(command, Address.MESSAGE_CONSTRAINTS); /* Case: invalid tag -> rejected */ command = AddCommand.COMMAND_WORD + NAME_DESC_AMY + PHONE_DESC_AMY + EMAIL_DESC_AMY + ADDRESS_DESC_AMY + INVALID_TAG_DESC; - assertCommandFailure(command, Tag.MESSAGE_TAG_CONSTRAINTS); + assertCommandFailure(command, Tag.MESSAGE_CONSTRAINTS); } /** diff --git a/src/test/java/systemtests/AddressBookSystemTest.java b/src/test/java/systemtests/AddressBookSystemTest.java index bf1743e3d16e..0baeaf860e6e 100644 --- a/src/test/java/systemtests/AddressBookSystemTest.java +++ b/src/test/java/systemtests/AddressBookSystemTest.java @@ -4,10 +4,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static seedu.address.ui.BrowserPanel.DEFAULT_PAGE; import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_INITIAL; import static seedu.address.ui.StatusBarFooter.SYNC_STATUS_UPDATED; -import static seedu.address.ui.UiPart.FXML_FILE_FOLDER; import static seedu.address.ui.testutil.GuiTestAssert.assertListMatching; import java.net.MalformedURLException; @@ -30,9 +28,7 @@ import guitests.guihandles.PersonListPanelHandle; import guitests.guihandles.ResultDisplayHandle; import guitests.guihandles.StatusBarFooterHandle; -import seedu.address.MainApp; import seedu.address.TestApp; -import seedu.address.commons.core.EventsCenter; import seedu.address.commons.core.index.Index; import seedu.address.logic.commands.ClearCommand; import seedu.address.logic.commands.FindCommand; @@ -78,7 +74,6 @@ public void setUp() { @After public void tearDown() { setupHelper.tearDownStage(); - EventsCenter.clearSubscribers(); } /** @@ -196,12 +191,12 @@ private void rememberStates() { } /** - * Asserts that the previously selected card is now deselected and the browser's url remains displaying the details - * of the previously selected person. + * Asserts that the previously selected card is now deselected and the browser's url is now displaying the + * default page. * @see BrowserPanelHandle#isUrlChanged() */ protected void assertSelectedCardDeselected() { - assertFalse(getBrowserPanel().isUrlChanged()); + assertEquals(BrowserPanel.DEFAULT_PAGE, getBrowserPanel().getLoadedUrl()); assertFalse(getPersonListPanel().isAnyCardSelected()); } @@ -277,7 +272,7 @@ private void assertApplicationStartingStateIsCorrect() { assertEquals("", getCommandBox().getInput()); assertEquals("", getResultDisplay().getText()); assertListMatching(getPersonListPanel(), getModel().getFilteredPersonList()); - assertEquals(MainApp.class.getResource(FXML_FILE_FOLDER + DEFAULT_PAGE), getBrowserPanel().getLoadedUrl()); + assertEquals(BrowserPanel.DEFAULT_PAGE, getBrowserPanel().getLoadedUrl()); assertEquals(Paths.get(".").resolve(testApp.getStorageSaveLocation()).toString(), getStatusBarFooter().getSaveLocation()); assertEquals(SYNC_STATUS_INITIAL, getStatusBarFooter().getSyncStatus()); diff --git a/src/test/java/systemtests/ClearCommandSystemTest.java b/src/test/java/systemtests/ClearCommandSystemTest.java index 805a59784e29..fb01059d9fe4 100644 --- a/src/test/java/systemtests/ClearCommandSystemTest.java +++ b/src/test/java/systemtests/ClearCommandSystemTest.java @@ -27,7 +27,7 @@ public void clear() { /* Case: undo clearing address book -> original address book restored */ String command = UndoCommand.COMMAND_WORD; String expectedResultMessage = UndoCommand.MESSAGE_SUCCESS; - assertCommandSuccess(command, expectedResultMessage, defaultModel); + assertCommandSuccess(command, expectedResultMessage, defaultModel); assertSelectedCardUnchanged(); /* Case: redo clearing address book -> cleared */ diff --git a/src/test/java/systemtests/EditCommandSystemTest.java b/src/test/java/systemtests/EditCommandSystemTest.java index 9e9e4572ddb2..5046250c0016 100644 --- a/src/test/java/systemtests/EditCommandSystemTest.java +++ b/src/test/java/systemtests/EditCommandSystemTest.java @@ -73,8 +73,7 @@ public void edit() { /* Case: redo editing the last person in the list -> last person edited again */ command = RedoCommand.COMMAND_WORD; expectedResultMessage = RedoCommand.MESSAGE_SUCCESS; - model.updatePerson( - getModel().getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()), editedPerson); + model.setPerson(getModel().getFilteredPersonList().get(INDEX_FIRST_PERSON.getZeroBased()), editedPerson); assertCommandSuccess(command, model, expectedResultMessage); /* Case: edit a person with new values same as existing values -> edited */ @@ -165,23 +164,23 @@ public void edit() { /* Case: invalid name -> rejected */ assertCommandFailure(EditCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased() + INVALID_NAME_DESC, - Name.MESSAGE_NAME_CONSTRAINTS); + Name.MESSAGE_CONSTRAINTS); /* Case: invalid phone -> rejected */ assertCommandFailure(EditCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased() + INVALID_PHONE_DESC, - Phone.MESSAGE_PHONE_CONSTRAINTS); + Phone.MESSAGE_CONSTRAINTS); /* Case: invalid email -> rejected */ assertCommandFailure(EditCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased() + INVALID_EMAIL_DESC, - Email.MESSAGE_EMAIL_CONSTRAINTS); + Email.MESSAGE_CONSTRAINTS); /* Case: invalid address -> rejected */ assertCommandFailure(EditCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased() + INVALID_ADDRESS_DESC, - Address.MESSAGE_ADDRESS_CONSTRAINTS); + Address.MESSAGE_CONSTRAINTS); /* Case: invalid tag -> rejected */ assertCommandFailure(EditCommand.COMMAND_WORD + " " + INDEX_FIRST_PERSON.getOneBased() + INVALID_TAG_DESC, - Tag.MESSAGE_TAG_CONSTRAINTS); + Tag.MESSAGE_CONSTRAINTS); /* Case: edit a person with new values same as another person's values -> rejected */ executeCommand(PersonUtil.getAddCommand(BOB)); @@ -234,7 +233,7 @@ private void assertCommandSuccess(String command, Index toEdit, Person editedPer private void assertCommandSuccess(String command, Index toEdit, Person editedPerson, Index expectedSelectedCardIndex) { Model expectedModel = getModel(); - expectedModel.updatePerson(expectedModel.getFilteredPersonList().get(toEdit.getZeroBased()), editedPerson); + expectedModel.setPerson(expectedModel.getFilteredPersonList().get(toEdit.getZeroBased()), editedPerson); expectedModel.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS); assertCommandSuccess(command, expectedModel, diff --git a/src/test/java/systemtests/ErrorDialogGuiTest.java b/src/test/java/systemtests/ErrorDialogGuiTest.java deleted file mode 100644 index f8ec17fdb2a7..000000000000 --- a/src/test/java/systemtests/ErrorDialogGuiTest.java +++ /dev/null @@ -1,34 +0,0 @@ -package systemtests; - -import static org.junit.Assert.assertEquals; -import static seedu.address.testutil.EventsUtil.postLater; -import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_CONTENT_MESSAGE; -import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_HEADER_MESSAGE; -import static seedu.address.ui.UiManager.FILE_OPS_ERROR_DIALOG_STAGE_TITLE; - -import java.io.IOException; - -import org.junit.Test; - -import guitests.GuiRobot; -import guitests.guihandles.AlertDialogHandle; -import seedu.address.commons.events.storage.DataSavingExceptionEvent; - -public class ErrorDialogGuiTest extends AddressBookSystemTest { - - private static final IOException IO_EXCEPTION_STUB = new IOException("Stub"); - private final GuiRobot guiRobot = new GuiRobot(); - - @Test - public void showErrorDialogs() { - postLater(new DataSavingExceptionEvent(IO_EXCEPTION_STUB)); - - guiRobot.waitForEvent(() -> guiRobot.isWindowShown(FILE_OPS_ERROR_DIALOG_STAGE_TITLE)); - - AlertDialogHandle alertDialog = new AlertDialogHandle(guiRobot.getStage(FILE_OPS_ERROR_DIALOG_STAGE_TITLE)); - assertEquals(FILE_OPS_ERROR_DIALOG_HEADER_MESSAGE, alertDialog.getHeaderText()); - assertEquals(FILE_OPS_ERROR_DIALOG_CONTENT_MESSAGE + ":\n" + IO_EXCEPTION_STUB.toString(), - alertDialog.getContentText()); - } - -}