Non blocking JSON parser #2106

vietj · 2017-08-29T14:43:27Z

Provide an event driven and non blocking JSON parser that can also be used as a read stream. The parser can emit fine grained events for JSON structures or emit them as values in a dynamic fashio.

EmadAlblueshi · 2017-08-29T15:48:24Z

+1

tsegismont

A few comments/concerns. Otherwise, great work!

tsegismont · 2017-08-30T14:04:15Z

pom.xml

@@ -117,10 +117,12 @@
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-core</artifactId>
+      <version>2.9.0</version>


Shouldn't we update the BOM (vertx-dependencies) rather than this POM?

yes it will be, before merging that PR, I'll update the vertx-dependencies, for now it's hardcoded here.

And it's not a BOM :-)

tsegismont · 2017-08-30T14:04:30Z

pom.xml

    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
+      <version>2.9.0</version>


Same here, BOM vs POM

it's not a BOM again :-)

tsegismont · 2017-08-30T14:05:53Z

src/main/asciidoc/java/parsetools.adoc

+== Json Parser
+
+You can easily parse JSON structures but that requires to provide the JSON string at once, but it


Minor: JSON content instead of JSON string. It's not always a string, it can be a buffer as well

the buffer is encoded as a JSON string though

ah ok, I see what you mean :-)

tsegismont · 2017-08-30T14:08:29Z

src/main/asciidoc/java/parsetools.adoc

+----
+
+The parser is non-blocking and emitted events are driven by the input buffers


Minor: missing final dot.

tsegismont · 2017-08-30T14:09:39Z

src/main/asciidoc/java/parsetools.adoc

+// start object event
+// "firstName":"Bob" event
+parser.handle(Buffer.buffer("[{\"firstName\":\"Bob\"},"));


I think the end parenthesis shouldn't be there.

it's valid java code

Yes the Java code compiles. I was wondering if the parenthesis inside the JSON content would not trigger an end object event. Am I wrong?

I don't see parenthesis in JSON content

you mean curly brace ?

Ha. Yeah, I meant curly brace. Sorry.

the content was wrong, I think I fixed it now

tsegismont · 2017-08-30T14:14:38Z

src/main/asciidoc/java/parsetools.adoc

+----
+
+Event driven parsing provides more control but comes at the price of flexibility. The parser allows you


Why at the price of flexibility? If the parser gives your more control, doesn't this make it more flexible?

I want to convey that if you deal with fine grained events, it becomes harder to parse because you have to care about lot of individual events which can be inconvenient, I will try to say that differently.

tsegismont · 2017-08-30T14:18:16Z

src/main/asciidoc/java/parsetools.adoc

+  switch (event.type()) {
+    case START_OBJECT:
+      // Set object value mode to handle each entry, from now on the parser won't emit start object events


IIUC, when object value mode is enabled, all nested objects/arrays are parsed at once?

I just reviewed the parser impl, it seems to be the case. Could you add tests for nested objects/arrays?

tsegismont · 2017-08-30T14:21:38Z

src/main/java/io/vertx/core/parsetools/JsonEvent.java

+   * @throws java.lang.ClassCastException if the value is not a String
+   * @throws java.time.format.DateTimeParseException if the String value is not a legal ISO 8601 encoded value
+   */
+  Instant instantValue();


I thought Instant didn't work with codegen

yes, I'm going to add "@GenIgnore", I was testing using the IDE that does not run the annotation processor but in reality it fails :-)

tsegismont · 2017-08-30T14:27:03Z

src/main/java/io/vertx/core/parsetools/JsonEventType.java

+  START_ARRAY,
+
+  /**
+   * Signals the end of a JSON object.


object => array

tsegismont · 2017-08-30T14:31:17Z

src/main/java/io/vertx/core/parsetools/JsonParser.java

+   * @return  a reference to this, so the API can be used fluently
+   */
+  @Fluent
+  JsonParser arrayValueMode();


Could you also add doc/example for this one?

vietj · 2017-08-31T08:12:21Z

PTAL

tsegismont · 2017-08-31T08:22:41Z

src/main/asciidoc/java/parsetools.adoc

-Event driven parsing provides more control but comes at the price of flexibility. The parser allows you
-to handle JSON structures as values when it is desired:
+Event driven parsing provides more control but comes at the price of dealing with fine grained events, which can be
+inconvenient sometimes. The JSON parser allows you to handle JSON structures as values when it is desired:


tsegismont · 2017-08-31T08:27:02Z

src/main/asciidoc/java/parsetools.adoc

@@ -103,17 +103,17 @@ JsonParser parser = JsonParser.newParser();
 // start array event
 // start object event
 // "firstName":"Bob" event
-parser.handle(Buffer.buffer("[{\"firstName\":\"Bob\"},"));
+parser.handle(Buffer.buffer("[{\"firstName\":\"Bob\","));


tsegismont · 2017-08-31T08:29:24Z

LGTM. I can't merge (permissions issue we talk about yesterday) but anyway you have to update the ~~BOM~~ vertx-dependencies module first ;)

vietj added 2 commits August 28, 2017 09:51

Non blocking json parser

e29fa50

Doc for JsonEventType

ee201b9

vietj added the to review label Aug 29, 2017

Update headers date

52bdf0f

tsegismont requested changes Aug 30, 2017

View reviewed changes

Address concerns provided in the PR feedback

ec296c7

tsegismont reviewed Aug 31, 2017

View reviewed changes

Fix wrong JSON content

ecbad27

tsegismont reviewed Aug 31, 2017

View reviewed changes

tsegismont approved these changes Aug 31, 2017

View reviewed changes

vietj merged commit ecbad27 into master Aug 31, 2017

vietj removed the to review label Aug 31, 2017

tsegismont mentioned this pull request Aug 31, 2017

Scala class should implement Handler via mixin when it has super types vert-x3/vertx-lang-scala#49

Closed

vietj deleted the jackson-non-blocking2 branch September 20, 2017 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Non blocking JSON parser #2106

Non blocking JSON parser #2106

vietj commented Aug 29, 2017

EmadAlblueshi commented Aug 29, 2017

tsegismont left a comment

tsegismont Aug 30, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

tsegismont Aug 31, 2017

vietj Aug 31, 2017

vietj Aug 31, 2017

tsegismont Aug 31, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

tsegismont Aug 30, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

tsegismont Aug 30, 2017

tsegismont Aug 30, 2017

vietj Aug 31, 2017

vietj commented Aug 31, 2017

tsegismont Aug 31, 2017

tsegismont Aug 31, 2017 •

edited

Loading

tsegismont commented Aug 31, 2017

		== Json Parser

		You can easily parse JSON structures but that requires to provide the JSON string at once, but it

		----

		The parser is non-blocking and emitted events are driven by the input buffers

		----

		Event driven parsing provides more control but comes at the price of flexibility. The parser allows you

Non blocking JSON parser #2106

Non blocking JSON parser #2106

Conversation

vietj commented Aug 29, 2017

EmadAlblueshi commented Aug 29, 2017

tsegismont left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vietj commented Aug 31, 2017

Choose a reason for hiding this comment

tsegismont Aug 31, 2017 • edited Loading

Choose a reason for hiding this comment

tsegismont commented Aug 31, 2017

tsegismont Aug 31, 2017 •

edited

Loading