You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2016/02/25 02:58:10 UTC
[50/50] [abbrv] lucene-solr git commit: SOLR-445: basic support and
tests for being tolerant of deletion failures
SOLR-445: basic support and tests for being tolerant of deletion failures
the tests that send deleteByQuerys to non shard leaders are failing because the resulting error details are null, need to re-review the DBQ logic in DUP to figure out where these failures are getting lost (see testVariousDeletesViaNoCollectionClient, testVariousDeletesViaShard1NonLeaderClient, testVariousDeletesViaShard2NonLeaderClient)
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/0f057192
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/0f057192
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/0f057192
Branch: refs/heads/jira/SOLR-445
Commit: 0f0571928012c071c62fb928d2142d21fa183e2b
Parents: 4ce376f
Author: Chris Hostetter <ho...@apache.org>
Authored: Wed Feb 24 18:38:24 2016 -0700
Committer: Chris Hostetter <ho...@apache.org>
Committed: Wed Feb 24 18:38:24 2016 -0700
----------------------------------------------------------------------
.../processor/TolerantUpdateProcessor.java | 69 +++---
.../cloud/TestTolerantUpdateProcessorCloud.java | 220 ++++++++++++++++---
.../processor/TolerantUpdateProcessorTest.java | 44 ++--
3 files changed, 262 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0f057192/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java
index 65b682d..68be135 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/TolerantUpdateProcessor.java
@@ -37,6 +37,7 @@ import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.schema.SchemaField;
import org.apache.solr.update.AddUpdateCommand;
+import org.apache.solr.update.DeleteUpdateCommand;
import org.apache.solr.update.SolrCmdDistributor.Error;
import org.apache.solr.update.processor.DistributedUpdateProcessor.DistribPhase;
@@ -109,12 +110,12 @@ public class TolerantUpdateProcessor extends UpdateRequestProcessor {
this.zkController = this.req.getCore().getCoreDescriptor().getCoreContainer().getZkController();
- // nocommit: assert existence of uniqueKey & record for future processAdd+processAddError calls
+ // nocommit: assert existence of uniqueKey field & record for future use
}
@Override
public void processAdd(AddUpdateCommand cmd) throws IOException {
- boolean isLeader = isLeader(cmd);
+ boolean isLeader = isLeader(cmd); // nocommit: is this needed? see below...
BytesRef id = null;
try {
@@ -127,33 +128,52 @@ public class TolerantUpdateProcessor extends UpdateRequestProcessor {
firstErrTracker.caught(t);
if (isLeader || distribPhase.equals(DistribPhase.NONE)) {
- processAddError(getPrintableId(id, cmd.getReq().getSchema().getUniqueKeyField()), t);
+ // nocommit: should we skip if condition and always do this? see comment in else...
+
+ knownErrors.add(new KnownErr(CmdType.ADD,
+ getPrintableId(id, cmd.getReq().getSchema().getUniqueKeyField()),
+ t.getMessage()));
if (knownErrors.size() > maxErrors) {
firstErrTracker.throwFirst();
}
} else {
+ // nocommit: is this if/else even relevant or important? ...
+ //
+ // 1) the factory won't even instantiate "this" if we are a replica being forwarded from our leader
+ // 2) so aren't we by definition either the leader or DistribPhase.NONE ?
+ // 3) even if we aren't, is there any downside to simplifying the code and always waiting until maxErors?
+
firstErrTracker.throwFirst();
}
}
}
-
-
- // nocommit: what about processCommit and processDelete and other UP methods? ...
- // nocommit: ...at a minimum use firstErrTracker to catch & rethrow so finish can annotate
- // nocommit: refactor this method away
- protected void processAddError(CharSequence id, Throwable error) {
- processAddError(id, error.getMessage());
+ @Override
+ public void processDelete(DeleteUpdateCommand cmd) throws IOException {
+
+ // nocommit: do we need special delById => isLeader(id) vs delByQ => isAnyLeader logic?
+
+ try {
+
+ super.processDelete(cmd);
+
+ } catch (Throwable t) { // nocommit: OOM trap
+ firstErrTracker.caught(t);
+
+ // nocommit: do we need isLeader type logic like processAdd ? does processAdd even need it?
+
+ CmdType type = cmd.isDeleteById() ? CmdType.DELID : CmdType.DELQ;
+ String id = cmd.isDeleteById() ? cmd.id : cmd.query;
+ knownErrors.add(new KnownErr(type, id, t.getMessage()));
+ if (knownErrors.size() > maxErrors) {
+ firstErrTracker.throwFirst();
+ }
+ }
}
+
- /**
- * Logs an error for the given id, and buffers it up to be
- * included in the response header
- */
- protected void processAddError(CharSequence id, CharSequence error) {
- // nocommit: need refactor the KnownErr wrapping up so this method can handle deletes & commits as well
- knownErrors.add(new KnownErr(CmdType.ADD, id.toString(), error.toString()));
- }
+ // nocommit: what about processCommit and other UpdateProcessor methods?
+ // nocommit: ...at a minimum use firstErrTracker to catch & rethrow so finish can annotate
@Override
public void finish() throws IOException {
@@ -189,17 +209,10 @@ public class TolerantUpdateProcessor extends UpdateRequestProcessor {
for (int i = 0; i < remoteErrMetadata.size(); i++) {
KnownErr err = KnownErr.parseMetadataIfKnownErr(remoteErrMetadata.getName(i),
remoteErrMetadata.getVal(i));
- if (null == err) {
- // some metadata unrelated to this update processor
- continue;
- }
-
- if (err.type.equals(CmdType.ADD)) { // nocommit: generalize this to work with any CmdType
- processAddError(err.id, err.errorValue);
- } else {
- log.error("found remote error metadata we can't handle key: " + err);
- assert false : "found remote error metadata we can't handle key: " + err;
+ if (null != err) {
+ knownErrors.add(err);
}
+ // else: some metadata unrelated to this update processor
}
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0f057192/solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java b/solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java
index 0cfcd17..e5c09ca 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestTolerantUpdateProcessorCloud.java
@@ -256,6 +256,7 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
public void testSanity() throws Exception {
// verify some basic sanity checking of indexing & querying across the collection
+ // w/o using our custom update processor chain
assertEquals(0, CLOUD_CLIENT.add(doc(f("id", S_ONE_PRE + "1"),
f("foo_i", 42))).getStatus());
@@ -267,33 +268,119 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
S_ONE_NON_LEADER_CLIENT, S_TWO_NON_LEADER_CLIENT,
NO_COLLECTION_CLIENT, CLOUD_CLIENT)) {
assertQueryDocIds(c, true, S_ONE_PRE + "1", S_TWO_PRE + "2");
- assertQueryDocIds(c, false, "42");
+ assertQueryDocIds(c, false, "id_not_exists");
+
+ // verify oportunistic concurrency deletions fail as we expect when docs are / aren't present
+ for (UpdateRequest r : new UpdateRequest[] {
+ update(params("commit", "true")).deleteById(S_ONE_PRE + "1", -1L),
+ update(params("commit", "true")).deleteById(S_TWO_PRE + "2", -1L),
+ update(params("commit", "true")).deleteById("id_not_exists", 1L) }) {
+ try {
+ UpdateResponse rsp = r.process(c);
+ fail("sanity check for oportunistic concurrency delete didn't fail: "
+ + r.toString() + " => " + rsp.toString());
+ } catch (SolrException e) {
+ assertEquals("not the expected oportunistic concurrency failure code: "
+ + r.toString() + " => " + e.getMessage(), 409, e.code());
+ }
+ }
}
+ }
+ //
+ public void testVariousDeletesViaCloudClient() throws Exception {
+ testVariousDeletes(CLOUD_CLIENT);
+ }
+ public void testVariousDeletesViaShard1LeaderClient() throws Exception {
+ testVariousDeletes(S_ONE_LEADER_CLIENT);
+ }
+ public void testVariousDeletesViaShard2LeaderClient() throws Exception {
+ testVariousDeletes(S_TWO_LEADER_CLIENT);
+ }
+ public void testVariousDeletesViaShard1NonLeaderClient() throws Exception {
+ testVariousDeletes(S_ONE_NON_LEADER_CLIENT);
+ }
+ public void testVariousDeletesViaShard2NonLeaderClient() throws Exception {
+ testVariousDeletes(S_TWO_NON_LEADER_CLIENT);
+ }
+ public void testVariousDeletesViaNoCollectionClient() throws Exception {
+ testVariousDeletes(NO_COLLECTION_CLIENT);
+ }
+
+ protected static void testVariousDeletes(SolrClient client) throws Exception {
+ assertNotNull("client not initialized", client);
+ // 2 docs, one on each shard
+ final String docId1 = S_ONE_PRE + "42";
+ final String docId2 = S_TWO_PRE + "666";
+ UpdateResponse rsp = null;
+
+ // add 1 doc to each shard
+ rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+ "commit", "true"),
+ doc(f("id", docId1), f("foo_i", "2001")),
+ doc(f("id", docId2), f("foo_i", "1976"))).process(client);
+ assertEquals(0, rsp.getStatus());
+
+ // attempt to delete individual doc id(s) that should fail because of oportunistic concurrency constraints
+ for (String id : new String[] { docId1, docId2 }) {
+ rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+ "commit", "true")).deleteById(id, -1L).process(client);
+ assertEquals(0, rsp.getStatus());
+ assertUpdateTolerantErrors("failed oportunistic concurrent delId="+id, rsp,
+ delIErr(id));
+ }
+
+ // attempt to delete multiple doc ids that should all fail because of oportunistic concurrency constraints
+ rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+ "commit", "true")).deleteById(docId2, -1L).deleteById(docId1, -1L).process(client);
+ assertEquals(0, rsp.getStatus());
+ assertUpdateTolerantErrors("failed oportunistic concurrent delete by id for 2 docs", rsp,
+ delIErr(docId1), delIErr(docId2));
+
+ // nocommit: deleteByQuery using malformed query
+ rsp = update(params("update.chain", "tolerant-chain-max-errors-10",
+ "commit", "true")).deleteByQuery("bogus_field:foo").process(client);
+ assertEquals(0, rsp.getStatus());
+ assertUpdateTolerantErrors("failed oportunistic concurrent delete by query", rsp,
+ delQErr("bogus_field:foo"));
+
+ // nocommit: mix 2 deleteByQuery, one malformed (fail) one not but doesn't match anything (ok)
+
+ // nocommit: mix 2 deleteById using _version_=-1, one for real doc1 (fail), one for bogus id (ok)
+
+ // nocommit: mix 2 deleteById using _version_=1, one for real doc1 (ok, deleted), one for bogus id (fail)
+
+ // nocommit: assertQueryDocIds doc2 only doc left
+
+ // nocommit: test multiple failed deletes from the same shard (bogus ids are fine)
+
+
}
- public void testVariousUpdatesViaCloudClient() throws Exception {
- testVariousUpdates(CLOUD_CLIENT);
+
+ //
+ public void testVariousAddsViaCloudClient() throws Exception {
+ testVariousAdds(CLOUD_CLIENT);
}
- public void testVariousUpdatesViaShard1LeaderClient() throws Exception {
- testVariousUpdates(S_ONE_LEADER_CLIENT);
+ public void testVariousAddsViaShard1LeaderClient() throws Exception {
+ testVariousAdds(S_ONE_LEADER_CLIENT);
}
- public void testVariousUpdatesViaShard2LeaderClient() throws Exception {
- testVariousUpdates(S_TWO_LEADER_CLIENT);
+ public void testVariousAddsViaShard2LeaderClient() throws Exception {
+ testVariousAdds(S_TWO_LEADER_CLIENT);
}
- public void testVariousUpdatesViaShard1NonLeaderClient() throws Exception {
- testVariousUpdates(S_ONE_NON_LEADER_CLIENT);
+ public void testVariousAddsViaShard1NonLeaderClient() throws Exception {
+ testVariousAdds(S_ONE_NON_LEADER_CLIENT);
}
- public void testVariousUpdatesViaShard2NonLeaderClient() throws Exception {
- testVariousUpdates(S_TWO_NON_LEADER_CLIENT);
+ public void testVariousAddsViaShard2NonLeaderClient() throws Exception {
+ testVariousAdds(S_TWO_NON_LEADER_CLIENT);
}
- public void testVariousUpdatesViaNoCollectionClient() throws Exception {
- testVariousUpdates(NO_COLLECTION_CLIENT);
+ public void testVariousAddsViaNoCollectionClient() throws Exception {
+ testVariousAdds(NO_COLLECTION_CLIENT);
}
- protected static void testVariousUpdates(SolrClient client) throws Exception {
+ protected static void testVariousAdds(SolrClient client) throws Exception {
assertNotNull("client not initialized", client);
UpdateResponse rsp = null;
@@ -306,7 +393,7 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
doc(f("id", S_ONE_PRE + "666"), f("foo_i", "1976"))).process(client);
assertEquals(0, rsp.getStatus());
- assertUpdateTolerantErrors("single shard, 1st doc should fail", rsp, S_ONE_PRE + "42");
+ assertUpdateTolerantAddErrors("single shard, 1st doc should fail", rsp, S_ONE_PRE + "42");
assertEquals(0, client.commit().getStatus());
assertQueryDocIds(client, false, S_ONE_PRE + "42");
assertQueryDocIds(client, true, S_ONE_PRE + "666");
@@ -319,7 +406,7 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
doc(f("id", S_ONE_PRE + "77"), f("foo_i", "bogus_val"))).process(client);
assertEquals(0, rsp.getStatus());
- assertUpdateTolerantErrors("single shard, 2nd doc should fail", rsp, S_ONE_PRE + "77");
+ assertUpdateTolerantAddErrors("single shard, 2nd doc should fail", rsp, S_ONE_PRE + "77");
assertQueryDocIds(client, false, S_ONE_PRE + "77");
assertQueryDocIds(client, true, S_ONE_PRE + "666", S_ONE_PRE + "55");
@@ -334,7 +421,7 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
doc(f("id", S_TWO_PRE + "666"), f("foo_i", "1976"))).process(client);
assertEquals(0, rsp.getStatus());
- assertUpdateTolerantErrors("two shards, 1st doc should fail", rsp, S_ONE_PRE + "42");
+ assertUpdateTolerantAddErrors("two shards, 1st doc should fail", rsp, S_ONE_PRE + "42");
assertEquals(0, client.commit().getStatus());
assertQueryDocIds(client, false, S_ONE_PRE + "42");
assertQueryDocIds(client, true, S_TWO_PRE + "666");
@@ -347,7 +434,7 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
doc(f("id", S_TWO_PRE + "77"), f("foo_i", "bogus_val"))).process(client);
assertEquals(0, rsp.getStatus());
- assertUpdateTolerantErrors("two shards, 2nd doc should fail", rsp, S_TWO_PRE + "77");
+ assertUpdateTolerantAddErrors("two shards, 2nd doc should fail", rsp, S_TWO_PRE + "77");
assertQueryDocIds(client, false, S_TWO_PRE + "77");
assertQueryDocIds(client, true, S_TWO_PRE + "666", S_ONE_PRE + "55");
@@ -372,9 +459,9 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
doc(f("id", S_TWO_PRE + "26"))).process(client);
assertEquals(0, rsp.getStatus());
- assertUpdateTolerantErrors("many docs, 1 from each shard should fail", rsp,
- S_ONE_PRE + "15",
- S_TWO_PRE + "22");
+ assertUpdateTolerantAddErrors("many docs, 1 from each shard should fail", rsp,
+ S_ONE_PRE + "15",
+ S_TWO_PRE + "22");
assertQueryDocIds(client, false, S_TWO_PRE + "22", S_ONE_PRE + "15");
assertQueryDocIds(client, true,
S_ONE_PRE + "11", S_TWO_PRE + "21", S_ONE_PRE + "12",
@@ -499,11 +586,76 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
}
- // nocommit: redesign so that we can assert errors of diff types besides "add" (ie: deletes)
+ //
+ public void testAddsMixedWithDeletesViaCloudClient() throws Exception {
+ testAddsMixedWithDeletes(CLOUD_CLIENT);
+ }
+ public void testAddsMixedWithDeletesViaShard1LeaderClient() throws Exception {
+ testAddsMixedWithDeletes(S_ONE_LEADER_CLIENT);
+ }
+ public void testAddsMixedWithDeletesViaShard2LeaderClient() throws Exception {
+ testAddsMixedWithDeletes(S_TWO_LEADER_CLIENT);
+ }
+ public void testAddsMixedWithDeletesViaShard1NonLeaderClient() throws Exception {
+ testAddsMixedWithDeletes(S_ONE_NON_LEADER_CLIENT);
+ }
+ public void testAddsMixedWithDeletesViaShard2NonLeaderClient() throws Exception {
+ testAddsMixedWithDeletes(S_TWO_NON_LEADER_CLIENT);
+ }
+ public void testAddsMixedWithDeletesViaNoCollectionClient() throws Exception {
+ testAddsMixedWithDeletes(NO_COLLECTION_CLIENT);
+ }
+
+ protected static void testAddsMixedWithDeletes(SolrClient client) throws Exception {
+ assertNotNull("client not initialized", client);
+
+ // nocommit: test adds & deletes mixed in a single UpdateRequest, w/ tolerated failures of both types
+ }
+
+ /** Asserts that the UpdateResponse contains the specified expectedErrs and no others */
public static void assertUpdateTolerantErrors(String assertionMsgPrefix,
UpdateResponse response,
- String... errorIdsExpected) {
+ ExpectedErr... expectedErrs) {
+ @SuppressWarnings("unchecked")
+ List<SimpleOrderedMap<String>> errors = (List<SimpleOrderedMap<String>>)
+ response.getResponseHeader().get("errors");
+
+ assertNotNull(assertionMsgPrefix + ": Null errors: " + response.toString(), errors);
+ assertEquals(assertionMsgPrefix + ": Num error ids: " + errors.toString(),
+ expectedErrs.length, errors.size());
+
+ for (SimpleOrderedMap<String> err : errors) {
+ String assertErrPre = assertionMsgPrefix + ": " + err.toString();
+ String id = err.get("id");
+ assertNotNull(assertErrPre + " ... null id", id);
+ String type = err.get("type");
+ assertNotNull(assertErrPre + " ... null type", type);
+
+ // inefficient scan, but good nough for the size of sets we're dealing with
+ boolean found = false;
+ for (ExpectedErr expected : expectedErrs) {
+ if (expected.type.equals(type) && expected.id.equals(id)) {
+ // nocommit: test err.get("message").contains(expected.someSubStr)
+ found = true;
+ break;
+ }
+ }
+ assertTrue(assertErrPre + " ... unexpected err, not found in: " + response.toString(), found);
+
+ }
+
+ // nocommit: retire numErrors, we've already checked errors.size()
+ assertEquals(assertionMsgPrefix + ": numErrors: " + response.toString(),
+ expectedErrs.length, response.getResponseHeader().get("numErrors"));
+
+ }
+
+ /** convinience method when the only type of errors you expect are 'add' errors */
+ // nocommit: refactor to be wrapper arround assertUpdateTolerantErrors
+ public static void assertUpdateTolerantAddErrors(String assertionMsgPrefix,
+ UpdateResponse response,
+ String... errorIdsExpected) {
@SuppressWarnings("unchecked")
List<SimpleOrderedMap<String>> errors = (List<SimpleOrderedMap<String>>)
response.getResponseHeader().get("errors");
@@ -516,7 +668,6 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
for (SimpleOrderedMap<String> err : errors) {
String assertErrPre = assertionMsgPrefix + ": " + err.toString();
- // nocommit: support other types
assertEquals(assertErrPre + " ... nocommit: this err type not handled yet",
"ADD", err.get("type"));
@@ -552,7 +703,6 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
r.add(Arrays.asList(docs));
return r;
}
-
public static SolrInputDocument doc(SolrInputField... fields) {
SolrInputDocument doc = new SolrInputDocument();
@@ -568,4 +718,24 @@ public class TestTolerantUpdateProcessorCloud extends LuceneTestCase {
return f;
}
+ /** simple helper struct */
+ public static final class ExpectedErr {
+ final String type;
+ final String id;
+ // nocommit: add errorMsgSubstring to test against message, or ignored if null
+
+ public ExpectedErr(String type, String id) {
+ this.type = type;
+ this.id = id;
+ }
+ }
+ public static ExpectedErr addErr(String id) {// nocommit: add errorMsgSubstring to test against message
+ return new ExpectedErr("ADD", id);
+ }
+ public static ExpectedErr delIErr(String id) {// nocommit: add errorMsgSubstring to test against message
+ return new ExpectedErr("DELID", id);
+ }
+ public static ExpectedErr delQErr(String id) {// nocommit: add errorMsgSubstring to test against message
+ return new ExpectedErr("DELQ", id);
+ }
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/0f057192/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
index 08da0c1..1123a0f 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/TolerantUpdateProcessorTest.java
@@ -128,7 +128,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
//expected
assertTrue(e.getMessage().contains("Document is missing mandatory uniqueKey field"));
}
- assertUSucceedsWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc}), null, 1, "(unknown)");
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc}), null, 1, "(unknown)");
//a valid doc
SolrInputDocument validDoc = doc(field("id", 1f, "1"), field("text", 1f, "the quick brown fox"));
@@ -147,7 +147,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
,"//result[@numFound='0']");
- assertUSucceedsWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc, validDoc}), null, 1, "(unknown)");
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc, validDoc}), null, 1, "(unknown)");
assertU(commit());
// verify that the good document made it in.
@@ -170,7 +170,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
assertQ(req("q","id:3")
,"//result[@numFound='0']");
- assertUSucceedsWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc, validDoc}), null, 1, "2");
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-10", Arrays.asList(new SolrInputDocument[]{invalidDoc, validDoc}), null, 1, "2");
assertU(commit());
// The valid document was indexed
@@ -187,7 +187,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
public void testMaxErrorsDefault() throws IOException {
try {
// by default the TolerantUpdateProcessor accepts all errors, so this batch should succeed with 10 errors.
- assertUSucceedsWithErrors("tolerant-chain-max-errors-not-set", docs, null, 10, badIds);
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, null, 10, badIds);
} catch(Exception e) {
fail("Shouldn't get an exception for this batch: " + e.getMessage());
}
@@ -200,7 +200,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
ModifiableSolrParams requestParams = new ModifiableSolrParams();
requestParams.add("maxErrors", "10");
// still OK
- assertUSucceedsWithErrors("tolerant-chain-max-errors-not-set", docs, requestParams, 10, badIds);
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, requestParams, 10, badIds);
assertU(commit());
assertQ(req("q","*:*")
,"//result[@numFound='10']");
@@ -212,7 +212,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
requestParams.add("maxErrors", "5");
try {
// should fail
- assertUSucceedsWithErrors("tolerant-chain-max-errors-not-set", docs, requestParams, 10, badIds);
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-not-set", docs, requestParams, 10, badIds);
fail("Expecting exception");
} catch (SolrException e) {
assertTrue(e.getMessage(),
@@ -234,7 +234,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
requestParams.add("maxErrors", "0");
try {
// should fail
- assertUSucceedsWithErrors("tolerant-chain-max-errors-10", smallBatch, requestParams, 1, "1");
+ assertAddsSucceedWithErrors("tolerant-chain-max-errors-10", smallBatch, requestParams, 1, "1");
fail("Expecting exception");
} catch (SolrException e) {
assertTrue(e.getMessage().contains("ERROR: [doc=1] Error adding field 'weight'='b' msg=For input string: \"b\""));
@@ -253,10 +253,13 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
"//int[@name='numErrors']=0"));
response = update("tolerant-chain-max-errors-10", delQ("invalidfield:1"));
- assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
- "//int[@name='numErrors']=1",
- "//lst[@name='errors']/lst[@name='invalidfield:1']",
- "//lst[@name='errors']/lst[@name='invalidfield:1']/str[@name='message']/text()='undefined field invalidfield'"));
+ assertNull(BaseTestHarness.validateXPath
+ (response,
+ "//int[@name='status']=0",
+ "//int[@name='numErrors']=1",
+ "//arr[@name='errors']/lst/str[@name='type']/text()='DELQ'",
+ "//arr[@name='errors']/lst/str[@name='id']/text()='invalidfield:1'",
+ "//arr[@name='errors']/lst/str[@name='message']/text()='undefined field invalidfield'"));
}
@Test
@@ -370,8 +373,15 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
}
}
- // nocommit: redesign so that we can assert errors of diff types besides "add" (ie: deletes)
- private void assertUSucceedsWithErrors(String chain, final Collection<SolrInputDocument> docs, SolrParams requestParams, int numErrors, String... ids) throws IOException {
+ private void assertAddsSucceedWithErrors(String chain,
+ final Collection<SolrInputDocument> docs,
+ SolrParams requestParams, int numErrors,
+ String... idsShouldFail) throws IOException {
+
+ // nocommit: retire numErrors from this method sig ... trappy
+ assertEquals("bad test, idsShouldFail.length doesn't match numErrors",
+ numErrors, idsShouldFail.length);
+
SolrQueryResponse response = add(chain, requestParams, docs);
@SuppressWarnings("unchecked")
@@ -379,17 +389,15 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
response.getResponseHeader().get("errors");
assertNotNull(errors);
- assertEquals("number of errors", ids.length, errors.size());
+ assertEquals("number of errors", idsShouldFail.length, errors.size());
// nocommit: retire numErrors, we've already checked errors.size()
assertEquals(numErrors, response.getResponseHeader().get("numErrors"));
- Set<String> addErrorIdsExpected = new HashSet<String>(Arrays.asList(ids));
+ Set<String> addErrorIdsExpected = new HashSet<String>(Arrays.asList(idsShouldFail));
for (SimpleOrderedMap<String> err : errors) {
- // nocommit: support other types
- assertEquals("nocommit: error type not handled yet",
- "ADD", err.get("type"));
+ assertEquals("this method only expects 'add' errors", "ADD", err.get("type"));
String id = err.get("id");
assertNotNull("null err id", id);