GEODE-5013 Replace org.json with Jackson in GfJsonObject

This removes org.json as a dependency in geode-core and associated test modules. GfJsonObject and GfJsonArray are ported to use the same basic Jackson ObjectMapper setup as QueryResultFormatter so that we now have a uniform mechanism for managing JSON documents for querying/gfsh and folks can use Jackson annotations like @JsonIgnore on their classes. The geode-web-api and geode-pulse modules still use the geode-json module in tests. When/if they are ported to use GfJsonObject/Array we can delete the geode-json module.
b-xiang · Mar 14, 2019 · aa38e22 · aa38e22
1 parent 326873f
commit aa38e22
Show file tree

Hide file tree

Showing 35 changed files with 901 additions and 972 deletions.
diff --git a/geode-core/build.gradle b/geode-core/build.gradle
@@ -269,7 +269,6 @@ dependencies {
   compile('com.healthmarketscience.rmiio:rmiio')
 
   compile(project(':geode-common'))
-  compile(project(':geode-json'))
   compile(project(':geode-management'))
 
   jcaAnnotationProcessor 'org.apache.logging.log4j:log4j-core'

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/QueryDataDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/QueryDataDUnitTest.java
@@ -37,9 +37,6 @@
 
 import javax.management.ObjectName;
 
-import org.json.JSONArray;
-import org.json.JSONException;
-import org.json.JSONObject;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -64,6 +61,8 @@
 import org.apache.geode.management.internal.SystemManagementService;
 import org.apache.geode.management.internal.beans.BeanUtilFuncs;
 import org.apache.geode.management.internal.beans.QueryDataFunction;
+import org.apache.geode.management.internal.cli.json.GfJsonArray;
+import org.apache.geode.management.internal.cli.json.GfJsonObject;
 import org.apache.geode.pdx.PdxInstance;
 import org.apache.geode.pdx.PdxInstanceFactory;
 import org.apache.geode.pdx.internal.PdxInstanceFactoryImpl;
@@ -72,7 +71,7 @@
 import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
 
 /**
- * Distributed tests for {@link DistributedSystemMXBean#queryData(String, String, int)}.
+ * Distributed tests for DistributedSystemMXBean#queryData(String, String, int).
  * </p>
  *
  * <pre>
@@ -246,16 +245,17 @@ public void testLimitForQuery() throws Exception {
       assertThat(jsonString).contains("result").doesNotContain("No Data Found");
       assertThat(jsonString).contains(BIG_COLLECTION_ELEMENT_);
 
-      JSONObject jsonObject = new JSONObject(jsonString);
-      JSONArray jsonArray = jsonObject.getJSONArray("result");
+      GfJsonObject jsonObject = new GfJsonObject(jsonString);
+      GfJsonArray jsonArray = jsonObject.getJSONArray("result");
       assertThat(jsonArray.length()).isEqualTo(DEFAULT_QUERY_LIMIT);
 
       // Get the first element
-      JSONArray jsonArray1 = jsonArray.getJSONArray(0);
+      GfJsonArray jsonArray1 = jsonArray.getJSONArray(0);
+      assertThat(jsonArray1).isNotNull();
 
       // Get the ObjectValue
-      JSONObject collectionObject = (JSONObject) jsonArray1.get(1);
-      assertThat(collectionObject.length()).isEqualTo(100);
+      GfJsonObject collectionObject = jsonArray1.getInternalJsonObject(1);
+      assertThat(collectionObject.size()).isEqualTo(100);
 
       // Query With Override Values
       int newQueryCollectionDepth = 150;
@@ -274,7 +274,7 @@ public void testLimitForQuery() throws Exception {
       verifyJsonIsValid(jsonString);
       assertThat(jsonString).contains("result").doesNotContain("No Data Found");
 
-      jsonObject = new JSONObject(jsonString);
+      jsonObject = new GfJsonObject(jsonString);
       assertThat(jsonString).contains(BIG_COLLECTION_ELEMENT_);
 
       jsonArray = jsonObject.getJSONArray("result");
@@ -284,8 +284,8 @@ public void testLimitForQuery() throws Exception {
       jsonArray1 = jsonArray.getJSONArray(0);
 
       // Get the ObjectValue
-      collectionObject = (JSONObject) jsonArray1.get(1);
-      assertThat(collectionObject.length()).isEqualTo(newQueryCollectionDepth);
+      collectionObject = jsonArray1.getInternalJsonObject(1);
+      assertThat(collectionObject.size()).isEqualTo(newQueryCollectionDepth);
     });
   }
 
@@ -482,10 +482,9 @@ private Date getDate(final int year, final int month, final int date) {
     return calendar.getTime();
   }
 
-  private void verifyJsonIsValid(final String jsonString) throws JSONException {
+  private void verifyJsonIsValid(final String jsonString) {
     assertThat(jsonString, isJson());
     assertThat(jsonString, hasJsonPath("$.result"));
-    assertThat(new JSONObject(jsonString)).isNotNull();
   }
 
   private void putDataInRegion(final String regionName, final Object[] portfolio, final int from,

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/pdx/JSONPdxClientServerDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/pdx/JSONPdxClientServerDUnitTest.java
@@ -44,6 +44,7 @@
 import org.apache.geode.internal.Assert;
 import org.apache.geode.internal.AvailablePortHelper;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
+import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.pdx.internal.json.PdxToJSON;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.NetworkUtils;
@@ -184,7 +185,7 @@ public Object call() throws Exception {
     });
   }
 
-  public void VerifyPdxInstanceAndJsonConversion() throws JsonProcessingException {
+  public void VerifyPdxInstanceAndJsonConversion() throws JsonProcessingException, GfJsonException {
     Region region = getRootRegion("testSimplePdx");
 
     // Create Object and initialize its members.
@@ -214,7 +215,7 @@ public void VerifyPdxInstanceAndJsonConversion() throws JsonProcessingException
   }
 
   private void validateReceivedJSON(Region region, TestObjectForJSONFormatter actualTestObject,
-      ObjectMapper objectMapper) throws JsonProcessingException {
+      ObjectMapper objectMapper) throws JsonProcessingException, GfJsonException {
     // 1. get the json from the object using Jackson Object Mapper
     String json = objectMapper.writeValueAsString(actualTestObject);
     String jsonWithClassType = actualTestObject.addClassTypeToJson(json);

diff --git a/.../src/integrationTest/java/org/apache/geode/management/DataQueryEngineIntegrationTest.java b/.../src/integrationTest/java/org/apache/geode/management/DataQueryEngineIntegrationTest.java
@@ -19,7 +19,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 
-import org.json.JSONObject;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -33,6 +32,7 @@
 import org.apache.geode.management.internal.SystemManagementService;
 import org.apache.geode.management.internal.beans.DataQueryEngine;
 import org.apache.geode.management.internal.beans.QueryDataFunction;
+import org.apache.geode.management.internal.cli.json.GfJsonObject;
 import org.apache.geode.management.model.EmptyObject;
 import org.apache.geode.management.model.Item;
 import org.apache.geode.management.model.Order;
@@ -105,7 +105,7 @@ public void testCyclicWithNestedObjectReference() throws Exception {
 
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   /**
@@ -135,7 +135,7 @@ public void testCyclicWithNestedClasses() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   /**
@@ -165,7 +165,7 @@ public void testCyclicWithNestedRefernce2ndLayer() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -189,7 +189,7 @@ public void testCyclicWithCollection1stLayer() throws Exception {
     System.out.println("Query Result: " + queryResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -215,7 +215,7 @@ public void testCyclicCollectionWithMultipleObjects() throws Exception {
     System.out.println("Query Result: " + queryResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -241,7 +241,7 @@ public void testCyclicArrayMultipleObjects() throws Exception {
     System.out.println("Query Result: " + queryResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
 
   }
 
@@ -254,7 +254,7 @@ public void testCyclicArrayMultipleObjectsMemberWise() throws Exception {
     System.out.println(queryResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -267,7 +267,7 @@ public void testEmptyObject() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -281,7 +281,7 @@ public void testSubClassOverridingMethods() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -304,7 +304,7 @@ public void testNestedPDXObject() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -322,7 +322,7 @@ public void testArrayWithNullValues() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
   @Test
@@ -340,7 +340,7 @@ public void testWithSqlDate() throws Exception {
     assertThat(queryResult).isEqualToIgnoringWhitespace(expectedResult);
 
     // If not correct JSON format this will throw a JSONException
-    new JSONObject(queryResult);
+    new GfJsonObject(queryResult);
   }
 
 

diff --git a/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java
@@ -24,7 +24,6 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import org.json.JSONException;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -34,6 +33,7 @@
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
+import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.pdx.internal.PdxInstanceImpl;
 import org.apache.geode.pdx.internal.PeerTypeRegistration;
 import org.apache.geode.test.junit.categories.SerializationTest;
@@ -58,7 +58,7 @@ public void tearDown() {
   }
 
   @Test
-  public void ValidatePdxInstanceToJsonConversion() {
+  public void ValidatePdxInstanceToJsonConversion() throws GfJsonException {
     TestObjectForJSONFormatter actualTestObject = new TestObjectForJSONFormatter();
     actualTestObject.defaultInitialization();
 
@@ -78,7 +78,7 @@ public void ValidatePdxInstanceToJsonConversion() {
   }
 
   @Test
-  public void verifyJsonToPdxInstanceConversion() throws JSONException, JsonProcessingException {
+  public void verifyJsonToPdxInstanceConversion() throws JsonProcessingException, GfJsonException {
     TestObjectForJSONFormatter expectedTestObject = new TestObjectForJSONFormatter();
     expectedTestObject.defaultInitialization();
 
@@ -113,7 +113,7 @@ public void verifyJsonToPdxInstanceConversion() throws JSONException, JsonProces
   }
 
   @Test
-  public void verifyJsonToPdxInstanceConversionWithJSONFormatter() {
+  public void verifyJsonToPdxInstanceConversionWithJSONFormatter() throws GfJsonException {
     TestObjectForJSONFormatter expectedTestObject = new TestObjectForJSONFormatter();
     expectedTestObject.defaultInitialization();
 

diff --git a/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt b/geode-core/src/integrationTest/resources/org/apache/geode/codeAnalysis/excludedClasses.txt
@@ -68,7 +68,7 @@ org/apache/geode/internal/tcp/VersionedByteBufferInputStream
 org/apache/geode/internal/util/concurrent/StoppableReadWriteLock
 org/apache/geode/management/internal/cli/commands/ShowMetricsCommand$Category
 org/apache/geode/management/internal/cli/exceptions/UserErrorException
-org/apache/geode/management/internal/cli/json/QueryResultFormatter$PreventReserializationModule
+org/apache/geode/management/internal/cli/json/AbstractJSONFormatter$PreventReserializationModule
 org/apache/geode/security/ResourcePermission
 org/apache/geode/security/ResourcePermission$Operation
 org/apache/geode/security/ResourcePermission$Resource

diff --git a/...c/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt b/...c/integrationTest/resources/org/apache/geode/codeAnalysis/sanctionedDataSerializables.txt
@@ -1981,7 +1981,7 @@ fromData,212
 toData,239
 
 org/apache/geode/internal/cache/versions/VersionTag,2
-fromData,187
+fromData,189
 toData,259
 
 org/apache/geode/internal/cache/wan/GatewaySenderAdvisor$GatewaySenderProfile,4

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java b/geode-core/src/main/java/org/apache/geode/distributed/LocatorLauncher.java
@@ -78,7 +78,6 @@
 import org.apache.geode.internal.process.ProcessUtils;
 import org.apache.geode.internal.process.UnableToControlProcessException;
 import org.apache.geode.lang.AttachAPINotFoundException;
-import org.apache.geode.management.internal.cli.json.GfJsonArray;
 import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.management.internal.cli.json.GfJsonObject;
 import org.apache.geode.management.internal.cli.util.HostUtils;
@@ -2001,7 +2000,7 @@ public static LocatorState fromJson(final String json) {
         final Status status = Status.valueOfDescription(gfJsonObject.getString(JSON_STATUS));
 
         final List<String> jvmArguments =
-            Arrays.asList(GfJsonArray.toStringArray(gfJsonObject.getJSONArray(JSON_JVMARGUMENTS)));
+            gfJsonObject.getJSONArray(JSON_JVMARGUMENTS).toStringList();
 
         return new LocatorState(status, gfJsonObject.getString(JSON_STATUSMESSAGE),
             gfJsonObject.getLong(JSON_TIMESTAMP), gfJsonObject.getString(JSON_LOCATION),

diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java b/geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java
@@ -85,7 +85,6 @@
 import org.apache.geode.internal.process.UnableToControlProcessException;
 import org.apache.geode.lang.AttachAPINotFoundException;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
-import org.apache.geode.management.internal.cli.json.GfJsonArray;
 import org.apache.geode.management.internal.cli.json.GfJsonException;
 import org.apache.geode.management.internal.cli.json.GfJsonObject;
 import org.apache.geode.management.internal.cli.util.HostUtils;
@@ -2596,7 +2595,7 @@ public static ServerState fromJson(final String json) {
 
         final Status status = Status.valueOfDescription(gfJsonObject.getString(JSON_STATUS));
         final List<String> jvmArguments =
-            Arrays.asList(GfJsonArray.toStringArray(gfJsonObject.getJSONArray(JSON_JVMARGUMENTS)));
+            gfJsonObject.getJSONArray(JSON_JVMARGUMENTS).toStringList();
 
         return new ServerState(status, gfJsonObject.getString(JSON_STATUSMESSAGE),
             gfJsonObject.getLong(JSON_TIMESTAMP), gfJsonObject.getString(JSON_LOCATION),

diff --git a/...in/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java b/...in/java/org/apache/geode/distributed/internal/membership/gms/membership/GMSJoinLeave.java
@@ -1078,8 +1078,6 @@ private void processViewMessage(final InstallViewMessage m) {
   private void forceDisconnect(String reason) {
     this.isStopping = true;
     if (!isJoined) {
-      logger.fatal("BRUCE: forcedDisconnect invoked.  isReconnecting={} isJoined={}",
-          services.getConfig().isReconnecting(), isJoined);
       joinResponse[0] =
           new JoinResponseMessage(
               "Stopping due to ForcedDisconnectException caused by '" + reason + "'", -1);

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionTag.java b/geode-core/src/main/java/org/apache/geode/internal/cache/versions/VersionTag.java
@@ -408,9 +408,10 @@ public void fromData(DataInput in) throws IOException, ClassNotFoundException {
         this.previousMemberID = readMember(in);
       }
     }
-    setIsRemoteForTesting();
+    setBits(BITS_IS_REMOTE_TAG);
   }
 
+  /** for unit testing receipt of version tags from another member of the cluster */
   public void setIsRemoteForTesting() {
     setBits(BITS_IS_REMOTE_TAG);
   }