Merge pull request #19509 from vespa-engine/jonmv/fix-docid-parse-bugv7.481.18

Fix document id parse bug in feed client

4 files changed, 80 insertions, 12 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
index 1afd78fc7c2..a07a36d800b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
@@ -514,4 +514,4 @@ public class StorageGroup {
         }
     }
 
-}
+}
\ No newline at end of file
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/DocumentId.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/DocumentId.java
index a0722f1f6dc..5474bcfda01 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/DocumentId.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/DocumentId.java
@@ -41,20 +41,34 @@ public class DocumentId {
     }
 
     public static DocumentId of(String serialized) {
-        String[] parts = serialized.split(":");
-        while (parts.length >= 5 && parts[0].equals("id")) {
-            if (parts[3].startsWith("n="))
-                return DocumentId.of(parts[1], parts[2], Long.parseLong(parts[3]), parts[4]);
-            if (parts[3].startsWith("g="))
-                return DocumentId.of(parts[1], parts[2], parts[3], parts[4]);
-            else if (parts[3].isEmpty())
-                return DocumentId.of(parts[1], parts[2], parts[4]);
-        }
+        DocumentId parsed = parse(serialized);
+        if (parsed != null) return parsed;
         throw new IllegalArgumentException("Document ID must be on the form " +
-                                           "'id:<namespace>:<document-type>:[n=number|g=group]:<user-specific>', " +
+                                           "'id:<namespace>:<document-type>:[n=<number>|g=<group>]:<user-specific>', " +
                                            "but was '" + serialized + "'");
     }
 
+    private static DocumentId parse(String serialized) {
+        int i, j = -1;
+        if ((j = serialized.indexOf(':', i = j + 1)) < i) return null;
+        if ( ! "id".equals(serialized.substring(i, j))) return null;
+        if ((j = serialized.indexOf(':', i = j + 1)) <= i) return null;
+        String namespace = serialized.substring(i, j);
+        if ((j = serialized.indexOf(':', i = j + 1)) <= i) return null;
+        String documentType = serialized.substring(i, j);
+        if ((j = serialized.indexOf(':', i = j + 1)) < i) return null;
+        String group = serialized.substring(i, j);
+        if (serialized.length() <= (i = j + 1)) return null;
+        String userSpecific = serialized.substring(i);
+        if (group.startsWith("n=") && group.length() > 2)
+            return DocumentId.of(namespace, documentType, Long.parseLong(group.substring(2)), userSpecific);
+        if (group.startsWith("g=") && group.length() > 2)
+            return DocumentId.of(namespace, documentType, group.substring(2), userSpecific);
+        if (group.isEmpty())
+            return DocumentId.of(namespace, documentType, userSpecific);
+        return null;
+    }
+
     public String documentType() {
         return documentType;
     }
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
index 042d0266de2..b878840d70f 100644
--- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
+++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/GracePeriodCircuitBreaker.java
@@ -61,7 +61,7 @@ public class GracePeriodCircuitBreaker implements FeedClient.CircuitBreaker {
 
     @Override
     public void failure(Throwable cause) {
-        failure(cause.getMessage());
+        failure(cause.toString());
     }
 
     private void failure(String detail) {
diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/DocumentIdTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/DocumentIdTest.java
new file mode 100644
index 00000000000..df790056309
--- /dev/null
+++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/DocumentIdTest.java
@@ -0,0 +1,54 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package ai.vespa.feed.client;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * @author jonmv
+ */
+class DocumentIdTest {
+
+    @Test
+    void testParsing() {
+        assertEquals("id:ns:type::user",
+                     DocumentId.of("id:ns:type::user").toString());
+
+        assertEquals("id:ns:type:n=123:user",
+                     DocumentId.of("id:ns:type:n=123:user").toString());
+
+        assertEquals("id:ns:type:g=foo:user",
+                     DocumentId.of("id:ns:type:g=foo:user").toString());
+
+        assertEquals("id:ns:type::user::specific",
+                     DocumentId.of("id:ns:type::user::specific").toString());
+
+        assertEquals("id:ns:type:::",
+                     DocumentId.of("id:ns:type:::").toString());
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("idd:ns:type:user"));
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("id:ns::user"));
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("id::type:user"));
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("id:ns:type:g=:user"));
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("id:ns:type:n=:user"));
+
+        assertThrows(NumberFormatException.class,
+                     () -> DocumentId.of("id:ns:type:n=foo:user"));
+
+        assertThrows(IllegalArgumentException.class,
+                     () -> DocumentId.of("id:ns:type::"));
+    }
+
+}