Skip to content

Bridge Dagger and Jersey, inject TransitService #5442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 7 commits into
base: dev-2.x
Choose a base branch
from

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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant