Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Validate that combination of route names and type are unique wi… #839

Merged
merged 3 commits into from
Mar 30, 2021
Merged

feat: Validate that combination of route names and type are unique wi… #839

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
546165a
feat: Validate that combination of route names and type are unique wi…
aababilov Mar 29, 2021
d4bdd1c
Update RULES.md
aababilov Mar 30, 2021
9a44345
Fix a typo
aababilov Mar 30, 2021
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions RULES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,15 +555,9 @@ At least one of the fields `is_producer`, `is_operator`, or `is_authority` shoul

#### DuplicateRouteNameNotice

All routes should have different `routes.route_long_name` - if two `routes.route_long_name` are the same, and the two routes belong to the same agency, a notice is generated.
All routes of the same `route_type` with the same `agency_id` should have unique combinations of `route_short_name` and `route_long_name`.

Note that there may be valid cases where routes may have the same `routes.route_long_name` and this notice can be ignored. For example, routes may have the same `routes.route_long_name` if they serve difference areas. However, they must not be different trips of the same route or different directions of the same route - these cases should always have unique `routes.route_long_name`.

All routes should have different `routes.route_short_name` - if two `routes.route_short_name` are the same, and the two routes belong to the same agency, a notice is generated.

Note that there may be valid cases where routes may have the same `routes.route_short_name` and this notice can be ignored. For example, routes may have the same routes.route_short_name if they serve difference areas. However, they must not be different trips of the same route or different directions of the same route - these cases should always have unique `routes.route_short_name`.

The same combination of `route_short_name` and `route_long_name` should not be used for more than one route.
Note that there may be valid cases where routes have the same short and long name, e.g., if they serve difference areas. However, different directions must be modeled as the same route.
lionel-nj marked this conversation as resolved.
Show resolved Hide resolved

