From 5b27b08048fe269bc368b7289fbedee2c859350d Mon Sep 17 00:00:00 2001 From: Koji Sekiguchi Date: Tue, 2 Nov 2010 16:09:05 +0000 Subject: [PATCH] SOLR-2057: DataImportHandler never calls UpdateRequestProcessor.finish() git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1030098 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 + .../solr/handler/dataimport/DocBuilder.java | 5 ++ .../solr/handler/dataimport/SolrWriter.java | 9 +++ .../AbstractDataImportHandlerTestCase.java | 74 +++++++++++++++++++ .../handler/dataimport/TestDocBuilder.java | 11 +++ .../handler/dataimport/TestDocBuilder2.java | 13 ++++ .../solr/conf/contentstream-solrconfig.xml | 8 +- .../solr/conf/dataimport-solrconfig.xml | 6 ++ 8 files changed, 128 insertions(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index cab115818dd..7128cf1b0e3 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -540,6 +540,9 @@ Bug Fixes * SOLR-2190: change xpath from RSS 0.9 to 1.0 in slashdot sample. (koji) * SOLR-1962: SolrCore#initIndex should not use a mix of indexPath and newIndexPath (Mark Miller) + +* SOLR-2057: DataImportHandler never calls UpdateRequestProcessor.finish() + (Drew Farris via koji) Other Changes ---------------------- diff --git a/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java b/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java index a2eff653082..0a34df9f640 100644 --- a/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java +++ b/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/DocBuilder.java @@ -205,6 +205,11 @@ public class DocBuilder { // Finished operation normally, commit now finish(lastIndexTimeProps); } + + if (writer != null) { + writer.finish(); + } + if (document.onImportEnd != null) { invokeEventListener(document.onImportEnd); } diff --git a/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/SolrWriter.java b/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/SolrWriter.java index 3bff581cad8..04a79ec10d0 100644 --- a/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/SolrWriter.java +++ b/solr/contrib/dataimporthandler/src/main/java/org/apache/solr/handler/dataimport/SolrWriter.java @@ -125,6 +125,15 @@ public class SolrWriter { } } + void finish() { + try { + processor.finish(); + } catch (IOException e) { + throw new DataImportHandlerException(DataImportHandlerException.SEVERE, + "Unable to call finish() on UpdateRequestProcessor", e); + } + } + Properties readIndexerProperties() { Properties props = new Properties(); InputStream propInput = null; diff --git a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTestCase.java b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTestCase.java index c19588e2adc..bd4b8322939 100644 --- a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTestCase.java +++ b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/AbstractDataImportHandlerTestCase.java @@ -19,6 +19,15 @@ package org.apache.solr.handler.dataimport; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.core.SolrCore; import org.apache.solr.request.LocalSolrQueryRequest; +import org.apache.solr.request.SolrQueryRequest; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.update.AddUpdateCommand; +import org.apache.solr.update.CommitUpdateCommand; +import org.apache.solr.update.DeleteUpdateCommand; +import org.apache.solr.update.MergeIndexesCommand; +import org.apache.solr.update.RollbackUpdateCommand; +import org.apache.solr.update.processor.UpdateRequestProcessor; +import org.apache.solr.update.processor.UpdateRequestProcessorFactory; import org.apache.solr.common.util.NamedList; import org.junit.After; import org.junit.Before; @@ -270,4 +279,69 @@ public abstract class AbstractDataImportHandlerTestCase extends return delegate.replaceTokens(template); } } + + public static class TestUpdateRequestProcessorFactory extends UpdateRequestProcessorFactory { + + @Override + public UpdateRequestProcessor getInstance(SolrQueryRequest req, + SolrQueryResponse rsp, UpdateRequestProcessor next) { + return new TestUpdateRequestProcessor(next); + } + + } + + public static class TestUpdateRequestProcessor extends UpdateRequestProcessor { + + public static boolean finishCalled = false; + public static boolean processAddCalled = false; + public static boolean processCommitCalled = false; + public static boolean processDeleteCalled = false; + public static boolean mergeIndexesCalled = false; + public static boolean rollbackCalled = false; + + public static void reset() { + finishCalled = false; + processAddCalled = false; + processCommitCalled = false; + processDeleteCalled = false; + mergeIndexesCalled = false; + rollbackCalled = false; + } + + public TestUpdateRequestProcessor(UpdateRequestProcessor next) { + super(next); + reset(); + } + + public void finish() throws IOException { + finishCalled = true; + super.finish(); + } + + public void processAdd(AddUpdateCommand cmd) throws IOException { + processAddCalled = true; + super.processAdd(cmd); + } + + public void processCommit(CommitUpdateCommand cmd) throws IOException { + processCommitCalled = true; + super.processCommit(cmd); + } + + public void processDelete(DeleteUpdateCommand cmd) throws IOException { + processDeleteCalled = true; + super.processDelete(cmd); + } + + public void processMergeIndexes(MergeIndexesCommand cmd) throws IOException { + mergeIndexesCalled = true; + super.processMergeIndexes(cmd); + } + + public void processRollback(RollbackUpdateCommand cmd) throws IOException { + rollbackCalled = true; + super.processRollback(cmd); + } + + } } diff --git a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder.java b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder.java index 0fec4302509..d90f0a65d7c 100644 --- a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder.java +++ b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder.java @@ -60,6 +60,7 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { di.runCmd(rp, swi); assertEquals(Boolean.TRUE, swi.deleteAllCalled); assertEquals(Boolean.TRUE, swi.commitCalled); + assertEquals(Boolean.TRUE, swi.finishCalled); assertEquals(0, swi.docs.size()); assertEquals(1, di.getDocBuilder().importStatistics.queryCount.get()); assertEquals(0, di.getDocBuilder().importStatistics.docCount.get()); @@ -81,6 +82,7 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { di.runCmd(rp, swi); assertEquals(Boolean.FALSE, swi.deleteAllCalled); assertEquals(Boolean.FALSE, swi.commitCalled); + assertEquals(Boolean.TRUE, swi.finishCalled); assertEquals(0, swi.docs.size()); assertEquals(1, di.getDocBuilder().importStatistics.queryCount.get()); assertEquals(0, di.getDocBuilder().importStatistics.docCount.get()); @@ -104,6 +106,7 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { di.runCmd(rp, swi); assertEquals(Boolean.TRUE, swi.deleteAllCalled); assertEquals(Boolean.TRUE, swi.commitCalled); + assertEquals(Boolean.TRUE, swi.finishCalled); assertEquals(1, swi.docs.size()); assertEquals(1, di.getDocBuilder().importStatistics.queryCount.get()); assertEquals(1, di.getDocBuilder().importStatistics.docCount.get()); @@ -134,6 +137,7 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { di.runCmd(rp, swi); assertEquals(Boolean.FALSE, swi.deleteAllCalled); assertEquals(Boolean.TRUE, swi.commitCalled); + assertEquals(Boolean.TRUE, swi.finishCalled); assertEquals(1, swi.docs.size()); assertEquals(1, di.getDocBuilder().importStatistics.queryCount.get()); assertEquals(1, di.getDocBuilder().importStatistics.docCount.get()); @@ -168,6 +172,7 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { di.runCmd(rp, swi); assertEquals(Boolean.TRUE, swi.deleteAllCalled); assertEquals(Boolean.TRUE, swi.commitCalled); + assertEquals(Boolean.TRUE, swi.finishCalled); assertEquals(3, swi.docs.size()); for (int i = 0; i < l.size(); i++) { Map map = (Map) l.get(i); @@ -189,6 +194,8 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { Boolean commitCalled = Boolean.FALSE; + Boolean finishCalled = Boolean.FALSE; + public SolrWriterImpl() { super(null, "."); } @@ -208,6 +215,10 @@ public class TestDocBuilder extends AbstractDataImportHandlerTestCase { public void commit(boolean b) { commitCalled = Boolean.TRUE; } + + public void finish() { + finishCalled = Boolean.TRUE; + } } public static final String dc_singleEntity = "\n" diff --git a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder2.java b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder2.java index de6d66eb2e3..f361eb20a43 100644 --- a/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder2.java +++ b/solr/contrib/dataimporthandler/src/test/java/org/apache/solr/handler/dataimport/TestDocBuilder2.java @@ -52,6 +52,10 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { runFullImport(loadDataConfig("single-entity-data-config.xml")); assertQ(req("id:1"), "//*[@numFound='1']"); + + assertTrue("Update request processor processAdd was not called", TestUpdateRequestProcessor.processAddCalled); + assertTrue("Update request processor processCommit was not callled", TestUpdateRequestProcessor.processCommitCalled); + assertTrue("Update request processor finish was not called", TestUpdateRequestProcessor.finishCalled); } @Test @@ -66,6 +70,8 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { assertQ(req("id:1"), "//*[@numFound='1']"); assertTrue("Start event listener was not called", StartEventListener.executed); assertTrue("End event listener was not called", EndEventListener.executed); + assertTrue("Update request processor processAdd was not called", TestUpdateRequestProcessor.processAddCalled); + assertTrue("Update request processor finish was not called", TestUpdateRequestProcessor.finishCalled); } @Test @@ -200,6 +206,9 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { assertQ(req("id:2"), "//*[@numFound='0']"); assertQ(req("id:3"), "//*[@numFound='1']"); + assertTrue("Update request processor processDelete was not called", TestUpdateRequestProcessor.processDeleteCalled); + assertTrue("Update request processor finish was not called", TestUpdateRequestProcessor.finishCalled); + MockDataSource.clearCache(); rows = new ArrayList(); rows.add(createMap("id", "1", "desc", "one")); @@ -212,6 +221,10 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { assertQ(req("id:1"), "//*[@numFound='0']"); assertQ(req("id:2"), "//*[@numFound='0']"); assertQ(req("id:3"), "//*[@numFound='1']"); + + assertTrue("Update request processor processDelete was not called", TestUpdateRequestProcessor.processDeleteCalled); + assertTrue("Update request processor finish was not called", TestUpdateRequestProcessor.finishCalled); + } @Test diff --git a/solr/contrib/dataimporthandler/src/test/resources/solr/conf/contentstream-solrconfig.xml b/solr/contrib/dataimporthandler/src/test/resources/solr/conf/contentstream-solrconfig.xml index fd457c89bab..ac23bf786a8 100644 --- a/solr/contrib/dataimporthandler/src/test/resources/solr/conf/contentstream-solrconfig.xml +++ b/solr/contrib/dataimporthandler/src/test/resources/solr/conf/contentstream-solrconfig.xml @@ -385,7 +385,7 @@ org.apache.solr.handler.UpdateRequestProcessor --> - + *:* @@ -395,5 +395,11 @@ --> + + + + + + diff --git a/solr/contrib/dataimporthandler/src/test/resources/solr/conf/dataimport-solrconfig.xml b/solr/contrib/dataimporthandler/src/test/resources/solr/conf/dataimport-solrconfig.xml index 4b7054a2159..4e1d718c965 100644 --- a/solr/contrib/dataimporthandler/src/test/resources/solr/conf/dataimport-solrconfig.xml +++ b/solr/contrib/dataimporthandler/src/test/resources/solr/conf/dataimport-solrconfig.xml @@ -391,5 +391,11 @@ --> + + + + + +