Skip to content

native subscription support for Kotlin Flow #358

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

Closed
dariuszkuc opened this issue Sep 20, 2019 · 8 comments · Fixed by #629
Closed

native subscription support for Kotlin Flow #358

dariuszkuc opened this issue Sep 20, 2019 · 8 comments · Fixed by #629
Labels
type: enhancement New feature or request

Comments

@dariuszkuc
Copy link
Collaborator

dariuszkuc commented Sep 20, 2019

Is your feature request related to a problem? Please describe.
As per graphql-java currently we only support subscription functions that return some Publisher (e.g. Reactor Flux). Since we support coroutines for queries/mutations we should also support Flow for subscriptions.

Describe the solution you'd like
Support Kotlin Flow type for subscriptions.

Describe alternatives you've considered
N/A

Additional context
N/A

@smyrick
Copy link
Contributor

smyrick commented Sep 20, 2019

@dariuszkuc graphql-java supports Publishers.
https://www.graphql-java.com/documentation/v13/subscriptions/

Flow can map to Publisher with Flow.asPublisher.

https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.flow/-flow/#

Do we want to have this mapping in the schema generator code or require schema developers to do this mapping for us?

@dariuszkuc
Copy link
Collaborator Author

@smyrick yes coroutines have nice interop with many different libs and definitely devs could do this conversion manually but the whole point of this issue is to support it natively. As a dev writing my graphql service I just want to be able to write it as

class MySubscription {
  fun foo(): Flow<Bar> { ... }
}

If possible we should do the mapping within the schema generator. Unsure if it will work though with current graphql-java model.

@smyrick
Copy link
Contributor

smyrick commented Sep 20, 2019

Tested locally, It does not work with graphql-java unless we change the type to Publisher so we can do this in the generator but I am not sure if that is any better that just supporting Publisher and having devs do that

@smyrick
Copy link
Contributor

smyrick commented Sep 25, 2019

@dariuszkuc Are we sure that we want to have this mapping in the library and not just support the types that graphql-java does (only publisher)

@dariuszkuc
Copy link
Collaborator Author

I think having native support would be great but yes until we move away from graphql-java this might not be feasible. Lets keep it open for now.

@josephlbarnett
Copy link
Contributor

Would it make sense to have graphql-kotllin provide a class that extends SubscriptionExecutionStrategy to treat Flows similar to how it currently treats Publishers? I think doing so to only support Flows would be somewhat straightforward port of the class, but probably want to be able to support either Flow or Publisher from the same strategy, which may be a little less straighforward to figure out?

@dariuszkuc
Copy link
Collaborator Author

Would it make sense to have graphql-kotllin provide a class that extends SubscriptionExecutionStrategy to treat Flows similar to how it currently treats Publishers? I think doing so to only support Flows would be somewhat straightforward port of the class, but probably want to be able to support either Flow or Publisher from the same strategy, which may be a little less straighforward to figure out?

Sounds like a good idea. Since Flow can be easily converted to reactor Flux this can be definitely done in custom execution strategy. Ideally both Flow and Publisher should be handled from single strategy but if it is not possible then having separate strategies should be fine as well.

josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Feb 28, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Feb 28, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
@josephlbarnett
Copy link
Contributor

note that due to issues like Kotlin/kotlinx.coroutines#1825 doing flow.asPublisher() in the schema function and then consuming the ExecutionResult's publisher .asFlow() can have unexpected side effects

josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 3, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 3, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
josephlbarnett added a commit to josephlbarnett/graphql-kotlin that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
smyrick pushed a commit that referenced this issue Mar 23, 2020
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes #358
dariuszkuc pushed a commit to dariuszkuc/graphql-kotlin that referenced this issue Aug 5, 2022
Implement a SubscriptionExecutionStrategy that allows
for `Flow`s and `Publisher`s to be returned from
graphql schema elements, and can be processed as a
`Flow` by subscription consumers.  Relax restrictions
that look for `Publisher`s to also allow `Flow`s.

Fixes ExpediaGroup#358
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

3 participants