[Feature][connector-http] Parameters support placeholder replacement by CosmosNi · Pull Request #9184 · apache/seatunnel

CosmosNi · 2025-04-16T03:15:25Z

Parameters support placeholder replacement.

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config
Update the release-note.

…_http_param # Conflicts: # docs/en/connector-v2/source/Http.md # seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/PageInfo.java # seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSource.java # seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java # seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/java/org/apache/seatunnel/e2e/connector/http/HttpIT.java # seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json

Copilot

Copilot reviewed 6 out of 13 changed files in this pull request and generated no comments.

Files not reviewed (7)

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_get_placeholder_replacement_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_get_placeholder_replacement_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_header_placeholder_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_header_placeholder_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_placeholder_replacement_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_placeholder_replacement_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json: Language not supported

Comments suppressed due to low confidence (2)

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/java/org/apache/seatunnel/e2e/connector/http/HttpIT.java:366

The variable 'execResult21' is declared twice in the same scope; consider renaming the second instance to avoid redeclaration.

Container.ExecResult execResult21 = container.executeJob("/http_placeholder_replacement_true.conf");

seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:187

The processPageMap method is incorrectly applied on 'this.httpParameter.getHeaders()' instead of 'this.httpParameter.getParams()'; update the argument to use the correct map.

processPageMap( this.httpParameter.getHeaders(), pageField, pageValue, usePlaceholderReplacement);

Copilot

Pull Request Overview

This PR adds support for placeholder replacement in the HTTP connector’s parameters, headers, and body while updating configuration and documentation accordingly.

Introduces new tests to validate placeholder replacement functionality in various request components.
Updates the HTTP source reader to incorporate placeholder replacement logic and new configuration options.
Enhances documentation with examples on using placeholder replacement.

Reviewed Changes

Copilot reviewed 6 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/java/org/apache/seatunnel/e2e/connector/http/HttpIT.java	Added E2E tests for placeholder replacement on headers, parameters, and GET requests.
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java	Modified updateRequestParam to support placeholder replacement; introduced processPageMap and processBodyPageMap.
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSource.java	Updated paging configuration to include use of placeholder replacement.
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/PageInfo.java	Added a boolean flag for placeholder replacement.
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/config/HttpSourceOptions.java	Added a new configuration option for placeholder replacement.
docs/en/connector-v2/source/Http.md	Updated documentation with details and examples for placeholder replacement.

Files not reviewed (7)

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_get_placeholder_replacement_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_get_placeholder_replacement_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_header_placeholder_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_header_placeholder_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_placeholder_replacement_false.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_placeholder_replacement_true.conf: Language not supported
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json: Language not supported

Comments suppressed due to low confidence (2)

seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/java/org/apache/seatunnel/e2e/connector/http/HttpIT.java:366

The variable 'execResult21' is redeclared, which may cause a compilation error due to duplicate variable declaration. Consider renaming one of the variables.

Container.ExecResult execResult21 = container.executeJob("/http_placeholder_replacement_true.conf");

seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:187

This line uses 'this.httpParameter.getHeaders()' while processing parameters. It seems intended to process the parameters map; replace it with 'this.httpParameter.getParams()'.

processPageMap(this.httpParameter.getHeaders(), pageField, pageValue, usePlaceholderReplacement);

…_http_param

CosmosNi · 2025-04-18T06:40:08Z

@Hisoka-X PTAL

Hisoka-X · 2025-04-21T03:56:44Z

+                    pageValue,
+                    usePlaceholderReplacement);

            // set page cursor as params


we should take care cursor base params too.

Hisoka-X · 2025-04-21T04:00:16Z

+                if (value.equals("${" + pageField + "}")) {
+                    // If the value is exactly the placeholder, use pageValue directly
+                    updatedMap.put(key, pageValue.toString());
+                }
+            }


we should take care value contains ${page} situation. E.g. page: 10${page}

Hisoka-X · 2025-04-21T04:01:25Z

+                if (value instanceof String) {
+                    String strValue = (String) value;
+                    // Check if the value is exactly the placeholder for pageField
+                    if (strValue.equals("${" + pageField + "}")) {
+                        // If the value is exactly the placeholder, use pageValue directly
+                        updatedBody.put(key, pageValue);
+                    }
+                }


