From 72498055357157f3913a380b55f36441b6b4a7bb Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 24 Jan 2025 10:12:05 -0800 Subject: [PATCH 1/4] Strict nullability with `: Any`, and made `RxBoxImp` a bit more efficient. --- .../com/diffplug/common/rx/ForwardingBox.kt | 10 ++-- .../com/diffplug/common/rx/MappedImp.java | 58 ------------------- .../java/com/diffplug/common/rx/MappedImp.kt | 43 ++++++++++++++ .../diffplug/common/rx/MultiSelectModel.kt | 8 ++- src/main/java/com/diffplug/common/rx/Rx.kt | 4 +- src/main/java/com/diffplug/common/rx/RxBox.kt | 36 +++++------- .../java/com/diffplug/common/rx/RxBoxImp.kt | 18 ++---- .../java/com/diffplug/common/rx/RxGetter.kt | 8 +-- .../java/com/diffplug/common/rx/RxLockBox.kt | 17 ++---- .../com/diffplug/common/rx/RxLockBoxImp.kt | 28 ++++----- 10 files changed, 96 insertions(+), 134 deletions(-) delete mode 100644 src/main/java/com/diffplug/common/rx/MappedImp.java create mode 100644 src/main/java/com/diffplug/common/rx/MappedImp.kt diff --git a/src/main/java/com/diffplug/common/rx/ForwardingBox.kt b/src/main/java/com/diffplug/common/rx/ForwardingBox.kt index bafedbc..2de5a7b 100644 --- a/src/main/java/com/diffplug/common/rx/ForwardingBox.kt +++ b/src/main/java/com/diffplug/common/rx/ForwardingBox.kt @@ -27,7 +27,7 @@ import kotlinx.coroutines.flow.Flow * * Especially useful for overridding set(). */ -open class ForwardingBox> +open class ForwardingBox> protected constructor(protected val delegate: BoxType) : Box { override fun get(): T { return delegate.get() @@ -37,7 +37,7 @@ protected constructor(protected val delegate: BoxType) : Box { delegate.set(value) } - class Cas protected constructor(delegate: CasBox) : + class Cas protected constructor(delegate: CasBox) : ForwardingBox>(delegate), CasBox { override fun compareAndSet(expect: T, update: T): Boolean { return delegate.compareAndSet(expect, update) @@ -48,21 +48,21 @@ protected constructor(protected val delegate: BoxType) : Box { } } - class Lock protected constructor(delegate: LockBox) : + class Lock protected constructor(delegate: LockBox) : ForwardingBox>(delegate), LockBox { override fun lock(): Any { return delegate.lock() } } - open class Rx protected constructor(delegate: RxBox) : + open class Rx protected constructor(delegate: RxBox) : ForwardingBox>(delegate), RxBox { override fun asFlow(): Flow { return delegate.asFlow() } } - class RxLock protected constructor(delegate: RxLockBox) : + class RxLock protected constructor(delegate: RxLockBox) : ForwardingBox>(delegate), RxLockBox { override fun lock(): Any { return delegate.lock() diff --git a/src/main/java/com/diffplug/common/rx/MappedImp.java b/src/main/java/com/diffplug/common/rx/MappedImp.java deleted file mode 100644 index 30c8c80..0000000 --- a/src/main/java/com/diffplug/common/rx/MappedImp.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2020 DiffPlug - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.diffplug.common.rx; - - -import com.diffplug.common.base.Box; -import com.diffplug.common.base.Converter; -import java.util.function.Function; - -class MappedImp> implements Box { - protected final BoxType delegate; - protected final Converter converter; - - public MappedImp(BoxType delegate, Converter converter) { - this.delegate = delegate; - this.converter = converter; - } - - @Override - public R get() { - return converter.convertNonNull(delegate.get()); - } - - @Override - public void set(R value) { - delegate.set(converter.revertNonNull(value)); - } - - /** Shortcut for doing a set() on the result of a get(). */ - @Override - public R modify(Function mutator) { - Box.Nullable result = Box.Nullable.ofNull(); - delegate.modify(input -> { - R unmappedResult = mutator.apply(converter.convertNonNull(input)); - result.set(unmappedResult); - return converter.revertNonNull(unmappedResult); - }); - return result.get(); - } - - @Override - public String toString() { - return "[" + delegate + " mapped to " + get() + " by " + converter + "]"; - } -} diff --git a/src/main/java/com/diffplug/common/rx/MappedImp.kt b/src/main/java/com/diffplug/common/rx/MappedImp.kt new file mode 100644 index 0000000..b32cccb --- /dev/null +++ b/src/main/java/com/diffplug/common/rx/MappedImp.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2020 DiffPlug + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.diffplug.common.rx + +import com.diffplug.common.base.Box +import com.diffplug.common.base.Converter +import java.util.function.Function + +internal open class MappedImp>( + @JvmField protected val delegate: BoxType, + @JvmField protected val converter: Converter +) : Box { + override fun get(): R = converter.convertNonNull(delegate.get()) + + override fun set(value: R) = delegate.set(converter.revertNonNull(value)) + + /** Shortcut for doing a set() on the result of a get(). */ + override fun modify(mutator: Function): R { + val result = Box.Nullable.ofNull() + delegate.modify { input: T -> + val unmappedResult = mutator.apply(converter.convertNonNull(input)) + result.set(unmappedResult) + converter.revertNonNull(unmappedResult) + } + return result.get() + } + + override fun toString(): String = + "[" + delegate + " mapped to " + get() + " by " + converter + "]" +} diff --git a/src/main/java/com/diffplug/common/rx/MultiSelectModel.kt b/src/main/java/com/diffplug/common/rx/MultiSelectModel.kt index cc41a73..267d3b8 100644 --- a/src/main/java/com/diffplug/common/rx/MultiSelectModel.kt +++ b/src/main/java/com/diffplug/common/rx/MultiSelectModel.kt @@ -171,10 +171,14 @@ class MultiSelectModel( companion object { /** Creates an Optional from an Either. */ - fun optEitherFrom(either: Either, Optional>): Optional> { + fun optEitherFrom( + either: Either, Optional> + ): Optional> { return either.fold({ leftOpt: Optional -> leftOpt.map { l: T -> Either.createLeft(l) } - }) { rightOpt: Optional -> rightOpt.map { r: U -> Either.createRight(r) } } + }) { rightOpt: Optional -> + rightOpt.map { r: U -> Either.createRight(r) } + } } } } diff --git a/src/main/java/com/diffplug/common/rx/Rx.kt b/src/main/java/com/diffplug/common/rx/Rx.kt index c6248a3..85406aa 100644 --- a/src/main/java/com/diffplug/common/rx/Rx.kt +++ b/src/main/java/com/diffplug/common/rx/Rx.kt @@ -371,7 +371,7 @@ object Rx { /** Reliable way to sync two RxBox to each other. */ @JvmStatic - fun sync(left: RxBox, right: RxBox) { + fun sync(left: RxBox, right: RxBox) { sync(sameThreadExecutor(), left, right) } @@ -380,7 +380,7 @@ object Rx { * changes */ @JvmStatic - fun sync(subscriber: RxSubscriber, left: RxBox, right: RxBox) { + fun sync(subscriber: RxSubscriber, left: RxBox, right: RxBox) { val firstChange = Box.Nullable.ofNull?>() subscriber.subscribe(left) { leftVal: T -> // the left changed before we could acknowledge it diff --git a/src/main/java/com/diffplug/common/rx/RxBox.kt b/src/main/java/com/diffplug/common/rx/RxBox.kt index fdd143d..2c6e74a 100644 --- a/src/main/java/com/diffplug/common/rx/RxBox.kt +++ b/src/main/java/com/diffplug/common/rx/RxBox.kt @@ -24,14 +24,12 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map /** [RxGetter] and [Box] combined in one: a value you can set, get, and subscribe to. */ -interface RxBox : RxGetter, Box { +interface RxBox : RxGetter, Box { /** Returns a read-only version of this `RxBox`. */ - fun readOnly(): RxGetter { - return this - } + fun readOnly(): RxGetter = this /** Maps one `RxBox` to another `RxBox`. */ - override fun map(converter: Converter): RxBox { + override fun map(converter: Converter): RxBox { return RxBoxImp.Mapped(this, converter) } @@ -70,30 +68,26 @@ interface RxBox : RxGetter, Box { companion object { /** Creates an `RxBox` with the given initial value. */ - @JvmStatic - fun of(initial: T): RxBox { - return RxBoxImp(initial) - } + @JvmStatic fun of(initial: T): RxBox = RxBoxImp(initial) /** * Creates an `RxBox` which implements the "getter" part with `RxGetter`, and the setter part * with `Consumer`. */ @JvmStatic - fun from(getter: RxGetter, setter: Consumer): RxBox { - return object : RxBox { - override fun asFlow(): Flow { - return getter.asFlow() - } + fun from(getter: RxGetter, setter: Consumer): RxBox = + object : RxBox { + override fun asFlow(): Flow { + return getter.asFlow() + } - override fun get(): T { - return getter.get() - } + override fun get(): T { + return getter.get() + } - override fun set(value: T) { - setter.accept(value) + override fun set(value: T) { + setter.accept(value) + } } - } - } } } diff --git a/src/main/java/com/diffplug/common/rx/RxBoxImp.kt b/src/main/java/com/diffplug/common/rx/RxBoxImp.kt index a50661d..8ab0aba 100644 --- a/src/main/java/com/diffplug/common/rx/RxBoxImp.kt +++ b/src/main/java/com/diffplug/common/rx/RxBoxImp.kt @@ -21,28 +21,22 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map -internal open class RxBoxImp private constructor(initial: T, subject: MutableStateFlow) : - RxBox { - private var value: T = initial - private val subject: MutableStateFlow = subject - - constructor(initial: T) : this(initial, MutableStateFlow(initial)) {} +internal open class RxBoxImp(initial: T) : RxBox { + private val subject = MutableStateFlow(initial) override fun set(newValue: T) { - if (newValue != value) { - value = newValue + if (subject.value != newValue) { subject.value = newValue } } - override fun get(): T = value + override fun get(): T = subject.value override fun asFlow(): Flow = subject - internal class Mapped(delegate: RxBox, converter: Converter) : + internal class Mapped(delegate: RxBox, converter: Converter) : MappedImp>(delegate, converter), RxBox { - val flow: Flow = - delegate.asFlow().map { a: T -> converter.convertNonNull(a) }.distinctUntilChanged() + val flow: Flow = delegate.asFlow().map(converter::convertNonNull).distinctUntilChanged() override fun asFlow(): Flow = flow } diff --git a/src/main/java/com/diffplug/common/rx/RxGetter.kt b/src/main/java/com/diffplug/common/rx/RxGetter.kt index aba1c79..961fada 100644 --- a/src/main/java/com/diffplug/common/rx/RxGetter.kt +++ b/src/main/java/com/diffplug/common/rx/RxGetter.kt @@ -36,7 +36,7 @@ import kotlinx.coroutines.flow.map * not change (e.g. a field is set to its current value, which produces no change) then the * `Observable` will not fire. */ -interface RxGetter : IFlowable, Supplier { +interface RxGetter : IFlowable, Supplier { /** * Maps an `RxGetter` to a new `RxGetter` by applying the `mapper` function to all of its values. * @@ -46,7 +46,7 @@ interface RxGetter : IFlowable, Supplier { * * Incorrect: `("A", "B", "C") -> map(String::length) = (1, 1, 1)` * * Correct: `("A", "B", "C") -> map(String::length) = (1)` */ - fun map(mapper: Function): RxGetter { + fun map(mapper: Function): RxGetter { val src = this val mapped = src.asFlow().map { t: T -> mapper.apply(t) } val observable = mapped.distinctUntilChanged() @@ -70,7 +70,7 @@ interface RxGetter : IFlowable, Supplier { * recorded by a non-volatile field. */ @JvmStatic - fun from(observable: Flow, initialValue: T): RxGetter { + fun from(observable: Flow, initialValue: T): RxGetter { val box = Box.of(initialValue) subscribe(observable) { value: T -> box.set(value) } return object : RxGetter { @@ -90,7 +90,7 @@ interface RxGetter : IFlowable, Supplier { * As with [.map], the observable only emits a new value if its value has changed. */ @JvmStatic - fun combineLatest( + fun combineLatest( t: RxGetter, u: RxGetter, combine: BiFunction diff --git a/src/main/java/com/diffplug/common/rx/RxLockBox.kt b/src/main/java/com/diffplug/common/rx/RxLockBox.kt index 7a67464..453fc3c 100644 --- a/src/main/java/com/diffplug/common/rx/RxLockBox.kt +++ b/src/main/java/com/diffplug/common/rx/RxLockBox.kt @@ -22,11 +22,10 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map /** [RxBox] and [LockBox] in one. */ -interface RxLockBox : LockBox, RxBox { +interface RxLockBox : LockBox, RxBox { /** RxLockBox must map to another kind of LockBox. */ - override fun map(converter: Converter): RxLockBox { - return RxLockBoxImp.Mapped(this, converter) - } + override fun map(converter: Converter): RxLockBox = + RxLockBoxImp.Mapped(this, converter) override fun enforce(enforcer: Function): RxLockBox { // this must be a plain-old observable, because it needs to fire @@ -40,15 +39,9 @@ interface RxLockBox : LockBox, RxBox { companion object { /** Creates an `RxLockBox` containing the given value, which uses itself as the lock. */ - @JvmStatic - fun of(value: T): RxLockBox { - return RxLockBoxImp(value) - } + @JvmStatic fun of(value: T): RxLockBox = RxLockBoxImp(value) /** Creates an `RxLockBox` containing the given value, which uses `lock` as the lock. */ - @JvmStatic - fun of(value: T, lock: Any): RxLockBox { - return RxLockBoxImp(value, lock) - } + @JvmStatic fun of(value: T, lock: Any): RxLockBox = RxLockBoxImp(value, lock) } } diff --git a/src/main/java/com/diffplug/common/rx/RxLockBoxImp.kt b/src/main/java/com/diffplug/common/rx/RxLockBoxImp.kt index 29aa3d8..8e3f50a 100644 --- a/src/main/java/com/diffplug/common/rx/RxLockBoxImp.kt +++ b/src/main/java/com/diffplug/common/rx/RxLockBoxImp.kt @@ -23,35 +23,31 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map -internal class RxLockBoxImp : LockBoxImp, RxLockBox { - val subject: MutableStateFlow +internal class RxLockBoxImp : LockBoxImp, RxLockBox { + val flow: MutableStateFlow constructor(value: T) : super(value) { - subject = MutableStateFlow(value) + flow = MutableStateFlow(value) } constructor(value: T, lock: Any) : super(value, lock) { - subject = MutableStateFlow(value) + flow = MutableStateFlow(value) } override fun set(newValue: T) { synchronized(lock()) { if (newValue != value) { value = newValue - subject.value = newValue + flow.value = newValue } } } - override fun asFlow(): Flow { - return subject - } + override fun asFlow(): Flow = flow - override fun toString(): String { - return "RxLockBox.of[" + get() + "]" - } + override fun toString(): String = "RxLockBox.of[" + get() + "]" - internal class Mapped(delegate: RxLockBox, converter: Converter) : + internal class Mapped(delegate: RxLockBox, converter: Converter) : MappedImp>(delegate, converter), RxLockBox { val flow: Flow @@ -60,13 +56,9 @@ internal class RxLockBoxImp : LockBoxImp, RxLockBox { flow = mapped.distinctUntilChanged() } - override fun lock(): Any { - return delegate.lock() - } + override fun lock(): Any = delegate.lock() - override fun asFlow(): Flow { - return flow - } + override fun asFlow(): Flow = flow override fun modify(mutator: Function): R { val result = Box.Nullable.ofNull() From 4d468deedb0005e9b815394aea7321ee61fac5c0 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 24 Jan 2025 11:08:19 -0800 Subject: [PATCH 2/4] Use `@JvmStatic` where appropriate. --- src/main/java/com/diffplug/common/rx/Rx.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/diffplug/common/rx/Rx.kt b/src/main/java/com/diffplug/common/rx/Rx.kt index 85406aa..54cc4d5 100644 --- a/src/main/java/com/diffplug/common/rx/Rx.kt +++ b/src/main/java/com/diffplug/common/rx/Rx.kt @@ -100,6 +100,7 @@ import kotlinx.coroutines.flow.merge * (https://diffplug.github.io/durian-swt/javadoc/snapshot/com/diffplug/common/swt/SwtExec.html) */ object Rx { + @JvmStatic fun createEmitFlow() = MutableSharedFlow(replay = 0, extraBufferCapacity = 1, BufferOverflow.SUSPEND) @@ -132,6 +133,7 @@ object Rx { * Creates an Rx instance which will call the given consumer whenever the followed stream or * future completes, whether with an error or not, and the error (if present) will be logged. */ + @JvmStatic fun onTerminateLogError(onTerminate: Consumer>): RxListener { return RxListener(Consumers.doNothing(), DefaultTerminate(onTerminate)) } @@ -449,5 +451,5 @@ object Rx { } } - val sentinelJob: Job = Job().apply { cancel() } + @JvmStatic val sentinelJob: Job = Job().apply { cancel() } } From e0705103f29a769a7ea532bc4c813b67046a9560 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 24 Jan 2025 11:09:44 -0800 Subject: [PATCH 3/4] Update changelog. --- CHANGES.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index af57d2a..79f198b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,8 +2,9 @@ ## [Unreleased] ### Changed -- Bump required java from 11 to 17. ([#9](https://github.com/diffplug/durian-rx/pull/9)) +- Add strict nullability to RxBox and improve efficiency. ([#12](https://github.com/diffplug/durian-rx/pull/12)) - Replace `RxJava Disposable` with `Kotlin Job`, and remove `rxjava` completely. ([#10](https://github.com/diffplug/durian-rx/pull/10)) +- Bump required java from 11 to 17. ([#9](https://github.com/diffplug/durian-rx/pull/9)) ## [4.0.1] - 2022-12-20 ### Fixed From 96218656cc8a5cf7f8c8ae427902a15bba030f15 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Fri, 24 Jan 2025 11:20:46 -0800 Subject: [PATCH 4/4] Use Kotlin features --- CHANGES.md | 2 +- src/main/java/com/diffplug/common/rx/GuardedExecutor.kt | 6 ++---- src/main/java/com/diffplug/common/rx/RxExecutor.kt | 5 +---- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 79f198b..8d72196 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,8 +2,8 @@ ## [Unreleased] ### Changed +- **BREAKING** Replace `RxJava Disposable` with `Kotlin Job`, and remove `rxjava` completely. ([#10](https://github.com/diffplug/durian-rx/pull/10)) - Add strict nullability to RxBox and improve efficiency. ([#12](https://github.com/diffplug/durian-rx/pull/12)) -- Replace `RxJava Disposable` with `Kotlin Job`, and remove `rxjava` completely. ([#10](https://github.com/diffplug/durian-rx/pull/10)) - Bump required java from 11 to 17. ([#9](https://github.com/diffplug/durian-rx/pull/9)) ## [4.0.1] - 2022-12-20 diff --git a/src/main/java/com/diffplug/common/rx/GuardedExecutor.kt b/src/main/java/com/diffplug/common/rx/GuardedExecutor.kt index 6040d20..d0592b1 100644 --- a/src/main/java/com/diffplug/common/rx/GuardedExecutor.kt +++ b/src/main/java/com/diffplug/common/rx/GuardedExecutor.kt @@ -22,7 +22,6 @@ import java.util.concurrent.Executor import java.util.function.Supplier import kotlinx.coroutines.Deferred import kotlinx.coroutines.Job -import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.Flow /** @@ -33,12 +32,11 @@ import kotlinx.coroutines.flow.Flow */ open class GuardedExecutor(val delegate: RxExecutor, val guard: Chit) : Executor, RxSubscriber { override fun execute(command: Runnable) { - delegate.executor().execute(guard.guard(command)) + delegate.executor.execute(guard.guard(command)) } /** Creates a runnable which runs on this Executor iff the guard widget is not disposed. */ fun wrap(delegate: Runnable): Runnable { - Objects.requireNonNull(delegate) return Runnable { execute(guard.guard(delegate)) } } @@ -48,7 +46,7 @@ open class GuardedExecutor(val delegate: RxExecutor, val guard: Chit) : Executor guard.runWhenDisposed { job.cancel() } job } else { - SupervisorJob().apply { cancel() } + Rx.sentinelJob } } diff --git a/src/main/java/com/diffplug/common/rx/RxExecutor.kt b/src/main/java/com/diffplug/common/rx/RxExecutor.kt index 004951e..e77880a 100644 --- a/src/main/java/com/diffplug/common/rx/RxExecutor.kt +++ b/src/main/java/com/diffplug/common/rx/RxExecutor.kt @@ -33,16 +33,13 @@ import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch -class RxExecutor -internal constructor(private val executor: Executor, val dispatcher: CoroutineDispatcher) : +class RxExecutor internal constructor(val executor: Executor, val dispatcher: CoroutineDispatcher) : RxSubscriber { interface Has : Executor { val rxExecutor: RxExecutor } - fun executor() = executor - override fun subscribe(flow: Flow, listener: RxListener) { subscribeDisposable(flow, listener) }