Fix imports when attaching a visualization (#1857)

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

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<Module> 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.
    *

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<Module> findModule(String moduleName) {
-    return context.findModule(moduleName);
-  }
-
   /**
    * Applies modifications to literal module sources.
    *

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.

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 =>

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())

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

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
+    }
 }