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

add reactive stream support #91

Closed
zigzago opened this issue Nov 7, 2018 · 6 comments
Closed

add reactive stream support #91

zigzago opened this issue Nov 7, 2018 · 6 comments
Milestone

Comments

@zigzago
Copy link
Member

zigzago commented Nov 7, 2018

https://mongodb.github.io/mongo-java-driver-reactivestreams/

as async driver is now deprecated

@zigzago zigzago added this to the 3.9.1 milestone Dec 22, 2018
@zigzago
Copy link
Member Author

zigzago commented Dec 30, 2018

target 3.9.2 because more impacts than expected

@zigzago zigzago modified the milestones: 3.9.1, 3.9.2 Dec 30, 2018
@jyemin
Copy link
Contributor

jyemin commented Dec 30, 2018

Can this be made transparent to applications via the coroutine support, or is the callback-based API exposed to applications?

@zigzago
Copy link
Member Author

zigzago commented Dec 30, 2018

KMongo supports 3 versions of async extensions:

  1. Callback style (kmongo-async)
  2. RxJava2 (kmongo-rxjava2)
  3. Coroutine (kmongo-coroutine)

My assumption is that for each version, it is doable to:

  1. async : Duplicate the extensions with reactivestreams base classes -> ok
  2. rxjava2 -> made it transparent -> seems ok
  3. coroutine -> made it transparent (using https://github.com/Kotlin/kotlinx.coroutines/blob/master/reactive/coroutines-guide-reactive.md and replacing current xIterable extensions with XPublisher extensions ) -> seems ok too

But I think I need more time to stabilize the whole thing. The new plan is to push the feature in a few days and release it at the end of January.

@BKaraargirov
Copy link

You have probably seen it, but Kotlin have released very good extension functions, to transform rxjava and reactive streams into coroutines:
https://github.com/Kotlin/kotlinx.coroutines/blob/master/reactive/coroutines-guide-reactive.md

I played around with the reactive stream one (https://github.com/Kotlin/kotlinx.coroutines/tree/master/reactive/kotlinx-coroutines-reactive) and it works pretty well.

I was going to try migrate the async driver and make pull request, but I haven't used Flapdoodle. And changing out the async driver hits it and some related custom abstractions. Unfortunately I don't have enough time right now to dig deeper

@zigzago
Copy link
Member Author

zigzago commented Jan 6, 2019

The feature is pushed. Async & rxjava2 modules looks good to me, but a little issue remains with coroutine.

This is because of the KMongo design, that use heavily extensions. The original idea was to add additional behavior to the java driver, and not to hide its methods. It is great because we avoid the problem that have all other frameworks: we don't need to duplicate all the new features of the mongo driver (and they advance rather quickly ;) )

But with coroutine, we change the behavior of existing methods. It was not a problem when extending the java async driver, as there was almost no signature clash. But with reactivestreams + coroutine, we have many signature clashes. For example, with the insertOne method:

Publisher<Success> insertOne(TDocument document)

We would like to write this extension method:

suspend fun <T> MongoCollection<T>.insertOne(document: T): Success 
 = insertOne(document).awaitSingle()

But as the signature is the same, the extension is shadowed. So the compiler will always use the java driver version. I have renamed the extensions using a 'AndAwait' suffix pattern, but this is quite error prone. I you write:

col.insertOneAndAwait(doc)

The insertion is applied. But it you write

col.insertOne(doc)

Nothing happens (of course).

I'm currently thinking of introducing a dedicated MongoCollection interface in the coroutine module, in order to avoid this kind of issue.

@phiSgr
Copy link

phiSgr commented Jan 9, 2019

I feel strongly that the right way out is to argue against the deprecation of the callback API.

zigzago added a commit that referenced this issue Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants