GEODE-2080 Rest POST put call not working with region valueConstraint

If you set a value constraint on a cache Region you will be unable to store
objects in the region via the Rest API.  This change-set modifies
LocalRegion's constraint check to look for a Rest document and use its
type name in the constraint check


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

Branch: refs/heads/feature/GEODE-1930
Commit: d65b79a4634325edb73e2136f1e59d6879845faf
Parents: e01dbe6
Author: Bruce Schuchardt <bschucha...@pivotal.io>
Authored: Thu Nov 10 15:10:43 2016 -0800
Committer: Bruce Schuchardt <bschucha...@pivotal.io>
Committed: Thu Nov 10 15:12:25 2016 -0800

----------------------------------------------------------------------
 .../geode/internal/cache/LocalRegion.java       | 103 +++++++++++--------
 .../geode/pdx/PdxClientServerDUnitTest.java     |  72 ++++++++++---
 2 files changed, 117 insertions(+), 58 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/d65b79a4/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java 
b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index d413b3b..fd4b6c7 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -15,42 +15,7 @@
 
 package org.apache.geode.internal.cache;
 
-import static 
org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.*;
-
-import java.io.DataInputStream;
-import java.io.DataOutputStream;
-import java.io.File;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.util.AbstractSet;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Hashtable;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.NoSuchElementException;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Future;
-import java.util.concurrent.RejectedExecutionException;
-import java.util.concurrent.Semaphore;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
-import org.apache.logging.log4j.Logger;
+import static 
org.apache.geode.internal.offheap.annotations.OffHeapIdentifier.ENTRY_EVENT_NEW_VALUE;
 
 import org.apache.geode.CancelCriterion;
 import org.apache.geode.CancelException;
@@ -209,6 +174,42 @@ import org.apache.geode.internal.sequencelog.EntryLogger;
 import org.apache.geode.internal.util.concurrent.FutureResult;
 import org.apache.geode.internal.util.concurrent.StoppableCountDownLatch;
 import org.apache.geode.internal.util.concurrent.StoppableReadWriteLock;
+import org.apache.geode.pdx.JSONFormatter;
+import org.apache.geode.pdx.PdxInstance;
+import org.apache.logging.log4j.Logger;
+
+import java.io.DataInputStream;
+import java.io.DataOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.util.AbstractSet;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Hashtable;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Implementation of a local scoped-region. Note that this class has a 
different meaning starting
@@ -508,17 +509,11 @@ public class LocalRegion extends AbstractRegion 
implements LoaderHelperFactory,
   /**
    * ThreadLocal used to set the current region being initialized.
    * 
-   * Currently used by the OpLog layer to initialize the {@link 
KeyWithRegionContext} if required.
+   * Currently used by the OpLog layer.
    */
   private final static ThreadLocal<LocalRegion> initializingRegion = new 
ThreadLocal<LocalRegion>();
 
   /**
-   * Set to true if the region contains keys implementing {@link 
KeyWithRegionContext} that require
-   * setting up of region specific context after deserialization or recovery 
from disk.
-   */
-  private boolean keyRequiresRegionContext;
-
-  /**
    * Get the current initializing region as set in the ThreadLocal.
    * 
    * Note that this value is cleared after the initialization of LocalRegion 
is done so is valid
@@ -3259,6 +3254,7 @@ public class LocalRegion extends AbstractRegion 
implements LoaderHelperFactory,
    */
   protected void validateValue(Object p_value) {
     Object value = p_value;
+
     // check validity of value against valueConstraint
     if (this.valueConstraint != null) {
       if (value != null) {
@@ -3269,11 +3265,28 @@ public class LocalRegion extends AbstractRegion 
implements LoaderHelperFactory,
             return;
           }
         }
-        if (!this.valueConstraint.isInstance(value))
+        if (!this.valueConstraint.isInstance(value)) {
+          String valueClassName = value.getClass().getName();
+          // check for a REST object, which has a @type field denoting its 
class
+          if (value instanceof PdxInstance) {
+            PdxInstance pdx = (PdxInstance) value;
+            if (pdx.getClassName().equals(JSONFormatter.JSON_CLASSNAME)) {
+              Object type = (String) pdx.getField("@type");
+              if (type != null && type instanceof String) {
+                valueClassName = (String) type;
+              } else {
+                return;
+              }
+            }
+            if (valueClassName.equals(this.valueConstraint.getName())) {
+              return;
+            }
+          }
           throw new ClassCastException(
               
LocalizedStrings.LocalRegion_VALUE_0_DOES_NOT_SATISFY_VALUECONSTRAINT_1
                   .toLocalizedString(
-                      new Object[] {value.getClass().getName(), 
this.valueConstraint.getName()}));
+                      new Object[] {valueClassName, 
this.valueConstraint.getName()}));
+        }
       }
     }
   }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/d65b79a4/geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java 
b/geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java
index 9a9680a..b0820f9 100644
--- 
a/geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/pdx/PdxClientServerDUnitTest.java
@@ -30,6 +30,8 @@ import org.apache.geode.cache.Cache;
 import org.apache.geode.cache.CacheFactory;
 import org.apache.geode.cache.DataPolicy;
 import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionFactory;
+import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.Scope;
 import org.apache.geode.cache.client.ClientCache;
 import org.apache.geode.cache.client.ClientCacheFactory;
@@ -70,7 +72,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm3 = host.getVM(3);
 
 
-    createServerRegion(vm0);
+    createServerRegion(vm0, SimpleClass.class);
     int port = createServerAccessor(vm3);
     createClientRegion(vm1, port);
     createClientRegion(vm2, port);
@@ -106,7 +108,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm1 = host.getVM(1);
     VM vm2 = host.getVM(2);
 
-    int port = createServerRegion(vm0);
+    int port = createServerRegion(vm0, SimpleClass.class);
     createClientRegion(vm1, port, false, true);
 
     // Define a PDX type with 2 fields that will be cached on the client
@@ -326,7 +328,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm2 = host.getVM(2);
 
 
-    int port = createServerRegion(vm0);
+    int port = createServerRegion(vm0, SimpleClass.class);
     createClientRegion(vm1, port, true);
     createClientRegion(vm2, port, true);
 
@@ -361,7 +363,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm2 = host.getVM(2);
 
 
-    int port = createServerRegion(vm0);
+    int port = createServerRegion(vm0, SimpleClass.class);
     createClientRegion(vm1, port);
     createClientRegion(vm2, port);
 
@@ -482,7 +484,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm2 = host.getVM(2);
 
 
-    int port = createServerRegion(vm0);
+    int port = createServerRegion(vm0, Object.class);
     createClientRegion(vm1, port);
     createClientRegion(vm2, port);
 
@@ -523,7 +525,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm2 = host.getVM(2);
 
 
-    final int port = createServerRegion(vm0);
+    final int port = createServerRegion(vm0, SimpleClass.class);
     SerializableCallable createRegion = new SerializableCallable() {
       public Object call() throws Exception {
         Properties props = new Properties();
@@ -574,7 +576,7 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     VM vm1 = host.getVM(1);
 
 
-    final int port = createServerRegion(vm0);
+    final int port = createServerRegion(vm0, SimpleClass.class);
     SerializableCallable createRegion = new SerializableCallable() {
       public Object call() throws Exception {
         Properties props = new Properties();
@@ -600,16 +602,60 @@ public class PdxClientServerDUnitTest extends 
JUnit4CacheTestCase {
     vm1.invoke(createRegion);
   }
 
-  private void createCacheWithAutoSerializer(VM vm, final String... patterns) {
-    vm.invoke(new SerializableRunnable() {
-      public void run() {
-        CacheFactory cf = new CacheFactory();
-        cf.setPdxSerializer(new ReflectionBasedAutoSerializer(patterns));
-        getCache(cf);
+  @Test
+  public void testCacheRejectsJSonPdxInstanceViolatingValueConstraint() {
+    Host host = Host.getHost(0);
+    VM vm0 = host.getVM(0);
+    VM vm1 = host.getVM(1);
+
+
+    int port = createServerRegion(vm0, SimpleClass.class);
+
+    vm0.invoke(new SerializableCallable("put value in region") {
+      public Object call() throws Exception {
+        Region r = basicGetCache().getRegion("testSimplePdx");
+        String className = "objects.PersonWithoutID";
+        try {
+          r.put("pdxObject", getTypedJSONPdxInstance(className));
+          fail("expected a ClassCastException");
+        } catch (ClassCastException e) {
+          assertTrue("wrong ClassCastException message: " + e.getMessage(),
+              e.getMessage().contains(className));
+        }
+        return null;
       }
     });
   }
 
+
+  public PdxInstance getTypedJSONPdxInstance(String className) throws 
Exception {
+
+    String aRESTishDoc = "{\"@type\": \" " + className
+        + "\" , \" firstName\" : \" John\" , \" lastName\" : \" Smith\" , \" 
age\" : 25 }";
+    PdxInstance pdxInstance = JSONFormatter.fromJSON(aRESTishDoc);
+
+    return pdxInstance;
+  }
+
+  private int createServerRegion(VM vm, final Class constraintClass) {
+    SerializableCallable createRegion = new SerializableCallable() {
+      public Object call() throws Exception {
+        CacheFactory cf = new CacheFactory(getDistributedSystemProperties());
+        Cache cache = getCache(cf);
+        RegionFactory rf = cache.createRegionFactory(RegionShortcut.REPLICATE);
+        rf.setValueConstraint(constraintClass);
+        rf.create("testSimplePdx");
+        CacheServer server = cache.addCacheServer();
+        int port = AvailablePortHelper.getRandomAvailableTCPPort();
+        server.setPort(port);
+        server.start();
+        return port;
+      }
+    };
+
+    return (Integer) vm.invoke(createRegion);
+  }
+
   private int createServerRegion(VM vm) {
     SerializableCallable createRegion = new SerializableCallable() {
       public Object call() throws Exception {

Reply via email to