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/03/12 01:26:55 UTC

[14/50] [abbrv] lucene-solr git commit: SOLR-445: remove redundent numErrors from response, ensure 'errors' is always returned to client, even if empty

SOLR-445: remove redundent numErrors from response, ensure 'errors' is always returned to client, even if empty


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/92f81fb7
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/92f81fb7
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/92f81fb7

Branch: refs/heads/jira/SOLR-445
Commit: 92f81fb7a00ecfbc94a7a09b694b61ef8596374a
Parents: 50697ee
Author: Chris Hostetter <ho...@apache.org>
Authored: Wed Mar 9 10:05:06 2016 -0700
Committer: Chris Hostetter <ho...@apache.org>
Committed: Wed Mar 9 10:05:06 2016 -0700

----------------------------------------------------------------------
 .../processor/TolerantUpdateProcessor.java      | 10 ++----
 .../DistribTolerantUpdateProcessorTest.java     | 17 +++++-----
 .../processor/TolerantUpdateProcessorTest.java  | 34 ++++++++++++--------
 3 files changed, 31 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/92f81fb7/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 cbfa1e2..dc07082 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
@@ -262,14 +262,8 @@ public class TolerantUpdateProcessor extends UpdateRequestProcessor {
       }
     }
 
-    // good or bad populate the response header
-    if (0 < knownErrors.size()) { // nocommit: we should just always set errors, even if empty?
-      
-      header.add("numErrors", knownErrors.size()); // nocommit: eliminate from response, client can count
-      header.add("errors", KnownErr.formatForResponseHeader(knownErrors));
-    } else {
-      header.add("numErrors", 0); // nocommit: eliminate from response, client can count
-    }
+    header.add("errors", KnownErr.formatForResponseHeader(knownErrors));
+    // include in response so client knows what effective value was (may have been server side config)
     header.add("maxErrors", maxErrors);
 
     // annotate any error that might be thrown (or was already thrown)

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/92f81fb7/solr/core/src/test/org/apache/solr/cloud/DistribTolerantUpdateProcessorTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/DistribTolerantUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/cloud/DistribTolerantUpdateProcessorTest.java
index 2c545be..bebe642 100644
--- a/solr/core/src/test/org/apache/solr/cloud/DistribTolerantUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/DistribTolerantUpdateProcessorTest.java
@@ -113,7 +113,12 @@ public class DistribTolerantUpdateProcessorTest extends AbstractFullDistribZkTes
   private void assertUSucceedsWithErrors(String chain, SolrInputDocument[] docs,
                                          SolrParams requestParams,
                                          int numErrors,
-                                         String... ids) throws Exception {
+                                         String... idsShouldFail) throws Exception {
+    
+    // nocommit: retire numErrors from this method sig ... trappy
+    assertEquals("bad test, idsShouldFail.length doesn't match numErrors",
+                 numErrors, idsShouldFail.length);
+    
     ModifiableSolrParams newParams = new ModifiableSolrParams(requestParams);
     newParams.set("update.chain", chain);
     UpdateResponse response = indexDoc(newParams, docs);
@@ -122,17 +127,13 @@ public class DistribTolerantUpdateProcessorTest extends AbstractFullDistribZkTes
       response.getResponseHeader().get("errors");
     assertNotNull("Null errors in response: " + response.toString(), errors);
 
-    assertEquals("number of errors in response: " + response.toString(), ids.length, errors.size());
-    
-    // nocommit: retire numErrors, we've already checked errors.size()
-    assertEquals("Wrong numErrors in response: " + response.toString(),
-                 numErrors, response.getResponseHeader().get("numErrors"));
+    assertEquals("number of errors in response: " + response.toString(), idsShouldFail.length, errors.size());
     
-    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",
+      assertEquals("nocommit: error type not handled yet by this method",
                    "ADD", err.get("type"));
       
       String id = err.get("id");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/92f81fb7/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 1123a0f..61dac44 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
@@ -249,14 +249,16 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
   public void testInvalidDelete() throws XPathExpressionException, SAXException {
     ignoreException("undefined field invalidfield");
     String response = update("tolerant-chain-max-errors-10", adoc("id", "1", "text", "the quick brown fox"));
-    assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=0"));
+    assertNull(BaseTestHarness.validateXPath(response,
+                                             "//int[@name='status']=0",
+                                             "//arr[@name='errors']",
+                                             "count(//arr[@name='errors']/lst)=0"));
     
     response = update("tolerant-chain-max-errors-10", delQ("invalidfield:1"));
     assertNull(BaseTestHarness.validateXPath
                (response,
                 "//int[@name='status']=0",
-                "//int[@name='numErrors']=1",
+                "count(//arr[@name='errors']/lst)=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'"));
@@ -266,15 +268,20 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
   public void testValidDelete() throws XPathExpressionException, SAXException {
     ignoreException("undefined field invalidfield");
     String response = update("tolerant-chain-max-errors-10", adoc("id", "1", "text", "the quick brown fox"));
-    assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=0"));
+    assertNull(BaseTestHarness.validateXPath(response,
+                                             "//int[@name='status']=0",
+                                             "//arr[@name='errors']",
+                                             "count(//arr[@name='errors']/lst)=0"));
+
     assertU(commit());
     assertQ(req("q","*:*")
         ,"//result[@numFound='1']");
     
     response = update("tolerant-chain-max-errors-10", delQ("id:1"));
-    assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=0"));
+    assertNull(BaseTestHarness.validateXPath(response,
+                                             "//int[@name='status']=0",
+                                             "//arr[@name='errors']",
+                                             "count(//arr[@name='errors']/lst)=0"));
     assertU(commit());
     assertQ(req("q","*:*")
         ,"//result[@numFound='0']");
@@ -283,11 +290,13 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
   @Test
   public void testResponse() throws SAXException, XPathExpressionException, IOException {
     String response = update("tolerant-chain-max-errors-10", adoc("id", "1", "text", "the quick brown fox"));
-    assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=0"));
+    assertNull(BaseTestHarness.validateXPath(response,
+                                             "//int[@name='status']=0",
+                                             "//arr[@name='errors']",
+                                             "count(//arr[@name='errors']/lst)=0"));
     response = update("tolerant-chain-max-errors-10", adoc("text", "the quick brown fox"));
     assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=1",
+        "count(//arr[@name='errors']/lst)=1",
         "//arr[@name='errors']/lst/str[@name='id']/text()='(unknown)'",
         "//arr[@name='errors']/lst/str[@name='message']/text()='Document is missing mandatory uniqueKey field: id'"));
     
@@ -300,7 +309,7 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
     builder.append("</add>");
     response = update("tolerant-chain-max-errors-10", builder.toString());
     assertNull(BaseTestHarness.validateXPath(response, "//int[@name='status']=0",
-        "//int[@name='numErrors']=10",
+        "count(//arr[@name='errors']/lst)=10",
         "not(//arr[@name='errors']/lst/str[@name='id']/text()='0')",
         "//arr[@name='errors']/lst/str[@name='id']/text()='1'",
         "not(//arr[@name='errors']/lst/str[@name='id']/text()='2')",
@@ -391,9 +400,6 @@ public class TolerantUpdateProcessorTest extends UpdateProcessorTestBase {
 
     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(idsShouldFail));
 
     for (SimpleOrderedMap<String> err : errors) {