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


##########
docs/en/connector-v2/source/Http.md:
##########
@@ -310,6 +314,35 @@ source {
 - Test data can be found at this link 
[mockserver-config.json](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json)
 - See this link for task configuration 
[http_jsonpath_to_assert.conf](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_jsonpath_to_assert.conf).
 
+  ### pageing
+
+  ```json

Review Comment:
   ```suggestion
     ```hocon
   ```



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -133,13 +170,30 @@ public void pollNext(Collector<SeaTunnelRow> output) 
throws Exception {
     }
 
     private void collect(Collector<SeaTunnelRow> output, String data) throws 
IOException {
+        String originData = data;

Review Comment:
   The `originData` seem like never be used.
   ```suggestion
   ```



##########
docs/en/connector-v2/source/Http.md:
##########
@@ -310,6 +314,35 @@ source {
 - Test data can be found at this link 
[mockserver-config.json](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json)
 - See this link for task configuration 
[http_jsonpath_to_assert.conf](../../../../seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/http_jsonpath_to_assert.conf).
 
+  ### pageing

Review Comment:
   Could you add two demo for different use way just like you added in e2e. 
Should including the data return by api fisrt. I think this way user can know 
how to use this feature easiler.



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -133,13 +170,30 @@ public void pollNext(Collector<SeaTunnelRow> output) 
throws Exception {
     }
 
     private void collect(Collector<SeaTunnelRow> output, String data) throws 
IOException {
+        String originData = data;
         if (contentJson != null) {
             data = JsonUtils.stringToJsonNode(getPartOfJson(data)).toString();
         }
         if (jsonField != null) {
             this.initJsonPath(jsonField);
             data = JsonUtils.toJsonNode(parseToMap(decodeJSON(data), 
jsonField)).toString();
         }
+        // page increase
+        if (pageInfo != null) {
+            // Determine whether the task is completed by specifying the 
presence of the 'total
+            // page'
+            // field.

Review Comment:
   ```suggestion
               // Determine whether the task is completed by specifying the 
presence of the 'total page' field.
   ```



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -133,13 +170,30 @@ public void pollNext(Collector<SeaTunnelRow> output) 
throws Exception {
     }
 
     private void collect(Collector<SeaTunnelRow> output, String data) throws 
IOException {
+        String originData = data;
         if (contentJson != null) {
             data = JsonUtils.stringToJsonNode(getPartOfJson(data)).toString();
         }
         if (jsonField != null) {
             this.initJsonPath(jsonField);
             data = JsonUtils.toJsonNode(parseToMap(decodeJSON(data), 
jsonField)).toString();
         }
+        // page increase
+        if (pageInfo != null) {
+            // Determine whether the task is completed by specifying the 
presence of the 'total
+            // page'
+            // field.
+            if (pageInfo.getTotalPageSize() > 0) {
+                noMoreElementFlag =
+                        pageInfo.getPageIndex() >= pageInfo.getTotalPageSize() 
? true : false;

Review Comment:
   ```suggestion
                   noMoreElementFlag = pageInfo.getPageIndex() >= 
pageInfo.getTotalPageSize();
   ```



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSource.java:
##########
@@ -135,12 +162,15 @@ public SeaTunnelDataType<SeaTunnelRow> getProducedType() {
     @Override
     public AbstractSingleSplitReader<SeaTunnelRow> createReader(
             SingleSplitReaderContext readerContext) throws Exception {
-        return new HttpSourceReader(
-                this.httpParameter,
-                readerContext,
-                this.deserializationSchema,
-                jsonField,
-                contentField);
+        HttpSourceReader httpSourceReader =
+                new HttpSourceReader(
+                        this.httpParameter,
+                        readerContext,
+                        this.deserializationSchema,
+                        jsonField,
+                        contentField);
+        httpSourceReader.setPageInfo(pageInfo);
+        return httpSourceReader;

Review Comment:
   Why not put `pageInfo` into constructor?



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -133,13 +170,30 @@ public void pollNext(Collector<SeaTunnelRow> output) 
throws Exception {
     }
 
     private void collect(Collector<SeaTunnelRow> output, String data) throws 
IOException {
+        String originData = data;
         if (contentJson != null) {
             data = JsonUtils.stringToJsonNode(getPartOfJson(data)).toString();
         }
         if (jsonField != null) {
             this.initJsonPath(jsonField);
             data = JsonUtils.toJsonNode(parseToMap(decodeJSON(data), 
jsonField)).toString();
         }
+        // page increase
+        if (pageInfo != null) {
+            // Determine whether the task is completed by specifying the 
presence of the 'total
+            // page'
+            // field.
+            if (pageInfo.getTotalPageSize() > 0) {
+                noMoreElementFlag =
+                        pageInfo.getPageIndex() >= pageInfo.getTotalPageSize() 
? true : false;
+            } else {
+                // no 'total page' configured
+                int readSize = JsonUtils.stringToJsonNode(data).size();
+                // if read size < 100 : read fininsh.
+                // if read size = 100 : read next page.

Review Comment:
   ```suggestion
                   // if read size < BatchSize : read fininsh.
                   // if read size = BatchSize : read next page.
   ```



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -48,19 +50,22 @@
 import java.util.Objects;
 
 @Slf4j
+@Setter
 public class HttpSourceReader extends AbstractSingleSplitReader<SeaTunnelRow> {
     protected final SingleSplitReaderContext context;
     protected final HttpParameter httpParameter;
     protected HttpClientProvider httpClient;
+    private final Configuration jsonConfiguration =

Review Comment:
   Please revert this useless change.



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -133,13 +170,30 @@ public void pollNext(Collector<SeaTunnelRow> output) 
throws Exception {
     }
 
     private void collect(Collector<SeaTunnelRow> output, String data) throws 
IOException {
+        String originData = data;
         if (contentJson != null) {
             data = JsonUtils.stringToJsonNode(getPartOfJson(data)).toString();
         }
         if (jsonField != null) {
             this.initJsonPath(jsonField);
             data = JsonUtils.toJsonNode(parseToMap(decodeJSON(data), 
jsonField)).toString();
         }
+        // page increase
+        if (pageInfo != null) {
+            // Determine whether the task is completed by specifying the 
presence of the 'total
+            // page'
+            // field.
+            if (pageInfo.getTotalPageSize() > 0) {
+                noMoreElementFlag =
+                        pageInfo.getPageIndex() >= pageInfo.getTotalPageSize() 
? true : false;
+            } else {
+                // no 'total page' configured
+                int readSize = JsonUtils.stringToJsonNode(data).size();
+                // if read size < 100 : read fininsh.
+                // if read size = 100 : read next page.
+                noMoreElementFlag = readSize < pageInfo.getBatchSize() ? true 
: false;

Review Comment:
   ditto



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -48,19 +50,22 @@
 import java.util.Objects;
 
 @Slf4j
+@Setter
 public class HttpSourceReader extends AbstractSingleSplitReader<SeaTunnelRow> {
     protected final SingleSplitReaderContext context;
     protected final HttpParameter httpParameter;
     protected HttpClientProvider httpClient;
+    private final Configuration jsonConfiguration =
+            Configuration.defaultConfiguration().addOptions(DEFAULT_OPTIONS);
     private final DeserializationCollector deserializationCollector;
     private static final Option[] DEFAULT_OPTIONS = {
         Option.SUPPRESS_EXCEPTIONS, Option.ALWAYS_RETURN_LIST, 
Option.DEFAULT_PATH_LEAF_TO_NULL
     };
     private JsonPath[] jsonPaths;
     private final JsonField jsonField;
     private final String contentJson;
-    private final Configuration jsonConfiguration =
-            Configuration.defaultConfiguration().addOptions(DEFAULT_OPTIONS);
+    private Boolean noMoreElementFlag = true;

Review Comment:
   ```suggestion
       private boolean noMoreElementFlag = true;
   ```



##########
seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceReader.java:
##########
@@ -48,19 +50,22 @@
 import java.util.Objects;
 
 @Slf4j
+@Setter
 public class HttpSourceReader extends AbstractSingleSplitReader<SeaTunnelRow> {
     protected final SingleSplitReaderContext context;
     protected final HttpParameter httpParameter;
     protected HttpClientProvider httpClient;
+    private final Configuration jsonConfiguration =
+            Configuration.defaultConfiguration().addOptions(DEFAULT_OPTIONS);
     private final DeserializationCollector deserializationCollector;
     private static final Option[] DEFAULT_OPTIONS = {
         Option.SUPPRESS_EXCEPTIONS, Option.ALWAYS_RETURN_LIST, 
Option.DEFAULT_PATH_LEAF_TO_NULL
     };
     private JsonPath[] jsonPaths;
     private final JsonField jsonField;
     private final String contentJson;
-    private final Configuration jsonConfiguration =
-            Configuration.defaultConfiguration().addOptions(DEFAULT_OPTIONS);
+    private Boolean noMoreElementFlag = true;
+    private PageInfo pageInfo;

Review Comment:
   Use `Optional<PageInfo>` instand `PageInfo` to avoid check `null`.



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