Don't let Kryo crawl into Rx subscriptions, and add a unit test to ensure this doesn't regress, as the rule is not intuitive.

This commit is contained in:
Mike Hearn 2016-08-04 17:19:10 +02:00
parent 845f2bdd64
commit d61356ca9e
2 changed files with 37 additions and 10 deletions

View File

@ -80,8 +80,8 @@ class ProgressTracker(vararg steps: Step) {
// This field won't be serialized. // This field won't be serialized.
private val _changes by TransientProperty { PublishSubject.create<Change>() } private val _changes by TransientProperty { PublishSubject.create<Change>() }
private data class Child(val tracker: ProgressTracker, @Transient val subscription: Subscription?)
private val childProgressTrackers = HashMap<Step, Pair<ProgressTracker, Subscription>>() private val childProgressTrackers = HashMap<Step, Child>()
init { init {
steps.forEach { steps.forEach {
@ -130,19 +130,19 @@ class ProgressTracker(vararg steps: Step) {
val currentStepRecursive: Step val currentStepRecursive: Step
get() = getChildProgressTracker(currentStep)?.currentStepRecursive ?: currentStep get() = getChildProgressTracker(currentStep)?.currentStepRecursive ?: currentStep
fun getChildProgressTracker(step: Step): ProgressTracker? = childProgressTrackers[step]?.first fun getChildProgressTracker(step: Step): ProgressTracker? = childProgressTrackers[step]?.tracker
fun setChildProgressTracker(step: ProgressTracker.Step, childProgressTracker: ProgressTracker) { fun setChildProgressTracker(step: ProgressTracker.Step, childProgressTracker: ProgressTracker) {
val subscription = childProgressTracker.changes.subscribe({ _changes.onNext(it) }, { _changes.onError(it) }) val subscription = childProgressTracker.changes.subscribe({ _changes.onNext(it) }, { _changes.onError(it) })
childProgressTrackers[step] = Pair(childProgressTracker, subscription) childProgressTrackers[step] = Child(childProgressTracker, subscription)
childProgressTracker.parent = this childProgressTracker.parent = this
_changes.onNext(Change.Structural(this, step)) _changes.onNext(Change.Structural(this, step))
} }
private fun removeChildProgressTracker(step: ProgressTracker.Step) { private fun removeChildProgressTracker(step: ProgressTracker.Step) {
childProgressTrackers.remove(step)?.let { childProgressTrackers.remove(step)?.let {
it.first.parent = null it.tracker.parent = null
it.second.unsubscribe() it.subscription?.unsubscribe()
} }
_changes.onNext(Change.Structural(this, step)) _changes.onNext(Change.Structural(this, step))
} }
@ -192,7 +192,6 @@ class ProgressTracker(vararg steps: Step) {
* if a step changed its label or rendering). * if a step changed its label or rendering).
*/ */
val changes: Observable<Change> get() = _changes val changes: Observable<Change> get() = _changes
} }

View File

@ -1,5 +1,11 @@
package com.r3corda.core.utilities package com.r3corda.core.utilities
import com.esotericsoftware.kryo.Kryo
import com.esotericsoftware.kryo.KryoSerializable
import com.esotericsoftware.kryo.io.Input
import com.esotericsoftware.kryo.io.Output
import com.r3corda.core.serialization.createKryo
import com.r3corda.core.serialization.serialize
import org.junit.Before import org.junit.Before
import org.junit.Test import org.junit.Test
import java.util.* import java.util.*
@ -24,10 +30,12 @@ class ProgressTrackerTest {
} }
lateinit var pt: ProgressTracker lateinit var pt: ProgressTracker
lateinit var pt2: ProgressTracker
@Before @Before
fun before() { fun before() {
pt = SimpleSteps.tracker() pt = SimpleSteps.tracker()
pt2 = ChildSteps.tracker()
} }
@Test @Test
@ -56,8 +64,6 @@ class ProgressTrackerTest {
@Test @Test
fun `nested children are stepped correctly`() { fun `nested children are stepped correctly`() {
val pt2 = ChildSteps.tracker()
val stepNotification = LinkedList<ProgressTracker.Change>() val stepNotification = LinkedList<ProgressTracker.Change>()
pt.changes.subscribe { pt.changes.subscribe {
stepNotification += it stepNotification += it
@ -82,10 +88,32 @@ class ProgressTrackerTest {
@Test @Test
fun `can be rewound`() { fun `can be rewound`() {
val pt2 = ChildSteps.tracker()
pt.setChildProgressTracker(SimpleSteps.TWO, pt2) pt.setChildProgressTracker(SimpleSteps.TWO, pt2)
repeat(4) { pt.nextStep() } repeat(4) { pt.nextStep() }
pt.currentStep = SimpleSteps.ONE pt.currentStep = SimpleSteps.ONE
assertEquals(SimpleSteps.TWO, pt.nextStep()) assertEquals(SimpleSteps.TWO, pt.nextStep())
} }
@Test
fun rxSubscriptionsAreNotSerialized() {
class Unserializable : KryoSerializable {
override fun write(kryo: Kryo?, output: Output?) = throw AssertionError("not called")
override fun read(kryo: Kryo?, input: Input?) = throw AssertionError("not called")
fun foo() {
println("bar")
}
}
val kryo = createKryo().apply {
// This is required to make sure Kryo walks through the auto-generated members for the lambda below.
fieldSerializerConfig.isIgnoreSyntheticFields = false
}
pt.setChildProgressTracker(SimpleSteps.TWO, pt2)
class Tmp {
val unserializable = Unserializable()
val subscription = pt2.changes.subscribe { unserializable.foo() }
}
Tmp()
pt.serialize(kryo)
}
} }