SOLR-4914: Factor out core discovery and persistence logic

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

solr/CHANGES.txt

@@ -64,6 +64,11 @@ Upgrading from Solr 4.4.0
 Detailed Change List
 ----------------------
 
+Other Changes
+----------------------
+
+* SOLR-4914: Factor out core list persistence and discovery into a
+  new CoresLocator interface. (Alan Woodward)
 
 ==================  4.4.0 ==================
 

solr/contrib/dataimporthandler/src/test-files/log4j.properties

@@ -0,0 +1,9 @@
+#  Logging level
+log4j.rootLogger=INFO, CONSOLE
+
+log4j.appender.CONSOLE=org.apache.log4j.ConsoleAppender
+log4j.appender.CONSOLE.Target=System.err
+log4j.appender.CONSOLE.layout=org.apache.solr.util.SolrLogLayout
+log4j.appender.CONSOLE.layout.ConversionPattern=%-5p - %d{yyyy-MM-dd HH:mm:ss.SSS}; %C; %m\n
+
+log4j.logger.org.apache.zookeeper=WARN

solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java

@@ -17,11 +17,16 @@ package org.apache.solr.cloud;
  * limitations under the License.
  */
 
-import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.util.PropertiesUtil;
+
+import java.util.Properties;
 
 public class CloudDescriptor {
+
   private String shardId;
   private String collectionName;
   private SolrParams params;
@@ -37,6 +42,21 @@ public class CloudDescriptor {
   volatile boolean isLeader = false;
   volatile String lastPublished = ZkStateReader.ACTIVE;
 
+  public static final String SHARD_STATE = "shardState";
+  public static final String NUM_SHARDS = "numShards";
+  public static final String SHARD_RANGE = "shardRange";
+
+  public CloudDescriptor(String coreName, Properties props) {
+    this.shardId = props.getProperty(CoreDescriptor.CORE_SHARD, null);
+    // If no collection name is specified, we default to the core name
+    this.collectionName = props.getProperty(CoreDescriptor.CORE_COLLECTION, coreName);
+    this.roles = props.getProperty(CoreDescriptor.CORE_ROLES, null);
+    this.nodeName = props.getProperty(CoreDescriptor.CORE_NODE_NAME);
+    this.shardState = props.getProperty(CloudDescriptor.SHARD_STATE, Slice.ACTIVE);
+    this.numShards = PropertiesUtil.toInteger(props.getProperty(CloudDescriptor.NUM_SHARDS), null);
+    this.shardRange = props.getProperty(CloudDescriptor.SHARD_RANGE, null);
+  }
+  
   public String getLastPublished() {
     return lastPublished;
   }

solr/core/src/java/org/apache/solr/cloud/ZkCLI.java

@@ -10,13 +10,11 @@ import org.apache.commons.cli.ParseException;
 import org.apache.commons.cli.PosixParser;
 import org.apache.solr.common.cloud.OnReconnect;
 import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.core.ConfigSolr;
+import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.SolrResourceLoader;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.data.ACL;
-import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
 
 import javax.xml.parsers.ParserConfigurationException;
@@ -173,17 +171,18 @@ public class ZkCLI {
                 + " is required for " + BOOTSTRAP);
             System.exit(1);
           }
-          SolrResourceLoader loader = new SolrResourceLoader(solrHome);
-          solrHome = loader.getInstanceDir();
 
-          ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome);
+          CoreContainer cc = new CoreContainer(solrHome);
 
           if(!ZkController.checkChrootPath(zkServerAddress, true)) {
             System.out.println("A chroot was specified in zkHost but the znode doesn't exist. ");
             System.exit(1);
           }
 
