SOLR-4790, Defining a core with the same name should throw an error

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

solr/CHANGES.txt

@@ -60,7 +60,7 @@ Upgrading from Solr 4.3.0
 * SOLR-4778: The signature of LogWatcher.registerListener has changed, from
   (ListenerConfig, CoreContainer) to (ListenerConfig).  Users implementing their
   own LogWatcher classes will need to change their code accordingly.
-
+  
 Detailed Change List
 ----------------------
 
@@ -95,9 +95,12 @@ Bug Fixes
 
 * SOLR-4616: HitRatio on caches is now exposed over JMX MBeans as a float.
   (Greg Bowyer)
-
+  
 * SOLR-4803: Fixed core discovery mode (ie: new style solr.xml) to treat 
   'collection1' as the default core name. (hossman)
+  
+* SOLR-4790: Throw an error if a core has the same name as another core, both old and
+  new style solr.xml
 
 * SOLR-4563: RSS DIH-example not working (janhoy)
 

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

@@ -178,16 +178,19 @@ public class ConfigSolrXmlOld extends ConfigSolr {
           log.error(msg);
         }
       }
-      
-      if (dataDir != null) {
-        if (!dirs.containsKey(dataDir)) {
-          dirs.put(dataDir, name);
+
+      String instDir = DOMUtil.getAttr(node, CoreDescriptor.CORE_INSTDIR, null);
+      if (dataDir != null && instDir != null) { // this won't load anyway if instDir not specified.
+
+        String absData = new File(instDir, dataDir).getCanonicalPath();
+        if (!dirs.containsKey(absData)) {
+          dirs.put(absData, 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);
+                  absData, dirs.get(absData), name);
           log.warn(msg);
         }
       }

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

@@ -343,22 +343,4 @@ public class CoreDescriptor {
   public void putProperty(String prop, String val) {
     coreProperties.put(prop, val);
   }
-
-  // This is particularly useful for checking if any two cores have the same
-  // data dir.
-  public String getAbsoluteDataDir() {
-    String dataDir = getDataDir();
-    if (dataDir == null) return null; // No worse than before.
-
-    if (new File(dataDir).isAbsolute()) {
-      return SolrResourceLoader.normalizeDir(
-          SolrResourceLoader.normalizeDir(dataDir));
-    }
-
-    if (coreContainer == null) return null;
-
-    return SolrResourceLoader.normalizeDir(coreContainer.getSolrHome() +
-        SolrResourceLoader.normalizeDir(dataDir));
-
-  }
 }

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

@@ -9,6 +9,7 @@ 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;
@@ -89,6 +90,11 @@ public class SolrCoreDiscoverer {
       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/schema/IndexSchema.java

@@ -440,6 +440,7 @@ public class IndexSchema {
       } else {
         sb.append("null");
       }
+      sb.append("]est");
       if (nd==null) {
         sb.append("schema has no name!");
         log.warn(sb.toString());

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

@@ -24,6 +24,7 @@ import java.util.Properties;
 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,10 +89,9 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
 
   }
 
-  private void addCoreWithProps(Properties stockProps) throws Exception {
+  private void addCoreWithProps(String name, Properties stockProps) throws Exception {
 
-    File propFile = new File(solrHomeDirectory,
-        stockProps.getProperty(CoreDescriptor.CORE_NAME) + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE);
+    File propFile = new File(new File(solrHomeDirectory, name), SolrCoreDiscoverer.CORE_PROP_FILE);
     File parent = propFile.getParentFile();
     assertTrue("Failed to mkdirs for " + parent.getAbsolutePath(), parent.mkdirs());
     addCoreWithProps(stockProps, propFile);
@@ -127,12 +127,12 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
     setMeUp();
 
     // name, isLazy, loadOnStartup
-    addCoreWithProps(makeCorePropFile("core1", false, true, "dataDir=core1"));
-    addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"));
+    addCoreWithProps("core1", makeCorePropFile("core1", false, true, "dataDir=core1"));
+    addCoreWithProps("core2", makeCorePropFile("core2", false, false, "dataDir=core2"));
 
     // I suspect what we're adding in here is a "configset" rather than a schema or solrconfig.
     //
-    addCoreWithProps(makeCorePropFile("lazy1", true, false, "dataDir=lazy1"));
+    addCoreWithProps("lazy1", makeCorePropFile("lazy1", true, false, "dataDir=lazy1"));
 
     CoreContainer cc = init();
     try {
@@ -170,6 +170,30 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
     }
   }
 
+  @Test
+  public void testDuplicateNames() throws Exception {
+    setMeUp();
+
+    // name, isLazy, loadOnStartup
+    addCoreWithProps("core1", makeCorePropFile("core1", false, true));
+    addCoreWithProps("core2", makeCorePropFile("core2", false, false, "name=core1"));
+    CoreContainer cc = null;
+    try {
+      cc = init();
+      fail("Should have thrown exception in testDuplicateNames");
+    } catch (SolrException se) {
+      assertTrue("Should have seen an exception because two cores had the same name",
+          "Core  + desc.getName() + \" defined twice".indexOf(se.getMessage()) != -1);
+      assertTrue("/core1 should have been mentioned in the message", "/core1".indexOf(se.getMessage()) != -1);
+      assertTrue("/core2 should have been mentioned in the message", "/core2".indexOf(se.getMessage()) != -1);
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+
   @Test
   public void testAlternateCoreDir() throws Exception {
     File alt = new File(TEMP_DIR, "alternateCoreDir");
@@ -193,7 +217,29 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
       if (alt.exists()) FileUtils.deleteDirectory(alt);
     }
   }
-
+  @Test
+  public void testNoCoreDir() throws Exception {
+    File noCoreDir = new File(TEMP_DIR, "noCoreDir");
+    if (noCoreDir.exists()) FileUtils.deleteDirectory(noCoreDir);
+    noCoreDir.mkdirs();
+    setMeUp(noCoreDir.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true),
+        new File(noCoreDir, "core1" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+    addCoreWithProps(makeCorePropFile("core2", false, false),
+        new File(noCoreDir, "core2" + File.separator + SolrCoreDiscoverer.CORE_PROP_FILE));
+    CoreContainer cc = init();
+    try {
+      SolrCore core1 = cc.getCore("core1");
+      SolrCore core2 = cc.getCore("core2");
+      assertNotNull(core1);
+      assertNotNull(core2);
+      core1.close();
+      core2.close();
+    } finally {
+      cc.shutdown();
+      if (noCoreDir.exists()) FileUtils.deleteDirectory(noCoreDir);
+    }
+  }
   // For testing whether finding a solr.xml overrides looking at solr.properties
   private final static String SOLR_XML = "<solr> " +
       "<int name=\"transientCacheSize\">2</int> " +