Murtadha Hubail has uploaded a new change for review.
https://asterix-gerrit.ics.uci.edu/2057
Change subject: [ASTERIXDB-2122][API] Ensure Valid JSON on Result Printing
......................................................................
[ASTERIXDB-2122][API] Ensure Valid JSON on Result Printing
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- Ensure results JSON field is closed when an exception is
encountered.
- Always return errors field if it exists in the API response.
- Add test case.
Change-Id: Ic16e9d234c5e127ea24e382c49f3c7cfa5bce9c7
---
M
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
A
asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
M
asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
3 files changed, 128 insertions(+), 32 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb
refs/changes/57/2057/1
diff --git
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
index 04ac0b3..c8d25f1 100644
---
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
+++
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
@@ -200,32 +200,33 @@
public void print(ResultReader resultReader) throws HyracksDataException {
printPrefix();
+ try {
+ final IFrameTupleAccessor fta =
resultReader.getFrameTupleAccessor();
+ final IFrame frame = new VSizeFrame(resultDisplayFrameMgr);
- final IFrameTupleAccessor fta = resultReader.getFrameTupleAccessor();
- final IFrame frame = new VSizeFrame(resultDisplayFrameMgr);
-
- while (resultReader.read(frame) > 0) {
- final ByteBuffer frameBuffer = frame.getBuffer();
- final byte[] frameBytes = frameBuffer.array();
- fta.reset(frameBuffer);
- final int last = fta.getTupleCount();
- for (int tIndex = 0; tIndex < last; tIndex++) {
- final int start = fta.getTupleStartOffset(tIndex);
- int length = fta.getTupleEndOffset(tIndex) - start;
- if (conf.fmt() == SessionConfig.OutputFormat.CSV
- && ((length > 0) && (frameBytes[start + length - 1] ==
'\n'))) {
- length--;
+ while (resultReader.read(frame) > 0) {
+ final ByteBuffer frameBuffer = frame.getBuffer();
+ final byte[] frameBytes = frameBuffer.array();
+ fta.reset(frameBuffer);
+ final int last = fta.getTupleCount();
+ for (int tIndex = 0; tIndex < last; tIndex++) {
+ final int start = fta.getTupleStartOffset(tIndex);
+ int length = fta.getTupleEndOffset(tIndex) - start;
+ if (conf.fmt() == SessionConfig.OutputFormat.CSV
+ && ((length > 0) && (frameBytes[start + length -
1] == '\n'))) {
+ length--;
+ }
+ String result = new String(frameBytes, start, length,
UTF_8);
+ if (wrapArray && notFirst) {
+ output.out().print(", ");
+ }
+ notFirst = true;
+ displayRecord(result);
}
- String result = new String(frameBytes, start, length, UTF_8);
- if (wrapArray && notFirst) {
- output.out().print(", ");
- }
- notFirst = true;
- displayRecord(result);
+ frameBuffer.clear();
}
- frameBuffer.clear();
+ } finally {
+ printPostfix();
}
-
- printPostfix();
}
}
diff --git
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
new file mode 100644
index 0000000..a733785
--- /dev/null
+++
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
@@ -0,0 +1,89 @@
+package org.apache.asterix.app.result;
+
+import static org.apache.hyracks.util.StorageUtil.StorageUnit.KILOBYTE;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.asterix.api.http.server.AbstractQueryApiServlet;
+import org.apache.asterix.api.http.server.ResultUtil;
+import org.apache.asterix.common.api.IApplicationContext;
+import org.apache.asterix.common.config.CompilerProperties;
+import org.apache.asterix.common.exceptions.AsterixException;
+import org.apache.asterix.test.common.ResultExtractor;
+import org.apache.asterix.translator.IStatementExecutor;
+import org.apache.asterix.translator.SessionConfig;
+import org.apache.asterix.translator.SessionOutput;
+import org.apache.commons.io.IOUtils;
+import org.apache.hyracks.util.StorageUtil;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+public class ResultPrinterTest {
+
+ /**
+ * Ensures that a valid JSON is returned when an exception is encountered
+ * while printing results and that the errors field is returned in the
+ * presence of results fields.
+ *
+ * @throws Exception
+ */
+ @Test
+ public void exceptionWhilePrinting() throws Exception {
+ final RuntimeException expectedException = new RuntimeException("Error
reading result");
+ final IApplicationContext appCtx =
Mockito.mock(IApplicationContext.class);
+ final CompilerProperties compilerProperties =
Mockito.mock(CompilerProperties.class);
+
Mockito.when(appCtx.getCompilerProperties()).thenReturn(compilerProperties);
+
Mockito.when(compilerProperties.getFrameSize()).thenReturn(StorageUtil.getIntSizeInBytes(32,
KILOBYTE));
+ final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ final PrintWriter out = new PrintWriter(baos, true);
+ final SessionOutput sessionOutput = createSessionOutput(out);
+ final ResultPrinter rs = new ResultPrinter(appCtx, sessionOutput, new
IStatementExecutor.Stats(), null);
+ final ResultReader resultReader = Mockito.mock(ResultReader.class);
+ // throw exception after the resultPrefix is written
+ Mockito.when(resultReader.getFrameTupleAccessor()).thenThrow(new
RuntimeException(expectedException));
+ out.print("{");
+ try {
+ rs.print(resultReader);
+ } catch (RuntimeException e) {
+ ResultUtil.printError(out, e, true);
+ printMetrics(out, 1);
+ }
+ out.print("}");
+ out.flush();
+ final String resultStr = new String(baos.toByteArray(),
StandardCharsets.UTF_8);
+ boolean exceptionThrown = false;
+ try {
+ // ensure result is valid json and error will be returned and not
results.
+ ResultExtractor.extract(IOUtils.toInputStream(resultStr,
StandardCharsets.UTF_8));
+ } catch (AsterixException e) {
+ exceptionThrown = true;
+
Assert.assertTrue(e.getMessage().contains(expectedException.getMessage()));
+ }
+ Assert.assertTrue(exceptionThrown);
+ }
+
+ private static SessionOutput createSessionOutput(PrintWriter resultWriter)
{
+ SessionOutput.ResultDecorator resultPrefix =
ResultUtil.createPreResultDecorator();
+ SessionOutput.ResultDecorator resultPostfix =
ResultUtil.createPostResultDecorator();
+ SessionOutput.ResultAppender appendHandle =
ResultUtil.createResultHandleAppender(null);
+ SessionOutput.ResultAppender appendStatus =
ResultUtil.createResultStatusAppender();
+ SessionConfig.OutputFormat format =
SessionConfig.OutputFormat.CLEAN_JSON;
+ SessionConfig sessionConfig = new SessionConfig(format);
+ sessionConfig.set(SessionConfig.FORMAT_WRAPPER_ARRAY, true);
+ sessionConfig.set(SessionConfig.FORMAT_INDENT_JSON, true);
+ sessionConfig.set(SessionConfig.FORMAT_QUOTE_RECORD,
+ format != SessionConfig.OutputFormat.CLEAN_JSON && format !=
SessionConfig.OutputFormat.LOSSLESS_JSON);
+ return new SessionOutput(sessionConfig, resultWriter, resultPrefix,
resultPostfix, appendHandle, appendStatus);
+ }
+
+ private static void printMetrics(PrintWriter pw, long errorCount) {
+ pw.print("\t\"");
+ pw.print(AbstractQueryApiServlet.ResultFields.METRICS.str());
+ pw.print("\": {\n");
+ ResultUtil.printField(pw, "errorCount", errorCount, false);
+ pw.print("\t}\n");
+ }
+}
\ No newline at end of file
diff --git
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
index 834c104..b95a283 100644
---
a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
+++
b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
@@ -81,11 +81,11 @@
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
public static InputStream extract(InputStream resultStream) throws
Exception {
- return extract(resultStream, EnumSet.of(ResultField.RESULTS,
ResultField.ERRORS));
+ return extract(resultStream, EnumSet.of(ResultField.RESULTS));
}
public static InputStream extractMetrics(InputStream resultStream) throws
Exception {
- return extract(resultStream, EnumSet.of(ResultField.METRICS,
ResultField.ERRORS));
+ return extract(resultStream, EnumSet.of(ResultField.METRICS));
}
public static String extractHandle(InputStream resultStream) throws
Exception {
@@ -110,7 +110,8 @@
final ObjectNode result = OBJECT_MAPPER.readValue(resultStr,
ObjectNode.class);
LOGGER.fine("+++++++\n" + result + "\n+++++++\n");
-
+ // if we have errors field in the results, we will always return it
+ checkForErrors(result);
final StringBuilder resultBuilder = new StringBuilder();
for (Iterator<String> fieldNameIter = result.fieldNames();
fieldNameIter.hasNext();) {
final String fieldName = fieldNameIter.next();
@@ -154,12 +155,6 @@
}
break;
- case ERRORS:
- final JsonNode errors = fieldValue.get(0).get("msg");
- if
(!result.get(ResultField.METRICS.getFieldName()).has("errorCount")) {
- throw new AsterixException("Request reported error but
not an errorCount");
- }
- throw new AsterixException(errors.asText());
case REQUEST_ID:
case METRICS:
case CLIENT_CONTEXT_ID:
@@ -174,4 +169,15 @@
}
return IOUtils.toInputStream(resultBuilder.toString(),
StandardCharsets.UTF_8);
}
+
+ private static void checkForErrors(ObjectNode result) throws
AsterixException {
+ final JsonNode errorsField =
result.get(ResultField.ERRORS.getFieldName());
+ if (errorsField != null) {
+ final JsonNode errors = errorsField.get(0).get("msg");
+ if
(!result.get(ResultField.METRICS.getFieldName()).has("errorCount")) {
+ throw new AsterixException("Request reported error but not an
errorCount");
+ }
+ throw new AsterixException(errors.asText());
+ }
+ }
}
\ No newline at end of file
--
To view, visit https://asterix-gerrit.ics.uci.edu/2057
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic16e9d234c5e127ea24e382c49f3c7cfa5bce9c7
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <[email protected]>
