SOLR-1286: Fix the commit parameter always defaulting to "true" even if "false" is explicitly passed in.

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

contrib/dataimporthandler/CHANGES.txt

@@ -248,6 +248,9 @@ Bug Fixes
 
 27.SOLR-1229: Fixes for deletedPkQuery, particularly when using transformed Solr unique id's
               (Lance Norskog, Noble Paul via ehatcher)
+              
+28.SOLR-1286: Fix the commit parameter always defaulting to "true" even if "false" is explicitly passed in.
+              (Jay Hill, Noble Paul via ehatcher)
              
 
 Documentation

contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTest.java

@@ -19,6 +19,7 @@ package org.apache.solr.handler.dataimport;
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.LocalSolrQueryRequest;
 import org.apache.solr.util.AbstractSolrTestCase;
+import org.apache.solr.common.util.NamedList;
 
 import java.io.IOException;
 import java.util.HashMap;
@@ -72,6 +73,17 @@ public abstract class AbstractDataImportHandlerTest extends
             dataConfig);
     h.query("/dataimport", request);
   }
+  protected void runFullImport(String dataConfig, Map<String ,String > extraParams) throws Exception {
+    NamedList l = new NamedList();
+    l.add("command", "full-import");
+    l.add("debug", "on");
+    l.add("dataConfig", dataConfig);
+    for (Map.Entry<String, String> e : extraParams.entrySet()) {
+      l.add(e.getKey(),e.getValue());
+    }
+    LocalSolrQueryRequest request = new LocalSolrQueryRequest(h.getCore(), l);  
+    h.query("/dataimport", request);
+  }
 
   /**
    * Helper for creating a Context instance. Useful for testing Transformers

contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DataImporter.java

@@ -515,8 +515,11 @@ public class DataImporter {
         rows = Integer.parseInt((String) requestParams.get("rows"));
       if (requestParams.containsKey("clean"))
         clean = Boolean.parseBoolean((String) requestParams.get("clean"));
-      if (requestParams.containsKey("optimize"))
+      if (requestParams.containsKey("optimize")) {
         optimize = Boolean.parseBoolean((String) requestParams.get("optimize"));
+        if (optimize)
+          commit = true;
+      }
 
       Object o = requestParams.get("entity");
 

contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java

@@ -182,11 +182,11 @@ public class DocBuilder {
       // Do not commit unnecessarily if this is a delta-import and no documents were created or deleted
       if (!requestParameters.clean) {
         if (importStatistics.docCount.get() > 0 || importStatistics.deletedDocCount.get() > 0) {
-          commit(lastIndexTimeProps);
+          finish(lastIndexTimeProps);
         }
       } else {
         // Finished operation normally, commit now
-        commit(lastIndexTimeProps);
+        finish(lastIndexTimeProps);
       }
       if (document.onImportEnd != null) {
         invokeEventListener(document.onImportEnd);
@@ -199,17 +199,18 @@ public class DocBuilder {
   }
 
   @SuppressWarnings("unchecked")
-  private void commit(Properties lastIndexTimeProps) {
-    LOG.info("Full Import completed successfully");
+  private void finish(Properties lastIndexTimeProps) {
+    LOG.info("Import completed successfully");
     statusMessages.put("", "Indexing completed. Added/Updated: "
             + importStatistics.docCount + " documents. Deleted "
             + importStatistics.deletedDocCount + " documents.");
-    writer.commit(requestParameters.optimize);
-    addStatusMessage("Committed");
-    if (requestParameters.optimize)
-      addStatusMessage("Optimized");
-    if (requestParameters.commit)
-      writer.persist(lastIndexTimeProps);
+    if(requestParameters.commit) {
+      writer.commit(requestParameters.optimize);
+      addStatusMessage("Committed");
+      if (requestParameters.optimize)
+        addStatusMessage("Optimized");
+    }
+    writer.persist(lastIndexTimeProps);
   }
 
   void rollback() {

contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestSqlEntityProcessor2.java

@@ -20,6 +20,7 @@ import org.junit.Test;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 /**
  * <p>
@@ -70,6 +71,24 @@ public class TestSqlEntityProcessor2 extends AbstractDataImportHandlerTest {
     assertQ(req("desc:hello"), "//*[@numFound='1']");
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  public void testCompositePk_FullImportNoCommit() throws Exception {
+    List parentRow = new ArrayList();
+    parentRow.add(createMap("id", "10"));
+    MockDataSource.setIterator("select * from x", parentRow.iterator());
+
+    List childRow = new ArrayList();
+    childRow.add(createMap("desc", "hello"));
+
+    MockDataSource.setIterator("select * from y where y.A=10", childRow
+            .iterator());
+
+
+    super.runFullImport(dataConfig,createMap("commit","false"));
+    assertQ(req("id:10"), "//*[@numFound='0']");
+  }
+
   @Test
   @SuppressWarnings("unchecked")
   public void testCompositePk_DeltaImport() throws Exception {