You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by cl...@apache.org on 2013/09/16 11:53:00 UTC

svn commit: r1523571 - in /felix/trunk/ipojo/runtime/core/src: main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java

Author: clement
Date: Mon Sep 16 09:53:00 2013
New Revision: 1523571

URL: http://svn.apache.org/r1523571
Log:
FELIX-4228 Improve dependency identification in log messages and exceptions

Define a standard format to identify service dependencies within messages.

Added:
    felix/trunk/ipojo/runtime/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java
Modified:
    felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java

Modified: felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java?rev=1523571&r1=1523570&r2=1523571&view=diff
==============================================================================
--- felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java (original)
+++ felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/DependencyHandler.java Mon Sep 16 09:53:00 2013
@@ -186,19 +186,20 @@ public class DependencyHandler extends P
         int index = dep.getConstructorParameterIndex();
 
         if (callbacks == null && field == null  && index == -1) {
-            throw new ConfigurationException("A service requirement requires at least binding methods, " +
-            		"a field or a constructor parameter");
+            throw new ConfigurationException("A service dependency requires at least binding methods, " +
+            		"a field or a constructor parameter " + getDependencyIdentifier(dep));
         }
 
         for (int i = 0; callbacks != null && i < callbacks.length; i++) {
             MethodMetadata[] mets = manipulation.getMethods(callbacks[i].getMethodName());
             if (mets.length == 0) {
-                debug("A requirement callback " + callbacks[i].getMethodName() + " does not exist in the implementation class, will try the super classes");
+                debug("A dependency callback " + callbacks[i].getMethodName() + " does not exist in the " +
+                        "implementation class, will try the super classes " + getDependencyIdentifier(dep));
             } else {
                 if (mets[0].getMethodArguments().length > 2) {
                     throw new ConfigurationException("Requirement Callback : A requirement callback "
                             + callbacks[i].getMethodName()
-                            + " must have 0, 1 or 2 arguments");
+                            + " must have 0, 1 or 2 arguments " + getDependencyIdentifier(dep));
                 }
 
                 callbacks[i].setArgument(mets[0].getMethodArguments());
@@ -214,7 +215,9 @@ public class DependencyHandler extends P
                            || mets[0].getMethodArguments()[1].equals(Dictionary.class.getName()) // callback with (service object, service properties in a dictionary)
                            || mets[0].getMethodArguments()[1].equals(Map.class.getName()))) { // callback with (service object, service properties in a map)
                         String message =
-                                "The requirement callback " + callbacks[i].getMethodName() + " must have a ServiceReference, a Dictionary or a Map as the second argument";
+                                "The requirement callback " + callbacks[i].getMethodName() + " must have a " +
+                                        "ServiceReference, a Dictionary or a Map as the second argument " +
+                                        getDependencyIdentifier(dep);
                         throw new ConfigurationException(message);
                     }
                     setSpecification(dep, mets[0].getMethodArguments()[0], false); // Just warn if a mismatch is discovered.
@@ -228,12 +231,13 @@ public class DependencyHandler extends P
             if (meta == null) {
                 throw new ConfigurationException("Requirement Callback : A requirement field "
                         + field
-                        + " does not exist in the implementation class");
+                        + " does not exist in the implementation class " + getDependencyIdentifier(dep));
             }
             String type = meta.getFieldType();
             if (type.endsWith("[]")) {
                 if (dep.isProxy()) {
-                    info("Arrays cannot be used for proxied dependencies - Disabling the proxy mode");
+                    info("Arrays cannot be used for dependencies using proxies - Disabling the proxy mode " +
+                            getDependencyIdentifier(dep));
                     dep.setProxy(false);
                 }
                 // Set the dependency to multiple
@@ -245,7 +249,8 @@ public class DependencyHandler extends P
             } else if (type.equals(Vector.class.getName())) {
                 dep.setType(VECTOR);
                 if (dep.isProxy()) {
-                    warn("Vectors cannot be used for proxied dependencies - Disabling the proxy mode");
+                    warn("Vectors cannot be used for dependencies using proxies - Disabling the proxy mode " +
+                            getDependencyIdentifier(dep));
                     dep.setProxy(false);
                 }
                 type = null;
@@ -256,7 +261,8 @@ public class DependencyHandler extends P
                 if (dep.isAggregate()) {
                     throw new ConfigurationException("A required service is not correct : the field "
                             + meta.getFieldName()
-                            + " must be an array to support aggregate injections");
+                            + " must be an array or a collection to support aggregate injections " +
+                            getDependencyIdentifier(dep));
                 }
             }
             setSpecification(dep, type, true); // Throws an exception if the field type mismatch.
@@ -265,7 +271,8 @@ public class DependencyHandler extends P
         // Constructor parameter
         if (index != -1) {
         	if (! dep.isProxy()) {
-        		throw new ConfigurationException("Services injected into constructor must be proxied");
+        		throw new ConfigurationException("Services injected into constructor must use proxies " +
+                        getDependencyIdentifier(dep));
         	}
 
     		MethodMetadata[] cts = manipulation.getConstructors();
@@ -274,12 +281,14 @@ public class DependencyHandler extends P
     		if (cts.length > 0  && cts[0].getMethodArguments().length > index) {
         		String type = cts[0].getMethodArguments()[index];
         		if (type.endsWith("[]")) {
-                    throw new ConfigurationException("Services injected into constructor cannot be arrays");
+                    throw new ConfigurationException("Services injected into constructor cannot be arrays " +
+                            getDependencyIdentifier(dep));
                 } else if (type.equals(List.class.getName()) || type.equals(Collection.class.getName())) {
                     dep.setType(LIST);
                     type = null;
                 } else if (type.equals(Vector.class.getName())) {
-                	throw new ConfigurationException("Services injected into constructor cannot be Vectors");
+                	throw new ConfigurationException("Services injected into constructor cannot be Vectors " +
+                            getDependencyIdentifier(dep));
                 } else if (type.equals(Set.class.getName())) {
                     dep.setType(SET);
                     type = null;
@@ -287,34 +296,29 @@ public class DependencyHandler extends P
                     if (dep.isAggregate()) {
                         throw new ConfigurationException("A required service is not correct : the constructor parameter "
                                 + index
-                                + " must be an aggregate type to support aggregate injections");
+                                + " must be an aggregate type to support aggregate injections " +
+                                getDependencyIdentifier(dep));
                     }
                 }
                 setSpecification(dep, type, true); // Throws an exception if the field type mismatch.
         	} else {
         		throw new ConfigurationException("Cannot determine the specification of the dependency " + index +
-        				", please use the specification attribute");
+        				", please use the specification attribute " + getDependencyIdentifier(dep));
         	}
         }
 
         // At this point we must have discovered the specification, it it's null, throw a ConfiguraitonException
         if (dep.getSpecification() == null) {
-            String id = dep.getId();
-            if (id == null) {
-                dep.getField();
-            }
-            if (id == null  && dep.getCallbacks() != null  && dep.getCallbacks().length > 0) {
-                id = dep.getCallbacks()[0].getMethodName();
-            }
-            throw new ConfigurationException("Cannot determine the targeted service specification for the dependency '" +
-                    id + "'");
+            String identifier = getDependencyIdentifier(dep);
+            throw new ConfigurationException("Cannot determine the targeted service specification for the dependency " +
+                    identifier);
         }
 
         // Disable proxy on scalar dependency targeting non-interface specification
         if (! dep.isAggregate()  && dep.isProxy()) {
         	if (! dep.getSpecification().isInterface()) {
         		warn("Proxies cannot be used on service dependency targeting non interface " +
-        				"service specification " + dep.getSpecification().getName());
+        				"service specification " + getDependencyIdentifier(dep));
         		dep.setProxy(false);
         	}
         }
