SOLR-5108: fail plugin info loading if multiple instances exist but only one is expected

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

solr/CHANGES.txt

@@ -66,9 +66,10 @@ Upgrading from Solr 4.4.0
   would be chosen arbitrarily and silently.  Starting with 4.5, configuration 
   parsing will fail with an error in situations like this.  If you see error 
   messages such as "solrconfig.xml contains more than one value for config path: 
-  indexConfig/infoStream" check your solrconfig.xml file for multiple occurrences 
-  of "infoStream" and delete the one that you do not wish to use.  See SOLR-4953
-  for more details.
+  XXXXX" or "Found Z configuration sections when at most 1 is allowed matching 
+  expression: XXXXX" check your solrconfig.xml file for multiple occurrences of 
+  XXXXX and delete the ones that you do not wish to use.  See SOLR-4953 & 
+  SOLR-5108 for more details.
 
 Detailed Change List
 ----------------------
@@ -164,8 +165,9 @@ Other Changes
 
 * SOLR-4951: Better randomization of MergePolicy in Solr tests (hossman)
 
-* SOLR-4953: Make XML Configuration parsing fail if an xpath matches multiple 
-  nodes when only a single value is expected.  (hossman)
+* SOLR-4953, SOLR-5108: Make XML Configuration parsing fail if an xpath matches 
+  multiple nodes when only a single value or plugin instance is expected.  
+  (hossman)
 
 * The routing parameter "shard.keys" is deprecated as part of SOLR-5017 .The new parameter name is '_route_' .
   The old parameter should continue to work for another release  (Noble Paul)

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

@@ -17,6 +17,7 @@
 
 package org.apache.solr.core;
 
