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.

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

fix: don't throw exception on calendar with no days #1519

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

Whoops
Copy link
Contributor

@Whoops Whoops commented Jun 28, 2023

Summary:

Fix ExpiredCalendarValidator throwing NoSuchElementException when checking a calendar with no days set to 1.

Expected behavior:

Checking a calendar.txt file where a row has no days set to 1 does not crash the validator.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

@welcome
Copy link

welcome bot commented Jun 28, 2023

Thanks for opening this pull request! You're awesome. We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of titles with semantic prefixes:

  • fix: Bug with ssl network connections + Java module permissions.
  • feat: Initial support for multiple @PrimaryKey annotations.
  • docs: update RELEASE.md with new process
    To get this PR to the finish line, please do the following:
  • Read our Contribution Guidelines
  • Follow Google Java style coding standards
  • Include tests when adding/changing behavior
  • Include screenshots

@CLAassistant
Copy link

CLAassistant commented Jun 28, 2023

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Whoops
Copy link
Contributor Author

Whoops commented Jun 29, 2023

Chasing down a few of the changes in the acceptance tests, I think that's to be expected. For example looking running the current validator on https://storage.googleapis.com/storage/v1/b/mdb-latest/o/us-georgia-metropolitan-atlanta-rapid-transit-authority-marta-gtfs-368.zip?alt=media results in this error during the run:

SEVERE: Runtime exception in validator org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator
java.util.NoSuchElementException
	at java.base/java.util.TreeMap.key(TreeMap.java:1602)
	at java.base/java.util.TreeMap.lastKey(TreeMap.java:298)
	at java.base/java.util.TreeSet.last(TreeSet.java:398)
	at org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator.validate(ExpiredCalendarValidator.java:57)
	at org.mobilitydata.gtfsvalidator.validator.ValidatorUtil.safeValidate(ValidatorUtil.java:74)
	at org.mobilitydata.gtfsvalidator.table.GtfsFeedLoader.lambda$loadAndValidate$1(GtfsFeedLoader.java:149)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

and no expired calendar warnings. With this change there are no system errors during the run, but it now has two expired_calendar warnings. The warnings should've been displayed all along but were being suppressed by the validation crashing.

@davidgamez
Copy link
Member

Chasing down a few of the changes in the acceptance tests, I think that's to be expected. For example looking running the current validator on https://storage.googleapis.com/storage/v1/b/mdb-latest/o/us-georgia-metropolitan-atlanta-rapid-transit-authority-marta-gtfs-368.zip?alt=media results in this error during the run:

SEVERE: Runtime exception in validator org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator
java.util.NoSuchElementException
	at java.base/java.util.TreeMap.key(TreeMap.java:1602)
	at java.base/java.util.TreeMap.lastKey(TreeMap.java:298)
	at java.base/java.util.TreeSet.last(TreeSet.java:398)
	at org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator.validate(ExpiredCalendarValidator.java:57)
	at org.mobilitydata.gtfsvalidator.validator.ValidatorUtil.safeValidate(ValidatorUtil.java:74)
	at org.mobilitydata.gtfsvalidator.table.GtfsFeedLoader.lambda$loadAndValidate$1(GtfsFeedLoader.java:149)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
	at java.base/java.lang.Thread.run(Thread.java:833)

and no expired calendar warnings. With this change there are no system errors during the run, but it now has two expired_calendar warnings. The warnings should've been displayed all along but were being suppressed by the validation crashing.

Hi @Whoops, I did a spot check on the acceptance test reports, and all new warnings are as expected as they were omitted by the previous implementation due to the exception. Great contribution!

@davidgamez davidgamez merged commit 5f7e8a5 into MobilityData:master Jun 29, 2023
332 of 333 checks passed
@welcome
Copy link

welcome bot commented Jun 29, 2023

🥳 Congrats on getting your first pull request merged!

@Whoops Whoops deleted the calendar-exception branch June 29, 2023 17:46
@tafflin
Copy link

tafflin commented Jan 15, 2024

@davidgamez
I got this error:

SEVERE: Runtime exception in validator org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator
java.util.NoSuchElementException: No value present
	at java.base/java.util.Optional.get(Optional.java:148)
	at org.mobilitydata.gtfsvalidator.validator.ExpiredCalendarValidator.validate(ExpiredCalendarValidator.java:61)
	at org.mobilitydata.gtfsvalidator.validator.ValidatorUtil.safeValidate(ValidatorUtil.java:74)
	at org.mobilitydata.gtfsvalidator.table.GtfsFeedLoader.lambda$loadAndValidate$1(GtfsFeedLoader.java:161)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

Is it related somehow to the abovementioned fix? I don't see any calendars with all days set to "0" in my data.

@davidgamez
Copy link
Member

Hi @tafflin, I found a possible root cause. Can you share the feed that you are testing to be able to reproduce the same issue on my side? Thanks.

@tafflin
Copy link

tafflin commented Jan 16, 2024

@davidgamez We process multiple feeds and just recently I discovered that some of them have issues when validating. One of issues I mentioned can be found when processing this feed: https://gtfs.pro/files/uran/improved-gtfs-dft-gtfs.zip. It's 400+ mb.
You can see the validation result here https://gtfs.pro/files/derivatives/dft-gtfs/mobilitydata/improved-dft-gtfs-mobilitydata-report.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants