[PAN-2721] Fix TopicParameter deserialization

* streamline `JsonRpcParameter.required` method * add test cases for PAN-2721 * clarify TopicParameter parsing comments * consume end of array in TopicParameter custom deserializer
PegaSysEng · Jun 13, 2019 · f39b0b6 · f39b0b6
1 parent d8a1dc7
commit f39b0b6
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 8 deletions.
diff --git a/...ain/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/JsonRpcParameter.java b/...ain/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/JsonRpcParameter.java
@@ -36,12 +36,11 @@ public class JsonRpcParameter {
    * @return Returns the parameter cast as T if available, otherwise throws exception.
    */
   public <T> T required(final Object[] params, final int index, final Class<T> paramClass) {
-    final Optional<T> optionalParam = optional(params, index, paramClass);
-    if (!optionalParam.isPresent()) {
-      throw new InvalidJsonRpcParameters("Missing required json rpc parameter at index " + index);
-    }
-
-    return optionalParam.get();
+    return optional(params, index, paramClass)
+        .orElseThrow(
+            () ->
+                new InvalidJsonRpcParameters(
+                    "Missing required json rpc parameter at index " + index));
   }
 
   /**

diff --git a/...n/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/TopicsDeserializer.java b/...n/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/TopicsDeserializer.java
@@ -37,11 +37,12 @@ public TopicsParameter deserialize(
     List<String> topicsList = new ArrayList<>();
 
     try {
-      // try standard method
+      // parse as list of lists
       return jsonparser.readValueAs(TopicsParameter.class);
     } catch (MismatchedInputException mie) {
-      // is there a single string value instead of expected list of list
+      // single list case
       String topics = jsonparser.getText();
+      jsonparser.nextToken(); // consume end of array character
       if (topics == null) {
         return new TopicsParameter(Collections.singletonList(topicsList));
       } else {

diff --git a/.../java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/FilterParameterTest.java b/.../java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/parameters/FilterParameterTest.java
@@ -111,6 +111,18 @@ public void jsonWithSingleAddressAndMultipleListsOfTopicsShouldSerializeSuccessf
         .isEqualToComparingFieldByFieldRecursively(expectedFilterParameter);
   }
 
+  @Test
+  public void jsonWithParamsInDifferentOrderShouldDeserializeIntoFilterParameterSuccessfully()
+      throws Exception {
+    final String jsonWithTopicsFirst =
+        "{\"topics\":[\"0x492e34c7da2a87c57444aa0f6143558999bceec63065f04557cfb20932e0d591\"], \"address\":\"0x8CdaF0CD259887258Bc13a92C0a6dA92698644C0\",\"fromBlock\":\"earliest\",\"toBlock\":\"latest\"}";
+    final String jsonWithTopicsLast =
+        "{\"address\":\"0x8CdaF0CD259887258Bc13a92C0a6dA92698644C0\",\"fromBlock\":\"earliest\",\"toBlock\":\"latest\",\"topics\":[\"0x492e34c7da2a87c57444aa0f6143558999bceec63065f04557cfb20932e0d591\"]}";
+
+    assertThat(readJsonAsFilterParameter(jsonWithTopicsFirst))
+        .isEqualToComparingFieldByFieldRecursively(readJsonAsFilterParameter(jsonWithTopicsLast));
+  }
+
   private FilterParameter filterParameterWithAddresses(final String... addresses) {
     return new FilterParameter("latest", "latest", Arrays.asList(addresses), null, null);
   }
@@ -137,4 +149,8 @@ private JsonRpcRequest readJsonAsJsonRpcRequest(final String jsonWithSingleAddre
       throws java.io.IOException {
     return new ObjectMapper().readValue(jsonWithSingleAddress, JsonRpcRequest.class);
   }
+
+  private FilterParameter readJsonAsFilterParameter(final String json) throws java.io.IOException {
+    return new ObjectMapper().readValue(json, FilterParameter.class);
+  }
 }