[tor-commits] [onionoo/release] Replace Gson with Jackson.

karsten at torproject.org karsten at torproject.org
Wed May 30 13:55:14 UTC 2018


commit 73f8dedfc994dd6476570798c576060bcffc6fa6
Author: Karsten Loesing <karsten.loesing at gmx.net>
Date:   Wed May 23 10:15:36 2018 +0200

    Replace Gson with Jackson.
    
    Update metrics-lib dependency to 2.4.0.
    
    Adapt touched logging statements to our standards.
    
    Implements #25848.
---
 CHANGELOG.md                                       |  6 +++
 build.xml                                          |  6 ++-
 .../org/torproject/onionoo/docs/DetailsStatus.java |  6 +--
 .../org/torproject/onionoo/docs/DocumentStore.java | 57 +++++++++++++-------
 .../torproject/onionoo/docs/SummaryDocument.java   | 63 ++++++++++------------
 .../torproject/onionoo/server/ResponseBuilder.java | 26 ++++++---
 .../onionoo/server/ResourceServletTest.java        | 20 +++++--
 .../onionoo/writer/GraphHistoryCompilerTest.java   | 17 ++++--
 8 files changed, 126 insertions(+), 75 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 199cf6f..7a792fc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# Changes in version 6.0-1.??.? - 2018-05-??
+
+ * Medium changes
+   - Replace Gson with Jackson.
+
+
 # Changes in version 6.0-1.13.0 - 2018-04-17
 
  * Medium changes
diff --git a/build.xml b/build.xml
index 557297d..7d752e2 100644
--- a/build.xml
+++ b/build.xml
@@ -11,7 +11,7 @@
   <property name="onionoo.protocol.version" value="6.0"/>
   <property name="release.version"
             value="${onionoo.protocol.version}-1.13.0-dev"/>
-  <property name="metricslibversion" value="2.2.0"/>
+  <property name="metricslibversion" value="2.4.0"/>
   <property name="jetty.version" value="-9.2.21.v20170120" />
   <property name="warfile"
             value="onionoo-${release.version}.war"/>
@@ -47,7 +47,9 @@
     <include name="commons-codec-1.10.jar"/>
     <include name="commons-compress-1.13.jar"/>
     <include name="commons-lang3-3.5.jar"/>
-    <include name="gson-2.4.jar"/>
+    <include name="jackson-annotations-2.8.6.jar"/>
+    <include name="jackson-core-2.8.6.jar"/>
+    <include name="jackson-databind-2.8.6.jar"/>
     <include name="logback-classic-1.1.9.jar"/>
     <include name="logback-core-1.1.9.jar"/>
     <include name="slf4j-api-1.7.22.jar"/>
