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

svn commit: r1466319 - in /lucene/dev/branches/branch_4x: ./ dev-tools/ lucene/ lucene/analysis/ lucene/analysis/icu/src/java/org/apache/lucene/collation/ lucene/backwards/ lucene/benchmark/ lucene/classification/ lucene/classification/src/ lucene/code...

Author: erick
Date: Wed Apr 10 03:03:24 2013
New Revision: 1466319

URL: http://svn.apache.org/r1466319
Log:
SOLR-4663, SOLR-4347, SOLR-1905. Warn for bad core configs, persist transient cores correctly, do not persist implicit properties

Modified:
    lucene/dev/branches/branch_4x/   (props changed)
    lucene/dev/branches/branch_4x/dev-tools/   (props changed)
    lucene/dev/branches/branch_4x/lucene/   (props changed)
    lucene/dev/branches/branch_4x/lucene/BUILD.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/CHANGES.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/JRE_VERSION_MIGRATION.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/LICENSE.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/MIGRATE.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/NOTICE.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/README.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/SYSTEM_REQUIREMENTS.txt   (props changed)
    lucene/dev/branches/branch_4x/lucene/analysis/   (props changed)
    lucene/dev/branches/branch_4x/lucene/analysis/icu/src/java/org/apache/lucene/collation/ICUCollationKeyFilterFactory.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/backwards/   (props changed)
    lucene/dev/branches/branch_4x/lucene/benchmark/   (props changed)
    lucene/dev/branches/branch_4x/lucene/build.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/classification/   (props changed)
    lucene/dev/branches/branch_4x/lucene/classification/build.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/classification/ivy.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/classification/src/   (props changed)
    lucene/dev/branches/branch_4x/lucene/codecs/   (props changed)
    lucene/dev/branches/branch_4x/lucene/common-build.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/index.40.cfs.zip   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/index.40.nocfs.zip   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/index.40.optimized.cfs.zip   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/index.40.optimized.nocfs.zip   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSort.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortDocValues.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestSortRandom.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestTopFieldCollector.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/search/TestTotalHitCountCollector.java   (props changed)
    lucene/dev/branches/branch_4x/lucene/demo/   (props changed)
    lucene/dev/branches/branch_4x/lucene/facet/   (props changed)
    lucene/dev/branches/branch_4x/lucene/grouping/   (props changed)
    lucene/dev/branches/branch_4x/lucene/highlighter/   (props changed)
    lucene/dev/branches/branch_4x/lucene/ivy-settings.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/join/   (props changed)
    lucene/dev/branches/branch_4x/lucene/licenses/   (props changed)
    lucene/dev/branches/branch_4x/lucene/memory/   (props changed)
    lucene/dev/branches/branch_4x/lucene/misc/   (props changed)
    lucene/dev/branches/branch_4x/lucene/module-build.xml   (props changed)
    lucene/dev/branches/branch_4x/lucene/queries/   (props changed)
    lucene/dev/branches/branch_4x/lucene/queryparser/   (props changed)
    lucene/dev/branches/branch_4x/lucene/sandbox/   (props changed)
    lucene/dev/branches/branch_4x/lucene/site/   (props changed)
    lucene/dev/branches/branch_4x/lucene/spatial/   (props changed)
    lucene/dev/branches/branch_4x/lucene/suggest/   (props changed)
    lucene/dev/branches/branch_4x/lucene/test-framework/   (props changed)
    lucene/dev/branches/branch_4x/lucene/tools/   (props changed)
    lucene/dev/branches/branch_4x/solr/   (props changed)
    lucene/dev/branches/branch_4x/solr/CHANGES.txt   (contents, props changed)
    lucene/dev/branches/branch_4x/solr/LICENSE.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/NOTICE.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/README.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/SYSTEM_REQUIREMENTS.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/build.xml   (props changed)
    lucene/dev/branches/branch_4x/solr/cloud-dev/   (props changed)
    lucene/dev/branches/branch_4x/solr/common-build.xml   (props changed)
    lucene/dev/branches/branch_4x/solr/contrib/   (props changed)
    lucene/dev/branches/branch_4x/solr/core/   (props changed)
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java
    lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestConfig.java   (props changed)
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
    lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
    lucene/dev/branches/branch_4x/solr/example/   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpclient-LICENSE-ASL.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpclient-NOTICE.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpcore-LICENSE-ASL.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpcore-NOTICE.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpmime-LICENSE-ASL.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/licenses/httpmime-NOTICE.txt   (props changed)
    lucene/dev/branches/branch_4x/solr/scripts/   (props changed)
    lucene/dev/branches/branch_4x/solr/site/   (props changed)
    lucene/dev/branches/branch_4x/solr/solrj/   (props changed)
    lucene/dev/branches/branch_4x/solr/test-framework/   (props changed)
    lucene/dev/branches/branch_4x/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
    lucene/dev/branches/branch_4x/solr/webapp/   (props changed)

Modified: lucene/dev/branches/branch_4x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/CHANGES.txt?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/solr/CHANGES.txt Wed Apr 10 03:03:24 2013
@@ -65,19 +65,6 @@ New Features
 * SOLR-4648 PreAnalyzedUpdateProcessorFactory allows using the functionality
   of PreAnalyzedField with other field types. See javadoc for details and
   examples. (Andrzej Bialecki)
-
-* SOLR-4196 (and others). Solr.xml is being deprecated in favor of a simple
-  properties file. In the absence of a <solr_home>/solr.xml but the presence of
-  <solr_home>/solr.properties, two things will happen
-  1> The attributes that, in the solr.xml file, were in the <solr> and <cores> tags will
-      be read from the solr.properties
-  2> The <solr_home> will be walked and any cores fond will be discovered, which will
-     be inferred by the presence of a "core.properties" file which will contain the 
-     data formerly in the individual <core> tags. The implication here is that there will
-     be no individual core information in the solr.properties file.
-  See the discussion on the wiki page titled "Core Discovery (4.3 and beyond)" for
-  the formats of both solr.properties and the individual core.properties files
-  (Erick Erickson)
   
 * SOLR-4623: Provide REST API read access to all elements of the live schema.
   Add a REST API request to return the entire live schema, in JSON, XML, and
@@ -106,6 +93,9 @@ New Features
   there are a large number of entries in the multiValued field. Conspicuously, this will
   prevent the "best" match from being found if it appears later in the MV list than the
   cutoff specified by either of these params. (Erick Erickson)
+  
+ * SOLR-4663: Complain loudly and do NOT load cores if they have the same data dir or the 
+  same name
 
 * SOLR-4675: Improve PostingsSolrHighlighter to support per-field/query-time overrides
   and add additional configuration parameters. See the javadocs for more details and
@@ -116,6 +106,18 @@ New Features
 
 * SOLR-4530: DIH: Provide configuration to use Tika's IdentityHtmlMapper
   (Alexandre Rafalovitch via shalin)
+  
+* SOLR-4663: Log an error if more than one core has the same name or points to the same
+  data directory. The new discovery-based core enumeration will be particularly sensitive
+  to this. (Erick Erickson)
+  
+* SOLR-4347: Insure that newly-created cores via Admin handler are persisted in solr.xml
+  (Erick Erickson)
+  
+* SOLR-1905: Cores created by the admin request handler should be persisted to solr.xml.
+  Also fixed a problem whereby properties like solr.solr.datadir would be persisted
+  to solr.xml. Also, cores that didn't happen to be loaded were not persisted. 
+  (Erick Erickson)
 
 Bug Fixes
 ----------------------

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolr.java Wed Apr 10 03:03:24 2013
@@ -75,4 +75,8 @@ public interface ConfigSolr {
   public Properties readCoreProperties(String coreName);
 
   public Map<String, String> readCoreAttributes(String coreName);
+
+  // If the core is not to be loaded (say two cores defined with the same name or with the same data dir), return
+  // the reason. If it's OK to load the core, return null.
+  public String getBadCoreMessage(String name);
 }

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/ConfigSolrXml.java Wed Apr 10 03:03:24 2013
@@ -52,9 +52,12 @@ import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
 
 /**
  * ConfigSolrXml
@@ -66,7 +69,7 @@ import java.util.Properties;
  *
  * It's a bit twisted, but we decided to NOT do the solr.properties switch. But since there's already an interface
  * it makes sense to leave it in so we can use other methods of providing the Solr information that is contained
- * in solr.xml. Perhapse something form SolrCloud in the future?
+ * in solr.xml. Perhaps something form SolrCloud in the future?
  *
  */
 
@@ -77,6 +80,8 @@ public class ConfigSolrXml extends Confi
 
   private final Map<String, CoreDescriptorPlus> coreDescriptorPlusMap = new HashMap<String, CoreDescriptorPlus>();
   private NodeList coreNodes = null;
+  private final Map<String, String> badCores = new HashMap<String, String>();
+    // List of cores that we should _never_ load. Ones with dup names or duplicate datadirs or...
 
   static {
     prefixes = new HashMap<ConfLevel, String>();
@@ -104,14 +109,47 @@ public class ConfigSolrXml extends Confi
     isAutoDiscover = getBool(ConfigSolr.ConfLevel.SOLR_CORES, "autoDiscoverCores", false);
     if (isAutoDiscover) {
       synchronized (coreDescriptorPlusMap) {
-        walkFromHere(new File(container.getSolrHome()), container);
+        walkFromHere(new File(container.getSolrHome()), container, new HashMap<String, CoreDescriptorPlus>());
       }
 
     } else {
       coreNodes = (NodeList) evaluate("solr/cores/core",
           XPathConstants.NODESET);
+      // Check a couple of error conditions
+      Set<String> names = new HashSet<String>(); // for duplicate names
+      Map<String, String> dirs = new HashMap<String, String>(); // for duplicate data dirs.
+
+      for (int idx = 0; idx < coreNodes.getLength(); ++idx) {
+        Node node = coreNodes.item(idx);
+        String name = DOMUtil.getAttr(node,  CoreDescriptor.CORE_NAME, null);
+        String dataDir = DOMUtil.getAttr(node,  CoreDescriptor.CORE_DATADIR, null);
+        if (name != null) {
+          if (! names.contains(name)) {
+            names.add(name);
+          } else {
+            String msg = String.format(Locale.ROOT, "More than one core defined for core named %s", name);
+            log.error(msg);
+            badCores.put(name, msg);
+          }
+        }
+
+        if (dataDir != null) {
+          if (! dirs.containsKey(dataDir)) {
+            dirs.put(dataDir, name);
+          } else {
+            String msg = String.format(Locale.ROOT, "More than one core points to data dir %s. They are in %s and %s",
+                dataDir, dirs.get(dataDir), name);
+            log.error(msg);
+            badCores.put(name, msg);
+          }
+        }
+      }
     }
+  }
 
+  @Override
+  public String getBadCoreMessage(String name) {
+    return badCores.get(name);
   }
   public static Document copyDoc(Document doc) throws TransformerException {
     TransformerFactory tfactory = TransformerFactory.newInstance();
@@ -219,7 +257,7 @@ public class ConfigSolrXml extends Confi
   // deeper in the tree.
   //
   // @param file - the directory we're to either read the properties file from or recurse into.
-  private void walkFromHere(File file, CoreContainer container) throws IOException {
+  private void walkFromHere(File file, CoreContainer container, Map<String, CoreDescriptorPlus> checkMap) throws IOException {
     log.info("Looking for cores in " + file.getAbsolutePath());
     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
@@ -251,11 +289,29 @@ public class ConfigSolrXml extends Confi
         }
         CoreDescriptor desc = new CoreDescriptor(container, props);
         CoreDescriptorPlus plus = new CoreDescriptorPlus(propFile.getAbsolutePath(), desc, propsOrig);
-        coreDescriptorPlusMap.put(desc.getName(), plus);
+        CoreDescriptorPlus check = coreDescriptorPlusMap.get(desc.getName());
+        if (check == null) { // It's bad to have two cores with the same name
+          coreDescriptorPlusMap.put(desc.getName(), plus);
+        } else {
+          String msg = String.format(Locale.ROOT, "More than one core defined for core named %s, paths are '%s' and '%s' ",
+              desc.getName(), check.getFilePath(), plus.getFilePath());
+          log.error(msg);
+          badCores.put(desc.getName(), msg);
+        }
+        check = coreDescriptorPlusMap.get(plus.getCoreDescriptor().getDataDir());
+        if (check == null) {
+          coreDescriptorPlusMap.put(desc.getName(), plus);
+        } else {
+          String msg = String.format(Locale.ROOT, "More than one core points to data dir %s. They are in %s and %s",
+              plus.getCoreDescriptor().getDataDir(), plus.getFilePath(), check.getFilePath());
+          log.error(msg);
+          badCores.put(plus.getCoreDescriptor().getName(), msg);
+        }
+
         continue; // Go on to the sibling directory
       }
       if (childFile.isDirectory()) {
-        walkFromHere(childFile, container);
+        walkFromHere(childFile, container, checkMap);
       }
     }
   }

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/core/CoreContainer.java Wed Apr 10 03:03:24 2013
@@ -963,12 +963,16 @@ public class CoreContainer
                dcore.getName(), instanceDir);
 
       // Initialize the solr config
+      SolrCore created = null;
       if (zkController != null) {
-        return createFromZk(instanceDir, dcore);
+        created = createFromZk(instanceDir, dcore);
       } else {
-        return createFromLocal(instanceDir, dcore);
+        created = createFromLocal(instanceDir, dcore);
       }
 
+      coreMaps.addCreated(created); // For persisting newly-created cores.
+      return created;
+
       // :TODO: Java7...
       // http://docs.oracle.com/javase/7/docs/technotes/guides/language/catch-multiple.html
     } catch (Exception ex) {
@@ -1007,6 +1011,16 @@ public class CoreContainer
   }
 
   /**
+   * Checks that the data dir passed is is NOT shared by any other core
+   * @param targetPath - path to check
+   * @return - null if this path is unique, core name of the first other core that shares this path.
+   */
+  public String checkUniqueDataDir(String targetPath) {
+    return coreMaps.checkUniqueDataDir(targetPath);
+
+  }
+
+  /**
    * Returns an immutable Map of Exceptions that occured when initializing 
    * SolrCores (either at startup, or do to runtime requests to create cores) 
    * keyed off of the name (String) of the SolrCore that had the Exception 
@@ -1044,6 +1058,13 @@ public class CoreContainer
     try {
       name = checkDefault(name);
 
+      if (cfg != null) { // Another test artifact.
+        String badMsg = cfg.getBadCoreMessage(name);
+        if (badMsg != null) {
+          throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, badMsg);
+        }
+      }
+
       SolrCore core = coreMaps.getCore(name);
       if (core == null)
         throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + name );
@@ -1098,6 +1119,7 @@ public class CoreContainer
     }
   }
 
+  //5.0 remove all checkDefaults?
   private String checkDefault(String name) {
     return (null == name || name.isEmpty()) ? defaultCoreName : name;
   } 
@@ -1149,6 +1171,14 @@ public class CoreContainer
   public SolrCore getCore(String name) {
 
     name = checkDefault(name);
+
+    if (cfg != null) { // Get this out of here sometime, this is test-code only stuff!
+      String badMsg = cfg.getBadCoreMessage(name);
+      if (badMsg != null) {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, badMsg);
+      }
+    }
+
     // Do this in two phases since we don't want to lock access to the cores over a load.
     SolrCore core = coreMaps.getCoreFromAnyList(name);
 
@@ -1401,6 +1431,9 @@ public class CoreContainer
     log.error(msg, ex);
     return new SolrException(ErrorCode.SERVER_ERROR, msg, ex);
   }
+  String getCoreToOrigName(SolrCore core) {
+    return coreMaps.getCoreToOrigName(core);
+  }
 }
 
 
@@ -1422,6 +1455,8 @@ class CoreMaps {
 
   private final Map<String, CoreDescriptor> dynamicDescriptors = new LinkedHashMap<String, CoreDescriptor>();
 
+  private final Map<String, SolrCore> createdCores = new LinkedHashMap<String, SolrCore>();
+
   private Map<SolrCore, String> coreToOrigName = new ConcurrentHashMap<SolrCore, String>();
 
   private final CoreContainer container;
@@ -1492,7 +1527,7 @@ class CoreMaps {
           CoreContainer.log.info("Core " + coreName + " moved from core container list before closing.");
         } else {
           try {
-            addPersistOneCore(cfg, core, container.loader);
+            addPersistOneCore(cfg, container.loader, core.getCoreDescriptor(), getCoreToOrigName(core));
 
             core.close();
           } catch (Throwable t) {
@@ -1611,6 +1646,7 @@ class CoreMaps {
       set.addAll(cores.keySet());
       set.addAll(transientCores.keySet());
       set.addAll(dynamicDescriptors.keySet());
+      set.addAll(createdCores.keySet());
     }
     return set;
   }
@@ -1645,12 +1681,20 @@ class CoreMaps {
   protected SolrCore remove(String name, boolean removeOrig) {
 
     synchronized (locker) {
-      SolrCore core = cores.remove(name);
-      if (removeOrig && core != null) {
-        coreToOrigName.remove(core);
-      }
-
-      return core;
+      SolrCore tmp = cores.remove(name);
+      SolrCore ret = null;
+      if (removeOrig && tmp != null) {
+        coreToOrigName.remove(tmp);
+      }
+      ret = (ret == null) ? tmp : ret;
+      // It could have been a newly-created core. It could have been a transient core. The newly-created cores
+      // in particular should be checked. It could have been a dynamic core.
+      tmp = transientCores.remove(name);
+      ret = (ret == null) ? tmp : ret;
+      tmp = createdCores.remove(name);
+      ret = (ret == null) ? tmp : ret;
+      dynamicDescriptors.remove(name);
+      return ret;
     }
   }
 
@@ -1757,21 +1801,43 @@ class CoreMaps {
     // This is expensive in the maximal case, but I think necessary. It should keep a reference open to all of the
     // current cores while they are saved. Remember that especially the transient core can come and go.
     //
-    // Maybe the right thing to do is keep all the core descriptors NOT in the SolrCore, but keep all of the
-    // core descriptors in SolrProperties exclusively.
-    // TODO: 5.0 move coreDescriptors out of SolrCore and keep them only once in SolrProperties
+    // TODO: 5.0. remove the possibility of storing core descirptors in solr.xml?
     //
     synchronized (locker) {
       if (cfg == null) {
         ConfigSolrXml.initPersistStatic();
         persistCores(cfg, cores, loader);
         persistCores(cfg, transientCores, loader);
+        // 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())) {
+            addPersistOneCore(cfg, loader, ent.getValue(), null);
+          }
+        }
+        for (Map.Entry<String, SolrCore> ent : createdCores.entrySet()) {
+          if (! cores.containsKey(ent.getKey()) && ! transientCores.containsKey(ent.getKey())
+              && ! dynamicDescriptors.containsKey(ent.getKey())) {
+            addPersistOneCore(cfg, loader, ent.getValue().getCoreDescriptor(), null);
+          }
+        }
         ConfigSolrXml.addPersistAllCoresStatic(containerProperties, rootSolrAttribs, coresAttribs,
             (file == null ? configFile : file));
       } else {
         cfg.initPersist();
         persistCores(cfg, cores, loader);
         persistCores(cfg, transientCores, loader);
+        // 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())) {
+            addPersistOneCore(cfg, loader, ent.getValue(), null);
+          }
+        }
+        for (Map.Entry<String, SolrCore> ent : createdCores.entrySet()) {
+          if (! cores.containsKey(ent.getKey()) && ! transientCores.containsKey(ent.getKey())
+              && ! dynamicDescriptors.containsKey(ent.getKey())) {
+            addPersistOneCore(cfg, loader, ent.getValue().getCoreDescriptor(), null);
+          }
+        }
         cfg.addPersistAllCores(containerProperties, rootSolrAttribs, coresAttribs, (file == null ? configFile : file));
       }
     }
@@ -1827,7 +1893,7 @@ class CoreMaps {
 
   protected void persistCores(ConfigSolr cfg, Map<String, SolrCore> whichCores, SolrResourceLoader loader) {
     for (SolrCore solrCore : whichCores.values()) {
-      addPersistOneCore(cfg, solrCore, loader);
+      addPersistOneCore(cfg, loader, solrCore.getCoreDescriptor(), getCoreToOrigName(solrCore));
     }
   }
 
@@ -1836,14 +1902,10 @@ class CoreMaps {
     coreAttribs.put(key, value);
   }
 
-  protected void addPersistOneCore(ConfigSolr cfg, SolrCore solrCore, SolrResourceLoader loader) {
-
-    CoreDescriptor dcore = solrCore.getCoreDescriptor();
+  protected void addPersistOneCore(ConfigSolr cfg, SolrResourceLoader loader, CoreDescriptor dcore, String origCoreName) {
 
     String coreName = dcore.getProperty(CoreDescriptor.CORE_NAME);
 
-    String origCoreName = null;
-
     Map<String, String> coreAttribs = new HashMap<String, String>();
     Properties persistProps = new Properties();
     CloudDescriptor cd = dcore.getCloudDescriptor();
@@ -1879,9 +1941,6 @@ class CoreMaps {
       }
 
     } else {
-
-      origCoreName = getCoreToOrigName(solrCore);
-
       if (origCoreName == null) {
         origCoreName = coreName;
       }
@@ -1911,16 +1970,6 @@ class CoreMaps {
     coreAttribs.put(CoreDescriptor.CORE_LOADONSTARTUP, Boolean.toString(dcore.isLoadOnStartup()));
     coreAttribs.put(CoreDescriptor.CORE_TRANSIENT, Boolean.toString(dcore.isTransient()));
 
-    // Now add back in any implicit properties that aren't in already. These are all "attribs" in this meaning
-    Properties implicit = dcore.initImplicitProperties();
-
-    if (! coreName.equals(container.getDefaultCoreName())) {
-      for (String prop : implicit.stringPropertyNames()) {
-        if (coreAttribs.get(prop) == null) {
-          coreAttribs.put(prop, implicit.getProperty(prop));
-        }
-      }
-    }
     if (cfg != null) {
       cfg.addPersistCore(coreName, persistProps, coreAttribs);
     } else {
@@ -1948,7 +1997,31 @@ class CoreMaps {
     return null;
   }
 
+  protected void addCreated(SolrCore core) {
+    synchronized (locker) {
+      createdCores.put(core.getName(), core);
+    }
+  }
 
+  protected String checkUniqueDataDir(String targetPath) {
+    // Have to check
+    // loaded cores
+    // transient cores
+    // dynamic cores
+    synchronized (locker) {
+      for (SolrCore core : cores.values()) {
+        if (targetPath.equals(core.getDataDir())) return core.getName();
+      }
+      for (SolrCore core : transientCores.values()) {
+        if (targetPath.equals(core.getDataDir())) return core.getName();
+      }
+      for (CoreDescriptor desc : dynamicDescriptors.values()) {
+        if (targetPath.equals(desc.getDataDir())) return desc.getName();
+      }
+    }
+
+    return null;
+  }
 }
 
 class CloserThread extends Thread {
@@ -1982,7 +2055,8 @@ class CloserThread extends Thread {
            removeMe != null && !container.isShutDown();
            removeMe = coreMaps.getCoreToClose()) {
         try {
-          coreMaps.addPersistOneCore(cfg, removeMe, container.loader);
+          coreMaps.addPersistOneCore(cfg, container.loader, removeMe.getCoreDescriptor(),
+              container.getCoreToOrigName(removeMe));
           removeMe.close();
         } finally {
           coreMaps.removeFromPendingOps(removeMe.getName());

Modified: lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java Wed Apr 10 03:03:24 2013
@@ -419,14 +419,12 @@ public class CoreAdminHandler extends Re
     try {
       
       //for now, do not allow creating new core with same name when in cloud mode
-      //XXX perhaps it should just be unregistered from cloud before readding it?, 
+      //XXX perhaps it should just be unregistered from cloud before reading it?,
       //XXX perhaps we should also check that cores are of same type before adding new core to collection?
-      if (coreContainer.isZooKeeperAware()) {
-        if (coreContainer.getCoreNames().contains(name)) {
-          log.info("Re-creating a core with existing name is not allowed in cloud mode");
-          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-              "Core with name '" + name + "' already exists.");
-        }
+      if (coreContainer.getAllCoreNames().contains(name)) {
+        log.warn("Re-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);
@@ -449,7 +447,7 @@ public class CoreAdminHandler extends Re
       opts = params.get(CoreAdminParams.DATA_DIR);
       if (opts != null)
         dcore.setDataDir(opts);
-      
+
       opts = params.get(CoreAdminParams.ULOG_DIR);
       if (opts != null)
         dcore.setUlogDir(opts);
@@ -513,6 +511,15 @@ public class CoreAdminHandler extends Re
       dcore.setCoreProperties(coreProperties);
       
       SolrCore core = coreContainer.create(dcore);
+
+      String sameDirCore = coreContainer.checkUniqueDataDir(core.getDataDir());
+      if (sameDirCore != null) {
+        if (core != null) core.close();
+        log.warn("Creating a core that points to the same data dir as core {} is not allowed", sameDirCore);
+        throw new SolrException(ErrorCode.SERVER_ERROR,
+            "Core with same data dir '" + sameDirCore + "' already exists.");
+      }
+
       coreContainer.register(name, core, false);
       rsp.add("core", core.getName());
       return coreContainer.isPersistent();

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java Wed Apr 10 03:03:24 2013
@@ -26,23 +26,36 @@ import java.util.ArrayList;
 import java.util.List;
 
 import javax.xml.parsers.ParserConfigurationException;
-import javax.xml.xpath.XPathExpressionException;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.lucene.util.IOUtils;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.xml.sax.SAXException;
 
 public class TestCoreContainer extends SolrTestCaseJ4 {
-  
+
+  private static String oldSolrHome;
+  private static final String SOLR_HOME_PROP = "solr.solr.home";
+
   @BeforeClass
   public static void beforeClass() throws Exception {
+    oldSolrHome = System.getProperty(SOLR_HOME_PROP);
     initCore("solrconfig.xml", "schema.xml");
   }
 
+  @AfterClass
+  public static void afterClass() {
+    if (oldSolrHome != null) {
+      System.setProperty(SOLR_HOME_PROP, oldSolrHome);
+    } else {
+      System.clearProperty(SOLR_HOME_PROP);
+    }
+  }
+
   private File solrHomeDirectory;
 
   private CoreContainer init(String dirName) throws Exception {
@@ -204,6 +217,9 @@ public class TestCoreContainer extends S
           "/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"));
       
@@ -212,9 +228,9 @@ public class TestCoreContainer extends S
       
       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)");
+          "/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
       
@@ -237,42 +253,13 @@ public class TestCoreContainer extends S
     }
   }
   
-  public void assertXmlFile(final File file, String... xpath)
-      throws IOException, SAXException {
-    
-    try {
-      String xml = FileUtils.readFileToString(file, "UTF-8");
-      String results = h.validateXPath(xml, xpath);
-      if (null != results) {
-        String msg = "File XPath failure: file=" + file.getPath() + " xpath="
-            + results + "\n\nxml was: " + xml;
-        fail(msg);
-      }
-    } catch (XPathExpressionException e2) {
-      throw new RuntimeException("XPath is invalid", e2);
-    }
-  }
 
+  @Test
   public void testNoCores() throws IOException, ParserConfigurationException, SAXException {
     //create solrHome
     File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName()
         + "_noCores");
-    if (solrHomeDirectory.exists()) {
-      FileUtils.deleteDirectory(solrHomeDirectory);
-    }
-    assertTrue("Failed to mkdirs workDir", solrHomeDirectory.mkdirs());
-    try {
-      File solrXmlFile = new File(solrHomeDirectory, "solr.xml");
-      BufferedWriter out = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(solrXmlFile), IOUtils.CHARSET_UTF_8));
-      out.write(EMPTY_SOLR_XML);
-      out.close();
-    } catch (IOException e) {
-      FileUtils.deleteDirectory(solrHomeDirectory);
-      throw e;
-    }
-
-    //init
-    System.setProperty("solr.solr.home", solrHomeDirectory.getAbsolutePath());
+    SetUpHome(solrHomeDirectory, EMPTY_SOLR_XML);
     CoreContainer.Initializer init = new CoreContainer.Initializer();
     CoreContainer cores = null;
     try {
@@ -311,6 +298,84 @@ public class TestCoreContainer extends S
   }
 
   @Test
+  public void testCoresSameName() throws IOException, ParserConfigurationException, SAXException {
+    //create solrHome
+    File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName()
+        + "_sameName");
+    SetUpHome(solrHomeDirectory, SOLR_XML_SAME_NAME);
+    copyMinConf(new File(solrHomeDirectory, "core1"));
+    copyMinConf(new File(solrHomeDirectory, "core2"));
+    CoreContainer.Initializer init = new CoreContainer.Initializer();
+    CoreContainer cores = null;
+    SolrCore core1 = null;
+    try {
+      cores = init.initialize();
+      core1 = cores.getCore("core1");
+    } catch(SolrException se) {
+      assertEquals("Exception code should be 500", 500, se.code());
+      assertTrue("Should have seen an exception when two cores have the same name",
+          se.getMessage().contains("More than one core defined for core named"));
+
+    } finally {
+      if (cores != null) {
+        if (core1 != null) core1.close();
+        cores.shutdown();
+      }
+      FileUtils.deleteDirectory(solrHomeDirectory);
+    }
+  }
+
+  @Test
+  public void testCoresSameDataDir() throws IOException, ParserConfigurationException, SAXException {
+    //create solrHome
+    File solrHomeDirectory = new File(TEMP_DIR, this.getClass().getName()
+        + "_sameDataDir");
+    SetUpHome(solrHomeDirectory, SOLR_XML_SAME_DATADIR);
+    copyMinConf(new File(solrHomeDirectory, "core1"));
+    copyMinConf(new File(solrHomeDirectory, "core2"));
+    CoreContainer.Initializer init = new CoreContainer.Initializer();
+    CoreContainer cores = null;
+    SolrCore core1 = null;
+    SolrCore core2 = null;
+    try {
+      cores = init.initialize();
+      core1 = cores.getCore("core1");
+      core2 = cores.getCore("core2");
+    }
+    catch(SolrException se) {
+      assertEquals("Exception code should be 500", 500, se.code());
+      assertTrue("Should have seen an exception when two cores have the same data dir",
+          se.getMessage().contains("More than one core points to data dir"));
+    } finally {
+      if (cores != null) {
+        if (core1 != null) core1.close();
+        if (core2 != null) core2.close();
+        cores.shutdown();
+      }
+      FileUtils.deleteDirectory(solrHomeDirectory);
+    }
+  }
+
+  private void SetUpHome(File solrHomeDirectory, String xmlFile) throws IOException {
+    if (solrHomeDirectory.exists()) {
+      FileUtils.deleteDirectory(solrHomeDirectory);
+    }
+    assertTrue("Failed to mkdirs workDir", solrHomeDirectory.mkdirs());
+    try {
+      File solrXmlFile = new File(solrHomeDirectory, "solr.xml");
+      BufferedWriter out = new BufferedWriter(new OutputStreamWriter(new FileOutputStream(solrXmlFile), IOUtils.CHARSET_UTF_8));
+      out.write(xmlFile);
+      out.close();
+    } catch (IOException e) {
+      FileUtils.deleteDirectory(solrHomeDirectory);
+      throw e;
+    }
+
+    //init
+    System.setProperty(SOLR_HOME_PROP, solrHomeDirectory.getAbsolutePath());
+  }
+
+  @Test
   public void testClassLoaderHierarchy() throws Exception {
     final CoreContainer cc = init("_classLoaderHierarchy");
     try {
@@ -335,5 +400,22 @@ public class TestCoreContainer extends S
       "  <cores adminPath=\"/admin/cores\" transientCacheSize=\"32\" >\n" +
       "  </cores>\n" +
       "</solr>";
-  
+
+  private static final String SOLR_XML_SAME_NAME ="<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n" +
+      "<solr persistent=\"false\">\n" +
+      "  <cores adminPath=\"/admin/cores\" transientCacheSize=\"32\" >\n" +
+      "    <core name=\"core1\" instanceDir=\"core1\" dataDir=\"core1\"/> \n" +
+      "    <core name=\"core1\" instanceDir=\"core2\" dataDir=\"core2\"/> \n " +
+      "  </cores>\n" +
+      "</solr>";
+
+  private static final String SOLR_XML_SAME_DATADIR ="<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n" +
+      "<solr persistent=\"false\">\n" +
+      "  <cores adminPath=\"/admin/cores\" transientCacheSize=\"32\" >\n" +
+      "    <core name=\"core2\" instanceDir=\"core2\" dataDir=\"../samedatadir\" schema=\"schema-tiny.xml\" config=\"solrconfig-minimal.xml\" /> \n" +
+      "    <core name=\"core1\" instanceDir=\"core2\" dataDir=\"../samedatadir\" schema=\"schema-tiny.xml\" config=\"solrconfig-minimal.xml\"  /> \n " +
+      "  </cores>\n" +
+      "</solr>";
+
+
 }

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java Wed Apr 10 03:03:24 2013
@@ -20,6 +20,7 @@ package org.apache.solr.core;
 import org.apache.commons.io.FileUtils;
 import org.apache.lucene.util.IOUtils;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
 import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -88,6 +89,24 @@ public class TestCoreDiscovery extends S
     addConfFiles(new File(parent, "conf"));
   }
 
+  // For testing error condition of having multiple cores with the same name.
+  private void addCoreWithPropsDir(String coreDir, Properties stockProps) throws Exception {
+
+    File propFile = new File(solrHomeDirectory, coreDir + File.separator + ConfigSolr.CORE_PROP_FILE);
+    File parent = propFile.getParentFile();
+    assertTrue("Failed to mkdirs for " + parent.getAbsolutePath(), parent.mkdirs());
+
+    FileOutputStream out = new FileOutputStream(propFile);
+    try {
+      stockProps.store(out, null);
+    } finally {
+      out.close();
+    }
+
+    addConfFiles(new File(parent, "conf"));
+  }
+
+
   private void addConfFiles(File confDir) throws Exception {
     String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
     assertTrue("Failed to mkdirs for " + confDir.getAbsolutePath(), confDir.mkdirs());
@@ -119,7 +138,7 @@ public class TestCoreDiscovery extends S
   // Test the basic setup, create some dirs with core.properties files in them, but solr.xml has discoverCores
   // set and insure that we find all the cores and can load them.
   @Test
-  public void testPropertiesFile() throws Exception {
+  public void testDiscover() throws Exception {
     setMeUp();
     addSolrXml();
     // name, isLazy, loadOnStartup
@@ -267,7 +286,6 @@ public class TestCoreDiscovery extends S
 
     try {
       cc.persist();
-//      checkSolrProperties(cc);
 
       checkCoreProps(makeCorePropFile("core1", false, true));
       checkCoreProps(makeCorePropFile("core2", false, false));
@@ -283,6 +301,127 @@ public class TestCoreDiscovery extends S
     }
   }
 
+  @Test
+  public void testCoresWithSameNameError() throws Exception {
+    setMeUp();
+    addSolrXml();
+    addCoreWithPropsDir("core1_1", makeCorePropFile("core1", false, true));
+    addCoreWithPropsDir("core1_2", makeCorePropFile("core1", false, true));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+  @Test
+  public void testCoresWithSameDataDirError() throws Exception{
+    setMeUp();
+    addSolrXml();
+    addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
+    addCoreWithProps(makeCorePropFile("core2", false, true, "dataDir=" + solrHomeDirectory + "datadir"));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+  @Test
+  public void testCoresWithSameNameErrorTransient() throws Exception {
+    setMeUp();
+    addSolrXml();
+    addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true, false));
+    addCoreWithPropsDir("core1_2", makeCorePropFile("core1", true, false));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+  @Test
+  public void testCoresWithSameDataDirErrorTransient() throws Exception{
+    setMeUp();
+    addSolrXml();
+    addCoreWithProps(makeCorePropFile("core1", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
+    addCoreWithProps(makeCorePropFile("core2", true, false, "dataDir=" + solrHomeDirectory + "datadir"));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+
+  @Test
+  public void testCoresWithSameNameErrorboth() throws Exception {
+    setMeUp();
+    addSolrXml();
+    addCoreWithPropsDir("core1_1", makeCorePropFile("core1", true,  false));
+    addCoreWithPropsDir("core1_2", makeCorePropFile("core1", false, false));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core defined for core named core1") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+  @Test
+  public void testCoresWithSameDataDirErrorBoth() throws Exception{
+    setMeUp();
+    addSolrXml();
+    addCoreWithProps(makeCorePropFile("core1", false, false, "dataDir=" + solrHomeDirectory + "datadir"));
+    addCoreWithProps(makeCorePropFile("core2", true,  false, "dataDir=" + solrHomeDirectory + "datadir"));
+    // Should just blow up here.
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException se) {
+      assertEquals("Should be returning proper error code of 500", 500, se.code());
+      assertTrue(se.getCause().getMessage().indexOf("More than one core points to data dir") != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
   void addCoreProps(SolrCore core, String... propPairs) {
     for (String keyval : propPairs) {
       String[] pair = keyval.split("=");

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestLazyCores.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestLazyCores.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestLazyCores.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/core/TestLazyCores.java Wed Apr 10 03:03:24 2013
@@ -20,9 +20,13 @@ package org.apache.solr.core;
 import org.apache.commons.io.FileUtils;
 import org.apache.lucene.util.IOUtils;
 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.handler.admin.CoreAdminHandler;
 import org.apache.solr.request.LocalSolrQueryRequest;
 import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
 import org.apache.solr.search.SolrIndexSearcher;
 import org.apache.solr.update.AddUpdateCommand;
 import org.apache.solr.update.CommitUpdateCommand;
@@ -48,14 +52,6 @@ public class TestLazyCores extends SolrT
 
   private final File solrHomeDirectory = new File(TEMP_DIR, "org.apache.solr.core.TestLazyCores_testlazy");
 
-  private void copyConfFiles(File home, String subdir) throws IOException {
-
-    File subHome = new File(new File(home, subdir), "conf");
-    assertTrue("Failed to make subdirectory ", subHome.mkdirs());
-    String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
-    FileUtils.copyFile(new File(top, "schema-tiny.xml"), new File(subHome, "schema-tiny.xml"));
-    FileUtils.copyFile(new File(top, "solrconfig-minimal.xml"), new File(subHome, "solrconfig-minimal.xml"));
-  }
 
   private CoreContainer init() throws Exception {
 
@@ -64,7 +60,7 @@ public class TestLazyCores extends SolrT
     }
     assertTrue("Failed to mkdirs workDir", solrHomeDirectory.mkdirs());
     for (int idx = 1; idx < 10; ++idx) {
-      copyConfFiles(solrHomeDirectory, "collection" + idx);
+      copyMinConf(new File(solrHomeDirectory, "collection" + idx));
     }
 
     File solrXml = new File(solrHomeDirectory, "solr.xml");
@@ -83,7 +79,6 @@ public class TestLazyCores extends SolrT
       FileUtils.deleteDirectory(solrHomeDirectory);
     }
   }
-
   @Test
   public void testLazyLoad() throws Exception {
     CoreContainer cc = init();
@@ -127,6 +122,7 @@ public class TestLazyCores extends SolrT
 
   // This is a little weak. I'm not sure how to test that lazy core2 is loaded automagically. The getCore
   // will, of course, load it.
+
   @Test
   public void testLazySearch() throws Exception {
     CoreContainer cc = init();
@@ -246,6 +242,7 @@ public class TestLazyCores extends SolrT
   }
 
   // Test case for SOLR-4300
+
   @Test
   public void testRace() throws Exception {
     final List<SolrCore> theCores = new ArrayList<SolrCore>();
@@ -280,6 +277,166 @@ public class TestLazyCores extends SolrT
     }
   }
 
+  private void tryCreateFail(CoreAdminHandler admin, String name, String dataDir, String... errs) throws Exception {
+    try {
+      SolrQueryResponse resp = new SolrQueryResponse();
+
+      SolrQueryRequest request = req(CoreAdminParams.ACTION,
+          CoreAdminParams.CoreAdminAction.CREATE.toString(),
+          CoreAdminParams.DATA_DIR, dataDir,
+          CoreAdminParams.NAME, name,
+          "schema", "schema-tiny.xml",
+          "config", "solrconfig-minimal.xml");
+
+      admin.handleRequestBody(request, resp);
+      fail("Should have thrown an error");
+    } catch (SolrException se) {
+      SolrException cause = (SolrException)se.getCause();
+      assertEquals("Exception code should be 500", 500, cause.code());
+      for (String err : errs) {
+       assertTrue("Should have seen an exception containing the an error",
+            cause.getMessage().contains(err));
+      }
+    }
+  }
+  @Test
+  public void testCreateSame() throws Exception {
+    final CoreContainer cc = init();
+    try {
+      // First, try all 4 combinations of load on startup and transient
+      final CoreAdminHandler admin = new CoreAdminHandler(cc);
+      SolrCore lc2 = cc.getCore("collectionLazy2");
+      SolrCore lc4 = cc.getCore("collectionLazy4");
+      SolrCore lc5 = cc.getCore("collectionLazy5");
+      SolrCore lc6 = cc.getCore("collectionLazy6");
+
+      copyMinConf(new File(solrHomeDirectory, "t2"));
+      copyMinConf(new File(solrHomeDirectory, "t4"));
+      copyMinConf(new File(solrHomeDirectory, "t5"));
+      copyMinConf(new File(solrHomeDirectory, "t6"));
+
+      tryCreateFail(admin, "t2", lc2.getDataDir(), "Core with same data dir", "collectionLazy2", "already exists");
+      tryCreateFail(admin, "t4", lc4.getDataDir(), "Core with same data dir", "collectionLazy4", "already exists");
+      tryCreateFail(admin, "t5", lc5.getDataDir(), "Core with same data dir", "collectionLazy5", "already exists");
+      tryCreateFail(admin, "t6", lc6.getDataDir(), "Core with same data dir", "collectionLazy6", "already exists");
+
+      // Insure a newly-created core fails too
+      CoreDescriptor d1 = new CoreDescriptor(cc, "core1", "./core1");
+      d1.setSchemaName("schema-tiny.xml");
+      d1.setConfigName("solrconfig-minimal.xml");
+      copyMinConf(new File(solrHomeDirectory, "core1"));
+      SolrCore core1 = cc.create(d1);
+      cc.register(core1, false);
+      copyMinConf(new File(solrHomeDirectory, "core77"));
+      tryCreateFail(admin, "core77", core1.getDataDir(), "Core with same data dir", "core1", "already exists");
+
+      // Should also fail with the same name
+      tryCreateFail(admin, "collectionLazy2", "t12", "Core with name", "collectionLazy2", "already exists");
+      tryCreateFail(admin, "collectionLazy4", "t14", "Core with name", "collectionLazy4", "already exists");
+      tryCreateFail(admin, "collectionLazy5", "t15", "Core with name", "collectionLazy5", "already exists");
+      tryCreateFail(admin, "collectionLazy6", "t16", "Core with name", "collectionLazy6", "already exists");
+      tryCreateFail(admin, "core1", "t10", "Core with name", "core1", "already exists");
+
+      core1.close();
+      lc2.close();
+      lc4.close();
+      lc5.close();
+      lc6.close();
+
+    } finally {
+      cc.shutdown();
+    }
+  }
+
+  //Make sure persisting not-loaded lazy cores is done. See SOLR-4347
+
+  @Test
+  public void testPersistence() throws Exception {
+    final CoreContainer cc = init();
+    try {
+      copyMinConf(new File(solrHomeDirectory, "core1"));
+      copyMinConf(new File(solrHomeDirectory, "core2"));
+      copyMinConf(new File(solrHomeDirectory, "core3"));
+      copyMinConf(new File(solrHomeDirectory, "core4"));
+
+      cc.setPersistent(true);
+      CoreDescriptor d1 = new CoreDescriptor(cc, "core1", "./core1");
+      d1.setTransient(true);
+      d1.setLoadOnStartup(true);
+      d1.setSchemaName("schema-tiny.xml");
+      d1.setConfigName("solrconfig-minimal.xml");
+      SolrCore core1 = cc.create(d1);
+
+      CoreDescriptor d2 = new CoreDescriptor(cc, "core2", "./core2");
+      d2.setTransient(true);
+      d2.setLoadOnStartup(false);
+      d2.setSchemaName("schema-tiny.xml");
+      d2.setConfigName("solrconfig-minimal.xml");
+      SolrCore core2 = cc.create(d2);
+
+      CoreDescriptor d3 = new CoreDescriptor(cc, "core3", "./core3");
+      d3.setTransient(false);
+      d3.setLoadOnStartup(true);
+      d3.setSchemaName("schema-tiny.xml");
+      d3.setConfigName("solrconfig-minimal.xml");
+      SolrCore core3 = cc.create(d3);
+
+      CoreDescriptor d4 = new CoreDescriptor(cc, "core4", "./core4");
+      d4.setTransient(false);
+      d4.setLoadOnStartup(false);
+      d4.setSchemaName("schema-tiny.xml");
+      d4.setConfigName("solrconfig-minimal.xml");
+      SolrCore core4 = cc.create(d4);
+
+      final File oneXml = new File(solrHomeDirectory, "lazy1.solr.xml");
+      cc.persistFile(oneXml);
+
+      assertXmlFile(oneXml,
+          "/solr/cores/core[@name='collection1']",
+          "/solr/cores/core[@name='collectionLazy2']",
+          "/solr/cores/core[@name='collectionLazy3']",
+          "/solr/cores/core[@name='collectionLazy4']",
+          "/solr/cores/core[@name='collectionLazy5']",
+          "/solr/cores/core[@name='collectionLazy6']",
+          "/solr/cores/core[@name='collectionLazy7']",
+          "/solr/cores/core[@name='collectionLazy8']",
+          "/solr/cores/core[@name='collectionLazy9']",
+          "/solr/cores/core[@name='core1']",
+          "/solr/cores/core[@name='core2']",
+          "/solr/cores/core[@name='core3']",
+          "/solr/cores/core[@name='core4']");
+      assertXmlFile(oneXml, "13=count(/solr/cores/core)");
+      core1.close();
+      core2.close();
+      core3.close();
+      core4.close();
+
+      removeOne(cc, "collectionLazy2");
+      removeOne(cc, "collectionLazy3");
+      removeOne(cc, "collectionLazy4");
+      removeOne(cc, "collectionLazy5");
+      removeOne(cc, "collectionLazy6");
+      removeOne(cc, "collectionLazy7");
+      removeOne(cc, "core1");
+      removeOne(cc, "core2");
+      removeOne(cc, "core3");
+      removeOne(cc, "core4");
+
+      // now test that unloading a core means the core is not persisted
+
+      final File twoXml = new File(solrHomeDirectory, "lazy2.solr.xml");
+      cc.persistFile(twoXml);
+
+      assertXmlFile(twoXml, "3=count(/solr/cores/core)");
+    } finally {
+      cc.shutdown();
+    }
+  }
+
+  private void removeOne(CoreContainer cc, String coreName) {
+    SolrCore tmp = cc.remove(coreName);
+    if (tmp != null) tmp.close();
+  }
   public static void checkNotInCores(CoreContainer cc, String... nameCheck) {
     Collection<String> names = cc.getCoreNames();
     for (String name : nameCheck) {

Modified: lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java (original)
+++ lucene/dev/branches/branch_4x/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java Wed Apr 10 03:03:24 2013
@@ -79,7 +79,7 @@ public class CoreAdminHandlerTest extend
     
     SolrQueryResponse resp = new SolrQueryResponse();
     admin.handleRequestBody
-      (req(CoreAdminParams.ACTION, 
+      (req(CoreAdminParams.ACTION,
            CoreAdminParams.CoreAdminAction.CREATE.toString(),
            CoreAdminParams.INSTANCE_DIR, instPropFile.getAbsolutePath(),
            CoreAdminParams.NAME, "props",
@@ -143,21 +143,4 @@ public class CoreAdminHandlerTest extend
 
   }
 
-  
-  public void assertXmlFile(final File file, String... xpath)
-      throws IOException, SAXException {
-    
-    try {
-      String xml = FileUtils.readFileToString(file, "UTF-8");
-      String results = h.validateXPath(xml, xpath);
-      if (null != results) {
-        String msg = "File XPath failure: file=" + file.getPath() + " xpath="
-            + results + "\n\nxml was: " + xml;
-        fail(msg);
-      }
-    } catch (XPathExpressionException e2) {
-      throw new RuntimeException("XPath is invalid", e2);
-    }
-  }
-  
 }

Modified: lucene/dev/branches/branch_4x/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java?rev=1466319&r1=1466318&r2=1466319&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java (original)
+++ lucene/dev/branches/branch_4x/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java Wed Apr 10 03:03:24 2013
@@ -23,6 +23,7 @@ import java.util.logging.*;
 
 import javax.xml.xpath.XPathExpressionException;
 
+import org.apache.commons.io.FileUtils;
 import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.lucene.util.QuickPatchThreadsFilter;
@@ -1442,4 +1443,30 @@ public abstract class SolrTestCaseJ4 ext
     }
     return result;
   }
+
+  public void assertXmlFile(final File file, String... xpath)
+      throws IOException, SAXException {
+
+    try {
+      String xml = FileUtils.readFileToString(file, "UTF-8");
+      String results = h.validateXPath(xml, xpath);
+      if (null != results) {
+        String msg = "File XPath failure: file=" + file.getPath() + " xpath="
+            + results + "\n\nxml was: " + xml;
+        fail(msg);
+      }
+    } catch (XPathExpressionException e2) {
+      throw new RuntimeException("XPath is invalid", e2);
+    }
+  }
+  // Creates a mininmal conf dir.
+  public void copyMinConf(File dstRoot) throws IOException {
+
+    File subHome = new File(dstRoot, "conf");
+    assertTrue("Failed to make subdirectory ", dstRoot.mkdirs());
+    String top = SolrTestCaseJ4.TEST_HOME() + "/collection1/conf";
+    FileUtils.copyFile(new File(top, "schema-tiny.xml"), new File(subHome, "schema-tiny.xml"));
+    FileUtils.copyFile(new File(top, "solrconfig-minimal.xml"), new File(subHome, "solrconfig-minimal.xml"));
+  }
+
 }