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.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@687294 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java
index 3d10c51..7d5fd84 100644
--- a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java
+++ b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/CompositeManager.java
@@ -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++) {
diff --git a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java
index e5554b1..817ec08 100644
--- a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java
+++ b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/architecture/ArchitectureHandler.java
@@ -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");
}
/**
diff --git a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java
index 64df649..48ead7d 100644
--- a/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java
+++ b/ipojo/composite/src/main/java/org/apache/felix/ipojo/composite/service/provides/ProvidedService.java
@@ -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);
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
index 03402c3..a3840084 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
@@ -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("name");
+
+ String name = (String) properties.get("instance.name");
+ if (name == null) {
+ 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());
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java
index 7333161..6b77440 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceCreator.java
@@ -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;
}
}
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
index e006eb9..371ec2e 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
@@ -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);
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
index f49390e..232fd07 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
@@ -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++) {
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java
index 6021c5a..5578170 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/architecture/ArchitectureHandler.java
@@ -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");
}
/**
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java
index fe033ac..3680d38 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/parser/ManifestMetadataParser.java
@@ -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