Repository: OpenRefine/OpenRefine (java)

Issue:
JSON parse errors not reported to the user
As reported in #3243, JSON parsing errors, at least during the original preview parse, are being silently ignored. They should be reported to the user.

### To Reproduce
Steps to reproduce the behavior:
1. First, do ...
2. Then, ...
3. Finally, ...



### Current Results
Invalid JSON data is silently ignored.

### Expected Behavior
Errors returned by the JSON parser should be reported to the user.



Pull request description:
Add error reporting to JSON preview parse. Fixes #6454
Fixes #6454

Changes proposed in this pull request:
- Add error reporting to JSON preview parse. 


Patch:
diff --git a/main/src/com/google/refine/importers/JsonImporter.java b/main/src/com/google/refine/importers/JsonImporter.java
index 1d8505dd9..ce8e0367e 100644
--- a/main/src/com/google/refine/importers/JsonImporter.java
+++ b/main/src/com/google/refine/importers/JsonImporter.java
@@ -44,6 +44,7 @@ import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.JsonParser.NumberType;
 import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.core.StreamReadFeature;
 import com.fasterxml.jackson.core.json.JsonReadFeature;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
@@ -92,7 +93,7 @@ public class JsonImporter extends TreeImportingParserBase {
                 ObjectNode firstFileRecord = fileRecords.get(0);
                 File file = ImportingUtilities.getFile(job, firstFileRecord);
                 JsonFactory factory = JsonFactory.builder().enable(JsonReadFeature.ALLOW_JAVA_COMMENTS)
-                        .enable(JsonReadFeature.ALLOW_YAML_COMMENTS).build();
+                        .enable(JsonReadFeature.ALLOW_YAML_COMMENTS).enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION).build();
                 JsonParser parser = factory.createParser(file);
                 parser.enable(JsonParser.Feature.ALLOW_UNQUOTED_CONTROL_CHARS);
 
@@ -103,6 +104,7 @@ public class JsonImporter extends TreeImportingParserBase {
                 }
             } catch (IOException e) {
                 logger.error("Error generating parser UI initialization data for JSON file", e);
+                JSONUtilities.safePut(options, "error", e.toString());
             }
         }
 
@@ -141,64 +143,52 @@ public class JsonImporter extends TreeImportingParserBase {
         return null;
     }
 
