-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Kotlin coroutine update. #1135
Kotlin coroutine update. #1135
Conversation
As the Flow interface is not stable for inheritance in 3rd party libraries. Using AbstractFlow as recommended instead via the sealed MongoAbstractFlow class. JAVA-4950
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at the changes, and I don't have objections, which is expected, given that I don't know Kotlin. However, I also can't reasonably approve the PR, so I will stay a bystander.
Hi @zigzago - just wanted to double check you are happy with this PR |
If you look at the implementation of MongoAbstractFlow, it adds some behaviour public final override suspend fun collect(collector: FlowCollector<T>) {
val safeCollector = SafeCollector(collector, coroutineContext)
try {
collectSafely(safeCollector)
} finally {
safeCollector.releaseIntercepted()
}
} Also in the doc of AbstractFlow (https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.flow/-abstract-flow/): "Base class for stateful implementations of Flow". I'm not sure that mongo flows are stateful. I don't say your implementation will not work, but it would be cleaner for my point of view to use the kotlin delegate pattern: class AggregateFlow<T : Any>(private val wrapped: AggregatePublisher<T>) : Flow<T> by wrapped.asFlow() Then you rely completely on Publisher.asFlow() kotlin implementation (which is safe). HTH Julien |
Hi @vsct-jburet, Good points, they are stateful in that they contain the cursor state, but that is all managed by the
Ross |
This reverts commit 4655b4c.
As the Flow interface is not stable for inheritance in 3rd party libraries, use delegation instead. JAVA-4950
The
Flow
interface is not stable for inheritance in 3rd party libraries. Updated code to use theAbstractFlow
as recommended instead via the sealedMongoAbstractFlow
class.JAVA-4950