You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hop.apache.org by ha...@apache.org on 2022/06/19 11:08:44 UTC

[hop] branch master updated: HOP-3994 EnhancedJsonOutput: Wrong JSON file produced in output

This is an automated email from the ASF dual-hosted git repository.

hansva pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hop.git


The following commit(s) were added to refs/heads/master by this push:
     new 152806bbf2 HOP-3994 EnhancedJsonOutput: Wrong JSON file produced in output
     new 0a5c5eba67 Merge pull request #1530 from sramazzina/HOP-3994
152806bbf2 is described below

commit 152806bbf2c87dd2d9e7ca23355f7e61fcdb677d
Author: sergio.ramazzina <se...@serasoft.it>
AuthorDate: Tue Jun 14 18:27:07 2022 +0200

    HOP-3994 EnhancedJsonOutput: Wrong JSON file produced in output
---
 .../jsonoutputenhanced/BaseFileOutputMeta.java     |  3 +-
 .../transforms/jsonoutputenhanced/JsonOutput.java  | 60 ++++++++----------
 .../jsonoutputenhanced/JsonOutputData.java         |  2 +
 .../jsonoutputenhanced/JsonOutputField.java        |  8 ++-
 .../jsonoutputenhanced/JsonOutputMeta.java         | 72 ++++++++++++++++------
 5 files changed, 88 insertions(+), 57 deletions(-)

diff --git a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/BaseFileOutputMeta.java b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/BaseFileOutputMeta.java
index f90935606a..4769fe755f 100644
--- a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/BaseFileOutputMeta.java
+++ b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/BaseFileOutputMeta.java
@@ -26,7 +26,8 @@ import java.text.SimpleDateFormat;
 import java.util.Date;
 
 /** A base implementation for all output file based metas. */
-public abstract class BaseFileOutputMeta<Main extends JsonOutput, Data extends JsonOutputData> extends BaseTransformMeta<Main, Data> {
+public abstract class BaseFileOutputMeta<Main extends JsonOutput, Data extends JsonOutputData>
+    extends BaseTransformMeta<Main, Data> {
 
   /** Flag: add the stepnr in the filename */
   @Injection(name = "INC_TRANSFORMNR_IN_FILENAME")
diff --git a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutput.java b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutput.java
index e0c772adcb..0823ec060e 100644
--- a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutput.java
+++ b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutput.java
@@ -81,8 +81,9 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
           || meta.getOperationType() == JsonOutputMeta.OPERATION_TYPE_BOTH) {
         // Init global json items array only if output to file is needed
         data.jsonItems = new ArrayList<>();
+        data.isWriteToFile = true;
         if (!meta.isDoNotOpenNewFileInit()) {
-          if (!openNewFile()) {
+          if (data.isWriteToFile && !openNewFile()) {
             logError(BaseMessages.getString(PKG, "JsonOutput.Error.OpenNewFile", buildFilename()));
             stopAll();
             setErrors(1);
@@ -91,10 +92,6 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
         }
       }
 
-      data.isWriteToFile =
-          meta.getOperationType() == JsonOutputMeta.OPERATION_TYPE_BOTH
-              || meta.getOperationType() == JsonOutputMeta.OPERATION_TYPE_WRITE_TO_FILE;
-
       data.realBlocName = Const.NVL(resolve(meta.getJsonBloc()), "");
       return true;
     }
@@ -111,10 +108,8 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
       if (!data.rowsAreSafe) {
         // Let's output the remaining unsafe data
         outPutRow(prevRow);
-        if (data.isWriteToFile) {
-          String value = serializeJson(data.jsonItems, true);
-          writeJsonFile(value);
-        }
+        if (data.isWriteToFile)
+          writeJsonFile();
       }
       setOutputDone();
       return false;
@@ -125,9 +120,10 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
       if (onFirstRecord(r)) return false;
     }
 
-    manageRowItems(r);
     data.rowsAreSafe = false;
 
+    manageRowItems(r);
+
     return true;
   }
 
@@ -213,8 +209,8 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
     if (meta.getSplitOutputAfter() > 0 && (data.nrRow) % meta.getSplitOutputAfter() == 0) {
       // Output the new row
       logDebug("Record Num: " + data.nrRow + " - Generating JSON chunk");
-      String value = serializeJson(data.jsonItems, true);
-      writeJsonFile(value);
+      serializeJson(data.jsonItems);
+      writeJsonFile();
       data.jsonItems = new ArrayList<>();
     }
   }