-    final static private JsonNode parseForPreview(JsonParser parser, PreviewParsingState state) {
-        try {
-            JsonToken token = parser.nextToken();
-            state.tokenCount++;
-            return parseForPreview(parser, state, token);
-        } catch (IOException e) {
-            return null;
-        }
+    final static private JsonNode parseForPreview(JsonParser parser, PreviewParsingState state) throws IOException {
+        JsonToken token = parser.nextToken();
+        state.tokenCount++;
+        return parseForPreview(parser, state, token);
     }
 
-    final static private ObjectNode parseObjectForPreview(JsonParser parser, PreviewParsingState state) {
+    final static private ObjectNode parseObjectForPreview(JsonParser parser, PreviewParsingState state) throws IOException {
         ObjectNode result = ParsingUtilities.mapper.createObjectNode();
         loop: while (state.tokenCount < PREVIEW_PARSING_LIMIT) {
-            try {
-                JsonToken token = parser.nextToken();
-                if (token == null) {
-                    break;
-                }
-                state.tokenCount++;
-
-                switch (token) {
-                    case FIELD_NAME:
-                        String fieldName = parser.getText();
-                        JsonNode fieldValue = parseForPreview(parser, state);
-                        JSONUtilities.safePut(result, fieldName, fieldValue);
-                        break;
-                    case END_OBJECT:
-                        break loop;
-                    default:
-                        break loop;
-                }
-            } catch (IOException e) {
+            JsonToken token = parser.nextToken();
+            if (token == null) {
                 break;
             }
+            state.tokenCount++;
+
+            switch (token) {
+                case FIELD_NAME:
+                    String fieldName = parser.getText();
+                    JsonNode fieldValue = parseForPreview(parser, state);
+                    JSONUtilities.safePut(result, fieldName, fieldValue);
+                    break;
+                case END_OBJECT:
+                    break loop;
+                default:
+                    break loop;
+            }
         }
         return result;
     }
 
-    final static private ArrayNode parseArrayForPreview(JsonParser parser, PreviewParsingState state) {
+    final static private ArrayNode parseArrayForPreview(JsonParser parser, PreviewParsingState state) throws IOException {
         ArrayNode result = ParsingUtilities.mapper.createArrayNode();
         loop: while (state.tokenCount < PREVIEW_PARSING_LIMIT) {
-            try {
-                JsonToken token = parser.nextToken();
-                if (token == null) {
-                    break;
-                }
-                state.tokenCount++;
-
-                switch (token) {
-                    case END_ARRAY:
-                        break loop;
-                    default:
-                        JsonNode element = parseForPreview(parser, state, token);
-                        result.add(element);
-                }
-            } catch (IOException e) {
+            JsonToken token = parser.nextToken();
+            if (token == null) {
                 break;
             }
+            state.tokenCount++;
+
+            switch (token) {
+                case END_ARRAY:
+                    break loop;
+                default:
+                    JsonNode element = parseForPreview(parser, state, token);
+                    result.add(element);
+            }
         }
         return result;
     }
diff --git a/main/webapp/modules/core/langs/translation-he.json b/main/webapp/modules/core/langs/translation-he.json
index db9a2b761..0688e7ef0 100644
--- a/main/webapp/modules/core/langs/translation-he.json
+++ b/main/webapp/modules/core/langs/translation-he.json
@@ -290,7 +290,7 @@
     "core-project/odf": "גיליון נתונים ODF",
     "core-project/custom-tabular": "מייצא טבלאי מותאם אישית…",
     "core-project/templating": "נוצרת תבנית…",
-    "core-project/json-invalid": "ה־JSON שהדבקת אינו תקני",
+    "core-project/json-invalid": "תבנית JSON שגויה: $1",
     "core-project/undo-history": "היסטוריית ביטולים אינסופית",
     "core-project/mistakes": "אל תחששו לבצע טעויות. כל שינוי שתעשו יוצג כאן, ותוכלו לבטל כל שינוי בכל עת.",
     "core-project/learn-more": "מידע נוסף &laquo;",
diff --git a/main/webapp/modules/core/langs/translation-ja.json b/main/webapp/modules/core/langs/translation-ja.json
index a9c31baad..45af7226b 100644
--- a/main/webapp/modules/core/langs/translation-ja.json
+++ b/main/webapp/modules/core/langs/translation-ja.json
@@ -331,7 +331,7 @@
     "core-project/clear-selection": "全ファセットの選択をクリア",
     "core-project/open": "開く",
     "core-project/facet-filter": "ファセット/フィルタ",
-    "core-project/json-invalid": "貼り付けたJSONは無効です",
+    "core-project/json-invalid": "JSON形式としては無効: $1",
     "core-project/last-op-er": "最後の操作でエラーが発生しました",
     "core-project/apply-operation": "操作履歴を適用",
     "core-project/sql-export": "SQL…",
@@ -917,5 +917,6 @@
     "core-dialogs/edit-name": "関数名",
     "core-dialogs/action": "アクション",
     "core-views/warning-function-name": "関数名を入力してください.",
-    "core-views/warning-delete-functions": "本当にこの関数を削除しますか?"
+    "core-views/warning-delete-functions": "本当にこの関数を削除しますか?",
+    "core-index-parser/store-blank-columns": "空白カラムも保存"
 }


Test patch:
diff --git a/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java b/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java
index 43dacc72d..726e1a4ce 100644
--- a/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java
+++ b/main/tests/server/src/com/google/refine/importers/JsonImporterTests.java
@@ -33,6 +33,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 package com.google.refine.importers;
 
+import static org.testng.Assert.assertEquals;
+
 import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.IOException;
@@ -40,6 +42,7 @@ import java.io.InputStream;
 import java.io.Serializable;
 import java.lang.reflect.Method;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.LinkedList;
 import java.util.List;
@@ -109,7 +112,7 @@ public class JsonImporterTests extends ImporterTest {
         Row row = project.rows.get(0);
         Assert.assertNotNull(row);
         Assert.assertNotNull(row.getCell(1));
-        Assert.assertEquals(row.getCell(1).value, "Author 1, The");
+        assertEquals(row.getCell(1).value, "Author 1, The");
     }
 
     @Test
@@ -120,14 +123,27 @@ public class JsonImporterTests extends ImporterTest {
         Row row = project.rows.get(0);
         Assert.assertNotNull(row);
         Assert.assertNotNull(row.getCell(1));
-        Assert.assertEquals(row.getCell(1).value, "Author 1, The");
+        assertEquals(row.getCell(1).value, "Author 1, The");
     }
 
     @Test
-    public void canThrowError() {
+    public void canThrowError() throws IOException {
         String errJSON = getSampleWithError();
+        ObjectNode fileRecord = ParsingUtilities.mapper.createObjectNode();
+        JSONUtilities.safePut(fileRecord, "origin", "clipboard");
+        JSONUtilities.safePut(fileRecord, "declaredEncoding", StandardCharsets.UTF_8.toString());
+        JSONUtilities.safePut(fileRecord, "declaredMimeType", "text/json");
+        JSONUtilities.safePut(fileRecord, "format", "text/json");
+        JSONUtilities.safePut(fileRecord, "fileName", "(clipboard)");
+        File jsonFile = new File(job.getRawDataDir(), "test.json");
+        Files.write(jsonFile.toPath(), errJSON.getBytes(StandardCharsets.UTF_8));
+        JSONUtilities.safePut(fileRecord, "location", "test.json");
         ObjectNode options = SUT.createParserUIInitializationData(
-                job, new LinkedList<>(), "text/json");
+                job, List.of(fileRecord), "text/json");
+        assertEquals(options.get("error").asText(),
+                "com.fasterxml.jackson.core.JsonParseException: Unexpected character (';' (code 59)): was expecting comma to separate Object entries\n"
+                        +
+                        " at [Source: (File); line: 1, column: 11]");
         ArrayNode path = ParsingUtilities.mapper.createArrayNode();
         JSONUtilities.append(path, JsonImporter.ANONYMOUS);
         JSONUtilities.safePut(options, "recordPath", path);
@@ -137,7 +153,7 @@ public class JsonImporterTests extends ImporterTest {
 
         inputStream = new ByteArrayInputStream(errJSON.getBytes(StandardCharsets.UTF_8));
         ImportColumnGroup rootColumnGroup = new ImportColumnGroup();
-        List<Exception> exceptions = new ArrayList<Exception>();
+        List<Exception> exceptions = new ArrayList<>();
 
         SUT.parseOneFile(
                 project,
@@ -150,7 +166,7 @@ public class JsonImporterTests extends ImporterTest {
                 options,
                 exceptions);
         Assert.assertFalse(exceptions.isEmpty());
-        Assert.assertEquals("Unexpected character (';' (code 59)): was expecting comma to separate Object entries",
+        assertEquals("Unexpected character (';' (code 59)): was expecting comma to separate Object entries",
                 exceptions.get(0).getMessage());
     }
 
@@ -369,8 +385,8 @@ public class JsonImporterTests extends ImporterTest {
             }
             i++;
             if (i == 3) {
-                Assert.assertEquals(Token.Value, token);
-                Assert.assertEquals("field", parser.getFieldName());
+                assertEquals(Token.Value, token);
+                assertEquals("field", parser.getFieldName());
             }
         }
 
@@ -384,8 +400,8 @@ public class JsonImporterTests extends ImporterTest {
             }
             i++;
             if (i == 3) {
-                Assert.assertEquals(Token.StartEntity, token);
-                Assert.assertEquals(parser.getFieldName(), "field");
+                assertEquals(Token.StartEntity, token);
+                assertEquals(parser.getFieldName(), "field");
             }
         }
 
@@ -399,16 +415,16 @@ public class JsonImporterTests extends ImporterTest {
             }
             i++;
             if (i == 3) {
-                Assert.assertEquals(token, Token.StartEntity);
-                Assert.assertEquals(parser.getFieldName(), "field");
+                assertEquals(token, Token.StartEntity);
+                assertEquals(parser.getFieldName(), "field");
             }
             if (i == 4) {
-                Assert.assertEquals(token, Token.StartEntity);
-                Assert.assertEquals(parser.getFieldName(), JsonImporter.ANONYMOUS);
+                assertEquals(token, Token.StartEntity);
+                assertEquals(parser.getFieldName(), JsonImporter.ANONYMOUS);
             }
             if (i == 6) {
-                Assert.assertEquals(token, Token.StartEntity);
-                Assert.assertEquals(parser.getFieldName(), JsonImporter.ANONYMOUS);
+                assertEquals(token, Token.StartEntity);
+                assertEquals(parser.getFieldName(), JsonImporter.ANONYMOUS);
             }
         }
     }
@@ -428,9 +444,9 @@ public class JsonImporterTests extends ImporterTest {
             }
             i++;
             if (i == 3) {
-                Assert.assertEquals(Token.Value, token);
-                Assert.assertEquals("\tfield", parser.getFieldName());
-                Assert.assertEquals("\tvalue", parser.getFieldValue());
+                assertEquals(Token.Value, token);
+                assertEquals("\tfield", parser.getFieldName());
+                assertEquals("\tvalue", parser.getFieldValue());
             }
         }
     }