diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/MissingRecommendedColumnNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/MissingRecommendedColumnNotice.java deleted file mode 100644 index 926186dbfc..0000000000 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/MissingRecommendedColumnNotice.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2020 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.mobilitydata.gtfsvalidator.notice; - -import static org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.SectionRef.TERM_DEFINITIONS; -import static org.mobilitydata.gtfsvalidator.notice.SeverityLevel.WARNING; - -import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice; -import org.mobilitydata.gtfsvalidator.annotation.GtfsValidationNotice.SectionRefs; - -/** A recommended column is missing in the input file. */ -@GtfsValidationNotice(severity = WARNING, sections = @SectionRefs(TERM_DEFINITIONS)) -public class MissingRecommendedColumnNotice extends ValidationNotice { - - /** The name of the faulty file. */ - private final String filename; - - /** The name of the missing column. */ - private final String fieldName; - - public MissingRecommendedColumnNotice(String filename, String fieldName) { - this.filename = filename; - this.fieldName = fieldName; - } -} diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearOriginNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearOriginNotice.java index 10baa5d88e..fc4313a484 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearOriginNotice.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearOriginNotice.java @@ -35,19 +35,22 @@ public class PointNearOriginNotice extends ValidationNotice { private final String filename; /** The row of the faulty row. */ - private final int csvRowNumber; + @Nullable private final Integer csvRowNumber; + + /** The index of the feature in the feature collection. */ + @Nullable private final Integer featureIndex; /** The id of the faulty entity. */ @Nullable private final String entityId; /** The name of the field that uses latitude value. */ - private final String latFieldName; + @Nullable private final String latFieldName; /** The latitude of the faulty row. */ private final double latFieldValue; /** The name of the field that uses longitude value. */ - private final String lonFieldName; + @Nullable private final String lonFieldName; /** The longitude of the faulty row */ private final double lonFieldValue; @@ -67,6 +70,7 @@ public PointNearOriginNotice( this.latFieldValue = latFieldValue; this.lonFieldName = lonFieldName; this.lonFieldValue = lonFieldValue; + this.featureIndex = null; } public PointNearOriginNotice( @@ -83,5 +87,22 @@ public PointNearOriginNotice( this.latFieldValue = latFieldValue; this.lonFieldName = lonFieldName; this.lonFieldValue = lonFieldValue; + this.featureIndex = null; + } + + public PointNearOriginNotice( + String filename, + String entityId, + double latFieldValue, + double lonFieldValue, + int featureIndex) { + this.filename = filename; + this.csvRowNumber = null; + this.entityId = entityId; + this.latFieldName = null; + this.latFieldValue = latFieldValue; + this.lonFieldName = null; + this.lonFieldValue = lonFieldValue; + this.featureIndex = featureIndex; } } diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearPoleNotice.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearPoleNotice.java index 8b577d0327..8b8296de88 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearPoleNotice.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/notice/PointNearPoleNotice.java @@ -35,21 +35,24 @@ public class PointNearPoleNotice extends ValidationNotice { private final String filename; /** The row of the faulty row. */ - private final int csvRowNumber; + @Nullable private final Integer csvRowNumber; + + /** The index of the feature in the feature collection. */ + @Nullable private final Integer featureIndex; /** The id of the faulty entity. */ @Nullable private final String entityId; /** The name of the field that uses latitude value. */ - private final String latFieldName; + @Nullable private final String latFieldName; /** The latitude of the faulty row. */ private final double latFieldValue; /** The name of the field that uses longitude value. */ - private final String lonFieldName; + @Nullable private final String lonFieldName; - /** The longitude of the faulty row. */ + /** The longitude of the faulty row */ private final double lonFieldValue; public PointNearPoleNotice( @@ -67,6 +70,7 @@ public PointNearPoleNotice( this.latFieldValue = latFieldValue; this.lonFieldName = lonFieldName; this.lonFieldValue = lonFieldValue; + this.featureIndex = null; } public PointNearPoleNotice( @@ -83,5 +87,22 @@ public PointNearPoleNotice( this.latFieldValue = latFieldValue; this.lonFieldName = lonFieldName; this.lonFieldValue = lonFieldValue; + this.featureIndex = null; + } + + public PointNearPoleNotice( + String filename, + String entityId, + double latFieldValue, + double lonFieldValue, + int featureIndex) { + this.filename = filename; + this.csvRowNumber = null; + this.entityId = entityId; + this.latFieldName = null; + this.latFieldValue = latFieldValue; + this.lonFieldName = null; + this.lonFieldValue = lonFieldValue; + this.featureIndex = featureIndex; } } diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/CsvFileLoader.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/CsvFileLoader.java index 1258c4634f..d97416d95e 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/CsvFileLoader.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/CsvFileLoader.java @@ -161,10 +161,6 @@ private NoticeContainer validateHeaders( .filter(GtfsColumnDescriptor::headerRequired) .map(GtfsColumnDescriptor::columnName) .collect(Collectors.toSet()), - columnDescriptors.stream() - .filter(GtfsColumnDescriptor::headerRecommended) - .map(GtfsColumnDescriptor::columnName) - .collect(Collectors.toSet()), headerNotices); return headerNotices; } diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsColumnDescriptor.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsColumnDescriptor.java index 0546e479d1..d75c87933d 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsColumnDescriptor.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/table/GtfsColumnDescriptor.java @@ -11,8 +11,6 @@ public abstract class GtfsColumnDescriptor { public abstract boolean headerRequired(); - public abstract boolean headerRecommended(); - public abstract FieldLevelEnum fieldLevel(); public abstract Optional numberBounds(); @@ -35,8 +33,6 @@ public abstract static class Builder { public abstract Builder setHeaderRequired(boolean value); - public abstract Builder setHeaderRecommended(boolean value); - public abstract Builder setFieldLevel(FieldLevelEnum value); public abstract Builder setNumberBounds(Optional value); diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultTableHeaderValidator.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultTableHeaderValidator.java index 3ef0f174c6..46776e9cc3 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultTableHeaderValidator.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/DefaultTableHeaderValidator.java @@ -23,7 +23,6 @@ import java.util.TreeSet; import org.mobilitydata.gtfsvalidator.notice.DuplicatedColumnNotice; import org.mobilitydata.gtfsvalidator.notice.EmptyColumnNameNotice; -import org.mobilitydata.gtfsvalidator.notice.MissingRecommendedColumnNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredColumnNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.UnknownColumnNotice; @@ -37,7 +36,6 @@ public void validate( CsvHeader actualHeader, Set supportedColumns, Set requiredColumns, - Set recommendedColumns, NoticeContainer noticeContainer) { if (actualHeader.getColumnCount() == 0) { // This is an empty file. @@ -48,9 +46,6 @@ public void validate( // We remove the columns that are properly present and well formed from that set, and at the // end only the missing required columns are left in the set. TreeSet missingColumns = new TreeSet<>(requiredColumns); - // We also want to find the recommended columns that are absent. We use the same scheme for - // these. - TreeSet missingRecommendedColumns = new TreeSet<>(recommendedColumns); for (int i = 0; i < actualHeader.getColumnCount(); ++i) { String column = actualHeader.getColumnName(i); // Column indices are zero-based. We add 1 to make them 1-based. @@ -69,20 +64,11 @@ public void validate( // If the column is present, it should not be in the missing required columns set missingColumns.remove(column); - - // If the column is present, it should not be in the missing recommended columns set - missingRecommendedColumns.remove(column); } if (!missingColumns.isEmpty()) { for (String column : missingColumns) { noticeContainer.addValidationNotice(new MissingRequiredColumnNotice(filename, column)); } } - - if (!missingRecommendedColumns.isEmpty()) { - for (String column : missingRecommendedColumns) { - noticeContainer.addValidationNotice(new MissingRecommendedColumnNotice(filename, column)); - } - } } } diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidator.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidator.java index 3cc5732d2d..ef42d43770 100644 --- a/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidator.java +++ b/core/src/main/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidator.java @@ -28,6 +28,5 @@ void validate( CsvHeader actualHeader, Set supportedHeaders, Set requiredHeaders, - Set recommendedHeaders, NoticeContainer noticeContainer); } diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/parsing/RowParserTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/parsing/RowParserTest.java index 260f3da2e2..c3c0db2cf7 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/parsing/RowParserTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/parsing/RowParserTest.java @@ -56,11 +56,6 @@ public boolean headerRequired() { return false; } - @Override - public boolean headerRecommended() { - return false; - } - @Override public FieldLevelEnum fieldLevel() { return FieldLevelEnum.REQUIRED; @@ -146,7 +141,6 @@ public void asString_recommended_missing() { GtfsColumnDescriptor.builder() .setColumnName("column name") .setHeaderRequired(false) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.RECOMMENDED) .setIsMixedCase(false) .setIsCached(false) diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/CsvTableLoaderTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/CsvTableLoaderTest.java index 93ea70f733..9a4387d38e 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/table/CsvTableLoaderTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/table/CsvTableLoaderTest.java @@ -88,7 +88,6 @@ public void validate( CsvHeader actualHeader, Set supportedHeaders, Set requiredHeaders, - Set recommendedHeaders, NoticeContainer noticeContainer) { noticeContainer.addValidationNotice(headerValidationNotice); } @@ -148,7 +147,6 @@ public void missingRequiredField() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.ID_FIELD_NAME) .setHeaderRequired(true) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.REQUIRED) .setIsMixedCase(false) .setIsCached(false) @@ -156,7 +154,6 @@ public void missingRequiredField() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.CODE_FIELD_NAME) .setHeaderRequired(false) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.REQUIRED) .setIsMixedCase(false) .setIsCached(false) diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor.java index 2da7a405a9..1b7d352cf1 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor.java @@ -40,7 +40,6 @@ public ImmutableList getColumns() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.ID_FIELD_NAME) .setHeaderRequired(true) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.REQUIRED) .setIsMixedCase(false) .setIsCached(false) @@ -49,7 +48,6 @@ public ImmutableList getColumns() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.CODE_FIELD_NAME) .setHeaderRequired(false) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.OPTIONAL) .setIsMixedCase(false) .setIsCached(false) diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java index a3896d7237..24246951c8 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/testgtfs/GtfsTestTableDescriptor2.java @@ -41,7 +41,6 @@ public ImmutableList getColumns() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.ID_FIELD_NAME) .setHeaderRequired(true) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.REQUIRED) .setIsMixedCase(false) .setIsCached(false) @@ -50,7 +49,6 @@ public ImmutableList getColumns() { GtfsColumnDescriptor.builder() .setColumnName(GtfsTestEntity.CODE_FIELD_NAME) .setHeaderRequired(false) - .setHeaderRecommended(false) .setFieldLevel(FieldLevelEnum.OPTIONAL) .setIsMixedCase(false) .setIsCached(false) diff --git a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidatorTest.java b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidatorTest.java index 1946ecd024..003c707c12 100644 --- a/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidatorTest.java +++ b/core/src/test/java/org/mobilitydata/gtfsvalidator/validator/TableHeaderValidatorTest.java @@ -19,13 +19,11 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableSet; -import java.util.Set; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mobilitydata.gtfsvalidator.notice.DuplicatedColumnNotice; import org.mobilitydata.gtfsvalidator.notice.EmptyColumnNameNotice; -import org.mobilitydata.gtfsvalidator.notice.MissingRecommendedColumnNotice; import org.mobilitydata.gtfsvalidator.notice.MissingRequiredColumnNotice; import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; import org.mobilitydata.gtfsvalidator.notice.UnknownColumnNotice; @@ -42,7 +40,6 @@ public void expectedColumns() { new CsvHeader(new String[] {"stop_id", "stop_name"}), ImmutableSet.of("stop_id", "stop_name", "stop_lat", "stop_lon"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()).isEmpty(); @@ -58,7 +55,6 @@ public void unknownColumnShouldGenerateNotice() { new CsvHeader(new String[] {"stop_id", "stop_name", "stop_extra"}), ImmutableSet.of("stop_id", "stop_name"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()) @@ -75,29 +71,12 @@ public void missingRequiredColumnShouldGenerateNotice() { new CsvHeader(new String[] {"stop_name"}), ImmutableSet.of("stop_id", "stop_name"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()) .containsExactly(new MissingRequiredColumnNotice("stops.txt", "stop_id")); } - @Test - public void missingRecommendedColumnShouldGenerateNotice() { - NoticeContainer container = new NoticeContainer(); - new DefaultTableHeaderValidator() - .validate( - "stops.txt", - new CsvHeader(new String[] {"stop_name"}), - Set.of("stop_id", "stop_name"), - Set.of(), - Set.of("stop_id"), - container); - - assertThat(container.getValidationNotices()) - .containsExactly(new MissingRecommendedColumnNotice("stops.txt", "stop_id")); - } - @Test public void duplicatedColumnShouldGenerateNotice() { NoticeContainer container = new NoticeContainer(); @@ -107,7 +86,6 @@ public void duplicatedColumnShouldGenerateNotice() { new CsvHeader(new String[] {"stop_id", "stop_name", "stop_id"}), ImmutableSet.of("stop_id", "stop_name"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()) @@ -124,7 +102,6 @@ public void emptyFileShouldNotGenerateNotice() { CsvHeader.EMPTY, ImmutableSet.of("stop_id", "stop_name"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()).isEmpty(); @@ -140,7 +117,6 @@ public void emptyColumnNameShouldGenerateNotice() { new CsvHeader(new String[] {"stop_id", null, "stop_name", ""}), ImmutableSet.of("stop_id", "stop_name"), ImmutableSet.of("stop_id"), - Set.of(), container); assertThat(container.getValidationNotices()) diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeatureMetadata.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeatureMetadata.java index dc190746f2..47721e8f76 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeatureMetadata.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeatureMetadata.java @@ -16,10 +16,6 @@ public String getFeatureName() { return featureName; } - public String getFeatureGroup() { - return featureGroup; - } - public String getDocUrl() { String formattedFeatureName = featureName.toLowerCase().replace(' ', '-'); String formattedFeatureGroup = featureGroup.toLowerCase().replace(' ', '_'); diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java index 60411d7d37..aa65b8e09c 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/report/model/FeedMetadata.java @@ -618,14 +618,4 @@ public void setFilenames(ImmutableSortedSet filenames) { public ImmutableSortedSet getFilenames() { return filenames; } - - public Boolean hasFlexFeatures() { - return specFeatures.keySet().stream() - .anyMatch( - feature -> - feature.getFeatureGroup() != null - && feature.getFeatureGroup().equals("Flexible Services") - && !Objects.equals(feature.getFeatureName(), "Continuous Stops") - && specFeatures.get(feature)); - } } diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java index 8549006143..1567aa634e 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoader.java @@ -181,6 +181,8 @@ public GtfsGeoJsonFeature extractFeature( String type = geometry.get(GtfsGeoJsonFeature.GEOMETRY_TYPE_FIELD_NAME).getAsString(); + validateCoordinates(noticeContainer, featureIndex, geometry, gtfsGeoJsonFeature); + if (type.equals(GeometryType.POLYGON.getType())) { gtfsGeoJsonFeature.setGeometryType(GeometryType.POLYGON); Polygon polygon = @@ -229,6 +231,40 @@ public GtfsGeoJsonFeature extractFeature( return null; } + private static void validateCoordinates( + NoticeContainer noticeContainer, + int featureIndex, + JsonObject geometry, + GtfsGeoJsonFeature gtfsGeoJsonFeature) { + // Validate that the coordinates are not near the origin or the poles + JsonArray coordinates = + geometry.getAsJsonArray(GtfsGeoJsonFeature.GEOMETRY_COORDINATES_FIELD_NAME); + for (int i = 0; i < coordinates.size(); i++) { + for (int j = 0; j < coordinates.get(i).getAsJsonArray().size(); j++) { + JsonArray point = coordinates.get(i).getAsJsonArray().get(j).getAsJsonArray(); + double lon = point.get(0).getAsDouble(); + double lat = point.get(1).getAsDouble(); + if (Math.abs(lon) <= 1 && Math.abs(lat) <= 1) { + noticeContainer.addValidationNotice( + new PointNearOriginNotice( + GtfsGeoJsonFeature.FILENAME, + gtfsGeoJsonFeature.featureId(), + lat, + lon, + featureIndex)); + } else if (Math.abs(lat) >= 89) { + noticeContainer.addValidationNotice( + new PointNearPoleNotice( + GtfsGeoJsonFeature.FILENAME, + gtfsGeoJsonFeature.featureId(), + lat, + lon, + featureIndex)); + } + } + } + } + private static void addMissingRequiredFieldsNotices( List missingRequiredFields, NoticeContainer noticeContainer, diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java index 9715cd0b5e..c79d918d19 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidator.java @@ -54,6 +54,9 @@ public void validate(NoticeContainer noticeContainer) { CalendarUtil.servicePeriodToServiceDatesMap( CalendarUtil.buildServicePeriodMap(calendarTable, calendarDateTable)); boolean isCalendarTableEmpty = calendarTable.getEntities().isEmpty(); + List noticesToReturn = new ArrayList<>(); + // Notices to return only if there are no calendars in `calendar.txt` and + // all calendar dates are in the past. List expiredCalendarDatesNotices = new ArrayList<>(); boolean allCalendarAreExpired = true; for (var serviceId : servicePeriodMap.keySet()) { @@ -63,7 +66,7 @@ public void validate(NoticeContainer noticeContainer) { } if (serviceDates.last().isBefore(dateForValidation.getDate())) { if (calendarTable.byServiceId(serviceId).isPresent()) { - noticeContainer.addValidationNotice( + noticesToReturn.add( new ExpiredCalendarNotice( calendarTable.byServiceId(serviceId).get().csvRowNumber(), serviceId)); } else if (isCalendarTableEmpty && allCalendarAreExpired) { @@ -82,8 +85,11 @@ public void validate(NoticeContainer noticeContainer) { } } if (isCalendarTableEmpty && allCalendarAreExpired) { - noticeContainer.addValidationNotices(expiredCalendarDatesNotices); + noticesToReturn.addAll(expiredCalendarDatesNotices); } + + noticesToReturn.sort(Comparator.comparing(ExpiredCalendarNotice::getCsvRowNumber)); + noticeContainer.addValidationNotices(noticesToReturn); } /** @@ -111,5 +117,9 @@ static class ExpiredCalendarNotice extends ValidationNotice { this.csvRowNumber = csvRowNumber; this.serviceId = serviceId; } + + public int getCsvRowNumber() { + return csvRowNumber; + } } } diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java index a7c9d796a6..30e864c95b 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidator.java @@ -40,7 +40,7 @@ * *
    *
  • {@link WrongParentLocationTypeNotice}. - *
  • {@link UnusedParentStationNotice}. + *
  • {@link UnusedStationNotice}. *
*/ @GtfsValidator @@ -114,7 +114,7 @@ public void validate(NoticeContainer noticeContainer) { } GtfsStop stationStop = optionalStationStop.get(); noticeContainer.addValidationNotice( - new UnusedParentStationNotice( + new UnusedStationNotice( stationStop.csvRowNumber(), stationStop.stopId(), stationStop.stopName())); } } @@ -190,12 +190,12 @@ static class WrongParentLocationTypeNotice extends ValidationNotice { } /** - * Unused parent station. + * Unused station. * *

A stop has `location_type` STATION (1) but does not appear in any stop's `parent_station`. */ @GtfsValidationNotice(severity = INFO, files = @FileRefs({GtfsStopSchema.class})) - static class UnusedParentStationNotice extends ValidationNotice { + static class UnusedStationNotice extends ValidationNotice { /** The row number of the faulty record. */ private final int csvRowNumber; @@ -205,7 +205,7 @@ static class UnusedParentStationNotice extends ValidationNotice { /** The name of the faulty stop. */ private final String stopName; - UnusedParentStationNotice(int csvRowNumber, String stopId, String stopName) { + UnusedStationNotice(int csvRowNumber, String stopId, String stopName) { this.csvRowNumber = csvRowNumber; this.stopId = stopId; this.stopName = stopName; diff --git a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidator.java b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidator.java index bcca748636..63aca742b1 100644 --- a/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidator.java +++ b/main/src/main/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidator.java @@ -116,8 +116,8 @@ static class StopTimeTimepointWithoutTimesNotice extends ValidationNotice { /** * `stop_times.timepoint` value is missing for a record. * - *

Even though the column `timepoint` is optional in `stop_times.txt` according to the - * specification, `stop_times.timepoint` should not be empty when provided. + *

When at least one of `stop_times.arrival_time` or `stop_times.departure_time` are provided, + * `stop_times.timepoint` should be defined */ @GtfsValidationNotice(severity = WARNING, files = @FileRefs(GtfsStopTimeSchema.class)) static class MissingTimepointValueNotice extends ValidationNotice { diff --git a/main/src/main/resources/report.html b/main/src/main/resources/report.html index 78c73bba71..813ef72028 100644 --- a/main/src/main/resources/report.html +++ b/main/src/main/resources/report.html @@ -221,14 +221,6 @@ align-content: center; flex-wrap: wrap; } - - .warning-display { - padding: 10px; - border: solid 2px orange; - border-radius: 11px; - width: fit-content; - font-weight: bold; - } @@ -247,13 +239,6 @@

GTFS Schedule Validation Report

Use this report alongside our documentation.

-
-

- ⚠ This feed contains GTFS Flex features. Please note that GTFS Flex validation support is still in development. - You can manually review all the validation rules for Flex data here. -

-
-

A new version of the Canonical GTFS Schedule validator is available! Please update to get the latest/best validation results.

diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoaderTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoaderTest.java index 286dc34484..e8e57d860a 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoaderTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/table/GeoJsonFileLoaderTest.java @@ -12,9 +12,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mobilitydata.gtfsvalidator.notice.DuplicateGeoJsonKeyNotice; -import org.mobilitydata.gtfsvalidator.notice.InvalidGeometryNotice; -import org.mobilitydata.gtfsvalidator.notice.NoticeContainer; +import org.mobilitydata.gtfsvalidator.notice.*; /** Runs GeoJsonFileLoader on test json data. */ @RunWith(JUnit4.class) @@ -23,6 +21,7 @@ public class GeoJsonFileLoaderTest { static String validGeoJsonData; static String invalidPolygonGeoJsonData; static String duplicateGeoJsonKeyData; + static String invalidPointsGeoJsonData; NoticeContainer noticeContainer; @BeforeClass @@ -138,10 +137,54 @@ public static void setUpBeforeClass() { " ]", "}"); + invalidPointsGeoJsonData = + String.join( + "\n", + "{", + " 'type': 'FeatureCollection',", + " 'features': [", + " {", + " 'id': 'id1',", + " 'type': 'Feature',", + " 'geometry': {", + " 'type': 'Polygon',", + " 'coordinates': [", + " [", + " [0.0, 0.0],", + " [101.0, 0.0],", + " [101.0, 1.0],", + " [100.0, 1.0],", + " [100.0, 0.0]", + " ]", + " ]", + " },", + " 'properties': {}", + " },", + " {", + " 'id': 'id2',", + " 'type': 'Feature',", + " 'geometry': {", + " 'type': 'Polygon',", + " 'coordinates': [", + " [", + " [200.0, 0.0],", + " [201.0, 0.0],", + " [201.0, 2.0],", + " [200.0, -89.75],", + " [200.0, 0.0]", + " ]", + " ]", + " },", + " 'properties': {}", + " }", + " ]", + "}"); + // Replace single quotes with double quotes for JSON compliance validGeoJsonData = validGeoJsonData.replace("'", "\""); invalidPolygonGeoJsonData = invalidPolygonGeoJsonData.replace("'", "\""); duplicateGeoJsonKeyData = duplicateGeoJsonKeyData.replace("'", "\""); + invalidPointsGeoJsonData = invalidPointsGeoJsonData.replace("'", "\""); } @Before @@ -200,6 +243,26 @@ public void testDuplicateGeoJsonKeyNotice() { assertThat(notices.size()).isGreaterThan(0); } + @Test + public void testPointNearOriginAndPole() { + // Testing for invalid points near origin and pole + createLoader(invalidPointsGeoJsonData); + + // Check if the correct validation notice is generated for the invalid points + List originNotices = + noticeContainer.getValidationNotices().stream() + .filter(PointNearOriginNotice.class::isInstance) + .map(PointNearOriginNotice.class::cast) + .collect(Collectors.toList()); + List poleNotices = + noticeContainer.getValidationNotices().stream() + .filter(PointNearPoleNotice.class::isInstance) + .map(PointNearPoleNotice.class::cast) + .collect(Collectors.toList()); + assertThat(originNotices.size()).isGreaterThan(0); + assertThat(poleNotices.size()).isGreaterThan(0); + } + private GtfsEntityContainer createLoader(String jsonData) { GeoJsonFileLoader loader = new GeoJsonFileLoader(); var fileDescriptor = new GtfsGeoJsonFileDescriptor(); diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidatorTest.java index 2ad7a1fe0c..61e1386be3 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ExpiredCalendarValidatorTest.java @@ -316,6 +316,12 @@ public void calendarDateWithAllCalendarDatesExpiredShouldGenerateNotice() { .setDate(GtfsDate.fromLocalDate(TEST_NOW.minusDays(3))) .setExceptionType(GtfsCalendarDateExceptionType.SERVICE_ADDED) .build(), + new GtfsCalendarDate.Builder() + .setCsvRowNumber(4) + .setServiceId("SERVICE_ID_4") + .setDate(GtfsDate.fromLocalDate(TEST_NOW.minusDays(7))) + .setExceptionType(GtfsCalendarDateExceptionType.SERVICE_ADDED) + .build(), new GtfsCalendarDate.Builder() .setCsvRowNumber(1) .setServiceId("SERVICE_ID_1") @@ -326,9 +332,12 @@ public void calendarDateWithAllCalendarDatesExpiredShouldGenerateNotice() { new ExpiredCalendarValidator(new DateForValidation(TEST_NOW), calendarTable, calendarDateTable) .validate(container); assertThat(container.getValidationNotices()) + // We verify that entries are sorted by row number. .containsExactly( - new ExpiredCalendarValidator.ExpiredCalendarNotice(3, "SERVICE_ID_3"), + new ExpiredCalendarValidator.ExpiredCalendarNotice(1, "SERVICE_ID_1"), new ExpiredCalendarValidator.ExpiredCalendarNotice(2, "SERVICE_ID_2"), - new ExpiredCalendarValidator.ExpiredCalendarNotice(1, "SERVICE_ID_1")); + new ExpiredCalendarValidator.ExpiredCalendarNotice(3, "SERVICE_ID_3"), + new ExpiredCalendarValidator.ExpiredCalendarNotice(4, "SERVICE_ID_4")) + .inOrder(); } } diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java index 9543d859d8..f2144f1639 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/ParentStationValidatorTest.java @@ -28,7 +28,7 @@ import org.mobilitydata.gtfsvalidator.table.GtfsLocationType; import org.mobilitydata.gtfsvalidator.table.GtfsStop; import org.mobilitydata.gtfsvalidator.table.GtfsStopTableContainer; -import org.mobilitydata.gtfsvalidator.validator.ParentStationValidator.UnusedParentStationNotice; +import org.mobilitydata.gtfsvalidator.validator.ParentStationValidator.UnusedStationNotice; import org.mobilitydata.gtfsvalidator.validator.ParentStationValidator.WrongParentLocationTypeNotice; @RunWith(JUnit4.class) @@ -94,7 +94,7 @@ public void stopParent() { public void entranceParent() { assertThat(validateChildAndParent(GtfsLocationType.ENTRANCE, GtfsLocationType.STATION)) // No issue with the types, but the station has no stop (just an entrance). - .containsExactly(new UnusedParentStationNotice(2, "parent", "Parent location")); + .containsExactly(new UnusedStationNotice(2, "parent", "Parent location")); assertThat(validateChildAndParent(GtfsLocationType.ENTRANCE, GtfsLocationType.STOP)) .containsExactly( new WrongParentLocationTypeNotice( @@ -113,7 +113,7 @@ public void entranceParent() { public void genericNodeParent() { assertThat(validateChildAndParent(GtfsLocationType.GENERIC_NODE, GtfsLocationType.STATION)) // No issue with the types, but the station has no stop. - .containsExactly(new UnusedParentStationNotice(2, "parent", "Parent location")); + .containsExactly(new UnusedStationNotice(2, "parent", "Parent location")); assertThat(validateChildAndParent(GtfsLocationType.GENERIC_NODE, GtfsLocationType.STOP)) .containsExactly( new WrongParentLocationTypeNotice( @@ -145,7 +145,7 @@ public void boardingAreaParent() { "Parent location", GtfsLocationType.STATION.getNumber(), GtfsLocationType.STOP.getNumber()), - new UnusedParentStationNotice(2, "parent", "Parent location"))); + new UnusedStationNotice(2, "parent", "Parent location"))); } @Test @@ -185,7 +185,7 @@ public void unusedStation() { noticeContainer)) .validate(noticeContainer); assertThat(noticeContainer.getValidationNotices()) - .containsExactly(new UnusedParentStationNotice(2, "unused_station", "Unused station")); + .containsExactly(new UnusedStationNotice(2, "unused_station", "Unused station")); } @Test diff --git a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidatorTest.java b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidatorTest.java index dcdab56f47..55d62c1c10 100644 --- a/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidatorTest.java +++ b/main/src/test/java/org/mobilitydata/gtfsvalidator/validator/TimepointTimeValidatorTest.java @@ -60,25 +60,6 @@ private static List generateNotices( return noticeContainer.getValidationNotices(); } - // Using this header will trigger a MissingRecommendedColumnNotice since the timepoint column is - // missing. - private static CsvHeader createLegacyHeader() { - return new CsvHeader( - new String[] { - TRIP_ID_FIELD_NAME, - ARRIVAL_TIME_FIELD_NAME, - DEPARTURE_TIME_FIELD_NAME, - STOP_ID_FIELD_NAME, - STOP_SEQUENCE_FIELD_NAME, - STOP_HEADSIGN_FIELD_NAME, - PICKUP_TYPE_FIELD_NAME, - DROP_OFF_TYPE_FIELD_NAME, - CONTINUOUS_PICKUP_FIELD_NAME, - CONTINUOUS_DROP_OFF_FIELD_NAME, - SHAPE_DIST_TRAVELED_FIELD_NAME - }); - } - private static CsvHeader createHeaderWithTimepointColumn() { return new CsvHeader( new String[] { diff --git a/model/src/main/java/org/mobilitydata/gtfsvalidator/annotation/RecommendedColumn.java b/model/src/main/java/org/mobilitydata/gtfsvalidator/annotation/RecommendedColumn.java deleted file mode 100644 index f292ebd22d..0000000000 --- a/model/src/main/java/org/mobilitydata/gtfsvalidator/annotation/RecommendedColumn.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2020 Google LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.mobilitydata.gtfsvalidator.annotation; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Adds a validation that it's recommended that a column be present. A value for the field may be - * optional. - * - *

Example. - * - * 

- *   {@literal @}GtfsTable("stop_times.txt")
- *    public interface GtfsStopTimeSchema extends GtfsEntity {
- *
- *     {@literal @}DefaultValue("1")
- *     {@literal @}RecommendedColumn
- *      GtfsStopTimeTimepoint timepoint();
- *   }
- *
- */ -@Target({ElementType.METHOD, ElementType.TYPE}) -@Retention(RetentionPolicy.SOURCE) -public @interface RecommendedColumn {} diff --git a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/Analyser.java b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/Analyser.java index 0a15d5577b..f2d52b825c 100644 --- a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/Analyser.java +++ b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/Analyser.java @@ -71,7 +71,6 @@ public GtfsFileDescriptor analyzeGtfsFileType(TypeElement type) { fieldBuilder.setRecommended(method.getAnnotation(Recommended.class) != null); fieldBuilder.setColumnRequired(method.getAnnotation(RequiredColumn.class) != null); fieldBuilder.setValueRequired(method.getAnnotation(Required.class) != null); - fieldBuilder.setColumnRecommended(method.getAnnotation(RecommendedColumn.class) != null); fieldBuilder.setMixedCase(method.getAnnotation(MixedCase.class) != null); PrimaryKey primaryKey = method.getAnnotation(PrimaryKey.class); if (primaryKey != null) { diff --git a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/GtfsFieldDescriptor.java b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/GtfsFieldDescriptor.java index c5c6374fac..d94b62c8ef 100644 --- a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/GtfsFieldDescriptor.java +++ b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/GtfsFieldDescriptor.java @@ -59,8 +59,6 @@ public static GtfsFieldDescriptor.Builder builder() { public abstract boolean columnRequired(); - public abstract boolean columnRecommended(); - public boolean isHeaderRequired() { return columnRequired() || valueRequired(); } @@ -83,8 +81,6 @@ public abstract static class Builder { public abstract Builder setColumnRequired(boolean value); - public abstract Builder setColumnRecommended(boolean value); - public abstract Builder setMixedCase(boolean value); public abstract Builder setPrimaryKey(PrimaryKey annotation); diff --git a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/TableDescriptorGenerator.java b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/TableDescriptorGenerator.java index 62b01ac216..d9edc405e5 100644 --- a/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/TableDescriptorGenerator.java +++ b/processor/src/main/java/org/mobilitydata/gtfsvalidator/processor/TableDescriptorGenerator.java @@ -195,14 +195,12 @@ private MethodSpec generateGetColumnsMethod() { "GtfsColumnDescriptor.builder()\n" + ".setColumnName($T.$L)\n" + ".setHeaderRequired($L)\n" - + ".setHeaderRecommended($L)\n" + ".setFieldLevel($T.$L)\n" + ".setIsMixedCase($L)\n" + ".setIsCached($L)\n", gtfsEntityType, fieldNameField(field.name()), field.isHeaderRequired(), - field.columnRecommended(), FieldLevelEnum.class, getFieldLevel(field), field.mixedCase(), diff --git a/processor/tests/src/main/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchema.java b/processor/tests/src/main/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchema.java deleted file mode 100644 index dff043ff42..0000000000 --- a/processor/tests/src/main/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchema.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.mobilitydata.gtfsvalidator.processor.tests; - -import org.mobilitydata.gtfsvalidator.annotation.GtfsTable; -import org.mobilitydata.gtfsvalidator.annotation.RecommendedColumn; - -@GtfsTable("recommended_column.txt") -public interface RecommendedColumnAnnotationSchema { - - @RecommendedColumn - String columnRecommended(); -} diff --git a/processor/tests/src/test/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchemaTest.java b/processor/tests/src/test/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchemaTest.java deleted file mode 100644 index fd600103e2..0000000000 --- a/processor/tests/src/test/java/org/mobilitydata/gtfsvalidator/processor/tests/RecommendedColumnAnnotationSchemaTest.java +++ /dev/null @@ -1,51 +0,0 @@ -package org.mobilitydata.gtfsvalidator.processor.tests; - -import static com.google.common.truth.Truth.assertThat; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mobilitydata.gtfsvalidator.notice.MissingRequiredColumnNotice; -import org.mobilitydata.gtfsvalidator.table.RecommendedColumnAnnotationTableDescriptor; -import org.mobilitydata.gtfsvalidator.testing.LoadingHelper; -import org.mobilitydata.gtfsvalidator.validator.ValidatorLoaderException; - -@RunWith(JUnit4.class) -public class RecommendedColumnAnnotationSchemaTest { - private RecommendedColumnAnnotationTableDescriptor tableDescriptor; - private LoadingHelper helper; - - @Before - public void setup() throws ValidatorLoaderException { - tableDescriptor = new RecommendedColumnAnnotationTableDescriptor(); - helper = new LoadingHelper(); - } - - @Test - public void includingRecommendedColumnHeaderWithoutValueShouldNotGenerateNotice() - throws ValidatorLoaderException { - - helper.load(tableDescriptor, "some_column,column_recommended", "value,"); - - assertThat( - !helper - .getValidationNotices() - .contains( - new MissingRequiredColumnNotice("recommended_column.txt", "column_recommended"))); - } - - @Test - public void missingRecommendedColumnHeaderShouldGenerateNotice() throws ValidatorLoaderException { - - helper.load(tableDescriptor, "column", "value"); - // Since we use an unknown column ("column") we have to expect at least one unknown_column - // notice along with the - // missing_recommended_column notice. - assertThat( - helper - .getValidationNotices() - .contains( - new MissingRequiredColumnNotice("recommended_column.txt", "column_recommended"))); - } -}