Even the value is another map we should replace the value too.

Hisoka-X · 2025-04-21T04:03:39Z

+    @TestTemplate
+    public void testPlaceholderReplacement(TestContainer container)


Let's move all test case to unit test. You can refer https://github.com/apache/seatunnel/pull/9103/files#diff-4926ea699715eac0714ccf51f2093fa36b3d15f0a72239490b4ba77de5674bceR54

Hisoka-X · 2025-04-24T11:33:50Z

+     */
+    private String replacePlaceholder(String value, String pageField, Object pageValue) {
+        if (value == null || pageField == null || !value.contains("${" + pageField + "}")) {
+            return null;


Suggested change

return null;

return value;

I think return value without change is better than null.

Hisoka-X · 2025-04-24T11:33:56Z

+            String suffix = value.substring(placeholderIndex + placeholder.length());
+            return prefix + pageValue + suffix;
+        }
+        return null;


Hisoka-X · 2025-04-24T11:35:30Z

+        Method updateRequestParamMethod =
+                HttpSourceReader.class.getDeclaredMethod(
+                        "updateRequestParam", PageInfo.class, boolean.class);
+        updateRequestParamMethod.setAccessible(true);
+        updateRequestParamMethod.invoke(httpSourceReader, pageInfo, true);


Update updateRequestParam scope to default and add @VisibleForTesting annotation to avoid reflection.

Hisoka-X · 2025-04-24T11:41:35Z

+            boolean usePlaceholderReplacement) {
+        if (usePlaceholderReplacement) {
+            String jsonBody = JsonUtils.toJsonString(bodyMap);
+            String placeholder = "\"${" + pageField + "}\"";


why we should special care "${key}" with quotes?

please delete it.

…_http_param

Hisoka-X · 2025-04-29T02:32:16Z

+                                                entry -> entry.getValue().unwrapped(),
+                                                (v1, v2) -> v2));
+            } catch (Exception e) {
+                log.warn("Failed to parse body string to map: {}", body, e);


we shoud throw exception directly.

Hisoka-X · 2025-04-29T02:34:00Z

+     */
+    private String replacePlaceholder(String value, String pageField, Object pageValue) {
+        if (value == null || pageField == null || !value.contains("${" + pageField + "}")) {
+            return null;


Hisoka-X · 2025-04-29T02:35:30Z

+                log.warn(
+                        "Failed to parse body string for key-based replacement: {}", bodyString, e);


throw exception too.

Hisoka-X · 2025-04-29T02:35:59Z

+            boolean usePlaceholderReplacement) {
+        if (usePlaceholderReplacement) {
+            String jsonBody = JsonUtils.toJsonString(bodyMap);
+            String placeholder = "\"${" + pageField + "}\"";


please delete it.

Hisoka-X · 2025-04-29T08:29:09Z

+- As a number without quotes: `${page}` (in JSON body)
+- In nested JSON structures: `{"pagination":{"page":${page}}}`

+#### Example 1: Using page parameters in body and params


Suggested change

#### Example 1: Using page parameters in body and params

##### Example 1: Using page parameters in body and params

Hisoka-X · 2025-04-29T08:29:16Z

 }
+```
+
+#### Example 2: Using page parameters in headers


Hisoka-X · 2025-04-30T02:09:15Z

+}
+```
+
+#### Example 3: Using key-based replacement (without placeholders)


Suggested change

#### Example 3: Using key-based replacement (without placeholders)

##### Example 3: Using key-based replacement (without placeholders)

Hisoka-X · 2025-04-30T02:09:23Z

+}
+```

+#### Example 4: Using prefixed page number in headers


Suggested change

#### Example 4: Using prefixed page number in headers

##### Example 4: Using prefixed page number in headers

Hisoka-X · 2025-04-30T02:09:33Z

+}
+```
+
+#### Example 5: Using unquoted page number in body


Suggested change

#### Example 5: Using unquoted page number in body

##### Example 5: Using unquoted page number in body

Hisoka-X · 2025-04-30T02:09:37Z

+}
+```
+
+#### Example 6: Using nested JSON structure with page parameter


Suggested change

#### Example 6: Using nested JSON structure with page parameter

##### Example 6: Using nested JSON structure with page parameter

Hisoka-X

LGTM if ci passes. Thanks @CosmosNi

CosmosNi added 2 commits April 15, 2025 12:47

[Feature][connector-http] Support use placeholder replacement

461e289

[Feature][connector-http] Support use placeholder replacement

3f5e68c

github-actions Bot added document connectors-v2 e2e labels Apr 16, 2025

CosmosNi added 3 commits April 16, 2025 13:56

[Feature][connector-http] Support use placeholder replacement

e8f2b70

[Feature][connector-http] Support use placeholder replacement

b0a3b69

hailin0 requested a review from Copilot April 16, 2025 06:43

Copilot AI reviewed Apr 16, 2025

View reviewed changes

nielifeng requested a review from Copilot April 16, 2025 06:51

Copilot AI reviewed Apr 16, 2025

View reviewed changes

CosmosNi added 5 commits April 17, 2025 09:48

[Feature][connector-http] Support use placeholder replacement

e51defe

Merge remote-tracking branch 'origin/feature_http_param' into feature…

3f8ffc3

…_http_param

[Feature][connector-http] Support use placeholder replacement

567fc0e

[Feature][connector-http] Support use placeholder replacement

119d8ca

[Feature][connector-http] Support use placeholder replacement

344e6c6

Hisoka-X reviewed Apr 21, 2025

View reviewed changes

CosmosNi added 3 commits April 23, 2025 09:47

Merge remote-tracking branch 'upstream/dev' into feature_http_param

cbed2c7

[Feature][connector-http] Support use placeholder replacement

005f820

[Feature][connector-http] Support use placeholder replacement

a024b3c

CosmosNi requested a review from Hisoka-X April 23, 2025 03:24

Hisoka-X reviewed Apr 24, 2025

View reviewed changes

CosmosNi added 5 commits April 27, 2025 10:02

[Feature][connector-http] Support use placeholder replacement

cbaf551

[Feature][connector-http] Support use placeholder replacement

b0331da

[Feature][connector-http] Support use placeholder replacement

f9328b8

[Feature][connector-http] Support use placeholder replacement

af4964d

[Feature][connector-http] Support use placeholder replacement

3583eb3

Merge remote-tracking branch 'origin/feature_http_param' into feature…

98e9dcd

…_http_param

github-actions Bot removed the http label Apr 27, 2025

CosmosNi added 3 commits April 27, 2025 18:09

[Feature][connector-http] Support use placeholder replacement

c90d4a2

[Feature][connector-http] Support use placeholder replacement

f1f4859

Merge remote-tracking branch 'upstream/dev' into feature_http_param

27c720b

CosmosNi requested a review from Hisoka-X April 28, 2025 09:14

Hisoka-X reviewed Apr 29, 2025

View reviewed changes

[Feature][connector-http] Support use placeholder replacement

59a0fc9

CosmosNi requested a review from Hisoka-X April 29, 2025 06:42

[Feature][connector-http] Support use placeholder replacement

11434b7

Hisoka-X reviewed Apr 29, 2025

View reviewed changes

[Feature][connector-http] Support use placeholder replacement

f60564a

Hisoka-X reviewed Apr 30, 2025

View reviewed changes

[Feature][connector-http] Support use placeholder replacement

ea5510d

Hisoka-X approved these changes Apr 30, 2025

View reviewed changes

github-actions Bot added approved reviewed labels Apr 30, 2025

hailin0 approved these changes Apr 30, 2025

View reviewed changes

hailin0 merged commit 8617014 into apache:dev Apr 30, 2025

		@TestTemplate
		public void testPlaceholderReplacement(TestContainer container)

		log.warn(
		"Failed to parse body string for key-based replacement: {}", bodyString, e);

	#### Example 1: Using page parameters in body and params
	##### Example 1: Using page parameters in body and params

	#### Example 3: Using key-based replacement (without placeholders)
	##### Example 3: Using key-based replacement (without placeholders)

	#### Example 4: Using prefixed page number in headers
	##### Example 4: Using prefixed page number in headers

	#### Example 5: Using unquoted page number in body
	##### Example 5: Using unquoted page number in body

	#### Example 6: Using nested JSON structure with page parameter
	##### Example 6: Using nested JSON structure with page parameter

Uh oh!

Conversation

CosmosNi commented Apr 16, 2025

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

CosmosNi commented Apr 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants