You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by js...@apache.org on 2017/08/22 16:42:15 UTC

geode git commit: GEODE-3306: Remove whitespace StringBuffers/nodes created by Apache Xerces

Repository: geode
Updated Branches:
  refs/heads/develop d11e1b9ee -> f2891791b


GEODE-3306: Remove whitespace StringBuffers/nodes created by Apache Xerces

This commit makes Geode compatible with the official Apache Xerces
implementation, which calls `characters()` when it reads ignorable
whitespace in `cache.xml`.

The while loop is required to handle comments in `cache.xml`, i.e.
a comment with whitespace before and after will generate two
empty StringBuffers (one for each set of whitespace before and after)
on the parse stack. The while loop removes all "consecutive" whitespace
StringBuffers from the top of the stack.

(This closes #668.)


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/f2891791
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/f2891791
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/f2891791

Branch: refs/heads/develop
Commit: f2891791bf8b84856ef398b63829913439c5c123
Parents: d11e1b9
Author: Darren Foong <da...@gmail.com>
Authored: Sun Jul 30 01:52:37 2017 +0800
Committer: Jared Stewart <js...@pivotal.io>
Committed: Tue Aug 22 09:41:16 2017 -0700

----------------------------------------------------------------------
 geode-core/build.gradle                         |  2 +
 .../internal/cache/xmlcache/CacheXmlParser.java | 23 ++++++++++
 .../cache/xmlcache/CacheXmlParserJUnitTest.java | 45 ++++++++++++++++----
 gradle/dependency-versions.properties           |  1 +
 4 files changed, 63 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/f2891791/geode-core/build.gradle
----------------------------------------------------------------------
diff --git a/geode-core/build.gradle b/geode-core/build.gradle
index 795c124..8145a63 100755
--- a/geode-core/build.gradle
+++ b/geode-core/build.gradle
@@ -144,6 +144,8 @@ dependencies {
   testCompile 'com.pholser:junit-quickcheck-core:' + project.'junit-quickcheck.version'
   testCompile 'com.pholser:junit-quickcheck-generators:' + project.'junit-quickcheck.version'
   testCompile 'com.pholser:junit-quickcheck-guava:' + project.'junit-quickcheck.version'
+
+  testRuntime 'xerces:xercesImpl:' + project.'xercesImpl.version'
 }
 
 def generatedResources = "$buildDir/generated-resources/main"

http://git-wip-us.apache.org/repos/asf/geode/blob/f2891791/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java
index 53e39ee..ca0b170 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParser.java
@@ -37,6 +37,7 @@ import javax.xml.parsers.SAXParserFactory;
 
 import org.apache.geode.cache.util.GatewayConflictResolver;
 import org.apache.logging.log4j.Logger;
+import org.apache.commons.lang.StringUtils;
 import org.xml.sax.Attributes;
 import org.xml.sax.ContentHandler;
 import org.xml.sax.InputSource;
@@ -2596,6 +2597,17 @@ public class CacheXmlParser extends CacheXml implements ContentHandler {
 
   public void startElement(String namespaceURI, String localName, String qName, Attributes atts)
       throws SAXException {
+    // This while loop pops all StringBuffers at the top of the stack
+    // that contain only whitespace; see GEODE-3306
+    while (!stack.empty()) {
+      Object o = stack.peek();
+      if (o instanceof StringBuffer && StringUtils.isBlank(((StringBuffer) o).toString())) {
+        stack.pop();
+      } else {
+        break;
+      }
+    }
+
     if (qName.equals(CACHE)) {
       startCache(atts);
     } else if (qName.equals(CLIENT_CACHE)) {
@@ -2872,6 +2884,17 @@ public class CacheXmlParser extends CacheXml implements ContentHandler {
   }
 
   public void endElement(String namespaceURI, String localName, String qName) throws SAXException {
+    // This while loop pops all StringBuffers at the top of the stack
+    // that contain only whitespace; see GEODE-3306
+    while (!stack.empty()) {
+      Object o = stack.peek();
+      if (o instanceof StringBuffer && StringUtils.isBlank(((StringBuffer) o).toString())) {
+        stack.pop();
+      } else {
+        break;
+      }
+    }
+
     try {
       // logger.debug("endElement namespaceURI=" + namespaceURI
       // + "; localName = " + localName + "; qName = " + qName);

http://git-wip-us.apache.org/repos/asf/geode/blob/f2891791/geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
index c168668..8f6ab5a 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/xmlcache/CacheXmlParserJUnitTest.java
@@ -23,11 +23,15 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
 import org.apache.geode.distributed.internal.DM;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.After;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 import org.junit.experimental.categories.Category;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
@@ -47,6 +51,9 @@ import java.util.Properties;
 @Category(UnitTest.class)
 public class CacheXmlParserJUnitTest {
 
+  @Rule
+  public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
+
   private static final String NAMESPACE_URI =
       "urn:java:org.apache.geode.internal.cache.xmlcache.MockXmlParser";
 
@@ -103,18 +110,26 @@ public class CacheXmlParserJUnitTest {
     assertNotNull("Did not find simple config.xml file", getClass()
         .getResourceAsStream("CacheXmlParserJUnitTest.testSimpleClientCacheXml.cache.xml"));
 
-    DM dm = mock(DM.class);
-
     Properties nonDefault = new Properties();
     nonDefault.setProperty(MCAST_PORT, "0"); // loner
 
-    InternalDistributedSystem system =
-        InternalDistributedSystem.newInstanceForTesting(dm, nonDefault);
-    when(dm.getSystem()).thenReturn(system);
-    InternalDistributedSystem.connect(nonDefault);
+    ClientCache cache = new ClientCacheFactory(nonDefault).set("cache-xml-file",
+        "xmlcache/CacheXmlParserJUnitTest.testSimpleClientCacheXml.cache.xml").create();
+    cache.close();
+  }
 
-    CacheXmlParser.parse(getClass()
-        .getResourceAsStream("CacheXmlParserJUnitTest.testSimpleClientCacheXml.cache.xml"));
+  /**
+   * Test that {@link CacheXmlParser} can parse the test cache.xml file, using the Apache Xerces XML
+   * parser.
+   * 
+   * @since Geode 1.3
+   */
+  @Test
+  public void testCacheXmlParserWithSimplePoolXerces() {
+    System.setProperty("javax.xml.parsers.SAXParserFactory",
+        "org.apache.xerces.jaxp.SAXParserFactoryImpl");
+
+    testCacheXmlParserWithSimplePool();
   }
 
   /**
@@ -138,6 +153,20 @@ public class CacheXmlParserJUnitTest {
   }
 
   /**
+   * Test that {@link CacheXmlParser} falls back to DTD parsing when locale language is not English,
+   * using the Apache Xerces XML parser.
+   * 
+   * @since Geode 1.3
+   */
+  @Test
+  public void testDTDFallbackWithNonEnglishLocalXerces() {
+    System.setProperty("javax.xml.parsers.SAXParserFactory",
+        "org.apache.xerces.jaxp.SAXParserFactoryImpl");
+
+    testDTDFallbackWithNonEnglishLocal();
+  }
+
+  /**
    * Get access to {@link CacheXmlParser} protected methods and fields.
    * 
    * @since GemFire 8.1

http://git-wip-us.apache.org/repos/asf/geode/blob/f2891791/gradle/dependency-versions.properties
----------------------------------------------------------------------
diff --git a/gradle/dependency-versions.properties b/gradle/dependency-versions.properties
index 121ec28..1de4e73 100644
--- a/gradle/dependency-versions.properties
+++ b/gradle/dependency-versions.properties
@@ -99,3 +99,4 @@ tomcat6.version = 6.0.37
 tomcat7.version = 7.0.73
 tomcat8.version = 8.5.9
 commons-validator.version = 1.6
+xercesImpl.version = 2.11.0