diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2021-06-28 15:00:31 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2021-06-28 15:00:31 +0200 |
commit | 982f6a310ffa4ee2db74781170622f890615fb5c (patch) | |
tree | c1eedc18427f0701a9ddca3b8437b33cee8b0645 /vespa-feed-client | |
parent | af4891e7c47634a9b36fd07ef672fb49d4d5e9ed (diff) |
Test and fix removes in JSON feed parser
Diffstat (limited to 'vespa-feed-client')
3 files changed, 84 insertions, 28 deletions
diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/JsonFeeder.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/JsonFeeder.java index eb163a02bf0..68e102ab079 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/JsonFeeder.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/JsonFeeder.java @@ -3,6 +3,7 @@ package ai.vespa.feed.client; import ai.vespa.feed.client.FeedClient.OperationType; import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonLocation; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.JsonToken; @@ -358,6 +359,12 @@ public class JsonFeeder implements Closeable { abstract String getDocumentJson(long start, long end); + OperationParseException parseException(String error) { + JsonLocation location = parser.getTokenLocation(); + return new OperationParseException(error + " at offset " + location.getByteOffset() + + " (line " + location.getLineNr() + ", column " + location.getColumnNr() + ")"); + } + CompletableFuture<Result> next() throws IOException { JsonToken token = parser.nextToken(); if (multipleOperations && ! arrayPrefixParsed && token == START_ARRAY) { @@ -366,7 +373,7 @@ public class JsonFeeder implements Closeable { } if (token == END_ARRAY && multipleOperations) return null; else if (token == null && ! arrayPrefixParsed) return null; - else if (token != START_OBJECT) throw new OperationParseException("Unexpected token '" + parser.currentToken() + "' at offset " + parser.getTokenLocation().getByteOffset()); + else if (token != START_OBJECT) throw parseException("Unexpected token '" + parser.currentToken() + "'"); long start = 0, end = -1; OperationType type = null; DocumentId id = null; @@ -392,8 +399,7 @@ public class JsonFeeder implements Closeable { end = parser.getTokenLocation().getByteOffset() + 1; break; } - default: throw new OperationParseException("Unexpected field name '" + parser.getText() + "' at offset " + - parser.getTokenLocation().getByteOffset()); + default: throw parseException("Unexpected field name '" + parser.getText() + "'"); } break; @@ -401,15 +407,20 @@ public class JsonFeeder implements Closeable { break loop; default: - throw new OperationParseException("Unexpected token '" + parser.currentToken() + "' at offset " + - parser.getTokenLocation().getByteOffset()); + throw parseException("Unexpected token '" + parser.currentToken() + "'"); } } if (id == null) - throw new OperationParseException("No document id for document at offset " + start); + throw parseException("No document id for document"); + if (type == REMOVE) { + if (end >= start) + throw parseException("Illegal 'fields' object for remove operation"); + else + start = end = parser.getTokenLocation().getByteOffset(); // getDocumentJson advances buffer overwrite head. + } + else if (end < start) + throw parseException("No 'fields' object for document"); - if (end < start) - throw new OperationParseException("No 'fields' object for document at offset " + parser.getTokenLocation().getByteOffset()); String payload = getDocumentJson(start, end); switch (type) { case PUT: return client.put (id, payload, parameters); @@ -479,4 +490,5 @@ public class JsonFeeder implements Closeable { } } + } diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/Result.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/Result.java index 7be7aadc188..790b5cf0b4a 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/Result.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/Result.java @@ -33,4 +33,14 @@ public class Result { public Optional<String> resultMessage() { return Optional.ofNullable(resultMessage); } public Optional<String> traceMessage() { return Optional.ofNullable(traceMessage); } + @Override + public String toString() { + return "Result{" + + "type=" + type + + ", documentId=" + documentId + + ", resultMessage='" + resultMessage + '\'' + + ", traceMessage='" + traceMessage + '\'' + + '}'; + } + } diff --git a/vespa-feed-client/src/test/java/ai/vespa/feed/client/JsonFeederTest.java b/vespa-feed-client/src/test/java/ai/vespa/feed/client/JsonFeederTest.java index 79dd68f241a..da0c45c94b5 100644 --- a/vespa-feed-client/src/test/java/ai/vespa/feed/client/JsonFeederTest.java +++ b/vespa-feed-client/src/test/java/ai/vespa/feed/client/JsonFeederTest.java @@ -10,6 +10,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.Arrays; +import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -26,6 +27,7 @@ import static java.util.stream.Collectors.joining; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class JsonFeederTest { @@ -60,8 +62,8 @@ class JsonFeederTest { long startNanos = System.nanoTime(); MockClient feedClient = new MockClient(); JsonFeeder.builder(feedClient).build() - .feedMany(in, 1 << 7, - new JsonFeeder.ResultCallback() { // TODO: hangs when buffer is smaller than largest document + .feedMany(in, 1 << 10, + new JsonFeeder.ResultCallback() { @Override public void onNextResult(Result result, FeedException error) { resultsReceived.incrementAndGet(); } @@ -108,22 +110,30 @@ class JsonFeederTest { public void multipleJsonLOperationsAreDispatchedToFeedClient() throws IOException, ExecutionException, InterruptedException { MockClient client = new MockClient(); try (JsonFeeder feeder = JsonFeeder.builder(client).build()) { - String json = "{" + - " \"put\": \"id:ns:type::abc1\",\n" + + String json = "{\n" + + " \"remove\": \"id:ns:type::abc1\"\n" + + "}\n" + + "{\n" + " \"fields\": {\n" + - " \"lul\":\"lal\"\n" + - " }\n" + + " \"lul\": { \"assign\": \"lal\" }\n" + + " },\n" + + " \"update\": \"id:ns:type::abc2\"\n" + "}\n" + - "{" + - " \"put\": \"id:ns:type::abc2\",\n" + + "{\n" + + " \"put\": \"id:ns:type::abc3\",\n" + " \"fields\": {\n" + - " \"lul\":\"lal\"\n" + + " \"lul\": \"lal\"\n" + " }\n" + "}\n"; - feeder.feedMany(new ByteArrayInputStream(json.getBytes(UTF_8))).get(); - client.assertPutDocumentIds("abc1", "abc2"); - client.assertPutOperation("abc1", "{\"fields\":{\n \"lul\":\"lal\"\n }}"); - client.assertPutOperation("abc2", "{\"fields\":{\n \"lul\":\"lal\"\n }}"); + + feeder.feedMany(new ByteArrayInputStream(json.getBytes(UTF_8)), + new JsonFeeder.ResultCallback() { }) + .get(); + client.assertRemoveDocumentIds("abc1"); + client.assertUpdateDocumentIds("abc2"); + client.assertUpdateOperation("abc2", "{\"fields\":{\n \"lul\": { \"assign\": \"lal\" }\n }}"); + client.assertPutDocumentIds("abc3"); + client.assertPutOperation("abc3", "{\"fields\":{\n \"lul\": \"lal\"\n }}"); } } @@ -146,6 +156,8 @@ class JsonFeederTest { private static class MockClient implements FeedClient { final Map<DocumentId, String> putOperations = new LinkedHashMap<>(); + final Map<DocumentId, String> updateOperations = new LinkedHashMap<>(); + final Map<DocumentId, String> removeOperations = new LinkedHashMap<>(); @Override public CompletableFuture<Result> put(DocumentId documentId, String documentJson, OperationParameters params) { @@ -155,11 +167,13 @@ class JsonFeederTest { @Override public CompletableFuture<Result> update(DocumentId documentId, String updateJson, OperationParameters params) { + updateOperations.put(documentId, updateJson); return createSuccessResult(documentId); } @Override public CompletableFuture<Result> remove(DocumentId documentId, OperationParameters params) { + removeOperations.put(documentId, null); return createSuccessResult(documentId); } @@ -176,23 +190,43 @@ class JsonFeederTest { return CompletableFuture.completedFuture(new Result(Result.Type.success, documentId, "success", null)); } - void assertPutDocumentIds(String... expectedUserSpecificIds) { + void assertDocumentIds(Collection<DocumentId> keys, String... expectedUserSpecificIds) { List<String> expected = Arrays.stream(expectedUserSpecificIds) - .map(userSpecific -> "id:ns:type::" + userSpecific) - .sorted() - .collect(Collectors.toList()); - List<String> actual = putOperations.keySet().stream() - .map(DocumentId::toString).sorted() - .collect(Collectors.toList()); + .map(userSpecific -> "id:ns:type::" + userSpecific) + .sorted() + .collect(Collectors.toList()); + List<String> actual = keys.stream() + .map(DocumentId::toString).sorted() + .collect(Collectors.toList()); assertEquals(expected, actual, "Document ids must match"); } + void assertPutDocumentIds(String... expectedUserSpecificIds) { + assertDocumentIds(putOperations.keySet(), expectedUserSpecificIds); + } + + void assertUpdateDocumentIds(String... expectedUserSpecificIds) { + assertDocumentIds(updateOperations.keySet(), expectedUserSpecificIds); + } + + void assertRemoveDocumentIds(String... expectedUserSpecificIds) { + assertDocumentIds(removeOperations.keySet(), expectedUserSpecificIds); + } + void assertPutOperation(String userSpecificId, String expectedJson) { DocumentId docId = DocumentId.of("id:ns:type::" + userSpecificId); String json = putOperations.get(docId); assertNotNull(json); assertEquals(expectedJson.trim(), json.trim()); } + + void assertUpdateOperation(String userSpecificId, String expectedJson) { + DocumentId docId = DocumentId.of("id:ns:type::" + userSpecificId); + String json = updateOperations.get(docId); + assertNotNull(json); + assertEquals(expectedJson.trim(), json.trim()); + } + } } |