[FABCJ-183] Extra properties appear in JSON from Java Objects

mbwhite · jt-nti · commit e32e404676dc · 2019-12-05T11:49:29.000Z
- Objects returned from a transaction function now will not have
  any spurious properties in them
- In testing located handling of array types was wrong; provided
  improved handling for those
- Updating number of original tests for different cases

Signed-off-by: Matthew B. White &lt;whitemat@uk.ibm.com&gt;
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/JSONTransactionSerializer.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/JSONTransactionSerializer.java
@@ -14,6 +14,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 
 import org.hyperledger.fabric.Logger;
 import org.hyperledger.fabric.contract.ContractRuntimeException;
@@ -57,7 +58,7 @@ public byte[] toBuffer(Object value, TypeSchema ts) {
             if (type != null) {
                 switch (type) {
                 case "array":
-                    JSONArray array = new JSONArray(value);
+                    JSONArray array = normalizeArray(new JSONArray(value),ts);
                     buffer = array.toString().getBytes(UTF_8);
                     break;
                 case "string":
@@ -70,13 +71,73 @@ public byte[] toBuffer(Object value, TypeSchema ts) {
                     buffer = (value).toString().getBytes(UTF_8);
                 }
             } else {
-                JSONObject obj = new JSONObject(value);
+                // at this point we can assert that the value is 
+                // representing a complex data type
+                // so we can get this from 
+                // the type registry, and get the list of propertyNames 
+                // it should have
+                DataTypeDefinition dtd = this.typeRegistry.getDataType(ts);
+                Set<String> keySet = dtd.getProperties().keySet();
+                String[] propNames = keySet.toArray(new String[keySet.size()]);
+                
+                // Note: whilst the current JSON library does pretty much
+                // everything is required, this part is hard.
+                // we want to create a JSON Object based on the value, 
+                // with certain property names.
+
+                // Based on the constructors available we need to have a two 
+                // step process, create a JSON Object, then create the object
+                // we really want based on the propNames
+                JSONObject obj = new JSONObject(new JSONObject(value),propNames);
                 buffer = obj.toString().getBytes(UTF_8);
             }
         }
         return buffer;
     }
 
+    /**
+     * We need to take the JSON array, and if there are complex datatypes within it
+     * ensure that they don't get spurious JSON properties appearing
+     * 
+     * This method needs to be general so has to copy with nested arrays
+     * and with primitive and Object types
+     */
+    private JSONArray normalizeArray(JSONArray jsonArray, TypeSchema ts){
+        JSONArray normalizedArray;
+        
+        // Need to work with what type of array this is
+        TypeSchema items = ts.getItems();
+        String type = items.getType();
+        
+        if (type != null && type != "array" ){
+            // primitive - can return this directly
+            normalizedArray = jsonArray;
+        } else if ( type != null && type == "array") {
+            // nested arrays, get the type of what it makes up
+            // Need to loop over all elements and normalize each one
+            normalizedArray = new JSONArray();
+            for (int i=0; i<jsonArray.length(); i++){
+                normalizedArray.put(i,normalizeArray(jsonArray.getJSONArray(i),items));
+            }
+        } else {
+            // get the permitted propeties in the type, 
+            // then loop over the array and ensure they are correct
+            DataTypeDefinition dtd = this.typeRegistry.getDataType(items);
+            Set<String> keySet = dtd.getProperties().keySet();
+            String[] propNames = keySet.toArray(new String[keySet.size()]);
+          
+            normalizedArray = new JSONArray();
+            // array of objects
+            // iterate over said array
+            for (int i=0; i<jsonArray.length(); i++){         
+                JSONObject obj = new JSONObject(jsonArray.getJSONObject(i),propNames);
+                normalizedArray.put(i,obj);
+            }
+
+        }
+        return normalizedArray;
+    }
+
     /**
      * Take the byte buffer and return the object as required
      *
@@ -103,6 +164,46 @@ public Object fromBuffer(byte[] buffer, TypeSchema ts) {
         }
     }
 
+    /** We need to be able to map between the primative class types
+     * and the object variants. In the case where this is needed
+     * Java auto-boxing doesn't actually help.
+     * 
+     * For other types the parameter is passed directly back
+     * 
+     * @param primitive class for the primitive 
+     * @return Class for the Object variant
+     */
+    private Class<?> mapPrimitive(Class<?> primitive){
+        String primitiveType;
+        boolean isArray = primitive.isArray();
+        if (isArray){
+            primitiveType = primitive.getComponentType().getName();
+        } else {
+            primitiveType = primitive.getName();
+        }
+
+        switch (primitiveType) {
+            case "int":
+                return isArray ? Integer[].class : Integer.class;
+            case "long":
+                return isArray ? Long[].class: Long.class;
+            case "float":
+                return isArray ? Float[].class:Float.class;
+            case "double":
+                return isArray ? Double[].class:Double.class;
+            case "short":
+                return isArray ? Short[].class:Short.class;
+            case "byte":
+                return isArray ? Byte[].class:Byte.class;
+            case "char":
+                return isArray ? Character[].class:Character.class;
+            case "boolean":
+                return isArray ? Boolean[].class:Boolean.class;
+             default:
+                return primitive;
+        }
+    }
+
     /*
      * Internal method to do the conversion
      */
