Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/PTAssistantPlugin.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/PTAssistantPlugin.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/PTAssistantPlugin.java (revision 32230) @@ -2,4 +2,10 @@ package org.openstreetmap.josm.plugins.pt_assistant; +import static org.openstreetmap.josm.tools.I18n.tr; + +import javax.swing.JOptionPane; + +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.plugins.Plugin; @@ -17,5 +23,5 @@ */ public class PTAssistantPlugin extends Plugin { - + /** * Main constructor. @@ -34,4 +40,6 @@ } + + Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/RouteUtils.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/RouteUtils.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/RouteUtils.java (revision 32230) @@ -1,5 +1,15 @@ package org.openstreetmap.josm.plugins.pt_assistant.utils; +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.ArrayList; +import java.util.List; + +import javax.swing.JCheckBox; +import javax.swing.JOptionPane; + +import org.openstreetmap.josm.actions.DownloadPrimitiveAction; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.data.osm.PrimitiveId; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.RelationMember; @@ -14,4 +24,16 @@ public class RouteUtils { + // indicates if the user needs to be asked before fetching incomplete + // members of a relation. + + private enum ASK_TO_FETCH { + DO_ASK, DONT_ASK_AND_FETCH, DONT_ASK_AND_DONT_FETCH + }; + + private static ASK_TO_FETCH askToFetch = ASK_TO_FETCH.DO_ASK; + + // checks that the same relation is only fetched once + private static Relation lastRelationToFetch = null; + private RouteUtils() { // private constructor for util classes @@ -66,5 +88,6 @@ || w.hasTag("railway", "platform") || w.hasTag("public_transport", "platform_entry_only") || w.hasTag("highway", "platform_entry_only") || w.hasTag("railway", "platform_entry_only") - || w.hasTag("public_transport", "platform_exit_only") || w.hasTag("highway", "platform_exit_only") || w.hasTag("railway", "platform_exit_only")) { + || w.hasTag("public_transport", "platform_exit_only") || w.hasTag("highway", "platform_exit_only") + || w.hasTag("railway", "platform_exit_only")) { return true; } @@ -113,3 +136,101 @@ } + /** + * Checks if all members of a relation are complete. If not, the user is + * asked to confirm the permission to fetch them, and they are fetched. The + * completeness of the relation itself is not checked. + * + * @param r + * relation + * @return true if all relation members are complete (or fetched), false + * otherwise (including if the user denies permission to download + * data) + * TODO: what should be done in case the connection to the server is broken + */ + public static boolean ensureMemberCompleteness(Relation r) { + + if (r == null) { + return false; + } + + boolean isComplete = true; + + // check if there is at least one incomplete relation member: + for (RelationMember rm : r.getMembers()) { + if ((rm.isNode() && rm.getNode().isIncomplete()) || (rm.isWay() && rm.getWay().isIncomplete()) + || (rm.isRelation() && rm.getRelation().isIncomplete())) { + isComplete = false; + break; + } + } + + if (!isComplete && !r.equals(lastRelationToFetch)) { + + int userInput = Integer.MIN_VALUE; + + + if (askToFetch == ASK_TO_FETCH.DO_ASK) { + String message = tr("The relation (id=" + r.getId() + + ") has incomplete members.\nThey need to be downloaded to proceed with validation of this relation.\nDo you want to download incomplete members?"); + JCheckBox checkbox = new JCheckBox(tr("Remember my choice and don't ask me again in this session")); + Object[] params = { message, checkbox }; + String[] options = { tr("Yes"), tr("No") }; + // ask the user: + userInput = JOptionPane.showOptionDialog(null, params, tr("Fetch Request"), JOptionPane.YES_NO_OPTION, + JOptionPane.QUESTION_MESSAGE, null, options, 0); + + + // if the user does not want to be asked: + if (checkbox.isSelected()) { + if (userInput == 0) { + askToFetch = ASK_TO_FETCH.DONT_ASK_AND_FETCH; + } else { + askToFetch = ASK_TO_FETCH.DONT_ASK_AND_DONT_FETCH; + } + } + } + + // if the user does want to fetch: + if (userInput == 0 || askToFetch == ASK_TO_FETCH.DONT_ASK_AND_FETCH) { + List list = new ArrayList<>(1); + list.add(r); + DownloadPrimitiveAction.processItems(false, list, false, true); + isComplete = true; + lastRelationToFetch = r; + + } + + } + + return isComplete; + } + + + public static boolean hasIncompleteMembers(Relation r) { + if (r == null) { + return true; + } + for (RelationMember rm: r.getMembers()) { + if ((rm.isNode() && rm.getNode().isIncomplete()) || (rm.isWay() && rm.getWay().isIncomplete()) + || (rm.isRelation() && rm.getRelation().isIncomplete())) { + return true; + } + } + + return false; + } + +// /** +// * TODO: this is temporal +// */ +// public static String getFetch() { +// if (askToFetch == ASK_TO_FETCH.DO_ASK) { +// return "do ask"; +// } +// if (askToFetch == ASK_TO_FETCH.DONT_ASK_AND_FETCH) { +// return "don;t ask and fetch"; +// } +// return "don't ask and don't fetch"; +// } + } Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/StopToWayAssigner.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/StopToWayAssigner.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/utils/StopToWayAssigner.java (revision 32230) @@ -1,29 +1,66 @@ package org.openstreetmap.josm.plugins.pt_assistant.utils; -import java.util.LinkedList; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; public final class StopToWayAssigner { - - private StopToWayAssigner() { - // Hide default constructor for utils classes + + private static HashMap stopToWay = new HashMap<>(); + + private StopToWayAssigner(Relation r) { + // Hide default constructor for utils classes + } + + public static Way getWay(OsmPrimitive stop, Relation route) { + if (stopToWay.containsKey(stop.getId())) { + return stopToWay.get(stop.getId()); + } + + + + + if (stop.getType().equals(OsmPrimitiveType.NODE)) { + List referrers = stop.getReferrers(); + List referredWays = new ArrayList<>(); + for (OsmPrimitive referrer: referrers) { + if (referrer.getType().equals(OsmPrimitiveType.WAY)) { + referredWays.add((Way)referrer); + } + } + if (stop.hasTag("public_transport", "stop_position")) { + // TODO + Node n = (Node) stop; + } + } + + // TODO: algorithm with growing bounding boxes + // TODO: if found, add to + return null; } - public static Way getWay(Node stop, Relation route) { - List ways = new LinkedList<>(); - List members = route.getMembers(); - for (RelationMember member: members) { - if (member.getType().equals(OsmPrimitiveType.WAY)) { - ways.add(member.getWay()); - } + /** + * Remove a map entry + * @param stopId + */ + public static void removeStopKey(long stopId) { + Long id = new Long(stopId); + if (stopToWay.containsKey(id)) { + stopToWay.remove(id); } - // TODO: algorithm with growing bounding boxes - return null; + } + + /** + * May be needed if the correspondence between stops and ways has changed + * significantly + */ + public static void reinitiate() { + stopToWay = new HashMap<>(); } Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/DirectionTest.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/DirectionTest.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/DirectionTest.java (revision 32230) @@ -6,7 +6,10 @@ import java.util.List; +import javax.swing.JOptionPane; + import org.openstreetmap.josm.command.ChangeCommand; import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; @@ -25,8 +28,10 @@ public static final int ERROR_CODE_DIRECTION = 3731; - public static final int ERROR_CODE_ROUNDABOUT = 3732; + public static final int ERROR_CODE_UNSPLIT_ROUNDABOUT = 3732; + public static final int ERROR_CODE_SPLIT_ROUNDABOUT = 3733; public DirectionTest() { super(tr("Direction Test"), tr("Checks if the route runs against the direction of underlying one-way roads")); + } @@ -34,55 +39,79 @@ public void visit(Relation r) { - if (RouteUtils.isTwoDirectionRoute(r)) { + if (!RouteUtils.isTwoDirectionRoute(r)) { + return; + } - List waysToCheck = new ArrayList<>(); +// boolean isComplete = RouteUtils.ensureMemberCompleteness(r); +// if (!isComplete) { +// return; +// } + + if (RouteUtils.hasIncompleteMembers(r)) { + return; + } - for (RelationMember rm : r.getMembers()) { - if (RouteUtils.isPTWay(rm) && rm.getType().equals(OsmPrimitiveType.WAY)) { - waysToCheck.add(rm); + List waysToCheck = new ArrayList<>(); + + for (RelationMember rm : r.getMembers()) { + if (RouteUtils.isPTWay(rm) && rm.getType().equals(OsmPrimitiveType.WAY)) { + waysToCheck.add(rm); + } + } + + if (waysToCheck.isEmpty()) { + return; + } + + WayConnectionTypeCalculator connectionTypeCalculator = new WayConnectionTypeCalculator(); + final List links = connectionTypeCalculator.updateLinks(waysToCheck); + + for (int i = 0; i < links.size(); i++) { + if ((OsmUtils.isTrue(waysToCheck.get(i).getWay().get("oneway")) + && links.get(i).direction.equals(WayConnectionType.Direction.BACKWARD)) + || (OsmUtils.isReversed(waysToCheck.get(i).getWay().get("oneway")) + && links.get(i).direction.equals(WayConnectionType.Direction.FORWARD))) { + + // At this point, the PTWay is going against the oneway + // direction. Check if this road allows buses to disregard + // the oneway restriction: + + if (!waysToCheck.get(i).getWay().hasTag("busway", "lane") + && !waysToCheck.get(i).getWay().hasTag("oneway:bus", "no") + && !waysToCheck.get(i).getWay().hasTag("busway", "opposite_lane") + && !waysToCheck.get(i).getWay().hasTag("oneway:psv", "no") + && !waysToCheck.get(i).getWay().hasTag("trolley_wire", "backward")) { + List primitives = new ArrayList<>(1); + primitives.add(r); + List highlighted = new ArrayList<>(1); + highlighted.add(waysToCheck.get(i).getWay()); + errors.add(new TestError(this, Severity.WARNING, + tr("PT: Route passes a oneway road in wrong direction"), ERROR_CODE_DIRECTION, primitives, + highlighted)); + return; } + } - if (waysToCheck.isEmpty()) { - return; - } - - WayConnectionTypeCalculator connectionTypeCalculator = new WayConnectionTypeCalculator(); - final List links = connectionTypeCalculator.updateLinks(waysToCheck); - - for (int i = 0; i < links.size(); i++) { - if ((OsmUtils.isTrue(waysToCheck.get(i).getWay().get("oneway")) - && links.get(i).direction.equals(WayConnectionType.Direction.BACKWARD)) - || (OsmUtils.isReversed(waysToCheck.get(i).getWay().get("oneway")) - && links.get(i).direction.equals(WayConnectionType.Direction.FORWARD))) { - - // At this point, the PTWay is going against the oneway - // direction. Check if this road allows buses to disregard - // the oneway restriction: - - if (!waysToCheck.get(i).getWay().hasTag("busway", "lane") - && !waysToCheck.get(i).getWay().hasTag("oneway:bus", "no") - && !waysToCheck.get(i).getWay().hasTag("busway", "opposite_lane") - && !waysToCheck.get(i).getWay().hasTag("oneway:psv", "no") - && !waysToCheck.get(i).getWay().hasTag("trolley_wire", "backward")) { - List primitives = new ArrayList<>(1); - primitives.add(r); - List highlighted = new ArrayList<>(1); - highlighted.add(waysToCheck.get(i).getWay()); - errors.add(new TestError(this, Severity.WARNING, - tr("PT: Route passes a oneway road in wrong direction"), ERROR_CODE_DIRECTION, - primitives, highlighted)); - } - - } - - if (links.get(i).direction.equals(WayConnectionType.Direction.ROUNDABOUT_LEFT) - || links.get(i).direction.equals(WayConnectionType.Direction.ROUNDABOUT_RIGHT)) { - errors.add(new TestError(this, Severity.WARNING, tr("PT: Route passes on an unsplit roundabout"), - ERROR_CODE_ROUNDABOUT, waysToCheck.get(i).getWay())); - } - } +// if (links.get(i).direction.equals(WayConnectionType.Direction.ROUNDABOUT_LEFT) +// || links.get(i).direction.equals(WayConnectionType.Direction.ROUNDABOUT_RIGHT)) { +// errors.add(new TestError(this, Severity.WARNING, tr("PT: Route passes on an unsplit roundabout"), +// ERROR_CODE_UNSPLIT_ROUNDABOUT, waysToCheck.get(i).getWay())); +// return; +// } + +// if (waysToCheck.get(i).getWay().hasTag("junction", "roundabout")) { +// JOptionPane.showMessageDialog(null, waysToCheck.get(i).getWay().getId() + " linkPrev: " + links.get(i).linkPrev + ", linkNext: " + links.get(i).linkNext); +//// Node firstNode = waysToCheck.get(i).getWay().firstNode(); +//// Node lastNode = waysToCheck.get(index) +//// if (i == 0 && waysToCheck.size() > 1) { +//// // if this is the very first way: +//// +//// } +// } + } + } Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/GapTest.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/GapTest.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/GapTest.java (revision 32230) @@ -5,4 +5,6 @@ import java.util.ArrayList; import java.util.List; + +import javax.swing.JOptionPane; import org.openstreetmap.josm.command.ChangeCommand; @@ -26,4 +28,5 @@ public static final int ERROR_CODE_SORTING = 3711; public static final int ERROR_CODE_OVERSHOOT = 3712; + public static final int ERROR_CODE_SPLITTING = 3713; public static final int ERROR_CODE_OTHER_GAP = 3719; @@ -36,40 +39,58 @@ @Override public void visit(Relation r) { - - if (RouteUtils.isTwoDirectionRoute(r)) { - List members = r.getMembers(); - final List waysToCheck = new ArrayList<>(); - for (RelationMember member : members) { - - if (RouteUtils.isPTWay(member)) { - waysToCheck.add(member); - } - } - - if (waysToCheck.isEmpty()) { - return; - } - - if (hasGap(waysToCheck)) { - RelationSorter sorter = new RelationSorter(); - List correctedList = sorter.sortMembers(waysToCheck); - - if (!hasGap(correctedList)) { + + if (!RouteUtils.isTwoDirectionRoute(r)) { + return; + } + +// boolean isComplete = RouteUtils.ensureMemberCompleteness(r); +// if (!isComplete) { +// return; +// } + + if (RouteUtils.hasIncompleteMembers(r)) { + return; + } + + List members = r.getMembers(); + final List waysToCheck = new ArrayList<>(); + for (RelationMember member : members) { + + if (RouteUtils.isPTWay(member)) { + waysToCheck.add(member); + } + } + + if (waysToCheck.isEmpty()) { + return; + } + + if (hasGap(waysToCheck)) { + RelationSorter sorter = new RelationSorter(); + List correctedList = sorter.sortMembers(waysToCheck); + + if (!hasGap(correctedList)) { + errors.add(new TestError(this, Severity.WARNING, + tr("PT: Route contains a gap that can be fixed by sorting"), ERROR_CODE_SORTING, r)); + } else { + // List overshoots = + // this.getOvershoots(correctedList); + // if (!overshoots.isEmpty()) { + // // TODO: make sure that duplicates are removed first + // for (RelationMember overshoot : overshoots) { + // List primitives = new ArrayList<>(1); + // primitives.add(r); + // List highlighted = new ArrayList<>(1); + // highlighted.add(overshoot.getWay()); + // errors.add(new TestError(this, Severity.WARNING, tr("PT: + // Route contains an overshoot"), + // ERROR_CODE_OVERSHOOT, primitives, highlighted)); + // } + // + // } else { errors.add(new TestError(this, Severity.WARNING, - tr("PT: Route contains a gap that can be fixed by sorting"), ERROR_CODE_SORTING, r)); - } else { - List overshoots = this.getOvershoots(correctedList); - if (!overshoots.isEmpty()) { - // TODO: make sure that duplicates are removed first - overshootList = overshoots; - errors.add(new TestError(this, Severity.WARNING, tr("PT: Route contains an overshoot"), - ERROR_CODE_OVERSHOOT, r)); - } else { - errors.add(new TestError(this, Severity.WARNING, - tr("PT: Route contains a gap that cannot be fixed by sorting the ways"), - ERROR_CODE_OTHER_GAP, r)); - } - } - + tr("PT: Route contains a gap that cannot be fixed by sorting the ways"), + ERROR_CODE_OTHER_GAP, r)); +// } } @@ -98,6 +119,4 @@ } } - - return false; @@ -191,5 +210,6 @@ } else { // stops: if (member.getRole().equals("stop_positon")) { - // it is not expected that stop_positions could be relations + // it is not expected that stop_positions could + // be relations if (member.getType().equals(OsmPrimitiveType.NODE)) { RelationMember modifiedMember = new RelationMember("stop", member.getNode()); @@ -230,49 +250,50 @@ // if the error is a single overshoot: - if (testError.getCode() == ERROR_CODE_OVERSHOOT) { - - for (OsmPrimitive primitive : testError.getPrimitives()) { - Relation originalRelation = (Relation) primitive; - Relation modifiedRelation = new Relation(originalRelation); - List modifiedMembers = new ArrayList<>(); - // add stops of a public transport route first: - for (RelationMember rm : originalRelation.getMembers()) { - if (RouteUtils.isPTStop(rm)) { - if (rm.hasRole("stop_position")) { - // it is not expected that stop_positions could be relations - if (rm.getType().equals(OsmPrimitiveType.NODE)) { - RelationMember modifiedMember = new RelationMember("stop", rm.getNode()); - modifiedMembers.add(modifiedMember); - } else { // if it is a primitive of type "way": - RelationMember modifiedMember = new RelationMember("stop", rm.getWay()); - modifiedMembers.add(modifiedMember); - } - } else { - modifiedMembers.add(rm); - } - - } - - } - // add ways of a public transport route (if they are not - // overshoots): - for (RelationMember rm : originalRelation.getMembers()) { - if (RouteUtils.isPTWay(rm) && !overshootList.contains(rm)) { - - if (rm.getRole().equals("")) { - modifiedMembers.add(rm); - } else { - RelationMember modifiedMember = new RelationMember("", rm.getWay()); - modifiedMembers.add(modifiedMember); - } - } - } - modifiedRelation.setMembers(modifiedMembers); - - ChangeCommand changeCommand = new ChangeCommand(originalRelation, modifiedRelation); - commands.add(changeCommand); - } - - } +// if (testError.getCode() == ERROR_CODE_OVERSHOOT) { +// +// for (OsmPrimitive primitive : testError.getPrimitives()) { +// Relation originalRelation = (Relation) primitive; +// Relation modifiedRelation = new Relation(originalRelation); +// List modifiedMembers = new ArrayList<>(); +// // add stops of a public transport route first: +// for (RelationMember rm : originalRelation.getMembers()) { +// if (RouteUtils.isPTStop(rm)) { +// if (rm.hasRole("stop_position")) { +// // it is not expected that stop_positions could +// // be relations +// if (rm.getType().equals(OsmPrimitiveType.NODE)) { +// RelationMember modifiedMember = new RelationMember("stop", rm.getNode()); +// modifiedMembers.add(modifiedMember); +// } else { // if it is a primitive of type "way": +// RelationMember modifiedMember = new RelationMember("stop", rm.getWay()); +// modifiedMembers.add(modifiedMember); +// } +// } else { +// modifiedMembers.add(rm); +// } +// +// } +// +// } +// // add ways of a public transport route (if they are not +// // overshoots): +// for (RelationMember rm : originalRelation.getMembers()) { +// if (RouteUtils.isPTWay(rm) && !overshootList.contains(rm)) { +// +// if (rm.getRole().equals("")) { +// modifiedMembers.add(rm); +// } else { +// RelationMember modifiedMember = new RelationMember("", rm.getWay()); +// modifiedMembers.add(modifiedMember); +// } +// } +// } +// modifiedRelation.setMembers(modifiedMembers); +// +// ChangeCommand changeCommand = new ChangeCommand(originalRelation, modifiedRelation); +// commands.add(changeCommand); +// } +// +// } } @@ -294,5 +315,5 @@ @Override public boolean isFixable(TestError testError) { - if (testError.getCode() == ERROR_CODE_SORTING || testError.getCode() == ERROR_CODE_OVERSHOOT) { + if (testError.getCode() == ERROR_CODE_SORTING ) { return true; } Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/PlatformsFirstTest.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/PlatformsFirstTest.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/PlatformsFirstTest.java (revision 32230) @@ -11,34 +11,39 @@ import org.openstreetmap.josm.data.validation.Test; import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.plugins.pt_assistant.utils.RouteUtils; -public class PlatformsFirstTest extends Test { - +public class PlatformsFirstTest extends Test { + public static final int ERROR_CODE = 3701; - - /** - * Constructs a new {@code InternetTags} test. - */ - public PlatformsFirstTest() { - super(tr("Platforms first"), tr("Checks if platforms are listed before ways in the route relation.")); - } - - @Override - public void visit(Relation r) { - - if (r.hasKey("route")) { - List members = r.getMembers(); - RelationMember prevMember = null; - for (RelationMember currMember: members) { - if (prevMember != null) { - // check if the current member is a platform, while the previous member is a way: - if (currMember.hasRole("platform") && prevMember.getType().equals(OsmPrimitiveType.WAY) && prevMember.hasRole("")) { - this.errors.add(new TestError(this, Severity.WARNING, tr("PT: route relation(s) contain(s) way(s) before platform(s) in the members list"), ERROR_CODE, r)); - return; - } - } - prevMember = currMember; - } - } - } + /** + * Constructs a new {@code InternetTags} test. + */ + public PlatformsFirstTest() { + super(tr("Platforms first"), tr("Checks if platforms are listed before ways in the route relation.")); + } + + @Override + public void visit(Relation r) { + + if (RouteUtils.isTwoDirectionRoute(r)) { + + List members = r.getMembers(); + RelationMember prevMember = null; + for (RelationMember currMember : members) { + if (prevMember != null) { + // check if the current member is a platform, while the + // previous member is a way: + if (currMember.hasRole("platform") && prevMember.getType().equals(OsmPrimitiveType.WAY) + && prevMember.hasRole("")) { + this.errors.add(new TestError(this, Severity.WARNING, + tr("PT: route relation(s) contain(s) way(s) before platform(s) in the members list"), + ERROR_CODE, r)); + return; + } + } + prevMember = currMember; + } + } + } } Index: applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/RoadTypeTest.java =================================================================== --- applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/RoadTypeTest.java (revision 32229) +++ applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/validation/RoadTypeTest.java (revision 32230) @@ -31,62 +31,70 @@ public void visit(Relation r) { - if (RouteUtils.isTwoDirectionRoute(r)) { - - List members = r.getMembers(); + if (!RouteUtils.isTwoDirectionRoute(r)) { + return; + } - for (RelationMember rm : members) { - if (RouteUtils.isPTWay(rm)) { - - Way way = rm.getWay(); - // at this point, the relation has already been checked to - // be a route of public_transport:version 2 - boolean isCorrectRoadType = true; - if (r.hasTag("route", "bus") || r.hasTag("route", "share_taxi")) { - if (way.getId()==388339788 || way.getId() == 388339789) { +// boolean isComplete = RouteUtils.ensureMemberCompleteness(r); +// if (!isComplete) { +// return; +// } + + if (RouteUtils.hasIncompleteMembers(r)) { + return; + } + + List members = r.getMembers(); - } - if (!RouteUtils.isWaySuitableForBuses(way)) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "trolleybus")) { - if (!(RouteUtils.isWaySuitableForBuses(way) && way.hasTag("trolley_wire", "yes"))) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "tram")) { - if (!r.hasTag("railway", "tram")) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "subway")) { - if (!r.hasTag("railway", "subway")) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "light_rail")) { - if (!r.hasTag("raiilway", "subway")) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "light_rail")) { - if (!r.hasTag("railway", "light_rail")) { - isCorrectRoadType = false; - } - } else if (r.hasTag("route", "train")) { - if (!r.hasTag("railway", "train")) { - isCorrectRoadType = false; - } + for (RelationMember rm : members) { + if (RouteUtils.isPTWay(rm)) { + + Way way = rm.getWay(); + // at this point, the relation has already been checked to + // be a route of public_transport:version 2 + boolean isCorrectRoadType = true; + if (r.hasTag("route", "bus") || r.hasTag("route", "share_taxi")) { + if (!RouteUtils.isWaySuitableForBuses(way)) { + isCorrectRoadType = false; } + } else if (r.hasTag("route", "trolleybus")) { + if (!(RouteUtils.isWaySuitableForBuses(way) && way.hasTag("trolley_wire", "yes"))) { + isCorrectRoadType = false; + } + } else if (r.hasTag("route", "tram")) { + if (!r.hasTag("railway", "tram")) { + isCorrectRoadType = false; + } + } else if (r.hasTag("route", "subway")) { + if (!r.hasTag("railway", "subway")) { + isCorrectRoadType = false; + } + } else if (r.hasTag("route", "light_rail")) { + if (!r.hasTag("raiilway", "subway")) { + isCorrectRoadType = false; + } + } else if (r.hasTag("route", "light_rail")) { + if (!r.hasTag("railway", "light_rail")) { + isCorrectRoadType = false; + } + } else if (r.hasTag("route", "train")) { + if (!r.hasTag("railway", "train")) { + isCorrectRoadType = false; + } + } - if (!isCorrectRoadType) { - - List primitiveList = new ArrayList<>(2); - primitiveList.add(0, r); - primitiveList.add(1, way); - - errors.add(new TestError(this, Severity.WARNING, - tr("PT: Route type does not match the type of the road it passes on"), - ERROR_CODE_ROAD_TYPE, primitiveList)); - } + if (!isCorrectRoadType) { + List primitives = new ArrayList<>(1); + primitives.add(r); + List highlighted = new ArrayList<>(1); + highlighted.add(way); + errors.add(new TestError(this, Severity.WARNING, + tr("PT: Route type does not match the type of the road it passes on"), ERROR_CODE_ROAD_TYPE, + primitives, highlighted)); } + } } + } @@ -97,15 +105,16 @@ if (testError.getTester().getClass().equals(RoadTypeTest.class) && testError.isFixable()) { - List primitiveList = (List) testError.getPrimitives(); - Relation originalRelation = (Relation) primitiveList.get(0); - Way wayToRemove = (Way) primitiveList.get(1); - + List primitives = (List) testError.getPrimitives(); + Relation originalRelation = (Relation) primitives.get(0); + List highlighted = (List) testError.getHighlighted(); + Way wayToRemove = (Way) highlighted.get(0); + Relation modifiedRelation = new Relation(originalRelation); - List modifiedRelationMembers = new ArrayList<>(originalRelation.getMembersCount()-1); - + List modifiedRelationMembers = new ArrayList<>(originalRelation.getMembersCount() - 1); + // copy PT stops first, PT ways last: - for (RelationMember rm: originalRelation.getMembers()) { + for (RelationMember rm : originalRelation.getMembers()) { if (RouteUtils.isPTStop(rm)) { - + if (rm.getRole().equals("stop_position")) { if (rm.getType().equals(OsmPrimitiveType.NODE)) { @@ -116,14 +125,15 @@ modifiedRelationMembers.add(newMember); } - } else { - // if the relation member does not have the role "stop_position": + } else { + // if the relation member does not have the role + // "stop_position": modifiedRelationMembers.add(rm); } - - } + + } } - + // now copy PT ways: - for (RelationMember rm: originalRelation.getMembers()) { + for (RelationMember rm : originalRelation.getMembers()) { if (RouteUtils.isPTWay(rm)) { Way wayToCheck = rm.getWay(); @@ -138,12 +148,12 @@ } } - + modifiedRelation.setMembers(modifiedRelationMembers); - + ChangeCommand changeCommand = new ChangeCommand(originalRelation, modifiedRelation); commands.add(changeCommand); - + } - + if (commands.isEmpty()) { return null; @@ -153,8 +163,7 @@ return commands.get(0); } - - return new SequenceCommand(tr("Remove way from route if it does not match the route type"), commands); - + return new SequenceCommand(tr("Remove way from route if it does not match the route type"), commands); + } @@ -167,5 +176,5 @@ return true; } - return false; + return false; }