-          ZkController.bootstrapConf(zkClient, cfg, solrHome);
+          ZkController.bootstrapConf(zkClient, cc, solrHome);
+
+          // No need to shutdown the CoreContainer, as it wasn't started
+          // up in the first place...
           
         } else if (line.getOptionValue(CMD).equals(UPCONFIG)) {
           if (!line.hasOption(CONFDIR) || !line.hasOption(CONFNAME)) {

solr/core/src/java/org/apache/solr/cloud/ZkController.java

@@ -18,6 +18,7 @@ package org.apache.solr.cloud;
  */
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;
 import org.apache.solr.client.solrj.request.CoreAdminRequest.WaitForState;
 import org.apache.solr.common.SolrException;
@@ -36,14 +37,12 @@ import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.cloud.ZooKeeperException;
 import org.apache.solr.common.params.SolrParams;
-import org.apache.solr.core.ConfigSolr;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.handler.component.ShardHandler;
 import org.apache.solr.update.UpdateLog;
 import org.apache.solr.update.UpdateShardHandler;
-import org.apache.solr.util.PropertiesUtil;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.KeeperException.NoNodeException;
@@ -1493,27 +1492,22 @@ public final class ZkController {
   /**
    * If in SolrCloud mode, upload config sets for each SolrCore in solr.xml.
    */
-  public static void bootstrapConf(SolrZkClient zkClient, ConfigSolr cfg, String solrHome) throws IOException,
+  public static void bootstrapConf(SolrZkClient zkClient, CoreContainer cc, String solrHome) throws IOException,
       KeeperException, InterruptedException {
 
-    List<String> allCoreNames = cfg.getAllCoreNames();
+    //List<String> allCoreNames = cfg.getAllCoreNames();
+    List<CoreDescriptor> cds = cc.getCoresLocator().discover(cc);
     
-    log.info("bootstraping config for " + allCoreNames.size() + " cores into ZooKeeper using solr.xml from " + solrHome);
+    log.info("bootstrapping config for " + cds.size() + " cores into ZooKeeper using solr.xml from " + solrHome);
 
-    for (String coreName : allCoreNames) {
+    for (CoreDescriptor cd : cds) {
-      String rawName = PropertiesUtil.substituteProperty(cfg.getProperty(coreName, "name", null), new Properties());
+      String coreName = cd.getName();
-      String instanceDir = cfg.getProperty(coreName, "instanceDir", null);
+      String confName = cd.getCollectionName();
-      File idir = new File(instanceDir);
+      if (StringUtils.isEmpty(confName))
-      System.out.println("idir:" + idir);
+        confName = coreName;
-      if (!idir.isAbsolute()) {
+      String instanceDir = cd.getInstanceDir();
-        idir = new File(solrHome, instanceDir);
+      File udir = new File(instanceDir, "conf");
-      }
+      log.info("Uploading directory " + udir + " with name " + confName + " for SolrCore " + coreName);
-      String confName = PropertiesUtil.substituteProperty(cfg.getProperty(coreName, "collection", null), new Properties());
-      if (confName == null) {
-        confName = rawName;
-      }
-      File udir = new File(idir, "conf");
-      log.info("Uploading directory " + udir + " with name " + confName + " for SolrCore " + rawName);
       ZkController.uploadConfigDir(zkClient, udir, confName);
     }
   }

solr/core/src/java/org/apache/solr/core/ConfigSolr.java

@@ -18,6 +18,7 @@ package org.apache.solr.core;
  */
 
 import com.google.common.base.Charsets;
+import com.google.common.io.ByteStreams;
 import org.apache.commons.io.IOUtils;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.util.DOMUtil;
@@ -32,11 +33,11 @@ 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.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
@@ -59,7 +60,10 @@ public abstract class ConfigSolr {
       else {
         inputStream = new FileInputStream(configFile);
       }
-      return fromInputStream(loader, inputStream);
+      ByteArrayOutputStream baos = new ByteArrayOutputStream();
+      ByteStreams.copy(inputStream, baos);
+      String originalXml = IOUtils.toString(new ByteArrayInputStream(baos.toByteArray()), "UTF-8");
+      return fromInputStream(loader, new ByteArrayInputStream(baos.toByteArray()), configFile, originalXml);
     }
     catch (Exception e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
@@ -70,15 +74,14 @@ public abstract class ConfigSolr {
     }
   }
 
-  public static ConfigSolr fromString(SolrResourceLoader loader, String xml) {
+  public static ConfigSolr fromString(String xml) {
-    return fromInputStream(loader, new ByteArrayInputStream(xml.getBytes(Charsets.UTF_8)));
+    return fromInputStream(null, new ByteArrayInputStream(xml.getBytes(Charsets.UTF_8)), null, xml);
   }
 
-  public static ConfigSolr fromInputStream(SolrResourceLoader loader, InputStream is) {
+  public static ConfigSolr fromInputStream(SolrResourceLoader loader, InputStream is, File file, String originalXml) {
     try {
       Config config = new Config(loader, null, new InputSource(is), null, false);
-      //config.substituteProperties();
+      return fromConfig(config, file, originalXml);
-      return fromConfig(config);
     }
     catch (Exception e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
@@ -89,12 +92,14 @@ public abstract class ConfigSolr {
     return fromFile(loader, new File(solrHome, SOLR_XML_FILE));
   }
 
-  public static ConfigSolr fromConfig(Config config) {
+  public static ConfigSolr fromConfig(Config config, File file, String originalXml) {
     boolean oldStyle = (config.getNode("solr/cores", false) != null);
-    return oldStyle ? new ConfigSolrXmlOld(config)
+    return oldStyle ? new ConfigSolrXmlOld(config, file, originalXml)
-                    : new ConfigSolrXml(config, null);
+                    : new ConfigSolrXml(config);
   }
   
+  public abstract CoresLocator getCoresLocator();
+
 
   public PluginInfo getShardHandlerFactoryPluginInfo() {
     Node node = config.getNode(getShardHandlerFactoryConfigPath(), false);
@@ -171,12 +176,6 @@ public abstract class ConfigSolr {
     return (val == null) ? def : val;
   }
 
-  // For saving the original property, ${} syntax and all.
-  public String getOrigProp(CfgProp prop, String def) {
-    String val = propMap.get(prop);
-    return (val == null) ? def : val;
-  }
-
   public Properties getSolrProperties(String path) {
     try {
       return readProperties(((NodeList) config.evaluate(
@@ -200,15 +199,5 @@ public abstract class ConfigSolr {
     return properties;
   }
 
-  public abstract void substituteProperties();
-
-  public abstract List<String> getAllCoreNames();
-
-  public abstract String getProperty(String coreName, String property, String defaultVal);
-
-  public abstract Properties readCoreProperties(String coreName);
-
-  public abstract Map<String, String> readCoreAttributes(String coreName);
-
 }
 

solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java

@@ -17,40 +17,32 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import org.apache.commons.io.IOUtils;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.util.PropertiesUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Properties;
 
 
 /**
  *
  */
 public class ConfigSolrXml extends ConfigSolr {
+
   protected static Logger log = LoggerFactory.getLogger(ConfigSolrXml.class);
 
-  private SolrCoreDiscoverer solrCoreDiscoverer = new SolrCoreDiscoverer();
+  private final CoresLocator coresLocator;
-  private final Map<String, CoreDescriptor> coreDescriptorMap;
 
-  public ConfigSolrXml(Config config, CoreContainer container) {
+  public ConfigSolrXml(Config config) {
     super(config);
     try {
       checkForIllegalConfig();
       fillPropMap();
       config.substituteProperties();
-      String coreRoot = get(CfgProp.SOLR_COREROOTDIRECTORY, (container == null ? config.getResourceLoader().getInstanceDir() : container.getSolrHome()));
+      log.info("Config-defined core root directory: {}", get(CfgProp.SOLR_COREROOTDIRECTORY, ""));
-      coreDescriptorMap = solrCoreDiscoverer.discover(container, new File(coreRoot));
+      String coreRoot = get(CfgProp.SOLR_COREROOTDIRECTORY, config.getResourceLoader().getInstanceDir());
+      coresLocator = new CorePropertiesLocator(coreRoot);
     }
     catch (IOException e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
@@ -133,68 +125,14 @@ public class ConfigSolrXml extends ConfigSolr {
     propMap.put(CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, doSub("solr/logging/watcher/int[@name='threshold']"));
   }
 
-
-
-  @Override
-  public Map<String,String> readCoreAttributes(String coreName) {
-    Map<String,String> attrs = new HashMap<String,String>();
-    
-    return attrs; // this is a no-op.... intentionally
-  }
-
-  @Override
-  public List<String> getAllCoreNames() {
-    List<String> ret = new ArrayList<String>(coreDescriptorMap.keySet());
-    
-    return ret;
-  }
-  
-  @Override
-  public String getProperty(String coreName, String property, String defaultVal) {
-    CoreDescriptor cd = coreDescriptorMap.get(coreName);
-    if (cd == null) return defaultVal;
-
-    return cd.getProperty(property, defaultVal);
-  }
-
-  @Override
-  public Properties readCoreProperties(String coreName) {
-    CoreDescriptor cd = coreDescriptorMap.get(coreName);
-    if (cd == null) return null;
-    return new Properties(cd.getCoreProperties());
-  }
-
-  static Properties getCoreProperties(String instanceDir, CoreDescriptor dcore) {
-    String file = dcore.getPropertiesName();
-    if (file == null) file = "conf" + File.separator + "solrcore.properties";
-    File corePropsFile = new File(file);
-    if (!corePropsFile.isAbsolute()) {
-      corePropsFile = new File(instanceDir, file);
-    }
-    Properties p = dcore.getCoreProperties();
-    if (corePropsFile.exists() && corePropsFile.isFile()) {
-      p = new Properties(dcore.getCoreProperties());
-      InputStream is = null;
-      try {
-        is = new FileInputStream(corePropsFile);
-        p.load(is);
-      } catch (IOException e) {
-        log.warn("Error loading properties ", e);
-      } finally {
-        IOUtils.closeQuietly(is);
-      }
-    }
-    return p;
-  }
-
   @Override
   protected String getShardHandlerFactoryConfigPath() {
     return "solr/shardHandlerFactory";
   }
 
   @Override
-  public void substituteProperties() {
+  public CoresLocator getCoresLocator() {
-    config.substituteProperties();
+    return coresLocator;
   }
 
 }

solr/core/src/java/org/apache/solr/core/ConfigSolrXmlOld.java

@@ -22,7 +22,6 @@ import org.apache.solr.util.DOMUtil;
 import org.apache.solr.util.PropertiesUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.w3c.dom.NamedNodeMap;
 import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
@@ -44,28 +43,38 @@ import java.util.Set;
  *
  */
 public class ConfigSolrXmlOld extends ConfigSolr {
+
   protected static Logger log = LoggerFactory.getLogger(ConfigSolrXmlOld.class);
 
   private NodeList coreNodes = null;
   
+  private final CoresLocator persistor;
+
   @Override
   protected String getShardHandlerFactoryConfigPath() {
     return "solr/cores/shardHandlerFactory";
   }
 
-  public ConfigSolrXmlOld(Config config) {
+  public ConfigSolrXmlOld(Config config, File configFile, String originalXML) {
     super(config);
     try {
       checkForIllegalConfig();
       fillPropMap();
       config.substituteProperties();
       initCoreList();
+      this.persistor = isPersistent() ? new SolrXMLCoresLocator(configFile, originalXML, this)
+                                      : new SolrXMLCoresLocator.NonPersistingLocator(configFile, originalXML, this);
     }
     catch (IOException e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
     }
   }
 
+  @Override
+  public CoresLocator getCoresLocator() {
+    return this.persistor;
+  }
+  
   private void checkForIllegalConfig() throws IOException {
     // Do sanity checks - we don't want to find new style
     // config
@@ -101,6 +110,10 @@ public class ConfigSolrXmlOld extends ConfigSolr {
     }
   }
 
+  public boolean isPersistent() {
+    return config.getBool("solr/@persistent", false);
+  }
+  
   private void fillPropMap() {
     
     propMap.put(CfgProp.SOLR_CORELOADTHREADS,
@@ -208,34 +221,6 @@ public class ConfigSolrXmlOld extends ConfigSolr {
     
   }
 
-  @Override
-  public Map<String, String> readCoreAttributes(String coreName) {
-    Map<String, String> attrs = new HashMap<String, String>();
-
-    synchronized (coreNodes) {
-      for (int idx = 0; idx < coreNodes.getLength(); ++idx) {
-        Node node = coreNodes.item(idx);
-        if (coreName.equals(DOMUtil.getAttr(node, CoreDescriptor.CORE_NAME, null))) {
-          NamedNodeMap attributes = node.getAttributes();
-          for (int i = 0; i < attributes.getLength(); i++) {
-            Node attribute = attributes.item(i);
-            String val = PropertiesUtil.substituteProperty(attribute.getNodeValue(), null);
-            if (CoreDescriptor.CORE_DATADIR.equals(attribute.getNodeName()) ||
-                CoreDescriptor.CORE_INSTDIR.equals(attribute.getNodeName())) {
-              if (val.indexOf('$') == -1) {
-                val = (val != null && !val.endsWith("/")) ? val + '/' : val;
-              }
-            }
-            attrs.put(attribute.getNodeName(), val);
-          }
-          return attrs;
-        }
-      }
-    }
-    return attrs;
-  }
-
-  @Override
   public List<String> getAllCoreNames() {
     List<String> ret = new ArrayList<String>();
     
@@ -249,7 +234,6 @@ public class ConfigSolrXmlOld extends ConfigSolr {
     return ret;
   }
 
-  @Override
   public String getProperty(String coreName, String property, String defaultVal) {
     
     synchronized (coreNodes) {
@@ -257,7 +241,9 @@ public class ConfigSolrXmlOld extends ConfigSolr {
         Node node = coreNodes.item(idx);
         if (coreName.equals(DOMUtil.getAttr(node, CoreDescriptor.CORE_NAME,
             null))) {
-          String propVal = DOMUtil.getAttr(node, property, defaultVal);
+          String propVal = DOMUtil.getAttr(node, property);
+          if (propVal == null)
+            propVal = defaultVal;
           return PropertiesUtil.substituteProperty(propVal, null);
         }
       }
@@ -266,24 +252,20 @@ public class ConfigSolrXmlOld extends ConfigSolr {
     
   }
 
-  @Override
+  public Properties getCoreProperties(String coreName) {
-  public Properties readCoreProperties(String coreName) {
-    
     synchronized (coreNodes) {
-      for (int idx = 0; idx < coreNodes.getLength(); ++idx) {
+      for (int idx = 0; idx < coreNodes.getLength(); idx++) {
         Node node = coreNodes.item(idx);
-        if (coreName.equals(DOMUtil.getAttr(node, CoreDescriptor.CORE_NAME,
+        if (coreName.equals(DOMUtil.getAttr(node, CoreDescriptor.CORE_NAME, null))) {
-            null))) {
           try {
             return readProperties(node);
           } catch (XPathExpressionException e) {
-            return null;
+            SolrException.log(log, e);
           }
         }
       }
     }
-    
+    return new Properties();
-    return null;
   }
 
   public static final String DEF_SOLR_XML = "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
@@ -298,9 +280,4 @@ public class ConfigSolrXmlOld extends ConfigSolr {
         + "\" shard=\"${shard:}\" collection=\"${collection:collection1}\" instanceDir=\"collection1\" />\n"
         + "  </cores>\n" + "</solr>";
 
-  @Override
-  public void substituteProperties() {
-    config.substituteProperties();
-  }
-
 }

solr/core/src/java/org/apache/solr/core/CoreContainer.java

@@ -17,6 +17,7 @@
 
 package org.apache.solr.core;
 
+import com.google.common.collect.Maps;
 import org.apache.solr.cloud.ZkController;
 import org.apache.solr.cloud.ZkSolrResourceLoader;
 import org.apache.solr.common.SolrException;
@@ -33,18 +34,15 @@ import org.apache.solr.schema.IndexSchema;
 import org.apache.solr.schema.IndexSchemaFactory;
 import org.apache.solr.util.DefaultSolrThreadFactory;
 import org.apache.solr.util.FileUtils;
-import org.apache.solr.util.PropertiesUtil;
 import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.xml.xpath.XPathExpressionException;
 import java.io.File;
 import java.text.SimpleDateFormat;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -116,8 +114,11 @@ public class CoreContainer
   protected final ConfigSolr cfg;
   protected final SolrResourceLoader loader;
   protected final String solrHome;
+
   private InfoHandler infoHandler;
 
+  protected final CoresLocator coresLocator;
+
   {
     log.info("New CoreContainer " + System.identityHashCode(this));
   }
@@ -152,8 +153,9 @@ public class CoreContainer
   }
 
   /**
-   * Create a new CoreContainer using the given SolrResourceLoader and
+   * Create a new CoreContainer using the given SolrResourceLoader,
-   * configuration.  The container's cores are not loaded.
+   * configuration and CoresLocator.  The container's cores are
+   * not loaded.
    * @param loader the SolrResourceLoader
    * @param config a ConfigSolr representation of this container's configuration
    * @see #load()
@@ -162,6 +164,14 @@ public class CoreContainer
     this.loader = checkNotNull(loader);
     this.solrHome = loader.getInstanceDir();
     this.cfg = checkNotNull(config);
+    this.coresLocator = config.getCoresLocator();
+  }
+
+  public CoreContainer(SolrResourceLoader loader, ConfigSolr config, CoresLocator locator) {
+    this.loader = checkNotNull(loader);
+    this.solrHome = loader.getInstanceDir();
+    this.cfg = checkNotNull(config);
+    this.coresLocator = locator;
   }
 
   /**
@@ -280,81 +290,20 @@ public class CoreContainer
           coreLoadExecutor);
       Set<Future<SolrCore>> pending = new HashSet<Future<SolrCore>>();
 
-      List<String> allCores = cfg.getAllCoreNames();
+      List<CoreDescriptor> cds = coresLocator.discover(this);
+      checkForDuplicateCoreNames(cds);
 
-      for (String oneCoreName : allCores) {
+      for (final CoreDescriptor cd : cds) {
 
+        final String name = cd.getName();
         try {
-          String rawName = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_NAME, null);
 
-          if (null == rawName) {
+          if (cd.isTransient() || ! cd.isLoadOnStartup()) {
-            throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
-                "Each core in solr.xml must have a 'name'");
-          }
-          final String name = rawName;
-          final CoreDescriptor p = new CoreDescriptor(this, name,
-              cfg.getProperty(oneCoreName, CoreDescriptor.CORE_INSTDIR, null));
-          
-          // deal with optional settings
-          String opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_CONFIG, null);
-          
-          if (opt != null) {
-            p.setConfigName(opt);
-          }
-          opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_SCHEMA, null);
-          if (opt != null) {
-            p.setSchemaName(opt);
-          }
-          
-          if (zkSys.getZkController() != null) {
-            opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_SHARD, null);
-            if (opt != null && opt.length() > 0) {
-              p.getCloudDescriptor().setShardId(opt);
-            }
-            opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_COLLECTION, null);
-            if (opt != null) {
-              p.getCloudDescriptor().setCollectionName(opt);
-            }
-            opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_ROLES, null);
-            if (opt != null) {
-              p.getCloudDescriptor().setRoles(opt);
-            }
-
-            opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_NODE_NAME, null);
-            if (opt != null && opt.length() > 0) {
-              p.getCloudDescriptor().setCoreNodeName(opt);
-            }
-          }
-          opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_PROPERTIES, null);
-          if (opt != null) {
-            p.setPropertiesName(opt);
-          }
-          opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_DATADIR, null);
-          if (opt != null) {
-            p.setDataDir(opt);
-          }
-          
-          p.setCoreProperties(cfg.readCoreProperties(oneCoreName));
-          
-          opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_LOADONSTARTUP, null);
-          if (opt != null) {
-            p.setLoadOnStartup(("true".equalsIgnoreCase(opt) || "on"
-                .equalsIgnoreCase(opt)) ? true : false);
-          }
-          
-          opt = cfg.getProperty(oneCoreName, CoreDescriptor.CORE_TRANSIENT, null);
-          if (opt != null) {
-            p.setTransient(("true".equalsIgnoreCase(opt) || "on"
-                .equalsIgnoreCase(opt)) ? true : false);
-          }
-
-          if (p.isTransient() || ! p.isLoadOnStartup()) {
             // Store it away for later use. includes non-transient but not
             // loaded at startup cores.
-            solrCores.putDynamicDescriptor(rawName, p);
+            solrCores.putDynamicDescriptor(name, cd);
           }
-
+          if (cd.isLoadOnStartup()) { // The normal case
-          if (p.isLoadOnStartup()) { // The normal case
 
             Callable<SolrCore> task = new Callable<SolrCore>() {
               @Override
@@ -362,14 +311,14 @@ public class CoreContainer
                 SolrCore c = null;
                 try {
                   if (zkSys.getZkController() != null) {
-                    preRegisterInZk(p);
+                    preRegisterInZk(cd);
                   }
-                  c = create(p);
+                  c = create(cd);
-                  registerCore(p.isTransient(), name, c, false);
+                  registerCore(cd.isTransient(), name, c, false);
                 } catch (Throwable t) {
                   if (isZooKeeperAware()) {
                     try {
-                      zkSys.zkController.unregister(name, p);
+                      zkSys.zkController.unregister(name, cd);
                     } catch (InterruptedException e) {
                       Thread.currentThread().interrupt();
                       SolrException.log(log, null, e);
@@ -427,6 +376,18 @@ public class CoreContainer
     }
   }
 
+  private static void checkForDuplicateCoreNames(List<CoreDescriptor> cds) {
+    Map<String, String> addedCores = Maps.newHashMap();
+    for (CoreDescriptor cd : cds) {
+      final String name = cd.getName();
+      if (addedCores.containsKey(name))
+        throw new SolrException(ErrorCode.SERVER_ERROR,
+            String.format(Locale.ROOT, "Found multiple cores with the name [%s], with instancedirs [%s] and [%s]",
+                name, addedCores.get(name), cd.getInstanceDir()));
+      addedCores.put(name, cd.getInstanceDir());
+    }
+  }
+
   private volatile boolean isShutDown = false;
   
   public boolean isShutDown() {
@@ -521,6 +482,10 @@ public class CoreContainer
     }
   }
 
+  public CoresLocator getCoresLocator() {
+    return coresLocator;
+  }
+
   protected SolrCore registerCore(boolean isTransientCore, String name, SolrCore core, boolean returnPrevNotClosed) {
     if( core == null ) {
       throw new RuntimeException( "Can not register a null core." );
@@ -548,7 +513,6 @@ public class CoreContainer
       */
 
     core.setName(name);
-    core.getCoreDescriptor().putProperty(CoreDescriptor.CORE_NAME, name);
 
     synchronized (coreInitFailures) {
       coreInitFailures.remove(name);
@@ -587,7 +551,7 @@ public class CoreContainer
     SolrResourceLoader solrLoader = null;
 
     SolrConfig config = null;
-    solrLoader = new SolrResourceLoader(instanceDir, loader.getClassLoader(), ConfigSolrXml.getCoreProperties(instanceDir, dcore));
+    solrLoader = new SolrResourceLoader(instanceDir, loader.getClassLoader(), dcore.getCoreProperties());
     try {
       config = new SolrConfig(solrLoader, dcore.getConfigName(), null);
     } catch (Exception e) {
@@ -609,11 +573,11 @@ public class CoreContainer
             schemaFile.lastModified()));
         schema = indexSchemaCache.get(key);
         if (schema == null) {
-          log.info("creating new schema object for core: " + dcore.getProperty(CoreDescriptor.CORE_NAME));
+          log.info("creating new schema object for core: " + dcore.getName());
           schema = IndexSchemaFactory.buildIndexSchema(dcore.getSchemaName(), config);
           indexSchemaCache.put(key, schema);
         } else {
-          log.info("re-using schema object for core: " + dcore.getProperty(CoreDescriptor.CORE_NAME));
+          log.info("re-using schema object for core: " + dcore.getName());
         }
       }
     }
@@ -752,7 +716,8 @@ public class CoreContainer
                  cd.getName(), instanceDir.getAbsolutePath());
         SolrResourceLoader solrLoader;
         if(zkSys.getZkController() == null) {
-          solrLoader = new SolrResourceLoader(instanceDir.getAbsolutePath(), loader.getClassLoader(), ConfigSolrXml.getCoreProperties(instanceDir.getAbsolutePath(), cd));
+          solrLoader = new SolrResourceLoader(instanceDir.getAbsolutePath(), loader.getClassLoader(),
+                                                cd.getCoreProperties());
         } else {
           try {
             String collection = cd.getCloudDescriptor().getCollectionName();
@@ -765,7 +730,7 @@ public class CoreContainer
                                            "Could not find config name for collection:" + collection);
             }
             solrLoader = new ZkSolrResourceLoader(instanceDir.getAbsolutePath(), zkConfigName, loader.getClassLoader(),
-                ConfigSolrXml.getCoreProperties(instanceDir.getAbsolutePath(), cd), zkSys.getZkController());
+                cd.getCoreProperties(), zkSys.getZkController());
           } catch (KeeperException e) {
             log.error("", e);
             throw new ZooKeeperException(SolrException.ErrorCode.SERVER_ERROR,
@@ -808,15 +773,18 @@ public class CoreContainer
     n1 = checkDefault(n1);
     solrCores.swap(n0, n1);
 
+    coresLocator.persist(this, solrCores.getCoreDescriptor(n0), solrCores.getCoreDescriptor(n1));
+
     log.info("swapped: "+n0 + " with " + n1);
   }
   
   /** Removes and returns registered core w/o decrementing it's reference count */
   public SolrCore remove( String name ) {
     name = checkDefault(name);
-
+    CoreDescriptor cd = solrCores.getCoreDescriptor(name);
-    return solrCores.remove(name, true);
+    SolrCore removed = solrCores.remove(name, true);
-
+    coresLocator.delete(this, cd);
+    return removed;
   }
 
   public void rename(String name, String toName) {
@@ -825,7 +793,8 @@ public class CoreContainer
       if (core != null) {
         registerCore(false, toName, core, false);
         name = checkDefault(name);
-        solrCores.remove(name, false);
+        SolrCore old = solrCores.remove(name, false);
+        coresLocator.rename(this, old.getCoreDescriptor(), core.getCoreDescriptor());
       }
     } finally {
       if (core != null) {
@@ -833,6 +802,24 @@ public class CoreContainer
       }
     }
   }
+
+  /**
+   * Get the CoreDescriptors for all cores managed by this container
+   * @return a List of CoreDescriptors
+   */
+  public List<CoreDescriptor> getCoreDescriptors() {
+    return solrCores.getCoreDescriptors();
+  }
+
+  public CoreDescriptor getCoreDescriptor(String coreName) {
+    // TODO make this less hideous!
+    for (CoreDescriptor cd : getCoreDescriptors()) {
+      if (cd.getName().equals(coreName))
+        return cd;
+    }
+    return null;
+  }
+
   /** 
    * Gets a core by name and increase its refcount.
    *
@@ -972,10 +959,6 @@ public class CoreContainer
     logging = v;
   }
 
-  public File getConfigFile() {
-    return new File(solrHome, ConfigSolr.SOLR_XML_FILE);
-  }
-
   /**
    * Determines whether the core is already loaded or not but does NOT load the core
    *
@@ -984,12 +967,6 @@ public class CoreContainer
     return solrCores.isLoaded(name);
   }
 
-  /** Persists the cores config file in cores.xml. */
-  @Deprecated
-  public void persist() {
-    persistFile(getConfigFile());
-  }
-
   /**
    * Gets a solr core descriptor for a core that is not loaded. Note that if the caller calls this on a
    * loaded core, the unloaded descriptor will be returned.
@@ -1001,126 +978,6 @@ public class CoreContainer
     return solrCores.getUnloadedCoreDescriptor(cname);
   }
 
-  /** Persists the cores config file in a user provided file. */
-  @Deprecated
-  public void persistFile(File file) {
-    assert file != null;
-    // only the old solrxml persists
-    if (cfg != null && !(cfg instanceof ConfigSolrXmlOld)) return;
-
-    log.info("Persisting cores config to " + (file == null ? getConfigFile() : file));
-
-    // <solr attrib="value">
-    Map<String,String> rootSolrAttribs = new HashMap<String,String>();
-
-    addAttrib(rootSolrAttribs, ConfigSolr.CfgProp.SOLR_SHAREDLIB, "sharedLib", this.libDir);
-    addAttrib(rootSolrAttribs, ConfigSolr.CfgProp.SOLR_PERSISTENT, "persistent",
-        Boolean.toString(isPersistent()), "false");
-    addAttrib(rootSolrAttribs, ConfigSolr.CfgProp.SOLR_CORELOADTHREADS, "coreLoadThreads",
-        Integer.toString(this.coreLoadThreads), Integer.toString(CORE_LOAD_THREADS));
-    addAttrib(rootSolrAttribs, ConfigSolr.CfgProp.SOLR_ZKHOST, "zkHost", this.zkHost);
-
-    // <solr attrib="value"> <cores attrib="value">
-    Map<String,String> coresAttribs = new HashMap<String,String>();
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ADMINPATH, "adminPath", this.adminPath, this.getAdminPath());
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ADMINHANDLER, "adminHandler", this.adminHandler);
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_SHARESCHEMA, "shareSchema",
-        Boolean.toString(this.shareSchema),
-        Boolean.toString(DEFAULT_SHARE_SCHEMA));
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOST, "host", zkSys.getHost());
-
-    if (! (null == defaultCoreName || defaultCoreName.equals("")) ) {
-      coresAttribs.put("defaultCoreName", defaultCoreName);
-    }
-
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOSTPORT, "hostPort", zkSys.getHostPort());
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_ZKCLIENTTIMEOUT, "zkClientTimeout",
-        intToString(this.zkClientTimeout),
-        Integer.toString(DEFAULT_ZK_CLIENT_TIMEOUT));
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_HOSTCONTEXT, "hostContext",
-        zkSys.getHostContext());
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_LEADERVOTEWAIT, "leaderVoteWait",
-        zkSys.getLeaderVoteWait(), LEADER_VOTE_WAIT);
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_GENERICCORENODENAMES, "genericCoreNodeNames",
-        Boolean.toString(zkSys.getGenericCoreNodeNames()), "false");
-    if (transientCacheSize != Integer.MAX_VALUE) { // This test
-    // is a consequence of testing. I really hate it.
-      addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_TRANSIENTCACHESIZE, "transientCacheSize",
-          Integer.toString(this.transientCacheSize), Integer.toString(Integer.MAX_VALUE));
-    }
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_DISTRIBUPDATECONNTIMEOUT, "distribUpdateConnTimeout",
-        Integer.toString(this.distribUpdateConnTimeout), Integer.toString(this.distribUpdateConnTimeout));
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_DISTRIBUPDATESOTIMEOUT, "distribUpdateSoTimeout",
-        Integer.toString(this.distribUpdateSoTimeout), Integer.toString(this.distribUpdateSoTimeout));
-    addAttrib(coresAttribs, ConfigSolr.CfgProp.SOLR_MANAGEMENTPATH, "managementPath",
-        this.managementPath);
-
-    // don't forget the logging stuff
-    Map<String, String> loggingAttribs = new HashMap<String, String>();
-    addAttrib(loggingAttribs, ConfigSolr.CfgProp.SOLR_LOGGING_CLASS, "class",
-        cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_CLASS, null));
-    addAttrib(loggingAttribs, ConfigSolr.CfgProp.SOLR_LOGGING_ENABLED, "enabled",
-        cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_ENABLED, null));
-
-    Map<String, String> watcherAttribs = new HashMap<String, String>();
-    addAttrib(watcherAttribs, ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_SIZE, "size",
-        cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_SIZE, null));
-    addAttrib(watcherAttribs, ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, "threshold",
-        cfg.get(ConfigSolr.CfgProp.SOLR_LOGGING_WATCHER_THRESHOLD, null));
-
-
-    /*
-    Map<String, String> shardHandlerAttrib = new HashMap<String, String>();
-    addAttrib(shardHandlerAttrib, ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CLASS, "class",
-        cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CLASS, null));
-    addAttrib(shardHandlerAttrib, ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_NAME, "name",
-        cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_NAME, null));
-
-    Map<String, String> shardHandlerProps = new HashMap<String, String>();
-    addAttrib(shardHandlerProps, ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, "connTimeout",
-        cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_CONNTIMEOUT, null));
-    addAttrib(shardHandlerProps, ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, "socketTimeout",
-        cfg.get(ConfigSolr.CfgProp.SOLR_SHARDHANDLERFACTORY_SOCKETTIMEOUT, null));
-    */
-
-    try {
-      solrCores.persistCores(cfg.config.getOriginalConfig(), containerProperties, rootSolrAttribs,coresAttribs,
-          loggingAttribs, watcherAttribs, cfg.getUnsubsititutedShardHandlerFactoryPluginNode(), file, loader);
-    } catch (XPathExpressionException e) {
-      throw new SolrException(ErrorCode.SERVER_ERROR, null, e);
-    }
-
-  }
-  private String intToString(Integer integer) {
-    if (integer == null) return null;
-    return Integer.toString(integer);
-  }
-
-  private void addAttrib(Map<String, String> attribs, ConfigSolr.CfgProp prop,
-                         String attribName, String attribValue) {
-    addAttrib(attribs, prop, attribName, attribValue, null);
-  }
-
-    private void addAttrib(Map<String, String> attribs, ConfigSolr.CfgProp prop,
-                         String attribName, String attribValue, String defaultValue) {
-    if (cfg == null) {
-      attribs.put(attribName, attribValue);
-      return;
-    }
-
-    if (attribValue != null) {
-      String origValue = cfg.getOrigProp(prop, null);
-
-      if (origValue == null && defaultValue != null && attribValue.equals(defaultValue)) return;
-
-      if (attribValue.equals(PropertiesUtil.substituteProperty(origValue, loader.getCoreProperties()))) {
-        attribs.put(attribName, origValue);
-      } else {
-        attribs.put(attribName, attribValue);
-      }
-    }
-  }
-
   public void preRegisterInZk(final CoreDescriptor p) {
     zkSys.getZkController().preRegister(p);
   }

solr/core/src/java/org/apache/solr/core/CoreDescriptor.java

@@ -17,12 +17,20 @@
 
 package org.apache.solr.core;
 
-import java.util.Properties;
+import com.google.common.collect.ImmutableList;
-import java.io.File;
+import com.google.common.collect.ImmutableMap;
-
 import org.apache.commons.lang.StringUtils;
 import org.apache.solr.cloud.CloudDescriptor;
-import org.apache.solr.core.ConfigSolr.CfgProp;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.util.PropertiesUtil;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.util.Locale;
+import java.util.Properties;
+
+import static com.google.common.base.Preconditions.checkNotNull;
 
 /**
  * A Solr core descriptor
@@ -34,7 +42,8 @@ public class CoreDescriptor {
   // Properties file name constants
   public static final String CORE_NAME = "name";
   public static final String CORE_CONFIG = "config";
-  public static final String CORE_INSTDIR = "instanceDir"; // should probably be removed after 4x
+  public static final String CORE_INSTDIR = "instanceDir";
+  public static final String CORE_ABS_INSTDIR = "absoluteInstDir";
   public static final String CORE_DATADIR = "dataDir";
   public static final String CORE_ULOGDIR = "ulogDir";
   public static final String CORE_SCHEMA = "schema";
@@ -46,153 +55,207 @@ public class CoreDescriptor {
   public static final String CORE_TRANSIENT = "transient";
   public static final String CORE_NODE_NAME = "coreNodeName";
 
-  public static final String[] standardPropNames = {
+  public static final String DEFAULT_EXTERNAL_PROPERTIES_FILE = "conf" + File.separator + "solrcore.properties";
+
+  /**
+   * Get the standard properties in persistable form
+   * @return the standard core properties in persistable form
+   */
+  public Properties getPersistableStandardProperties() {
+    return originalCoreProperties;
+  }
+
+  /**
+   * Get user-defined core properties in persistable form
+   * @return user-defined core properties in persistable form
+   */
+  public Properties getPersistableUserProperties() {
+    return originalExtraProperties;
+  }
+
+  private static ImmutableMap<String, String> defaultProperties = ImmutableMap.of(
+      CORE_CONFIG, "solrconfig.xml",
+      CORE_SCHEMA, "schema.xml",
+      CORE_DATADIR, "data" + File.separator,
+      CORE_TRANSIENT, "false",
+      CORE_LOADONSTARTUP, "true"
+  );
+
+  private static ImmutableList<String> requiredProperties = ImmutableList.of(
+      CORE_NAME, CORE_INSTDIR, CORE_ABS_INSTDIR
+  );
+
+  public static ImmutableList<String> standardPropNames = ImmutableList.of(
       CORE_NAME,
       CORE_CONFIG,
       CORE_INSTDIR,
       CORE_DATADIR,
       CORE_ULOGDIR,
       CORE_SCHEMA,
+      CORE_PROPERTIES,
+      CORE_LOADONSTARTUP,
+      CORE_TRANSIENT,
+      // cloud props
       CORE_SHARD,
       CORE_COLLECTION,
       CORE_ROLES,
-      CORE_PROPERTIES,
+      CORE_NODE_NAME,
-      CORE_LOADONSTARTUP,
+      CloudDescriptor.NUM_SHARDS,
-      CORE_TRANSIENT
+      CloudDescriptor.SHARD_STATE
-  };
+  );
-
-  // As part of moving away from solr.xml (see SOLR-4196), it's _much_ easier to keep these as properties than set
-  // them individually.
-  private Properties coreProperties = new Properties();
-
-  //TODO: 5.0 remove this, this is solely a hack for persistence. And perhaps creating cores in discovery mode?
-  private Properties createdProperties = new Properties();
-
-  private boolean loadedImplicit = false;
 
   private final CoreContainer coreContainer;
 
-  private CloudDescriptor cloudDesc;
+  private final CloudDescriptor cloudDesc;
 
-  private CoreDescriptor(CoreContainer cont) {
+  /** The original standard core properties, before substitution */
-    // Just a place to put initialization since it's a pain to add to the descriptor in every c'tor.
+  protected final Properties originalCoreProperties = new Properties();
-    this.coreContainer = cont;
-    coreProperties.put(CORE_LOADONSTARTUP, "true");
-    coreProperties.put(CORE_TRANSIENT, "false");
 
-  }
+  /** The original extra core properties, before substitution */
+  protected final Properties originalExtraProperties = new Properties();
 
-  public CoreDescriptor(CoreContainer container, String name, String instanceDir) {
+  /** The properties for this core, as available through getProperty() */
-    this(container);
+  protected final Properties coreProperties = new Properties();
-    doInit(name, instanceDir);
-  }
 
+  /**
+   * Create a new CoreDescriptor.
+   * @param container       the CoreDescriptor's container
+   * @param name            the CoreDescriptor's name
+   * @param instanceDir     a String containing the instanceDir
+   * @param coreProps       a Properties object of the properties for this core
+   */
+  public CoreDescriptor(CoreContainer container, String name, String instanceDir,
+                        Properties coreProps) {
 
-  public CoreDescriptor(CoreDescriptor descr) {
+    this.coreContainer = container;
-    this(descr.coreContainer);
+
-    coreProperties.put(CORE_INSTDIR, descr.getInstanceDir());
+    originalCoreProperties.setProperty(CORE_NAME, name);
-    coreProperties.put(CORE_CONFIG, descr.getConfigName());
+    originalCoreProperties.setProperty(CORE_INSTDIR, instanceDir);
-    coreProperties.put(CORE_SCHEMA, descr.getSchemaName());
+
-    coreProperties.put(CORE_NAME, descr.getName());
+    Properties containerProperties = container.getContainerProperties();
-    coreProperties.put(CORE_DATADIR, descr.getDataDir());
+    name = PropertiesUtil.substituteProperty(checkPropertyIsNotEmpty(name, CORE_NAME),
+                                             containerProperties);
+    instanceDir = PropertiesUtil.substituteProperty(checkPropertyIsNotEmpty(instanceDir, CORE_INSTDIR),
+                                                    containerProperties);
+
+    coreProperties.putAll(defaultProperties);
+    coreProperties.put(CORE_NAME, name);
+    coreProperties.put(CORE_INSTDIR, instanceDir);
+    coreProperties.put(CORE_ABS_INSTDIR, convertToAbsolute(instanceDir, container.getSolrHome()));
+
+    for (String propname : coreProps.stringPropertyNames()) {
+
+      String propvalue = coreProps.getProperty(propname);
+
+      if (isUserDefinedProperty(propname))
+        originalExtraProperties.put(propname, propvalue);
+      else
+        originalCoreProperties.put(propname, propvalue);
+
+      if (!requiredProperties.contains(propname))   // Required props are already dealt with
+        coreProperties.setProperty(propname,
+            PropertiesUtil.substituteProperty(propvalue, containerProperties));
+    }
+
+    loadExtraProperties();
+
+    // TODO maybe make this a CloudCoreDescriptor subclass?
+    if (container.isZooKeeperAware()) {
+      cloudDesc = new CloudDescriptor(name, coreProperties);
+    }
+    else {
+      cloudDesc = null;
+    }
   }
 
   /**
-   * CoreDescriptor - create a core descriptor given default properties from a core.properties file. This will be
+   * Load properties specified in an external properties file.
-   * used in the "solr.xml-less (See SOLR-4196) world where there are no &lt;core&gt; &lt;/core&gt; tags at all, thus  much
-   * of the initialization that used to be done when reading solr.xml needs to be done here instead, particularly
-   * setting any defaults (e.g. schema.xml, directories, whatever).
    *
-   * @param container - the CoreContainer that holds all the information about our cores, loaded, lazy etc.
+   * The file to load can be specified in a {@code properties} property on
-   * @param propsIn - A properties structure "core.properties" found while walking the file tree to discover cores.
+   * the original Properties object used to create this CoreDescriptor.  If
-   *                  Any properties set in this param will overwrite the any defaults.
+   * this has not been set, then we look for {@code conf/solrcore.properties}
+   * underneath the instance dir.
+   *
+   * File paths are taken as read from the core's instance directory
+   * if they are not absolute.
    */
-  public CoreDescriptor(CoreContainer container, Properties propsIn) {
+  protected void loadExtraProperties() {
-    this(container);
+    String filename = coreProperties.getProperty(CORE_PROPERTIES, DEFAULT_EXTERNAL_PROPERTIES_FILE);
-
+    File propertiesFile = resolvePaths(filename);
-    // Set some default, normalize a directory or two
+    if (propertiesFile.exists()) {
-    doInit(propsIn.getProperty(CORE_NAME), propsIn.getProperty(CORE_INSTDIR));
+      try {
-
+        Properties externalProps = new Properties();
-    coreProperties.putAll(propsIn);
+        externalProps.load(new FileInputStream(propertiesFile));
-  }
+        coreProperties.putAll(externalProps);
-
+      }
-  private void doInit(String name, String instanceDir) {
+      catch (IOException e) {
-    if (name == null) {
+        String message = String.format(Locale.ROOT, "Could not load properties from %s: %s:",
-      throw new RuntimeException("Core needs a name");
+            propertiesFile.getAbsoluteFile(), e.toString());
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, message);
+      }
     }
+  }
 
-    coreProperties.put(CORE_NAME, name);
+  protected File resolvePaths(String filepath) {
+    File file = new File(filepath);
+    if (file.isAbsolute())
+      return file;
+    return new File(getInstanceDir(), filepath);
+  }
 
-    if(coreContainer != null && coreContainer.getZkController() != null) {
+  /**
-      this.cloudDesc = new CloudDescriptor();
+   * Is this property a Solr-standard property, or is it an extra property
-      // cloud collection defaults to core name
+   * defined per-core by the user?
-      cloudDesc.setCollectionName(name);
+   * @param propName the Property name
+   * @return @{code true} if this property is user-defined
+   */
+  protected static boolean isUserDefinedProperty(String propName) {
+    return !standardPropNames.contains(propName);
+  }
+
+  public static String checkPropertyIsNotEmpty(String value, String propName) {
+    if (StringUtils.isEmpty(value)) {
+      String message = String.format(Locale.ROOT, "Cannot create core with empty %s value", propName);
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, message);
     }
-
+    return value;
-    if (instanceDir == null) {
-      throw new NullPointerException("Missing required \'instanceDir\'");
-    }
-    instanceDir = SolrResourceLoader.normalizeDir(instanceDir);
-    coreProperties.put(CORE_INSTDIR, instanceDir);
-    coreProperties.put(CORE_CONFIG, getDefaultConfigName());
-    coreProperties.put(CORE_SCHEMA, getDefaultSchemaName());
   }
 
-  public Properties initImplicitProperties() {
+  /**
-
+   * Create a new CoreDescriptor with a given name and instancedir
-    Properties implicitProperties = new Properties();
+   * @param container     the CoreDescriptor's container
-    if (coreContainer != null && coreContainer.getContainerProperties() != null){
+   * @param name          the CoreDescriptor's name
-      implicitProperties.putAll(coreContainer.getContainerProperties());
+   * @param instanceDir   the CoreDescriptor's instancedir
-    }
+   */
-    implicitProperties.setProperty("solr.core.name", getName());
+  public CoreDescriptor(CoreContainer container, String name, String instanceDir) {
-    implicitProperties.setProperty("solr.core.instanceDir", getInstanceDir());
+    this(container, name, instanceDir, new Properties());
-    implicitProperties.setProperty("solr.core.dataDir", getDataDir());
-    implicitProperties.setProperty("solr.core.configName", getConfigName());
-    implicitProperties.setProperty("solr.core.schemaName", getSchemaName());
-    return implicitProperties;
   }
 
-  /**@return the default config name. */
+  /**
-  public String getDefaultConfigName() {
+   * Create a new CoreDescriptor using the properties of an existing one
-    return "solrconfig.xml";
+   * @param coreName the new CoreDescriptor's name
-  }
+   * @param other    the CoreDescriptor to copy
-
+   */
-  /**@return the default schema name. */
+  public CoreDescriptor(String coreName, CoreDescriptor other) {
-  public String getDefaultSchemaName() {
+    this.coreContainer = other.coreContainer;
-    return "schema.xml";
+    this.originalExtraProperties.putAll(other.originalExtraProperties);
-  }
+    this.originalCoreProperties.putAll(other.originalCoreProperties);
-
+    this.coreProperties.putAll(other.coreProperties);
-  /**@return the default data directory. */
+    this.coreProperties.setProperty(CORE_NAME, coreName);
-  public String getDefaultDataDir() {
+    this.originalCoreProperties.setProperty(CORE_NAME, coreName);
-    return "data" + File.separator;
+    this.cloudDesc = other.cloudDesc;
   }
 
   public String getPropertiesName() {
     return coreProperties.getProperty(CORE_PROPERTIES);
   }
 
-  public void setPropertiesName(String propertiesName) {
-    coreProperties.put(CORE_PROPERTIES, propertiesName);
-  }
-
   public String getDataDir() {
-    String dataDir = coreProperties.getProperty(CORE_DATADIR);
+    return coreProperties.getProperty(CORE_DATADIR);
-    if (dataDir == null) dataDir = getDefaultDataDir();
-    return dataDir;
-  }
-
-  public void setDataDir(String s) {
-    // normalize zero length to null.
-    if (StringUtils.isBlank(s)) {
-      coreProperties.remove(s);
-    } else {
-      coreProperties.put(CORE_DATADIR, s);
-    }
   }
   
   public boolean usingDefaultDataDir() {
-    // DO NOT use the getDataDir method here since it'll assign something regardless.
+    return defaultProperties.get(CORE_DATADIR).equals(coreProperties.getProperty(CORE_DATADIR));
-    return coreProperties.getProperty(CORE_DATADIR) == null;
   }
 
   /**@return the core instance directory. */
@@ -200,37 +263,20 @@ public class CoreDescriptor {
     return coreProperties.getProperty(CORE_INSTDIR);
   }
 
+  private static String convertToAbsolute(String instDir, String solrHome) {
+    checkNotNull(instDir);
+    File f = new File(instDir);
+    if (f.isAbsolute())
+      return SolrResourceLoader.normalizeDir(instDir);
+    return SolrResourceLoader.normalizeDir(solrHome + SolrResourceLoader.normalizeDir(instDir));
+  }
+
   /**
    *
    * @return the core instance directory, prepended with solr_home if not an absolute path.
    */
   public String getInstanceDir() {
-    String instDir = coreProperties.getProperty(CORE_INSTDIR);
+    return coreProperties.getProperty(CORE_ABS_INSTDIR);
-    if (instDir == null) return null;
-
-    if (new File(instDir).isAbsolute()) {
-      return SolrResourceLoader.normalizeDir(
-          SolrResourceLoader.normalizeDir(instDir));
-    }
-
-    if (coreContainer == null) return null;
-    if( coreContainer.cfg != null) {
-      String coreRootDir = coreContainer.cfg.get(
-          CfgProp.SOLR_COREROOTDIRECTORY, null);
-      if (coreRootDir != null) {
-        return SolrResourceLoader.normalizeDir(coreRootDir
-            + SolrResourceLoader.normalizeDir(instDir));
-      }
-    }
-    return SolrResourceLoader.normalizeDir(coreContainer.getSolrHome() +
-        SolrResourceLoader.normalizeDir(instDir));
-  }
-
-  /**Sets the core configuration resource name. */
-  public void setConfigName(String name) {
-    if (name == null || name.length() == 0)
-      throw new IllegalArgumentException("name can not be null or empty");
-    coreProperties.put(CORE_CONFIG, name);
   }
 
   /**@return the core configuration resource name. */
@@ -238,13 +284,6 @@ public class CoreDescriptor {
     return coreProperties.getProperty(CORE_CONFIG);
   }
 
-  /**Sets the core schema resource name. */
-  public void setSchemaName(String name) {
-    if (name == null || name.length() == 0)
-      throw new IllegalArgumentException("name can not be null or empty");
-    coreProperties.put(CORE_SCHEMA, name);
-  }
-
   /**@return the core schema resource name. */
   public String getSchemaName() {
     return coreProperties.getProperty(CORE_SCHEMA);
@@ -255,103 +294,57 @@ public class CoreDescriptor {
     return coreProperties.getProperty(CORE_NAME);
   }
 
+  public String getCollectionName() {
+    return cloudDesc == null ? null : cloudDesc.getCollectionName();
+  }
+
   public CoreContainer getCoreContainer() {
     return coreContainer;
   }
 
-  Properties getCoreProperties() {
-    return coreProperties;
-  }
-
-  /**
-   * Set this core's properties. Please note that some implicit values will be added to the
-   * Properties instance passed into this method. This means that the Properties instance
-   * sent to this method will have different (less) key/value pairs than the Properties
-   * instance returned by #getCoreProperties method.
-   *
-   * Under any circumstance, the properties passed in will override any already present.Merge
-   */
-  public void setCoreProperties(Properties coreProperties) {
-    if (! loadedImplicit) {
-      loadedImplicit = true;
-      Properties p = initImplicitProperties();
-      this.coreProperties.putAll(p);
-      // The caller presumably wants whatever properties passed in to override the current core props, so just add them.
-      if (coreProperties != null) {
-        this.coreProperties.putAll(coreProperties);
-      }
-    }
-  }
-
-  public void addCreatedProperty(String key, String value) {
-    createdProperties.put(key, value);
-  }
-
-  public final Properties getCreatedProperties() {
-    return createdProperties;
-  }
-
   public CloudDescriptor getCloudDescriptor() {
     return cloudDesc;
   }
 
-  public void setCloudDescriptor(CloudDescriptor cloudDesc) {
-    this.cloudDesc = cloudDesc;
-  }
   public boolean isLoadOnStartup() {
     String tmp = coreProperties.getProperty(CORE_LOADONSTARTUP, "false");
     return Boolean.parseBoolean(tmp);
   }
 
-  public void setLoadOnStartup(boolean loadOnStartup) {
-    coreProperties.put(CORE_LOADONSTARTUP, Boolean.toString(loadOnStartup));
-  }
-
   public boolean isTransient() {
     String tmp = coreProperties.getProperty(CORE_TRANSIENT, "false");
-    return (Boolean.parseBoolean(tmp));
+    return PropertiesUtil.toBoolean(tmp);
-  }
-
-  public void setTransient(boolean isTransient) {
-    coreProperties.put(CORE_TRANSIENT, Boolean.toString(isTransient));
   }
 
   public String getUlogDir() {
     return coreProperties.getProperty(CORE_ULOGDIR);
   }
 
-  public void setUlogDir(String ulogDir) {
-    coreProperties.put(CORE_ULOGDIR, ulogDir);
-  }
-
   /**
-   * Reads a property defined in the core.properties file that's replacing solr.xml (if present).
+   * Returns a specific property defined on this CoreDescriptor
    * @param prop    - value to read from the properties structure.
    * @param defVal  - return if no property found.
    * @return associated string. May be null.
    */
-  public String getProperty(String prop, String defVal) {
+  public String getCoreProperty(String prop, String defVal) {
     return coreProperties.getProperty(prop, defVal);
   }
 
   /**
-   * gReads a property defined in the core.properties file that's replacing solr.xml (if present).
+   * Returns all properties defined on this CoreDescriptor
-   * @param prop  value to read from the properties structure.
+   * @return all properties defined on this CoreDescriptor
-   * @return associated string. May be null.
    */
-  public String getProperty(String prop) {
+  public Properties getCoreProperties() {
-    return coreProperties.getProperty(prop);
+    return coreProperties;
   }
-  /**
+
-   * This will eventually replace _all_ of the setters. Puts a value in the "new" (obsoleting solr.xml JIRAs) properties
+  @Override
-   * structures.
+  public String toString() {
-   *
+    return new StringBuilder("CoreDescriptor[name=")
-   * Will replace any currently-existing property with the key "prop".
+        .append(this.getName())
-   *
+        .append(";instanceDir=")
-   * @param prop - property name
+        .append(this.getInstanceDir())
-   * @param val  - property value
+        .append("]")
-   */
+        .toString();
-  public void putProperty(String prop, String val) {
-    coreProperties.put(prop, val);
   }
 }

solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java

@@ -0,0 +1,158 @@
+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 com.google.common.base.Charsets;
+import com.google.common.collect.Lists;
+import org.apache.solr.common.SolrException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.util.Date;
+import java.util.List;
+import java.util.Properties;
+
+/**
+ * Persists CoreDescriptors as properties files
+ */
+public class CorePropertiesLocator implements CoresLocator {
+
+  public static final String PROPERTIES_FILENAME = "core.properties";
+
+  private static final Logger logger = LoggerFactory.getLogger(CoresLocator.class);
+
+  private final File rootDirectory;
+
+  public CorePropertiesLocator(String coreDiscoveryRoot) {
+    this.rootDirectory = new File(coreDiscoveryRoot);
+  }
+
+  @Override
+  public void create(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    for (CoreDescriptor cd : coreDescriptors) {
+      File propFile = new File(new File(cd.getInstanceDir()), PROPERTIES_FILENAME);
+      if (propFile.exists())
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+                                "Could not create a new core in " + cd.getInstanceDir()
+                              + "as another core is already defined there");
+      try {
+        Properties p = buildCoreProperties(cd);
+        Writer writer = new OutputStreamWriter(new FileOutputStream(propFile), Charsets.UTF_8);
+        p.store(writer, "Written by CorePropertiesLocator on " + new Date());
+      }
+      catch (IOException e) {
+        logger.error("Couldn't persist core properties to {}: {}", propFile.getAbsolutePath(), e);
+      }
+    }
+  }
+
+  // TODO, this isn't atomic!  If we crash in the middle of a rename, we
+  // could end up with two cores with identical names, in which case one of
+  // them won't start up.  Are we happy with this?
+
+  @Override
+  public void persist(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    for (CoreDescriptor cd : coreDescriptors) {
+      File propFile = new File(new File(cd.getInstanceDir()), PROPERTIES_FILENAME);
+      try {
+        Properties p = buildCoreProperties(cd);
+        Writer writer = new OutputStreamWriter(new FileOutputStream(propFile), Charsets.UTF_8);
+        p.store(writer, "Written by CorePropertiesLocator on " + new Date());
+      }
+      catch (IOException e) {
+        logger.error("Couldn't persist core properties to {}: {}", propFile.getAbsolutePath(), e);
+      }
+    }
+  }
+
+  @Override
+  public void delete(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    for (CoreDescriptor cd : coreDescriptors) {
+      File instanceDir = new File(cd.getInstanceDir());
+      File propertiesFile = new File(instanceDir, PROPERTIES_FILENAME);
+      propertiesFile.renameTo(new File(instanceDir, PROPERTIES_FILENAME + ".unloaded"));
+      // This is a best-effort: the core.properties file may already have been
+      // deleted by the core unload, so we don't worry about checking if the
+      // rename has succeeded.
+    }
+  }
+
+  @Override
+  public void rename(CoreContainer cc, CoreDescriptor oldCD, CoreDescriptor newCD) {
+    persist(cc, newCD);
+  }
+
+  @Override
+  public List<CoreDescriptor> discover(CoreContainer cc) {
+    logger.info("Looking for core definitions underneath {}", rootDirectory.getAbsolutePath());
+    List<CoreDescriptor> cds = Lists.newArrayList();
+    discoverUnder(rootDirectory, cds, cc);
+    logger.info("Found {} core definitions", cds.size());
+    return cds;
+  }
+
+  private void discoverUnder(File root, List<CoreDescriptor> cds, CoreContainer cc) {
+    if (!root.exists())
+      return;
+    for (File child : root.listFiles()) {
+      File propertiesFile = new File(child, PROPERTIES_FILENAME);
+      if (propertiesFile.exists()) {
+        CoreDescriptor cd = buildCoreDescriptor(propertiesFile, cc);
+        logger.info("Found core {} in {}", cd.getName(), cd.getInstanceDir());
+        cds.add(cd);
+        continue;
+      }
+      if (child.isDirectory())
+        discoverUnder(child, cds, cc);
+    }
+  }
+
+  protected CoreDescriptor buildCoreDescriptor(File propertiesFile, CoreContainer cc) {
+    try {
+      File instanceDir = propertiesFile.getParentFile();
+      Properties coreProperties = new Properties();
+      coreProperties.load(new FileInputStream(propertiesFile));
+      String name = createName(coreProperties, instanceDir);
+      return new CoreDescriptor(cc, name, instanceDir.getAbsolutePath(), coreProperties);
+    } catch (IOException e) {
+      logger.error("Couldn't load core descriptor from {}:{}", propertiesFile.getAbsolutePath(), e.toString());
+      return null;
+    }
+  }
+
+  protected static String createName(Properties p, File instanceDir) {
+    return p.getProperty(CoreDescriptor.CORE_NAME, instanceDir.getName());
+  }
+
+  protected Properties buildCoreProperties(CoreDescriptor cd) {
+    Properties p = new Properties();
+    p.putAll(cd.getPersistableStandardProperties());
+    p.putAll(cd.getPersistableUserProperties());
+    // We don't persist the instance directory, as that's defined by the location
+    // of the properties file.
+    p.remove(CoreDescriptor.CORE_INSTDIR);
+    return p;
+  }
+
+}

solr/core/src/java/org/apache/solr/core/CoresLocator.java

@@ -0,0 +1,65 @@
+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 java.util.List;
+
+/**
+ * Manage the discovery and persistence of core definitions across Solr restarts
+ */
+public interface CoresLocator {
+
+  /**
+   * Make new cores available for discovery
+   * @param cc              the CoreContainer
+   * @param coreDescriptors CoreDescriptors to persist
+   */
+  public void create(CoreContainer cc, CoreDescriptor... coreDescriptors);
+
+  /**
+   * Ensure that the core definitions from the passed in CoreDescriptors
+   * will persist across container restarts.
+   * @param cc              the CoreContainer
+   * @param coreDescriptors CoreDescriptors to persist
+   */
+  public void persist(CoreContainer cc, CoreDescriptor... coreDescriptors);
+
+  /**
+   * Ensure that the core definitions from the passed in CoreDescriptors
+   * are not available for discovery
+   * @param cc              the CoreContainer
+   * @param coreDescriptors CoreDescriptors of the cores to remove
+   */
+  public void delete(CoreContainer cc, CoreDescriptor... coreDescriptors);
+
+  /**
+   * Persist the new name of a renamed core
+   * @param cc    the CoreContainer
+   * @param oldCD the CoreDescriptor of the core before renaming
+   * @param newCD the CoreDescriptor of the core after renaming
+   */
+  public void rename(CoreContainer cc, CoreDescriptor oldCD, CoreDescriptor newCD);
+
+  /**
+   * Load all the CoreDescriptors from persistence store
+   * @param cc the CoreContainer
+   * @return a list of all CoreDescriptors found
+   */
+  public List<CoreDescriptor> discover(CoreContainer cc);
+
+}

solr/core/src/java/org/apache/solr/core/SolrCore.java

@@ -17,42 +17,6 @@
 
 package org.apache.solr.core;
 
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.io.Writer;
-import java.lang.reflect.Constructor;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.IdentityHashMap;
-import java.util.LinkedHashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Properties;
-import java.util.Set;
-import java.util.StringTokenizer;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.ReentrantLock;
-
-import javax.xml.parsers.ParserConfigurationException;
-
 import org.apache.commons.io.IOUtils;
 import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.index.DirectoryReader;
@@ -65,7 +29,6 @@ import org.apache.lucene.store.IndexInput;
 import org.apache.lucene.store.LockObtainFailedException;
 import org.apache.solr.cloud.CloudDescriptor;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.CommonParams.EchoParamStyle;
@@ -129,6 +92,38 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
 
+import javax.xml.parsers.ParserConfigurationException;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Writer;
+import java.lang.reflect.Constructor;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.StringTokenizer;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReentrantLock;
+
 
 /**
  *
@@ -146,7 +141,7 @@ public final class SolrCore implements SolrInfoMBean {
 
   private String name;
   private String logid; // used to show what name is set
-  private final CoreDescriptor coreDescriptor;
+  private CoreDescriptor coreDescriptor;
 
   private boolean isReloaded = false;
 
@@ -306,6 +301,7 @@ public final class SolrCore implements SolrInfoMBean {
   public void setName(String v) {
     this.name = v;
     this.logid = (v==null)?"":("["+v+"] ");
+    this.coreDescriptor = new CoreDescriptor(v, this.coreDescriptor);
   }
 
   public String getLogId()
@@ -849,12 +845,6 @@ public final class SolrCore implements SolrInfoMBean {
     
     CoreContainer cc = cd.getCoreContainer();
 
-    if (cc != null) {
-      if (cc.cfg != null && cc.cfg instanceof ConfigSolrXml) {
-        writePropFile(cd, cc);
-      }
-    }
-
     if (cc != null && cc.isZooKeeperAware() && Slice.CONSTRUCTION.equals(cd.getCloudDescriptor().getShardState())) {
       // set update log to buffer before publishing the core
       getUpdateHandler().getUpdateLog().bufferUpdates();
@@ -868,56 +858,6 @@ public final class SolrCore implements SolrInfoMBean {
 //    openHandles.put(this, new RuntimeException("unclosed core - name:" + getName() + " refs: " + refCount.get()));
   }
     
-  private void writePropFile(CoreDescriptor cd, CoreContainer cc) {
-    File propFile = new File(cd.getInstanceDir(), "core.properties");
-    if (!propFile.exists()) {
-      propFile.getParentFile().mkdirs();
-      Properties props = new Properties();
-      props.put("name", cd.getName());
-
-      // This must be being created since there's no file here already. So write out all of the params we were
-      // created with. This _may_ overwrite the name above, but that's OK.
-      Collection<String> stds = new HashSet(Arrays.asList(CoreDescriptor.standardPropNames));
-      for (String prop : cd.getCreatedProperties().stringPropertyNames()) {
-        // Only preserve things that are legal, and let's just keep instDir right out of the persisted file even
-        // though it's part of the create properties on the URL.
-        if (! CoreDescriptor.CORE_INSTDIR.equals(prop) && stds.contains(prop)) {
-          props.put(prop, cd.getCreatedProperties().getProperty(prop));
-        }
-      }
-
-      if (cc.isZooKeeperAware()) {
-        String collection = cd.getCloudDescriptor().getCollectionName();
-        if (collection != null) {
-          props.put("collection", collection);
-        }
-        String coreNodeName = cd.getCloudDescriptor().getCoreNodeName();
-        if (coreNodeName != null) {
-          props.put("coreNodeName", coreNodeName);
-        }
-        String roles = cd.getCloudDescriptor().getRoles();
-        if (roles != null) {
-          props.put("roles", roles);
-        }
-        String shardId = cd.getCloudDescriptor().getShardId();
-        if (shardId != null) {
-          props.put("shard", shardId);
-        }
-      }
-      OutputStream out = null;
-      try {
-        out = new FileOutputStream(propFile);
-        props.store(out, "");
-      } catch (IOException e) {
-        throw new SolrException(ErrorCode.SERVER_ERROR, null, e);
-      } finally {
-        if (out != null) {
-          IOUtils.closeQuietly(out);
-        }
-      }
-    }
-  }
-    
   private Codec initCodec(SolrConfig solrConfig, final IndexSchema schema) {
     final PluginInfo info = solrConfig.getPluginInfo(CodecFactory.class.getName());
     final CodecFactory factory;

solr/core/src/java/org/apache/solr/core/SolrCoreDiscoverer.java

@@ -1,100 +0,0 @@
-package org.apache.solr.core;
-
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Properties;
-
-import org.apache.commons.io.IOUtils;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.util.PropertiesUtil;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/*
- * 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.
- */
-
-public class SolrCoreDiscoverer {
-  protected static Logger log = LoggerFactory.getLogger(SolrCoreDiscoverer.class);
-  
-  public final static String CORE_PROP_FILE = "core.properties";
-  
-  public Map<String, CoreDescriptor> discover(CoreContainer container, File root) throws IOException {
-    Map<String, CoreDescriptor> coreDescriptorMap = new HashMap<String, CoreDescriptor>();
-
-    walkFromHere(root, container, coreDescriptorMap);
-    
-    return coreDescriptorMap;
-  }
-  
-  // Basic recursive tree walking, looking for "core.properties" files. Once one is found, we'll stop going any
-  // deeper in the tree.
-  //
-  private void walkFromHere(File file, CoreContainer container, Map<String,CoreDescriptor> coreDescriptorMap)
-      throws IOException {
-    log.info("Looking for cores in " + file.getCanonicalPath());
-    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 stop recursing deep but continue looking wide.
-      File propFile = new File(childFile, CORE_PROP_FILE);
-      if (propFile.exists()) { // Stop looking after processing this file!
-        addCore(container, childFile, propFile, coreDescriptorMap);
-        continue; // Go on to the sibling directory, don't descend any deeper.
-      }
-      if (childFile.isDirectory()) {
-        walkFromHere(childFile, container, coreDescriptorMap);
-      }
-    }
-  }
-  
-  private void addCore(CoreContainer container, File childFile, File propFile, Map<String,CoreDescriptor> coreDescriptorMap) 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.getCanonicalPath());
-
-    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);
-    CoreDescriptor check = coreDescriptorMap.get(desc.getName());
-    if (check != null) {
-      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Core " + desc.getName() +
-          " defined more than once, once in " + desc.getInstanceDir() + " and once in " + check.getInstanceDir());
-    }
-    coreDescriptorMap.put(desc.getName(), desc);
-  }
-}

solr/core/src/java/org/apache/solr/core/SolrCores.java

@@ -17,32 +17,24 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
-import org.apache.commons.lang.StringUtils;
+import com.google.common.collect.Lists;
-import org.apache.solr.cloud.CloudDescriptor;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.CoreAdminParams;
+import org.slf4j.Logger;
-import org.apache.solr.core.SolrXMLSerializer.SolrCoreXMLDef;
+import org.slf4j.LoggerFactory;
-import org.apache.solr.util.DOMUtil;
-import org.w3c.dom.Node;
 
-import javax.xml.xpath.XPathExpressionException;
-import java.io.File;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Properties;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
 
 
 class SolrCores {
-  private static SolrXMLSerializer SOLR_XML_SERIALIZER = new SolrXMLSerializer();
+
   private static Object modifyLock = new Object(); // for locking around manipulating any of the core maps.
   private final Map<String, SolrCore> cores = new LinkedHashMap<String, SolrCore>(); // For "permanent" cores
 
@@ -57,6 +49,8 @@ class SolrCores {
 
   private final CoreContainer container;
 
+  private static final Logger logger = LoggerFactory.getLogger(SolrCores.class);
+
   // This map will hold objects that are being currently operated on. The core (value) may be null in the case of
   // initial load. The rule is, never to any operation on a core that is currently being operated upon.
   private static final Set<String> pendingCoreOps = new HashSet<String>();
@@ -80,7 +74,9 @@ class SolrCores {
         protected boolean removeEldestEntry(Map.Entry<String, SolrCore> eldest) {
           if (size() > transientCacheSize) {
             synchronized (modifyLock) {
-              pendingCloses.add(eldest.getValue()); // Essentially just queue this core up for closing.
+              SolrCore coreToClose = eldest.getValue();
+              logger.info("Closing transient core [{}]", coreToClose.getName());
+              pendingCloses.add(coreToClose); // Essentially just queue this core up for closing.
               modifyLock.notifyAll(); // Wakes up closer thread too
             }
             return true;
@@ -227,9 +223,7 @@ class SolrCores {
       cores.put(n1, c0);
 
       c0.setName(n1);
-      c0.getCoreDescriptor().putProperty(CoreDescriptor.CORE_NAME, n1);
       c1.setName(n0);
-      c1.getCoreDescriptor().putProperty(CoreDescriptor.CORE_NAME, n0);
     }
 
   }
@@ -314,7 +308,7 @@ class SolrCores {
       if (desc == null) {
         return null;
       }
-      return new CoreDescriptor(desc);
+      return new CoreDescriptor(cname, desc);
     }
 
   }
@@ -325,47 +319,6 @@ class SolrCores {
     }
   }
 
-  public void persistCores(Config cfg, Properties containerProperties,
-      Map<String,String> rootSolrAttribs, Map<String,String> coresAttribs,
-      Map<String, String> loggingAttribs, Map<String,String> watcherAttribs,
-      Node shardHandlerNode,
-      File file, SolrResourceLoader loader) throws XPathExpressionException {
-
-
-    List<SolrXMLSerializer.SolrCoreXMLDef> solrCoreXMLDefs = new ArrayList<SolrXMLSerializer.SolrCoreXMLDef>();
-    synchronized (modifyLock) {
-
-      persistCores(cfg, cores, loader, solrCoreXMLDefs);
-      persistCores(cfg, transientCores, loader, solrCoreXMLDefs);
-      // add back all the cores that aren't loaded, either in cores or transient
-      // cores
-      for (Map.Entry<String,CoreDescriptor> ent : dynamicDescriptors.entrySet()) {
-        if (!cores.containsKey(ent.getKey())
-            && !transientCores.containsKey(ent.getKey())) {
-          addCoreToPersistList(cfg, loader, ent.getValue(), null, solrCoreXMLDefs);
-        }
-      }
-      for (Map.Entry<String,SolrCore> ent : createdCores.entrySet()) {
-        if (!cores.containsKey(ent.getKey())
-            && !transientCores.containsKey(ent.getKey())
-            && !dynamicDescriptors.containsKey(ent.getKey())) {
-          addCoreToPersistList(cfg, loader, ent.getValue().getCoreDescriptor(),
-              null, solrCoreXMLDefs);
-        }
-      }
-
-      SolrXMLSerializer.SolrXMLDef solrXMLDef = new SolrXMLSerializer.SolrXMLDef();
-      solrXMLDef.coresDefs = solrCoreXMLDefs;
-      solrXMLDef.containerProperties = containerProperties;
-      solrXMLDef.solrAttribs = rootSolrAttribs;
-      solrXMLDef.coresAttribs = coresAttribs;
-      solrXMLDef.loggingAttribs = loggingAttribs;
-      solrXMLDef.watcherAttribs = watcherAttribs;
-      solrXMLDef.shardHandlerNode = shardHandlerNode;
-      SOLR_XML_SERIALIZER.persistFile(file, solrXMLDef);
-    }
-
-  }
   // Wait here until any pending operations (load, unload or reload) are completed on this core.
   protected SolrCore waitAddPendingCoreOps(String name) {
 
@@ -414,170 +367,6 @@ class SolrCores {
     }
   }
 
-
-  protected void persistCores(Config cfg, Map<String, SolrCore> whichCores, SolrResourceLoader loader, List<SolrCoreXMLDef> solrCoreXMLDefs) throws XPathExpressionException {
-    for (SolrCore solrCore : whichCores.values()) {
-      addCoreToPersistList(cfg, loader, solrCore.getCoreDescriptor(), getCoreToOrigName(solrCore), solrCoreXMLDefs);
-    }
-  }
-
-  private void addCoreProperty(Map<String,String> propMap, SolrResourceLoader loader, Node node, String name,
-                               String value) {
-    addCoreProperty(propMap, loader, node, name, value, null);
-  }
-
-  private void addCoreProperty(Map<String,String> propMap, SolrResourceLoader loader, Node node, String name,
-      String value, String defaultValue) {
-
-    if (node == null) {
-      propMap.put(name, value);
-      return;
-    }
-
-    if (node != null) {
-      String rawAttribValue = DOMUtil.getAttr(node, name, null);
-
-      if (rawAttribValue == null) {
-        return; // It was never in the original definition.
-      }
-
-      if (value == null) {
-        propMap.put(name, rawAttribValue);
-        return;
-      }
-
-      // There are some _really stupid_ additions/subtractions of the slash that we should look out for. I'm (EOE)
-      // ashamed of this but it fixes some things and we're throwing persistence away anyway (although
-      // maybe not for core.properties files).
-      String defComp = regularizeAttr(defaultValue);
-
-      if (defComp != null && regularizeAttr(value).equals(defComp)) {
-        return;
-      }
-      String rawComp = regularizeAttr(rawAttribValue);
-      if (rawComp != null && regularizeAttr(value).equals(
-          regularizeAttr(DOMUtil.substituteProperty(rawAttribValue, loader.getCoreProperties())))) {
-        propMap.put(name, rawAttribValue);
-      } else {
-        propMap.put(name, value);
-      }
-    }
-  }
-
-  protected String regularizeAttr(String path) {
-    if (path == null)
-      return null;
-    path = path.replace('/', File.separatorChar);
-    path = path.replace('\\', File.separatorChar);
-    if (path.endsWith(File.separator)) {
-      path = path.substring(0, path.length() - 1);
-    }
-    return path;
-  }
-  protected void addCoreToPersistList(Config cfg, SolrResourceLoader loader,
-      CoreDescriptor dcore, String origCoreName,
-      List<SolrCoreXMLDef> solrCoreXMLDefs) throws XPathExpressionException {
-
-    Map<String,String> coreAttribs = new HashMap<String,String>();
-    Properties newProps = new Properties();
-
-    // This is simple, just take anything sent in and saved away in at core creation and write it out.
-    if (dcore.getCreatedProperties().size() > 0) {
-      final List<String> stdNames = new ArrayList<String>(Arrays.asList(CoreDescriptor.standardPropNames));
-      coreAttribs.put(CoreDescriptor.CORE_NAME, dcore.getName()); // NOTE: may have been swapped or renamed!
-      for (String key : dcore.getCreatedProperties().stringPropertyNames()) {
-        if (! stdNames.contains(key) && ! key.startsWith(CoreAdminParams.PROPERTY_PREFIX)) continue;
-        if (key.indexOf(CoreAdminParams.PROPERTY_PREFIX) == 0) {
-          newProps.put(key.substring(CoreAdminParams.PROPERTY_PREFIX.length()), dcore.getCreatedProperties().getProperty(key));
-        } else if (! CoreDescriptor.CORE_NAME.equals(key)) {
-          coreAttribs.put(key, dcore.getCreatedProperties().getProperty(key));
-        }
-      }
-      // Insure instdir is persisted if it's the default since it's checked at startup even if not specified on the
-      // create command.
-      if (! dcore.getCreatedProperties().containsKey(CoreDescriptor.CORE_INSTDIR)) {
-        coreAttribs.put(CoreDescriptor.CORE_INSTDIR, dcore.getRawInstanceDir());
-      }
-    } else {
-
-      String coreName = dcore.getProperty(CoreDescriptor.CORE_NAME);
-
-      CloudDescriptor cd = dcore.getCloudDescriptor();
-      String collection = null;
-      if (cd != null) collection = cd.getCollectionName();
-
-      if (origCoreName == null) {
-        origCoreName = coreName;
-      }
-
-      Node node = null;
-      if (cfg != null) {
-        node = cfg.getNode("/solr/cores/core[@name='" + origCoreName + "']",
-            false);
-      }
-
-      coreAttribs.put(CoreDescriptor.CORE_NAME, coreName);
-      //coreAttribs.put(CoreDescriptor.CORE_INSTDIR, dcore.getRawInstanceDir());
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_INSTDIR, dcore.getRawInstanceDir(), null);
-
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_COLLECTION,
-          StringUtils.isNotBlank(collection) ? collection : dcore.getName());
-
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_DATADIR,
-          dcore.getDataDir());
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_ULOGDIR,
-          dcore.getUlogDir());
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_TRANSIENT,
-          Boolean.toString(dcore.isTransient()));
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_LOADONSTARTUP,
-          Boolean.toString(dcore.isLoadOnStartup()));
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_CONFIG,
-          dcore.getConfigName());
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_SCHEMA,
-          dcore.getSchemaName());
-
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_COLLECTION,
-          collection, dcore.getName());
-
-      String shard = null;
-      String roles = null;
-      String node_name = null;
-      if (cd != null) {
-        shard = cd.getShardId();
-        roles = cd.getRoles();
-        node_name = cd.getCoreNodeName();
-      }
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_SHARD,
-          shard);
-
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_ROLES,
-          roles);
-
-      addCoreProperty(coreAttribs, loader, node, CoreDescriptor.CORE_NODE_NAME,
-          node_name);
-
-
-      for (Object key : dcore.getCoreProperties().keySet()) {
-
-        if (cfg != null) {
-          Node propNode = cfg.getNode("/solr/cores/core[@name='" + origCoreName + "']/property[@name='" + key + "']",
-              false);
-
-          if (propNode != null) { // This means it was in the original DOM element, so just copy it.
-            newProps.put(DOMUtil.getAttr(propNode, "name", null), DOMUtil.getAttr(propNode, "value", null));
-          }
-        }
-      }
-    }
-
-
-
-    SolrXMLSerializer.SolrCoreXMLDef solrCoreXMLDef = new SolrXMLSerializer.SolrCoreXMLDef();
-    solrCoreXMLDef.coreAttribs = coreAttribs;
-    solrCoreXMLDef.coreProperties = newProps;
-    solrCoreXMLDefs.add(solrCoreXMLDef);
-  }
-
   protected Object getModifyLock() {
     return modifyLock;
   }