+import static org.apache.solr.core.SolrConfig.PluginOpts.*;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.schema.IndexSchemaFactory;
@@ -70,7 +71,15 @@ public class SolrConfig extends Config {
   
   public static final String DEFAULT_CONF_FILE = "solrconfig.xml";
 
-
+  static enum PluginOpts { 
+    MULTI_OK, 
+    REQUIRE_NAME,
+    REQUIRE_CLASS,
+    // EnumSet.of and/or EnumSet.copyOf(Collection) are anoying
+    // because of type determination
+    NOOP
+    }
+  
   /** Creates a default instance from the solrconfig.xml. */
   public SolrConfig()
   throws ParserConfigurationException, IOException, SAXException {
@@ -207,27 +216,46 @@ public class SolrConfig extends Config {
     }
      maxWarmingSearchers = getInt("query/maxWarmingSearchers",Integer.MAX_VALUE);
 
-     loadPluginInfo(SolrRequestHandler.class,"requestHandler",true, true);
-     loadPluginInfo(QParserPlugin.class,"queryParser",true, true);
-     loadPluginInfo(QueryResponseWriter.class,"queryResponseWriter",true, true);
-     loadPluginInfo(ValueSourceParser.class,"valueSourceParser",true, true);
-     loadPluginInfo(TransformerFactory.class,"transformer",true, true);
-     loadPluginInfo(SearchComponent.class,"searchComponent",true, true);
-     loadPluginInfo(QueryConverter.class,"queryConverter",true, true);
+     loadPluginInfo(SolrRequestHandler.class,"requestHandler",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+     loadPluginInfo(QParserPlugin.class,"queryParser",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+     loadPluginInfo(QueryResponseWriter.class,"queryResponseWriter",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+     loadPluginInfo(ValueSourceParser.class,"valueSourceParser",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+     loadPluginInfo(TransformerFactory.class,"transformer",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+     loadPluginInfo(SearchComponent.class,"searchComponent",
+                    REQUIRE_NAME, REQUIRE_CLASS, MULTI_OK);
+
+     // TODO: WTF is up with queryConverter???
+     // it aparently *only* works as a singleton? - SOLR-4304
+     // and even then -- only if there is a single SpellCheckComponent
+     // because of queryConverter.setAnalyzer
+     loadPluginInfo(QueryConverter.class,"queryConverter",
+                    REQUIRE_NAME, REQUIRE_CLASS);
 
      // this is hackish, since it picks up all SolrEventListeners,
      // regardless of when/how/why they are used (or even if they are 
      // declared outside of the appropriate context) but there's no nice 
      // way around that in the PluginInfo framework
-     loadPluginInfo(SolrEventListener.class, "//listener",false, true);
+     loadPluginInfo(SolrEventListener.class, "//listener", 
+                    REQUIRE_CLASS, MULTI_OK);
 
-     loadPluginInfo(DirectoryFactory.class,"directoryFactory",false, true);
-     loadPluginInfo(IndexDeletionPolicy.class,indexConfigPrefix+"/deletionPolicy",false, true);
-     loadPluginInfo(CodecFactory.class,"codecFactory",false, false);
-     loadPluginInfo(IndexReaderFactory.class,"indexReaderFactory",false, true);
-     loadPluginInfo(UpdateRequestProcessorChain.class,"updateRequestProcessorChain",false, false);
-     loadPluginInfo(UpdateLog.class,"updateHandler/updateLog",false, false);
-     loadPluginInfo(IndexSchemaFactory.class,"schemaFactory",false, true);
+     loadPluginInfo(DirectoryFactory.class,"directoryFactory", 
+                    REQUIRE_CLASS);
+     loadPluginInfo(IndexDeletionPolicy.class,indexConfigPrefix+"/deletionPolicy", 
+                    REQUIRE_CLASS);
+     loadPluginInfo(CodecFactory.class,"codecFactory", 
+                    REQUIRE_CLASS);
+     loadPluginInfo(IndexReaderFactory.class,"indexReaderFactory", 
+                    REQUIRE_CLASS);
+     loadPluginInfo(UpdateRequestProcessorChain.class,"updateRequestProcessorChain", 
+                    MULTI_OK);
+     loadPluginInfo(UpdateLog.class,"updateHandler/updateLog");
+     loadPluginInfo(IndexSchemaFactory.class,"schemaFactory", 
+                    REQUIRE_CLASS);
 
      updateHandlerInfo = loadUpdatehandlerInfo();
 
@@ -245,8 +273,19 @@ public class SolrConfig extends Config {
             getBool("updateHandler/commitWithin/softCommit",true));
   }
 
-  private void loadPluginInfo(Class clazz, String tag, boolean requireName, boolean requireClass) {
+  private void loadPluginInfo(Class clazz, String tag, PluginOpts... opts) {
+    EnumSet<PluginOpts> options = EnumSet.<PluginOpts>of(NOOP, opts);
+    boolean requireName = options.contains(REQUIRE_NAME);
+    boolean requireClass = options.contains(REQUIRE_CLASS);
+
     List<PluginInfo> result = readPluginInfos(tag, requireName, requireClass);
+
+    if (1 < result.size() && ! options.contains(MULTI_OK)) {
+        throw new SolrException
+          (SolrException.ErrorCode.SERVER_ERROR,
+           "Found " + result.size() + " configuration sections when at most "
+           + "1 is allowed matching expression: " + tag);
+    }
     if(!result.isEmpty()) pluginStore.put(clazz.getName(),result);
   }
 
@@ -446,7 +485,15 @@ public class SolrConfig extends Config {
   }
   public PluginInfo getPluginInfo(String  type){
     List<PluginInfo> result = pluginStore.get(type);
-    return result == null || result.isEmpty() ? null: result.get(0);
+    if (result == null || result.isEmpty()) {
+      return null;
+    }
+    if (1 == result.size()) {
+      return result.get(0);
+    }
+
+    throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                            "Multiple plugins configured for type: " + type);
   }
   
   private void initLibs() {

solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-dirfactory.xml

@@ -0,0 +1,34 @@
+<?xml version="1.0" ?>
+
+<!--
+ 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.
+-->
+
+<config>
+
+  <luceneMatchVersion>${tests.luceneMatchVersion:LUCENE_CURRENT}</luceneMatchVersion>
+
+  <xi:include href="solrconfig.snippet.randomindexconfig.xml" xmlns:xi="http://www.w3.org/2001/XInclude"/>
+
+  <directoryFactory name="DirectoryFactory" class="solr.RAMDirectoryFactory"/>
+
+  <!-- BEGIN: BAD -->
+  <directoryFactory name="DirectoryFactory" class="NRTCachingDirectoryFactory"/>
+  <!-- END: BAD -->
+
+
+
+</config>

solr/core/src/test-files/solr/collection1/conf/solrconfig-test-misc.xml

@@ -39,8 +39,6 @@
   <lib dir="../../lib-dirs/c" regex="c1" />
   <lib path="../../lib-dirs/d/d1/" />
   
-  <directoryFactory name="DirectoryFactory" class="${solr.directoryFactory:solr.RAMDirectoryFactory}"/>
-
   <!-- see TestConfig.testJavaProperty -->
   <propTest attr1="${solr.test.sys.prop1}-$${literal}"
             attr2="${non.existent.sys.prop:default-from-config}">prefix-${solr.test.sys.prop2}-suffix</propTest>

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

@@ -27,6 +27,10 @@ public class TestBadConfig extends AbstractBadConfigTestBase {
     assertConfigs("bad_solrconfig.xml","schema.xml","unset.sys.property");
   }
 
+  public void testMultipleDirectoryFactories() throws Exception {
+      assertConfigs("bad-solrconfig-multiple-dirfactory.xml", "schema12.xml",
+                    "directoryFactory");
+  }
   public void testMultipleIndexConfigs() throws Exception {
       assertConfigs("bad-solrconfig-multiple-indexconfigs.xml", "schema12.xml",
                     "indexConfig");