False positives for duplicate_fare_rule_zone_id_fields in fare_rules.txt

[isabelle-dr]

Describe the bug

This was flagged by SDMTS.

False positives for the duplicate_fare_rule_zone_id_fields rule.
They only have (fare_id, route_id) fields defined, every row has a unique combination of both.
The spec mentions that the primary key for fare_rules.txt is (*) which means: all fields (reference)

They had duplicate_fare_rule_zone_id_fields triggered (which is an ERROR) and it looks like a problem with the way this rule is designed.

Steps/Code to Reproduce

• download dataset provided
• run validator
• open report

Expected Results

No issue with fare rules

Actual Results

The notice duplicate_fare_rule_zone_id_fields is triggered 296 times.

Screenshots

No response

Files used

https://www.sdmts.com/google_transit_files/google_transit.zip

Validator version

4.0.0

Operating system

MacOS

Java version

No response

Additional notes

Now that we support multiple @PrimaryKey annotations, we don't need to have the duplicate_fare_rule_zone_id_fields rule in this validator at all.
We can use the @PrimaryKey annotation in fare_rules.txt directly, and the notice duplicate_key will be triggered if we have duplicate records.

[bdferris-v2]

Generally agreed that this validation rule doesn't seem to match the spec. Interestingly enough, I believe Google disables this check in our instance of the validator, probably for the same reason.

Per the acceptance tests, there are a number of feeds (~60) that trigger the existing duplicate_fare_rule_zone_id_fields notice. Spot checking a few feeds, many of them are indeed specifying the same combination of fare rules with different fare ids. The corresponding fare attributes often have a different price, probably indicating that the producer is attempting to model some other aspect of their fare system (different fare products? different rider categories?) despite the fact that this is not really supported in Fares v1. While it's maybe not obvious how feed consumers should productively use this data, other than showing the cheapest fare, the spec technically allows it, so the validator should as well.

The acceptance test shows that a number of feeds (~10) newly trigger a duplicate_key notice. These are all feeds that really due have a duplicate combination of all fields in fare_rules.txt. However, each of these feeds was already triggering a duplicate_fare_rule_zone_id_fields notice, so there are no newly-invalidated feeds from this change.

All in all, +1 to making a change to validation here.