You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by ja...@apache.org on 2015/01/21 17:25:18 UTC

svn commit: r1653581 - in /felix/trunk/metatype/src: main/java/org/apache/felix/metatype/ test/java/org/apache/felix/metatype/

Author: jawi
Date: Wed Jan 21 16:25:18 2015
New Revision: 1653581

URL: http://svn.apache.org/r1653581
Log:
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.


Modified:
    felix/trunk/metatype/src/main/java/org/apache/felix/metatype/AD.java
    felix/trunk/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java
    felix/trunk/metatype/src/test/java/org/apache/felix/metatype/ADTest.java
    felix/trunk/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java

Modified: felix/trunk/metatype/src/main/java/org/apache/felix/metatype/AD.java
URL: http://svn.apache.org/viewvc/felix/trunk/metatype/src/main/java/org/apache/felix/metatype/AD.java?rev=1653581&r1=1653580&r2=1653581&view=diff
==============================================================================
--- felix/trunk/metatype/src/main/java/org/apache/felix/metatype/AD.java (original)
+++ felix/trunk/metatype/src/main/java/org/apache/felix/metatype/AD.java Wed Jan 21 16:25:18 2015
@@ -219,11 +219,17 @@ public class AD extends OptionalAttribut
     }
 
     /**
-     * @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 @@ public class AD extends OptionalAttribut
     }
 
     /**
-     * @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 @@ public class AD extends OptionalAttribut
         {
             char ch = listString.charAt(i);
             final boolean isWhitespace = Character.isWhitespace(ch);
-            if ( start )
+            if (start)
             {
-                if ( isWhitespace )
+                if (isWhitespace)
                 {
                     continue;
                 }
@@ -387,24 +352,25 @@ public class AD extends OptionalAttribut
                     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 @@ public class AD extends OptionalAttribut
         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;

Modified: felix/trunk/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java
URL: http://svn.apache.org/viewvc/felix/trunk/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java?rev=1653581&r1=1653580&r2=1653581&view=diff
==============================================================================
--- felix/trunk/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java (original)
+++ felix/trunk/metatype/src/main/java/org/apache/felix/metatype/MetaDataReader.java Wed Jan 21 16:25:18 2015
@@ -400,8 +400,8 @@ public class MetaDataReader
         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 @@ public class MetaDataReader
 
         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;
     }
 

Modified: felix/trunk/metatype/src/test/java/org/apache/felix/metatype/ADTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/metatype/src/test/java/org/apache/felix/metatype/ADTest.java?rev=1653581&r1=1653580&r2=1653581&view=diff
==============================================================================
--- felix/trunk/metatype/src/test/java/org/apache/felix/metatype/ADTest.java (original)
+++ felix/trunk/metatype/src/test/java/org/apache/felix/metatype/ADTest.java Wed Jan 21 16:25:18 2015
@@ -123,7 +123,7 @@ public class ADTest extends TestCase
     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 @@ public class ADTest extends TestCase
     }
 
     /**
-     * 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("A,B,C");
+        assertArrayEquals(new String[] { "A", "B" }, ad.getDefaultValue());
 
-        ad.setDefaultValue(new String[] {"A", "B", "C"});
-        equalsArray(new String[] {"A", "B"}, ad.getDefaultValue());
+        ad.setDefaultValue("X,Y,B");
+        assertArrayEquals(new String[] { "B" }, ad.getDefaultValue());
+
+        ad.setDefaultValue("X,Y,Z");
+        assertArrayEquals(new String[0], ad.getDefaultValue());
+
+        ad.setDefaultValue(null);
+        assertNull(ad.getDefaultValue());
+    }
 
-        ad.setDefaultValue(new String[] {"X", "Y", "B"});
-        equalsArray(new String[] {"B"}, ad.getDefaultValue());
+    /**
+     * FELIX-3884/FELIX-4665 - Default values.
+     */
+    public void testDefaultValuesForSingleValuedAttributes()
+    {
+        AD ad = new AD();
+        ad.setCardinality(0);
+        ad.setType("String");
+        ad.setRequired(false);
 
-        ad.setDefaultValue(new String[] {"X", "Y", "Z"});
+        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 void equalsArray(String[] a, String[] b)
+    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]);
         }

Modified: felix/trunk/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java?rev=1653581&r1=1653580&r2=1653581&view=diff
==============================================================================
--- felix/trunk/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java (original)
+++ felix/trunk/metatype/src/test/java/org/apache/felix/metatype/MetaDataReaderTest.java Wed Jan 21 16:25:18 2015
@@ -21,6 +21,7 @@ package org.apache.felix.metatype;
 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 @@ public class MetaDataReaderTest extends
     }
 
     /**
+     * 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