Ticket #18138: 18138.5.patch

File 18138.5.patch, 18.2 KB (added by Traaker_L, 6 years ago)

New patch addressing the crashes encountered when validating all connectivity relations

  • data/defaultpresets.xml

     
    75807580                <role key="to" text="to way" requisite="required" count="1" type="way" />
    75817581            </roles>
    75827582        </item> <!-- Turn Restriction -->
     7583        <item name="Lane Connectivity" type="relation" preset_name_label="true" icon="presets/transport/way/relation_connectivity.svg">
     7584            <link wiki="Relation:connectivity" />
     7585            <space />
     7586            <key key="type" value="connectivity" />
     7587            <text key="connectivity" text="Lane Connectivity" />
     7588            <roles>
     7589                <role key="from" text="from way" requisite="required" count="1" type="way" />
     7590                <role key="via" text="via node or ways" requisite="required" type="way,node" />
     7591                <role key="to" text="to way" requisite="required" count="1" type="way" />
     7592            </roles>
     7593        </item> <!-- Lane Connectivity -->
    75837594        <item name="Enforcement" icon="presets/vehicle/restriction/speed_camera.svg" type="relation" preset_name_label="true">
    75847595            <link wiki="Relation:enforcement" />
    75857596            <space />

  • images/presets/transport/way/relation_connectivity.svg

     
     1<svg xmlns="http://www.w3.org/2000/svg" version="1.1" viewBox="0 0 16 16" width="16" height="16">
     2  <path d="M0,0 h16v16h-16z" fill="#8d8"/>
     3  <path d="M1.5,16v-6.5l4,-4v-6l8,0v16.5" stroke="#fff" fill="#888"/>
     4  <path d="M9.5,1v2m0,2v2m0,2v2m0,2v2m-4,0v-2m0,-2v-2" stroke="#fff"/>
     5</svg>

  • src/org/openstreetmap/josm/data/validation/OsmValidator.java

     
    4242import org.openstreetmap.josm.data.validation.tests.BarriersEntrances;
    4343import org.openstreetmap.josm.data.validation.tests.Coastlines;
    4444import org.openstreetmap.josm.data.validation.tests.ConditionalKeys;
     45import org.openstreetmap.josm.data.validation.tests.ConnectivityRelations;
    4546import org.openstreetmap.josm.data.validation.tests.CrossingWays;
    4647import org.openstreetmap.josm.data.validation.tests.DuplicateNode;
    4748import org.openstreetmap.josm.data.validation.tests.DuplicateRelation;
    … …  
    150151        PublicTransportRouteTest.class, // 3600 .. 3699
    151152        RightAngleBuildingTest.class, // 3700 .. 3799
    152153        SharpAngles.class, // 3800 .. 3899
     154        ConnectivityRelations.class, // 3900 .. 3999
    153155    };
    154156
    155157    /**

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

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.validation.tests;
     3
     4import static org.openstreetmap.josm.tools.I18n.tr;
     5
     6import java.util.ArrayList;
     7import java.util.Collections;
     8import java.util.Comparator;
     9import java.util.HashMap;
     10import java.util.List;
     11import java.util.Map;
     12import java.util.Map.Entry;
     13import java.util.regex.Pattern;
     14
     15import org.openstreetmap.josm.data.osm.Node;
     16import org.openstreetmap.josm.data.osm.OsmPrimitive;
     17import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
     18import org.openstreetmap.josm.data.osm.Relation;
     19import org.openstreetmap.josm.data.osm.RelationMember;
     20import org.openstreetmap.josm.data.osm.Way;
     21import org.openstreetmap.josm.data.validation.Severity;
     22import org.openstreetmap.josm.data.validation.Test;
     23import org.openstreetmap.josm.data.validation.TestError;
     24import org.openstreetmap.josm.tools.Logging;
     25
     26/**
     27 * Check for inconsistencies in lane information between relation and members.
     28 */
     29public class ConnectivityRelations extends Test {
     30
     31    protected static final int INCONSISTENT_LANE_COUNT = 3900;
     32
     33    protected static final int UNKNOWN_CONNECTIVITY_ROLE = 3901;
     34
     35    protected static final int NO_CONNECTIVITY_TAG = 3902;
     36
     37    protected static final int MALFORMED_CONNECTIVITY_TAG = 3903;
     38
     39    protected static final int TOO_MANY_ROLES = 3904;
     40
     41    protected static final int MISSING_ROLE = 3905;
     42
     43    protected static final int MEMBER_MISSING_LANES = 3906;
     44
     45    private static final String CONNECTIVITY_TAG = "connectivity";
     46    private static final String VIA = "via";
     47    private static final String TO = "to";
     48    private static final String FROM = "from";
     49    private static final int BW = -1000;
     50    private static final Pattern OPTIONAL_LANE_PATTERN = Pattern.compile("\\([0-9-]+\\)");
     51    private static final Pattern TO_LANE_PATTERN = Pattern.compile("\\p{Zs}*[,:;]\\p{Zs}*");
     52
     53    /**
     54    * Constructor
     55    */
     56    public ConnectivityRelations() {
     57        super(tr("Connectivity Relation Check"), tr("Checks that lane count of relation matches with lanes of members"));
     58    }
     59
     60    /**
     61     * Convert the connectivity tag into a map of values
     62     *
     63     * @param relation A relation with a {@code connectivity} tag.
     64     * @return A Map in the form of {@code Map<Lane From, Map<Lane To, Optional>>} May contain nulls when errors are encountered
     65     * @since xxx
     66     */
     67    public static Map<Integer, Map<Integer, Boolean>> parseConnectivityTag(Relation relation) {
     68        final String joined = relation.get(CONNECTIVITY_TAG).replaceAll("bw", Integer.toString(BW));
     69
     70        if (joined == null) {
     71            return Collections.emptyMap();
     72        }
     73
     74        final Map<Integer, Map<Integer, Boolean>> result = new HashMap<>();
     75        String[] lanes = joined.split("\\|", -1);
     76        for (int i = 0; i < lanes.length; i++) {
     77            String[] lane = lanes[i].split(":", -1);
     78
     79            int laneNumber;
     80            //Ignore connections from bw, since we cannot derive a lane number from bw
     81            if (!lane[0].equals("bw")) {
     82                laneNumber = Integer.parseInt(lane[0].trim());
     83            } else {
     84                laneNumber = BW;
     85            }
     86            Map<Integer, Boolean> connections = new HashMap<>();
     87            String[] toLanes = TO_LANE_PATTERN.split(lane[1]);
     88            for (int j = 0; j < toLanes.length; j++) {
     89                String toLane = toLanes[j].trim();
     90                try {
     91                    if (OPTIONAL_LANE_PATTERN.matcher(toLane).matches()) {
     92                        toLane = toLane.replace("(", "").replace(")", "").trim();
     93                        if (!toLane.equals("bw")) {
     94                            connections.put(Integer.parseInt(toLane), Boolean.TRUE);
     95                        } else
     96                            connections.put(BW, Boolean.TRUE);
     97                    } else {
     98                        if (!toLane.contains("bw")) {
     99                            connections.put(Integer.parseInt(toLane), Boolean.FALSE);
     100                        } else {
     101                            connections.put(BW, Boolean.FALSE);
     102                        }
     103                    }
     104                } catch (NumberFormatException e) {
     105                    Logging.debug(e);
     106                    connections.put(null, null);
     107                }
     108            }
     109            result.put(laneNumber, connections);
     110
     111        }
     112        return result;
     113    }
     114
     115    @Override
     116    public void visit(Relation r) {
     117        if (r.hasTag("type", CONNECTIVITY_TAG)) {
     118            if (!r.hasKey(CONNECTIVITY_TAG)) {
     119                errors.add(TestError.builder(this, Severity.WARNING, NO_CONNECTIVITY_TAG)
     120                        .message(tr("No connectivity tag in connectivity relation")).primitives(r).build());
     121            } else if (!r.hasIncompleteMembers()) {
     122                boolean badRole = checkForBadRole(r);
     123                boolean missingRole = checkForMissingRole(r);
     124                if (!badRole && !missingRole) {
     125                    checkForInconsistentLanes(r);
     126                } else if (missingRole) {
     127                    createMissingRole(r);
     128                }
     129            }
     130        }
     131    }
     132
     133    private void checkForInconsistentLanes(Relation relation) {
     134        String lanelessRoles = "";
     135        // Lane count from connectivity tag
     136        Map<Integer, Map<Integer, Boolean>> connTagLanes = parseConnectivityTag(relation);
     137        // Lane count from member tags
     138        Map<String, Integer> roleLanes = new HashMap<>();
     139
     140
     141        for (RelationMember rM : relation.getMembers()) {
     142            // Check lanes
     143            if (rM.getType() == OsmPrimitiveType.WAY) {
     144                OsmPrimitive prim = rM.getMember();
     145                if (!VIA.equals(rM.getRole())) {
     146                    if (prim.hasKey("lanes"))
     147                        roleLanes.put(rM.getRole(), Integer.parseInt(prim.get("lanes")));
     148                    else {
     149                        String addString = "'" + rM.getRole() + "'";
     150                        if (!lanelessRoles.isEmpty()) {
     151                            addString = ", " + addString;
     152                        }
     153                        lanelessRoles += addString;
     154                    }
     155                }
     156            }
     157        }
     158
     159        if (lanelessRoles.isEmpty()) {
     160            boolean fromCheck = roleLanes.get(FROM) < Collections
     161                    .max(connTagLanes.entrySet(), Comparator.comparingInt(Map.Entry::getKey)).getKey();
     162            boolean toCheck = false;
     163            for (Entry<Integer, Map<Integer, Boolean>> to : connTagLanes.entrySet()) {
     164                if (!to.getValue().containsKey(null)) {
     165                    toCheck = roleLanes.get(TO) < Collections
     166                            .max(to.getValue().entrySet(), Comparator.comparingInt(Map.Entry::getKey)).getKey();
     167                } else {
     168                    errors.add(TestError.builder(this, Severity.ERROR, MALFORMED_CONNECTIVITY_TAG)
     169                            .message(tr("Connectivity tag contains unusual data")).primitives(relation)
     170                            .build());
     171                }
     172            }
     173            if (fromCheck || toCheck) {
     174                errors.add(TestError.builder(this, Severity.WARNING, INCONSISTENT_LANE_COUNT)
     175                        .message(tr("Inconsistent lane numbering between relation and members")).primitives(relation)
     176                        .build());
     177            }
     178        } else {
     179            errors.add(TestError.builder(this, Severity.OTHER, MEMBER_MISSING_LANES)
     180                    .message(tr("Relation {0} member(s) missing lanes tag", lanelessRoles)).primitives(relation)
     181                    .build());
     182        }
     183    }
     184
     185    private boolean checkForBadRole(Relation relation) {
     186        // Check role names
     187        int viaWays = 0;
     188        int viaNodes = 0;
     189        int toWays = 0;
     190        int fromWays = 0;
     191        for (RelationMember relationMember : relation.getMembers()) {
     192            if (relationMember.getMember() instanceof Way) {
     193                if (relationMember.hasRole(FROM))
     194                    fromWays++;
     195                else if (relationMember.hasRole(TO))
     196                    toWays++;
     197                else if (relationMember.hasRole(VIA))
     198                    viaWays++;
     199                else {
     200                    createUnknownRole(relation, relationMember.getMember());
     201                    return true;
     202                }
     203            } else if (relationMember.getMember() instanceof Node) {
     204                if (!relationMember.hasRole(VIA)) {
     205                    createUnknownRole(relation, relationMember.getMember());
     206                    return true;
     207                }
     208                viaNodes++;
     209            }
     210        }
     211        return mixedViaNodeAndWay(relation, viaWays, viaNodes, toWays, fromWays);
     212    }
     213
     214    private boolean checkForMissingRole(Relation relation) {
     215        List<String> necessaryRoles = new ArrayList<>();
     216        necessaryRoles.add(FROM);
     217        necessaryRoles.add(VIA);
     218        necessaryRoles.add(TO);
     219
     220        List<String> roleList = new ArrayList<>();
     221        for (RelationMember relationMember: relation.getMembers()) {
     222            roleList.add(relationMember.getRole());
     223        }
     224
     225
     226        return !roleList.containsAll(necessaryRoles);
     227    }
     228
     229    private boolean mixedViaNodeAndWay(Relation relation, int viaWays, int viaNodes, int toWays, int fromWays) {
     230        String message = "";
     231        if ((viaWays != 0 && viaNodes != 0) || viaNodes > 1) {
     232            message = tr("Relation contains {1} {0} roles.", VIA, viaWays + viaNodes);
     233        } else if (toWays != 1) {
     234            message = tr("Relation contains too many {0} roles", TO);
     235        } else if (fromWays != 1) {
     236            message = tr("Relation contains too many {0} roles", FROM);
     237        }
     238        if (message.isEmpty()) {
     239            return false;
     240        } else {
     241            errors.add(TestError.builder(this, Severity.WARNING, TOO_MANY_ROLES)
     242                    .message(message).primitives(relation).build());
     243            return true;
     244        }
     245    }
     246
     247    private void createUnknownRole(Relation relation, OsmPrimitive primitive) {
     248        errors.add(TestError.builder(this, Severity.WARNING, UNKNOWN_CONNECTIVITY_ROLE)
     249                .message(tr("Unkown role in connectivity relation")).primitives(relation).highlight(primitive).build());
     250    }
     251
     252    private void createMissingRole(Relation relation) {
     253        errors.add(TestError.builder(this, Severity.WARNING, MISSING_ROLE)
     254                .message(tr("Connectivity relation is missing at least one necessary role")).primitives(relation)
     255                .build());
     256    }
     257}

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

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.validation.tests;
     3
     4import org.junit.Assert;
     5import org.junit.Before;
     6import org.junit.Test;
     7import org.openstreetmap.josm.JOSMFixture;
     8import org.openstreetmap.josm.TestUtils;
     9import org.openstreetmap.josm.data.coor.LatLon;
     10import org.openstreetmap.josm.data.osm.Node;
     11import org.openstreetmap.josm.data.osm.Relation;
     12import org.openstreetmap.josm.data.osm.RelationMember;
     13
     14/**
     15 * Test the ConnectivityRelations validation test
     16 *
     17 * @author Taylor Smock
     18 */
     19public class ConnectivityRelationsTest {
     20    private ConnectivityRelations check;
     21    private static final String CONNECTIVITY = "connectivity";
     22    /**
     23     * Setup test.
     24     *
     25     * @throws Exception if an error occurs
     26     */
     27    @Before
     28    public void setUp() throws Exception {
     29        JOSMFixture.createUnitTestFixture().init();
     30        check = new ConnectivityRelations();
     31    }
     32
     33    private Relation createDefaultTestRelation() {
     34        Node connection = new Node(new LatLon(0, 0));
     35        return TestUtils.newRelation("type=connectivity connectivity=1:1",
     36                new RelationMember("from", TestUtils.newWay("lanes=4", new Node(new LatLon(-0.1, -0.1)), connection)),
     37                new RelationMember("via", connection),
     38                new RelationMember("to", TestUtils.newWay("lanes=4", connection, new Node(new LatLon(0.1, 0.1)))));
     39    }
     40
     41    /**
     42     * Test for connectivity relations without a connectivity tag
     43     */
     44    @Test
     45    public void testNoConnectivityTag() {
     46        Relation relation = createDefaultTestRelation();
     47        check.visit(relation);
     48
     49        Assert.assertEquals(0, check.getErrors().size());
     50
     51        relation.remove(CONNECTIVITY);
     52        check.visit(relation);
     53        Assert.assertEquals(1, check.getErrors().size());
     54    }
     55
     56    /**
     57     * Check for lanes that don't make sense
     58     */
     59    @Test
     60    public void testMisMatchedLanes() {
     61        Relation relation = createDefaultTestRelation();
     62        check.visit(relation);
     63        int expectedFailures = 0;
     64
     65        Assert.assertEquals(expectedFailures, check.getErrors().size());
     66
     67        relation.put(CONNECTIVITY, "45000:1");
     68        check.visit(relation);
     69        Assert.assertEquals(++expectedFailures, check.getErrors().size());
     70
     71        relation.put(CONNECTIVITY, "1:45000");
     72        check.visit(relation);
     73        Assert.assertEquals(++expectedFailures, check.getErrors().size());
     74
     75        relation.put(CONNECTIVITY, "1:1,2");
     76        check.visit(relation);
     77        Assert.assertEquals(expectedFailures, check.getErrors().size());
     78
     79        relation.put(CONNECTIVITY, "1:1,(2)");
     80        check.visit(relation);
     81        Assert.assertEquals(expectedFailures, check.getErrors().size());
     82
     83        relation.put(CONNECTIVITY, "1:1,(20000)");
     84        check.visit(relation);
     85        Assert.assertEquals(++expectedFailures, check.getErrors().size());
     86    }
     87
     88    /**
     89     * Check for bad roles (not from/via/to)
     90     */
     91    @Test
     92    public void testForBadRole() {
     93        Relation relation = createDefaultTestRelation();
     94        check.visit(relation);
     95        int expectedFailures = 0;
     96
     97        Assert.assertEquals(expectedFailures, check.getErrors().size());
     98
     99        for (int i = 0; i < relation.getMembers().size(); i++) {
     100            String tRole = replaceMember(relation, i, "badRole");
     101            check.visit(relation);
     102            Assert.assertEquals(++expectedFailures, check.getErrors().size());
     103            replaceMember(relation, i, tRole);
     104            check.visit(relation);
     105            Assert.assertEquals(expectedFailures, check.getErrors().size());
     106        }
     107    }
     108
     109    private String replaceMember(Relation relation, int index, String replacementRole) {
     110        RelationMember relationMember = relation.getMember(index);
     111        String currentRole = relationMember.getRole();
     112        relation.removeMember(index);
     113        relation.addMember(index, new RelationMember(replacementRole, relationMember.getMember()));
     114        return currentRole;
     115    }
     116}