Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions WooCommerce/Classes/POS/Models/Cart+BarcodeScanError.swift
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ extension PointOfSaleBarcodeScanError {
)

static let incompleteScan = NSLocalizedString(
"pointOfSale.barcodeScan.error.incompleteScan",
value: "Partial barcode scan",
comment: "Error message shown when scan is incomplete."
"pointOfSale.barcodeScan.error.incompleteScan2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: I think the convention in the code base before I started following it is *.versionNumber but it's really a minor detail that is internal to the code base.

Suggested change
"pointOfSale.barcodeScan.error.incompleteScan2",
"pointOfSale.barcodeScan.error.incompleteScan.2",

value: "The scanner did not send an end-of-line character",
comment: "Error message shown when scanner times out without sending end-of-line character."
)

static let parsingError = NSLocalizedString(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ final class GameControllerBarcodeParser {
private var buffer = ""
private var lastKeyPressTime: Date?
private var scanStartTime: Date?
private var timeoutTimer: Timer?

init(configuration: HIDBarcodeParserConfiguration,
onScan: @escaping (HIDBarcodeParserResult) -> Void,
Expand Down Expand Up @@ -49,6 +50,7 @@ final class GameControllerBarcodeParser {
}

buffer.append(character)
scheduleTimeoutTimer()
}
}

Expand Down Expand Up @@ -186,6 +188,31 @@ final class GameControllerBarcodeParser {
buffer = ""
lastKeyPressTime = nil
scanStartTime = nil
cancelTimeoutTimer()
}

private func scheduleTimeoutTimer() {
cancelTimeoutTimer()
timeoutTimer = timeProvider.scheduleTimer(
timeInterval: configuration.maximumInterCharacterTime,
target: self,
selector: #selector(handleTimeoutExpiry)
)
}

private func cancelTimeoutTimer() {
timeoutTimer?.invalidate()
timeoutTimer = nil
}

@objc private func handleTimeoutExpiry() {
guard !buffer.isEmpty else { return }

let scanDurationMs = calculateScanDurationMs()
let result = HIDBarcodeParserResult.failure(error: HIDBarcodeParserError.timedOut(barcode: buffer), scanDurationMs: scanDurationMs)

onScan(result)
resetScan()
}

private func calculateScanDurationMs() -> Int {
Expand All @@ -194,6 +221,7 @@ final class GameControllerBarcodeParser {
}

private func processScan() {
cancelTimeoutTimer()
checkForTimeoutBetweenKeystrokes()
let scanDurationMs = calculateScanDurationMs()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import Foundation

protocol TimeProvider {
func now() -> Date
func scheduleTimer(timeInterval: TimeInterval, target: Any, selector: Selector) -> Timer
}

struct DefaultTimeProvider: TimeProvider {
func now() -> Date {
Date()
}

func scheduleTimer(timeInterval: TimeInterval, target: Any, selector: Selector) -> Timer {
return Timer.scheduledTimer(timeInterval: timeInterval, target: target, selector: selector, userInfo: nil, repeats: false)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,150 @@ struct GameControllerBarcodeParserTests {
}
}

@Test("proactive timeout triggers error without next character")
func timeout_whenTimerFires_triggersTimeoutError() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super nit: the test function names feel like an interesting mix of snake & camel case 😆

// Given
var results: [HIDBarcodeParserResult] = []
let configuration = HIDBarcodeParserConfiguration(
terminatingStrings: ["\r", "\n"],
minimumBarcodeLength: 3,
maximumInterCharacterTime: 0.2
)
let mockTimeProvider = MockTimeProvider()
let parser = GameControllerBarcodeParser(
configuration: configuration,
onScan: { results.append($0) },
timeProvider: mockTimeProvider
)

// When - Type partial barcode and simulate timer firing
parser.processKeyPress(GCKeyCode.one)
parser.processKeyPress(GCKeyCode.two)
parser.processKeyPress(GCKeyCode.three)

// Advance time beyond timeout period - this will automatically fire timers
mockTimeProvider.advance(by: 0.25)

// Then - Should get timeout error automatically
#expect(results.count == 1)
if case .failure(let error, let duration) = results.first {
if case .timedOut(let barcode) = error {
#expect(barcode == "123")
#expect(duration >= 0)
} else {
Issue.record("Expected timedOut error")
}
} else {
Issue.record("Expected timeout failure")
}
}

@Test("timer cancelled on successful scan completion")
func timerCancelled_whenScanCompletes_preventsTimeoutError() {
// Given
var results: [HIDBarcodeParserResult] = []
let configuration = HIDBarcodeParserConfiguration(
terminatingStrings: ["\r", "\n"],
minimumBarcodeLength: 3,
maximumInterCharacterTime: 0.2
)
let mockTimeProvider = MockTimeProvider()
let parser = GameControllerBarcodeParser(
configuration: configuration,
onScan: { results.append($0) },
timeProvider: mockTimeProvider
)

// When - Type partial barcode then complete it before timer fires
parser.processKeyPress(GCKeyCode.one)
parser.processKeyPress(GCKeyCode.two)
parser.processKeyPress(GCKeyCode.three)
parser.processKeyPress(GCKeyCode.returnOrEnter) // Complete scan before timeout

// Try to advance time beyond timeout period - timer should not fire since it was cancelled
mockTimeProvider.advance(by: 0.25)

// Then - Should only get success result, no timeout error
#expect(results.count == 1)
if case .success(let barcode, _) = results.first {
#expect(barcode == "123")
} else {
Issue.record("Expected successful scan")
}
}

@Test("timer cancelled on manual scan cancellation")
func timerCancelled_whenScanCancelled_preventsTimeoutError() {
// Given
var results: [HIDBarcodeParserResult] = []
let configuration = HIDBarcodeParserConfiguration(
terminatingStrings: ["\r", "\n"],
minimumBarcodeLength: 3,
maximumInterCharacterTime: 0.2
)
let mockTimeProvider = MockTimeProvider()
let parser = GameControllerBarcodeParser(
configuration: configuration,
onScan: { results.append($0) },
timeProvider: mockTimeProvider
)

// When - Type partial barcode then cancel before timer fires
parser.processKeyPress(GCKeyCode.one)
parser.processKeyPress(GCKeyCode.two)
parser.processKeyPress(GCKeyCode.three)
parser.cancel() // Cancel scan before timeout

// Try to advance time beyond timeout period - timer should not fire since it was cancelled
mockTimeProvider.advance(by: 0.25)

// Then - Should have no results since scan was cancelled
#expect(results.isEmpty)
}

@Test("new character input cancels previous timer and starts new one")
func newCharacterInput_whenReceived_cancelsOldTimerAndStartsNew() {
// Given
var results: [HIDBarcodeParserResult] = []
let configuration = HIDBarcodeParserConfiguration(
terminatingStrings: ["\r", "\n"],
minimumBarcodeLength: 6,
maximumInterCharacterTime: 0.2
)
let mockTimeProvider = MockTimeProvider()
let parser = GameControllerBarcodeParser(
configuration: configuration,
onScan: { results.append($0) },
timeProvider: mockTimeProvider
)

// When - Type characters with timing that would trigger timeout if timer wasn't reset
parser.processKeyPress(GCKeyCode.one)

// Advance time by 0.15 seconds (less than timeout) and add next character
mockTimeProvider.advance(by: 0.15)
parser.processKeyPress(GCKeyCode.two) // This should cancel the first timer

// Advance another 0.15 seconds (would be 0.3 total, but timer should have reset)
mockTimeProvider.advance(by: 0.15)
parser.processKeyPress(GCKeyCode.three)
parser.processKeyPress(GCKeyCode.four)
parser.processKeyPress(GCKeyCode.five)
parser.processKeyPress(GCKeyCode.six)
parser.processKeyPress(GCKeyCode.returnOrEnter)

// Final advance to ensure no leftover timers fire
mockTimeProvider.advance(by: 0.1)

// Then - Should get successful scan, not timeout
#expect(results.count == 1)
if case .success(let barcode, _) = results.first {
#expect(barcode == "123456")
} else {
Issue.record("Expected successful scan")
}
}

@Test("scan duration is properly tracked for successful scan")
func scanDuration_whenSuccessfulScan_isProperlyTracked() {
// Given
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,32 @@
import Foundation
@testable import WooCommerce

final class MockTimer: Timer {
var isCancelled = false
let mockTimeProvider: MockTimeProvider
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could there be a retain cycle between MockTimeProvider and MockTimer since they hold on to each other? Not sure if it's worth keeping one as weak var.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good observation, it would create retain cycles. We can make it weak to avoid some issues in tests.

let timerInterval: TimeInterval
let target: Any
let selector: Selector
let repeats: Bool

init(timeInterval: TimeInterval, target: Any, selector: Selector, mockTimeProvider: MockTimeProvider) {
self.timerInterval = timeInterval
self.target = target
self.selector = selector
self.repeats = false
self.mockTimeProvider = mockTimeProvider
super.init()
}

override func invalidate() {
isCancelled = true
mockTimeProvider.removeTimer(self)
}
}

final class MockTimeProvider: TimeProvider {
private var currentTime: Date
private var activeTimers: [MockTimer] = []

init(startTime: Date = Date(timeIntervalSince1970: 0)) {
self.currentTime = startTime
Expand All @@ -14,5 +38,33 @@ final class MockTimeProvider: TimeProvider {

func advance(by interval: TimeInterval) {
currentTime = currentTime.addingTimeInterval(interval)

// Check and fire any timers that should have triggered during this time advancement
let timersToFire = activeTimers.filter { !$0.isCancelled && $0.timerInterval <= interval }
for timer in timersToFire {
_ = (timer.target as AnyObject).perform(timer.selector, with: timer.userInfo)
if !timer.repeats {
removeTimer(timer)
}
}
}

func scheduleTimer(timeInterval: TimeInterval, target: Any, selector: Selector) -> Timer {
let mockTimer = MockTimer(
timeInterval: timeInterval,
target: target,
selector: selector,
mockTimeProvider: self
)
activeTimers.append(mockTimer)
return mockTimer
}

func removeTimer(_ timer: MockTimer) {
activeTimers.removeAll { $0 === timer }
}

func clearScheduledTimers() {
activeTimers.removeAll()
}
}
Loading