Skip to content
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

Evaluate requirements/feasibility of supporting custom endpoints #139

Closed
bashir2 opened this issue Apr 4, 2023 · 2 comments · Fixed by #182
Closed

Evaluate requirements/feasibility of supporting custom endpoints #139

bashir2 opened this issue Apr 4, 2023 · 2 comments · Fixed by #182
Assignees
Labels
enhancement New feature or request P1:must As issue that definitely needs to be implemented in near future.

Comments

@bashir2
Copy link
Collaborator

bashir2 commented Apr 4, 2023

The current architecture assumes that any request to the gateway is eventually passed to the FHIR server for returning a response. There are use-cases where we may want to support custom (non-FHIR) endpoints, e.g., in the context of sync as mentioned in #122. There are different ways to support this. We should first evaluate the requirements for this feature and make a conscious decision whether to support it. If the answer is yes, we should have a mini-design to close this issue.

@bashir2 bashir2 added enhancement New feature or request P3:TBE An issue that is not evaluated and/or planned for implementation yet. labels Apr 4, 2023
@vivekmittal07
Copy link
Collaborator

Current implementation intercepts all the requests and doesn’t pass the control to Spring native Rest controllers.
The interceptor assumes the incoming request is for a FHIR resource. It does the necessary access checks and then makes Http call the Fhir server.

  • Option1: Limit interceptor to FHIR resources
    We define the Servlet to intercept all the request at


    This can be changed to only intercept FHIR requests. One way to do that is add a "fhir" prefix to all the requests. eg - /fhir/Patient. We can configure the interceptor to handle all "fhir/*" requests. This will allow the clients to define custom endpoints using standard Spring components.
    Eg - Created this sample PR for testing this - [TEST] Custom endpoints support in FHIR gateway #166

  • Option2(Hard no) : Allow custom endpoints in the existing interceptor.
    We will need to support custom datafetcher in the existing interceptor. Currently FHIR client is responsible for fetching data and we don’t have any abstraction on datafetcher.
    This is outside the scope of the design of the Access checker framework. Supporting this will make the logic very complex and won't provide clean interfaces for access check.

We can use Option1 to support custom endpoints in Fhir Gateway.
We might have to do the following to support this -

  • Use dependency injection to create core components. This will allow custom endpoints implementation to inject the required components.
  • Move some interceptor core logic to utils for custom endpoints.

We will have to look at what is the best interface to support option1 but the current example provides the technical feasibility of supporting this feature.

@bashir2 bashir2 self-assigned this Aug 11, 2023
@bashir2 bashir2 added P1:must As issue that definitely needs to be implemented in near future. and removed P3:TBE An issue that is not evaluated and/or planned for implementation yet. labels Aug 17, 2023
@bashir2
Copy link
Collaborator Author

bashir2 commented Aug 18, 2023

Thanks @vivekmittal07 for your analysis and proposals. We basically went ahead with Option 1 but with the caveats that we need to provide good support for basic core functionality like JWT verification and FHIR-server access (for custom endpoint implementations). For these a bunch of refactoring was needed which are covered in PR #182.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P1:must As issue that definitely needs to be implemented in near future.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants