Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ext-plugin-post-resp): support get response body by extra_info #7947

Merged
merged 11 commits into from
Sep 23, 2022

Conversation

soulbird
Copy link
Contributor

@soulbird soulbird commented Sep 20, 2022

Description

Provide the ext-plugin-post-resp plugin with the ability to get the response body, so that the External Plugin will be able to do some processing on the response body.
As an additional feature, the ext-plugin-post-resp plugin also supports getting the request context in the response phase.

Fixes #7575

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)

@soulbird soulbird marked this pull request as ready for review September 20, 2022 07:02
@soulbird soulbird requested review from tzssangglass and spacewander and removed request for tzssangglass September 20, 2022 07:25



=== TEST 10: ask request body (not exist)
Copy link
Member

Choose a reason for hiding this comment

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

why we check request body here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reused the handle_extra_info function to get the response body, the request body is part of this function.

tzssangglass
tzssangglass previously approved these changes Sep 22, 2022
@tzssangglass
Copy link
Member

LGTM

apisix/plugins/ext-plugin/init.lua Outdated Show resolved Hide resolved
t/plugin/ext-plugin/extra-info.t Outdated Show resolved Hide resolved



=== TEST 5: ask response body (not exist)
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a test with empty response body

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't make sense to add a test with an empty response body in the ext-plugin-pre-req plugin. I guess you want me to add this test case in the ext-plugin-post-resp plugin.
However, an empty response body usually occurs when the upstream returns a 304 status code or the request method is HEAD, which needs to be handled separately in subsequent PRs. I have added TODO.


repeat
local chunk, read_err, cb_err
-- TODO: HEAD or 304
Copy link
Member

Choose a reason for hiding this comment

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

Could you use at least one sentence for this TODO? I can't recall why this comment is added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@spacewander spacewander merged commit 1d29d4b into apache:master Sep 23, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 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.

feat:ext-plugin-post-resp plugin need get original response body information
3 participants