Fix issue Felix-815.
Properties becomes optional by default.
Unvalued properties are not published with the service reference until they receive a value.
Setter methods are not called until properties receive a value
Injected values in fields follow standard Java initialization (null for object and arrays, 0 for numeric types, false for boolean).
Mandatory properties can be set with the 'mandatory' attribute (supported in XML and in the annotations).
The core.xsd XML Schema is also modified to add the new attribute.
This improvement allows removing default value in properties (such as in the architecture handlers).
Provides test about this new improvement
Fix issue Felix-816
The comparator attribute is now supported for any binding policy.
Provides test about using custom comparator with any binding policy
Fix issue Felix-817
Solve an issue in the ServiceExporter when a service in unregistered twice (throws an IllegalStateException since a recent modification of the Felix framework).
Fix issue Felix-818
Implement the ServiceReferenceImpl compareTo method (method added in OSGi R4.1). This method reuse the same implementation as the Felix framework ServiceReferenceImpl method.
Remove junit4osgi embedded tests
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@713976 13f79535-47bb-0310-9956-ffa450edef68
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 c09359c..0f63fa8 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
@@ -424,8 +424,8 @@
throw new UnacceptableConfiguration("The property " + props[i] + " cannot be overide : immutable property"); // The instance configuration tries to override an immutable property.
}
// Is the property required ?
- if (props[i].getValue() == null && conf.get(props[i].getName()) == null) {
- throw new UnacceptableConfiguration("The property " + props[i].getName() + " is missing"); // The property must be set.
+ if (props[i].isMandatory() && props[i].getValue() == null && conf.get(props[i].getName()) == null) {
+ throw new UnacceptableConfiguration("The mandatory property " + props[i].getName() + " is missing"); // The property must be set.
}
}
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/PropertyDescription.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/PropertyDescription.java
index 222be42..6af31f6 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/PropertyDescription.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/PropertyDescription.java
@@ -47,17 +47,23 @@
/**
* Immutable property flag
- * IF true, the property cannot be override by the instance configuration.
+ * If set to <code>true</code>, the property cannot be override by the instance configuration.
* Moreover, immutable properties are exposed on the factory service too.
*/
private boolean m_immutable = false;
+
+ /**
+ * A property is mandatory. So, either the component type description provides a value or
+ * the instance configuration must provide a value. Immutable properties are mandatories.
+ */
+ private boolean m_isMandatory = false;
/**
* Constructor.
*
- * @param name : name of the property
- * @param type : type of the property
- * @param value : default value of the property
+ * @param name the name of the property
+ * @param type the type of the property
+ * @param value the default value of the property, can be <code>null</code>
*/
public PropertyDescription(String name, String type, String value) {
m_name = name;
@@ -68,10 +74,10 @@
/**
* Constructor.
*
- * @param name : name of the property
- * @param type : type of the property
- * @param value : default value of the property
- * @param immutable : the property is immutable.
+ * @param name the name of the property
+ * @param type the type of the property
+ * @param value the default value (String form) of the property, can be <code>null</code>
+ * @param immutable the property is immutable.
*/
public PropertyDescription(String name, String type, String value, boolean immutable) {
m_name = name;
@@ -81,7 +87,7 @@
}
/**
- * Get the current property name.
+ * Gets the current property name.
* @return the property name.
*/
public String getName() {
@@ -89,7 +95,7 @@
}
/**
- * Get the current property type.
+ * Gets the current property type.
* @return the property type.
*/
public String getType() {
@@ -97,8 +103,9 @@
}
/**
- * Get the current property value.
- * @return the default value for the property.
+ * Gets the current property value.
+ * @return the default value for the property,
+ * <code>null</code> if the property hasn't a value..
*/
public String getValue() {
return m_value;
@@ -106,17 +113,39 @@
/**
* Is the property immutable.
- * @return true if the property is immutable.
+ * @return <code>true</code> if the property is immutable.
*/
public boolean isImmutable() {
return m_immutable;
}
+
/**
- * Get the object value of the current immutable property.
- * @param context : bundle context to use to load classes.
- * @return the object value of the current property.
+ * Sets the property as mandatory.
+ */
+ public void setMandatory() {
+ m_isMandatory = true;
+ }
+
+ /**
+ * Is the property mandatory.
+ * @return <code>true</code> if the property is mandatory,
+ * <code>false</code> otherwise.
+ */
+ public boolean isMandatory() {
+ return m_isMandatory;
+ }
+
+ /**
+ * Gets the object value of the current immutable property.
+ * @param context the bundle context to use to load classes.
+ * @return the object value of the current property or <code>
+ * null</code> if the current value is <code>null</code>.
*/
public Object getObjectValue(BundleContext context) {
+ if (m_value == null) {
+ return null;
+ }
+
Class type = null;
try {
type = Property.computeType(m_type, context);
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/context/ServiceReferenceImpl.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/context/ServiceReferenceImpl.java
index 046bf15..9889882 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/context/ServiceReferenceImpl.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/context/ServiceReferenceImpl.java
@@ -22,6 +22,7 @@
import org.apache.felix.ipojo.ComponentInstance;
import org.osgi.framework.Bundle;
+import org.osgi.framework.Constants;
import org.osgi.framework.ServiceReference;
/**
@@ -117,15 +118,39 @@
/**
* Service Reference compare method.
- * This method is not yet supported.
- * @param arg0 the object
- * @return this methods is not yet supported, and throws an {@link UnsupportedOperationException}.
+ * @param reference the service reference
+ * @return this methods is not yet supported, and throws an
+ * {@link UnsupportedOperationException}.
* @see org.osgi.framework.ServiceReference#compareTo(java.lang.Object)
- * TODO implements this method
*/
- public int compareTo(Object arg0) {
- throw new UnsupportedOperationException("This feature has not yet been implemented.");
+ public int compareTo(Object reference) {
+ ServiceReference other = (ServiceReference) reference;
+
+ Long id = (Long) getProperty(Constants.SERVICE_ID);
+ Long otherId = (Long) other.getProperty(Constants.SERVICE_ID);
+
+ if (id.equals(otherId)) {
+ return 0; // same service
+ }
+
+ Integer rank = (Integer) getProperty(Constants.SERVICE_RANKING);
+ Integer otherRank = (Integer) other
+ .getProperty(Constants.SERVICE_RANKING);
+
+ // If no rank, then spec says it defaults to zero.
+ rank = (rank == null) ? new Integer(0) : rank;
+ otherRank = (otherRank == null) ? new Integer(0) : otherRank;
+
+ // Sort by rank in ascending order.
+ if (rank.compareTo(otherRank) < 0) {
+ return -1; // lower rank
+ } else if (rank.compareTo(otherRank) > 0) {
+ return 1; // higher rank
+ }
+
+ // If ranks are equal, then sort by service id in descending order.
+ return (id.compareTo(otherId) < 0) ? 1 : -1;
}
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/context/StringMap.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/context/StringMap.java
index 31f7331..737c95b 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/context/StringMap.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/context/StringMap.java
@@ -18,6 +18,7 @@
*/
package org.apache.felix.ipojo.context;
+import java.io.Serializable;
import java.util.Comparator;
import java.util.Map;
import java.util.TreeMap;
@@ -92,7 +93,12 @@
((StringComparator) comparator()).setCaseSensitive(flag);
}
- private static class StringComparator implements Comparator {
+ private static class StringComparator implements Comparator, Serializable {
+ /**
+ * Id.
+ */
+ private static final long serialVersionUID = 1L;
+
/**
* Is the map case sensitive?
*/
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
index f229a44..e4986b3 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
@@ -152,15 +152,24 @@
// Is the property set to immutable
boolean immutable = false;
String imm = configurables[i].getAttribute("immutable");
- if (imm != null && imm.equalsIgnoreCase("true")) {
- immutable = true;
+ immutable = imm != null && imm.equalsIgnoreCase("true");
+
+ boolean mandatory = false;
+ String man = configurables[i].getAttribute("mandatory");
+ mandatory = man != null && man.equalsIgnoreCase("true");
+
+ PropertyDescription pd = null;
+ if (value == null) {
+ pd = new PropertyDescription(name, type, null, false); // Cannot be immutable if we have no value.
+ } else {
+ pd = new PropertyDescription(name, type, value, immutable);
}
- if (value == null) {
- desc.addProperty(new PropertyDescription(name, type, null, false)); // Cannot be immutable if we have no value.
- } else {
- desc.addProperty(new PropertyDescription(name, type, value, immutable));
+ if (mandatory) {
+ pd.setMandatory();
}
+
+ desc.addProperty(pd);
}
}
@@ -194,7 +203,6 @@
m_managedServicePID = instanceMSPID;
}
-
for (int i = 0; configurables != null && i < configurables.length; i++) {
String fieldName = configurables[i].getAttribute("field");
String methodName = configurables[i].getAttribute("method");
@@ -250,7 +258,9 @@
if (m_mustPropagate) {
for (int i = 0; i < m_configurableProperties.size(); i++) {
Property prop = (Property) m_configurableProperties.get(i);
- m_toPropagate.put(prop.getName(), prop.getValue());
+ if (prop.getValue() != Property.NO_VALUE && prop.getValue() != null) { // No injected value, or null
+ m_toPropagate.put(prop.getName(), prop.getValue());
+ }
}
reconfigure(m_toPropagate);
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
index 997b3b8..6161cfb 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
@@ -40,7 +40,6 @@
import org.osgi.framework.BundleContext;
import org.osgi.framework.Filter;
import org.osgi.framework.ServiceReference;
-import org.osgi.framework.SynchronousBundleListener;
/**
* Represent a service dependency of the component instance.
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java
index 74afe7c..2fb75d8 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java
@@ -273,11 +273,12 @@
* @return the properties attached to the provided service.
*/
private Properties getServiceProperties() {
- // Contruct the service properties list
+ // Build the service properties list
Properties serviceProperties = new Properties();
for (int i = 0; i < m_properties.length; i++) {
- if (m_properties[i].getValue() != null) {
- serviceProperties.put(m_properties[i].getName(), m_properties[i].getValue());
+ Object value = m_properties[i].getValue();
+ if (value != null && value != Property.NO_VALUE) {
+ serviceProperties.put(m_properties[i].getName(), value);
}
}
return serviceProperties;
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedServiceHandler.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedServiceHandler.java
index 6a67d3c..3844cba 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedServiceHandler.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedServiceHandler.java
@@ -454,7 +454,7 @@
Properties props = new Properties();
for (int k = 0; k < svc.getProperties().length; k++) {
Property prop = svc.getProperties()[k];
- if (prop.getValue() != null) {
+ if (prop.getValue() != null && prop.getValue() != Property.NO_VALUE) {
props.put(prop.getName(), prop.getValue().toString());
}
}
@@ -548,6 +548,7 @@
String value = props[j].getAttribute("value");
String type = props[j].getAttribute("type");
String field = props[j].getAttribute("field");
+
// Get property name :
if (field != null && name == null) {
@@ -573,8 +574,14 @@
if (imm != null && imm.equalsIgnoreCase("true")) {
immutable = true;
}
-
- desc.addProperty(new PropertyDescription(name, type, value, immutable));
+
+ PropertyDescription pd = new PropertyDescription(name, type, value, immutable);
+ desc.addProperty(pd);
+
+ String man = props[j].getAttribute("mandatory");
+ if (man != null && man.equalsIgnoreCase("true")) {
+ pd.setMandatory();
+ }
}
}
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
index de5b188..e53530d 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/DependencyModel.java
@@ -319,9 +319,17 @@
synchronized (this) {
m_matchingRefs.add(ref);
- // Sort the collection if needed.
+ // Sort the collection if needed, if not sort, services are append to the list.
if (m_comparator != null) {
- Collections.sort(m_matchingRefs, m_comparator);
+ // The collection must be sort only if:
+ // The policy is dynamic-priority
+ // No services are already used
+ // If so, sorting can imply a re-binding, and so don't follow the Dynamic Binding policy
+ if (m_policy == DYNAMIC_PRIORITY_BINDING_POLICY
+ || m_tracker.getUsedServiceReferences() == null
+ || m_tracker.getUsedServiceReferences().isEmpty()) {
+ Collections.sort(m_matchingRefs, m_comparator);
+ }
}
size = m_matchingRefs.size();
@@ -383,6 +391,14 @@
if (obj == null) {
computeDependencyState(); // check if the dependency stills valid.
} else {
+ // A used service disappears, we have to sort the available providers to choose the best one.
+ // However, the sort has to be done only for scalar dependencies following the dynamic binding
+ // policy. Static dependencies will be broken, DP dependencies are always sorted.
+ // Aggregate dependencies does not need to be sort, as it will change the array
+ // order.
+ if (m_comparator != null && m_policy == DYNAMIC_BINDING_POLICY && ! m_aggregate) {
+ Collections.sort(m_matchingRefs, m_comparator);
+ }
onServiceDeparture(ref);
ServiceReference newRef = getServiceReference();
if (newRef == null) { // Check if there is another provider.
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Property.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Property.java
index a022d71..2f3dffc 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Property.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/Property.java
@@ -36,6 +36,8 @@
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
*/
public class Property implements FieldInterceptor {
+
+ public static final Object NO_VALUE = new Object();
/**
* The name of the property (field name if not set).
@@ -58,7 +60,7 @@
/**
* The value of the property.
*/
- private Object m_value;
+ private Object m_value = NO_VALUE;
/**
* Flag tracking is the method was
@@ -160,7 +162,7 @@
} catch (ClassNotFoundException e) {
throw new ConfigurationException("Class not found exception in setValue on " + type + " : " + e.getMessage());
} catch (SecurityException e) {
- throw new ConfigurationException("Security excption in setValue on " + type + " : " + e.getMessage());
+ throw new ConfigurationException("Security execption in setValue on " + type + " : " + e.getMessage());
} catch (IllegalArgumentException e) {
throw new ConfigurationException("Argument issue when calling the constructor of the type " + type);
}
@@ -252,11 +254,32 @@
public boolean hasField() {
return m_field != null;
}
-
+
public synchronized Object getValue() {
return m_value;
}
+
+ /**
+ * Gets the NO VALUE Object.
+ * This method returns the object to inject when the property
+ * was not assigned to a value.
+ * @param type the type of the value.
+ * @return the object to inject when the property has no value.
+ */
+ private static Object getNoValue(Class type) {
+ if (Boolean.TYPE.equals(type)) { return new Boolean(false); }
+ if (Byte.TYPE.equals(type)) { return new Byte((byte) 0); }
+ if (Short.TYPE.equals(type)) { return new Short((short) 0); }
+ if (Integer.TYPE.equals(type)) { return new Integer(0); }
+ if (Long.TYPE.equals(type)) { return new Long(0); }
+ if (Float.TYPE.equals(type)) { return new Float(0); }
+ if (Double.TYPE.equals(type)) { return new Double(0); }
+ if (Character.TYPE.equals(type)) { return new Character((char) 0); }
+ // If all other case, return null.
+ return null;
+ }
+
/**
* Sets the value of the property.
* @param value the new value.
@@ -451,6 +474,11 @@
return; // Already called.
}
+ if (m_value == NO_VALUE) {
+ // Don't call method if no value
+ return;
+ }
+
try {
if (instance == null) {
m_method.call(new Object[] { m_value });
@@ -478,8 +506,11 @@
* @return the value if the handler want to inject this value.
* @see org.apache.felix.ipojo.FieldInterceptor#onGet(java.lang.Object, java.lang.String, java.lang.Object)
*/
- public Object onGet(Object pojo, String fieldName, Object value) {
- return getValue();
+ public synchronized Object onGet(Object pojo, String fieldName, Object value) {
+ if (m_value == NO_VALUE) {
+ return getNoValue(m_type);
+ }
+ return m_value;
}
/**
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/ServiceReferenceRankingComparator.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/ServiceReferenceRankingComparator.java
index 0cc3bf2..649116c 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/util/ServiceReferenceRankingComparator.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/util/ServiceReferenceRankingComparator.java
@@ -18,6 +18,7 @@
*/
package org.apache.felix.ipojo.util;
+import java.io.Serializable;
import java.util.Comparator;
import org.osgi.framework.Constants;
@@ -28,7 +29,12 @@
* This comparator follows OSGi Ranking policy.
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
*/
-public class ServiceReferenceRankingComparator implements Comparator {
+public class ServiceReferenceRankingComparator implements Comparator, Serializable {
+
+ /**
+ * Id.
+ */
+ private static final long serialVersionUID = 1L;
/**
* Compares two service reference.
diff --git a/ipojo/core/src/main/resources/core.xsd b/ipojo/core/src/main/resources/core.xsd
index 9cdbfcf..4584e26 100644
--- a/ipojo/core/src/main/resources/core.xsd
+++ b/ipojo/core/src/main/resources/core.xsd
@@ -329,30 +329,52 @@
</xs:annotation></xs:attribute>
</xs:complexType>
<xs:complexType name="PropertyType">
- <xs:annotation>
- <xs:documentation>Defines a component property.</xs:documentation>
- </xs:annotation>
- <xs:attribute name="field" type="xs:string" use="optional">
- <xs:annotation>
- <xs:documentation>Field of the property</xs:documentation>
- </xs:annotation></xs:attribute>
- <xs:attribute name="method" type="xs:string" use="optional">
+ <xs:annotation>
+ <xs:documentation>
+ Defines a component property.
+ </xs:documentation>
+ </xs:annotation>
+ <xs:attribute name="field" type="xs:string" use="optional">
<xs:annotation>
- <xs:documentation>Setter method of the property. This method is called to inject property value.</xs:documentation>
- </xs:annotation></xs:attribute>
+ <xs:documentation>
+ Field of the property
+ </xs:documentation>
+ </xs:annotation>
+ </xs:attribute>
+ <xs:attribute name="method" type="xs:string" use="optional">
+ <xs:annotation>
+ <xs:documentation>
+ Setter method of the property. This method is called
+ to inject property value.
+ </xs:documentation>
+ </xs:annotation>
+ </xs:attribute>
<xs:attribute name="name" type="xs:string" use="optional">
<xs:annotation>
- <xs:documentation>Name of the property.</xs:documentation>
- </xs:annotation></xs:attribute>
+ <xs:documentation>
+ Name of the property.
+ </xs:documentation>
+ </xs:annotation>
+ </xs:attribute>
<xs:attribute name="value" type="xs:string" use="optional">
<xs:annotation>
- <xs:documentation>Default value of the property.</xs:documentation>
- </xs:annotation></xs:attribute>
+ <xs:documentation>
+ Default value of the property.
+ </xs:documentation>
+ </xs:annotation>
+ </xs:attribute>
<xs:attribute name="type" type="xs:string" use="optional">
<xs:annotation>
- <xs:documentation>Type of the property.</xs:documentation>
- </xs:annotation></xs:attribute>
- </xs:complexType>
+ <xs:documentation>
+ Type of the property.
+ </xs:documentation>
+ </xs:annotation>
+ </xs:attribute>
+ <xs:attribute name="mandatory" type="xs:boolean" use="optional" default="false">
+ <xs:annotation>
+ <xs:documentation>Set the property as mandatory. A mandatory property MUST receive a value either in the component type description or in the instance configuration. Properties are optional by default.</xs:documentation>
+ </xs:annotation></xs:attribute>
+ </xs:complexType>
<xs:element name="callback" type="CallbackType" id="callback"></xs:element>
<xs:element name="controller" type="ControllerType" id="controller">
<xs:annotation>
diff --git a/ipojo/core/src/main/resources/metadata.xml b/ipojo/core/src/main/resources/metadata.xml
index 354dca3..6e49d69 100644
--- a/ipojo/core/src/main/resources/metadata.xml
+++ b/ipojo/core/src/main/resources/metadata.xml
@@ -41,7 +41,7 @@
classname="org.apache.felix.ipojo.handlers.architecture.ArchitectureHandler"
name="architecture" architecture="false">
<provides interface="org.apache.felix.ipojo.architecture.Architecture">
- <property field="m_name" name="architecture.instance" value="" />
+ <property field="m_name" name="architecture.instance"/>
</provides>
</handler>
</ipojo>
\ No newline at end of file