From 5504aa0ac77ba0fe7827663d682b8d8184840581 Mon Sep 17 00:00:00 2001 From: updraft0 Date: Fri, 21 Jun 2024 12:43:08 +0100 Subject: [PATCH] fix(map,delete): removing multiple systems now properly updates other systems' signatures when they were connected --- .../updraft0/controltower/db/query/map.scala | 7 ++ .../updraft0/controltower/protocol/map.scala | 11 +- .../controltower/server/auth/Esi.scala | 2 +- .../controltower/server/db/MapQueries.scala | 6 +- .../controltower/server/map/MapReactive.scala | 103 ++++++++++-------- .../controltower/server/map/MapSession.scala | 24 +++- .../page/map/view/MapController.scala | 20 +++- 7 files changed, 103 insertions(+), 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 index 0271648..8e93d36 100644 --- 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 index a636f89..6231375 100644 --- 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 index eb702a7..a23c030 100644 --- 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 index c2f4a08..0a8d66b 100644 --- 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 index fdb9d9a..1f17919 100644 --- 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 index a8d96fd..7e83ae8 100644 --- 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 index 10022c6..96b2f14 100644 --- 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(