@@ -232,16 +228,15 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
   @SuppressWarnings("unchecked")
   private void outPutRow(Object[] rowData) throws HopException {
     // We can now output an object
-    String value = null;
     ObjectNode globalItemNode = null;
 
     if (data.jsonKeyGroupItems == null || data.jsonKeyGroupItems.size() == 0) return;
 
     if (data.jsonKeyGroupItems != null && data.jsonKeyGroupItems.size() > 0) {
-      value = serializeJson(data.jsonKeyGroupItems, false);
+      serializeJson(data.jsonKeyGroupItems);
     }
 
-    int jsonLength = value.length();
+    data.jsonLength = data.jsonSerialized.length();
 
     if (data.outputRowMeta != null) {
 
@@ -294,7 +289,7 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
         try {
           // TODO: Maybe there will be an opportunity for better code here without going through
           // JSON serialization here...
-          JsonNode jsonNode = mapper.readTree(value);
+          JsonNode jsonNode = mapper.readTree(data.jsonSerialized);
           if (meta.getOutputValue() != null) globalItemNode.put(meta.getOutputValue(), jsonNode);
         } catch (IOException e) {
           // TBD Exception must be properly managed
@@ -305,12 +300,12 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
 
       Object[] additionalRowFields = new Object[1];
 
-      additionalRowFields[0] = value;
+      additionalRowFields[0] = data.jsonSerialized;
       int nextFieldPos = 1;
 
       // Fill accessory fields
       if (meta.getJsonSizeFieldname() != null && meta.getJsonSizeFieldname().length() > 0) {
-        additionalRowFields[nextFieldPos] = Long.valueOf(jsonLength);
+        additionalRowFields[nextFieldPos] = Long.valueOf(data.jsonLength);
         nextFieldPos++;
       }
 
@@ -324,20 +319,15 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
     data.rowsAreSafe = true;
   }
 
-  private void writeJsonFile(String value) throws HopTransformException {
+  private void writeJsonFile() throws HopTransformException {
     // Open a file
-    if (!openNewFile()) {
+    if (data.isWriteToFile && !openNewFile()) {
       throw new HopTransformException(
           BaseMessages.getString(PKG, "JsonOutput.Error.OpenNewFile", buildFilename()));
     }
     // Write data to file
-
-    if (data.jsonItems != null && data.jsonItems.size() > 0) {
-      value = serializeJson(data.jsonItems, true);
-    }
-
     try {
-      data.writer.write(value);
+      data.writer.write(data.jsonSerialized);
     } catch (Exception e) {
       throw new HopTransformException(BaseMessages.getString(PKG, "JsonOutput.Error.Writing"), e);
     }
@@ -345,10 +335,9 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
     closeFile();
   }
 
-  private String serializeJson(List<ObjectNode> jsonItemsList, boolean outputToFile) {
+  private void serializeJson(List<ObjectNode> jsonItemsList) {
 
     ObjectNode theNode = new ObjectNode(nc);
-    String value = null;
 
     try {
       if (meta.getJsonBloc() != null && meta.getJsonBloc().length() > 0) {
@@ -362,12 +351,12 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
                         : (!meta.isUseArrayWithSingleInstance()
                             ? jsonItemsList.get(0)
                             : jsonItemsList))));
-        if (meta.isJsonPrittified() && outputToFile)
-          value = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(theNode);
-        else value = mapper.writeValueAsString(theNode);
+        if (meta.isJsonPrittified())
+          data.jsonSerialized = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(theNode);
+        else data.jsonSerialized = mapper.writeValueAsString(theNode);
       } else {
-        if (meta.isJsonPrittified() && outputToFile)
-          value =
+        if (meta.isJsonPrittified())
+          data.jsonSerialized =
               mapper
                   .writerWithDefaultPrettyPrinter()
                   .writeValueAsString(
@@ -377,7 +366,7 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
                               ? jsonItemsList.get(0)
                               : jsonItemsList)));
         else
-          value =
+          data.jsonSerialized =
               mapper.writeValueAsString(
                   (jsonItemsList.size() > 1
                       ? jsonItemsList
@@ -389,8 +378,6 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
       // TBD Exception must be properly managed
       e.printStackTrace();
     }
-
-    return value;
   }
 
   // Is the row r of the same group as previous?
@@ -515,6 +502,7 @@ public class JsonOutput extends BaseTransform<JsonOutputMeta, JsonOutputData> {
   }
 
   public boolean openNewFile() {
+
     if (data.writer != null) {
       return true;
     }
diff --git a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputData.java b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputData.java
index 296a432a44..493227acf5 100644
--- a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputData.java
+++ b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputData.java
@@ -44,6 +44,8 @@ public class JsonOutputData extends BaseTransformData implements ITransformData
   public int splitnr;
   public Writer writer;
   public boolean isWriteToFile;
+  public String jsonSerialized;
+  public long jsonLength;
 
   /** */
   public JsonOutputData() {
diff --git a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputField.java b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputField.java
index 67237ab75b..cd6cd54baf 100644
--- a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputField.java
+++ b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputField.java
@@ -98,12 +98,16 @@ public class JsonOutputField implements Cloneable {
     this.fieldName = fieldname;
   }
 
-  /** @return Returns the elementName. */
+  /**
+   * @return Returns the elementName.
+   */
   public String getElementName() {
     return elementName;
   }
 
-  /** @param elementName The elementName to set. */
+  /**
+   * @param elementName The elementName to set.
+   */
   public void setElementName(String elementName) {
     this.elementName = elementName;
   }
diff --git a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputMeta.java b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputMeta.java
index b2dfb1d137..4849382f84 100644
--- a/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputMeta.java
+++ b/plugins/transforms/json/src/main/java/org/apache/hop/pipeline/transforms/jsonoutputenhanced/JsonOutputMeta.java
@@ -136,73 +136,99 @@ public class JsonOutputMeta extends BaseFileOutputMeta<JsonOutput, JsonOutputDat
     this.doNotOpenNewFileInit = doNotOpenNewFileInit;
   }
 
-  /** @return Returns the create parent folder flag. */
+  /**
+   * @return Returns the create parent folder flag.
+   */
   public boolean isCreateParentFolder() {
     return createparentfolder;
   }
 
-  /** @param createparentfolder The create parent folder flag to set. */
+  /**
+   * @param createparentfolder The create parent folder flag to set.
+   */
   public void setCreateParentFolder(boolean createparentfolder) {
     this.createparentfolder = createparentfolder;
   }
 
-  /** @return Returns the extension. */
+  /**
+   * @return Returns the extension.
+   */
   @Override
   public String getExtension() {
     return extension;
   }
 
-  /** @param extension The extension to set. */
+  /**
+   * @param extension The extension to set.
+   */
   @Override
   public void setExtension(String extension) {
     this.extension = extension;
   }
 
-  /** @return Returns the fileAppended. */
+  /**
+   * @return Returns the fileAppended.
+   */
   public boolean isFileAppended() {
     return fileAppended;
   }
 
-  /** @param fileAppended The fileAppended to set. */
+  /**
+   * @param fileAppended The fileAppended to set.
+   */
   public void setFileAppended(boolean fileAppended) {
     this.fileAppended = fileAppended;
   }
 
-  /** @return Returns the fileName. */
+  /**
+   * @return Returns the fileName.
+   */
   @Override
   public String getFileName() {
     return fileName;
   }
 
-  /** @return Returns the timeInFilename. */
+  /**
+   * @return Returns the timeInFilename.
+   */
   @Override
   public boolean isTimeInFilename() {
     return timeInFilename;
   }
 
-  /** @return Returns the dateInFilename. */
+  /**
+   * @return Returns the dateInFilename.
+   */
   @Override
   public boolean isDateInFilename() {
     return dateInFilename;
   }
 
-  /** @param dateInFilename The dateInFilename to set. */
+  /**
+   * @param dateInFilename The dateInFilename to set.
+   */
   public void setDateInFilename(boolean dateInFilename) {
     this.dateInFilename = dateInFilename;
   }
 
-  /** @param timeInFilename The timeInFilename to set. */
+  /**
+   * @param timeInFilename The timeInFilename to set.
+   */
   public void setTimeInFilename(boolean timeInFilename) {
     this.timeInFilename = timeInFilename;
   }
 
-  /** @param fileName The fileName to set. */
+  /**
+   * @param fileName The fileName to set.
+   */
   @Override
   public void setFileName(String fileName) {
     this.fileName = fileName;
   }
 
-  /** @return Returns the Add to result filename flag. */
+  /**
+   * @return Returns the Add to result filename flag.
+   */
   public boolean addToResult() {
     return addToResult;
   }
@@ -249,12 +275,16 @@ public class JsonOutputMeta extends BaseFileOutputMeta<JsonOutput, JsonOutputDat
     this.operationType = operationType;
   }
 
-  /** @return Returns the outputFields. */
+  /**
+   * @return Returns the outputFields.
+   */
   public JsonOutputField[] getOutputFields() {
     return outputFields;
   }
 
-  /** @param outputFields The outputFields to set. */
+  /**
+   * @param outputFields The outputFields to set.
+   */
   public void setOutputFields(JsonOutputField[] outputFields) {
     this.outputFields = outputFields;
   }
@@ -304,7 +334,9 @@ public class JsonOutputMeta extends BaseFileOutputMeta<JsonOutput, JsonOutputDat
     return retval;
   }
 
-  /** @param addToResult The Add file to result to set. */
+  /**
+   * @param addToResult The Add file to result to set.
+   */
   public void setAddToResult(boolean addToResult) {
     this.addToResult = addToResult;
   }
@@ -632,12 +664,16 @@ public class JsonOutputMeta extends BaseFileOutputMeta<JsonOutput, JsonOutputDat
     this.encoding = encoding;
   }
 
-  /** @return Returns the jsonBloc. */
+  /**
+   * @return Returns the jsonBloc.
+   */
   public String getJsonBloc() {
     return jsonBloc;
   }
 
-  /** @param jsonBloc The root node to set. */
+  /**
+   * @param jsonBloc The root node to set.
+   */
   public void setJsonBloc(String jsonBloc) {
     this.jsonBloc = jsonBloc;
   }