From 74c0718604f1736f85eb793d1c021537bc1ccde4 Mon Sep 17 00:00:00 2001 From: Dmitry Bushev Date: Thu, 20 Apr 2023 17:57:37 +0100 Subject: [PATCH] Project folder not renamed after project name change (#6369) close #6254 Changelog: - fix: race when the actor system may be stopped before the shutdown hooks are executed - fix: project management spec - fix: recover from `readLine` failure during the shutdown --- .../jsonrpc/test/JsonRpcServerTestKit.scala | 2 +- .../org/enso/jsonrpc/JsonRpcServer.scala | 3 +- .../projectmanager/boot/ProjectManager.scala | 9 +- .../ShutdownHookActivator.scala | 22 ++- .../languageserver/ShutdownHookRunner.scala | 6 +- .../protocol/ProjectManagementApiSpec.scala | 173 +++++++++++------- 6 files changed, 131 insertions(+), 84 deletions(-) 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 074fcda0747..a700a080c1b 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 @@ -65,7 +65,7 @@ abstract class JsonRpcServerTestKit } override def afterEach(): Unit = { - val _ = binding.unbind() + Await.ready(binding.terminate(10.seconds.dilated), 15.seconds.dilated) super.afterEach() } diff --git a/lib/scala/json-rpc-server/src/main/scala/org/enso/jsonrpc/JsonRpcServer.scala b/lib/scala/json-rpc-server/src/main/scala/org/enso/jsonrpc/JsonRpcServer.scala index 48ab41a0a1b..f9af9a2398b 100644 --- a/lib/scala/json-rpc-server/src/main/scala/org/enso/jsonrpc/JsonRpcServer.scala +++ b/lib/scala/json-rpc-server/src/main/scala/org/enso/jsonrpc/JsonRpcServer.scala @@ -1,6 +1,5 @@ package org.enso.jsonrpc -import java.util.UUID import akka.NotUsed import akka.actor.{ActorRef, ActorSystem, Props} import akka.http.scaladsl.Http @@ -11,6 +10,8 @@ import akka.stream.scaladsl.{Flow, Sink, Source} import akka.stream.{Materializer, OverflowStrategy} import com.typesafe.scalalogging.LazyLogging +import java.util.UUID + import scala.concurrent.duration._ import scala.concurrent.{ExecutionContext, Future} diff --git a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/boot/ProjectManager.scala b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/boot/ProjectManager.scala index 22618c5f904..e1b1dbc4fb6 100644 --- a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/boot/ProjectManager.scala +++ b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/boot/ProjectManager.scala @@ -73,7 +73,7 @@ object ProjectManager extends ZIOAppDefault with LazyLogging { for { binding <- bindServer(mainModule) _ <- logServerStartup() - _ <- readLine + _ <- tryReadLine _ <- ZIO.succeed { logger.info("Stopping server...") } _ <- ZIO.succeed { binding.unbind() } _ <- killAllLanguageServer(mainModule) @@ -82,6 +82,13 @@ object ProjectManager extends ZIOAppDefault with LazyLogging { } yield () } + private def tryReadLine: ZIO[ZAny, Nothing, String] = + readLine.catchAll { err => + ZIO + .succeed { logger.warn("Failed to read line.", err) } + .as("") + } + private def killAllLanguageServer(mainModule: MainModule[ZIO[ZAny, +*, +*]]) = mainModule.languageServerGateway .killAllServers() diff --git a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookActivator.scala b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookActivator.scala index ff281411ff1..80f0b706c4a 100644 --- a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookActivator.scala +++ b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookActivator.scala @@ -1,7 +1,5 @@ package org.enso.projectmanager.infrastructure.languageserver -import java.util.UUID - import akka.actor.{Actor, Props} import com.typesafe.scalalogging.LazyLogging import org.enso.projectmanager.control.core.CovariantFlatMap @@ -9,11 +7,14 @@ import org.enso.projectmanager.control.effect.Exec import org.enso.projectmanager.event.ProjectEvent.ProjectClosed import org.enso.projectmanager.infrastructure.languageserver.ShutdownHookActivator.{ ArePendingShutdownHooks, - RegisterShutdownHook + RegisterShutdownHook, + ShutdownHooksFired } import org.enso.projectmanager.infrastructure.shutdown.ShutdownHook import org.enso.projectmanager.util.UnhandledLogging +import java.util.UUID + /** The ShutdownHookActivator is responsible for receiving asynchronously * [[ProjectClosed]] events and invoking all shutdown hooks for the closed * project. @@ -44,9 +45,11 @@ class ShutdownHookActivator[F[+_, +_]: Exec: CovariantFlatMap] context.actorOf( ShutdownHookRunner.props[F](projectId, projectHooks.reverse) ) - context.become(running(hooks - projectId)) } + case ShutdownHooksFired(projectId) => + context.become(running(hooks - projectId)) + case ArePendingShutdownHooks => val arePending = hooks.values.map(_.size).sum != 0 sender() ! arePending @@ -56,17 +59,18 @@ class ShutdownHookActivator[F[+_, +_]: Exec: CovariantFlatMap] object ShutdownHookActivator { - /** A command used to register a project shutdown hook. - */ - case class RegisterShutdownHook[F[+_, +_]]( + /** A command used to register a project shutdown hook. */ + sealed case class RegisterShutdownHook[F[+_, +_]]( projectId: UUID, hook: ShutdownHook[F] ) - /** Requests an activator if there are pending shutdown hooks. - */ + /** Requests an activator if there are pending shutdown hooks. */ case object ArePendingShutdownHooks + /** Notifies that all shutdown hooks are fired for the project. */ + sealed case class ShutdownHooksFired(projectId: UUID) + /** Creates a configuration object used to create a [[ShutdownHookActivator]]. * * @return a configuration object diff --git a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookRunner.scala b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookRunner.scala index 3fec9bf569a..3b2d8f32e32 100644 --- a/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookRunner.scala +++ b/lib/scala/project-manager/src/main/scala/org/enso/projectmanager/infrastructure/languageserver/ShutdownHookRunner.scala @@ -1,7 +1,5 @@ package org.enso.projectmanager.infrastructure.languageserver -import java.util.UUID - import akka.actor.{Actor, Props, Status} import akka.pattern.pipe import com.typesafe.scalalogging.LazyLogging @@ -12,6 +10,8 @@ import org.enso.projectmanager.infrastructure.languageserver.ShutdownHookRunner. import org.enso.projectmanager.infrastructure.shutdown.ShutdownHook import org.enso.projectmanager.util.UnhandledLogging +import java.util.UUID + /** An actor that invokes a shutdown hook. * * @param projectId a project id for which a hook is invoked @@ -42,10 +42,12 @@ class ShutdownHookRunner[F[+_, +_]: Exec: CovariantFlatMap]( s"An error occurred during running shutdown hooks for project [$projectId].", th ) + context.parent ! ShutdownHookActivator.ShutdownHooksFired(projectId) context.stop(self) case Right(_) => logger.info("All shutdown hooks fired for project [{}].", projectId) + context.parent ! ShutdownHookActivator.ShutdownHooksFired(projectId) context.stop(self) } diff --git a/lib/scala/project-manager/src/test/scala/org/enso/projectmanager/protocol/ProjectManagementApiSpec.scala b/lib/scala/project-manager/src/test/scala/org/enso/projectmanager/protocol/ProjectManagementApiSpec.scala index b3c706c47ce..6307cef950a 100644 --- a/lib/scala/project-manager/src/test/scala/org/enso/projectmanager/protocol/ProjectManagementApiSpec.scala +++ b/lib/scala/project-manager/src/test/scala/org/enso/projectmanager/protocol/ProjectManagementApiSpec.scala @@ -7,17 +7,14 @@ import org.apache.commons.io.FileUtils import org.enso.editions.SemVerJson._ import org.enso.projectmanager.{BaseServerSpec, ProjectManagementOps} import org.enso.testkit.{FlakySpec, RetrySpec} -import org.scalatest.Ignore import java.io.File import java.nio.file.{Files, Paths} import java.util.UUID + import scala.concurrent.duration._ import scala.io.Source -// TODO [DB] The suite became quite flaky and frequently fails the CI. -// Disabled until #479 is implemented. -@Ignore class ProjectManagementApiSpec extends BaseServerSpec with FlakySpec @@ -120,7 +117,7 @@ class ProjectManagementApiSpec } "fail when the project with the same name exists" in { - val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) client.send(json""" { "jsonrpc": "2.0", "method": "project/create", @@ -130,12 +127,13 @@ class ProjectManagementApiSpec } } """) + val projectId = getGeneratedUUID client.expectJson(json""" { "jsonrpc" : "2.0", "id" : 1, "result" : { - "projectId" : $getGeneratedUUID, + "projectId" : $projectId, "projectName" : "Foo" } } @@ -159,14 +157,18 @@ class ProjectManagementApiSpec } } """) + + //teardown + deleteProject(projectId) + } "create project structure" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName) + val projectId = createProject(projectName) val projectDir = new File(userProjectDir, projectName) val packageFile = new File(projectDir, "package.yaml") @@ -176,14 +178,18 @@ class ProjectManagementApiSpec packageFile shouldBe Symbol("file") mainEnso shouldBe Symbol("file") meta shouldBe Symbol("file") + + //teardown + deleteProject(projectId) } "create project from default template" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName, projectTemplate = Some("default")) + val projectId = + createProject(projectName, projectTemplate = Some("default")) val projectDir = new File(userProjectDir, projectName) val packageFile = new File(projectDir, "package.yaml") @@ -193,14 +199,18 @@ class ProjectManagementApiSpec packageFile shouldBe Symbol("file") mainEnso shouldBe Symbol("file") meta shouldBe Symbol("file") + + //teardown + deleteProject(projectId) } "create project from orders template" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName, projectTemplate = Some("orders")) + val projectId = + createProject(projectName, projectTemplate = Some("orders")) val projectDir = new File(userProjectDir, projectName) val packageFile = new File(projectDir, "package.yaml") @@ -213,14 +223,18 @@ class ProjectManagementApiSpec mainEnso shouldBe Symbol("file") storeDataXlsx shouldBe Symbol("file") meta shouldBe Symbol("file") + + //teardown + deleteProject(projectId) } "create project from restaurants template" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName, projectTemplate = Some("restaurants")) + val projectId = + createProject(projectName, projectTemplate = Some("restaurants")) val projectDir = new File(userProjectDir, projectName) val packageFile = new File(projectDir, "package.yaml") @@ -236,14 +250,18 @@ class ProjectManagementApiSpec laDistrictsCsv shouldBe Symbol("file") restaurantsCsv shouldBe Symbol("file") meta shouldBe Symbol("file") + + //teardown + deleteProject(projectId) } "create project from stargazers template" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName, projectTemplate = Some("stargazers")) + val projectId = + createProject(projectName, projectTemplate = Some("stargazers")) val projectDir = new File(userProjectDir, projectName) val packageFile = new File(projectDir, "package.yaml") @@ -253,15 +271,19 @@ class ProjectManagementApiSpec packageFile shouldBe Symbol("file") mainEnso shouldBe Symbol("file") meta shouldBe Symbol("file") + + //teardown + deleteProject(projectId) } "find a name when project is created from template" in { val projectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName, projectTemplate = Some("default")) - createProject( + val projectId1 = + createProject(projectName, projectTemplate = Some("default")) + val projectId2 = createProject( projectName, projectTemplate = Some("default"), nameSuffix = Some(1) @@ -271,10 +293,14 @@ class ProjectManagementApiSpec val packageFile = new File(projectDir, "package.yaml") Files.readAllLines(packageFile.toPath) contains "name: Foo_1" + + //teardown + deleteProject(projectId1) + deleteProject(projectId2) } "create project with specific version" in { - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) client.send(json""" { "jsonrpc": "2.0", "method": "project/create", @@ -285,16 +311,20 @@ class ProjectManagementApiSpec } } """) + val projectId = getGeneratedUUID client.expectJson(json""" { "jsonrpc" : "2.0", "id" : 1, "result" : { - "projectId" : $getGeneratedUUID, + "projectId" : $projectId, "projectName" : "Foo" } } """) + + //teardown + deleteProject(projectId) } "create a project dir with a suffix if a directory is taken" in { @@ -305,11 +335,14 @@ class ProjectManagementApiSpec val packageFile = new File(projectDirWithSuffix2, "package.yaml") projectDir.mkdirs() projectDirWithSuffix1.mkdirs() - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) - createProject(projectName) + val projectId = createProject(projectName) projectDirWithSuffix2.isDirectory shouldBe true packageFile.isFile shouldBe true + + //teardown + deleteProject(projectId) } } @@ -341,9 +374,9 @@ class ProjectManagementApiSpec } "fail when project is running" taggedAs Flaky in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") + val projectId = createProject("Foo") openProject(projectId) //when client.send(json""" @@ -373,11 +406,11 @@ class ProjectManagementApiSpec } "remove project structure" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - val projectName = "To_Remove" - implicit val client = new WsTestClient(address) - val projectId = createProject(projectName) - val projectDir = new File(userProjectDir, projectName) + val projectName = "To_Remove" + val projectId = createProject(projectName) + val projectDir = new File(userProjectDir, projectName) projectDir shouldBe Symbol("directory") //when client.send(json""" @@ -406,9 +439,10 @@ class ProjectManagementApiSpec "project/open" must { "open a project" taggedAs Flaky in { - val projectName = "Test_Project" implicit val client: WsTestClient = new WsTestClient(address) - val projectId = createProject(projectName) + + val projectName = "Test_Project" + val projectId = createProject(projectName) client.send(json""" { "jsonrpc": "2.0", "method": "project/open", @@ -421,6 +455,8 @@ class ProjectManagementApiSpec val result = openProjectData result.projectName shouldEqual projectName result.engineVersion shouldEqual SemVer("0.0.1").get + + // teardown closeProject(projectId) deleteProject(projectId) } @@ -481,10 +517,10 @@ class ProjectManagementApiSpec } "start the Language Server if not running" taggedAs Flaky in { + implicit val client: WsTestClient = new WsTestClient(address) //given - val projectName = "To_Remove" - implicit val client = new WsTestClient(address) - val projectId = createProject(projectName) + val projectName = "To_Remove" + val projectId = createProject(projectName) //when val socket = openProject(projectId) val languageServerClient = @@ -548,10 +584,10 @@ class ProjectManagementApiSpec } "start the Language Server after moving the directory" taggedAs Flaky in { + implicit val client: WsTestClient = new WsTestClient(address) //given - val projectName = "Foo" - implicit val client = new WsTestClient(address) - val projectId = createProject(projectName) + val projectName = "Foo" + val projectId = createProject(projectName) val newName = "bar" val newProjectDir = new File(userProjectDir, newName) @@ -600,14 +636,13 @@ class ProjectManagementApiSpec } "deduplicate project ids" taggedAs Flaky in { - val projectName1 = "Foo" - - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) // given - val projectId1 = createProject(projectName1) - val projectDir1 = new File(userProjectDir, projectName1) - val projectDir2 = new File(userProjectDir, "Test") + val projectName1 = "Foo" + val projectId1 = createProject(projectName1) + val projectDir1 = new File(userProjectDir, projectName1) + val projectDir2 = new File(userProjectDir, "Test") FileUtils.copyDirectory(projectDir1, projectDir2) // when @@ -683,10 +718,10 @@ class ProjectManagementApiSpec } "close project when the requester is the only client" taggedAs Flaky in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") - val socket = openProject(projectId) + val projectId = createProject("Foo") + val socket = openProject(projectId) val languageServerClient = new WsTestClient(s"ws://${socket.host}:${socket.port}") languageServerClient.send("test") @@ -715,7 +750,7 @@ class ProjectManagementApiSpec "project/list" must { "return a list sorted by creation time if none of projects was opened" in { - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //given val fooId = createProject("Foo") testClock.moveTimeForward() @@ -768,7 +803,7 @@ class ProjectManagementApiSpec } "return a list of projects even if editions of some of them cannot be resolved" taggedAs Retry in { - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //given val fooId = createProject("Foo") testClock.moveTimeForward() @@ -819,7 +854,7 @@ class ProjectManagementApiSpec } "returned sorted list of recently opened projects" in { - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //given val fooId = createProject("Foo") val barId = createProject("Bar") @@ -882,14 +917,13 @@ class ProjectManagementApiSpec } "resolve clashing ids" taggedAs Flaky in { - val projectName1 = "Foo" - - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) // given - val projectId1 = createProject(projectName1) - val projectDir1 = new File(userProjectDir, projectName1) - val projectDir2 = new File(userProjectDir, "Test") + val projectName1 = "Foo" + val projectId1 = createProject(projectName1) + val projectDir1 = new File(userProjectDir, projectName1) + val projectDir2 = new File(userProjectDir, "Test") FileUtils.copyDirectory(projectDir1, projectDir2) // when @@ -938,7 +972,7 @@ class ProjectManagementApiSpec "project/rename" must { "rename a project and move project dir" in { - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //given val newProjectName = "Bar" val projectId = createProject("Foo") @@ -972,10 +1006,10 @@ class ProjectManagementApiSpec } "create a project dir with a suffix if a directory is taken" taggedAs Flaky in { - val oldProjectName = "Foobar" - val newProjectName = "Foo" - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //given + val oldProjectName = "Foobar" + val newProjectName = "Foo" val projectId = createProject(oldProjectName) val primaryProjectDir = new File(userProjectDir, newProjectName) primaryProjectDir.mkdirs() @@ -1009,8 +1043,8 @@ class ProjectManagementApiSpec } "reply with an error when the project with the same name exists" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) val oldProjectName = "Foo" val newProjectName = "Bar" val projectId = createProject(oldProjectName) @@ -1044,7 +1078,7 @@ class ProjectManagementApiSpec "reply with an error when the project doesn't exist" in { //given - implicit val client = new WsTestClient(address) + implicit val client: WsTestClient = new WsTestClient(address) //when client.send(json""" { "jsonrpc": "2.0", @@ -1069,9 +1103,9 @@ class ProjectManagementApiSpec } "check if project name is not empty" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") + val projectId = createProject("Foo") //when client.send(json""" { "jsonrpc": "2.0", @@ -1095,9 +1129,9 @@ class ProjectManagementApiSpec } "validate project name for forbidden characters" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") + val projectId = createProject("Foo") //when client.send(json""" { "jsonrpc": "2.0", @@ -1124,9 +1158,9 @@ class ProjectManagementApiSpec } "validate project name should start with a capital letter" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") + val projectId = createProject("Foo") //when client.send(json""" { "jsonrpc": "2.0", @@ -1153,9 +1187,9 @@ class ProjectManagementApiSpec } "validate project name should be in upper snake case" in { + implicit val client: WsTestClient = new WsTestClient(address) //given - implicit val client = new WsTestClient(address) - val projectId = createProject("Foo") + val projectId = createProject("Foo") //when client.send(json""" { "jsonrpc": "2.0", @@ -1180,6 +1214,5 @@ class ProjectManagementApiSpec //teardown deleteProject(projectId) } - } }