fix: Drop DuplicateFareRuleZoneIdFieldsValidator and replace with multi-column @PrimaryKey for fare_rules.txt#1297
Conversation
…ecified multi-column @PrimaryKey for fare_rules.txt
|
❌ Invalid acceptance test. |
|
Thanks for working on this @bdferris-v2! |
isabelle-dr
left a comment
There was a problem hiding this comment.
Can you remove the outdated rule from the documentation, please?
Then we are good to merge this PR :)
|
RULES.md has been updated. |
isabelle-dr
left a comment
There was a problem hiding this comment.
Thanks! I think we are good to merge without an additional code review here.
|
❌ Invalid acceptance test. |
Closes #1296.
Per discussion in #1296,
DuplicateFareRuleZoneIdFieldsValidatorsays the combination ofroute_id,origin_id,destination_id, andcontains_idmust be unique infare_rules.txt, when the spec saysfare_idshould be included (aka the primary key is all fields). This PR drops the custom validator in favor of proper @PrimaryKey annotations forfare_rules.txt.Per the acceptance tests, there are a number of feeds (~60) that trigger the existing
duplicate_fare_rule_zone_id_fieldsnotice. 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_keynotice. These are all feeds that really due have a duplicate combination of all fields infare_rules.txt. However, each of these feeds were already triggering aduplicate_fare_rule_zone_id_fieldsnotice, so there are no newly-invalidated feeds from this change.gradle testto make sure you didn't break anything