Changeset 31514 in osm for applications

Timestamp:

2015-08-21T20:44:24+02:00 (9 years ago)

Author:

nokutu

Message:

Code improvements and fixed a bug caused by trying to upload a picture without being logged in.

Location:

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary

Files:

• MapillaryAbstractImage.java (modified) (4 diffs)
• actions/MapillaryUploadAction.java (modified) (1 diff)
• actions/WalkThread.java (modified) (5 diffs)
• downloads/MapillarySequenceDownloadThread.java (modified) (4 diffs)
• gui/MapillaryPreferenceSetting.java (modified) (1 diff)
• gui/MapillaryUploadDialog.java (modified) (3 diffs)
• oauth/MapillaryUser.java (modified) (1 diff)
• traffico/TrafficoSignElement.java (modified) (2 diffs)

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryAbstractImage.java

@@ -4 +4 @@
 import java.util.Calendar;
 import java.util.Date;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import org.openstreetmap.josm.Main;
@@ -18 +16 @@
  */
 public abstract class MapillaryAbstractImage {
-
-  /**
-   * Lock that locks {@link MapillaryAbstractImage#next()} and
-   * {@link MapillaryAbstractImage#previous()} methods. Used when downloading
-   * images to prevent concurrency problems.
-   */
-  public static final Lock LOCK = new ReentrantLock();
 
   /** The time the image was captured, in Epoch format. */
@@ -205 +196 @@
    */
   public MapillaryAbstractImage next() {
-    LOCK.lock();
-    try {
-      if (this.getSequence() == null) {
+    synchronized (this.getClass()) {
+      if (this.getSequence() == null)
         return null;
-      }
       return this.getSequence().next(this);
-    } finally {
-      LOCK.unlock();
     }
   }
@@ -223 +210 @@
    */
   public MapillaryAbstractImage previous() {
-    LOCK.lock();
-    try {
-      if (this.getSequence() == null) {
+    synchronized (this.getClass()) {
+      if (this.getSequence() == null)
         return null;
-      }
       return this.getSequence().previous(this);
-    } finally {
-      LOCK.unlock();
-    }
-
+    }
   }
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/MapillaryUploadAction.java

@@ -53 +53 @@
 
     if (pane.getValue() != null
-        && (int) pane.getValue() == JOptionPane.OK_OPTION) {
+        && (int) pane.getValue() == JOptionPane.OK_OPTION
+        && dialog.delete != null) {
       if (dialog.sequence.isSelected()) {
         UploadUtils.uploadSequence(MapillaryLayer.getInstance().getData()

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/actions/WalkThread.java

@@ -2 +2 @@
 
 import java.awt.image.BufferedImage;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.swing.SwingUtilities;
@@ -24 +22 @@
   private final int interval;
   private final MapillaryData data;
-  private final Lock lock;
   private boolean end = false;
   private final boolean waitForFullQuality;
@@ -53 +50 @@
     this.data = MapillaryLayer.getInstance().getData();
     this.data.addListener(this);
-    this.lock = new ReentrantLock();
   }
 
@@ -110 +106 @@
           this.lastImage = MapillaryMainDialog.getInstance().mapillaryImageDisplay
               .getImage();
-          this.lock.lock();
-          try {
+          synchronized (this) {
             if (this.goForward)
               this.data.selectNext(this.followSelected);
             else
               this.data.selectPrevious(this.followSelected);
-          } finally {
-            this.lock.unlock();
           }
         } catch (InterruptedException e) {
@@ -130 +123 @@
 
   @Override
-  public void interrupt() {
-    this.lock.lock();
-    try {
-      super.interrupt();
-    } catch (Exception e) {
-    } finally {
-      this.lock.unlock();
-    }
+  public synchronized void interrupt() {
+    super.interrupt();
   }
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/downloads/MapillarySequenceDownloadThread.java

@@ -8 +8 @@
 import java.util.List;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.json.Json;
@@ -33 +31 @@
   private static final String URL = MapillaryDownloader.BASE_URL + "search/s/";
 
-  /** Lock to prevent multiple downloads to be imported at the same time. */
-  private static final Lock LOCK = new ReentrantLock();
-
   private final String queryString;
   private final ExecutorService ex;
@@ -58 +53 @@
     try {
       BufferedReader br;
-      br = new BufferedReader(new InputStreamReader(
-          new URL(URL + this.queryString).openStream(), "UTF-8"));
+      br = new BufferedReader(new InputStreamReader(new URL(URL
+          + this.queryString).openStream(), "UTF-8"));
       JsonObject jsonall = Json.createReader(br).readObject();
 
@@ -95 +90 @@
             finalImages.remove(img);
         }
-
-        LOCK.lock();
-        MapillaryAbstractImage.LOCK.lock();
-        try {
-          for (MapillaryImage img : finalImages) {
-            if (this.layer.getData().getImages().contains(img)) {
-              // The image in finalImages is substituted by the one in the
-              // database, as they represent the same picture.
-              img = (MapillaryImage) this.layer.getData().getImages()
-                  .get(this.layer.getData().getImages().indexOf(img));
-              sequence.add(img);
-              ((MapillaryImage) this.layer.getData().getImages()
-                  .get(this.layer.getData().getImages().indexOf(img)))
-                  .setSequence(sequence);
-              finalImages.set(finalImages.indexOf(img), img);
-            } else {
-              img.setSequence(sequence);
-              sequence.add(img);
+        synchronized (this.getClass()) {
+          synchronized (MapillaryAbstractImage.class) {
+            for (MapillaryImage img : finalImages) {
+              if (this.layer.getData().getImages().contains(img)) {
+                // The image in finalImages is substituted by the one in the
+                // database, as they represent the same picture.
+                img = (MapillaryImage) this.layer.getData().getImages()
+                    .get(this.layer.getData().getImages().indexOf(img));
+                sequence.add(img);
+                ((MapillaryImage) this.layer.getData().getImages()
+                    .get(this.layer.getData().getImages().indexOf(img)))
+                    .setSequence(sequence);
+                finalImages.set(finalImages.indexOf(img), img);
+              } else {
+                img.setSequence(sequence);
+                sequence.add(img);
+              }
             }
           }
-        } finally {
-          MapillaryAbstractImage.LOCK.unlock();
-          LOCK.unlock();
         }
 

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/gui/MapillaryPreferenceSetting.java

@@ -83 +83 @@
     panel.add(this.moveTo);
     this.login = new JButton(new LoginAction());
-    if (Main.pref.get("mapillary.access-token") == null)
+    if (MapillaryUser.getUsername() == null)
       this.login.setText("Login");
     else

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/gui/MapillaryUploadDialog.java

@@ -6 +6 @@
 import javax.swing.ButtonGroup;
 import javax.swing.JCheckBox;
+import javax.swing.JLabel;
 import javax.swing.JPanel;
 import javax.swing.JRadioButton;
@@ -12 +13 @@
 import org.openstreetmap.josm.plugins.mapillary.MapillaryImportedImage;
 import org.openstreetmap.josm.plugins.mapillary.MapillaryLayer;
+import org.openstreetmap.josm.plugins.mapillary.oauth.MapillaryUser;
 
 /**
@@ -33 +35 @@
   public MapillaryUploadDialog() {
     setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
+    if (MapillaryUser.getUsername() != null) {
+      this.group = new ButtonGroup();
 
-    this.group = new ButtonGroup();
+      this.sequence = new JRadioButton(tr("Upload selected sequence"));
+      if (MapillaryLayer.getInstance().getData().getSelectedImage() == null
+          || !(MapillaryLayer.getInstance().getData().getSelectedImage() instanceof MapillaryImportedImage))
+        this.sequence.setEnabled(false);
+      this.group.add(this.sequence);
+      add(this.sequence);
+      this.group.setSelected(this.sequence.getModel(), true);
 
-    this.sequence = new JRadioButton(tr("Upload selected sequence"));
-    if (MapillaryLayer.getInstance().getData().getSelectedImage() == null
-        || !(MapillaryLayer.getInstance().getData().getSelectedImage() instanceof MapillaryImportedImage))
-      this.sequence.setEnabled(false);
-    this.group.add(this.sequence);
-    add(this.sequence);
-    this.group.setSelected(this.sequence.getModel(), true);
-
-    this.delete = new JCheckBox(tr("Delete after upload"));
-    this.delete.setSelected(Main.pref.getBoolean(
-        "mapillary.delete-after-upload", true));
-    add(this.delete);
-
+      this.delete = new JCheckBox(tr("Delete after upload"));
+      this.delete.setSelected(Main.pref.getBoolean(
+          "mapillary.delete-after-upload", true));
+      add(this.delete);
+    } else {
+      this.add(new JLabel("Go to setting and log in to Mapillary before uploading."));
+    }
   }
 }

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/oauth/MapillaryUser.java

@@ -61 +61 @@
         Main.error(e);
         isTokenValid = false;
-        isTokenValid = false;
       }
     hash.put("images_hash", images_hash);

applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/traffico/TrafficoSignElement.java

@@ -20 +20 @@
    */
   public Color getColor() {
-    return color;
+    return this.color;
   }
 
@@ -27 +27 @@
    */
   public char getGlyph() {
-    return glyph;
+    return this.glyph;
   }
 }