Hisoka-X commented on code in PR #8434:
URL: https://github.com/apache/seatunnel/pull/8434#discussion_r1947421346
##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java:
##########
@@ -115,14 +120,42 @@ public HttpResponse execute(
String method,
Map<String, String> headers,
Map<String, String> params,
- String body)
+ Map<String, Object> body,
+ Map<String, Object> pageParams,
+ boolean keepParamsAsForm)
throws Exception {
// convert method option to uppercase
method = method.toUpperCase(Locale.ROOT);
+ // Keep the original post logic
+ if (HttpPost.METHOD_NAME.equals(method) && keepParamsAsForm) {
+ URI uri = URI.create(url);
+ Map<String, Object> paramsMap = new HashMap<>();
+ if (MapUtils.isNotEmpty(params)) {
+ paramsMap.putAll(params);
+ }
+ if (MapUtils.isNotEmpty(pageParams)) {
+ paramsMap.putAll(params);
+ }
Review Comment:
when enable `keepParamsAsForm`, why not put params into request body? Then
add header `application/x-www-form-urlencoded`. So that we can handle it with
normally logic.
##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java:
##########
@@ -115,14 +120,42 @@ public HttpResponse execute(
String method,
Map<String, String> headers,
Map<String, String> params,
- String body)
+ Map<String, Object> body,
+ Map<String, Object> pageParams,
+ boolean keepParamsAsForm)
throws Exception {
// convert method option to uppercase
method = method.toUpperCase(Locale.ROOT);
+ // Keep the original post logic
+ if (HttpPost.METHOD_NAME.equals(method) && keepParamsAsForm) {
+ URI uri = URI.create(url);
+ Map<String, Object> paramsMap = new HashMap<>();
+ if (MapUtils.isNotEmpty(params)) {
+ paramsMap.putAll(params);
+ }
+ if (MapUtils.isNotEmpty(pageParams)) {
+ paramsMap.putAll(params);
+ }
+ return doPost(uri, headers, paramsMap, body, true);
+ }
if (HttpPost.METHOD_NAME.equals(method)) {
- return doPost(url, headers, params, body);
+ // Create access address
+ URIBuilder uriBuilder = new URIBuilder(url);
+ // add parameter to uri
+ addParameters(uriBuilder, params);
Review Comment:
why not move `addParameters` into `doPost` method, just like `doGet`?
##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java:
##########
@@ -429,6 +482,58 @@ private void addHeaders(HttpRequestBase request,
Map<String, String> headers) {
headers.forEach(request::addHeader);
}
+ private void addBody(
+ HttpEntityEnclosingRequestBase request,
+ Map<String, Object> body,
+ Map<String, Object> params,
+ boolean keepParamsAsForm)
+ throws UnsupportedEncodingException {
+ Map<String, Object> bodyMap = new HashedMap<>();
+ if (MapUtils.isNotEmpty(body)) {
+ bodyMap = body;
+ }
+ boolean isFormSubmit =
+ request.getHeaders(HTTP.CONTENT_TYPE) != null
+ && request.getHeaders(HTTP.CONTENT_TYPE).length > 0
+ && APPLICATION_FORM.equalsIgnoreCase(
+
request.getHeaders(HTTP.CONTENT_TYPE)[0].getValue());
Review Comment:
Please update the doc to tell user how to submit http request with json or
form.
##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/client/HttpClientProvider.java:
##########
@@ -292,15 +325,35 @@ public HttpResponse doPost(
/**
* Send a post request with request headers , request parameters and
request body
*
- * @param url request address
* @param headers request header map
* @param params request parameter map
* @param body request body
* @return http response result
* @throws Exception information
*/
public HttpResponse doPost(
- String url, Map<String, String> headers, Map<String, String>
params, String body)
+ URI uri,
+ Map<String, String> headers,
+ Map<String, Object> params,
+ Map<String, Object> body,
+ boolean keepParamsAsForm)
Review Comment:
doPost should not care `keepParamsAsForm` if we can handle old logic in
https://github.com/apache/seatunnel/pull/8434/files#diff-84c2e7b09f0d771042057899d626749a07479adaa72a7db60d17ed0ec3592448R130
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
