Stable pdx instance (not yet ready for review)

geode-core/src/main/java/org/apache/geode/cache/query/internal/AttributeDescriptor.java

@@ -34,10 +34,10 @@
 import org.apache.geode.cache.query.QueryService;
 import org.apache.geode.internal.cache.Token;
 import org.apache.geode.pdx.JSONFormatter;
-import org.apache.geode.pdx.PdxInstance;
 import org.apache.geode.pdx.PdxSerializationException;
-import org.apache.geode.pdx.internal.FieldNotFoundInPdxVersion;
-import org.apache.geode.pdx.internal.PdxInstanceImpl;
+import org.apache.geode.pdx.internal.InternalPdxInstance;
+import org.apache.geode.pdx.internal.PdxType;
+import org.apache.geode.pdx.internal.TypeRegistry;
 
 /**
  * Utility for managing an attribute
@@ -49,12 +49,15 @@
 public class AttributeDescriptor {
   private final String _name;
   private final MethodInvocationAuthorizer _methodInvocationAuthorizer;
+  private final TypeRegistry _pdxRegistry;
   /** cache for remembering the correct Member for a class and attribute */
   private static final ConcurrentMap<List, Member> _localCache = new ConcurrentHashMap();
 
 
 
-  public AttributeDescriptor(MethodInvocationAuthorizer methodInvocationAuthorizer, String name) {
+  public AttributeDescriptor(TypeRegistry pdxRegistry,
+      MethodInvocationAuthorizer methodInvocationAuthorizer, String name) {
+    _pdxRegistry = pdxRegistry;
     _methodInvocationAuthorizer = methodInvocationAuthorizer;
     _name = name;
   }
@@ -75,8 +78,8 @@ public Object read(Object target) throws NameNotFoundException, QueryInvocationT
     if (target == null || target == QueryService.UNDEFINED) {
       return QueryService.UNDEFINED;
     }
-    if (target instanceof PdxInstance) {
-      return readPdx((PdxInstance) target);
+    if (target instanceof InternalPdxInstance) {
+      return readPdx((InternalPdxInstance) target);
     }
     // for non pdx objects
     return readReflection(target);
@@ -202,70 +205,58 @@ private Method getReadMethod(Class targetType, String methodName) {
    *
    * @return the value of the field from PdxInstance
    */
-  private Object readPdx(PdxInstance target)
+  private Object readPdx(InternalPdxInstance pdxInstance)
       throws NameNotFoundException, QueryInvocationTargetException {
-    if (target instanceof PdxInstanceImpl) {
-      PdxInstanceImpl pdxInstance = (PdxInstanceImpl) target;
-      // if the field is present in the pdxinstance
-      if (pdxInstance.hasField(_name)) {
-        // return PdxString if field is a String otherwise invoke readField
-        return pdxInstance.getRawField(_name);
-      } else {
-        // field not found in the pdx instance, look for the field in any of the
-        // PdxTypes (versions of the pdxinstance) in the type registry
-        String className = pdxInstance.getClassName();
-
-        // don't look further for field or method or reflect on GemFire JSON data
-        if (className.equals(JSONFormatter.JSON_CLASSNAME)) {
-          return QueryService.UNDEFINED;
-        }
+    // if the field is present in the pdxinstance
+    if (pdxInstance.hasField(_name)) {
+      // return PdxString if field is a String otherwise invoke readField
+      return pdxInstance.getRawField(_name);
+    } else {
+      // field not found in the pdx instance, look for the field in any of the
+      // PdxTypes (versions of the pdxinstance) in the type registry
+      String className = pdxInstance.getClassName();
 
+      // don't look further for field or method or reflect on GemFire JSON data
+      if (className.equals(JSONFormatter.JSON_CLASSNAME)) {
+        return QueryService.UNDEFINED;
+      }
 
-        // check if the field was not found previously
-        if (!isFieldAlreadySearchedAndNotFound(className, _name)) {
-          try {
-            return pdxInstance.getDefaultValueIfFieldExistsInAnyPdxVersions(_name, className);
-          } catch (FieldNotFoundInPdxVersion e1) {
-            // remember the field that is not present in any version to avoid
-            // trips to the registry next time
-            updateClassToFieldsMap(className, _name);
-          }
+      // check if the field was not found previously
+      if (!isFieldAlreadySearchedAndNotFound(className, _name)) {
+        PdxType pdxType = _pdxRegistry.getPdxTypeForField(_name, className);
+        if (pdxType == null) {
+          // remember the field that is not present in any version to avoid
+          // trips to the registry next time
+          updateClassToFieldsMap(className, _name);
+        } else {
+          return pdxType.getPdxField(_name).getFieldType().getDefaultValue();
         }
-        // if the field is not present in any of the versions try to
-        // invoke implicit method call
-        if (!this.isMethodAlreadySearchedAndNotFound(className, _name)) {
-          try {
-            return readFieldFromDeserializedObject(pdxInstance, target);
-          } catch (NameNotFoundException ex) {
-            updateClassToMethodsMap(pdxInstance.getClassName(), _name);
-            throw ex;
-          }
-        } else
-          return QueryService.UNDEFINED;
-      }
-    } else {
-      // target could be another implementation of PdxInstance like
-      // PdxInstanceEnum, in this case getRawField and getCachedOjects methods are
-      // not available
-      if (((PdxInstance) target).hasField(_name)) {
-        return ((PdxInstance) target).getField(_name);
       }
-      throw new NameNotFoundException(
-          String.format("Field ' %s ' in class ' %s ' is not accessible to the query processor",
-              new Object[] {_name, target.getClass().getName()}));
+      // if the field is not present in any of the versions try to
+      // invoke implicit method call
+      if (!this.isMethodAlreadySearchedAndNotFound(className, _name)) {
+        try {
+          return readFieldFromDeserializedObject(pdxInstance);
+        } catch (NameNotFoundException ex) {
+          updateClassToMethodsMap(pdxInstance.getClassName(), _name);
+          throw ex;
+        }
+      } else
+        return QueryService.UNDEFINED;
     }
   }
 
-  private Object readFieldFromDeserializedObject(PdxInstanceImpl pdxInstance, Object target)
+  private Object readFieldFromDeserializedObject(InternalPdxInstance pdxInstance)
       throws NameNotFoundException, QueryInvocationTargetException {
+    Object obj = null;
     try {
-      Object obj = pdxInstance.getCachedObject();
-      return readReflection(obj);
+      obj = pdxInstance.getCachedObject();
     } catch (PdxSerializationException e) {
       throw new NameNotFoundException( // the domain object is not available
-          String.format("Field ' %s ' in class ' %s ' is not accessible to the query processor",
-              new Object[] {_name, target.getClass().getName()}));
+          String.format("Field '%s' is not accessible to the query processor because: %s",
+              new Object[] {_name, e.getMessage()}));
     }
+    return readReflection(obj);
   }
 
   private void updateClassToFieldsMap(String className, String field) {

geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledOperation.java

@@ -35,7 +35,7 @@
 import org.apache.geode.internal.cache.PartitionedRegion;
 import org.apache.geode.pdx.PdxInstance;
 import org.apache.geode.pdx.PdxSerializationException;
-import org.apache.geode.pdx.internal.PdxInstanceImpl;
+import org.apache.geode.pdx.internal.InternalPdxInstance;
 import org.apache.geode.pdx.internal.PdxString;
 
 /**
@@ -280,13 +280,9 @@ private Object eval0(Object receiver, Class resolutionType, ExecutionContext con
       // cache
       CompiledOperation.cache.putIfAbsent(key, methodDispatch);
     }
-    if (receiver instanceof PdxInstance) {
+    if (receiver instanceof InternalPdxInstance) {
       try {
-        if (receiver instanceof PdxInstanceImpl) {
-          receiver = ((PdxInstanceImpl) receiver).getCachedObject();
-        } else {
-          receiver = ((PdxInstance) receiver).getObject();
-        }
+        receiver = ((InternalPdxInstance) receiver).getCachedObject();
       } catch (PdxSerializationException ex) {
         throw new QueryInvocationTargetException(ex);
       }

geode-core/src/main/java/org/apache/geode/cache/query/internal/PathUtils.java

@@ -72,7 +72,7 @@ public static Object evaluateAttribute(ExecutionContext context, Object target,
       }
     }
     try {
-      return new AttributeDescriptor(
+      return new AttributeDescriptor(context.getCache().getPdxRegistry(),
           context.getCache().getQueryService().getMethodInvocationAuthorizer(), attribute)
               .read(target);
     } catch (NameNotFoundException nfe) {
@@ -112,7 +112,7 @@ public static ObjectType[] calculateTypesAlongPath(ExecutionContext context,
 
     for (int i = 1; i < types.length; i++) {
       ObjectType currentType = types[i - 1];
-      Member member = new AttributeDescriptor(
+      Member member = new AttributeDescriptor(context.getCache().getPdxRegistry(),
           context.getCache().getQueryService().getMethodInvocationAuthorizer(), pathArray[i - 1])
               .getReadMember(currentType.resolveClass());
 
@@ -170,7 +170,7 @@ public static ObjectType computeElementTypeOfExpression(ExecutionContext context
             stepStr = stepStr.substring(0, stepStr.length() - 2);
             member = clazz.getMethod(stepStr, (Class[]) null);
           } else {
-            member = new AttributeDescriptor(
+            member = new AttributeDescriptor(context.getCache().getPdxRegistry(),
                 context.getCache().getQueryService().getMethodInvocationAuthorizer(), stepStr)
                     .getReadMember(clazz);
           }

geode-core/src/main/java/org/apache/geode/cache/query/internal/RuntimeIterator.java

@@ -196,7 +196,7 @@ boolean containsProperty(ExecutionContext context, String name, int numArgs, boo
     // if there are zero arguments and it's an attribute, then defer to
     // AttributeDescriptor
     // to see if there's a match
-    return new AttributeDescriptor(
+    return new AttributeDescriptor(context.getCache().getPdxRegistry(),
         context.getCache().getQueryService().getMethodInvocationAuthorizer(), name)
             .validateReadType(clazz);
   }

geode-core/src/main/java/org/apache/geode/pdx/internal/EnumInfo.java

@@ -27,7 +27,6 @@
 import org.apache.geode.internal.DataSerializableFixedID;
 import org.apache.geode.internal.HeapDataOutputStream;
 import org.apache.geode.internal.InternalDataSerializer;
-import org.apache.geode.internal.Sendable;
 import org.apache.geode.internal.Version;
 import org.apache.geode.pdx.PdxInstance;
 import org.apache.geode.pdx.PdxSerializationException;
@@ -191,7 +190,7 @@ public PdxInstance getPdxInstance(int enumId) {
   }
 
   public static class PdxInstanceEnumInfo
-      implements PdxInstance, Sendable, ConvertableToBytes, ComparableEnum {
+      implements InternalPdxInstance, ComparableEnum {
     private static final long serialVersionUID = 7907582104525106416L;
     private final int enumId;
     private final EnumInfo ei;

geode-core/src/main/java/org/apache/geode/pdx/internal/InternalPdxInstance.java

@@ -14,7 +14,33 @@
  */
 package org.apache.geode.pdx.internal;
 
+import org.apache.geode.internal.Sendable;
 import org.apache.geode.pdx.PdxInstance;
+import org.apache.geode.pdx.PdxSerializationException;
+
+public interface InternalPdxInstance extends PdxInstance, ConvertableToBytes, Sendable {
+  /**
+   * The same as calling getObject() but may also cache the result and future calls
+   * of this method will return the cached object instead of recomputing it.
+   * Implementors that do not want to support a cache can just use the default implementation
+   * which simply calls getObject().
+   *
+   * @throws PdxSerializationException if the instance could not be deserialized
+   */
+  default Object getCachedObject() {
+    return getObject();
+  }
+
+  /**
+   * This same as calling getField(fieldName) except that some implementations may support
+   * returning a PdxString instead of String.
+   * Implementors that do not support PdxString can use use the default implementation
+   * which simply calls getField(fieldName).
+   *
+   * @throws PdxSerializationException if the field could not be deserialized
+   */
+  default Object getRawField(String fieldName) {
+    return getField(fieldName);
+  }
 
-public interface InternalPdxInstance extends PdxInstance, ConvertableToBytes {
 }

geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceEnum.java

@@ -24,9 +24,7 @@
 import org.apache.geode.internal.DSCODE;
 import org.apache.geode.internal.HeapDataOutputStream;
 import org.apache.geode.internal.InternalDataSerializer;
-import org.apache.geode.internal.Sendable;
 import org.apache.geode.internal.Version;
-import org.apache.geode.pdx.PdxInstance;
 import org.apache.geode.pdx.PdxSerializationException;
 import org.apache.geode.pdx.WritablePdxInstance;
 
@@ -35,7 +33,7 @@
  *
  * @since GemFire 6.6.2
  */
-public class PdxInstanceEnum implements PdxInstance, Sendable, ConvertableToBytes, ComparableEnum {
+public class PdxInstanceEnum implements InternalPdxInstance, ComparableEnum {
   private static final long serialVersionUID = -7417287878052772302L;
   private final String className;
   private final String enumName;

geode-core/src/main/java/org/apache/geode/pdx/internal/PdxInstanceImpl.java

@@ -35,7 +35,6 @@
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.DSCODE;
 import org.apache.geode.internal.InternalDataSerializer;
-import org.apache.geode.internal.Sendable;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.tcp.ByteBufferInputStream;
@@ -55,7 +54,7 @@
  * We do not use this normal java io serialization when serializing this class in GemFire because
  * Sendable takes precedence over Serializable.
  */
-public class PdxInstanceImpl extends PdxReaderImpl implements InternalPdxInstance, Sendable {
+public class PdxInstanceImpl extends PdxReaderImpl implements InternalPdxInstance {
 
   private static final long serialVersionUID = -1669268527103938431L;
 
@@ -186,7 +185,7 @@ public byte[] toBytes() {
     }
   }
 
-  // this is for internal use of the query engine.
+  @Override
   public Object getCachedObject() {
     Object result = this.cachedObjectForm;
     if (result == null) {
@@ -652,20 +651,8 @@ public boolean isEnum() {
     return false;
   }
 
+  @Override
   public Object getRawField(String fieldName) {
     return getUnmodifiableReader(fieldName).readRawField(fieldName);
   }
-
-  public Object getDefaultValueIfFieldExistsInAnyPdxVersions(String fieldName, String className)
-      throws FieldNotFoundInPdxVersion {
-    PdxType pdxType =
-        GemFireCacheImpl.getForPdx("PDX registry is unavailable because the Cache has been closed.")
-            .getPdxRegistry().getPdxTypeForField(fieldName, className);
-    if (pdxType == null) {
-      throw new FieldNotFoundInPdxVersion(
-          "PdxType with field " + fieldName + " is not found for class " + className);
-    }
-    return pdxType.getPdxField(fieldName).getFieldType().getDefaultValue();
-  }
-
 }

geode-core/src/main/resources/org/apache/geode/internal/sanctioned-geode-core-serializables.txt

@@ -636,7 +636,6 @@ org/apache/geode/pdx/PdxInitializationException,true,5098737377658808834
 org/apache/geode/pdx/PdxRegistryMismatchException,true,-2329989020829052537
 org/apache/geode/pdx/PdxSerializationException,true,-3843814927034345635
 org/apache/geode/pdx/internal/EnumInfo$PdxInstanceEnumInfo,true,7907582104525106416,ei:org/apache/geode/pdx/internal/EnumInfo,enumId:int
-org/apache/geode/pdx/internal/FieldNotFoundInPdxVersion,true,1292033563588485577
 org/apache/geode/pdx/internal/PdxInputStream,false
 org/apache/geode/pdx/internal/PdxReaderImpl,true,-6094553093860427759,blobType:org/apache/geode/pdx/internal/PdxType,dis:org/apache/geode/pdx/internal/PdxInputStream
 org/apache/geode/redis/internal/CoderException,true,4707944288714910949