diff --git a/RELEASES.md b/RELEASES.md index d99009ec8e3e..0edb9cd641d8 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -1,5 +1,11 @@ # Enso Next +## Interpreter/Runtime + +- Ensure that the module used by a visualization is preloaded when the + visualization is being attached + ([#1857](https://github.com/enso-org/enso/pull/1857)). + ## Tooling - Implemented an HTTP endponint returning the time that the language server has diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/Context.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/Context.java index ae8c1fa8daff..15fa10bcb566 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/Context.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/Context.java @@ -15,6 +15,7 @@ import org.enso.compiler.Compiler; import org.enso.compiler.PackageRepository; import org.enso.compiler.data.CompilerConfig; +import org.enso.editions.LibraryName; import org.enso.interpreter.Language; import org.enso.interpreter.OptionsHelper; import org.enso.interpreter.instrument.NotificationHandler; @@ -220,6 +221,15 @@ public Optional getModuleForFile(File path) { return getModuleNameForFile(path).flatMap(n -> getTopScope().getModule(n.toString())); } + /** + * Ensures that a module is preloaded if it can be loaded at all. + * + * @param moduleName name of the module to preload + */ + public void ensureModuleIsLoaded(String moduleName) { + LibraryName.fromModuleName(moduleName).foreach(packageRepository::ensurePackageIsLoaded); + } + /** * Fetches a module with a given name. * diff --git a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java index b903a6e75ce6..f43ff4ed50fa 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/service/ExecutionService.java @@ -260,16 +260,6 @@ public void resetModuleSources(File path) { module.ifPresent(Module::unsetLiteralSource); } - /** - * Finds a module by qualified name. - * - * @param moduleName the qualified name of the module - * @return the relevant module, if exists - */ - public Optional findModule(String moduleName) { - return context.findModule(moduleName); - } - /** * Applies modifications to literal module sources. * diff --git a/engine/runtime/src/main/scala/org/enso/compiler/PackageRepository.scala b/engine/runtime/src/main/scala/org/enso/compiler/PackageRepository.scala index 147eab864ef0..440438fdcc3c 100644 --- a/engine/runtime/src/main/scala/org/enso/compiler/PackageRepository.scala +++ b/engine/runtime/src/main/scala/org/enso/compiler/PackageRepository.scala @@ -57,11 +57,6 @@ trait PackageRepository { /** Modifies package and module names to reflect the project name change. */ def renameProject(namespace: String, oldName: String, newName: String): Unit - - /** This is a temporary workaround that should be removed once we get - * integrated with the editions. - */ - def registerForPreload(packages: Seq[Package[TruffleFile]]): Unit } object PackageRepository { @@ -198,8 +193,7 @@ object PackageRepository { /** @inheritdoc */ override def ensurePackageIsLoaded( libraryName: LibraryName - ): Either[Error, Unit] = { - runPreload() + ): Either[Error, Unit] = if (loadedPackages.contains(libraryName)) Right(()) else { logger.trace(s"Resolving library $libraryName.") @@ -239,25 +233,18 @@ object PackageRepository { } } } - } /** @inheritdoc */ - override def getLoadedModules(): Seq[Module] = { - runPreload() + override def getLoadedModules(): Seq[Module] = loadedModules.values.toSeq - } /** @inheritdoc */ - override def getLoadedPackages(): Seq[Package[TruffleFile]] = { - runPreload() + override def getLoadedPackages(): Seq[Package[TruffleFile]] = loadedPackages.values.toSeq.flatten - } /** @inheritdoc */ - override def getLoadedModule(qualifiedName: String): Option[Module] = { - runPreload() + override def getLoadedModule(qualifiedName: String): Option[Module] = loadedModules.get(qualifiedName) - } /** @inheritdoc */ override def registerModuleCreatedInRuntime(module: Module): Unit = @@ -317,24 +304,6 @@ object PackageRepository { loadedModules.put(module.getName.toString, module) } } - - /** Temporary workaround, will be removed once editions are integrated. */ - private var toPreload: List[Package[TruffleFile]] = Nil - private def runPreload(): Unit = { - for (pkg <- toPreload) { - registerPackageInternal( - libraryName = pkg.libraryName, - pkg = pkg, - libraryVersion = LibraryVersion.Local, - isLibrary = true - ) - } - toPreload = Nil - } - - /** @inheritdoc */ - override def registerForPreload(packages: Seq[Package[TruffleFile]]): Unit = - toPreload ++= packages } /** Creates a [[PackageRepository]] for the run. diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala index 996959ac8b00..ad77a7a02de3 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/EnsureCompiledJob.scala @@ -423,7 +423,7 @@ class EnsureCompiledJob(protected val files: Iterable[File]) private def getCacheMetadata( stack: Iterable[InstrumentFrame] )(implicit ctx: RuntimeContext): Option[CachePreferenceAnalysis.Metadata] = - stack.lastOption flatMap { + stack.lastOption.flatMap { case InstrumentFrame(Api.StackItem.ExplicitCall(ptr, _, _), _, _) => ctx.executionService.getContext.findModule(ptr.module).toScala.map { module => diff --git a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualisationJob.scala b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualisationJob.scala index 6c274b10c498..8940edd8dbe0 100644 --- a/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualisationJob.scala +++ b/engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/UpsertVisualisationJob.scala @@ -129,7 +129,10 @@ class UpsertVisualisationJob( moduleName: String, expression: String )(implicit ctx: RuntimeContext): Either[EvalFailure, AnyRef] = { - val maybeModule = ctx.executionService.findModule(moduleName) + val context = ctx.executionService.getContext + // TODO [RW] more specific error when the module cannot be installed (#1861) + context.ensureModuleIsLoaded(moduleName) + val maybeModule = context.findModule(moduleName) val notFoundOrModule = if (maybeModule.isPresent) Right(maybeModule.get()) diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualisationsTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualisationsTest.scala index b580fea9b657..3b5219654f9e 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualisationsTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualisationsTest.scala @@ -110,13 +110,18 @@ class RuntimeVisualisationsTest Option(messageQueue.poll()) } - def receive: Option[Api.Response] = { - Option(messageQueue.poll(10, TimeUnit.SECONDS)) - } + def receive: Option[Api.Response] = + receiveTimeout(10) - def receive(n: Int): List[Api.Response] = { - Iterator.continually(receive).take(n).flatten.toList - } + def receiveTimeout(timeoutSeconds: Int): Option[Api.Response] = + Option(messageQueue.poll(timeoutSeconds.toLong, TimeUnit.SECONDS)) + + def receive(n: Int, timeoutSeconds: Int = 10): List[Api.Response] = + Iterator + .continually(receiveTimeout(timeoutSeconds)) + .take(n) + .flatten + .toList def consumeOut: List[String] = { val result = out.toString @@ -1318,6 +1323,94 @@ class RuntimeVisualisationsTest ) } + it should "be able to use external libraries if they are needed by the visualisation" in { + val idMain = context.Main.metadata.addItem(87, 1) + val contents = context.Main.code + val mainFile = context.writeMain(context.Main.code) + val moduleName = "Enso_Test.Test.Main" + + 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)) + ) + + // Open the new file + context.send( + Api.Request(Api.OpenFileNotification(mainFile, contents, true)) + ) + context.receiveNone shouldEqual None + + // push main + val item1 = Api.StackItem.ExplicitCall( + Api.MethodPointer(moduleName, "Enso_Test.Test.Main", "main"), + None, + Vector() + ) + context.send( + Api.Request(requestId, Api.PushContextRequest(contextId, item1)) + ) + context.receive(6) 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), + TestMessages.update(contextId, idMain, Constants.INTEGER), + context.executionComplete(contextId) + ) + + // attach visualisation + context.send( + Api.Request( + requestId, + Api.AttachVisualisation( + visualisationId, + idMain, + Api.VisualisationConfiguration( + contextId, + "Standard.Visualization.Id", + "x -> x.default_visualization.to_text" + ) + ) + ) + ) + + val attachVisualisationResponses = + context.receive(n = 5, timeoutSeconds = 60) + attachVisualisationResponses should contain allOf ( + Api.Response(requestId, Api.VisualisationAttached()), + context.executionComplete(contextId) + ) + + val Some(data) = attachVisualisationResponses.collectFirst { + case Api.Response( + None, + Api.VisualisationUpdate( + Api.VisualisationContext( + `visualisationId`, + `contextId`, + `idMain` + ), + data + ) + ) => + data + } + + data.sameElements("(Builtin 'JSON')".getBytes) shouldBe true + + val loadedLibraries = attachVisualisationResponses.collect { + case Api.Response(None, Api.LibraryLoaded(namespace, name, _, _)) => + (namespace, name) + } + + loadedLibraries should contain(("Standard", "Visualization")) + } + it should "return VisualisationExpressionFailed error when attaching visualisation" in { val idMain = context.Main.metadata.addItem(87, 1) val contents = context.Main.code diff --git a/lib/scala/editions/src/main/scala/org/enso/editions/LibraryName.scala b/lib/scala/editions/src/main/scala/org/enso/editions/LibraryName.scala index 14377baf068c..e70b06596d44 100644 --- a/lib/scala/editions/src/main/scala/org/enso/editions/LibraryName.scala +++ b/lib/scala/editions/src/main/scala/org/enso/editions/LibraryName.scala @@ -31,14 +31,23 @@ object LibraryName { } yield name } + private val separator = '.' + /** Creates a [[LibraryName]] from its string representation. * * Returns an error message on failure. */ def fromString(str: String): Either[String, LibraryName] = { - str.split('.') match { - case Array(prefix, name) => Right(LibraryName(prefix, name)) - case _ => Left(s"`$str` is not a valid library name.") + str.split(separator) match { + case Array(namespace, name) => Right(LibraryName(namespace, name)) + case _ => Left(s"`$str` is not a valid library name.") } } + + /** Extracts the [[LibraryName]] from a full name of a module. */ + def fromModuleName(module: String): Option[LibraryName] = + module.split(separator) match { + case Array(namespace, name, _ @_*) => Some(LibraryName(namespace, name)) + case _ => None + } }