From 6587de95f2d732f9d3259b2baac60d7edf0fd965 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Mon, 11 Oct 2021 22:13:45 +0200 Subject: Fix document id parse bug in feed client --- .../yahoo/vespa/model/content/StorageGroup.java | 2 +- .../main/java/ai/vespa/feed/client/DocumentId.java | 34 ++++++++++---- .../feed/client/GracePeriodCircuitBreaker.java | 2 +- .../java/ai/vespa/feed/client/DocumentIdTest.java | 54 ++++++++++++++++++++++ 4 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 vespa-feed-client/src/test/java/ai/vespa/feed/client/DocumentIdTest.java 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 0997f29729f..bc824088936 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 @@ -507,4 +507,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 39fc9fb28e0..b4a888d671c 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:::[n=number|g=group]:', " + + "'id:::[n=|g=]:', " + "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 2c4641f38d5..55c766ab474 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::")); + } + +} -- cgit v1.2.3