Correctly refresh when needed only

yalishanda42 · May 13, 2023 · 5cfc783 · 5cfc783
1 parent e2776f6
commit 5cfc783
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 38 deletions.
diff --git a/BDZDelays/bdz-delays/Sources/StationDomain/StationReducer.swift b/BDZDelays/bdz-delays/Sources/StationDomain/StationReducer.swift
@@ -21,7 +21,7 @@ public struct StationReducer: ReducerProtocol {
 
         public init(
             station: BGStation,
-            loadingState: RefreshState = .loading,
+            loadingState: RefreshState = .loaded,
             trains: [TrainAtStation] = [],
             lastUpdateTime: Date? = nil
         ) {
@@ -36,11 +36,14 @@ public struct StationReducer: ReducerProtocol {
         case refresh
         case receive(TaskResult<[TrainAtStation]>)
 
+        /// Received on every new second.
+        case tick
+
         /// Execute the long-running effect,
         /// associated with the lifetime of the feature.
         case task
 
-        /// To be invoked before destroying
+        /// To be invoked before destroying.
         case finalize
     }
 
@@ -54,37 +57,39 @@ public struct StationReducer: ReducerProtocol {
     public func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
         switch action {
         case .task:
-            return .run { [
-                lastUpdateDate = state.lastUpdateTime,
-                loadingState = state.loadingState
-            ] send in
+            return .run { send in
                 for await _ in clock.timer(interval: .seconds(1)) {
-                    let isInTheSameMinute = lastUpdateDate
-                        .map {
-                            let components: Set<Calendar.Component> = [.day, .hour, .minute]
-                            let entryComponents = calendar.dateComponents(components, from: $0)
-                            let nowComponents = calendar.dateComponents(components, from: now)
-                            return entryComponents == nowComponents
-                        }
-                        ?? false
-
-                    if loadingState != .loading && !isInTheSameMinute {
-                        await send(.refresh)
-                    }
+                    await send(.tick)
                 }
             }
+
+        case .tick:
+            // Refresh on every new calendar minute.
+            let isInTheSameMinute = state.lastUpdateTime
+                .map {
+                    let components: Set<Calendar.Component> = [.day, .hour, .minute]
+                    let entryComponents = calendar.dateComponents(components, from: $0)
+                    let nowComponents = calendar.dateComponents(components, from: now)
+                    return entryComponents == nowComponents
+                }
+                ?? false
+
+            if !isInTheSameMinute {
+                return .send(.refresh)
+            }
+
         case .refresh:
+            guard state.loadingState != .loading else {
+                return .none
+            }
+
             state.loadingState = .loading
-            return .concatenate(
-                // First cancel ongoing fetch
-                .cancel(id: TrainsTaskCancelID.self),
-                // Then send another
-                .task { [station = state.station] in
-                    await .receive(TaskResult {
-                        try await stationRepository.fetchTrainsAtStation(station)
-                    })
-                }.cancellable(id: TrainsTaskCancelID.self)
-            )
+
+            return .task { [station = state.station] in
+                await .receive(TaskResult {
+                    try await stationRepository.fetchTrainsAtStation(station)
+                })
+            }
 
         case .receive(.success(let trains)):
             state.lastUpdateTime = now

diff --git a/BDZDelays/bdz-delays/Sources/StationView/StationView.swift b/BDZDelays/bdz-delays/Sources/StationView/StationView.swift
@@ -47,6 +47,9 @@ public struct StationView: View {
                 .onAppear {
                     vs.send(.refresh)
                 }
+                .task {
+                    await vs.send(.task).finish()
+                }
         }
     }
 

diff --git a/BDZDelays/bdz-delays/Tests/StationDomainTests/StationDomainTests.swift b/BDZDelays/bdz-delays/Tests/StationDomainTests/StationDomainTests.swift
@@ -20,7 +20,9 @@ final class StationDomainTests: XCTestCase {
             $0.date.now = refreshDate
         }
 
-        await store.send(.refresh)
+        await store.send(.refresh) {
+            $0.loadingState = .loading
+        }
         await store.receive(.receive(.success(expected))) {
             $0.loadingState = .loaded
             $0.trains = expected
@@ -83,7 +85,7 @@ final class StationDomainTests: XCTestCase {
         }
     }
 
-    func test_refreshTwice_cancelsPrevious() async throws {
+    func test_refreshTwice_doesNotCancelOrDouble() async throws {
         let results = [Self.expectedOne, Self.expectedTwo]
 
         let clock = TestClock()
@@ -99,7 +101,7 @@ final class StationDomainTests: XCTestCase {
                 fetchTrainsAtStation: { _ in
                     let count = await counter.incremented()
                     // modelling first request to be taking a lot of time
-                    // so that the second can complete before it
+                    // so that the second would complete before it if allowed to
                     let seconds = count == 2 ? 1 : 42
                     for await _ in clock.timer(interval: .seconds(seconds)) {
                         return results[count - 1]
@@ -116,18 +118,18 @@ final class StationDomainTests: XCTestCase {
             }()
         }
 
-        await store.send(.refresh)
+        await store.send(.refresh) {
+            $0.loadingState = .loading
+        }
         await store.send(.refresh)
 
-        await clock.advance(by: .seconds(1))
-
-        await store.receive(.receive(.success(Self.expectedTwo))) {
+        await clock.advance(by: .seconds(1)) // nothing should be received
+        await clock.advance(by: .seconds(41)) // first request is now completed
+        await store.receive(.receive(.success(Self.expectedOne))) {
             $0.loadingState = .loaded
-            $0.trains = Self.expectedTwo
+            $0.trains = Self.expectedOne
             $0.lastUpdateTime = Date(timeIntervalSinceReferenceDate: 1)
         }
-
-        await clock.advance(by: .seconds(41)) // nothing should be received
     }
 }