From 67cd1efbc249809d314b0b0f052685b3a5ef3f31 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 30 May 2024 07:57:42 -0600 Subject: [PATCH 1/3] Better handle authorization problems Signed-off-by: Taylor Smock --- .../MapRouletteDownloadTaskBox.java | 23 ++-------------- .../josm/plugins/maproulette/api/TaskAPI.java | 1 + .../gui/task/list/LockUnlockTaskAction.java | 15 ----------- .../maproulette/util/ExceptionDialogUtil.java | 26 +++++++++++++++++- .../maproulette/util/OsmPreferenceUtils.java | 27 ++++++++++++------- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java index 1235499..7a6b9d3 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/actions/downloadtasks/MapRouletteDownloadTaskBox.java @@ -1,7 +1,6 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.maproulette.actions.downloadtasks; -import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl; import static org.openstreetmap.josm.tools.I18n.tr; import java.io.IOException; @@ -12,21 +11,14 @@ import java.util.function.Function; import java.util.stream.Collectors; -import javax.swing.JOptionPane; -import javax.swing.SwingUtilities; - import org.openstreetmap.josm.actions.downloadtasks.AbstractDownloadTask; import org.openstreetmap.josm.actions.downloadtasks.DownloadParams; import org.openstreetmap.josm.data.Bounds; -import org.openstreetmap.josm.data.UserIdentityManager; -import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; +import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.PleaseWaitRunnable; -import org.openstreetmap.josm.gui.preferences.PreferenceDialog; -import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference; import org.openstreetmap.josm.gui.progress.ProgressMonitor; import org.openstreetmap.josm.gui.progress.ProgressTaskId; -import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.io.OsmApiException; import org.openstreetmap.josm.io.OsmTransferException; import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI; @@ -96,18 +88,7 @@ protected void realRun() throws IOException, OsmTransferException { TaskCache.isHidden(task); } } catch (UnauthorizedException unauthorizedException) { - final var message = UserIdentityManager.getInstance().isAnonymous() - ? tr("Please log in to OpenStreetMap in JOSM") - : tr("Please log in to the MapRoulette instance at least once: {0}", getBaseUrl()); - GuiHelper.runInEDT(() -> { - ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in", - MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE); - if (UserIdentityManager.getInstance().isAnonymous()) { - final var p = new PreferenceDialog(MainApplication.getMainFrame()); - SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class)); - p.setVisible(true); - } - }); + ExceptionDialogUtil.explainException(unauthorizedException); // This is specifically so that user's don't get a bug report message final var transferException = new OsmApiException(unauthorizedException); transferException.setUrl(MapRouletteConfig.getBaseUrl()); diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java index 93d1cc1..80d6c12 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/api/TaskAPI.java @@ -119,6 +119,7 @@ public static Task get(long task) throws IOException { * @param task The task to start * @return The updated task * @throws IOException if there was a problem communicating with the server + * @throws UnauthorizedException If we aren't authorized for the server */ public static Task start(long task) throws IOException { final var client = HttpClientUtils.get(getBaseUrl() + TASK + "/" + task + "/start"); diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java index a245248..5e1cd49 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java @@ -8,29 +8,23 @@ import java.io.IOException; import java.io.Serial; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; -import javax.swing.JOptionPane; import javax.swing.JTable; import org.openstreetmap.josm.actions.AutoScaleAction; import org.openstreetmap.josm.actions.JosmAction; import org.openstreetmap.josm.data.osm.IPrimitive; -import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI; -import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException; import org.openstreetmap.josm.plugins.maproulette.api.enums.TaskStatus; import org.openstreetmap.josm.plugins.maproulette.api.model.Task; import org.openstreetmap.josm.plugins.maproulette.gui.ModifiedObjects; import org.openstreetmap.josm.plugins.maproulette.gui.layer.MapRouletteClusteredPointLayer; import org.openstreetmap.josm.plugins.maproulette.util.ExceptionDialogUtil; import org.openstreetmap.josm.tools.ImageProvider; -import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Shortcut; /** @@ -77,7 +71,6 @@ private void lockTasks() { final var selected = this.table.getSelectionModel().getSelectedIndices(); final var model = (TaskTableModel) this.table.getModel(); final var selectedTasks = new ArrayList(); - final var messages = new HashSet(); for (int index : selected) { try { final var i = table.getRowSorter().convertRowIndexToModel(index); @@ -85,18 +78,10 @@ private void lockTasks() { final var lockedTask = TaskAPI.start(task.id()); ModifiedObjects.addLockedTask(lockedTask); selectedTasks.add(lockedTask); - } catch (UnauthorizedException unauthorizedException) { - Logging.trace(unauthorizedException); - messages.add(unauthorizedException.getMessage()); } catch (IOException e) { ExceptionDialogUtil.explainException(e); } } - if (!messages.isEmpty()) { - ConditionalOptionPaneUtil.showMessageDialog("maproulette.tasks.locked", MainApplication.getMainFrame(), - "" + messages.stream().sorted().collect(Collectors.joining("
")) + "", - tr("Could not lock tasks"), JOptionPane.INFORMATION_MESSAGE); - } ((TaskTableModel) this.table.getModel()).fireTableDataChanged(); reselect(selected); diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java index f60eb09..10fe3a4 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/ExceptionDialogUtil.java @@ -2,6 +2,7 @@ package org.openstreetmap.josm.plugins.maproulette.util; import static org.openstreetmap.josm.gui.help.HelpUtil.ht; +import static org.openstreetmap.josm.plugins.maproulette.config.MapRouletteConfig.getBaseUrl; import static org.openstreetmap.josm.tools.I18n.tr; import java.awt.Component; @@ -10,9 +11,16 @@ import java.net.UnknownHostException; import javax.swing.JOptionPane; +import javax.swing.SwingUtilities; +import org.openstreetmap.josm.data.UserIdentityManager; +import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; import org.openstreetmap.josm.gui.HelpAwareOptionPane; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.preferences.PreferenceDialog; +import org.openstreetmap.josm.gui.preferences.server.ServerAccessPreference; +import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.plugins.maproulette.api.UnauthorizedException; import org.openstreetmap.josm.tools.ExceptionUtil; /** @@ -28,7 +36,23 @@ private ExceptionDialogUtil() { * @param exception The exception to explain to the user */ public static void explainException(Exception exception) { - if (exception instanceof SocketException socketException) { + if (exception instanceof UnauthorizedException) { + OsmPreferenceUtils.clearCachedKey(); + final var message = UserIdentityManager.getInstance().isAnonymous() + ? tr("Please log in to OpenStreetMap in JOSM") + : tr("Please log in to the MapRoulette instance at least once: {0}\n NOTE: you may need to reset your MapRoulette API key", + getBaseUrl()); + GuiHelper.runInEDT(() -> { + ConditionalOptionPaneUtil.showMessageDialog("maproulette.user.not.logged.in", + MainApplication.getMainFrame(), message, message, JOptionPane.ERROR_MESSAGE); + if (UserIdentityManager.getInstance().isAnonymous()) { + final var p = new PreferenceDialog(MainApplication.getMainFrame()); + SwingUtilities.invokeLater(() -> p.selectPreferencesTabByClass(ServerAccessPreference.class)); + p.setVisible(true); + } + }); + showErrorDialog(message, tr("Unauthorized"), null); + } else if (exception instanceof SocketException socketException) { final var message = tr("Failed to open a connection to the remote server
" + "''{0}''.
" + "Please check your internet connection.", socketException.getMessage()) + ""; showErrorDialog(message, tr("Network exception"), ht("/ErrorMessages#NestedSocketException")); diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java index c8fba86..3b33faf 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/util/OsmPreferenceUtils.java @@ -37,16 +37,11 @@ private OsmPreferenceUtils() { */ static String getMapRouletteApiKey() throws UnauthorizedException { final var user = UserIdentityManager.getInstance().getUserInfo(); - final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList()); if (user == null) { - // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users - for (var userId : userList.get()) { - final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + userId; - Config.getPref().put(preferenceKey, null); - } + clearCachedKey(); throw new UnauthorizedException("User is not logged in"); } - final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + "." + user.getId(); + final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + user.getId(); final var possibleApiKey = Config.getPref().get(preferenceKey); if (!Utils.isStripEmpty(possibleApiKey) && !"Couldn't authenticate you".equals(possibleApiKey)) { return possibleApiKey; @@ -55,12 +50,14 @@ static String getMapRouletteApiKey() throws UnauthorizedException { "maproulette_apikey_v2"); final var reader = new OsmServerUserPreferencesReader(); final var monitor = new PleaseWaitProgressMonitor(tr("Fetching OpenStreetMap User Preferences")); + final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList()); try { final var key = reader.fetchUserPreferences(monitor, tr("Getting MapRoulette API Key")) .getOrDefault(osmServerKey, null); + final String userId = String.valueOf(user.getId()); List userIds = new ArrayList<>(userList.get()); - if (!userIds.contains(String.valueOf(user.getId()))) { - userIds.add(String.valueOf(user.getId())); + if (!userIds.contains(userId)) { + userIds.add(userId); userList.put(userIds); } Config.getPref().put(preferenceKey, key); @@ -71,4 +68,16 @@ static String getMapRouletteApiKey() throws UnauthorizedException { monitor.close(); } } + + /** + * Remove all cached keys (this can happen due to auth failure) + */ + static void clearCachedKey() { + final var userList = new ListProperty("maproulette.openstreetmap.users", Collections.emptyList()); + // Right now JOSM doesn't support multiple users, so just wipe everything. If JOSM ever supports multiple users + for (var userId : userList.get()) { + final var preferenceKey = "maproulette.openstreetmap." + getBaseUrl() + '.' + userId; + Config.getPref().put(preferenceKey, null); + } + } } From 1f6b86cd6f13e03a8ee5702b8569686ce1f547ab Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 30 May 2024 08:18:20 -0600 Subject: [PATCH 2/3] Update task status on unlock, add task id to upload question Signed-off-by: Taylor Smock --- .../maproulette/gui/task/list/LockUnlockTaskAction.java | 3 +-- .../josm/plugins/maproulette/io/upload/EarlyUploadHook.java | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java index 5e1cd49..aa4f028 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/gui/task/list/LockUnlockTaskAction.java @@ -19,7 +19,6 @@ import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.plugins.maproulette.api.TaskAPI; -import org.openstreetmap.josm.plugins.maproulette.api.enums.TaskStatus; import org.openstreetmap.josm.plugins.maproulette.api.model.Task; import org.openstreetmap.josm.plugins.maproulette.gui.ModifiedObjects; import org.openstreetmap.josm.plugins.maproulette.gui.layer.MapRouletteClusteredPointLayer; @@ -112,7 +111,7 @@ private void unlockTasks() { final var unlockedTask = TaskAPI.release(cluster.id()); if (task != null && task.id() == unlockedTask.id()) { final var modified = ModifiedObjects.getModifiedTask(task.id()); - if (modified != null && modified.status() != TaskStatus.FIXED) { + if (modified != null) { TaskAPI.updateStatus(task.id(), modified.status(), modified.comment(), modified.tags(), modified.reviewRequested(), modified.completionResponses()); ModifiedObjects.removeModifiedTask(modified); diff --git a/src/main/java/org/openstreetmap/josm/plugins/maproulette/io/upload/EarlyUploadHook.java b/src/main/java/org/openstreetmap/josm/plugins/maproulette/io/upload/EarlyUploadHook.java index f2aa45d..03328ba 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/maproulette/io/upload/EarlyUploadHook.java +++ b/src/main/java/org/openstreetmap/josm/plugins/maproulette/io/upload/EarlyUploadHook.java @@ -107,8 +107,8 @@ public boolean checkUpload(APIDataSet apiDataSet) { final var descriptivePanel = createDescriptivePanel(task, apiDataSet); final var didFix = ConditionalOptionPaneUtil.showConfirmationDialog(PREF_CHECK_IF_FINISHED, MainApplication.getMainFrame(), descriptivePanel, - tr("Did you finish the following MapRoulette Task?"), JOptionPane.YES_NO_CANCEL_OPTION, - JOptionPane.QUESTION_MESSAGE, JOptionPane.YES_OPTION); + tr("Did you finish the following MapRoulette Task: {0}?", task.id()), + JOptionPane.YES_NO_CANCEL_OPTION, JOptionPane.QUESTION_MESSAGE, JOptionPane.YES_OPTION); if (didFix) { final var doc = (HTMLDocument) ((JosmEditorPane) descriptivePanel.getComponent(1)).getDocument(); ModifiedObjects.addModifiedTask( From c05029a31e411cb738a5919e5c5a74caef0b0e33 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 30 May 2024 09:36:53 -0600 Subject: [PATCH 3/3] Update README Signed-off-by: Taylor Smock --- README | 11 ----------- README.md | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 11 deletions(-) delete mode 100644 README create mode 100644 README.md diff --git a/README b/README deleted file mode 100644 index d388c37..0000000 --- a/README +++ /dev/null @@ -1,11 +0,0 @@ -README -====== - -Readme for your plugin - - * Plugin author and contact email address. - - * The license for your plugin source code. If you have no special preferences, - you can pick the license that is used for JOSM ("GPL v2 or later"). - - * Notes for future developers, if needed. diff --git a/README.md b/README.md new file mode 100644 index 0000000..ff62375 --- /dev/null +++ b/README.md @@ -0,0 +1,25 @@ +README +====== + +# Usage +## Download MapRoulette Tasks +There are two methods to download tasks from MapRoulette. +1. From the JOSM `Download data` window (`OpenStreetMap data`, `Raw GPS data`, `Notes`, ..., `MapRoulette Tasks`) +2. From the `MapRoulette Tasks` toggle dialog (`Download Data`) which downloads data in the current viewport + +## Workflow +1. Select a MapRoulette task in the mapview *or* in the `MapRoulette Tasks` window +2. Click on `Start Task` -- this will "lock" the task on MapRoulette until you finish and click on `Stop Task`. + This helps ensure that you aren't mapping something concurrently with someone else. +3. Perform the task. Instructions can be viewed in the `Current MapRoulette Task` toggle dialog. +4. In `Current MapRoulette Task`, select the appropriate action (`False Positive`, `Too hard/Cannot see`, `Fixed`, + `Already Fixed/Not an issue`, or `Skipped`; some options are hidden in drop-down menus). +5. Uploading a changeset or `Stop Task` will then upload the task status. + +If a task specifies an OSM primitive + +# Author +* Taylor Smock + +# License +GPLv3 or any later version \ No newline at end of file