Repository: spotify/voyager (language: python)

Issue:
remove Jackson dependency, do JSON encoding manually
We are pulling in the whole Jackson dep just to write a Java list as a JSON array and read it back, we can just do that manually since it is such a trivial JSON use case


Discussion / hints:

Fix patch:
diff --git a/java/pom.xml b/java/pom.xml
index 0008e999..7c21a906 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -32,22 +32,25 @@
   </properties>
 
   <dependencies>
+    <!-- test -->
     <dependency>
       <groupId>com.google.guava</groupId>
       <artifactId>guava</artifactId>
       <version>31.1-jre</version>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-core</artifactId>
       <version>2.13.0</version>
+      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>com.fasterxml.jackson.core</groupId>
       <artifactId>jackson-databind</artifactId>
       <version>2.13.0</version>
+      <scope>test</scope>
     </dependency>
-    <!-- test -->
     <dependency>
       <groupId>junit</groupId>
       <artifactId>junit</artifactId>
diff --git a/java/src/main/java/com/spotify/voyager/jni/StringIndex.java b/java/src/main/java/com/spotify/voyager/jni/StringIndex.java
index 84397d14..83400d4e 100644
--- a/java/src/main/java/com/spotify/voyager/jni/StringIndex.java
+++ b/java/src/main/java/com/spotify/voyager/jni/StringIndex.java
@@ -20,19 +20,17 @@
 
 package com.spotify.voyager.jni;
 
-import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.ObjectMapper;
 import com.spotify.voyager.jni.Index.SpaceType;
 import com.spotify.voyager.jni.Index.StorageDataType;
+import com.spotify.voyager.jni.utils.TinyJson;
 import java.io.BufferedInputStream;
 import java.io.BufferedOutputStream;
-import java.io.BufferedWriter;
 import java.io.Closeable;
