diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/AttachVisualisationCmd.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/AttachVisualisationCmd.scala index d83b07ac0e35..4ec66b42590e 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/AttachVisualisationCmd.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/AttachVisualisationCmd.scala @@ -22,10 +22,16 @@ class AttachVisualisationCmd( ctx: RuntimeContext, ec: ExecutionContext ): Future[Unit] = { - if (doesContextExist) { - attachVisualisation() - } else { - replyWithContextNotExistError() + val contextId = request.visualisationConfig.executionContextId + ctx.locking.acquireContextLock(contextId) + try { + if (doesContextExist) { + attachVisualisation() + } else { + replyWithContextNotExistError() + } + } finally { + ctx.locking.releaseContextLock(contextId) } } diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/DetachVisualisationCmd.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/DetachVisualisationCmd.scala index b55eec9069b5..e0f612fe8a90 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/DetachVisualisationCmd.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/DetachVisualisationCmd.scala @@ -1,6 +1,7 @@ package org.enso.interpreter.instrument.command import org.enso.interpreter.instrument.execution.RuntimeContext +import org.enso.interpreter.instrument.job.DetachVisualisationJob import org.enso.polyglot.runtime.Runtime.Api import org.enso.polyglot.runtime.Runtime.Api.RequestId @@ -20,22 +21,43 @@ class DetachVisualisationCmd( override def execute(implicit ctx: RuntimeContext, ec: ExecutionContext - ): Future[Unit] = - Future { - if (doesContextExist) { - ctx.contextManager.removeVisualisation( - request.contextId, - request.expressionId, - request.visualisationId - ) - reply(Api.VisualisationDetached()) - } else { - reply(Api.ContextNotExistError(request.contextId)) - } + ): Future[Unit] = { + ctx.locking.acquireContextLock(request.contextId) + try { + if (doesContextExist) { + detachVisualization() + } else { + replyWithContextNotExistError() + } + } finally { + ctx.locking.releaseContextLock(request.contextId) } + } private def doesContextExist(implicit ctx: RuntimeContext): Boolean = { ctx.contextManager.contains(request.contextId) } + private def detachVisualization()(implicit + ctx: RuntimeContext + ): Future[Unit] = + ctx.jobProcessor.run( + new DetachVisualisationJob( + maybeRequestId, + request.visualisationId, + request.expressionId, + request.contextId, + Api.VisualisationDetached() + ) + ) + + private def replyWithContextNotExistError()(implicit + ctx: RuntimeContext, + ec: ExecutionContext + ): Future[Unit] = { + Future { + reply(Api.ContextNotExistError(request.contextId)) + } + } + } diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/ModifyVisualisationCmd.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/ModifyVisualisationCmd.scala index 8825675fc8fb..f936635aa2c2 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/ModifyVisualisationCmd.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/command/ModifyVisualisationCmd.scala @@ -26,10 +26,16 @@ class ModifyVisualisationCmd( ctx: RuntimeContext, ec: ExecutionContext ): Future[Unit] = { - if (doesContextExist) { - modifyVisualisation() - } else { - replyWithContextNotExistError() + val contextId = request.visualisationConfig.executionContextId + ctx.locking.acquireContextLock(contextId) + try { + if (doesContextExist) { + modifyVisualisation() + } else { + replyWithContextNotExistError() + } + } finally { + ctx.locking.releaseContextLock(contextId) } } diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/LocationFilter.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/LocationFilter.scala index 2291a3f3d910..2f7a3162dc1e 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/LocationFilter.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/execution/LocationFilter.scala @@ -82,14 +82,13 @@ object LocationFilter { element match { case IR.Expression.Binding(_, function: IR.Function, _, _, _) - if isSynthetic(function) => + if !isSynthetic(function) => builder ++= getLocation(function) case IR.Expression.Binding(_, block: IR.Expression.Block, _, _, _) => builder ++= getLocation(block) - case function: IR.Function => - if (!isSynthetic(function)) { - builder ++= location - } + case arg: IR.CallArgument => + queue += arg.value + case _: IR.Function => case _ => builder ++= location queue ++= element.children diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/DetachVisualisationJob.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/DetachVisualisationJob.scala new file mode 100644 index 000000000000..646d2e116d30 --- /dev/null +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/DetachVisualisationJob.scala @@ -0,0 +1,43 @@ +package org.enso.interpreter.instrument.job + +import org.enso.interpreter.instrument.execution.RuntimeContext +import org.enso.polyglot.runtime.Runtime.Api.{ + ContextId, + ExpressionId, + RequestId, + VisualisationId +} +import org.enso.polyglot.runtime.Runtime.{Api, ApiResponse} + +/** A job that detaches a visualisation. + * + * @param requestId maybe a request id + * @param visualisationId an identifier of visualisation + * @param expressionId an identifier of expression + * @param contextId an execution context id + * @param response a response used to reply to a client + */ +class DetachVisualisationJob( + requestId: Option[RequestId], + visualisationId: VisualisationId, + expressionId: ExpressionId, + contextId: ContextId, + response: ApiResponse +) extends Job[Unit](List(contextId), true, false) + with ProgramExecutionSupport { + + /** @inheritdoc */ + override def run(implicit ctx: RuntimeContext): Unit = { + ctx.locking.acquireContextLock(contextId) + try { + ctx.contextManager.removeVisualisation( + contextId, + expressionId, + visualisationId + ) + ctx.endpoint.sendToClient(Api.Response(requestId, response)) + } finally { + ctx.locking.releaseContextLock(contextId) + } + } +} diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala index c67c88561b40..57b71b1b848d 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeInstrumentTest.scala @@ -589,7 +589,8 @@ class RuntimeInstrumentTest val metadata = new Metadata val aExpr = metadata.addItem(41, 14) - val lam = metadata.addItem(42, 10) + // lambda + metadata.addItem(42, 10) // lambda expression metadata.addItem(47, 5) val lamArg = metadata.addItem(54, 1) @@ -631,10 +632,9 @@ class RuntimeInstrumentTest ) ) ) - context.receive(6) should contain theSameElementsAs Seq( + context.receive(5) should contain theSameElementsAs Seq( Api.Response(requestId, Api.PushContextResponse(contextId)), TestMessages.update(contextId, aExpr, Constants.INTEGER), - TestMessages.update(contextId, lam, Constants.FUNCTION), TestMessages.update(contextId, lamArg, Constants.INTEGER), TestMessages.update(contextId, mainRes, Constants.NOTHING), context.executionComplete(contextId) @@ -881,4 +881,77 @@ class RuntimeInstrumentTest ) } + it should "not instrument a lambda in argument" in { + val contextId = UUID.randomUUID() + val requestId = UUID.randomUUID() + val moduleName = "Test.Main" + val metadata = new Metadata + + // body of id method + metadata.addItem(42, 1) + // body of id1 function + metadata.addItem(78, 3) + // default lambda argument a->a in id method + metadata.addItem(34, 4) + // default lambda argument a->a in id1 function + metadata.addItem(70, 4) + // first x->x argument + metadata.addItem(94, 4) + // second x->x argument + metadata.addItem(148, 4) + val arg1 = metadata.addItem(90, 2) + val arg2 = metadata.addItem(101, 2) + val arg3 = metadata.addItem(133, 2) + + val code = + """from Builtins import all + | + |id (x = a->a) = x + | + |main = + | id1 x=42 (y = a->a) = y x + | id1 42 + | id1 42 x->x + | here.id + | here.id 42 + | here.id x->x + |""".stripMargin.linesIterator.mkString("\n") + val contents = metadata.appendToCode(code) + val mainFile = context.writeMain(contents) + + // create context + context.send(Api.Request(requestId, Api.CreateContextRequest(contextId))) + context.receive shouldEqual Some( + Api.Response(requestId, Api.CreateContextResponse(contextId)) + ) + + // Open the new file + context.send( + Api.Request(Api.OpenFileNotification(mainFile, contents, true)) + ) + context.receiveNone shouldEqual None + + // push main + context.send( + Api.Request( + requestId, + Api.PushContextRequest( + contextId, + Api.StackItem.ExplicitCall( + Api.MethodPointer(moduleName, "Test.Main", "main"), + None, + Vector() + ) + ) + ) + ) + context.receive(5) should contain theSameElementsAs Seq( + Api.Response(requestId, Api.PushContextResponse(contextId)), + TestMessages.update(contextId, arg1, Constants.INTEGER), + TestMessages.update(contextId, arg2, Constants.INTEGER), + TestMessages.update(contextId, arg3, Constants.INTEGER), + context.executionComplete(contextId) + ) + } + } diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala index e76a2ff55f0f..9bf11dc6f9d2 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeServerTest.scala @@ -1581,14 +1581,14 @@ class RuntimeServerTest val moduleName = "Test.Main" val metadata = new Metadata - val xId = metadata.addItem(41, 5) - val mainRes = metadata.addItem(51, 12) + val xId = metadata.addItem(41, 10) + val mainRes = metadata.addItem(56, 12) val code = """from Builtins import all | |main = - | x = _ + 1 + | x = a -> a + 1 | IO.println x |""".stripMargin.linesIterator.mkString("\n") val contents = metadata.appendToCode(code) @@ -3627,6 +3627,140 @@ class RuntimeServerTest ) } + it should "not reorder visualization commands" in { + val contents = context.Main.code + val mainFile = context.writeMain(contents) + val visualisationFile = + context.writeInSrcDir("Visualisation", context.Visualisation.code) + + // open files + context.send( + Api.Request(Api.OpenFileNotification(mainFile, contents, true)) + ) + context.receiveNone shouldEqual None + context.send( + Api.Request( + Api.OpenFileNotification( + visualisationFile, + context.Visualisation.code, + true + ) + ) + ) + context.receiveNone shouldEqual None + + val contextId = UUID.randomUUID() + val requestId = UUID.randomUUID() + val visualisationId = UUID.randomUUID() + + // create context + context.send(Api.Request(requestId, Api.CreateContextRequest(contextId))) + context.receive shouldEqual Some( + Api.Response(requestId, Api.CreateContextResponse(contextId)) + ) + + // push main + val item1 = Api.StackItem.ExplicitCall( + Api.MethodPointer("Test.Main", "Test.Main", "main"), + None, + Vector() + ) + context.send( + Api.Request(requestId, Api.PushContextRequest(contextId, item1)) + ) + context.receive(5) should contain theSameElementsAs Seq( + Api.Response(requestId, Api.PushContextResponse(contextId)), + context.Main.Update.mainX(contextId), + context.Main.Update.mainY(contextId), + context.Main.Update.mainZ(contextId), + context.executionComplete(contextId) + ) + + // attach visualisation + context.send( + Api.Request( + requestId, + Api.AttachVisualisation( + visualisationId, + context.Main.idMainX, + Api.VisualisationConfiguration( + contextId, + "Test.Visualisation", + "x -> here.encode x" + ) + ) + ) + ) + + val attachVisualisationResponses = context.receive(2) + attachVisualisationResponses should contain( + Api.Response(requestId, Api.VisualisationAttached()) + ) + val expectedExpressionId = context.Main.idMainX + val Some(data) = attachVisualisationResponses.collectFirst { + case Api.Response( + None, + Api.VisualisationUpdate( + Api.VisualisationContext( + `visualisationId`, + `contextId`, + `expectedExpressionId` + ), + data + ) + ) => + data + } + data.sameElements("6".getBytes) shouldBe true + + // modify visualisation + context.send( + Api.Request( + requestId, + Api.ModifyVisualisation( + visualisationId, + Api.VisualisationConfiguration( + contextId, + "Test.Visualisation", + "x -> here.incAndEncode x" + ) + ) + ) + ) + // detach visualisation + context.send( + Api.Request( + requestId, + Api.DetachVisualisation( + contextId, + visualisationId, + context.Main.idMainX + ) + ) + ) + val modifyVisualisationResponses = context.receive(3) + modifyVisualisationResponses should contain allOf ( + Api.Response(requestId, Api.VisualisationModified()), + Api.Response(requestId, Api.VisualisationDetached()) + ) + val Some(dataAfterModification) = + modifyVisualisationResponses.collectFirst { + case Api.Response( + None, + Api.VisualisationUpdate( + Api.VisualisationContext( + `visualisationId`, + `contextId`, + `expectedExpressionId` + ), + data + ) + ) => + data + } + dataAfterModification.sameElements("7".getBytes) shouldBe true + } + it should "rename a project" in { val contents = context.Main.code val mainFile = context.writeMain(contents)