Changeset 16300 in josm for trunk/src/org


Ignore:
Timestamp:
2020-04-14T10:27:34+02:00 (5 years ago)
Author:
GerdP
Message:

fix #18670, #19042: Unglue ways: False positives about affected relations

  • avoid to warn about possibly broken turn restrictions when we can make sure they are not broken
  • when no way is selected, try to find one that produces fewer updates

TODO: handle dataset without download areas

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/actions/UnGlueAction.java

    r15152 r16300  
    109109                // If there aren't enough ways, maybe the user wanted to unglue the nodes
    110110                // (= copy tags to a new node)
    111                 if (!selfCrossing)
    112                     if (checkForUnglueNode(selection)) {
     111                if (!selfCrossing) {
     112                    if (selection.size() == 1 && selectedNode.isTagged()) {
    113113                        unglueOneNodeAtMostOneWay(e);
    114114                    } else {
     
    116116                        errMsg = tr("This node is not glued to anything else.");
    117117                    }
     118                }
    118119            } else {
    119120                // and then do the work.
     
    246247
    247248    /**
    248      * Checks if selection is suitable for ungluing. This is the case when there's a single,
    249      * tagged node selected that's part of at least one way (ungluing an unconnected node does
    250      * not make sense. Due to the call order in actionPerformed, this is only called when the
    251      * node is only part of one or less ways.
    252      *
    253      * @param selection The selection to check against
    254      * @return {@code true} if selection is suitable
    255      */
    256     private boolean checkForUnglueNode(Collection<? extends OsmPrimitive> selection) {
    257         if (selection.size() != 1)
    258             return false;
    259         OsmPrimitive n = (OsmPrimitive) selection.toArray()[0];
    260         if (!(n instanceof Node))
    261             return false;
    262         if (((Node) n).getParentWays().isEmpty())
    263             return false;
    264 
    265         selectedNode = (Node) n;
    266         return selectedNode.isTagged();
    267     }
    268 
    269     /**
    270249     * Checks if the selection consists of something we can work with.
    271250     * Checks only if the number and type of items selected looks good.
     
    292271            if (p instanceof Node) {
    293272                selectedNode = (Node) p;
    294                 if (size == 1 || selectedWay != null)
    295                     return size == 1 || selectedWay.containsNode(selectedNode);
     273                if (size == 1 || (selectedWay != null && selectedWay.containsNode(selectedNode)))
     274                    return true;
    296275            } else if (p instanceof Way) {
    297276                selectedWay = (Way) p;
     
    360339     * @param cmds List of commands that will contain the new "add node" command
    361340     * @param newNodes List of nodes that will contain the new node
    362      * @return new way The modified way. Change command mus be handled by the caller
     341     * @return new way The modified way. Change command must be handled by the caller
    363342     */
    364343    private static Way modifyWay(Node originalNode, Way w, List<Command> cmds, List<Node> newNodes) {
     
    446425        List<Node> newNodes = new LinkedList<>();
    447426        if (selectedWay == null) {
    448             Way wayWithSelectedNode = null;
    449             LinkedList<Way> parentWays = new LinkedList<>();
    450             for (OsmPrimitive osm : selectedNode.getReferrers()) {
    451                 if (osm.isUsable() && osm instanceof Way) {
    452                     Way w = (Way) osm;
    453                     if (wayWithSelectedNode == null && !w.isFirstLastNode(selectedNode)) {
    454                         wayWithSelectedNode = w;
    455                     } else {
    456                         parentWays.add(w);
    457                     }
    458                 }
    459             }
    460             if (wayWithSelectedNode == null) {
    461                 parentWays.removeFirst();
    462             }
     427            LinkedList<Way> parentWays = selectedNode.referrers(Way.class).filter(Way::isUsable)
     428                    .collect(Collectors.toCollection(LinkedList::new));
     429            // see #5452 and #18670
     430            parentWays.sort((o1, o2) -> {
     431                int d = Boolean.compare(!o1.isNew() && !o1.isModified(), !o2.isNew() && !o2.isModified());
     432                if (d == 0) {
     433                    d = Integer.compare(o2.getReferrers().size(), o1.getReferrers().size()); // reversed
     434                }
     435                if (d == 0) {
     436                    d = Boolean.compare(o1.isFirstLastNode(selectedNode), o2.isFirstLastNode(selectedNode));
     437                }
     438                return d;
     439            });
     440            // first way should not be changed, preferring older ways and those with fewer parents
     441            parentWays.removeFirst();
     442
     443            Set<Way> warnParents = new HashSet<>();
    463444            for (Way w : parentWays) {
     445                if (w.isFirstLastNode(selectedNode))
     446                    warnParents.add(w);
    464447                cmds.add(new ChangeCommand(w, modifyWay(selectedNode, w, cmds, newNodes)));
    465448            }
    466             notifyWayPartOfRelation(parentWays);
     449            notifyWayPartOfRelation(warnParents);
    467450        } else {
    468             cmds.add(new ChangeCommand(selectedWay, modifyWay(selectedNode, selectedWay, cmds, newNodes)));
    469             notifyWayPartOfRelation(Collections.singleton(selectedWay));
     451            Way modWay = modifyWay(selectedNode, selectedWay, cmds, newNodes);
     452            addCheckedChangeNodesCmd(cmds, selectedWay, modWay.getNodes());
    470453        }
    471454
     
    528511            return false;
    529512        }
    530         cmds.add(new ChangeNodesCommand(way, newNodes));
    531         notifyWayPartOfRelation(Collections.singleton(way));
     513        addCheckedChangeNodesCmd(cmds, way, newNodes);
    532514        try {
    533515            final PropertiesMembershipChoiceDialog dialog = PropertiesMembershipChoiceDialog.showIfNecessary(
     
    569551            allNewNodes.addAll(newNodes);
    570552        }
    571         cmds.add(new ChangeCommand(selectedWay, tmpWay)); // only one changeCommand for a way, else garbage will happen
    572         notifyWayPartOfRelation(Collections.singleton(selectedWay));
     553        // only one changeCommand for a way, else garbage will happen
     554        addCheckedChangeNodesCmd(cmds, selectedWay, tmpWay.getNodes());
    573555
    574556        UndoRedoHandler.getInstance().add(new SequenceCommand(
     
    576558                        selectedNodes.size(), selectedNodes.size(), selectedNodes.size()+allNewNodes.size()), cmds));
    577559        getLayerManager().getEditDataSet().setSelected(allNewNodes);
     560    }
     561
     562    private void addCheckedChangeNodesCmd(List<Command> cmds, Way w, List<Node> nodes) {
     563        boolean relationCheck = w.firstNode() != nodes.get(0) || w.lastNode() != nodes.get(nodes.size() - 1);
     564        cmds.add(new ChangeNodesCommand(w, nodes));
     565        if (relationCheck) {
     566            notifyWayPartOfRelation(Collections.singleton(w));
     567        }
    578568    }
    579569
     
    612602
    613603    protected void notifyWayPartOfRelation(final Collection<Way> ways) {
    614         final Set<String> affectedRelations = ways.stream()
    615                 .flatMap(w -> w.getReferrers().stream())
    616                 .filter(ref -> ref instanceof Relation && ref.isUsable())
    617                 .map(ref -> ref.getDisplayName(DefaultNameFormatter.getInstance()))
    618                 .collect(Collectors.toSet());
     604        final Set<Node> affectedNodes = (selectedNodes != null) ? selectedNodes : Collections.singleton(selectedNode);
     605        final Set<String> affectedRelations = new HashSet<>();
     606        for (Relation r: OsmPrimitive.getParentRelations(ways)) {
     607            if (!r.isUsable())
     608                continue;
     609            // see #18670: suppress notification when well known restriction types are not affected
     610            if (r.hasTag("type", "restriction", "connectivity", "destination_sign") && !r.hasIncompleteMembers()) {
     611                int count = 0;
     612                for (RelationMember rm : r.getMembers()) {
     613                    if (rm.isNode() && affectedNodes.contains(rm.getNode()))
     614                        count++;
     615                    if (rm.isWay() && ways.contains(rm.getWay())) {
     616                        count++;
     617                        if ("via".equals(rm.getRole())) {
     618                            count++;
     619                        }
     620                    }
     621                }
     622                if (count < 2)
     623                    continue;
     624            }
     625            affectedRelations.add(r.getDisplayName(DefaultNameFormatter.getInstance()));
     626        }
    619627        if (affectedRelations.isEmpty()) {
    620628            return;
     
    622630
    623631        final int size = affectedRelations.size();
    624         final String msg1 = trn("Unglueing affected {0} relation: {1}", "Unglueing affected {0} relations: {1}",
     632        final String msg1 = trn("Unglueing possibly affected {0} relation: {1}", "Unglueing possibly affected {0} relations: {1}",
    625633                size, size, Utils.joinAsHtmlUnorderedList(affectedRelations));
    626634        final String msg2 = trn("Ensure that the relation has not been broken!", "Ensure that the relations have not been broken!",
Note: See TracChangeset for help on using the changeset viewer.