Changeset 34848 in osm

Timestamp:

2019-01-21T09:26:11+01:00 (6 years ago)

Author:

gerdp

Message:

see #17201 Improve progress monitor and react on cancel button, also fix some sonar and checkstyle issues

TODO:

• cancel doesn't stop reading the file yet. This would require more changes.
• progress monitor for download from url doesn't work yet

Location:

applications/editors/josm/plugins/pbf

Files:

• src/org/openstreetmap/josm/plugins/pbf/io/PbfReader.java (modified) (9 diffs)
• src/org/openstreetmap/josm/plugins/pbf/io/PbfServerReader.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/plugins/pbf/io/PbfImporterTest.java (modified) (2 diffs)

applications/editors/josm/plugins/pbf/src/org/openstreetmap/josm/plugins/pbf/io/PbfReader.java

@@ -29 +29 @@
 import org.openstreetmap.josm.io.AbstractReader;
 import org.openstreetmap.josm.io.IllegalDataException;
+import org.openstreetmap.josm.io.ImportCancelException;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.Logging;
@@ -108 +109 @@
         @Override
         public boolean skipBlock(FileBlockPosition block) {
-            return exception != null;
+            return exception != null || cancel;
         }
 
@@ -146 +147 @@
                     for (int i = 0; i < nodes.getIdCount(); i++) {
                         // Id (delta) and version (normal)
-                        NodeData nd = new NodeData(nodeId += nodes.getId(i));
+                        nodeId += nodes.getId(i);
+                        NodeData nd = new NodeData(nodeId);
                         nd.setVersion(nodes.hasDenseinfo() ? nodes.getDenseinfo().getVersion(i) : 1);
                         // Lat/Lon (delta)
-                        nd.setCoor(new LatLon(parseLat(nodeLat += nodes.getLat(i)),
-                                                parseLon(nodeLon += nodes.getLon(i))).getRoundedToOsmPrecision());
+                        nodeLat += nodes.getLat(i);
+                        nodeLon += nodes.getLon(i);
+                        nd.setCoor(new LatLon(parseLat(nodeLat), parseLon(nodeLon)).getRoundedToOsmPrecision());
                         checkCoordinates(nd.getCoor());
                         if (nodes.hasDenseinfo()) {
@@ -156 +159 @@
                             // Changeset (delta)
                             if (info.getChangesetCount() > i) {
-                                checkChangesetId(changesetId += info.getChangeset(i));
+                                changesetId += info.getChangeset(i);
+                                checkChangesetId(changesetId);
                                 nd.setChangesetId((int) changesetId);
                             }
                             // User (delta)
                             if (info.getUidCount() > i && info.getUserSidCount() > i) {
-                                nd.setUser(User.createOsmUser(uid += info.getUid(i),
-                                                 getStringById(suid += info.getUserSid(i))));
+                                uid += info.getUid(i);
+                                suid += info.getUserSid(i);
+                                nd.setUser(User.createOsmUser(uid, getStringById(suid)));
                             }
                             // Timestamp (delta)
                             if (info.getTimestampCount() > i) {
-                                checkTimestamp(timestamp += info.getTimestamp(i));
+                                timestamp += info.getTimestamp(i);
+                                checkTimestamp(timestamp);
                                 nd.setTimestamp(new Date(date_granularity * timestamp));
                             }
@@ -235 +241 @@
                         }
                         wd.setKeys(keys);
-                        long previousId = 0; // Node ids are delta coded
+                        long id = 0; // Node ids are delta coded
                         Collection<Long> nodeIds = new ArrayList<>();
-                        for (Long id : w.getRefsList()) {
-                            nodeIds.add(previousId += id);
+                        for (Long idDelta : w.getRefsList()) {
+                            id += idDelta;
+                            nodeIds.add(id);
                         }
                         ways.put(wd.getUniqueId(), nodeIds);
@@ -265 +272 @@
                         }
                         rd.setKeys(keys);
-                        long previousId = 0; // Member ids are delta coded
+                        long memId = 0; // Member ids are delta coded
                         Collection<RelationMemberData> members = new ArrayList<>();
                         for (int i = 0; i < r.getMemidsCount(); i++) {
+                            memId += r.getMemids(i);
                             members.add(new RelationMemberData(
                                     getStringById(r.getRolesSid(i)),
                                     mapOsmType(r.getTypes(i)),
-                                    previousId += r.getMemids(i)));
+                                    memId));
                         }
                         relations.put(rd.getUniqueId(), members);
@@ -325 +333 @@
     protected DataSet doParseDataSet(InputStream source, ProgressMonitor monitor)
             throws IllegalDataException {
+        ProgressMonitor.CancelListener cancelListener = () -> {
+            cancel = true;
+            monitor.indeterminateSubTask(tr("Canceled: Waiting for reader to react"));
+        };
+        monitor.addCancelListener(cancelListener);
+
         try {
-            monitor.beginTask(tr("Prepare OSM data...", 2));
+            monitor.beginTask(tr("Prepare OSM data..."), 3);
             monitor.indeterminateSubTask(tr("Reading OSM data..."));
 
             parse(source);
             monitor.worked(1);
-
+            if (cancel) {
+                throw new ParsingCancelException(tr("Import was canceled"));
+            }
             monitor.indeterminateSubTask(tr("Preparing data set..."));
             prepareDataSet();
+            if (cancel) {
+                throw new ParsingCancelException(tr("Import was canceled"));
+            }
             monitor.worked(1);
             return getDataSet();
@@ -341 +360 @@
             throw new IllegalDataException(e);
         } finally {
-            monitor.finishTask();
+            monitor.removeCancelListener(cancelListener);
         }
     }
@@ -351 +370 @@
         }
     }
+
+    /**
+     * Exception thrown after user cancellation.
+     */
+    private static final class ParsingCancelException extends Exception implements ImportCancelException {
+        private static final long serialVersionUID = 1L;
+
+        ParsingCancelException(String msg) {
+            super(msg);
+        }
+    }
+
 }

applications/editors/josm/plugins/pbf/src/org/openstreetmap/josm/plugins/pbf/io/PbfServerReader.java

@@ -28 +28 @@
             throws OsmTransferException {
         try {
-            progressMonitor.beginTask(tr("Contacting Server...", 10));
+            progressMonitor.beginTask(tr("Contacting Server..."));
             return new PbfImporter().parseDataSet(url);
         } catch (Exception e) {

applications/editors/josm/plugins/pbf/test/unit/org/openstreetmap/josm/plugins/pbf/io/PbfImporterTest.java

@@ -80 +80 @@
         assertEquals(97, ds.getRelations().size());
     }
-    
+
     /**
      * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/14545">Ticket #14545</a>.
@@ -94 +94 @@
     }
 
-    
 }