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

Support for FHIR Subscription mechanism #1289

Open
chgl opened this issue Jun 26, 2020 · 3 comments
Open

Support for FHIR Subscription mechanism #1289

chgl opened this issue Jun 26, 2020 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@chgl
Copy link
Contributor

chgl commented Jun 26, 2020

Is your feature request related to a problem? Please describe.
Currently, the server does not emit notifications based on any FHIR Subscription resources, instead using the static configuration in fhir-server-config.json.

Describe the solution you'd like
Support for the FHIR Subscription mechanism.

This would allow for configuring notification endpoints dynamically. Additionally, individual endpoints could be configured based on FHIR Search criteria increasing the flexibility and possible use cases of notifications.

Describe alternatives you've considered
It's always possible to use the existing notification system and apply any resource filtering in custom receivers.

@prb112 prb112 added enhancement New feature or request help wanted Extra attention is needed labels Jun 26, 2020
@lmsurpre
Copy link
Member

Thanks @chgl, did you happen to try our fhir-notification-websockets support?
I think that one obeys the Subscription mechanism but we disable it by default due tenant-privacy concerns in multi-tenant deploys and significant limitations in multi-server environments.
You are right that the kafka and nats notifiers are based solely on static config.

Its also worth noting that the Subscription mechanism in the FHIR spec is getting heavily reworked for R5 and so our current thinking is to let that shake out before updating this component.

@chgl
Copy link
Contributor Author

chgl commented Jun 26, 2020

I wasn't aware of the significant rework from R4 to R5 (just checked, looks very exciting 😄), makes sense to wait! For my particular use case I have an existing service which creates a basic webhook subscription, but reworking that for WebSockets shouldn't be too challenging. Thanks for the tip!

lmsurpre added a commit that referenced this issue Sep 20, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 20, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 21, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 21, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 21, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 21, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Sep 21, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 18, 2021
1. Introduce beforeInvoke and afterInvoke to the
FHIRPersistenceInterceptor interface. I wanted these to take the
FHIROperationContext which is in fhir-server, so that lead to:

2. Move the FHIRPersistenceInterceptor (and related classes) from
fhir-persistence to fhir-server (where they are called from).
Unfortunately, fhir-notification implements FHIRPersistenceInterceptor,
but fhir-server depends on fhir-notification. I tried breaking this
dependency but it was difficult because fhir-server currently
initializes the websocket notification impl differently than the kafka
and nats ones. So that lead to:

3. Move fhir-notification and all 3 implementations into fhir-server. At
first I didn't like this, but this was the easiest way out of the
dependency mess. If we want to break them back out, we should introduce
a proper notification spi. This should get revisited when we get to
#1289

4. Finally, I made the change I actually wanted to make, which is to add
support for `system/[resourceType].[read|write]` scopes in fhir-smart
and check these scopes before we allow requests to `$export`, `$import`,
and `$bulkdata-status`.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@jordanperrin
Copy link

Do I have to use the FHIRNotification package in order to have the websocket messages send? or can I just create the Subscriptions resource and the server itself will recognize the resource and send messages based on the Subscription resource, I thought just creating the Subscription resource would be sufficient to send messages over websockets based on the FHIR docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants