Ticket #20182: 20182.patch

File 20182.patch, 10.5 KB (added by GerdP, 5 years ago)

  • src/org/openstreetmap/josm/data/validation/tests/ConnectivityRelations.java

     
    7070     * Convert the connectivity tag into a map of values
    7171     *
    7272     * @param relation A relation with a {@code connectivity} tag.
    73      * @return A Map in the form of {@code Map<Lane From, Map<Lane To, Optional>>} May contain nulls when errors are encountered
     73     * @return A Map in the form of {@code Map<Lane From, Map<Lane To, Optional>>} May contain nulls when errors are encountered,
     74     * empty collection if {@code connectivity} tag contains unusual values
    7475     */
    7576    public static Map<Integer, Map<Integer, Boolean>> parseConnectivityTag(Relation relation) {
    76         String cnTag = relation.get(CONNECTIVITY_TAG);
    77         if (cnTag == null) {
     77        final String cnTag = relation.get(CONNECTIVITY_TAG);
     78        if (cnTag == null || cnTag.isEmpty()) {
    7879            return Collections.emptyMap();
    7980        }
    8081        final String joined = cnTag.replace("bw", Integer.toString(BW));
    8182
    8283        final Map<Integer, Map<Integer, Boolean>> result = new HashMap<>();
    83         String[] lanes = joined.split("\\|", -1);
    84         for (int i = 0; i < lanes.length; i++) {
    85             final String[] lane = lanes[i].split(":", -1);
     84        String[] lanePairs = joined.split("\\|", -1);
     85        for (final String lanePair : lanePairs) {
     86            final String[] lane = lanePair.split(":", -1);
     87            if (lane.length < 2)
     88                return Collections.emptyMap();
    8689            int laneNumber;
    87             //Ignore connections from bw, since we cannot derive a lane number from bw
    88             if (!"bw".equals(lane[0])) {
     90            try {
    8991                laneNumber = Integer.parseInt(lane[0].trim());
    90             } else {
    91                 laneNumber = BW;
     92            } catch (NumberFormatException e) {
     93                return Collections.emptyMap();
    9294            }
     95
    9396            Map<Integer, Boolean> connections = new HashMap<>();
    9497            String[] toLanes = TO_LANE_PATTERN.split(lane[1], -1);
    95             for (int j = 0; j < toLanes.length; j++) {
    96                 String toLane = toLanes[j].trim();
     98            for (String toLane : toLanes) {
    9799                try {
    98100                    if (OPTIONAL_LANE_PATTERN.matcher(toLane).matches()) {
    99101                        toLane = toLane.replace("(", "").replace(")", "").trim();
    100                         if (!"bw".equals(toLane)) {
    101                             connections.put(Integer.parseInt(toLane), Boolean.TRUE);
    102                         } else
    103                             connections.put(BW, Boolean.TRUE);
     102                        connections.put(Integer.parseInt(toLane), Boolean.TRUE);
    104103                    } else {
    105                         if (!toLane.contains("bw")) {
    106                             connections.put(Integer.parseInt(toLane), Boolean.FALSE);
    107                         } else {
    108                             connections.put(BW, Boolean.FALSE);
    109                         }
     104                        connections.put(Integer.parseInt(toLane), Boolean.FALSE);
    110105                    }
    111106                } catch (NumberFormatException e) {
    112107                    if (MISSING_COMMA_PATTERN.matcher(toLane).matches()) {
    113                         connections.put(null, true);
     108                        connections.put(null, Boolean.TRUE);
    114109                    } else {
    115110                        connections.put(null, null);
    116111                    }
    … …  
    128123                errors.add(TestError.builder(this, Severity.WARNING, NO_CONNECTIVITY_TAG)
    129124                        .message(tr("Connectivity relation without connectivity tag")).primitives(r).build());
    130125            } else if (!r.hasIncompleteMembers()) {
    131                 boolean badRole = checkForBadRole(r);
    132                 boolean missingRole = checkForMissingRole(r);
    133                 if (!badRole && !missingRole) {
    134                     Map<String, Integer> roleLanes = checkForInconsistentLanes(r);
    135                     checkForImpliedConnectivity(r, roleLanes);
     126                Map<Integer, Map<Integer, Boolean>> connTagLanes = parseConnectivityTag(r);
     127                if (connTagLanes.isEmpty()) {
     128                    errors.add(TestError.builder(this, Severity.ERROR, MALFORMED_CONNECTIVITY_TAG)
     129                            .message(tr("Connectivity tag contains unusual data")).primitives(r).build());
     130                } else {
     131                    boolean badRole = checkForBadRole(r);
     132                    boolean missingRole = checkForMissingRole(r);
     133                    if (!badRole && !missingRole) {
     134                        Map<String, Integer> roleLanes = checkForInconsistentLanes(r, connTagLanes);
     135                        checkForImpliedConnectivity(r, roleLanes, connTagLanes);
     136                    }
    136137                }
    137138            }
    138139        }
    … …  
    142143     * Compare lane tags of members to values in the {@code connectivity} tag of the relation
    143144     *
    144145     * @param relation A relation with a {@code connectivity} tag.
     146     * @param connTagLanes result of {@link ConnectivityRelations#parseConnectivityTag(Relation)}
    145147     * @return A Map in the form of {@code Map<Role, Lane Count>}
    146148     */
    147     private Map<String, Integer> checkForInconsistentLanes(Relation relation) {
     149    private Map<String, Integer> checkForInconsistentLanes(Relation relation, Map<Integer, Map<Integer, Boolean>> connTagLanes) {
    148150        StringBuilder lanelessRoles = new StringBuilder();
    149151        int lanelessRolesCount = 0;
    150152        // Lane count from connectivity tag
    151         Map<Integer, Map<Integer, Boolean>> connTagLanes = parseConnectivityTag(relation);
     153        Map<String, Integer> roleLanes = new HashMap<>();
     154        if (connTagLanes.isEmpty())
     155            return roleLanes;
     156
    152157        // If the ways involved in the connectivity tag are assuming a standard 2-way bi-directional highway
    153158        boolean defaultLanes = true;
    154159        for (Entry<Integer, Map<Integer, Boolean>> thisEntry : connTagLanes.entrySet()) {
    … …  
    164169            }
    165170        }
    166171        // Lane count from member tags
    167         Map<String, Integer> roleLanes = new HashMap<>();
    168172        for (RelationMember rM : relation.getMembers()) {
    169173            // Check lanes
    170174            if (rM.getType() == OsmPrimitiveType.WAY) {
    … …  
    186190                        }
    187191                    }
    188192
    189                     if (!laneCounts.equals(Collections.emptyList())) {
     193                    if (!laneCounts.isEmpty()) {
    190194                        maxLaneCount = Collections.max(laneCounts);
    191195                        roleLanes.put(rM.getRole(), (int) maxLaneCount);
    192196                    } else {
    193                         String addString = "'" + rM.getRole() + "'";
    194                         StringBuilder sb = new StringBuilder(addString);
    195197                        if (lanelessRoles.length() > 0) {
    196                             sb.insert(0, " and ");
     198                            lanelessRoles.append(" and ");
    197199                        }
    198                         lanelessRoles.append(sb.toString());
     200                        lanelessRoles.append('\'').append(rM.getRole()).append('\'');
    199201                        lanelessRolesCount++;
    200202                    }
    201203                }
    … …  
    202204            }
    203205        }
    204206
    205         if (lanelessRoles.toString().isEmpty()) {
     207        if (lanelessRoles.length() == 0) {
    206208            boolean fromCheck = roleLanes.get(FROM) < Collections
    207209                    .max(connTagLanes.entrySet(), Comparator.comparingInt(Map.Entry::getKey)).getKey();
    208210            boolean toCheck = false;
    … …  
    242244     *
    243245     * @param relation A relation with a {@code connectivity} tag.
    244246     * @param roleLanes The lane counts for each relation role
     247     * @param connTagLanes result of {@link ConnectivityRelations#parseConnectivityTag(Relation)}
    245248     */
    246     private void checkForImpliedConnectivity(Relation relation, Map<String, Integer> roleLanes) {
    247         Map<Integer, Map<Integer, Boolean>> connTagLanes = parseConnectivityTag(relation);
     249    private void checkForImpliedConnectivity(Relation relation, Map<String, Integer> roleLanes,
     250            Map<Integer, Map<Integer, Boolean>> connTagLanes) {
    248251        // Don't flag connectivity as already implied when:
    249252        // - Lane counts are different on the roads
    250253        // - Placement tags convey the connectivity

  • test/unit/org/openstreetmap/josm/data/validation/tests/ConnectivityRelationsTest.java

     
    22package org.openstreetmap.josm.data.validation.tests;
    33
    44import org.junit.Assert;
     5import org.junit.jupiter.api.BeforeAll;
    56import org.junit.jupiter.api.BeforeEach;
    67import org.junit.jupiter.api.Test;
    78import org.openstreetmap.josm.JOSMFixture;
    … …  
    1920class ConnectivityRelationsTest {
    2021    private ConnectivityRelations check;
    2122    private static final String CONNECTIVITY = "connectivity";
     23
    2224    /**
    2325     * Setup test.
    24      *
    25      * @throws Exception if an error occurs
    2626     */
     27    @BeforeAll
     28    public static void setUp() {
     29        JOSMFixture.createUnitTestFixture().init();
     30    }
     31
    2732    @BeforeEach
    28     public void setUp() throws Exception {
    29         JOSMFixture.createUnitTestFixture().init();
     33    public void setUpCheck() throws Exception {
    3034        check = new ConnectivityRelations();
    3135    }
    3236
    … …  
    8589        Assert.assertEquals(++expectedFailures, check.getErrors().size());
    8690    }
    8791
     92    /**
     93     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/201821">Bug #20182</a>.
     94     * @throws Exception if an error occurs
     95     */
     96    @Test
     97    void testTicket20182() throws Exception {
     98        Relation relation = createDefaultTestRelation();
     99        check.visit(relation);
     100        int expectedFailures = 0;
     101
     102        Assert.assertEquals(expectedFailures, check.getErrors().size());
     103
     104        relation.put(CONNECTIVITY, "left_turn");
     105        check.visit(relation);
     106        Assert.assertEquals(++expectedFailures, check.getErrors().size());
     107
     108        relation.put(CONNECTIVITY, "1");
     109        check.visit(relation);
     110        Assert.assertEquals(++expectedFailures, check.getErrors().size());
     111    }
    88112}