Ticket #13018: patch-test-AutosaveTaskTest-use-rules.patch

src/org/openstreetmap/josm/data/AutosaveTask.java

@@ -23 +23 @@
 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 +109 @@
         return autosaveDir.toPath();
     }
 
+    /**
+     * Starts the autosave background task.
+     */
     public void schedule() {
         if (PROP_INTERVAL.get() > 0) {
 
@@ -128 +133 @@
 
             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 +356 @@
         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<File> 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 +415 @@
         }
     }
 
+    /**
+     * Mark all unsaved layers as deleted. They are still preserved in the deleted layers folder.
+     */
     public void discardUnsavedLayers() {
         for (File f: getUnsavedLayersFiles()) {
             moveToDeletedLayersFolder(f);

src/org/openstreetmap/josm/gui/MainApplication.java

@@ -409 +409 @@
                 !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);

src/org/openstreetmap/josm/gui/MainPanel.java

@@ -40 +40 @@
     public MainPanel(MainLayerManager layerManager) {
         super(new BorderLayout());
         this.layerManager = layerManager;
-        reAddListeners();
     }
 
     /**

test/unit/org/openstreetmap/josm/data/AutosaveTaskTest.java

@@ -8 +8 @@
 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 +67 @@
      */
     @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<File> 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<File> files = task.getUnsavedLayersFiles();
+        assertEquals(2, files.size());
+        assertTrue(files.contains(layer1));
+        assertTrue(files.contains(layer2));
+        assertFalse(files.contains(dir));
     }
 
     /**
@@ -78 +85 @@
      */
     @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<File> 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("<?xml version=\"1.0\"?><osm version=\"0.6\"><node id=\"1\" lat=\"1\" lon=\"2\" version=\"1\"/></osm>");
+        }
+
+        assertEquals(2, countFiles());
+        task.recoverUnsavedLayers().get();
+
+        assertEquals(1, countFiles());
+    }
 }