[CORDA-2738] Allow the ProgressTracker to cope with child trackers with the same steps (#4894)

This commit is contained in:
JamesHR3 2019-03-18 13:50:29 +00:00 committed by Shams Asari
parent 31100cd708
commit a90f394d43
5 changed files with 206 additions and 88 deletions

View File

@ -50,7 +50,9 @@ class ProgressTracker(vararg inputSteps: Step) {
}
}
/** The superclass of all step objects. */
/**
* The superclass of all step objects.
*/
@CordaSerializable
open class Step(open val label: String) {
open val changes: Observable<Change> get() = Observable.empty()
@ -84,7 +86,9 @@ class ProgressTracker(vararg inputSteps: Step) {
private val childProgressTrackers = mutableMapOf<Step, Child>()
/** The steps in this tracker, same as the steps passed to the constructor but with UNSTARTED and DONE inserted. */
/**
* 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)
private var _allStepsCache: List<Pair<Int, Step>> = _allSteps()
@ -94,6 +98,10 @@ class ProgressTracker(vararg inputSteps: Step) {
private val _stepsTreeChanges by transient { ReplaySubject.create<List<Pair<Int, String>>>() }
private val _stepsTreeIndexChanges by transient { ReplaySubject.create<Int>() }
/**
* Reading returns the value of steps[stepIndex], writing moves the position of the current tracker. Once moved to
* the [DONE] state, this tracker is finished and the current step cannot be moved again.
*/
var currentStep: Step
get() = steps[stepIndex]
set(value) {
@ -134,6 +142,9 @@ class ProgressTracker(vararg inputSteps: Step) {
steps.forEach {
configureChildTrackerForStep(it)
}
// Immediately update the step tree observable to ensure the first update the client receives is the initial state of the progress
// tracker.
_stepsTreeChanges.onNext(allStepsLabels)
this.currentStep = UNSTARTED
}
@ -144,13 +155,17 @@ class ProgressTracker(vararg inputSteps: Step) {
}
}
/** The zero-based index of the current step in the [steps] array (i.e. with UNSTARTED and DONE) */
/**
* The zero-based index of the current step in the [steps] array (i.e. with UNSTARTED and DONE)
*/
var stepIndex: Int = 0
private set(value) {
field = value
}
/** The zero-bases index of the current step in a [allStepsLabels] list */
/**
* The zero-bases index of the current step in a [allStepsLabels] list
*/
var stepsTreeIndex: Int = -1
private set(value) {
if (value != field) {
@ -160,26 +175,12 @@ class ProgressTracker(vararg inputSteps: Step) {
}
/**
* Reading returns the value of steps[stepIndex], writing moves the position of the current tracker. Once moved to
* the [DONE] state, this tracker is finished and the current step cannot be moved again.
* Returns the current step, descending into children to find the deepest step we are up to.
*/
/** Returns the current step, descending into children to find the deepest step we are up to. */
@Suppress("unused")
val currentStepRecursive: Step
get() = getChildProgressTracker(currentStep)?.currentStepRecursive ?: currentStep
/** Returns the current step, descending into children to find the deepest started step we are up to. */
private val currentStartedStepRecursive: Step
get() {
val step = getChildProgressTracker(currentStep)?.currentStartedStepRecursive ?: currentStep
return if (step == UNSTARTED) currentStep else step
}
private fun currentStepRecursiveWithoutUnstarted(): Step {
val stepRecursive = getChildProgressTracker(currentStep)?.currentStartedStepRecursive
return if (stepRecursive == null || stepRecursive == UNSTARTED) currentStep else stepRecursive
}
fun getChildProgressTracker(step: Step): ProgressTracker? = childProgressTrackers[step]?.tracker
fun setChildProgressTracker(step: ProgressTracker.Step, childProgressTracker: ProgressTracker) {
@ -213,12 +214,17 @@ class ProgressTracker(vararg inputSteps: Step) {
_stepsTreeChanges.onError(error)
}
/** The parent of this tracker: set automatically by the parent when a tracker is added as a child */
/**
* The parent of this tracker: set automatically by the parent when a tracker is added as a child
*/
var parent: ProgressTracker? = null
private set
/** Walks up the tree to find the top level tracker. If this is the top level tracker, returns 'this' */
@Suppress("unused") // TODO: Review by EOY2016 if this property is useful anywhere.
/**
* Walks up the tree to find the top level tracker. If this is the top level tracker, returns 'this'.
* Required for API compatibility.
*/
@Suppress("unused")
val topLevelTracker: ProgressTracker
get() {
var cursor: ProgressTracker = this
@ -233,9 +239,21 @@ class ProgressTracker(vararg inputSteps: Step) {
recalculateStepsTreeIndex()
}
private fun getStepIndexAtLevel(): Int {
// This gets the index of the current step in the context of this progress tracker, so it will always be at the top level in
// the allStepsCache.
val index = _allStepsCache.indexOf(Pair(0, currentStep))
return if (index >= 0) index else 0
}
private fun getCurrentStepTreeIndex(): Int {
val indexAtLevel = getStepIndexAtLevel()
val additionalIndex = getChildProgressTracker(currentStep)?.getCurrentStepTreeIndex() ?: 0
return indexAtLevel + additionalIndex
}
private fun recalculateStepsTreeIndex() {
val step = currentStepRecursiveWithoutUnstarted()
stepsTreeIndex = _allStepsCache.indexOfFirst { it.second == step }
stepsTreeIndex = getCurrentStepTreeIndex()
}
private fun _allSteps(level: Int = 0): List<Pair<Int, Step>> {
@ -290,7 +308,9 @@ class ProgressTracker(vararg inputSteps: Step) {
*/
val stepsTreeIndexChanges: Observable<Int> get() = _stepsTreeIndexChanges
/** Returns true if the progress tracker has ended, either by reaching the [DONE] step or prematurely with an error */
/**
* Returns true if the progress tracker has ended, either by reaching the [DONE] step or prematurely with an error
*/
val hasEnded: Boolean get() = _changes.hasCompleted() || _changes.hasThrowable()
}
// TODO: Expose the concept of errors.

View File

@ -36,12 +36,14 @@ class ProgressTrackerTest {
lateinit var pt: ProgressTracker
lateinit var pt2: ProgressTracker
lateinit var pt3: ProgressTracker
lateinit var pt4: ProgressTracker
@Before
fun before() {
pt = SimpleSteps.tracker()
pt2 = ChildSteps.tracker()
pt3 = BabySteps.tracker()
pt4 = ChildSteps.tracker()
}
@Test
@ -129,8 +131,8 @@ class ProgressTrackerTest {
assertCurrentStepsTree(6, SimpleSteps.THREE)
// Assert no structure changes and proper steps propagation.
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(1, 2, 4, 6))
assertThat(stepsTreeNotification).hasSize(1) // One entry per child progress tracker set
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 1, 2, 4, 6))
assertThat(stepsTreeNotification).hasSize(2) // The initial tree state, plus one per tree update
}
@Test
@ -164,8 +166,8 @@ class ProgressTrackerTest {
assertCurrentStepsTree(7, ChildSteps.SEA)
// Assert no structure changes and proper steps propagation.
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(1, 4, 7))
assertThat(stepsTreeNotification).hasSize(2) // One entry per child progress tracker set
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 1, 4, 7))
assertThat(stepsTreeNotification).hasSize(3) // The initial tree state, plus one per update
}
@Test
@ -201,8 +203,8 @@ class ProgressTrackerTest {
assertCurrentStepsTree(10, SimpleSteps.FOUR)
// Assert no structure changes and proper steps propagation.
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(2, 7, 10))
assertThat(stepsTreeNotification).hasSize(2) // One state per child progress tracker set
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 2, 7, 10))
assertThat(stepsTreeNotification).hasSize(3) // The initial tree state, plus one per update.
}
@Test
@ -236,8 +238,8 @@ class ProgressTrackerTest {
assertCurrentStepsTree(3, BabySteps.UNOS)
// Assert no structure changes and proper steps propagation.
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(2, 5, 3))
assertThat(stepsTreeNotification).hasSize(2) // One state per child progress tracker set
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 2, 5, 3))
assertThat(stepsTreeNotification).hasSize(3) // The initial tree state, plus one per update
}
@Test
@ -256,13 +258,13 @@ class ProgressTrackerTest {
pt.currentStep = SimpleSteps.TWO
val stepsIndexNotifications = LinkedList<Int>()
pt.stepsTreeIndexChanges.subscribe() {
pt.stepsTreeIndexChanges.subscribe {
stepsIndexNotifications += it
}
pt2.currentStep = ChildSteps.AYY
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(1, 2, 3))
assertThat(stepsIndexNotifications).containsExactlyElementsOf(listOf(0, 1, 2, 3))
}
@Test
@ -281,20 +283,41 @@ class ProgressTrackerTest {
@Test
fun `all tree changes seen if subscribed mid flow`() {
val stepTreeNotifications = mutableListOf<List<Pair<Int, String>>>()
pt.setChildProgressTracker(SimpleSteps.TWO, pt2)
val firstStepLabels = pt.allStepsLabels
pt.currentStep = SimpleSteps.ONE
pt.currentStep = SimpleSteps.TWO
pt.setChildProgressTracker(SimpleSteps.TWO, pt2)
val secondStepLabels = pt.allStepsLabels
pt.setChildProgressTracker(SimpleSteps.TWO, pt3)
val thirdStepLabels = pt.allStepsLabels
pt.stepsTreeChanges.subscribe { stepTreeNotifications.add(it)}
fun assertStepsTree(index: Int, step: ProgressTracker.Step) {
assertEquals(step.label, stepTreeNotifications[index][pt.stepsTreeIndex].second)
}
// Should have one notification for original tree, then one for each time it changed.
assertEquals(3, stepTreeNotifications.size)
assertEquals(listOf(firstStepLabels, secondStepLabels, thirdStepLabels), stepTreeNotifications)
}
@Test
fun `trees with child trackers with duplicate steps reported correctly`() {
val stepTreeNotifications = mutableListOf<List<Pair<Int, String>>>()
val stepIndexNotifications = mutableListOf<Int>()
pt.stepsTreeChanges.subscribe { stepTreeNotifications += it }
pt.stepsTreeIndexChanges.subscribe { stepIndexNotifications += it }
pt.setChildProgressTracker(SimpleSteps.ONE, pt2)
pt.setChildProgressTracker(SimpleSteps.TWO, pt4)
pt.currentStep = SimpleSteps.ONE
pt2.currentStep = ChildSteps.AYY
pt3.currentStep = BabySteps.UNOS
assertStepsTree(0, ChildSteps.AYY)
assertStepsTree(1, BabySteps.UNOS)
pt2.nextStep()
pt2.nextStep()
pt.nextStep()
pt4.currentStep = ChildSteps.AYY
assertEquals(listOf(0, 1, 2, 3, 4, 5, 6), stepIndexNotifications)
}
@Test
fun `cannot assign step not belonging to this progress tracker`() {
assertFails { pt.currentStep = BabySteps.UNOS }
}
}

