diff --git a/app/src/main/scala/AppState.scala b/app/src/main/scala/AppState.scala index 19fdc12d..008fc379 100644 --- a/app/src/main/scala/AppState.scala +++ b/app/src/main/scala/AppState.scala @@ -47,13 +47,17 @@ object AppState: case Some(task) if task.isAcquiredBy(key) => GetTaskResult.Found(task) case Some(task) => GetTaskResult.AcquiredByOther(task) - def updateOrGiveUp(candidates: List[Work.Task]): (AppState, List[Work.Task]) = + def unassignOrGiveUp(candidates: List[Work.Task]): (AppState, List[Work.Task]) = candidates.foldLeft(state -> Nil) { case ((state, xs), task) => - task.clearAssignedKey match - case None => (state - task.id, task :: xs) - case Some(unAssignedTask) => (state.updated(task.id, unAssignedTask), xs) + val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) + (newState, maybeGivenUp.fold(xs)(_ :: xs)) } + def unassignOrGiveUp(task: Work.Task): (AppState, Option[Work.Task]) = + task.clearAssignedKey match + case None => (state - task.id, Some(task)) + case Some(unassignedTask) => (state.updated(task.id, unassignedTask), None) + def acquiredBefore(since: Instant): List[Work.Task] = state.values.filter(_.acquiredBefore(since)).toList diff --git a/app/src/main/scala/Executor.scala b/app/src/main/scala/Executor.scala index 6de38f68..6be34fbb 100644 --- a/app/src/main/scala/Executor.scala +++ b/app/src/main/scala/Executor.scala @@ -68,19 +68,17 @@ object Executor: state.remove(task.id) -> (monitor.success(task) >> client.send(Lila.Response(task.request.id, task.request.moves, uci))) case _ => - val (newState, io) = task.clearAssignedKey match - case None => - state.remove(workId) -> Logger[IO].warn( - s"Give up move due to invalid move $response by $key for $task" - ) - case Some(updated) => state.add(updated) -> IO.unit - newState -> io *> failure(task, key) + val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) + val logs = maybeGivenUp.traverse_(task => + Logger[IO].warn(s"Give up move due to invalid move $response by $key for $task") + ) *> failure(task, key) + newState -> logs def clean(since: Instant): IO[Unit] = ref.flatModify: state => val timedOut = state.acquiredBefore(since) val timedOutLogs = logTimedOut(state, timedOut) - val (newState, gavedUpMoves) = state.updateOrGiveUp(timedOut) + val (newState, gavedUpMoves) = state.unassignOrGiveUp(timedOut) newState -> timedOutLogs *> gavedUpMoves.traverse_(m => Logger[IO].warn(s"Give up move due to clean up: $m")) *> monitor.updateSize(newState) diff --git a/app/src/test/scala/AppStateTest.scala b/app/src/test/scala/AppStateTest.scala index 99c9f636..e031cb6a 100644 --- a/app/src/test/scala/AppStateTest.scala +++ b/app/src/test/scala/AppStateTest.scala @@ -9,8 +9,6 @@ import Arbitraries.given class AppStateTest extends ScalaCheckSuite: - override def scalaCheckInitialSeed = "lwfNzhdC038hCsaHpM4QBkFYs5eFtR9GLPHuzIE08KP=" - test("tasks.fromTasks == identity"): forAll: (state: AppState) => assertEquals(AppState.fromTasks(state.tasks), state) @@ -71,30 +69,30 @@ class AppStateTest extends ScalaCheckSuite: test("updateOrGiveUp is a subset of given tasks"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) givenUp.toSet.subsetOf(candidates.toSet) test("updateOrGiveUp preserves size"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (newState, givenUp) = state.updateOrGiveUp(candidates) + val (newState, givenUp) = state.unassignOrGiveUp(candidates) newState.size + givenUp.size == state.size test("all given up tasks are outOfTries"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) givenUp.forall(_.isOutOfTries) test("all candidates that are not given up are not outOfTries"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) val rest = candidates.filterNot(givenUp.contains) rest.forall(!_.isOutOfTries) test("after cleanup, acquiredBefore is empty"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (newState, _) = state.updateOrGiveUp(candidates) + val (newState, _) = state.unassignOrGiveUp(candidates) newState.acquiredBefore(before).isEmpty