##### References:
* [routes.txt specification](http://gtfs.org/reference/static/#routestxt)
Expand Down
105 changes: 25 additions & 80 deletions main/src/main/java/org/mobilitydata/gtfsvalidator/validator/DuplicateRouteNameValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,7 @@
import org.mobilitydata.gtfsvalidator.table.GtfsRoute;
import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer;

/**
* Validates unicity of short and long name for all routes.
*
* <p>When a {@code GtfsRoute} short and/or long names are found to be duplicate a {@code
* DuplicateRouteNameNotice} is generated and added to the {@code NoticeContainer} except if routes
* are from the same agency (values for "route.agency_id" are case-sensitive) or routes have
* different "routes.route_type".
*
* <p>Generated notice:
*
* <ul>
* <li>{@link DuplicateRouteNameNotice}
* </ul>
*/
/** Validates that combinations of route type, short and long name are unique within an agency. */
@GtfsValidator
public class DuplicateRouteNameValidator extends FileValidator {
private final GtfsRouteTableContainer routeTable;
Expand All @@ -53,82 +40,40 @@ public class DuplicateRouteNameValidator extends FileValidator {

@Override
public void validate(NoticeContainer noticeContainer) {
final Map<Integer, GtfsRoute> routeByLongName = new HashMap<>(routeTable.entityCount());
final Map<Integer, GtfsRoute> routeByShortName = new HashMap<>(routeTable.entityCount());
routeTable
.getEntities()
.forEach(
route -> {
GtfsRoute otherRoute;
if (route.hasRouteLongName()) {
otherRoute = routeByLongName.putIfAbsent(getLongNameAndTypeHash(route), route);
if (otherRoute != null) {
if (areRoutesFromSameAgency(route, otherRoute)) {
noticeContainer.addValidationNotice(
new DuplicateRouteNameNotice(
"route_long_name", route.csvRowNumber(), route.routeId()));
}
}
}
if (route.hasRouteShortName()) {
otherRoute = routeByShortName.putIfAbsent(getShortNameAndTypeHash(route), route);
if (otherRoute != null) {
if (areRoutesFromSameAgency(route, otherRoute)) {
noticeContainer.addValidationNotice(
new DuplicateRouteNameNotice(
"route_short_name", route.csvRowNumber(), route.routeId()));
}
}
}
});
}

/**
* Determines if two routes are from the same agency: ids are case-sensitive.
*
* @param route first {@code GtfsRoute}
* @param otherRoute second {@code GtfsRoute}
* @return true if both agency ids are equals returns false otherwise.
*/
private boolean areRoutesFromSameAgency(final GtfsRoute route, final GtfsRoute otherRoute) {
return route.agencyId().equals(otherRoute.agencyId());
}

/**
* Generate an hash associated to "routes.route_long_name" and "routes.route_type". This hash is
* used to interact with routeByLongName (variable defined in this class' validate method) to
* store and retrieve routes by short name.
*
* @param route the {@code GtfsRoute} to generate the hash from
* @return the hash associated to "routes.route_long_name" and "routes.route_type".
*/
private int getLongNameAndTypeHash(GtfsRoute route) {
return Objects.hash(route.routeLongName(), route.routeType());
final Map<Integer, GtfsRoute> routeByKey = new HashMap<>(routeTable.entityCount());
for (GtfsRoute route : routeTable.getEntities()) {
GtfsRoute prevRoute = routeByKey.putIfAbsent(getRouteKey(route), route);
if (prevRoute != null) {
noticeContainer.addValidationNotice(new DuplicateRouteNameNotice(prevRoute, route));
}
}
}

/**
* Generate an hash associated to "routes.route_short_name" and "routes.route_type". This hash is
* used to interact with routeByShortName (variable defined in this class' validate method) to
* store and retrieve routes by short name.
*
* @param route the {@code GtfsRoute} to generate the hash from
* @return the hash associated to "routes.route_short_name" and "routes.route_type".
*/
private int getShortNameAndTypeHash(GtfsRoute route) {
return Objects.hash(route.routeShortName(), route.routeType());
/** Generates a hash based on route names, type and agency. */
private static int getRouteKey(GtfsRoute route) {
return Objects.hash(
route.routeLongName(), route.routeShortName(), route.routeType(), route.agencyId());
}

/**
* All routes should have different `routes.route_long_name`. All routes should have different
* `routes.route_short_name`.
* Describes two routes that have the same long and short names, route type and belong to the same
* agency.
*
* <p>Severity: {@code SeverityLevel.WARNING}
*/
static class DuplicateRouteNameNotice extends ValidationNotice {
DuplicateRouteNameNotice(String duplicatedField, long csvRowNumber, String routeId) {
DuplicateRouteNameNotice(GtfsRoute route1, GtfsRoute route2) {
super(
ImmutableMap.of(
"duplicatedField", duplicatedField, "csvRowNumber", csvRowNumber, "routeId", routeId),
new ImmutableMap.Builder<String, Object>()
.put("csvRowNumber1", route1.csvRowNumber())
.put("routeId1", route1.routeId())
.put("csvRowNumber2", route2.csvRowNumber())
.put("routeId2", route2.routeId())
.put("routeShortName", route1.routeShortName())
.put("routeLongName", route1.routeLongName())
.put("routeType", route1.routeTypeValue())
.put("agencyId", route1.agencyId())
.build(),
SeverityLevel.WARNING);
}
}
Expand Down
168 changes: 48 additions & 120 deletions ...c/test/java/org/mobilitydata/gtfsvalidator/validator/DuplicateRouteNameValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,36 @@
package org.mobilitydata.gtfsvalidator.validator;

import static com.google.common.truth.Truth.assertThat;
import static org.mobilitydata.gtfsvalidator.table.GtfsRouteType.BUS;
import static org.mobilitydata.gtfsvalidator.table.GtfsRouteType.LIGHT_RAIL;

import com.google.common.collect.ImmutableList;
import java.util.List;
import javax.annotation.Nullable;
import org.junit.Test;
import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
import org.mobilitydata.gtfsvalidator.notice.ValidationNotice;
import org.mobilitydata.gtfsvalidator.table.GtfsRoute;
import org.mobilitydata.gtfsvalidator.table.GtfsRouteTableContainer;
import org.mobilitydata.gtfsvalidator.table.GtfsRouteType;
import org.mobilitydata.gtfsvalidator.validator.DuplicateRouteNameValidator.DuplicateRouteNameNotice;

public class DuplicateRouteNameValidatorTest {

private GtfsRoute createRoute(
private static GtfsRoute createRoute(
int csvRowNumber,
String routeId,
String agencyId,
String shortName,
String longName,
int routeType) {
@Nullable String shortName,
@Nullable String longName,
GtfsRouteType routeType) {
return new GtfsRoute.Builder()
.setCsvRowNumber(csvRowNumber)
.setRouteId(routeId)
.setAgencyId(agencyId)
.setRouteShortName(shortName)
.setRouteLongName(longName)
.setRouteType(routeType)
.setRouteType(routeType.getNumber())
.build();
}

Expand All @@ -38,153 +42,77 @@ private static List<ValidationNotice> generateNotices(List<GtfsRoute> routes) {
}

@Test
public void duplicateRouteLongNamesOfRoutesFromDifferentAgenciesShouldNotGenerateNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(
2, "1st route id value", "agency id", "short name", "duplicate value", 2),
createRoute(
4,
"2nd route id value",
"other agency id",
"other short name",
"duplicate value",
3),
createRoute(8, "3rd route id value", null, "another one", "duplicate value", 2),
createRoute(
8,
"4th route id value",
"agenCY ID",
"other sname",
"duplicate value",
2))))
.isEmpty();
public void sameNamesTypeAgency_yieldsNotice() {
GtfsRoute route1 = createRoute(2, "route1", "agency1", "L1", "Dulwich Hill", LIGHT_RAIL);
GtfsRoute route2 = createRoute(3, "route2", "agency1", "L1", "Dulwich Hill", LIGHT_RAIL);
assertThat(generateNotices(ImmutableList.of(route1, route2)))
.containsExactly(new DuplicateRouteNameNotice(route1, route2));
}

@Test
public void duplicateRouteLongNamesOfRoutesFromSameAgencyShouldGenerateNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(
2, "1st route id value", "agency id", "short name", "duplicate value", 2),
createRoute(
4,
"2nd route id value",
"agency id",
"other short name",
"duplicate value",
2),
createRoute(
8,
"3rd route id value",
"agency id",
"another one",
"duplicate value",
2))))
public void manyRoutes_yieldsMultipleNotices() {
GtfsRoute route1 = createRoute(2, "route1", "agency1", "L1", "Dulwich Hill", LIGHT_RAIL);
GtfsRoute route2 = createRoute(3, "route2", "agency1", "L1", "Dulwich Hill", LIGHT_RAIL);
GtfsRoute route3 = createRoute(4, "route3", "agency1", "L1", "Dulwich Hill", LIGHT_RAIL);
assertThat(generateNotices(ImmutableList.of(route1, route2, route3)))
.containsExactly(
new DuplicateRouteNameNotice("route_long_name", 4, "2nd route id value"),
new DuplicateRouteNameNotice("route_long_name", 8, "3rd route id value"));
new DuplicateRouteNameNotice(route1, route2),
new DuplicateRouteNameNotice(route1, route3));
}