@@ -330,6 +334,40 @@ public class DependencyHandler extends P
     }
 
     /**
+     * Builds a description of this dependency to help the user to identify it. IT's not related to the Dependency
+     * Description, it's just a string containing dependency information to spot it easily in the code.
+     * @param dep the dependency
+     * @return the identifier containing (if defined) the id, the specification, the field and the callback.
+     * @since 1.10.1
+     */
+    public static String getDependencyIdentifier(Dependency dep) {
+        StringBuilder identifier = new StringBuilder("{");
+        if (dep.getId() != null) {
+            identifier.append("id=").append(dep.getId());
+        }
+        if (dep.getField() != null) {
+            if (identifier.length() > 1) {
+                identifier.append(", ");
+            }
+            identifier.append("field=").append(dep.getField());
+        }
+        if (dep.getCallbacks() != null  && dep.getCallbacks().length > 0) {
+            if (identifier.length() > 1) {
+                identifier.append(", ");
+            }
+            identifier.append("method=").append(dep.getCallbacks()[0].getMethodName());
+        }
+        if (dep.getSpecification() != null) {
+            if (identifier.length() > 1) {
+                identifier.append(", ");
+            }
+            identifier.append("specification=").append(dep.getSpecification().getName());
+        }
+        identifier.append("}");
+        return identifier.toString();
+    }
+
+    /**
      * Check if we have to set the dependency specification with the given class name.
      * @param dep : dependency to check
      * @param className : class name
@@ -348,7 +386,8 @@ public class DependencyHandler extends P
                 			id = Integer.toString(dep.getConstructorParameterIndex());
                 		}
                 	}
-                    throw new ConfigurationException("Cannot discover the required specification for " + id);
+                    throw new ConfigurationException("Cannot discover the required specification for " +
+                            getDependencyIdentifier(dep));
                 } else {
                     // If the specification is different, warn that we will override it.
                     info("Cannot discover the required specification for " + dep.getField());
@@ -362,7 +401,7 @@ public class DependencyHandler extends P
                             + className
                             + "] and the specified (or already discovered)  service interface ["
                             + dep.getSpecification().getName()
-                            + "] are not the same");
+                            + "] are not the same " + getDependencyIdentifier(dep));
                     } else {
                         // If the specification is different, warn that we will override it.
                         warn("["
@@ -371,7 +410,7 @@ public class DependencyHandler extends P
                             + className
                             + "] and the required service interface ["
                             + dep.getSpecification()
-                            + "] are not the same");
+                            + "] are not the same " + getDependencyIdentifier(dep));
                     }
                 }
 
@@ -380,7 +419,8 @@ public class DependencyHandler extends P
                     dep.setSpecification(bundle.loadClass(className));
                 } catch (ClassNotFoundException e) {
                     throw new ConfigurationException("The required service interface (" + className
-                            + ") cannot be loaded from bundle " + bundle.getBundleId(), e);
+                            + ") cannot be loaded from bundle " + bundle.getBundleId() + " " +
+                            getDependencyIdentifier(dep), e);
                 }
             }
         }

Added: felix/trunk/ipojo/runtime/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/runtime/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java?rev=1523571&view=auto
==============================================================================
--- felix/trunk/ipojo/runtime/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java (added)
+++ felix/trunk/ipojo/runtime/core/src/test/java/org/apache/felix/ipojo/handlers/dependency/DependencyIdentifierTest.java Mon Sep 16 09:53:00 2013
@@ -0,0 +1,158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.ipojo.handlers.dependency;
+
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import java.util.List;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+/**
+ * Checks the dependency identifier computation.
+ */
+public class DependencyIdentifierTest {
+
+    @Test
+    public void testDependencyWithAllInformation() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getId()).thenReturn("id1");
+        Mockito.when(dependency.getField()).thenReturn("fs");
+        DependencyCallback[] callbacks = new DependencyCallback[2];
+        callbacks[0] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[0].getMethodName()).thenReturn("bindFooService");
+        callbacks[1] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[1].getMethodName()).thenReturn("unbindFooService");
+        Mockito.when(dependency.getCallbacks()).thenReturn(callbacks);
+        Mockito.when(dependency.getSpecification()).thenReturn(List.class);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).startsWith("{");
+        assertThat(identifier).endsWith("}");
+        assertThat(identifier).contains("id=" + "id1");
+        assertThat(identifier).contains("field=" + "fs");
+        assertThat(identifier).contains("specification=" + List.class.getName());
+        assertThat(identifier).contains("method=" + "bindFooService");
+
+        System.out.println(identifier);
+    }
+
+    @Test
+    public void testDependencyWithIdOnly() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getId()).thenReturn("id1");
+        Mockito.when(dependency.getField()).thenReturn(null);
+        Mockito.when(dependency.getCallbacks()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).startsWith("{");
+        assertThat(identifier).endsWith("}");
+        assertThat(identifier).contains("id=" + "id1");
+        assertThat(identifier).isEqualTo("{id=" + "id1}");
+        assertThat(identifier).doesNotContain("field");
+        assertThat(identifier).doesNotContain("specification");
+        assertThat(identifier).doesNotContain("method");
+    }
+
+    @Test
+    public void testDependencyWithFieldOnly() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn("fs");
+        Mockito.when(dependency.getId()).thenReturn(null);
+        Mockito.when(dependency.getCallbacks()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{field=" + "fs}");
+    }
+
+    @Test
+    public void testDependencyWithSpecificationOnly() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn(null);
+        Mockito.when(dependency.getId()).thenReturn(null);
+        Mockito.when(dependency.getCallbacks()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(List.class);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{specification=" + List.class.getName() + "}");
+    }
+
+    @Test
+    public void testDependencyWithNothing() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn(null);
+        Mockito.when(dependency.getId()).thenReturn(null);
+        Mockito.when(dependency.getCallbacks()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{}");
+    }
+
+    @Test
+    public void testDependencyWithCallbackOnly() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn(null);
+        Mockito.when(dependency.getId()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+        DependencyCallback[] callbacks = new DependencyCallback[2];
+        callbacks[0] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[0].getMethodName()).thenReturn("bindFooService");
+        callbacks[1] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[1].getMethodName()).thenReturn("unbindFooService");
+        Mockito.when(dependency.getCallbacks()).thenReturn(callbacks);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{method=bindFooService}");
+    }
+
+    @Test
+    public void testDependencyWithIdAndField() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn("fs");
+        Mockito.when(dependency.getId()).thenReturn("id1");
+        Mockito.when(dependency.getCallbacks()).thenReturn(null);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{id=id1, field=fs}");
+    }
+
+    @Test
+    public void testDependencyWithFieldAndMethod() {
+        Dependency dependency = Mockito.mock(Dependency.class);
+        Mockito.when(dependency.getField()).thenReturn("fs");
+        Mockito.when(dependency.getId()).thenReturn(null);
+        DependencyCallback[] callbacks = new DependencyCallback[2];
+        callbacks[0] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[0].getMethodName()).thenReturn("bindFooService");
+        callbacks[1] = Mockito.mock(DependencyCallback.class);
+        Mockito.when(callbacks[1].getMethodName()).thenReturn("unbindFooService");
+        Mockito.when(dependency.getCallbacks()).thenReturn(callbacks);
+        Mockito.when(dependency.getSpecification()).thenReturn(null);
+
+        String identifier = DependencyHandler.getDependencyIdentifier(dependency);
+        assertThat(identifier).isEqualTo("{field=fs, method=bindFooService}");
+    }
+
+
+}