SOLR-5459: Try loading a temporary core when saving a file in the admin UI config editing mode

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

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

@@ -19,6 +19,7 @@ package org.apache.solr.handler.admin;
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
+import org.apache.solr.cloud.ZkController;
 import org.apache.solr.cloud.ZkSolrResourceLoader;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
@@ -30,6 +31,7 @@ import org.apache.solr.common.util.ContentStreamBase;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
 import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrResourceLoader;
 import org.apache.solr.handler.RequestHandlerBase;
@@ -128,7 +130,12 @@ public class ShowFileRequestHandler extends RequestHandlerBase
   public static final String USE_CONTENT_TYPE = "contentType";
   
   protected Set<String> hiddenFiles;
-  
+
+  private final static String OP_PARAM = "op";
+  private final static String OP_WRITE = "write";
+  private final static String OP_TEST = "test";
+
+
   public ShowFileRequestHandler()
   {
     super();
@@ -160,14 +167,14 @@ public class ShowFileRequestHandler extends RequestHandlerBase
       throws InterruptedException, KeeperException, IOException {
 
     CoreContainer coreContainer = req.getCore().getCoreDescriptor().getCoreContainer();
-    String op = req.getParams().get("op");
+    String op = req.getParams().get(OP_PARAM);
     if (op == null) {
       if (coreContainer.isZooKeeperAware()) {
         showFromZooKeeper(req, rsp, coreContainer);
       } else {
         showFromFileSystem(req, rsp);
       }
-    } else if ("write".equalsIgnoreCase(op)) {
+    } else if (OP_WRITE.equalsIgnoreCase(op) || OP_TEST.equalsIgnoreCase(op)) {
       String fname = req.getParams().get("file", null);
       if (fname == null) {
         rsp.setException(new SolrException(ErrorCode.BAD_REQUEST, "No file name specified for write operation."));
@@ -175,7 +182,7 @@ public class ShowFileRequestHandler extends RequestHandlerBase
         fname = fname.replace('\\', '/');
         if (isHiddenFile(req, rsp, fname, true) == false) {
           if (coreContainer.isZooKeeperAware()) {
-            writeToZooKeeper(req, rsp, coreContainer);
+            writeToZooKeeper(req, rsp);
           } else {
             writeToFileSystem(req, rsp);
           }
@@ -264,8 +271,10 @@ public class ShowFileRequestHandler extends RequestHandlerBase
   // file=velocity/error.vm or file=schema.xml
   //
   // Important: Assumes that the file already exists in ZK, so far we aren't creating files there.
-  private void writeToZooKeeper(SolrQueryRequest req, SolrQueryResponse rsp, CoreContainer coreContainer)
+  private void writeToZooKeeper(SolrQueryRequest req, SolrQueryResponse rsp)
       throws KeeperException, InterruptedException, IOException {
+
+    CoreContainer coreContainer = req.getCore().getCoreDescriptor().getCoreContainer();
     SolrZkClient zkClient = coreContainer.getZkController().getZkClient();
 
     String adminFile = getAdminFileFromZooKeeper(req, rsp, zkClient);
@@ -276,6 +285,10 @@ public class ShowFileRequestHandler extends RequestHandlerBase
 
     byte[] data = IOUtils.toByteArray(new InputStreamReader(stream.getStream(), "UTF-8"), "UTF-8");
     String fname = req.getParams().get("file", null);
+    if (OP_TEST.equals(req.getParams().get(OP_PARAM)))  {
+      testReloadSuccess(req, rsp, stream);
+      return;
+    }
     // Persist the managed schema
     try {
       // Assumption: the path exists
@@ -395,11 +408,89 @@ public class ShowFileRequestHandler extends RequestHandlerBase
       rsp.setException(new SolrException( ErrorCode.BAD_REQUEST, "File " + fname + " is a directory."));
       return;
     }
+    if (OP_TEST.equals(req.getParams().get(OP_PARAM))) {
+      testReloadSuccess(req, rsp, stream);
+      return;
+    }
 
     FileUtils.copyInputStreamToFile(stream.getStream(), adminFile);
     log.info("Successfully saved file " + adminFile.getAbsolutePath() + " locally");
   }
 
+  private boolean testReloadSuccess(SolrQueryRequest req, SolrQueryResponse rsp, ContentStream stream) {
+    // Try writing the config to a temporary core and reloading to see that we don't allow people to shoot themselves
+    // in the foot.
+    File home = null;
+    try {
+      home = new File(FileUtils.getTempDirectory(), "SOLR_5459"); // Unlikely to name a core or collection this!
+      FileUtils.writeStringToFile(new File(home, "solr.xml"), "<solr></solr>"); // Use auto-discovery
+      File coll = new File(home, "SOLR_5459");
+
+      SolrCore core = req.getCore();
+      CoreDescriptor desc = core.getCoreDescriptor();
+      CoreContainer coreContainer = desc.getCoreContainer();
+
+      if (coreContainer.isZooKeeperAware()) {
+        try {
+          String confPath = ((ZkSolrResourceLoader) core.getResourceLoader()).getCollectionZkPath();
+
+          ZkController.downloadConfigDir(coreContainer.getZkController().getZkClient(), confPath,
+              new File(coll, "conf"));
+        } catch (Exception ex) {
+          log.error("Error when attempting to download conf from ZooKeeper: " + ex.getMessage());
+          rsp.setException(new SolrException(ErrorCode.BAD_REQUEST,
+              "Error when attempting to download conf from ZooKeeper" + ex.getMessage()));
+          return false;
+        }
+      } else {
+        FileUtils.copyDirectory(new File(desc.getInstanceDir(), "conf"),
+            new File(coll, "conf"));
+      }
+
+      FileUtils.writeStringToFile(new File(coll, "core.properties"), "name=SOLR_5459");
+
+      FileUtils.copyInputStreamToFile(stream.getStream(),
+          new File(new File(coll, "conf"), req.getParams().get("file", null)));
+
+      return tryReloading(rsp, home);
+
+    } catch (IOException ex) {
+      log.warn("Caught IO exception when trying to verify configs. " + ex.getMessage());
+      rsp.setException(new SolrException(ErrorCode.SERVER_ERROR,
+          "Caught IO exception when trying to verify configs. " + ex.getMessage()));
+      return false;
+    } finally {
+      if (home != null) {
+        try {
+          FileUtils.deleteDirectory(home);
+        } catch (IOException e) {
+          log.warn("Caught IO exception trying to delete temporary directory " + home + e.getMessage());
+          return true; // Don't fail for this reason!
+        }
+      }
+    }
+  }
+
+  private boolean tryReloading(SolrQueryResponse rsp, File home) {
+    CoreContainer cc = null;
+    try {
+      cc = CoreContainer.createAndLoad(home.getAbsolutePath(), new File(home, "solr.xml"));
+      if (cc.getCoreInitFailures().size() > 0) {
+        for (Exception ex : cc.getCoreInitFailures().values()) {
+          log.error("Error when attempting to reload core: " + ex.getMessage());
+          rsp.setException(new SolrException( ErrorCode.BAD_REQUEST,
+              "Error when attempting to reload core after writing config" + ex.getMessage()));
+        }
+        return false;
+      }
+      return true;
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
   // Find the file indicated by the "file=XXX" parameter or the root of the conf directory on the local
   // file system. Respects all the "interesting" stuff around what the resource loader does to find files.
   private File getAdminFileFromFileSystem(SolrQueryRequest req, SolrQueryResponse rsp) {

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

@@ -48,6 +48,7 @@ public class TestModifyConfFiles extends AbstractFullDistribZkTestBase {
 
     params.remove("file");
     params.set("stream.body", "Testing rewrite of schema.xml file.");
+    params.set("op", "test");
     request = new QueryRequest(params);
     request.setPath("/admin/file");
     try {
@@ -57,6 +58,7 @@ public class TestModifyConfFiles extends AbstractFullDistribZkTestBase {
       assertEquals(e.getMessage(), "No file name specified for write operation.");
     }
 
+    params.set("op", "write");
     params.set("file", "bogus.txt");
     request = new QueryRequest(params);
     request.setPath("/admin/file");
@@ -76,8 +78,6 @@ public class TestModifyConfFiles extends AbstractFullDistribZkTestBase {
     SolrZkClient zkClient = cloudClient.getZkStateReader().getZkClient();
     String contents = new String(zkClient.getData("/configs/conf1/schema.xml", null, null, true), "UTF-8");
 
-    //String schema = getFileContentFromZooKeeper("schema.xml");
-
     assertTrue("Schema contents should have changed!", "Testing rewrite of schema.xml file.".equals(contents));
 
     // Create a velocity/whatever node. Put a bit of data in it. See if you can change it.

solr/core/src/test/org/apache/solr/schema/ModifyConfFileTest.java

@@ -86,15 +86,16 @@ public class ModifyConfFileTest extends SolrTestCaseJ4 {
 
       ArrayList<ContentStream> streams = new ArrayList<ContentStream>( 2 );
       streams.add( new ContentStreamBase.StringStream( "Testing rewrite of schema.xml file." ) );
-      //streams.add( new ContentStreamBase.StringStream( "there" ) );
 
-      params = params("op", "write", "file", "schema.xml", "stream.body", "Testing rewrite of schema.xml file.");
+      params = params("op", "test", "file", "schema.xml", "stream.body", "Testing rewrite of schema.xml file.");
       LocalSolrQueryRequest locReq = new LocalSolrQueryRequest(core, params);
       locReq.setContentStreams(streams);
       core.execute(handler, locReq, rsp);
 
+      assertTrue("Schema should have caused core reload to fail!",
+          rsp.getException().getMessage().indexOf("SAXParseException") != -1);
       String contents = FileUtils.readFileToString(new File(core.getCoreDescriptor().getInstanceDir(), "conf/schema.xml"));
-      assertEquals("Schema contents should have changed!", "Testing rewrite of schema.xml file.", contents);
+      assertFalse("Schema contents should NOT have changed!", contents.contains("Testing rewrite of schema.xml file."));
 
       streams.add(new ContentStreamBase.StringStream("This should barf"));
       locReq = new LocalSolrQueryRequest(core, params);

solr/example/solr/collection1/conf/schema.xml

@@ -63,7 +63,7 @@
             (int, float, boolean, string...)
      -->
 
- <fields>
+<fields>
    <!-- Valid attributes for fields:
      name: mandatory - the name for the field
      type: mandatory - the name of a field type from the