@Test
public void duplicateRouteShortNamesOfRoutesFromDifferentAgenciesShouldNotGenerateNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(
2, "1st route id value", null, "duplicate value", "1st long name", 2),
createRoute(
4,
"2nd route id value",
"agency id",
"duplicate value",
"2nd long name",
3),
createRoute(
8,
"3rd route id value",
"other agency id",
"duplicate value",
"3rd long name",
3))))
.isEmpty();
public void noShortNames_yieldsNotice() {
GtfsRoute route1 = createRoute(2, "route1", "agency1", null, "Dulwich Hill", LIGHT_RAIL);
GtfsRoute route2 = createRoute(3, "route2", "agency1", null, "Dulwich Hill", LIGHT_RAIL);
assertThat(generateNotices(ImmutableList.of(route1, route2)))
.containsExactly(new DuplicateRouteNameNotice(route1, route2));
}

@Test
public void noLongNames_yieldsNotice() {
GtfsRoute route1 = createRoute(2, "route1", "agency1", "L1", null, LIGHT_RAIL);
GtfsRoute route2 = createRoute(3, "route2", "agency1", "L1", null, LIGHT_RAIL);
assertThat(generateNotices(ImmutableList.of(route1, route2)))
.containsExactly(new DuplicateRouteNameNotice(route1, route2));
}

