You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2016/12/12 22:36:20 UTC

[17/46] geode git commit: GEODE-2127 old client support service is not set up to handle arrays

GEODE-2127 old client support service is not set up to handle arrays

Array classes start with some number of left-brackets ("[") followed by
"L" and the name of the class the array contains.  This adds a unit test
for arrays and adds support for handling them in OldClientSupportProvider.


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

Branch: refs/heads/feature/GEODE-1930
Commit: cbff37e7d521f2d22be34182bad4d63b7fc27a91
Parents: 9ec9233
Author: Bruce Schuchardt <bs...@pivotal.io>
Authored: Fri Nov 18 10:47:49 2016 -0800
Committer: Bruce Schuchardt <bs...@pivotal.io>
Committed: Fri Nov 18 10:48:41 2016 -0800

----------------------------------------------------------------------
 .../java/org/apache/geode/DataSerializer.java   | 54 +------------
 .../geode/internal/InternalDataSerializer.java  | 48 +++++++++++-
 .../internal/DataSerializableJUnitTest.java     |  0
 .../gemfire/OldClientSupportProvider.java       | 82 ++++++++++++++------
 .../apache/geode/OldClientSupportDUnitTest.java | 47 ++++++++++-
 5 files changed, 152 insertions(+), 79 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/cbff37e7/geode-core/src/main/java/org/apache/geode/DataSerializer.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/DataSerializer.java b/geode-core/src/main/java/org/apache/geode/DataSerializer.java
index 29b20a2..375ff3b 100644
--- a/geode-core/src/main/java/org/apache/geode/DataSerializer.java
+++ b/geode-core/src/main/java/org/apache/geode/DataSerializer.java
@@ -59,7 +59,6 @@ import org.apache.geode.internal.cache.CachedDeserializable;
 import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
-import org.apache.geode.internal.cache.tier.sockets.OldClientSupportService;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.logging.log4j.LogMarker;
@@ -221,7 +220,7 @@ public abstract class DataSerializer {
       // one indicates it's a non-primitive Class
       out.writeByte(DSCODE.CLASS);
       String cname = c.getName();
-      cname = swizzleClassNameForWrite(cname, out);
+      cname = InternalDataSerializer.processOutgoingClassName(cname, out);
       writeString(cname, out);
     }
   }
@@ -243,7 +242,7 @@ public abstract class DataSerializer {
       logger.trace(LogMarker.SERIALIZER, "Writing Class name {}", className);
     }
 
