You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2014/07/22 00:18:38 UTC

svn commit: r1612419 - in /lucene/dev/trunk/solr: ./ core/src/java/org/apache/solr/core/ core/src/test-files/solr/ core/src/test/org/apache/solr/cloud/ core/src/test/org/apache/solr/core/

Author: hossman
Date: Mon Jul 21 22:18:38 2014
New Revision: 1612419

URL: http://svn.apache.org/r1612419
Log:
SOLR-5746: Bugs in solr.xml parsing have been fixed to more correctly deal with the various datatypes of options people can specify, additional error handling of duplicated/unidentified options has also been added

Modified:
    lucene/dev/trunk/solr/CHANGES.txt
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java
    lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Mon Jul 21 22:18:38 2014
@@ -114,6 +114,13 @@ Upgrading from Solr 4.9
 * CoreContainer.remove() has been removed.  You should now use CoreContainer.unload() to
   delete a SolrCore (see SOLR-6232).
 
+* solr.xml parsing has been improved to better account for the expected data types of 
+  various options.  As part of this fix, additional error checking has also been added to
+  provide errors in the event of duplicated options, or unknown option names that may 
+  indicate a typo.  Users who have modified their solr.xml in the past and now upgrade may 
+  get errors on startup if they have typos or unexpected options specified in their solr.xml 
+  file.  (See SOLR-5746 for more information.)
+
 Detailed Change List
 ----------------------
 
@@ -205,6 +212,11 @@ Bug Fixes
   ArrayIndexOutOfBoundsException when using the composite id router.
   (Steve Rowe)
 
+* SOLR-5746: Bugs in solr.xml parsing have been fixed to more correctly deal with the various 
+  datatypes of options people can specify, additional error handling of duplicated/unidentified 
+  options has also been added. (Maciej Zasada, hossman)
+
+
 Optimizations
 ---------------------
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolr.java Mon Jul 21 22:18:38 2014
@@ -17,7 +17,17 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import com.google.common.io.ByteStreams;
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.solr.cloud.CloudConfigSetService;
@@ -32,20 +42,6 @@ import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 import org.xml.sax.InputSource;
 
-import javax.xml.xpath.XPath;
-import javax.xml.xpath.XPathConstants;
-import javax.xml.xpath.XPathExpressionException;
-
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.nio.charset.StandardCharsets;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Properties;
-
 
 public abstract class ConfigSolr {
   protected static Logger log = LoggerFactory.getLogger(ConfigSolr.class);
@@ -55,24 +51,18 @@ public abstract class ConfigSolr {
   public static ConfigSolr fromFile(SolrResourceLoader loader, File configFile) {
     log.info("Loading container configuration from {}", configFile.getAbsolutePath());
 
-    InputStream inputStream = null;
+    if (!configFile.exists()) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+          "solr.xml does not exist in " + configFile.getAbsolutePath() + " cannot start Solr");
+    }
 
-    try {
-      if (!configFile.exists()) {
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-            "solr.xml does not exist in " + configFile.getAbsolutePath() + " cannot start Solr");
-      }
-      else {
-        inputStream = new FileInputStream(configFile);
-      }
+    try (InputStream inputStream = new FileInputStream(configFile)) {
       return fromInputStream(loader, inputStream);
-    }
-    catch (Exception e) {
+    } catch (SolrException exc) {
+      throw exc;
+    } catch (Exception exc) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-          "Could not load SOLR configuration", e);
-    }
-    finally {
-      IOUtils.closeQuietly(inputStream);
+          "Could not load SOLR configuration", exc);
     }
   }
 
@@ -84,9 +74,12 @@ public abstract class ConfigSolr {
     try {
       byte[] buf = IOUtils.toByteArray(is);
       String originalXml = new String(buf, StandardCharsets.UTF_8);
-      ByteArrayInputStream dup = new ByteArrayInputStream(buf);
-      Config config = new Config(loader, null, new InputSource(dup), null, false);
-      return fromConfig(config, originalXml);
+      try (ByteArrayInputStream dup = new ByteArrayInputStream(buf)) {
+        Config config = new Config(loader, null, new InputSource(dup), null, false);
+        return fromConfig(config, originalXml);
+      }
+    } catch (SolrException exc) {
+      throw exc;
     } catch (Exception e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
     }
@@ -133,7 +126,7 @@ public abstract class ConfigSolr {
     String sysProp = System.getProperty("zkClientTimeout");
     if (sysProp != null)
       return Integer.parseInt(sysProp);
-    return getInt(CfgProp.SOLR_ZKCLIENTTIMEOUT, DEFAULT_ZK_CLIENT_TIMEOUT);
+    return get(CfgProp.SOLR_ZKCLIENTTIMEOUT, DEFAULT_ZK_CLIENT_TIMEOUT);
   }
 
   private static final int DEFAULT_ZK_CLIENT_TIMEOUT = 15000;
@@ -156,35 +149,35 @@ public abstract class ConfigSolr {
   }
 
   public int getLeaderVoteWait() {
-    return getInt(CfgProp.SOLR_LEADERVOTEWAIT, DEFAULT_LEADER_VOTE_WAIT);
+    return get(CfgProp.SOLR_LEADERVOTEWAIT, DEFAULT_LEADER_VOTE_WAIT);
   }
   
   public int getLeaderConflictResolveWait() {
-    return getInt(CfgProp.SOLR_LEADERCONFLICTRESOLVEWAIT, DEFAULT_LEADER_CONFLICT_RESOLVE_WAIT);
+    return get(CfgProp.SOLR_LEADERCONFLICTRESOLVEWAIT, DEFAULT_LEADER_CONFLICT_RESOLVE_WAIT);
   }
 
   public boolean getGenericCoreNodeNames() {
-    return getBool(CfgProp.SOLR_GENERICCORENODENAMES, false);
+    return get(CfgProp.SOLR_GENERICCORENODENAMES, false);
   }
 
   public int getDistributedConnectionTimeout() {
-    return getInt(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, 0);
+    return get(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, 0);
   }
 
   public int getDistributedSocketTimeout() {
-    return getInt(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, 0);
+    return get(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, 0);
   }
   
   public int getMaxUpdateConnections() {
-    return getInt(CfgProp.SOLR_MAXUPDATECONNECTIONS, 10000);
+    return get(CfgProp.SOLR_MAXUPDATECONNECTIONS, 10000);
   }
 
   public int getMaxUpdateConnectionsPerHost() {
-    return getInt(CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, 100);
+    return get(CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, 100);
   }
 
   public int getCoreLoadThreadCount() {
-    return getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, DEFAULT_CORE_LOAD_THREADS);
+    return get(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, DEFAULT_CORE_LOAD_THREADS);
   }
 
   public String getSharedLibDirectory() {
@@ -214,7 +207,7 @@ public abstract class ConfigSolr {
   }
 
   public boolean hasSchemaCache() {
-    return getBool(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, false);
+    return get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, false);
   }
 
   public String getManagementPath() {
@@ -226,16 +219,18 @@ public abstract class ConfigSolr {
   }
 
   public LogWatcherConfig getLogWatcherConfig() {
+    String loggingClass = get(CfgProp.SOLR_LOGGING_CLASS, null);
+    String loggingWatcherThreshold = get(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, null);
     return new LogWatcherConfig(
-        getBool(CfgProp.SOLR_LOGGING_ENABLED, true),
-        get(CfgProp.SOLR_LOGGING_CLASS, null),
-        get(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, null),
-        getInt(CfgProp.SOLR_LOGGING_WATCHER_SIZE, 50)
+        get(CfgProp.SOLR_LOGGING_ENABLED, true),
+        loggingClass,
+        loggingWatcherThreshold,
+        get(CfgProp.SOLR_LOGGING_WATCHER_SIZE, 50)
     );
   }
 
   public int getTransientCacheSize() {
-    return getInt(CfgProp.SOLR_TRANSIENTCACHESIZE, Integer.MAX_VALUE);
+    return get(CfgProp.SOLR_TRANSIENTCACHESIZE, Integer.MAX_VALUE);
   }
 
   public ConfigSetService createCoreConfigService(SolrResourceLoader loader, ZkController zkController) {
@@ -282,11 +277,11 @@ public abstract class ConfigSolr {
   }
 
   protected Config config;
-  protected Map<CfgProp, String> propMap = new HashMap<>();
+  protected Map<CfgProp, Object> propMap = new HashMap<>();
 
   public ConfigSolr(Config config) {
     this.config = config;
-
+    config.substituteProperties();
   }
 
   // for extension & testing.
@@ -298,22 +293,12 @@ public abstract class ConfigSolr {
     return config;
   }
 
-  public int getInt(CfgProp prop, int def) {
-    String val = propMap.get(prop);
-    if (val != null) val = PropertiesUtil.substituteProperty(val, null);
-    return (val == null) ? def : Integer.parseInt(val);
-  }
-
-  public boolean getBool(CfgProp prop, boolean defValue) {
-    String val = propMap.get(prop);
-    if (val != null) val = PropertiesUtil.substituteProperty(val, null);
-    return (val == null) ? defValue : Boolean.parseBoolean(val);
-  }
-
-  public String get(CfgProp prop, String def) {
-    String val = propMap.get(prop);
-    if (val != null) val = PropertiesUtil.substituteProperty(val, null);
-    return (val == null) ? def : val;
+  @SuppressWarnings("unchecked")
+  public <T> T get(CfgProp key, T defaultValue) {
+    if (propMap.containsKey(key) && propMap.get(key) != null) {
+      return (T) propMap.get(key);
+    }
+    return defaultValue;
   }
 
   public Properties getSolrProperties(String path) {

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java Mon Jul 21 22:18:38 2014
@@ -18,11 +18,23 @@ package org.apache.solr.core;
  */
 
 import org.apache.solr.common.SolrException;
-import org.apache.solr.util.PropertiesUtil;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.util.DOMUtil;
+
+import com.google.common.base.Function;
+import com.google.common.base.Functions;
+
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Locale;
 
 
 /**
@@ -39,27 +51,25 @@ public class ConfigSolrXml extends Confi
     try {
       checkForIllegalConfig();
       fillPropMap();
-      config.substituteProperties();
       coresLocator = new CorePropertiesLocator(getCoreRootDirectory());
-    }
-    catch (IOException e) {
+    } catch (IOException e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
     }
   }
-  
+
   private void checkForIllegalConfig() throws IOException {
-    
+
     // Do sanity checks - we don't want to find old style config
     failIfFound("solr/@coreLoadThreads");
     failIfFound("solr/@persistent");
     failIfFound("solr/@sharedLib");
     failIfFound("solr/@zkHost");
-    
+
     failIfFound("solr/logging/@class");
     failIfFound("solr/logging/@enabled");
     failIfFound("solr/logging/watcher/@size");
     failIfFound("solr/logging/watcher/@threshold");
-    
+
     failIfFound("solr/cores/@adminHandler");
     failIfFound("solr/cores/@distribUpdateConnTimeout");
     failIfFound("solr/cores/@distribUpdateSoTimeout");
@@ -73,7 +83,7 @@ public class ConfigSolrXml extends Confi
     failIfFound("solr/cores/@shareSchema");
     failIfFound("solr/cores/@transientCacheSize");
     failIfFound("solr/cores/@zkClientTimeout");
-    
+
     // These have no counterpart in 5.0, asking for any of these in Solr 5.0
     // will result in an error being
     // thrown.
@@ -82,7 +92,7 @@ public class ConfigSolrXml extends Confi
     failIfFound("solr/cores/@adminPath");
 
   }
-  
+
   private void failIfFound(String xPath) {
 
     if (config.getVal(xPath, false) != null) {
@@ -91,43 +101,144 @@ public class ConfigSolrXml extends Confi
     }
   }
 
-  // We can do this in 5.0 when we read the solr.xml since we don't need to keep the original around for persistence.
-  private String doSub(String path) {
-    String val = config.getVal(path, false);
-    if (val != null) {
-      val = PropertiesUtil.substituteProperty(val, null);
+  private NamedList<Object> readNodeListAsNamedList(String path) {
+    NodeList nodes = config.getNodeList(path, false);
+    if (nodes != null) {
+      NamedList<Object> namedList = DOMUtil.nodesToNamedList(nodes);
+      return namedList;
     }
-    return val;
+    return new NamedList<>();
   }
-  
+
   private void fillPropMap() {
-    propMap.put(CfgProp.SOLR_ADMINHANDLER, doSub("solr/str[@name='adminHandler']"));
-    propMap.put(CfgProp.SOLR_COLLECTIONSHANDLER, doSub("solr/str[@name='collectionsHandler']"));
-    propMap.put(CfgProp.SOLR_INFOHANDLER, doSub("solr/str[@name='infoHandler']"));
-    propMap.put(CfgProp.SOLR_CORELOADTHREADS, doSub("solr/int[@name='coreLoadThreads']"));
-    propMap.put(CfgProp.SOLR_COREROOTDIRECTORY, doSub("solr/str[@name='coreRootDirectory']"));
-    propMap.put(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, doSub("solr/solrcloud/int[@name='distribUpdateConnTimeout']"));
-    propMap.put(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, doSub("solr/solrcloud/int[@name='distribUpdateSoTimeout']"));
-    propMap.put(CfgProp.SOLR_MAXUPDATECONNECTIONS, doSub("solr/solrcloud/int[@name='maxUpdateConnections']"));
-    propMap.put(CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, doSub("solr/solrcloud/int[@name='maxUpdateConnectionsPerHost']"));
-    propMap.put(CfgProp.SOLR_HOST, doSub("solr/solrcloud/str[@name='host']"));
-    propMap.put(CfgProp.SOLR_HOSTCONTEXT, doSub("solr/solrcloud/str[@name='hostContext']"));
-    propMap.put(CfgProp.SOLR_HOSTPORT, doSub("solr/solrcloud/int[@name='hostPort']"));
-    propMap.put(CfgProp.SOLR_LEADERVOTEWAIT, doSub("solr/solrcloud/int[@name='leaderVoteWait']"));
-    propMap.put(CfgProp.SOLR_LEADERCONFLICTRESOLVEWAIT, doSub("solr/solrcloud/int[@name='leaderConflictResolveWait']"));
-    propMap.put(CfgProp.SOLR_GENERICCORENODENAMES, doSub("solr/solrcloud/bool[@name='genericCoreNodeNames']"));
-    propMap.put(CfgProp.SOLR_MANAGEMENTPATH, doSub("solr/str[@name='managementPath']"));
-    propMap.put(CfgProp.SOLR_SHAREDLIB, doSub("solr/str[@name='sharedLib']"));
-    propMap.put(CfgProp.SOLR_SHARESCHEMA, doSub("solr/str[@name='shareSchema']"));
-    propMap.put(CfgProp.SOLR_TRANSIENTCACHESIZE, doSub("solr/int[@name='transientCacheSize']"));
-    propMap.put(CfgProp.SOLR_ZKCLIENTTIMEOUT, doSub("solr/solrcloud/int[@name='zkClientTimeout']"));
-    propMap.put(CfgProp.SOLR_ZKHOST, doSub("solr/solrcloud/str[@name='zkHost']"));
-    propMap.put(CfgProp.SOLR_CONFIGSETBASEDIR, doSub("solr/str[@name='configSetBaseDir']"));
-
-    propMap.put(CfgProp.SOLR_LOGGING_CLASS, doSub("solr/logging/str[@name='class']"));
-    propMap.put(CfgProp.SOLR_LOGGING_ENABLED, doSub("solr/logging/str[@name='enabled']"));
-    propMap.put(CfgProp.SOLR_LOGGING_WATCHER_SIZE, doSub("solr/logging/watcher/int[@name='size']"));
-    propMap.put(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, doSub("solr/logging/watcher/int[@name='threshold']"));
+    NamedList<Object> unknownConfigParams = new NamedList<>();
+
+    // shardHandlerFactory is parsed differently in the base class as a plugin, so we're excluding this node from the node list
+    fillSolrSection(readNodeListAsNamedList("solr/*[@name][not(name()='shardHandlerFactory')]"));
+
+    thereCanBeOnlyOne("solr/solrcloud","<solrcloud>");
+    fillSolrCloudSection(readNodeListAsNamedList("solr/solrcloud/*[@name]"));
+
+    thereCanBeOnlyOne("solr/logging","<logging>");
+    thereCanBeOnlyOne("solr/logging/watcher","Logging <watcher>");
+    fillLoggingSection(readNodeListAsNamedList("solr/logging/*[@name]"), 
+                       readNodeListAsNamedList("solr/logging/watcher/*[@name]"));
+  }
+
+  private void fillSolrSection(NamedList<Object> nl) {
+    String s = "Main";
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_ADMINHANDLER, "adminHandler");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_COLLECTIONSHANDLER, "collectionsHandler");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_INFOHANDLER, "infoHandler");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_COREROOTDIRECTORY, "coreRootDirectory");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_MANAGEMENTPATH, "managementPath");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_SHAREDLIB, "sharedLib");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_CONFIGSETBASEDIR, "configSetBaseDir");
+
+    storeConfigPropertyAsBoolean(s, nl, CfgProp.SOLR_SHARESCHEMA, "shareSchema");
+
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_CORELOADTHREADS, "coreLoadThreads");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_TRANSIENTCACHESIZE, "transientCacheSize");
+
+    errorOnLeftOvers(s, nl);
+  }
+
+  private void fillSolrCloudSection(NamedList<Object> nl) {
+    
+    String s = "<solrcloud>";
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, "distribUpdateConnTimeout");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, "distribUpdateSoTimeout");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_MAXUPDATECONNECTIONS, "maxUpdateConnections");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, "maxUpdateConnectionsPerHost");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_LEADERVOTEWAIT, "leaderVoteWait");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_LEADERCONFLICTRESOLVEWAIT, "leaderConflictResolveWait");
+    storeConfigPropertyAsInt(s, nl, CfgProp.SOLR_ZKCLIENTTIMEOUT, "zkClientTimeout");
+
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_HOST, "host");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_HOSTCONTEXT, "hostContext");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_HOSTPORT, "hostPort");
+    storeConfigPropertyAsString(s, nl, CfgProp.SOLR_ZKHOST, "zkHost");
+
+    storeConfigPropertyAsBoolean(s, nl, CfgProp.SOLR_GENERICCORENODENAMES, "genericCoreNodeNames");
+    
+    errorOnLeftOvers(s, nl);
+  }
+
+  private void fillLoggingSection(NamedList<Object> loggingConfig, 
+                                  NamedList<Object> loggingWatcherConfig) {
+    
+    String s = "<logging>";
+    storeConfigPropertyAsString(s, loggingConfig, CfgProp.SOLR_LOGGING_CLASS, "class");
+    storeConfigPropertyAsBoolean(s, loggingConfig, CfgProp.SOLR_LOGGING_ENABLED, "enabled");
+
+    errorOnLeftOvers(s, loggingConfig);
+
+    s = "Logging <watcher>";
+    storeConfigPropertyAsInt(s, loggingWatcherConfig, CfgProp.SOLR_LOGGING_WATCHER_SIZE, "size");
+    storeConfigPropertyAsString(s, loggingWatcherConfig, CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, "threshold");
+
+    errorOnLeftOvers(s, loggingWatcherConfig);
+  }
+
+
+  private <T> void storeConfigProperty(String section, NamedList<Object> config, CfgProp propertyKey, String name, Function<Object, T> valueTransformer, Class<T> clazz) {
+    List<Object> values = config.removeAll(name); 
+    if (null != values && 0 != values.size()) {
+      if (1 < values.size()) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                                String.format(Locale.ROOT, 
+                                              "%s section of solr.xml contains duplicated '%s'"+
+                                              " in solr.xml: %s", section, name, values));
+      } else {
+        Object value = values.get(0);
+        if (value != null) {
+          if (value.getClass().isAssignableFrom(clazz)) {
+            propMap.put(propertyKey, value);
+          } else {
+            propMap.put(propertyKey, valueTransformer.apply(value));
+          }
+        } else {
+          propMap.put(propertyKey, null);
+        }
+      }
+    }
+  }
+
+  private void storeConfigPropertyAsString(String section, NamedList<Object> config, CfgProp propertyKey, String name) {
+    storeConfigProperty(section, config, propertyKey, name, Functions.toStringFunction(), String.class);
+  }
+
+  private void storeConfigPropertyAsInt(String section, NamedList<Object> config, CfgProp propertyKey, String xmlElementName) {
+    storeConfigProperty(section, config, propertyKey, xmlElementName, TO_INT_FUNCTION, Integer.class);
+  }
+
+  private void storeConfigPropertyAsBoolean(String section, NamedList<Object> config, CfgProp propertyKey, String name) {
+    storeConfigProperty(section, config, propertyKey, name, TO_BOOLEAN_FUNCTION, Boolean.class);
+  }
+
+  /** throws an error if more then one element matching the xpath */
+  private void thereCanBeOnlyOne(String xpath, String section) {
+    NodeList lst = config.getNodeList(xpath, false);
+    if (1 < lst.getLength())
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                              lst.getLength() + " instances of " + section + " found in solr.xml");
+  }
+
+  /** logs each item in leftovers and then throws an exception with a summary */
+  private void errorOnLeftOvers(String section, NamedList<Object> leftovers) {
+
+    if (null == leftovers || 0 == leftovers.size()) return;
+
+    List<String> unknownElements = new ArrayList<String>(leftovers.size());
+    for (Map.Entry<String, Object> unknownElement : leftovers) {
+      log.error("Unknown config parameter in {} section of solr.xml: {} -> {}", 
+                section, unknownElement.getKey(), unknownElement.getValue());
+      unknownElements.add(unknownElement.getKey());
+    }
+    if (! unknownElements.isEmpty() ) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                              String.format(Locale.ROOT, "%s section of solr.xml contains %d unknown config parameter(s): %s", section, unknownElements.size(), unknownElements));
+    }
   }
 
   @Override
@@ -155,5 +266,34 @@ public class ConfigSolrXml extends Confi
     return coresLocator;
   }
 
+  private static final Function<Map.Entry<String, Object>, String> GET_KEY_FUNCTION = new Function<Map.Entry<String, Object>, String>() {
+    @Override
+    public String apply(Map.Entry<String, Object> input) {
+      return input.getKey();
+    }
+  };
+
+  private static final Function<Object, Integer> TO_INT_FUNCTION = new Function<Object, Integer>() {
+    @Override
+    public Integer apply(Object input) {
+      try {
+        return Integer.parseInt(String.valueOf(input));
+      } catch (NumberFormatException exc) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, 
+                                String.format(Locale.ROOT, 
+                                              "Value of '%s' can not be parsed as 'int'", input));
+      }
+    }
+  };
+
+  private static final Function<Object, Boolean> TO_BOOLEAN_FUNCTION = new Function<Object, Boolean>() {
+    @Override
+    public Boolean apply(Object input) {
+      if (input instanceof String) {
+        return Boolean.valueOf(String.valueOf(input));
+      }
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, String.format(Locale.ROOT, "Value of '%s' can not be parsed as 'bool'", input));
+    }
+  };
 }
 

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java Mon Jul 21 22:18:38 2014
@@ -17,14 +17,6 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import org.apache.solr.common.SolrException;
-import org.apache.solr.util.DOMUtil;
-import org.apache.solr.util.PropertiesUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;
-
 import javax.xml.xpath.XPathConstants;
 import javax.xml.xpath.XPathExpressionException;
 import java.io.File;
