fix(map,delete): removing multiple systems now properly updates other…

… systems' signatures when they were connected
updraft0 · Jun 21, 2024 · 5504aa0 · 5504aa0
1 parent 8ec24a8
commit 5504aa0
Show file tree

Hide file tree

Showing 7 changed files with 103 additions and 70 deletions.
diff --git a/db/src/main/scala/org/updraft0/controltower/db/query/map.scala b/db/src/main/scala/org/updraft0/controltower/db/query/map.scala
@@ -323,6 +323,13 @@ object map:
         .delete
     )
 
+  def deleteMapSystemDisplays(mapId: MapId, systemIds: Chunk[SystemId]): DbOperation[Long] =
+    ctx.run(
+      mapSystemDisplay
+        .filter(msd => msd.mapId == lift(mapId) && liftQuery(systemIds).contains(msd.systemId))
+        .delete
+    )
+
   def deleteMapWormholeConnection(mapId: MapId, id: ConnectionId, byCharacterId: CharacterId): DbOperation[Long] =
     ctx.run(
       mapWormholeConnection

diff --git a/protocol/shared/src/main/scala/org/updraft0/controltower/protocol/map.scala b/protocol/shared/src/main/scala/org/updraft0/controltower/protocol/map.scala
@@ -312,10 +312,6 @@ enum MapRequest derives CanEqual:
     */
   case RemoveSystemConnection(connectionId: ConnectionId)
 
-// TODO: have a think about connections and when they get cleaned up (delete connection *and* delete signature?)
-// TODO: enforce the level of permissions that a user can have! (and document that)
-// TODO: move to opaque types
-
 enum MapMessage:
   case CharacterLocations(locations: Map[constant.SystemId, Array[CharacterLocation]])
   case ConnectionSnapshot(connection: MapWormholeConnectionWithSigs)
@@ -333,10 +329,11 @@ enum MapMessage:
       connections: Map[ConnectionId, MapWormholeConnectionWithSigs]
   )
   case SystemDisplayUpdate(systemId: SystemId, name: Option[String], displayData: SystemDisplayData)
