Fix null issues with URI conversion, add tests (#1211)

Fixes #1202 and similar ## Changes Added additional check in case of `null` components of `URI` are received, added tests. ## Test plan Automatic tests added
sourcegraph · Mar 28, 2024 · 143982e · 143982e
1 parent 745209a
commit 143982e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 9 deletions.
diff --git a/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ContextItem.kt b/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ContextItem.kt
@@ -43,8 +43,11 @@ sealed class ContextItem {
     val path = uri.path
     val pathComponents = path.split("/") // uri path is posix-style
     if (pathComponents.size > 3) {
-      return "...${File.separator}${pathComponents.subList(pathComponents.size - 3, pathComponents.size).joinToString(
-                File.separator)}"
+      return "...${File.separator}${
+        pathComponents.subList(pathComponents.size - 3, pathComponents.size).joinToString(
+          File.separator
+        )
+      }"
     }
     return path.replace("/", File.separator)
   }
@@ -116,19 +119,22 @@ data class ContextItemSymbol(
 
 val uriDeserializer =
     JsonDeserializer { jsonElement: JsonElement?, _: Type, _: JsonDeserializationContext ->
+      fun asStringOrNull(elem: JsonElement?): String? {
+        return if (elem == null || elem.isJsonNull) null else elem.asString
+      }
+
       val j = jsonElement?.asJsonObject
       if (j == null || j.isJsonNull) {
         null
       } else if (j.isJsonPrimitive) {
         j.asString
       } else {
         URI(
-            j["scheme"]?.asString,
-            j["authority"]?.asString,
-            j["path"]?.asString,
-            j["query"]?.asString,
-            j["fragment"]?.asString,
-        )
+            asStringOrNull(j["scheme"]),
+            asStringOrNull(j["authority"]),
+            asStringOrNull(j["path"]),
+            asStringOrNull(j["query"]),
+            asStringOrNull(j["fragment"]))
       }
     }
 

diff --git a/src/test/kotlin/com/sourcegraph/cody/agent/protocol/ContextItemTest.kt b/src/test/kotlin/com/sourcegraph/cody/agent/protocol/ContextItemTest.kt
@@ -1,5 +1,7 @@
 package com.sourcegraph.cody.agent.protocol
 
+import com.google.gson.Gson
+import com.google.gson.GsonBuilder
 import com.intellij.openapi.util.SystemInfoRt
 import com.intellij.testFramework.fixtures.BasePlatformTestCase
 import java.net.URI
@@ -48,11 +50,38 @@ class ContextItemTest : BasePlatformTestCase() {
       assertEquals("c:\\a\\b\\c\\d.java", contextFilePath("/c:/a/b/c/d.java"))
       assertEquals("c:\\a\\b\\c\\d.java", contextFilePath("c:/a/b/c/d.java"))
       assertEquals("c:\\a\\b\\c\\d.java", contextFilePath("c:/a/b/c/d.java?#"))
-      assertEquals("c:\\a\\b\\c\\d.java", contextFilePath("c:/a/b/c/d.java?#"))
+      assertEquals("c:\\a\\b\\c\\d.java", contextFilePath("/c:/a/b/c/d.java?#"))
     } else {
       assertEquals("/a/b/c/d.java", contextFilePath("/a/b/c/d.java"))
       assertEquals("/a/b/c/d.java", contextFilePath("/a/b/c/d.java?#"))
       assertEquals("/a/b/c/d.java", contextFilePath("file:///a/b/c/d.java"))
     }
   }
+
+  fun `test uri serialization and deserialization`() {
+    val gson: Gson =
+        GsonBuilder()
+            .registerTypeAdapter(URI::class.java, uriDeserializer)
+            .registerTypeAdapter(URI::class.java, uriSerializer)
+            .serializeNulls()
+            .create()
+
+    fun roundtripConversion(path: String) =
+        gson.fromJson(gson.toJson(URI.create(path)), URI::class.java).toString()
+
+    if (SystemInfoRt.isWindows) {
+      assertEquals("file:/c:/a/b/c/d.java", roundtripConversion("file:///c:/a/b/c/d.java"))
+      assertEquals("file://c:/a/b/c/d.java", roundtripConversion("file://c:/a/b/c/d.java"))
+      assertEquals("file:/c:/a/b/c/d.java?#", roundtripConversion("file:///c:/a/b/c/d.java?#"))
+      assertEquals("file://c:/a/b/c/d.java?#", roundtripConversion("file://c:/a/b/c/d.java?#"))
+      assertEquals("/c:/a/b/c/d.java", roundtripConversion("/c:/a/b/c/d.java"))
+      assertEquals("c:/a/b/c/d.java", roundtripConversion("c:/a/b/c/d.java"))
+      assertEquals("c:/a/b/c/d.java?#", roundtripConversion("c:/a/b/c/d.java?#"))
+      assertEquals("/c:/a/b/c/d.java?#", roundtripConversion("/c:/a/b/c/d.java?#"))
+    } else {
+      assertEquals("/a/b/c/d.java", roundtripConversion("/a/b/c/d.java"))
+      assertEquals("/a/b/c/d.java?#", roundtripConversion("/a/b/c/d.java?#"))
+      assertEquals("file:/a/b/c/d.java", roundtripConversion("file:///a/b/c/d.java"))
+    }
+  }
 }