agile6v commented on a change in pull request #1632: URL: https://github.com/apache/incubator-apisix/pull/1632#discussion_r436804879
########## File path: apisix/plugins/echo-plugin.lua ########## @@ -0,0 +1,60 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local core = require("apisix.core") +local balancer = require("ngx.balancer") +local ngx = ngx + +local schema = { + type = "object", + properties = { + before_body = {type = "string"}, + body = {type = "string"}, + after_body = {type = "string"} + }, + required = {"before_body", "body", "after_body"} +} + +local plugin_name = "echo-plugin" Review comment: How about change `echo-plugin` to `echo` ? ########## File path: apisix/plugins/echo-plugin.lua ########## @@ -0,0 +1,60 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local core = require("apisix.core") +local balancer = require("ngx.balancer") +local ngx = ngx + +local schema = { + type = "object", + properties = { + before_body = {type = "string"}, + body = {type = "string"}, + after_body = {type = "string"} + }, + required = {"before_body", "body", "after_body"} +} + +local plugin_name = "echo-plugin" + +local _M = { + version = 0.1, + priority = 412, + name = plugin_name, + schema = schema, +} + + +function _M.check_schema(conf) + return core.schema.check(schema, conf) +end + +function _M.body_filter(conf, ctx) + if(conf.body) then Review comment: `()` is not required. ########## File path: t/debug/debug-mode.t ########## @@ -76,6 +76,7 @@ loaded plugin and sort by priority: 900 name: redirect loaded plugin and sort by priority: 899 name: response-rewrite loaded plugin and sort by priority: 506 name: grpc-transcode loaded plugin and sort by priority: 500 name: prometheus +loaded plugin and sort by priority: 412 name: kafka-logger Review comment: `kafka-logger` should be `echo` plugin ########## File path: apisix/plugins/echo-plugin.lua ########## @@ -0,0 +1,60 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local core = require("apisix.core") +local balancer = require("ngx.balancer") +local ngx = ngx + +local schema = { + type = "object", + properties = { + before_body = {type = "string"}, + body = {type = "string"}, + after_body = {type = "string"} + }, + required = {"before_body", "body", "after_body"} Review comment: One of them is required. ########## File path: apisix/plugins/echo-plugin.lua ########## @@ -0,0 +1,60 @@ +-- +-- Licensed to the Apache Software Foundation (ASF) under one or more +-- contributor license agreements. See the NOTICE file distributed with +-- this work for additional information regarding copyright ownership. +-- The ASF licenses this file to You under the Apache License, Version 2.0 +-- (the "License"); you may not use this file except in compliance with +-- the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, software +-- distributed under the License is distributed on an "AS IS" BASIS, +-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +-- See the License for the specific language governing permissions and +-- limitations under the License. +-- +local core = require("apisix.core") +local balancer = require("ngx.balancer") +local ngx = ngx + +local schema = { + type = "object", + properties = { + before_body = {type = "string"}, + body = {type = "string"}, + after_body = {type = "string"} + }, + required = {"before_body", "body", "after_body"} +} + +local plugin_name = "echo-plugin" + +local _M = { + version = 0.1, + priority = 412, + name = plugin_name, + schema = schema, +} + + +function _M.check_schema(conf) + return core.schema.check(schema, conf) +end + +function _M.body_filter(conf, ctx) + if(conf.body) then + ngx.arg[1] = conf.body + end + + if(conf.before_body) then + ngx.arg[1] = conf.before_body .. ' ' .. ngx.arg[1] Review comment: `' '` should be specified in before_body or after_body. So it can be removed. ########## File path: t/plugin/echo.t ########## @@ -0,0 +1,140 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +no_long_string(); +no_root_location(); +run_tests; + +__DATA__ + +=== TEST 1: sanity +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.echo") + local ok, err = plugin.check_schema({before_body = "body before", body = "body to attach" , + after_body = "body to attach"}) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- request +GET /t +--- response_body +done +--- no_error_log +[error] + + + +=== TEST 2: wrong type of integer +--- config + location /t { + content_by_lua_block { + local plugin = require("apisix.plugins.echo") + local ok, err = plugin.check_schema({before_body = "body before", body = "body to attach" , + after_body = 10}) + if not ok then + ngx.say(err) + end + + ngx.say("done") + } + } +--- request +GET /t +--- response_body +property "after_body" validation failed: wrong type: expected string, got number +done +--- no_error_log +[error] + + + +=== TEST 3: add plugin +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local code, body = t('/apisix/admin/routes/1', + ngx.HTTP_PUT, + [[{ + "plugins": { + "echo": { + "before_body": "before the body modification ", + "body":"hello upstream" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }]], + [[{ + "node": { + "value": { + "plugins": { + "echo": { + "before_body": "before the body modification ", + "body":"hello upstream" + } + }, + "upstream": { + "nodes": { + "127.0.0.1:1980": 1 + }, + "type": "roundrobin" + }, + "uri": "/hello" + }, + "key": "/apisix/routes/1" + }, + "action": "set" + }]] + ) + + if code >= 300 then + ngx.status = code + end + ngx.say(body) + } + } +--- request +GET /t +--- response_body +passed +--- no_error_log +[error] + + + +=== TEST 4: access Review comment: The body returns by upstream(test case) also needs to be added. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
