ExternalVendorCode/corda
1
0
Fork 0
mirror of https://github.com/corda/corda.git synced 2024-12-20 21:43:14 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

CORDA-2825 Fix Progress Tracker bug (#4986)

Browse Source 
* CORDA-2825 Fix Progress Tracker bug

CORDA-2825 Add discriminator

CORDA-2825 Fix hashcode

CORDA-2825 remove todo

CORDA-2825 more tests

* CORDA-2825 Address code review comments
This commit is contained in:
Tudor Malene 2019-04-05 12:08:18 +01:00 committed by Rick Parker
parent b93a4d91d3
commit 685f94bf66
2 changed files with 82 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
core/src/main/kotlin/net/corda/core/utilities/ProgressTracker.kt
View File

@ -2,6 +2,7 @@ package net.corda.core.utilities
import net.corda.core.DeleteForDJVM
import net.corda.core.internal.STRUCTURAL_STEP_PREFIX
import net.corda.core.internal.warnOnce
import net.corda.core.serialization.CordaSerializable
import rx.Observable
import rx.Subscription
@ -34,6 +35,10 @@ import java.util.*
@DeleteForDJVM
class ProgressTracker(vararg inputSteps: Step) {
private companion object {
private val log = contextLogger()
}
@CordaSerializable
@DeleteForDJVM
sealed class Change(val progressTracker: ProgressTracker) {
@ -55,6 +60,11 @@ class ProgressTracker(vararg inputSteps: Step) {
*/
@CordaSerializable
open class Step(open val label: String) {
private fun definitionLocation(): String = Exception().stackTrace.first { it.className != ProgressTracker.Step::class.java.name }.let { "${it.className}:${it.lineNumber}" }
// Required when Steps with the same name are defined in multiple places.
private val discriminator: String = definitionLocation()
open val changes: Observable<Change> get() = Observable.empty()
open fun childProgressTracker(): ProgressTracker? = null
/**
@ -63,6 +73,17 @@ class ProgressTracker(vararg inputSteps: Step) {
* Even if empty the basic details (i.e. label) of the step will be recorded for audit purposes.
*/
open val extraAuditData: Map<String, String> get() = emptyMap()
override fun equals(other: Any?) = when (other) {
is Step -> this.label == other.label && this.discriminator == other.discriminator
else -> false
}
override fun hashCode(): Int {
var result = label.hashCode()
result = 31 * result + discriminator.hashCode()
return result
}
}
// Sentinel objects. Overrides equals() to survive process restarts and serialization.
@ -89,7 +110,12 @@ class ProgressTracker(vararg inputSteps: Step) {
/**
* The steps in this tracker, same as the steps passed to the constructor but with UNSTARTED and DONE inserted.
*/
val steps = arrayOf(UNSTARTED, STARTING, *inputSteps, DONE)
val steps = arrayOf(UNSTARTED, STARTING, *inputSteps, DONE).also { stepsArray ->
val labels = stepsArray.map { it.label }
if (labels.toSet().size < labels.size) {
log.warnOnce("Found ProgressTracker Step(s) with the same label: ${labels.groupBy { it }.filter { it.value.size > 1 }.map { it.key }}")
}
}
private var _allStepsCache: List<Pair<Int, Step>> = _allSteps()
@ -137,7 +163,6 @@ class ProgressTracker(vararg inputSteps: Step) {
}
}
init {
steps.forEach {
configureChildTrackerForStep(it)

62
core/src/test/kotlin/net/corda/core/utilities/ProgressTrackerTest.kt
View File

@ -1,13 +1,25 @@
package net.corda.core.utilities
import net.corda.core.serialization.internal.checkpointDeserialize
import net.corda.core.serialization.internal.checkpointSerialize
import net.corda.core.utilities.ProgressTrackerTest.NonSingletonSteps.first
import net.corda.core.utilities.ProgressTrackerTest.NonSingletonSteps.first2
import net.corda.testing.core.internal.CheckpointSerializationEnvironmentRule
import org.assertj.core.api.Assertions.assertThat
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.util.*
import kotlin.test.assertEquals
import kotlin.test.assertFails
import kotlin.test.assertNotEquals
class ProgressTrackerTest {
@Rule
@JvmField
val testCheckpointSerialization = CheckpointSerializationEnvironmentRule()
object SimpleSteps {
object ONE : ProgressTracker.Step("one")
object TWO : ProgressTracker.Step("two")
@ -92,7 +104,7 @@ class ProgressTrackerTest {
assertEquals(pt2.currentStep, ProgressTracker.UNSTARTED)
assertEquals(ProgressTracker.STARTING, pt2.nextStep())
assertEquals(ChildSteps.AYY, pt2.nextStep())
assertEquals((stepNotification.last as ProgressTracker.Change.Position).newStep, ChildSteps.AYY)
assertEquals((stepNotification.last as ProgressTracker.Change.Position).newStep, ChildSteps.AYY)
assertEquals(ChildSteps.BEE, pt2.nextStep())
}
@ -112,7 +124,7 @@ class ProgressTrackerTest {
stepsTreeNotification += it
}
fun assertCurrentStepsTree(index:Int, step: ProgressTracker.Step) {
fun assertCurrentStepsTree(index: Int, step: ProgressTracker.Step) {
assertEquals(index, pt.stepsTreeIndex)
assertEquals(step, allSteps[pt.stepsTreeIndex].second)
}
@ -169,7 +181,7 @@ class ProgressTrackerTest {
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 1, 4, 7))
assertThat(stepsTreeNotification).hasSize(3) // The initial tree state, plus one per update
}
@Test
fun `structure changes are pushed down when progress trackers are added`() {
pt.setChildProgressTracker(SimpleSteps.TWO, pt2)
@ -186,7 +198,7 @@ class ProgressTrackerTest {
stepsTreeNotification += it
}
fun assertCurrentStepsTree(index:Int, step: ProgressTracker.Step) {
fun assertCurrentStepsTree(index: Int, step: ProgressTracker.Step) {
assertEquals(index, pt.stepsTreeIndex)
assertEquals(step.label, stepsTreeNotification.last()[pt.stepsTreeIndex].second)
}
@ -223,7 +235,7 @@ class ProgressTrackerTest {
stepsTreeNotification += it
}
fun assertCurrentStepsTree(index:Int, step: ProgressTracker.Step) {
fun assertCurrentStepsTree(index: Int, step: ProgressTracker.Step) {
assertEquals(index, pt.stepsTreeIndex)
assertEquals(step.label, stepsTreeNotification.last()[pt.stepsTreeIndex].second)
}
@ -273,7 +285,7 @@ class ProgressTrackerTest {
pt.nextStep()
pt.nextStep()
pt.nextStep()
pt.changes.subscribe { steps.add(it.toString())}
pt.changes.subscribe { steps.add(it.toString()) }
pt.nextStep()
pt.nextStep()
pt.nextStep()
@ -290,7 +302,7 @@ class ProgressTrackerTest {
pt.setChildProgressTracker(SimpleSteps.TWO, pt3)
val thirdStepLabels = pt.allStepsLabels
pt.stepsTreeChanges.subscribe { stepTreeNotifications.add(it)}
pt.stepsTreeChanges.subscribe { stepTreeNotifications.add(it) }
// Should have one notification for original tree, then one for each time it changed.
assertEquals(3, stepTreeNotifications.size)
@ -320,4 +332,40 @@ class ProgressTrackerTest {
fun `cannot assign step not belonging to this progress tracker`() {
assertFails { pt.currentStep = BabySteps.UNOS }
}
object NonSingletonSteps {
val first = ProgressTracker.Step("first")
val second = ProgressTracker.Step("second")
val first2 = ProgressTracker.Step("first")
fun tracker() = ProgressTracker(first, second, first2)
}
@Test
fun `Serializing and deserializing a tracker maintains equality`() {
val step = NonSingletonSteps.first
val recreatedStep = step
.checkpointSerialize(testCheckpointSerialization.checkpointSerializationContext)
.checkpointDeserialize(testCheckpointSerialization.checkpointSerializationContext)
assertEquals(step, recreatedStep)
}
@Test
fun `can assign a recreated equal step`() {
val tracker = NonSingletonSteps.tracker()
val recreatedStep = first
.checkpointSerialize(testCheckpointSerialization.checkpointSerializationContext)
.checkpointDeserialize(testCheckpointSerialization.checkpointSerializationContext)
tracker.currentStep = recreatedStep
}
@Test
fun `Steps with the same label defined in different places are not equal`() {
val one = ProgressTracker.Step("one")
assertNotEquals(one, SimpleSteps.ONE)
}
@Test
fun `Steps with the same label defined in the same place are also not equal`() {
assertNotEquals(first, first2)
}
}