@Test
public void duplicateRouteShortNamesOfRoutesFromSameAgencyShouldGenerateNotice() {
public void differentShortNames_yieldsNoNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(
2,
"1st route id value",
"agency id",
"duplicate value",
"1st long name",
2),
createRoute(
4,
"2nd route id value",
"agency id",
"duplicate value",
"2nd long name",
2),
createRoute(
8,
"3rd route id value",
"agency id",
"duplicate value",
"3rd long name",
2))))
.containsExactly(
new DuplicateRouteNameNotice("route_short_name", 4, "2nd route id value"),
new DuplicateRouteNameNotice("route_short_name", 8, "3rd route id value"));
createRoute(2, "route1", "agency1", "L1", "North Line", LIGHT_RAIL),
createRoute(2, "route2", "agency1", "L2", "North Line", LIGHT_RAIL))))
.isEmpty();
}

@Test
public void uniqueRouteLongNameShouldNotGenerateNotice() {
public void differentLongNames_yieldsNoNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(2, "1st route id value", "agency id", null, "1st value", 2),
createRoute(5, "4th route id value", "agency id", null, "1st value", 3),
createRoute(4, "2nd route id value", "another agency id", null, "2nd value", 3),
createRoute(8, "3rd route id value", "another one", null, "3rd value", 3))))
createRoute(2, "route1", "agency1", "L1", "North Line", LIGHT_RAIL),
createRoute(2, "route2", "agency1", "L1", "South Line", LIGHT_RAIL))))
.isEmpty();
}

@Test
public void uniqueRouteShortNameShouldNotGenerateNotice() {
public void differentAgencies_yieldsNoNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(2, "1st route id value", "agency id", "1st value", null, 2),
createRoute(5, "4th route id value", "agency id", "1st value", null, 3),
createRoute(4, "2nd route id value", "another agency id", "2nd value", null, 3),
createRoute(8, "3rd route id value", "another one", "3rd value", null, 3))))
createRoute(2, "route1", "agency1", "L1", "North Line", LIGHT_RAIL),
createRoute(2, "route2", "agency2", "L1", "North Line", LIGHT_RAIL))))
.isEmpty();
}

@Test
public void duplicateRouteNamesCombinationShouldGenerateNotice() {
public void differentTypes_yieldsNoNotice() {
assertThat(
generateNotices(
ImmutableList.of(
createRoute(2, "1st route id value", "agency id", "short name", "long name", 2),
createRoute(
5, "4th route id value", "agency id", "short name", "long value", 3),
createRoute(4, "2nd route id value", "agency id", "short name", "long name", 2),
createRoute(
8,
"3rd route id value",
"agency id",
"other short value",
"other long name",
3))))
.containsExactly(
new DuplicateRouteNameNotice("route_short_name", 4, "2nd route id value"),
new DuplicateRouteNameNotice("route_long_name", 4, "2nd route id value"));
createRoute(2, "route1", "agency1", "L1", "North Line", LIGHT_RAIL),
createRoute(2, "route2", "agency1", "L2", "North Line", BUS))))
.isEmpty();
}
}