Changeset 18965 in josm for trunk


Ignore:
Timestamp:
2024-01-30T20:06:45+01:00 (10 months ago)
Author:
taylor.smock
Message:

See #23444: Fix an additional NPE found during testing ImproveWayAccuracyAction

The NPE could be reproduced by selecting a node with only an incomplete
parent way, and switching from select mode to improve way mode.

This additionally adds simple tests for ImproveWayAccuracyAction and refactors
some methods to have a lower complexity.

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyAction.java

    r18274 r18965  
    3232import org.openstreetmap.josm.data.UndoRedoHandler;
    3333import org.openstreetmap.josm.data.coor.EastNorth;
     34import org.openstreetmap.josm.data.coor.ILatLon;
    3435import org.openstreetmap.josm.data.osm.DataSelectionListener;
    3536import org.openstreetmap.josm.data.osm.DataSet;
    … …  
    269270            // Non-native highlighting is used here as well.
    270271
    271             // Finding endpoints
    272             Node p1 = null;
    273             Node p2 = null;
    274272            if (ctrl && candidateSegment != null) {
    275                 g.setStroke(ADD_NODE_STROKE.get());
    276                 try {
    277                     p1 = candidateSegment.getFirstNode();
    278                     p2 = candidateSegment.getSecondNode();
    279                 } catch (ArrayIndexOutOfBoundsException e) {
    280                     Logging.error(e);
    281                 }
     273                paintAddNodeStroke(g);
    282274            } else if (!alt && !ctrl && candidateNode != null) {
    283                 g.setStroke(MOVE_NODE_STROKE.get());
    284                 List<Pair<Node, Node>> wpps = targetWay.getNodePairs(false);
    285                 for (Pair<Node, Node> wpp : wpps) {
    286                     if (wpp.a == candidateNode) {
    287                         p1 = wpp.b;
    288                     }
    289                     if (wpp.b == candidateNode) {
    290                         p2 = wpp.a;
    291                     }
    292                     if (p1 != null && p2 != null) {
    293                         break;
    294                     }
    295                 }
     275                paintMoveNodeStroke(g);
    296276            } else if (alt && !ctrl && candidateNode != null) {
    297                 g.setStroke(DELETE_NODE_STROKE.get());
    298                 List<Node> nodes = targetWay.getNodes();
    299                 int index = nodes.indexOf(candidateNode);
    300 
    301                 // Only draw line if node is not first and/or last
    302                 if (index > 0 && index < (nodes.size() - 1)) {
    303                     p1 = nodes.get(index - 1);
    304                     p2 = nodes.get(index + 1);
    305                 } else if (targetWay.isClosed()) {
    306                     p1 = targetWay.getNode(1);
    307                     p2 = targetWay.getNode(nodes.size() - 2);
    308                 }
    309                 // TODO: indicate what part that will be deleted? (for end nodes)
    310             }
    311 
    312 
    313             // Drawing preview lines
    314             MapViewPath b = new MapViewPath(mv);
    315             if (alt && !ctrl) {
    316                 // In delete mode
    317                 if (p1 != null && p2 != null) {
    318                     b.moveTo(p1);
    319                     b.lineTo(p2);
    320                 }
    321             } else {
    322                 // In add or move mode
    323                 if (p1 != null) {
    324                     b.moveTo(mousePos.x, mousePos.y);
    325                     b.lineTo(p1);
    326                 }
    327                 if (p2 != null) {
    328                     b.moveTo(mousePos.x, mousePos.y);
    329                     b.lineTo(p2);
    330                 }
    331             }
     277                paintDeleteStroke(g);
     278            }
     279        }
     280    }
     281
     282    /**
     283     * Paint the add stroke for {@link State#IMPROVING}
     284     * @param g The graphics
     285     */
     286    private void paintAddNodeStroke(Graphics2D g) {
     287        g.setStroke(ADD_NODE_STROKE.get());
     288        ILatLon p1 = null;
     289        ILatLon p2 = null;
     290        try {
     291            p1 = candidateSegment.getFirstNode();
     292            p2 = candidateSegment.getSecondNode();
     293        } catch (ArrayIndexOutOfBoundsException e) {
     294            Logging.error(e);
     295        }
     296        paintImprovingPreviewLines(g, p1, p2);
     297    }
     298
     299    /**
     300     * Paint the move stroke for {@link State#IMPROVING}
     301     * @param g The graphics
     302     */
     303    private void paintMoveNodeStroke(Graphics2D g) {
     304        g.setStroke(MOVE_NODE_STROKE.get());
     305        List<Pair<Node, Node>> wpps = targetWay.getNodePairs(false);
     306        ILatLon p1 = null;
     307        ILatLon p2 = null;
     308        for (Pair<Node, Node> wpp : wpps) {
     309            if (wpp.a == candidateNode) {
     310                p1 = wpp.b;
     311            }
     312            if (wpp.b == candidateNode) {
     313                p2 = wpp.a;
     314            }
     315            if (p1 != null && p2 != null) {
     316                break;
     317            }
     318        }
     319        paintImprovingPreviewLines(g, p1, p2);
     320    }
     321
     322    /**
     323     * Paint the delete stroke for {@link State#IMPROVING}
     324     * @param g The graphics
     325     */
     326    private void paintDeleteStroke(Graphics2D g) {
     327        g.setStroke(DELETE_NODE_STROKE.get());
     328        List<Node> nodes = targetWay.getNodes();
     329        int index = nodes.indexOf(candidateNode);
     330        ILatLon p1 = null;
     331        ILatLon p2 = null;
     332        // Only draw line if node is not first and/or last
     333        if (index > 0 && index < (nodes.size() - 1)) {
     334            p1 = nodes.get(index - 1);
     335            p2 = nodes.get(index + 1);
     336        } else if (targetWay.isClosed()) {
     337            p1 = targetWay.getNode(1);
     338            p2 = targetWay.getNode(nodes.size() - 2);
     339        }
     340        paintImprovingPreviewLines(g, p1, p2);
     341        // TODO: indicate what part that will be deleted? (for end nodes)
     342    }
     343
     344    /**
     345     * Paint the preview lines for {@link State#IMPROVING}
     346     * @param g The graphics
     347     * @param p1 The first endpoint
     348     * @param p2 The second endpoint
     349     */
     350    private void paintImprovingPreviewLines(Graphics2D g, ILatLon p1, ILatLon p2) {
     351        // Drawing preview lines
     352        MapViewPath b = new MapViewPath(mv);
     353        if (alt && !ctrl) {
     354            // In delete mode
     355            if (p1 != null && p2 != null) {
     356                b.moveTo(p1);
     357                b.lineTo(p2);
     358            }
     359        } else {
     360            // In add or move mode
     361            if (p1 != null) {
     362                b.moveTo(mousePos.x, mousePos.y);
     363                b.lineTo(p1);
     364            }
     365            if (p2 != null) {
     366                b.moveTo(mousePos.x, mousePos.y);
     367                b.lineTo(p2);
     368            }
     369        }
     370        g.draw(b.computeClippedLine(g.getStroke()));
     371
     372        // Highlighting candidateNode
     373        if (candidateNode != null) {
     374            p1 = candidateNode;
     375            g.fill(new MapViewPath(mv).shapeAround(p1, SymbolShape.SQUARE, DOT_SIZE.get()));
     376        }
     377
     378        if (!alt && !ctrl && candidateNode != null) {
     379            b.reset();
     380            drawIntersectingWayHelperLines(mv, b);
     381            g.setStroke(MOVE_NODE_INTERSECTING_STROKE.get());
    332382            g.draw(b.computeClippedLine(g.getStroke()));
    333 
    334             // Highlighting candidateNode
    335             if (candidateNode != null) {
    336                 p1 = candidateNode;
    337                 g.fill(new MapViewPath(mv).shapeAround(p1, SymbolShape.SQUARE, DOT_SIZE.get()));
    338             }
    339 
    340             if (!alt && !ctrl && candidateNode != null) {
    341                 b.reset();
    342                 drawIntersectingWayHelperLines(mv, b);
    343                 g.setStroke(MOVE_NODE_INTERSECTING_STROKE.get());
    344                 g.draw(b.computeClippedLine(g.getStroke()));
    345             }
    346 
    347383        }
    348384    }
    … …  
    440476
    441477            if (ctrl && !alt && candidateSegment != null) {
    442                 // Add a new node to the highlighted segment.
    443                 Collection<WaySegment> virtualSegments = new LinkedList<>();
    444 
    445                 // Check if other ways have the same segment.
    446                 // We have to make sure that we add the new node to all of them.
    447                 Set<Way> commonParentWays = new HashSet<>(candidateSegment.getFirstNode().getParentWays());
    448                 commonParentWays.retainAll(candidateSegment.getSecondNode().getParentWays());
    449                 for (Way w : commonParentWays) {
    450                     for (int i = 0; i < w.getNodesCount() - 1; i++) {
    451                         WaySegment testWS = new WaySegment(w, i);
    452                         if (testWS.isSimilar(candidateSegment)) {
    453                             virtualSegments.add(testWS);
    454                         }
    455                     }
    456                 }
    457 
    458                 Collection<Command> virtualCmds = new LinkedList<>();
    459                 // Create the new node
    460                 Node virtualNode = new Node(mv.getEastNorth(mousePos.x, mousePos.y));
    461                 virtualCmds.add(new AddCommand(ds, virtualNode));
    462 
    463                 // Adding the node to all segments found
    464                 for (WaySegment virtualSegment : virtualSegments) {
    465                     Way w = virtualSegment.getWay();
    466                     List<Node> modNodes = w.getNodes();
    467                     modNodes.add(virtualSegment.getUpperIndex(), virtualNode);
    468                     virtualCmds.add(new ChangeNodesCommand(w, modNodes));
    469                 }
    470 
    471                 // Finishing the sequence command
    472                 String text = trn("Add a new node to way",
    473                         "Add a new node to {0} ways",
    474                         virtualSegments.size(), virtualSegments.size());
    475 
    476                 UndoRedoHandler.getInstance().add(new SequenceCommand(text, virtualCmds));
    477 
     478                addNode(ds);
    478479            } else if (alt && !ctrl && candidateNode != null) {
    479                 // Deleting the highlighted node
    480 
    481                 //check to see if node is in use by more than one object
    482                 long referrersCount = candidateNode.referrers(OsmPrimitive.class).count();
    483                 long referrerWayCount = candidateNode.referrers(Way.class).count();
    484                 if (referrersCount != 1 || referrerWayCount != 1) {
    485                     // detach node from way
    486                     final List<Node> nodes = targetWay.getNodes();
    487                     nodes.remove(candidateNode);
    488                     if (nodes.size() < 2) {
    489                         final Command deleteCmd = DeleteCommand.delete(Collections.singleton(targetWay), true);
    490                         if (deleteCmd != null) {
    491                             UndoRedoHandler.getInstance().add(deleteCmd);
    492                         }
    493                     } else {
    494                         UndoRedoHandler.getInstance().add(new ChangeNodesCommand(targetWay, nodes));
    495                     }
    496                 } else if (candidateNode.isTagged()) {
    497                     JOptionPane.showMessageDialog(MainApplication.getMainFrame(),
    498                             tr("Cannot delete node that has tags"),
    499                             tr("Error"), JOptionPane.ERROR_MESSAGE);
    500                 } else {
    501                     final Command deleteCmd = DeleteCommand.delete(Collections.singleton(candidateNode), true);
    502                     if (deleteCmd != null) {
    503                         UndoRedoHandler.getInstance().add(deleteCmd);
    504                     }
    505                 }
    506 
     480                deleteHighlightedNode();
    507481            } else if (candidateNode != null) {
    508                 // Moving the highlighted node
    509                 EastNorth nodeEN = candidateNode.getEastNorth();
    510                 EastNorth cursorEN = mv.getEastNorth(mousePos.x, mousePos.y);
    511 
    512                 UndoRedoHandler.getInstance().add(
    513                         new MoveCommand(candidateNode, cursorEN.east() - nodeEN.east(), cursorEN.north() - nodeEN.north()));
    514 
    515                 SelectAction.checkCommandForLargeDistance(UndoRedoHandler.getInstance().getLastCommand());
     482                moveHighlightedNode();
    516483            }
    517484        }
    … …  
    523490    }
    524491
     492    /**
     493     * Add a new node to the currently highlighted segment
     494     * @param ds The dataset to add the node to
     495     */
     496    private void addNode(DataSet ds) {
     497        // Add a new node to the highlighted segment.
     498        Collection<WaySegment> virtualSegments = new LinkedList<>();
     499
     500        // Check if other ways have the same segment.
     501        // We have to make sure that we add the new node to all of them.
     502        Set<Way> commonParentWays = new HashSet<>(candidateSegment.getFirstNode().getParentWays());
     503        commonParentWays.retainAll(candidateSegment.getSecondNode().getParentWays());
     504        for (Way w : commonParentWays) {
     505            for (int i = 0; i < w.getNodesCount() - 1; i++) {
     506                WaySegment testWS = new WaySegment(w, i);
     507                if (testWS.isSimilar(candidateSegment)) {
     508                    virtualSegments.add(testWS);
     509                }
     510            }
     511        }
     512
     513        Collection<Command> virtualCmds = new LinkedList<>();
     514        // Create the new node
     515        Node virtualNode = new Node(mv.getEastNorth(mousePos.x, mousePos.y));
     516        virtualCmds.add(new AddCommand(ds, virtualNode));
     517
     518        // Adding the node to all segments found
     519        for (WaySegment virtualSegment : virtualSegments) {
     520            Way w = virtualSegment.getWay();
     521            List<Node> modNodes = w.getNodes();
     522            modNodes.add(virtualSegment.getUpperIndex(), virtualNode);
     523            virtualCmds.add(new ChangeNodesCommand(w, modNodes));
     524        }
     525
     526        // Finishing the sequence command
     527        String text = trn("Add a new node to way",
     528                "Add a new node to {0} ways",
     529                virtualSegments.size(), virtualSegments.size());
     530
     531        UndoRedoHandler.getInstance().add(new SequenceCommand(text, virtualCmds));
     532    }
     533
     534    /**
     535     * Delete the highlighted node
     536     */
     537    private void deleteHighlightedNode() {
     538        // Deleting the highlighted node
     539
     540        //check to see if node is in use by more than one object
     541        long referrersCount = candidateNode.referrers(OsmPrimitive.class).count();
     542        long referrerWayCount = candidateNode.referrers(Way.class).count();
     543        if (referrersCount != 1 || referrerWayCount != 1) {
     544            // detach node from way
     545            final List<Node> nodes = targetWay.getNodes();
     546            nodes.remove(candidateNode);
     547            if (nodes.size() < 2) {
     548                final Command deleteCmd = DeleteCommand.delete(Collections.singleton(targetWay), true);
     549                if (deleteCmd != null) {
     550                    UndoRedoHandler.getInstance().add(deleteCmd);
     551                }
     552            } else {
     553                UndoRedoHandler.getInstance().add(new ChangeNodesCommand(targetWay, nodes));
     554            }
     555        } else if (candidateNode.isTagged()) {
     556            JOptionPane.showMessageDialog(MainApplication.getMainFrame(),
     557                    tr("Cannot delete node that has tags"),
     558                    tr("Error"), JOptionPane.ERROR_MESSAGE);
     559        } else {
     560            final Command deleteCmd = DeleteCommand.delete(Collections.singleton(candidateNode), true);
     561            if (deleteCmd != null) {
     562                UndoRedoHandler.getInstance().add(deleteCmd);
     563            }
     564        }
     565    }
     566
     567    /**
     568     * Move the highlighted node
     569     */
     570    private void moveHighlightedNode() {
     571        // Moving the highlighted node
     572        EastNorth nodeEN = candidateNode.getEastNorth();
     573        EastNorth cursorEN = mv.getEastNorth(mousePos.x, mousePos.y);
     574
     575        UndoRedoHandler.getInstance().add(
     576                new MoveCommand(candidateNode, cursorEN.east() - nodeEN.east(), cursorEN.north() - nodeEN.north()));
     577
     578        SelectAction.checkCommandForLargeDistance(UndoRedoHandler.getInstance().getLastCommand());
     579    }
     580
    525581    @Override
    526582    public void mouseExited(MouseEvent e) {
    … …  
    551607                    : cursorSelectHover, this);
    552608        } else if (state == State.IMPROVING) {
    553             if (alt && !ctrl) {
    554                 mv.setNewCursor(cursorImproveDelete, this);
    555             } else if (shift || dragging) {
    556                 if (ctrl) {
    557                     mv.setNewCursor(cursorImproveAddLock, this);
    558                 } else {
    559                     mv.setNewCursor(cursorImproveLock, this);
    560                 }
    561             } else if (ctrl && !alt) {
    562                 mv.setNewCursor(cursorImproveAdd, this);
     609            updateCursorImproving();
     610        }
     611    }
     612
     613    /**
     614     * Update the mouse cursor for the {@link State#IMPROVING} mode
     615     */
     616    private void updateCursorImproving() {
     617        if (alt && !ctrl) {
     618            mv.setNewCursor(cursorImproveDelete, this);
     619        } else if (shift || dragging) {
     620            if (ctrl) {
     621                mv.setNewCursor(cursorImproveAddLock, this);
    563622            } else {
    564                 mv.setNewCursor(cursorImprove, this);
    565             }
     623                mv.setNewCursor(cursorImproveLock, this);
     624            }
     625        } else if (ctrl && !alt) {
     626            mv.setNewCursor(cursorImproveAdd, this);
     627        } else {
     628            mv.setNewCursor(cursorImprove, this);
    566629        }
    567630    }
    … …  
    642705     */
    643706    private void updateStateByCurrentSelection() {
     707        final DataSet ds = getLayerManager().getEditDataSet();
     708        if (ds != null && selectWay(ds)) {
     709            return;
     710        }
     711
     712        // Starting selecting by default
     713        startSelecting();
     714    }
     715
     716    /**
     717     * Select the initial way
     718     * @param ds The dataset to get the selection from
     719     * @return {@code true} if a way was selected
     720     */
     721    private boolean selectWay(DataSet ds) {
    644722        final List<Node> nodeList = new ArrayList<>();
    645723        final List<Way> wayList = new ArrayList<>();
    646         final DataSet ds = getLayerManager().getEditDataSet();
    647         if (ds != null) {
    648             final Collection<OsmPrimitive> sel = ds.getSelected();
    649 
    650             // Collecting nodes and ways from the selection
    651             for (OsmPrimitive p : sel) {
    652                 if (p instanceof Way) {
    653                     wayList.add((Way) p);
    654                 }
    655                 if (p instanceof Node) {
    656                     nodeList.add((Node) p);
    657                 }
    658             }
    659 
    660             if (wayList.size() == 1) {
    661                 // Starting improving the single selected way
    662                 startImproving(wayList.get(0));
    663                 return;
    664             } else if (nodeList.size() == 1) {
    665                 // Starting improving the only way of the single selected node
    666                 List<OsmPrimitive> r = nodeList.get(0).getReferrers();
    667                 if (r.size() == 1 && (r.get(0) instanceof Way)) {
    668                     startImproving((Way) r.get(0));
    669                     return;
    670                 }
    671             }
    672         }
    673 
    674         // Starting selecting by default
    675         startSelecting();
     724        final Collection<OsmPrimitive> sel = ds.getSelected();
     725
     726        // Collecting nodes and ways from the selection
     727        for (OsmPrimitive p : sel) {
     728            if (p instanceof Way) {
     729                wayList.add((Way) p);
     730            }
     731            if (p instanceof Node) {
     732                nodeList.add((Node) p);
     733            }
     734        }
     735
     736        if (wayList.size() == 1) {
     737            // Starting improving the single selected way
     738            startImproving(wayList.get(0));
     739            return true;
     740        } else if (nodeList.size() == 1) {
     741            // Starting improving the only way of the single selected node
     742            List<OsmPrimitive> r = nodeList.get(0).getReferrers();
     743            r.removeIf(osm -> !osm.isUsable());
     744            if (r.size() == 1 && (r.get(0) instanceof Way)) {
     745                startImproving((Way) r.get(0));
     746                return true;
     747            }
     748        }
     749        return false;
    676750    }
    677751

  • trunk/test/unit/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyActionTest.java

    r18963 r18965  
    22package org.openstreetmap.josm.actions.mapmode;
    33
     4import static org.junit.jupiter.api.Assertions.assertAll;
    45import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
    56import static org.junit.jupiter.api.Assertions.assertEquals;
    67import static org.junit.jupiter.api.Assertions.assertFalse;
     8import static org.junit.jupiter.api.Assertions.assertNotEquals;
    79import static org.junit.jupiter.api.Assertions.assertTrue;
    810
    911import java.awt.Graphics2D;
    1012import java.awt.Point;
     13import java.awt.Rectangle;
     14import java.awt.event.InputEvent;
    1115import java.awt.event.MouseEvent;
    1216import java.awt.image.BufferedImage;
    13 
     17import java.util.Collection;
     18import java.util.concurrent.atomic.AtomicReference;
     19
     20import org.apache.commons.lang3.function.TriConsumer;
    1421import org.junit.jupiter.api.Test;
    15 import org.junit.jupiter.api.extension.RegisterExtension;
    1622import org.openstreetmap.josm.TestUtils;
    1723import org.openstreetmap.josm.actions.mapmode.ImproveWayAccuracyAction.State;
     24import org.openstreetmap.josm.command.DeleteCommand;
    1825import org.openstreetmap.josm.data.Bounds;
    1926import org.openstreetmap.josm.data.coor.ILatLon;
    … …  
    2128import org.openstreetmap.josm.data.osm.DataSet;
    2229import org.openstreetmap.josm.data.osm.Node;
     30import org.openstreetmap.josm.data.osm.OsmPrimitive;
     31import org.openstreetmap.josm.data.osm.Relation;
     32import org.openstreetmap.josm.data.osm.RelationToChildReference;
    2333import org.openstreetmap.josm.data.osm.Way;
     34import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
    2435import org.openstreetmap.josm.gui.MainApplication;
    2536import org.openstreetmap.josm.gui.MapFrame;
    2637import org.openstreetmap.josm.gui.MapView;
     38import org.openstreetmap.josm.gui.layer.Layer;
    2739import org.openstreetmap.josm.gui.layer.OsmDataLayer;
    2840import org.openstreetmap.josm.gui.util.GuiHelper;
    2941import org.openstreetmap.josm.testutils.annotations.Main;
    3042import org.openstreetmap.josm.testutils.annotations.Projection;
    31 import org.openstreetmap.josm.testutils.mockers.WindowlessMapViewStateMocker;
    32 import org.openstreetmap.josm.testutils.mockers.WindowlessNavigatableComponentMocker;
    33 
    34 import mockit.Mock;
    3543
    3644/**
    3745 * Unit tests for class {@link ImproveWayAccuracyAction}.
    3846 */
     47@Main
    3948@Projection
    4049class ImproveWayAccuracyActionTest {
    41     @RegisterExtension
    42     Main.MainExtension mainExtension = new Main.MainExtension().setMapViewMocker(SizedWindowlessMapViewStateMocker::new)
    43             .setNavigableComponentMocker(SizedWindowlessNavigatableComponentMocker::new);
    44 
    4550    private static final int WIDTH = 800;
    4651    private static final int HEIGHT = 600;
    47     private static class SizedWindowlessMapViewStateMocker extends WindowlessMapViewStateMocker {
    48         @Mock
    49         public int getWidth() {
    50             return WIDTH;
    51         }
    52 
    53         @Mock
    54         public int getHeight() {
    55             return HEIGHT;
    56         }
    57     }
    58 
    59     private static class SizedWindowlessNavigatableComponentMocker extends WindowlessNavigatableComponentMocker {
    60         @Mock
    61         public int getWidth() {
    62             return WIDTH;
    63         }
    64 
    65         @Mock
    66         public int getHeight() {
    67             return HEIGHT;
    68         }
    69     }
    70 
    71     private static MouseEvent generateEvent(MapView mapView, ILatLon location) {
     52
     53    private static class AlwaysDeleteCallback implements DeleteCommand.DeletionCallback {
     54        @Override
     55        public boolean checkAndConfirmOutlyingDelete(Collection<? extends OsmPrimitive> primitives, Collection<? extends OsmPrimitive> ignore) {
     56            return true;
     57        }
     58
     59        @Override
     60        public boolean confirmRelationDeletion(Collection<Relation> relations) {
     61            return true;
     62        }
     63
     64        @Override
     65        public boolean confirmDeletionFromRelation(Collection<RelationToChildReference> references) {
     66            return true;
     67        }
     68    }
     69
     70    private static void setupMapView(DataSet ds) {
     71        // setup a reasonable size for the edit window
     72        MainApplication.getMap().mapView.setBounds(new Rectangle(WIDTH, HEIGHT));
     73        if (ds.getDataSourceBoundingBox() != null) {
     74            MainApplication.getMap().mapView.zoomTo(ds.getDataSourceBoundingBox());
     75        } else {
     76            BoundingXYVisitor v = new BoundingXYVisitor();
     77            for (Layer l : MainApplication.getLayerManager().getLayers()) {
     78                l.visitBoundingBox(v);
     79            }
     80            MainApplication.getMap().mapView.zoomTo(v);
     81        }
     82    }
     83
     84    /**
     85     * Generate a mouse event
     86     * @param mapView The current map view
     87     * @param location The location to generate the event for
     88     * @param modifiers The modifiers for {@link MouseEvent} (see {@link InputEvent#getModifiersEx()})
     89     * @return The generated event
     90     */
     91    private static MouseEvent generateEvent(MapView mapView, ILatLon location, int modifiers) {
    7292        final Point p = mapView.getPoint(location);
    73         return new MouseEvent(mapView, 0, 0, 0, p.x, p.y, p.x, p.y, 1, false, MouseEvent.BUTTON1);
     93        return new MouseEvent(mapView, 0, 0, modifiers, p.x, p.y, p.x, p.y, 1, false, MouseEvent.BUTTON1);
    7494    }
    7595
    … …  
    102122
    103123    @Test
     124    void testNonRegression23444Selection() {
     125        final DataSet dataSet = new DataSet();
     126        final OsmDataLayer layer = new OsmDataLayer(dataSet, "ImproveWayAccuracyActionTest#testNonRegression23444Selection", null);
     127        MainApplication.getLayerManager().addLayer(layer);
     128        final ImproveWayAccuracyAction mapMode = new ImproveWayAccuracyAction();
     129        final MapFrame map = MainApplication.getMap();
     130        final Way testWay = TestUtils.newWay("", new Node(1, 1), new Node(2, 1),
     131                new Node(3), new Node(4, 1), new Node(5, 1));
     132        testWay.firstNode().setCoor(new LatLon(0, 0));
     133        testWay.lastNode().setCoor(new LatLon(0.001, 0.001));
     134        testWay.getNode(1).setCoor(new LatLon(0.0001, 0.0001));
     135        testWay.getNode(3).setCoor(new LatLon(0.0009, 0.0009));
     136        dataSet.addPrimitiveRecursive(testWay);
     137        assertFalse(testWay.getNode(2).isLatLonKnown(), "The second node should not have valid coordinates");
     138        dataSet.setSelected(testWay.firstNode());
     139        assertTrue(map.selectMapMode(mapMode));
     140        assertEquals(mapMode, map.mapMode);
     141        // This is where the exception occurs; we shouldn't be setting the incomplete way as the target when we enter the mode.
     142        setupMapView(dataSet);
     143        assertDoesNotThrow(() -> GuiHelper.runInEDTAndWaitWithException(() -> {
     144            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0001), 0));
     145        }));
     146    }
     147
     148    @Test
    104149    void testNonRegression23444() {
     150        testSimplifyWayAction((mapMode, map, testWay) -> {
     151            testWay.getNode(2).setCoor(null);
     152            assertFalse(testWay.getNode(2).isLatLonKnown(), "The second node should not have valid coordinates");
     153            mapMode.startSelecting();
     154            mapMode.mouseMoved(generateEvent(map.mapView, testWay.getNode(1), 0));
     155        });
     156    }
     157
     158    @Test
     159    void testAdd() {
     160        AtomicReference<Way> referenceWay = new AtomicReference<>();
     161        testSimplifyWayAction((mapMode, map, testWay) -> {
     162            // Add a node at 0.0001, 0.0005 (not on the direct line)
     163            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), InputEvent.CTRL_DOWN_MASK));
     164            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), InputEvent.CTRL_DOWN_MASK));
     165            referenceWay.set(testWay);
     166        });
     167        final Way testWay = referenceWay.get();
     168        // There should be a new node between nodes 1 and 2 (old node 2 is now node 3)
     169        assertAll(() -> assertEquals(6, testWay.getNodesCount()),
     170                () -> assertFalse(testWay.getNode(1).isNew()),
     171                () -> assertTrue(testWay.getNode(2).isNew()),
     172                () -> assertFalse(testWay.getNode(3).isNew()));
     173        // These aren't expected to be 0.0001 and 0.0005 exactly, due zoom and conversions between point and latlon.
     174        assertAll(() -> assertEquals(0.0001, testWay.getNode(2).lat(), 1e-5),
     175                () -> assertEquals(0.0005, testWay.getNode(2).lon(), 1e-5));
     176    }
     177
     178    @Test
     179    void testAddLock() {
     180        final AtomicReference<Way> referenceWay = new AtomicReference<>();
     181        testSimplifyWayAction((mapMode, map, testWay) -> {
     182            // Add a node at 0.0009, 0.0005 (not on the direct line) that is between nodes 1 and 2 but not 2 and 3.
     183            // First get the waysegment selected
     184            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), InputEvent.CTRL_DOWN_MASK));
     185            // Then move to another location with ctrl+shift
     186            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0009, 0.0005), InputEvent.CTRL_DOWN_MASK | InputEvent.SHIFT_DOWN_MASK));
     187            // Finally, release the mouse with ctrl+shift
     188            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0009, 0.0005),
     189                    InputEvent.CTRL_DOWN_MASK | InputEvent.SHIFT_DOWN_MASK));
     190            referenceWay.set(testWay);
     191        });
     192        final Way testWay = referenceWay.get();
     193        // There should be a new node between nodes 1 and 2 (old node 2 is now node 3)
     194        assertAll(() -> assertEquals(6, testWay.getNodesCount()),
     195                () -> assertFalse(testWay.getNode(1).isNew()),
     196                () -> assertTrue(testWay.getNode(2).isNew()),
     197                () -> assertFalse(testWay.getNode(3).isNew()));
     198        // These aren't expected to be 0.0009 and 0.0005 exactly, due zoom and conversions between point and latlon.
     199        assertAll(() -> assertEquals(0.0009, testWay.getNode(2).lat(), 1e-5),
     200                () -> assertEquals(0.0005, testWay.getNode(2).lon(), 1e-5));
     201    }
     202
     203    @Test
     204    void testMove() {
     205        final AtomicReference<Way> referenceWay = new AtomicReference<>();
     206        testSimplifyWayAction((mapMode, map, testWay) -> {
     207            // Move node to 0.0001, 0.0005 (not on the direct line)
     208            // First get the waysegment selected
     209            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), 0));
     210            // Finally, release the mouse
     211            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), 0));
     212            referenceWay.set(testWay);
     213        });
     214        final Way testWay = referenceWay.get();
     215        assertEquals(5, testWay.getNodesCount());
     216        // These aren't expected to be 0.0001 and 0.0005 exactly, due zoom and conversions between point and latlon.
     217        assertAll(() -> assertEquals(0.0001, testWay.getNode(2).lat(), 1e-5),
     218                () -> assertEquals(0.0005, testWay.getNode(2).lon(), 1e-5));
     219    }
     220
     221    @Test
     222    void testMoveLock() {
     223        final AtomicReference<Way> referenceWay = new AtomicReference<>();
     224        testSimplifyWayAction((mapMode, map, testWay) -> {
     225            // Move node to 0.0001, 0.0005 (not on the direct line)
     226            // First get the waysegment selected
     227            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), 0));
     228            // Then move to another location
     229            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0009, 0.0005), InputEvent.SHIFT_DOWN_MASK));
     230            // Finally, release the mouse
     231            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0009, 0.0005), InputEvent.SHIFT_DOWN_MASK));
     232            referenceWay.set(testWay);
     233        });
     234        final Way testWay = referenceWay.get();
     235        assertEquals(5, testWay.getNodesCount());
     236        // These aren't expected to be 0.0009 and 0.0005 exactly, due zoom and conversions between point and latlon.
     237        assertAll(() -> assertEquals(0.0009, testWay.getNode(2).lat(), 1e-5),
     238                () -> assertEquals(0.0005, testWay.getNode(2).lon(), 1e-5));
     239    }
     240
     241    @Test
     242    void testDelete() {
     243        DeleteCommand.setDeletionCallback(new AlwaysDeleteCallback());
     244        final AtomicReference<Way> referenceWay = new AtomicReference<>();
     245        testSimplifyWayAction((mapMode, map, testWay) -> {
     246            // Move node to 0.0001, 0.0005 (not on the direct line)
     247            // First get the waysegment selected
     248            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), InputEvent.ALT_DOWN_MASK));
     249            // Finally, release the mouse
     250            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), InputEvent.ALT_DOWN_MASK));
     251            referenceWay.set(testWay);
     252        });
     253        final Way testWay = referenceWay.get();
     254        assertEquals(4, testWay.getNodesCount());
     255        assertAll(testWay.getNodes().stream().map(n -> () -> assertNotEquals(3, n.getUniqueId())));
     256    }
     257
     258    @Test
     259    void testDeleteLock() {
     260        DeleteCommand.setDeletionCallback(new AlwaysDeleteCallback());
     261        final AtomicReference<Way> referenceWay = new AtomicReference<>();
     262        testSimplifyWayAction((mapMode, map, testWay) -> {
     263            // Move node to 0.0001, 0.0005 (not on the direct line)
     264            // First get the waysegment selected
     265            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0005), 0));
     266            // Then move to another location
     267            mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0009, 0.0005), InputEvent.SHIFT_DOWN_MASK | InputEvent.ALT_DOWN_MASK));
     268            // Finally, release the mouse
     269            mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0009, 0.0005), InputEvent.SHIFT_DOWN_MASK | InputEvent.ALT_DOWN_MASK));
     270            referenceWay.set(testWay);
     271        });
     272        final Way testWay = referenceWay.get();
     273        assertEquals(4, testWay.getNodesCount());
     274        assertAll(testWay.getNodes().stream().map(n -> () -> assertNotEquals(3, n.getUniqueId())));
     275    }
     276
     277    private void testSimplifyWayAction(TriConsumer<ImproveWayAccuracyAction, MapFrame, Way> runnable) {
    105278        final DataSet dataSet = new DataSet();
    106279        final OsmDataLayer layer = new OsmDataLayer(dataSet, "ImproveWayAccuracyActionT", null);
    … …  
    115288        testWay.lastNode().setCoor(new LatLon(0.001, 0.001));
    116289        testWay.getNode(1).setCoor(new LatLon(0.0001, 0.0001));
     290        testWay.getNode(2).setCoor(new LatLon(0.0005, 0.0005));
    117291        testWay.getNode(3).setCoor(new LatLon(0.0009, 0.0009));
    118292        dataSet.addPrimitiveRecursive(testWay);
    119         assertFalse(testWay.getNode(2).isLatLonKnown(), "The second node should not have valid coordinates");
     293        setupMapView(dataSet);
    120294        final Graphics2D g2d = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_ARGB).createGraphics();
    121295        g2d.setClip(0, 0, WIDTH, HEIGHT);
    … …  
    124298            assertDoesNotThrow(() -> map.mapView.paint(g2d), "The mapview should be able to handle a null coordinate node");
    125299            // Ensure that the test way is selected (and use the methods from the action to do so)
    126             GuiHelper.runInEDTAndWaitWithException(() -> {
    127                 map.mapView.zoomTo(new Bounds(0, 0, 0.001, 0.001));
    128                 // Get the target way selected (note: not selected in dataset -- use mapMode.mapReleased for that)
    129                 mapMode.mouseMoved(generateEvent(map.mapView, testWay.getNode(1)));
    130             });
    131             // mouseMoved shouldn't cause the way to get selected
    132             assertFalse(dataSet.getAllSelected().contains(testWay));
     300            assertDoesNotThrow(() -> GuiHelper.runInEDTAndWaitWithException(() -> {
     301                // Set the way as selected
     302                mapMode.mouseMoved(generateEvent(map.mapView, testWay.getNode(1), 0));
     303                mapMode.mouseReleased(generateEvent(map.mapView, testWay.getNode(1), 0));
     304                // And then run the test case
     305                runnable.accept(mapMode, map, testWay);
     306            }));
    133307
    134308            // Now check painting (where the problem should occur; the mapMode.paint call should be called as part of the map.mapView.paint call)
    … …  
    136310            assertDoesNotThrow(() -> mapMode.paint(g2d, map.mapView, new Bounds(0, 0, 0.001, 0.001)));
    137311
    138             // Finally, check painting during selection
     312            // Then perform the action(s)
    139313            GuiHelper.runInEDTAndWaitWithException(() -> {
    140                 // Set the way as selected
    141                 mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0001, 0.0001)));
    142314                // Set the mouse location (unset in mouseReleased call)
    143                 mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0001)));
     315                // This is required by testNonRegression23444, and it doesn't hurt during the other tests
     316                mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0001), 0));
    144317            });
    145             // The way shouldn't be selected, since it isn't usable for the improve way tool
    146             assertFalse(dataSet.getAllSelected().contains(testWay));
    147318            // Now check painting again (just in case)
    148319            assertDoesNotThrow(() -> map.paint(g2d));
Note: See TracChangeset for help on using the changeset viewer.