Fixing several binding-related bugs

* @Optional annotation added duplicate annotations to BindingRequest for
certain classes
* @Identifier annotation was not actually removed from the
BindingRequest
* BindingRequest hashCode calculation used annotation args excluding the
ones possibly found from the clazz.getAnnotations()
* Prevent replacing existing bindings in BindingRepository.classMap

gitlab #313

Change-Id: I774649584c288c197f1f8ca6af78682b296b63d4

diff --git a/bundles/org.simantics.databoard/src/org/simantics/databoard/Bindings.java b/bundles/org.simantics.databoard/src/org/simantics/databoard/Bindings.java
index f9cf1002317a7c4ddd2285e8a6306bd17e3812a5..b1f53d0b611bb542197cdf7bb299b60a3438149b 100644 (file)
--- a/bundles/org.simantics.databoard/src/org/simantics/databoard/Bindings.java
+++ b/bundles/org.simantics.databoard/src/org/simantics/databoard/Bindings.java
@@ -8,6 +8,7 @@
  *
  *  Contributors:
  *      VTT Technical Research Centre of Finland - initial API and implementation
  *
  *  Contributors:
  *      VTT Technical Research Centre of Finland - initial API and implementation

+ *      Semantum Oy - gitlab #313

  *******************************************************************************/
 package org.simantics.databoard;
 
  *******************************************************************************/
 package org.simantics.databoard;
 


@@ -886,7 +887,14 @@ public class Bindings {
        
        databoard.initialize();
        
        
        databoard.initialize();
        

-               DATATYPE = getBindingUnchecked(Datatype.class);
+        DATATYPE = getBindingUnchecked(Datatype.class);
+        /**
+         * {@link Datatype} class has annotations but it can be considered a "class
+         * request" as it is a fundamental building block of Databoard and it has a
+         * fixed structure. Therefore {@link BindingRepository#classMap} is allowed
+         * to contain a cached Datatype.class -> Binding mapping.
+         */
+        bindingRepository.registerClassMapping(Datatype.class, DATATYPE);

     }
 
 }
     }
 
 }

diff --git a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/factory/BindingRepository.java b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/factory/BindingRepository.java
index ba24353712c7f90f415b4b2652f7699dba5b27d0..5a0239f26199a6a54174857298ffc4b0581b4422 100644 (file)
--- a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/factory/BindingRepository.java
+++ b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/factory/BindingRepository.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
 /*******************************************************************************

- * Copyright (c) 2007, 2018 Association for Decentralized Information Management in
+ * Copyright (c) 2007, 2019 Association for Decentralized Information Management in

  * Industry THTH ry.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * Industry THTH ry.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0


@@ -8,7 +8,7 @@
  *
  * Contributors:
  *     VTT Technical Research Centre of Finland - initial API and implementation
  *
  * Contributors:
  *     VTT Technical Research Centre of Finland - initial API and implementation

- *     Semantum Oy - gitlab #82
+ *     Semantum Oy - gitlab #82, gitlab #313

  *******************************************************************************/
 package org.simantics.databoard.binding.factory;
 
  *******************************************************************************/
 package org.simantics.databoard.binding.factory;
 


@@ -18,10 +18,12 @@ import java.util.Map.Entry;
 
 import org.simantics.databoard.binding.Binding;
 import org.simantics.databoard.binding.reflection.BindingRequest;
 
 import org.simantics.databoard.binding.Binding;
 import org.simantics.databoard.binding.reflection.BindingRequest;