-import java.io.File;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
@@ -45,7 +43,6 @@
  * index ID to a provided String.
  */
 public class StringIndex implements Closeable {
-  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
   private static final String INDEX_FILE_NAME = "index.hnsw";
   private static final String NAMES_LIST_FILE_NAME = "names.json";
   private final Index hnswIndex;
@@ -122,10 +119,10 @@ public static StringIndex load(
       int numDimensions,
       StorageDataType storageDataType) {
     Index index = Index.load(indexFilename, spaceType, numDimensions, storageDataType);
+
     List<String> names;
     try {
-      names =
-          OBJECT_MAPPER.readValue(new File(nameListFilename), new TypeReference<List<String>>() {});
+      names = TinyJson.readStringList(Files.newInputStream(Paths.get(nameListFilename)));
     } catch (IOException e) {
       throw new RuntimeException("Error reading names list from: " + nameListFilename, e);
     }
@@ -151,16 +148,11 @@ public static StringIndex load(
       SpaceType spaceType,
       int numDimensions,
       StorageDataType storageDataType) {
+
     // use buffered stream to keep read speeds high, reading up to 100MB at once
     BufferedInputStream inputStream = new BufferedInputStream(indexInputStream, 1024 * 1024 * 100);
     Index index = Index.load(inputStream, spaceType, numDimensions, storageDataType);
-
-    List<String> names;
-    try {
-      names = OBJECT_MAPPER.readValue(nameListInputStream, new TypeReference<List<String>>() {});
-    } catch (IOException e) {
-      throw new RuntimeException("unable to read names list");
-    }
+    List<String> names = TinyJson.readStringList(nameListInputStream);
 
     return new StringIndex(index, names);
   }
@@ -177,10 +169,10 @@ public void saveIndex(String outputDirectory) throws IOException {
     this.hnswIndex.saveIndex(indexFilename);
 
     String namesListFilename = outputDirectory + "/" + NAMES_LIST_FILE_NAME;
-    BufferedWriter writer = new BufferedWriter(new FileWriter(namesListFilename));
-    writer.write(OBJECT_MAPPER.writeValueAsString(this.names));
-    writer.flush();
-    writer.close();
+    final OutputStream outputStream = Files.newOutputStream(Paths.get(namesListFilename));
+    TinyJson.writeStringList(this.names, outputStream);
+    outputStream.flush();
+    outputStream.close();
   }
 
   /**
@@ -195,7 +187,7 @@ public void saveIndex(OutputStream indexOutputStream, OutputStream namesListOutp
     BufferedOutputStream outputStream =
         new BufferedOutputStream(indexOutputStream, 1024 * 1024 * 100);
     this.hnswIndex.saveIndex(outputStream);
-    namesListOutputStream.write(OBJECT_MAPPER.writeValueAsBytes(this.names));
+    TinyJson.writeStringList(this.names, namesListOutputStream);
 
     outputStream.flush();
     outputStream.close();
diff --git a/java/src/main/java/com/spotify/voyager/jni/utils/TinyJson.java b/java/src/main/java/com/spotify/voyager/jni/utils/TinyJson.java
new file mode 100644
index 00000000..c470f959
--- /dev/null
+++ b/java/src/main/java/com/spotify/voyager/jni/utils/TinyJson.java
@@ -0,0 +1,87 @@
+/*-
+ * -\-\-
+ * voyager
+ * --
+ * Copyright (C) 2016 - 2023 Spotify AB
+ * --
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * -/-/-
+ */
+
+package com.spotify.voyager.jni.utils;
+
+import java.io.BufferedWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Scanner;
+
+/**
+ * A dependency-free, super tiny JSON serde class that only supports reading and writing lists of
+ * strings.
+ */
+public class TinyJson {
+  public static List<String> readStringList(InputStream stream) {
+    Scanner scanner = new Scanner(stream).useDelimiter("\"");
+
+    List<String> outputList = new ArrayList<>();
+
+    boolean insideString = false;
+    while (true) {
+      String token = scanner.next();
+      if (insideString) {
+        outputList.add(token);
+        insideString = false;
+      } else {
+        token = token.trim();
+        if (token.equals(",") || token.equals("[")) {
+          insideString = true;
+        } else if (token.equals("]") || token.equals("[]")) {
+          break;
+        } else {
+          throw new IllegalArgumentException(
+              "Unexpected token found when parsing JSON list: " + token);
+        }
+      }
+    }
+
+    return outputList;
+  }
+
+  public static void writeStringList(final Iterable<String> items, final OutputStream stream)
+      throws IOException {
+    BufferedWriter output = new BufferedWriter(new OutputStreamWriter(stream));
+    output.write("[");
+    boolean isFirst = true;
+    for (final String item : items) {
+      if (!isFirst) {
+        output.write(',');
+      }
+      isFirst = false;
+      output.write('"');
+      if (item.contains("\\") || item.contains("\"")) {
+        throw new IllegalArgumentException(
+            "Voyager string keys may not contain backslashes "
+                + "or double quotes, but found key: "
+                + item);
+      }
+      output.write(item);
+      output.write('"');
+    }
+    output.write("]");
+    output.flush();
+  }
+}


Test patch:
diff --git a/java/src/test/java/com/spotify/voyager/jni/utils/TinyJsonTest.java b/java/src/test/java/com/spotify/voyager/jni/utils/TinyJsonTest.java
new file mode 100644
index 00000000..bf077003
--- /dev/null
+++ b/java/src/test/java/com/spotify/voyager/jni/utils/TinyJsonTest.java
@@ -0,0 +1,86 @@
+/*-
+ * -\-\-
+ * voyager
+ * --
+ * Copyright (C) 2016 - 2023 Spotify AB
+ * --
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * -/-/-
+ */
+
+package com.spotify.voyager.jni.utils;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
+import org.junit.Test;
+
+public class TinyJsonTest {
+  @Test
+  public void testSerializeList() throws IOException {
+    final ByteArrayOutputStream os = new ByteArrayOutputStream();
+    TinyJson.writeStringList(Arrays.asList("a", "b", "c"), os);
+    assertEquals("[\"a\",\"b\",\"c\"]", os.toString());
+  }
+
+  @Test
+  public void testDeserializeList() {
+    final ByteArrayInputStream is = new ByteArrayInputStream("[\"a\",\"b\",\"c\"]".getBytes(UTF_8));
+    List<String> result = TinyJson.readStringList(is);
+    assertEquals(Arrays.asList("a", "b", "c"), result);
+  }
+
+  @Test
+  public void testDeserializeListWithWhitespace() {
+    final ByteArrayInputStream is =
+        new ByteArrayInputStream("[\"a\", \"b\",\n\n \"c\"]".getBytes(UTF_8));
+    List<String> result = TinyJson.readStringList(is);
+    assertEquals(Arrays.asList("a", "b", "c"), result);
+  }
+
+  @Test
+  public void testDeserializeNonList() {
+    final ByteArrayInputStream is = new ByteArrayInputStream("{}".getBytes(UTF_8));
+    try {
+      TinyJson.readStringList(is);
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+  }
+
+  @Test
+  public void testDeserializeNonStringList() {
+    final ByteArrayInputStream is = new ByteArrayInputStream("[\"a\", 1]".getBytes(UTF_8));
+    try {
+      TinyJson.readStringList(is);
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+  }
+
+  @Test
+  public void testDeserializeInvalidList() {
+    final ByteArrayInputStream is = new ByteArrayInputStream("[\"a\"}".getBytes(UTF_8));
+    try {
+      TinyJson.readStringList(is);
+      fail();
+    } catch (IllegalArgumentException ignored) {
+    }
+  }
+}