Skip to content

Conversation

@t2gran
Copy link
Member

@t2gran t2gran commented Oct 20, 2023

Summary

This PR is a POK on how to BRIDGE Dagger and Jersey. The ideal would be to replace Jersey DI with Dagger, but bridging is much simpler. The only code needed to do this is in the JerseyToDaggerBridge, the rest of the changes is to proof that it works by injecting the TransitService.

In the current design the OtpServerRequestContext is responsible for providing a consistent set of Services for a Request coped Resource. I think we can merge this PR, but before we continue injecting other services directly into Resources we need to make sure we can provide a consistent set of RequestScopedServices.

Also, I do NOT want to inject anything else then Services into the Resources, so if resource AbcResource is using the e.g. RouterConfig then a AbcResourceService class should be created. Dagger should then wire the AbcResourceService and Jersey should inject AbcResourceService into AbcResource.

Unit tests

Fixed.

Documentation

TODO: I would like to document the layers and injection strategy:

Changelog

Not relevant

Bumping the serialization version id

No.

@t2gran t2gran added the !Technical Debt Improve code quality, no functional changes. label Oct 20, 2023
@t2gran t2gran added this to the 2.5 (next release) milestone Oct 20, 2023
@t2gran t2gran requested a review from a team as a code owner October 20, 2023 11:09
@t2gran t2gran marked this pull request as draft October 20, 2023 11:09
@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Attention: 85 lines in your changes are missing coverage. Please review.

Comparison is base (4e4af20) 67.50% compared to head (40aea58) 66.76%.
Report is 6 commits behind head on dev-2.x.

❗ Current head 40aea58 differs from pull request most recent head 103e08e. Consider uploading reports for the commit 103e08e to get more accurate results

Files Patch % Lines
.../main/java/org/opentripplanner/index/IndexAPI.java 0.00% 32 Missing ⚠️
...opentripplanner/ext/geocoder/GeocoderResource.java 0.00% 9 Missing ⚠️
...lanner/standalone/server/JerseyToDaggerBridge.java 0.00% 6 Missing ⚠️
...ipplanner/ext/vectortiles/VectorTilesResource.java 0.00% 5 Missing ⚠️
...planner/ext/reportapi/resource/ReportResource.java 0.00% 4 Missing ⚠️
...api/resource/GraphInspectorVectorTileResource.java 0.00% 4 Missing ⚠️
.../org/opentripplanner/apis/gtfs/GtfsGraphQLAPI.java 0.00% 4 Missing ⚠️
...ipplanner/standalone/server/OTPWebApplication.java 0.00% 4 Missing ⚠️
...ripplanner/api/resource/UpdaterStatusResource.java 0.00% 3 Missing ⚠️
...java/org/opentripplanner/api/resource/Routers.java 0.00% 2 Missing ⚠️
... and 10 more
Additional details and impacted files
@@              Coverage Diff              @@
##             dev-2.x    #5442      +/-   ##
=============================================
- Coverage      67.50%   66.76%   -0.75%     
+ Complexity     16285    15446     -839     
=============================================
  Files           1888     1799      -89     
  Lines          71547    69795    -1752     
  Branches        7408     7354      -54     
=============================================
- Hits           48301    46600    -1701     
+ Misses         20747    20733      -14     
+ Partials        2499     2462      -37     

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

@leonardehrenfried leonardehrenfried changed the title Bridge Dagger and Jersy, inject TransitService Bridge Dagger and Jersey, inject TransitService Oct 20, 2023
@t2gran t2gran modified the milestones: 2.5, 2.6 (next release) Mar 13, 2024
@t2gran t2gran modified the milestones: 2.6, 2.7 (next release) Sep 18, 2024
@t2gran t2gran modified the milestones: 2.7, 2.8 (next release) Mar 12, 2025
@t2gran t2gran modified the milestones: 2.8, 2.9 (next release) Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

!Technical Debt Improve code quality, no functional changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant