Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryData.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryData.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/MapillaryData.java (revision 31500) @@ -67,10 +67,13 @@ /** - * Removes an image from the database. - * - * @param image + * Removes an image from the database. From the ArrayList in this object and + * from its {@link MapillarySequence}. + * + * @param image + * The {@link MapillaryAbstractImage} that is going to be deleted. */ public synchronized void remove(MapillaryAbstractImage image) { - if (MapillaryMainDialog.getInstance().getImage() != null) { + if (Main.main != null + && MapillaryMainDialog.getInstance().getImage() != null) { MapillaryMainDialog.getInstance().setImage(null); MapillaryMainDialog.getInstance().updateImage(); @@ -88,4 +91,6 @@ * * @param images + * The set of {@link MapillaryAbstractImage} objects that are going + * to be removed. */ public synchronized void remove(List images) { Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandDelete.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandDelete.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandDelete.java (revision 31500) @@ -10,4 +10,6 @@ /** + * Command used to delete a set of images. + * * @author nokutu * @@ -18,5 +20,8 @@ /** + * Main constructor. + * * @param images + * The set of images that are going to be deleted. */ public CommandDelete(List images) { @@ -27,5 +32,4 @@ @Override public void sum(MapillaryCommand command) { - // Ignored } Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandImport.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandImport.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandImport.java (revision 31500) @@ -22,4 +22,6 @@ * * @param images + * The set of images that are going to be added. Might be in the same + * sequence or not. */ public CommandImport(List images) { @@ -47,5 +49,4 @@ @Override public void sum(MapillaryCommand command) { - // IGNORE } Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandJoin.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandJoin.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandJoin.java (revision 31500) @@ -6,5 +6,4 @@ import org.openstreetmap.josm.plugins.mapillary.MapillaryAbstractImage; -import org.openstreetmap.josm.plugins.mapillary.MapillaryImportedImage; import org.openstreetmap.josm.plugins.mapillary.utils.MapillaryUtils; @@ -21,7 +20,13 @@ * * @param images + * The two images that are going to be joined. Must be of exactly + * size 2. The first one joins to the second one. + * @throws IllegalArgumentException + * if the List size is different from 2. */ public CommandJoin(List images) { super(images); + if (images.size() != 2) + throw new IllegalArgumentException(); } @@ -33,18 +38,14 @@ @Override public void undo() { - MapillaryUtils.unjoin((MapillaryImportedImage) this.images.get(0), - (MapillaryImportedImage) this.images.get(1)); + MapillaryUtils.unjoin(this.images.get(0), this.images.get(1)); } @Override public void redo() { - MapillaryUtils.join((MapillaryImportedImage) this.images.get(0), - (MapillaryImportedImage) this.images.get(1)); + MapillaryUtils.join(this.images.get(0), this.images.get(1)); } @Override public void sum(MapillaryCommand command) { - // TODO Auto-generated method stub - } Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandUnjoin.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandUnjoin.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/CommandUnjoin.java (revision 31500) @@ -6,5 +6,4 @@ import org.openstreetmap.josm.plugins.mapillary.MapillaryAbstractImage; -import org.openstreetmap.josm.plugins.mapillary.MapillaryImportedImage; import org.openstreetmap.josm.plugins.mapillary.utils.MapillaryUtils; @@ -21,7 +20,13 @@ * * @param images + * The two images that are going to be unjoined. Must be of exactly + * size 2. + * @throws IllegalArgumentException + * if the List size is different from 2. */ public CommandUnjoin(List images) { super(images); + if (images.size() != 2) + throw new IllegalArgumentException(); } @@ -33,17 +38,14 @@ @Override public void undo() { - MapillaryUtils.join((MapillaryImportedImage) this.images.get(0), - (MapillaryImportedImage) this.images.get(1)); + MapillaryUtils.join(this.images.get(0), this.images.get(1)); } @Override public void redo() { - MapillaryUtils.unjoin((MapillaryImportedImage) this.images.get(0), - (MapillaryImportedImage) this.images.get(1)); + MapillaryUtils.unjoin(this.images.get(0), this.images.get(1)); } @Override public void sum(MapillaryCommand command) { - // IGNORE } Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/MapillaryExecutableCommand.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/MapillaryExecutableCommand.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/history/commands/MapillaryExecutableCommand.java (revision 31500) @@ -17,4 +17,5 @@ * * @param images + * The set of images affected by the command. */ public MapillaryExecutableCommand(List images) { @@ -23,5 +24,6 @@ /** - * Executes the command. + * Executes the command. It is run when the command is added to the history + * record. */ public abstract void execute(); Index: /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryUtils.java =================================================================== --- /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryUtils.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/src/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryUtils.java (revision 31500) @@ -19,5 +19,4 @@ import org.openstreetmap.josm.plugins.mapillary.MapillaryAbstractImage; import org.openstreetmap.josm.plugins.mapillary.MapillaryData; -import org.openstreetmap.josm.plugins.mapillary.MapillaryImportedImage; import org.openstreetmap.josm.plugins.mapillary.MapillaryLayer; import org.openstreetmap.josm.plugins.mapillary.MapillarySequence; @@ -137,15 +136,15 @@ * Joins two images into the same sequence. * - * @param img1 - * @param img2 - */ - public synchronized static void join(MapillaryImportedImage img1, - MapillaryImportedImage img2) { - MapillaryImportedImage firstImage = img1; - MapillaryImportedImage secondImage = img2; - - if (img1.next() != null) { - firstImage = img2; - secondImage = img1; + * @param mapillaryAbstractImage + * @param mapillaryAbstractImage2 + */ + public synchronized static void join(MapillaryAbstractImage mapillaryAbstractImage, + MapillaryAbstractImage mapillaryAbstractImage2) { + MapillaryAbstractImage firstImage = mapillaryAbstractImage; + MapillaryAbstractImage secondImage = mapillaryAbstractImage2; + + if (mapillaryAbstractImage.next() != null) { + firstImage = mapillaryAbstractImage2; + secondImage = mapillaryAbstractImage; } if (firstImage.getSequence() == null) { @@ -157,5 +156,5 @@ MapillarySequence seq = new MapillarySequence(); seq.add(secondImage); - img2.setSequence(seq); + mapillaryAbstractImage2.setSequence(seq); } @@ -171,15 +170,15 @@ * Separates two images belonging to the same sequence. * - * @param img1 - * @param img2 - */ - public synchronized static void unjoin(MapillaryImportedImage img1, - MapillaryImportedImage img2) { - MapillaryImportedImage firstImage = img1; - MapillaryImportedImage secondImage = img2; - - if (img1.next() != img2) { - firstImage = img2; - secondImage = img1; + * @param mapillaryAbstractImage + * @param mapillaryAbstractImage2 + */ + public synchronized static void unjoin(MapillaryAbstractImage mapillaryAbstractImage, + MapillaryAbstractImage mapillaryAbstractImage2) { + MapillaryAbstractImage firstImage = mapillaryAbstractImage; + MapillaryAbstractImage secondImage = mapillaryAbstractImage2; + + if (mapillaryAbstractImage.next() != mapillaryAbstractImage2) { + firstImage = mapillaryAbstractImage2; + secondImage = mapillaryAbstractImage; } Index: /applications/editors/josm/plugins/mapillary/test/unit/org/openstreetmap/josm/plugins/mapillary/history/MapillaryRecordTest.java =================================================================== --- /applications/editors/josm/plugins/mapillary/test/unit/org/openstreetmap/josm/plugins/mapillary/history/MapillaryRecordTest.java (revision 31499) +++ /applications/editors/josm/plugins/mapillary/test/unit/org/openstreetmap/josm/plugins/mapillary/history/MapillaryRecordTest.java (revision 31500) @@ -2,4 +2,5 @@ import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import java.util.Arrays; @@ -9,8 +10,14 @@ import org.openstreetmap.josm.plugins.mapillary.AbstractTest; import org.openstreetmap.josm.plugins.mapillary.MapillaryAbstractImage; +import org.openstreetmap.josm.plugins.mapillary.MapillaryData; import org.openstreetmap.josm.plugins.mapillary.MapillaryImage; +import org.openstreetmap.josm.plugins.mapillary.MapillaryLayer; import org.openstreetmap.josm.plugins.mapillary.history.MapillaryRecord; +import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandDelete; +import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandImport; +import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandJoin; import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandMove; import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandTurn; +import org.openstreetmap.josm.plugins.mapillary.history.commands.CommandUnjoin; import org.openstreetmap.josm.plugins.mapillary.history.commands.MapillaryCommand; @@ -38,4 +45,5 @@ this.img2 = new MapillaryImage("key2", 0.2, 0.2, 0.2); this.img3 = new MapillaryImage("key3", 0.3, 0.3, 0.3); + MapillaryLayer.getInstance().getData().getImages().clear(); } @@ -55,6 +63,5 @@ 0.1, 0.1); MapillaryCommand cmd1 = new CommandMove( - Arrays.asList(new MapillaryAbstractImage[] { this.img1 }), - 0.1, 0.1); + Arrays.asList(new MapillaryAbstractImage[] { this.img1 }), 0.1, 0.1); MapillaryCommand cmd31 = new CommandMove( Arrays.asList(new MapillaryAbstractImage[] { this.img3, this.img1 }), @@ -98,5 +105,5 @@ /** - * Tests CommandMoveImage class. + * Tests {@link CommandMove} class. */ @Test @@ -132,5 +139,5 @@ /** - * Tests CommandTurnImage class. + * Tests {@link CommandTurn} class. */ @Test @@ -161,3 +168,133 @@ assertEquals(0.1, this.img1.getCa(), 0.01); } + + /** + * Tests {@link CommandJoin} class. + */ + @Test + public void commandJoinClass() { + CommandJoin cmd1 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img1, this.img2 })); + CommandJoin cmd2 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + this.record.addCommand(cmd1); + assertEquals(2, this.img1.getSequence().getImages().size()); + assertEquals(this.img2, this.img1.next()); + this.record.undo(); + assertEquals(1, this.img1.getSequence().getImages().size()); + this.record.redo(); + this.record.addCommand(cmd2); + assertEquals(3, this.img1.getSequence().getImages().size()); + assertEquals(this.img3, this.img1.next().next()); + + try { + this.record.addCommand(new CommandJoin(Arrays + .asList(new MapillaryAbstractImage[] { this.img1, this.img2, + this.img3 }))); + fail(); + } catch (IllegalArgumentException e) { + // Expected output. + } catch (Exception e) { + fail(); + } + } + + /** + * Tests {@link CommandUnjoin} class. + */ + @Test + public void commandUnjoinClass() { + CommandJoin join1 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img1, this.img2 })); + CommandJoin join2 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + CommandUnjoin cmd1 = new CommandUnjoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img1, this.img2 })); + CommandUnjoin cmd2 = new CommandUnjoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + this.record.addCommand(join1); + this.record.addCommand(join2); + + this.record.addCommand(cmd1); + assertEquals(1, this.img1.getSequence().getImages().size()); + this.record.undo(); + assertEquals(3, this.img1.getSequence().getImages().size()); + this.record.redo(); + this.record.addCommand(cmd2); + assertEquals(1, this.img1.getSequence().getImages().size()); + assertEquals(1, this.img2.getSequence().getImages().size()); + + try { + this.record.addCommand(new CommandUnjoin(Arrays + .asList(new MapillaryAbstractImage[] { this.img1, this.img2, + this.img3 }))); + fail(); + } catch (IllegalArgumentException e) { + // Expected output. + } catch (Exception e) { + fail(); + } + } + + /** + * Test {@link CommandDelete} class. + */ + @Test + public void commandDeleteTest() { + CommandJoin join1 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img1, this.img2 })); + CommandJoin join2 = new CommandJoin( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + CommandDelete cmd1 = new CommandDelete( + Arrays.asList(new MapillaryAbstractImage[] { this.img1 })); + CommandDelete cmd2 = new CommandDelete( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + this.record.addCommand(join1); + this.record.addCommand(join2); + + MapillaryLayer + .getInstance() + .getData() + .add( + Arrays.asList(new MapillaryAbstractImage[] { this.img1, this.img2, + this.img3 })); + + this.record.addCommand(cmd1); + assertEquals(false, MapillaryLayer.getInstance().getData().getImages() + .contains(this.img1)); + assertEquals(null, this.img2.previous()); + this.record.undo(); + assertEquals(true, MapillaryLayer.getInstance().getData().getImages() + .contains(this.img1)); + this.record.redo(); + this.record.addCommand(cmd2); + assertEquals(0, MapillaryLayer.getInstance().getData().size()); + } + + /** + * Test {@link CommandImport} class. + */ + @Test + public void commandImportTest() { + MapillaryData data = MapillaryLayer.getInstance().getData(); + data.remove(data.getImages()); + + CommandImport cmd1 = new CommandImport( + Arrays.asList(new MapillaryAbstractImage[] { this.img1 })); + CommandImport cmd2 = new CommandImport( + Arrays.asList(new MapillaryAbstractImage[] { this.img2, this.img3 })); + + this.record.addCommand(cmd1); + assertEquals(1, data.size()); + this.record.undo(); + assertEquals(0, data.size()); + this.record.redo(); + this.record.addCommand(cmd2); + assertEquals(3, data.size()); + } }