From 7da643fc8f895c35c3aa1613a2a43690d4ce0c88 Mon Sep 17 00:00:00 2001
From: Daniel Chao <dan.chao@apple.com>
Date: Wed, 8 May 2024 07:22:57 -0700
Subject: [PATCH] Handle `null` pathElements gracefully in message passing API
 (#480)

In messages "List Resources Response" and "List Modules Response",
if `pathElements` and `error` are both null, default to an empty list.
---
 .../pages/message-passing-api.adoc            |  4 +
 .../org/pkl/server/ClientModuleKeyFactory.kt  |  2 +-
 .../org/pkl/server/ClientResourceReader.kt    |  6 +-
 .../test/kotlin/org/pkl/server/ServerTest.kt  | 78 +++++++++++++++++++
 4 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/docs/modules/bindings-specification/pages/message-passing-api.adoc b/docs/modules/bindings-specification/pages/message-passing-api.adoc
index c53cb6141..7071a0d3d 100644
--- a/docs/modules/bindings-specification/pages/message-passing-api.adoc
+++ b/docs/modules/bindings-specification/pages/message-passing-api.adoc
@@ -451,6 +451,8 @@ The response to <<list-resources-request>>.
 If successful, `pathElements` is set.
 Otherwise, `error` is set.
 
+If neither are set, `pathElements` default to an empty list.
+
 [source,pkl]
 ----
 /// A number identifying this request.
@@ -507,6 +509,8 @@ The response to <<list-modules-request>>.
 If successful, `pathElements` is set.
 Otherwise, `error` is set.
 
+If neither are set, `pathElements` default to an empty list.
+
 [source,pkl]
 ----
 /// A number identifying this request.
diff --git a/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt b/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt
index 4d4f954d6..90e70875d 100644
--- a/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt
+++ b/pkl-server/src/main/kotlin/org/pkl/server/ClientModuleKeyFactory.kt
@@ -97,7 +97,7 @@ internal class ClientModuleKeyFactory(
                     if (response.error != null) {
                       completeExceptionally(IOException(response.error))
                     } else {
-                      complete(response.pathElements!!)
+                      complete(response.pathElements ?: emptyList())
                     }
                   }
                   else -> completeExceptionally(ProtocolException("unexpected response"))
diff --git a/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt b/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt
index 998d6e2b4..a88a1e899 100644
--- a/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt
+++ b/pkl-server/src/main/kotlin/org/pkl/server/ClientResourceReader.kt
@@ -68,10 +68,10 @@ internal class ClientResourceReader(
           transport.send(request) { response ->
             when (response) {
               is ListResourcesResponse ->
-                if (response.pathElements != null) {
-                  complete(response.pathElements)
-                } else {
+                if (response.error != null) {
                   completeExceptionally(IOException(response.error))
+                } else {
+                  complete(response.pathElements ?: emptyList())
                 }
               else -> completeExceptionally(ProtocolException("Unexpected response"))
             }
diff --git a/pkl-server/src/test/kotlin/org/pkl/server/ServerTest.kt b/pkl-server/src/test/kotlin/org/pkl/server/ServerTest.kt
index f29099f81..ac72985d0 100644
--- a/pkl-server/src/test/kotlin/org/pkl/server/ServerTest.kt
+++ b/pkl-server/src/test/kotlin/org/pkl/server/ServerTest.kt
@@ -294,6 +294,43 @@ class ServerTest {
       )
   }
 
+  @Test
+  fun `glob resources -- null pathElements and null error`() {
+    val reader = ResourceReaderSpec(scheme = "bird", hasHierarchicalUris = true, isGlobbable = true)
+    val evaluatorId = client.sendCreateEvaluatorRequest(resourceReaders = listOf(reader))
+    client.send(
+      EvaluateRequest(
+        requestId = 1,
+        evaluatorId = evaluatorId,
+        moduleUri = URI("repl:text"),
+        moduleText =
+          """
+        res = read*("bird:/**.txt").keys
+      """
+            .trimIndent(),
+        expr = "res"
+      )
+    )
+    val listResourcesRequest = client.receive<ListResourcesRequest>()
+    client.send(
+      ListResourcesResponse(
+        requestId = listResourcesRequest.requestId,
+        evaluatorId = listResourcesRequest.evaluatorId,
+        pathElements = null,
+        error = null
+      )
+    )
+    val evaluateResponse = client.receive<EvaluateResponse>()
+    assertThat(evaluateResponse.result!!.debugYaml)
+      .isEqualTo(
+        """
+        - 6
+        - []
+        """
+          .trimIndent()
+      )
+  }
+
   @Test
   fun `glob resource error`() {
     val reader = ResourceReaderSpec(scheme = "bird", hasHierarchicalUris = true, isGlobbable = true)
@@ -505,6 +542,47 @@ class ServerTest {
       )
   }
 
+  @Test
+  fun `glob module -- null pathElements and null error`() {
+    val reader =
+      ModuleReaderSpec(
+        scheme = "bird",
+        hasHierarchicalUris = true,
+        isLocal = true,
+        isGlobbable = true
+      )
+    val evaluatorId = client.sendCreateEvaluatorRequest(moduleReaders = listOf(reader))
+
+    client.send(
+      EvaluateRequest(
+        requestId = 1,
+        evaluatorId = evaluatorId,
+        moduleUri = URI("repl:text"),
+        moduleText = """res = import*("bird:/**.pkl").keys""",
+        expr = "res"
+      )
+    )
+    val listModulesMsg = client.receive<ListModulesRequest>()
+    client.send(
+      ListModulesResponse(
+        requestId = listModulesMsg.requestId,
+        evaluatorId = evaluatorId,
+        pathElements = null,
+        error = null
+      )
+    )
+
+    val evaluateResponse = client.receive<EvaluateResponse>()
+    assertThat(evaluateResponse.result!!.debugRendering)
+      .isEqualTo(
+        """
+      - 6
+      - []
+    """
+          .trimIndent()
+      )
+  }
+
   @Test
   fun `glob module error`() {
     val reader =