@@ -38,6 +30,15 @@ import java.util.Map;
 import java.util.Properties;
 import java.util.Set;
 
+import org.apache.commons.lang.StringUtils;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.util.DOMUtil;
+import org.apache.solr.util.PropertiesUtil;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
 
 /**
  *
@@ -62,7 +63,6 @@ public class ConfigSolrXmlOld extends Co
     try {
       checkForIllegalConfig();
       fillPropMap();
-      config.substituteProperties();
       initCoreList();
       this.persistor = isPersistent() ? new SolrXMLCoresLocator(originalXML, this)
                                       : new SolrXMLCoresLocator.NonPersistingLocator(originalXML, this);
@@ -123,63 +123,59 @@ public class ConfigSolrXmlOld extends Co
   }
   
   private void fillPropMap() {
-    
-    propMap.put(CfgProp.SOLR_CORELOADTHREADS,
-        config.getVal("solr/@coreLoadThreads", false));
-    propMap
-        .put(CfgProp.SOLR_SHAREDLIB, config.getVal("solr/@sharedLib", false));
-    propMap.put(CfgProp.SOLR_ZKHOST, config.getVal("solr/@zkHost", false));
-    
-    propMap.put(CfgProp.SOLR_LOGGING_CLASS,
-        config.getVal("solr/logging/@class", false));
-    propMap.put(CfgProp.SOLR_LOGGING_ENABLED,
-        config.getVal("solr/logging/@enabled", false));
-    propMap.put(CfgProp.SOLR_LOGGING_WATCHER_SIZE,
-        config.getVal("solr/logging/watcher/@size", false));
-    propMap.put(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD,
-        config.getVal("solr/logging/watcher/@threshold", false));
-    
-    propMap.put(CfgProp.SOLR_ADMINHANDLER,
-        config.getVal("solr/cores/@adminHandler", false));
-    propMap.put(CfgProp.SOLR_COLLECTIONSHANDLER, config.getVal("solr/cores/@collectionsHandler", false));
-    propMap.put(CfgProp.SOLR_INFOHANDLER, config.getVal("solr/cores/@infoHandler", false));
-    propMap.put(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT,
-        config.getVal("solr/cores/@distribUpdateConnTimeout", false));
-    propMap.put(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT,
-        config.getVal("solr/cores/@distribUpdateSoTimeout", false));
-    propMap.put(CfgProp.SOLR_MAXUPDATECONNECTIONS,
-        config.getVal("solr/cores/@maxUpdateConnections", false));
-    propMap.put(CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST,
-        config.getVal("solr/cores/@maxUpdateConnectionsPerHost", false));
-    propMap.put(CfgProp.SOLR_HOST, config.getVal("solr/cores/@host", false));
-    propMap.put(CfgProp.SOLR_HOSTCONTEXT,
-        config.getVal("solr/cores/@hostContext", false));
-    propMap.put(CfgProp.SOLR_HOSTPORT,
-        config.getVal("solr/cores/@hostPort", false));
-    propMap.put(CfgProp.SOLR_LEADERVOTEWAIT,
-        config.getVal("solr/cores/@leaderVoteWait", false));
-    propMap.put(CfgProp.SOLR_GENERICCORENODENAMES,
-        config.getVal("solr/cores/@genericCoreNodeNames", false));
-    propMap.put(CfgProp.SOLR_MANAGEMENTPATH,
-        config.getVal("solr/cores/@managementPath", false));
-    propMap.put(CfgProp.SOLR_SHARESCHEMA,
-        config.getVal("solr/cores/@shareSchema", false));
-    propMap.put(CfgProp.SOLR_TRANSIENTCACHESIZE,
-        config.getVal("solr/cores/@transientCacheSize", false));
-    propMap.put(CfgProp.SOLR_ZKCLIENTTIMEOUT,
-        config.getVal("solr/cores/@zkClientTimeout", false));
-    propMap.put(CfgProp.SOLR_CONFIGSETBASEDIR, config.getVal("solr/cores/@configSetBaseDir", false));
+    storeConfigPropertyAsInt(CfgProp.SOLR_CORELOADTHREADS, "solr/@coreLoadThreads");
+    storeConfigPropertyAsString(CfgProp.SOLR_SHAREDLIB, "solr/@sharedLib");
+    storeConfigPropertyAsString(CfgProp.SOLR_ZKHOST, "solr/@zkHost");
+    storeConfigPropertyAsString(CfgProp.SOLR_LOGGING_CLASS, "solr/logging/@class");
+    storeConfigPropertyAsBoolean(CfgProp.SOLR_LOGGING_ENABLED, "solr/logging/@enabled");
+    storeConfigPropertyAsInt(CfgProp.SOLR_LOGGING_WATCHER_SIZE, "solr/logging/watcher/@size");
+    storeConfigPropertyAsString(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, "solr/logging/watcher/@threshold");
+    storeConfigPropertyAsString(CfgProp.SOLR_ADMINHANDLER, "solr/cores/@adminHandler");
+    storeConfigPropertyAsString(CfgProp.SOLR_COLLECTIONSHANDLER, "solr/cores/@collectionsHandler");
+    storeConfigPropertyAsString(CfgProp.SOLR_INFOHANDLER, "solr/cores/@infoHandler");
+    storeConfigPropertyAsInt(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, "solr/cores/@distribUpdateConnTimeout");
+    storeConfigPropertyAsInt(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, "solr/cores/@distribUpdateSoTimeout");
+    storeConfigPropertyAsInt(CfgProp.SOLR_MAXUPDATECONNECTIONS, "solr/cores/@maxUpdateConnections");
+    storeConfigPropertyAsInt(CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, "solr/cores/@maxUpdateConnectionsPerHost");
+    storeConfigPropertyAsString(CfgProp.SOLR_HOST, "solr/cores/@host");
+    storeConfigPropertyAsString(CfgProp.SOLR_HOSTCONTEXT, "solr/cores/@hostContext");
+    storeConfigPropertyAsString(CfgProp.SOLR_HOSTPORT, "solr/cores/@hostPort");
+    storeConfigPropertyAsInt(CfgProp.SOLR_LEADERVOTEWAIT, "solr/cores/@leaderVoteWait");
+    storeConfigPropertyAsBoolean(CfgProp.SOLR_GENERICCORENODENAMES, "solr/cores/@genericCoreNodeNames");
+    storeConfigPropertyAsString(CfgProp.SOLR_MANAGEMENTPATH, "solr/cores/@managementPath");
+    storeConfigPropertyAsBoolean(CfgProp.SOLR_SHARESCHEMA, "solr/cores/@shareSchema");
+    storeConfigPropertyAsInt(CfgProp.SOLR_TRANSIENTCACHESIZE, "solr/cores/@transientCacheSize");
+    storeConfigPropertyAsInt(CfgProp.SOLR_ZKCLIENTTIMEOUT, "solr/cores/@zkClientTimeout");
+    storeConfigPropertyAsString(CfgProp.SOLR_CONFIGSETBASEDIR, "solr/cores/@configSetBaseDir");
 
     // These have no counterpart in 5.0, asking, for any of these in Solr 5.0
     // will result in an error being
     // thrown.
-    propMap.put(CfgProp.SOLR_CORES_DEFAULT_CORE_NAME,
-        config.getVal("solr/cores/@defaultCoreName", false));
-    propMap.put(CfgProp.SOLR_PERSISTENT,
-        config.getVal("solr/@persistent", false));
-    propMap.put(CfgProp.SOLR_ADMINPATH,
-        config.getVal("solr/cores/@adminPath", false));
-    
+    storeConfigPropertyAsString(CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, "solr/cores/@defaultCoreName");
+    storeConfigPropertyAsString(CfgProp.SOLR_PERSISTENT, "solr/@persistent");
+    storeConfigPropertyAsString(CfgProp.SOLR_ADMINPATH, "solr/cores/@adminPath");
+  }
+
+  private void storeConfigPropertyAsInt(CfgProp key, String xmlPath) {
+    String valueAsString = config.getVal(xmlPath, false);
+    if (StringUtils.isNotBlank(valueAsString)) {
+      propMap.put(key, Integer.parseInt(valueAsString));
+    } else {
+      propMap.put(key, null);
+    }
+  }
+
+  private void storeConfigPropertyAsBoolean(CfgProp key, String xmlPath) {
+    String valueAsString = config.getVal(xmlPath, false);
+    if (StringUtils.isNotBlank(valueAsString)) {
+      propMap.put(key, Boolean.parseBoolean(valueAsString));
+    } else {
+      propMap.put(key, null);
+    }
+  }
+
+  private void storeConfigPropertyAsString(CfgProp key, String xmlPath) {
+    propMap.put(key, config.getVal(xmlPath, false));
   }
 
   private void initCoreList() throws IOException {
@@ -196,7 +192,6 @@ public class ConfigSolrXmlOld extends Co
       String name = DOMUtil.getAttr(node, CoreDescriptor.CORE_NAME, null);
 
       String dataDir = DOMUtil.getAttr(node, CoreDescriptor.CORE_DATADIR, null);
-      if (dataDir != null) dataDir = PropertiesUtil.substituteProperty(dataDir, null);
       if (name != null) {
         if (!names.contains(name)) {
           names.add(name);
@@ -208,7 +203,6 @@ public class ConfigSolrXmlOld extends Co
       }
 
       String instDir = DOMUtil.getAttr(node, CoreDescriptor.CORE_INSTDIR, null);
-      if (instDir != null) instDir = PropertiesUtil.substituteProperty(instDir, null);
 
       if (dataDir != null) {
         String absData = null;
@@ -259,7 +253,7 @@ public class ConfigSolrXmlOld extends Co
           String propVal = DOMUtil.getAttr(node, property);
           if (propVal == null)
             propVal = defaultVal;
-          return PropertiesUtil.substituteProperty(propVal, null);
+          return propVal;
         }
       }
     }

Modified: lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml (original)
+++ lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml Mon Jul 21 22:18:38 2014
@@ -23,7 +23,7 @@
   <str name="infoHandler">testInfoHandler</str>
   <str name="managementPath">testManagementPath</str>
   <str name="sharedLib">testSharedLib</str>
-  <str name="shareSchema">${shareSchema:testShareSchema}</str>
+  <str name="shareSchema">${shareSchema:true}</str>
   <int name="transientCacheSize">66</int>
 
   <solrcloud>
@@ -41,7 +41,7 @@
 
   <logging>
     <str name="class">testLoggingClass</str>
-    <str name="enabled">testLoggingEnabled</str>
+    <str name="enabled">true</str>
     <watcher>
       <int name="size">88</int>
       <int name="threshold">99</int>

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/SolrXmlInZkTest.java Mon Jul 21 22:18:38 2014
@@ -158,7 +158,7 @@ public class SolrXmlInZkTest extends Sol
       fail("Should have thrown an exception here");
     } catch (InvocationTargetException ite) {
       assertTrue("Should be failing to create default solr.xml in code",
-          ite.getTargetException().getCause().getMessage().indexOf("solr.xml does not exist") != -1);
+          ite.getCause().getMessage().contains("solr.xml does not exist"));
     } finally {
       closeZK();
     }

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java?rev=1612419&r1=1612418&r2=1612419&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java Mon Jul 21 22:18:38 2014
@@ -19,65 +19,77 @@ package org.apache.solr.core;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Random;
+import java.util.Locale;
 
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import com.google.common.base.Charsets;
 import org.apache.commons.io.FileUtils;
+import org.apache.lucene.util.TestUtil;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
 import org.junit.Rule;
-import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
-
 public class TestSolrXml extends SolrTestCaseJ4 {
 
   @Rule
   public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
-  private final File solrHome = createTempDir();
+  // tmp dir, cleanedup automaticly.
+  private static File solrHome = null;
+  private static SolrResourceLoader loader = null;
+
+  @BeforeClass
+  public static void setupLoader() throws Exception {
+    solrHome = createTempDir();
+    loader = new SolrResourceLoader(solrHome.getAbsolutePath());
+  }
+
+  @AfterClass
+  public static void cleanupLoader() throws Exception {
+    solrHome = null;
+    loader = null;
+  }
 
-  @Test
   public void testAllInfoPresent() throws IOException {
 
     File testSrcRoot = new File(SolrTestCaseJ4.TEST_HOME());
     FileUtils.copyFile(new File(testSrcRoot, "solr-50-all.xml"), new File(solrHome, "solr.xml"));
 
-    SolrResourceLoader loader = null;
-    try {
-      loader = new SolrResourceLoader(solrHome.getAbsolutePath());
-      ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
-
-      assertEquals("Did not find expected value", "testAdminHandler", cfg.get(ConfigSolr.CfgProp.SOLR_ADMINHANDLER, null));
-      assertEquals("Did not find expected value", "testCollectionsHandler", cfg.get(ConfigSolr.CfgProp.SOLR_COLLECTIONSHANDLER, null));
-      assertEquals("Did not find expected value", "testInfoHandler", cfg.get(ConfigSolr.CfgProp.SOLR_INFOHANDLER, null));
-      assertEquals("Did not find expected value", 11, cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, 0));
-      assertEquals("Did not find expected value", "testCoreRootDirectory", cfg.get(ConfigSolr.CfgProp.SOLR_COREROOTDIRECTORY, null));
-      assertEquals("Did not find expected value", 22, cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, 0));
-      assertEquals("Did not find expected value", 33, cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, 0));
-      assertEquals("Did not find expected value", 3, cfg.getInt(ConfigSolr.CfgProp.SOLR_MAXUPDATECONNECTIONS, 0));
-      assertEquals("Did not find expected value", 37, cfg.getInt(ConfigSolr.CfgProp.SOLR_MAXUPDATECONNECTIONSPERHOST, 0));
-      assertEquals("Did not find expected value", "testHost", cfg.get(ConfigSolr.CfgProp.SOLR_HOST, null));
-      assertEquals("Did not find expected value", "testHostContext", cfg.get(ConfigSolr.CfgProp.SOLR_HOSTCONTEXT, null));
-      assertEquals("Did not find expected value", 44, cfg.getInt(ConfigSolr.CfgProp.SOLR_HOSTPORT, 0));
-      assertEquals("Did not find expected value", 55, cfg.getInt(ConfigSolr.CfgProp.SOLR_LEADERVOTEWAIT, 0));
-      assertEquals("Did not find expected value", "testLoggingClass", cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_CLASS, null));
-      assertEquals("Did not find expected value", "testLoggingEnabled", cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_ENABLED, null));
-      assertEquals("Did not find expected value", 88, cfg.getInt(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_SIZE, 0));
-      assertEquals("Did not find expected value", 99, cfg.getInt(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, 0));
-      assertEquals("Did not find expected value", "testManagementPath", cfg.get(ConfigSolr.CfgProp.SOLR_MANAGEMENTPATH, null));
-      assertEquals("Did not find expected value", "testSharedLib", cfg.get(ConfigSolr.CfgProp.SOLR_SHAREDLIB, null));
-      assertEquals("Did not find expected value", "testShareSchema", cfg.get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, null));
-      assertEquals("Did not find expected value", 66, cfg.getInt(ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, 0));
-      assertEquals("Did not find expected value", 77, cfg.getInt(ConfigSolr.CfgProp.SOLR_ZKCLIENTTIMEOUT, 0));
-      assertEquals("Did not find expected value", "testZkHost", cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null));
-      assertNull("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_PERSISTENT, null));
-      assertNull("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, null));
-      assertNull("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, null));
-    }
-    finally {
-      loader.close();
-    }
-
+    ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
+    
+    assertEquals("core admin handler class", "testAdminHandler", cfg.getCoreAdminHandlerClass());
+    assertEquals("collection handler class", "testCollectionsHandler", cfg.getCollectionsHandlerClass());
+    assertEquals("info handler class", "testInfoHandler", cfg.getInfoHandlerClass());
+    assertEquals("core load threads", 11, cfg.getCoreLoadThreadCount());
+    assertEquals("core root dir", "testCoreRootDirectory", cfg.getCoreRootDirectory());
+    assertEquals("distrib conn timeout", 22, cfg.getDistributedConnectionTimeout());
+    assertEquals("distrib socket timeout", 33, cfg.getDistributedSocketTimeout());
+    assertEquals("max update conn", 3, cfg.getMaxUpdateConnections());
+    assertEquals("max update conn/host", 37, cfg.getMaxUpdateConnectionsPerHost());
+    assertEquals("host", "testHost", cfg.getHost());
+    assertEquals("zk host context", "testHostContext", cfg.getZkHostContext());
+    assertEquals("zk host port", "44", cfg.getZkHostPort());
+    assertEquals("leader vote wait", 55, cfg.getLeaderVoteWait());
+    assertEquals("logging class", "testLoggingClass", cfg.getLogWatcherConfig().getLoggingClass());
+    assertEquals("log watcher", true, cfg.getLogWatcherConfig().isEnabled());
+    assertEquals("log watcher size", 88, cfg.getLogWatcherConfig().getWatcherSize());
+    assertEquals("log watcher thresh", "99", cfg.getLogWatcherConfig().getWatcherThreshold());
+    assertEquals("manage path", "testManagementPath", cfg.getManagementPath());
+    assertEquals("shardLib", "testSharedLib", cfg.getSharedLibDirectory());
+    assertEquals("schema cache", true, cfg.hasSchemaCache());
+    assertEquals("trans cache size", 66, cfg.getTransientCacheSize());
+    assertEquals("zk client timeout", 77, cfg.getZkClientTimeout());
+    assertEquals("zk host", "testZkHost", cfg.getZkHost());
+    assertEquals("persistent", true, cfg.isPersistent());
+    assertEquals("core admin path", ConfigSolr.DEFAULT_CORE_ADMIN_PATH, cfg.getAdminPath());
   }
 
   // Test  a few property substitutions that happen to be in solr-50-all.xml.
@@ -85,24 +97,213 @@ public class TestSolrXml extends SolrTes
 
     System.setProperty("coreRootDirectory", "myCoreRoot");
     System.setProperty("hostPort", "8888");
-    System.setProperty("shareSchema", "newShareSchema");
+    System.setProperty("shareSchema", "false");
     System.setProperty("socketTimeout", "220");
     System.setProperty("connTimeout", "200");
 
     File testSrcRoot = new File(SolrTestCaseJ4.TEST_HOME());
     FileUtils.copyFile(new File(testSrcRoot, "solr-50-all.xml"), new File(solrHome, "solr.xml"));
 
-    SolrResourceLoader loader = null;
-    try {
-      loader = new SolrResourceLoader(solrHome.getAbsolutePath());
-      ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
-      assertEquals("Did not find expected value", "myCoreRoot", cfg.get(ConfigSolr.CfgProp.SOLR_COREROOTDIRECTORY, null));
-      assertEquals("Did not find expected value", 8888, cfg.getInt(ConfigSolr.CfgProp.SOLR_HOSTPORT, 0));
-      assertEquals("Did not find expected value", "newShareSchema", cfg.get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, null));
-    }
-    finally {
-      loader.close();
-    }
+    ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
+    assertEquals("core root dir", "myCoreRoot", cfg.getCoreRootDirectory());
+    assertEquals("zk host port", "8888", cfg.getZkHostPort());
+    assertEquals("schema cache", false, cfg.hasSchemaCache());
+  }
+
+  public void testExplicitNullGivesDefaults() throws IOException {
+    // 2 diff options, one where the default is in fact null, and one where it isn't
+    String solrXml = "<solr><solrcloud><null name=\"host\"/><null name=\"leaderVoteWait\"/></solrcloud></solr>";
+
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+    assertEquals("host", null, cfg.getHost());
+    assertEquals("leaderVoteWait", 180000, cfg.getLeaderVoteWait());
+  }
+
+  public void testIntAsLongBad() throws IOException {
+    String bad = ""+TestUtil.nextLong(random(), Integer.MAX_VALUE, Long.MAX_VALUE);
+    String solrXml = "<solr><long name=\"transientCacheSize\">"+bad+"</long></solr>";
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Value of '%s' can not be parsed as 'int'", bad));
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testIntAsLongOk() throws IOException {
+    int ok = random().nextInt();
+    String solrXml = "<solr><long name=\"transientCacheSize\">"+ok+"</long></solr>";
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+    assertEquals(ok, cfg.getTransientCacheSize());
+  }
+
+  public void testMultiCloudSectionError() throws IOException {
+    String solrXml = "<solr>"
+      + "<solrcloud><bool name=\"genericCoreNodeNames\">true</bool></solrcloud>"
+      + "<solrcloud><bool name=\"genericCoreNodeNames\">false</bool></solrcloud>"
+      + "</solr>";
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("2 instances of <solrcloud> found in solr.xml");
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testMultiLoggingSectionError() throws IOException {
+    String solrXml = "<solr>"
+      + "<logging><str name=\"class\">foo</str></logging>"
+      + "<logging><str name=\"class\">foo</str></logging>"
+      + "</solr>";
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("2 instances of <logging> found in solr.xml");
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testMultiLoggingWatcherSectionError() throws IOException {
+    String solrXml = "<solr><logging>"
+      + "<watcher><int name=\"threshold\">42</int></watcher>"
+      + "<watcher><int name=\"threshold\">42</int></watcher>"
+      + "<watcher><int name=\"threshold\">42</int></watcher>"
+      + "</logging></solr>";
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("3 instances of Logging <watcher> found in solr.xml");
+    ConfigSolr cfg = ConfigSolr.fromString(loader, solrXml);
+  }
+ 
+  public void testValidStringValueWhenBoolTypeIsExpected() throws IOException {
+    boolean genericCoreNodeNames = random().nextBoolean();
+    String solrXml = String.format(Locale.ROOT, "<solr><solrcloud><str name=\"genericCoreNodeNames\">%s</str></solrcloud></solr>", genericCoreNodeNames);
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+    assertEquals("gen core node names", genericCoreNodeNames, configSolr.getGenericCoreNodeNames());
+  }
+
+  public void testValidStringValueWhenIntTypeIsExpected() throws IOException {
+    int maxUpdateConnections = random().nextInt();
+    String solrXml = String.format(Locale.ROOT, "<solr><solrcloud><str name=\"maxUpdateConnections\">%d</str></solrcloud></solr>", maxUpdateConnections);
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+    assertEquals("max update conn", maxUpdateConnections, configSolr.getMaxUpdateConnections());
+  }
+
+  public void testFailAtConfigParseTimeWhenIntTypeIsExpectedAndLongTypeIsGiven() throws IOException {
+    long val = TestUtil.nextLong(random(), Integer.MAX_VALUE, Long.MAX_VALUE);
+    String solrXml = String.format(Locale.ROOT, "<solr><solrcloud><long name=\"maxUpdateConnections\">%d</long></solrcloud></solr>", val);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Value of '%d' can not be parsed as 'int'", val));
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenBoolTypeIsExpectedAndLongTypeIsGiven() throws IOException {
+    long val = random().nextLong();
+    String solrXml = String.format(Locale.ROOT, "<solr><solrcloud><long name=\"genericCoreNodeNames\">%d</long></solrcloud></solr>", val);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Value of '%d' can not be parsed as 'bool'", val));
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenBoolTypeIsExpectedAndDoubleTypeIsGiven() throws IOException {
+    String val = ""+random().nextDouble();
+    String solrXml = String.format(Locale.ROOT, "<solr><solrcloud><double name=\"genericCoreNodeNames\">%s</double></solrcloud></solr>", val);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Value of '%s' can not be parsed as 'bool'", val));
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenBoolTypeIsExpectedAndValueIsInvalidString() throws IOException {
+    String solrXml = "<solr><solrcloud><bool name=\"genericCoreNodeNames\">NOT_A_BOOLEAN</bool></solrcloud></solr>";
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("invalid boolean value: NOT_A_BOOLEAN");
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenIntTypeIsExpectedAndBoolTypeIsGiven() throws IOException {
+    // given:
+    boolean randomBoolean = random().nextBoolean();
+    String solrXml = String.format(Locale.ROOT, "<solr><logging><int name=\"unknown-option\">%s</int></logging></solr>", randomBoolean);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Value of 'unknown-option' can not be parsed as 'int': \"%s\"", randomBoolean));
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenUnrecognizedSolrCloudOptionWasFound() throws IOException {
+    String solrXml = "<solr><solrcloud><bool name=\"unknown-option\">true</bool></solrcloud></solr>";
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("<solrcloud> section of solr.xml contains 1 unknown config parameter(s): [unknown-option]");
+    
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenUnrecognizedSolrOptionWasFound() throws IOException {
+    String solrXml = "<solr><bool name=\"unknown-bool-option\">true</bool><str name=\"unknown-str-option\">true</str></solr>";
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("Main section of solr.xml contains 2 unknown config parameter(s): [unknown-bool-option, unknown-str-option]");
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenUnrecognizedLoggingOptionWasFound() throws IOException {
+    String solrXml = String.format(Locale.ROOT, "<solr><logging><bool name=\"unknown-option\">%s</bool></logging></solr>", random().nextBoolean());
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage("<logging> section of solr.xml contains 1 unknown config parameter(s): [unknown-option]");
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenLoggingConfigParamsAreDuplicated() throws IOException {
+    String v1 = ""+random().nextInt();
+    String v2 = ""+random().nextInt();
+    String solrXml = String.format(Locale.ROOT,
+                                   "<solr><logging>" +
+                                   "<str name=\"class\">%s</str>" +
+                                   "<str name=\"class\">%s</str>" +
+                                   "</logging></solr>",
+                                   v1, v2);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "<logging> section of solr.xml contains duplicated 'class' in solr.xml: [%s, %s]", v1, v2));
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenSolrCloudConfigParamsAreDuplicated() throws IOException {
+    String v1 = ""+random().nextInt();
+    String v2 = ""+random().nextInt();
+    String v3 = ""+random().nextInt();
+    String solrXml = String.format(Locale.ROOT,
+                                   "<solr><solrcloud>" +
+                                   "<int name=\"zkClientTimeout\">%s</int>" +
+                                   "<int name=\"zkClientTimeout\">%s</int>" +
+                                   "<str name=\"zkHost\">foo</str>" + // other ok val in middle
+                                   "<int name=\"zkClientTimeout\">%s</int>" +
+                                   "</solrcloud></solr>",
+                                   v1, v2, v3);
+    
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "<solrcloud> section of solr.xml contains duplicated 'zkClientTimeout' in solr.xml: [%s, %s, %s]", v1, v2, v3));
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
+  }
+
+  public void testFailAtConfigParseTimeWhenSolrConfigParamsAreDuplicated() throws IOException {
+    String v1 = ""+random().nextInt();
+    String v2 = ""+random().nextInt();
+    String solrXml = String.format(Locale.ROOT, 
+                                   "<solr>" +
+                                   "<int name=\"coreLoadThreads\">%s</int>" +
+                                   "<str name=\"coreLoadThreads\">%s</str>" +
+                                   "</solr>",
+                                   v1, v2);
+
+    expectedException.expect(SolrException.class);
+    expectedException.expectMessage(String.format(Locale.ROOT, "Main section of solr.xml contains duplicated 'coreLoadThreads' in solr.xml: [%s, %s]", v1, v2));
+
+    ConfigSolr configSolr = ConfigSolr.fromString(loader, solrXml);
   }
 }