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 2008/08/20 10:53:59 UTC

svn commit: r687294 - in /felix/trunk/ipojo: composite/src/main/java/org/apache/felix/ipojo/composite/ composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ cor...

Author: clement
Date: Wed Aug 20 01:53:58 2008
New Revision: 687294

URL: http://svn.apache.org/viewvc?rev=687294&view=rev
Log:
Fix issue Felix-688 and Felix-689
Improve error reporting when an instance creation failed
Modify the management of the 'name' property (deprecated) in order to use the 'instance.name' property.

Modified:
    felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java
    felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java
    felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java
    felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java

Modified: felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java (original)
+++ felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java Wed Aug 20 01:53:58 2008
@@ -131,7 +131,7 @@
      */
     public void configure(Element metadata, Dictionary configuration) throws ConfigurationException {        
         // Add the name
-        m_name = (String) configuration.get("name");
+        m_name = (String) configuration.get("instance.name");
         
         // Create the standard handlers and add these handlers to the list
         for (int i = 0; i < m_handlers.length; i++) {

Modified: felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java (original)
+++ felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java Wed Aug 20 01:53:58 2008
@@ -46,7 +46,7 @@
      * org.apache.felix.ipojo.metadata.Element, java.util.Dictionary)
      */
     public void configure(Element metadata, Dictionary configuration) {
-        m_name = (String) configuration.get("name");
+        m_name = (String) configuration.get("instance.name");
     }
 
     /**

Modified: felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java (original)
+++ felix/trunk/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java Wed Aug 20 01:53:58 2008
@@ -199,7 +199,7 @@
      */
     public void register() {
         Properties props = new Properties();
-        props.put("name", m_instanceName);
+        props.put("instance.name", m_instanceName);
         List fields = m_composition.getFieldList();
         for (int i = 0; i < fields.size(); i++) {
             FieldMetadata field = (FieldMetadata) fields.get(i);

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java Wed Aug 20 01:53:58 2008
@@ -229,14 +229,19 @@
         }
 
         String name;
-        if (configuration.get("name") == null) {
+        if (configuration.get("instance.name") == null && configuration.get("name") == null) { // Support both instance.name & name
             name = generateName();
-            configuration.put("name", name);
+            configuration.put("instance.name", name);
         } else {
-            name = (String) configuration.get("name");
+            name = (String) configuration.get("instance.name");
+            if (name == null) {
+                name = (String) configuration.get("name");
+                getLogger().log(Logger.WARNING, "The 'name' (" + name + ") attribute, used as the instance name, is deprecated, please use the 'instance.name' attribute");
+                configuration.put("instance.name", name);
+            }
             if (m_instancesName.contains(name)) {
                 m_logger.log(Logger.ERROR, "The configuration is not acceptable : Name already used");
-                throw new UnacceptableConfiguration("Name already used : " + name);
+                throw new UnacceptableConfiguration(getFactoryName() + " : Name already used : " + name);
             }
         }
         // Here we are sure to be valid until the end of the method.
@@ -397,10 +402,14 @@
      * @see org.apache.felix.ipojo.Factory#reconfigure(java.util.Dictionary)
      */
     public synchronized void reconfigure(Dictionary properties) throws UnacceptableConfiguration, MissingHandlerException {
-        if (properties == null || properties.get("name") == null) {
-            throw new UnacceptableConfiguration("The configuration does not contains the \"name\" property");
+        if (properties == null || (properties.get("instance.name") == null && properties.get("name") == null)) { // Support both instance.name and name
+            throw new UnacceptableConfiguration("The configuration does not contains the \"instance.name\" property");
+        }
+
+        String name = (String) properties.get("instance.name");
+        if (name == null) {
+            name = (String) properties.get("name");
         }
-        String name = (String) properties.get("name");
         
         ComponentInstance instance = (ComponentInstance) m_componentInstances.get(name);
         if (instance == null) { // The instance does not exists.
@@ -523,7 +532,7 @@
         
         if (instance == null) {
             try {
-                properties.put("name", name); // Add the name in the configuration
+                properties.put("instance.name", name); // Add the name in the configuration
                 // If an instance with this name was created before, this creation will failed.
                 createComponentInstance(properties);
             } catch (UnacceptableConfiguration e) {
@@ -538,7 +547,7 @@
             }
         } else {
             try {
-                properties.put("name", name); // Add the name in the configuration
+                properties.put("instance.name", name); // Add the name in the configuration
                 reconfigure(properties); // re-configure the component
             } catch (UnacceptableConfiguration e) {
                 m_logger.log(Logger.ERROR, "The configuration is not acceptable : " + e.getMessage());

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java Wed Aug 20 01:53:58 2008
@@ -295,11 +295,17 @@
                 // Test factory accessibility
                 if (factory.m_isPublic || factory.getBundleContext().getBundle().getBundleId() == m_bundleId) {
                     // Test the configuration validity.
-                    if (factory.isAcceptable(m_configuration)) {
+                    try {
+                        factory.checkAcceptability(m_configuration);
                         return true;
-                    } else {
+                    } catch (UnacceptableConfiguration e) {
                         m_logger.log(Logger.ERROR, "An instance can be bound to a matching factory, however the configuration seems unacceptable : "
-                                + m_configuration);
+                                + e.getMessage());
+                        return false;
+                    } catch (MissingHandlerException e) {
+                        m_logger.log(Logger.ERROR, "An instance can be bound to a matching factory, but this factory cannot be used : "
+                                + e.getMessage());
+                        return false;
                     }
                 }
             }

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java Wed Aug 20 01:53:58 2008
@@ -147,7 +147,7 @@
         m_className = metadata.getAttribute("classname");
 
         // Add the name
-        m_name = (String) configuration.get("name");
+        m_name = (String) configuration.get("instance.name");
 
         // Get the factory method if presents.
         m_factoryMethod = (String) metadata.getAttribute("factory-method");
@@ -794,8 +794,8 @@
             initialValue = m_fields.get(fieldName);
         }
         Object result = initialValue;
+        boolean hasChanged = false;
         // Get the list of registered handlers
-
         FieldInterceptor[] list = (FieldInterceptor[]) m_fieldRegistration.get(fieldName); // Immutable list.
         for (int i = 0; list != null && i < list.length; i++) {
             // Call onGet outside of a synchronized block.
@@ -804,6 +804,7 @@
                 continue; // Non-binding case (default implementation).
             } else {
                 if (result != initialValue) {
+                    //TODO analyze impact of removing conflict detection
                     if ((handlerResult != null && !handlerResult.equals(result)) || (result != null && handlerResult == null)) {
                         m_factory.getLogger().log(
                                                   Logger.WARNING,
@@ -812,11 +813,14 @@
                     }
                 }
                 result = handlerResult;
+                hasChanged = true;
             }
         }
-
-        if ((result != null && !result.equals(initialValue)) || (result == null && initialValue != null)) {
+        // TODO use a boolean to speed up the test
+        //if ((result != null && !result.equals(initialValue)) || (result == null && initialValue != null)) {
+        if (hasChanged) {
             // A change occurs => notify the change
+            //TODO consider just changing the reference, however multiple thread can be an issue
             synchronized (this) {
                 m_fields.put(fieldName, result);
             }

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java Wed Aug 20 01:53:58 2008
@@ -109,9 +109,9 @@
      */
     public void addProperty(PropertyDescription pd) { //NOPMD remove the instance name of the 'name' property.
         String name = pd.getName();
-        if ("name".equals(name)) {
-            pd = new PropertyDescription(name, pd.getType(), null); //NOPMD Instance name case.
-        } 
+//        if ("name".equals(name)) {
+//            pd = new PropertyDescription(name, pd.getType(), null); //NOPMD Instance name case.
+//        } 
         
         // Check if the property is not already in the array
         for (int i = 0; i < m_properties.length; i++) {

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java Wed Aug 20 01:53:58 2008
@@ -43,7 +43,7 @@
      * @see org.apache.felix.ipojo.Handler#configure(org.apache.felix.ipojo.metadata.Element, java.util.Dictionary)
      */
     public void configure(Element metadata, Dictionary configuration) {
-        m_name = (String) configuration.get("name");
+        m_name = (String) configuration.get("instance.name");
     }
 
     /**

Modified: felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java?rev=687294&r1=687293&r2=687294&view=diff
==============================================================================
--- felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java (original)
+++ felix/trunk/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java Wed Aug 20 01:53:58 2008
@@ -85,7 +85,7 @@
         String name = instance.getAttribute("name");
         String comp = instance.getAttribute("component");
         if (name != null) {
-            dict.put("name", instance.getAttribute("name"));
+            dict.put("instance.name", instance.getAttribute("name"));
         }
 
         if (comp == null) {
@@ -115,6 +115,7 @@
         if (name == null) {
             throw new ParseException("A property does not have the 'name' attribute: " + prop);
         }
+        
         //case : the property element has no 'value' attribute
         if (value == null) {
             // Recursive case