SOLR-4048: improve and clean up findRecursive

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1484386 13f79535-47bb-0310-9956-ffa450edef68

solr/solrj/src/java/org/apache/solr/common/util/NamedList.java

@@ -23,8 +23,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
-
-
 /**
  * A simple container class for modeling an ordered list of name/value pairs.
  *
@@ -50,9 +48,10 @@ import java.util.Map;
  * or simply use a regular {@link Map}
  * </p>
  *
- *
  */
 public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry<String,T>> {
+
+  private static final long serialVersionUID = 1957981902839867821L;
   protected final List<Object> nvPairs;
 
   /** Creates an empty instance */
@@ -60,7 +59,6 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
     nvPairs = new ArrayList<Object>();
   }
 
-
   /**
    * Creates a NamedList instance containing the "name,value" pairs contained in the
    * Entry[].
@@ -83,8 +81,8 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
    * pairwise names/values.
    *
    * <p>
-   * When using this constructor, runtime typesafety is only guaranteed if the all
+   * When using this constructor, runtime type safety is only guaranteed if
-   * even numbered elements of the input list are of type "T".
+   * all even numbered elements of the input list are of type "T".
    * </p>
    *
    * @param nameValuePairs underlying List which should be used to implement a NamedList
@@ -156,6 +154,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
 
   /**
    * Modifies the value of the pair at the specified index.
+   *
    * @return the value that used to be at index
    */
   public T setVal(int idx, T val) {
@@ -168,6 +167,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
 
   /**
    * Removes the name/value pair at the specified index.
+   *
    * @return the value at the index removed
    */
   public T remove(int idx) {
@@ -206,6 +206,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
    * NOTE: this runs in linear time (it scans starting at the
    * beginning of the list until it finds the first pair with
    * the specified name).
+   *
    * @return null if not found or if the value stored was null.
    * @see #indexOf
    * @see #get(String,int)
@@ -222,6 +223,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
    * NOTE: this runs in linear time (it scans starting at the
    * specified position until it finds the first pair with
    * the specified name).
+   *
    * @return null if not found or if the value stored was null.
    * @see #indexOf
    */
@@ -240,6 +242,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
 
   /**
    * Gets the values for the the specified name
+   *
    * @param name Name
    * @return List of values
    */
@@ -256,42 +259,66 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
   }
 
   /**
-   * Recursively parses the NamedList structure to arrive at a
+   * Recursively parses the NamedList structure to arrive at a specific element.
-   * specific element.  As you descend the NamedList tree, the
+   * As you descend the NamedList tree, the last element can be any type,
-   * last element can be any type, including NamedList, but
+   * including NamedList, but the previous elements MUST be NamedList objects
-   * the previous elements MUST be NamedList ojects themselves.
+   * themselves. A null value is returned if the indicated hierarchy doesn't
-   * This method is particularly useful for parsing the response
+   * exist, but NamedList allows null values so that could be the actual value
-   * from Solr's /admin/mbeans handler. 
+   * at the end of the path.
+   * 
+   * This method is particularly useful for parsing the response from Solr's
+   * /admin/mbeans handler, but it also works for any complex structure.
+   * 
+   * Explicitly casting the return value is recommended. An even safer option is
+   * to accept the return value as an object and then check its type.
    * 
-   * Explicit casts are recommended.
    * Usage examples:
    * 
-   * String coreName = (String) response.findRecursive(
+   * String coreName = (String) response.findRecursive
-   * "solr-mbeans", "CORE", "core", "stats", "coreName");
+   * ("solr-mbeans", "CORE", "core", "stats", "coreName");
-   * long numDoc = (long) response.findRecursive(
+   * long numDoc = (long) response.findRecursive
-   * "solr-mbeans", "CORE", "searcher", "stats", "numDocs");
+   * ("solr-mbeans", "CORE", "searcher", "stats", "numDocs");
    * 
-   * @param args One or more strings specifying the tree to navigate.
+   * @param args
-   * @return the last entry in the tree, null if not found.
+   *          One or more strings specifying the tree to navigate.
+   * @return the last entry in the given path hierarchy, null if not found.
    */
-  public T findRecursive(String... args) {
+  public Object findRecursive(String... args) {
-    NamedList<T> list = null;
+    NamedList<?> currentList = null;
-    T value = null;
+    Object value = null;
-    for (String key : args) {
+    for (int i = 0; i < args.length; i++) {
-      /* First pass: this list.  Later passes: previous value. */
+      String key = args[i];
-      if (list == null) {
+      /*
-        list = this;
+       * The first time through the loop, the current list is null, so we assign
-      }
+       * it to this list. Then we retrieve the first key from this list and
-      else
+       * assign it to value.
-      {
+       * 
-        if (NamedList.class.isInstance(value)) {
+       * On the next loop, we check whether the retrieved value is a NamedList.
-          list = (NamedList<T>) value;
+       * If it is, then we drop down to that NamedList, grab the value of the
+       * next key, and start the loop over. If it is not a NamedList, then we
+       * assign the value to null and break out of the loop.
+       * 
+       * Assigning the value to null and then breaking out of the loop seems
+       * like the wrong thing to do, but there's a very simple reason that it
+       * works: If we have reached the last key, then the loop ends naturally
+       * after we retrieve the value, and that code is never executed.
+       */
+      if (currentList == null) {
+        currentList = this;
+      } else {
+        if (value instanceof NamedList) {
+          currentList = (NamedList<?>) value;
         } else {
           value = null;
           break;
         }
       }
-      value = list.get(key);
+      /*
+       * We do not need to do a null check on currentList for the following
+       * assignment. The instanceof check above will fail if the current list is
+       * null, and if that happens, the loop will end before this point.
+       */
+      value = currentList.get(key, 0);
     }
     return value;
   }
@@ -317,7 +344,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
    * Helper class implementing Map.Entry&lt;String, T&gt; to store the key-value
    * relationship in NamedList (the keys of which are String-s)
    */
-  public static final class NamedListEntry<T> implements Map.Entry<String, T> {
+  public static final class NamedListEntry<T> implements Map.Entry<String,T> {
     
     public NamedListEntry() {
 
@@ -335,7 +362,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
 
     @Override
     public T getValue() {
-      return  value;
+      return value;
     }
 
     @Override
@@ -354,8 +381,8 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
    * Iterates over the Map and sequentially adds it's key/value pairs
    */
   public boolean addAll(Map<String,T> args) {
-    for( Map.Entry<String, T> entry : args.entrySet() ) {
+    for (Map.Entry<String, T> entry : args.entrySet() ) {
-      add( entry.getKey(), entry.getValue() );
+      add(entry.getKey(), entry.getValue());
     }
     return args.size()>0;
   }
@@ -376,7 +403,6 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
     return new NamedList<T>(newList);
   }
 
-
   //----------------------------------------------------------------------------
   // Iterable interface
   //----------------------------------------------------------------------------
@@ -408,14 +434,12 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
           }
 
           @Override
-          @SuppressWarnings("unchecked")
           public T getValue() {
             return list.getVal( index );
           }
 
           @Override
-          public String toString()
+          public String toString() {
-          {
             return getKey()+"="+getValue();
           }
 
@@ -446,7 +470,6 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
     return null;
   }
 
-
   public void clear() {
     nvPairs.clear();
   }
@@ -459,7 +482,7 @@ public class NamedList<T> implements Cloneable, Serializable, Iterable<Map.Entry
   @Override
   public boolean equals(Object obj) {
     if (!(obj instanceof NamedList)) return false;
-    NamedList nl = (NamedList) obj;
+    NamedList<?> nl = (NamedList<?>) obj;
     return this.nvPairs.equals(nl.nvPairs);
   }
 }

solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java

@@ -29,6 +29,7 @@ public class NamedListTest extends LuceneTestCase {
     assertEquals("value1", value);
     assertEquals(1, nl.size());
   }
+  
   public void testRecursive() {
     // key1
     // key2
@@ -37,6 +38,7 @@ public class NamedListTest extends LuceneTestCase {
     // --- key2b1
     // --- key2b2
     // - key2c
+    // - k2int1
     // key3
     // - key3a
     // --- key3a1
@@ -44,50 +46,78 @@ public class NamedListTest extends LuceneTestCase {
     // --- key3a3
     // - key3b
     // - key3c
-    NamedList<Object> nl2b = new NamedList<Object>();
+    
+    // this is a varied NL structure.
+    NamedList<String> nl2b = new NamedList<String>();
     nl2b.add("key2b1", "value2b1");
     nl2b.add("key2b2", "value2b2");
-    NamedList<Object> nl3a = new NamedList<Object>();
+    NamedList<String> nl3a = new NamedList<String>();
     nl3a.add("key3a1", "value3a1");
     nl3a.add("key3a2", "value3a2");
     nl3a.add("key3a3", "value3a3");
     NamedList<Object> nl2 = new NamedList<Object>();
     nl2.add("key2a", "value2a");
     nl2.add("key2b", nl2b);
-    int int1 = 5;
+    nl2.add("k2int1", (int) 5);
-    Integer int2 = 7;
-    int int3 = 48;
-    nl2.add("k2int1", int1);
-    nl2.add("k2int2", int2);
-    nl2.add("k2int3", int3);
     NamedList<Object> nl3 = new NamedList<Object>();
     nl3.add("key3a", nl3a);
     nl3.add("key3b", "value3b");
     nl3.add("key3c", "value3c");
+    nl3.add("key3c", "value3c2");
     NamedList<Object> nl = new NamedList<Object>();
     nl.add("key1", "value1");
     nl.add("key2", nl2);
     nl.add("key3", nl3);
     
+    // Simple three-level checks.
     String test1 = (String) nl.findRecursive("key2", "key2b", "key2b2");
-    assertEquals(test1, "value2b2");
+    assertEquals("value2b2", test1);
     String test2 = (String) nl.findRecursive("key3", "key3a", "key3a3");
-    assertEquals(test2, "value3a3");
+    assertEquals("value3a3", test2);
+    // Two-level check.
     String test3 = (String) nl.findRecursive("key3", "key3c");
-    assertEquals(test3, "value3c");
+    assertEquals("value3c", test3);
+    // Checking that invalid values return null.
     String test4 = (String) nl.findRecursive("key3", "key3c", "invalid");
-    assertEquals(test4, null);
+    assertEquals(null, test4);
     String test5 = (String) nl.findRecursive("key3", "invalid", "invalid");
-    assertEquals(test5, null);
+    assertEquals(null, test5);
     String test6 = (String) nl.findRecursive("invalid", "key3c");
-    assertEquals(test6, null);
+    assertEquals(null, test6);
-    Object nltest = nl.findRecursive("key2", "key2b");
+    // Verify that retrieved NamedList objects have the right type.
-    assertTrue(nltest instanceof NamedList);
+    Object test7 = nl.findRecursive("key2", "key2b");
-    Integer int1test = (Integer) nl.findRecursive("key2", "k2int1");
+    assertTrue(test7 instanceof NamedList);
-    assertEquals(int1test, (Integer) 5);
+    // Integer check.
-    int int2test = (int) nl.findRecursive("key2", "k2int2");
+    int test8 = (Integer) nl.findRecursive("key2", "k2int1");
-    assertEquals(int2test, 7);
+    assertEquals(5, test8);
-    int int3test = (int) nl.findRecursive("key2", "k2int3");
+    // Check that a single argument works the same as get(String).
-    assertEquals(int3test, 48);
+    String test9 = (String) nl.findRecursive("key1");
+    assertEquals("value1", test9);
+    // enl == explicit nested list
+    //
+    // key1
+    // - key1a
+    // - key1b
+    // key2 (null list)
+    NamedList<NamedList<String>> enl = new NamedList<NamedList<String>>();
+    NamedList<String> enlkey1 = new NamedList<String>();
+    NamedList<String> enlkey2 = null;
+    enlkey1.add("key1a", "value1a");
+    enlkey1.add("key1b", "value1b");
+    enl.add("key1", enlkey1);
+    enl.add("key2", enlkey2);
+    
+    // Tests that are very similar to the test above, just repeated
+    // on the explicitly nested object type.
+    String enltest1 = (String) enl.findRecursive("key1", "key1a");
+    assertEquals("value1a", enltest1);
+    String enltest2 = (String) enl.findRecursive("key1", "key1b");
+    assertEquals("value1b", enltest2);
+    // Verify that when a null value is stored, the standard get method
+    // says it is null, then check the recursive method.
+    Object enltest3 = enl.get("key2");
+    assertNull(enltest3);
+    Object enltest4 = enl.findRecursive("key2");
+    assertNull(enltest4);
   }
 }