When adding an Optional to a list or an array, only

add those that are actually present.
SeleniumHQ · Aug 1, 2019 · de923a2 · de923a2
1 parent 2b89c4d
commit de923a2
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 16 deletions.
diff --git a/java/client/src/org/openqa/selenium/json/JsonOutput.java b/java/client/src/org/openqa/selenium/json/JsonOutput.java
@@ -33,6 +33,7 @@
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.function.Consumer;
 import java.util.function.Function;
@@ -156,7 +157,9 @@ public class JsonOutput implements Closeable {
         Collection.class::isAssignableFrom,
         (obj, depth) -> {
           beginArray();
-          ((Collection<?>) obj).forEach(o -> write(o, depth - 1));
+          ((Collection<?>) obj).stream()
+            .filter(o -> (!(o instanceof Optional) || ((Optional<?>) o).isPresent()))
+            .forEach(o -> write(o, depth - 1));
           endArray();
         });
 
@@ -165,17 +168,34 @@ public class JsonOutput implements Closeable {
         (obj, depth) -> {
           beginObject();
           ((Map<?, ?>) obj).forEach(
-              (key, value) -> name(String.valueOf(key)).write(value, depth - 1));
+              (key, value) -> {
+                if (value instanceof Optional && !((Optional) value).isPresent()) {
+                  return;
+                }
+                name(String.valueOf(key)).write(value, depth - 1);
+              });
           endObject();
         });
     builder.put(
         Class::isArray,
         (obj, depth) -> {
           beginArray();
-          Stream.of((Object[]) obj).forEach(o -> write(o, depth - 1));
+          Stream.of((Object[]) obj)
+            .filter(o -> (!(o instanceof Optional) || ((Optional<?>) o).isPresent()))
+            .forEach(o -> write(o, depth - 1));
           endArray();
         });
 
+    builder.put(Optional.class::isAssignableFrom, (obj, depth) -> {
+      Optional<?> optional = (Optional<?>) obj;
+      if (!optional.isPresent()) {
+        append("null");
+        return;
+      }
+
+      write(optional.get(), depth);
+    });
+
     // Finally, attempt to convert as an object
     builder.put(cls -> true, (obj, depth) -> mapObject(obj, depth - 1));
 

diff --git a/java/client/test/org/openqa/selenium/json/JsonOutputTest.java b/java/client/test/org/openqa/selenium/json/JsonOutputTest.java
@@ -17,25 +17,13 @@
 
 package org.openqa.selenium.json;
 
-import static java.lang.Integer.valueOf;
-import static java.util.Arrays.asList;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.openqa.selenium.json.Json.MAP_TYPE;
-import static org.openqa.selenium.logging.LogType.BROWSER;
-import static org.openqa.selenium.logging.LogType.CLIENT;
-import static org.openqa.selenium.logging.LogType.DRIVER;
-import static org.openqa.selenium.logging.LogType.SERVER;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSortedSet;
 import com.google.gson.JsonArray;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
 import com.google.gson.JsonPrimitive;
-
 import org.junit.Test;
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.Cookie;
@@ -55,7 +43,7 @@
 import org.openqa.selenium.remote.DriverCommand;
 import org.openqa.selenium.remote.SessionId;
 
-import java.awt.Point;
+import java.awt.*;
 import java.io.IOException;
 import java.io.StringReader;
 import java.io.StringWriter;
@@ -67,10 +55,22 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.stream.Stream;
 
+import static java.lang.Integer.valueOf;
+import static java.util.Arrays.asList;
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.openqa.selenium.json.Json.MAP_TYPE;
+import static org.openqa.selenium.logging.LogType.BROWSER;
+import static org.openqa.selenium.logging.LogType.CLIENT;
+import static org.openqa.selenium.logging.LogType.DRIVER;
+import static org.openqa.selenium.logging.LogType.SERVER;
+
 public class JsonOutputTest {
 
   @Test
@@ -626,6 +626,26 @@ public void shouldEncodeLogLevelsAsStrings() {
     assertThat(converted).isEqualTo("\"INFO\"");
   }
 
+  @Test
+  public void shouldNotWriteOptionalFieldsThatAreEmptyInAMap() {
+    String json = convert(ImmutableMap.of("there", Optional.of("cheese"), "notThere", Optional.empty()));
+
+    JsonObject converted = new JsonParser().parse(json).getAsJsonObject();
+
+    assertThat(converted.has("notThere")).isFalse();
+    assertThat(converted.get("there").getAsString()).isEqualTo("cheese");
+  }
+
+  @Test
+  public void shouldNotWriteOptionalsThatAreNotPresentToAList() {
+    String json = convert(ImmutableList.of(Optional.of("cheese"), Optional.empty()));
+
+    JsonArray converted = new JsonParser().parse(json).getAsJsonArray();
+
+    assertThat(converted.size()).isEqualTo(1);
+    assertThat(converted.get(0).getAsString()).isEqualTo("cheese");
+  }
+
   private String convert(Object toConvert) {
     try (Writer writer = new StringWriter();
          JsonOutput jsonOutput = new Json().newOutput(writer)) {