[Core] Use null safe-messages (#2497)

With cucumber/common#1879 `messages` now uses `Optional<T>` for optional types avoiding some unexpected null references. This resulted in some updating of `gherkin` and `html-formatter`. Additionally with this change the dependency on Jackson has been removed from `messages`. However we still have to serialize the message objects into json. So for now Jackson is shaded into `core` instead. And eventually we'll be able to spin this out to it's own module.
cucumber · Apr 19, 2022 · cace210 · cace210
1 parent 655eb1f
commit cace210
Show file tree

Hide file tree

Showing 43 changed files with 511 additions and 507 deletions.
diff --git a/.revapi/api-changes.json b/.revapi/api-changes.json
@@ -106,6 +106,63 @@
             "old": "class io.cucumber.gherkin.internal.com.eclipsesource.json.JsonWriter",
             "new": "class io.cucumber.gherkin.internal.com.eclipsesource.json.JsonWriter",
             "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.method.returnTypeChanged",
+            "old": ".* io\\.cucumber\\.messages\\..*",
+            "new": ".* io\\.cucumber\\.messages\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.method.removed",
+            "old": ".* io\\.cucumber\\.messages\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.nowFinal",
+            "old": ".* io\\.cucumber\\.messages\\..*",
+            "new": ".* io\\.cucumber\\.messages\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.method.numberOfParametersChanged",
+            "old": ".* io\\.cucumber\\.messages\\..*",
+            "new": ".* io\\.cucumber\\.messages\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.removed",
+            "old": ".* io\\.cucumber\\.messages\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.removed",
+            "old": ".* io\\.cucumber\\.core\\.gherkin\\.messages\\.internal\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.removed",
+            "old": ".* io\\.cucumber\\.gherkin\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.removed",
+            "old": ".* io\\.cucumber\\.gherkin\\.internal\\..*",
+            "justification": "Internal API"
+          },
+          {
+            "regex": true,
+            "code": "java.class.nonPublicPartOfAPI",
+            "new": ".* io\\.cucumber\\.core\\.internal\\..*",
+            "justification": "Internal API"
           }
         ]
       }

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,9 +11,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 *  [JUnit Platform] Support `cucumber.features` property ([#2498](https://github.com/cucumber/cucumber-jvm/pull/2498) M.P. Korstanje)
 
 ### Changed
+* [Core] Use null-safe messages ([#2497](https://github.com/cucumber/cucumber-jvm/pull/2497) M.P. Korstanje)
+  * Update dependency io.cucumber:html-formatter to v19.1
+      - Removed work around for 'Uncaught TypeError: e.git is undefined'
+  * Update dependency io.cucumber:messages to v18
+  * Update dependency io.cucumber:gherkin to v23
+  * Moved shaded jackson from `messages` to `core`. 
+
 * Update dependency io.cucumber:ci-environment to v9 ([#2475](https://github.com/cucumber/cucumber-jvm/pull/2475) M.P. Korstanje)
-* Update dependency io.cucumber:html-formatter to v18 ([#2476](https://github.com/cucumber/cucumber-jvm/pull/2476) M.P. Korstanje)
-  - Removed work around for 'Uncaught TypeError: e.git is undefined'    
 * Update dependency com.google.inject:guice to v5.1.0 ([#2473](https://github.com/cucumber/cucumber-jvm/pull/2473) M.P. Korstanje)
 * Update dependency org.testng:testng to v7.5 ([#2457](https://github.com/cucumber/cucumber-jvm/pull/2457) M.P. Korstanje)
 

diff --git a/bom/pom.xml b/bom/pom.xml
@@ -14,8 +14,7 @@
     <properties>
         <ci-environment.version>9.0.4</ci-environment.version>
         <cucumber-expressions.version>15.0.2</cucumber-expressions.version>
-        <datatable.version>4.1.0</datatable.version>
-        <html-formatter.version>18.0.0</html-formatter.version>
+        <html-formatter.version>19.1.0</html-formatter.version>
         <tag-expressions.version>4.1.0</tag-expressions.version>
     </properties>
 

diff --git a/core/pom.xml b/core/pom.xml
@@ -15,7 +15,7 @@
     <properties>
         <project.Automatic-Module-Name>io.cucumber.core</project.Automatic-Module-Name>
         <apiguardian-api.version>1.1.2</apiguardian-api.version>
-        <jackson-databind.version>2.13.2.2</jackson-databind.version>
+        <jackson-databind.version>2.13.2.20220328</jackson-databind.version>
         <jsoup.version>1.14.3</jsoup.version>
         <junit-jupiter.version>5.8.2</junit-jupiter.version>
         <xmlunit.version>2.9.0</xmlunit.version>
@@ -42,14 +42,17 @@
                 <type>pom</type>
                 <scope>import</scope>
             </dependency>
+            <dependency>
+                <groupId>com.fasterxml.jackson</groupId>
+                <artifactId>jackson-bom</artifactId>
+                <version>${jackson-databind.version}</version>
+                <type>pom</type>
+                <scope>import</scope>
+            </dependency>
         </dependencies>
     </dependencyManagement>
 
     <dependencies>
-        <dependency>
-            <groupId>io.cucumber</groupId>
-            <artifactId>gherkin</artifactId>
-        </dependency>
         <dependency>
             <groupId>io.cucumber</groupId>
             <artifactId>cucumber-gherkin</artifactId>
@@ -95,6 +98,14 @@
             <artifactId>apiguardian-api</artifactId>
             <version>${apiguardian-api.version}</version>
         </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.core</groupId>
+            <artifactId>jackson-databind</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>com.fasterxml.jackson.datatype</groupId>
+            <artifactId>jackson-datatype-jdk8</artifactId>
+        </dependency>
 
         <dependency>
             <groupId>org.xmlunit</groupId>
@@ -136,42 +147,12 @@
             <artifactId>vertx-web</artifactId>
             <version>${vertx.version}</version>
             <scope>test</scope>
-            <exclusions>
-                <!-- Fix dependency convergence -->
-                <exclusion>
-                    <groupId>com.fasterxml.jackson.core</groupId>
-                    <artifactId>jackson-databind</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>com.fasterxml.jackson.core</groupId>
-                    <artifactId>jackson-core</artifactId>
-                </exclusion>
-            </exclusions>
         </dependency>
         <dependency>
             <groupId>io.vertx</groupId>
             <artifactId>vertx-junit5</artifactId>
             <version>${vertx.version}</version>
             <scope>test</scope>
-            <exclusions>
-                <!-- Fix dependency convergence -->
-                <exclusion>
-                    <groupId>org.junit.jupiter</groupId>
-                    <artifactId>junit-jupiter-params</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>org.reactivestreams</groupId>
-                    <artifactId>reactive-streams</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>com.fasterxml.jackson.core</groupId>
-                    <artifactId>jackson-databind</artifactId>
-                </exclusion>
-                <exclusion>
-                    <groupId>com.fasterxml.jackson.core</groupId>
-                    <artifactId>jackson-core</artifactId>
-                </exclusion>
-            </exclusions>
         </dependency>
         <!-- Fix dependency convergence -->
         <dependency>
@@ -204,13 +185,6 @@
                 </exclusion>
             </exclusions>
         </dependency>
-        <dependency>
-            <groupId>com.fasterxml.jackson.core</groupId>
-            <artifactId>jackson-databind</artifactId>
-            <version>${jackson-databind.version}</version>
-            <scope>test</scope>
-        </dependency>
-
     </dependencies>
 
     <build>
@@ -267,21 +241,51 @@
                         <configuration>
                             <artifactSet>
                                 <includes>
-                                    <include>io.cucumber:gherkin</include>
+                                    <include>com.fasterxml.jackson.core:jackson-databind</include>
+                                    <include>com.fasterxml.jackson.core:jackson-core</include>
+                                    <include>com.fasterxml.jackson.core:jackson-annotations</include>
+                                    <include>com.fasterxml.jackson.datatype:jackson-datatype-jdk8</include>
                                 </includes>
                             </artifactSet>
-                            <!-- Do not enable relocation. Stuck with it until the next major. -->
-                            <!--<relocations>-->
-                            <!--    <relocation>-->
-                            <!--        <pattern>io.cucumber.gherkin</pattern>-->
-                            <!--        <shadedPattern>io.cucumber.core.internal.gherkin</shadedPattern>-->
-                            <!--    </relocation>-->
-                            <!--</relocations>-->
+                            <relocations>
+                                <relocation>
+                                    <pattern>com.fasterxml</pattern>
+                                    <shadedPattern>io.cucumber.core.internal.com.fasterxml</shadedPattern>
+                                </relocation>
+                            </relocations>
                             <filters>
                                 <filter>
-                                    <artifact>io.cucumber:gherkin</artifact>
+                                    <artifact>com.fasterxml.jackson.core:jackson-databind</artifact>
+                                    <excludes>
+                                        <exclude>**/module-info.class</exclude>
+                                        <exclude>META-INF/MANIFEST.MF</exclude>
+                                        <exclude>META-INF/LICENSE</exclude>
+                                        <exclude>META-INF/NOTICE</exclude>
+                                    </excludes>
+                                </filter>
+                                <filter>
+                                    <artifact>com.fasterxml.jackson.core:jackson-core</artifact>
+                                    <excludes>
+                                        <exclude>**/module-info.class</exclude>
+                                        <exclude>META-INF/MANIFEST.MF</exclude>
+                                        <exclude>META-INF/LICENSE</exclude>
+                                        <exclude>META-INF/NOTICE</exclude>
+                                    </excludes>
+                                </filter>
+                                <filter>
+                                    <artifact>com.fasterxml.jackson.core:jackson-annotations</artifact>
+                                    <excludes>
+                                        <exclude>**/module-info.class</exclude>
+                                        <exclude>META-INF/MANIFEST.MF</exclude>
+                                        <exclude>META-INF/LICENSE</exclude>
+                                    </excludes>
+                                </filter>
+                                <filter>
+                                    <artifact>com.fasterxml.jackson.datatype:jackson-datatype-jdk8</artifact>
                                     <excludes>
+                                        <exclude>**/module-info.class</exclude>
                                         <exclude>META-INF/MANIFEST.MF</exclude>
+                                        <exclude>META-INF/LICENSE</exclude>
                                     </excludes>
                                 </filter>
                             </filters>

diff --git a/core/src/main/java/io/cucumber/core/options/CommandlineOptionsParser.java b/core/src/main/java/io/cucumber/core/options/CommandlineOptionsParser.java
@@ -6,9 +6,9 @@
 import io.cucumber.core.logging.Logger;
 import io.cucumber.core.logging.LoggerFactory;
 import io.cucumber.datatable.DataTable;
+import io.cucumber.datatable.DataTableFormatter;
 import io.cucumber.gherkin.GherkinDialect;
 import io.cucumber.gherkin.GherkinDialectProvider;
-import io.cucumber.gherkin.IGherkinDialectProvider;
 import io.cucumber.tagexpressions.TagExpressionParser;
 
 import java.io.BufferedReader;
@@ -198,7 +198,7 @@ private String removeArgFor(String arg, List<String> args) {
     }
 
     private byte printI18n(String language) {
-        IGherkinDialectProvider dialectProvider = new GherkinDialectProvider();
+        GherkinDialectProvider dialectProvider = new GherkinDialectProvider();
         List<String> languages = dialectProvider.getLanguages();
 
         if (language.equalsIgnoreCase("help")) {
@@ -233,7 +233,7 @@ private String loadUsageText() {
                 BufferedReader br = new BufferedReader(new InputStreamReader(usageResourceStream, UTF_8))) {
             return br.lines().collect(joining(System.lineSeparator()));
         } catch (Exception e) {
-            return "Could not load usage text: " + e.toString();
+            return "Could not load usage text: " + e;
         }
     }
 
@@ -271,8 +271,11 @@ private byte printKeywordsFor(GherkinDialect dialect) {
         addCodeKeywordRow(table, "then", dialect.getThenKeywords());
         addCodeKeywordRow(table, "and", dialect.getAndKeywords());
         addCodeKeywordRow(table, "but", dialect.getButKeywords());
-        DataTable.create(table).print(builder);
-        out.println(builder.toString());
+        DataTableFormatter.builder()
+                .prefixRow("      ")
+                .build()
+                .formatTo(DataTable.create(table), builder);
+        out.println(builder);
         return 0x0;
     }
 

diff --git a/core/src/main/java/io/cucumber/core/plugin/HtmlFormatter.java b/core/src/main/java/io/cucumber/core/plugin/HtmlFormatter.java
@@ -14,7 +14,7 @@ public final class HtmlFormatter implements ConcurrentEventListener {
 
     @SuppressWarnings("WeakerAccess") // Used by PluginFactory
     public HtmlFormatter(OutputStream out) throws IOException {
-        this.writer = new MessagesToHtmlWriter(new UTF8OutputStreamWriter(out));
+        this.writer = new MessagesToHtmlWriter(out, Jackson.OBJECT_MAPPER::writeValue);
     }
 
     @Override
@@ -25,7 +25,8 @@ public void setEventPublisher(EventPublisher publisher) {
     private void write(Envelope event) {
         // Workaround to reduce the size of the report
         // See: https://github.com/cucumber/cucumber/issues/1232
-        if (event.getStepDefinition() != null || event.getHook() != null || event.getParameterType() != null) {
+        if (event.getStepDefinition().isPresent() || event.getHook().isPresent()
+                || event.getParameterType().isPresent()) {
             return;
         }
 
@@ -37,7 +38,7 @@ private void write(Envelope event) {
 
         // TODO: Plugins should implement the closable interface
         // and be closed by Cucumber
-        if (event.getTestRunFinished() != null) {
+        if (event.getTestRunFinished().isPresent()) {
             try {
                 writer.close();
             } catch (IOException e) {

diff --git a/core/src/main/java/io/cucumber/core/plugin/Jackson.java b/core/src/main/java/io/cucumber/core/plugin/Jackson.java
@@ -0,0 +1,27 @@
+package io.cucumber.core.plugin;
+
+import com.fasterxml.jackson.annotation.JsonInclude.Include;
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.databind.cfg.ConstructorDetector;
+import com.fasterxml.jackson.databind.json.JsonMapper;
+import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
+
+final class Jackson {
+    public static final ObjectMapper OBJECT_MAPPER = JsonMapper.builder()
+            .addModule(new Jdk8Module())
+            .serializationInclusion(Include.NON_ABSENT)
+            .constructorDetector(ConstructorDetector.USE_PROPERTIES_BASED)
+            .enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING)
+            .enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING)
+            .enable(DeserializationFeature.USE_LONG_FOR_INTS)
+            .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+            .disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET)
+            .build();
+
+    private Jackson() {
+    }
+
+}
diff --git a/core/src/main/java/io/cucumber/core/plugin/JsonFormatter.java b/core/src/main/java/io/cucumber/core/plugin/JsonFormatter.java
@@ -1,6 +1,5 @@
 package io.cucumber.core.plugin;
 
-import io.cucumber.messages.JSON;
 import io.cucumber.messages.types.Background;
 import io.cucumber.messages.types.Feature;
 import io.cucumber.messages.types.Scenario;
@@ -145,7 +144,7 @@ private void finishReport(TestRunFinished event) {
         }
 
         try {
-            JSON.writeValue(writer, featureMaps);
+            Jackson.OBJECT_MAPPER.writeValue(writer, featureMaps);
             writer.close();
         } catch (IOException e) {
             throw new RuntimeException(e);

diff --git a/core/src/main/java/io/cucumber/core/plugin/MessageFormatter.java b/core/src/main/java/io/cucumber/core/plugin/MessageFormatter.java
@@ -1,6 +1,5 @@
 package io.cucumber.core.plugin;
 
-import io.cucumber.messages.JSON;
 import io.cucumber.messages.types.Envelope;
 import io.cucumber.plugin.ConcurrentEventListener;
 import io.cucumber.plugin.event.EventPublisher;
@@ -24,10 +23,10 @@ public void setEventPublisher(EventPublisher publisher) {
 
     private void writeMessage(Envelope envelope) {
         try {
-            JSON.writeValue(writer, envelope);
+            Jackson.OBJECT_MAPPER.writeValue(writer, envelope);
             writer.write("\n");
             writer.flush();
-            if (envelope.getTestRunFinished() != null) {
+            if (envelope.getTestRunFinished().isPresent()) {
                 writer.close();
             }
         } catch (IOException e) {