@@ -604,4 +393,37 @@ class SolrCores {
       createdCores.put(core.getName(), core);
     }
   }
+
+  /**
+   * Return the CoreDescriptor corresponding to a given core name.
+   * @param coreName the name of the core
+   * @return the CoreDescriptor
+   */
+  public CoreDescriptor getCoreDescriptor(String coreName) {
+    synchronized (modifyLock) {
+      if (cores.containsKey(coreName))
+        return cores.get(coreName).getCoreDescriptor();
+      if (dynamicDescriptors.containsKey(coreName))
+        return dynamicDescriptors.get(coreName);
+      return null;
+    }
+  }
+
+  /**
+   * Get the CoreDescriptors for every SolrCore managed here
+   * @return a List of CoreDescriptors
+   */
+  public List<CoreDescriptor> getCoreDescriptors() {
+    List<CoreDescriptor> cds = Lists.newArrayList();
+    synchronized (modifyLock) {
+      for (String coreName : getAllCoreNames()) {
+        // TODO: This null check is a bit suspicious - it seems that
+        // getAllCoreNames might return deleted cores as well?
+        CoreDescriptor cd = getCoreDescriptor(coreName);
+        if (cd != null)
+          cds.add(cd);
+      }
+    }
+    return cds;
+  }
 }

solr/core/src/java/org/apache/solr/core/SolrXMLCoresLocator.java

@@ -0,0 +1,221 @@
+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 com.google.common.base.Charsets;
+import com.google.common.collect.ImmutableList;
+import org.apache.commons.lang.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Writes any changes in core definitions to this instance's solr.xml
+ */
+public class SolrXMLCoresLocator implements CoresLocator {
+
+  private static final Logger logger = LoggerFactory.getLogger(SolrXMLCoresLocator.class);
+
+  private final File file;
+  private final String solrXmlTemplate;
+  private final ConfigSolrXmlOld cfg;
+
+  /** Core name to use if a core definition has no name */
+  public static final String DEFAULT_CORE_NAME = "collection1";
+
+  /**
+   * Create a new SolrXMLCoresLocator
+   * @param file          a File object representing the file to write out to
+   * @param originalXML   the original content of the solr.xml file
+   * @param cfg           the CoreContainer's config object
+   */
+  public SolrXMLCoresLocator(File file, String originalXML, ConfigSolrXmlOld cfg) {
+    this.solrXmlTemplate = buildTemplate(originalXML);
+    this.file = file;
+    this.cfg = cfg;
+  }
+
+  private static Pattern POPULATED_CORES_TAG
+      = Pattern.compile("^(.*<cores[^>]*>)(.*)(</cores>.*)$", Pattern.DOTALL);
+  private static Pattern EMPTY_CORES_TAG
+      = Pattern.compile("^(.*<cores[^>]*)/>(.*)$", Pattern.DOTALL);
+
+  private static Pattern SHARD_HANDLER_TAG
+      = Pattern.compile("(<shardHandlerFactory[^>]*>.*</shardHandlerFactory>)|(<shardHandlerFactory[^>]*/>)",
+                          Pattern.DOTALL);
+
+  private static String CORES_PLACEHOLDER = "{{CORES_PLACEHOLDER}}";
+
+  // Package-private for testing
+  // We replace the existing <cores></cores> contents with a template pattern
+  // that we can later replace with the up-to-date core definitions.  We also
+  // need to extract the <shardHandlerFactory> section, as, annoyingly, it's
+  // kept inside <cores/>.
+  static String buildTemplate(String originalXML) {
+
+    String shardHandlerConfig = "";
+    Matcher shfMatcher = SHARD_HANDLER_TAG.matcher(originalXML);
+    if (shfMatcher.find()) {
+      shardHandlerConfig = shfMatcher.group(0);
+    }
+
+    Matcher popMatcher = POPULATED_CORES_TAG.matcher(originalXML);
+    if (popMatcher.matches()) {
+      return new StringBuilder(popMatcher.group(1))
+          .append(CORES_PLACEHOLDER).append(shardHandlerConfig).append(popMatcher.group(3)).toString();
+    }
+
+    // Self-closing <cores/> tag gets expanded to <cores></cores>
+    Matcher emptyMatcher = EMPTY_CORES_TAG.matcher(originalXML);
+    if (emptyMatcher.matches())
+      return new StringBuilder(emptyMatcher.group(1))
+          .append(">").append(CORES_PLACEHOLDER).append("</cores>")
+          .append(emptyMatcher.group(2)).toString();
+
+    // If there's no <cores> tag at all, add one at the end of the file
+    return originalXML.replace("</solr>", "<cores>" + CORES_PLACEHOLDER + "</cores></solr>");
+  }
+
+  // protected access for testing
+  protected String buildSolrXML(List<CoreDescriptor> cds) {
+    StringBuilder builder = new StringBuilder();
+    for (CoreDescriptor cd : cds) {
+      builder.append(buildCoreTag(cd));
+    }
+    return solrXmlTemplate.replace(CORES_PLACEHOLDER, builder.toString());
+  }
+
+  public static final String NEWLINE = System.getProperty("line.separator");
+  public static final String INDENT = "    ";
+
+  /**
+   * Serialize a coredescriptor as a String containing an XML &lt;core> tag.
+   * @param cd the CoreDescriptor
+   * @return an XML representation of the CoreDescriptor
+   */
+  protected static String buildCoreTag(CoreDescriptor cd) {
+
+    StringBuilder builder = new StringBuilder(NEWLINE).append(INDENT).append("<core");
+    for (Map.Entry<Object, Object> entry : cd.getPersistableStandardProperties().entrySet()) {
+      builder.append(" ").append(entry.getKey()).append("=\"").append(entry.getValue()).append("\"");
+    }
+
+    Properties userProperties = cd.getPersistableUserProperties();
+    if (userProperties.isEmpty()) {
+      return builder.append("/>").append(NEWLINE).toString();
+    }
+
+    builder.append(">").append(NEWLINE);
+    for (Map.Entry<Object, Object> entry : userProperties.entrySet()) {
+      builder.append(INDENT).append(INDENT)
+          .append("<property name=\"").append(entry.getKey()).append("\" value=\"")
+          .append(entry.getValue()).append("\"/>").append(NEWLINE);
+    }
+
+    return builder.append("</core>").append(NEWLINE).toString();
+
+  }
+
+  @Override
+  public final void persist(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    doPersist(buildSolrXML(cc.getCoreDescriptors()));
+  }
+
+  protected void doPersist(String xml) {
+    try {
+      Writer writer = new OutputStreamWriter(new FileOutputStream(file), Charsets.UTF_8);
+      writer.write(xml);
+      writer.close();
+      logger.info("Persisted core descriptions to {}", file.getAbsolutePath());
+    }
+    catch (IOException e) {
+      logger.error("Couldn't persist core descriptions to {} : {}", file.getAbsolutePath(), e);
+    }
+  }
+
+  @Override
+  public void create(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    this.persist(cc, coreDescriptors);
+  }
+
+  @Override
+  public void delete(CoreContainer cc, CoreDescriptor... coreDescriptors) {
+    this.persist(cc, coreDescriptors);
+  }
+
+  @Override
+  public void rename(CoreContainer cc, CoreDescriptor oldCD, CoreDescriptor newCD) {
+    this.persist(cc, oldCD, newCD);
+  }
+
+  @Override
+  public List<CoreDescriptor> discover(CoreContainer cc) {
+
+    ImmutableList.Builder<CoreDescriptor> listBuilder = ImmutableList.builder();
+
+    for (String coreName : cfg.getAllCoreNames()) {
+
+      String name = cfg.getProperty(coreName, CoreDescriptor.CORE_NAME, DEFAULT_CORE_NAME);
+      String instanceDir = cfg.getProperty(coreName, CoreDescriptor.CORE_INSTDIR, "");
+
+      Properties coreProperties = new Properties();
+      for (String propName : CoreDescriptor.standardPropNames) {
+        String propValue = cfg.getProperty(coreName, propName, "");
+        if (StringUtils.isNotEmpty(propValue))
+          coreProperties.setProperty(propName, propValue);
+      }
+      coreProperties.putAll(cfg.getCoreProperties(coreName));
+
+      listBuilder.add(new CoreDescriptor(cc, name, instanceDir, coreProperties));
+    }
+
+    return listBuilder.build();
+  }
+
+  // for testing
+  String getTemplate() {
+    return solrXmlTemplate;
+  }
+
+  public static class NonPersistingLocator extends SolrXMLCoresLocator {
+
+    public NonPersistingLocator(File file, String originalXML, ConfigSolrXmlOld cfg) {
+      super(file, originalXML, cfg);
+      this.xml = originalXML;
+    }
+
+    @Override
+    public void doPersist(String xml) {
+      this.xml = xml;
+    }
+
+    public String xml;
+
+  }
+
+}

