Skip to content

Commit e6b6b15

Browse files
committed
bug fix: fraction calculations for totalCount nil -> non-nil
1 parent 85d2ce4 commit e6b6b15

File tree

2 files changed

+79
-5
lines changed

2 files changed

+79
-5
lines changed

Sources/FoundationEssentials/ProgressManager/ProgressManager.swift

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ internal struct FractionState {
2727
var overallFraction: _ProgressFraction {
2828
selfFraction + childFraction
2929
}
30+
var children: Set<ProgressManager>
3031
var interopChild: ProgressManager? // read from this if self is actually an interop ghost
3132
}
3233

@@ -153,6 +154,13 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
153154
}
154155
state.fractionState.selfFraction.total = newValue ?? 0
155156

157+
// Updating my own childFraction here because previously they did not get updated because my total was 0
158+
if !state.fractionState.children.isEmpty {
159+
for child in state.fractionState.children {
160+
child.updateChildFractionSpecial(of: manager, state: &state)
161+
}
162+
}
163+
156164
// if newValue is nil, reset indeterminate to true
157165
if newValue != nil {
158166
state.fractionState.indeterminate = false
@@ -227,16 +235,15 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
227235
}
228236

229237
internal let parents: LockedState<[ProgressManager: Int]>
230-
private let children: LockedState<Set<ProgressManager>>
231238
private let state: LockedState<State>
232239

233240
internal init(total: Int?, ghostReporter: ProgressManager?, interopObservation: (any Sendable)?) {
234241
self.parents = .init(initialState: [:])
235-
self.children = .init(initialState: Set())
236242
let fractionState = FractionState(
237243
indeterminate: total == nil ? true : false,
238244
selfFraction: _ProgressFraction(completed: 0, total: total ?? 0),
239245
childFraction: _ProgressFraction(completed: 0, total: 1),
246+
children: Set<ProgressManager>(),
240247
interopChild: nil
241248
)
242249
let state = State(fractionState: fractionState, otherProperties: [:], childrenOtherProperties: [:])
@@ -324,7 +331,7 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
324331
return try state.withLock { (state) throws(E) -> T in
325332
var values = Values(manager: self, state: state)
326333
// This is done to avoid copy on write later
327-
state = State(fractionState: FractionState(indeterminate: true, selfFraction: _ProgressFraction(), childFraction: _ProgressFraction()), otherProperties: [:], childrenOtherProperties: [:])
334+
state = State(fractionState: FractionState(indeterminate: true, selfFraction: _ProgressFraction(), childFraction: _ProgressFraction(), children: Set()), otherProperties: [:], childrenOtherProperties: [:])
328335
let result = try closure(&values)
329336
state = values.state
330337
return result
@@ -402,6 +409,23 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
402409
return UpdateState(previous: previous, current: current)
403410
}
404411

412+
// This is used when parent has its lock acquired and wants its child to update parent's childFraction to reflect child's own changes
413+
private func updateChildFractionSpecial(of manager: ProgressManager, state managerState: inout State) {
414+
let portion = parents.withLock { parents in
415+
return parents[manager]
416+
}
417+
418+
if let portionOfParent = portion {
419+
let myFraction = state.withLock { $0.fractionState.overallFraction }
420+
421+
if myFraction.isFinished {
422+
// If I'm not finished, update my entry in parent's childFraction
423+
managerState.fractionState.childFraction = managerState.fractionState.childFraction + _ProgressFraction(completed: portionOfParent, total: managerState.fractionState.selfFraction.total) * myFraction
424+
425+
}
426+
}
427+
}
428+
405429
private func updateFractionCompleted(from: _ProgressFraction, to: _ProgressFraction) {
406430
_$observationRegistrar.withMutation(of: self, keyPath: \.fractionCompleted) {
407431
if from != to {
@@ -491,8 +515,8 @@ internal struct AnyMetatypeWrapper: Hashable, Equatable, Sendable {
491515
}
492516

493517
internal func addToChildren(childManager: ProgressManager) {
494-
_ = children.withLock { children in
495-
children.insert(childManager)
518+
_ = state.withLock { state in
519+
state.fractionState.children.insert(childManager)
496520
}
497521
}
498522

Tests/FoundationEssentialsTests/ProgressManager/ProgressManagerTests.swift

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,56 @@ class TestProgressReporter: XCTestCase {
645645
XCTAssertEqual(altManager2.fractionCompleted, 0.4)
646646
}
647647

648+
func testAssignToProgressReporterThenSetTotalCount() {
649+
let overall = ProgressManager(totalCount: nil)
650+
651+
let child1 = ProgressManager(totalCount: 10)
652+
overall.assign(count: 10, to: child1.reporter)
653+
child1.complete(count: 5)
654+
655+
let child2 = ProgressManager(totalCount: 20)
656+
overall.assign(count: 20, to: child2.reporter)
657+
child2.complete(count: 20)
658+
659+
overall.withProperties { properties in
660+
properties.totalCount = 30
661+
}
662+
XCTAssertEqual(overall.completedCount, 20)
663+
XCTAssertEqual(overall.fractionCompleted, 25 / 30)
664+
665+
child1.complete(count: 5)
666+
667+
XCTAssertEqual(overall.completedCount, 30)
668+
XCTAssertEqual(overall.fractionCompleted, 1.0)
669+
}
670+
671+
func testMakeSubprogressThenSetTotalCount() async {
672+
let overall = ProgressManager(totalCount: nil)
673+
674+
let reporter1 = await dummy(index: 1, subprogress: overall.subprogress(assigningCount: 10))
675+
676+
let reporter2 = await dummy(index: 2, subprogress: overall.subprogress(assigningCount: 20))
677+
678+
XCTAssertEqual(reporter1.fractionCompleted, 0.5)
679+
680+
XCTAssertEqual(reporter2.fractionCompleted, 0.5)
681+
682+
overall.withProperties { properties in
683+
properties.totalCount = 30
684+
}
685+
686+
XCTAssertEqual(overall.totalCount, 30)
687+
XCTAssertEqual(overall.fractionCompleted, 0.5)
688+
}
689+
690+
func dummy(index: Int, subprogress: consuming Subprogress) async -> ProgressReporter {
691+
let manager = subprogress.start(totalCount: index * 10)
692+
693+
manager.complete(count: (index * 10) / 2)
694+
695+
return manager.reporter
696+
}
697+
648698
/// All of these test cases hit the precondition for cycle detection, but currently there's no way to check for hitting precondition in xctest.
649699
// func testProgressReporterDirectCycleDetection() {
650700
// let manager = ProgressManager(totalCount: 2)

0 commit comments

Comments
 (0)