feat: xml-json-conversion plugin convert xml data from request body to json response, and vice versa #5016

mangoGoForward · 2021-09-08T09:55:32Z

What this PR does / why we need it:

Summary

Name
Attributes
How To Enable
Test Plugin
Disable Plugin

name

xml-json-conversion plugin convert xml data from request body to json response, and vice versa

Attributes

Name	Type	Requirement	Default	Valid	Description
from	string	optional	xml	["xml", "json"]	input type
to	string	optional	json	["xml", "json"]	output type

how-to-enable

Here's an example, enable this plugin on the specified route:

curl -i http://127.0.0.1:9080/apisix/admin/routes/1  -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/hello",
    "plugins": {
        "xml-json-conversion": {
            "from": "xml",
            "to": "json"
        }
    },
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "39.97.63.215:80": 1
        }
    }
}'

test-plugin

Via curl to access

curl -X GET http://127.0.0.1:9080/hello \
-H 'Content-Type: text/xml' \
--data '
<people>
  <person>
    <name>Manoel</name>
    <city>Palmas-TO</city>
  </person>
</people>'


{"people":{"person":{"name":"Manoel","city":"Palmas-TO"}}}

disable-plugin

When you want to disable the xml-json-conversion plugin, it is very simple,
you can delete the corresponding json configuration in the plugin configuration,
no need to restart the service, it will take effect immediately:

curl -i http://127.0.0.1:9080/apisix/admin/routes/1  -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
{
    "uri": "/hello",
    "upstream": {
        "type": "roundrobin",
        "nodes": {
            "39.97.63.215:80": 1
        }
    }
}'

Pre-submission checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

when eureka server return compressed data, use lua-zlib to unzip received response data

xml-json-conversion plugin convert xml data from request body to json response, and vice versa Docs: xml-json-conversion.md, include en and zh TEST: xml-json-conversion.t

mangoGoForward · 2021-09-13T01:24:53Z

@tzssangglass @tokers Can you help me review this PR? Thanks

tzssangglass · 2021-09-13T06:47:54Z

rockspec/apisix-master-0.rockspec

@@ -70,6 +70,8 @@ dependencies = {
    "ext-plugin-proto = 0.3.0",
    "casbin = 1.26.0",
    "api7-snowflake = 2.0-1",
+    "xml2lua = 1.5-2",
+    "lua-cjson = 2.1.0.6"


lua-cjson has installed? refer to:

apisix/apisix/core/json.lua

Line 17 in baf8434

local json_encode = require("cjson.safe").encode

cc @spacewander

done. lua-cjson is unneccessary

tzssangglass · 2021-09-13T06:49:37Z

apisix/plugins/xml-json-conversion.lua

+
+local _M = {
+    version = 0.1,
+    priority = 90,


in config-defalt.yaml, priority is 9, pls keep same.

tzssangglass · 2021-09-13T06:55:00Z

apisix/plugins/xml-json-conversion.lua

+    local req_body, err = core.request.get_body()
+    if err or req_body == nil or req_body == '' then
+        core.log.error("failed to read request body: ", err)
+        core.response.exit(400, {error_msg = "invalid request body: " .. err})
+    end
+
+    local from = conf.from
+    local to = conf.to
+    if from == to then
+        return req_body
+    end
+
+    local content_type = core.request.headers()["Content-Type"]
+    local _f_anon = _switch_anonymous[from]
+    if _f_anon then
+        return _f_anon(content_type, req_body, to)
+    else
+        return 400, {message = "Operation not supported"}
+    end


This code is almost the same as the following get_json(), can we extract it into a function?

done. Thanks

docs/zh/latest/plugins/xml-json-conversion.md

tzssangglass · 2021-09-13T07:11:21Z

t/plugin/xml-json-conversion.t

+                return
+            end
+
+            ngx.say(body)


should check the request body is converted to json format?

arthur-zhang · 2021-09-16T03:34:10Z

apisix/plugins/xml-json-conversion.lua

+
+local function json2xml(table_data)
+    local xmlStr = xml2lua.toXml(cjson.decode(table_data))
+    xmlStr = string.gsub(xmlStr, "%s+", "")


I think Camel-Case naming is not good here

done. Thanks

arthur-zhang · 2021-09-16T03:34:33Z

apisix/plugins/xml-json-conversion.lua

+end
+
+local function xml2json(xml_data)
+    local convertHandler = handler:new()


I think Camel-Case naming is not good here

arthur-zhang · 2021-09-16T03:36:52Z

apisix/plugins/xml-json-conversion.lua

+
+local _switch_anonymous = {
+    ["json"] = function(content_type, req_body, to)
+        if "application/json" ~= content_type then


Maybe you should use the flowing code to check header is json ,not the equal compare

if string.find(content_type, "application/json", 1, true)

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

tzssangglass · 2021-09-22T01:54:30Z

@mangoGoForward hi, pls resolve the conflicting files

arthur-zhang · 2021-09-22T02:06:36Z

What are scenes can this plugin be used?

mangoGoForward · 2021-09-22T08:26:14Z

@mangoGoForward hi, pls resolve the conflicting files

done. Already resolved.

…config.json

mangoGoForward · 2021-10-18T09:45:27Z

Could you help me to review this PR? Or give me some suggestions @tokers @arthur-zhang

spacewander · 2021-10-20T03:19:56Z

apisix/plugins/xml-json-conversion.lua

+    return {
+        {
+            methods = {"GET"},
+            uri = "/apisix/plugin/xml-json-conversion",


Could we remove the API? We don't want to expose more APIs to the public network.

spacewander · 2021-10-20T03:20:19Z

apisix/plugins/xml-json-conversion.lua

+
+local _switch_anonymous = {
+    ["json"] = function(content_type, req_body, to)
+        if string.find(content_type, "application/json", 1, true) == nil then


Better to use core.string.find

spacewander · 2021-10-20T03:21:01Z

apisix/plugins/xml-json-conversion.lua

+            default = "json"
+        }
+    },
+    additionalProperties = false,


We have already removed additionalProperties from other plugins.

spacewander · 2021-10-20T03:21:14Z

apisix/plugins/xml-json-conversion.lua

+local string  = require("string")
+local parser  = require("xml2lua").parser
+local table_to_xml  = require("xml2lua").toXml
+local json_decode   = require('cjson.safe').decode


Better to use core.json

spacewander · 2021-10-20T03:21:56Z

apisix/plugins/xml-json-conversion.lua

+
+local function json2xml(table_data)
+    local xmlStr = table_to_xml(json_decode(table_data))
+    xmlStr = string.gsub(xmlStr, "%s+", "")


Could we use re.gsub?

spacewander · 2021-10-20T03:23:08Z

apisix/plugins/xml-json-conversion.lua

+local function xml2json(xml_data)
+    local convert_handler = handler:new()
+    local parser_handler = parser(convert_handler)
+    parser_handler:parse(xml_data)


We should check if the parse fails.

spacewander · 2021-10-20T03:23:14Z

apisix/plugins/xml-json-conversion.lua

+end
+
+local function json2xml(table_data)
+    local xmlStr = table_to_xml(json_decode(table_data))


spacewander · 2021-10-20T03:24:11Z

conf/config-default.yaml

@@ -347,6 +347,7 @@ plugins:                          # plugin list (sorted by priority)
  #- log-rotate                    # priority: 100
  # <- recommend to use priority (0, 100) for your custom plugins
  - example-plugin                 # priority: 0
+  - xml-json-conversion            # priority: 9


Why "priority: 9" is after "priority: 0"? And we should avoid '(0, 100)' for builtin plugins

spacewander · 2021-10-20T03:35:23Z

t/plugin/xml-json-conversion.t

+GET /t
+--- response_body
+done
+--- no_error_log


We can check no_error_log by default like:

apisix/t/plugin/gzip.t

Line 40 in b5d72cb

if ((!defined $block->error_log) && (!defined $block->no_error_log)) {

spacewander · 2021-10-20T03:36:21Z

I am curious about the use case of this plugin. If it is just converting XML to JSON online, and echoing it back, why should we add it as a feature of API gateway? Maybe we should pass the converted body to the upstream, which could be useful.

arthur-zhang · 2021-10-20T03:37:21Z

I am curious about the use case of this plugin. If it is just converting XML to JSON online, and echoing it back, why should we add it as a feature of API gateway? Maybe we should pass the converted body to the upstream, which could be useful.

agree

mangoGoForward · 2021-10-20T04:04:26Z

I am curious about the use case of this plugin. If it is just converting XML to JSON online, and echoing it back, why should we add it as a feature of API gateway? Maybe we should pass the converted body to the upstream, which could be useful.

I think so right now, thanks for your suggestion. @spacewander @arthur-zhang

spacewander · 2021-10-21T01:09:54Z

We can introduce an "echo_back" option (default to false), so the current behavior can be persisted when echo_back is true.

mangoGoForward · 2021-10-21T01:46:39Z

We can introduce an "echo_back" option (default to false), so the current behavior can be persisted when echo_back is true.

I‘m a little bit incomprehensible, could you please provide more detailed description. And It seems that the plugin is unsuitable as a apisix plugin.

spacewander · 2021-10-21T02:21:02Z

"xml-json-conversion": {
            "from": "xml",
            "to": "json",
			"echo_back": true
        }

will give us the current behavior.

"xml-json-conversion": {
            "from": "xml",
            "to": "json"
        }

will pass the rewritten body to the upstream.

mangoGoForward · 2021-10-21T08:38:22Z

"xml-json-conversion": {
            "from": "xml",
            "to": "json",
			"echo_back": true
        }

will give us the current behavior.

"xml-json-conversion": {
            "from": "xml",
            "to": "json"
        }

will pass the rewritten body to the upstream.

I think echo_back is unneccessary, we can registry a route or consumer with this plugin if need to process, and if not need, why we should use this plugin. I'm confused that the plugin's scenes mainly, maybe close this PR is better.

xuwei added 11 commits September 1, 2021 13:46

fix:

dc2570a

when eureka server return compressed data, use lua-zlib to unzip received response data

Plugin:

45013ca

xml-json-conversion plugin convert xml data from request body to json response, and vice versa Docs: xml-json-conversion.md, include en and zh TEST: xml-json-conversion.t

code lint

991275d

remove debug log

b06487d

ngx.request => apisix.core

327f14f

show json decode error

c335957

remove metadata_schema

a6b22a7

add schema properties to let users can specify input and output type

919bfd4

add plugin docs

6e9c44d

add json to xml test cases

52ec618

delete function of control_api

b402da5

tzssangglass reviewed Sep 13, 2021

View reviewed changes

arthur-zhang reviewed Sep 16, 2021

View reviewed changes

tzssangglass mentioned this pull request Sep 17, 2021

request help: How to set the request return format as Json format #5087

Closed

mangoGoForward and others added 8 commits September 18, 2021 10:03

Update docs/zh/latest/plugins/xml-json-conversion.md

77ae422

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

Update docs/zh/latest/plugins/xml-json-conversion.md

1d7c170

Co-authored-by: tzssangglass <tzssangglass@gmail.com>

style: remove lua-cjson

58ecb03

Merge remote-tracking branch 'origin/develop' into develop

70b3cab

style: use snake case to handle

644b47d

fix: if not find, and return 400

93f2fda

refactor: use snake code style

54a7b23

style:

3e86062

Merge branch 'master' into develop

5604d29

xuwei and others added 13 commits October 12, 2021 10:48

style:

e075437

fix: add docs index to docs/en/lasted/config.json and docs/zh/lasted/…

c4eb509

…config.json

fix: reindex t/plugin/xml-json-conversion.t

fc29658

docs: update how-to-build.md (#5136)

4aaf14a

remove some test logic in discovery/eureka.lua

33a0012

Merge remote-tracking branch 'origin/develop' into develop

87de0a7

Merge branch 'master' into develop

0719010

resolve conf yaml

a1d86d1

Merge remote-tracking branch 'origin/develop' into develop

5f995dc

add "xml-json-conversion" to get plugins' name test case

e2ebbe4

add "xml-json-conversion" to get plugins' name test case

1d42f10

Merge remote-tracking branch 'upstream/master'

3845a27

Merge branch 'master' into develop

2ff966a

tzssangglass approved these changes Oct 18, 2021

View reviewed changes

tzssangglass requested review from tokers and spacewander October 19, 2021 13:39

spacewander reviewed Oct 20, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master' into develop

9fd7877

xuwei added 2 commits October 21, 2021 16:29

change cjson to core.json and string to core.string

de5fa30

Merge remote-tracking branch 'upstream/master' into develop

c191502

mangoGoForward closed this Oct 25, 2021

feat: xml-json-conversion plugin convert xml data from request body to json response, and vice versa #5016

feat: xml-json-conversion plugin convert xml data from request body to json response, and vice versa #5016

Conversation

mangoGoForward commented Sep 8, 2021 • edited Loading

What this PR does / why we need it:

Summary

name

Attributes

how-to-enable

test-plugin

disable-plugin

Pre-submission checklist:

mangoGoForward commented Sep 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tzssangglass commented Sep 22, 2021

arthur-zhang commented Sep 22, 2021

mangoGoForward commented Sep 22, 2021

mangoGoForward commented Oct 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spacewander commented Oct 20, 2021

arthur-zhang commented Oct 20, 2021

mangoGoForward commented Oct 20, 2021 • edited Loading

spacewander commented Oct 21, 2021

mangoGoForward commented Oct 21, 2021

spacewander commented Oct 21, 2021

mangoGoForward commented Oct 21, 2021

mangoGoForward commented Sep 8, 2021 •

edited

Loading

mangoGoForward commented Oct 20, 2021 •

edited

Loading