-    writeString(swizzleClassNameForWrite(className, out), out);
+    writeString(InternalDataSerializer.processOutgoingClassName(className, out), out);
   }
 
   /**
@@ -261,7 +260,6 @@ public abstract class DataSerializer {
     byte typeCode = in.readByte();
     if (typeCode == DSCODE.CLASS) {
       String className = readString(in);
-      className = swizzleClassNameForRead(className, in);
       Class<?> c = InternalDataSerializer.getCachedClass(className); // fix for bug 41206
       return c;
     } else {
@@ -270,52 +268,6 @@ public abstract class DataSerializer {
   }
 
   /**
-   * For backward compatibility we must swizzle the package of some classes that had to be moved
-   * when GemFire was open- sourced. This preserves backward-compatibility.
-   * 
-   * @param name the fully qualified class name
-   * @param in the source of the class name
-   * @return the name of the class in this implementation
-   */
-  private static String swizzleClassNameForRead(String name, DataInput in) {
-    // TCPServer classes are used before a cache exists and support for old clients has been
-    // initialized
-    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
-    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
-    if (name.startsWith(oldPackage)) {
-      return newPackage + name.substring(oldPackage.length());
-    }
-    OldClientSupportService svc = InternalDataSerializer.getOldClientSupportService();
-    if (svc != null) {
-      return svc.processIncomingClassName(name, in);
-    }
-    return name;
-  }
-
-  /**
-   * For backward compatibility we must swizzle the package of some classes that had to be moved
-   * when GemFire was open- sourced. This preserves backward-compatibility.
-   * 
-   * @param name the fully qualified class name
-   * @param out the consumer of the serialized object
-   * @return the name of the class in this implementation
-   */
-  private static String swizzleClassNameForWrite(String name, DataOutput out) {
-    // TCPServer classes are used before a cache exists and support for old clients has been
-    // initialized
-    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
-    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
-    if (name.startsWith(newPackage)) {
-      return oldPackage + name.substring(newPackage.length());
-    }
-    OldClientSupportService svc = InternalDataSerializer.getOldClientSupportService();
-    if (svc != null) {
-      return svc.processOutgoingClassName(name, out);
-    }
-    return name;
-  }
-
-  /**
    * Reads name of an instance of <code>Class</code> from a <code>DataInput</code>.
    * 
    * The return value may be <code>null</code>.
@@ -327,7 +279,7 @@ public abstract class DataSerializer {
 
     InternalDataSerializer.checkIn(in);
 
-    return swizzleClassNameForRead(readString(in), in);
+    return InternalDataSerializer.processIncomingClassName(readString(in));
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/geode/blob/cbff37e7/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
index d136b87..82fbf9d 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
@@ -145,6 +145,51 @@ public abstract class InternalDataSerializer extends DataSerializer implements D
   private static OldClientSupportService oldClientSupportService;
 
   /**
+   * For backward compatibility we must swizzle the package of some classes that had to be moved
+   * when GemFire was open- sourced. This preserves backward-compatibility.
+   * 
+   * @param name the fully qualified class name
+   * @return the name of the class in this implementation
+   */
+  public static String processIncomingClassName(String name) {
+    // TCPServer classes are used before a cache exists and support for old clients has been
+    // initialized
+    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
+    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
+    if (name.startsWith(oldPackage)) {
+      return newPackage + name.substring(oldPackage.length());
+    }
+    OldClientSupportService svc = getOldClientSupportService();
+    if (svc != null) {
+      return svc.processIncomingClassName(name);
+    }
+    return name;
+  }
+
+  /**
+   * For backward compatibility we must swizzle the package of some classes that had to be moved
+   * when GemFire was open- sourced. This preserves backward-compatibility.
+   * 
+   * @param name the fully qualified class name
+   * @param out the consumer of the serialized object
+   * @return the name of the class in this implementation
+   */
+  public static String processOutgoingClassName(String name, DataOutput out) {
+    // TCPServer classes are used before a cache exists and support for old clients has been
+    // initialized
+    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
+    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
+    if (name.startsWith(newPackage)) {
+      return oldPackage + name.substring(newPackage.length());
+    }
+    OldClientSupportService svc = getOldClientSupportService();
+    if (svc != null) {
+      return svc.processOutgoingClassName(name, out);
+    }
+    return name;
+  }
+
+  /**
    * Any time new serialization format is added then a new enum needs to be added here.
    * 
    * @since GemFire 6.6.2
@@ -3954,7 +3999,8 @@ public abstract class InternalDataSerializer extends DataSerializer implements D
       LOAD_CLASS_EACH_TIME ? null : new CopyOnWriteHashMap<String, WeakReference<Class<?>>>();
   private static final Object cacheAccessLock = new Object();
 
-  public static Class<?> getCachedClass(String className) throws ClassNotFoundException {
+  public static Class<?> getCachedClass(String p_className) throws ClassNotFoundException {
+    String className = processIncomingClassName(p_className);
     if (LOAD_CLASS_EACH_TIME) {
       return ClassPathLoader.getLatest().forName(className);
     } else {

http://git-wip-us.apache.org/repos/asf/geode/blob/cbff37e7/geode-core/src/test/java/org/apache/geode/internal/DataSerializableJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/internal/DataSerializableJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/DataSerializableJUnitTest.java
old mode 100644
new mode 100755

http://git-wip-us.apache.org/repos/asf/geode/blob/cbff37e7/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
----------------------------------------------------------------------
diff --git a/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java b/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
index a012c16..1b3a9d2 100644
--- a/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
+++ b/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
@@ -16,8 +16,11 @@ package com.gemstone.gemfire;
 
 import java.io.DataInput;
 import java.io.DataOutput;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.geode.cache.Cache;
+import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.InternalDataSerializer;
 import org.apache.geode.internal.Version;
 import org.apache.geode.internal.VersionedDataOutputStream;
@@ -35,6 +38,15 @@ public class OldClientSupportProvider implements OldClientSupportService {
   static final String GEODE = "org.apache.geode";
   static final String GEMFIRE = "com.gemstone.gemfire";
 
+  static final String ALWAYS_CONVERT_CLASSES_NAME =
+      DistributionConfig.GEMFIRE_PREFIX + "old-client-support.convert-all";
+
+  /** whether to always convert new package names to old on outgoing serialization */
+  static final boolean ALWAYS_CONVERT_CLASSES = Boolean.getBoolean(ALWAYS_CONVERT_CLASSES_NAME);
+
+  private final Map<String, String> oldClassNamesToNew = new ConcurrentHashMap<>();
+  private final Map<String, String> newClassNamesToOld = new ConcurrentHashMap<>();
+
   /** returns the cache's OldClientSupportService */
   public static OldClientSupportService getService(Cache cache) {
     return (OldClientSupportService) ((InternalCache) cache)
@@ -59,57 +71,50 @@ public class OldClientSupportProvider implements OldClientSupportService {
 
   @Override
   public String processIncomingClassName(String name) {
-    if (name.startsWith(GEMFIRE)) {
-      return GEODE + name.substring(GEMFIRE.length());
+    // tcpserver was moved to a different package in Geode.
+    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
+    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
+    if (name.startsWith(oldPackage)) {
+      String cached = oldClassNamesToNew.get(name);
+      if (cached == null) {
+        cached = newPackage + name.substring(oldPackage.length());
+        oldClassNamesToNew.put(name, cached);
+      }
+      return cached;
     }
-    return name;
+    return processClassName(name, GEMFIRE, GEODE, oldClassNamesToNew);
   }
 
 
   @Override
   public String processIncomingClassName(String name, DataInput in) {
-    // tcpserver was moved to a different package in Geode.
-    String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
-    String newPackage = "org.apache.geode.distributed.internal.tcpserver";
-    if (name.startsWith(oldPackage)) {
-      return newPackage + name.substring(oldPackage.length());
-    }
-    if (name.startsWith(GEMFIRE)) {
-      return GEODE + name.substring(GEMFIRE.length());
-    }
-    return name;
+    return processIncomingClassName(name);
   }
 
 
   @Override
   public String processOutgoingClassName(String name, DataOutput out) {
-    // tcpserver was moved to a different package in Geode
+    // tcpserver was moved to a different package
     String oldPackage = "com.gemstone.org.jgroups.stack.tcpserver";
     String newPackage = "org.apache.geode.distributed.internal.tcpserver";
     if (name.startsWith(newPackage)) {
       return oldPackage + name.substring(newPackage.length());
     }
+    if (ALWAYS_CONVERT_CLASSES) {
+      return processClassName(name, GEODE, GEMFIRE, newClassNamesToOld);
+    }
     // if the client is old then it needs com.gemstone.gemfire package names
     if (out instanceof VersionedDataOutputStream) {
       VersionedDataOutputStream vout = (VersionedDataOutputStream) out;
       Version version = vout.getVersion();
       if (version != null && version.compareTo(Version.GFE_90) < 0) {
-        if (name.startsWith(GEODE)) {
-          name = GEMFIRE + name.substring(GEODE.length());
-        }
+        return processClassName(name, GEODE, GEMFIRE, newClassNamesToOld);
       }
     }
     return name;
   }
 
-
-  /**
-   * translates the given exception into one that can be sent to an old GemFire client
-   * 
-   * @param theThrowable the exception to convert
-   * @param clientVersion the version of the client
-   * @return the exception to give the client
-   */
+  @Override
   public Throwable getThrowable(Throwable theThrowable, Version clientVersion) {
 
     if (theThrowable == null) {
@@ -131,4 +136,31 @@ public class OldClientSupportProvider implements OldClientSupportService {
     return theThrowable;
   }
 
+
+  private String processClassName(String p_className, String oldPackage, String newPackage,
+      Map<String, String> cache) {
+    String cached = cache.get(p_className);
+    if (cached != null) {
+      return cached;
+    }
+
+    String className = p_className;
+
+    if (className.startsWith(oldPackage)) {
+      className = newPackage + className.substring(oldPackage.length());
+
+    } else if (className.startsWith("[") && className.contains("[L" + oldPackage)) {
+      int idx = className.indexOf("[L") + 2;
+      className =
+          className.substring(0, idx) + newPackage + className.substring(idx, oldPackage.length());
+    }
+
+    if (className != p_className) {
+      cache.put(p_className, className);
+    }
+
+    return className;
+  }
+
+
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/cbff37e7/geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java b/geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
index 0844bf0..7975fa8 100644
--- a/geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
+++ b/geode-old-client-support/src/test/java/org/apache/geode/OldClientSupportDUnitTest.java
@@ -14,13 +14,18 @@
  */
 package org.apache.geode;
 
+import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
 import java.io.DataOutputStream;
 import java.lang.reflect.Constructor;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 
+import org.apache.geode.internal.HeapDataOutputStream;
+import org.apache.geode.internal.VersionedDataInputStream;
+import org.apache.geode.internal.VersionedDataOutputStream;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -43,6 +48,16 @@ public class OldClientSupportDUnitTest extends JUnit4CacheTestCase {
   static private final List<String> allGeodeThrowableClasses =
       Arrays.asList(new String[] {"org.apache.geode.cache.execute.EmptyRegionFunctionException",});
 
+  static private final List<String> newArrayClassNames = Arrays.asList(new String[] {
+      "[Lorg.apache.geode.class1", "[[Lorg.apache.geode.class1", "[[[Lorg.apache.geode.class1"});
+
+  static private final List<String> oldArrayClassNames =
+      Arrays.asList(new String[] {"[Lcom.gemstone.gemfire.class1", "[[Lcom.gemstone.gemfire.class1",
+          "[[[Lcom.gemstone.gemfire.class1"});
+
+  static private final List<String> allNonconformingArrayClassNames = Arrays.asList(
+      new String[] {"[Lmypackage.org.apache.geode.class2", "[[Lmypackage.org.apache.geode.class2"});
+
   private Cache myCache;
 
   @Override
@@ -67,7 +82,7 @@ public class OldClientSupportDUnitTest extends JUnit4CacheTestCase {
 
     for (String geodeClassName : allGeodeThrowableClasses) {
       try {
-        convertThrowableForOldClient(geodeClassName);
+        convertThrowable(geodeClassName);
       } catch (Exception e) {
         System.out.println("-- failed");
         Exception failure =
@@ -82,7 +97,35 @@ public class OldClientSupportDUnitTest extends JUnit4CacheTestCase {
     }
   }
 
-  private void convertThrowableForOldClient(String geodeClassName) throws Exception {
+  @Test
+  public void testConversionOfArrayTypes() throws Exception {
+    OldClientSupportService oldClientSupport = OldClientSupportProvider.getService(myCache);
+
+    Version oldClientVersion = Version.GFE_82;
+    VersionedDataOutputStream dout = new VersionedDataOutputStream(
+        new HeapDataOutputStream(10, oldClientVersion), oldClientVersion);
+
+    for (String geodeClassName : newArrayClassNames) {
+      String newName = oldClientSupport.processOutgoingClassName(geodeClassName, dout);
+      Assert.assertNotEquals(geodeClassName, newName);
+    }
+
+    for (String className : allNonconformingArrayClassNames) {
+      String newName = oldClientSupport.processOutgoingClassName(className, dout);
+      Assert.assertEquals(className, newName);
+    }
+
+    VersionedDataInputStream din = new VersionedDataInputStream(
+        new DataInputStream(new ByteArrayInputStream(new byte[10])), oldClientVersion);
+
+    for (String oldClassName : oldArrayClassNames) {
+      String newName = oldClientSupport.processIncomingClassName(oldClassName, din);
+      Assert.assertNotEquals(oldClassName, newName);
+    }
+
+  }
+
+  private void convertThrowable(String geodeClassName) throws Exception {
     Version oldClientVersion = Version.GFE_82;
     final String comGemstoneGemFire = "com.gemstone.gemfire";
     final int comGemstoneGemFireLength = comGemstoneGemFire.length();