Use xml2js for xmlToJson()

[dmabupt]

To resolve issue #9 & #47 , open this draft PR for tracking code changes.

Since xml2js provides an async interface, the usage of xmlToJson() has to be updated.

The output JSON structure has been changed as well -->

Old JSON:

[
  {
    "type": "cmd",
    "success": true,
    "cmd": "RTVJOBA USRLIBL(?) SYSLIBL(?)",
    "data": [
      {
        "name": "USRLIBL",
        "value": "QGPL       QTEMP      QDEVELOP   QBLDSYS    QBLDSYSR"
      },
      {
        "name": "SYSLIBL",
        "value": "QSYS       QSYS2      QHLPSYS    QUSRSYS"
      }
    ]
  },
  {
    "type": "sh",
    "data": "..."
  },
  {
    "type": "pgm",
    "success": true,
    "pgm": "QWCRSVAL",
    "lib": "QSYS",
    "data": [
      {
        "value": "1",
        "type": "10i0"
      },
      {
        "value": "44",
        "type": "10i0"
      },
      {
        "value": "0",
        "type": "36h"
      },
      {
        "value": "QCCSID",
        "type": "10A"
      },
      {
        "value": "B",
        "type": "1A"
      },
      {
        "value": "",
        "type": "1A"
      },
      {
        "value": "4",
        "type": "10i0"
      },
      {
        "value": "37",
        "type": "10i0"
      },
      {
        "value": "1077936194",
        "type": "10i0"
      },
      {
        "value": "1",
        "type": "10i0"
      },
      {
        "value": "QCCSID",
        "type": "10A"
      },
      {
        "value": "",
        "type": "[object Object]",
        "io": "both",
        "len": "rec2"
      }
    ]
  },
  {
    "type": "sql",
    "success": true,
    "stmt": "call qsys2.tcpip_info()",
    "result": [
      [
        {
          "desc": "HOSTNAME",
          "value": "UT25BP17.RCH.STGLABS.IBM.COM"
        },
        {
          "desc": "VRM",
          "value": "V7R2M0"
        },
        {
          "desc": "DBGROUP",
          "value": ""
        },
        {
          "desc": "IPTYPE",
          "value": "NONE"
        },
        {
          "desc": "IPADDR",
          "value": "UNKNOWN"
        },
        {
          "desc": "PORT",
          "value": "0"
        }
      ]
    ]
  }
]

New JSON:

[
  {
    "type": "cmd",
    "exec": "rexx",
    "error": "fast",
    "success": true,
    "cmd": "RTVJOBA USRLIBL(?) SYSLIBL(?)",
    "data": [
      {
        "name": "USRLIBL",
        "value": "QGPL       QTEMP      QDEVELOP   QBLDSYS    QBLDSYSR"
      },
      {
        "name": "SYSLIBL",
        "value": "QSYS       QSYS2      QHLPSYS    QUSRSYS"
      }
    ]
  },
  {
    "type": "sh",
    "error": "fast",
    "data": "..."
  },
  {
    "type": "pgm",
    "name": "QWCRSVAL",
    "lib": "QSYS",
    "error": "fast",
    "success": true,
    "cmd": "QSYS QWCRSVAL ",
    "data": [
      {
        "io": "out",
        "param": [
          {
            "type": "10i0",
            "value": "1"
          },
          {
            "type": "10i0",
            "value": "44"
          },
          {
            "type": "36h",
            "value": "0"
          },
          {
            "type": "10A",
            "value": "QCCSID"
          },
          {
            "type": "1A",
            "value": "B"
          },
          {
            "type": "1A"
          },
          {
            "type": "10i0",
            "value": "4"
          },
          {
            "type": "10i0",
            "value": "37"
          }
        ]
      },
      {
        "param": {
          "type": "10i0",
          "value": "1077936194"
        }
      },
      {
        "param": {
          "type": "10i0",
          "value": "1"
        }
      },
      {
        "param": {
          "type": "10A",
          "value": "QCCSID"
        }
      },
      {
        "io": "both",
        "param": {
          "type": "[object Object]",
          "io": "both",
          "len": "rec2"
        }
      }
    ]
  },
  {
    "type": "sql",
    "data": [
      {
        "command": "prepare",
        "error": "fast",
        "conn": "conn1",
        "stmt": "stmt1",
        "success": true,
        "sql": "call qsys2.tcpip_info()"
      },
      {
        "command": "execute",
        "error": "fast",
        "stmt": "stmt1",
        "success": true,
        "sql": "stmt1"
      },
      {
        "command": "fetch",
        "block": "all",
        "desc": "on",
        "stmt": "stmt1",
        "success": true,
        "sql": "stmt1",
        "result": [
          {
            "desc": "HOSTNAME",
            "value": "UT25BP17.RCH.STGLABS.IBM.COM"
          },
          {
            "desc": "VRM",
            "value": "V7R2M0"
          },
          {
            "desc": "DBGROUP"
          },
          {
            "desc": "IPTYPE",
            "value": "NONE"
          },
          {
            "desc": "IPADDR",
            "value": "UNKNOWN"
          },
          {
            "desc": "PORT",
            "value": "0"
          }
        ]
      },
      {
        "command": "free",
        "success": true,
        "sql": ""
      }
    ]
  }
]

[kadler]

I'm not sure changing Xml2Json format and how it's used (async) is such a good idea. That's a breaking change and will affect all users. Perhaps instead, we leave Xml2Json as-is and allow users to opt-in to xml2js somehow, instead?

Or do we not necessarily care about compatibility before 1.0?

[dmabupt]

I'm not sure changing Xml2Json format and how it's used (async) is such a good idea. That's a breaking change and will affect all users. Perhaps instead, we leave Xml2Json as-is and allow users to opt-in to xml2js somehow, instead?

Or do we not necessarily care about compatibility before 1.0?

Agree. It is better to keep the old xmlToJson API (mark it as deprecated?) and provide a new one (xmlToJs ?).

The old xmlToJson API breaks the structure of the original xml output, especially for the pgm and sql output. So the new API (to keep all the original data and order) can not be 100% compatible with the old one.

[dmabupt]

Now the code changes pass all test cases.
One comment -> https://github.com/IBM/nodejs-itoolkit/pull/53/files#diff-e7cfb3504922bf38f952e03a86b73ca9R146

As a fix for issue #11 , previously the BLANK item has a value of '', now it is undefined.

[kadler]

I think the best thing to do for compatibility is to have an option to use XmlToJson instead of using xml2js, just like we have for returning an error. The deprecated constructors would use XmlToJson always, but the object constructor could have an option to use XmlToJson if desired (off by default and deprecated). Then at V2 we can remove XmlToJson.

[dmabupt]

I think the best thing to do for compatibility is to have an option to use XmlToJson instead of using xml2js, just like we have for returning an error. The deprecated constructors would use XmlToJson always, but the object constructor could have an option to use XmlToJson if desired (off by default and deprecated). Then at V2 we can remove XmlToJson.

The basic functions like CommandCall, ProgramCall, SqlCall ( or the old iCmd, iPgm, iSh, iSql... ) require calling xmlToJson/xmlToJs explicitly and the usages are very different, I do not think an option is needed to distinguish them.

const results = xmlToJson(xmlOut); vs. const results = await xmlToJs(xmlOut);

As for the toolkit APIs (lib/Toolkit.js), we can check a new option in the Connection object to use xmlToJson or xmlToJs.

@kadler, So the option only affects the toolkit APIs, right?

[kadler]

Ok, so it seems that the only usages of xmlToJson within the code is usage by tests and internal implementation defails of the high level wrappers (eg. sendToDataQueue). From that point of view, we don't have any compatibility issues to deal with.

I would suggest then not exporting any xmlToJs API or similar. Let users import and use xml2js themselves (and recommend it to users). We can internally use it in place of xmlToJson, but I don't see much use for wrapping it in our own xmlToJs API.

[kadler]

The v1.0-dev branch is no more. Please re-open to master.