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

git-svn-id: http://svn.openstreetmap.org/applications/editors/josm/plugins/mapillary@31514 b9d5c4c9-76e1-0310-9c85-f3177eceb1e4

Author: nokutu

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

@@ -3,8 +3,6 @@
 import java.text.SimpleDateFormat;
 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;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -18,13 +16,6 @@
  */
 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. */
   private long capturedAt;
   /** Sequence of pictures containing this object. */
@@ -204,14 +195,10 @@ public void move(double x, double y) {
    * @return The following MapillaryImage, or null if there is none.
    */
   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();
     }
   }
 
@@ -222,16 +209,11 @@ public MapillaryAbstractImage next() {
    * @return The previous MapillaryImage, or null if there is none.
    */
   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();
     }
-
   }
 
   /**

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

@@ -52,7 +52,8 @@ public void actionPerformed(ActionEvent arg0) {
     dlg.setVisible(true);
 
     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()
             .getSelectedImage().getSequence(), dialog.delete.isSelected());

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

@@ -1,8 +1,6 @@
 package org.openstreetmap.josm.plugins.mapillary.actions;
 
 import java.awt.image.BufferedImage;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 
 import javax.swing.SwingUtilities;
 
@@ -23,7 +21,6 @@
 public class WalkThread extends Thread implements MapillaryDataListener {
   private final int interval;
   private final MapillaryData data;
-  private final Lock lock;
   private boolean end = false;
   private final boolean waitForFullQuality;
   private final boolean followSelected;
@@ -52,7 +49,6 @@ public WalkThread(int interval, boolean waitForPicture,
     this.goForward = goForward;
     this.data = MapillaryLayer.getInstance().getData();
     this.data.addListener(this);
-    this.lock = new ReentrantLock();
   }
 
   @Override
@@ -109,14 +105,11 @@ public void run() {
           }
           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) {
           return;
@@ -129,14 +122,8 @@ public void run() {
   }
 
   @Override
-  public void interrupt() {
-    this.lock.lock();
-    try {
-      super.interrupt();
-    } catch (Exception e) {
-    } finally {
-      this.lock.unlock();
-    }
+  public synchronized void interrupt() {
+    super.interrupt();
   }
 
   @Override

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

@@ -7,8 +7,6 @@
 import java.util.ArrayList;
 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;
 import javax.json.JsonArray;
@@ -32,9 +30,6 @@
 public class MapillarySequenceDownloadThread extends Thread {
   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;
   private final MapillaryLayer layer;
@@ -57,8 +52,8 @@ public MapillarySequenceDownloadThread(ExecutorService ex, String queryString) {
   public void run() {
     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();
 
       if (!jsonall.getBoolean("more") && !this.ex.isShutdown())
@@ -94,29 +89,25 @@ public void run() {
           if (!isInside(img))
             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();
         }
 
         this.layer.getData().add(

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

@@ -82,7 +82,7 @@ public void addGui(PreferenceTabbedPane gui) {
     panel.add(this.format24);
     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
       this.login.setText("Logged as: " + MapillaryUser.getUsername()

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

@@ -5,12 +5,14 @@
 import javax.swing.BoxLayout;
 import javax.swing.ButtonGroup;
 import javax.swing.JCheckBox;
+import javax.swing.JLabel;
 import javax.swing.JPanel;
 import javax.swing.JRadioButton;
 
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.plugins.mapillary.MapillaryImportedImage;
 import org.openstreetmap.josm.plugins.mapillary.MapillaryLayer;
+import org.openstreetmap.josm.plugins.mapillary.oauth.MapillaryUser;
 
 /**
  * @author nokutu
@@ -32,21 +34,23 @@ public class MapillaryUploadDialog extends JPanel {
    */
   public MapillaryUploadDialog() {
     setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
-
-    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.delete = new JCheckBox(tr("Delete after upload"));
-    this.delete.setSelected(Main.pref.getBoolean(
-        "mapillary.delete-after-upload", true));
-    add(this.delete);
-
+    if (MapillaryUser.getUsername() != null) {
+      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.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."));
+    }
   }
 }

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

@@ -60,7 +60,6 @@ public static HashMap<String, String> getSecrets() {
       } catch (IOException e) {
         Main.error(e);
         isTokenValid = false;
-        isTokenValid = false;
       }
     hash.put("images_hash", images_hash);
     if (images_policy == null)

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

@@ -19,13 +19,13 @@ public TrafficoSignElement(char glyph, Color c) {
    * @return the color
    */
   public Color getColor() {
-    return color;
+    return this.color;
   }
 
   /**
    * @return the glyph
    */
   public char getGlyph() {
-    return glyph;
+    return this.glyph;
   }
 }