Implement dict entry (#3)

* Move packages around to be able to execute single tests + fix signature parsing for nested objects

* move kotlin test file to correct package + write more tests for the signature parser

* Add dict_entry signature parsing and validation

* Refactor whole java serialization process (+13% performance)

* Refactor serialization test (WIP) + fix primitive array support + add test for them

* Fix class cache

* Finish test refactor + optimize imports

* Implement dict_entry serialization + small JNI code refactor/optimization

* Fix nested array serialization in JNI code

* Set dict_entry signature to NULL(refer to dbus doc for mroe info)

* Comment code + small JNI refactor + fix some bug about complex arrays

* Get ready for release

* Fix merge conflicts

Author: SeekDaSky

README.md

@@ -95,7 +95,7 @@ public class SubObject extends Message{
 
 ### Arrays
 
-DBus array type is mapped to the `List` collection. A List can contain anything serializable, including other lists. As JNIDBus uses reflection to know the type of the List items, the generic type must always be explicitly used in the getters and setters
+The DBus array type is mapped to either the `List` class or to native arrays . A List/array can contain anything serializable, including other lists/arrays. As JNIDBus uses reflection to know the type of the List items, the generic type must always be explicitly used in the getters and setters
 
 *<u>example for a nested list message:</u>*
 
@@ -116,6 +116,32 @@ public class CollectionOfCollectionArray extends Message {
 }
 ```
 
+### Dictionaries (`Maps`)
+
+The DBus arrays of `dict_entries` are mapped to the `Map` class, the key must be a primitive DBus type (refer to the DBus documentation for a definition of primitive) and its value can contain nested objects, arrays or maps.  As for the `List`, the generic types must be explicitly used in the getters/setters.
+
+*<u>example for a map message:</u>*
+
+```java
+@DBusType(
+    /* This signature correspond to an array containing dict_entries of a string and an        * integer. In the java world, it's a Map with a string as key and integers as values
+     */
+    signature = "a{si}",
+    fields = "map"
+)
+public class MapMessage extends Message {
+    private Map<String,Integer> map;
+
+    //always explicitly give the precise generic type
+    public Map<String,Integer> getMap() ...
+        
+	//for setters too
+    public void setMap(Map<String,Integer> map) ...
+}
+```
+
+
+
 ### The EmptyMessage
 
 if your message does not contain any data, the `Message` class contains a static `EMPTY` property which contains an empty Message that can be used. Using this object allows some internal optimizations and it is recommended to use it whenever you can.
@@ -344,19 +370,22 @@ suspend fun suspendingDBusCall() : SingleStringMessage{
 
 
 
-## Planned features
+## Planned tasks
 
-- Support `DICT_ENTRY` type
+- Support `OBJECT_PATH` type
+- Refactor `serialization.cpp` to use proper OOP and cleanup the code
+- Add the capability to create downcasted typed array in JNI code instead of plain `Object[]` objects
+- Use direct `ByteBuffer` instead of plain `Object` arrays to avoid copies.
 
 ## FAQ
 
 ##### Which java versions are compatible?
 
-Java 7 and Java 11 where tested. You will also need `libc` and `libdbus-1` to run the native code 
+Java 8 to Java 11 are tested by the CI, Java 7 should run but is not tested. You will also need `libc` and `libdbus-1` to run the native code 
 
 ##### How fast is this library
 
-I was able to get around 35k complex signals (nested lists and objects) sent and received on a single event loop on my modest i3-4130 work machine. I was able to get around 58k empty signals with the same setup. This should satisfy most of the use cases so unless you really need to push DBus to its limit it's enough.
+I was able to get around 40k/s "complex" signals sent and received on a single event loop on my modest i3-4130 work machine. I was able to get around 95k/s empty signals with the same setup. This should satisfy most of the use cases so unless you really need to push DBus to its limit it's enough.
 
 ##### I found a bug
 

build.gradle

@@ -33,10 +33,14 @@ allprojects{
         //add JNIDBus JNI library to path
         jvmArgs "-Djava.library.path=${root}/src/main/resources"
 
+        //set JNI in berserk mode
+        //jvmArgs "-Xcheck:jni"
+
         //show only failed test and their full exception
         testLogging {
             events "failed"
             exceptionFormat "full"
+            //showStandardStreams = true
         }
 
         //always execute tests

gradle.properties

@@ -1,4 +1,4 @@
-jnidbus_version 0.3.0
+jnidbus_version 0.4.0
 
 java_version 1.7
 kotlin_version 1.3.50

jnidbus-kotlin/src/main/kotlin/fr/viveris/jnidbus/dispatching/KotlinGenericHandler.kt

@@ -3,7 +3,7 @@ package fr.viveris.jnidbus.dispatching
 import fr.viveris.jnidbus.message.Promise
 import fr.viveris.jnidbus.serialization.DBusType
 import fr.viveris.jnidbus.serialization.Serializable
-import java.util.HashMap
+import java.util.*
 import kotlin.reflect.KClass
 import kotlin.reflect.full.createType
 import kotlin.reflect.full.declaredMemberFunctions

jnidbus-kotlin/src/test/kotlin/fr/viveris/jnidbus/test/PendingCallExtensionTest.kt

@@ -1,7 +1,5 @@
 package fr.viveris.jnidbus.test
 
-import fr.viveris.jnidbus.test.common.DBusObjects.SingleStringMessage
-import fr.viveris.jnidbus.test.common.DBusTestCase
 import fr.viveris.jnidbus.await
 import fr.viveris.jnidbus.dispatching.GenericHandler
 import fr.viveris.jnidbus.dispatching.MemberType
@@ -13,6 +11,8 @@ import fr.viveris.jnidbus.message.PendingCall
 import fr.viveris.jnidbus.message.Promise
 import fr.viveris.jnidbus.remote.RemoteInterface
 import fr.viveris.jnidbus.remote.RemoteMember
+import fr.viveris.jnidbus.test.common.DBusObjects.SingleStringMessage
+import fr.viveris.jnidbus.test.common.DBusTestCase
 import kotlinx.coroutines.*
 import kotlin.test.Test
 import kotlin.test.assertEquals

jnidbus-kotlin/src/test/kotlin/fr/viveris/jnidbus/test/SuspendingHandlerTest.kt

@@ -1,7 +1,5 @@
 package fr.viveris.jnidbus.test
 
-import fr.viveris.jnidbus.test.common.DBusObjects.SingleStringMessage
-import fr.viveris.jnidbus.test.common.DBusTestCase
 import fr.viveris.jnidbus.await
 import fr.viveris.jnidbus.dispatching.KotlinGenericHandler
 import fr.viveris.jnidbus.dispatching.KotlinMethodInvocator
@@ -12,7 +10,11 @@ import fr.viveris.jnidbus.message.Message
 import fr.viveris.jnidbus.message.PendingCall
 import fr.viveris.jnidbus.remote.RemoteInterface
 import fr.viveris.jnidbus.remote.RemoteMember
-import kotlinx.coroutines.*
+import fr.viveris.jnidbus.test.common.DBusObjects.SingleStringMessage
+import fr.viveris.jnidbus.test.common.DBusTestCase
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.withTimeout
 import kotlin.test.BeforeTest
 import kotlin.test.Test
 import kotlin.test.assertEquals

src/jmh/java/fr/viveris/SameThreadSignals.java

@@ -3,9 +3,6 @@
  */
 package fr.viveris;
 
-import fr.viveris.jnidbus.test.common.DBusObjects.ArrayRecursiveObject;
-import fr.viveris.jnidbus.test.common.DBusObjects.SimpleMessage;
-import fr.viveris.jnidbus.test.common.DBusTestCase;
 import fr.viveris.jnidbus.BusType;
 import fr.viveris.jnidbus.Dbus;
 import fr.viveris.jnidbus.bindings.bus.EventLoop;
@@ -18,6 +15,9 @@
 import fr.viveris.jnidbus.remote.RemoteInterface;
 import fr.viveris.jnidbus.remote.RemoteMember;
 import fr.viveris.jnidbus.remote.Signal;
+import fr.viveris.jnidbus.test.common.DBusObjects.ArrayRecursiveObject;
+import fr.viveris.jnidbus.test.common.DBusObjects.SimpleMessage;
+import fr.viveris.jnidbus.test.common.DBusTestCase;
 import org.openjdk.jmh.annotations.*;
 
 import java.util.concurrent.CountDownLatch;

src/main/java/fr/viveris/jnidbus/Dbus.java

@@ -88,7 +88,7 @@ public Dbus(BusType type, String busName) throws ConnectionException {
     public void addHandler(GenericHandler handler){
         LOG.info("Adding DBus handler {}",handler.getClass().getName());
         //get annotation
-        Handler handlerAnnotation = handler.getClass().getAnnotation(Handler.class);
+        Handler handlerAnnotation = handler.getHandlerAnnotation();
         if(handlerAnnotation == null) throw new IllegalStateException("The given handler does not have the Handler annotation");
 
         //get all criteria provided by this handler
@@ -124,7 +124,7 @@ public void addHandler(GenericHandler handler){
 
     public void removeHandler(GenericHandler handler){
         //get annotation
-        Handler handlerAnnotation = handler.getClass().getAnnotation(Handler.class);
+        Handler handlerAnnotation = handler.getHandlerAnnotation();
         if(handlerAnnotation == null) throw new IllegalStateException("The given handler does not have the Handler annotation");
 
         Dispatcher dispatcher = this.dispatchers.get(handlerAnnotation.path());
@@ -170,7 +170,7 @@ public <T> T createRemoteObject(String destinationBus, String objectPath, Class<
      * @param signal
      */
     public void sendSignal(String objectPath, Signal signal){
-        SignalMetadata meta = RemoteObjectInterceptor.getFromCache(signal.getClass());
+        SignalMetadata meta = RemoteObjectInterceptor.getFromCache(signal);
         this.eventLoop.send(new SignalSendingRequest(signal.getParam().serialize(),objectPath,meta.getInterfaceName(),meta.getMember()));
     }
 

src/main/java/fr/viveris/jnidbus/cache/MessageMetadata.java

@@ -4,23 +4,21 @@
 package fr.viveris.jnidbus.cache;
 
 import fr.viveris.jnidbus.exception.MessageCheckException;
-import fr.viveris.jnidbus.message.Message;
 import fr.viveris.jnidbus.serialization.DBusType;
 import fr.viveris.jnidbus.serialization.Serializable;
+import fr.viveris.jnidbus.serialization.serializers.*;
 import fr.viveris.jnidbus.serialization.signature.Signature;
 import fr.viveris.jnidbus.serialization.signature.SignatureElement;
 import fr.viveris.jnidbus.serialization.signature.SupportedTypes;
 
 import java.lang.reflect.*;
 import java.util.HashMap;
-import java.util.List;
 
 /**
- * A MessageMetadata contains all the data available through reflection that are used for the serialization and unserialization
- * process. This optimizes the process and allows the library to do a lot of checks on the Messages classes without any
- * cost at runtime.
- *
- * The class caches: getters, setters, annotation data and constructor instance for faster instantiation
+ * A MessageMetadata contains the list of fields managed by JNIDBus, their setters and getters. In order lower the cost of
+ * the heavy use of Reflection, the fields are mapped to a pair of Serializer/deserializer which will analyze the signature
+ * and type of the field, save those information and reuse them when doing the actual serialization. This mechanism speeds
+ * up a lot the serialization process.
  */
 public class MessageMetadata {
     /**
@@ -44,6 +42,11 @@ public class MessageMetadata {
     private HashMap<String, Method> setters = new HashMap<>();
     private HashMap<String, Method> getters = new HashMap<>();
 
+    /**
+     * Serializer and deserializers by Methods
+     */
+    private HashMap<String, Serializer> fieldSerializers = new HashMap<>();
+
     /**
      * Message class object
      */
@@ -62,14 +65,14 @@ public MessageMetadata(Class<? extends Serializable> clazz) throws MessageCheckE
 
         //check annotation
         DBusType type = clazz.getAnnotation(DBusType.class);
-        if(type == null) throw new IllegalStateException("No DBusType annotation found");
+        if(type == null) throw new MessageCheckException("no DBusType annotation found",this.clazz);
 
         //check constructor
         Constructor<? extends Serializable> constructor;
         try {
             constructor = clazz.getConstructor();
         } catch (NoSuchMethodException e) {
-            throw new IllegalStateException("No public empty constructor found");
+            throw new MessageCheckException("No public empty constructor found",this.clazz);
         }
 
         //create a new cache entity
@@ -89,34 +92,52 @@ public MessageMetadata(Class<? extends Serializable> clazz) throws MessageCheckE
             try{
                 field = clazz.getDeclaredField(fieldName);
             }catch (Exception e){
-                throw new MessageCheckException("Could not find the field "+fieldName);
+                throw new MessageCheckException("Could not find field",this.clazz,fieldName);
             }
 
-            //check the field against the annotation signature
-            MessageMetadata.checkField(field,field.getGenericType(),element);
+            Type fieldType = field.getGenericType();
 
             try {
+                //get the getter and create its serializer, the serializer will check signature and type validity
                 Method getter = clazz.getDeclaredMethod(getterName);
                 this.getters.put(fieldName,getter);
+                if(!fieldType.equals(getter.getGenericReturnType())){
+                    throw new MessageCheckException("The getter return type is not the same as the field",this.clazz,fieldName);
+                }
             } catch (NoSuchMethodException e) {
-                throw new MessageCheckException("Could not find the getter for the field "+fieldName);
+                throw new MessageCheckException("Could not find getter",this.clazz,fieldName);
             }
 
             try {
+                //get the setter and create its deserializer, the deserializer will check signature and type validity
                 Method setter = clazz.getDeclaredMethod(setterName,field.getType());
                 this.setters.put(fieldName,setter);
+                if(setter.getGenericParameterTypes().length != 1){
+                    throw new MessageCheckException("The setter must only have one parameter",this.clazz,fieldName);
+                }
+                if(!fieldType.equals(setter.getGenericParameterTypes()[0])){
+                    throw new MessageCheckException("The setter return type is not the same as the field",this.clazz,fieldName);
+                }
             } catch (NoSuchMethodException e) {
-                throw new MessageCheckException("Could not find the setter for the field "+fieldName);
+                throw new MessageCheckException("Could not find setter",this.clazz,fieldName);
             }
 
+            this.fieldSerializers.put(fieldName,this.generateSerializerForSignature(fieldType,element,fieldName));
             i++;
         }
+
+        if(i != this.fields.length){
+            throw new MessageCheckException("Incomplete signature, there is too much fields",this.clazz);
+        }
     }
 
     public Method getGetter(String fieldName){
         return this.getters.get(fieldName);
     }
 
+    public Serializer getFieldSerializer(String fieldName){
+        return this.fieldSerializers.get(fieldName);
+    }
     public Method getSetter(String fieldName){
         return this.setters.get(fieldName);
     }
@@ -132,7 +153,7 @@ public String[] getFields() {
     public Class<? extends Serializable> getMessageClass(){ return this.clazz; }
 
     /**
-     * Create a new empty instances of the Message object (for unserialization).
+     * Create a new empty instances of the Message object (for deserialization).
      *
      * @return new empty instance of the message
      */
@@ -145,42 +166,52 @@ public Serializable newInstance(){
     }
 
     /**
-     * Check if the given field is valid according to the given signature element
-     *
-     * @param field field to check
-     * @param element signature element used to check
+     * This method will try to find a serializer matching a generic type and a signature, the created serializer will
+     * perform all the necessary checks and try to find incoherence.
+     * @param genericType the Type of the method, could be a parametrized type or standard one
+     * @param element
+     * @return
      * @throws MessageCheckException
      */
-    private static void checkField(Field field, Type fieldType, SignatureElement element) throws MessageCheckException{
-        //get primitive will catch primitive fields and primitive arrays
-        if(element.getPrimitive() != null){
-            boolean isList = fieldType instanceof ParameterizedType;
-            //check the generic is a List and if it matched the signature
-            if(isList && (!List.class.isAssignableFrom((Class)((ParameterizedType)fieldType).getRawType()) || element.getContainerType() != SupportedTypes.ARRAY))
-                throw new MessageCheckException("the field "+field.getName()+" is not of a List or the signature is not expecting a list");
-
-            //check if the generic type contains the correct type
-            SupportedTypes type = element.getPrimitive();
-            if(isList){
-                ParameterizedType paramType = (ParameterizedType) fieldType;
-                if(!paramType.getActualTypeArguments()[0].equals(type.getBoxedType()) && !paramType.getActualTypeArguments()[0].equals(type.getPrimitiveType()))
-                    throw new MessageCheckException("the field "+field.getName()+" is not of a List of Integer");
-            }else {
-                if (!fieldType.equals(type.getBoxedType()) && !fieldType.equals(type.getPrimitiveType())) throw new MessageCheckException("the field " + field.getName() + " is not of type "+type);
-            }
-        }else if(element.isArray()){
-            //recursively check the content of the nested list
-            MessageMetadata.checkField(field,((ParameterizedType)fieldType).getActualTypeArguments()[0],element.getSignature().getFirst());
-
-        }else if(element.isObject()){
-            if(!Serializable.class.isAssignableFrom((Class)fieldType)) throw new MessageCheckException("the field "+field.getName()+" does not contain a serializable type");
+    private Serializer generateSerializerForSignature(Type genericType, SignatureElement element, String fieldName) throws MessageCheckException {
+        //TypeVariables are not usable and should not be allowed
+        if(genericType instanceof TypeVariable) throw new MessageCheckException("Unspecified generic type found",this.clazz,fieldName);
+
+        //if the method returns a generic type, extract it, this allows us to support generic messages and nested lists
+        Class<?> clazz;
+        if(genericType instanceof ParameterizedType){
+            clazz = (Class<?>) ((ParameterizedType)genericType).getRawType();
+        }else{
+            clazz = (Class<?>) genericType;
+        }
 
-            Class<? extends Serializable> clazz = ((Class)fieldType).asSubclass(Serializable.class);
-            MessageMetadata testedEntity = new MessageMetadata(clazz);
-            Message.addToCache(clazz,testedEntity);
+        //if we have an array container type and a primitive type, it means we have a primitive array
+        if(element.getContainerType() == SupportedTypes.ARRAY && element.getPrimitive() != null){
+            return new PrimitiveArraySerializer(clazz,element,this.clazz,fieldName);
+
+        // if there is a primitive type but no container, we have a primitive type
+        }else if(element.getContainerType() == null && element.getPrimitive() != null){
+            return new PrimitiveSerializer(clazz,element,this.clazz,fieldName);
+
+        //as we checked for primitive arrays before, if there is an array container type it means we have a complex array or a map
+        }else if(element.getContainerType() == SupportedTypes.ARRAY){
+            //check first element type to se if we have a Map
+            if(element.getSignature().getFirst().getContainerType() == SupportedTypes.DICT_ENTRY_BEGIN){
+                return new MapSerializer(genericType,element,this.clazz,fieldName);
+            }else{
+                return new ComplexArraySerializer(genericType,element,this.clazz,fieldName);
+            }
 
-            if(!testedEntity.getSignature().equals(element.getSignatureString()))
-                throw new MessageCheckException("the field "+field.getName()+" signature do not match its type signature");
+        //if there is no primitive type and an object container type, we have an object
+        }else if(element.getContainerType() == SupportedTypes.OBJECT_BEGIN) {
+            if (!Serializable.class.isAssignableFrom(clazz)) {
+                throw new MessageCheckException("Field not Serializable", this.clazz, fieldName);
+            }
+            return new ObjectSerializer(clazz.asSubclass(Serializable.class), element, this.clazz, fieldName);
+        }else if( element.getContainerType() == SupportedTypes.DICT_ENTRY_BEGIN){
+            throw new MessageCheckException("A dict_entry can not be placed outside of an array", this.clazz, fieldName);
+        }else{
+            throw new MessageCheckException("The given type could not be mapped to any serializer",this.clazz,fieldName);
         }
     }
 }