From b93edb050bd6dd79edb6273953abb74b93caa8c7 Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Thu, 4 May 2023 01:30:20 +0200 Subject: [PATCH] Force pending saves if client closes abruptly (#6514) Force pending saves when JsonSession is terminated for the client. Potentially closes #6395. --- .../text/CollaborativeBuffer.scala | 18 ++- .../websocket/json/BaseServerTest.scala | 8 ++ .../websocket/json/TextOperationsTest.scala | 128 +++++++++++++++++- .../jsonrpc/test/JsonRpcServerTestKit.scala | 4 + .../LanguageServerController.scala | 5 + 5 files changed, 159 insertions(+), 4 deletions(-) diff --git a/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala b/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala index c9510c0d5293..24f3056e6eac 100644 --- a/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala +++ b/engine/language-server/src/main/scala/org/enso/languageserver/text/CollaborativeBuffer.scala @@ -136,7 +136,23 @@ class CollaborativeBuffer( case JsonSessionTerminated(client) => if (clients.contains(client.clientId)) { - removeClient(buffer, clients, lockHolder, client.clientId, autoSave) + autoSave.get(client.clientId) match { + case Some((contentVersion, cancellable)) => + cancellable.cancel() + saveFile( + buffer, + clients, + lockHolder, + client.clientId, + contentVersion, + autoSave, + isAutoSave = true, + onClose = Some(client.clientId), + reportProgress = Some(sender()) + ) + case None => + removeClient(buffer, clients, lockHolder, client.clientId, autoSave) + } } case CloseFile(clientId, _) => diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/BaseServerTest.scala b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/BaseServerTest.scala index e34496602e46..a7fef1577b8c 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/BaseServerTest.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/BaseServerTest.scala @@ -362,6 +362,14 @@ class BaseServerTest client } + def getInitialisedWsClientAndId( + debug: Boolean = false + ): (WsTestClient, ClientId) = { + val client = new WsTestClient(address, debugMessages = debug) + val uuid = initSession(client) + (client, uuid) + } + private def initSession(client: WsTestClient): UUID = { initPackage val clientId = UUID.randomUUID() diff --git a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala index 8169e5924757..50c35949babd 100644 --- a/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala +++ b/engine/language-server/src/test/scala/org/enso/languageserver/websocket/json/TextOperationsTest.scala @@ -2,8 +2,9 @@ package org.enso.languageserver.websocket.json import akka.testkit.TestProbe import io.circe.literal._ -import org.enso.languageserver.event.BufferClosed +import org.enso.languageserver.event.{BufferClosed, JsonSessionTerminated} import org.enso.languageserver.filemanager.Path +import org.enso.languageserver.session.JsonSession import org.enso.testkit.FlakySpec import scala.concurrent.duration._ @@ -2601,8 +2602,6 @@ class TextOperationsTest extends BaseServerTest with FlakySpec { "result": null } """) - Thread.sleep(5.seconds.toMillis) - // Should return the original contents of the file client2.send(json""" { "jsonrpc": "2.0", "method": "file/read", @@ -2621,6 +2620,129 @@ class TextOperationsTest extends BaseServerTest with FlakySpec { "result": { "contents": "bar123456789foo" } } """) + + } + + "be triggered when a client closed session abruptly" in { + this.timingsConfig.withAutoSave(10.seconds) + + val (client1, client1Id) = getInitialisedWsClientAndId() + val client2 = getInitialisedWsClient() + client1.send(json""" + { "jsonrpc": "2.0", + "method": "file/write", + "id": 0, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + }, + "contents": "123456789" + } + } + """) + client1.expectJson(json""" + { "jsonrpc": "2.0", + "id": 0, + "result": null + } + """) + client1.send(json""" + { "jsonrpc": "2.0", + "method": "text/openFile", + "id": 1, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client1.expectJson(json""" + { + "jsonrpc" : "2.0", + "id" : 1, + "result" : { + "writeCapability" : { + "method" : "text/canEdit", + "registerOptions" : { + "path" : { + "rootId" : $testContentRootId, + "segments" : [ + "foo.txt" + ] + } + } + }, + "content" : "123456789", + "currentVersion" : "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522" + } + } + """) + + client1.send(json""" + { "jsonrpc": "2.0", + "method": "text/applyEdit", + "id": 2, + "params": { + "edit": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + }, + "oldVersion": "5795c3d628fd638c9835a4c79a55809f265068c88729a1a3fcdf8522", + "newVersion": "ebe55342f9c8b86857402797dd723fb4a2174e0b56d6ace0a6929ec3", + "edits": [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 0, "character": 0 } + }, + "text": "bar" + }, + { + "range": { + "start": { "line": 0, "character": 12 }, + "end": { "line": 0, "character": 12 } + }, + "text": "foo" + } + ] + } + } + } + """) + client1.expectJson(json""" + { "jsonrpc": "2.0", + "id": 2, + "result": null + } + """) + + // Simulate client1 closing + system.eventStream.publish( + JsonSessionTerminated(JsonSession(client1Id, client1.actorRef())) + ) + client2.send(json""" + { "jsonrpc": "2.0", + "method": "file/read", + "id": 4, + "params": { + "path": { + "rootId": $testContentRootId, + "segments": [ "foo.txt" ] + } + } + } + """) + client2.expectJson(json""" + { "jsonrpc": "2.0", + "id": 4, + "result": { "contents": "bar123456789foo" } + } + """) + } } diff --git a/lib/scala/json-rpc-server-test/src/main/scala/org/enso/jsonrpc/test/JsonRpcServerTestKit.scala b/lib/scala/json-rpc-server-test/src/main/scala/org/enso/jsonrpc/test/JsonRpcServerTestKit.scala index a700a080c1be..e26dc4a948f9 100644 --- a/lib/scala/json-rpc-server-test/src/main/scala/org/enso/jsonrpc/test/JsonRpcServerTestKit.scala +++ b/lib/scala/json-rpc-server-test/src/main/scala/org/enso/jsonrpc/test/JsonRpcServerTestKit.scala @@ -140,6 +140,10 @@ abstract class JsonRpcServerTestKit } def expectNoMessage(): Unit = outActor.expectNoMessage() + + def actorRef(): ActorRef = { + outActor.ref + } } } diff --git a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/LanguageServerController.scala b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/LanguageServerController.scala index 0c58130a6cc9..283066070324 100644 --- a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/LanguageServerController.scala +++ b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/LanguageServerController.scala @@ -292,6 +292,11 @@ class LanguageServerController( maybeRequester.foreach(_ ! ServerShutdownTimedOut) stop() + case ClientDisconnected(clientId) => + logger.debug( + s"Received client ($clientId) disconnect request during shutdown. Ignoring." + ) + case m: StartServer => // This instance has not yet been shut down. Retry context.parent.forward(Retry(m))