diff --git a/src/main/java/org/torproject/onionoo/docs/DetailsStatus.java b/src/main/java/org/torproject/onionoo/docs/DetailsStatus.java
index f838ec0..7ee0bb6 100644
--- a/src/main/java/org/torproject/onionoo/docs/DetailsStatus.java
+++ b/src/main/java/org/torproject/onionoo/docs/DetailsStatus.java
@@ -34,12 +34,12 @@ public class DetailsStatus extends Document {
   private String desc_published;
 
   public void setDescPublished(Long descPublished) {
-    this.desc_published = DateTimeHelper.format(descPublished);
+    this.desc_published = null == descPublished ? null
+        : DateTimeHelper.format(descPublished);
   }
 
   public Long getDescPublished() {
-    return this.desc_published == null ? null :
-        DateTimeHelper.parse(this.desc_published);
+    return DateTimeHelper.parse(this.desc_published);
   }
 
   private String last_restarted;
diff --git a/src/main/java/org/torproject/onionoo/docs/DocumentStore.java b/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
index f1f3803..3fac5c9 100644
--- a/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
+++ b/src/main/java/org/torproject/onionoo/docs/DocumentStore.java
@@ -5,9 +5,12 @@ package org.torproject.onionoo.docs;
 
 import org.torproject.onionoo.util.FormattingUtils;
 
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
-import com.google.gson.JsonParseException;
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,6 +44,12 @@ public class DocumentStore {
   private static Logger log = LoggerFactory.getLogger(
       DocumentStore.class);
 
+  private static ObjectMapper objectMapper = new ObjectMapper()
+      .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
+      .setSerializationInclusion(JsonInclude.Include.NON_EMPTY)
+      .setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
+      .setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+
   private final File statusDir = new File("status");
 
   private File outDir = null;
@@ -162,12 +171,11 @@ public class DocumentStore {
         String line = null;
         try (BufferedReader br = new BufferedReader(new FileReader(
             summaryFile))) {
-          Gson gson = new Gson();
           while ((line = br.readLine()) != null) {
             if (line.length() == 0) {
               continue;
             }
-            SummaryDocument summaryDocument = gson.fromJson(line,
+            SummaryDocument summaryDocument = objectMapper.readValue(line,
                 SummaryDocument.class);
             if (summaryDocument != null) {
               parsedSummaryDocuments.put(summaryDocument.getFingerprint(),
@@ -178,11 +186,8 @@ public class DocumentStore {
           this.listedFiles += parsedSummaryDocuments.size();
           this.listOperations++;
         } catch (IOException e) {
-          log.error("Could not read file '"
-              + summaryFile.getAbsolutePath() + "'.", e);
-        } catch (JsonParseException e) {
-          log.error("Could not parse summary document '" + line
-              + "' in file '" + summaryFile.getAbsolutePath() + "'.", e);
+          log.error("Could not parse summary document '{}' from file '{}'.",
+              line, summaryFile.getAbsolutePath(), e);
         }
       }
     }
@@ -303,13 +308,15 @@ public class DocumentStore {
         || document instanceof WeightsDocument
         || document instanceof ClientsDocument
         || document instanceof UptimeDocument) {
-      Gson gson = new Gson();
-      documentString = gson.toJson(document);
+      try {
+        documentString = objectMapper.writeValueAsString(document);
+      } catch (JsonProcessingException e) {
+        log.error("Serializing failed for type {}.",
+            document.getClass().getName(), e);
+        return false;
+      }
     } else if (document instanceof DetailsStatus
         || document instanceof DetailsDocument) {
-      /* Don't escape HTML characters, like < and >, contained in
-       * strings. */
-      Gson gson = new GsonBuilder().disableHtmlEscaping().create();
       /* We must ensure that details files only contain ASCII characters
        * and no UTF-8 characters.  While UTF-8 characters are perfectly
        * valid in JSON, this would break compatibility with existing files
@@ -317,7 +324,14 @@ public class DocumentStore {
        * objects are escaped JSON, e.g., \u00F2.  When Gson serlializes
        * this string, it escapes the \ to \\, hence writes \\u00F2.  We
        * need to undo this and change \\u00F2 back to \u00F2. */
-      documentString = FormattingUtils.replaceValidUtf(gson.toJson(document));
+      try {
+        documentString = FormattingUtils.replaceValidUtf(
+            objectMapper.writeValueAsString(document));
+      } catch (JsonProcessingException e) {
+        log.error("Serializing failed for type {}.",
+            document.getClass().getName(), e);
+        return false;
+      }
       /* Existing details statuses don't contain opening and closing curly
        * brackets, so we should remove them from new details statuses,
        * too. */
@@ -536,9 +550,8 @@ public class DocumentStore {
   private <T extends Document> T retrieveParsedDocumentFile(
       Class<T> documentType, String documentString) {
     T result = null;
-    Gson gson = new Gson();
     try {
-      result = gson.fromJson(documentString, documentType);
+      result = objectMapper.readValue(documentString, documentType);
     } catch (Throwable e) {
       /* Handle below. */
       log.error(documentString);
@@ -777,10 +790,14 @@ public class DocumentStore {
       return;
     }
     StringBuilder sb = new StringBuilder();
-    Gson gson = new Gson();
     for (SummaryDocument summaryDocument :
         this.cachedSummaryDocuments.values()) {
-      String line = gson.toJson(summaryDocument);
+      String line = null;
+      try {
+        line = objectMapper.writeValueAsString(summaryDocument);
+      } catch (JsonProcessingException e) {
+        line = null;
+      }
       if (line != null) {
         sb.append(line + "\n");
       } else {
diff --git a/src/main/java/org/torproject/onionoo/docs/SummaryDocument.java b/src/main/java/org/torproject/onionoo/docs/SummaryDocument.java
index 42a5a64..31cd84c 100644
--- a/src/main/java/org/torproject/onionoo/docs/SummaryDocument.java
+++ b/src/main/java/org/torproject/onionoo/docs/SummaryDocument.java
@@ -3,8 +3,8 @@
 
 package org.torproject.onionoo.docs;
 
-import com.google.gson.annotations.Expose;
-import com.google.gson.annotations.SerializedName;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
 import org.apache.commons.codec.DecoderException;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.codec.binary.Hex;
@@ -20,8 +20,7 @@ import java.util.regex.Pattern;
 
 public class SummaryDocument extends Document {
 
-  @Expose
-  @SerializedName("t")
+  @JsonProperty("t")
   private boolean isRelay;
 
   public void setRelay(boolean isRelay) {
@@ -32,8 +31,7 @@ public class SummaryDocument extends Document {
     return this.isRelay;
   }
 
-  @Expose
-  @SerializedName("f")
+  @JsonProperty("f")
   private String fingerprint;
 
   /** Sets the fingerprint to the given 40 hex characters and clears
@@ -57,6 +55,7 @@ public class SummaryDocument extends Document {
     return this.fingerprint;
   }
 
+  @JsonIgnore
   private transient String hashedFingerprint = null;
 
   /** Returns the SHA1-hashed fingerprint, or <code>null</code> if no
@@ -73,6 +72,7 @@ public class SummaryDocument extends Document {
     return this.hashedFingerprint;
   }
 
+  @JsonIgnore
   private transient String base64Fingerprint = null;
 
   /** Returns the base64-encoded fingerprint, or <code>null</code> if no
@@ -89,6 +89,7 @@ public class SummaryDocument extends Document {
     return this.base64Fingerprint;
   }
 
+  @JsonIgnore
   private transient String[] fingerprintSortedHexBlocks = null;
 
   /** Returns a sorted array containing blocks of 4 upper-case hex
@@ -109,8 +110,7 @@ public class SummaryDocument extends Document {
     return this.fingerprintSortedHexBlocks;
   }
 
-  @Expose
-  @SerializedName("n")
+  @JsonProperty("n")
   private String nickname;
 
   @SuppressWarnings("checkstyle:javadocmethod")
@@ -126,8 +126,7 @@ public class SummaryDocument extends Document {
     return this.nickname == null ? "Unnamed" : this.nickname;
   }
 
-  @Expose
-  @SerializedName("ad")
+  @JsonProperty("ad")
   private String[] addresses;
 
   public void setAddresses(List<String> addresses) {
@@ -169,8 +168,7 @@ public class SummaryDocument extends Document {
     return sortedSet;
   }
 
-  @Expose
-  @SerializedName("cc")
+  @JsonProperty("cc")
   private String countryCode;
 
   public void setCountryCode(String countryCode) {
@@ -181,8 +179,7 @@ public class SummaryDocument extends Document {
     return this.countryCode;
   }
 
-  @Expose
-  @SerializedName("as")
+  @JsonProperty("as")
   private String asNumber;
 
   public void setAsNumber(String asNumber) {
@@ -193,8 +190,7 @@ public class SummaryDocument extends Document {
     return this.asNumber;
   }
 
-  @Expose
-  @SerializedName("fs")
+  @JsonProperty("fs")
   private String firstSeenMillis;
 
   public void setFirstSeenMillis(long firstSeenMillis) {
@@ -205,8 +201,7 @@ public class SummaryDocument extends Document {
     return DateTimeHelper.parse(this.firstSeenMillis);
   }
 
-  @Expose
-  @SerializedName("ls")
+  @JsonProperty("ls")
   private String lastSeenMillis;
 
   public void setLastSeenMillis(long lastSeenMillis) {
@@ -217,8 +212,7 @@ public class SummaryDocument extends Document {
     return DateTimeHelper.parse(this.lastSeenMillis);
   }
 
-  @Expose
-  @SerializedName("rf")
+  @JsonProperty("rf")
   private String[] relayFlags;
 
   public void setRelayFlags(SortedSet<String> relayFlags) {
@@ -229,8 +223,7 @@ public class SummaryDocument extends Document {
     return this.stringArrayToSortedSet(this.relayFlags);
   }
 
-  @Expose
-  @SerializedName("cw")
+  @JsonProperty("cw")
   private long consensusWeight;
 
   public void setConsensusWeight(long consensusWeight) {
@@ -241,8 +234,7 @@ public class SummaryDocument extends Document {
     return this.consensusWeight;
   }
 
-  @Expose
-  @SerializedName("r")
+  @JsonProperty("r")
   private boolean running;
 
   public void setRunning(boolean isRunning) {
@@ -253,8 +245,7 @@ public class SummaryDocument extends Document {
     return this.running;
   }
 
-  @Expose
-  @SerializedName("c")
+  @JsonProperty("c")
   private String contact;
 
   @SuppressWarnings("checkstyle:javadocmethod")
@@ -273,8 +264,7 @@ public class SummaryDocument extends Document {
   /* This attribute can go away once all Onionoo services had their hourly
    * updater write effective families to summary documents at least once.
    * Remove this code after September 8, 2015. */
-  @Expose
-  @SerializedName("ff")
+  @JsonProperty("ff")
   private String[] familyFingerprints;
 
   public void setFamilyFingerprints(
@@ -286,8 +276,7 @@ public class SummaryDocument extends Document {
     return this.stringArrayToSortedSet(this.familyFingerprints);
   }
 
-  @Expose
-  @SerializedName("ef")
+  @JsonProperty("ef")
   private String[] effectiveFamily;
 
   public void setEffectiveFamily(SortedSet<String> effectiveFamily) {
@@ -298,8 +287,7 @@ public class SummaryDocument extends Document {
     return this.stringArrayToSortedSet(this.effectiveFamily);
   }
 
-  @Expose
-  @SerializedName("v")
+  @JsonProperty("v")
   private String version;
 
   public void setVersion(String version) {
@@ -310,8 +298,7 @@ public class SummaryDocument extends Document {
     return this.version;
   }
 
-  @Expose
-  @SerializedName("h")
+  @JsonProperty("h")
   private String hostName;
 
   public void setHostName(String hostName) {
@@ -322,8 +309,7 @@ public class SummaryDocument extends Document {
     return this.hostName;
   }
 
-  @Expose
-  @SerializedName("rv")
+  @JsonProperty("rv")
   private Boolean recommendedVersion;
 
   public void setRecommendedVersion(Boolean recommendedVersion) {
@@ -334,6 +320,11 @@ public class SummaryDocument extends Document {
     return this.recommendedVersion;
   }
 
+  /** Instantiate an empty summary document. */
+  public SummaryDocument() {
+    /* empty */
+  }
+
   /* The familyFingerprints parameter can go away after September 8, 2015.
    * See above. */
   /** Instantiates a summary document with all given properties. */
diff --git a/src/main/java/org/torproject/onionoo/server/ResponseBuilder.java b/src/main/java/org/torproject/onionoo/server/ResponseBuilder.java
index 64e23ed..1dcfeeb 100644
--- a/src/main/java/org/torproject/onionoo/server/ResponseBuilder.java
+++ b/src/main/java/org/torproject/onionoo/server/ResponseBuilder.java
@@ -14,8 +14,12 @@ import org.torproject.onionoo.docs.UptimeDocument;
 import org.torproject.onionoo.docs.WeightsDocument;
 import org.torproject.onionoo.util.FormattingUtils;
 
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,6 +35,12 @@ public class ResponseBuilder {
   private static final Logger logger =
       LoggerFactory.getLogger(ResponseBuilder.class);
 
+  private static ObjectMapper objectMapper = new ObjectMapper()
+      .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
+      .setSerializationInclusion(JsonInclude.Include.NON_EMPTY)
+      .setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
+      .setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+
   private DocumentStore documentStore;
   private String buildRevision;
 
@@ -343,13 +353,15 @@ public class ResponseBuilder {
             dd.setVersionStatus(detailsDocument.getVersionStatus());
           }
         }
-        /* Don't escape HTML characters, like < and >, contained in
-         * strings. */
-        Gson gson = new GsonBuilder().disableHtmlEscaping().create();
-        /* Whenever we provide Gson with a string containing an escaped
+        /* Whenever we provide Jackson with a string containing an escaped
          * non-ASCII character like \u00F2, it escapes the \ to \\, which
          * we need to undo before including the string in a response. */
-        return FormattingUtils.replaceValidUtf(gson.toJson(dd));
+        try {
+          return FormattingUtils.replaceValidUtf(
+              objectMapper.writeValueAsString(dd));
+        } catch (JsonProcessingException e) {
+          return "";
+        }
       } else {
         // TODO We should probably log that we didn't find a details
         // document that we expected to exist.
diff --git a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
index c371d71..d64d59d 100644
--- a/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
+++ b/src/test/java/org/torproject/onionoo/server/ResourceServletTest.java
@@ -13,7 +13,11 @@ import org.torproject.onionoo.docs.DocumentStoreFactory;
 import org.torproject.onionoo.docs.DummyDocumentStore;
 import org.torproject.onionoo.docs.UpdateStatus;
 
-import com.google.gson.Gson;
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -36,6 +40,12 @@ import java.util.TreeSet;
  * which tests servlet specifics. */
 public class ResourceServletTest {
 
+  private static ObjectMapper objectMapper = new ObjectMapper()
+      .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
+      .setSerializationInclusion(JsonInclude.Include.NON_EMPTY)
+      .setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
+      .setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+
   private SortedMap<String, org.torproject.onionoo.docs.SummaryDocument> relays;
   private SortedMap<String, org.torproject.onionoo.docs.SummaryDocument>
       bridges;
@@ -263,11 +273,10 @@ public class ResourceServletTest {
     rs.doGet(this.request, this.response, TEST_TIME);
   }
 
-  private void parseResponse() {
+  private void parseResponse() throws IOException {
     this.responseString = this.response.getWrittenContent();
     if (this.responseString != null) {
-      Gson gson = new Gson();
-      this.summaryDocument = gson.fromJson(this.responseString,
+      this.summaryDocument = objectMapper.readValue(this.responseString,
           SummaryDocument.class);
     }
   }
@@ -344,6 +353,9 @@ public class ResourceServletTest {
 
   @SuppressWarnings("MemberName")
   private static class SummaryDocument {
+    private String version;
+    private String next_major_version_scheduled;
+    private String build_revision;
     private String relays_published;
     private int relays_skipped;
     private int relays_truncated;
diff --git a/src/test/java/org/torproject/onionoo/writer/GraphHistoryCompilerTest.java b/src/test/java/org/torproject/onionoo/writer/GraphHistoryCompilerTest.java
index 6d9c461..b055426 100644
--- a/src/test/java/org/torproject/onionoo/writer/GraphHistoryCompilerTest.java
+++ b/src/test/java/org/torproject/onionoo/writer/GraphHistoryCompilerTest.java
@@ -9,7 +9,11 @@ import static org.junit.Assert.assertNotNull;
 import org.torproject.onionoo.docs.DateTimeHelper;
 import org.torproject.onionoo.docs.GraphHistory;
 
-import com.google.gson.Gson;
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.PropertyNamingStrategy;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -17,6 +21,7 @@ import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameter;
 import org.junit.runners.Parameterized.Parameters;
 
+import java.io.IOException;
 import java.time.Period;
 import java.util.Arrays;
 import java.util.Collection;
@@ -25,6 +30,12 @@ import java.util.Map;
 @RunWith(Parameterized.class)
 public class GraphHistoryCompilerTest {
 
+  private static ObjectMapper objectMapper = new ObjectMapper()
+      .setPropertyNamingStrategy(PropertyNamingStrategy.SNAKE_CASE)
+      .setSerializationInclusion(JsonInclude.Include.NON_EMPTY)
+      .setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
+      .setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+
   /** Provide test data. */
   @Parameters
   public static Collection<Object[]> data() {
@@ -156,7 +167,7 @@ public class GraphHistoryCompilerTest {
       DateTimeHelper.TEN_DAYS };
 
   @Test
-  public void test() {
+  public void test() throws IOException {
     GraphHistoryCompiler ghc = new GraphHistoryCompiler(DateTimeHelper.parse(
         "2018-01-01 00:00:00"));
     ghc.setDivisible(this.divisible);
@@ -172,7 +183,7 @@ public class GraphHistoryCompilerTest {
     Map<String, GraphHistory> compiledGraphHistories =
         ghc.compileGraphHistories();
     String message = this.testDescription + "; "
-        + new Gson().toJson(compiledGraphHistories);
+        + objectMapper.writeValueAsString(compiledGraphHistories);
     assertEquals(message, this.expectedGraphs, compiledGraphHistories.size());
     if (null != this.expectedGraphName) {
       GraphHistory gh = compiledGraphHistories.get(this.expectedGraphName);





More information about the tor-commits mailing list