View File

@ -33,13 +33,22 @@ class CashIssueAndPaymentFlow(val amount: Amount<Currency>,
issueRef: OpaqueBytes,
recipient: Party,
anonymous: Boolean,
notary: Party) : this(amount, issueRef, recipient, anonymous, notary, ProgressTracker())
notary: Party) : this(amount, issueRef, recipient, anonymous, notary, tracker())
constructor(request: IssueAndPaymentRequest) : this(request.amount, request.issueRef, request.recipient, request.anonymous, request.notary, ProgressTracker())
constructor(request: IssueAndPaymentRequest) : this(request.amount, request.issueRef, request.recipient, request.anonymous, request.notary, tracker())
companion object {
val ISSUING_CASH = ProgressTracker.Step("Issuing cash")
val PAYING_RECIPIENT = ProgressTracker.Step("Paying recipient")
fun tracker() = ProgressTracker(ISSUING_CASH, PAYING_RECIPIENT)
}
@Suspendable
override fun call(): Result {
progressTracker.currentStep = ISSUING_CASH
subFlow(CashIssueFlow(amount, issueRef, notary))
progressTracker.currentStep = PAYING_RECIPIENT
return subFlow(CashPaymentFlow(amount, recipient, anonymous, notary))
}

View File

@ -16,22 +16,23 @@ import org.fusesource.jansi.Ansi
import org.fusesource.jansi.Ansi.Attribute
import org.fusesource.jansi.AnsiConsole
import org.fusesource.jansi.AnsiOutputStream
import rx.Observable.combineLatest
import rx.Subscription
import java.util.*
import java.util.stream.IntStream
import kotlin.streams.toList
abstract class ANSIProgressRenderer {
private var subscriptionIndex: Subscription? = null
private var subscriptionTree: Subscription? = null
private var updatesSubscription: Subscription? = null
protected var usingANSI = false
protected var checkEmoji = false
private val usingUnicode = !SystemUtils.IS_OS_WINDOWS
protected var treeIndex: Int = 0
protected var treeIndexProcessed: MutableSet<Int> = mutableSetOf()
protected var tree: List<Pair<Int,String>> = listOf()
private var treeIndex: Int = 0
private var treeIndexProcessed: MutableSet<Int> = mutableSetOf()
protected var tree: List<ProgressStep> = listOf()
private var installedYet = false
@ -42,15 +43,18 @@ abstract class ANSIProgressRenderer {
// prevLinesDraw is just for ANSI mode.
protected var prevLinesDrawn = 0
data class ProgressStep(val level: Int, val description: String, val parentIndex: Int?)
data class InputTreeStep(val level: Int, val description: String)
private fun done(error: Throwable?) {
if (error == null) _render(null)
if (error == null) renderInternal(null)
draw(true, error)
onDone()
}
fun render(flowProgressHandle: FlowProgressHandle<*>, onDone: () -> Unit = {}) {
this.onDone = onDone
_render(flowProgressHandle)
renderInternal(flowProgressHandle)
}
protected abstract fun printLine(line:String)
@ -59,9 +63,8 @@ abstract class ANSIProgressRenderer {
protected abstract fun setup()
private fun _render(flowProgressHandle: FlowProgressHandle<*>?) {
subscriptionIndex?.unsubscribe()
subscriptionTree?.unsubscribe()
private fun renderInternal(flowProgressHandle: FlowProgressHandle<*>?) {
updatesSubscription?.unsubscribe()
treeIndex = 0
treeIndexProcessed.clear()
tree = listOf()
@ -75,27 +78,64 @@ abstract class ANSIProgressRenderer {
prevLinesDrawn = 0
draw(true)
val treeUpdates = flowProgressHandle?.stepsTreeFeed?.updates
val indexUpdates = flowProgressHandle?.stepsTreeIndexFeed?.updates
flowProgressHandle?.apply {
stepsTreeIndexFeed?.apply {
treeIndexProcessed.add(snapshot)
subscriptionIndex = updates.subscribe({
treeIndex = it
treeIndexProcessed.add(it)
if (treeUpdates == null || indexUpdates == null) {
renderInBold("Cannot print progress for this flow as the required data is missing", Ansi())
} else {
// By combining the two observables, a race condition where both emit items at roughly the same time is avoided. This could
// result in steps being incorrectly marked as skipped. Instead, whenever either observable emits an item, a pair of the
// last index and last tree is returned, which ensures that updates to either are processed in series.
updatesSubscription = combineLatest(treeUpdates, indexUpdates) { tree, index -> Pair(tree, index) }.subscribe(
{
val newTree = transformTree(it.first.map { elem -> InputTreeStep(elem.first, elem.second) })
// Process indices first, as if the tree has changed the associated index with this update is for the old tree. Note
// that the one case where this isn't true is the very first update, but in this case the index should be 0 (as this
// update is for the initial state). The remapping on a new tree assumes the step at index 0 is always at least current,
// so this case is handled there.
treeIndex = it.second
treeIndexProcessed.add(it.second)
if (newTree != tree) {
remapIndices(newTree)
tree = newTree
}
draw(true)
}, { done(it) }, { done(null) })
}
stepsTreeFeed?.apply {
subscriptionTree = updates.subscribe({
remapIndices(it)
tree = it
draw(true)
}, { done(it) }, { done(null) })
}
},
{ done(it) },
{ done(null) }
)
}
}
private fun remapIndices(newTree: List<Pair<Int, String>>) {
// Create a new tree of steps that also holds a reference to the parent of each step. This is required to uniquely identify each step
// (assuming that each step label is unique at a given level).
private fun transformTree(inputTree: List<InputTreeStep>): List<ProgressStep> {
if (inputTree.isEmpty()) {
return listOf()
}
val stack = Stack<Pair<Int, InputTreeStep>>()
stack.push(Pair(0, inputTree[0]))
return inputTree.mapIndexed { index, step ->
val parentIndex = try {
val top = stack.peek()
val levelDifference = top.second.level - step.level
if (levelDifference >= 0) {
// The top of the stack is at the same or lower level than the current step. Remove items from the top until the topmost
// item is at a higher level - this is the parent step.
repeat(levelDifference + 1) { stack.pop() }
}
stack.peek().first
} catch (e: EmptyStackException) {
// If there is nothing on the stack at any point, it implies that this step is at the top level and has no parent.
null
}
stack.push(Pair(index, step))
ProgressStep(step.level, step.description, parentIndex)
}
}
private fun remapIndices(newTree: List<ProgressStep>) {
val newIndices = newTree.filter {
treeIndexProcessed.contains(tree.indexOf(it))
}.map {
@ -108,7 +148,7 @@ abstract class ANSIProgressRenderer {
@Synchronized protected fun draw(moveUp: Boolean, error: Throwable? = null) {
if (!usingANSI) {
val currentMessage = tree.getOrNull(treeIndex)?.second
val currentMessage = tree.getOrNull(treeIndex)?.description
if (currentMessage != null && currentMessage != prevMessagePrinted) {
printLine(currentMessage)
prevMessagePrinted = currentMessage
@ -182,13 +222,13 @@ abstract class ANSIProgressRenderer {
error -> if (usingUnicode) "${Emoji.noEntry} " else "ERROR: "
else -> " " // Not reached yet.
}
a(" ".repeat(step.first))
a(" ".repeat(step.level))
a(marker)
when {
activeStep -> renderInBold(step.second, ansi)
skippedStep -> renderInFaint(step.second, ansi)
else -> a(step.second)
activeStep -> renderInBold(step.description, ansi)
skippedStep -> renderInFaint(step.description, ansi)
else -> a(step.description)
}
eraseLine(Ansi.Erase.FORWARD)

View File

@ -40,6 +40,10 @@ class ANSIProgressRendererTest {
fun stepActive(stepLabel: String): String {
return if (SystemUtils.IS_OS_WINDOWS) """CURRENT: $INTENSITY_BOLD_ON_ASCII$stepLabel$INTENSITY_OFF_ASCII""" else """▶︎ $INTENSITY_BOLD_ON_ASCII$stepLabel$INTENSITY_OFF_ASCII"""
}
fun stepNotRun(stepLabel: String): String {
return """ $stepLabel"""
}
}
lateinit var printWriter: RenderPrintWriter
@ -59,35 +63,57 @@ class ANSIProgressRendererTest {
flowProgressHandle = FlowProgressHandleImpl(StateMachineRunId.createRandom(), openFuture<String>(), Observable.empty(), stepsTreeIndexFeed, stepsTreeFeed)
}
private fun checkTrackingState(captor: KArgumentCaptor<Ansi>, updates: Int, trackerState: List<String>) {
verify(printWriter, times(updates)).print(captor.capture())
assertThat(captor.lastValue.toString()).containsSequence(trackerState)
verify(printWriter, times(updates)).flush()
}
@Test
fun `test that steps are rendered appropriately depending on their status`() {
progressRenderer.render(flowProgressHandle)
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(0, STEP_2_LABEL), Pair(0, STEP_3_LABEL)))
// The flow is currently at step 3, while step 1 has been completed and step 2 has been skipped.
indexSubject.onNext(0)
indexSubject.onNext(2)
val captor = argumentCaptor<Ansi>()
verify(printWriter, times(2)).print(captor.capture())
assertThat(captor.secondValue.toString()).containsSequence(stepSuccess(STEP_1_LABEL), stepSkipped(STEP_2_LABEL), stepActive(STEP_3_LABEL))
verify(printWriter, times(2)).flush()
checkTrackingState(captor, 2, listOf(stepSuccess(STEP_1_LABEL), stepSkipped(STEP_2_LABEL), stepActive(STEP_3_LABEL)))
}
@Test
fun `changing tree causes correct steps to be marked as done`() {
progressRenderer.render(flowProgressHandle)
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(1, STEP_2_LABEL), Pair(1, STEP_3_LABEL), Pair(0, STEP_4_LABEL), Pair(0, STEP_5_LABEL)))
indexSubject.onNext(0)
indexSubject.onNext(1)
indexSubject.onNext(2)
val captor = argumentCaptor<Ansi>()
verify(printWriter, times(3)).print(captor.capture())
assertThat(captor.lastValue.toString()).containsSequence(stepSuccess(STEP_1_LABEL), stepSuccess(STEP_2_LABEL), stepActive(STEP_3_LABEL))
verify(printWriter, times(3)).flush()
checkTrackingState(captor, 3, listOf(stepSuccess(STEP_1_LABEL), stepSuccess(STEP_2_LABEL), stepActive(STEP_3_LABEL)))
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(0, STEP_4_LABEL), Pair(0, STEP_5_LABEL)))
verify(printWriter, times(4)).print(captor.capture())
assertThat(captor.lastValue.toString()).containsSequence(stepActive(STEP_1_LABEL))
assertThat(captor.lastValue.toString()).doesNotContain(stepActive(STEP_5_LABEL))
verify(printWriter, times(4)).flush()
checkTrackingState(captor, 4, listOf(stepActive(STEP_1_LABEL), stepNotRun(STEP_4_LABEL), stepNotRun(STEP_5_LABEL)))
}
@Test
fun `duplicate steps in different children handled correctly`() {
val captor = argumentCaptor<Ansi>()
progressRenderer.render(flowProgressHandle)
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(0, STEP_2_LABEL)))
indexSubject.onNext(0)
checkTrackingState(captor, 1, listOf(stepActive(STEP_1_LABEL), stepNotRun(STEP_2_LABEL)))
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(1, STEP_3_LABEL), Pair(0, STEP_2_LABEL), Pair(1, STEP_3_LABEL)))
indexSubject.onNext(1)
indexSubject.onNext(2)
indexSubject.onNext(3)
checkTrackingState(captor, 5, listOf(stepSuccess(STEP_1_LABEL), stepSuccess(STEP_3_LABEL), stepSuccess(STEP_2_LABEL), stepActive(STEP_3_LABEL)))
feedSubject.onNext(listOf(Pair(0, STEP_1_LABEL), Pair(1, STEP_3_LABEL), Pair(0, STEP_2_LABEL), Pair(1, STEP_3_LABEL), Pair(2, STEP_4_LABEL)))
checkTrackingState(captor, 6, listOf(stepSuccess(STEP_1_LABEL), stepSuccess(STEP_3_LABEL), stepSuccess(STEP_2_LABEL), stepActive(STEP_3_LABEL), stepNotRun(STEP_4_LABEL)))
}
}