763: Omit wrapper "results" JSON object in HTTP API output for JSON results;
Wrap JSON output in outer [array]; consolidate output logic into ResultUtils;
use UTF-8 explicitly when converting byte[] from Hyracks to Strings

Change-Id: Idbeac3ebbf397b0ef73cc54b7d901a21ac855932
Reviewed-on: http://fulliautomatix.ics.uci.edu:8443/123
Tested-by: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Reviewed-by: Keren-Audrey Ouaknine <kereno@gmail.com>
diff --git a/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/QueryResultAPIServlet.java b/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/QueryResultAPIServlet.java
index 9c93671..6d8e43e 100644
--- a/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/QueryResultAPIServlet.java
+++ b/asterix-app/src/main/java/edu/uci/ics/asterix/api/http/servlet/QueryResultAPIServlet.java
@@ -16,7 +16,6 @@
 
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.nio.ByteBuffer;
 
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServlet;
@@ -26,6 +25,7 @@
 import org.json.JSONArray;
 import org.json.JSONObject;
 
+import edu.uci.ics.asterix.api.common.APIFramework.DisplayFormat;
 import edu.uci.ics.asterix.result.ResultReader;
 import edu.uci.ics.asterix.result.ResultUtils;
 import edu.uci.ics.hyracks.api.client.HyracksConnection;
@@ -74,18 +74,13 @@
             JSONArray handle = handleObj.getJSONArray("handle");
             JobId jobId = new JobId(handle.getLong(0));
             ResultSetId rsId = new ResultSetId(handle.getLong(1));
-            ByteBuffer buffer = ByteBuffer.allocate(ResultReader.FRAME_SIZE);
+
             ResultReader resultReader = new ResultReader(hcc, hds);
             resultReader.open(jobId, rsId);