@@ -124,8 +225,14 @@ private Object _convert(String stringData, TypeSchema ts)
             String intFormat = ts.getFormat();
             if (intFormat.contentEquals("int32")) {
                 value = Integer.parseInt(stringData);
-            } else {
+            } else if (intFormat.contentEquals("int8")) {
+                value = Byte.parseByte(stringData);
+            } else if (intFormat.contentEquals("int16")){
+                value = Short.parseShort(stringData);
+            } else if (intFormat.contentEquals("int64")){
                 value = Long.parseLong(stringData);
+            } else {
+                throw new RuntimeException("Unknown format for integer "+intFormat);
             }
         } else if (type.contentEquals("number")) {
             String numFormat = ts.getFormat();
@@ -141,17 +248,27 @@ private Object _convert(String stringData, TypeSchema ts)
         } else if (type.contentEquals("array")) {
             JSONArray jsonArray = new JSONArray(stringData);
             TypeSchema itemSchema = ts.getItems();
-            Object[] data = (Object[]) Array.newInstance(itemSchema.getTypeClass(this.typeRegistry),
+
+            // note here that the type has to be converted in the case of primitives
+            Object[] data = (Object[]) Array.newInstance(mapPrimitive(itemSchema.getTypeClass(this.typeRegistry)),
                     jsonArray.length());
             for (int i = 0; i < jsonArray.length(); i++) {
-                data[i] = _convert(jsonArray.get(i).toString(), itemSchema);
+                Object convertedData = _convert(jsonArray.get(i).toString(), itemSchema);
+                data[i] = convertedData;
             }
             value = data;
 
         }
         return value;
     }
 
