feat: Initial support for Fares v2 validation.

[bdferris-v2]

Per issue #1201, adds validation for files and fields added for Fares v2 base spec changes.

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

[bdferris-v2]

Initial draft, mostly to see what happens when validating a few existing feeds.

[isabelle-dr]

Just a question: if we have a negative integer in this field, will this trigger the invalid_integer notice?

[bdferris-v2]

I can confirm it triggers at number_out_of_range validation error.

[bdferris-v2]

Now that #1248 has been committed and integrated, this PR is ready for official review.

[KClough]

Not an official reviewer, but lgtm.

[isabelle-dr]

Thanks, @bdferris-v2 🔥
Two things:

1. We are missing documentation for invalid_fare_transfer_rule_transfer_count and fare_transfer_rule_missing_transfer_count.

2. I would like to include three additional rules in the scope of this PR.

• missing_fare_rule_file
  • spec mention in Dataset files: fare_rules.txt is required if fare_attributes.txt is defined.
  • pseudo code: if fare_attributes.txt if present and fare_rules.txt is absent, trigger the notice.
• forbidden_fare_rule_file
  • spec mention in Dataset files: fare_rules.txt is forbidden if fare_attributes.txt is not defined.
  • pseudo code: if fare_attributes.txt if absent and fare_rules.txt is present, trigger the notice.
• fare_transfer_rule_forbidden_duration_limit
  • spec mention in fare_transfer_rules.duration_limit_type: Forbidden if fare_transfer_rules.duration_limit is empty.
  • pseudo code: If [fare_transfer_rules.duration_limit == '' and fare_transfer_rules.duration_limit_type != ''], trigger the notice

I have a copy of the fares v2 spec changes with my comments in https://bit.ly/faresv2-validator.

[maximearmstrong]

fare_transfer_rule_forbidden_duration_limit

• spec mention in fare_transfer_rules.duration_limit_type: Forbidden if fare_transfer_rules.duration_limit is empty.
• pseudo code: If [fare_transfer_rules.duration_limit == '' and fare_transfer_rules.duration_limit_type != ''], trigger the notice

@isabelle-dr I think this has been already added with the FareTransferRuleDurationLimitWithoutTypeNotice here. Could you confirm this is what you were looking for?

[maximearmstrong]

Great work! I've only one small comment. I think after Isabelle's requested changes are done, we'll be ready to merge. Thanks @bdferris-v2

[bdferris-v2]

I believe @isabelle-dr was correct that I didn't have a validation check for when duration_limit_type was specified but duration_limit was not. I've added that now.

I believe all validation rules have documentation in RULES.md at this point. Let me know if you see things differently.

As for fare_rules.txt and fare_attributes.txt, those seem to be clarifications of the Fares V1 spec. I'm happy to add support for that, but I'd argue for splitting it out into a separate PR to keep things (relatively) focused. Thoughts?

[isabelle-dr]

Hello! Thanks for making this advance 🙏

For the documentation, I believe we are still missing the first rule you have in FareTransferCountValidator.java (line 47). It is invalid_fare_transfer_rule_transfer_count.

As for the two additional rules I mentioned here, I see fare_rules.txt was introduced in Fares v1, but it went from being optional to conditionally required/forbidden in the PR that added GTFS-Fares v2 base implementation on google/transit (reference).
If the scope of this PR is adding validation for the changes introduced in this PR, I would say it's in scope.

[isabelle-dr]

Looks great!