FELIX-4665 - Allow empty default values:
- make sure that for optional single-valued attributes an empty default
value can be defined;
- added some test cases to verify the behaviour for single-valued and
multi-valued attributes.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1653581 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/metatype/src/main/java/org/apache/felix/metatype/AD.java b/metatype/src/main/java/org/apache/felix/metatype/AD.java
index b6bc239..1a2ff9a 100644
--- a/metatype/src/main/java/org/apache/felix/metatype/AD.java
+++ b/metatype/src/main/java/org/apache/felix/metatype/AD.java
@@ -219,11 +219,17 @@
}
/**
- * @param defaultValue the defaultValue to set
+ * Sets the default value(s) for this AD.
+ * <p>
+ * NOTE: this method is depending on the value of {@link #getCardinality()}! Make sure that the
+ * cardinality is properly set <b>before</b> calling this method.
+ * </p>
+ *
+ * @param defaultValue the default value to set, as encoded string-value (using comma's as separator), can be <code>null</code>.
*/
public void setDefaultValue(String defaultValue)
{
- this.setDefaultValue( splitList(defaultValue) );
+ setDefaultValue(splitList(defaultValue), Math.abs(this.cardinality));
}
/**
@@ -243,47 +249,6 @@
}
/**
- * @param values the defaultValue to set
- */
- public void setDefaultValue(String[] values)
- {
- if ( values != null )
- {
- int count = 0;
- for(int i=0; i<values.length; i++)
- {
- if ( "".equals(ADValidator.validate(this, values[i])) )
- {
- count++;
- }
- else
- {
- values[i] = null;
- }
- }
- if ( count == 0 )
- {
- values = null;
- }
- else if ( count != values.length )
- {
- String[] filterValues = new String[count];
- int index = 0;
- for(int i=0; i<values.length; i++)
- {
- if ( values[i] != null )
- {
- filterValues[index] = values[i];
- index++;
- }
- }
- values = filterValues;
- }
- }
- this.defaultValue = values;
- }
-
- /**
* @param isRequired the isRequired to set
*/
public void setRequired(boolean isRequired)
@@ -360,9 +325,9 @@
{
char ch = listString.charAt(i);
final boolean isWhitespace = Character.isWhitespace(ch);
- if ( start )
+ if (start)
{
- if ( isWhitespace )
+ if (isWhitespace)
{
continue;
}
@@ -387,24 +352,25 @@
spaceCount = 0;
continue;
}
- } else if ( ch == ' ')
+ }
+ else if (ch == ' ')
{
// space is only ignored at beginning and end but not if escaped
- if (!escaped )
+ if (!escaped)
{
spaceCount++;
continue;
}
}
- else if (isWhitespace )
+ else if (isWhitespace)
{
// Other whitespaces are ignored...
continue;
}
- if ( spaceCount > 0)
+ if (spaceCount > 0)
{
- for(int m = 0; m<spaceCount; m++)
+ for (int m = 0; m < spaceCount; m++)
{
sb.append(" ");
}
@@ -460,6 +426,48 @@
return null;
}
+ /**
+ * @param values the defaultValue to set
+ */
+ protected void setDefaultValue(String[] values, int cardinality)
+ {
+ if (values != null)
+ {
+ int count = 0;
+ int max = Math.min(values.length, Math.max(1, cardinality));
+ for (int i = 0; count < max && i < values.length; i++)
+ {
+ if ("".equals(ADValidator.validate(this, values[i])))
+ {
+ count++;
+ }
+ else
+ {
+ values[i] = null;
+ }
+ }
+ if (count == 0)
+ {
+ values = cardinality == 0 ? null : new String[0];
+ }
+ else if (count != values.length)
+ {
+ String[] filterValues = new String[count];
+ int index = 0;
+ for (int i = 0; index < count && i < values.length; i++)
+ {
+ if (values[i] != null)
+ {
+ filterValues[index] = values[i];
+ index++;
+ }
+ }
+ values = filterValues;
+ }
+ }
+ this.defaultValue = values;
+ }
+
private static class ComparableBoolean implements Comparable
{
private boolean value;
diff --git a/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java b/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java
index 08c0aae..85aa0dc 100644
--- a/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java
+++ b/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java
@@ -400,8 +400,8 @@
ad.setCardinality(getOptionalAttribute("cardinality", 0));
ad.setMin(getOptionalAttribute("min"));
ad.setMax(getOptionalAttribute("max"));
- ad.setDefaultValue(getOptionalAttribute("default"));
ad.setRequired(getOptionalAttribute("required", true));
+ String dfltValue = getOptionalAttribute("default");
readOptionalAttributes(ad, AD_ATTRIBUTES);
@@ -439,11 +439,12 @@
ad.setOptions(options);
- // reset value to force an options check (FELIX-3884)
- if (ad.getDefaultValue() != null)
+ // set value as late as possible to force an options check (FELIX-3884, FELIX-4665)...
+ if (dfltValue != null)
{
- ad.setDefaultValue(ad.getDefaultValue());
+ ad.setDefaultValue(dfltValue);
}
+
return ad;
}
diff --git a/metatype/src/test/java/org/apache/felix/metatype/ADTest.java b/metatype/src/test/java/org/apache/felix/metatype/ADTest.java
index b271c82..c830cfe 100644
--- a/metatype/src/test/java/org/apache/felix/metatype/ADTest.java
+++ b/metatype/src/test/java/org/apache/felix/metatype/ADTest.java
@@ -123,7 +123,7 @@
public void testSpaces()
{
String value = "Hello World";
- String listString = BLANK + value + BLANK + "," + BLANK + value + BLANK + "," +value;
+ String listString = BLANK + value + BLANK + "," + BLANK + value + BLANK + "," + value;
String[] list = AD.splitList(listString);
assertNotNull(list);
assertEquals(3, list.length);
@@ -206,35 +206,95 @@
}
/**
- * FELIX-3884 : if an AD has options, default values must be in the option
- * values.
+ * FELIX-3884 : if an AD has options, default values must be in the option values.
*/
public void testOptionsAndDefaultValues()
{
- final AD ad = new AD();
+ AD ad = new AD();
+ ad.setCardinality(2);
ad.setType("String");
- final Map options = new HashMap();
+ ad.setRequired(false);
+
+ Map options = new HashMap();
options.put("A", "L-A");
options.put("B", "L-B");
ad.setOptions(options);
- ad.setDefaultValue(new String[] {"A", "B"});
- equalsArray(new String[] {"A", "B"}, ad.getDefaultValue());
+ ad.setDefaultValue("A,B");
+ assertArrayEquals(new String[] { "A", "B" }, ad.getDefaultValue());
- ad.setDefaultValue(new String[] {"A", "B", "C"});
- equalsArray(new String[] {"A", "B"}, ad.getDefaultValue());
+ ad.setDefaultValue("A,B,C");
+ assertArrayEquals(new String[] { "A", "B" }, ad.getDefaultValue());
- ad.setDefaultValue(new String[] {"X", "Y", "B"});
- equalsArray(new String[] {"B"}, ad.getDefaultValue());
+ ad.setDefaultValue("X,Y,B");
+ assertArrayEquals(new String[] { "B" }, ad.getDefaultValue());
- ad.setDefaultValue(new String[] {"X", "Y", "Z"});
+ ad.setDefaultValue("X,Y,Z");
+ assertArrayEquals(new String[0], ad.getDefaultValue());
+
+ ad.setDefaultValue(null);
assertNull(ad.getDefaultValue());
}
- private void equalsArray(String[] a, String[] b)
+ /**
+ * FELIX-3884/FELIX-4665 - Default values.
+ */
+ public void testDefaultValuesForSingleValuedAttributes()
+ {
+ AD ad = new AD();
+ ad.setCardinality(0);
+ ad.setType("String");
+ ad.setRequired(false);
+
+ ad.setDefaultValue(null);
+ assertNull(ad.getDefaultValue());
+
+ ad.setDefaultValue("A,B");
+ assertArrayEquals(new String[] { "A" }, ad.getDefaultValue());
+
+ ad.setDefaultValue("");
+ assertArrayEquals(new String[] { "" }, ad.getDefaultValue());
+
+ // corner case: in case of required values, an empty default makes no sense
+ // for single values, hence that the empty default is coerced into null...
+ ad.setRequired(true);
+ ad.setDefaultValue("");
+ assertNull(ad.getDefaultValue());
+ }
+
+ /**
+ * FELIX-3884/FELIX-4665 - Default values.
+ */
+ public void testDefaultValuesForMultiValuedAttributes()
+ {
+ AD ad = new AD();
+ ad.setCardinality(-2); // sign doesn't matter in this case
+ ad.setType("String");
+ ad.setRequired(false);
+
+ ad.setDefaultValue(null);
+ assertNull(ad.getDefaultValue());
+
+ ad.setDefaultValue("A,B");
+ assertArrayEquals(new String[] { "A", "B" }, ad.getDefaultValue());
+
+ ad.setDefaultValue(",,");
+ assertArrayEquals(new String[] { "", "" }, ad.getDefaultValue());
+
+ ad.setDefaultValue("");
+ assertArrayEquals(new String[] { "" }, ad.getDefaultValue());
+
+ // corner case: in case of required values, an empty default is coerced
+ // into a empty array...
+ ad.setRequired(true);
+ ad.setDefaultValue("");
+ assertArrayEquals(new String[0], ad.getDefaultValue());
+ }
+
+ private static void assertArrayEquals(String[] a, String[] b)
{
assertEquals(a.length, b.length);
- for(int i=0; i<a.length;i++)
+ for (int i = 0; i < a.length; i++)
{
assertEquals(a[i], b[i]);
}
diff --git a/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java b/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java
index 894c698..7461fb1 100644
--- a/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java
+++ b/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java
@@ -21,6 +21,7 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.util.Arrays;
import java.util.Map;
import junit.framework.TestCase;
@@ -183,6 +184,43 @@
}
/**
+ * FELIX-4665 - Default values can be empty.
+ */
+ public void testAttributeWithDefaultValueOk() throws IOException, XmlPullParserException
+ {
+ String xml;
+
+ xml = "<MetaData><OCD id=\"ocd\" name=\"ocd\"><AD id=\"attr\" type=\"String\" required=\"false\" default=\"\" /></OCD></MetaData>";
+ MetaData mti = read(xml);
+ OCD ocd = (OCD) mti.getObjectClassDefinitions().get("ocd");
+ AD ad = (AD) ocd.getAttributeDefinitions().get("attr");
+ // Cardinality = 0, so *always* return a array with length 1...
+ assertTrue(Arrays.deepEquals(new String[] { "" }, ad.getDefaultValue()));
+
+ xml = "<MetaData><OCD id=\"ocd\" name=\"ocd\"><AD id=\"attr\" cardinality=\"1\" type=\"String\" required=\"false\" /></OCD></MetaData>";
+ mti = read(xml);
+ ocd = (OCD) mti.getObjectClassDefinitions().get("ocd");
+ ad = (AD) ocd.getAttributeDefinitions().get("attr");
+ // Cardinality = 1, return an array with *up to* 1 elements...
+ assertNull(ad.getDefaultValue());
+
+ xml = "<MetaData><OCD id=\"ocd\" name=\"ocd\"><AD id=\"attr\" cardinality=\"1\" type=\"String\" required=\"false\" default=\"\" /></OCD></MetaData>";
+ mti = read(xml);
+ ocd = (OCD) mti.getObjectClassDefinitions().get("ocd");
+ ad = (AD) ocd.getAttributeDefinitions().get("attr");
+ // Cardinality = 1, return an array with *up to* 1 elements...
+ assertTrue(Arrays.deepEquals(new String[] { "" }, ad.getDefaultValue()));
+
+ // The metatype spec defines that getDefaultValue should never have more entries than defined in its (non-zero) cardinality...
+ xml = "<MetaData><OCD id=\"ocd\" name=\"ocd\"><AD id=\"attr\" cardinality=\"1\" type=\"String\" required=\"false\" default=\",\" /></OCD></MetaData>";
+ mti = read(xml);
+ ocd = (OCD) mti.getObjectClassDefinitions().get("ocd");
+ ad = (AD) ocd.getAttributeDefinitions().get("attr");
+ // Cardinality = 1, return an array with *up to* 1 elements...
+ assertTrue(Arrays.deepEquals(new String[] { "" }, ad.getDefaultValue()));
+ }
+
+ /**
* FELIX-4644 - Enforce that we can only have one Object in a Designate element.
*/
public void testOCDWithoutADFail() throws IOException, XmlPullParserException