-  case SystemRemoved(
-      removedSystem: MapSystemSnapshot,
+  case SystemsRemoved(
+      removedSystemIds: Array[SystemId],
       removedConnectionIds: Array[ConnectionId],
-      connections: Map[ConnectionId, MapWormholeConnectionWithSigs]
+      updatedSystems: Array[MapSystemSnapshot],
+      updatedConnections: Map[ConnectionId, MapWormholeConnectionWithSigs]
   )
   case ServerStatus(status: MapServerStatus)
 

diff --git a/server/src/main/scala/org/updraft0/controltower/server/auth/Esi.scala b/server/src/main/scala/org/updraft0/controltower/server/auth/Esi.scala
@@ -81,7 +81,7 @@ private def extractAndValidateJwt(r: JwtAuthResponse): ZIO[Config, EsiError, (Jw
     characterId <- ZIO
       .attempt(esiInfo.sub.stripPrefix(CharacterSubjectPrefix).toLong)
       .mapError(e => EsiError.ValidationError(s"Cannot get character id: ${e}"))
-    _ <- ZIO.logDebug("validated jwt") // TODO: add aspects for character id
+    _ <- ZIO.logTrace("validated jwt")
   yield (r, EsiTokenMeta(CharacterId(characterId), esiInfo.name, esiInfo.owner, Instant.ofEpochSecond(esiInfo.exp)))
 
 private def validateEsiInfo(esiInfo: JwtEsiInfo, loginHost: String): IO[EsiError, Unit] =

diff --git a/server/src/main/scala/org/updraft0/controltower/server/db/MapQueries.scala b/server/src/main/scala/org/updraft0/controltower/server/db/MapQueries.scala
@@ -251,7 +251,7 @@ object MapQueries:
       (for
         map <- mapModel.filter(_.id == lift(mapId))
         sys <- mapSystem.join(ms => ms.mapId == map.id && lift(systemId).forall(_ == ms.systemId))
-        dis <- mapSystemDisplay.leftJoin(sd =>
+        dis <- mapSystemDisplay.join(sd =>
           sd.systemId == sys.systemId && sd.mapId == sys.mapId && sd.displayType == map.displayType
         )
         mss <- mapSystemStructure.leftJoin(ss => ss.systemId == sys.systemId && ss.mapId == sys.mapId && !ss.isDeleted)
@@ -260,11 +260,11 @@ object MapQueries:
         mhc <- mapWormholeConnection.leftJoin(whc =>
           whc.mapId == map.id && (whc.fromSystemId == sys.systemId || whc.toSystemId == sys.systemId) && !whc.isDeleted
         )
-      yield (sys, dis.map(_.data), mss, msn, msi, mhc)).groupByMap((ms, _, _, _, _, _) => (ms))(
+      yield (sys, dis.data, mss, msn, msi, mhc)).groupByMap((ms, _, _, _, _, _) => (ms))(
         (ms, dis, mss, msn, msi, mhc) =>
           (
             ms,
-            dis,
+            Some(dis),
             jsonGroupArrayFilterNullDistinct[model.MapSystemStructure](
               jsonObject11(
                 "mapId",

diff --git a/server/src/main/scala/org/updraft0/controltower/server/map/MapReactive.scala b/server/src/main/scala/org/updraft0/controltower/server/map/MapReactive.scala
@@ -71,6 +71,9 @@ private[map] case class MapState(
 
   def connectionsForSystem(id: SystemId): Map[ConnectionId, MapWormholeConnectionWithSigs] =
     systems(id).connections.map(c => c.id -> connections(c.id)).toMap
+  def connectionsForSystems(ids: Chunk[SystemId]): Chunk[MapWormholeConnection] =
+    ids.foldLeft(Chunk.empty)((s, sId) => s ++ systems.get(sId).map(_.connections).getOrElse(Chunk.empty))
+
   def connectionRanksForSystem(id: SystemId): Map[ConnectionId, MapWormholeConnectionRank] =
     systems(id).connections.map(c => c.id -> connectionRanks(c.id)).toMap
 
@@ -110,27 +113,18 @@ private[map] case class MapState(
       }
     )
 
-  def removeSystem(
-      removedSystemId: model.SystemId,
+  def removeSystems(
+      removedSystemIds: Chunk[model.SystemId],
       removedConnectionIds: Chunk[ConnectionId],
-      otherSystemIds: Chunk[model.SystemId],
+      refreshedSystemIds: Chunk[model.SystemId],
       connectionRanks: List[MapWormholeConnectionRank],
       connectionsWithSigs: List[MapWormholeConnectionWithSigs]
   ): MapState =
     this.copy(
-      systems = otherSystemIds.foldLeft(this.systems.updatedWith(removedSystemId) {
-        case None => None
-        case Some(prev) =>
-          Some(
-            prev.copy(
-              display = None,
-              connections = Chunk.empty,
-              signatures = prev.signatures.filterNot(_.wormholeConnectionId.exists(removedConnectionIds.contains))
-            )
-          )
-      })((ss, nextSystemId) =>
-        ss.updatedWith(nextSystemId) {
-          case None => None
+      systems = refreshedSystemIds.foldLeft(this.systems.removedAll(removedSystemIds))((ss, refreshedSystemId) =>
+        ss.updatedWith(refreshedSystemId) {
+          case None =>
+            throw new IllegalStateException(s"System ${refreshedSystemId} was refreshed but not present in state")
           case Some(prev) =>
             Some(
               prev.copy(
@@ -267,7 +261,7 @@ enum MapRequest derives CanEqual:
   case UpdateSystemSignatures(systemId: SystemId, replaceAll: Boolean, scanned: List[NewMapSystemSignature])
   case RenameSystem(systemId: SystemId, name: Option[String])
   case RemoveSystem(systemId: SystemId)
-  case RemoveSystems(systemIds: Chunk[SystemId])
+  case RemoveSystems(systemIds: NonEmptyChunk[SystemId])
   case RemoveSystemSignatures(systemId: SystemId, signatures: Option[NonEmptyChunk[SigId]])
   case RemoveSystemConnection(connectionId: ConnectionId)
   // internals
@@ -294,11 +288,12 @@ enum MapResponse:
       connectionRanks: Map[ConnectionId, MapWormholeConnectionRank]
   )
   case SystemDisplayUpdate(systemId: SystemId, name: Option[String], displayData: model.SystemDisplayData)
-  case SystemRemoved(
-      removedSystem: MapSystemWithAll,
+  case SystemsRemoved(
+      removedSystemIds: Chunk[SystemId],
       removedConnectionIds: Chunk[ConnectionId],
-      connections: Map[ConnectionId, MapWormholeConnectionWithSigs],
-      connectionRanks: Map[ConnectionId, MapWormholeConnectionRank]
+      updatedSystems: Chunk[MapSystemWithAll],
+      updatedConnections: Map[ConnectionId, MapWormholeConnectionWithSigs],
+      updatedConnectionRanks: Map[ConnectionId, MapWormholeConnectionRank]
   )
   case CharacterLocations(locations: Map[CharacterId, CharacterLocationState.InSystem])
 
@@ -388,18 +383,22 @@ object MapEntity extends ReactiveEntity[MapEnv, MapId, MapState, Identified[MapR
                 )
               case Identified(Some(sid), rs: MapRequest.RemoveSystem) =>
                 whenSystemExists(rs.systemId, state)(
-                  identified(sid, "removeFromDisplay", removeSystemAndConnection(mapId, state, sid, rs.systemId))
+                  identified(
+                    sid,
+                    "removeFromDisplay",
+                    removeSystemsAndConnections(mapId, state, sid, NonEmptyChunk(rs.systemId))
+                  )
                 )
               case Identified(Some(sid), rss: MapRequest.RemoveSystems) =>
-                // TODO this could be optimized further
-                foldLeft(
-                  state,
-                  rss.systemIds,
-                  (systemId, state) =>
-                    whenSystemExists(systemId, state)(
-                      identified(sid, "removeFromDisplay", removeSystemAndConnection(mapId, state, sid, systemId))
+                NonEmptyChunk
+                  .fromChunk(rss.systemIds.filter(state.hasSystem))
+                  .fold(ZIO.logDebug("no-op removing systems that do not exist").as(state -> Chunk.empty))(systemIds =>
+                    identified(
+                      sid,
+                      "removeMultipleFromDisplay",
+                      removeSystemsAndConnections(mapId, state, sid, systemIds)
                     )
-                )
+                  )
               case Identified(Some(sid), rsc: MapRequest.RemoveSystemConnection) =>
                 identified(sid, "removeSystemConnection", removeSystemConnection(mapId, state, sid, rsc))
               case Identified(Some(sid), rss: MapRequest.RemoveSystemSignatures) =>
@@ -630,42 +629,50 @@ object MapEntity extends ReactiveEntity[MapEnv, MapId, MapState, Identified[MapR
         else reloadSystemSnapshot(mapId, uss.systemId)(state)
     yield resp
 
-  private def removeSystemAndConnection(
+  private def removeSystemsAndConnections(
       mapId: MapId,
       state: MapState,
       sessionId: MapSessionId,
-      systemId: SystemId
+      systemIds: NonEmptyChunk[SystemId]
   ) =
-    val connections   = state.connectionsForSystem(systemId)
-    val connectionIds = Chunk.from(connections.valuesIterator.map(_.connection.id))
-    val otherSystemIds = Chunk.from(connections.valuesIterator.map(_.connection.toSystemId).filter(_ != systemId)) ++
-      Chunk.from(connections.valuesIterator.map(_.connection.fromSystemId).filter(_ != systemId))
+    val connections   = state.connectionsForSystems(systemIds.toChunk)
+    val connectionIds = connections.map(_.id).sorted.dedupe
+    val refreshedSystemIds = connections
+      .flatMap(c => Chunk(c.toSystemId, c.fromSystemId).filterNot(sId => systemIds.contains(sId)))
+      .sorted
+      .dedupe
     for
+      // trace log
+      _ <- ZIO.logTrace(
+        s"Removing multiple systems [${systemIds.mkString(",")}], caused ${connectionIds.size} connection removals and ${refreshedSystemIds.size} system updates"
+      )
       // mark connections as removed
-      _ <- query.map.deleteMapWormholeConnections(mapId, connectionIds, sessionId.characterId)
+      _                  <- query.map.deleteMapWormholeConnections(mapId, connectionIds, sessionId.characterId)
+      deletedConnections <- query.map.getWormholeConnections(mapId, connectionIds, isDeleted = true)
       // remove signatures that have those connections
       _ <- query.map.deleteSignaturesWithConnectionIds(mapId, connectionIds, sessionId.characterId)
       // remove the display of the system
-      _ <- query.map.deleteMapSystemDisplay(mapId, systemId)
+      _ <- query.map.deleteMapSystemDisplays(mapId, systemIds.toChunk)
       // recompute all the connection ranks
       connectionRanks <- MapQueries.getWormholeConnectionRanksAll(mapId)
-      // load all the affected connections with sigs
-      connectionsWithSigs <- MapQueries.getWormholeConnectionsWithSigsBySystemIds(mapId, otherSystemIds)
+      // load all the refreshed connections with sigs
+      connectionsWithSigs <- MapQueries.getWormholeConnectionsWithSigsBySystemIds(mapId, refreshedSystemIds)
     yield withState(
-      state.removeSystem(
-        systemId,
+      state.removeSystems(
+        systemIds.toChunk,
         connectionIds,
-        otherSystemIds,
+        refreshedSystemIds,
         connectionRanks,
         connectionsWithSigs
       )
     )(nextState =>
       nextState -> broadcast(
-        MapResponse.SystemRemoved(
-          state.systems(systemId),
-          connectionIds,
-          connectionsWithSigs.map(whcs => whcs.connection.id -> whcs).toMap,
-          connectionRanks = nextState.connectionRanks
+        MapResponse.SystemsRemoved(
+          removedSystemIds = systemIds.toChunk,
+          removedConnectionIds = connectionIds,
+          updatedSystems = refreshedSystemIds.map(nextState.systems),
+          updatedConnections = connectionsWithSigs.map(whcs => whcs.connection.id -> whcs).toMap,
+          updatedConnectionRanks = nextState.connectionRanks
         )
       )
     )

diff --git a/server/src/main/scala/org/updraft0/controltower/server/map/MapSession.scala b/server/src/main/scala/org/updraft0/controltower/server/map/MapSession.scala
@@ -238,7 +238,11 @@ object MapSession:
     case protocol.MapRequest.RemoveSystem(systemId) =>
       ctx.mapQ.offer(Identified(Some(ctx.sessionId), MapRequest.RemoveSystem(systemId)))
     case protocol.MapRequest.RemoveSystems(systemIds) =>
-      ctx.mapQ.offer(Identified(Some(ctx.sessionId), MapRequest.RemoveSystems(Chunk.fromArray(systemIds))))
+      NonEmptyChunk
+        .fromChunk(Chunk.fromArray(systemIds))
+        .fold(ZIO.unit)(systemIds =>
+          ctx.mapQ.offer(Identified(Some(ctx.sessionId), MapRequest.RemoveSystems(systemIds)))
+        )
     // signatures
     case addSig: protocol.MapRequest.AddSystemSignature =>
       ctx.mapQ.offer(
@@ -315,13 +319,21 @@ private def toProto(msg: MapResponse): Option[protocol.MapMessage] = msg match
     )
   case MapResponse.SystemDisplayUpdate(systemId, name, displayData) =>
     Some(protocol.MapMessage.SystemDisplayUpdate(systemId, name, toProtoDisplay(displayData)))
-  case MapResponse.SystemRemoved(removedSystem, removedConnectionIds, connections, connectionRanks) =>
+  case MapResponse.SystemsRemoved(
+        removedSystemIds,
+        removedConnectionIds,
+        updatedSystems,
+        updatedConnections,
+        updatedConnectionRanks
+      ) =>
     Some(
-      protocol.MapMessage.SystemRemoved(
-        removedSystem = toProtoSystemSnapshot(removedSystem),
+      protocol.MapMessage.SystemsRemoved(
+        removedSystemIds = removedSystemIds.toArray,
         removedConnectionIds = removedConnectionIds.toArray,
-        connections =
-          connections.view.mapValues(c => toProtoConnectionWithSigs(c, connectionRanks(c.connection.id))).toMap
+        updatedSystems = updatedSystems.map(toProtoSystemSnapshot).toArray,
+        updatedConnections = updatedConnections.view
+          .mapValues(c => toProtoConnectionWithSigs(c, updatedConnectionRanks(c.connection.id)))
+          .toMap
       )
     )
   case MapResponse.CharacterLocations(locationMap) =>

diff --git a/ui/src/main/scala/controltower/page/map/view/MapController.scala b/ui/src/main/scala/controltower/page/map/view/MapController.scala
@@ -138,6 +138,8 @@ class MapController(rds: ReferenceDataStore, val clock: Signal[Instant])(using O
         case (MapAction.Remove(systemId), _) =>
           Some(MapRequest.RemoveSystem(systemId))
         case (MapAction.RemoveMultiple(systemIds), _) =>
+          // reset selection
+          bulkSelectedSystemIds.set(Array.empty)
           Some(MapRequest.RemoveSystems(systemIds))
         case (MapAction.RemoveConnection(connectionId), _) =>
           Some(MapRequest.RemoveSystemConnection(connectionId))
@@ -232,18 +234,26 @@ class MapController(rds: ReferenceDataStore, val clock: Signal[Instant])(using O
             }
             .onComplete(_ => doUpdate())
         else doUpdate()
-      case MapMessage.SystemRemoved(removedSystem, removedConnectionIds, updatedConnections) =>
-        val systemId = removedSystem.system.systemId
+      case MapMessage.SystemsRemoved(removedSystemIds, removedConnectionIds, updatedSystems, updatedConnections) =>
         Var.update(
-          allSystems.current -> ((map: Map[Long, MapSystemSnapshot]) => map.updated(systemId, removedSystem)),
-          selectedSystemId   -> ((sOpt: Option[Long]) => sOpt.filterNot(_ == systemId)),
-          pos.systemDisplayData(systemId) -> ((_: Option[SystemDisplayData]) => None),
+          allSystems.current -> ((map: Map[Long, MapSystemSnapshot]) =>
+            updatedSystems.foldLeft(map.removedAll(removedSystemIds)) { case (st, mss) =>
+              st.updated(mss.system.systemId, mss)
+            }
+          ),
+          selectedSystemId      -> ((sOpt: Option[Long]) => sOpt.filterNot(removedSystemIds.contains)),
+          bulkSelectedSystemIds -> ((selection: Array[SystemId]) => selection.filterNot(removedSystemIds.contains)),
           allConnections.current -> ((conns: Map[ConnectionId, MapWormholeConnectionWithSigs]) =>
             updatedConnections.foldLeft(conns.removedAll(removedConnectionIds)) { case (conns, (cid, whc)) =>
               conns.updated(cid, whc)
             }
           )
         )
+        Var.update(
+          removedSystemIds.toSeq.map(systemId =>
+            pos.systemDisplayData(systemId) -> ((_: Option[SystemDisplayData]) => None)
+          )*
+        )
       case MapMessage.SystemSnapshot(systemId, system, connections) =>
         inline def doUpdate(@unused solarSystem: SolarSystem) =
           Var.update(