Skip to content

Conversation

@Hazel6869
Copy link
Contributor

@Hazel6869 Hazel6869 commented Aug 17, 2022

Description

#7722

Fixes # (issue)

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@Hazel6869 Hazel6869 marked this pull request as draft August 18, 2022 00:14
@Hazel6869 Hazel6869 changed the title feat: upgrade response rewrite policy and add test cases feat: upgrade response rewrite plugin and add test cases Aug 18, 2022
@Hazel6869 Hazel6869 marked this pull request as ready for review August 19, 2022 03:07
…#7621)

* Update README.md

Co-authored-by: Ming Wen <moonbingbing@gmail.com>

Co-authored-by: Sylvia <39793568+SylviaBABY@users.noreply.github.com>
Co-authored-by: Ming Wen <moonbingbing@gmail.com>
location /t {
content_by_lua_block {
local plugin = require("apisix.plugins.response-rewrite")
local ok, err = plugin.check_schema({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added test doesn't cover the changed part?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hazel6869 You should write another case which really uses the configuration.

@spacewander
Copy link
Member

Please put the issue id after Fixes # so it can be linked.

Comment on lines 705 to 724
=== TEST 25: set an nil body with setting body_base64 to false
--- config
location /t {
content_by_lua_block {
local plugin = require("apisix.plugins.response-rewrite")
local ok, err = plugin.check_schema({
body_base64 = false
})
if not ok then
ngx.say(err)
return
end
}
}
--- request
GET /t
--- response_body

--- no_error_log
[error]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case would have passed even without the code changes.

tzssangglass and others added 15 commits August 22, 2022 09:33
* docs: refactor kafka-logger plugin doc

* Update docs/zh/latest/plugins/kafka-logger.md

Co-authored-by: Sylvia <39793568+SylviaBABY@users.noreply.github.com>

Co-authored-by: hf400159 <hf400159@163.com>
Co-authored-by: Sylvia <39793568+SylviaBABY@users.noreply.github.com>
* docs: refactor rocketmq-logger plugin doc

* docs: fix typo

* docs: update keywords

Co-authored-by: hf400159 <hf400159@163.com>
* docs: refactor loggers plugin doc

* Update docs/zh/latest/plugins/clickhouse-logger.md

Co-authored-by: Sylvia <39793568+SylviaBABY@users.noreply.github.com>

Co-authored-by: hf400159 <hf400159@163.com>
Co-authored-by: Sylvia <39793568+SylviaBABY@users.noreply.github.com>
* docs: update loggly

Co-authored-by: hf400159 <hf400159@163.com>
* docs: update Architecture docs
* move deployment guide and reference in installation docs
* add license header


=== TEST 25: set an nil body with setting body_base64 to false
=== TEST 25: set header(rewrite body with "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
=== TEST 25: set header(rewrite body with "")
=== TEST 25: set header and rewrite body to empty

@Hazel6869 Hazel6869 marked this pull request as draft August 30, 2022 02:32
@tzssangglass
Copy link
Member

hi @Hazel6869 I think you merged the branches by mistake.

@Hazel6869
Copy link
Contributor Author

hi @Hazel6869 I think you merged the branches by mistake.

i will fix it, thanks

@Hazel6869 Hazel6869 closed this Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.