Changeset 19152 in josm for trunk/src/org/openstreetmap

Timestamp:

2024-07-25T20:56:22+02:00 (4 months ago)

Author:

taylor.smock

Message:

See #23821: Refactor LoadAndZoomHandler and add tests

This fixes some lint issues, specifically in regard to method complexity.

File:

• trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java (modified) (11 diffs)

trunk/src/org/openstreetmap/josm/io/remotecontrol/handler/LoadAndZoomHandler.java

@@ -6 +6 @@
 import java.awt.geom.Area;
 import java.awt.geom.Rectangle2D;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -18 +17 @@
 import java.util.concurrent.TimeoutException;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.swing.JOptionPane;
@@ -65 +65 @@
     public static final String command2 = "zoom";
     private static final String CURRENT_SELECTION = "currentselection";
+    private static final String SELECT = "select";
+    private static final String ADDTAGS = "addtags";
+    private static final String CHANGESET_COMMENT = "changeset_comment";
+    private static final String CHANGESET_SOURCE = "changeset_source";
+    private static final String CHANGESET_HASHTAGS = "changeset_hashtags";
+    private static final String CHANGESET_TAGS = "changeset_tags";
+    private static final String SEARCH = "search";
 
     // Mandatory arguments
@@ -81 +88 @@
         String msg = tr("Remote Control has been asked to load data from the API.") +
                 "<br>" + tr("Bounding box: ") + new BBox(minlon, minlat, maxlon, maxlat).toStringCSV(", ");