solr/core/src/java/org/apache/solr/core/ZkContainer.java

@@ -203,7 +203,7 @@ public class ZkContainer {
 
         
         if(boostrapConf) {
-          ZkController.bootstrapConf(zkController.getZkClient(), cc.cfg, solrHome);
+          ZkController.bootstrapConf(zkController.getZkClient(), cc, solrHome);
         }
         
       } catch (InterruptedException e) {
@@ -249,8 +249,7 @@ public class ZkContainer {
             "Could not find config name for collection:" + collection);
       }
       solrLoader = new ZkSolrResourceLoader(instanceDir, zkConfigName,
-          loader.getClassLoader(), ConfigSolrXml.getCoreProperties(instanceDir,
+          loader.getClassLoader(), dcore.getCoreProperties(), zkController);
-              dcore), zkController);
       config = getSolrConfigFromZk(zkConfigName, dcore.getConfigName(),
           solrLoader);
       schema = IndexSchemaFactory.buildIndexSchema(dcore.getSchemaName(),

solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java

@@ -17,6 +17,7 @@
 
 package org.apache.solr.handler.admin;
 
+import com.google.common.collect.ImmutableMap;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.lucene.index.DirectoryReader;
@@ -57,7 +58,6 @@ import org.apache.solr.update.UpdateLog;
 import org.apache.solr.update.processor.UpdateRequestProcessor;
 import org.apache.solr.update.processor.UpdateRequestProcessorChain;
 import org.apache.solr.util.NumberUtils;
-import org.apache.solr.util.PropertiesUtil;
 import org.apache.solr.util.RefCounted;
 import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
@@ -127,7 +127,7 @@ public class CoreAdminHandler extends RequestHandlerBase {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
               "Core container instance missing");
     }
-    boolean doPersist = false;
+    //boolean doPersist = false;
 
     // Pick the action
     SolrParams params = req.getParams();
@@ -136,54 +136,54 @@ public class CoreAdminHandler extends RequestHandlerBase {
     if (a != null) {
       action = CoreAdminAction.get(a);
       if (action == null) {
-        doPersist = this.handleCustomAction(req, rsp);
+        this.handleCustomAction(req, rsp);
       }
     }
     if (action != null) {
       switch (action) {
         case CREATE: {
-          doPersist = this.handleCreateAction(req, rsp);
+          this.handleCreateAction(req, rsp);
           break;
         }
 
         case RENAME: {
-          doPersist = this.handleRenameAction(req, rsp);
+          this.handleRenameAction(req, rsp);
           break;
         }
 
         case UNLOAD: {
-          doPersist = this.handleUnloadAction(req, rsp);
+          this.handleUnloadAction(req, rsp);
           break;
         }
 
         case STATUS: {
-          doPersist = this.handleStatusAction(req, rsp);
+          this.handleStatusAction(req, rsp);
           break;
 
         }
 
         case PERSIST: {
-          doPersist = this.handlePersistAction(req, rsp);
+          this.handlePersistAction(req, rsp);
           break;
         }
 
         case RELOAD: {
-          doPersist = this.handleReloadAction(req, rsp);
+          this.handleReloadAction(req, rsp);
           break;
         }
 
         case SWAP: {
-          doPersist = this.handleSwapAction(req, rsp);
+          this.handleSwapAction(req, rsp);
           break;
         }
 
         case MERGEINDEXES: {
-          doPersist = this.handleMergeAction(req, rsp);
+          this.handleMergeAction(req, rsp);
           break;
         }
 
         case SPLIT: {
-          doPersist = this.handleSplitAction(req, rsp);
+          this.handleSplitAction(req, rsp);
           break;
         }
 
@@ -209,28 +209,21 @@ public class CoreAdminHandler extends RequestHandlerBase {
         }
         
         default: {
-          doPersist = this.handleCustomAction(req, rsp);
+          this.handleCustomAction(req, rsp);
           break;
         }
         case LOAD:
           break;
       }
     }
-    // Should we persist the changes?
-    if (doPersist) {
-      cores.persist();
-      rsp.add("saved", cores.getConfigFile().getAbsolutePath());
-    }
     rsp.setHttpCaching(false);
   }
 
   
   /**
    * Handle the core admin SPLIT action.
-   * @return true if a modification has resulted that requires persistence 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleSplitAction(SolrQueryRequest adminReq, SolrQueryResponse rsp) throws IOException {
+  protected void handleSplitAction(SolrQueryRequest adminReq, SolrQueryResponse rsp) throws IOException {
     SolrParams params = adminReq.getParams();
     List<DocRouter.Range> ranges = null;
 
@@ -299,11 +292,10 @@ public class CoreAdminHandler extends RequestHandlerBase {
       }
     }
 
-    return false;
   }
 
 
-  protected boolean handleMergeAction(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {
+  protected void handleMergeAction(SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {
     SolrParams params = req.getParams();
     String cname = params.required().get(CoreAdminParams.CORE);
     SolrCore core = coreContainer.getCore(cname);
@@ -386,7 +378,6 @@ public class CoreAdminHandler extends RequestHandlerBase {
         core.close();
       }
     }
-    return coreContainer.isPersistent();
   }
 
   /**
@@ -395,140 +386,93 @@ public class CoreAdminHandler extends RequestHandlerBase {
    * This method could be overridden by derived classes to handle custom actions. <br> By default - this method throws a
    * solr exception. Derived classes are free to write their derivation if necessary.
    */
-  protected boolean handleCustomAction(SolrQueryRequest req, SolrQueryResponse rsp) {
+  protected void handleCustomAction(SolrQueryRequest req, SolrQueryResponse rsp) {
     throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unsupported operation: " +
             req.getParams().get(CoreAdminParams.ACTION));
   }
 
+  public static ImmutableMap<String, String> paramToProp = ImmutableMap.<String, String>builder()
+      .put(CoreAdminParams.CONFIG, CoreDescriptor.CORE_CONFIG)
+      .put(CoreAdminParams.SCHEMA, CoreDescriptor.CORE_SCHEMA)
+      .put(CoreAdminParams.DATA_DIR, CoreDescriptor.CORE_DATADIR)
+      .put(CoreAdminParams.ULOG_DIR, CoreDescriptor.CORE_ULOGDIR)
+      .put(CoreAdminParams.LOAD_ON_STARTUP, CoreDescriptor.CORE_LOADONSTARTUP)
+      .put(CoreAdminParams.TRANSIENT, CoreDescriptor.CORE_TRANSIENT)
+      .put(CoreAdminParams.SHARD, CoreDescriptor.CORE_SHARD)
+      .put(CoreAdminParams.COLLECTION, CoreDescriptor.CORE_COLLECTION)
+      .put(CoreAdminParams.ROLES, CoreDescriptor.CORE_ROLES)
+      .put(CoreAdminParams.CORE_NODE_NAME, CoreDescriptor.CORE_NODE_NAME)
+      .put(CoreAdminParams.SHARD_STATE, CloudDescriptor.SHARD_STATE)
+      .put(CoreAdminParams.SHARD_RANGE, CloudDescriptor.SHARD_RANGE)
+      .put(ZkStateReader.NUM_SHARDS_PROP, CloudDescriptor.NUM_SHARDS)
+      .build();
+
+  public static ImmutableMap<String, String> cloudParamToProp;
+
+  protected static CoreDescriptor buildCoreDescriptor(SolrParams params, CoreContainer container) {
+
+    String name = checkNotEmpty(params.get(CoreAdminParams.NAME),
+        "Missing parameter [" + CoreAdminParams.NAME + "]");
+    String instancedir = params.get(CoreAdminParams.INSTANCE_DIR);
+    if (StringUtils.isEmpty(instancedir))
+      instancedir = container.getSolrHome() + File.separator + name;
+
+    Properties coreProps = new Properties();
+    for (String param : paramToProp.keySet()) {
+      String value = params.get(param, null);
+      if (StringUtils.isNotEmpty(value)) {
+        coreProps.setProperty(paramToProp.get(param), value);
+      }
+    }
+    Iterator<String> paramsIt = params.getParameterNamesIterator();
+    while (paramsIt.hasNext()) {
+      String param = paramsIt.next();
+      if (!param.startsWith(CoreAdminParams.PROPERTY_PREFIX))
+        continue;
+      String propName = param.substring(CoreAdminParams.PROPERTY_PREFIX.length());
+      String propValue = params.get(param);
+      coreProps.setProperty(propName, propValue);
+    }
+
+    return new CoreDescriptor(container, name, instancedir, coreProps);
+  }
+
+  private static String checkNotEmpty(String value, String message) {
+    if (StringUtils.isEmpty(value))
+      throw new SolrException(ErrorCode.BAD_REQUEST, message);
+    return value;
+  }
+
   /**
    * Handle 'CREATE' action.
    *
-   * @return true if a modification has resulted that requires persistance 
-   *         of the CoreContainer configuration.
-   *
    * @throws SolrException in case of a configuration error.
    */
