diff --git a/.travis.yml b/.travis.yml index 9e7b0561..eaf78ba6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,8 @@ language: java matrix: include: - - env: TESTTYPE=headless - jdk: oraclejdk8 -script: travis_retry ./gradlew clean $TESTTYPE jacocoRootReport coveralls + - jdk: oraclejdk8 +script: travis_retry ./gradlew clean headless allTests coverage coveralls -i before_install: - "export DISPLAY=:99.0" - "sh -e /etc/init.d/xvfb start" diff --git a/UpdateData.json b/UpdateData.json index d44eb7ea..eab443ca 100644 --- a/UpdateData.json +++ b/UpdateData.json @@ -1,81 +1,81 @@ { - "version" : "V1.1.0ea", - "mainApp" : "https://github.com/HubTurbo/addressbook/releases/download/V1.1.0ea/resource-V1.1.0ea.jar", + "version" : "V1.2.0ea", + "mainApp" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/resource-V1.2.0ea.jar", "libraries" : [ { "filename" : "licence.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.2/licence.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/licence.jar", "os" : "ANY" }, { "filename" : "log4j-api-2.6.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/log4j-api-2.6.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/log4j-api-2.6.jar", "os" : "ANY" }, { "filename" : "log4j-core-2.6.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/log4j-core-2.6.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/log4j-core-2.6.jar", "os" : "ANY" }, { "filename" : "slf4j-simple-1.6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/slf4j-simple-1.6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/slf4j-simple-1.6.4.jar", "os" : "ANY" }, { "filename" : "commons-io-2.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/commons-io-2.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/commons-io-2.4.jar", "os" : "ANY" }, { "filename" : "controlsfx-8.40.10.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/controlsfx-8.40.10.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/controlsfx-8.40.10.jar", "os" : "ANY" }, { "filename" : "jackson-datatype-jsr310-2.7.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jackson-datatype-jsr310-2.7.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jackson-datatype-jsr310-2.7.4.jar", "os" : "ANY" }, { "filename" : "guava-19.0.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/guava-19.0.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/guava-19.0.jar", "os" : "ANY" }, { "filename" : "jxbrowser-win-6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jxbrowser-win-6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jxbrowser-win-6.4.jar", "os" : "WINDOWS" }, { "filename" : "jxbrowser-mac-6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jxbrowser-mac-6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jxbrowser-mac-6.4.jar", "os" : "MAC" }, { "filename" : "jxbrowser-linux32-6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jxbrowser-linux32-6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jxbrowser-linux32-6.4.jar", "os" : "LINUX32" }, { "filename" : "jxbrowser-linux64-6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jxbrowser-linux64-6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jxbrowser-linux64-6.4.jar", "os" : "LINUX64" }, { "filename" : "jkeymaster-1.2.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/jkeymaster-1.2.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jkeymaster-1.2.jar", "os" : "ANY" }, { "filename" : "jackson-annotations-2.7.0.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jackson-annotations-2.7.0.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jackson-annotations-2.7.0.jar", "os" : "ANY" }, { "filename" : "jxbrowser-6.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jxbrowser-6.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jxbrowser-6.4.jar", "os" : "ANY" }, { "filename" : "jna-4.2.1.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/jna-4.2.1.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jna-4.2.1.jar", "os" : "ANY" }, { "filename" : "jackson-core-2.7.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jackson-core-2.7.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jackson-core-2.7.4.jar", "os" : "ANY" }, { "filename" : "jackson-databind-2.7.4.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.0/jackson-databind-2.7.4.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/jackson-databind-2.7.4.jar", "os" : "ANY" }, { "filename" : "slf4j-api-1.7.13.jar", - "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/V0.0.1/slf4j-api-1.7.13.jar", + "downloadLink" : "https://github.com/HubTurbo/addressbook/releases/download/Resources/slf4j-api-1.7.13.jar", "os" : "ANY" } ] } \ No newline at end of file diff --git a/build.gradle b/build.gradle index 29722de6..f68f1220 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ plugins { allprojects { - version = 'V1.1.0ea' + version = 'V1.2.0ea' apply plugin: 'idea' apply plugin: 'java' @@ -51,8 +51,12 @@ allprojects { mockServerVersion = '3.10.1' powermockVersion = '1.6.5' testFxVersion = '3.1.0' + monocleVersion = '1.8.0_20' + slf4jSimpleVersion = '1.6.4' + commonsIoVersion = '2.4' mainAppArchiveName = 'resource-' + project.version + '.jar' + jarUpdaterDestDir = 'src/main/resources/updater' jarUpdaterArchiveName = 'jarUpdater.jar' } @@ -108,8 +112,8 @@ allprojects { dependencies { compile "org.apache.logging.log4j:log4j-api:$log4jVersion" compile "org.apache.logging.log4j:log4j-core:$log4jVersion" - compile "org.slf4j:slf4j-simple:1.6.4" // Required to suppress warning, for jkeymaster, see http://www.slf4j.org/codes.html#StaticLoggerBinder - compile 'commons-io:commons-io:2.4' + compile "org.slf4j:slf4j-simple:$slf4jSimpleVersion" // Required to suppress warning, for jkeymaster, see http://www.slf4j.org/codes.html#StaticLoggerBinder + compile "commons-io:commons-io:$commonsIoVersion" compile "org.controlsfx:controlsfx:$controlsFxVersion" compile "com.fasterxml.jackson.core:jackson-databind:$jacksonVersion" compile "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jacksonDataTypeVersion" @@ -129,10 +133,9 @@ allprojects { exclude group: "junit", module: "junit" } testCompile "org.mockito:mockito-core:$mockitoVersion" - testCompile group: "org.mock-server", name: "mockserver-netty", version: "$mockServerVersion" testCompile "org.powermock:powermock-api-mockito:$powermockVersion" testCompile "org.powermock:powermock-module-junit4:$powermockVersion" - testCompile 'org.testfx:openjfx-monocle:1.8.0_20' + testCompile "org.testfx:openjfx-monocle:$monocleVersion" installerCompile "com.fasterxml.jackson.core:jackson-databind:$jacksonVersion" installerCompile "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jacksonDataTypeVersion" @@ -159,6 +162,12 @@ allprojects { args arguments.split() } + task removeCurrentJarUpdater(type: Delete) { + group = "Release" + + delete project.ext.jarUpdaterDestDir + '/' + project.ext.jarUpdaterArchiveName + } + task createJarUpdater(type: Jar) { group = "Release" @@ -171,16 +180,18 @@ allprojects { } archiveName = project.ext.jarUpdaterArchiveName - destinationDir = file('src/main/resources/updater') + destinationDir = file(project.ext.jarUpdaterDestDir) // Specify classes needed for Jar Updater from(sourceSets.main.output.resourcesDir) { include 'log4j2.json' } from sourceSets.main.output + exclude '**/jarUpdater.jar' } createJarUpdater.dependsOn clean + createJarUpdater.dependsOn removeCurrentJarUpdater task copyDependencyLibrariesToReleaseDir(type: Copy) { group = "Release" @@ -254,12 +265,18 @@ allprojects { createInstallerJar.dependsOn createMainAppExecutable } -test { - forkEvery = 1 - systemProperty 'testfx.setup.timeout', '60000' +task wrapper(type: Wrapper) { + gradleVersion = '2.12' +} + +tasks.withType(FindBugs) { + reports { + xml.enabled = false + html.enabled = true + } } -task jacocoRootReport(type: JacocoReport) { +task coverage(type: JacocoReport) { sourceDirectories = files(allprojects.sourceSets.main.allSource.srcDirs) classDirectories = files(allprojects.sourceSets.main.output) executionData = files(allprojects.jacocoTestReport.executionData) @@ -276,57 +293,80 @@ task jacocoRootReport(type: JacocoReport) { coveralls { sourceDirs = allprojects.sourceSets.main.allSource.srcDirs.flatten() - jacocoReportPath = "${buildDir}/reports/jacoco/jacocoRootReport/jacocoRootReport.xml" + jacocoReportPath = "${buildDir}/reports/jacoco/coverage/coverage.xml" } tasks.coveralls { - group = 'Coverage reports' - description = 'Uploads the aggregated coverage report to Coveralls' - - dependsOn jacocoRootReport + dependsOn coverage onlyIf { System.env.'CI' } } -task wrapper(type: Wrapper) { - gradleVersion = '2.12' +task checkStyle { // A dummy task to run a set of tasks } -tasks.withType(FindBugs) { - reports { - xml.enabled = false - html.enabled = true +class AddressBookTest extends Test { + public AddressBookTest() { + forkEvery = 1 + systemProperty 'testfx.setup.timeout', '60000' } -} -task coverage { -} -task checkStyle { + public void setHeadless() { + systemProperty 'java.awt.robot', 'true' + systemProperty 'testfx.robot', 'glass' + systemProperty 'testfx.headless', 'true' + systemProperty 'prism.order', 'sw' + systemProperty 'prism.text', 't2k' + } } +task guiTests(type: AddressBookTest) { + include 'guitests/**' + jacoco { + destinationFile = new File("${buildDir}/jacoco/test.exec") + } +} -task headless(type: Test) { - systemProperty 'java.awt.robot', 'true' - systemProperty 'testfx.robot', 'glass' - systemProperty 'testfx.headless', 'true' - systemProperty 'prism.order', 'sw' - systemProperty 'prism.text', 't2k' - +task guiUnitTests(type: AddressBookTest) { + include 'guiunittests/**' jacoco { destinationFile = new File("${buildDir}/jacoco/test.exec") } } +task unitTests(type: AddressBookTest) { + include 'address/**' + jacoco { + destinationFile = new File("${buildDir}/jacoco/test.exec") + } +} -task headful(type: Test) { +task allTests(type: AddressBookTest) { jacoco { destinationFile = new File("${buildDir}/jacoco/test.exec") } } +task headless << { + println "Setting headless mode properties." + guiTests.setHeadless() + guiUnitTests.setHeadless() + unitTests.setHeadless() + allTests.setHeadless() +} + +// Makes sure that headless properties are set before running tests +unitTests.mustRunAfter headless +guiUnitTests.mustRunAfter headless +guiTests.mustRunAfter headless +allTests.mustRunAfter headless + headless.shouldRunAfter checkStyle -headful.shouldRunAfter checkStyle +unitTests.shouldRunAfter checkStyle +guiUnitTests.shouldRunAfter checkStyle +guiTests.shouldRunAfter checkStyle +allTests.shouldRunAfter checkStyle + checkStyle.shouldRunAfter clean checkStyle.dependsOn checkstyleMain, checkstyleTest, findbugsMain, findbugsTest, pmdMain, pmdTest -coverage.dependsOn jacocoTestReport -defaultTasks 'clean', 'headless', 'jacocoRootReport' +defaultTasks 'clean', 'headless', 'allTests', 'coverage' diff --git a/docs/KeyboardShortcuts.md b/docs/KeyboardShortcuts.md index f72457a1..a77b3e6b 100644 --- a/docs/KeyboardShortcuts.md +++ b/docs/KeyboardShortcuts.md @@ -5,8 +5,9 @@ | Shortcut | Action | | ------------------------------------: |--------| -| D | **D**elete selected person | -| E | **E**dit selected person | +| A | t**A**g the selected person | +| D | **D**elete the selected person | +| E | **E**dit the selected person | | G,B | **G**o to **B**ottom of the list | | G,T | **G**o to **T**op of the list | | Ctrl + N | **N**ew file | @@ -22,4 +23,4 @@ Unlike keyboard shortcuts given above, hotkeys work even when the app is not in | Shortcut | Action | | ------------------------------------: |--------| | Ctrl + Alt + X | Minimize app | -| Ctrl + Shift + X | Maximize app | \ No newline at end of file +| Ctrl + Shift + X | Toggle between maximum and default app Window sizes| \ No newline at end of file diff --git a/docs/Release.md b/docs/Release.md index e462dc7a..f21958a8 100644 --- a/docs/Release.md +++ b/docs/Release.md @@ -79,8 +79,8 @@ as `true` and add `ea` at the end of version in `build.gradle`. - This is so that the git tag that GitHub release creates will appropriately tag the commit with updated `UpdateData.json` 5. Create a release in [GitHub](https://github.com/HubTurbo/addressbook/releases) and tag the corresponding branch (`early-access` or `stable`) 6. Run `gradle` task `createInstallerJar` under `release` category (this must be run again to use the updated `UpdateData.json`) -7. Upload the following as binaries to the latest release: - - addressbook.jar +7. Upload `addressbook.jar` to the latest release. +8. Upload the following as binaries to the (`resource` release)[https://github.com/HubTurbo/addressbook/releases/tag/resources]: - resource-\.jar - all the jars inside `lib` directory which are mentioned in (2) @@ -158,6 +158,10 @@ Currently, Installer needs Jackson to parse the update data for dependency setti to Installer JAR; it will need `lib/[jackson].jar` to work. Those Jackson JARs are inside Installer JAR as resource, though, which will be unpacked anyway. Hence, JSON parsing in Installer must be called only after it unpacks itself. +Installer will launch the main application with `enable assertion` argument to enable assertion in production. On first +run, it will unpack the libraries JARs and the main app JAR then launch the main app. On the next runs, it acts as a +launcher to enable assertion in production. + ## To be improved ### Automate generateUpdateData diff --git a/docs/Testing.md b/docs/Testing.md index 3e744582..eab566ff 100644 --- a/docs/Testing.md +++ b/docs/Testing.md @@ -1,43 +1,87 @@ -# Running the Tests +# Testing -**GRADLE TASKS** -There are a few key gradle tasks defined that we can play around with: -- `headless` to run **headless testing** -- `headful` to run **headful testing** -- `checkStyle` to run code style checks -- `clean` to remove previously built files +## Background +We are using [Gradle](https://docs.gradle.org/)'s [wrapper](https://docs.gradle.org/current/userguide/gradle_wrapper.html) to setup testing configurations and running tests. +Gradle tasks are run using `gradlew ` on Windows and `./gradlew ` on Mac/Linux. -**Local Testing** -*Common commands used* -- `./gradlew` to run `clean`, `headless` tasks. -- `./gradlew checkStyle headful clean` - - HT's way of doing testing -- `./gradlew headless` to run **headless testing** only. - - Running this or `headful` repeatedly will not re-run the tests unless the build files are `clean`ed -- `./gradlew headful` to run **headful testing** only. +We work towards enabling **headless testing** (using [TestFX](https://github.com/TestFX/TestFX)) so that the test results will be more consistent between different machines. In addition, it will reduce disruption for the tester - the tester can continue to do his or her own work on the machine! -**CI Testing** -- `./gradlew` is run -At the moment, we do not check the code style. It is up to the contributor to verify his or her coding style locally by running `./gradlew checkStyle`. -- Automatically retries up to 3 times if a task fails +Related resources: + - [Gradle's User Guide](https://docs.gradle.org/current/userguide/userguide.html) + - [Gradle's Java Plugin](https://docs.gradle.org/current/userguide/java_plugin.html) + - [Gradle's Jacoco Plugin](https://docs.gradle.org/current/userguide/jacoco_plugin.html) + - [Coveralls Gradle Plugin](https://github.com/kt3k/coveralls-gradle-plugin) + - [Travis CI Documentation](https://docs.travis-ci.com/) -# Types of Tests +## Tests Information -We have grouped the tests into the following types. +Tests can be found in the `./src/test/java` folder. We have grouped and packaged the tests into the following types. -1. Unit Tests +1. Unit Tests - `address` package - Logic Testing -2. GUI Unit Testing +2. GUI Unit Tests - `guiunittests` package - Tests the UI interaction within a single component, and ensure its behaviour holds. -3. GUI Testing (Integration) +3. System Tests - `guitests` package - Tests the UI interaction with the user as well as the interaction between various components (e.g. passing of data) -# Current issues +## Running Tests + +Tests' settings are mostly contained in `build.gradle` and `.travis.yml`. + +### Available Gradle Tasks +There are a few key gradle tasks defined that we can play around with: +- `allTests` to run all tests +- `guiTests` to run tests in the `guitests` package +- `guiUnitTests` to run tests in the `guiunittests` package +- `unitTests` to run tests in the `address` package +- `headless` to set headless properties + - applies only to the above test tasks, and not to gradle's default `test` tasks +- `checkStyle` to run code style checks + - `PMD`, `FindBugs` and `Checkstyle` +- `clean` to remove previously built files + - Running tasks repeatedly may not work unless the build files are `clean`ed first. +- `coverage` to generate coverage information after tests have been run + - Generated coverage reports can be found at `./build/reports/jacoco/coverage/coverage.xml` +- `coveralls` to upload data from CI services to coveralls.io (no reason to run this locally) + +### Local Testing +#### How to do some common testing-related tasks +- To run all tests in headless mode: `./gradlew` + - It will run `clean`, `headless`, `allTests`, `coverage` tasks. +- To run all tests in headful mode: `./gradlew clean allTests coverage` +- To run checkstyle followed by headful tests `./gradlew clean checkStyle allTests coverage` +- Running specific test classes in a specific order + - When troubleshooting test failures, + you might want to run some specific tests in a specific order. + - Create a test suite (to specify the test order): + ```java + package address; + + import org.junit.runner.RunWith; + import org.junit.runners.Suite; + + @RunWith(Suite.class) + @Suite.SuiteClasses({ + /*The tests to run, in the order they should run*/ + address.browser.BrowserManagerTest.class, + guitests.PersonOverviewTest.class + }) + + public class TestsInOrder { + // the class remains empty, + // used only as a holder for the above annotations + } + ``` + - Run the test suite using the gradle command
+ `./gradlew clean headless allTests --tests address.TestsInOrder` + -**Headless Testing** - - Running headless tests on Mac OSX no longer causes the window to lose focus! - - [Causes JVM to crash when using Mac OSX](https://github.com/HubTurbo/addressbook/issues/108) - - Temporarily disabled initializing the browser in test mode. This should and will be changed to a mock instead in future PRs. +### CI Testing +- We run our CI builds on [Travis CI](https://travis-ci.org/HubTurbo/addressbook) +- The current Travis CI set up (also found in `.travis.yml`): + - runs the `./gradlew clean headless allTests coverage coveralls -i` command. + - At the moment, we do not check the code style. It is up to the contributor to verify his or her coding style locally by running `./gradlew checkStyle`. + - Automatically retries up to 3 times if a task fails diff --git a/src/main/java/address/MainApp.java b/src/main/java/address/MainApp.java index a911693a..b06215ef 100644 --- a/src/main/java/address/MainApp.java +++ b/src/main/java/address/MainApp.java @@ -4,7 +4,10 @@ import address.keybindings.KeyBindingsManager; import address.model.UserPrefs; import address.storage.StorageManager; +import address.sync.RemoteManager; import address.sync.SyncManager; +import address.sync.cloud.CloudSimulator; +import address.sync.cloud.IRemote; import address.ui.Ui; import address.updater.UpdateManager; import address.util.*; @@ -13,6 +16,9 @@ import javafx.application.Platform; import javafx.stage.Stage; +import java.io.File; +import java.util.Map; + /** * The main entry point to the application. */ @@ -20,7 +26,7 @@ public class MainApp extends Application { private static final AppLogger logger = LoggerManager.getLogger(MainApp.class); private static final int VERSION_MAJOR = 1; - private static final int VERSION_MINOR = 1; + private static final int VERSION_MINOR = 2; private static final int VERSION_PATCH = 0; private static final boolean VERSION_EARLY_ACCESS = true; @@ -38,6 +44,7 @@ public class MainApp extends Application { protected ModelManager modelManager; protected SyncManager syncManager; protected UpdateManager updateManager; + protected RemoteManager remoteManager; protected Ui ui; protected KeyBindingsManager keyBindingsManager; protected Config config; @@ -50,31 +57,58 @@ public void init() throws Exception { logger.info("Initializing app ..."); super.init(); new DependencyChecker(REQUIRED_JAVA_VERSION, this::quit).verify(); - config = initConfig(); + Map applicationParameters = getParameters().getNamed(); + config = initConfig(applicationParameters.get("config")); userPrefs = initPrefs(config); initComponents(config, userPrefs); } - protected Config initConfig() { - Config config = StorageManager.getConfig(); - logger.info("Config successfully obtained from StorageManager"); - return config; + protected Config initConfig(String configFilePath) { + return StorageManager.getConfig(configFilePath); } protected UserPrefs initPrefs(Config config) { - UserPrefs userPrefs = StorageManager.getUserPrefs(config.getPrefsFileLocation()); - return userPrefs; + return StorageManager.getUserPrefs(config.getPrefsFileLocation()); } - protected void initComponents(Config config, UserPrefs userPrefs) { + private void initComponents(Config config, UserPrefs userPrefs) { LoggerManager.init(config); - modelManager = new ModelManager(userPrefs); - storageManager = new StorageManager(modelManager::resetData, config, userPrefs); - ui = new Ui(this, modelManager, config); - syncManager = new SyncManager(config, userPrefs.getSaveLocation().getName()); - keyBindingsManager = new KeyBindingsManager(); - updateManager = new UpdateManager(VERSION); + modelManager = initModelManager(config); + storageManager = initStorageManager(modelManager, config, userPrefs); + ui = initUi(config, modelManager); + remoteManager = initRemoteManager(config); + syncManager = initSyncManager(remoteManager, config); + keyBindingsManager = initKeyBindingsManager(); + updateManager = initUpdateManager(VERSION); + } + + protected UpdateManager initUpdateManager(Version version) { + return new UpdateManager(version); + } + + protected KeyBindingsManager initKeyBindingsManager() { + return new KeyBindingsManager(); + } + + protected RemoteManager initRemoteManager(Config config) { + return new RemoteManager(new CloudSimulator(config)); + } + + protected SyncManager initSyncManager(RemoteManager remoteManager, Config config) { + return new SyncManager(remoteManager, config, config.getAddressBookName()); + } + + protected Ui initUi(Config config, ModelManager modelManager) { + return new Ui(this, modelManager, config, userPrefs); + } + + protected StorageManager initStorageManager(ModelManager modelManager, Config config, UserPrefs userPrefs) { + return new StorageManager(modelManager::resetData, config, userPrefs); + } + + protected ModelManager initModelManager(Config config) { + return new ModelManager(config); } @Override @@ -90,6 +124,7 @@ public void start(Stage primaryStage) { public void stop() { logger.info("Stopping application."); ui.stop(); + storageManager.savePrefsToFile(userPrefs); updateManager.stop(); syncManager.stop(); keyBindingsManager.stop(); diff --git a/src/main/java/address/browser/BrowserManager.java b/src/main/java/address/browser/BrowserManager.java index 609a1b03..7ef85681 100644 --- a/src/main/java/address/browser/BrowserManager.java +++ b/src/main/java/address/browser/BrowserManager.java @@ -51,7 +51,7 @@ public class BrowserManager { private ObservableList filteredPersons; - private Optional hyperBrowser; + private Optional hyperBrowser = Optional.empty(); private StringProperty selectedPersonUsername; diff --git a/src/main/java/address/controller/ActivityHistoryCardController.java b/src/main/java/address/controller/ActivityHistoryCardController.java new file mode 100644 index 00000000..ec88a46b --- /dev/null +++ b/src/main/java/address/controller/ActivityHistoryCardController.java @@ -0,0 +1,44 @@ +package address.controller; + +import address.model.CommandInfo; +import javafx.fxml.FXML; +import javafx.fxml.FXMLLoader; +import javafx.scene.control.Label; +import javafx.scene.layout.HBox; + +import java.io.IOException; + +/** + * TODO: Bind CommandInfo. Currently CommandInfo doesn't use Property class yet. + */ +public class ActivityHistoryCardController { + + @FXML + private HBox mainPane; + + @FXML + private Label activityLabel; + + private CommandInfo info; + + public ActivityHistoryCardController(CommandInfo info) { + this.info = info; + FXMLLoader fxmlLoader = new FXMLLoader(getClass().getResource("/view/ActivityHistoryCard.fxml")); + fxmlLoader.setController(this); + try { + fxmlLoader.load(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + @FXML + public void initialize() { + activityLabel.setText(info.getName() + " " + info.statusString()); + } + + public HBox getLayout() { + return mainPane; + } + +} diff --git a/src/main/java/address/controller/ActivityHistoryController.java b/src/main/java/address/controller/ActivityHistoryController.java new file mode 100644 index 00000000..35f6d3f6 --- /dev/null +++ b/src/main/java/address/controller/ActivityHistoryController.java @@ -0,0 +1,32 @@ +package address.controller; + +import address.model.CommandInfo; +import address.ui.CommandInfoListViewCell; +import address.util.FxViewUtil; +import javafx.collections.ObservableList; +import javafx.fxml.FXML; +import javafx.scene.control.ListView; +import javafx.scene.layout.AnchorPane; + +/** + * + */ +public class ActivityHistoryController { + + @FXML + private AnchorPane mainPane; + + private ObservableList infos; + + public void setConnections(ObservableList infos) { + this.infos = infos; + } + + public void init() { + ListView listView = new ListView<>(); + listView.setItems(infos); + listView.setCellFactory(lv -> new CommandInfoListViewCell()); + FxViewUtil.applyAnchorBoundaryParameters(listView, 0.0, 0.0, 0.0, 0.0); + this.mainPane.getChildren().add(listView); + } +} diff --git a/src/main/java/address/controller/HelpController.java b/src/main/java/address/controller/HelpController.java new file mode 100644 index 00000000..165740ff --- /dev/null +++ b/src/main/java/address/controller/HelpController.java @@ -0,0 +1,27 @@ +package address.controller; + +import hubturbo.EmbeddedBrowser; +import hubturbo.embeddedbrowser.fxbrowser.FxBrowserAdapter; +import javafx.fxml.FXML; +import javafx.scene.layout.AnchorPane; +import javafx.scene.web.WebView; + +/** + * Controller for a help page + */ +public class HelpController { + + @FXML + private AnchorPane mainPane; + + public HelpController() { + + } + + @FXML + public void initialize() { + EmbeddedBrowser browser = new FxBrowserAdapter(new WebView()); + browser.loadHTML("This is working :)"); + mainPane.getChildren().add(browser.getBrowserView()); + } +} diff --git a/src/main/java/address/controller/MainController.java b/src/main/java/address/controller/MainController.java index 75250c63..85edb348 100644 --- a/src/main/java/address/controller/MainController.java +++ b/src/main/java/address/controller/MainController.java @@ -4,6 +4,7 @@ import address.browser.BrowserManager; import address.events.*; import address.exceptions.DuplicateTagException; +import address.model.UserPrefs; import address.model.datatypes.person.ReadOnlyViewablePerson; import address.model.ModelManager; import address.model.datatypes.person.ReadOnlyPerson; @@ -14,6 +15,7 @@ import javafx.application.Platform; import javafx.collections.ObservableList; import javafx.fxml.FXMLLoader; +import javafx.scene.Node; import javafx.scene.Scene; import javafx.scene.control.Alert; import javafx.scene.control.Alert.AlertType; @@ -39,6 +41,8 @@ */ public class MainController extends UiController{ private static final AppLogger logger = LoggerManager.getLogger(MainController.class); + private static final String FXML_ACTIVITY_HISTORY = "/view/ActivityHistory.fxml"; + private static final String FXML_HELP = "/view/help.fxml"; private static final String FXML_STATUS_BAR_FOOTER = "/view/StatusBarFooter.fxml"; private static final String FXML_TAG_EDIT_DIALOG = "/view/TagEditDialog.fxml"; private static final String FXML_PERSON_EDIT_DIALOG = "/view/PersonEditDialog.fxml"; @@ -50,6 +54,10 @@ public class MainController extends UiController{ private static final String ICON_APPLICATION = "/images/address_book_32.png"; private static final String ICON_EDIT = "/images/edit.png"; private static final String ICON_CALENDAR = "/images/calendar.png"; + private static final String ICON_INFO = "/images/info_icon.png"; + private static final String ICON_HELP = "/images/help_icon.png"; + public static final int MIN_HEIGHT = 600; + public static final int MIN_WIDTH = 740; private Stage primaryStage; private VBox rootLayout; @@ -58,6 +66,7 @@ public class MainController extends UiController{ private BrowserManager browserManager; private MainApp mainApp; private Config config; + private UserPrefs prefs; private StatusBarHeaderController statusBarHeaderController; @@ -70,11 +79,12 @@ public class MainController extends UiController{ * @param modelManager * @param config should have appTitle and updateInterval set */ - public MainController(MainApp mainApp, ModelManager modelManager, Config config) { + public MainController(MainApp mainApp, ModelManager modelManager, Config config, UserPrefs prefs) { super(); this.mainApp = mainApp; this.modelManager = modelManager; this.config = config; + this.prefs = prefs; this.personList = modelManager.getAllViewablePersonsReadOnly(); this.browserManager = new BrowserManager(personList, config.getBrowserNoOfPages(), config.getBrowserType()); this.browserManager.initBrowser(); @@ -84,7 +94,7 @@ public void start(Stage primaryStage) { logger.info("Starting main controller."); this.primaryStage = primaryStage; this.browserManager.start(); - primaryStage.setTitle(config.appTitle); + primaryStage.setTitle(config.getAppTitle()); // Set the application icon. this.primaryStage.getIcons().add(getImage(ICON_APPLICATION)); @@ -109,32 +119,23 @@ private void showTipOfTheDay() { public void initRootLayout() { logger.debug("Initializing root layout."); final String fxmlResourcePath = FXML_ROOT_LAYOUT; - try { - // Load root layout from fxml file. - FXMLLoader loader = loadFxml(fxmlResourcePath); - rootLayout = loader.load(); - - // Show the scene containing the root layout. - Scene scene = new Scene(rootLayout); - scene.setOnKeyPressed(event -> raisePotentialEvent(new KeyBindingEvent(event))); - primaryStage.setMinHeight(400); - primaryStage.setMinWidth(740); - primaryStage.setHeight(600); - primaryStage.setWidth(740); - primaryStage.setScene(scene); - - // Give the rootController access to the main controller and modelManager - RootLayoutController rootController = loader.getController(); - rootController.setConnections(mainApp, this, modelManager); - rootController.setAccelerators(); - - primaryStage.show(); - } catch (IOException e) { - logger.warn("Error initializing root layout: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml root layout.", - "IOException when trying to load " + fxmlResourcePath); - //TODO: this dialog doesn't show up - } + // Load root layout from fxml file. + FXMLLoader loader = loadFxml(fxmlResourcePath); + rootLayout = (VBox) loadLoader(loader, "Error initializing root layout"); + + // Show the scene containing the root layout. + Scene scene = new Scene(rootLayout); + scene.setOnKeyPressed(event -> raisePotentialEvent(new KeyBindingEvent(event))); + setMinSize(); + setDefaultSize(); + primaryStage.setScene(scene); + + // Give the rootController access to the main controller and modelManager + RootLayoutController rootController = loader.getController(); + rootController.setConnections(mainApp, this, modelManager); + rootController.setAccelerators(); + + primaryStage.show(); } /** @@ -143,22 +144,17 @@ public void initRootLayout() { public void showPersonOverview() { logger.debug("Loading person overview."); final String fxmlResourcePath = FXML_PERSON_OVERVIEW; - try { - // Load person overview. - FXMLLoader loader = loadFxml(fxmlResourcePath); - AnchorPane personOverview = loader.load(); - SplitPane pane = (SplitPane) rootLayout.lookup("#splitPane"); - SplitPane.setResizableWithParent(personOverview, false); - // Give the personOverviewController access to the main app and modelManager. - PersonOverviewController personOverviewController = loader.getController(); - personOverviewController.setConnections(this, modelManager, personList); - - pane.getItems().add(personOverview); - } catch (IOException e) { - logger.warn("Error loading person overview: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for person overview.", - "IOException when trying to load " + fxmlResourcePath); - } + // Load person overview. + FXMLLoader loader = loadFxml(fxmlResourcePath); + VBox personOverview = (VBox) loadLoader(loader, "Error loading person overview"); + AnchorPane pane = (AnchorPane) rootLayout.lookup("#personOverview"); + SplitPane.setResizableWithParent(pane, false); + // Give the personOverviewController access to the main app and modelManager. + PersonOverviewController personOverviewController = loader.getController(); + personOverviewController.setConnections(this, modelManager, personList); + + pane.getChildren().add(personOverview); + } public StatusBarHeaderController getStatusBarHeaderController() { @@ -166,10 +162,10 @@ public StatusBarHeaderController getStatusBarHeaderController() { } private void showHeaderStatusBar() { - statusBarHeaderController = new StatusBarHeaderController(); + statusBarHeaderController = new StatusBarHeaderController(this); AnchorPane sbPlaceHolder = (AnchorPane) rootLayout.lookup("#headerStatusbarPlaceholder"); - assert sbPlaceHolder != null : "footerStatusbarPlaceHolder node not found in rootLayout"; + assert sbPlaceHolder != null : "headerStatusbarPlaceHolder node not found in rootLayout"; FxViewUtil.applyAnchorBoundaryParameters(statusBarHeaderController.getHeaderStatusBarView(), 0.0, 0.0, 0.0, 0.0); sbPlaceHolder.getChildren().add(statusBarHeaderController.getHeaderStatusBarView()); @@ -178,17 +174,24 @@ private void showHeaderStatusBar() { private void showFooterStatusBar() { logger.debug("Loading footer status bar."); final String fxmlResourcePath = FXML_STATUS_BAR_FOOTER; + FXMLLoader loader = loadFxml(fxmlResourcePath); + GridPane gridPane = (GridPane) loadLoader(loader, "Error Loading footer status bar"); + gridPane.getStyleClass().add("grid-pane"); + StatusBarFooterController controller = loader.getController(); + controller.init(config.getUpdateInterval(), config.getAddressBookName()); + AnchorPane placeHolder = (AnchorPane) rootLayout.lookup("#footerStatusbarPlaceholder"); + FxViewUtil.applyAnchorBoundaryParameters(gridPane, 0.0, 0.0, 0.0, 0.0); + placeHolder.getChildren().add(gridPane); + } + + private Node loadLoader(FXMLLoader loader, String errorMsg ) { try { - FXMLLoader loader = loadFxml(fxmlResourcePath); - GridPane gridPane = loader.load(); - gridPane.getStyleClass().add("grid-pane"); - StatusBarFooterController controller = loader.getController(); - controller.init(config.updateInterval, modelManager.getPrefs().getSaveLocation()); - rootLayout.getChildren().add(gridPane); + return loader.load(); } catch (IOException e) { - logger.warn("Error Loading footer status bar: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for footer status bar.", - "IOException when trying to load " + fxmlResourcePath); + logger.fatal(errorMsg + ": {}", e); + showFatalErrorDialogAndShutdown("FXML Load Error", errorMsg, + "IOException when trying to load ", loader.getLocation().toExternalForm()); + return null; } } @@ -210,40 +213,32 @@ private FXMLLoader loadFxml(String fxmlResourcePath) { public Optional getPersonDataInput(ReadOnlyPerson initialData, String dialogTitle) { logger.debug("Loading dialog for person edit."); final String fxmlResourcePath = FXML_PERSON_EDIT_DIALOG; - try { // Load the fxml file and create a new stage for the popup dialog. - FXMLLoader loader = loadFxml(fxmlResourcePath); - AnchorPane page = loader.load(); + FXMLLoader loader = loadFxml(fxmlResourcePath); + AnchorPane page = (AnchorPane) loadLoader(loader, "Error loading person edit dialog"); - Scene scene = new Scene(page); - Stage dialogStage = loadDialogStage(dialogTitle, primaryStage, scene); - dialogStage.getIcons().add(getImage(ICON_EDIT)); - - scene.setOnKeyPressed(event -> { - if (event.getCode() == KeyCode.ESCAPE) { - dialogStage.close(); - } - }); - - // Pass relevant data into the controller. - PersonEditDialogController controller = loader.getController(); - controller.setDialogStage(dialogStage); - controller.setInitialPersonData(initialData); - controller.setTags(modelManager.getTagsAsReadOnlyObservableList(), - new ArrayList<>(initialData.getObservableTagList())); - - dialogStage.showAndWait(); - if (controller.isOkClicked()) { - logger.debug("Person collected: " + controller.getEditedPerson().toString()); - return Optional.of(controller.getEditedPerson()); - } else { - return Optional.empty(); + Scene scene = new Scene(page); + Stage dialogStage = loadDialogStage(dialogTitle, primaryStage, scene); + dialogStage.getIcons().add(getImage(ICON_EDIT)); + + scene.setOnKeyPressed(event -> { + if (event.getCode() == KeyCode.ESCAPE) { + dialogStage.close(); } - } catch (IOException e) { - e.printStackTrace(); - logger.warn("Error loading person edit dialog: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for edit person dialog.", - "IOException when trying to load " + fxmlResourcePath); + }); + + // Pass relevant data into the controller. + PersonEditDialogController controller = loader.getController(); + controller.setDialogStage(dialogStage); + controller.setInitialPersonData(initialData); + controller.setTags(modelManager.getTagsAsReadOnlyObservableList(), + new ArrayList<>(initialData.getObservableTagList())); + + dialogStage.showAndWait(); + if (controller.isOkClicked()) { + logger.debug("Person collected: " + controller.getEditedPerson().toString()); + return Optional.of(controller.getEditedPerson()); + } else { return Optional.empty(); } } @@ -251,14 +246,8 @@ public Optional getPersonDataInput(ReadOnlyPerson initialData, S public Optional> getPersonsTagsInput(List persons) { FXMLLoader loader = new FXMLLoader(); loader.setLocation(MainApp.class.getResource(FXML_TAG_SELECTION_EDIT_DIALOG)); - AnchorPane pane = null; - try { - pane = loader.load(); + AnchorPane pane = (AnchorPane) loadLoader(loader, "Error launching tag selection dialog"); - } catch (IOException e) { - logger.warn("Error launching tag selection dialog: {}", e); - assert false : "Error loading fxml : " + FXML_TAG_SELECTION_EDIT_DIALOG; - } // Create the dialog Stage. Stage dialogStage = new Stage(); @@ -376,30 +365,23 @@ private boolean isUpdateSuccessful(Tag originalTag, Tag newTag) { public Optional getTagDataInput(Tag tag, String dialogTitle) { logger.debug("Loading dialog for tag edit."); final String fxmlResourcePath = FXML_TAG_EDIT_DIALOG; - try { // Load the fxml file and create a new stage for the popup dialog. - FXMLLoader loader = loadFxml(fxmlResourcePath); - AnchorPane page = loader.load(); + FXMLLoader loader = loadFxml(fxmlResourcePath); + AnchorPane page = (AnchorPane) loadLoader(loader, "Error loading tag edit dialog"); - Scene scene = new Scene(page); - Stage dialogStage = loadDialogStage(dialogTitle, primaryStage, scene); - dialogStage.getIcons().add(getImage(ICON_EDIT)); - - // Pass relevant data to the controller. - TagEditDialogController controller = loader.getController(); - controller.setDialogStage(dialogStage); - controller.setInitialTagData(tag); - - dialogStage.showAndWait(); - if (controller.isOkClicked()) { - return Optional.of(controller.getEditedTag()); - } else { - return Optional.empty(); - } - } catch (IOException e) { - logger.warn("Error loading tag edit dialog: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for edit tag dialog.", - "IOException when trying to load " + fxmlResourcePath); + Scene scene = new Scene(page); + Stage dialogStage = loadDialogStage(dialogTitle, primaryStage, scene); + dialogStage.getIcons().add(getImage(ICON_EDIT)); + + // Pass relevant data to the controller. + TagEditDialogController controller = loader.getController(); + controller.setDialogStage(dialogStage); + controller.setInitialTagData(tag); + + dialogStage.showAndWait(); + if (controller.isOkClicked()) { + return Optional.of(controller.getEditedTag()); + } else { return Optional.empty(); } } @@ -407,28 +389,39 @@ public Optional getTagDataInput(Tag tag, String dialogTitle) { public void showTagList(ObservableList tags) { logger.debug("Loading tag list."); final String fxmlResourcePath = FXML_TAG_LIST; - try { - // Load the fxml file and create a new stage for the popup dialog. - FXMLLoader loader = loadFxml(fxmlResourcePath); - AnchorPane page = loader.load(); + // Load the fxml file and create a new stage for the popup dialog. + FXMLLoader loader = loadFxml(fxmlResourcePath); + AnchorPane page = (AnchorPane) loadLoader(loader, "Error loading tag list view"); - Scene scene = new Scene(page); - Stage dialogStage = loadDialogStage("List of Tags", primaryStage, scene); + Scene scene = new Scene(page); + Stage dialogStage = loadDialogStage("List of Tags", primaryStage, scene); - // Set the tag into the controller. - TagListController tagListController = loader.getController(); - tagListController.setMainController(this); - tagListController.setModelManager(modelManager); - tagListController.setTags(tags); - tagListController.setStage(dialogStage); + // Set the tag into the controller. + TagListController tagListController = loader.getController(); + tagListController.setMainController(this); + tagListController.setModelManager(modelManager); + tagListController.setTags(tags); + tagListController.setStage(dialogStage); - // Show the dialog and wait until the user closes it - dialogStage.showAndWait(); - } catch (IOException e) { - logger.warn("Error loading tag list view: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for tag list.", - "IOException when trying to load " + fxmlResourcePath); - } + // Show the dialog and wait until the user closes it + dialogStage.showAndWait(); + } + + /** + * Opens a dialog to show the help page + */ + public void showHelpPage() { + logger.debug("Loading help page."); + final String fxmlResourcePath = FXML_HELP; + // Load the fxml file and create a new stage for the popup dialog. + FXMLLoader loader = loadFxml(fxmlResourcePath); + AnchorPane page = (AnchorPane) loadLoader(loader, "Error loading help page"); + + Scene scene = new Scene(page); + Stage dialogStage = loadDialogStage("Help", null, scene); + dialogStage.getIcons().add(getImage(ICON_HELP)); + // Show the dialog and wait until the user closes it + dialogStage.showAndWait(); } /** @@ -437,24 +430,41 @@ public void showTagList(ObservableList tags) { public void showBirthdayStatistics() { logger.debug("Loading birthday statistics."); final String fxmlResourcePath = FXML_BIRTHDAY_STATISTICS; + // Load the fxml file and create a new stage for the popup. + FXMLLoader loader = loadFxml(fxmlResourcePath); + AnchorPane page = (AnchorPane) loadLoader(loader, "Error loading birthday statistics view"); + + Scene scene = new Scene(page); + Stage dialogStage = loadDialogStage("Birthday Statistics", primaryStage, scene); + dialogStage.getIcons().add(getImage(ICON_CALENDAR)); + + // Set the persons into the controller. + BirthdayStatisticsController controller = loader.getController(); + controller.setPersonData(modelManager.getAllViewablePersonsReadOnly()); + + dialogStage.show(); + } + + public void showActivityHistoryDialog() { + logger.debug("Loading Activity History."); + final String fxmlResourcePath = FXML_ACTIVITY_HISTORY; try { // Load the fxml file and create a new stage for the popup. FXMLLoader loader = loadFxml(fxmlResourcePath); AnchorPane page = loader.load(); Scene scene = new Scene(page); - Stage dialogStage = loadDialogStage("Birthday Statistics", primaryStage, scene); - dialogStage.getIcons().add(getImage(ICON_CALENDAR)); - + Stage dialogStage = loadDialogStage("Activity History", primaryStage, scene); + dialogStage.getIcons().add(getImage(ICON_INFO)); // Set the persons into the controller. - BirthdayStatisticsController controller = loader.getController(); - controller.setPersonData(modelManager.getAllViewablePersonsReadOnly()); - + ActivityHistoryController controller = loader.getController(); + controller.setConnections(modelManager.getFinishedCommands()); + controller.init(); dialogStage.show(); } catch (IOException e) { - logger.warn("Error loading birthday statistics view: {}", e); - showAlertDialogAndWait(AlertType.ERROR, "FXML Load Error", "Cannot load fxml for birthday stats.", - "IOException when trying to load " + fxmlResourcePath); + logger.fatal("Error loading activity history view: {}", e); + showFatalErrorDialogAndShutdown("FXML Load Error", "Cannot load fxml for activity history.", + "IOException when trying to load ", fxmlResourcePath); } } @@ -515,14 +525,14 @@ public void loadGithubProfilePage(ReadOnlyViewablePerson person){ } public void showPersonWebPage() { - SplitPane pane = (SplitPane) rootLayout.lookup("#splitPane"); - pane.getItems().add(browserManager.getHyperBrowserView()); + AnchorPane pane = (AnchorPane) rootLayout.lookup("#personWebpage"); + pane.getChildren().add(browserManager.getHyperBrowserView()); } @Subscribe - private void handleMaximizeAppRequestEvent(MaximizeAppRequestEvent event){ - logger.debug("Handling the maximize app window request"); - Platform.runLater(this::maximizeWindow); + private void handleResizeAppRequestEvent(ResizeAppRequestEvent event){ + logger.debug("Handling the resize app window request"); + Platform.runLater(this::resizeWindow); } @Subscribe @@ -531,6 +541,34 @@ private void handleMinimizeAppRequestEvent(MinimizeAppRequestEvent event){ Platform.runLater(this::minimizeWindow); } + /** + * Toggles between maximized and default size. + * If not currently at the maximized size, goes to maximised size. + * If currently maximized, goes to default size. + */ + private void resizeWindow() { + if (primaryStage.isMaximized()) { + setDefaultSize(); + } else { + maximizeWindow(); + } + } + + protected void setDefaultSize() { + 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()); + } + primaryStage.setMaximized(false); + primaryStage.setIconified(false); + } + + private void setMinSize() { + primaryStage.setMinHeight(MIN_HEIGHT); + primaryStage.setMinWidth(MIN_WIDTH); + } private void minimizeWindow() { primaryStage.setIconified(true); @@ -546,4 +584,11 @@ public void stop() { getPrimaryStage().hide(); releaseResourcesForAppTermination(); } + + private void showFatalErrorDialogAndShutdown(String title, String headerText, String contentText, String errorLocation) { + showAlertDialogAndWait(AlertType.ERROR, title, headerText, + contentText + errorLocation); + Platform.exit(); + System.exit(1); + } } diff --git a/src/main/java/address/controller/PersonCardController.java b/src/main/java/address/controller/PersonCardController.java index 903466d0..1ba122d9 100644 --- a/src/main/java/address/controller/PersonCardController.java +++ b/src/main/java/address/controller/PersonCardController.java @@ -12,23 +12,33 @@ import javafx.application.Platform; import javafx.beans.binding.StringBinding; +import javafx.beans.property.SimpleStringProperty; +import javafx.beans.property.StringProperty; import javafx.fxml.FXML; import javafx.fxml.FXMLLoader; import javafx.scene.control.Label; +import javafx.scene.control.ProgressIndicator; +import javafx.scene.control.Tooltip; import javafx.scene.image.Image; import javafx.scene.image.ImageView; import javafx.scene.layout.HBox; import javafx.util.Duration; public class PersonCardController extends UiController{ - public static final String PENDING_STATE_MESSAGE = "Syncing in %d seconds"; + + private enum PendingState { + COUNTING_DOWN, SYNCING, SYNCING_DONE + } + + public static final String DELETING_PENDING_STATE_MESSAGE = "Deleted"; + public static final String EDITING_PENDING_STATE_MESSAGE = "Edited"; + public static final String CREATED_PENDING_STATE_MESSAGE = "Created"; + @FXML private HBox cardPane; @FXML private ImageView profileImage; @FXML - private Label idLabel; - @FXML private Label firstName; @FXML private Label lastName; @@ -41,10 +51,15 @@ public class PersonCardController extends UiController{ @FXML private Label pendingStateLabel; @FXML - private ImageView syncingImageView; + private ProgressIndicator syncIndicator; + @FXML + private HBox pendingStateHolder; + @FXML + private Label pendingCountdownIndicator; private ReadOnlyViewablePerson person; private FadeTransition deleteTransition; + private StringProperty idTooltipString = new SimpleStringProperty(""); { deleteTransition = new FadeTransition(Duration.millis(1000), cardPane); @@ -72,12 +87,9 @@ public void initialize() { setProfileImage(); } - if (person.isDeleted()) { - Platform.runLater(() -> cardPane.setOpacity(0.1f)); - } FxViewUtil.configureCircularImageView(profileImage); - initIdLabel(); + initIdTooltip(); firstName.textProperty().bind(person.firstNameProperty()); lastName.textProperty().bind(person.lastNameProperty()); @@ -125,49 +137,71 @@ protected String computeValue() { return person.tagsString(); } }); - person.isDeletedProperty().addListener((observable, oldValue, newValue) -> { - if (newValue) { - handleDelete(); - } else { -// deleteTransition.stop(); - } - }); + person.githubUsernameProperty().addListener((observable, oldValue, newValue) -> { if (newValue.length() > 0) { setProfileImage(); } }); if (person.getSecondsLeftInPendingState() > 0) { - pendingStateLabel.setText(String.format(PENDING_STATE_MESSAGE, person.getSecondsLeftInPendingState())); - pendingStateLabel.setVisible(true); + setPendingStateMessage(person.getChangeInProgress()); + pendingStateHolder.setVisible(true); + pendingCountdownIndicator.setVisible(true); } person.secondsLeftInPendingStateProperty().addListener((observable, oldValue, newValue) -> { if (newValue.intValue() > 0) { - pendingStateLabel.setText(String.format(PENDING_STATE_MESSAGE, newValue)); - pendingStateLabel.setVisible(true); + setPendingStateMessage(person.getChangeInProgress()); + setVisibilitySettings(PendingState.COUNTING_DOWN); } else { cardPane.setStyle(null); - pendingStateLabel.setText("Syncing..."); - pendingStateLabel.setVisible(true); - syncingImageView.setVisible(true); + setPendingStateMessage(person.getChangeInProgress()); + setVisibilitySettings(PendingState.SYNCING); person.onRemoteIdConfirmed((Integer id) -> { - syncingImageView.setVisible(false); + setVisibilitySettings(PendingState.SYNCING_DONE); pendingStateLabel.setText(""); - pendingStateLabel.setVisible(false); }); } }); + pendingCountdownIndicator.textProperty().bind(person.secondsLeftInPendingStateProperty().asString()); } - private void initIdLabel() { - idLabel.setText(person.idString()); - person.onRemoteIdConfirmed(id -> { - if (Platform.isFxApplicationThread()) { - idLabel.setText(person.idString()); - } else { - Platform.runLater(() -> idLabel.setText(person.idString())); - } - }); + private void setVisibilitySettings(PendingState state) { + switch (state) { + case COUNTING_DOWN: + pendingStateHolder.setVisible(true); + pendingCountdownIndicator.setVisible(true); + break; + case SYNCING: + pendingStateHolder.setVisible(true); + pendingCountdownIndicator.setVisible(false); + syncIndicator.setVisible(true); + break; + case SYNCING_DONE: + syncIndicator.setVisible(false); + pendingStateHolder.setVisible(false); + break; + } + } + + private void setPendingStateMessage(ReadOnlyViewablePerson.ChangeInProgress changeInProgress) { + + + if (changeInProgress == ReadOnlyViewablePerson.ChangeInProgress.ADDING) { + pendingStateLabel.setText(CREATED_PENDING_STATE_MESSAGE); + } else if (changeInProgress == ReadOnlyViewablePerson.ChangeInProgress.EDITING) { + pendingStateLabel.setText(EDITING_PENDING_STATE_MESSAGE); + } else if (changeInProgress == ReadOnlyViewablePerson.ChangeInProgress.DELETING) { + pendingStateLabel.setText(DELETING_PENDING_STATE_MESSAGE); + } + } + + private void initIdTooltip() { + Tooltip tp = new Tooltip(); + tp.textProperty().bind(idTooltipString); + firstName.setTooltip(tp); + lastName.setTooltip(tp); + idTooltipString.set(person.idString()); + person.onRemoteIdConfirmed(id -> idTooltipString.set(person.idString())); } /** diff --git a/src/main/java/address/controller/PersonOverviewController.java b/src/main/java/address/controller/PersonOverviewController.java index afd0a2f6..694f6609 100644 --- a/src/main/java/address/controller/PersonOverviewController.java +++ b/src/main/java/address/controller/PersonOverviewController.java @@ -122,7 +122,7 @@ private void initialize() { private void handleDeletePersons() { final List selected = personListView.getSelectionModel().getSelectedItems(); - if (isSelectionValid()) { + if (!isSelectionValid()) { showInvalidSelectionAlert(); } else { selected.stream() @@ -135,7 +135,7 @@ private void handleDeletePersons() { private boolean isSelectionValid() { final List selected = personListView.getSelectionModel().getSelectedItems(); - return selected.isEmpty() || selected.stream().anyMatch(Objects::isNull); + return !selected.isEmpty() && !selected.stream().anyMatch(Objects::isNull); } /** @@ -153,22 +153,11 @@ private void handleNewPerson() { */ private void handleRetagPersons() { List selectedPersons = personListView.getSelectionModel().getSelectedItems(); - - if (isSelectionValid()) { + if (!isSelectionValid()) { showInvalidSelectionAlert(); return; } - - Optional> listOfFinalAssignedTags = mainController.getPersonsTagsInput(selectedPersons); - - if (listOfFinalAssignedTags.isPresent()) { - selectedPersons.stream() - .forEach(p -> { - Person editedPerson = new Person(p); - editedPerson.setTags(listOfFinalAssignedTags.get()); - modelManager.editPersonThroughUI(p, () -> Optional.of(editedPerson)); - }); - } + modelManager.retagPersonsThroughUI(selectedPersons, () -> mainController.getPersonsTagsInput(selectedPersons)); } /** @@ -188,7 +177,7 @@ private void handleEditPerson() { private void handleCancelPersonOperations() { final List selectedPersons = personListView.getSelectionModel().getSelectedItems(); - if (isSelectionValid()) { + if (!isSelectionValid()) { showInvalidSelectionAlert(); return; } diff --git a/src/main/java/address/controller/RootLayoutController.java b/src/main/java/address/controller/RootLayoutController.java index 8119c3c2..593c11c7 100644 --- a/src/main/java/address/controller/RootLayoutController.java +++ b/src/main/java/address/controller/RootLayoutController.java @@ -29,13 +29,7 @@ public class RootLayoutController extends UiController{ private MainApp mainApp; @FXML - private MenuItem menuFileNew; - @FXML - private MenuItem menuFileOpen; - @FXML - private MenuItem menuFileSave; - @FXML - private MenuItem menuFileSaveAs; + private MenuItem helpMenuItem; public RootLayoutController() { super(); @@ -47,83 +41,14 @@ public void setConnections(MainApp mainApp, MainController mainController, Model this.mainApp = mainApp; } - public void setAccelerators(){ - menuFileNew.setAccelerator(KeyBindingsManager.getAcceleratorKeyCombo("FILE_NEW_ACCELERATOR").get()); - menuFileOpen.setAccelerator(KeyBindingsManager.getAcceleratorKeyCombo("FILE_OPEN_ACCELERATOR").get()); - menuFileSave.setAccelerator(KeyBindingsManager.getAcceleratorKeyCombo("FILE_SAVE_ACCELERATOR").get()); - menuFileSaveAs.setAccelerator(KeyBindingsManager.getAcceleratorKeyCombo("FILE_SAVE_AS_ACCELERATOR").get()); - } - - /** - * @return a file chooser for choosing xml files. The initial folder is set to the same folder that the - * current data file is located (if any). - */ - private FileChooser getXmlFileChooser() { - // Set extension filter - final FileChooser.ExtensionFilter extFilter = new FileChooser.ExtensionFilter("XML files (*.xml)", "*.xml"); - final FileChooser fileChooser = new FileChooser(); - - fileChooser.getExtensionFilters().add(extFilter); - final File currentFile = modelManager.getPrefs().getSaveLocation(); - fileChooser.setInitialDirectory(currentFile.getParentFile()); - return fileChooser; + public void setAccelerators() { + helpMenuItem.setAccelerator(KeyBindingsManager.getAcceleratorKeyCombo("HELP_PAGE_ACCELERATOR").get()); } - - /** - * Creates a new empty address book - */ @FXML - private void handleNew() { - logger.debug("Wiping current model data."); - modelManager.clearPrefsSaveLocation(); // so as not to overwrite previous addressbook file - modelManager.clearModel(); - } - - /** - * Opens a FileChooser to let the user select an address book to load. - */ - @FXML - private void handleOpen() { - logger.debug("Prompting file dialog for data source."); - // Show open file dialog - File toOpen = getXmlFileChooser().showOpenDialog(mainController.getPrimaryStage()); - if (toOpen == null) return; - modelManager.setPrefsSaveLocation(toOpen.getPath()); - raise(new LoadDataRequestEvent(toOpen)); - } - - /** - * Saves the file to the person file that is currently open. If there is no - * open file, the "save as" dialog is shown. - */ - @FXML - private void handleSave() { - final File saveFile = modelManager.getPrefs().getSaveLocation(); - logger.debug("Requesting save to: {}.", saveFile); - raise(new SaveDataRequestEvent(saveFile, modelManager)); - } - - /** - * Opens a FileChooser to let the user select a file to save to. - */ - @FXML - private void handleSaveAs() { - logger.debug("Prompting file dialog for save destination."); - // Show save file dialog - File file = getXmlFileChooser().showSaveDialog(mainController.getPrimaryStage()); - - if (file == null) return; - - // Make sure it has the correct extension - if (!file.getPath().endsWith(".xml")) { - file = new File(file.getPath() + ".xml"); - } - - modelManager.setPrefsSaveLocation(file.getPath()); - // TODO temp patch to create cloud file if non found - raise(new CreateAddressBookOnRemoteRequestEvent(new CompletableFuture<>(), file.getName())); - raise(new SaveDataRequestEvent(file, modelManager)); + private void handleHelp() { + logger.debug("Showing help page about the application."); + mainController.showHelpPage(); } /** diff --git a/src/main/java/address/controller/StatusBarFooterController.java b/src/main/java/address/controller/StatusBarFooterController.java index eaea62a5..20666d66 100644 --- a/src/main/java/address/controller/StatusBarFooterController.java +++ b/src/main/java/address/controller/StatusBarFooterController.java @@ -1,7 +1,6 @@ package address.controller; import address.events.*; -import address.model.UserPrefs; import address.util.*; import com.google.common.eventbus.Subscribe; import javafx.application.Platform; @@ -16,16 +15,13 @@ import java.util.concurrent.*; public class StatusBarFooterController extends UiController{ + private static final String ADDRESS_BOOK_LABEL_PREFIX = "Address Book: "; @FXML private AnchorPane updaterStatusBarPane; - @FXML private AnchorPane syncStatusBarPane; - private static final String SAVE_LOC_TEXT_PREFIX = "Save File: "; - private static final String LOC_TEXT_NOT_SET = "[NOT SET]"; - public static StatusBar syncStatusBar; public static StatusBar updaterStatusBar; @@ -58,16 +54,16 @@ private void syncSchedulerOnTimeOut() { /** * Initializes the status bar * @param updateInterval The sync period - * @param saveLocation The location to save the file for storing Addressbook contacts. + * @param addressBookName name of the active address book */ - public void init(long updateInterval, File saveLocation) { + public void init(long updateInterval, String addressBookName) { initSyncTimer(updateInterval); initStatusBar(); - initSaveLocationLabel(saveLocation); + initAddressBookLabel(addressBookName); } - private void initSaveLocationLabel(File saveLocation) { - updateSaveLocationDisplay(saveLocation); + private void initAddressBookLabel(String addressBookName) { + updateSaveLocationDisplay(addressBookName); secondaryStatusBarLabel.setTextAlignment(TextAlignment.LEFT); setTooltip(secondaryStatusBarLabel); } @@ -151,12 +147,7 @@ public void handleUpdaterFailedEvent(UpdaterFailedEvent ufe) { }); } - @Subscribe - private void handleSaveLocationChangedEvent(SaveLocationChangedEvent e) { - updateSaveLocationDisplay(e.saveFile); - } - - private void updateSaveLocationDisplay(File saveFile) { - secondaryStatusBarLabel.setText(SAVE_LOC_TEXT_PREFIX + ((saveFile != null) ? saveFile.getName() : LOC_TEXT_NOT_SET)); + private void updateSaveLocationDisplay(String addressBookName) { + secondaryStatusBarLabel.setText(ADDRESS_BOOK_LABEL_PREFIX + addressBookName); } } diff --git a/src/main/java/address/controller/StatusBarHeaderController.java b/src/main/java/address/controller/StatusBarHeaderController.java index d0dc19f4..b4e7cffc 100644 --- a/src/main/java/address/controller/StatusBarHeaderController.java +++ b/src/main/java/address/controller/StatusBarHeaderController.java @@ -9,12 +9,15 @@ public class StatusBarHeaderController extends UiController{ private StatusBar headerStatusBar; + private MainController mainController; - public StatusBarHeaderController() { + public StatusBarHeaderController(MainController mainController) { + this.mainController = mainController; headerStatusBar = new StatusBar(); headerStatusBar.getStyleClass().removeAll(); headerStatusBar.getStyleClass().add("status-bar-with-border"); headerStatusBar.setText(""); + headerStatusBar.setOnMouseClicked(event -> mainController.showActivityHistoryDialog()); } public StatusBar getHeaderStatusBarView() { diff --git a/src/main/java/address/controller/TagListController.java b/src/main/java/address/controller/TagListController.java index 707e16f6..5f1179a3 100644 --- a/src/main/java/address/controller/TagListController.java +++ b/src/main/java/address/controller/TagListController.java @@ -27,7 +27,10 @@ public class TagListController extends UiController{ public void setStage(Stage stage) { stage.getScene().setOnKeyPressed(e -> { - if (e.getCode() == KeyCode.ESCAPE) stage.close(); + if (e.getCode() == KeyCode.ESCAPE) { + e.consume(); + stage.close(); + } }); this.stage = stage; } diff --git a/src/main/java/address/events/BaseEvent.java b/src/main/java/address/events/BaseEvent.java index dc0c4684..3774235f 100644 --- a/src/main/java/address/events/BaseEvent.java +++ b/src/main/java/address/events/BaseEvent.java @@ -15,7 +15,7 @@ public abstract class BaseEvent { * even if they are two different event objects. This is a weaker form of equality. * e.g. if both events intended the selection to jump to the 1st element in the list.
* The default implementation returns true of both objects are of the same class. - * For example, comparing two distinct {@link MaximizeAppRequestEvent} will return + * For example, comparing two distinct {@link ResizeAppRequestEvent} will return * true (because both intend to maximize the app).
* If the events are parameterized, e.g. {@link JumpToListRequestEvent} this method * should be overriden to compare the parameters. diff --git a/src/main/java/address/events/KeyBindingEvent.java b/src/main/java/address/events/KeyBindingEvent.java index 1d248d38..7be1b746 100644 --- a/src/main/java/address/events/KeyBindingEvent.java +++ b/src/main/java/address/events/KeyBindingEvent.java @@ -5,9 +5,6 @@ import java.util.Optional; -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.NANOSECONDS; - /** * Indicates a key event occurred that is potentially a key binding being used */ @@ -33,20 +30,6 @@ public KeyBindingEvent(KeyCombination keyCombination){ this.keyCombination = Optional.of(keyCombination); } - /** - * Returns the elapsed time between the given two events. - * @param firstEvent - * @param secondEvent - * @return elapsed time in milli seconds. - */ - public static long elapsedTimeInMilliseconds(KeyBindingEvent firstEvent, - KeyBindingEvent secondEvent){ - long durationInNanoSeconds = secondEvent.time - firstEvent.time; - long elapsedTimeInMilliseconds = MILLISECONDS.convert(durationInNanoSeconds, NANOSECONDS); - assert elapsedTimeInMilliseconds >= 0; - return elapsedTimeInMilliseconds; - } - /** * @param potentialMatch * @return true if the key combination matches this key binding diff --git a/src/main/java/address/events/MaximizeAppRequestEvent.java b/src/main/java/address/events/MaximizeAppRequestEvent.java deleted file mode 100644 index 534e746b..00000000 --- a/src/main/java/address/events/MaximizeAppRequestEvent.java +++ /dev/null @@ -1,11 +0,0 @@ -package address.events; - -/** - * Represents a user request to maximize the app window - */ -public class MaximizeAppRequestEvent extends BaseEvent { - @Override - public String toString() { - return "Request to maximize app window"; - } -} diff --git a/src/main/java/address/events/ResizeAppRequestEvent.java b/src/main/java/address/events/ResizeAppRequestEvent.java new file mode 100644 index 00000000..ae9d0074 --- /dev/null +++ b/src/main/java/address/events/ResizeAppRequestEvent.java @@ -0,0 +1,11 @@ +package address.events; + +/** + * Represents a user request to resize the app window (toggles between default size and max size) + */ +public class ResizeAppRequestEvent extends BaseEvent { + @Override + public String toString() { + return "Request to resize app window"; + } +} diff --git a/src/main/java/address/events/SyncCompletedEvent.java b/src/main/java/address/events/SyncCompletedEvent.java index 7a015ae8..cd8f829b 100644 --- a/src/main/java/address/events/SyncCompletedEvent.java +++ b/src/main/java/address/events/SyncCompletedEvent.java @@ -4,6 +4,7 @@ import address.model.datatypes.tag.Tag; import java.util.List; +import java.util.Optional; /** * An event triggered when Syncing (down) is completed. @@ -12,14 +13,14 @@ */ public class SyncCompletedEvent extends BaseEvent { List updatedPersons; - List latestTags; + Optional> latestTags; - public SyncCompletedEvent(List updatedPersons, List latestTags) { + public SyncCompletedEvent(List updatedPersons, Optional> latestTags) { this.updatedPersons = updatedPersons; this.latestTags = latestTags; } - public List getLatestTags() { + public Optional> getLatestTags() { return latestTags; } @@ -29,6 +30,9 @@ public List getUpdatedPersons() { @Override public String toString() { - return updatedPersons.size() + " updatedPersons and " + latestTags.size() + " latest tags."; + String stringToReturn = updatedPersons.size() + " updatedPersons"; + stringToReturn += latestTags.isPresent() ? " and " + latestTags.get().size() + " latest tags" + : " and no updates to tags"; + return stringToReturn; } } diff --git a/src/main/java/address/keybindings/Accelerator.java b/src/main/java/address/keybindings/Accelerator.java index 0d92d722..578d6ae9 100644 --- a/src/main/java/address/keybindings/Accelerator.java +++ b/src/main/java/address/keybindings/Accelerator.java @@ -10,8 +10,7 @@ */ public class Accelerator extends KeyBinding{ - - Accelerator(String name, KeyCombination keyCombination) { + public Accelerator(String name, KeyCombination keyCombination) { super(name, keyCombination, new AcceleratorIgnoredEvent(name)); } diff --git a/src/main/java/address/keybindings/Bindings.java b/src/main/java/address/keybindings/Bindings.java index 3fce852d..434c6e3c 100644 --- a/src/main/java/address/keybindings/Bindings.java +++ b/src/main/java/address/keybindings/Bindings.java @@ -17,29 +17,25 @@ public class Bindings { * List of accelerators used. * They are here for the purpose of record keeping. Handled automatically by JavaFX. */ - private List accelerators = new ArrayList<>(); + protected List accelerators = new ArrayList<>(); /** * List of global hotkeys. */ - private List hotkeys = new ArrayList<>(); + protected List hotkeys = new ArrayList<>(); /** List of key sequences */ - private List sequences = new ArrayList<>(); + protected List sequences = new ArrayList<>(); /** * List of keyboard shortcuts. */ - private List shortcuts = new ArrayList<>(); + protected List shortcuts = new ArrayList<>(); /* key bindings in alphabetical order of name */ - public List APP_MAXIMIZE_HOTKEY = new ArrayList<>(); + public List APP_RESIZE_HOTKEY = new ArrayList<>(); public List APP_MINIMIZE_HOTKEY = new ArrayList<>(); - public Accelerator FILE_NEW_ACCELERATOR; - public Accelerator FILE_OPEN_ACCELERATOR; - public Accelerator FILE_SAVE_ACCELERATOR; - public Accelerator FILE_SAVE_AS_ACCELERATOR; public KeySequence LIST_GOTO_TOP_SEQUENCE; public KeySequence LIST_GOTO_BOTTOM_SEQUENCE; public Shortcut LIST_ENTER_SHORTCUT; @@ -47,13 +43,14 @@ public class Bindings { public Accelerator PERSON_DELETE_ACCELERATOR; public Accelerator PERSON_EDIT_ACCELERATOR; public Accelerator PERSON_TAG_ACCELERATOR; + public Accelerator HELP_PAGE_ACCELERATOR; - public Bindings(){ + public Bindings() { init(); } - private void init(){ + private void init() { /*====== bindings A-Z keys (in alphabetical order of main key =====================*/ @@ -62,15 +59,12 @@ private void init(){ PERSON_EDIT_ACCELERATOR = setAccelerator("PERSON_EDIT_ACCELERATOR", "E"); LIST_GOTO_BOTTOM_SEQUENCE = setSequence("LIST_GOTO_BOTTOM_SEQUENCE", "G", "B", new JumpToListRequestEvent(-1)); LIST_GOTO_TOP_SEQUENCE = setSequence("LIST_GOTO_TOP_SEQUENCE", "G", "T", new JumpToListRequestEvent(1)); - FILE_NEW_ACCELERATOR = setAccelerator("FILE_NEW_ACCELERATOR", "SHORTCUT + N"); - FILE_OPEN_ACCELERATOR = setAccelerator("FILE_OPEN_ACCELERATOR", "SHORTCUT + O"); - FILE_SAVE_ACCELERATOR = setAccelerator("FILE_SAVE_ACCELERATOR", "SHORTCUT + S"); - FILE_SAVE_AS_ACCELERATOR = setAccelerator("FILE_SAVE_AS_ACCELERATOR", "SHORTCUT + ALT + S"); APP_MINIMIZE_HOTKEY.add(setHotkey("APP_MINIMIZE_HOTKEY", "CTRL + ALT + X", new MinimizeAppRequestEvent())); APP_MINIMIZE_HOTKEY.add(setHotkey("APP_MINIMIZE_HOTKEY", "META + ALT + X", new MinimizeAppRequestEvent())); - APP_MAXIMIZE_HOTKEY.add(setHotkey("APP_MAXIMIZE_HOTKEY", "CTRL + SHIFT + X", new MaximizeAppRequestEvent())); - APP_MAXIMIZE_HOTKEY.add(setHotkey("APP_MAXIMIZE_HOTKEY", "META + SHIFT + X", new MaximizeAppRequestEvent())); + APP_RESIZE_HOTKEY.add(setHotkey("APP_RESIZE_HOTKEY", "CTRL + SHIFT + X", new ResizeAppRequestEvent())); + APP_RESIZE_HOTKEY.add(setHotkey("APP_RESIZE_HOTKEY", "META + SHIFT + X", new ResizeAppRequestEvent())); PERSON_CHANGE_CANCEL_ACCELERATOR = setAccelerator("PERSON_CHANGE_CANCEL_ACCELERATOR", "SHORTCUT + Z"); + HELP_PAGE_ACCELERATOR = setAccelerator("HELP_PAGE_ACCELERATOR", "F1"); /*====== other keys ======================================================*/ @@ -166,19 +160,16 @@ private Optional findMatchingHotkey(KeyBindingEvent keyboardShortc /** * Returns the matching key sequence, if any - * @param currentEvent * @param previousEvent + * @param currentEvent */ - protected Optional findMatchingSequence(KeyBindingEvent currentEvent, - KeyBindingEvent previousEvent) { + protected Optional findMatchingSequence(KeyBindingEvent previousEvent, KeyBindingEvent currentEvent) { if (previousEvent == null){ return Optional.empty(); } - long elapsedTime = KeyBindingEvent.elapsedTimeInMilliseconds(previousEvent, currentEvent); - - if (elapsedTime > KeySequence.KEY_SEQUENCE_MAX_DELAY_BETWEEN_KEYS){ + if(!KeySequence.isElapsedTimePermissibile(previousEvent.time, currentEvent.time)){ return Optional.empty(); } @@ -193,11 +184,11 @@ protected Optional findMatchingSequence(KeyBindingEvent currentEven * @param previous the previous key event (this is needed to match for key sequences) * @return the matching key binding, if any */ - public Optional getBinding(KeyBindingEvent current, - KeyBindingEvent previous){ + public Optional getBinding(KeyBindingEvent previous, + KeyBindingEvent current){ Optional matchingBinding; - matchingBinding = findMatchingSequence(current, previous); + matchingBinding = findMatchingSequence(previous, current); if (matchingBinding.isPresent()) { return matchingBinding; } matchingBinding = findMatchingHotkey(current); diff --git a/src/main/java/address/keybindings/GlobalHotkeyProvider.java b/src/main/java/address/keybindings/GlobalHotkeyProvider.java index 1178f6b7..c58a02d5 100644 --- a/src/main/java/address/keybindings/GlobalHotkeyProvider.java +++ b/src/main/java/address/keybindings/GlobalHotkeyProvider.java @@ -15,7 +15,7 @@ */ public class GlobalHotkeyProvider { /** Provider for global hotkeys */ - private final Provider provider = Provider.getCurrentProvider(false); + protected Provider provider = Provider.getCurrentProvider(false); private final AppLogger logger; private EventManager eventManager; diff --git a/src/main/java/address/keybindings/KeyBinding.java b/src/main/java/address/keybindings/KeyBinding.java index 29cf8c58..a8901ca2 100644 --- a/src/main/java/address/keybindings/KeyBinding.java +++ b/src/main/java/address/keybindings/KeyBinding.java @@ -6,12 +6,16 @@ /** * Parent class for different key binding types. */ -public class KeyBinding { +public abstract class KeyBinding { protected String name; protected KeyCombination keyCombination; protected BaseEvent eventToRaise; protected KeyBinding (String name, KeyCombination keyCombination, BaseEvent eventToRaise) { + assert name != null : "name cannot be null"; + assert keyCombination != null : "key combination cannot be null"; + assert eventToRaise != null : "event cannot be null"; + this.name = name; this.keyCombination = keyCombination; this.eventToRaise = eventToRaise; diff --git a/src/main/java/address/keybindings/KeyBindingsManager.java b/src/main/java/address/keybindings/KeyBindingsManager.java index 24c8be43..bd12259e 100644 --- a/src/main/java/address/keybindings/KeyBindingsManager.java +++ b/src/main/java/address/keybindings/KeyBindingsManager.java @@ -1,6 +1,5 @@ package address.keybindings; -import address.events.AcceleratorIgnoredEvent; import address.events.BaseEvent; import address.events.KeyBindingEvent; import address.main.ComponentManager; @@ -9,18 +8,17 @@ import com.google.common.eventbus.Subscribe; import javafx.scene.input.KeyCombination; -import java.util.List; import java.util.Optional; /** * Manages key bindings. */ -public class KeyBindingsManager extends ComponentManager{ +public class KeyBindingsManager extends ComponentManager { private static final AppLogger logger = LoggerManager.getLogger(KeyBindingsManager.class); /** Manages global hotkey detection */ - private GlobalHotkeyProvider hotkeyProvider = new GlobalHotkeyProvider(eventManager, logger); + protected GlobalHotkeyProvider hotkeyProvider = new GlobalHotkeyProvider(eventManager, logger); /** To keep track of the previous keyboard event, to match for key sequences */ private KeyBindingEvent previousKeyEvent = null; @@ -39,7 +37,7 @@ public KeyBindingsManager() { @Subscribe public void handleKeyBindingEvent(KeyBindingEvent currentKeyEvent) { - Optional kb = BINDINGS.getBinding(currentKeyEvent, previousKeyEvent); + Optional kb = BINDINGS.getBinding(previousKeyEvent, currentKeyEvent); previousKeyEvent = currentKeyEvent; if (!kb.isPresent()) { @@ -50,7 +48,6 @@ public void handleKeyBindingEvent(KeyBindingEvent currentKeyEvent) { logger.info("Handling {}", kb.get()); BaseEvent event = kb.get().getEventToRaise(); raise (event); - } /** @@ -60,8 +57,6 @@ public void stop() { hotkeyProvider.clear(); } - - /** * Returns the key combination of the accelerator matching the name given. */ @@ -73,5 +68,4 @@ public static Optional getAcceleratorKeyCombo(String name) { return keyBinding.isPresent() ? Optional.of(keyBinding.get().getKeyCombination()) : Optional.empty(); } - } diff --git a/src/main/java/address/keybindings/KeySequence.java b/src/main/java/address/keybindings/KeySequence.java index ff09c9c6..2fb3816b 100644 --- a/src/main/java/address/keybindings/KeySequence.java +++ b/src/main/java/address/keybindings/KeySequence.java @@ -3,24 +3,29 @@ import address.events.BaseEvent; import javafx.scene.input.KeyCombination; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.concurrent.TimeUnit.NANOSECONDS; + /** * Represents a shortcut that is a key sequence e.g. G followed by P */ public class KeySequence extends KeyBinding{ /** Max delay (in milliseconds) allowed between key presses of a key sequence */ - public static final int KEY_SEQUENCE_MAX_DELAY_BETWEEN_KEYS = 1000; + protected static final int KEY_SEQUENCE_MAX_MILLISECONDS_BETWEEN_KEYS = 1000; protected KeyCombination secondKeyCombination; + @Deprecated public KeySequence(String name, KeyCombination firstKeyCombination, BaseEvent eventToRaise) { super(name, firstKeyCombination, eventToRaise); assert false : "Invalid constructor called"; } - public KeySequence(String name, KeyCombination firstKeyCombination, KeyCombination secondKeyCombination, - BaseEvent eventToRaise) { + public KeySequence(String name, KeyCombination firstKeyCombination, + KeyCombination secondKeyCombination, BaseEvent eventToRaise) { super(name, firstKeyCombination, eventToRaise); + assert secondKeyCombination != null: "Second key combination cannot be null"; this.secondKeyCombination = secondKeyCombination; } @@ -38,8 +43,30 @@ public String toString(){ * in this key sequence. */ public boolean isIncluded(KeyCombination otherKeyCombination) { + + if (otherKeyCombination == null) {return false; } + return keyCombination.toString().equals(otherKeyCombination.toString()) || secondKeyCombination.toString().equals(otherKeyCombination.toString()); + //TODO: make the comparison smarter so that it can detect a match between CTRL+X and SHORTCUT+X } + + /** + * Returns true if {@code (timeOfSecondKeyEvent - timeOfFirstKeyEvent)} is within the permissible delay between + * two key events for them to be considered a key sequence.
+ * Assumption: {@code timeOfFirstKeyEvent =< timeOfSecondKeyEvent} + * @param timeOfFirstKeyEvent {@code System.nanoTime() when the first key event occurred} + * @param timeOfSecondKeyEvent {@code System.nanoTime() when the second key event occurred} + */ + public static boolean isElapsedTimePermissibile(long timeOfFirstKeyEvent, long timeOfSecondKeyEvent) { + assert timeOfFirstKeyEvent >= 0 : "times cannot be negative"; + assert timeOfSecondKeyEvent >= 0 : "times cannot be negative"; + + long durationInNanoSeconds = timeOfSecondKeyEvent - timeOfFirstKeyEvent; + assert durationInNanoSeconds >= 0 : "second key event cannot happen before the first one"; + + return durationInNanoSeconds <= NANOSECONDS.convert(KEY_SEQUENCE_MAX_MILLISECONDS_BETWEEN_KEYS, MILLISECONDS); + } + } diff --git a/src/main/java/address/keybindings/Shortcut.java b/src/main/java/address/keybindings/Shortcut.java index 39a51ab6..53495e12 100644 --- a/src/main/java/address/keybindings/Shortcut.java +++ b/src/main/java/address/keybindings/Shortcut.java @@ -8,15 +8,10 @@ */ public class Shortcut extends KeyBinding{ - Shortcut(String name, KeyCombination keyCombination, BaseEvent eventToRaise) { + public Shortcut(String name, KeyCombination keyCombination, BaseEvent eventToRaise) { super(name, keyCombination, eventToRaise); } - public KeyCombination getKeyCombination() { - return keyCombination; - } - - @Override public String toString(){ return "Keyboard shortcut " + getDisplayText(); diff --git a/src/main/java/address/main/ComponentManager.java b/src/main/java/address/main/ComponentManager.java index 5cfbaeb5..5608ca42 100644 --- a/src/main/java/address/main/ComponentManager.java +++ b/src/main/java/address/main/ComponentManager.java @@ -8,7 +8,7 @@ * * Registers the class' event handlers in eventManager */ -public class ComponentManager { +public abstract class ComponentManager { protected EventManager eventManager; /** diff --git a/src/main/java/address/model/AddPersonCommand.java b/src/main/java/address/model/AddPersonCommand.java index 4a11a7db..de70db14 100644 --- a/src/main/java/address/model/AddPersonCommand.java +++ b/src/main/java/address/model/AddPersonCommand.java @@ -1,6 +1,8 @@ package address.model; import static address.model.ChangeObjectInModelCommand.State.*; +import static address.model.datatypes.person.ReadOnlyViewablePerson.ChangeInProgress.*; + import address.events.BaseEvent; import address.events.CreatePersonOnRemoteRequestEvent; import address.model.datatypes.person.Person; @@ -34,14 +36,23 @@ public class AddPersonCommand extends ChangePersonInModelCommand { * @param inputRetriever Will run on execution {@link #run()} thread. This should handle thread concurrency * logic (eg. {@link PlatformExecUtil#call(Callable)} within itself. * If the returned Optional is empty, the command will be cancelled. - * @see super#ChangePersonInModelCommand(Supplier, int) + * @see super#ChangePersonInModelCommand(int, Supplier, int) */ - protected AddPersonCommand(Supplier> inputRetriever, int gracePeriodDurationInSeconds, - Consumer eventRaiser, ModelManager model) { - super(inputRetriever, gracePeriodDurationInSeconds); + public AddPersonCommand(int commandId, Supplier> inputRetriever, int gracePeriodDurationInSeconds, + Consumer eventRaiser, ModelManager model, String addressbookName) { + super(commandId, inputRetriever, gracePeriodDurationInSeconds); this.model = model; this.eventRaiser = eventRaiser; - this.addressbookName = model.getPrefs().getSaveLocation().getName(); + this.addressbookName = addressbookName; + } + + protected ViewablePerson getViewableToAdd() { + return viewableToAdd; + } + + @Override + public String getName() { + return "Add Person " + (viewableToAdd == null ? "" : viewableToAdd.idString()); } @Override @@ -60,8 +71,10 @@ protected void before() { @Override protected void after() { if (viewableToAdd != null) { + viewableToAdd.setChangeInProgress(NONE); model.unassignOngoingChangeForPerson(viewableToAdd.getId()); } + model.trackFinishedCommand(this); } /** @@ -72,25 +85,20 @@ protected void after() { protected State simulateResult() { assert input != null; // create VP and add to model - viewableToAdd = ViewablePerson.withoutBacking(new Person(input)); - logger.debug("simulateResult: Going to add " + viewableToAdd.toString()); PlatformExecUtil.runAndWait(() -> { - model.addViewablePerson(viewableToAdd); - logger.debug("simulateResult: Added " + viewableToAdd.toString() + " to visible person list in model"); - model.assignOngoingChangeToPerson(viewableToAdd.getId(), this); + viewableToAdd = model.addViewablePersonWithoutBacking(input); + viewableToAdd.setChangeInProgress(ADDING); }); + logger.debug("simulateResult: Going to add " + viewableToAdd.toString()); + model.assignOngoingChangeToPerson(viewableToAdd.getId(), this); + logger.debug("simulateResult: Added " + viewableToAdd.toString() + " to visible person list in model"); return GRACE_PERIOD; } - @Override - protected void beforeGracePeriod() { - // nothing needed for now - } - @Override protected void handleChangeToSecondsLeftInGracePeriod(int secondsLeft) { assert viewableToAdd != null; - PlatformExecUtil.runLater(() -> viewableToAdd.setSecondsLeftInPendingState(secondsLeft)); + PlatformExecUtil.runAndWait(() -> viewableToAdd.setSecondsLeftInPendingState(secondsLeft)); } @Override @@ -110,11 +118,6 @@ protected State handleDeleteInGracePeriod() { return CANCELLED; } - @Override - protected State handleCancelInGracePeriod() { - return CANCELLED; - } - @Override protected Optional getRemoteConflict() { return Optional.empty(); // no possible conflict for add command @@ -138,16 +141,16 @@ protected State requestChangeToRemote() { logger.debug("requestChangeToRemote: Removing mapping for old id:" + getTargetPersonId()); model.unassignOngoingChangeForPerson(getTargetPersonId()); // removes mapping for old id - model.assignOngoingChangeToPerson(backingPerson.getId(), this); // remap this change for the new id PlatformExecUtil.runAndWait(() -> { model.addPersonToBackingModelSilently(backingPerson); // so it wont trigger creation of another VP viewableToAdd.connectBackingObject(backingPerson); // changes id to that of backing person + model.assignOngoingChangeToPerson(backingPerson.getId(), this); // remap this change for the new id }); logger.debug("requestChangeToRemote -> id of viewable person updated to " + viewableToAdd.getId()); return SUCCESSFUL; } catch (ExecutionException | InterruptedException e) { - return CANCELLED; // figure out a policy for syncup fail + return FAILED; } } @@ -165,7 +168,7 @@ protected void finishWithSuccess() { @Override protected void finishWithFailure() { - // no way to fail for now + finishWithCancel(); // TODO figure out failure handling } } diff --git a/src/main/java/address/model/ChangeObjectInModelCommand.java b/src/main/java/address/model/ChangeObjectInModelCommand.java index 14bd7a3e..568d1d2e 100644 --- a/src/main/java/address/model/ChangeObjectInModelCommand.java +++ b/src/main/java/address/model/ChangeObjectInModelCommand.java @@ -19,25 +19,40 @@ * * Should be run OUTSIDE THE FX THREAD because the {@link #run()} method involves blocking calls. */ -public abstract class ChangeObjectInModelCommand implements Runnable { +public abstract class ChangeObjectInModelCommand implements Runnable, CommandInfo { public enum State { // Initial state - NEWLY_CREATED, + NEWLY_CREATED ("Newly Created"), // Intermediate states - RETRIEVING_INPUT, - SIMULATING_RESULT, - GRACE_PERIOD, - CHECKING_AND_RESOLVING_REMOTE_CONFLICT, - REQUESTING_REMOTE_CHANGE, + RETRIEVING_INPUT ("Retrieving Input"), + SIMULATING_RESULT ("Optimistically Simulating Result"), + GRACE_PERIOD ("Pending / Grace Period"), + CHECKING_AND_RESOLVING_REMOTE_CONFLICT ("Handling any Remote Conflicts"), + REQUESTING_REMOTE_CHANGE ("Request to Remote"), // Terminal states - CANCELLED, SUCCESSFUL, FAILED, + CANCELLED ("Cancelled"), + SUCCESSFUL ("Successful"), + FAILED ("Failed"); + + private final String msg; + + State(String msg) { + this.msg = msg; + } + + @Override + public String toString() { + return msg; + } } private static final AppLogger logger = LoggerManager.getLogger(ChangeObjectInModelCommand.class); + private final int commandId; + private final CountDownLatch completionLatch; protected final int gracePeriodDurationInSeconds; protected final ObjectProperty state; // current state @@ -51,10 +66,16 @@ public enum State { clearGracePeriodOverride(); } - protected ChangeObjectInModelCommand(int gracePeriodDurationInSeconds) { + protected ChangeObjectInModelCommand(int commandId, int gracePeriodDurationInSeconds) { + this.commandId = commandId; this.gracePeriodDurationInSeconds = gracePeriodDurationInSeconds; } + @Override + public int getCommandId() { + return commandId; + } + public void waitForCompletion() throws InterruptedException { completionLatch.await(); } @@ -63,12 +84,13 @@ public State getState() { return state.getValue(); } - void setState(State newState) { - state.setValue(newState); + @Override + public String statusString() { + return getState().toString(); } - public ReadOnlyObjectProperty stateProperty() { - return state; + void setState(State newState) { + state.setValue(newState); } /** @@ -228,8 +250,6 @@ private State handleAndTransitionState(State state) { private State countdownGracePeriodAndHandleOverrides() { beforeGracePeriod(); - // Ensure that any override signals detected happen during the current grace period. - clearGracePeriodOverride(); // Countdown loop that listens for any overriding signals for (int i = gracePeriodDurationInSeconds; i > 0; i--) { diff --git a/src/main/java/address/model/ChangePersonInModelCommand.java b/src/main/java/address/model/ChangePersonInModelCommand.java index b94c6623..1fa76b97 100644 --- a/src/main/java/address/model/ChangePersonInModelCommand.java +++ b/src/main/java/address/model/ChangePersonInModelCommand.java @@ -29,9 +29,9 @@ public abstract class ChangePersonInModelCommand extends ChangeObjectInModelComm * logic (eg. {@link PlatformExecUtil#call(Callable)} within itself. * If the returned Optional is empty, the command will be cancelled. */ - protected ChangePersonInModelCommand(Supplier> inputRetriever, + protected ChangePersonInModelCommand(int commandId, Supplier> inputRetriever, int gracePeriodDurationInSeconds) { - super(gracePeriodDurationInSeconds); + super(commandId, gracePeriodDurationInSeconds); this.inputRetriever = inputRetriever; } @@ -52,6 +52,12 @@ protected State retrieveInput() { return CANCELLED; } + @Override + protected void beforeGracePeriod() { + // Ensure that any override signals detected happen during the current grace period. + clearGracePeriodOverride(); + } + /** * Request to override this command with an edit command using {@code newInputSupplier} to supply input. * Only works if the command is currently in the {@link State#GRACE_PERIOD} state. @@ -97,6 +103,11 @@ synchronized void deleteInGracePeriod() { */ protected abstract State handleDeleteInGracePeriod(); + @Override + protected State handleCancelInGracePeriod() { + return CANCELLED; + } + @Override protected State checkAndHandleRemoteConflict() { final Optional conflict = getRemoteConflict(); diff --git a/src/main/java/address/model/CommandInfo.java b/src/main/java/address/model/CommandInfo.java new file mode 100644 index 00000000..3948f742 --- /dev/null +++ b/src/main/java/address/model/CommandInfo.java @@ -0,0 +1,7 @@ +package address.model; + +public interface CommandInfo { + int getCommandId(); // id == command counter (starting from 1) + String getName(); + String statusString(); +} diff --git a/src/main/java/address/model/DeletePersonCommand.java b/src/main/java/address/model/DeletePersonCommand.java index 3b858405..ba887210 100644 --- a/src/main/java/address/model/DeletePersonCommand.java +++ b/src/main/java/address/model/DeletePersonCommand.java @@ -1,6 +1,8 @@ package address.model; import static address.model.ChangeObjectInModelCommand.State.*; +import static address.model.datatypes.person.ReadOnlyViewablePerson.ChangeInProgress.*; + import address.events.BaseEvent; import address.events.DeletePersonOnRemoteRequestEvent; import address.model.datatypes.person.ReadOnlyPerson; @@ -8,7 +10,6 @@ import address.util.PlatformExecUtil; import java.util.Optional; -import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.function.Consumer; @@ -26,16 +27,20 @@ public class DeletePersonCommand extends ChangePersonInModelCommand { private final String addressbookName; /** - * @see super#ChangePersonInModelCommand(Supplier, int) + * @see super#ChangePersonInModelCommand(int, Supplier, int) */ - protected DeletePersonCommand(ViewablePerson target, int gracePeriodDurationInSeconds, - Consumer eventRaiser, ModelManager model) { + public DeletePersonCommand(int commandId, ViewablePerson target, int gracePeriodDurationInSeconds, + Consumer eventRaiser, ModelManager model, String addressbookName) { // no input needed for delete commands - super(() -> Optional.of(target), gracePeriodDurationInSeconds); + super(commandId, () -> Optional.of(target), gracePeriodDurationInSeconds); this.target = target; this.model = model; this.eventRaiser = eventRaiser; - this.addressbookName = model.getPrefs().getSaveLocation().getName(); + this.addressbookName = addressbookName; + } + + protected ViewablePerson getViewable() { + return target; } @Override @@ -43,6 +48,11 @@ public int getTargetPersonId() { return target.getId(); } + @Override + public String getName() { + return "Delete Person " + target.idString(); + } + @Override protected void before() { if (model.personHasOngoingChange(target)) { @@ -52,6 +62,7 @@ protected void before() { e.printStackTrace(); } } + PlatformExecUtil.runAndWait(() -> target.setChangeInProgress(DELETING)); model.assignOngoingChangeToPerson(target.getId(), this); target.stopSyncingWithBackingObject(); } @@ -59,28 +70,23 @@ protected void before() { @Override protected void after() { PlatformExecUtil.runAndWait(() -> { + target.setChangeInProgress(NONE); target.continueSyncingWithBackingObject(); target.forceSyncFromBacking(); - target.setIsDeleted(false); }); model.unassignOngoingChangeForPerson(target.getId()); + model.trackFinishedCommand(this); } @Override protected State simulateResult() { - PlatformExecUtil.runAndWait(() -> - target.setIsDeleted(true)); + // delete changeinprogress field already set in #before return GRACE_PERIOD; } - @Override - protected void beforeGracePeriod() { - // nothing needed for now - } - @Override protected void handleChangeToSecondsLeftInGracePeriod(int secondsLeft) { - PlatformExecUtil.runLater(() -> target.setSecondsLeftInPendingState(secondsLeft)); + PlatformExecUtil.runAndWait(() -> target.setSecondsLeftInPendingState(secondsLeft)); } @Override @@ -94,11 +100,6 @@ protected State handleDeleteInGracePeriod() { return GRACE_PERIOD; // nothing to be done } - @Override - protected State handleCancelInGracePeriod() { - return CANCELLED; - } - @Override protected Optional getRemoteConflict() { return Optional.empty(); // TODO add after cloud individual check implemented @@ -120,7 +121,7 @@ protected State requestChangeToRemote() { PlatformExecUtil.runAndWait(() -> model.backingModel().removePerson(target)); return SUCCESSFUL; } catch (ExecutionException | InterruptedException e) { - return CANCELLED; // figure out a policy for syncup fail + return FAILED; } } @@ -136,6 +137,6 @@ protected void finishWithSuccess() { @Override protected void finishWithFailure() { - // Impossible for now + finishWithCancel(); // TODO figure out failure handling } } diff --git a/src/main/java/address/model/EditPersonCommand.java b/src/main/java/address/model/EditPersonCommand.java index d7539407..3fe0dd68 100644 --- a/src/main/java/address/model/EditPersonCommand.java +++ b/src/main/java/address/model/EditPersonCommand.java @@ -1,9 +1,11 @@ package address.model; import static address.model.ChangeObjectInModelCommand.State.*; +import static address.model.datatypes.person.ReadOnlyViewablePerson.ChangeInProgress.*; + import address.events.BaseEvent; -import address.events.LocalModelChangedEvent; import address.events.UpdatePersonOnRemoteRequestEvent; +import address.model.datatypes.person.Person; import address.model.datatypes.person.ReadOnlyPerson; import address.model.datatypes.person.ViewablePerson; import address.util.PlatformExecUtil; @@ -27,19 +29,24 @@ public class EditPersonCommand extends ChangePersonInModelCommand { private final String addressbookName; /** - * @param inputRetriever Will run on execution {@link #run()} thread. This should handle thread concurrency - * logic (eg. {@link PlatformExecUtil#call(Callable)} within itself. - * If the returned Optional is empty, the command will be cancelled. - * @see super#ChangePersonInModelCommand(Supplier, int) + * @param inputRetriever Will run on execution {@link #run()} thread. This should handle thread concurrency + * logic (eg. {@link PlatformExecUtil#call(Callable)} within itself. + * If the returned Optional is empty, the command will be cancelled. + * @see super#ChangePersonInModelCommand(int, Supplier, int) */ - protected EditPersonCommand(ViewablePerson target, Supplier> inputRetriever, - int gracePeriodDurationInSeconds, Consumer eventRaiser, - ModelManager model) { - super(inputRetriever, gracePeriodDurationInSeconds); + public EditPersonCommand(int commandId, ViewablePerson target, Supplier> inputRetriever, + int gracePeriodDurationInSeconds, Consumer eventRaiser, + ModelManager model, String addressbookName) { + super(commandId, inputRetriever, gracePeriodDurationInSeconds); + assert target != null; this.target = target; this.model = model; this.eventRaiser = eventRaiser; - this.addressbookName = model.getPrefs().getSaveLocation().getName(); + this.addressbookName = addressbookName; + } + + protected ViewablePerson getViewable() { + return target; } @Override @@ -47,6 +54,11 @@ public int getTargetPersonId() { return target.getId(); } + @Override + public String getName() { + return "Edit Person " + target.idString(); + } + @Override protected void before() { if (model.personHasOngoingChange(target)) { @@ -56,6 +68,7 @@ protected void before() { e.printStackTrace(); } } + PlatformExecUtil.runAndWait(() -> target.setChangeInProgress(EDITING)); model.assignOngoingChangeToPerson(target.getId(), this); target.stopSyncingWithBackingObject(); } @@ -63,31 +76,24 @@ protected void before() { @Override protected void after() { PlatformExecUtil.runAndWait(() -> { + target.setChangeInProgress(NONE); target.continueSyncingWithBackingObject(); target.forceSyncFromBacking(); - target.setIsEdited(false); }); model.unassignOngoingChangeForPerson(target.getId()); + model.trackFinishedCommand(this); } @Override protected State simulateResult() { assert input != null; - PlatformExecUtil.runAndWait(() -> { - target.setIsEdited(true); - target.simulateUpdate(input); - }); + PlatformExecUtil.runAndWait(() -> target.simulateUpdate(input)); return GRACE_PERIOD; } - @Override - protected void beforeGracePeriod() { - // nothing needed for now - } - @Override protected void handleChangeToSecondsLeftInGracePeriod(int secondsLeft) { - PlatformExecUtil.runLater(() -> target.setSecondsLeftInPendingState(secondsLeft)); + PlatformExecUtil.runAndWait(() -> target.setSecondsLeftInPendingState(secondsLeft)); } @Override @@ -107,11 +113,6 @@ protected State handleDeleteInGracePeriod() { return CANCELLED; } - @Override - protected State handleCancelInGracePeriod() { - return CANCELLED; - } - @Override protected Optional getRemoteConflict() { return Optional.empty(); // TODO add after cloud individual check implemented @@ -119,7 +120,6 @@ protected Optional getRemoteConflict() { @Override protected State resolveRemoteConflict(ReadOnlyPerson remoteVersion) { - assert false; // TODO figure out what to show to users return null; } @@ -134,7 +134,7 @@ protected State requestChangeToRemote() { PlatformExecUtil.runAndWait(() -> target.getBacking().update(input)); return SUCCESSFUL; } catch (ExecutionException | InterruptedException e) { - return CANCELLED; // figure out a policy for syncup fail + return FAILED; } } @@ -145,11 +145,11 @@ protected void finishWithCancel() { @Override protected void finishWithSuccess() { - model.raiseLocalModelChangedEvent(); + // nothing to do for now } @Override protected void finishWithFailure() { - // can't happen yet + finishWithCancel(); // TODO figure out failure handling } } diff --git a/src/main/java/address/model/ModelManager.java b/src/main/java/address/model/ModelManager.java index 70fae6a9..4aec2c17 100644 --- a/src/main/java/address/model/ModelManager.java +++ b/src/main/java/address/model/ModelManager.java @@ -7,19 +7,19 @@ import address.model.datatypes.*; import address.model.datatypes.person.*; import address.model.datatypes.tag.Tag; -import address.model.datatypes.UniqueData; -import address.util.AppLogger; -import address.util.LoggerManager; -import address.util.PlatformExecUtil; +import address.util.*; import address.util.collections.UnmodifiableObservableList; import com.google.common.eventbus.Subscribe; -import javafx.collections.ListChangeListener; +import javafx.application.Platform; +import javafx.collections.FXCollections; import javafx.collections.ObservableList; import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -32,22 +32,27 @@ public class ModelManager extends ComponentManager implements ReadOnlyAddressBoo private final AddressBook backingModel; private final ViewableAddressBook visibleModel; - private final Map personChangesInProgress; - final Executor commandExecutor; + private final Map personChangesInProgress; + private final ObservableList finishedCommands; + private final Executor commandExecutor; + private final AtomicInteger commandCounter; - private UserPrefs prefs; + private String saveFilePath; + private String addressBookNameToUse; { personChangesInProgress = new HashMap<>(); commandExecutor = Executors.newCachedThreadPool(); + finishedCommands = FXCollections.observableArrayList(); + commandCounter = new AtomicInteger(0); } /** * Initializes a ModelManager with the given AddressBook * AddressBook and its variables should not be null */ - public ModelManager(AddressBook src, UserPrefs prefs) { + public ModelManager(AddressBook src, Config config) { super(); if (src == null) { logger.fatal("Attempted to initialize with a null AddressBook"); @@ -58,23 +63,12 @@ public ModelManager(AddressBook src, UserPrefs prefs) { backingModel = new AddressBook(src); visibleModel = backingModel.createVisibleAddressBook(); - // update changes need to go through #editPersonThroughUI or #updateTag to trigger the LMCEvent - final ListChangeListener modelChangeListener = change -> { - while (change.next()) { - if (change.wasAdded() || change.wasRemoved()) { - raise(new LocalModelChangedEvent(this)); - return; - } - } - }; - backingModel.getPersons().addListener(modelChangeListener); - backingTagList().addListener(modelChangeListener); - - this.prefs = prefs; + this.saveFilePath = config.getLocalDataFilePath(); + this.addressBookNameToUse = config.getAddressBookName(); } - public ModelManager(UserPrefs prefs) { - this(new AddressBook(), prefs); + public ModelManager(Config config) { + this(new AddressBook(), config); } /** @@ -94,6 +88,10 @@ public void clearModel() { //// EXPOSING MODEL + public UnmodifiableObservableList getFinishedCommands() { + return new UnmodifiableObservableList<>(finishedCommands); + } + /** * @return all persons in visible model IN AN UNMODIFIABLE VIEW */ @@ -137,11 +135,11 @@ private ObservableList backingTagList() { return backingModel.getTags(); } - AddressBook backingModel() { + protected AddressBook backingModel() { return backingModel; } - ViewableAddressBook visibleModel() { + protected ViewableAddressBook visibleModel() { return visibleModel; } @@ -159,7 +157,7 @@ public synchronized void createPersonThroughUI(Callable } /** - * Request to updaate a person. Simulates the change optimistically until remote confirmation, and provides a grace + * Request to update a person. Simulates the change optimistically until remote confirmation, and provides a grace * period for cancellation, editing, or deleting. TODO listen on Person properties and not manually raise events * @param target The Person to be changed. * @param userInputRetriever callback to retrieve user's input. Will be run on fx application thread @@ -177,6 +175,51 @@ public synchronized void editPersonThroughUI(ReadOnlyPerson target, } } + /** + * Request to set the tags for a group of Persons. Simulates change optimistically until remote confirmation, + * and provides a grace period for cancellation, editing, or deleting. + * @param targets Persons to be retagged + * @param newTagsRetriever callback to retrieve the tags to set for every person in {@code targets}. + * Will be run on fx application thread + */ + public void retagPersonsThroughUI(Collection targets, + Callable>> newTagsRetriever) { + + final CompletableFuture>> chosenTags = new CompletableFuture<>(); + final AtomicBoolean alreadyRetrieved = new AtomicBoolean(false); + + final Function>> editInputRetrieverFactory = p -> () -> { + // run ui input retriever on this thread if no other thread did it yet + boolean shouldRunTagsRetriever = !alreadyRetrieved.getAndSet(true); + if (shouldRunTagsRetriever) { + chosenTags.complete(PlatformExecUtil.callAndWait(newTagsRetriever, Optional.empty())); + } + + try { + Optional> chosenTagsInput = chosenTags.get(); + if (!chosenTagsInput.isPresent()) { + return Optional.empty(); + } + Person afterRetag = new Person(p); + afterRetag.setTags(chosenTagsInput.get()); + return Optional.of(afterRetag); + + } catch (InterruptedException | ExecutionException e) { + e.printStackTrace(); + return Optional.empty(); + } + }; + // handle edit commands for each target + targets.forEach(target -> { + if (personHasOngoingChange(target)) { + getOngoingChangeForPerson(target.getId()).editInGracePeriod(editInputRetrieverFactory.apply(target)); + } else { + final ViewablePerson toEdit = visibleModel.findPerson(target).get(); + execNewEditPersonCommand(toEdit, editInputRetrieverFactory.apply(target)); + } + }); + } + /** * Request to delete a person. Simulates the change optimistically until remote confirmation, and provides a grace * period for cancellation, editing, or deleting. @@ -201,30 +244,30 @@ public synchronized void cancelPersonChangeCommand(ReadOnlyPerson target) { } } - void execNewAddPersonCommand(Supplier> inputRetriever) { + protected void execNewAddPersonCommand(Supplier> inputRetriever) { final int GRACE_PERIOD_DURATION = 3; - commandExecutor.execute(new AddPersonCommand(inputRetriever, GRACE_PERIOD_DURATION, this::raise, this)); + commandExecutor.execute(new AddPersonCommand(assignCommandId(), inputRetriever, GRACE_PERIOD_DURATION, this::raise, this, addressBookNameToUse)); } - void execNewEditPersonCommand(ViewablePerson target, Supplier> editInputRetriever) { + protected void execNewEditPersonCommand(ViewablePerson target, Supplier> editInputRetriever) { final int GRACE_PERIOD_DURATION = 3; - commandExecutor.execute(new EditPersonCommand(target, editInputRetriever, GRACE_PERIOD_DURATION, - this::raise, this)); + commandExecutor.execute(new EditPersonCommand(assignCommandId(), target, editInputRetriever, + GRACE_PERIOD_DURATION, this::raise, this, addressBookNameToUse)); } - void execNewDeletePersonCommand(ViewablePerson target) { + protected void execNewDeletePersonCommand(ViewablePerson target) { final int GRACE_PERIOD_DURATION = 3; - commandExecutor.execute(new DeletePersonCommand(target, GRACE_PERIOD_DURATION, this::raise, this)); + commandExecutor.execute(new DeletePersonCommand(assignCommandId(), target, GRACE_PERIOD_DURATION, this::raise, this, addressBookNameToUse)); } /** * @param changeInProgress the active change command on the person with id {@code targetPersonId} */ - synchronized void assignOngoingChangeToPerson(ReadOnlyPerson target, ChangePersonInModelCommand changeInProgress) { + protected synchronized void assignOngoingChangeToPerson(ReadOnlyPerson target, ChangePersonInModelCommand changeInProgress) { assignOngoingChangeToPerson(target.getId(), changeInProgress); } - synchronized void assignOngoingChangeToPerson(int targetId, ChangePersonInModelCommand changeInProgress) { + protected synchronized void assignOngoingChangeToPerson(int targetId, ChangePersonInModelCommand changeInProgress) { assert targetId == changeInProgress.getTargetPersonId() : "Must map to correct id"; if (personChangesInProgress.containsKey(targetId)) { throw new IllegalStateException("Only 1 ongoing change allowed per person."); @@ -236,19 +279,19 @@ synchronized void assignOngoingChangeToPerson(int targetId, ChangePersonInModelC * Removed the target person's mapped changeInProgress, freeing it for other change commands. * @return the removed change command, or null if there was no mapping found */ - synchronized ChangePersonInModelCommand unassignOngoingChangeForPerson(ReadOnlyPerson person) { + protected synchronized ChangePersonInModelCommand unassignOngoingChangeForPerson(ReadOnlyPerson person) { return personChangesInProgress.remove(person.getId()); } - synchronized ChangePersonInModelCommand unassignOngoingChangeForPerson(int targetId) { + protected synchronized ChangePersonInModelCommand unassignOngoingChangeForPerson(int targetId) { return personChangesInProgress.remove(targetId); } - synchronized ChangePersonInModelCommand getOngoingChangeForPerson(ReadOnlyPerson person) { + protected synchronized ChangePersonInModelCommand getOngoingChangeForPerson(ReadOnlyPerson person) { return getOngoingChangeForPerson(person.getId()); } - synchronized ChangePersonInModelCommand getOngoingChangeForPerson(int targetId) { + protected synchronized ChangePersonInModelCommand getOngoingChangeForPerson(int targetId) { return personChangesInProgress.get(targetId); } @@ -260,23 +303,30 @@ boolean personHasOngoingChange(int personId) { return personChangesInProgress.containsKey(personId); } - void raiseLocalModelChangedEvent() { - raise(new LocalModelChangedEvent(this)); + void trackFinishedCommand(ChangeObjectInModelCommand finished) { + Platform.runLater(() -> finishedCommands.add(finished)); + } + + int assignCommandId() { + return commandCounter.incrementAndGet(); } //// CREATE /** - * Manually add a ViewablePerson to the visible model + * Manually add a ViewablePerson without a backing person to the visible model + * @return the added ViewablePerson */ - synchronized void addViewablePerson(ViewablePerson vp) { - visibleModel.addPerson(vp); + protected synchronized ViewablePerson addViewablePersonWithoutBacking(ReadOnlyPerson data) { + final ViewablePerson toAdd = ViewablePerson.withoutBacking(data); + visibleModel.addPerson(toAdd); + return toAdd; } /** * Manually add person to backing model without auto-creating a {@link ViewablePerson} for it in the visible model. */ - synchronized void addPersonToBackingModelSilently(Person p) { + protected synchronized void addPersonToBackingModelSilently(Person p) { visibleModel.specifyViewableAlreadyCreated(p.getId()); backingModel.addPerson(p); } @@ -300,7 +350,7 @@ public synchronized void addTagToBackingModel(Tag tagToAdd) throws DuplicateTagE throw new DuplicateTagException(tagToAdd); } backingTagList().add(tagToAdd); - raise(new CreateTagOnRemoteRequestEvent(new CompletableFuture<>(), getPrefs().getSaveLocation().getName(), tagToAdd)); + raise(new CreateTagOnRemoteRequestEvent(new CompletableFuture<>(), addressBookNameToUse, tagToAdd)); } //// UPDATE @@ -320,7 +370,7 @@ public synchronized void updateTag(Tag original, Tag updated) throws DuplicateTa String originalName = original.getName(); original.update(updated); raise(new EditTagOnRemoteRequestEvent(new CompletableFuture<>(), - getPrefs().getSaveLocation().getName(), originalName, updated)); + addressBookNameToUse, originalName, updated)); } //// DELETE @@ -333,22 +383,30 @@ public synchronized void updateTag(Tag original, Tag updated) throws DuplicateTa public synchronized boolean deleteTag(Tag tagToDelete) { boolean result = backingTagList().remove(tagToDelete); raise(new DeleteTagOnRemoteRequestEvent(new CompletableFuture<>(), - getPrefs().getSaveLocation().getName(), tagToDelete.getName())); + addressBookNameToUse, tagToDelete.getName())); return result; } //// EVENT HANDLERS @Subscribe - private void handleSyncCompletedEvent(SyncCompletedEvent uce) { + private void handleSyncCompletedEvent(SyncCompletedEvent uce) { // Sync is done outside FX Application thread + if (uce.getLatestTags().isPresent()) { + syncTags(uce.getLatestTags().get()); + } + syncPersons(uce.getUpdatedPersons()); + raise(new LocalModelChangedEvent(this)); + } + + private void syncPersons(Collection syncData) { Set deletedPersonIds = new HashSet<>(); - Map updatedPersons = new HashMap<>(); - uce.getUpdatedPersons().forEach(p -> { + Map newOrUpdatedPersons = new HashMap<>(); + syncData.forEach(p -> { if (p.isDeleted()) { deletedPersonIds.add(p.getId()); } else { - updatedPersons.put(p.getId(), p); + newOrUpdatedPersons.put(p.getId(), p); } }); PlatformExecUtil.runLater(() -> { @@ -357,39 +415,22 @@ private void handleSyncCompletedEvent(SyncCompletedEvent uce) { .filter(p -> deletedPersonIds.contains(p.getId())).collect(Collectors.toList())); // removeIf() not optimised // edits backingModel.getPersons().forEach(p -> { - if (updatedPersons.containsKey(p.getId())) { - p.update(updatedPersons.remove(p.getId())); + if (newOrUpdatedPersons.containsKey(p.getId())) { + p.update(newOrUpdatedPersons.remove(p.getId())); } }); // new - backingModel.getPersons().addAll(updatedPersons.values().stream() + backingModel.getPersons().addAll(newOrUpdatedPersons.values().stream() .map(Person::new).collect(Collectors.toList())); }); + } - - Set latestTags = new HashSet<>(uce.getLatestTags()); + private void syncTags(Collection syncData) { + Set latestTags = new HashSet<>(syncData); backingModel.getTags().retainAll(latestTags); // delete PlatformExecUtil.runLater(() -> { latestTags.removeAll(backingModel.getTags()); // latest tags no longer contains tags already in model backingModel.getTags().addAll(latestTags); // add }); } - - -//// PREFS - - public UserPrefs getPrefs() { - return prefs; - } - - public void setPrefsSaveLocation(String saveLocation) { - prefs.setSaveLocation(saveLocation); - raise(new SaveLocationChangedEvent(saveLocation)); - raise(new SavePrefsRequestEvent(prefs)); - raise(new ChangeActiveAddressBookRequestEvent(saveLocation)); - } - - public void clearPrefsSaveLocation() { - setPrefsSaveLocation(null); - } } diff --git a/src/main/java/address/model/UserPrefs.java b/src/main/java/address/model/UserPrefs.java index e4e8009d..b3b8f112 100644 --- a/src/main/java/address/model/UserPrefs.java +++ b/src/main/java/address/model/UserPrefs.java @@ -1,34 +1,19 @@ package address.model; -import java.io.File; +import address.util.GuiSettings; /** * Represents User's preferences. */ public class UserPrefs { - private static final String DEFAULT_TEMP_FILE_PATH = ".$TEMP_ADDRESS_BOOK"; - /** - * Full path (including file name) of the data file to be used for local storage - */ - private volatile String saveLocation; + public GuiSettings guiSettings; - public synchronized void setSaveLocation(String saveLocation) { - this.saveLocation = saveLocation; + public GuiSettings getGuiSettings() { + return guiSettings == null ? new GuiSettings() : guiSettings; } - /** - * @return the current save file location or the default temp file location if there is no recorded preference. - */ - public synchronized File getSaveLocation() { - return saveLocation == null ? new File(DEFAULT_TEMP_FILE_PATH) : new File(saveLocation); - } - - public String getSaveLocationString() { - return saveLocation; - } - - public boolean isSaveLocationSet() { - return getSaveLocation() != null; + public void setGuiSettings(GuiSettings guiSettings) { + this.guiSettings = guiSettings; } } diff --git a/src/main/java/address/model/datatypes/ReadOnlyViewableDataType.java b/src/main/java/address/model/datatypes/ReadOnlyViewableDataType.java index e9e30d1b..33a205df 100644 --- a/src/main/java/address/model/datatypes/ReadOnlyViewableDataType.java +++ b/src/main/java/address/model/datatypes/ReadOnlyViewableDataType.java @@ -1,6 +1,5 @@ package address.model.datatypes; -import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.ReadOnlyIntegerProperty; /** @@ -11,11 +10,5 @@ public interface ReadOnlyViewableDataType { ReadOnlyIntegerProperty secondsLeftInPendingStateProperty(); int getSecondsLeftInPendingState(); - ReadOnlyBooleanProperty isEditedProperty(); - boolean isEdited(); - - ReadOnlyBooleanProperty isDeletedProperty(); - boolean isDeleted(); - boolean isSyncingWithBackingObject(); } diff --git a/src/main/java/address/model/datatypes/Viewable.java b/src/main/java/address/model/datatypes/Viewable.java index f9c17507..00edfd61 100644 --- a/src/main/java/address/model/datatypes/Viewable.java +++ b/src/main/java/address/model/datatypes/Viewable.java @@ -22,13 +22,9 @@ public abstract class Viewable extends UniqueData implemen protected boolean isSyncingWithBackingObject; protected final IntegerProperty secondsLeftInPendingState; // 0 when not in pending state - protected final BooleanProperty isDeleted; - protected final BooleanProperty isEdited; { secondsLeftInPendingState = new SimpleIntegerProperty(-1); - isDeleted = new SimpleBooleanProperty(false); - isEdited = new SimpleBooleanProperty(false); } /** @@ -122,35 +118,6 @@ public void decrementSecondsLeftInPendingState() { secondsLeftInPendingState.set(secondsLeftInPendingState.get() - 1); } - @Override - public boolean isDeleted() { - return isDeleted.get(); - } - - @Override - public ReadOnlyBooleanProperty isDeletedProperty() { - return isDeleted; - } - - public void setIsDeleted(boolean isDeleted) { - this.isDeleted.set(isDeleted); - } - - @Override - public boolean isEdited() { - return isEdited.get(); - } - - @Override - public ReadOnlyBooleanProperty isEditedProperty() { - return isEdited; - } - - public void setIsEdited(boolean isEdited) { - this.isEdited.set(isEdited); - } - - // VISIBLE--BACKING binding controls /** diff --git a/src/main/java/address/model/datatypes/person/ReadOnlyPerson.java b/src/main/java/address/model/datatypes/person/ReadOnlyPerson.java index 7da27abf..1c036e49 100644 --- a/src/main/java/address/model/datatypes/person/ReadOnlyPerson.java +++ b/src/main/java/address/model/datatypes/person/ReadOnlyPerson.java @@ -170,6 +170,17 @@ default String tagsString() { } } + default boolean dataFieldsEqual(ReadOnlyPerson other) { + final Set othersTags = new HashSet<>(other.getTagList()); + return fullName().equals(other.fullName()) + && getGithubUsername().equals(other.getGithubUsername()) + && getStreet().equals(other.getStreet()) + && getPostalCode().equals(other.getPostalCode()) + && getCity().equals(other.getCity()) + && getBirthday().equals(other.getBirthday()) + && getTagList().stream().allMatch(othersTags::contains); + } + //// Operations below are optional; override if they will be needed. //// Eg. implementing a simple person data object should not require using javafx classes like Property, so the //// below methods make no sense for that context. diff --git a/src/main/java/address/model/datatypes/person/ReadOnlyViewablePerson.java b/src/main/java/address/model/datatypes/person/ReadOnlyViewablePerson.java index e77a3600..1fd74bd0 100644 --- a/src/main/java/address/model/datatypes/person/ReadOnlyViewablePerson.java +++ b/src/main/java/address/model/datatypes/person/ReadOnlyViewablePerson.java @@ -2,6 +2,7 @@ import address.model.datatypes.ReadOnlyViewableDataType; import javafx.beans.Observable; +import javafx.beans.property.ReadOnlyProperty; import java.util.Arrays; import java.util.function.Consumer; @@ -12,6 +13,16 @@ */ public interface ReadOnlyViewablePerson extends ReadOnlyPerson, ReadOnlyViewableDataType { + enum ChangeInProgress { + ADDING, + EDITING, + DELETING, + NONE + } + + ChangeInProgress getChangeInProgress(); + ReadOnlyProperty changeInProgressProperty(); + /** * @return whether this person exists on the remote server * @see #hasConfirmedRemoteID() @@ -35,12 +46,12 @@ default boolean existsOnRemote() { default Observable[] extractObservables() { final Observable[] obs = { secondsLeftInPendingStateProperty(), - isEditedProperty(), - isDeletedProperty() }; return Stream.concat( Arrays.stream(ReadOnlyPerson.super.extractObservables()), Arrays.stream(obs)) .toArray(Observable[]::new); } + + boolean hasName(String firstName, String lastName); } diff --git a/src/main/java/address/model/datatypes/person/ViewablePerson.java b/src/main/java/address/model/datatypes/person/ViewablePerson.java index ae2c76ce..fa588dbd 100644 --- a/src/main/java/address/model/datatypes/person/ViewablePerson.java +++ b/src/main/java/address/model/datatypes/person/ViewablePerson.java @@ -1,6 +1,7 @@ package address.model.datatypes.person; -import address.model.ChangePersonInModelCommand; +import static address.model.datatypes.person.ReadOnlyViewablePerson.ChangeInProgress.*; + import address.model.datatypes.UniqueData; import address.model.datatypes.Viewable; import address.model.datatypes.tag.Tag; @@ -34,9 +35,11 @@ public class ViewablePerson extends Viewable implements ReadOnlyViewable * can only be changed by {@link #connectBackingObject} */ private int id; + private final Property changeInProgress; { remoteIdConfirmationHandlers = new ArrayList<>(); + changeInProgress = new SimpleObjectProperty<>(NONE); } /** @@ -63,12 +66,12 @@ private ViewablePerson(Person backingPerson, Function visibleFac * @see super#Viewable(UniqueData) * @see #connectBackingObject(Person) */ - public static ViewablePerson withoutBacking(Person visiblePerson) { - return new ViewablePerson(visiblePerson); + public static ViewablePerson withoutBacking(ReadOnlyPerson personData) { + return new ViewablePerson(new Person(personData)); } /** - * @see #withoutBacking(Person) + * @see #withoutBacking(ReadOnlyPerson) * @see super#Viewable(UniqueData) */ private ViewablePerson(Person visiblePerson) { @@ -132,6 +135,20 @@ public void connectBackingObject(Person backingPerson) { //// OPTIMISTIC UPDATING + @Override + public ChangeInProgress getChangeInProgress() { + return changeInProgress.getValue(); + } + + @Override + public ReadOnlyProperty changeInProgressProperty() { + return changeInProgress; + } + + public void setChangeInProgress(ChangeInProgress currentChange) { + changeInProgress.setValue(currentChange); + } + /** * Updates the visible data, backing data remains untouched. */ @@ -270,6 +287,11 @@ public String tagsString() { } + @Override + public boolean hasName(String firstName, String lastName) { + return firstNameProperty().getValue().equals(firstName) + && lastNameProperty().getValue().equals(lastName); + } /** * Use backing Person for comparison. */ diff --git a/src/main/java/address/storage/StorageManager.java b/src/main/java/address/storage/StorageManager.java index 0ee9c510..3c0d6355 100644 --- a/src/main/java/address/storage/StorageManager.java +++ b/src/main/java/address/storage/StorageManager.java @@ -19,21 +19,28 @@ */ public class StorageManager extends ComponentManager { private static final AppLogger logger = LoggerManager.getLogger(StorageManager.class); - private static final String CONFIG_FILE = "config.json"; - private Config config; + private static final String DEFAULT_CONFIG_FILE = "config.json"; + private UserPrefs userPrefs; + private File saveFile; + private File userPrefsFile; private final Consumer loadedDataCallback; - private UserPrefs prefs; - public StorageManager(Consumer loadedDataCallback, Config config, UserPrefs prefs) { + public StorageManager(Consumer loadedDataCallback, Config config, UserPrefs userPrefs) { super(); this.loadedDataCallback = loadedDataCallback; - this.prefs = prefs; - this.config = config; + this.saveFile = new File(config.getLocalDataFilePath()); + this.userPrefsFile = config.getPrefsFileLocation(); + this.userPrefs = userPrefs; } - public static Config getConfig() { - File configFile = new File(CONFIG_FILE); + private static File getConfigFile(String configFilePath) { + if (configFilePath == null) return new File(DEFAULT_CONFIG_FILE); + return new File(configFilePath); + } + + public static Config getConfig(String configFilePath) { + File configFile = getConfigFile(configFilePath); Config config; if (configFile.exists()) { @@ -111,7 +118,7 @@ public void handleLoadDataRequestEvent(LoadDataRequestEvent ldre) { @Subscribe public void handleLocalModelChangedEvent(LocalModelChangedEvent lmce) { logger.info("Local data changed, saving to primary data file"); - saveDataToFile(prefs.getSaveLocation(), lmce.data); + saveDataToFile(saveFile, lmce.data); } /** @@ -158,9 +165,9 @@ public void handleSavePrefsRequestEvent(SavePrefsRequestEvent spre) { */ public void savePrefsToFile(UserPrefs prefs) { try { - serializeObjectToJsonFile(config.getPrefsFileLocation(), prefs); + serializeObjectToJsonFile(userPrefsFile, prefs); } catch (IOException e) { - raise(new FileSavingExceptionEvent(e, config.getPrefsFileLocation())); + raise(new FileSavingExceptionEvent(e, userPrefsFile)); } } @@ -186,7 +193,7 @@ public static UserPrefs getUserPrefs(File prefsFile) { */ public void start() { logger.info("Starting storage manager."); - loadDataFromFile(prefs.getSaveLocation()); + loadDataFromFile(saveFile); } protected void loadDataFromFile(File dataFile) { @@ -200,8 +207,8 @@ protected void loadDataFromFile(File dataFile) { } public ReadOnlyAddressBook getData() throws FileNotFoundException, DataConversionException { - logger.debug("Attempting to read data from file: {}", prefs.getSaveLocation()); - return XmlFileStorage.loadDataFromSaveFile(prefs.getSaveLocation()); + logger.debug("Attempting to read data from file: {}", saveFile); + return XmlFileStorage.loadDataFromSaveFile(saveFile); } public static void serializeObjectToJsonFile(File jsonFile, T objectToSerialize) throws IOException { diff --git a/src/main/java/address/sync/RemoteManager.java b/src/main/java/address/sync/RemoteManager.java index 533328f7..7cc52855 100644 --- a/src/main/java/address/sync/RemoteManager.java +++ b/src/main/java/address/sync/RemoteManager.java @@ -3,8 +3,8 @@ import address.model.datatypes.person.Person; import address.model.datatypes.person.ReadOnlyPerson; import address.model.datatypes.tag.Tag; +import address.sync.cloud.IRemote; import address.util.AppLogger; -import address.util.Config; import address.util.LoggerManager; import java.io.IOException; @@ -28,9 +28,9 @@ public class RemoteManager { private HashMap> updateInformation; private LocalDateTime personLastUpdatedAt; - public RemoteManager(Config config) { + public RemoteManager(IRemote remote) { updateInformation = new HashMap<>(); - remoteService = new RemoteService(config); + remoteService = new RemoteService(remote); } public RemoteManager(RemoteService remoteService) { @@ -60,7 +60,10 @@ public Optional> getUpdatedPersons(String addressBookName) throws I logger.debug("Last updated time for page {} found: {}", curPage, personLastUpdatedAt); response = remoteService.getUpdatedPersonsSince(addressBookName, curPage, personLastUpdatedAt, null); } - if (!response.getData().isPresent()) return Optional.empty(); + if (!response.getData().isPresent()) { + logger.debug("No data found from response, terminating paged requests."); + return Optional.empty(); + } personList.addAll(response.getData().get()); curPage++; } while (response.getNextPage() != 0); // may have problems if RESOURCES_PER_PAGE issues have been updated at the same second diff --git a/src/main/java/address/sync/RemoteService.java b/src/main/java/address/sync/RemoteService.java index f5d047ab..ea56919d 100644 --- a/src/main/java/address/sync/RemoteService.java +++ b/src/main/java/address/sync/RemoteService.java @@ -3,12 +3,11 @@ import address.model.datatypes.person.ReadOnlyPerson; import address.model.datatypes.tag.Tag; import address.model.datatypes.person.Person; -import address.sync.cloud.CloudSimulator; +import address.sync.cloud.IRemote; import address.sync.cloud.RemoteResponse; import address.sync.cloud.model.CloudPerson; import address.sync.cloud.model.CloudTag; import address.util.AppLogger; -import address.util.Config; import address.util.JsonUtil; import address.util.LoggerManager; @@ -28,14 +27,10 @@ public class RemoteService implements IRemoteService { private static final AppLogger logger = LoggerManager.getLogger(RemoteService.class); private static final int RESOURCES_PER_PAGE = 100; - private final CloudSimulator remote; + private final IRemote remote; - public RemoteService(Config config) { - remote = new CloudSimulator(config); - } - - public RemoteService(CloudSimulator cloudSimulator) { - remote = cloudSimulator; + public RemoteService(IRemote remote) { + this.remote = remote; } /** @@ -391,23 +386,21 @@ private long getRateResetFromHeader(HashMap header) { } private List convertToPersonList(List cloudPersonList) { - List convertedList = new ArrayList<>(); - cloudPersonList.stream() - .forEach(CloudPerson -> convertedList.add(convertToPerson(CloudPerson))); - - return convertedList; + return cloudPersonList.stream() + .map(this::convertToPerson) + .collect(Collectors.toCollection(ArrayList::new)); } private List convertToTagList(List cloudTagList) { - List convertedList = new ArrayList<>(); - cloudTagList.stream() - .forEach(cloudTag -> convertedList.add(convertToTag(cloudTag))); - - return convertedList; + return cloudTagList.stream() + .map(this::convertToTag) + .collect(Collectors.toCollection(ArrayList::new)); } private List convertToCloudTagList(List tagList) { - return tagList.stream().map(this::convertToCloudTag).collect(Collectors.toCollection(ArrayList::new)); + return tagList.stream() + .map(this::convertToCloudTag) + .collect(Collectors.toCollection(ArrayList::new)); } private Person convertToPerson(CloudPerson cloudPerson) { diff --git a/src/main/java/address/sync/SyncManager.java b/src/main/java/address/sync/SyncManager.java index 002fc664..5cdd4375 100644 --- a/src/main/java/address/sync/SyncManager.java +++ b/src/main/java/address/sync/SyncManager.java @@ -43,9 +43,9 @@ public class SyncManager extends ComponentManager { * @param config should have updateInterval (milliseconds) and simulateUnreliableNetwork set * @param activeAddressBookName name of active addressbook to start with */ - public SyncManager(Config config, String activeAddressBookName) { - this(config, new RemoteManager(config), Executors.newCachedThreadPool(), - Executors.newScheduledThreadPool(1), activeAddressBookName); + public SyncManager(RemoteManager remoteManager, Config config, String activeAddressBookName) { + this(config, remoteManager, Executors.newCachedThreadPool(), + Executors.newSingleThreadScheduledExecutor(), activeAddressBookName); } /** @@ -96,10 +96,9 @@ public void setActiveAddressBook(String activeAddressBookName) { */ public void start() { logger.info("Starting sync manager."); - long initialDelay = 300; // temp fix for issue #66 Runnable syncTask = new GetUpdatesFromRemoteTask(remoteManager, this::raise, this::getActiveAddressBook); - logger.debug("Scheduling synchronization task with interval of {} milliseconds", config.updateInterval); - scheduler.scheduleWithFixedDelay(syncTask, initialDelay, config.updateInterval, TimeUnit.MILLISECONDS); + logger.debug("Scheduling synchronization task with interval of {} milliseconds", config.getUpdateInterval()); + scheduler.scheduleWithFixedDelay(syncTask, 0, config.getUpdateInterval(), TimeUnit.MILLISECONDS); } public void stop() { diff --git a/src/main/java/address/sync/cloud/CloudFileHandler.java b/src/main/java/address/sync/cloud/CloudFileHandler.java index 7c2f3cff..9345e7c3 100644 --- a/src/main/java/address/sync/cloud/CloudFileHandler.java +++ b/src/main/java/address/sync/cloud/CloudFileHandler.java @@ -14,24 +14,19 @@ public class CloudFileHandler { private static final AppLogger logger = LoggerManager.getLogger(CloudFileHandler.class); private static final String CLOUD_DIRECTORY = "cloud/"; - public CloudAddressBook readCloudAddressBookFromFile(String addressBookName) throws FileNotFoundException, + public CloudAddressBook readCloudAddressBookFromExternalFile(String cloudDataFilePath) throws FileNotFoundException, + DataConversionException { + File cloudFile = new File(cloudDataFilePath); + return readFromCloudFile(cloudFile); + } + + public CloudAddressBook readCloudAddressBook(String addressBookName) throws FileNotFoundException, DataConversionException { File cloudFile = getCloudDataFile(addressBookName); - try { - logger.debug("Reading from cloud file '{}'.", cloudFile.getName()); - CloudAddressBook CloudAddressBook = XmlUtil.getDataFromFile(cloudFile, CloudAddressBook.class); - if (CloudAddressBook.getName() == null) throw new DataConversionException("AddressBook name is null."); - return CloudAddressBook; - } catch (FileNotFoundException e) { - logger.warn("Cloud file '{}' not found.", cloudFile.getName()); - throw e; - } catch (DataConversionException e) { - logger.warn("Error reading from cloud file '{}'.", cloudFile.getName()); - throw e; - } + return readFromCloudFile(cloudFile); } - public void writeCloudAddressBookToFile(CloudAddressBook CloudAddressBook) throws FileNotFoundException, + public void writeCloudAddressBook(CloudAddressBook CloudAddressBook) throws FileNotFoundException, DataConversionException { String addressBookName = CloudAddressBook.getName(); File cloudFile = getCloudDataFile(addressBookName); @@ -44,26 +39,103 @@ public void writeCloudAddressBookToFile(CloudAddressBook CloudAddressBook) throw } } - public void createCloudAddressBookFile(String addressBookName) throws IOException, DataConversionException, + /** + * Attempts to create a file with an empty address book + * Deletes any existing file on the same path + * + * @param addressBookName + * @throws IOException + * @throws DataConversionException + */ + public void initializeAddressBook(String addressBookName) throws IOException, DataConversionException { + File cloudFile = getCloudDataFile(addressBookName); + if (cloudFile.exists()) { + cloudFile.delete(); + } + + createCloudFile(new CloudAddressBook(addressBookName)); + } + + /** + * Attempts to create an empty address book on the cloud + * Fails if address book already exists + * + * @param addressBookName + * @throws IOException + * @throws DataConversionException + * @throws IllegalArgumentException if cloud file already exists + */ + public void createAddressBook(String addressBookName) throws IOException, DataConversionException, + IllegalArgumentException { + createCloudFile(new CloudAddressBook(addressBookName)); + } + + /** + * Attempts to create an empty address book on the cloud if it does not exist + * + * @param addressBookName + * @throws IOException + * @throws DataConversionException + * @throws IllegalArgumentException + */ + public void createAddressBookIfAbsent(String addressBookName) throws IOException, DataConversionException, IllegalArgumentException { File cloudFile = getCloudDataFile(addressBookName); + if (cloudFile.exists()) return; + try { + createCloudFile(new CloudAddressBook(addressBookName)); + } catch (IllegalArgumentException e) { + assert false : "Error in logic: createCloudFile should not be called since address book is present"; + } + } + + private CloudAddressBook readFromCloudFile(File cloudFile) throws FileNotFoundException, DataConversionException { + try { + logger.debug("Reading from cloud file '{}'.", cloudFile.getName()); + CloudAddressBook cloudAddressBook = XmlUtil.getDataFromFile(cloudFile, CloudAddressBook.class); + if (cloudAddressBook.getName() == null) throw new DataConversionException("AddressBook name is null."); + return cloudAddressBook; + } catch (FileNotFoundException e) { + logger.warn("Cloud file '{}' not found.", cloudFile.getName()); + throw e; + } catch (DataConversionException e) { + logger.warn("Error reading from cloud file '{}'.", cloudFile.getName()); + throw e; + } + } + + /** + * Attempts to create the cloud file in the cloud directory, containing an empty address book + * File will be named the same as the address book + * + * The cloud directory will also be created if it does not exist + * + * @param cloudAddressBook + * @throws IOException + * @throws DataConversionException + * @throws IllegalArgumentException if cloud file already exists + */ + private void createCloudFile(CloudAddressBook cloudAddressBook) throws IOException, DataConversionException, IllegalArgumentException { + File cloudFile = getCloudDataFile(cloudAddressBook.getName()); if (cloudFile.exists()) { - logger.warn("Cannot create an addressbook that already exists: '{}'.", addressBookName); - throw new IllegalArgumentException("AddressBook '" + addressBookName + "' already exists!"); + logger.warn("Cannot create an address book that already exists: '{}'.", cloudAddressBook.getName()); + throw new IllegalArgumentException("AddressBook '" + cloudAddressBook.getName() + "' already exists!"); } + File cloudDirectory = new File(CLOUD_DIRECTORY); if (!cloudDirectory.exists() && !cloudDirectory.mkdir()) { logger.warn("Error creating directory: '{}'", CLOUD_DIRECTORY); throw new IOException("Error creating directory: " + CLOUD_DIRECTORY); } + if (!cloudFile.createNewFile()) { - logger.warn("Error creating cloud file: '{}'", getCloudDataFilePath(addressBookName)); - throw new IOException("Error creating cloud file for addressbook: " + getCloudDataFilePath(addressBookName)); + logger.warn("Error creating cloud file: '{}'", getCloudDataFilePath(cloudAddressBook.getName())); + throw new IOException("Error creating cloud file for address book: " + getCloudDataFilePath(cloudAddressBook.getName())); } - writeCloudAddressBookToFile(new CloudAddressBook(addressBookName)); + writeCloudAddressBook(cloudAddressBook); } private File getCloudDataFile(String addressBookName) { diff --git a/src/main/java/address/sync/cloud/CloudSimulator.java b/src/main/java/address/sync/cloud/CloudSimulator.java index b2499c29..1089e738 100644 --- a/src/main/java/address/sync/cloud/CloudSimulator.java +++ b/src/main/java/address/sync/cloud/CloudSimulator.java @@ -13,7 +13,6 @@ import java.net.HttpURLConnection; import java.time.LocalDateTime; import java.util.*; -import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; /** @@ -23,82 +22,62 @@ * Requests for a full list of objects should be done in pages. Responses * will include first page/prev page/next page/last page if they exist. * - * Any bad requests due to inappropriate parameters will still consume API - * usage. - * - * In addition, data returned by this cloud may be modified due to - * simulated corruption or its responses may have significant delays, - * if the cloud is initialized with an unreliable network parameter + * Providing previous request's eTag may return a NOT_MODIFIED response if the response's eTag has not changed. + * All requests (including bad ones) will consume API, unless it is a response with NOT_MODIFIED. */ -public class CloudSimulator implements ICloudSimulator { +public class CloudSimulator implements IRemote { private static final AppLogger logger = LoggerManager.getLogger(CloudSimulator.class); private static final int API_QUOTA_PER_HOUR = 5000; - private static final Random RANDOM_GENERATOR = new Random(); - private static final double FAILURE_PROBABILITY = 0.1; - private static final double NETWORK_DELAY_PROBABILITY = 0.2; - private static final int MIN_DELAY_IN_SEC = 1; - private static final int DELAY_RANGE = 5; - private static final double MODIFY_PERSON_PROBABILITY = 0.1; - private static final double MODIFY_TAG_PROBABILITY = 0.05; - private static final double ADD_PERSON_PROBABILITY = 0.05; - private static final double ADD_TAG_PROBABILITY = 0.025; - private static final int MAX_NUM_PERSONS_TO_ADD = 2; - private CloudRateLimitStatus cloudRateLimitStatus; - private boolean shouldSimulateUnreliableNetwork; - private CloudFileHandler fileHandler; - - public CloudSimulator(CloudFileHandler fileHandler, CloudRateLimitStatus cloudRateLimitStatus, - boolean shouldSimulateUnreliableNetwork) { + + protected CloudRateLimitStatus cloudRateLimitStatus; + protected CloudFileHandler fileHandler; + + protected CloudSimulator(CloudFileHandler fileHandler, CloudRateLimitStatus cloudRateLimitStatus) { this.fileHandler = fileHandler; this.cloudRateLimitStatus = cloudRateLimitStatus; - this.shouldSimulateUnreliableNetwork = shouldSimulateUnreliableNetwork; } public CloudSimulator(Config config) { fileHandler = new CloudFileHandler(); cloudRateLimitStatus = new CloudRateLimitStatus(API_QUOTA_PER_HOUR); - this.shouldSimulateUnreliableNetwork = config.simulateUnreliableNetwork; cloudRateLimitStatus.restartQuotaTimer(); + try { + fileHandler.createAddressBookIfAbsent(config.getAddressBookName()); + } catch (IOException | DataConversionException e) { + logger.fatal("Error initializing cloud file for '{}'", config.getAddressBookName()); + assert false : "Error initializing cloud file: " + config.getAddressBookName(); + } } /** * Attempts to create a person if quota is available * - * A new ID for the new person will be generated; and the ID field in the given newPerson will be ignored + * A new ID for the new person will be generated, and the ID field in the given newPerson will be ignored *

* Consumes 1 API usage * * @param addressBookName * @param newPerson + * @param previousETag * @return a response wrapper, containing the added person if successful */ @Override public RemoteResponse createPerson(String addressBookName, CloudPerson newPerson, String previousETag) { logger.debug("createPerson called with: addressbook {}, person {}, prevETag {}", addressBookName, newPerson, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); CloudPerson returnedPerson = addPerson(fileData.getAllPersons(), newPerson); - fileHandler.writeCloudAddressBookToFile(fileData); - - modifyCloudPersonBasedOnChance(returnedPerson); - - RemoteResponse remoteResponse = new RemoteResponse(HttpURLConnection.HTTP_CREATED, returnedPerson, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(remoteResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); - return remoteResponse; + fileHandler.writeCloudAddressBook(fileData); + return new RemoteResponse(HttpURLConnection.HTTP_CREATED, returnedPerson, cloudRateLimitStatus, + previousETag); } catch (IllegalArgumentException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -119,28 +98,23 @@ public RemoteResponse getPersons(String addressBookName, int pageNumber, int res String previousETag) { logger.debug("getPersons called with: addressbook {}, page {}, resourcesperpage {}, prevETag {}", addressBookName, pageNumber, resourcesPerPage, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); List fullPersonList = new ArrayList<>(); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); fullPersonList.addAll(fileData.getAllPersons()); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); List queryResults = getQueryResults(pageNumber, resourcesPerPage, fullPersonList); - - mutateCloudPersonList(queryResults); - RemoteResponse contentResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, queryResults, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(contentResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); + cloudRateLimitStatus, previousETag); - cloudRateLimitStatus.useQuota(1); + if (isNotModifiedResponse(contentResponse)) return contentResponse; if (isValidPageNumber(fullPersonList.size(), pageNumber, resourcesPerPage)) { fillInPageNumbers(pageNumber, resourcesPerPage, fullPersonList, contentResponse); @@ -148,6 +122,10 @@ public RemoteResponse getPersons(String addressBookName, int pageNumber, int res return contentResponse; } + private boolean isNotModifiedResponse(RemoteResponse contentResponse) { + return contentResponse.getResponseCode() == HttpURLConnection.HTTP_NOT_MODIFIED; + } + /** * Returns a response wrapper containing the list of tags if quota is available *

@@ -156,34 +134,30 @@ public RemoteResponse getPersons(String addressBookName, int pageNumber, int res * @param addressBookName * @param pageNumber * @param resourcesPerPage + * @param previousETag * @return */ @Override public RemoteResponse getTags(String addressBookName, int pageNumber, int resourcesPerPage, String previousETag) { logger.debug("getTags called with: addressbook {}, page {}, resourcesperpage {}, prevETag {}", addressBookName, pageNumber, resourcesPerPage, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); List fullTagList = new ArrayList<>(); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); fullTagList.addAll(fileData.getAllTags()); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); List queryResults = getQueryResults(pageNumber, resourcesPerPage, fullTagList); - modifyCloudTagListBasedOnChance(queryResults); - RemoteResponse contentResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, queryResults, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(contentResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); + cloudRateLimitStatus, previousETag); + if (isNotModifiedResponse(contentResponse)) return contentResponse; if (isValidPageNumber(fullTagList.size(), pageNumber, resourcesPerPage)) { fillInPageNumbers(pageNumber, resourcesPerPage, fullTagList, contentResponse); @@ -196,16 +170,14 @@ public RemoteResponse getTags(String addressBookName, int pageNumber, int resour *

* This does NOT cost any API usage * + * @param previousETag * @return */ @Override public RemoteResponse getRateLimitStatus(String previousETag) { + // TODO: Figure out GitHub response for limit status if ETag is provided logger.debug("getRateLimitStatus called with: prevETag {}", previousETag); - RemoteResponse remoteResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, getHeaders(cloudRateLimitStatus), - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(remoteResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - return remoteResponse; + return RemoteResponse.getLimitStatusResponse(cloudRateLimitStatus); } /** @@ -216,6 +188,7 @@ public RemoteResponse getRateLimitStatus(String previousETag) { * @param addressBookName * @param personId * @param updatedPerson + * @param previousETag * @return */ @Override @@ -224,29 +197,18 @@ public RemoteResponse updatePerson(String addressBookName, int personId, logger.debug("updatePerson called with: addressbook {}, personid {}, person {}, prevETag {}", addressBookName, personId, updatedPerson, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); CloudPerson resultingPerson = updatePersonDetails(fileData.getAllPersons(), fileData.getAllTags(), personId, updatedPerson); - fileHandler.writeCloudAddressBookToFile(fileData); - - modifyCloudPersonBasedOnChance(resultingPerson); - - RemoteResponse remoteResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, resultingPerson, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(remoteResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); - return remoteResponse; + fileHandler.writeCloudAddressBook(fileData); + return new RemoteResponse(HttpURLConnection.HTTP_OK, resultingPerson, cloudRateLimitStatus, previousETag); } catch (NoSuchElementException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -263,21 +225,18 @@ public RemoteResponse updatePerson(String addressBookName, int personId, @Override public RemoteResponse deletePerson(String addressBookName, int personId) { logger.debug("deletePerson called with: addressbook {}, personid {}", addressBookName, personId); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); deletePersonFromData(fileData.getAllPersons(), personId); - fileHandler.writeCloudAddressBookToFile(fileData); + fileHandler.writeCloudAddressBook(fileData); - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_NO_CONTENT); } catch (NoSuchElementException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -289,34 +248,24 @@ public RemoteResponse deletePerson(String addressBookName, int personId) { * * @param addressBookName * @param newTag tag name should not already be used + * @param previousETag * @return */ @Override public RemoteResponse createTag(String addressBookName, CloudTag newTag, String previousETag) { logger.debug("createTag called with: addressbook {}, tag {}, prevETag {}", addressBookName, newTag, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); CloudTag returnedTag = addTag(fileData.getAllTags(), newTag); - fileHandler.writeCloudAddressBookToFile(fileData); - - modifyCloudTagBasedOnChance(returnedTag); - - RemoteResponse remoteResponse = new RemoteResponse(HttpURLConnection.HTTP_CREATED, returnedTag, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(remoteResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); - return remoteResponse; + fileHandler.writeCloudAddressBook(fileData); + return new RemoteResponse(HttpURLConnection.HTTP_CREATED, returnedTag, cloudRateLimitStatus, previousETag); } catch (IllegalArgumentException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -327,37 +276,27 @@ public RemoteResponse createTag(String addressBookName, CloudTag newTag, String * Consumes 1 API usage * * @param addressBookName - * @param oldTagName should match a existing tag's name + * @param oldTagName should match an existing tag's name * @param updatedTag + * @param previousETag * @return */ @Override public RemoteResponse editTag(String addressBookName, String oldTagName, CloudTag updatedTag, String previousETag) { logger.debug("editTag called with: addressbook {}, tagname {}, tag {}, prevETag {}", addressBookName, oldTagName, updatedTag, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); CloudTag returnedTag = updateTagDetails(fileData.getAllPersons(), fileData.getAllTags(), oldTagName, updatedTag); - fileHandler.writeCloudAddressBookToFile(fileData); - - modifyCloudTagBasedOnChance(returnedTag); - - RemoteResponse remoteResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, returnedTag, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(remoteResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); - return remoteResponse; + fileHandler.writeCloudAddressBook(fileData); + return new RemoteResponse(HttpURLConnection.HTTP_OK, returnedTag, cloudRateLimitStatus, previousETag); } catch (NoSuchElementException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -375,21 +314,18 @@ public RemoteResponse editTag(String addressBookName, String oldTagName, CloudTa @Override public RemoteResponse deleteTag(String addressBookName, String tagName) { logger.debug("deleteTag called with: addressbook {}, tagname {}", addressBookName, tagName); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); deleteTagFromData(fileData.getAllPersons(), fileData.getAllTags(), tagName); - fileHandler.writeCloudAddressBookToFile(fileData); + fileHandler.writeCloudAddressBook(fileData); - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_NO_CONTENT); } catch (NoSuchElementException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } } @@ -405,21 +341,16 @@ public RemoteResponse deleteTag(String addressBookName, String tagName) { @Override public RemoteResponse createAddressBook(String addressBookName) { logger.debug("createAddressBook called with: addressbook {}", addressBookName); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); - - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); try { - fileHandler.createCloudAddressBookFile(addressBookName); + fileHandler.createAddressBook(addressBookName); - cloudRateLimitStatus.useQuota(1); //TODO: Return a wrapped simplified version of an empty addressbook (e.g. only fields such as name) return getEmptyResponse(HttpURLConnection.HTTP_CREATED); } catch (DataConversionException | IOException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } catch (IllegalArgumentException e) { - cloudRateLimitStatus.useQuota(1); return getEmptyResponse(HttpURLConnection.HTTP_BAD_REQUEST); } } @@ -427,10 +358,13 @@ public RemoteResponse createAddressBook(String addressBookName) { /** * Gets the list of persons that have been updated after a certain time, if quota is available *

- * Consumes 1 + floor(updated person list/resourcesPerPage) API usage + * Consumes 1 API usage * * @param addressBookName * @param timeString + * @param pageNumber + * @param resourcesPerPage + * @param previousETag * @return */ @Override @@ -438,32 +372,26 @@ public RemoteResponse getUpdatedPersons(String addressBookName, String timeStrin int resourcesPerPage, String previousETag) { logger.debug("getUpdatedPersons called with: addressbook {}, time {}, pageno {}, resourcesperpage {}, prevETag {}", addressBookName, timeString, pageNumber, resourcesPerPage, previousETag); - if (shouldSimulateNetworkFailure()) return getNetworkFailedResponse(); - if (shouldSimulateSlowResponse()) delayRandomAmount(); + if (!hasApiQuotaRemaining()) return RemoteResponse.getForbiddenResponse(cloudRateLimitStatus); List fullPersonList = new ArrayList<>(); try { - CloudAddressBook fileData = fileHandler.readCloudAddressBookFromFile(addressBookName); + CloudAddressBook fileData = fileHandler.readCloudAddressBook(addressBookName); fullPersonList.addAll(fileData.getAllPersons()); - } catch (FileNotFoundException | DataConversionException e) { + } catch (FileNotFoundException e) { + return getEmptyResponse(HttpURLConnection.HTTP_NOT_FOUND); + } catch (DataConversionException e) { return getEmptyResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); } - if (!hasApiQuotaRemaining()) return getEmptyResponse(HttpURLConnection.HTTP_FORBIDDEN); - LocalDateTime time = LocalDateTime.parse(timeString); List filteredList = filterPersonsByTime(fullPersonList, time); List queryResults = getQueryResults(pageNumber, resourcesPerPage, filteredList); - mutateCloudPersonList(queryResults); - RemoteResponse contentResponse = new RemoteResponse(HttpURLConnection.HTTP_OK, queryResults, - getHeaders(cloudRateLimitStatus)); - String eTag = getResponseETag(contentResponse); - if (eTag.equals(previousETag)) return getNotModifiedResponse(); - - cloudRateLimitStatus.useQuota(1); + cloudRateLimitStatus, previousETag); + if (isNotModifiedResponse(contentResponse)) return contentResponse; if (isValidPageNumber(filteredList.size(), pageNumber, resourcesPerPage)) { fillInPageNumbers(pageNumber, resourcesPerPage, filteredList, contentResponse); @@ -471,18 +399,6 @@ public RemoteResponse getUpdatedPersons(String addressBookName, String timeStrin return contentResponse; } - private String getResponseETag(RemoteResponse response) { - return response.getHeaders().get("ETag"); - } - - private HashMap getHeaders(CloudRateLimitStatus cloudRateLimitStatus) { - HashMap headers = new HashMap<>(); - headers.put("X-RateLimit-Limit", String.valueOf(cloudRateLimitStatus.getQuotaLimit())); - headers.put("X-RateLimit-Remaining", String.valueOf(cloudRateLimitStatus.getQuotaRemaining())); - headers.put("X-RateLimit-Reset", String.valueOf(cloudRateLimitStatus.getQuotaReset())); - return headers; - } - /** * Fills in the page index details for a cloud response * @@ -522,14 +438,9 @@ private List getQueryResults(int pageNumber, int resourcesPerPage, List filterPersonsByTime(List personList, LocalDateTime time) { @@ -538,19 +449,6 @@ private List filterPersonsByTime(List personList, Loca .collect(Collectors.toList()); } - private boolean shouldSimulateNetworkFailure() { - return shouldSimulateUnreliableNetwork && RANDOM_GENERATOR.nextDouble() <= FAILURE_PROBABILITY; - } - - private boolean shouldSimulateSlowResponse() { - return shouldSimulateUnreliableNetwork && RANDOM_GENERATOR.nextDouble() <= NETWORK_DELAY_PROBABILITY; - } - - private RemoteResponse getNetworkFailedResponse() { - logger.info("Simulated network failure occurred!"); - return new RemoteResponse(HttpURLConnection.HTTP_INTERNAL_ERROR); - } - private boolean hasApiQuotaRemaining() { logger.info("Current quota left: {}", cloudRateLimitStatus.getQuotaRemaining()); return cloudRateLimitStatus.getQuotaRemaining() > 0; @@ -581,10 +479,7 @@ private boolean isExistingTag(List tagList, CloudTag targetTag) { private CloudPerson addPerson(List personList, CloudPerson newPerson) throws IllegalArgumentException { if (newPerson == null) throw new IllegalArgumentException("Person cannot be null"); - if (!newPerson.isValid()) { - throw new IllegalArgumentException("Fields cannot be null"); - } - if (isExistingPerson(personList, newPerson)) throw new IllegalArgumentException("Person already exists"); + if (!newPerson.isValid()) throw new IllegalArgumentException("Invalid person"); CloudPerson personToAdd = generateIdForPerson(personList, newPerson); personList.add(personToAdd); @@ -593,7 +488,7 @@ private CloudPerson addPerson(List personList, CloudPerson newPerso } private CloudPerson generateIdForPerson(List personList, CloudPerson newPerson) { - newPerson.setId(personList.size() + 1); // starts from one + newPerson.setId(personList.size() + 1); return newPerson; } @@ -604,8 +499,7 @@ private Optional getPerson(List personList, int person } private CloudPerson updatePersonDetails(List personList, List tagList, int personId, - CloudPerson updatedPerson) - throws NoSuchElementException { + CloudPerson updatedPerson) throws NoSuchElementException { CloudPerson oldPerson = getPersonIfExists(personList, personId); oldPerson.updatedBy(updatedPerson); @@ -624,72 +518,6 @@ private CloudPerson getPersonIfExists(List personList, int personId return personQueryResult.get(); } - private List mutateCloudPersonList(List CloudPersonList) { - modifyCloudPersonList(CloudPersonList); - addCloudPersonsBasedOnChance(CloudPersonList); - return CloudPersonList; - } - - private List mutateCloudTagList(List CloudTagList) { - modifyCloudTagListBasedOnChance(CloudTagList); - addCloudTagsBasedOnChance(CloudTagList); - return CloudTagList; - } - - private void modifyCloudPersonList(List cloudPersonList) { - cloudPersonList.stream() - .forEach(this::modifyCloudPersonBasedOnChance); - } - - private void modifyCloudTagListBasedOnChance(List cloudTagList) { - cloudTagList.stream() - .forEach(this::modifyCloudTagBasedOnChance); - } - - private void addCloudPersonsBasedOnChance(List personList) { - for (int i = 0; i < MAX_NUM_PERSONS_TO_ADD; i++) { - if (shouldSimulateUnreliableNetwork && RANDOM_GENERATOR.nextDouble() <= ADD_PERSON_PROBABILITY) { - CloudPerson person = new CloudPerson(java.util.UUID.randomUUID().toString(), - java.util.UUID.randomUUID().toString()); - logger.info("Cloud simulator: adding '{}'", person); - personList.add(person); - } - } - } - - private void addCloudTagsBasedOnChance(List tagList) { - for (int i = 0; i < MAX_NUM_PERSONS_TO_ADD; i++) { - if (shouldSimulateUnreliableNetwork && RANDOM_GENERATOR.nextDouble() <= ADD_TAG_PROBABILITY) { - CloudTag tag = new CloudTag(java.util.UUID.randomUUID().toString()); - logger.debug("Cloud simulator: adding tag '{}'", tag); - tagList.add(tag); - } - } - } - - private void modifyCloudPersonBasedOnChance(CloudPerson cloudPerson) { - if (!shouldSimulateUnreliableNetwork || RANDOM_GENERATOR.nextDouble() > MODIFY_PERSON_PROBABILITY) return; - logger.debug("Cloud simulator: modifying person '{}'", cloudPerson); - cloudPerson.setCity(java.util.UUID.randomUUID().toString()); - cloudPerson.setStreet(java.util.UUID.randomUUID().toString()); - cloudPerson.setPostalCode(String.valueOf(RANDOM_GENERATOR.nextInt(999999))); - } - - private void modifyCloudTagBasedOnChance(CloudTag cloudTag) { - if (!shouldSimulateUnreliableNetwork || RANDOM_GENERATOR.nextDouble() > MODIFY_TAG_PROBABILITY) return; - logger.debug("Cloud simulator: modifying tag '{}'", cloudTag); - cloudTag.setName(UUID.randomUUID().toString()); - } - - private void delayRandomAmount() { - long delayAmount = RANDOM_GENERATOR.nextInt(DELAY_RANGE) + MIN_DELAY_IN_SEC; - try { - TimeUnit.SECONDS.sleep(delayAmount); - } catch (InterruptedException e) { - logger.warn("Error occurred while delaying cloud response."); - } - } - private boolean isValidPageNumber(int dataSize, int pageNumber, int resourcesPerPage) { return pageNumber == 1 || getLastPageNumber(dataSize, resourcesPerPage) >= pageNumber; } @@ -706,7 +534,7 @@ private void deletePersonFromData(List personList, int personId) private CloudTag addTag(List tagList, CloudTag newTag) { if (newTag == null) throw new IllegalArgumentException("Tag cannot be null"); - if (!newTag.isValid()) throw new IllegalArgumentException("Fields cannot be null"); + if (!newTag.isValid()) throw new IllegalArgumentException("Invalid tag"); if (isExistingTag(tagList, newTag)) throw new IllegalArgumentException("Tag already exists"); tagList.add(newTag); return newTag; @@ -726,8 +554,7 @@ private CloudTag getTagIfExists(List tagList, String tagName) { } private CloudTag updateTagDetails(List personList, List tagList, String oldTagName, - CloudTag updatedTag) - throws NoSuchElementException { + CloudTag updatedTag) throws NoSuchElementException { CloudTag oldTag = getTagIfExists(tagList, oldTagName); oldTag.updatedBy(updatedTag); personList.stream() diff --git a/src/main/java/address/sync/cloud/ICloudSimulator.java b/src/main/java/address/sync/cloud/IRemote.java similarity index 97% rename from src/main/java/address/sync/cloud/ICloudSimulator.java rename to src/main/java/address/sync/cloud/IRemote.java index 203aa6d9..65f2a239 100644 --- a/src/main/java/address/sync/cloud/ICloudSimulator.java +++ b/src/main/java/address/sync/cloud/IRemote.java @@ -3,7 +3,7 @@ import address.sync.cloud.model.CloudPerson; import address.sync.cloud.model.CloudTag; -public interface ICloudSimulator { +public interface IRemote { RemoteResponse createPerson(String addressBookName, CloudPerson newPerson, String previousETag); RemoteResponse getPersons(String addressBookName, int pageNumber, int resourcesPerPage, String previousETag); RemoteResponse getUpdatedPersons(String addressBookName, String timeString, int pageNumber, int resourcesPerPage, String previousETag); diff --git a/src/main/java/address/sync/cloud/RemoteResponse.java b/src/main/java/address/sync/cloud/RemoteResponse.java index e00a9695..ee4acbf4 100644 --- a/src/main/java/address/sync/cloud/RemoteResponse.java +++ b/src/main/java/address/sync/cloud/RemoteResponse.java @@ -16,6 +16,14 @@ import java.util.Formatter; import java.util.HashMap; +/** + * This class is meant to mimic the response of a GitHub request + * + * Construction of an object will use up an API quota in the given cloudRateLimitStatus if the previousETag is not + * provided or is found to be different to the given object's eTag + * + * RemoteResponse instances obtained via other means e.g. RemoteResponse.getForbiddenResponse should not use up quota + */ public class RemoteResponse { private static final AppLogger logger = LoggerManager.getLogger(RemoteResponse.class); @@ -27,6 +35,41 @@ public class RemoteResponse { private int firstPageNo; private int lastPageNo; + public RemoteResponse(int responseCode, Object body, CloudRateLimitStatus cloudRateLimitStatus, String previousETag) { + String newETag = getETag(convertToInputStream(body)); + + if (previousETag != null && previousETag.equals(newETag)) { + this.responseCode = HttpURLConnection.HTTP_NOT_MODIFIED; + this.headers = getRateLimitStatusHeader(cloudRateLimitStatus); + return; + } + + cloudRateLimitStatus.useQuota(1); + this.responseCode = responseCode; + this.headers = getHeaders(cloudRateLimitStatus, newETag); + this.body = convertToInputStream(body); + } + + private RemoteResponse(int responseCode, CloudRateLimitStatus cloudRateLimitStatus) { + this.responseCode = responseCode; + this.headers = getRateLimitStatusHeader(cloudRateLimitStatus); + this.body = convertToInputStream(getRateLimitStatusHeader(cloudRateLimitStatus)); + } + + private RemoteResponse(int responseCode, Object body, CloudRateLimitStatus cloudRateLimitStatus) { + this.responseCode = responseCode; + this.headers = getRateLimitStatusHeader(cloudRateLimitStatus); + this.body = convertToInputStream(body); + } + + public static RemoteResponse getForbiddenResponse(CloudRateLimitStatus cloudRateLimitStatus) { + return new RemoteResponse(HttpURLConnection.HTTP_FORBIDDEN, null, cloudRateLimitStatus); + } + + public static RemoteResponse getLimitStatusResponse(CloudRateLimitStatus cloudRateLimitStatus) { + return new RemoteResponse(HttpURLConnection.HTTP_OK, cloudRateLimitStatus); + } + public int getNextPageNo() { return nextPageNo; } @@ -59,26 +102,6 @@ public void setLastPageNo(int lastPageNo) { this.lastPageNo = lastPageNo; } - private void addETagToHeader(HashMap header, String eTag) { - header.put("ETag", eTag); - } - - public RemoteResponse(int responseCode, Object body, HashMap header) { - this.responseCode = responseCode; - if (body != null) { - this.body = convertToInputStream(body); - addETagToHeader(header, getETag(convertToInputStream(body))); - } - this.headers = header; - } - - public RemoteResponse(int responseCode) { - assert responseCode == HttpURLConnection.HTTP_INTERNAL_ERROR : "RemoteResponse constructor misused"; - this.responseCode = responseCode; - this.headers = new HashMap<>(); - } - - public int getResponseCode() { return responseCode; } @@ -94,13 +117,14 @@ public HashMap getHeaders() { /** * Calculates the hash of the input stream if it has content * - * The input stream will be digested. Caller should clone or + * WARNING: The input stream will be digested. Caller should clone or * duplicate the stream before calling this method. * * @param bodyStream * @return */ - public static String getETag(InputStream bodyStream) { + private String getETag(InputStream bodyStream) { + if (bodyStream == null) return null; try { // Adapted from http://www.javacreed.com/how-to-compute-hash-code-of-streams/ DigestInputStream digestInputStream = new DigestInputStream(new BufferedInputStream(bodyStream), @@ -122,11 +146,30 @@ public static String getETag(InputStream bodyStream) { } } - private static ByteArrayInputStream convertToInputStream(Object object) { + private void addETagToHeader(HashMap header, String eTag) { + header.put("ETag", eTag); + } + + private HashMap getRateLimitStatusHeader(CloudRateLimitStatus cloudRateLimitStatus) { + HashMap headers = new HashMap<>(); + headers.put("X-RateLimit-Limit", String.valueOf(cloudRateLimitStatus.getQuotaLimit())); + headers.put("X-RateLimit-Remaining", String.valueOf(cloudRateLimitStatus.getQuotaRemaining())); + headers.put("X-RateLimit-Reset", String.valueOf(cloudRateLimitStatus.getQuotaReset())); + return headers; + } + + private HashMap getHeaders(CloudRateLimitStatus cloudRateLimitStatus, String eTag) { + HashMap headers = getRateLimitStatusHeader(cloudRateLimitStatus); + addETagToHeader(headers, eTag); + return headers; + } + + private ByteArrayInputStream convertToInputStream(Object object) { + if (object == null) return null; try { return new ByteArrayInputStream(JsonUtil.toJsonString(object).getBytes()); } catch (JsonProcessingException e) { - e.printStackTrace(); + logger.warn("Error converting object {} to input stream", object); return null; } } diff --git a/src/main/java/address/sync/cloud/model/CloudPerson.java b/src/main/java/address/sync/cloud/model/CloudPerson.java index 528e8a72..d65d05a0 100644 --- a/src/main/java/address/sync/cloud/model/CloudPerson.java +++ b/src/main/java/address/sync/cloud/model/CloudPerson.java @@ -27,15 +27,32 @@ public class CloudPerson { private LocalDate birthday; public CloudPerson() { + this.id = 0; + this.tags = new ArrayList<>(); + this.firstName = ""; + this.lastName = ""; + this.street = ""; + this.city = ""; + this.postalCode = ""; + this.isDeleted = false; } public CloudPerson(String firstName, String lastName) { + this(); this.firstName = firstName; this.lastName = lastName; - this.tags = new ArrayList<>(); setLastUpdatedAt(LocalDateTime.now()); } + public CloudPerson(String firstName, String lastName, int id) { + this(firstName, lastName); + setId(id); + } + + public CloudPerson(CloudPerson cloudPerson) { + updatedBy(cloudPerson); + } + public int getId() { return id; } diff --git a/src/main/java/address/sync/cloud/model/CloudTag.java b/src/main/java/address/sync/cloud/model/CloudTag.java index 3ce41b35..4107b3ba 100644 --- a/src/main/java/address/sync/cloud/model/CloudTag.java +++ b/src/main/java/address/sync/cloud/model/CloudTag.java @@ -22,6 +22,10 @@ public CloudTag(String name) { setLastUpdatedAt(LocalDateTime.now()); } + public CloudTag(CloudTag cloudTag) { + updatedBy(cloudTag); + } + @XmlElement(name = "name") public String getName() { return name; diff --git a/src/main/java/address/sync/task/GetUpdatesFromRemoteTask.java b/src/main/java/address/sync/task/GetUpdatesFromRemoteTask.java index 5dffb62c..2f67e67a 100644 --- a/src/main/java/address/sync/task/GetUpdatesFromRemoteTask.java +++ b/src/main/java/address/sync/task/GetUpdatesFromRemoteTask.java @@ -38,9 +38,9 @@ public void run() { } try { List updatedPersons = getUpdatedPersons(syncActiveAddressBookName.get()); - logger.logList("Found updated persons: {}", updatedPersons); - List latestTags = getLatestTags(syncActiveAddressBookName.get()); - logger.logList("Found latest tags: {}", latestTags); + logger.debug("Updated persons: {}", updatedPersons); + Optional> latestTags = getLatestTags(syncActiveAddressBookName.get()); + logger.debug("Latest tags: {}", latestTags); eventRaiser.accept(new SyncCompletedEvent(updatedPersons, latestTags)); } catch (SyncErrorException e) { @@ -61,12 +61,8 @@ public void run() { */ private List getUpdatedPersons(String addressBookName) throws SyncErrorException { try { - Optional> updatedPersons; - updatedPersons = remoteManager.getUpdatedPersons(addressBookName); - + Optional> updatedPersons = remoteManager.getUpdatedPersons(addressBookName); if (!updatedPersons.isPresent()) throw new SyncErrorException("getUpdatedPersons failed."); - - logger.logList("Updated persons: {}", updatedPersons.get()); return updatedPersons.get(); } catch (IOException e) { throw new SyncErrorException("Error getting updated persons."); @@ -80,17 +76,9 @@ private List getUpdatedPersons(String addressBookName) throws SyncErrorE * @return * @throws SyncErrorException if bad response code, missing data or network error */ - private List getLatestTags(String addressBookName) throws SyncErrorException { + private Optional> getLatestTags(String addressBookName) throws SyncErrorException { try { - Optional> latestTags = remoteManager.getLatestTagList(addressBookName); - - if (!latestTags.isPresent()) { - logger.info("No updates to tags."); - return null; - } else { - logger.logList("Latest tags: {}", latestTags.get()); - return latestTags.get(); - } + return remoteManager.getLatestTagList(addressBookName); } catch (IOException e) { throw new SyncErrorException("Error getting latest tags."); } diff --git a/src/main/java/address/ui/CommandInfoListViewCell.java b/src/main/java/address/ui/CommandInfoListViewCell.java new file mode 100644 index 00000000..74e22f6e --- /dev/null +++ b/src/main/java/address/ui/CommandInfoListViewCell.java @@ -0,0 +1,22 @@ +package address.ui; + +import address.controller.ActivityHistoryCardController; +import address.model.CommandInfo; +import javafx.scene.control.ListCell; + +/** + * + */ +public class CommandInfoListViewCell extends ListCell { + + @Override + protected void updateItem(CommandInfo item, boolean empty) { + super.updateItem(item, empty); + if (item == null || empty) { + setGraphic(null); + setText(""); + } else { + setGraphic(new ActivityHistoryCardController(item).getLayout()); + } + } +} diff --git a/src/main/java/address/ui/Ui.java b/src/main/java/address/ui/Ui.java index 9bd363df..a300a3d6 100644 --- a/src/main/java/address/ui/Ui.java +++ b/src/main/java/address/ui/Ui.java @@ -1,10 +1,11 @@ package address.ui; import address.MainApp; -import address.browser.BrowserManager; import address.controller.MainController; import address.model.ModelManager; +import address.model.UserPrefs; import address.util.Config; +import address.util.GuiSettings; import javafx.scene.control.Alert; import javafx.stage.Stage; @@ -13,9 +14,11 @@ */ public class Ui { MainController mainController; + UserPrefs pref; - public Ui(MainApp mainApp, ModelManager modelManager, Config config){ - mainController = new MainController(mainApp, modelManager, config); + public Ui(MainApp mainApp, ModelManager modelManager, Config config, UserPrefs pref){ + mainController = new MainController(mainApp, modelManager, config, pref); + this.pref = pref; } public void start(Stage primaryStage) { @@ -27,6 +30,10 @@ public void showAlertDialogAndWait(Alert.AlertType alertType, String alertTitle, } public void stop() { + Stage stage = mainController.getPrimaryStage(); + GuiSettings guiSettings = new GuiSettings(stage.getWidth(), stage.getHeight(), + (int)stage.getX(), (int)stage.getY()); + pref.setGuiSettings(guiSettings); mainController.stop(); } } diff --git a/src/main/java/address/updater/BackupHandler.java b/src/main/java/address/updater/BackupHandler.java index c87ee842..0196d4c5 100644 --- a/src/main/java/address/updater/BackupHandler.java +++ b/src/main/java/address/updater/BackupHandler.java @@ -8,8 +8,11 @@ import java.io.File; import java.io.IOException; +import java.io.InputStream; import java.net.URISyntaxException; +import java.nio.file.Files; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -21,10 +24,13 @@ public class BackupHandler { private static final AppLogger logger = LoggerManager.getLogger(BackupHandler.class); private static final int MAX_BACKUP_JAR_KEPT = 3; + private static final String BACKUP_DIR = "past_versions"; private static final String BACKUP_MARKER = "_"; private static final String BACKUP_FILENAME_STRING_FORMAT = "addressbook" + BACKUP_MARKER + "%s.jar"; private static final String BACKUP_FILENAME_PATTERN_STRING = "addressbook" + BACKUP_MARKER + "(" + Version.VERSION_PATTERN_STRING + ")\\.(jar|JAR)$"; + private static final String BACKUP_INSTRUCTION_FILENAME = "Instruction to use past versions.txt"; + private static final String BACKUP_INSTRUCTION_RESOURCE_PATH = "updater/Instruction to use past versions.txt"; private final Version currentVersion; private DependencyHistoryHandler dependencyHistoryHandler; @@ -47,16 +53,49 @@ public void createBackupOfApp(Version version) throws IOException, URISyntaxExce return; } + try { + createBackupDirIfMissing(); + } catch (IOException e) { + logger.debug("Failed to create backup directory", e); + throw e; + } + + extractInstructionToUseBackupVersion(); + String backupFilename = getBackupFilename(version); try { - FileUtil.copyFile(mainAppJar.toPath(), Paths.get(backupFilename), true); + FileUtil.copyFile(mainAppJar.toPath(), Paths.get(BACKUP_DIR, backupFilename), true); } catch (IOException e) { logger.debug("Failed to create backup", e); throw e; } } + private void createBackupDirIfMissing() throws IOException { + File backupDir = new File(BACKUP_DIR); + + if (!FileUtil.isDirExists(backupDir)) { + FileUtil.createDirs(new File(BACKUP_DIR)); + } + } + + private void extractInstructionToUseBackupVersion() throws IOException { + File backupInstructionFile = new File(BACKUP_INSTRUCTION_FILENAME); + + if (!backupInstructionFile.exists() && !backupInstructionFile.createNewFile()) { + throw new IOException("Failed to create backup instruction empty file"); + } + + try (InputStream in = + BackupHandler.class.getClassLoader().getResourceAsStream(BACKUP_INSTRUCTION_RESOURCE_PATH)) { + Files.copy(in, backupInstructionFile.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + logger.debug("Failed to extract backup instruction"); + throw e; + } + } + private boolean isRunFromBackupJar(String jarName) { return jarName.contains(BACKUP_MARKER); } @@ -69,6 +108,7 @@ private String getBackupFilename(Version version) { * Assumes user won't change backup filenames */ public void cleanupBackups() { + logger.debug("Cleaning backups"); if (dependencyHistoryHandler.getCurrentVersionDependencies() == null || dependencyHistoryHandler.getCurrentVersionDependencies().isEmpty()) { @@ -85,7 +125,7 @@ public void cleanupBackups() { logger.debug("Deleting {}", allBackupFilenames.get(i)); try { - FileUtil.deleteFile(allBackupFilenames.get(i)); + FileUtil.deleteFile(BACKUP_DIR + File.separator + allBackupFilenames.get(i)); deletedVersions.add(getVersionOfBackupFileFromFilename(allBackupFilenames.get(i))); } catch (IOException e) { logger.warn("Failed to delete old backup file: {}", e); @@ -123,20 +163,24 @@ public void cleanupBackups() { * @return all backup filenames aside from current app sorted from lowest version to latest */ private List getAllBackupFilenamesAsideFromCurrent() { - File currDirectory = new File("."); + File backupDir = new File(BACKUP_DIR); - File[] filesInCurrentDirectory = currDirectory.listFiles(); + if (!FileUtil.isDirExists(backupDir)) { + logger.debug("No backup directory"); + return new ArrayList<>(); + } - if (filesInCurrentDirectory == null) { - // current directory always exists - assert false; + File[] backupFiles = backupDir.listFiles(); + + if (backupFiles == null) { + logger.debug("No backup files found"); return new ArrayList<>(); } - List listOfFilesInCurrDirectory = new ArrayList<>(Arrays.asList(filesInCurrentDirectory)); + List listOfBackupFiles = new ArrayList<>(Arrays.asList(backupFiles)); // Exclude current version in case user is running backup Jar - return listOfFilesInCurrDirectory.stream() + return listOfBackupFiles.stream() .filter(f -> !f.getName().equals(getBackupFilename(currentVersion)) && f.getName().matches(BACKUP_FILENAME_PATTERN_STRING)) diff --git a/src/main/java/address/updater/UpdateManager.java b/src/main/java/address/updater/UpdateManager.java index ae2279c9..d5016ede 100644 --- a/src/main/java/address/updater/UpdateManager.java +++ b/src/main/java/address/updater/UpdateManager.java @@ -53,6 +53,8 @@ public class UpdateManager extends ComponentManager { private static final String VERSION_DESCRIPTOR_ON_SERVER_EARLY = "https://raw.githubusercontent.com/HubTurbo/addressbook/early-access/UpdateData.json"; private static final File VERSION_DESCRIPTOR_FILE = new File(UPDATE_DIR + File.separator + "UpdateData.json"); + private static final String LIB_DIR = "lib" + File.separator; + private static final String MAIN_APP_FILEPATH = LIB_DIR + "resource.jar"; private final ExecutorService pool = Executors.newCachedThreadPool(); private final DependencyHistoryHandler dependencyHistoryHandler; @@ -161,7 +163,7 @@ private void checkForUpdate() { raise(new UpdaterFinishedEvent("Update will be applied on next launch")); this.isUpdateApplicable = true; - updateDownloadedVersionsData(latestVersion.get()); + downloadedVersions.add(latestVersion.get()); } /** @@ -227,12 +229,13 @@ private HashMap collectAllUpdateFilesToBeDownloaded(VersionDescript mainAppDownloadLink = versionDescriptor.getDownloadLinkForMainApp(); - filesToBeDownloaded.put("addressbook.jar", mainAppDownloadLink); + filesToBeDownloaded.put(MAIN_APP_FILEPATH, mainAppDownloadLink); versionDescriptor.getLibraries().stream() .filter(libDesc -> libDesc.getOs() == OsDetector.Os.ANY || libDesc.getOs() == OsDetector.getOs()) - .filter(libDesc -> !FileUtil.isFileExists("lib/" + libDesc.getFilename())) - .forEach(libDesc -> filesToBeDownloaded.put("lib/" + libDesc.getFilename(), libDesc.getDownloadLink())); + .filter(libDesc -> !FileUtil.isFileExists(LIB_DIR + libDesc.getFilename())) + .forEach(libDesc -> filesToBeDownloaded.put(LIB_DIR + libDesc.getFilename(), + libDesc.getDownloadLink())); return filesToBeDownloaded; } @@ -299,11 +302,6 @@ private void extractJarUpdater() throws IOException { } } - private void updateDownloadedVersionsData(Version latestVersionDownloaded) { - downloadedVersions.add(latestVersionDownloaded); - writeDownloadedVersionsToFile(); - } - private void writeDownloadedVersionsToFile() { try { if (FileUtil.isFileExists(DOWNLOADED_VERSIONS_FILE.toString())) { @@ -345,6 +343,8 @@ public void applyUpdate() { return; } + writeDownloadedVersionsToFile(); + String restarterAppPath = JAR_UPDATER_APP_PATH; String localUpdateSpecFilepath = LocalUpdateSpecificationHelper.getLocalUpdateSpecFilepath(); String cmdArg = String.format("--update-specification=%s --source-dir=%s", localUpdateSpecFilepath, UPDATE_DIR); diff --git a/src/main/java/address/util/Config.java b/src/main/java/address/util/Config.java index c0991dd7..fe99e51e 100644 --- a/src/main/java/address/util/Config.java +++ b/src/main/java/address/util/Config.java @@ -5,50 +5,53 @@ import java.io.File; import java.util.HashMap; +import java.util.Optional; /** * Config values used by the app */ public class Config { - private static final String CONFIG_FILE = "config.ini"; - // Default values private static final long DEFAULT_UPDATE_INTERVAL = 10000; private static final Level DEFAULT_LOGGING_LEVEL = Level.INFO; - private static final boolean DEFAULT_NETWORK_UNRELIABLE_MODE = false; private static final HashMap DEFAULT_SPECIAL_LOG_LEVELS = new HashMap<>(); private static final int DEFAULT_BROWSER_NO_OF_PAGES = 3; private static final BrowserType DEFAULT_BROWSER_TYPE = BrowserType.FULL_FEATURE_BROWSER; + private static final String DEFAULT_LOCAL_DATA_FILE_PATH = "test.xml"; + private static final String DEFAULT_CLOUD_DATA_FILE_PATH = null; // For use in CloudManipulator for manual testing + private static final String DEFAULT_ADDRESS_BOOK_NAME = "MyAddressBook"; // Config values - public String appTitle = "Address App"; + private String appTitle = "Address App"; // Customizable through config file - public long updateInterval = DEFAULT_UPDATE_INTERVAL; - public boolean simulateUnreliableNetwork = DEFAULT_NETWORK_UNRELIABLE_MODE; + private long updateInterval = DEFAULT_UPDATE_INTERVAL; public Level currentLogLevel = DEFAULT_LOGGING_LEVEL; public HashMap specialLogLevels = DEFAULT_SPECIAL_LOG_LEVELS; private File prefsFileLocation = new File("preferences.json"); //Default user preferences file public int browserNoOfPages = DEFAULT_BROWSER_NO_OF_PAGES; public BrowserType browserType = DEFAULT_BROWSER_TYPE; + private String localDataFilePath = DEFAULT_LOCAL_DATA_FILE_PATH; + private String cloudDataFilePath = DEFAULT_CLOUD_DATA_FILE_PATH; + private String addressBookName = DEFAULT_ADDRESS_BOOK_NAME; public Config() { } - public long getUpdateInterval() { - return updateInterval; + public String getAppTitle() { + return appTitle; } - public void setUpdateInterval(long updateInterval) { - this.updateInterval = updateInterval; + public void setAppTitle(String appTitle) { + this.appTitle = appTitle; } - public boolean isSimulateUnreliableNetwork() { - return simulateUnreliableNetwork; + public long getUpdateInterval() { + return updateInterval; } - public void setSimulateUnreliableNetwork(boolean simulateUnreliableNetwork) { - this.simulateUnreliableNetwork = simulateUnreliableNetwork; + public void setUpdateInterval(long updateInterval) { + this.updateInterval = updateInterval; } public Level getCurrentLogLevel() { @@ -86,4 +89,30 @@ public BrowserType getBrowserType() { public void setBrowserType(BrowserType browserType) { this.browserType = browserType; } + + public String getLocalDataFilePath() { + return localDataFilePath; + } + + public void setLocalDataFilePath(String localDataFilePath) { + this.localDataFilePath = localDataFilePath; + } + + public String getCloudDataFilePath() { + return cloudDataFilePath; + } + + public void setCloudDataFilePath(String cloudDataFilePath) { + this.cloudDataFilePath = cloudDataFilePath; + } + + public String getAddressBookName() { + return addressBookName; + } + + public void setAddressBookName(String addressBookName) { + this.addressBookName = addressBookName; + } + + } diff --git a/src/main/java/address/util/FileUtil.java b/src/main/java/address/util/FileUtil.java index d5c730bc..4c468c06 100644 --- a/src/main/java/address/util/FileUtil.java +++ b/src/main/java/address/util/FileUtil.java @@ -192,4 +192,16 @@ public static String getPath(String pathWithForwardSlash) { assert pathWithForwardSlash.contains("/"); return pathWithForwardSlash.replace("/", File.separator); } + + /** + * Gets the file name from the given file path, assuming that + * path components are '/'-separated + * + * @param filePath should not be null + * @return + */ + public static String getFileName(String filePath) { + String[] pathComponents = filePath.split("/"); + return pathComponents[pathComponents.length - 1]; + } } diff --git a/src/main/java/address/util/GuiSettings.java b/src/main/java/address/util/GuiSettings.java new file mode 100644 index 00000000..6d153003 --- /dev/null +++ b/src/main/java/address/util/GuiSettings.java @@ -0,0 +1,41 @@ +package address.util; + +import java.awt.*; +import java.io.Serializable; + +/** + * A Serializable class that contains the GUI settings. + */ +public class GuiSettings implements Serializable { + + public static final double DEFAULT_HEIGHT = 600; + public static final double DEFAULT_WIDTH = 740; + + private Double windowWidth; + private Double windowHeight; + private Point windowCoordinates; + + public GuiSettings() { + this.windowWidth = DEFAULT_WIDTH; + this.windowHeight = DEFAULT_HEIGHT; + this.windowCoordinates = null; // null represent no coordinates + } + + public GuiSettings(Double windowWidth, Double windowHeight, int xPosition, int yPosition) { + this.windowWidth = windowWidth; + this.windowHeight = windowHeight; + this.windowCoordinates = new Point(xPosition, yPosition); + } + + public Double getWindowWidth() { + return windowWidth; + } + + public Double getWindowHeight() { + return windowHeight; + } + + public Point getWindowCoordinates() { + return windowCoordinates; + } +} diff --git a/src/main/java/address/util/PlatformExecUtil.java b/src/main/java/address/util/PlatformExecUtil.java index 247aa014..ec7eddb1 100644 --- a/src/main/java/address/util/PlatformExecUtil.java +++ b/src/main/java/address/util/PlatformExecUtil.java @@ -23,7 +23,7 @@ public static void runLater(Runnable action) { */ public static Future call(Callable callback) { final FutureTask task = new FutureTask<>(callback); - if (Platform.isFxApplicationThread()) { + if (isFxThread()) { task.run(); } else { runLater(task); @@ -92,13 +92,17 @@ public static void runLaterAndWait(Runnable action) { */ public static void runAndWait(Runnable action) { assert action != null : "Non-null action required"; - if (Platform.isFxApplicationThread()) { + if (isFxThread()) { action.run(); return; } runLaterAndWait(action); } + public static boolean isFxThread() { + return Platform.isFxApplicationThread(); + } + private PlatformExecUtil() { } } diff --git a/src/main/java/hubturbo/embeddedbrowser/HyperBrowser.java b/src/main/java/hubturbo/embeddedbrowser/HyperBrowser.java index 8a744514..d18ff5b9 100644 --- a/src/main/java/hubturbo/embeddedbrowser/HyperBrowser.java +++ b/src/main/java/hubturbo/embeddedbrowser/HyperBrowser.java @@ -55,7 +55,7 @@ public HyperBrowser(BrowserType browserType, int noOfPages, Optional initi private void initialiseHyperBrowser(){ this.hyperBrowserView = new AnchorPane(); - + FxViewUtil.applyAnchorBoundaryParameters(hyperBrowserView, 0.0, 0.0, 0.0, 0.0); if (initialScreen.isPresent()) { hyperBrowserView.getChildren().add(initialScreen.get()); } diff --git a/src/main/java/hubturbo/updater/Installer.java b/src/main/java/hubturbo/updater/Installer.java index 7e7eea6e..27fbe8ea 100644 --- a/src/main/java/hubturbo/updater/Installer.java +++ b/src/main/java/hubturbo/updater/Installer.java @@ -44,6 +44,7 @@ public class Installer extends Application { private static final String ERROR_RUNNING = "Failed to run application"; private static final String ERROR_TRY_AGAIN = "Please try again, or contact developer if it keeps failing."; private static final String LIB_DIR = "lib"; + private static final Path MAIN_APP_FILEPATH = Paths.get(LIB_DIR, new File("resource.jar").getName()); private final ExecutorService pool = Executors.newSingleThreadExecutor(); private ProgressBar progressBar; @@ -139,6 +140,17 @@ private void unpackAllJarsInsideSelf() throws IOException, URISyntaxException { String filename = jarEntry.getName(); Path extractDest = Paths.get(LIB_DIR, new File(filename).getName()); + // For MainApp resource, only extract if it is not present + if (filename.startsWith("resource") && filename.endsWith(".jar")) { + if (!MAIN_APP_FILEPATH.toFile().exists()) { + try (InputStream in = jar.getInputStream(jarEntry)) { + Files.copy(in, MAIN_APP_FILEPATH, StandardCopyOption.REPLACE_EXISTING); + } + } + continue; + } + + // For other JARs, extract if existing files are of different sizes if (filename.endsWith(".jar") && jarEntry.getSize() != extractDest.toFile().length()) { try (InputStream in = jar.getInputStream(jarEntry)) { Files.copy(in, extractDest, StandardCopyOption.REPLACE_EXISTING); @@ -146,7 +158,7 @@ private void unpackAllJarsInsideSelf() throws IOException, URISyntaxException { } } } catch (IOException e) { - System.out.println("Failed to extract jar updater"); + System.out.println("Failed to extract libraries"); throw e; } diff --git a/src/main/java/hubturbo/updater/UpdateDataGenerator.java b/src/main/java/hubturbo/updater/UpdateDataGenerator.java index 120d782e..eb668035 100644 --- a/src/main/java/hubturbo/updater/UpdateDataGenerator.java +++ b/src/main/java/hubturbo/updater/UpdateDataGenerator.java @@ -6,11 +6,11 @@ import address.updater.VersionDescriptor; import address.util.FileUtil; import address.util.JsonUtil; -import address.util.OsDetector; import java.io.File; import java.io.IOException; import java.net.MalformedURLException; +import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -20,8 +20,8 @@ * Used to help developer create release in generating the update data */ public class UpdateDataGenerator { - private static final String MAIN_APP_BASE_DOWNLOAD_LINK = - "https://github.com/HubTurbo/addressbook/releases/download/"; + private static final String BASE_DOWNLOAD_LINK = + "https://github.com/HubTurbo/addressbook/releases/download/Resources/"; private static final File UPDATE_DATA_FILE = new File("UpdateData.json"); public static void main(String[] args) { @@ -49,12 +49,15 @@ public static void main(String[] args) { } ArrayList currentLibrariesName = new ArrayList<>(arguments.subList(1, arguments.size())); + ArrayList currentLibraryDescriptors = currentLibrariesName.stream() - .map(libName -> new LibraryDescriptor(libName, null, OsDetector.Os.ANY)) + .map(libName -> new LibraryDescriptor(libName, null, null)) .collect(Collectors.toCollection(ArrayList::new)); - populateCurrLibDescriptorWithExistingDownloadLink(previousVersionDescriptor.getLibraries(), - currentLibraryDescriptors); + populateCurrLibDescriptorDownloadLink(currentLibraryDescriptors); + + populateCurrLibDescriptorWithExistingLibDescriptorOs(previousVersionDescriptor.getLibraries(), + currentLibraryDescriptors); versionDescriptor.setLibraries(currentLibraryDescriptors); @@ -75,13 +78,28 @@ private static VersionDescriptor getPreviousUpdateData() throws IOException { private static void setUpdateDataMainAppDownloadLink(VersionDescriptor versionDescriptor, String mainAppFilename) throws MalformedURLException { - String mainAppDownloadLinkString = MAIN_APP_BASE_DOWNLOAD_LINK + MainApp.VERSION.toString() + "/" + - mainAppFilename; + String mainAppDownloadLinkString = BASE_DOWNLOAD_LINK + mainAppFilename; versionDescriptor.setMainAppDownloadLink(mainAppDownloadLinkString); } - private static void populateCurrLibDescriptorWithExistingDownloadLink( + private static URL getDownloadLinkForLib(String libFilename) throws MalformedURLException { + return new URL(BASE_DOWNLOAD_LINK + libFilename); + } + + private static void populateCurrLibDescriptorDownloadLink(ArrayList currentLibraryDescriptors) { + currentLibraryDescriptors.stream().forEach(libDesc -> { + try { + libDesc.setDownloadLink(getDownloadLinkForLib(libDesc.getFilename())); + } catch (MalformedURLException e) { + e.printStackTrace(); + System.out.println("Failed to set download link for " + libDesc.getFilename() + + "; please update the download link manually"); + } + }); + } + + private static void populateCurrLibDescriptorWithExistingLibDescriptorOs( ArrayList previousLibraryDescriptors, ArrayList currentLibraryDescriptors) { currentLibraryDescriptors.stream() @@ -89,16 +107,13 @@ private static void populateCurrLibDescriptorWithExistingDownloadLink( previousLibraryDescriptors.stream() .filter(prevLibDesc -> prevLibDesc.getFilename().equals(libDesc.getFilename())) .findFirst() - .ifPresent(prevLibDesc -> { - libDesc.setDownloadLink(prevLibDesc.getDownloadLink()); - libDesc.setOs(prevLibDesc.getOs()); - })); + .ifPresent(prevLibDesc -> libDesc.setOs(prevLibDesc.getOs()))); } private static void notifyOfNewLibrariesToBeGivenMoreInformation(ArrayList libraryDescriptors) { System.out.println("------------------------------------------------------------"); System.out.println("New libraries to be uploaded, given download URL and set OS:"); - libraryDescriptors.stream().filter(libDesc -> libDesc.getDownloadLink() == null) + libraryDescriptors.stream().filter(libDesc -> libDesc.getOs() == null) .forEach(libDesc -> System.out.println(libDesc.getFilename())); System.out.println("------------------------------------------------------------"); } diff --git a/src/main/resources/images/clock.png b/src/main/resources/images/clock.png new file mode 100644 index 00000000..0807cbf6 Binary files /dev/null and b/src/main/resources/images/clock.png differ diff --git a/src/main/resources/images/fail.png b/src/main/resources/images/fail.png new file mode 100644 index 00000000..6daf0129 Binary files /dev/null and b/src/main/resources/images/fail.png differ diff --git a/src/main/resources/images/help_icon.png b/src/main/resources/images/help_icon.png new file mode 100644 index 00000000..f8e80d6c Binary files /dev/null and b/src/main/resources/images/help_icon.png differ diff --git a/src/main/resources/images/info_icon.png b/src/main/resources/images/info_icon.png new file mode 100644 index 00000000..f8cef714 Binary files /dev/null and b/src/main/resources/images/info_icon.png differ diff --git a/src/main/resources/images/sync_icon.png b/src/main/resources/images/sync_icon.png deleted file mode 100644 index 4d69b8f6..00000000 Binary files a/src/main/resources/images/sync_icon.png and /dev/null differ diff --git a/src/main/resources/updater/Instruction to use past versions.txt b/src/main/resources/updater/Instruction to use past versions.txt new file mode 100644 index 00000000..70c0e07d --- /dev/null +++ b/src/main/resources/updater/Instruction to use past versions.txt @@ -0,0 +1 @@ +If the latest version does not work for some reason, you may want to use a past version until the latest version is fixed. In such a case, you can copy the JAR file of the version you want to use from the past_versions folder to the root folder (no need to rename it) and run that JAR file. \ No newline at end of file diff --git a/src/main/resources/view/ActivityHistory.fxml b/src/main/resources/view/ActivityHistory.fxml new file mode 100644 index 00000000..13d2d654 --- /dev/null +++ b/src/main/resources/view/ActivityHistory.fxml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/src/main/resources/view/ActivityHistoryCard.fxml b/src/main/resources/view/ActivityHistoryCard.fxml new file mode 100644 index 00000000..bfbf1acb --- /dev/null +++ b/src/main/resources/view/ActivityHistoryCard.fxml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/src/main/resources/view/DarkTheme.css b/src/main/resources/view/DarkTheme.css index 9bc57cc8..465e4fe6 100644 --- a/src/main/resources/view/DarkTheme.css +++ b/src/main/resources/view/DarkTheme.css @@ -245,6 +245,14 @@ #pendingStateLabel { -fx-font-size: 11px; -fx-text-fill: #F70D1A; +} + +#pendingCountdownIndicator { + -fx-font-size: 11px; + -fx-text-fill: #F70D1A; +} + +#pendingStateHolder { -fx-background-radius: 5; -fx-background-color: white; -fx-padding-background-color: transparent; @@ -252,3 +260,7 @@ -fx-effect: dropshadow(three-pass-box, rgba(0,0,0,0.8), 10, 0, 0, 0); } +#activityLabel { + -fx-font-size: 12px; +} + diff --git a/src/main/resources/view/Help.fxml b/src/main/resources/view/Help.fxml new file mode 100644 index 00000000..3853f03d --- /dev/null +++ b/src/main/resources/view/Help.fxml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/src/main/resources/view/PersonListCard.fxml b/src/main/resources/view/PersonListCard.fxml index 23742a06..2c119bc9 100644 --- a/src/main/resources/view/PersonListCard.fxml +++ b/src/main/resources/view/PersonListCard.fxml @@ -1,5 +1,6 @@ + @@ -9,13 +10,13 @@ - + - + @@ -27,7 +28,6 @@ - @@ -35,17 +35,22 @@ diff --git a/src/main/resources/view/PersonOverview.fxml b/src/main/resources/view/PersonOverview.fxml index 9a92c73e..ba359e77 100644 --- a/src/main/resources/view/PersonOverview.fxml +++ b/src/main/resources/view/PersonOverview.fxml @@ -6,24 +6,20 @@ - + - + + + - - - - - + + + + + + + + + + + + + + +

+ +
+
+ + +
+ +
+ + +
+
+
+ + + +

+
Michael
+
1
+

+ + + + + + + + + +
+ +
+ + +
+
+ + + + + +
+ +
+
+ + +
+ + + +
+ + + + +
+ + +
+ +
+
+ + +
+

+ 0 contributions in the last year +

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Jul + Aug + Sep + Oct + Nov + Dec + Jan + Feb + Mar + Apr + May + Jun + + M + + W + + F + + + + +
+ + +
+
+ +
+ +
+ + +
+ +

Contribution activity

+ + +
+
+

1 has no activity during this period.

+
+ + + + +
+ +
+ +
+ +
+ +
+
+ +
+
+ +
+ +
+ + + + + + + +
+ + + Something went wrong with that request. Please try again. +
+ + + + + + + + + + + + + + + +