Merge pull request #1477 from marklogic/feature/compression-chunking-test

Improved test for HTTP chunking

Author: rjrudin

Jenkinsfile

@@ -83,7 +83,6 @@ pipeline{
           export PATH=$GRADLE_USER_HOME:$JAVA_HOME/bin:$PATH
           cd java-client-api
           ./gradlew marklogic-client-api:test  || true
-          ./gradlew marklogic-client-api-functionaltests:testFastFunctionalTests || true
         '''
         sh label:'ml development tool test', script: '''#!/bin/bash
           export JAVA_HOME=$JAVA_HOME_DIR

marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java

@@ -3986,11 +3986,7 @@ public RESTServiceResultIterator postMultipartForm(
       requestBldr = setupRequest(requestBldr, multiBuilder, null);
       requestBldr = addTransactionScopedCookies(requestBldr, transaction);
       requestBldr = addTelemetryAgentId(requestBldr);
-
-      if ("rows".equals(path)) {
-          requestBldr.addHeader("ML-Check-ML11-Headers", "true");
-          requestBldr.addHeader("TE", "trailers");
-      }
+      requestBldr = addTrailerHeadersIfNecessary(requestBldr, path);
 
       Function<Request.Builder, Response> doPostFunction = funcBuilder ->
               doPost(reqlog, funcBuilder.header(HEADER_ACCEPT, multipartMixedWithBoundary()), multiBuilder.build());
@@ -4053,11 +4049,7 @@ private <U extends OkHttpResultIterator> U postIteratedResourceImpl(
     requestBldr = setupRequest(requestBldr, inputMimetype, null);
     requestBldr = addTransactionScopedCookies(requestBldr, transaction);
     requestBldr = addTelemetryAgentId(requestBldr);
-
-    if ("rows".equals(path)) {
-        requestBldr.addHeader("ML-Check-ML11-Headers", "true");
-        requestBldr.addHeader("TE", "trailers");
-    }
+    requestBldr = addTrailerHeadersIfNecessary(requestBldr, path);
 
     Consumer<Boolean> resendableConsumer = new Consumer<Boolean>() {
       public void accept(Boolean resendable) {
@@ -4394,6 +4386,17 @@ private Request.Builder addTelemetryAgentId(Request.Builder requestBldr) {
     return requestBldr.header("ML-Agent-ID", "java");
   }
 
+    private Request.Builder addTrailerHeadersIfNecessary(Request.Builder requestBldr, String path) {
+        if ("rows".equals(path)) {
+            // Standard header supported by ML 11
+            requestBldr.addHeader("TE", "trailers");
+
+            // Proprietary header recognized by ML 10.0-9, per https://docs.marklogic.com/guide/relnotes/chap3#id_73268
+            requestBldr.addHeader("ML-Check-ML11-Headers", "true");
+        }
+        return requestBldr;
+    }
+
   private <W extends AbstractWriteHandle> boolean addParts(
     MultipartBody.Builder multiPart, RequestLogger reqlog, W[] input)
   {
@@ -4594,22 +4597,27 @@ private <U extends OkHttpResultIterator> U makeResults(
     }
     List<BodyPart> partList = getPartList(entity);
 
+    String mlErrorCode = null;
+    String mlErrorMessage = null;
     try {
-        Headers trailer = response.trailers();
-        String code = trailer.get("ml-error-code");
-        String msg = trailer.get("ml-error-message");
-        String sha = trailer.get("ml-content-sha256");
-
-        if (code != null && !"N/A".equals(code)) {
-            FailedRequest failure = new FailedRequest();
-            failure.setMessageString(code);
-            failure.setStatusString(msg);
-            throw new FailedRequestException("failed to " + operation + " "
-                    + entityType + " at rows" + ": " + code + ", " + msg);
-        }
+        Headers trailers = response.trailers();
+        mlErrorCode = trailers.get("ml-error-code");
+        mlErrorMessage = trailers.get("ml-error-message");
     } catch (IOException e) {
-        throw new RuntimeException("No trailer header in repsonse");
+        // This does not seem worthy of causing the entire operation to fail; we also don't expect this to occur, as it
+        // should only occur due to a programming error where the response body has already been consumed
+        logger.warn("Unexpected IO error while getting HTTP response trailers: " + e.getMessage());
+    }
+
+    if (mlErrorCode != null && !"N/A".equals(mlErrorCode)) {
+      FailedRequest failure = new FailedRequest();
+      failure.setMessageString(mlErrorCode);
+      failure.setStatusString(mlErrorMessage);
+      failure.setStatusCode(500);
+      throw new FailedRequestException("failed to " + operation + " "
+          + entityType + " at rows" + ": " + mlErrorCode + ", " + mlErrorMessage, failure);
     }
+
     Closeable closeable = response;
     return makeResults(constructor, reqlog, operation, entityType, partList, response, closeable);
   }

marklogic-client-api/src/test/java/com/marklogic/client/test/rows/RowManagerTest.java

@@ -15,38 +15,23 @@
  */
 package com.marklogic.client.test.rows;
 
-import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import java.io.IOException;
-import java.io.LineNumberReader;
-import java.io.StringWriter;
-import java.util.*;
-import java.util.stream.Collectors;
-
-import javax.xml.namespace.QName;
-import javax.xml.transform.OutputKeys;
-import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerConfigurationException;
-import javax.xml.transform.TransformerException;
-import javax.xml.transform.TransformerFactory;
-import javax.xml.transform.TransformerFactoryConfigurationError;
-import javax.xml.transform.dom.DOMSource;
-import javax.xml.transform.stream.StreamResult;
-import javax.xml.xpath.XPathExpressionException;
-
-import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.marklogic.client.FailedRequestException;
 import com.marklogic.client.datamovement.DataMovementManager;
 import com.marklogic.client.datamovement.WriteBatcher;
+import com.marklogic.client.document.DocumentManager;
+import com.marklogic.client.expression.PlanBuilder;
 import com.marklogic.client.io.*;
 import com.marklogic.client.query.DeleteQueryDefinition;
 import com.marklogic.client.query.QueryManager;
 import com.marklogic.client.row.*;
+import com.marklogic.client.row.RowManager.RowSetPart;
+import com.marklogic.client.row.RowManager.RowStructure;
+import com.marklogic.client.row.RowRecord.ColumnKind;
+import com.marklogic.client.test.Common;
 import com.marklogic.client.type.*;
+import com.marklogic.client.util.EditableNamespaceContext;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Ignore;
@@ -57,15 +42,19 @@
 import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.marklogic.client.document.DocumentManager;
-import com.marklogic.client.expression.PlanBuilder;
-import com.marklogic.client.row.RowManager.RowSetPart;
-import com.marklogic.client.row.RowManager.RowStructure;
-import com.marklogic.client.row.RowRecord.ColumnKind;
-import com.marklogic.client.test.Common;
-import com.marklogic.client.util.EditableNamespaceContext;
+import javax.xml.namespace.QName;
+import javax.xml.transform.*;
+import javax.xml.transform.dom.DOMSource;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.xpath.XPathExpressionException;
+import java.io.IOException;
+import java.io.LineNumberReader;
+import java.io.StringWriter;
+import java.util.*;
+import java.util.stream.Collectors;
+
+import static org.custommonkey.xmlunit.XMLAssert.assertXMLEqual;
+import static org.junit.Assert.*;
 
 public class RowManagerTest {
   private static String[]             uris           = null;
@@ -528,26 +517,42 @@ public void testSQL0Result() {
   }
 
   @Test
-  public void testSQLException() {
+  public void testErrorWhileStreamingRows() {
     if (!Common.markLogicIsVersion11OrHigher()) {
       return;
     }
 
-    RowManager rowMgr = Common.client.newRowManager();
-    PlanBuilder p = rowMgr.newPlanBuilder();
-    PlanBuilder.ExportablePlan builtPlan =
-        p.fromSql("select case when lastName = 'Davis' then fn_error(fn_qname('', 'SQL-TABLENOTFOUND'), 'Internal Server Error') end, opticUnitTest.musician.* from (select * from opticUnitTest.musician order by lastName)");
-    String exception = "";
-    int rowNum = 0;
-    try {
-      for (RowRecord row: rowMgr.resultRows(builtPlan)) {
-        rowNum++;
-      }
-    } catch (Exception e) {
-      exception = e.toString();
-    }
-    assertEquals(0, rowNum);
-    assertEquals("com.marklogic.client.FailedRequestException: failed to apply resource at rows: SQL-TABLENOTFOUND, Internal Server Error", exception);
+    final String validQueryThatEventuallyThrowsAnError = "select case " +
+        "when lastName = 'Davis' then fn_error(fn_qname('', 'SQL-TABLENOTFOUND'), 'Internal Server Error') end, " +
+        "opticUnitTest.musician.* from (select * from opticUnitTest.musician order by lastName)";
+
+    RowManager rowManager = Common.client.newRowManager();
+    PlanBuilder.ModifyPlan plan = rowManager.newPlanBuilder().fromSql(validQueryThatEventuallyThrowsAnError);
+
+    FailedRequestException ex = assertThrows(
+        "The SQL query is designed to not immediately fail - it will immediately return a 200 status code to the " +
+            "Java Client because the query itself can be executed - but will fail later as it streams rows; " +
+            "specifically, it will fail on the fourth row, which is the 'Davis' row. " +
+            "If chunking is configured correctly for the /v1/rows requests - i.e. if the " +
+            "'TE' header is present - then ML should return trailers in the HTTP response named 'ml-error-code' and " +
+            "'ml-error-message'. Those are intended to indicate that while a 200 was returned, an error occurred later " +
+            "while streaming data back. The Java Client is then expected to detect those trailers and throw a " +
+            "FailedRequestException. If the Java Client does not do that, then no exception will be thrown and this " +
+            "assertion will fail.", FailedRequestException.class, () -> rowManager.resultRows(plan));
+
+    assertEquals("A 500 is expected, even though ML immediately returned a 200 before it started streaming any data " +
+        "back; a 500 is used instead of a 400 here as we don't have a reliable way of knowing if the error " +
+        "occurred due to a bad request by the user, since the query was valid in the sense that it could be " +
+        "executed", 500, ex.getServerStatusCode());
+
+    assertEquals("The server error message is expected to be the value of the 'ml-error-message' trailer",
+        "SQL-TABLENOTFOUND", ex.getServerMessage());
+
+    assertEquals(
+        "The exception message is expected to be a formatted message containing the values of the 'ml-error-code' and " +
+            "'ml-error-message' trailers",
+        "Local message: failed to apply resource at rows: SQL-TABLENOTFOUND, Internal Server Error. Server Message: SQL-TABLENOTFOUND",
+        ex.getMessage());
   }
 
   @Test