-  protected boolean handleCreateAction(SolrQueryRequest req, SolrQueryResponse rsp) throws SolrException {
+  protected void handleCreateAction(SolrQueryRequest req, SolrQueryResponse rsp) throws SolrException {
+
     SolrParams params = req.getParams();
-    String name = params.get(CoreAdminParams.NAME);
+    CoreDescriptor dcore = buildCoreDescriptor(params, coreContainer);
-    if (null == name || "".equals(name)) {
+
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+    if (coreContainer.getAllCoreNames().contains(dcore.getName())) {
-                              "Core name is mandatory to CREATE a SolrCore");
+      log.warn("Creating a core with existing name is not allowed");
+      throw new SolrException(ErrorCode.SERVER_ERROR,
+          "Core with name '" + dcore.getName() + "' already exists.");
     }
 
-    CoreDescriptor dcore = null;
+    // TODO this should be moved into CoreContainer, really...
     try {
-      
-      if (coreContainer.getAllCoreNames().contains(name)) {
-        log.warn("Creating a core with existing name is not allowed");
-        throw new SolrException(ErrorCode.SERVER_ERROR,
-            "Core with name '" + name + "' already exists.");
-      }
-
-      String instanceDir = params.get(CoreAdminParams.INSTANCE_DIR);
-      if (instanceDir == null) {
-        // instanceDir = coreContainer.getSolrHome() + "/" + name;
-        instanceDir = name; // bare name is already relative to solr home
-      } else {
-        instanceDir = PropertiesUtil.substituteProperty(instanceDir, null);
-      }
-
-      dcore = new CoreDescriptor(coreContainer, name, instanceDir);
-
-      //  fillup optional parameters
-      String opts = params.get(CoreAdminParams.CONFIG);
-      if (opts != null) {
-        opts = PropertiesUtil.substituteProperty(opts, null);
-        dcore.setConfigName(opts);
-      }
-
-      opts = params.get(CoreAdminParams.SCHEMA);
-      if (opts != null) {
-        opts = PropertiesUtil.substituteProperty(opts, null);
-        dcore.setSchemaName(opts);
-      }
-
-      opts = params.get(CoreAdminParams.DATA_DIR);
-      if (opts != null) {
-        opts = PropertiesUtil.substituteProperty(opts, null);
-        dcore.setDataDir(opts);
-      }
-
-      opts = params.get(CoreAdminParams.ULOG_DIR);
-      if (opts != null)
-        dcore.setUlogDir(opts);
-
-      opts = params.get(CoreAdminParams.LOAD_ON_STARTUP);
-      if (opts != null){
-        Boolean value = Boolean.valueOf(opts);
-        dcore.setLoadOnStartup(value);
-      }
-      
-      opts = params.get(CoreAdminParams.TRANSIENT);
-      if (opts != null){
-        Boolean value = Boolean.valueOf(opts);
-        dcore.setTransient(value);
-      }
-      
-      CloudDescriptor cd = dcore.getCloudDescriptor();
-      if (cd != null) {
-        cd.setParams(req.getParams());
-
-        opts = params.get(CoreAdminParams.COLLECTION);
-        if (opts != null)
-          cd.setCollectionName(opts);
-        
-        opts = params.get(CoreAdminParams.SHARD);
-        if (opts != null)
-          cd.setShardId(opts);
-        
-        opts = params.get(CoreAdminParams.SHARD_RANGE);
-        if (opts != null)
-          cd.setShardRange(opts);
-
-        opts = params.get(CoreAdminParams.SHARD_STATE);
-        if (opts != null)
-          cd.setShardState(opts);
-        
-        opts = params.get(CoreAdminParams.ROLES);
-        if (opts != null)
-          cd.setRoles(opts);
-        
-        opts = params.get(CoreAdminParams.CORE_NODE_NAME);
-        if (opts != null)
-          cd.setCoreNodeName(opts);
-                        
-        Integer numShards = params.getInt(ZkStateReader.NUM_SHARDS_PROP);
-        if (numShards != null)
-          cd.setNumShards(numShards);
-      }
-      
-      // Process all property.name=value parameters and set them as name=value core properties
-      Properties coreProperties = new Properties();
-      Iterator<String> parameterNamesIterator = params.getParameterNamesIterator();
-      while (parameterNamesIterator.hasNext()) {
-          String parameterName = parameterNamesIterator.next();
-          String parameterValue = params.get(parameterName);
-          dcore.addCreatedProperty(parameterName, parameterValue); // Need this junk for silly persistence
-
-          if(parameterName.startsWith(CoreAdminParams.PROPERTY_PREFIX)) {
-            String propertyName = parameterName.substring(CoreAdminParams.PROPERTY_PREFIX.length()); // skip prefix
-            coreProperties.put(propertyName, parameterValue);
-          }
-      }
-      dcore.setCoreProperties(coreProperties);
       if (coreContainer.getZkController() != null) {
         coreContainer.preRegisterInZk(dcore);
       }
+      coreContainer.getCoresLocator().create(coreContainer, dcore);
       SolrCore core = coreContainer.create(dcore);
-
+      coreContainer.register(dcore.getName(), core, false);
-      coreContainer.register(name, core, false);
       rsp.add("core", core.getName());
-      return coreContainer.isPersistent();
+    }
-    } catch (Exception ex) {
+    catch (Exception ex) {
       if (coreContainer.isZooKeeperAware() && dcore != null) {
         try {
-          coreContainer.getZkController().unregister(name, dcore);
+          coreContainer.getZkController().unregister(dcore.getName(), dcore);
         } catch (InterruptedException e) {
           Thread.currentThread().interrupt();
           SolrException.log(log, null, e);
@@ -552,46 +496,37 @@ public class CoreAdminHandler extends RequestHandlerBase {
       }
       
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-                              "Error CREATEing SolrCore '" + name + "': " +
+                              "Error CREATEing SolrCore '" + dcore.getName() + "': " +
-                              ex.getMessage() + rootMsg, ex);
+                              ex.getMessage(), ex);
     }
   }
 
   /**
    * Handle "RENAME" Action
-   *
-   * @return true if a modification has resulted that requires persistence 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleRenameAction(SolrQueryRequest req, SolrQueryResponse rsp) throws SolrException {
+  protected void handleRenameAction(SolrQueryRequest req, SolrQueryResponse rsp) throws SolrException {
     SolrParams params = req.getParams();
 
     String name = params.get(CoreAdminParams.OTHER);
     String cname = params.get(CoreAdminParams.CORE);
-    boolean doPersist = false;
 
-    if (cname.equals(name)) return doPersist;
+    if (cname.equals(name)) return;
 
-    doPersist = coreContainer.isPersistent();
     coreContainer.rename(cname, name);
 
-    return doPersist;
   }
 
   /**
    * Handle "ALIAS" action
-   *
-   * @return true if a modification has resulted that requires persistance 
-   *         of the CoreContainer configuration.
    */
   @Deprecated
-  protected boolean handleAliasAction(SolrQueryRequest req, SolrQueryResponse rsp) {
+  protected void handleAliasAction(SolrQueryRequest req, SolrQueryResponse rsp) {
     SolrParams params = req.getParams();
 
     String name = params.get(CoreAdminParams.OTHER);
     String cname = params.get(CoreAdminParams.CORE);
     boolean doPersist = false;
-    if (cname.equals(name)) return doPersist;
+    if (cname.equals(name)) return;
 
     SolrCore core = coreContainer.getCore(cname);
     if (core != null) {
@@ -599,17 +534,14 @@ public class CoreAdminHandler extends RequestHandlerBase {
       coreContainer.register(name, core, false);
       // no core.close() since each entry in the cores map should increase the ref
     }
-    return doPersist;
+    return;
   }
 
 
   /**
    * Handle "UNLOAD" Action
-   *
-   * @return true if a modification has resulted that requires persistance 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleUnloadAction(SolrQueryRequest req,
+  protected void handleUnloadAction(SolrQueryRequest req,
       SolrQueryResponse rsp) throws SolrException {
     SolrParams params = req.getParams();
     String cname = params.get(CoreAdminParams.CORE);
@@ -687,17 +619,13 @@ public class CoreAdminHandler extends RequestHandlerBase {
         core.close();
       }
     }
-    return coreContainer.isPersistent();
     
   }
 
   /**
    * Handle "STATUS" action
-   *
-   * @return true if a modification has resulted that requires persistance 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleStatusAction(SolrQueryRequest req, SolrQueryResponse rsp)
+  protected void handleStatusAction(SolrQueryRequest req, SolrQueryResponse rsp)
           throws SolrException {
     SolrParams params = req.getParams();
 
@@ -722,8 +650,6 @@ public class CoreAdminHandler extends RequestHandlerBase {
         status.add(cname, getCoreStatus(coreContainer, cname, isIndexInfoNeeded));
       }
       rsp.add("status", status);
-      doPersist = false; // no state change
-      return doPersist;
     } catch (Exception ex) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
               "Error handling 'status' action ", ex);
@@ -732,42 +658,20 @@ public class CoreAdminHandler extends RequestHandlerBase {
 
   /**
    * Handler "PERSIST" action
-   *
-   * @return true if a modification has resulted that requires persistence 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handlePersistAction(SolrQueryRequest req, SolrQueryResponse rsp)
+  protected void handlePersistAction(SolrQueryRequest req, SolrQueryResponse rsp)
           throws SolrException {
-    SolrParams params = req.getParams();
+    rsp.add("message", "The PERSIST action has been deprecated");
-    boolean doPersist = false;
-    String fileName = params.get(CoreAdminParams.FILE);
-    if (fileName != null) {
-      File file = new File(fileName);
-      if (!file.isAbsolute())
-        file = new File(coreContainer.getConfigFile().getParentFile(), fileName);
-      coreContainer.persistFile(file);
-      rsp.add("saved", file.getAbsolutePath());
-      doPersist = false;
-    } else if (!coreContainer.isPersistent()) {
-      throw new SolrException(SolrException.ErrorCode.FORBIDDEN, "Persistence is not enabled");
-    } else
-      doPersist = true;
-
-    return doPersist;
   }
 
   /**
    * Handler "RELOAD" action
-   *
-   * @return true if a modification has resulted that requires persistence 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleReloadAction(SolrQueryRequest req, SolrQueryResponse rsp) {
+  protected void handleReloadAction(SolrQueryRequest req, SolrQueryResponse rsp) {
     SolrParams params = req.getParams();
     String cname = params.get(CoreAdminParams.CORE);
     try {
       coreContainer.reload(cname);
-      return false; // no change on reload
     } catch (Exception ex) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error handling 'reload' action", ex);
     }
@@ -775,19 +679,14 @@ public class CoreAdminHandler extends RequestHandlerBase {
 
   /**
    * Handle "SWAP" action
-   *
-   * @return true if a modification has resulted that requires persistence 
-   *         of the CoreContainer configuration.
    */
-  protected boolean handleSwapAction(SolrQueryRequest req, SolrQueryResponse rsp) {
+  protected void handleSwapAction(SolrQueryRequest req, SolrQueryResponse rsp) {
     final SolrParams params = req.getParams();
     final SolrParams required = params.required();
 
     final String cname = params.get(CoreAdminParams.CORE);
-    boolean doPersist = params.getBool(CoreAdminParams.PERSISTENT, coreContainer.isPersistent());
     String other = required.get(CoreAdminParams.OTHER);
     coreContainer.swap(cname, other);
-    return doPersist;
 
   }
   

solr/core/src/java/org/apache/solr/util/PropertiesUtil.java

@@ -129,4 +129,24 @@ public class PropertiesUtil {
     }
   }
 
+  /**
+   * Parse the given String value as an integer.  If the string cannot
+   * be parsed, returns the default
+   * @param value    the value to parse
+   * @param defValue the default to return if the value cannot be parsed
+   * @return an integer version of the passed in value
+   */
+  public static Integer toInteger(String value, Integer defValue) {
+    try {
+      return Integer.parseInt(value);
+    }
+    catch (NumberFormatException e) {
+      return defValue;
+    }
+  }
+
+  public static boolean toBoolean(String value) {
+    return "true".equalsIgnoreCase(value) || "on".equalsIgnoreCase(value);
+  }
+
 }

solr/core/src/test/org/apache/solr/cloud/ClusterStateUpdateTest.java

@@ -150,14 +150,8 @@ public class ClusterStateUpdateTest extends SolrTestCaseJ4  {
         CreateMode.PERSISTENT, true);
     zkClient.close();
 
-    CoreDescriptor dcore = new CoreDescriptor(container1, "testcore",
+    CoreDescriptor dcore = buildCoreDescriptor(container1, "testcore", "testcore")
-        "testcore");
+                              .withDataDir(dataDir4.getAbsolutePath()).build();
-    
-    dcore.setDataDir(dataDir4.getAbsolutePath());
-    
-    CloudDescriptor cloudDesc = new CloudDescriptor();
-    cloudDesc.setCollectionName("testcore");
-    dcore.setCloudDescriptor(cloudDesc);
 
     if (container1.getZkController() != null) {
       container1.preRegisterInZk(dcore);

solr/core/src/test/org/apache/solr/core/TestCoreContainer.java

@@ -143,117 +143,6 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
 
   }
 
-  @Test
-  public void testPersist() throws Exception {
-    final File workDir = new File(TEMP_DIR, this.getClass().getName()
-        + "_persist");
-    if (workDir.exists()) {
-      FileUtils.deleteDirectory(workDir);
-    }
-    assertTrue("Failed to mkdirs workDir", workDir.mkdirs());
-    
-    final CoreContainer cores = h.getCoreContainer();
-
-    cores.setPersistent(true); // is this needed since we make explicit calls?
-
-    String instDir = null;
-    {
-      SolrCore template = null;
-      try {
-        template = cores.getCore("collection1");
-        instDir = template.getCoreDescriptor().getRawInstanceDir();
-      } finally {
-        if (null != template) template.close();
-      }
-    }
-    
-    final File instDirFile = new File(cores.getSolrHome(), instDir);
-    assertTrue("instDir doesn't exist: " + instDir, instDirFile.exists());
-    
-    // sanity check the basic persistence of the default init
-    
-    final File oneXml = new File(workDir, "1.solr.xml");
-    cores.persistFile(oneXml);
-
-    assertXmlFile(oneXml, "/solr[@persistent='true']",
-        "/solr/cores[@defaultCoreName='collection1' and not(@transientCacheSize)]",
-        "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir +
-        "' and @transient='false' and @loadOnStartup='true' ]", "1=count(/solr/cores/core)");
-
-    // create some new cores and sanity check the persistence
-    
-    final File dataXfile = new File(workDir, "dataX");
-    final String dataX = dataXfile.getAbsolutePath();
-    assertTrue("dataXfile mkdirs failed: " + dataX, dataXfile.mkdirs());
-    
-    final File instYfile = new File(workDir, "instY");
-    FileUtils.copyDirectory(instDirFile, instYfile);
-    
-    // :HACK: dataDir leaves off trailing "/", but instanceDir uses it
-    final String instY = instYfile.getAbsolutePath() + "/";
-    
-    final CoreDescriptor xd = new CoreDescriptor(cores, "X", instDir);
-    xd.setDataDir(dataX);
-    
-    final CoreDescriptor yd = new CoreDescriptor(cores, "Y", instY);
-    
-    SolrCore x = null;
-    SolrCore y = null;
-    try {
-      x = cores.create(xd);
-      y = cores.create(yd);
-      cores.register(x, false);
-      cores.register(y, false);
-      
-      assertEquals("cores not added?", 3, cores.getCoreNames().size());
-      
-      final File twoXml = new File(workDir, "2.solr.xml");
-
-      cores.persistFile(twoXml);
-
-      assertXmlFile(twoXml, "/solr[@persistent='true']",
-          "/solr/cores[@defaultCoreName='collection1']",
-          "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir
-              + "']", "/solr/cores/core[@name='X' and @instanceDir='" + instDir
-              + "' and @dataDir='" + dataX + "']",
-          "/solr/cores/core[@name='Y' and @instanceDir='" + instY + "']",
-          "3=count(/solr/cores/core)");
-
-      // Test for saving implicit properties, we should not do this.
-      assertXmlFile(twoXml, "/solr/cores/core[@name='X' and not(@solr.core.instanceDir) and not (@solr.core.configName)]");
-
-      // delete a core, check persistence again
-      assertNotNull("removing X returned null", cores.remove("X"));
-      
-      final File threeXml = new File(workDir, "3.solr.xml");
-      cores.persistFile(threeXml);
-      
-      assertXmlFile(threeXml, "/solr[@persistent='true']",
-          "/solr/cores[@defaultCoreName='collection1']",
-          "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir + "']",
-          "/solr/cores/core[@name='Y' and @instanceDir='" + instY + "']",
-          "2=count(/solr/cores/core)");
-      
-      // sanity check that persisting w/o changes has no changes
-      
-      final File fourXml = new File(workDir, "4.solr.xml");
-      cores.persistFile(fourXml);
-      
-      assertTrue("3 and 4 should be identical files",
-          FileUtils.contentEquals(threeXml, fourXml));
-      
-    } finally {
-      // y is closed by the container, but
-      // x has been removed from the container
-      if (x != null) {
-        try {
-          x.close();
-        } catch (Exception e) {
-          log.error("", e);
-        }
-      }
-    }
-  }
 
 
   @Test

solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java

@@ -91,7 +91,7 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
 
   private void addCoreWithProps(String name, Properties stockProps) throws Exception {
 
-    File propFile = new File(new File(solrHomeDirectory, name), SolrCoreDiscoverer.CORE_PROP_FILE);
+    File propFile = new File(new File(solrHomeDirectory, name), CorePropertiesLocator.PROPERTIES_FILENAME);
     File parent = propFile.getParentFile();
     assertTrue("Failed to mkdirs for " + parent.getAbsolutePath(), parent.mkdirs());
     addCoreWithProps(stockProps, propFile);
@@ -144,17 +144,17 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
 
       // Let's assert we did the right thing for implicit properties too.
       CoreDescriptor desc = core1.getCoreDescriptor();
-      assertEquals("core1", desc.getProperty("solr.core.name"));
+      assertEquals("core1", desc.getName());
-
-      // 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"));
+      assertNotNull(desc.getInstanceDir());
 
-      assertEquals("core1", desc.getProperty("solr.core.dataDir"));
+      // Prove we're ignoring this even though it's set in the properties file
-      assertEquals("solrconfig-minimal.xml", desc.getProperty("solr.core.configName"));
+      assertFalse("InstanceDir should be ignored", desc.getInstanceDir().contains("totallybogus"));
-      assertEquals("schema-tiny.xml", desc.getProperty("solr.core.schemaName"));
+
+      assertEquals("core1", desc.getDataDir());
+      assertEquals("solrconfig-minimal.xml", desc.getConfigName());
+      assertEquals("schema-tiny.xml", desc.getSchemaName());
 
       SolrCore core2 = cc.getCore("core2");
       SolrCore lazy1 = cc.getCore("lazy1");
@@ -180,10 +180,9 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
       cc = init();
       fail("Should have thrown exception in testDuplicateNames");
     } catch (SolrException se) {
-      Throwable cause = se.getCause();
+      String message = se.getMessage();
-      String message = cause.getMessage();
+      assertTrue("Wrong exception thrown on duplicate core names",
-      assertTrue("Should have seen an exception because two cores had the same name",
+          message.indexOf("Found multiple cores with the name [core1]") != -1);
-          message.indexOf("Core core1 defined more than once") != -1);
       assertTrue(File.separator + "core1 should have been mentioned in the message: " + message,
           message.indexOf(File.separator + "core1") != -1);
       assertTrue(File.separator + "core2 should have been mentioned in the message:" + message,
@@ -203,9 +202,9 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
     alt.mkdirs();
     setMeUp(alt.getAbsolutePath());
     addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=core1"),
-        new File(alt, "core1" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+        new File(alt, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
     addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"),
-        new File(alt, "core2" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+        new File(alt, "core2" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
     CoreContainer cc = init();
     try {
       SolrCore core1 = cc.getCore("core1");
@@ -226,9 +225,9 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
     noCoreDir.mkdirs();
     setMeUp(noCoreDir.getAbsolutePath());
     addCoreWithProps(makeCorePropFile("core1", false, true),
-        new File(noCoreDir, "core1" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+        new File(noCoreDir, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
     addCoreWithProps(makeCorePropFile("core2", false, false),
-        new File(noCoreDir, "core2" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+        new File(noCoreDir, "core2" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
     CoreContainer cc = init();
     try {
       SolrCore core1 = cc.getCore("core1");

solr/core/src/test/org/apache/solr/core/TestLazyCores.java

@@ -32,6 +32,7 @@ import org.apache.solr.update.AddUpdateCommand;
 import org.apache.solr.update.CommitUpdateCommand;
 import org.apache.solr.update.UpdateHandler;
 import org.apache.solr.util.RefCounted;
+import org.apache.solr.util.TestHarness;
 import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -52,7 +53,6 @@ public class TestLazyCores extends SolrTestCaseJ4 {
 
   private final File solrHomeDirectory = new File(TEMP_DIR, "org.apache.solr.core.TestLazyCores_testlazy");
 
-
   private CoreContainer init() throws Exception {
 
     if (solrHomeDirectory.exists()) {
@@ -63,13 +63,17 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       copyMinConf(new File(solrHomeDirectory, "collection" + idx));
     }
 
+    SolrResourceLoader loader = new SolrResourceLoader(solrHomeDirectory.getAbsolutePath());
+
     File solrXml = new File(solrHomeDirectory, "solr.xml");
     FileUtils.write(solrXml, LOTS_SOLR_XML, IOUtils.CHARSET_UTF_8.toString());
-    final CoreContainer cores = new CoreContainer(solrHomeDirectory.getAbsolutePath());
+    ConfigSolrXmlOld config = (ConfigSolrXmlOld) ConfigSolr.fromFile(loader, solrXml);
-    cores.load();
-    //  h.getCoreContainer().load(solrHomeDirectory.getAbsolutePath(), new File(solrHomeDirectory, "solr.xml"));
 
-    cores.setPersistent(false);
+    CoresLocator locator = new SolrXMLCoresLocator.NonPersistingLocator(solrXml, LOTS_SOLR_XML, config);
+
+
+    final CoreContainer cores = new CoreContainer(loader, config, locator);
+    cores.load();
     return cores;
   }
 
@@ -91,15 +95,15 @@ public class TestLazyCores extends SolrTestCaseJ4 {
 
       SolrCore core1 = cc.getCore("collection1");
       assertFalse("core1 should not be transient", core1.getCoreDescriptor().isTransient());
-      assertTrue("core1 should  be loadable", core1.getCoreDescriptor().isLoadOnStartup());
+      assertTrue("core1 should be loadable", core1.getCoreDescriptor().isLoadOnStartup());
       assertNotNull(core1.getSolrConfig());
 
       SolrCore core2 = cc.getCore("collectionLazy2");
-      assertTrue("core2 should not be transient", core2.getCoreDescriptor().isTransient());
+      assertTrue("core2 should be transient", core2.getCoreDescriptor().isTransient());
       assertTrue("core2 should be loadable", core2.getCoreDescriptor().isLoadOnStartup());
 
       SolrCore core3 = cc.getCore("collectionLazy3");
-      assertTrue("core3 should not be transient", core3.getCoreDescriptor().isTransient());
+      assertTrue("core3 should be transient", core3.getCoreDescriptor().isTransient());
       assertFalse("core3 should not be loadable", core3.getCoreDescriptor().isLoadOnStartup());
 
       SolrCore core4 = cc.getCore("collectionLazy4");
@@ -108,7 +112,7 @@ public class TestLazyCores extends SolrTestCaseJ4 {
 
       SolrCore core5 = cc.getCore("collectionLazy5");
       assertFalse("core5 should not be transient", core5.getCoreDescriptor().isTransient());
-      assertTrue("core5 should  be loadable", core5.getCoreDescriptor().isLoadOnStartup());
+      assertTrue("core5 should be loadable", core5.getCoreDescriptor().isLoadOnStartup());
 
       core1.close();
       core2.close();
@@ -291,11 +295,11 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       admin.handleRequestBody(request, resp);
       fail("Should have thrown an error");
     } catch (SolrException se) {
-      SolrException cause = (SolrException)se.getCause();
+      //SolrException cause = (SolrException)se.getCause();
-      assertEquals("Exception code should be 500", 500, cause.code());
+      assertEquals("Exception code should be 500", 500, se.code());
       for (String err : errs) {
        assertTrue("Should have seen an exception containing the an error",
-            cause.getMessage().contains(err));
+            se.getMessage().contains(err));
       }
     }
   }
@@ -343,39 +347,25 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       copyMinConf(new File(solrHomeDirectory, "core3"));
       copyMinConf(new File(solrHomeDirectory, "core4"));
 
-      cc.setPersistent(true);
+      final CoreDescriptor cd1 = buildCoreDescriptor(cc, "core1", "./core1")
-      CoreDescriptor d1 = new CoreDescriptor(cc, "core1", "./core1");
+          .isTransient(true).loadOnStartup(true).build();
-      d1.setTransient(true);
+      final CoreDescriptor cd2 = buildCoreDescriptor(cc, "core2", "./core2")
-      d1.setLoadOnStartup(true);
+          .isTransient(true).loadOnStartup(false).build();
-      d1.setSchemaName("schema.xml");
+      final CoreDescriptor cd3 = buildCoreDescriptor(cc, "core3", "./core3")
-      d1.setConfigName("solrconfig.xml");
+          .isTransient(false).loadOnStartup(true).build();
-      SolrCore core1 = cc.create(d1);
+      final CoreDescriptor cd4 = buildCoreDescriptor(cc, "core4", "./core4")
+          .isTransient(false).loadOnStartup(false).build();
 
-      CoreDescriptor d2 = new CoreDescriptor(cc, "core2", "./core2");
-      d2.setTransient(true);
-      d2.setLoadOnStartup(false);
-      d2.setSchemaName("schema.xml");
-      d2.setConfigName("solrconfig.xml");
-      SolrCore core2 = cc.create(d2);
 
-      CoreDescriptor d3 = new CoreDescriptor(cc, "core3", "./core3");
+      SolrCore core1 = cc.create(cd1);
-      d3.setTransient(false);
+      SolrCore core2 = cc.create(cd2);
-      d3.setLoadOnStartup(true);
+      SolrCore core3 = cc.create(cd3);
-      d3.setSchemaName("schema.xml");
+      SolrCore core4 = cc.create(cd4);
-      d3.setConfigName("solrconfig.xml");
-      SolrCore core3 = cc.create(d3);
 
-      CoreDescriptor d4 = new CoreDescriptor(cc, "core4", "./core4");
+      SolrXMLCoresLocator.NonPersistingLocator locator =
-      d4.setTransient(false);
+          (SolrXMLCoresLocator.NonPersistingLocator) cc.getCoresLocator();
-      d4.setLoadOnStartup(false);
-      d4.setSchemaName("schema.xml");
-      d4.setConfigName("solrconfig.xml");
-      SolrCore core4 = cc.create(d4);
 
-      final File oneXml = new File(solrHomeDirectory, "lazy1.solr.xml");
+      TestHarness.validateXPath(locator.xml,
-      cc.persistFile(oneXml);
-
-      assertXmlFile(oneXml,
           "/solr/cores/core[@name='collection1']",
           "/solr/cores/core[@name='collectionLazy2']",
           "/solr/cores/core[@name='collectionLazy3']",
@@ -388,8 +378,8 @@ public class TestLazyCores extends SolrTestCaseJ4 {
           "/solr/cores/core[@name='core1']",
           "/solr/cores/core[@name='core2']",
           "/solr/cores/core[@name='core3']",
-          "/solr/cores/core[@name='core4']");
+          "/solr/cores/core[@name='core4']",
-      assertXmlFile(oneXml, "13=count(/solr/cores/core)");
+          "13=count(/solr/cores/core)");
 
       removeOne(cc, "collectionLazy2");
       removeOne(cc, "collectionLazy3");
@@ -403,11 +393,8 @@ public class TestLazyCores extends SolrTestCaseJ4 {
       removeOne(cc, "core4");
 
       // now test that unloading a core means the core is not persisted
+      TestHarness.validateXPath(locator.xml, "3=count(/solr/cores/core)");
 
-      final File twoXml = new File(solrHomeDirectory, "lazy2.solr.xml");
-      cc.persistFile(twoXml);
-
-      assertXmlFile(twoXml, "3=count(/solr/cores/core)");
     } finally {
       cc.shutdown();
     }

solr/core/src/test/org/apache/solr/core/TestSolrXml.java

@@ -17,37 +17,34 @@ package org.apache.solr.core;
  * limitations under the License.
  */
 
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
 import org.apache.commons.io.FileUtils;
 import org.apache.solr.SolrTestCaseJ4;
+import org.junit.Rule;
 import org.junit.Test;
-import org.xml.sax.InputSource;
+import org.junit.rules.RuleChain;
-import org.xml.sax.SAXException;
+import org.junit.rules.TestRule;
-
-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 {
+
+  @Rule
+  public TestRule solrTestRules = RuleChain.outerRule(new SystemPropertiesRestoreRule());
+
   private final File solrHome = new File(TEMP_DIR, TestSolrXml.getClassName() + File.separator + "solrHome");
 
   @Test
-  public void testAllInfoPresent() throws IOException, ParserConfigurationException, SAXException {
+  public void testAllInfoPresent() throws IOException {
-    CoreContainer cc = null;
+
     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 {
-      InputStream is = new FileInputStream(new File(solrHome, "solr.xml"));
+      loader = new SolrResourceLoader(solrHome.getAbsolutePath());
-      Config config = new Config(new SolrResourceLoader("solr/collection1"), null, new InputSource(is), null, false);
+      ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
-      boolean oldStyle = (config.getNode("solr/cores", false) != null);
-      ConfigSolr cfg;
-      if (oldStyle) {
-        cfg = new ConfigSolrXmlOld(config);
-      } else {
-        cfg = new ConfigSolrXml(config, cc);
-      }
 
       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);
@@ -71,59 +68,35 @@ public class TestSolrXml extends SolrTestCaseJ4 {
       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();
     }
+    finally {
+      loader.close();
+    }
+
   }
 
   // Test  a few property substitutions that happen to be in solr-50-all.xml.
-  public void testPropretySub() throws IOException, ParserConfigurationException, SAXException {
+  public void testPropertySub() throws IOException {
 
-    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"));
+
+    SolrResourceLoader loader = null;
     try {
-      InputStream is = new FileInputStream(new File(solrHome, "solr.xml"));
+      loader = new SolrResourceLoader(solrHome.getAbsolutePath());
-      Config config = new Config(new SolrResourceLoader("solr/collection1"), null, new InputSource(is), null, false);
+      ConfigSolr cfg = ConfigSolr.fromSolrHome(loader, solrHome.getAbsolutePath());
-      boolean oldStyle = (config.getNode("solr/cores", false) != null);
-      ConfigSolr cfg;
-      if (oldStyle) {
-        cfg = new ConfigSolrXmlOld(config);
-      } else {
-        cfg = new ConfigSolrXml(config, cc);
-      }
       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.get(ConfigSolr.CfgProp.SOLR_SHARESCHEMA, null), "newShareSchema");
-
+    }
-    } finally {
+    finally {
-      if (cc != null) cc.shutdown();
+      loader.close();
-      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");
-
     }
   }
 }

solr/core/src/test/org/apache/solr/core/TestSolrXmlPersistence.java

@@ -18,6 +18,7 @@
 package org.apache.solr.core;
 
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import com.google.common.base.Charsets;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.lucene.util.IOUtils;
@@ -25,6 +26,7 @@ import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.handler.admin.CoreAdminHandler;
 import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.TestHarness;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
@@ -38,8 +40,11 @@ import org.xml.sax.SAXException;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
+import java.io.ByteArrayInputStream;
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -47,13 +52,6 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
 
   private File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName());
 
-  /*
-  @BeforeClass
-  public static void beforeClass() throws Exception {
-    initCore("solrconfig-minimal.xml", "schema-tiny.xml");
-  }
-  */
-
   @Rule
   public TestRule solrTestRules =
       RuleChain.outerRule(new SystemPropertiesRestoreRule());
@@ -89,18 +87,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
 
     CoreContainer cc = init(SOLR_XML_LOTS_SYSVARS, "SystemVars1", "SystemVars2");
     try {
-
+      origMatchesPersist(cc, SOLR_XML_LOTS_SYSVARS);
-      // This seems odd, but it's just a little self check to see if the comparison strings are being created correctly
-      persistContainedInOrig(cc, new File(solrHomeDirectory, "solr_copy.xml"));
-
-      // Is everything in the persisted file identical to the original?
-      final File persistXml = new File(solrHomeDirectory, "sysvars.solr.xml");
-      // Side effect here is that the new file is persisted and available later.
-      persistContainedInOrig(cc, persistXml);
-
-      // Is everything in the original contained in the persisted one?
-      assertXmlFile(persistXml, getAllNodes(new File(solrHomeDirectory, "solr.xml")));
-
     } finally {
       cc.shutdown();
       if (solrHomeDirectory.exists()) {
@@ -130,8 +117,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
               resp);
       assertNull("Exception on reload", resp.getException());
 
-      persistContainedInOrig(cc, new File(solrHomeDirectory, "reload1.solr.xml"));
+      origMatchesPersist(cc, SOLR_XML_LOTS_SYSVARS);
-
     } finally {
       cc.shutdown();
       if (solrHomeDirectory.exists()) {
@@ -149,6 +135,9 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
 
   private void doTestRename(String which) throws Exception {
     CoreContainer cc = init(SOLR_XML_LOTS_SYSVARS, "SystemVars1", "SystemVars2");
+    SolrXMLCoresLocator.NonPersistingLocator locator
+        = (SolrXMLCoresLocator.NonPersistingLocator) cc.getCoresLocator();
+
     try {
       final CoreAdminHandler admin = new CoreAdminHandler(cc);
       SolrQueryResponse resp = new SolrQueryResponse();
@@ -160,31 +149,29 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
               resp);
       assertNull("Exception on rename", resp.getException());
 
-      File persistXml = new File(solrHomeDirectory, "rename.solr.xml");
-      File origXml = new File(solrHomeDirectory, "solr.xml");
-
       // OK, Assure that if I change everything that has been renamed with the original value for the core, it matches
       // the old list
-      cc.persistFile(persistXml);
+      String[] persistList = getAllNodes();
-      String[] persistList = getAllNodes(persistXml);
       String[] expressions = new String[persistList.length];
 
       for (int idx = 0; idx < persistList.length; ++idx) {
         expressions[idx] = persistList[idx].replaceAll("RenamedCore", which);
       }
 
-      assertXmlFile(origXml, expressions);
+      //assertXmlFile(origXml, expressions);
+      TestHarness.validateXPath(SOLR_XML_LOTS_SYSVARS, expressions);
 
       // Now the other way, If I replace the original name in the original XML file with "RenamedCore", does it match
       // what was persisted?
-      persistList = getAllNodes(origXml);
+      persistList = getAllNodes(SOLR_XML_LOTS_SYSVARS);
       expressions = new String[persistList.length];
       for (int idx = 0; idx < persistList.length; ++idx) {
         // /solr/cores/core[@name='SystemVars1' and @collection='${collection:collection1}']
         expressions[idx] = persistList[idx].replace("@name='" + which + "'", "@name='RenamedCore'");
       }
 
-      assertXmlFile(persistXml, expressions);
+      TestHarness.validateXPath(locator.xml, expressions);
+
     } finally {
       cc.shutdown();
       if (solrHomeDirectory.exists()) {
@@ -212,11 +199,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
               resp);
       assertNull("Exception on swap", resp.getException());
 
-      File persistXml = new File(solrHomeDirectory, "rename.solr.xml");
+      String[] persistList = getAllNodes();
-      File origXml = new File(solrHomeDirectory, "solr.xml");
-
-      cc.persistFile(persistXml);
-      String[] persistList = getAllNodes(persistXml);
       String[] expressions = new String[persistList.length];
 
       // Now manually change the names back and it should match exactly to the original XML.
@@ -230,7 +213,8 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
         }
       }
 
-      assertXmlFile(origXml, expressions);
+      //assertXmlFile(origXml, expressions);
+      TestHarness.validateXPath(SOLR_XML_LOTS_SYSVARS, expressions);
 
     } finally {
       cc.shutdown();
@@ -244,8 +228,8 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
   public void testMinimalXml() throws Exception {
     CoreContainer cc = init(SOLR_XML_MINIMAL, "SystemVars1");
     try {
-      persistContainedInOrig(cc, new File(solrHomeDirectory, "minimal.solr.xml"));
+      cc.shutdown();
-      origContainedInPersist(cc, new File(solrHomeDirectory, "minimal.solr.xml"));
+      origMatchesPersist(cc, SOLR_XML_MINIMAL);
     } finally {
       cc.shutdown();
       if (solrHomeDirectory.exists()) {
@@ -254,7 +238,13 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
     }
   }
 
+  private void origMatchesPersist(CoreContainer cc, String originalSolrXML) throws Exception  {
+    String[] expressions = getAllNodes(originalSolrXML);
+    SolrXMLCoresLocator.NonPersistingLocator locator
+        = (SolrXMLCoresLocator.NonPersistingLocator) cc.getCoresLocator();
 
+    TestHarness.validateXPath(locator.xml, expressions);
+  }
 
   @Test
   public void testUnloadCreate() throws Exception {
@@ -275,7 +265,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
               resp);
       assertNull("Exception on unload", resp.getException());
 
-      persistContainedInOrig(cc, new File(solrHomeDirectory, "unloadcreate1.solr.xml"));
+      //origMatchesPersist(cc, new File(solrHomeDirectory, "unloadcreate1.solr.xml"));
 
       String instPath = new File(solrHomeDirectory, which).getAbsolutePath();
       admin.handleRequestBody
@@ -286,11 +276,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
               resp);
       assertNull("Exception on create", resp.getException());
 
-      File persistXml = new File(solrHomeDirectory, "rename.solr.xml");
+      String[] persistList = getAllNodes();
-      File origXml = new File(solrHomeDirectory, "solr.xml");
-
-      cc.persistFile(persistXml);
-      String[] persistList = getAllNodes(persistXml);
       String[] expressions = new String[persistList.length];
 
       // Now manually change the names back and it should match exactly to the original XML.
@@ -312,8 +298,8 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
         }
       }
 
-      assertXmlFile(origXml, expressions);
+      //assertXmlFile(origXml, expressions);
-
+      TestHarness.validateXPath(SOLR_XML_LOTS_SYSVARS, expressions);
 
     } finally {
       cc.shutdown();
@@ -323,134 +309,17 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
     }
   }
 
-  private void persistContainedInOrig(CoreContainer cc, File persistXml) throws IOException,
-      SAXException, ParserConfigurationException {
-    cc.persistFile(persistXml);
-    // Is everything that's in the original file persisted?
-    String[] expressions = getAllNodes(persistXml);
-    assertXmlFile(new File(solrHomeDirectory, "solr.xml"), expressions);
-  }
-
-  private void origContainedInPersist(CoreContainer cc, File persistXml) throws IOException,
-      SAXException, ParserConfigurationException {
-    cc.persistFile(persistXml);
-    // Is everything that's in the original file persisted?
-    String[] expressions = getAllNodes(new File(solrHomeDirectory, "solr.xml"));
-    assertXmlFile(persistXml, expressions);
-  }
-
-
-  @Test
-  public void testCreateAndManipulateCores() throws Exception {
-    CoreContainer cc = init(SOLR_XML_LOTS_SYSVARS, "SystemVars1", "SystemVars2", "new_one", "new_two");
-    try {
-      final CoreAdminHandler admin = new CoreAdminHandler(cc);
-      String instPathOne = new File(solrHomeDirectory, "new_one").getAbsolutePath();
-      SolrQueryResponse resp = new SolrQueryResponse();
-      admin.handleRequestBody
-          (req(CoreAdminParams.ACTION,
-              CoreAdminParams.CoreAdminAction.CREATE.toString(),
-              CoreAdminParams.INSTANCE_DIR, instPathOne,
-              CoreAdminParams.NAME, "new_one"),
-              resp);
-      assertNull("Exception on create", resp.getException());
-
-      admin.handleRequestBody
-          (req(CoreAdminParams.ACTION,
-              CoreAdminParams.CoreAdminAction.CREATE.toString(),
-              CoreAdminParams.NAME, "new_two"),
-              resp);
-      assertNull("Exception on create", resp.getException());
-
-      File persistXml1 = new File(solrHomeDirectory, "create_man_1.xml");
-      origContainedInPersist(cc, persistXml1);
-
-      // We know all the original data is in persist, now check for newly-created files.
-      String[] expressions = new  String[2];
-      String instHome = new File(solrHomeDirectory, "new_one").getAbsolutePath();
-      expressions[0] = "/solr/cores/core[@name='new_one' and @instanceDir='" + instHome + "']";
-      expressions[1] = "/solr/cores/core[@name='new_two' and @instanceDir='new_two" + File.separator + "']";
-
-      assertXmlFile(persistXml1, expressions);
-
-      // Next, swap a created core and check
-      resp = new SolrQueryResponse();
-      admin.handleRequestBody
-          (req(CoreAdminParams.ACTION,
-              CoreAdminParams.CoreAdminAction.SWAP.toString(),
-              CoreAdminParams.CORE, "new_one",
-              CoreAdminParams.OTHER, "SystemVars2"),
-              resp);
-      assertNull("Exception on swap", resp.getException());
-
-      File persistXml2 = new File(solrHomeDirectory, "create_man_2.xml");
-
-      cc.persistFile(persistXml2);
-      String[] persistList = getAllNodes(persistXml2);
-      expressions = new String[persistList.length];
-
-      // Now manually change the names back and it should match exactly to the original XML.
-      for (int idx = 0; idx < persistList.length; ++idx) {
-        String fromName = "@name='new_one'";
-        String toName = "@name='SystemVars2'";
-        if (persistList[idx].contains(fromName)) {
-          expressions[idx] = persistList[idx].replace(fromName, toName);
-        } else {
-          expressions[idx] = persistList[idx].replace(toName, fromName);
-        }
-      }
-
-      assertXmlFile(persistXml1, expressions);
-
-      // Then rename the other created core and check
-      admin.handleRequestBody
-          (req(CoreAdminParams.ACTION,
-              CoreAdminParams.CoreAdminAction.RENAME.toString(),
-              CoreAdminParams.CORE, "new_two",
-              CoreAdminParams.OTHER, "RenamedCore"),
-              resp);
-      assertNull("Exception on rename", resp.getException());
-
-      File persistXml3 = new File(solrHomeDirectory, "create_man_3.xml");
-
-      // OK, Assure that if I change everything that has been renamed with the original value for the core, it matches
-      // the old list
-      cc.persistFile(persistXml3);
-      persistList = getAllNodes(persistXml3);
-      expressions = new String[persistList.length];
-
-      for (int idx = 0; idx < persistList.length; ++idx) {
-        expressions[idx] = persistList[idx].replaceAll("RenamedCore", "new_two");
-      }
-      assertXmlFile(persistXml2, expressions);
-
-      // Now the other way, If I replace the original name in the original XML file with "RenamedCore", does it match
-      // what was persisted?
-      persistList = getAllNodes(persistXml2);
-      expressions = new String[persistList.length];
-      for (int idx = 0; idx < persistList.length; ++idx) {
-        // /solr/cores/core[@name='SystemVars1' and @collection='${collection:collection1}']
-        expressions[idx] = persistList[idx].replace("@name='new_two'", "@name='RenamedCore'");
-      }
-      assertXmlFile(persistXml3, expressions);
-
-    } finally {
-      cc.shutdown();
-      if (solrHomeDirectory.exists()) {
-        FileUtils.deleteDirectory(solrHomeDirectory);
-      }
-    }
-
-
-  }
   @Test
   public void testCreatePersistCore() throws Exception {
     // Template for creating a core.
     CoreContainer cc = init(SOLR_XML_LOTS_SYSVARS, "SystemVars1", "SystemVars2", "props1", "props2");
+    SolrXMLCoresLocator.NonPersistingLocator locator
+        = (SolrXMLCoresLocator.NonPersistingLocator) cc.getCoresLocator();
+
     try {
       final CoreAdminHandler admin = new CoreAdminHandler(cc);
       // create a new core (using CoreAdminHandler) w/ properties
-      String instPath1 = new File(solrHomeDirectory, "props1").getAbsolutePath();
+
       SolrQueryResponse resp = new SolrQueryResponse();
       admin.handleRequestBody
           (req(CoreAdminParams.ACTION,
@@ -480,14 +349,13 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
       assertNull("Exception on create", resp.getException());
 
       // Everything that was in the original XML file should be in the persisted one.
-      final File persistXml = new File(solrHomeDirectory, "persist_create_core.solr.xml");
+      TestHarness.validateXPath(locator.xml, getAllNodes(SOLR_XML_LOTS_SYSVARS));
-      cc.persistFile(persistXml);
-      assertXmlFile(persistXml, getAllNodes(new File(solrHomeDirectory, "solr.xml")));
 
       // And the params for the new core should be in the persisted file.
-      assertXmlFile
+      TestHarness.validateXPath
-          (persistXml
+          (
-              , "/solr/cores/core[@name='props1']/property[@name='prefix1' and @value='valuep1']"
+              locator.xml,
+              "/solr/cores/core[@name='props1']/property[@name='prefix1' and @value='valuep1']"
               , "/solr/cores/core[@name='props1']/property[@name='prefix2' and @value='valueP2']"
               , "/solr/cores/core[@name='props1' and @transient='true']"
               , "/solr/cores/core[@name='props1' and @loadOnStartup='true']"
@@ -511,18 +379,124 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
     }
   }
 
-  private String[] getAllNodes(File xmlFile) throws ParserConfigurationException, IOException, SAXException {
+  @Test
+  public void testPersist() throws Exception {
+
+    final CoreContainer cores = init(ConfigSolrXmlOld.DEF_SOLR_XML, "collection1");
+    SolrXMLCoresLocator.NonPersistingLocator locator
+        = (SolrXMLCoresLocator.NonPersistingLocator) cores.getCoresLocator();
+
+    String instDir = null;
+    {
+      SolrCore template = null;
+      try {
+        template = cores.getCore("collection1");
+        instDir = template.getCoreDescriptor().getRawInstanceDir();
+      } finally {
+        if (null != template) template.close();
+      }
+    }
+
+    final File instDirFile = new File(cores.getSolrHome(), instDir);
+    assertTrue("instDir doesn't exist: " + instDir, instDirFile.exists());
+
+    // sanity check the basic persistence of the default init
+    TestHarness.validateXPath(locator.xml,
+        "/solr[@persistent='true']",
+        "/solr/cores[@defaultCoreName='collection1' and not(@transientCacheSize)]",
+        "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir +
+            "' and @transient='false' and @loadOnStartup='true' ]",
+        "1=count(/solr/cores/core)");
+
+    // create some new cores and sanity check the persistence
+
+    final File dataXfile = new File(solrHomeDirectory, "dataX");
+    final String dataX = dataXfile.getAbsolutePath();
+    assertTrue("dataXfile mkdirs failed: " + dataX, dataXfile.mkdirs());
+
+    final File instYfile = new File(solrHomeDirectory, "instY");
+    FileUtils.copyDirectory(instDirFile, instYfile);
+
+    // :HACK: dataDir leaves off trailing "/", but instanceDir uses it
+    final String instY = instYfile.getAbsolutePath() + "/";
+
+    final CoreDescriptor xd = buildCoreDescriptor(cores, "X", instDir)
+        .withDataDir(dataX).build();
+
+    final CoreDescriptor yd = new CoreDescriptor(cores, "Y", instY);
+
+    SolrCore x = null;
+    SolrCore y = null;
+    try {
+      x = cores.create(xd);
+      y = cores.create(yd);
+      cores.register(x, false);
+      cores.register(y, false);
+
+      assertEquals("cores not added?", 3, cores.getCoreNames().size());
+
+      TestHarness.validateXPath(locator.xml,
+          "/solr[@persistent='true']",
+          "/solr/cores[@defaultCoreName='collection1']",
+          "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir
+              + "']", "/solr/cores/core[@name='X' and @instanceDir='" + instDir
+              + "' and @dataDir='" + dataX + "']",
+          "/solr/cores/core[@name='Y' and @instanceDir='" + instY + "']",
+          "3=count(/solr/cores/core)");
+
+      // Test for saving implicit properties, we should not do this.
+      TestHarness.validateXPath(locator.xml,
+          "/solr/cores/core[@name='X' and not(@solr.core.instanceDir) and not (@solr.core.configName)]");
+
+      // delete a core, check persistence again
+      assertNotNull("removing X returned null", cores.remove("X"));
+
+      TestHarness.validateXPath(locator.xml, "/solr[@persistent='true']",
+          "/solr/cores[@defaultCoreName='collection1']",
+          "/solr/cores/core[@name='collection1' and @instanceDir='" + instDir + "']",
+          "/solr/cores/core[@name='Y' and @instanceDir='" + instY + "']",
+          "2=count(/solr/cores/core)");
+
+    } finally {
+      // y is closed by the container, but
+      // x has been removed from the container
+      if (x != null) {
+        try {
+          x.close();
+        } catch (Exception e) {
+          log.error("", e);
+        }
+      }
+      cores.shutdown();
+    }
+  }
+
+
+  private String[] getAllNodes(InputStream is) throws ParserConfigurationException, IOException, SAXException {
     List<String> expressions = new ArrayList<String>(); // XPATH and value for all elements in the indicated XML
     DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory
         .newInstance();
     DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder();
-    Document document = docBuilder.parse(xmlFile);
+    Document document = docBuilder.parse(is);
 
     Node root = document.getDocumentElement();
     gatherNodes(root, expressions, "");
     return expressions.toArray(new String[expressions.size()]);
   }
 
+  private String[] getAllNodes() throws ParserConfigurationException, IOException, SAXException {
+    return getAllNodes(new FileInputStream(new File(solrHomeDirectory, "solr.xml")));
+  }
+
+  private String[] getAllNodes(String xmlString) throws ParserConfigurationException, IOException, SAXException {
+    return getAllNodes(new ByteArrayInputStream(xmlString.getBytes(Charsets.UTF_8)));
+  }
+
+  /*
+  private void assertSolrXmlFile(String... xpathExpressions) throws IOException, SAXException {
+    assertXmlFile(new File(solrHomeDirectory, "solr.xml"), xpathExpressions);
+  }
+  */
 
   // Note this is pretty specialized for a solr.xml file because working with the DOM is such a pain.
 
@@ -595,7 +569,7 @@ public class TestSolrXmlPersistence extends SolrTestCaseJ4 {
     }
   }
 
-  private static String SOLR_XML_LOTS_SYSVARS =
+  public static String SOLR_XML_LOTS_SYSVARS =
       "<solr persistent=\"${solr.xml.persist:false}\" coreLoadThreads=\"12\" sharedLib=\"${something:.}\" >\n" +
           "  <logging class=\"${logclass:log4j.class}\" enabled=\"{logenable:true}\">\n" +
           "     <watcher size=\"{watchSize:13}\" threshold=\"${logThresh:54}\" />\n" +

solr/core/src/test/org/apache/solr/core/TestSolrXmlPersistor.java

@@ -0,0 +1,122 @@
+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 com.google.common.collect.ImmutableList;
+import org.junit.Test;
+
+import java.io.File;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestSolrXmlPersistor {
+
+  private static final List<CoreDescriptor> EMPTY_CD_LIST = ImmutableList.<CoreDescriptor>builder().build();
+
+  @Test
+  public void selfClosingCoresTagIsPersisted() {
+
+    final String solrxml = "<solr><cores adminHandler=\"/admin\"/></solr>";
+
+    SolrXMLCoresLocator persistor = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertEquals(persistor.buildSolrXML(EMPTY_CD_LIST),
+        "<solr><cores adminHandler=\"/admin\"></cores></solr>");
+
+  }
+
+  @Test
+  public void emptyCoresTagIsPersisted() {
+    final String solrxml = "<solr><cores adminHandler=\"/admin\"></cores></solr>";
+
+    SolrXMLCoresLocator persistor = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertEquals(persistor.buildSolrXML(EMPTY_CD_LIST), "<solr><cores adminHandler=\"/admin\"></cores></solr>");
+  }
+
+  @Test
+  public void emptySolrXmlIsPersisted() {
+    final String solrxml = "<solr></solr>";
+
+    SolrXMLCoresLocator persistor = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertEquals(persistor.buildSolrXML(EMPTY_CD_LIST), "<solr><cores></cores></solr>");
+  }
+
+  @Test
+  public void simpleCoreDescriptorIsPersisted() {
+
+    final String solrxml = "<solr><cores></cores></solr>";
+
+    SolrResourceLoader loader = new SolrResourceLoader("solr/example/solr");
+    CoreContainer cc = new CoreContainer(loader);
+
+    final CoreDescriptor cd = new CoreDescriptor(cc, "testcore", "instance/dir/");
+    List<CoreDescriptor> cds = ImmutableList.of(cd);
+
+    SolrXMLCoresLocator persistor = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertEquals(persistor.buildSolrXML(cds),
+          "<solr><cores>\n"
+        + "    <core name=\"testcore\" instanceDir=\"instance/dir/\"/>\n"
+        + "</cores></solr>");
+  }
+
+  @Test
+  public void shardHandlerInfoIsPersisted() {
+
+    final String solrxml =
+        "<solr>" +
+          "<cores adminHandler=\"whatever\">" +
+            "<core name=\"testcore\" instanceDir=\"instance/dir/\"/>" +
+            "<shardHandlerFactory name=\"shardHandlerFactory\" class=\"HttpShardHandlerFactory\">" +
+              "<int name=\"socketTimeout\">${socketTimeout:500}</int>" +
+              "<str name=\"arbitrary\">arbitraryValue</str>" +
+            "</shardHandlerFactory>" +
+          "</cores>" +
+        "</solr>";
+
+    SolrXMLCoresLocator locator = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertTrue(locator.getTemplate().contains("{{CORES_PLACEHOLDER}}"));
+    assertTrue(locator.getTemplate().contains("<shardHandlerFactory "));
+    assertTrue(locator.getTemplate().contains("${socketTimeout:500}"));
+
+  }
+
+  @Test
+  public void simpleShardHandlerInfoIsPersisted() {
+
+    final String solrxml =
+        "<solr>" +
+          "<cores adminHandler=\"whatever\">" +
+            "<core name=\"testcore\" instanceDir=\"instance/dir/\"/>" +
+            "<shardHandlerFactory name=\"shardHandlerFactory\" class=\"HttpShardHandlerFactory\"/>" +
+          "</cores>" +
+        "</solr>";
+
+    SolrXMLCoresLocator locator = new SolrXMLCoresLocator(new File("testfile.xml"), solrxml, null);
+    assertTrue(locator.getTemplate().contains("{{CORES_PLACEHOLDER}}"));
+    assertTrue(locator.getTemplate().contains("<shardHandlerFactory "));
+  }
+
+  @Test
+  public void complexXmlIsParsed() {
+    SolrXMLCoresLocator locator = new SolrXMLCoresLocator(new File("testfile.xml"),
+                                        TestSolrXmlPersistence.SOLR_XML_LOTS_SYSVARS, null);
+    assertTrue(locator.getTemplate().contains("{{CORES_PLACEHOLDER}}"));
+  }
+
+}

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminCreateDiscoverTest.java

@@ -19,9 +19,9 @@ package org.apache.solr.handler.admin;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CoreAdminParams;
-import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.CorePropertiesLocator;
-import org.apache.solr.core.SolrCoreDiscoverer;
 import org.apache.solr.response.SolrQueryResponse;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -30,9 +30,6 @@ import org.junit.Test;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.HashSet;
 import java.util.Properties;
 
 public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
@@ -43,6 +40,7 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
 
   private static String coreNormal = "normal";
   private static String coreSysProps = "sys_props";
+  private static String coreDuplicate = "duplicate";
 
   @BeforeClass
   public static void beforeClass() throws Exception {
@@ -111,7 +109,7 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
     // verify props are in persisted file
 
     Properties props = new Properties();
-    File propFile = new File(solrHomeDirectory, coreSysProps + "/" + SolrCoreDiscoverer.CORE_PROP_FILE);
+    File propFile = new File(solrHomeDirectory, coreSysProps + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
     FileInputStream is = new FileInputStream(propFile);
     try {
       props.load(is);
@@ -131,7 +129,8 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
     assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
         props.getProperty(CoreAdminParams.DATA_DIR), "${DATA_TEST}");
 
-    checkOnlyKnown(propFile);
+    assertEquals(props.size(), 4);
+    //checkOnlyKnown(propFile);
 
     // Now assert that certain values are properly dereferenced in the process of creating the core, see
     // SOLR-4982. Really, we should be able to just verify that the index files exist.
@@ -150,6 +149,47 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
 
   }
 
+  @Test
+  public void testCannotCreateTwoCoresWithSameInstanceDir() throws Exception {
+
+    setupCore(coreDuplicate, true);
+
+    File workDir = new File(solrHomeDirectory, coreDuplicate);
+    File data = new File(workDir, "data");
+
+    // Create one core
+    SolrQueryResponse resp = new SolrQueryResponse();
+    admin.handleRequestBody
+        (req(CoreAdminParams.ACTION,
+            CoreAdminParams.CoreAdminAction.CREATE.toString(),
+            CoreAdminParams.NAME, coreDuplicate,
+            CoreAdminParams.INSTANCE_DIR, workDir.getAbsolutePath(),
+            CoreAdminParams.CONFIG, "solrconfig_ren.xml",
+            CoreAdminParams.SCHEMA, "schema_ren.xml",
+            CoreAdminParams.DATA_DIR, data.getAbsolutePath()),
+            resp);
+    assertNull("Exception on create", resp.getException());
+
+    // Try to create another core with a different name, but the same instance dir
+    SolrQueryResponse resp2 = new SolrQueryResponse();
+    try {
+      admin.handleRequestBody
+          (req(CoreAdminParams.ACTION,
+              CoreAdminParams.CoreAdminAction.CREATE.toString(),
+              CoreAdminParams.NAME, "different_name_core",
+              CoreAdminParams.INSTANCE_DIR, workDir.getAbsolutePath(),
+              CoreAdminParams.CONFIG, "solrconfig_ren.xml",
+              CoreAdminParams.SCHEMA, "schema_ren.xml",
+              CoreAdminParams.DATA_DIR, data.getAbsolutePath()),
+              resp2);
+      fail("Creating two cores with a shared instance dir should throw an exception");
+    }
+    catch (SolrException e) {
+      assertTrue(e.getMessage().contains("already defined there"));
+    }
+
+  }
+
   @Test
   public void testCreateSavesRegProps() throws Exception {
 
@@ -174,7 +214,7 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
 
     // verify props are in persisted file
     Properties props = new Properties();
-    File propFile = new File(solrHomeDirectory, coreNormal + "/" + SolrCoreDiscoverer.CORE_PROP_FILE);
+    File propFile = new File(solrHomeDirectory, coreNormal + "/" + CorePropertiesLocator.PROPERTIES_FILENAME);
     FileInputStream is = new FileInputStream(propFile);
     try {
       props.load(is);
@@ -194,7 +234,9 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
     assertEquals("Unexpected value preserved in properties file " + propFile.getAbsolutePath(),
         props.getProperty(CoreAdminParams.DATA_DIR), data.getAbsolutePath());
 
-    checkOnlyKnown(propFile);
+    assertEquals(props.size(), 4);
+
+    //checkOnlyKnown(propFile);
     // For the other 3 vars, we couldn't get past creating the core if dereferencing didn't work correctly.
 
     // Should have segments in the directory pointed to by the ${DATA_TEST}.
@@ -205,23 +247,4 @@ public class CoreAdminCreateDiscoverTest extends SolrTestCaseJ4 {
 
   }
 
-  // Insure that all the props we've preserved are ones that _should_ be in the properties file
-  private void checkOnlyKnown(File propFile) throws IOException {
-
-    Properties props = new Properties();
-    FileInputStream is = new FileInputStream(propFile);
-    try {
-      props.load(is);
-    } finally {
-      org.apache.commons.io.IOUtils.closeQuietly(is);
-    }
-
-    // Should never be preserving instanceDir in a core.properties file.
-    assertFalse("Should not be preserving instanceDir!", props.containsKey(CoreAdminParams.INSTANCE_DIR));
-
-    Collection<String> stds = new HashSet(Arrays.asList(CoreDescriptor.standardPropNames));
-    for (String key : props.stringPropertyNames()) {
-      assertTrue("Property '" + key + "' should NOT be preserved in the properties file", stds.contains(key));
-    }
-  }
 }

solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java

@@ -18,25 +18,25 @@
 package org.apache.solr.handler.admin;
 
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
-import org.apache.solr.core.CoreContainer;
+import org.apache.commons.io.FileUtils;
-import org.apache.solr.core.SolrCore;
+import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.util.NamedList;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.core.SolrXMLCoresLocator;
 import org.apache.solr.response.SolrQueryResponse;
-import org.apache.solr.SolrTestCaseJ4;
-
-import java.util.Map;
-import java.io.File;
-
-import org.apache.commons.io.FileUtils;
-
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
+import java.io.File;
+import java.util.Map;
+
 public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
   
   @BeforeClass
@@ -72,7 +72,8 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
         new File(subHome, "solrconfig.snippet.randomindexconfig.xml"));
 
     final CoreContainer cores = h.getCoreContainer();
-    cores.setPersistent(false); // we'll do this explicitly as needed
+    SolrXMLCoresLocator.NonPersistingLocator locator
+        = (SolrXMLCoresLocator.NonPersistingLocator) cores.getCoresLocator();
 
     final CoreAdminHandler admin = new CoreAdminHandler(cores);
 
@@ -96,14 +97,9 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
             resp);
     assertNull("Exception on create", resp.getException());
 
-    // verify props are in persisted file
-
-    final File xml = new File(workDir, "persist-solr.xml");
-    cores.persistFile(xml);
-
     // First assert that these values are persisted.
-    assertXmlFile
+    h.validateXPath
-        (xml
+        (locator.xml
             ,"/solr/cores/core[@name='" + getCoreName() + "' and @instanceDir='${INSTDIR_TEST}']"
             ,"/solr/cores/core[@name='" + getCoreName() + "' and @dataDir='${DATA_TEST}']"
             ,"/solr/cores/core[@name='" + getCoreName() + "' and @schema='${SCHEMA_TEST}']"
@@ -140,7 +136,6 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
     assertTrue("Failed to mkdirs workDir", workDir.mkdirs());
     
     final CoreContainer cores = h.getCoreContainer();
-    cores.setPersistent(false); // we'll do this explicitly as needed
 
     final CoreAdminHandler admin = new CoreAdminHandler(cores);
 
@@ -173,16 +168,10 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
        resp);
     assertNull("Exception on create", resp.getException());
 
-    // verify props are in persisted file
+    CoreDescriptor cd = cores.getCoreDescriptor("props");
-
+    assertNotNull("Core not added!", cd);
-    final File xml = new File(workDir, "persist-solr.xml");
+    assertEquals(cd.getCoreProperty("hoss", null), "man");
-    cores.persistFile(xml);
+    assertEquals(cd.getCoreProperty("foo", null), "baz");
-    
-    assertXmlFile
-      (xml
-       ,"/solr/cores/core[@name='props']/property[@name='hoss' and @value='man']"
-       ,"/solr/cores/core[@name='props']/property[@name='foo' and @value='baz']"
-       );
 
     // attempt to create a bogus core and confirm failure
     ignoreException("Could not load config");

solr/core/src/test/org/apache/solr/update/SolrIndexSplitterTest.java

@@ -133,21 +133,18 @@ public class SolrIndexSplitterTest extends SolrTestCaseJ4 {
 
     SolrCore core1 = null, core2 = null;
     try {
-      CoreDescriptor dcore1 = new CoreDescriptor(h.getCoreContainer(), "split1", h.getCore().getCoreDescriptor().getInstanceDir());
+      String instanceDir = h.getCore().getCoreDescriptor().getInstanceDir();
-      dcore1.setDataDir(indexDir1.getAbsolutePath());
-      dcore1.setSchemaName("schema12.xml");
 
+      CoreDescriptor dcore1 = buildCoreDescriptor(h.getCoreContainer(), "split1", instanceDir)
+          .withDataDir(indexDir1.getAbsolutePath()).withSchema("schema12.xml").build();
       if (h.getCoreContainer().getZkController() != null) {
         h.getCoreContainer().preRegisterInZk(dcore1);
       }
-      
       core1 = h.getCoreContainer().create(dcore1);
       h.getCoreContainer().register(core1, false);
 
-      CoreDescriptor dcore2 = new CoreDescriptor(h.getCoreContainer(), "split2", h.getCore().getCoreDescriptor().getInstanceDir());
+      CoreDescriptor dcore2 = buildCoreDescriptor(h.getCoreContainer(), "split2", instanceDir)
-      dcore2.setDataDir(indexDir2.getAbsolutePath());
+          .withDataDir(indexDir2.getAbsolutePath()).withSchema("schema12.xml").build();
-      dcore2.setSchemaName("schema12.xml");
-      
       if (h.getCoreContainer().getZkController() != null) {
         h.getCoreContainer().preRegisterInZk(dcore2);
       }

solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java

@@ -17,22 +17,22 @@
 
 package org.apache.solr.client.solrj.request;
 
-import java.io.IOException;
-import java.util.Collection;
-import java.util.List;
-import java.util.Arrays;
-
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServer;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.response.CoreAdminResponse;
 import org.apache.solr.common.cloud.ZkStateReader;
-import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.CoreAdminParams;
-import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ContentStream;
 
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+
 /**
  * This class is experimental and subject to change.
  *
@@ -534,6 +534,7 @@ public class CoreAdminRequest extends SolrRequest
     return req.process( server );
   }
 
+  @Deprecated
   public static CoreAdminResponse persist(String fileName, SolrServer server) throws SolrServerException, IOException 
   {
     CoreAdminRequest.Persist req = new CoreAdminRequest.Persist();

solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java

@@ -23,7 +23,7 @@ import java.util.Locale;
 /**
  * @since solr 1.3
  */
-public interface CoreAdminParams 
+public abstract class CoreAdminParams
 {
   /** What Core are we talking about **/
   public final static String CORE = "core";

solr/solrj/src/test-files/solrj/solr/shared/solr.xml

@@ -22,7 +22,7 @@
   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="true">
+<solr persistent="false">
   <property name="version" value="1.3"/>
   <property name="lang" value="english, french"/>
 

solr/solrj/src/test/org/apache/solr/client/solrj/embedded/AbstractEmbeddedSolrServerTestCase.java

@@ -63,7 +63,7 @@ public abstract class AbstractEmbeddedSolrServerTestCase extends LuceneTestCase
     System.setProperty("tempDir", tempDir.getAbsolutePath());
     System.setProperty("tests.shardhandler.randomSeed", Long.toString(random().nextLong()));
     cores = CoreContainer.createAndLoad(SOLR_HOME.getAbsolutePath(), getSolrXml());
-    cores.setPersistent(false);
+    //cores.setPersistent(false);
   }
   
   protected abstract File getSolrXml() throws Exception;

solr/solrj/src/test/org/apache/solr/client/solrj/embedded/TestSolrProperties.java

@@ -18,8 +18,6 @@
 package org.apache.solr.client.solrj.embedded;
 
 import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
-import com.google.common.io.ByteStreams;
-import org.apache.commons.io.IOUtils;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServer;
@@ -29,30 +27,16 @@ import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.response.CoreAdminResponse;
 import org.apache.solr.common.SolrInputDocument;
-import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrXMLCoresLocator;
-import org.apache.solr.util.FileUtils;
+import org.apache.solr.util.TestHarness;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.w3c.dom.Document;
-import org.w3c.dom.Node;
 
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.xpath.XPath;
-import javax.xml.xpath.XPathConstants;
-import javax.xml.xpath.XPathExpressionException;
-import javax.xml.xpath.XPathFactory;
 import java.io.File;
-import java.io.FileInputStream;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
 
 /**
  *
@@ -62,56 +46,14 @@ public class TestSolrProperties extends AbstractEmbeddedSolrServerTestCase {
   protected static Logger log = LoggerFactory.getLogger(TestSolrProperties.class);
 
   private static final String SOLR_XML = "solr.xml";
-  private static final String SOLR_PERSIST_XML = "solr-persist.xml";
 
   @Rule
   public TestRule solrTestRules = 
     RuleChain.outerRule(new SystemPropertiesRestoreRule());
 
-  private static final XPathFactory xpathFactory = XPathFactory.newInstance();
-
-  @Override
-  @Before
-  public void setUp() throws Exception {
-    super.setUp();
-  }
-  
-  @Override
-  @After
-  public void tearDown() throws Exception {
-   super.tearDown(); 
-  }
-
-  private static void dumpFile(File fileToDump) throws IOException {
-    System.out.println("Dumping " + fileToDump.getAbsolutePath());
-    InputStream is = new FileInputStream(fileToDump);
-    try {
-      ByteStreams.copy(is, System.out);
-    }
-    finally {
-      IOUtils.closeQuietly(is);
-    }
-  }
-  
   @Override
   protected File getSolrXml() throws Exception {
-    //This test writes on the directory where the solr.xml is located. Better to copy the solr.xml to
+    return new File(SOLR_HOME, SOLR_XML);
-    //the temporary directory where we store the index
-    File origSolrXml = new File(SOLR_HOME, SOLR_XML);
-    File solrXml = new File(tempDir, SOLR_XML);
-    FileUtils.copyFile(origSolrXml, solrXml);
-    return solrXml;
-  }
-
-  @Override
-  protected void deleteAdditionalFiles() {
-    super.deleteAdditionalFiles();
-
-    //Cleans the solr.xml persisted while testing and the solr.xml copied to the temporary directory
-    File persistedFile = new File(tempDir, SOLR_PERSIST_XML);
-    assertTrue("Failed to delete "+persistedFile, persistedFile.delete());
-    File solrXml = new File(tempDir, SOLR_XML);
-    assertTrue("Failed to delete "+ solrXml, solrXml.delete());
   }
 
   protected SolrServer getSolrAdmin() {
@@ -125,8 +67,8 @@ public class TestSolrProperties extends AbstractEmbeddedSolrServerTestCase {
   @Test
   public void testProperties() throws Exception {
 
-    String persistedSolrXml = new File(tempDir, SOLR_PERSIST_XML).getAbsolutePath();
+    SolrXMLCoresLocator.NonPersistingLocator locator
-    log.info("persistedSolrXml: {}", persistedSolrXml);
+        = (SolrXMLCoresLocator.NonPersistingLocator) cores.getCoresLocator();
 
     UpdateRequest up = new UpdateRequest();
     up.setAction(ACTION.COMMIT, true, true);
@@ -197,51 +139,22 @@ public class TestSolrProperties extends AbstractEmbeddedSolrServerTestCase {
     long after = mcr.getStartTime(name).getTime();
     assertTrue("should have more recent time: " + after + "," + before, after > before);
 
-    mcr = CoreAdminRequest.persist(persistedSolrXml, coreadmin);
+    TestHarness.validateXPath(locator.xml,
-
+        "/solr/cores[@defaultCoreName='core0']",
-    DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
+        "/solr/cores[@host='127.0.0.1']",
-    FileInputStream fis = new FileInputStream(new File(persistedSolrXml));
+        "/solr/cores[@hostPort='${hostPort:8983}']",
-    try {
+        "/solr/cores[@zkClientTimeout='8000']",
-      Document document = builder.parse(fis);
+        "/solr/cores[@hostContext='${hostContext:solr}']",
-      fis.close();
+        "/solr/cores[@genericCoreNodeNames='${genericCoreNodeNames:true}']"
-      fis = new FileInputStream(new File(persistedSolrXml));
+        );
-      String solrPersistXml = IOUtils.toString(new InputStreamReader(fis, "UTF-8"));
-      //System.out.println("xml:" + solrPersistXml);
-      assertTrue("\"/solr/cores[@defaultCoreName='core0']\" doesn't match in:\n" + solrPersistXml,
-                 exists("/solr/cores[@defaultCoreName='core0']", document));
-      assertTrue("\"/solr/cores[@host='127.0.0.1']\" doesn't match in:\n" + solrPersistXml,
-                 exists("/solr/cores[@host='127.0.0.1']", document));
-      assertTrue("\"/solr/cores[@hostPort='${hostPort:8983}']\" doesn't match in:\n" + solrPersistXml,
-                 exists("/solr/cores[@hostPort='${hostPort:8983}']", document));
-      assertTrue("\"/solr/cores[@zkClientTimeout='8000']\" doesn't match in:\n" + solrPersistXml,
-                 exists("/solr/cores[@zkClientTimeout='8000']", document));
-      assertTrue("\"/solr/cores[@hostContext='${hostContext:solr}']\" doesn't match in:\n" + solrPersistXml,
-                 exists("/solr/cores[@hostContext='${hostContext:solr}']", document));
-      assertTrue("\"/solr/cores[@genericCoreNodeNames='${genericCoreNodeNames:true}']\" doesn't match in:\n" + solrPersistXml,
-          exists("/solr/cores[@genericCoreNodeNames='${genericCoreNodeNames:true}']", document));
-    } finally {
-      fis.close();
-    }
     
     CoreAdminRequest.renameCore(name, "renamed_core", coreadmin);
 
-    mcr = CoreAdminRequest.persist(persistedSolrXml, getRenamedSolrAdmin());
+    TestHarness.validateXPath(locator.xml,
-    
+        "/solr/cores/core[@name='renamed_core']",
-//    fis = new FileInputStream(new File(tempDir, SOLR_PERSIST_XML));
+        "/solr/cores/core[@instanceDir='${theInstanceDir:./}']",
-//    String solrPersistXml = IOUtils.toString(fis);
+        "/solr/cores/core[@collection='${collection:acollection}']"
-//    System.out.println("xml:" + solrPersistXml);
+        );
-//    fis.close();
-    
-    fis = new FileInputStream(new File(persistedSolrXml));
-    try {
-      Document document = builder.parse(fis);
-      assertTrue(exists("/solr/cores/core[@name='renamed_core']", document));
-      assertTrue(exists("/solr/cores/core[@instanceDir='${theInstanceDir:./}']", document));
-      assertTrue(exists("/solr/cores/core[@collection='${collection:acollection}']", document));
-      
-    } finally {
-      fis.close();
-    }
     
     coreadmin = getRenamedSolrAdmin();
     File dataDir = new File(tempDir,"data3");
@@ -251,49 +164,8 @@ public class TestSolrProperties extends AbstractEmbeddedSolrServerTestCase {
         coreadmin, null, null, dataDir.getAbsolutePath(),
         tlogDir.getAbsolutePath());
 
-//    fis = new FileInputStream(new File(solrXml.getParent(), SOLR_PERSIST_XML));
+    TestHarness.validateXPath(locator.xml, "/solr/cores/core[@name='collection1' and @instanceDir='.']");
-//    solrPersistXml = IOUtils.toString(fis);
-//    System.out.println("xml:" + solrPersistXml);
-//    fis.close();
 
-    mcr = CoreAdminRequest.persist(persistedSolrXml, getRenamedSolrAdmin());
-    
-//    fis = new FileInputStream(new File(solrXml.getParent(), SOLR_PERSIST_XML));
-//    solrPersistXml = IOUtils.toString(fis);
-//    System.out.println("xml:" + solrPersistXml);
-//    fis.close();
-    
-    fis = new FileInputStream(new File(persistedSolrXml));
-    try {
-      Document document = builder.parse(fis);
-      assertTrue(exists("/solr/cores/core[@name='collection1' and @instanceDir='.']", document));
-    } finally {
-      fis.close();
-    }
-    
-    // test reload and parse
-    cores.shutdown();
-    
-//   fis = new FileInputStream(new File(getSolrXml().getParent(),
-//   SOLR_PERSIST_XML));
-//   String solrPersistXml = IOUtils.toString(fis);
-//   System.out.println("xml:" + solrPersistXml);
-//   fis.close();
-    
-    cores = CoreContainer.createAndLoad(SOLR_HOME.getAbsolutePath(), new File(persistedSolrXml));
- 
-    //mcr = CoreAdminRequest.persist(SOLR_PERSIST_XML, getRenamedSolrAdmin());
-    
-//     fis = new FileInputStream(new File(solrXml.getParent(),
-//     SOLR_PERSIST_XML));
-//     solrPersistXml = IOUtils.toString(fis);
-//     System.out.println("xml:" + solrPersistXml);
-//     fis.close();
   }
 
-  public static boolean exists(String xpathStr, Node node)
-      throws XPathExpressionException {
-    XPath xpath = xpathFactory.newXPath();
-    return (Boolean) xpath.evaluate(xpathStr, node, XPathConstants.BOOLEAN);
-  }
 }

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -35,6 +35,7 @@ import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.XML;
 import org.apache.solr.core.ConfigSolr;
 import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.core.SolrConfig;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrResourceLoader;
@@ -73,6 +74,7 @@ import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.logging.ConsoleHandler;
 import java.util.logging.Handler;
 import java.util.logging.Level;
@@ -1576,4 +1578,51 @@ public abstract class SolrTestCaseJ4 extends LuceneTestCase {
     FileUtils.copyFile(new File(top, "solrconfig.snippet.randomindexconfig.xml"), new File(subHome, "solrconfig.snippet.randomindexconfig.xml"));
   }
 
+  public static CoreDescriptorBuilder buildCoreDescriptor(CoreContainer container, String name, String instancedir) {
+    return new CoreDescriptorBuilder(container, name, instancedir);
+  }
+
+  public static class CoreDescriptorBuilder {
+
+    final String name;
+    final String instanceDir;
+    final CoreContainer container;
+    final Properties properties = new Properties();
+
+    public CoreDescriptorBuilder(CoreContainer container, String name, String instancedir) {
+      this.name = name;
+      this.instanceDir = instancedir;
+      this.container = container;
+    }
+
+    public CoreDescriptorBuilder withSchema(String schema) {
+      properties.setProperty(CoreDescriptor.CORE_SCHEMA, schema);
+      return this;
+    }
+
+    public CoreDescriptorBuilder withConfig(String config) {
+      properties.setProperty(CoreDescriptor.CORE_CONFIG, config);
+      return this;
+    }
+
+    public CoreDescriptorBuilder withDataDir(String datadir) {
+      properties.setProperty(CoreDescriptor.CORE_DATADIR, datadir);
+      return this;
+    }
+
+    public CoreDescriptor build() {
+      return new CoreDescriptor(container, name, instanceDir, properties);
+    }
+
+    public CoreDescriptorBuilder isTransient(boolean isTransient) {
+      properties.setProperty(CoreDescriptor.CORE_TRANSIENT, Boolean.toString(isTransient));
+      return this;
+    }
+
+    public CoreDescriptorBuilder loadOnStartup(boolean loadOnStartup) {
+      properties.setProperty(CoreDescriptor.CORE_LOADONSTARTUP, Boolean.toString(loadOnStartup));
+      return this;
+    }
+  }
+
 }

solr/test-framework/src/java/org/apache/solr/util/TestHarness.java

@@ -17,7 +17,6 @@
 
 package org.apache.solr.util;
 
-import com.google.common.base.Charsets;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.NamedList;
@@ -38,14 +37,12 @@ import org.apache.solr.schema.IndexSchema;
 import org.apache.solr.schema.IndexSchemaFactory;
 import org.apache.solr.servlet.DirectSolrConnection;
 
-import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.HashMap;
 import java.util.Map;
 
-
 /**
  * This class provides a simple harness that may be useful when
  * writing testcases.
@@ -156,7 +153,7 @@ public class TestHarness extends BaseTestHarness {
    */
   public TestHarness(String solrHome, String solrXml) {
     this(new SolrResourceLoader(solrHome),
-          ConfigSolr.fromInputStream(null, new ByteArrayInputStream(solrXml.getBytes(Charsets.UTF_8))));
+          ConfigSolr.fromString(solrXml));
   }
 
   /**
@@ -188,7 +185,7 @@ public class TestHarness extends BaseTestHarness {
         + "\" transient=\"false\" loadOnStartup=\"true\""
         + " shard=\"${shard:shard1}\" collection=\"${collection:collection1}\" instanceDir=\"" + coreName + "/\" />\n"
         + "  </cores>\n" + "</solr>";
-    return ConfigSolr.fromString(new SolrResourceLoader(dataDir), solrxml);
+    return ConfigSolr.fromString(solrxml);
   }
   
   public CoreContainer getCoreContainer() {
@@ -423,4 +420,6 @@ public class TestHarness extends BaseTestHarness {
       return new LocalSolrQueryRequest(TestHarness.this.getCore(), new NamedList(entries));
     }
   }
+
+
 }