Index: src/org/openstreetmap/josm/actions/UploadAction.java =================================================================== --- src/org/openstreetmap/josm/actions/UploadAction.java (revision 13124) +++ src/org/openstreetmap/josm/actions/UploadAction.java (working copy) @@ -9,6 +9,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import javax.swing.JOptionPane; @@ -24,6 +25,7 @@ import org.openstreetmap.josm.data.osm.Changeset; import org.openstreetmap.josm.gui.HelpAwareOptionPane; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.io.AsynchronousUploadPrimitivesTask; import org.openstreetmap.josm.gui.io.UploadDialog; import org.openstreetmap.josm.gui.io.UploadPrimitivesTask; import org.openstreetmap.josm.gui.layer.AbstractModifiableLayer; @@ -57,6 +59,8 @@ private static final List UPLOAD_HOOKS = new LinkedList<>(); private static final List LATE_UPLOAD_HOOKS = new LinkedList<>(); + private static final String IS_ASYNC_UPLOAD_ENABLED = "asynchronous.upload"; + static { /** * Calls validator before upload. @@ -259,14 +263,25 @@ hook.modifyChangesetTags(changesetTags); } - MainApplication.worker.execute( - new UploadPrimitivesTask( - UploadDialog.getUploadDialog().getUploadStrategySpecification(), - layer, - apiData, - cs - ) - ); + if (Main.pref.get(IS_ASYNC_UPLOAD_ENABLED).isEmpty() || + !Main.pref.get(IS_ASYNC_UPLOAD_ENABLED).equalsIgnoreCase("disabled")) { + Optional asyncUploadTask = AsynchronousUploadPrimitivesTask.createAsynchronousUploadTask( + UploadDialog.getUploadDialog().getUploadStrategySpecification(), + layer, + apiData, + cs); + + if (asyncUploadTask.isPresent()) { + MainApplication.worker.execute(asyncUploadTask.get()); + } + } else { + MainApplication.worker.execute( + new UploadPrimitivesTask( + UploadDialog.getUploadDialog().getUploadStrategySpecification(), + layer, + apiData, + cs)); + } } @Override Index: src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java =================================================================== --- src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java (nonexistent) +++ src/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTask.java (working copy) @@ -0,0 +1,153 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.io; + +import org.openstreetmap.josm.data.APIDataSet; +import org.openstreetmap.josm.data.osm.Changeset; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.dialogs.LayerListDialog; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.gui.progress.ProgressTaskId; +import org.openstreetmap.josm.gui.util.GuiHelper; +import org.openstreetmap.josm.io.UploadStrategySpecification; + + +import javax.swing.JOptionPane; +import java.awt.GraphicsEnvironment; +import java.util.Optional; + +import static org.openstreetmap.josm.tools.I18n.tr; + +/** + * Task for uploading primitives using background worker threads. The actual upload is delegated to the + * {@link UploadPrimitivesTask}. This class is a wrapper over that to make the background upload process safe. There + * can only be one instance of this class, hence background uploads are limited to one at a time. This class also + * changes the editLayer of {@link org.openstreetmap.josm.gui.layer.MainLayerManager} to null during upload so that + * any changes to the uploading layer are prohibited. + * + * @author udit + * @since xxx + */ +public final class AsynchronousUploadPrimitivesTask extends UploadPrimitivesTask { + + /** + * Static instance + */ + private static AsynchronousUploadPrimitivesTask asynchronousUploadPrimitivesTask = null; + + /** + * Member fields + */ + private final ProgressTaskId taskId; + private final OsmDataLayer uploadDataLayer; + + /** + * Private constructor to restrict creating more Asynchronous upload tasks + * + * @param uploadStrategySpecification UploadStrategySpecification for the DataLayer + * @param osmDataLayer Datalayer to be uploaded + * @param apiDataSet ApiDataSet that contains the primitives to be uploaded + * @param changeset Changeset for the datalayer + * + * @throws IllegalArgumentException if layer is null + * @throws IllegalArgumentException if toUpload is null + * @throws IllegalArgumentException if strategy is null + * @throws IllegalArgumentException if changeset is null + */ + private AsynchronousUploadPrimitivesTask(UploadStrategySpecification uploadStrategySpecification, + OsmDataLayer osmDataLayer, APIDataSet apiDataSet, Changeset changeset) { + super(uploadStrategySpecification, + osmDataLayer, + apiDataSet, + changeset); + + uploadDataLayer = osmDataLayer; + // Create a ProgressTaskId for background upload + taskId = new ProgressTaskId("core", "async-upload"); + } + + /** + * Creates an instance of AsynchronousUploadPrimitiveTask + * + * @param uploadStrategySpecification UploadStrategySpecification for the DataLayer + * @param dataLayer Datalayer to be uploaded + * @param apiDataSet ApiDataSet that contains the primitives to be uploaded + * @param changeset Changeset for the datalayer + * @return Returns an {@literal Optional } if there is no + * background upload in progress. Otherwise returns an {@literal Optional.empty()} + * + * @throws IllegalArgumentException if layer is null + * @throws IllegalArgumentException if toUpload is null + * @throws IllegalArgumentException if strategy is null + * @throws IllegalArgumentException if changeset is null + */ + public static Optional createAsynchronousUploadTask( + UploadStrategySpecification uploadStrategySpecification, + OsmDataLayer dataLayer, APIDataSet apiDataSet, Changeset changeset) { + synchronized (AsynchronousUploadPrimitivesTask.class) { + if (asynchronousUploadPrimitivesTask != null) { + if (!GraphicsEnvironment.isHeadless()) { + GuiHelper.runInEDTAndWait(() -> + JOptionPane.showMessageDialog(MainApplication.parent, + tr("A background upload is already in progress. " + + "Kindly wait for it to finish before uploading new changes"))); + } + return Optional.empty(); + } else { + // Create an asynchronous upload task + asynchronousUploadPrimitivesTask = new AsynchronousUploadPrimitivesTask( + uploadStrategySpecification, + dataLayer, + apiDataSet, + changeset); + return Optional.ofNullable(asynchronousUploadPrimitivesTask); + } + } + } + + /** + * Get the current upload task + * @return {@literal Optional } + */ + public static Optional getCurrentAsynchronousUploadTask() { + return Optional.ofNullable(asynchronousUploadPrimitivesTask); + } + + @Override + public ProgressTaskId canRunInBackground() { + return taskId; + } + + @Override + protected void realRun() { + // Lock the data layer before upload in EDT + GuiHelper.runInEDTAndWait(() -> { + // Remove the commands from the undo stack + MainApplication.undoRedo.clean(uploadDataLayer.data); + MainApplication.getLayerManager().prepareLayerForUpload(uploadDataLayer); + + // Repainting the Layer List dialog to update the icon of the active layer + LayerListDialog.getInstance().repaint(); + }); + super.realRun(); + } + + @Override + protected void cancel() { + super.cancel(); + asynchronousUploadPrimitivesTask = null; + } + + @Override + protected void finish() { + try { + // Unlock the data layer in EDT + GuiHelper.runInEDTAndWait(() -> { + MainApplication.getLayerManager().processLayerAfterUpload(uploadDataLayer); + LayerListDialog.getInstance().repaint(); + }); + super.finish(); + } finally { + asynchronousUploadPrimitivesTask = null; + } + } +} Index: src/org/openstreetmap/josm/gui/layer/MainLayerManager.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (revision 13124) +++ src/org/openstreetmap/josm/gui/layer/MainLayerManager.java (working copy) @@ -1,6 +1,11 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.gui.layer; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.util.GuiHelper; + +import javax.swing.JOptionPane; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -7,8 +12,7 @@ import java.util.ListIterator; import java.util.concurrent.CopyOnWriteArrayList; -import org.openstreetmap.josm.data.osm.DataSet; -import org.openstreetmap.josm.gui.util.GuiHelper; +import static org.openstreetmap.josm.tools.I18n.tr; /** * This class extends the layer manager by adding an active and an edit layer. @@ -205,13 +209,23 @@ } /** - * Set the active layer. If the layer is an OsmDataLayer, the edit layer is also changed. + * Set the active layer iff the layer is not read only. + * If the layer is an OsmDataLayer, the edit layer is also changed. * @param layer The active layer. */ public void setActiveLayer(final Layer layer) { // we force this on to the EDT Thread to make events fire from there. // The synchronization lock needs to be held by the EDT. - GuiHelper.runInEDTAndWaitWithException(() -> realSetActiveLayer(layer)); + if (layer instanceof OsmDataLayer && ((OsmDataLayer) layer).isReadOnly()) { + GuiHelper.runInEDT(() -> + JOptionPane.showMessageDialog( + MainApplication.parent, + tr("Trying to set a read only data layer as edit layer"), + tr("Warning"), + JOptionPane.WARNING_MESSAGE)); + } else { + GuiHelper.runInEDTAndWaitWithException(() -> realSetActiveLayer(layer)); + } } protected synchronized void realSetActiveLayer(final Layer layer) { @@ -309,7 +323,15 @@ * @return the currently active layer (may be null) */ public synchronized Layer getActiveLayer() { - return activeLayer; + if (activeLayer instanceof OsmDataLayer) { + if (!((OsmDataLayer) activeLayer).isReadOnly()) { + return activeLayer; + } else { + return null; + } + } else { + return activeLayer; + } } /** @@ -318,7 +340,10 @@ * @return the current edit layer. May be null. */ public synchronized OsmDataLayer getEditLayer() { - return editLayer; + if (editLayer != null && !editLayer.isReadOnly()) + return editLayer; + else + return null; } /** @@ -378,4 +403,30 @@ activeLayerChangeListeners.clear(); layerAvailabilityListeners.clear(); } + + public void prepareLayerForUpload(OsmDataLayer layer) { + + GuiHelper.assertCallFromEdt(); + layer.setReadOnly(); + + // Reset only the edit layer as empty + if (editLayer == layer) { + ActiveLayerChangeEvent activeLayerChangeEvent = new ActiveLayerChangeEvent(this, editLayer, activeLayer); + editLayer = null; + fireActiveLayerChange(activeLayerChangeEvent); + } + } + + public void processLayerAfterUpload(OsmDataLayer layer) { + GuiHelper.assertCallFromEdt(); + layer.unsetReadOnly(); + + // Set the layer as edit layer + // iff the edit layer is empty. + if (editLayer == null) { + ActiveLayerChangeEvent layerChangeEvent = new ActiveLayerChangeEvent(this, editLayer, activeLayer); + editLayer = layer; + fireActiveLayerChange(layerChangeEvent); + } + } } Index: src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 13124) +++ src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (working copy) @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; @@ -129,6 +130,7 @@ private boolean requiresSaveToFile; private boolean requiresUploadToServer; + private final AtomicBoolean isReadOnly = new AtomicBoolean(false); /** * List of validation errors in this layer. @@ -420,6 +422,11 @@ if (isUploadDiscouraged() || data.getUploadPolicy() == UploadPolicy.BLOCKED) { base.addOverlay(new ImageOverlay(new ImageProvider("warning-small"), 0.5, 0.5, 1.0, 1.0)); } + + if (isReadOnly()) { + // If the layer is read only then change the default icon to a clock + base = new ImageProvider("clock").setMaxSize(ImageSizes.LAYER); + } return base.get(); } @@ -1142,4 +1149,20 @@ } super.setName(name); } + + public void setReadOnly() { + if (!isReadOnly.compareAndSet(false, true)) { + Logging.warn("Trying to set readOnly flag on a readOnly layer ", this.getName()); + } + } + + public void unsetReadOnly() { + if (!isReadOnly.compareAndSet(true, false)) { + Logging.warn("Trying to unset readOnly flag on a non-readOnly layer ", this.getName()); + } + } + + public boolean isReadOnly() { + return isReadOnly.get(); + } } Index: test/unit/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTaskTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTaskTest.java (nonexistent) +++ test/unit/org/openstreetmap/josm/gui/io/AsynchronousUploadPrimitivesTaskTest.java (working copy) @@ -0,0 +1,74 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.io; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.junit.Rule; +import org.junit.Before; +import org.junit.After; +import org.junit.Test; +import org.junit.Assert; +import org.openstreetmap.josm.data.APIDataSet; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.Changeset; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.io.UploadStrategySpecification; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import java.util.Optional; + +public class AsynchronousUploadPrimitivesTaskTest { + + private UploadStrategySpecification strategy; + private OsmDataLayer layer; + private APIDataSet toUpload; + private Changeset changeset; + private AsynchronousUploadPrimitivesTask uploadPrimitivesTask; + + /** + * Setup tests + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules(); + + @Before + public void bootStrap() { + DataSet dataSet = new DataSet(); + Node node1 = new Node(); + Node node2 = new Node(); + node1.setCoor(new LatLon(0, 0)); + node2.setCoor(new LatLon(30, 30)); + Way way = new Way(); + way.addNode(node1); + way.addNode(node2); + dataSet.addPrimitive(node1); + dataSet.addPrimitive(node2); + dataSet.addPrimitive(way); + + toUpload = new APIDataSet(dataSet); + layer = new OsmDataLayer(dataSet, "uploadTest", null); + strategy = new UploadStrategySpecification(); + changeset = new Changeset(); + uploadPrimitivesTask = AsynchronousUploadPrimitivesTask.createAsynchronousUploadTask(strategy, layer, toUpload, changeset).get(); + } + + @After + public void tearDown() { + toUpload = null; + layer = null; + strategy = null; + changeset = null; + uploadPrimitivesTask = null; + } + + @Test + public void testSingleUploadInstance() { + Optional task = AsynchronousUploadPrimitivesTask. + createAsynchronousUploadTask(strategy, layer, toUpload, changeset); + Assert.assertNotNull(uploadPrimitivesTask); + Assert.assertFalse(task.isPresent()); + } +}