You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by mh...@apache.org on 2017/10/05 21:30:30 UTC
asterixdb git commit: [ASTERIXDB-2122][API] Ensure Valid JSON on
Result Printing
Repository: asterixdb
Updated Branches:
refs/heads/master 561d960d8 -> 9ac01a25f
[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
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2057
Sonar-Qube: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Reviewed-by: Ian Maxon <im...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo
Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/9ac01a25
Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/9ac01a25
Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/9ac01a25
Branch: refs/heads/master
Commit: 9ac01a25fdeb2a8bb5f672a3218cdb6964198bd6
Parents: 561d960
Author: Murtadha Hubail <mh...@apache.org>
Authored: Thu Oct 5 20:38:33 2017 +0300
Committer: Murtadha Hubail <mh...@apache.org>
Committed: Thu Oct 5 14:29:55 2017 -0700
----------------------------------------------------------------------
.../asterix/app/result/ResultPrinter.java | 49 ++++-----
.../asterix/app/result/ResultPrinterTest.java | 107 +++++++++++++++++++
.../asterix/test/common/ResultExtractor.java | 24 +++--
3 files changed, 147 insertions(+), 33 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/9ac01a25/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/result/ResultPrinter.java
----------------------------------------------------------------------
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 class ResultPrinter {
public void print(ResultReader resultReader) throws HyracksDataException {
printPrefix();
-
- 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--;
- }
- String result = new String(frameBytes, start, length, UTF_8);
- if (wrapArray && notFirst) {
- output.out().print(", ");
+ try {
+ 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--;
+ }
+ String result = new String(frameBytes, start, length, UTF_8);
+ if (wrapArray && notFirst) {
+ output.out().print(", ");
+ }
+ notFirst = true;
+ displayRecord(result);
}
- notFirst = true;
- displayRecord(result);
+ frameBuffer.clear();
}
- frameBuffer.clear();
+ } finally {
+ printPostfix();
}
-
- printPostfix();
}
}
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/9ac01a25/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
----------------------------------------------------------------------
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..6810e19
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/result/ResultPrinterTest.java
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+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
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/9ac01a25/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/ResultExtractor.java
----------------------------------------------------------------------
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 @@ public class ResultExtractor {
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 @@ public class ResultExtractor {
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 @@ public class ResultExtractor {
}
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 @@ public class ResultExtractor {
}
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