-        if (args.containsKey("select") && !toSelect.isEmpty()) {
+        if (args.containsKey(SELECT) && !toSelect.isEmpty()) {
             msg += "<br>" + tr("Selection: {0}", toSelect.size());
         }
@@ -94 +101 @@
     @Override
     public String[] getOptionalParams() {
-        return new String[] {"new_layer", "layer_name", "addtags", "select", "zoom_mode",
-                "changeset_comment", "changeset_source", "changeset_hashtags", "changeset_tags",
-                "search", "layer_locked", "download_policy", "upload_policy"};
+        return new String[] {"new_layer", "layer_name", ADDTAGS, SELECT, "zoom_mode",
+                CHANGESET_COMMENT, CHANGESET_SOURCE, CHANGESET_HASHTAGS, CHANGESET_TAGS,
+                SEARCH, "layer_locked", "download_policy", "upload_policy"};
     }
 
@@ -127 +134 @@
     @Override
     protected void handleRequest() throws RequestHandlerErrorException {
+        download();
+        /*
+         * deselect objects if parameter addtags given
+         */
+        if (args.containsKey(ADDTAGS) && !isKeepingCurrentSelection) {
+            GuiHelper.executeByMainWorkerInEDT(() -> {
+                DataSet ds = MainApplication.getLayerManager().getEditDataSet();
+                if (ds == null) // e.g. download failed
+                    return;
+                ds.clearSelection();
+            });
+        }
+
+        Collection<OsmPrimitive> forTagAdd = performSearchZoom();
+
+        // This comes before the other changeset tags, so that they can be overridden
+        parseChangesetTags(args);
+
+        // add changeset tags after download if necessary
+        addChangesetTags();
+
+        // add tags to objects
+        addTags(forTagAdd);
+    }
+
+    private void download() throws RequestHandlerErrorException {
         DownloadOsmTask osmTask = new DownloadOsmTask();
         try {
@@ -137 +170 @@
                     Area toDownload = null;
                     if (!settings.isNewLayer()) {
-                        // find out whether some data has already been downloaded
-                        Area present = null;
-                        DataSet ds = MainApplication.getLayerManager().getEditDataSet();
-                        if (ds != null) {
-                            present = ds.getDataSourceArea();
-                        }
-                        if (present != null && !present.isEmpty()) {
-                            toDownload = new Area(new Rectangle2D.Double(minlon, minlat, maxlon-minlon, maxlat-minlat));
-                            toDownload.subtract(present);
-                            if (!toDownload.isEmpty()) {
-                                // the result might not be a rectangle (L shaped etc)
-                                Rectangle2D downloadBounds = toDownload.getBounds2D();
-                                minlat = downloadBounds.getMinY();
-                                minlon = downloadBounds.getMinX();
-                                maxlat = downloadBounds.getMaxY();
-                                maxlon = downloadBounds.getMaxX();
-                            }
-                        }
+                        toDownload = removeAlreadyDownloadedArea();
                     }
                     if (toDownload != null && toDownload.isEmpty()) {
                         Logging.info("RemoteControl: no download necessary");
                     } else {
-                        Future<?> future = MainApplication.worker.submit(
-                                new PostDownloadHandler(osmTask, osmTask.download(settings, new Bounds(minlat, minlon, maxlat, maxlon),
-                                        null /* let the task manage the progress monitor */)));
-                        GuiHelper.executeByMainWorkerInEDT(() -> {
-                            try {
-                                future.get(OSM_DOWNLOAD_TIMEOUT.get(), TimeUnit.SECONDS);
-                                if (osmTask.isFailed()) {
-                                    Object error = osmTask.getErrorObjects().get(0);
-                                    if (error instanceof OsmApiException) {
-                                        throw (OsmApiException) error;
-                                    }
-                                    List<Throwable> exceptions = osmTask.getErrorObjects().stream()
-                                                    .filter(Throwable.class::isInstance).map(Throwable.class::cast)
-                                                    .collect(Collectors.toList());
-                                    OsmTransferException osmTransferException =
-                                            new OsmTransferException(String.join(", ", osmTask.getErrorMessages()));
-                                    if (!exceptions.isEmpty()) {
-                                        osmTransferException.initCause(exceptions.get(0));
-                                        exceptions.remove(0);
-                                        exceptions.forEach(osmTransferException::addSuppressed);
-                                    }
-                                    throw osmTransferException;
-                                }
-                            } catch (InterruptedException | ExecutionException | TimeoutException |
-                                    OsmTransferException | RuntimeException ex) { // NOPMD
-                                ExceptionDialogUtil.explainException(ex);
-                            }
-                        });
+                        performDownload(osmTask, settings);
                     }
                 }
@@ -195 +184 @@
             throw new RequestHandlerErrorException(ex);
         }
-
-        /*
-         * deselect objects if parameter addtags given
-         */
-        if (args.containsKey("addtags") && !isKeepingCurrentSelection) {
-            GuiHelper.executeByMainWorkerInEDT(() -> {
-                DataSet ds = MainApplication.getLayerManager().getEditDataSet();
-                if (ds == null) // e.g. download failed
-                    return;
-                ds.clearSelection();
-            });
-        }
-
+    }
+
+    /**
+     * Remove areas that has already been downloaded
+     * @return The area to download
+     */
+    private Area removeAlreadyDownloadedArea() {
+        // find out whether some data has already been downloaded
+        Area toDownload = null;
+        Area present = null;
+        DataSet ds = MainApplication.getLayerManager().getEditDataSet();
+        if (ds != null) {
+            present = ds.getDataSourceArea();
+        }
+        if (present != null && !present.isEmpty()) {
+            toDownload = new Area(new Rectangle2D.Double(minlon, minlat, maxlon-minlon, maxlat-minlat));
+            toDownload.subtract(present);
+            if (!toDownload.isEmpty()) {
+                // the result might not be a rectangle (L shaped etc)
+                Rectangle2D downloadBounds = toDownload.getBounds2D();
+                minlat = downloadBounds.getMinY();
+                minlon = downloadBounds.getMinX();
+                maxlat = downloadBounds.getMaxY();
+                maxlon = downloadBounds.getMaxX();
+            }
+        }
+        return toDownload;
+    }
+
+    private void performDownload(DownloadOsmTask osmTask, DownloadParams settings) {
+        Future<?> future = MainApplication.worker.submit(
+                new PostDownloadHandler(osmTask, osmTask.download(settings, new Bounds(minlat, minlon, maxlat, maxlon),
+                        null /* let the task manage the progress monitor */)));
+        GuiHelper.executeByMainWorkerInEDT(() -> {
+            try {
+                future.get(OSM_DOWNLOAD_TIMEOUT.get(), TimeUnit.SECONDS);
+                if (osmTask.isFailed()) {
+                    Object error = osmTask.getErrorObjects().get(0);
+                    if (error instanceof OsmApiException) {
+                        throw (OsmApiException) error;
+                    }
+                    List<Throwable> exceptions = osmTask.getErrorObjects().stream()
+                            .filter(Throwable.class::isInstance).map(Throwable.class::cast)
+                            .collect(Collectors.toList());
+                    OsmTransferException osmTransferException =
+                            new OsmTransferException(String.join(", ", osmTask.getErrorMessages()));
+                    if (!exceptions.isEmpty()) {
+                        osmTransferException.initCause(exceptions.get(0));
+                        exceptions.remove(0);
+                        exceptions.forEach(osmTransferException::addSuppressed);
+                    }
+                    throw osmTransferException;
+                }
+            } catch (InterruptedException ex) {
+                Thread.currentThread().interrupt();
+                ExceptionDialogUtil.explainException(ex);
+            } catch (ExecutionException | TimeoutException |
+                     OsmTransferException | RuntimeException ex) { // NOPMD
+                ExceptionDialogUtil.explainException(ex);
+            }
+        });
+    }
+
+    private Collection<OsmPrimitive> performSearchZoom() throws RequestHandlerErrorException {
         final Collection<OsmPrimitive> forTagAdd = new LinkedHashSet<>();
         final Bounds bbox = new Bounds(minlat, minlon, maxlat, maxlon);
-        if (args.containsKey("select") && PermissionPrefWithDefault.CHANGE_SELECTION.isAllowed()) {
+        if (args.containsKey(SELECT) && PermissionPrefWithDefault.CHANGE_SELECTION.isAllowed()) {
             // select objects after downloading, zoom to selection.
-            GuiHelper.executeByMainWorkerInEDT(() -> {
-                Set<OsmPrimitive> newSel = new LinkedHashSet<>();
-                DataSet ds = MainApplication.getLayerManager().getEditDataSet();
-                if (ds == null) // e.g. download failed
-                    return;
-                for (SimplePrimitiveId id : toSelect) {
-                    final OsmPrimitive p = ds.getPrimitiveById(id);
-                    if (p != null) {
-                        newSel.add(p);
-                        forTagAdd.add(p);
-                    }
-                }
-                if (isKeepingCurrentSelection) {
-                    Collection<OsmPrimitive> sel = ds.getSelected();
-                    newSel.addAll(sel);
-                    forTagAdd.addAll(sel);
-                }
-                toSelect.clear();
-                ds.setSelected(newSel);
-                zoom(newSel, bbox);
-                MapFrame map = MainApplication.getMap();
-                if (MainApplication.isDisplayingMapView() && map.relationListDialog != null) {
-                    map.relationListDialog.selectRelations(null); // unselect all relations to fix #7342
-                    map.relationListDialog.dataChanged(null);
-                    map.relationListDialog.selectRelations(Utils.filteredCollection(newSel, Relation.class));
-                }
-            });
-        } else if (args.containsKey("search") && PermissionPrefWithDefault.CHANGE_SELECTION.isAllowed()) {
-            try {
-                final SearchCompiler.Match search = SearchCompiler.compile(args.get("search"));
-                MainApplication.worker.submit(() -> {
-                    final DataSet ds = MainApplication.getLayerManager().getEditDataSet();
-                    final Collection<OsmPrimitive> filteredPrimitives = SubclassFilteredCollection.filter(ds.allPrimitives(), search);
-                    ds.setSelected(filteredPrimitives);
-                    forTagAdd.addAll(filteredPrimitives);
-                    zoom(filteredPrimitives, bbox);
-                });
-            } catch (SearchParseError ex) {
-                Logging.error(ex);
-                throw new RequestHandlerErrorException(ex);
-            }
+            GuiHelper.executeByMainWorkerInEDT(() -> selectAndZoom(forTagAdd, bbox));
+        } else if (args.containsKey(SEARCH) && PermissionPrefWithDefault.CHANGE_SELECTION.isAllowed()) {
+            searchAndZoom(forTagAdd, bbox);
         } else {
             // after downloading, zoom to downloaded area.
-            zoom(Collections.<OsmPrimitive>emptySet(), bbox);
-        }
-
-        // This comes before the other changeset tags, so that they can be overridden
-        parseChangesetTags(args);
-
-        // add changeset tags after download if necessary
-        if (args.containsKey("changeset_comment") || args.containsKey("changeset_source") || args.containsKey("changeset_hashtags")) {
+            zoom(Collections.emptySet(), bbox);
+        }
+        return forTagAdd;
+    }
+
+    private void selectAndZoom(Collection<OsmPrimitive> forTagAdd, Bounds bbox) {
+        Set<OsmPrimitive> newSel = new LinkedHashSet<>();
+        DataSet ds = MainApplication.getLayerManager().getEditDataSet();
+        if (ds == null) // e.g. download failed
+            return;
+        for (SimplePrimitiveId id : toSelect) {
+            final OsmPrimitive p = ds.getPrimitiveById(id);
+            if (p != null) {
+                newSel.add(p);
+                forTagAdd.add(p);
+            }
+        }
+        if (isKeepingCurrentSelection) {
+            Collection<OsmPrimitive> sel = ds.getSelected();
+            newSel.addAll(sel);
+            forTagAdd.addAll(sel);
+        }
+        toSelect.clear();
+        ds.setSelected(newSel);
+        zoom(newSel, bbox);
+        MapFrame map = MainApplication.getMap();
+        if (MainApplication.isDisplayingMapView() && map.relationListDialog != null) {
+            map.relationListDialog.selectRelations(null); // unselect all relations to fix #7342
+            map.relationListDialog.dataChanged(null);
+            map.relationListDialog.selectRelations(Utils.filteredCollection(newSel, Relation.class));
+        }
+    }
+
+    private void searchAndZoom(Collection<OsmPrimitive> forTagAdd, Bounds bbox) throws RequestHandlerErrorException {
+        try {
+            final SearchCompiler.Match search = SearchCompiler.compile(args.get(SEARCH));
             MainApplication.worker.submit(() -> {
-                DataSet ds = MainApplication.getLayerManager().getEditDataSet();
-                if (ds != null) {
-                    for (String tag : Arrays.asList("changeset_comment", "changeset_source", "changeset_hashtags")) {
-                        if (args.containsKey(tag)) {
-                            final String tagKey = tag.substring("changeset_".length());
-                            final String value = args.get(tag);
-                            if (!Utils.isStripEmpty(value)) {
-                                ds.addChangeSetTag(tagKey, value);
-                            } else {
-                                ds.addChangeSetTag(tagKey, null);
-                            }
-                        }
-                    }
-                }
+                final DataSet ds = MainApplication.getLayerManager().getEditDataSet();
+                final Collection<OsmPrimitive> filteredPrimitives = SubclassFilteredCollection.filter(ds.allPrimitives(), search);
+                ds.setSelected(filteredPrimitives);
+                forTagAdd.addAll(filteredPrimitives);
+                zoom(filteredPrimitives, bbox);
             });
-        }
-
-        // add tags to objects
-        if (args.containsKey("addtags")) {
+        } catch (SearchParseError ex) {
+            Logging.error(ex);
+            throw new RequestHandlerErrorException(ex);
+        }
+    }
+
+    private void addChangesetTags() {
+        List<String> values = Stream.of(CHANGESET_COMMENT, CHANGESET_SOURCE, CHANGESET_HASHTAGS)
+                .filter(args::containsKey).collect(Collectors.toList());
+        if (values.isEmpty()) {
+            return;
+        }
+        MainApplication.worker.submit(() -> {
+            DataSet ds = MainApplication.getLayerManager().getEditDataSet();
+            if (ds != null) {
+                for (String tag : values) {
+                    final String tagKey = tag.substring("changeset_".length());
+                    final String value = args.get(tag);
+                    if (!Utils.isStripEmpty(value)) {
+                        ds.addChangeSetTag(tagKey, value);
+                    } else {
+                        ds.addChangeSetTag(tagKey, null);
+                    }
+                }
+            }
+        });
+    }
+
+    private void addTags(Collection<OsmPrimitive> forTagAdd) {
+        if (args.containsKey(ADDTAGS)) {
             // needs to run in EDT since forTagAdd is updated in EDT as well
             GuiHelper.executeByMainWorkerInEDT(() -> {
@@ -290 +337 @@
                     new Notification(isKeepingCurrentSelection
                             ? tr("You clicked on a JOSM remotecontrol link that would apply tags onto selected objects.\n"
-                                    + "Since no objects have been selected before this click, no tags were added.\n"
-                                    + "Select one or more objects and click the link again.")
+                            + "Since no objects have been selected before this click, no tags were added.\n"
+                            + "Select one or more objects and click the link again.")
                             : tr("You clicked on a JOSM remotecontrol link that would apply tags onto objects.\n"
-                                    + "Unfortunately that link seems to be broken.\n"
-                                    + "Technical explanation: the URL query parameter ''select='' or ''search='' has an invalid value.\n"
-                                    + "Ask someone at the origin of the clicked link to fix this.")
-                        ).setIcon(JOptionPane.WARNING_MESSAGE).setDuration(Notification.TIME_LONG).show();
+                            + "Unfortunately that link seems to be broken.\n"
+                            + "Technical explanation: the URL query parameter ''select='' or ''search='' has an invalid value.\n"
+                            + "Ask someone at the origin of the clicked link to fix this.")
+                    ).setIcon(JOptionPane.WARNING_MESSAGE).setDuration(Notification.TIME_LONG).show();
                 }
             });
@@ -303 +350 @@
 
     static void parseChangesetTags(Map<String, String> args) {
-        if (args.containsKey("changeset_tags")) {
+        if (args.containsKey(CHANGESET_TAGS)) {
             MainApplication.worker.submit(() -> {
                 DataSet ds = MainApplication.getLayerManager().getEditDataSet();
                 if (ds != null) {
-                    AddTagsDialog.parseUrlTagsToKeyValues(args.get("changeset_tags")).forEach(ds::addChangeSetTag);
+                    AddTagsDialog.parseUrlTagsToKeyValues(args.get(CHANGESET_TAGS)).forEach(ds::addChangeSetTag);
                 }
             });
@@ -366 +413 @@
 
         // Process optional argument 'select'
-        if (args != null && args.containsKey("select")) {
+        validateSelect();
+    }
+
+    private void validateSelect() {
+        if (args != null && args.containsKey(SELECT)) {
             toSelect.clear();
-            for (String item : args.get("select").split(",", -1)) {
+            for (String item : args.get(SELECT).split(",", -1)) {
                 if (!item.isEmpty()) {
                     if (CURRENT_SELECTION.equalsIgnoreCase(item)) {