Merge pull request #288 from Masynchin/unassignOrGiveUp

Add unassignOrGiveUp
lichess-org · Mar 14, 2024 · 7463f49 · 7463f49
2 parents 21320b1 + bec9dbf
commit 7463f49
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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