+    /** Create new instance of the specificied object from the supplied JSON String
+     * 
+     * @param format      Details of the format needed
+     * @param jsonString  JSON string
+     * @param ts          TypeSchema
+     * @return new object
+     */
     Object createComponentInstance(String format, String jsonString, TypeSchema ts) {
 
         DataTypeDefinition dtd = this.typeRegistry.getDataType(format);
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/impl/ContractExecutionService.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/execution/impl/ContractExecutionService.java
@@ -50,7 +50,7 @@ public Chaincode.Response executeRequest(TxFunction txFn, InvocationRequest req,
 
             final List<Object> args = convertArgs(req.getArgs(), txFn);
             args.add(0, context); // force context into 1st position, other elements move up
-
+            
             contractObject.beforeTransaction(context);
             Object value = rd.getMethod().invoke(contractObject, args.toArray());
             contractObject.afterTransaction(context, value);
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/metadata/TypeSchema.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/metadata/TypeSchema.java
@@ -100,7 +100,37 @@ public Class<?> getTypeClass(TypeRegistry typeRegistry) {
         if (type.contentEquals("string")) {
             clz = String.class;
         } else if (type.contentEquals("integer")) {
-            clz = int.class;
+            // need to check the format
+            String format = getFormat();
+            switch(format) {
+                case "int8":
+                  clz = byte.class;
+                  break;
+                case "int16":
+                  clz = short.class;
+                  break;
+                case "int32":
+                  clz = int.class;
+                  break;
+                case "int64":
+                  clz = long.class;
+                  break;
+                default:
+                  throw new RuntimeException("Unkown format for integer of "+format);
+            }
+        } else if (type.contentEquals("number")) {
+            // need to check the format
+            String format = getFormat();
+            switch(format) {
+                case "double":
+                  clz = double.class;
+                  break;
+                case "float":
+                  clz = float.class;
+                  break;
+                default:
+                  throw new RuntimeException("Unkown format for number of "+format);
+            }
         } else if (type.contentEquals("boolean")) {
             clz = boolean.class;
         } else if (type.contentEquals("object")) {
@@ -132,8 +162,17 @@ public static TypeSchema typeConvert(Class<?> clz) {
         if (clz.isArray()) {
             returnschema.put("type", "array");
             schema = new TypeSchema();
-            returnschema.put("items", schema);
-            className = className.substring(0, className.length() - 2);
+            
+            // double check the componentType
+            Class<?> componentClass = clz.getComponentType();
+            if (componentClass.isArray()){
+                // nested arrays
+                returnschema.put("items",TypeSchema.typeConvert(componentClass));
+            } else {
+                returnschema.put("items", schema);
+            }
+            
+            className = componentClass.getTypeName();
         } else {
             schema = returnschema;
         }
@@ -177,7 +216,6 @@ public static TypeSchema typeConvert(Class<?> clz) {
             schema.put("type", "boolean");
             break;
         default:
-
             schema.put("$ref", "#/components/schemas/" + className.substring(className.lastIndexOf('.') + 1));
         }
 
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/DataTypeDefinition.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/DataTypeDefinition.java
@@ -6,6 +6,7 @@
 package org.hyperledger.fabric.contract.routing;
 
 import java.util.Map;
+import org.hyperledger.fabric.contract.metadata.TypeSchema;
 
 public interface DataTypeDefinition {
 
@@ -15,5 +16,7 @@ public interface DataTypeDefinition {
 
 	String getSimpleName();
 
-    Class<?> getTypeClass();
+	Class<?> getTypeClass();
+	
+	TypeSchema getSchema();
 }
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/TypeRegistry.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/TypeRegistry.java
@@ -6,8 +6,8 @@
 package org.hyperledger.fabric.contract.routing;
 
 import java.util.Collection;
-
 import org.hyperledger.fabric.contract.routing.impl.TypeRegistryImpl;
+import org.hyperledger.fabric.contract.metadata.TypeSchema;
 
 public interface TypeRegistry {
 
@@ -21,6 +21,8 @@ static TypeRegistry getRegistry(){
 
 	DataTypeDefinition getDataType(String name);
 
+	DataTypeDefinition getDataType(TypeSchema schema);
+
 	Collection<DataTypeDefinition> getAllDataTypes();
 
 }
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/DataTypeDefinitionImpl.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/DataTypeDefinitionImpl.java
@@ -70,6 +70,10 @@ public Class<?> getTypeClass() {
 		return this.clazz;
 	}
 
+	public TypeSchema getSchema() {
+		return TypeSchema.typeConvert(this.clazz);
+	}
+
 	/*
 	 * (non-Javadoc)
 	 *
@@ -102,4 +106,9 @@ public String getSimpleName() {
 		return simpleName;
 	}
 
+	@Override
+	public String toString() {
+		return this.simpleName + " " + properties;
+	}
+
 }
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/SerializerRegistryImpl.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/SerializerRegistryImpl.java
@@ -49,7 +49,6 @@ public SerializerRegistryImpl() {
      */
     public SerializerInterface getSerializer(String name, Serializer.TARGET target) {
         String key = name+":"+target;
-        System.out.println("Getting "+key);
         return contents.get(key);
     }
 
@@ -58,7 +57,6 @@ private SerializerInterface add(String name, Serializer.TARGET target, Class<Ser
         try{
         	String key = name+":"+target;
             SerializerInterface newObj = clazz.newInstance();
-            System.out.println("Addding "+key);
             this.contents.put(key,newObj);
 
             return newObj;
diff --git a/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/TypeRegistryImpl.java b/fabric-chaincode-shim/src/main/java/org/hyperledger/fabric/contract/routing/impl/TypeRegistryImpl.java
@@ -9,7 +9,7 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-
+import org.hyperledger.fabric.contract.metadata.TypeSchema;
 import org.hyperledger.fabric.contract.routing.DataTypeDefinition;
 import org.hyperledger.fabric.contract.routing.TypeRegistry;
 
@@ -20,7 +20,6 @@
  */
 public class TypeRegistryImpl implements TypeRegistry {
 
-
 	private static TypeRegistryImpl singletonInstance;
 
 	public static TypeRegistry getInstance(){
@@ -60,4 +59,11 @@ public DataTypeDefinition getDataType(String name) {
 		return this.components.get(name);
 	}
 
+	@Override
+	public DataTypeDefinition getDataType(TypeSchema schema) {
+		String ref = schema.getRef();
+		String format = ref.substring(ref.lastIndexOf("/") + 1);
+		return getDataType(format);
+	}
+
 }
diff --git a/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/MyType.java b/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/MyType.java
@@ -15,6 +15,23 @@ public class MyType {
 	@Property()
 	private String value;
 
+	private String state="";
+
+	public final static String STARTED = "STARTED";
+	public final static String STOPPED = "STOPPED";
+
+	public void setState(String state){
+		this.state = state;
+	}
+
+	public boolean isStarted(){
+		return state.equals(STARTED);
+	}
+
+	public boolean isStopped(){
+		return state.equals(STARTED);
+	}
+
 	public MyType setValue(String value) {
 		this.value = value;
 		return this;
diff --git a/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/execution/JSONTransactionSerializerTest.java b/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/execution/JSONTransactionSerializerTest.java
diff --git a/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/metadata/TypeSchemaTest.java b/fabric-chaincode-shim/src/test/java/org/hyperledger/fabric/contract/metadata/TypeSchemaTest.java

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,10 @@ public Class<?> getTypeClass() {`
`70`	`70`	`return this.clazz;`
`71`	`71`	`}`
`72`	`72`
	`73`	`+ public TypeSchema getSchema() {`
	`74`	`+ return TypeSchema.typeConvert(this.clazz);`
	`75`	`+ }`
	`76`	`+`
`73`	`77`	`/*`
`74`	`78`	`* (non-Javadoc)`
`75`	`79`	`*`
`@@ -102,4 +106,9 @@ public String getSimpleName() {`
`102`	`106`	`return simpleName;`
`103`	`107`	`}`
`104`	`108`
	`109`	`+ @Override`
	`110`	`+ public String toString() {`
	`111`	`+ return this.simpleName + " " + properties;`
	`112`	`+ }`
	`113`	`+`
`105`	`114`	`}`