You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2013/04/16 02:34:50 UTC

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

Author: erick
Date: Tue Apr 16 00:34:49 2013
New Revision: 1468284

URL: http://svn.apache.org/r1468284
Log:
Changes for SOLR-4662, finalizing the new form of solrxml

Added:
    lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml   (with props)
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java   (with props)
Removed:
    lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress.properties
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/CoreContainer.java
    lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
    lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress-new.xml
    lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java

Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1468284&r1=1468283&r2=1468284&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Tue Apr 16 00:34:49 2013
@@ -111,9 +111,6 @@ New Features
   there are a large number of entries in the multiValued field. Conspicuously, this will
   prevent the "best" match from being found if it appears later in the MV list than the
   cutoff specified by either of these params. (Erick Erickson)
-  
-* SOLR-4663: Complain loudly and do NOT load cores if they have the same data dir or the 
-  same name
 
 * SOLR-4675: Improve PostingsSolrHighlighter to support per-field/query-time overrides
   and add additional configuration parameters. See the javadocs for more details and
@@ -136,6 +133,16 @@ New Features
   Also fixed a problem whereby properties like solr.solr.datadir would be persisted
   to solr.xml. Also, cores that didn't happen to be loaded were not persisted. 
   (Erick Erickson)
+  
+* SOLR-4662: Finalize what we're going to do with solr.xml, auto-discovery, config sets.
+  The format of solr.xml is changing. <core> and <cores> tags are no longer supported as
+  of 5.0 (but will be supported for 4.x). defaultCoreName, persistent and adminPath are
+  obsolete in 5.0 as well. Discovery mode will be the way cores are enumerated in 5.0.
+  Supports a new value (<cores> attribute coreRootDirectory in 4.x, element in <solr> in
+  5.0) for basing the enumeration of cores. In the new way of doing things, a
+  core.propeties file will mark the instanceDir for that core, and instanceDir will
+  be obsolete as well
+   
 
 Bug Fixes
 ----------------------

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=1468284&r1=1468283&r2=1468284&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 Tue Apr 16 00:34:49 2013
@@ -37,18 +37,48 @@ import java.util.Properties;
  */
 public interface ConfigSolr {
 
-  public static enum ConfLevel {
-    SOLR, SOLR_CORES, SOLR_CORES_CORE, SOLR_LOGGING, SOLR_LOGGING_WATCHER
+  // Ugly for now, but we'll at least be able to centralize all of the differences between 4x and 5x.
+  public static enum CfgProp {
+    SOLR_ADMINHANDLER,
+    SOLR_CORELOADTHREADS,
+    SOLR_COREROOTDIRECTORY,
+    SOLR_DISTRIBUPDATECONNTIMEOUT,
+    SOLR_DISTRIBUPDATESOTIMEOUT,
+    SOLR_HOST,
+    SOLR_HOSTCONTEXT,
+    SOLR_HOSTPORT,
+    SOLR_LEADERVOTEWAIT,
+    SOLR_LOGGING_CLASS,
+    SOLR_LOGGING_ENABLED,
+    SOLR_LOGGING_WATCHER_SIZE,
+    SOLR_LOGGING_WATCHER_THRESHOLD,
+    SOLR_MANAGEMENTPATH,
+    SOLR_SHAREDLIB,
+    SOLR_SHARDHANDLERFACTORY_CLASS,
+    SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT,
+    SOLR_SHARDHANDLERFACTORY_NAME,
+    SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT,
+    SOLR_SHARESCHEMA,
+    SOLR_TRANSIENTCACHESIZE,
+    SOLR_ZKCLIENTTIMEOUT,
+    SOLR_ZKHOST,
+
+    //TODO: Remove all of these elements for 5.0
+    SOLR_PERSISTENT,
+    SOLR_CORES_DEFAULT_CORE_NAME,
+    SOLR_ADMINPATH
   };
 
   public final static String CORE_PROP_FILE = "core.properties";
   public final static String SOLR_XML_FILE = "solr.xml";
 
-  public int getInt(ConfLevel level, String tag, int def);
+  public int getInt(CfgProp prop, int def);
 
-  public boolean getBool(ConfLevel level, String tag, boolean defValue);
+  public boolean getBool(CfgProp prop,boolean defValue);
 
-  public String get(ConfLevel level, String tag, String def);
+  public String get(CfgProp prop, String def);
+
+  public String getOrigProp(CfgProp prop, String def);
 
   public void substituteProperties();
 
@@ -79,4 +109,6 @@ public interface ConfigSolr {
   // If the core is not to be loaded (say two cores defined with the same name or with the same data dir), return
   // the reason. If it's OK to load the core, return null.
   public String getBadCoreMessage(String name);
+
+  public boolean is50OrLater();
 }

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=1468284&r1=1468283&r2=1468284&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 Tue Apr 16 00:34:49 2013
@@ -62,8 +62,8 @@ import java.util.Set;
 /**
  * ConfigSolrXml
  * <p/>
- * This class is entirely to localize the backwards compatibility for dealing with specific issues when transitioning
- * from solr.xml to a solr.properties-based, enumeration/discovery of defined cores. See SOLR-4196 for background.
+ * This class is entirely to localize the dealing with specific issues when transitioning from old-style solr.xml
+ * to new-style xml, enumeration/discovery of defined cores. See SOLR-4196 for background.
  * <p/>
  * @since solr 4.3
  *
@@ -73,45 +73,181 @@ import java.util.Set;
  *
  */
 
+
 public class ConfigSolrXml extends Config implements ConfigSolr {
 
-  private static Map<ConfLevel, String> prefixes;
-  private boolean isAutoDiscover = false;
+  private boolean is50OrLater = false;
 
   private final Map<String, CoreDescriptorPlus> coreDescriptorPlusMap = new HashMap<String, CoreDescriptorPlus>();
   private NodeList coreNodes = null;
   private final Map<String, String> badCores = new HashMap<String, String>();
     // List of cores that we should _never_ load. Ones with dup names or duplicate datadirs or...
 
-  static {
-    prefixes = new HashMap<ConfLevel, String>();
 
-    prefixes.put(ConfLevel.SOLR, "solr/@");
-    prefixes.put(ConfLevel.SOLR_CORES, "solr/cores/@");
-    prefixes.put(ConfLevel.SOLR_CORES_CORE, "solr/cores/core/@");
-    prefixes.put(ConfLevel.SOLR_LOGGING, "solr/logging/@");
-    prefixes.put(ConfLevel.SOLR_LOGGING_WATCHER, "solr/logging/watcher/@");
-  }
+  private Map<CfgProp, String> propMap = new HashMap<CfgProp, String>();
 
   public ConfigSolrXml(SolrResourceLoader loader, String name, InputStream is, String prefix,
-                       boolean subProps, CoreContainer container) throws ParserConfigurationException, IOException, SAXException {
+                       boolean subProps, CoreContainer container)
+      throws ParserConfigurationException, IOException, SAXException {
+
     super(loader, name, new InputSource(is), prefix, subProps);
-    initCoreList(container);
+    init(container);
   }
 
 
-  public ConfigSolrXml(SolrResourceLoader loader, Config cfg, CoreContainer container) throws TransformerException, IOException {
+  public ConfigSolrXml(SolrResourceLoader loader, Config cfg, CoreContainer container)
+      throws TransformerException, IOException {
+
     super(loader, null, copyDoc(cfg.getDocument())); // Mimics a call from CoreContainer.
+    init(container);
+  }
+  private void init(CoreContainer container) throws IOException {
+    is50OrLater = getNode("solr/cores", false) == null;
+
+    // Do sanity checks, old and new style. Pretty exhaustive for now, but want to hammer this.
+    // TODO: 5.0 maybe remove this checking, it's mostly for correctness as we make this transition.
+
+    if (is50OrLater()) {
+      insureFail("solr/@coreLoadThreads");
+      insureFail("solr/@persist");
+      insureFail("solr/@sharedLib");
+      insureFail("solr/@zkHost");
+
+      insureFail("solr/logging/@class");
+      insureFail("solr/logging/@enabled");
+      insureFail("solr/logging/watcher/@size");
+      insureFail("solr/logging/watcher/@threshold");
+
+      insureFail("solr/cores/@adminHandler");
+      insureFail("solr/cores/@distribUpdateConnTimeout");
+      insureFail("solr/cores/@distribUpdateSoTimeout");
+      insureFail("solr/cores/@host");
+      insureFail("solr/cores/@hostContext");
+      insureFail("solr/cores/@hostPort");
+      insureFail("solr/cores/@leaderVoteWait");
+      insureFail("solr/cores/@managementPath");
+      insureFail("solr/cores/@shareSchema");
+      insureFail("solr/cores/@transientCacheSize");
+      insureFail("solr/cores/@zkClientTimeout");
+
+      // These have no counterpart in 5.0, asking for any o fthese in Solr 5.0 will result in an error being
+      // thrown.
+      insureFail("solr/cores/@defaultCoreName");
+      insureFail("solr/@persistent");
+      insureFail("solr/cores/@adminPath");
+    } else {
+      insureFail("solr/str[@name='adminHandler']");
+      insureFail("solr/int[@name='coreLoadThreads']");
+      insureFail("solr/str[@name='coreRootDirectory']");
+      insureFail("solr/solrcloud/int[@name='distribUpdateConnTimeout']");
+      insureFail("solr/solrcloud/int[@name='distribUpdateSoTimeout']");
+      insureFail("solr/solrcloud/str[@name='host']");
+      insureFail("solr/solrcloud/str[@name='hostContext']");
+      insureFail("solr/solrcloud/int[@name='hostPort']");
+      insureFail("solr/solrcloud/int[@name='leaderVoteWait']");
+      insureFail("solr/str[@name='managementPath']");
+      insureFail("solr/str[@name='sharedLib']");
+      insureFail("solr/str[@name='shareSchema']");
+      insureFail("solr/int[@name='transientCacheSize']");
+      insureFail("solr/solrcloud/int[@name='zkClientTimeout']");
+      insureFail("solr/solrcloud/int[@name='zkHost']");
+
+      insureFail("solr/logging/str[@name='class']");
+      insureFail("solr/logging/str[@name='enabled']");
+
+      insureFail("solr/logging/watcher/int[@name='size']");
+      insureFail("solr/logging/watcher/int[@name='threshold']");
+
+    }
+    fillPropMap();
     initCoreList(container);
   }
+  private void insureFail(String xPath) {
+
+    if (getVal(xPath, false) != null) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Should not have found " + xPath +
+          " solr.xml may be a mix of old and new style formats.");
+    }
+  }
 
+  // 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 = getVal(path, false);
+    if (val != null) {
+      val = PropertiesUtil.substituteProperty(val, null);
+    }
+    return val;
+  }
+  private void fillPropMap() {
+    if (is50OrLater) { // Can do the prop subs early here since we don't need to preserve them for persistence.
+      propMap.put(CfgProp.SOLR_ADMINHANDLER, doSub("solr/str[@name='adminHandler']"));
+      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_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_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_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']"));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_CLASS, doSub("solr/shardHandlerFactory/@class"));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_NAME, doSub("solr/shardHandlerFactory/@name"));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, doSub("solr/shardHandlerFactory/int[@name='connTimeout']"));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, doSub("solr/shardHandlerFactory/int[@name='socketTimeout']"));
+    } else {
+      propMap.put(CfgProp.SOLR_CORELOADTHREADS, getVal("solr/@coreLoadThreads", false));
+      propMap.put(CfgProp.SOLR_SHAREDLIB, getVal("solr/@sharedLib", false));
+      propMap.put(CfgProp.SOLR_ZKHOST, getVal("solr/@zkHost", false));
+
+      propMap.put(CfgProp.SOLR_LOGGING_CLASS, getVal("solr/logging/@class", false));
+      propMap.put(CfgProp.SOLR_LOGGING_ENABLED, getVal("solr/logging/@enabled", false));
+      propMap.put(CfgProp.SOLR_LOGGING_WATCHER_SIZE, getVal("solr/logging/watcher/@size", false));
+      propMap.put(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, getVal("solr/logging/watcher/@threshold", false));
+
+      propMap.put(CfgProp.SOLR_ADMINHANDLER, getVal("solr/cores/@adminHandler", false));
+      propMap.put(CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, getVal("solr/cores/@distribUpdateConnTimeout", false));
+      propMap.put(CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, getVal("solr/cores/@distribUpdateSoTimeout", false));
+      propMap.put(CfgProp.SOLR_HOST, getVal("solr/cores/@host", false));
+      propMap.put(CfgProp.SOLR_HOSTCONTEXT, getVal("solr/cores/@hostContext", false));
+      propMap.put(CfgProp.SOLR_HOSTPORT, getVal("solr/cores/@hostPort", false));
+      propMap.put(CfgProp.SOLR_LEADERVOTEWAIT, getVal("solr/cores/@leaderVoteWait", false));
+      propMap.put(CfgProp.SOLR_MANAGEMENTPATH, getVal("solr/cores/@managementPath", false));
+      propMap.put(CfgProp.SOLR_SHARESCHEMA, getVal("solr/cores/@shareSchema", false));
+      propMap.put(CfgProp.SOLR_TRANSIENTCACHESIZE, getVal("solr/cores/@transientCacheSize", false));
+      propMap.put(CfgProp.SOLR_ZKCLIENTTIMEOUT, getVal("solr/cores/@zkClientTimeout", false));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_CLASS, getVal("solr/shardHandlerFactory/@class", false));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_NAME, getVal("solr/shardHandlerFactory/@name", false));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, getVal(  "solr/shardHandlerFactory/int[@connTimeout]", false));
+      propMap.put(CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, getVal("solr/shardHandlerFactory/int[@socketTimeout]", false));
+
+      // These have no counterpart in 5.0, asking, for any o, fthese in Solr 5.0 will result in an error being
+      // thrown.
+      propMap.put(CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, getVal("solr/cores/@defaultCoreName", false));
+      propMap.put(CfgProp.SOLR_PERSISTENT, getVal("solr/@persistent", false));
+      propMap.put(CfgProp.SOLR_ADMINPATH, getVal("solr/cores/@adminPath", false));
+    }
+  }
+
+  //NOTE:
   public void initCoreList(CoreContainer container) throws IOException {
-    isAutoDiscover = getBool(ConfigSolr.ConfLevel.SOLR_CORES, "autoDiscoverCores", false);
-    if (isAutoDiscover) {
-      synchronized (coreDescriptorPlusMap) {
-        walkFromHere(new File(container.getSolrHome()), container, new HashMap<String, CoreDescriptorPlus>());
+    if (is50OrLater) {
+      if (container != null) { //TODO: 5.0. Yet another bit of nonsense only because of the test harness.
+        synchronized (coreDescriptorPlusMap) {
+          String coreRoot = get(CfgProp.SOLR_COREROOTDIRECTORY, container.getSolrHome());
+          walkFromHere(new File(coreRoot), container, new HashMap<String, String>(), new HashMap<String, String>());
+        }
       }
-
     } else {
       coreNodes = (NodeList) evaluate("solr/cores/core",
           XPathConstants.NODESET);
@@ -160,21 +296,37 @@ public class ConfigSolrXml extends Confi
     return (Document) result.getNode();
   }
 
+  //TODO: For 5.0, you shouldn't have to do the sbustituteProperty, this is anothe bit
+  // of awkward back-compat due to persistence.
   @Override
-  public int getInt(ConfLevel level, String tag, int def) {
-    return getInt(prefixes.get(level) + tag, def);
+  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);
   }
 
   @Override
-  public boolean getBool(ConfLevel level, String tag, boolean defValue) {
-    return getBool(prefixes.get(level) + tag, defValue);
+  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);
   }
 
   @Override
-  public String get(ConfLevel level, String tag, String def) {
-    return get(prefixes.get(level) + tag, def);
+  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;
   }
 
+  // For saving the original property, ${} syntax and all.
+  @Override
+  public String getOrigProp(CfgProp prop, String def) {
+    String val = propMap.get(prop);
+    return (val == null) ? def : val;
+  }
+
+
   public ShardHandlerFactory initShardHandler() {
     PluginInfo info = null;
     Node shfn = getNode("solr/cores/shardHandlerFactory", false);
@@ -227,7 +379,7 @@ public class ConfigSolrXml extends Confi
   public Map<String, String> readCoreAttributes(String coreName) {
     Map<String, String> attrs = new HashMap<String, String>();
 
-    if (isAutoDiscover) {
+    if (is50OrLater) {
       return attrs; // this is a no-op.... intentionally
     }
     synchronized (coreNodes) {
@@ -257,65 +409,100 @@ public class ConfigSolrXml extends Confi
   // deeper in the tree.
   //
   // @param file - the directory we're to either read the properties file from or recurse into.
-  private void walkFromHere(File file, CoreContainer container, Map<String, CoreDescriptorPlus> checkMap) throws IOException {
+  private void walkFromHere(File file, CoreContainer container, Map<String, String>seenDirs, HashMap<String, String> seenCores)
+      throws IOException {
     log.info("Looking for cores in " + file.getAbsolutePath());
+    if (! file.exists()) return;
+
     for (File childFile : file.listFiles()) {
       // This is a little tricky, we are asking if core.properties exists in a child directory of the directory passed
       // in. In other words we're looking for core.properties in the grandchild directories of the parameter passed
       // in. That allows us to gracefully top recursing deep but continue looking wide.
       File propFile = new File(childFile, CORE_PROP_FILE);
       if (propFile.exists()) { // Stop looking after processing this file!
-        log.info("Discovered properties file {}, adding to cores", propFile.getAbsolutePath());
-        Properties propsOrig = new Properties();
-        InputStream is = new FileInputStream(propFile);
-        try {
-          propsOrig.load(is);
-        } finally {
-          IOUtils.closeQuietly(is);
-        }
-
-        Properties props = new Properties();
-        for (String prop : propsOrig.stringPropertyNames()) {
-          props.put(prop, PropertiesUtil.substituteProperty(propsOrig.getProperty(prop), null));
-        }
-
-        if (props.getProperty(CoreDescriptor.CORE_INSTDIR) == null) {
-          props.setProperty(CoreDescriptor.CORE_INSTDIR, childFile.getPath());
-        }
-
-        if (props.getProperty(CoreDescriptor.CORE_NAME) == null) {
-          // Should default to this directory
-          props.setProperty(CoreDescriptor.CORE_NAME, childFile.getName());
-        }
-        CoreDescriptor desc = new CoreDescriptor(container, props);
-        CoreDescriptorPlus plus = new CoreDescriptorPlus(propFile.getAbsolutePath(), desc, propsOrig);
-        CoreDescriptorPlus check = coreDescriptorPlusMap.get(desc.getName());
-        if (check == null) { // It's bad to have two cores with the same name
-          coreDescriptorPlusMap.put(desc.getName(), plus);
-        } else {
-          String msg = String.format(Locale.ROOT, "More than one core defined for core named %s, paths are '%s' and '%s' ",
-              desc.getName(), check.getFilePath(), plus.getFilePath());
-          log.error(msg);
-          badCores.put(desc.getName(), msg);
-        }
-        check = coreDescriptorPlusMap.get(plus.getCoreDescriptor().getDataDir());
-        if (check == null) {
-          coreDescriptorPlusMap.put(desc.getName(), plus);
-        } else {
-          String msg = String.format(Locale.ROOT, "More than one core points to data dir %s. They are in %s and %s",
-              plus.getCoreDescriptor().getDataDir(), plus.getFilePath(), check.getFilePath());
-          log.error(msg);
-          badCores.put(plus.getCoreDescriptor().getName(), msg);
-        }
-
-        continue; // Go on to the sibling directory
+        addCore(container, seenDirs, seenCores, childFile, propFile);
+        continue; // Go on to the sibling directory, don't descend any deeper.
       }
       if (childFile.isDirectory()) {
-        walkFromHere(childFile, container, checkMap);
+        walkFromHere(childFile, container, seenDirs, seenCores);
       }
     }
   }
 
+  private void addCore(CoreContainer container, Map<String, String> seenDirs, Map<String, String> seenCores,
+                       File childFile, File propFile) throws IOException {
+    log.info("Discovered properties file {}, adding to cores", propFile.getAbsolutePath());
+    Properties propsOrig = new Properties();
+    InputStream is = new FileInputStream(propFile);
+    try {
+      propsOrig.load(is);
+    } finally {
+      IOUtils.closeQuietly(is);
+    }
+
+    Properties props = new Properties();
+    for (String prop : propsOrig.stringPropertyNames()) {
+      props.put(prop, PropertiesUtil.substituteProperty(propsOrig.getProperty(prop), null));
+    }
+
+    // Too much of the code depends on this value being here, but it is NOT supported in discovery mode, so
+    // ignore it if present in the core.properties file.
+    props.setProperty(CoreDescriptor.CORE_INSTDIR, childFile.getPath());
+
+    if (props.getProperty(CoreDescriptor.CORE_NAME) == null) {
+      // Should default to this directory
+      props.setProperty(CoreDescriptor.CORE_NAME, childFile.getName());
+    }
+    CoreDescriptor desc = new CoreDescriptor(container, props);
+    CoreDescriptorPlus plus = new CoreDescriptorPlus(propFile.getAbsolutePath(), desc, propsOrig);
+
+    // It's bad to have two cores with the same name or same data dir.
+    if (! seenCores.containsKey(desc.getName()) && ! seenDirs.containsKey(desc.getAbsoluteDataDir())) {
+      coreDescriptorPlusMap.put(desc.getName(), plus);
+      // Use the full path to the prop file so we can unambiguously report the place the error is.
+      seenCores.put(desc.getName(), propFile.getAbsolutePath());
+      seenDirs.put(desc.getAbsoluteDataDir(), propFile.getAbsolutePath());
+      return;
+    }
+
+    // record the appropriate error
+    if (seenCores.containsKey(desc.getName())) {
+      String msg = String.format(Locale.ROOT, "More than one core defined for core named '%s', paths are '%s' and '%s'  Removing both cores.",
+          desc.getName(), propFile.getAbsolutePath(), seenCores.get(desc.getName()));
+      log.error(msg);
+      // Load up as many errors as there are.
+      if (badCores.containsKey(desc.getName())) msg += " " + badCores.get(desc.getName());
+      badCores.put(desc.getName(), msg);
+    }
+    // There's no reason both errors may not have occurred.
+    if (seenDirs.containsKey(desc.getAbsoluteDataDir())) {
+      String msg = String.format(Locale.ROOT, "More than one core points to data dir '%s'. They are in '%s' and '%s'. Removing all offending cores.",
+          desc.getAbsoluteDataDir(), propFile.getAbsolutePath(), seenDirs.get(desc.getAbsoluteDataDir()));
+      if (badCores.containsKey(desc.getName())) msg += " " + badCores.get(desc.getName());
+      log.error(msg);
+      badCores.put(desc.getName(), msg);
+
+      // find the core with this datadir and remove it
+      List<String> badNames = new ArrayList<String>();
+      for (Map.Entry<String, CoreDescriptorPlus> ent : coreDescriptorPlusMap.entrySet()) {
+        if (ent.getValue().getCoreDescriptor().getAbsoluteDataDir().equals(desc.getAbsoluteDataDir())) {
+          badNames.add(ent.getKey());
+          if (! badCores.containsKey(ent.getKey())) {
+            // Record that the first core is also a bad core.
+            badCores.put(ent.getKey(), msg);
+            log.error(msg);
+
+          }
+          break;
+        }
+      }
+      for (String badName : badNames) {
+        coreDescriptorPlusMap.remove(badName);
+      }
+    }
+    coreDescriptorPlusMap.remove(desc.getName());
+  }
+
   public IndexSchema getSchemaFromZk(ZkController zkController, String zkConfigName, String schemaName,
                                      SolrConfig config)
       throws KeeperException, InterruptedException {
@@ -383,7 +570,7 @@ public class ConfigSolrXml extends Confi
   @Override
   public String getCoreNameFromOrig(String origCoreName, SolrResourceLoader loader, String coreName) {
 
-    if (isAutoDiscover) {
+    if (is50OrLater) {
       // first look for an exact match
       for (Map.Entry<String, CoreDescriptorPlus> ent : coreDescriptorPlusMap.entrySet()) {
 
@@ -445,7 +632,7 @@ public class ConfigSolrXml extends Confi
   @Override
   public List<String> getAllCoreNames() {
     List<String> ret = new ArrayList<String>();
-    if (isAutoDiscover) {
+    if (is50OrLater) {
       ret = new ArrayList<String>(coreDescriptorPlusMap.keySet());
     } else {
       synchronized (coreNodes) {
@@ -460,7 +647,7 @@ public class ConfigSolrXml extends Confi
 
   @Override
   public String getProperty(String coreName, String property, String defaultVal) {
-    if (isAutoDiscover) {
+    if (is50OrLater) {
       CoreDescriptorPlus plus = coreDescriptorPlusMap.get(coreName);
       if (plus == null) return defaultVal;
       CoreDescriptor desc = plus.getCoreDescriptor();
@@ -481,7 +668,7 @@ public class ConfigSolrXml extends Confi
 
   @Override
   public Properties readCoreProperties(String coreName) {
-    if (isAutoDiscover) {
+    if (is50OrLater) {
       CoreDescriptorPlus plus = coreDescriptorPlusMap.get(coreName);
       if (plus == null) return null;
       return new Properties(plus.getCoreDescriptor().getCoreProperties());
@@ -502,6 +689,9 @@ public class ConfigSolrXml extends Confi
     return null;
   }
 
+  @Override
+  public boolean is50OrLater() { return is50OrLater; }
+
   static Properties getCoreProperties(String instanceDir, CoreDescriptor dcore) {
     String file = dcore.getPropertiesName();
     if (file == null) file = "conf" + File.separator + "solrcore.properties";
@@ -560,7 +750,7 @@ public class ConfigSolrXml extends Confi
 class CoreDescriptorPlus {
   private CoreDescriptor coreDescriptor;
   private String filePath;
-  private Properties propsOrig;
+  private Properties propsOrig; // TODO: 5.0. Remove this since it's only really used for persisting.
 
   CoreDescriptorPlus(String filePath, CoreDescriptor descriptor, Properties propsOrig) {
     coreDescriptor = descriptor;

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java?rev=1468284&r1=1468283&r2=1468284&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreContainer.java Tue Apr 16 00:34:49 2013
@@ -382,7 +382,7 @@ public class CoreContainer
 
     ConfigSolr cfg;
     
-    // keep orig config for persist to consult
+    // keep orig config for persist to consult. TODO: Remove this silly stuff for 5.0, persistence not supported.
     try {
       cfg = new ConfigSolrXml(loader, null, is, null, false, this);
       this.cfg = new ConfigSolrXml(loader, (ConfigSolrXml) cfg, this);
@@ -394,7 +394,7 @@ public class CoreContainer
     cfg.substituteProperties();
 
     // add the sharedLib to the shared resource loader before initializing cfg based plugins
-    libDir = cfg.get(ConfigSolr.ConfLevel.SOLR, "sharedLib", null);
+    libDir = cfg.get(ConfigSolr.CfgProp.SOLR_SHAREDLIB , null);
     if (libDir != null) {
       File f = FileUtils.resolvePath(new File(dir), libDir);
       log.info("loading shared library: " + f.getAbsolutePath());
@@ -407,9 +407,9 @@ public class CoreContainer
     coreMaps.allocateLazyCores(cfg, loader);
 
     // Initialize Logging
-    if (cfg.getBool(ConfigSolr.ConfLevel.SOLR_LOGGING, "enabled", true)) {
+    if (cfg.getBool(ConfigSolr.CfgProp.SOLR_LOGGING_ENABLED, true)) {
       String slf4jImpl = null;
-      String fname = cfg.get(ConfigSolr.ConfLevel.SOLR_LOGGING, "class", null);
+      String fname = cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_CLASS, null);
       try {
         slf4jImpl = StaticLoggerBinder.getSingleton()
             .getLoggerFactoryClassStr();
@@ -441,8 +441,8 @@ public class CoreContainer
         
         if (logging != null) {
           ListenerConfig v = new ListenerConfig();
-          v.size = cfg.getInt(ConfigSolr.ConfLevel.SOLR_LOGGING_WATCHER, "size", 50);
-          v.threshold = cfg.get(ConfigSolr.ConfLevel.SOLR_LOGGING_WATCHER, "threshold", null);
+          v.size = cfg.getInt(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_SIZE, 50);
+          v.threshold = cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, null);
           if (v.size > 0) {
             log.info("Registering Log Listener");
             logging.registerListener(v, this);
@@ -450,38 +450,43 @@ public class CoreContainer
         }
       }
     }
+
+
+    if (! cfg.is50OrLater()) { //TODO: Remove for 5.0
+      String dcoreName = cfg.get(ConfigSolr.CfgProp.SOLR_CORES_DEFAULT_CORE_NAME, null);
+      if (dcoreName != null && !dcoreName.isEmpty()) {
+        defaultCoreName = dcoreName;
+      }
+      persistent = cfg.getBool(ConfigSolr.CfgProp.SOLR_PERSISTENT, false);
+      adminPath = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINPATH, null);
+    }
+    zkHost = cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null);
+    coreLoadThreads = cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, CORE_LOAD_THREADS);
     
-    String dcoreName = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "defaultCoreName", null);
-    if (dcoreName != null && !dcoreName.isEmpty()) {
-      defaultCoreName = dcoreName;
-    }
-    persistent = cfg.getBool(ConfigSolr.ConfLevel.SOLR, "persistent", false);
-    zkHost = cfg.get(ConfigSolr.ConfLevel.SOLR, "zkHost", null);
-    coreLoadThreads = cfg.getInt(ConfigSolr.ConfLevel.SOLR, "coreLoadThreads", CORE_LOAD_THREADS);
-    
-    adminPath = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "adminPath", null);
-    shareSchema = cfg.getBool(ConfigSolr.ConfLevel.SOLR_CORES, "shareSchema", DEFAULT_SHARE_SCHEMA);
-    zkClientTimeout = cfg.getInt(ConfigSolr.ConfLevel.SOLR_CORES, "zkClientTimeout", DEFAULT_ZK_CLIENT_TIMEOUT);
+
+    shareSchema = cfg.getBool(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, DEFAULT_SHARE_SCHEMA);
+    zkClientTimeout = cfg.getInt(ConfigSolr.CfgProp.SOLR_ZKCLIENTTIMEOUT, DEFAULT_ZK_CLIENT_TIMEOUT);
     
-    distribUpdateConnTimeout = cfg.getInt(ConfigSolr.ConfLevel.SOLR_CORES, "distribUpdateConnTimeout", 0);
-    distribUpdateSoTimeout = cfg.getInt(ConfigSolr.ConfLevel.SOLR_CORES, "distribUpdateSoTimeout", 0);
+    distribUpdateConnTimeout = cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, 0);
+    distribUpdateSoTimeout = cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, 0);
 
     // Note: initZooKeeper will apply hardcoded default if cloud mode
-    hostPort = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "hostPort", null);
+    hostPort = cfg.get(ConfigSolr.CfgProp.SOLR_HOSTPORT, null);
     // Note: initZooKeeper will apply hardcoded default if cloud mode
-    hostContext = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "hostContext", null);
+    hostContext = cfg.get(ConfigSolr.CfgProp.SOLR_HOSTCONTEXT, null);
 
-    host = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "host", null);
+    host = cfg.get(ConfigSolr.CfgProp.SOLR_HOST, null);
     
-    leaderVoteWait = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "leaderVoteWait", LEADER_VOTE_WAIT);
+    leaderVoteWait = cfg.get(ConfigSolr.CfgProp.SOLR_LEADERVOTEWAIT, LEADER_VOTE_WAIT);
+
+    adminHandler = cfg.get(ConfigSolr.CfgProp.SOLR_ADMINHANDLER, null);
+    managementPath = cfg.get(ConfigSolr.CfgProp.SOLR_MANAGEMENTPATH, null);
+
+    transientCacheSize = cfg.getInt(ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, Integer.MAX_VALUE);
 
     if (shareSchema) {
       indexSchemaCache = new ConcurrentHashMap<String,IndexSchema>();
     }
-    adminHandler = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "adminHandler", null);
-    managementPath = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, "managementPath", null);
-
-    transientCacheSize = cfg.getInt(ConfigSolr.ConfLevel.SOLR_CORES, "transientCacheSize", Integer.MAX_VALUE);
 
     zkClientTimeout = Integer.parseInt(System.getProperty("zkClientTimeout",
         Integer.toString(zkClientTimeout)));
@@ -1336,7 +1341,10 @@ public class CoreContainer
   }
 
   /** Persists the cores config file in a user provided file. */
+  //TODO: obsolete in SOLR 5.0
   public void persistFile(File file) {
+    if (cfg != null && cfg.is50OrLater()) return;
+
     log.info("Persisting cores config to " + (file == null ? configFile : file));
 
     
@@ -1347,27 +1355,31 @@ public class CoreContainer
     
     // <solr attrib="value"> <cores attrib="value">
     Map<String,String> coresAttribs = new HashMap<String,String>();
-    addCoresAttrib(coresAttribs, "adminPath", this.adminPath, null);
-    addCoresAttrib(coresAttribs, "adminHandler", this.adminHandler, null);
-    addCoresAttrib(coresAttribs, "shareSchema",
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ADMINPATH, "adminPath", this.adminPath, null);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ADMINHANDLER, "adminHandler", this.adminHandler, null);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_SHARESCHEMA,"shareSchema",
         Boolean.toString(this.shareSchema),
         Boolean.toString(DEFAULT_SHARE_SCHEMA));
-    addCoresAttrib(coresAttribs, "host", this.host, null);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOST, "host", this.host, null);
 
     if (! (null == defaultCoreName || defaultCoreName.equals("")) ) {
       coresAttribs.put("defaultCoreName", defaultCoreName);
     }
 
-    addCoresAttrib(coresAttribs, "hostPort", this.hostPort, DEFAULT_HOST_PORT);
-    addCoresAttrib(coresAttribs, "zkClientTimeout",
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOSTPORT, "hostPort", this.hostPort, DEFAULT_HOST_PORT);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ZKCLIENTTIMEOUT, "zkClientTimeout",
         intToString(this.zkClientTimeout),
         Integer.toString(DEFAULT_ZK_CLIENT_TIMEOUT));
-    addCoresAttrib(coresAttribs, "hostContext", this.hostContext, DEFAULT_HOST_CONTEXT);
-    addCoresAttrib(coresAttribs, "leaderVoteWait", this.leaderVoteWait, LEADER_VOTE_WAIT);
-    addCoresAttrib(coresAttribs, "coreLoadThreads", Integer.toString(this.coreLoadThreads), Integer.toString(CORE_LOAD_THREADS));
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOSTCONTEXT, "hostContext",
+        this.hostContext, DEFAULT_HOST_CONTEXT);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_LEADERVOTEWAIT, "leaderVoteWait",
+        this.leaderVoteWait, LEADER_VOTE_WAIT);
+    addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, "coreLoadThreads",
+        Integer.toString(this.coreLoadThreads), Integer.toString(CORE_LOAD_THREADS));
     if (transientCacheSize != Integer.MAX_VALUE) { // This test
     // is a consequence of testing. I really hate it.
-      addCoresAttrib(coresAttribs, "transientCacheSize", Integer.toString(this.transientCacheSize), Integer.toString(Integer.MAX_VALUE));
+      addCoresAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, "transientCacheSize",
+          Integer.toString(this.transientCacheSize), Integer.toString(Integer.MAX_VALUE));
     }
 
     coreMaps.persistCores(cfg, containerProperties, rootSolrAttribs, coresAttribs, file, configFile, loader);
@@ -1378,18 +1390,21 @@ public class CoreContainer
     return Integer.toString(integer);
   }
 
-  private void addCoresAttrib(Map<String,String> coresAttribs, String attribName, String attribValue, String defaultValue) {
+  //TODO: Obsolete in 5.0 Having to pass prop is a hack to get us to 5.0.
+  private void addCoresAttrib(Map<String,String> coresAttribs, ConfigSolr.CfgProp prop,
+                              String attribName, String attribValue, String defaultValue) {
     if (cfg == null) {
       coresAttribs.put(attribName, attribValue);
       return;
     }
     
     if (attribValue != null) {
-      String rawValue = cfg.get(ConfigSolr.ConfLevel.SOLR_CORES, attribName, null);
-      if (rawValue == null && defaultValue != null && attribValue.equals(defaultValue)) return;
+      String origValue = cfg.getOrigProp(prop, null);
+
+      if (origValue == null && defaultValue != null && attribValue.equals(defaultValue)) return;
 
-      if (attribValue.equals(PropertiesUtil.substituteProperty(rawValue, loader.getCoreProperties()))) {
-        coresAttribs.put(attribName, rawValue);
+      if (attribValue.equals(PropertiesUtil.substituteProperty(origValue, loader.getCoreProperties()))) {
+        coresAttribs.put(attribName, origValue);
       } else {
         coresAttribs.put(attribName, attribValue);
       }
@@ -1434,6 +1449,11 @@ public class CoreContainer
   String getCoreToOrigName(SolrCore core) {
     return coreMaps.getCoreToOrigName(core);
   }
+
+  public String getBadCoreMessage(String name) {
+    return cfg.getBadCoreMessage(name);
+  }
+
 }
 
 
@@ -1476,7 +1496,7 @@ class CoreMaps {
   // Trivial helper method for load, note it implements LRU on transient cores. Also note, if
   // there is no setting for max size, nothing is done and all cores go in the regular "cores" list
   protected void allocateLazyCores(final ConfigSolr cfg, final SolrResourceLoader loader) {
-    final int transientCacheSize = cfg.getInt(ConfigSolr.ConfLevel.SOLR_CORES, "transientCacheSize", Integer.MAX_VALUE);
+    final int transientCacheSize = cfg.getInt(ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, Integer.MAX_VALUE);
     if (transientCacheSize != Integer.MAX_VALUE) {
       CoreContainer.log.info("Allocating transient cache for {} transient cores", transientCacheSize);
       transientCores = new LinkedHashMap<String, SolrCore>(transientCacheSize, 0.75f, true) {

Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java?rev=1468284&r1=1468283&r2=1468284&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/core/CoreDescriptor.java Tue Apr 16 00:34:49 2013
@@ -334,4 +334,22 @@ public class CoreDescriptor {
   public void putProperty(String prop, String val) {
     coreProperties.put(prop, val);
   }
+
+  // This is particularly useful for checking if any two cores have the same
+  // data dir.
+  public String getAbsoluteDataDir() {
+    String dataDir = getDataDir();
+    if (dataDir == null) return null; // No worse than before.
+
+    if (new File(dataDir).isAbsolute()) {
+      return SolrResourceLoader.normalizeDir(
+          SolrResourceLoader.normalizeDir(dataDir));
+    }
+
+    if (coreContainer == null) return null;
+
+    return SolrResourceLoader.normalizeDir(coreContainer.getSolrHome() +
+        SolrResourceLoader.normalizeDir(dataDir));
+
+  }
 }

Added: 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=1468284&view=auto
==============================================================================
--- lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml (added)
+++ lucene/dev/trunk/solr/core/src/test-files/solr/solr-50-all.xml Tue Apr 16 00:34:49 2013
@@ -0,0 +1,52 @@
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+  -->
+
+<solr>
+  <str name="adminHandler">testAdminHandler</str>
+  <int name="coreLoadThreads">11</int>
+  <str name="coreRootDirectory">${coreRootDirectory:testCoreRootDirectory}</str>
+  <str name="managementPath">testManagementPath</str>
+  <str name="sharedLib">testSharedLib</str>
+  <str name="shareSchema">${shareSchema:testShareSchema}</str>
+  <int name="transientCacheSize">66</int>
+
+  <solrcloud>
+    <int name="distribUpdateConnTimeout">22</int>
+    <int name="distribUpdateSoTimeout">33</int>
+    <int name="leaderVoteWait">55</int>
+    <str name="host">testHost</str>
+    <str name="hostContext">testHostContext</str>
+    <int name="hostPort">${hostPort:44}</int>
+    <int name="zkClientTimeout">77</int>
+    <str name="zkHost">testZkHost</str>
+  </solrcloud>
+
+  <logging>
+    <str name="class">testLoggingClass</str>
+    <str name="enabled">testLoggingEnabled</str>
+    <watcher>
+      <int name="size">88</int>
+      <int name="threshold">99</int>
+    </watcher>
+  </logging>
+
+  <shardHandlerFactory name="testShardHandlerFactory" class="testHttpShardHandlerFactory">
+    <int name="socketTimeout">${socketTimeout:100}</int>
+    <int name="connTimeout">${connTimeout:110}</int>
+  </shardHandlerFactory>
+
+</solr>
\ No newline at end of file

Modified: lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress-new.xml
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress-new.xml?rev=1468284&r1=1468283&r2=1468284&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress-new.xml (original)
+++ lucene/dev/trunk/solr/core/src/test-files/solr/solr-stress-new.xml Tue Apr 16 00:34:49 2013
@@ -19,21 +19,16 @@
 <!--
  All (relative) paths are relative to the installation path
 
-  persistent: Save changes made via the API to this file
-  sharedLib: path to a lib directory that will be shared across all cores
 -->
-<solr persistent="${solr.xml.persist:true}">
-
-  <!--
-  adminPath: RequestHandler path to manage cores.
-    If 'null' (or absent), cores will not be manageable via request handler
-  -->
-  <cores adminPath="/admin/cores" defaultCoreName="collection1" host="127.0.0.1" hostPort="${hostPort:8983}"
-         hostContext="${hostContext:solr}" autoDiscoverCores="true" >
-    <shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
-      <int name="socketTimeout">${socketTimeout:120000}</int>
-      <int name="connTimeout">${connTimeout:15000}</int>
-    </shardHandlerFactory>
-  </cores>
+<solr>
+  <solrcloud>
+    <str name="host">127.0.0.1</str>
+    <int name="hostPort">8983</int>
+    <str name="hostContext">${hostContext:solr}</str>
+  </solrcloud>
 
+  <shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
+    <int name="socketTimeout">${socketTimeout:120000}</int>
+    <int name="connTimeout">${connTimeout:15000}</int>
+  </shardHandlerFactory>
 </solr>

Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java?rev=1468284&r1=1468283&r2=1468284&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java Tue Apr 16 00:34:49 2013
@@ -28,7 +28,6 @@ import org.junit.Test;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
-import java.io.StringReader;
 import java.util.Properties;
 import java.util.Set;
 
@@ -42,17 +41,24 @@ public class TestCoreDiscovery extends S
 
   private final File solrHomeDirectory = new File(TEMP_DIR, "org.apache.solr.core.TestCoreDiscovery" + File.separator + "solrHome");
 
-  private void setMeUp() throws Exception {
+  private void setMeUp(String alternateCoreDir) throws Exception {
     if (solrHomeDirectory.exists()) {
       FileUtils.deleteDirectory(solrHomeDirectory);
     }
     assertTrue("Failed to mkdirs workDir", solrHomeDirectory.mkdirs());
+
     System.setProperty("solr.solr.home", solrHomeDirectory.getAbsolutePath());
+    String xmlStr = SOLR_XML;
+    if (alternateCoreDir != null) {
+      xmlStr = xmlStr.replace("<solr>", "<solr> <str name=\"coreRootDirectory\">" + alternateCoreDir + "</str> ");
+    }
+    File tmpFile = new File(solrHomeDirectory, ConfigSolr.SOLR_XML_FILE);
+    FileUtils.write(tmpFile, xmlStr, IOUtils.CHARSET_UTF_8.toString());
+
   }
 
-  private void addSolrXml() throws Exception {
-    File tmpFile = new File(solrHomeDirectory, ConfigSolr.SOLR_XML_FILE);
-    FileUtils.write(tmpFile, SOLR_XML, IOUtils.CHARSET_UTF_8.toString());
+  private void setMeUp() throws Exception {
+    setMeUp(null);
   }
 
   private Properties makeCorePropFile(String name, boolean isLazy, boolean loadOnStartup, String... extraProps) {
@@ -63,6 +69,7 @@ public class TestCoreDiscovery extends S
     props.put(CoreDescriptor.CORE_TRANSIENT, Boolean.toString(isLazy));
     props.put(CoreDescriptor.CORE_LOADONSTARTUP, Boolean.toString(loadOnStartup));
     props.put(CoreDescriptor.CORE_DATADIR, "${core.dataDir:stuffandnonsense}");
+    props.put(CoreDescriptor.CORE_INSTDIR, "totallybogus"); // For testing that this property is ignored if present.
 
     for (String extra : extraProps) {
       String[] parts = extra.split("=");
@@ -72,13 +79,8 @@ public class TestCoreDiscovery extends S
     return props;
   }
 
-  private void addCoreWithProps(Properties stockProps) throws Exception {
-
-    File propFile = new File(solrHomeDirectory,
-        stockProps.getProperty(CoreDescriptor.CORE_NAME) + File.separator + ConfigSolr.CORE_PROP_FILE);
-    File parent = propFile.getParentFile();
-    assertTrue("Failed to mkdirs for " + parent.getAbsolutePath(), parent.mkdirs());
-
+  private void addCoreWithProps(Properties stockProps, File propFile) throws Exception {
+    if (!propFile.getParentFile().exists()) propFile.getParentFile().mkdirs();
     FileOutputStream out = new FileOutputStream(propFile);
     try {
       stockProps.store(out, null);
@@ -86,7 +88,17 @@ public class TestCoreDiscovery extends S
       out.close();
     }
 
-    addConfFiles(new File(parent, "conf"));
+    addConfFiles(new File(propFile.getParent(), "conf"));
+
+  }
+
+  private void addCoreWithProps(Properties stockProps) throws Exception {
+
+    File propFile = new File(solrHomeDirectory,
+        stockProps.getProperty(CoreDescriptor.CORE_NAME) + File.separator + ConfigSolr.CORE_PROP_FILE);
+    File parent = propFile.getParentFile();
+    assertTrue("Failed to mkdirs for " + parent.getAbsolutePath(), parent.mkdirs());
+    addCoreWithProps(stockProps, propFile);
   }
 
   // For testing error condition of having multiple cores with the same name.
@@ -114,10 +126,6 @@ public class TestCoreDiscovery extends S
     FileUtils.copyFile(new File(top, "solrconfig-minimal.xml"), new File(confDir, "solrconfig-minimal.xml"));
   }
 
-  private void addConfigsForBackCompat() throws Exception {
-    addConfFiles(new File(solrHomeDirectory, "collection1" + File.separator + "conf"));
-  }
-
   private CoreContainer init() throws Exception {
 
     CoreContainer.Initializer init = new CoreContainer.Initializer();
@@ -138,23 +146,22 @@ public class TestCoreDiscovery extends S
   // Test the basic setup, create some dirs with core.properties files in them, but solr.xml has discoverCores
   // set and insure that we find all the cores and can load them.
   @Test
-  public void testDiscover() throws Exception {
+  public void testDiscovery() throws Exception {
     setMeUp();
-    addSolrXml();
+
     // name, isLazy, loadOnStartup
-    addCoreWithProps(makeCorePropFile("core1", false, true));
-    addCoreWithProps(makeCorePropFile("core2", false, false));
+    addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=core1"));
+    addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"));
 
     // I suspect what we're adding in here is a "configset" rather than a schema or solrconfig.
     //
-    addCoreWithProps(makeCorePropFile("lazy1", true, false));
+    addCoreWithProps(makeCorePropFile("lazy1", true, false, "dataDir=lazy1"));
 
     CoreContainer cc = init();
     try {
-      Properties props = cc.containerProperties;
+      assertNull("adminPath no longer allowed in solr.xml", cc.getAdminPath());
+      assertNull("defaultCore no longer allowed in solr.xml", cc.getDefaultCoreName());
 
-      assertEquals("/admin/cores", cc.getAdminPath());
-      assertEquals("defcore", cc.getDefaultCoreName());
       assertEquals("222.333.444.555", cc.getHost());
       assertEquals("6000", cc.getHostPort());
       assertEquals("solrprop", cc.getHostContext());
@@ -169,10 +176,13 @@ public class TestCoreDiscovery extends S
       CoreDescriptor desc = core1.getCoreDescriptor();
       assertEquals("core1", desc.getProperty("solr.core.name"));
 
+      // Prove we're ignoring this even though it's set in the properties file
+      assertFalse("InstanceDir should be ignored", desc.getProperty("solr.core.instanceDir").contains("totallybogus"));
+
       // This is too long and ugly to put in. Besides, it varies.
       assertNotNull(desc.getProperty("solr.core.instanceDir"));
 
-      assertEquals("stuffandnonsense", desc.getProperty("solr.core.dataDir"));
+      assertEquals("core1", desc.getProperty("solr.core.dataDir"));
       assertEquals("solrconfig-minimal.xml", desc.getProperty("solr.core.configName"));
       assertEquals("schema-tiny.xml", desc.getProperty("solr.core.schemaName"));
 
@@ -188,132 +198,49 @@ public class TestCoreDiscovery extends S
     }
   }
 
-
-  // Check that the various flavors of persistence work, including saving the state of a core when it's being swapped
-  // out. Added a test in here to insure that files that have config variables are saved with the config vars not the
-  // substitutions.
-  @Test
-  public void testPersistTrue() throws Exception {
-    setMeUp();
-    addSolrXml();
-    System.setProperty("solr.persistent", "true");
-
-    Properties special = makeCorePropFile("core1", false, true);
-    special.put(CoreDescriptor.CORE_INSTDIR, "${core1inst:anothersillypath}");
-    addCoreWithProps(special);
-    addCoreWithProps(makeCorePropFile("core2", false, false));
-    addCoreWithProps(makeCorePropFile("lazy1", true, true));
-    addCoreWithProps(makeCorePropFile("lazy2", true, true));
-    addCoreWithProps(makeCorePropFile("lazy3", true, false));
-
-    System.setProperty("core1inst", "core1");
-    CoreContainer cc = init();
-    SolrCore coreC1 = cc.getCore("core1");
-    addCoreProps(coreC1, "addedPropC1=addedC1", "addedPropC1B=foo", "addedPropC1C=bar");
-
-    SolrCore coreC2 = cc.getCore("core2");
-    addCoreProps(coreC2, "addedPropC2=addedC2", "addedPropC2B=foo", "addedPropC2C=bar");
-
-    SolrCore coreL1 = cc.getCore("lazy1");
-    addCoreProps(coreL1, "addedPropL1=addedL1", "addedPropL1B=foo", "addedPropL1C=bar");
-
-    SolrCore coreL2 = cc.getCore("lazy2");
-    addCoreProps(coreL2, "addedPropL2=addedL2", "addedPropL2B=foo", "addedPropL2C=bar");
-
-    SolrCore coreL3 = cc.getCore("lazy3");
-    addCoreProps(coreL3, "addedPropL3=addedL3", "addedPropL3B=foo", "addedPropL3C=bar");
-
-    try {
-      cc.persist();
-
-      // Insure that one of the loaded cores was swapped out, with a cache size of 2 lazy1 should be gone.
-      TestLazyCores.checkInCores(cc, "core1", "core2", "lazy2", "lazy3");
-      TestLazyCores.checkNotInCores(cc, "lazy1");
-
-      Properties orig = makeCorePropFile("core1", false, true);
-      orig.put(CoreDescriptor.CORE_INSTDIR, "${core1inst:anothersillypath}");
-      checkCoreProps(orig, "addedPropC1=addedC1", "addedPropC1B=foo", "addedPropC1C=bar");
-
-      orig = makeCorePropFile("core2", false, false);
-      checkCoreProps(orig, "addedPropC2=addedC2", "addedPropC2B=foo", "addedPropC2C=bar");
-
-      // This test insures that a core that was swapped out has its properties file persisted. Currently this happens
-      // as the file is removed from the cache.
-      orig = makeCorePropFile("lazy1", true, true);
-      checkCoreProps(orig, "addedPropL1=addedL1", "addedPropL1B=foo", "addedPropL1C=bar");
-
-      orig = makeCorePropFile("lazy2", true, true);
-      checkCoreProps(orig, "addedPropL2=addedL2", "addedPropL2B=foo", "addedPropL2C=bar");
-
-      orig = makeCorePropFile("lazy3", true, false);
-      checkCoreProps(orig, "addedPropL3=addedL3", "addedPropL3B=foo", "addedPropL3C=bar");
-
-      coreC1.close();
-      coreC2.close();
-      coreL1.close();
-      coreL2.close();
-      coreL3.close();
-
-    } finally {
-      cc.shutdown();
-    }
-  }
-
-  // Make sure that, even if we do call persist, nothing's saved unless the flag is set in solr.properties.
   @Test
-  public void testPersistFalse() throws Exception {
-    setMeUp();
-    addSolrXml();
-
-    addCoreWithProps(makeCorePropFile("core1", false, true));
-    addCoreWithProps(makeCorePropFile("core2", false, false));
-    addCoreWithProps(makeCorePropFile("lazy1", true, true));
-    addCoreWithProps(makeCorePropFile("lazy2", false, true));
-
+  public void testAlternateCoreDir() throws Exception {
+    File alt = new File(TEMP_DIR, "alternateCoreDir");
+    if (alt.exists()) FileUtils.deleteDirectory(alt);
+    alt.mkdirs();
+    setMeUp(alt.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=core1"),
+        new File(alt, "core1" + File.separator + ConfigSolr.CORE_PROP_FILE));
+    addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"),
+        new File(alt, "core2" + File.separator + ConfigSolr.CORE_PROP_FILE));
     CoreContainer cc = init();
-    SolrCore coreC1 = cc.getCore("core1");
-    addCoreProps(coreC1, "addedPropC1=addedC1", "addedPropC1B=foo", "addedPropC1C=bar");
-
-    SolrCore coreC2 = cc.getCore("core2");
-    addCoreProps(coreC2, "addedPropC2=addedC2", "addedPropC2B=foo", "addedPropC2C=bar");
-
-    SolrCore coreL1 = cc.getCore("lazy1");
-    addCoreProps(coreL1, "addedPropL1=addedL1", "addedPropL1B=foo", "addedPropL1C=bar");
-
-    SolrCore coreL2 = cc.getCore("lazy2");
-    addCoreProps(coreL2, "addedPropL2=addedL2", "addedPropL2B=foo", "addedPropL2C=bar");
-
-
     try {
-      cc.persist();
-
-      checkCoreProps(makeCorePropFile("core1", false, true));
-      checkCoreProps(makeCorePropFile("core2", false, false));
-      checkCoreProps(makeCorePropFile("lazy1", true, true));
-      checkCoreProps(makeCorePropFile("lazy2", false, true));
-
-      coreC1.close();
-      coreC2.close();
-      coreL1.close();
-      coreL2.close();
+      SolrCore core1 = cc.getCore("core1");
+      SolrCore core2 = cc.getCore("core2");
+      assertNotNull(core1);
+      assertNotNull(core2);
+      core1.close();
+      core2.close();
     } finally {
       cc.shutdown();
+      if (alt.exists()) FileUtils.deleteDirectory(alt);
     }
   }
 
   @Test
   public void testCoresWithSameNameError() throws Exception {
     setMeUp();
-    addSolrXml();
     addCoreWithPropsDir("core1_1", makeCorePropFile("core1", false, true));
     addCoreWithPropsDir("core1_2", makeCorePropFile("core1", false, true));
-    // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+      String msg = cc.getBadCoreMessage("core1");
+      assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -322,18 +249,33 @@ public class TestCoreDiscovery extends S
   }
 
   @Test
-  public void testCoresWithSameDataDirError() throws Exception{
+  public void testCoresWithSameDataDirError() throws Exception {
     setMeUp();
-    addSolrXml();
     addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
     addCoreWithProps(makeCorePropFile("core2", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
-    // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+      String msg = cc.getBadCoreMessage("core2");
+      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
+      try {
+        cc.getCore("core2");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
+
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -344,16 +286,22 @@ public class TestCoreDiscovery extends S
   @Test
   public void testCoresWithSameNameErrorTransient() throws Exception {
     setMeUp();
-    addSolrXml();
     addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true, false));
     addCoreWithPropsDir("core1_2", makeCorePropFile("core1", true, false));
-    // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+      String msg = cc.getBadCoreMessage("core1");
+      assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -362,18 +310,25 @@ public class TestCoreDiscovery extends S
   }
 
   @Test
-  public void testCoresWithSameDataDirErrorTransient() throws Exception{
+  public void testCoresWithSameDataDirErrorTransient() throws Exception {
     setMeUp();
-    addSolrXml();
     addCoreWithProps(makeCorePropFile("core1", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
     addCoreWithProps(makeCorePropFile("core2", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
     // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+      String msg = cc.getBadCoreMessage("core1");
+      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -383,18 +338,25 @@ public class TestCoreDiscovery extends S
 
 
   @Test
-  public void testCoresWithSameNameErrorboth() throws Exception {
+  public void testCoresWithSameNameErrorBoth() throws Exception {
     setMeUp();
-    addSolrXml();
-    addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true,  false));
+    addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true, false));
     addCoreWithPropsDir("core1_2", makeCorePropFile("core1", false, false));
     // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+      String msg = cc.getBadCoreMessage("core1");
+      assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertTrue("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -402,19 +364,41 @@ public class TestCoreDiscovery extends S
     }
   }
 
+
   @Test
-  public void testCoresWithSameDataDirErrorBoth() throws Exception{
+  public void testCoresWithSameDataDirErrorBoth() throws Exception {
     setMeUp();
-    addSolrXml();
-    addCoreWithProps(makeCorePropFile("core1", false, false, "dataDir=" + solrHomeDirectory + "datadir"));
-    addCoreWithProps(makeCorePropFile("core2", true,  false, "dataDir=" + solrHomeDirectory + "datadir"));
+    addCoreWithProps(makeCorePropFile("core1", false, false, "dataDir=" + solrHomeDirectory + "/datadir"));
+    addCoreWithProps(makeCorePropFile("core2", true, false, "dataDir=" + solrHomeDirectory + "/datadir"));
     // Should just blow up here.
     CoreContainer cc = null;
     try {
       cc = init();
-    } catch (SolrException se) {
-      assertEquals("Should be returning proper error code of 500", 500, se.code());
-      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+      String msg = cc.getBadCoreMessage("core2");
+      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+
+      msg = cc.getBadCoreMessage("core1");
+      assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+      assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+
+      try {
+        cc.getCore("core1");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
+      try {
+        cc.getCore("core2");
+      } catch (SolrException se) {
+        assertEquals("Should be returning proper error code of 500", 500, se.code());
+        msg = se.getMessage();
+        assertFalse("Should have found multiple cores with same name", msg.contains("More than one core defined for core named 'core1'"));
+        assertTrue("Should have found multiple cores with same data dir", msg.contains("More than one core points to data dir"));
+      }
+
     } finally {
       if (cc != null) {
         cc.shutdown();
@@ -422,41 +406,14 @@ public class TestCoreDiscovery extends S
     }
   }
 
-  void addCoreProps(SolrCore core, String... propPairs) {
-    for (String keyval : propPairs) {
-      String[] pair = keyval.split("=");
-      core.getCoreDescriptor().putProperty(pair[0], pair[1]);
-    }
-  }
-
-  // Insure that the properties in the core passed in are exactly what's in the default core.properties below plus
-  // whatever extra is passed in.
-  void checkCoreProps(Properties orig, String... extraProps) throws Exception {
-    // Read the persisted file.
-    Properties props = new Properties();
-    File propParent = new File(solrHomeDirectory, orig.getProperty(CoreDescriptor.CORE_NAME));
-    FileInputStream in = new FileInputStream(new File(propParent, ConfigSolr.CORE_PROP_FILE));
-    try {
-      props.load(in);
-    } finally {
-      in.close();
-    }
-    Set<String> propSet = props.stringPropertyNames();
-
-    assertEquals("Persisted properties should NOT contain extra properties", propSet.size(), orig.size());
-
-    for (String prop : orig.stringPropertyNames()) {
-      assertEquals("Original and new properties should be equal for " + prop, props.getProperty(prop), orig.getProperty(prop));
-    }
-    for (String prop : extraProps) {
-      String[] pair = prop.split("=");
-      assertNull("Modified parameters should not be present for " + prop, props.getProperty(pair[0]));
-    }
-  }
-
   // For testing whether finding a solr.xml overrides looking at solr.properties
-  private final static String SOLR_XML = " <solr persistent=\"${persistent:false}\"> " +
-      "<cores autoDiscoverCores=\"true\" adminPath=\"/admin/cores\" defaultCoreName=\"defcore\" transientCacheSize=\"2\" " +
-       " hostContext=\"solrprop\" zkClientTimeout=\"20\" host=\"222.333.444.555\" hostPort=\"6000\" />  " +
+  private final static String SOLR_XML = "<solr> " +
+      "<int name=\"transientCacheSize\">2</int> " +
+      "<solrcloud> " +
+      "<str name=\"hostContext\">solrprop</str> " +
+      "<int name=\"zkClientTimeout\">20</int> " +
+      "<str name=\"host\">222.333.444.555</str> " +
+      "<int name=\"hostPort\">6000</int>  " +
+      "</solrcloud> " +
       "</solr>";
 }

Added: 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=1468284&view=auto
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java (added)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/core/TestSolrXml.java Tue Apr 16 00:34:49 2013
@@ -0,0 +1,120 @@
+package org.apache.solr.core;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import org.apache.commons.io.FileUtils;
+import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Test;
+import org.xml.sax.SAXException;
+
+import javax.xml.parsers.ParserConfigurationException;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class TestSolrXml extends SolrTestCaseJ4 {
+  private final File solrHome = new File(TEMP_DIR, TestSolrXml.getClassName() + File.separator + "solrHome");
+
+  @Test
+  public void testAllInfoPresent() throws IOException, ParserConfigurationException, SAXException {
+    CoreContainer cc = null;
+    File testSrcRoot = new File(SolrTestCaseJ4.TEST_HOME());
+    FileUtils.copyFile(new File(testSrcRoot, "solr-50-all.xml"), new File(solrHome, "solr.xml"));
+    try {
+      InputStream is = new FileInputStream(new File(solrHome, "solr.xml"));
+      ConfigSolr cfg = new ConfigSolrXml(new SolrResourceLoader("solr/collection1"), null, is, null, false, null);
+
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_ADMINHANDLER, null), "testAdminHandler");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, 0), 11);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_COREROOTDIRECTORY, null), "testCoreRootDirectory");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, 0), 22);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, 0), 33);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_HOST, null), "testHost");
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_HOSTCONTEXT, null), "testHostContext");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_HOSTPORT, 0), 44);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_LEADERVOTEWAIT, 0), 55);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_CLASS, null), "testLoggingClass");
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_ENABLED, null), "testLoggingEnabled");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_SIZE, 0), 88);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, 0), 99);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_MANAGEMENTPATH, null), "testManagementPath");
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_SHAREDLIB, null), "testSharedLib");
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CLASS, null), "testHttpShardHandlerFactory");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, 0), 110);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_NAME, null), "testShardHandlerFactory");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, 0), 100);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, null), "testShareSchema");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, 0), 66);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_ZKCLIENTTIMEOUT, 0), 77);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_ZKHOST, null), "testZkHost");
+      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 {
+      if (cc != null) cc.shutdown();
+    }
+  }
+
+  // Test  a few property substitutions that happen to be in solr-50-all.xml.
+  public void testPropretySub() throws IOException, ParserConfigurationException, SAXException {
+
+    String coreRoot = System.getProperty("coreRootDirectory");
+    String hostPort = System.getProperty("hostPort");
+    String shareSchema = System.getProperty("shareSchema");
+    String socketTimeout = System.getProperty("socketTimeout");
+    String connTimeout = System.getProperty("connTimeout");
+    System.setProperty("coreRootDirectory", "myCoreRoot");
+    System.setProperty("hostPort", "8888");
+    System.setProperty("shareSchema", "newShareSchema");
+    System.setProperty("socketTimeout", "220");
+    System.setProperty("connTimeout", "200");
+
+    CoreContainer cc = null;
+    File testSrcRoot = new File(SolrTestCaseJ4.TEST_HOME());
+    FileUtils.copyFile(new File(testSrcRoot, "solr-50-all.xml"), new File(solrHome, "solr.xml"));
+    try {
+      InputStream is = new FileInputStream(new File(solrHome, "solr.xml"));
+      ConfigSolr cfg = new ConfigSolrXml(new SolrResourceLoader("solr/collection1"), null, is, null, false, null);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_COREROOTDIRECTORY, null), "myCoreRoot");
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_HOSTPORT, 0), 8888);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, 0), 200);
+      assertEquals("Did not find expected value", cfg.getInt(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, 0), 220);
+      assertEquals("Did not find expected value", cfg.get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, null), "newShareSchema");
+
+    } finally {
+      if (cc != null) cc.shutdown();
+      if (coreRoot != null) System.setProperty("coreRootDirectory", coreRoot);
+      else System.clearProperty("coreRootDirectory");
+
+      if (hostPort != null) System.setProperty("hostPort", hostPort);
+      else System.clearProperty("hostPort");
+
+      if (shareSchema != null) System.setProperty("shareSchema", shareSchema);
+      else System.clearProperty("shareSchema");
+
+      if (socketTimeout != null) System.setProperty("socketTimeout", socketTimeout);
+      else System.clearProperty("socketTimeout");
+
+      if (connTimeout != null) System.setProperty("connTimeout", connTimeout);
+      else System.clearProperty("connTimeout");
+
+    }
+  }
+}
+