-            buffer.clear();
-            JSONObject jsonResponse = new JSONObject();
-            JSONArray results = new JSONArray();
-            while (resultReader.read(buffer) > 0) {
-                ResultUtils.getJSONFromBuffer(buffer, resultReader.getFrameTupleAccessor(), results);
-                buffer.clear();
-            }
-            jsonResponse.put("results", results);
-            out.write(jsonResponse.toString());
+
+            // QQQ The DisplayFormat shouldn't be fixed; needs to be some way
+            // to select the output format from this API.
+            ResultUtils.displayResults(resultReader, out, DisplayFormat.TEXT);
 
         } catch (Exception e) {
             out.println(e.getMessage());
diff --git a/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java b/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java
index ccf2d35..c61c0b0 100644
--- a/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java
+++ b/asterix-app/src/main/java/edu/uci/ics/asterix/aql/translator/AqlTranslator.java
@@ -16,7 +16,6 @@
 
 import java.io.File;
 import java.io.PrintWriter;
-import java.nio.ByteBuffer;
 import java.rmi.RemoteException;
 import java.util.ArrayList;
 import java.util.Date;
@@ -2069,37 +2068,14 @@
                         hcc.waitForCompletion(jobId);
                         break;
                     case SYNC:
-                        if (pdf == DisplayFormat.HTML) {
-                            out.println("<h4>Results:</h4>");
-                            out.println("<pre>");
-                        }
-
-                        ByteBuffer buffer = ByteBuffer.allocate(ResultReader.FRAME_SIZE);
                         ResultReader resultReader = new ResultReader(hcc, hdc);
                         resultReader.open(jobId, metadataProvider.getResultSetId());
-                        buffer.clear();
 
-                        JSONArray results = new JSONArray();
-                        while (resultReader.read(buffer) > 0) {
-                            ResultUtils.getJSONFromBuffer(buffer, resultReader.getFrameTupleAccessor(), results);
-                            buffer.clear();
-                        }
+                        // In this case (the normal case), we don't use the
+                        // "response" JSONObject - just stream the results
+                        // to the "out" PrintWriter
+                        ResultUtils.displayResults(resultReader, out, pdf);
 
-                        response.put("results", results);
-                        switch (pdf) {
-                            case HTML:
-                                ResultUtils.prettyPrintHTML(out, response);
-                                break;
-                            case TEXT:
-                            case JSON:
-                                out.print(response);
-                                break;
-                        }
-                        out.flush();
-
-                        if (pdf == DisplayFormat.HTML) {
-                            out.println("</pre>");
-                        }
                         hcc.waitForCompletion(jobId);
                         break;
                     case ASYNC_DEFERRED:
diff --git a/asterix-app/src/main/java/edu/uci/ics/asterix/result/ResultUtils.java b/asterix-app/src/main/java/edu/uci/ics/asterix/result/ResultUtils.java
index 5c86254..a7c4fa0 100644
--- a/asterix-app/src/main/java/edu/uci/ics/asterix/result/ResultUtils.java
+++ b/asterix-app/src/main/java/edu/uci/ics/asterix/result/ResultUtils.java
@@ -21,6 +21,7 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.nio.ByteBuffer;
+import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -32,6 +33,7 @@
 
 import com.sun.el.parser.ParseException;
 
+import edu.uci.ics.asterix.api.common.APIFramework.DisplayFormat;
 import edu.uci.ics.asterix.api.http.servlet.APIServlet;
 import edu.uci.ics.hyracks.algebricks.common.exceptions.AlgebricksException;
 import edu.uci.ics.hyracks.api.comm.IFrameTupleAccessor;
@@ -39,6 +41,8 @@
 import edu.uci.ics.hyracks.dataflow.common.comm.util.ByteBufferInputStream;
 
 public class ResultUtils {
+    private static final Charset UTF_8 = Charset.forName("UTF-8");
+
     static Map<Character, String> HTML_ENTITIES = new HashMap<Character, String>();
 
     static {
@@ -57,27 +61,83 @@
         return s;
     }
 
-    public static void getJSONFromBuffer(ByteBuffer buffer, IFrameTupleAccessor fta, JSONArray resultRecords)
+    public static void displayResults(ResultReader resultReader, PrintWriter out, DisplayFormat pdf)
             throws HyracksDataException {
         ByteBufferInputStream bbis = new ByteBufferInputStream();
+        IFrameTupleAccessor fta = resultReader.getFrameTupleAccessor();
 
-        try {
-            fta.reset(buffer);
-            for (int tIndex = 0; tIndex < fta.getTupleCount(); tIndex++) {
-                int start = fta.getTupleStartOffset(tIndex);
-                int length = fta.getTupleEndOffset(tIndex) - start;
-                bbis.setByteBuffer(buffer, start);
-                byte[] recordBytes = new byte[length];
-                bbis.read(recordBytes, 0, length);
-                resultRecords.put(new String(recordBytes, 0, length));
-            }
-        } finally {
+        ByteBuffer buffer = ByteBuffer.allocate(ResultReader.FRAME_SIZE);
+        buffer.clear();
+
+        JSONArray adm_results = null;
+        switch (pdf) {
+        case HTML:
+            out.println("<h4>Results:</h4>");
+            out.println("<pre>");
+            break;
+        case JSON:
+            out.print("[ ");
+            break;
+        case TEXT:
+            // For now, keep the outer "results" JSON object for ADM results
+            out.print("{ \"results\": ");
+            adm_results = new JSONArray();
+            break;
+        }
+
+        while (resultReader.read(buffer) > 0) {
             try {
-                bbis.close();
-            } catch (IOException e) {
-                throw new HyracksDataException(e);
+                fta.reset(buffer);
+                int last = fta.getTupleCount();
+                String result;
+                for (int tIndex = 0; tIndex < last; tIndex++) {
+                    int start = fta.getTupleStartOffset(tIndex);
+                    int length = fta.getTupleEndOffset(tIndex) - start;
+                    bbis.setByteBuffer(buffer, start);
+                    byte[] recordBytes = new byte[length];
+                    bbis.read(recordBytes, 0, length);
+                    // Issue 796 - what if an instance from Hyracks exceeds
+                    // FRAME_SIZE?
+                    result = new String(recordBytes, 0, length, UTF_8);
+                    switch (pdf) {
+                    case JSON:
+                        if (tIndex != (last - 1)) {
+                            out.print(", ");
+                        }
+                        // fall through to next case to output results
+                    case HTML:
+                        out.print(result);
+                        break;
+                    case TEXT:
+                        adm_results.put(result);
+                        break;
+                    }
+                }
+                buffer.clear();
+            } finally {
+                try {
+                    bbis.close();
+                } catch (IOException e) {
+                    throw new HyracksDataException(e);
+                }
             }
         }
+
+        if (pdf == DisplayFormat.TEXT) {
+            out.print(adm_results);
+        }
+        out.flush();
+
+        switch (pdf) {
+        case HTML:
+            out.println("</pre>");
+            break;
+        case JSON:
+            out.println(" ]");
+            break;
+        case TEXT:
+            out.println(" }");
+        }
     }
 
     public static JSONObject getErrorResponse(int errorCode, String errorMessage, String errorSummary,
@@ -98,17 +158,6 @@
         return errorResp;
     }
 
-    public static void prettyPrintHTML(PrintWriter out, JSONObject jsonResultObj) {
-        try {
-            JSONArray resultsArray = jsonResultObj.getJSONArray("results");
-            for (int i = 0; i < resultsArray.length(); i++) {
-                out.print(resultsArray.getString(i));
-            }
-        } catch (JSONException e) {
-            // TODO(madhusudancs): Figure out what to do when JSONException occurs while building the results.
-        }
-    }
-
     public static void webUIErrorHandler(PrintWriter out, Exception e) {
         String errorTemplate = readTemplateFile("/webui/errortemplate.html", "%s\n%s\n%s");
 
diff --git a/asterix-app/src/test/resources/runtimets/results/json/int01/int01.1.json b/asterix-app/src/test/resources/runtimets/results/json/int01/int01.1.json
index 3448068..4ebe41c 100644
--- a/asterix-app/src/test/resources/runtimets/results/json/int01/int01.1.json
+++ b/asterix-app/src/test/resources/runtimets/results/json/int01/int01.1.json
@@ -1,7 +1,8 @@
-{ "orderedlist": [ { "int32":
+[ { "orderedlist": [ { "int32":
 1
 }
 , { "int32":
 2
 }
  ] }
+ ]
diff --git a/asterix-common/src/test/java/edu/uci/ics/asterix/test/aql/TestsUtils.java b/asterix-common/src/test/java/edu/uci/ics/asterix/test/aql/TestsUtils.java
index 921778f..37aa447 100644
--- a/asterix-common/src/test/java/edu/uci/ics/asterix/test/aql/TestsUtils.java
+++ b/asterix-common/src/test/java/edu/uci/ics/asterix/test/aql/TestsUtils.java
@@ -130,7 +130,10 @@
             String[] fields2 = row2.split(" ");
 
             for (int j = 0; j < fields1.length; j++) {
-                if (fields1[j].equals(fields2[j])) {
+                if (j >= fields2.length) {
+                    return false;
+                }
+                else if (fields1[j].equals(fields2[j])) {
                     continue;
                 } else if (fields1[j].indexOf('.') < 0) {
                     return false;
@@ -153,6 +156,18 @@
         return true;
     }
 
+    // For tests where you simply want the byte-for-byte output.
+    private static void writeOutputToFile(File actualFile, InputStream resultStream) throws Exception {
+        byte[] buffer = new byte[10240];
+        int len;
+        java.io.FileOutputStream out = new java.io.FileOutputStream(actualFile);
+        while ((len = resultStream.read(buffer)) != -1) {
+            out.write(buffer, 0, len);
+        }
+    }
+
+    // For tests where you expect the output to be a JSON object with a
+    // "results" key.
     private static void writeResultsToFile(File actualFile, InputStream resultStream) throws Exception {
         BufferedWriter writer = new BufferedWriter(new FileWriter(actualFile));
         try {
@@ -431,8 +446,9 @@
                                         + File.separator + "stop_and_start.sh");
                             }
                             InputStream resultStream = null;
+                            OutputFormat fmt = OutputFormat.forCompilationUnit(cUnit);
                             if (ctx.getType().equalsIgnoreCase("query"))
-                                resultStream = executeQuery(statement, OutputFormat.forCompilationUnit(cUnit));
+                                resultStream = executeQuery(statement, fmt);
                             else if (ctx.getType().equalsIgnoreCase("async"))
                                 resultStream = executeAnyAQLAsync(statement, false);
                             else if (ctx.getType().equalsIgnoreCase("asyncdefer"))
@@ -445,7 +461,13 @@
 
                             File actualResultFile = testCaseCtx.getActualResultFile(cUnit, new File(actualPath));
                             actualResultFile.getParentFile().mkdirs();
-                            TestsUtils.writeResultsToFile(actualResultFile, resultStream);
+                            // JSON results are pure JSON now, with no "results"
+                            // wrapper object
+                            if (fmt == OutputFormat.JSON) {
+                                TestsUtils.writeOutputToFile(actualResultFile, resultStream);
+                            } else {
+                                TestsUtils.writeResultsToFile(actualResultFile, resultStream);
+                            }
 
                             TestsUtils.runScriptAndCompareWithResult(testFile, new PrintWriter(System.err),
                                     expectedResultFile, actualResultFile);
diff --git a/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java b/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
index 595b565..4b66b99 100644
--- a/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
+++ b/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
@@ -53,7 +53,7 @@
             String mountPoint = devices.get(i).getPath().getPath();
             File mountPointDir = new File(mountPoint);
             if (!mountPointDir.exists()) {
-                throw new HyracksDataException(mountPointDir.getAbsolutePath() + "doesn't exist.");
+                throw new HyracksDataException(mountPointDir.getAbsolutePath() + " doesn't exist.");
             }
             if (!mountPoint.endsWith(System.getProperty("file.separator"))) {
                 mountPoints[i] = new String(mountPoint + System.getProperty("file.separator"));