Remove fare service from Graph

[leonardehrenfried]

Summary

This removes the fare service from the Graph instance and lets Dagger manage it instead.

Unit tests

A few unit tests had to be adjusted.

Bumping the serialization version id

✔️

[codecov]

Codecov Report

Attention: Patch coverage is 64.28571% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.30%. Comparing base (62b4982) to head (3f9ba8d).

Files with missing lines	Patch %	Lines
...n/java/org/opentripplanner/standalone/OTPMain.java	0.00%	2 Missing ⚠️
...entripplanner/apis/gtfs/GraphQLRequestContext.java	0.00%	1 Missing ⚠️
...ner/standalone/configure/ConstructApplication.java	0.00%	1 Missing ⚠️
...pplanner/standalone/configure/LoadApplication.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6292      +/-   ##
=============================================
- Coverage      70.31%   70.30%   -0.01%     
+ Complexity     18158    18151       -7     
=============================================
  Files           2058     2058              
  Lines          76939    76939              
  Branches        7771     7771              
=============================================
- Hits           54097    54091       -6     
- Misses         20088    20092       +4     
- Partials        2754     2756       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[habrahamsson-skanetrafiken]

Why do we create the fareService here instead of in the loadApp.appConstruction() like the other ones?

Doesn't this mean that we will always get a DefaultFareService during graph build? And then overwrite that DefaultFareService with whatever this factory returns?

And if you run with --build --serve you will always use the DefaultFareService from the loadApp.constructApplication(). Or is it just me that don't understand how this hangs together?

[leonardehrenfried]

Since this PR is so old I had to get into it again: the reason why it is so strange is that the fareServiceFactory acts a builder gathering up fares data from all input feeds. It actually lives in the build config and is life-cycle managed and serialized through that.

No doubt this is terrible. It should really be part of the OtpTransitServiceBuilder which fulfills the same role.

I would like to request to not make this PR any bigger and let me revisit this topic.

[habrahamsson-skanetrafiken]

But i think the second issue will still be a problem. If you run otp with both --build --serve then dagger won't know about the real fare service but will only have a reference to the dummy fare service from LoadApplication.java:85. Right?

[leonardehrenfried]

I tested this yesterday and it still works in this case. That is because the fareServiceFactory is stored inside and lifecycle-managed by buildConfig (!!!).

[tkalvas]

This PR appears to break the fare zone display on HSL's frontends. I'll give some more details in a bit.

[leonardehrenfried]

If I build a HSL graph and then use the following GraphiQL URL I can see some fares: https://tinyurl.com/26c2dcvg

[tkalvas]

For some reason clicking on that image gives me a blank page, and it is really hard to read it as it is.

[leonardehrenfried]

The image URLs have a limited validity. You need to refresh the page to get a new one.

[tkalvas]

Anyway, that's not the query function our frontend uses. My test:

query FareQuery {
  planConnection(
    after: "2025-02-13T09:30:26+02:00",
    origin: {
      location: {
        coordinate: {
          latitude: 60.2227651,
          longitude: 24.9399948120
        }
      }
    },
    destination: {
      location: {
        coordinate: {
          latitude: 60.1841232676,
          longitude: 24.7808647155
        }
      }
    }
  ) {
    edges {
      node {
        legs {
          mode
          route {
            shortName
          }
          fareProducts {
            product {
              id
            }
          }
        }
      }
    }
  }
}

[tkalvas]

I don't get any fare products with your query either. I can send you a new HSL-gtfs.zip if you want, I guess something has changed.

[leonardehrenfried]

I get them:

I use the very latest feed from https://dev.hsl.fi/gtfs/hsl.zip

Is that the correct URL?

[tkalvas]

It's definitely not the same data we use in production, but I'll have to ask Joel about the data pipeline now.

[tkalvas]

I still don't know what that zip is, but the trips2.txt in it is our trips.txt, and the trips.txt in it is something we don't have. Also the feed_info.txt are slightly different, we have a feed_id added:

% cat feed_info.txt 
feed_publisher_name,feed_publisher_url,feed_lang,feed_start_date,feed_end_date,feed_version,feed_id
Helsingin seudun liikenne,http://www.hsl.fi/,fi,20250212,20250412,2025-02-12 23:53:09,HSL

[tkalvas]

Our build-config.json:

{
  "dataImportReport": true,
  "areaVisibility": true,
  "staticParkAndRide": false,
  "subwayAccessTime": 0,
  "maxAreaNodes": 1000,
  "maxTransferDuration": "26m",
  "multiThreadElevationCalculations": true,
  "transitServiceStart": "-P2W",
  "transitServiceEnd": "P12W",
  "transitModelTimeZone": "Europe/Helsinki",
  "fares": "hsl",
  "transferRequests": [
    { "modes": "WALK" },
    {
      "modes": "WALK",
      "wheelchairAccessibility": {
        "enabled": true,
        "maxSlope": 0.125
      }
    }
  ],
  "gtfsDefaults": {
    "stationTransferPreference": "recommended"
  },
  "osmDefaults": {
    "timeZone": "Europe/Helsinki",
    "osmTagMapping": "finland",
    "includeOsmSubwayEntrances": true
  },
  "demDefaults": {
    "elevationUnitMultiplier": 0.1
  },
  "emissions": {
    "carAvgCo2PerKm": 170,
    "carAvgOccupancy": 1.3
  }
}

I can't actually even get your version of the gtfs dump to load, because the feed id needs to be set in the config when it's not in the gtfs... that might be a relevant difference.

[tkalvas]

Just for fun I printed to a debug log the type of fareService in DecorateWithFare and got

DecorateWithFare fare service type org.opentripplanner.ext.fares.impl.HSLFareService
DecorateWithFare fare service type org.opentripplanner.ext.fares.impl.HSLFareService
DecorateWithFare fare service type org.opentripplanner.ext.fares.impl.HSLFareService
DecorateWithFare fare service type org.opentripplanner.ext.fares.impl.DefaultFareService

The three first ones seem to come from startup, afterwards when I do a new query, I get just one DefaultFareService. That is surely not right.

[leonardehrenfried]

Okay, thanks for investigating. I might have made a mistake here. I will make this a draft.