-import org.simantics.databoard.type.Datatype;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;

 
 public class BindingRepository {
 
 public class BindingRepository {

-       
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(BindingRepository.class);

        /**
         * This map contains all the bindings
         */
        /**
         * This map contains all the bindings
         */


@@ -41,7 +43,7 @@ public class BindingRepository {
                this.requestMap = requests;
                for (Entry<BindingRequest, Binding> e : requests.entrySet()) {
                        if ( isClassRequest( e.getKey() ) ) {
                this.requestMap = requests;
                for (Entry<BindingRequest, Binding> e : requests.entrySet()) {
                        if ( isClassRequest( e.getKey() ) ) {

-                               classMap.put(e.getKey().getClazz(), e.getValue());
+                               registerClassMapping(e.getKey().getClazz(), e.getValue());

                        }
                }
        }
                        }
                }
        }


@@ -76,11 +78,15 @@ public class BindingRepository {
         */
        public synchronized void put( BindingRequest request, Binding binding ) {
                if ( isClassRequest(request) ) {
         */
        public synchronized void put( BindingRequest request, Binding binding ) {
                if ( isClassRequest(request) ) {

-                       classMap.put( request.getClazz(), binding );
+                   registerClassMapping(request.getClazz(), binding);
+               }
+               Binding existing = requestMap.put( request, binding );
+               if (existing != null && !existing.equals(binding)) {
+                   LOGGER.error("Replacing existing binding with a different one! {} {} {}", request, binding, existing);

                }
                }

-               requestMap.put( request, binding );

        }
        }

-       
+
+       @SuppressWarnings("unlikely-arg-type")

        public synchronized void remove(Binding binding) {
                for (Entry<BindingRequest, Binding> e : requestMap.entrySet()) {
                        if (e.getValue() == binding) {
        public synchronized void remove(Binding binding) {
                for (Entry<BindingRequest, Binding> e : requestMap.entrySet()) {
                        if (e.getValue() == binding) {


@@ -116,20 +122,15 @@ public class BindingRepository {
                classMap.clear();
        }
 
                classMap.clear();
        }
 

-       /**
-        * {@link Datatype} class has annotations but it can be considered a "class
-        * request" as it is a fundamental building block of Databoard and it has a
-        * fixed structure. Therefore {@link #classMap} is allowed to contain a cached
-        * Datatype.class -> Binding mapping.
-        */
-       private static final String DATATYPE_CLASS_NAME = Datatype.class.getName();
+       boolean isClassRequest(BindingRequest request) {
+               return (request.className != null && (request.annotations == null || request.annotations.length == 0));
+       }

 
 

-       boolean isClassRequest( BindingRequest request )
-       {
-               return (request.className != null
-                               && ((request.annotations==null || request.annotations.length==0)
-                                               || DATATYPE_CLASS_NAME.equals(request.className))
-                               );
+       public void registerClassMapping(Class<?> clazz, Binding binding) {
+               Binding previous = classMap.putIfAbsent(clazz, binding);
+               if (previous != null) {
+                       LOGGER.warn("WARN: Can not put same key again to classMap! {} mapping {} not replaced by {}", clazz, previous, binding);
+               }

        }
 
 }
        }
 
 }

diff --git a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/BindingRequest.java b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/BindingRequest.java
index 51feb39c3815861e4edde721cb4970ddf33898c0..ac2d9cf1b7246f6be353c8a15aea887ce3b8d45b 100644 (file)
--- a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/BindingRequest.java
+++ b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/BindingRequest.java
@@ -8,7 +8,7 @@
  *
  * Contributors:
  *     VTT Technical Research Centre of Finland - initial API and implementation
  *
  * Contributors:
  *     VTT Technical Research Centre of Finland - initial API and implementation

- *     Semantum Oy - gitlab #82
+ *     Semantum Oy - gitlab #82, gitlab #313

  *******************************************************************************/
 package org.simantics.databoard.binding.reflection;
 
  *******************************************************************************/
 package org.simantics.databoard.binding.reflection;
 


@@ -60,6 +60,23 @@ public class BindingRequest {
     
     transient int hash;
 
     
     transient int hash;
 

+    /**
+     * Cloning constructor with replacement annotations.
+     * 
+     * @param other the request to clone
+     * @param annotations the annotations to use while cloning
+     */
+    private BindingRequest(BindingRequest other, Annotation...annotations)
+    {
+        this.clazz = other.clazz;
+        this.cl = other.cl;
+        this.annotations = annotations;
+        this.className = other.className;
+        this.signature = other.signature;
+        this.descriptor = other.descriptor;
+        hash = calcHash(clazz.getName());
+    }
+

     /**
      * Create BindingRequest that creates class lazily. 
      * 
     /**
      * Create BindingRequest that creates class lazily. 
      * 


@@ -76,10 +93,7 @@ public class BindingRequest {
        this.signature = classSignature;
        this.annotations = annotations;
        this.descriptor = classDescriptor;
        this.signature = classSignature;
        this.annotations = annotations;
        this.descriptor = classDescriptor;

-        hash = className.hashCode();
-        for (Annotation a : annotations) {
-            hash = 7*hash + a.hashCode();
-        }
+       hash = calcHash(className);

     }
 
     /**
     }
 
     /**


@@ -114,17 +128,23 @@ public class BindingRequest {
         
         className = clazz.getCanonicalName();
         signature = getSignature(clazz);
         
         className = clazz.getCanonicalName();
         signature = getSignature(clazz);

-        List<Class<?>> args = createArgsList();
-        StringBuilder desc = new StringBuilder();
-        _buildDescriptor(desc, clazz, args, new MutableInteger(0));
-        descriptor = desc.toString();
-        hash = clazz.getName().hashCode();
-        for (Annotation a : annotations) {
-            hash = 7*hash + a.hashCode();
+        descriptor = _buildDescriptor(new StringBuilder(), clazz, createArgsList(), new MutableInteger(0)).toString();
+        hash = calcHash(clazz.getName());
+    }
+
+    public BindingRequest withAnnotations(Annotation... newAnnotations) {
+        return new BindingRequest(this, newAnnotations);
+    }
+
+    private int calcHash(String className) {
+        int hash = className.hashCode();
+        for (Annotation a : this.annotations) {
+            hash += a.hashCode();

         }
         }

+        return hash;

     }
     }

-    
-    private void _buildDescriptor(StringBuilder sb, Class<?> c, List<Class<?>> classes, MutableInteger pos)
+
+    private StringBuilder _buildDescriptor(StringBuilder sb, Class<?> c, List<Class<?>> classes, MutableInteger pos)

     {
        int genericCount = c.getTypeParameters().length;
        int genericsLeft = classes.size()-pos.value;
     {
        int genericCount = c.getTypeParameters().length;
        int genericsLeft = classes.size()-pos.value;


@@ -142,6 +162,7 @@ public class BindingRequest {
        } else {
                sb.append( getSignature(c) );
        }
        } else {
                sb.append( getSignature(c) );
        }

+       return sb;

     }
 
     public BindingRequest(Class<?> clazz, List<Annotation> annotations)
     }
 
     public BindingRequest(Class<?> clazz, List<Annotation> annotations)

diff --git a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/ClassBindingFactory.java b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/ClassBindingFactory.java
index 8d29ad70c24ac4c8896293b22ac82817b1c617f3..1295c1099ac0c8731b3be597dd967370e0b52afc 100644 (file)
--- a/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/ClassBindingFactory.java
+++ b/bundles/org.simantics.databoard/src/org/simantics/databoard/binding/reflection/ClassBindingFactory.java
@@ -1,3 +1,15 @@
+/*******************************************************************************
+ * Copyright (c) 2019 Association for Decentralized Information Management
+ * in Industry THTH ry.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     VTT Technical Research Centre of Finland - initial API and implementation
+ *     Semantum Oy - gitlab #313
+ *******************************************************************************/

 package org.simantics.databoard.binding.reflection;
 
 import java.lang.annotation.Annotation;
 package org.simantics.databoard.binding.reflection;
 
 import java.lang.annotation.Annotation;


@@ -200,8 +212,7 @@ public class ClassBindingFactory {
        if(request.hasAnnotation(Optional.class))
        {               
                Optional optional = request.getAnnotation(Optional.class); 
        if(request.hasAnnotation(Optional.class))
        {               
                Optional optional = request.getAnnotation(Optional.class); 

-               Annotation[] newAnnotations = ArrayUtils.dropElements(request.annotations, optional);
-               BindingRequest componentRequest = new BindingRequest(request.getClazz(), newAnnotations);
+               BindingRequest componentRequest = request.withAnnotations(ArrayUtils.dropElements(request.annotations, optional));

                OptionalType type = new OptionalType();
                OptionalBinding binding = new OptionalBindingDefault(type, null);
                        inprogress.put(request, binding);
                OptionalType type = new OptionalType();
                OptionalBinding binding = new OptionalBindingDefault(type, null);
                        inprogress.put(request, binding);


@@ -209,6 +220,7 @@ public class ClassBindingFactory {
                        type.componentType = binding.componentBinding.type();
                        inprogress.remove(request);
                        
                        type.componentType = binding.componentBinding.type();
                        inprogress.remove(request);
                        

+                       repository.put(request, binding);

                        return binding;
        }
 
                        return binding;
        }
 


@@ -570,7 +582,7 @@ public class ClassBindingFactory {
                
                Identifier idAnnotation = componentRequest.getAnnotation( Identifier.class );
                if ( idAnnotation!=null ) {
                
                Identifier idAnnotation = componentRequest.getAnnotation( Identifier.class );
                if ( idAnnotation!=null ) {

-                       componentRequest.dropAnnotations(1, idAnnotation);
+                       componentRequest = componentRequest.withAnnotations(componentRequest.dropAnnotations(1, idAnnotation));

                        identifierIndices.add( i );
                }
                Binding componentBinding = componentBindings[i] = construct( componentRequest );
                        identifierIndices.add( i );
                }
                Binding componentBinding = componentBindings[i] = construct( componentRequest );


@@ -602,8 +614,6 @@ public class ClassBindingFactory {
                                        binding = defaultBinding;
                        }
                        
                                        binding = defaultBinding;
                        }
                        

-                       repository.put(request, binding);
-                       

                        return binding;
                } catch (RangeException e) {
                        inprogress.remove( request );
                        return binding;
                } catch (RangeException e) {
                        inprogress.remove( request );