diff --git a/src/org/openstreetmap/josm/data/AutosaveTask.java b/src/org/openstreetmap/josm/data/AutosaveTask.java index 7dcddc6..9aa5580 100644 --- a/src/org/openstreetmap/josm/data/AutosaveTask.java +++ b/src/org/openstreetmap/josm/data/AutosaveTask.java @@ -23,6 +23,8 @@ import java.util.List; import java.util.Set; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import java.util.regex.Pattern; import org.openstreetmap.josm.Main; @@ -107,6 +109,9 @@ public class AutosaveTask extends TimerTask implements LayerChangeListener, List return autosaveDir.toPath(); } + /** + * Starts the autosave background task. + */ public void schedule() { if (PROP_INTERVAL.get() > 0) { @@ -128,12 +133,7 @@ public class AutosaveTask extends TimerTask implements LayerChangeListener, List timer = new Timer(true); timer.schedule(this, 1000L, PROP_INTERVAL.get() * 1000L); - Main.getLayerManager().addLayerChangeListener(this); - if (Main.isDisplayingMapView()) { - for (OsmDataLayer l: Main.getLayerManager().getLayersOfType(OsmDataLayer.class)) { - registerNewlayer(l); - } - } + Main.getLayerManager().addLayerChangeListener(this, true); } } @@ -356,15 +356,25 @@ public class AutosaveTask extends TimerTask implements LayerChangeListener, List return false; } - public void recoverUnsavedLayers() { + /** + * Recover the unsaved layers and open them asynchronously. + * @return A future that can be used to wait for the completion of this task. + */ + public Future recoverUnsavedLayers() { List files = getUnsavedLayersFiles(); final OpenFileTask openFileTsk = new OpenFileTask(files, null, tr("Restoring files")); - Main.worker.submit(openFileTsk); - Main.worker.submit(new Runnable() { + final Future openFilesFuture = Main.worker.submit(openFileTsk); + return Main.worker.submit(new Runnable() { @Override public void run() { - for (File f: openFileTsk.getSuccessfullyOpenedFiles()) { - moveToDeletedLayersFolder(f); + try { + // Wait for opened tasks to be generated. + openFilesFuture.get(); + for (File f: openFileTsk.getSuccessfullyOpenedFiles()) { + moveToDeletedLayersFolder(f); + } + } catch (InterruptedException | ExecutionException e) { + Main.error(e); } } }); @@ -405,6 +415,9 @@ public class AutosaveTask extends TimerTask implements LayerChangeListener, List } } + /** + * Mark all unsaved layers as deleted. They are still preserved in the deleted layers folder. + */ public void discardUnsavedLayers() { for (File f: getUnsavedLayersFiles()) { moveToDeletedLayersFolder(f); diff --git a/src/org/openstreetmap/josm/gui/MainApplication.java b/src/org/openstreetmap/josm/gui/MainApplication.java index 58ebfc2..da6d626 100644 --- a/src/org/openstreetmap/josm/gui/MainApplication.java +++ b/src/org/openstreetmap/josm/gui/MainApplication.java @@ -409,6 +409,7 @@ public class MainApplication extends Main { !args.containsKey(Option.NO_MAXIMIZE) && Main.pref.getBoolean("gui.maximized", false)); final MainFrame mainFrame = new MainFrame(contentPanePrivate, mainPanel, geometry); Main.parent = mainFrame; + mainPanel.reAddListeners(); if (args.containsKey(Option.LOAD_PREFERENCES)) { CustomConfigurator.XMLCommandProcessor config = new CustomConfigurator.XMLCommandProcessor(Main.pref); diff --git a/src/org/openstreetmap/josm/gui/MainPanel.java b/src/org/openstreetmap/josm/gui/MainPanel.java index e019a9e..0762e3b 100644 --- a/src/org/openstreetmap/josm/gui/MainPanel.java +++ b/src/org/openstreetmap/josm/gui/MainPanel.java @@ -40,7 +40,6 @@ public class MainPanel extends JPanel { public MainPanel(MainLayerManager layerManager) { super(new BorderLayout()); this.layerManager = layerManager; - reAddListeners(); } /** diff --git a/test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java b/test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java index 1eea7b7..baf3f73 100644 --- a/test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java +++ b/test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java @@ -8,37 +8,49 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.FileWriter; +import java.io.FilenameFilter; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; -import java.util.ArrayList; import java.util.Calendar; import java.util.Date; import java.util.List; +import java.util.concurrent.ExecutionException; -import org.junit.BeforeClass; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.openstreetmap.josm.JOSMFixture; +import org.openstreetmap.josm.Main; import org.openstreetmap.josm.data.AutosaveTask.AutosaveLayerInfo; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; /** * Unit tests for class {@link AutosaveTask}. */ public class AutosaveTaskTest { + /** + * We need preferences and a home directory for this. + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules().preferences().platform().projection(); - private static AutosaveTask task; + private AutosaveTask task; /** * Setup test. * @throws IOException if autosave directory cannot be created */ - @BeforeClass - public static void setUpBeforeClass() throws IOException { - JOSMFixture.createUnitTestFixture().init(); + @Before + public void setUp() throws IOException { task = new AutosaveTask(); - Files.createDirectories(task.getAutosaveDir()); } /** @@ -55,21 +67,16 @@ public class AutosaveTaskTest { */ @Test public void testGetUnsavedLayersFilesNotEmpty() throws IOException { + Files.createDirectories(task.getAutosaveDir()); String autodir = task.getAutosaveDir().toString(); File layer1 = Files.createFile(Paths.get(autodir, "layer1.osm")).toFile(); File layer2 = Files.createFile(Paths.get(autodir, "layer2.osm")).toFile(); File dir = Files.createDirectory(Paths.get(autodir, "dir.osm")).toFile(); - try { - List files = task.getUnsavedLayersFiles(); - assertEquals(2, files.size()); - assertTrue(files.contains(layer1)); - assertTrue(files.contains(layer2)); - assertFalse(files.contains(dir)); - } finally { - Files.delete(dir.toPath()); - Files.delete(layer2.toPath()); - Files.delete(layer1.toPath()); - } + List files = task.getUnsavedLayersFiles(); + assertEquals(2, files.size()); + assertTrue(files.contains(layer1)); + assertTrue(files.contains(layer2)); + assertFalse(files.contains(dir)); } /** @@ -78,42 +85,169 @@ public class AutosaveTaskTest { */ @Test public void testGetNewLayerFile() throws IOException { + Files.createDirectories(task.getAutosaveDir()); AutosaveLayerInfo info = new AutosaveLayerInfo(new OsmDataLayer(new DataSet(), "layer", null)); Calendar cal = Calendar.getInstance(); cal.set(2016, 0, 1, 1, 2, 3); cal.set(Calendar.MILLISECOND, 456); Date fixed = cal.getTime(); - List files = new ArrayList<>(); - - try { - for (int i = 0; i <= AutosaveTask.PROP_INDEX_LIMIT.get()+1; i++) { - // Only retry 2 indexes to avoid 1000*1000 disk operations - File f = task.getNewLayerFile(info, fixed, Math.max(0, i-2)); - files.add(f); - if (i > AutosaveTask.PROP_INDEX_LIMIT.get()) { - assertNull(f); + for (int i = 0; i <= AutosaveTask.PROP_INDEX_LIMIT.get() + 1; i++) { + // Only retry 2 indexes to avoid 1000*1000 disk operations + File f = task.getNewLayerFile(info, fixed, Math.max(0, i - 2)); + if (i > AutosaveTask.PROP_INDEX_LIMIT.get()) { + assertNull(f); + } else { + assertNotNull(f); + File pid = task.getPidFile(f); + assertTrue(pid.exists()); + assertTrue(f.exists()); + if (i == 0) { + assertEquals("null_20160101_010203456.osm", f.getName()); + assertEquals("null_20160101_010203456.pid", pid.getName()); } else { - assertNotNull(f); - File pid = task.getPidFile(f); - assertTrue(pid.exists()); - assertTrue(f.exists()); - if (i == 0) { - assertEquals("null_20160101_010203456.osm", f.getName()); - assertEquals("null_20160101_010203456.pid", pid.getName()); - } else { - assertEquals("null_20160101_010203456_"+i+".osm", f.getName()); - assertEquals("null_20160101_010203456_"+i+".pid", pid.getName()); - } + assertEquals("null_20160101_010203456_" + i + ".osm", f.getName()); + assertEquals("null_20160101_010203456_" + i + ".pid", pid.getName()); } } + } + } + + /** + * Tests if {@link AutosaveTask#schedule()} creates the directories. + */ + @Test + public void testScheduleCreatesDirectories() { + try { + task.schedule(); + assertTrue(task.getAutosaveDir().toFile().isDirectory()); } finally { - for (File f : files) { - if (f != null) { - Files.delete(task.getPidFile(f).toPath()); - Files.delete(f.toPath()); - } + task.cancel(); + } + } + + /** + * Tests that {@link AutosaveTask#run()} saves every layer + */ + @Test + public void testAutosaveIgnoresUnmodifiedLayer() { + OsmDataLayer layer = new OsmDataLayer(new DataSet(), "OsmData", null); + Main.getLayerManager().addLayer(layer); + try { + task.schedule(); + assertEquals(0, countFiles()); + task.run(); + assertEquals(0, countFiles()); + } finally { + task.cancel(); + } + } + + private int countFiles() { + return task.getAutosaveDir().toFile().list(new FilenameFilter() { + @Override + public boolean accept(File dir, String name) { + return name.endsWith(".osm"); } + }).length; + } + + /** + * Tests that {@link AutosaveTask#run()} saves every layer. + */ + @Test + public void testAutosaveSavesLayer() { + runAutosaveTaskSeveralTimes(1); + } + + /** + * Tests that {@link AutosaveTask#run()} saves every layer. + */ + @Test + public void testAutosaveSavesLayerMultipleTimes() { + AutosaveTask.PROP_FILES_PER_LAYER.put(3); + runAutosaveTaskSeveralTimes(5); + } + + private void runAutosaveTaskSeveralTimes(int times) { + DataSet data = new DataSet(); + OsmDataLayer layer = new OsmDataLayer(data, "OsmData", null); + Main.getLayerManager().addLayer(layer); + try { + task.schedule(); + assertEquals(0, countFiles()); + + for (int i = 0; i < times; i++) { + data.addPrimitive(new Node(new LatLon(10, 10))); + task.run(); + assertEquals(Math.min(i + 1, 3), countFiles()); + } + + } finally { + task.cancel(); + } + } + + /** + * Tests that {@link AutosaveTask#discardUnsavedLayers()} ignores layers from the current instance + * @throws IOException + */ + @Test + public void testDiscardUnsavedLayersIgnoresCurrentInstance() throws IOException { + runAutosaveTaskSeveralTimes(1); + try (FileWriter file = new FileWriter(new File(task.getAutosaveDir().toFile(), "any_other_file.osm"))) { + file.append(""); + } + assertEquals(2, countFiles()); + + task.discardUnsavedLayers(); + assertEquals(1, countFiles()); + } + + /** + * Tests that {@link AutosaveTask#run()} handles dupplicate layers + * @throws InterruptedException + */ + @Test + public void testAutosaveHandlesDupplicateNames() { + DataSet data1 = new DataSet(); + OsmDataLayer layer1 = new OsmDataLayer(data1, "OsmData", null); + Main.getLayerManager().addLayer(layer1); + + DataSet data2 = new DataSet(); + OsmDataLayer layer2 = new OsmDataLayer(data2, "OsmData", null); + + try { + task.schedule(); + assertEquals(0, countFiles()); + // also test adding layer later + Main.getLayerManager().addLayer(layer2); + + data1.addPrimitive(new Node(new LatLon(10, 10))); + data2.addPrimitive(new Node(new LatLon(10, 10))); + task.run(); + assertEquals(2, countFiles()); + } finally { + task.cancel(); } } + + /** + * Test that + * @throws IOException + * @throws ExecutionException + * @throws InterruptedException + */ + @Test + public void testRecoverLayers() throws IOException, InterruptedException, ExecutionException { + runAutosaveTaskSeveralTimes(1); + try (FileWriter file = new FileWriter(new File(task.getAutosaveDir().toFile(), "any_other_file.osm"))) { + file.append(""); + } + + assertEquals(2, countFiles()); + task.recoverUnsavedLayers().get(); + + assertEquals(1, countFiles()); + } }