Hisoka-X commented on code in PR #9184:
URL: https://github.com/apache/seatunnel/pull/9184#discussion_r2051923037


##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -170,17 +170,25 @@ private void updateRequestParam(PageInfo pageInfo) {
             }
             return;
         }
-
+        Long pageValue = pageInfo.getPageIndex();
+        String pageField = pageInfo.getPageField();
+
+        // Process headers
+        if (MapUtils.isNotEmpty(this.httpParameter.getHeaders())) {
+            processPageMap(
+                    this.httpParameter.getHeaders(),
+                    pageField,
+                    pageValue,
+                    usePlaceholderReplacement);
+        }
         // if not set keepPageParamAsHttpParam, but page field is in params, 
then set page index as
-        // params
         if (MapUtils.isNotEmpty(this.httpParameter.getParams())) {
 
-            // set page index as params
-            if 
(this.httpParameter.getParams().containsKey(pageInfo.getPageField())) {
-                this.httpParameter
-                        .getParams()
-                        .put(pageInfo.getPageField(), 
pageInfo.getPageIndex().toString());
-            }
+            processPageMap(
+                    this.httpParameter.getParams(),
+                    pageField,
+                    pageValue,
+                    usePlaceholderReplacement);
 
             // set page cursor as params

Review Comment:
   we should take care cursor base params too.



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -209,6 +214,72 @@ private void updateRequestParam(PageInfo pageInfo) {
         }
     }
 
+    /**
+     * Process a string map for parameter replacement
+     *
+     * @param map The map to process (headers or params)
+     * @param pageField The page field name
+     * @param pageValue The page value
+     * @param usePlaceholderReplacement Whether to use placeholder replacement
+     */
+    private void processPageMap(
+            Map<String, String> map,
+            String pageField,
+            Long pageValue,
+            boolean usePlaceholderReplacement) {
+        if (usePlaceholderReplacement) {
+            // Placeholder replacement
+            Map<String, String> updatedMap = new HashMap<>();
+            for (Map.Entry<String, String> entry : map.entrySet()) {
+                String key = entry.getKey();
+                String value = entry.getValue();
+                if (value.equals("${" + pageField + "}")) {
+                    // If the value is exactly the placeholder, use pageValue 
directly
+                    updatedMap.put(key, pageValue.toString());
+                }
+            }
+            map.putAll(updatedMap);
+        } else if (map.containsKey(pageField)) {
+            // Key-based replacement
+            map.put(pageField, pageValue.toString());
+        }
+    }
+
+    /**
+     * Process the body map for parameter replacement
+     *
+     * @param bodyMap The body map to process
+     * @param pageField The page field name
+     * @param pageValue The page index (Long value for direct replacement)
+     * @param usePlaceholderReplacement Whether to use placeholder replacement
+     */
+    private void processBodyPageMap(
+            Map<String, Object> bodyMap,
+            String pageField,
+            Long pageValue,
+            boolean usePlaceholderReplacement) {
+        if (usePlaceholderReplacement) {
+            // Placeholder replacement
+            Map<String, Object> updatedBody = new HashMap<>();
+            for (Map.Entry<String, Object> entry : bodyMap.entrySet()) {
+                String key = entry.getKey();
+                Object value = entry.getValue();
+                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);
+                    }
+                }

Review Comment:
   Even the value is another map we should replace the value too.



##########
seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/java/org/apache/seatunnel/e2e/connector/http/HttpIT.java:
##########
@@ -363,6 +363,39 @@ public void testSourceToAssertSink(TestContainer container)
         Assertions.assertEquals(0, execResult21.getExitCode());
     }
 
+    @TestTemplate
+    public void testPlaceholderReplacement(TestContainer container)

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



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -209,6 +214,72 @@ private void updateRequestParam(PageInfo pageInfo) {
         }
     }
 
+    /**
+     * Process a string map for parameter replacement
+     *
+     * @param map The map to process (headers or params)
+     * @param pageField The page field name
+     * @param pageValue The page value
+     * @param usePlaceholderReplacement Whether to use placeholder replacement
+     */
+    private void processPageMap(
+            Map<String, String> map,
+            String pageField,
+            Long pageValue,
+            boolean usePlaceholderReplacement) {
+        if (usePlaceholderReplacement) {
+            // Placeholder replacement
+            Map<String, String> updatedMap = new HashMap<>();
+            for (Map.Entry<String, String> entry : map.entrySet()) {
+                String key = entry.getKey();
+                String value = entry.getValue();
+                if (value.equals("${" + pageField + "}")) {
+                    // If the value is exactly the placeholder, use pageValue 
directly
+                    updatedMap.put(key, pageValue.toString());
+                }
+            }

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



-- 
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]

Reply via email to