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

commit 73f8dedfc994dd6476570798c576060bcffc6fa6 Author: Karsten Loesing karsten.loesing@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);