[Fix][connector-http] fix parsing httpjson, the number of two fields is inconsistent with the import failure by ocean-zhc · Pull Request #9103 · apache/seatunnel

ocean-zhc · 2025-04-02T03:26:43Z

…is inconsistent with the import failure

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.

ocean-zhc · 2025-04-02T03:27:10Z

#9093

Copilot

Pull Request Overview

This PR fixes the JSON parsing issue in the HTTP connector by ensuring that inconsistent field counts across JSON paths are handled gracefully. Instead of throwing an exception when field sizes differ, the code now calculates the maximum field count and pads shorter lists with null values.

Removed strict size equality check across parsed JSON results.
Introduced dynamic null-padding to standardize the size of each result list.
Updated log messaging to warn users about padded fields.

Comments suppressed due to low confidence (1)

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

[nitpick] Consider combining the message string into a single literal with placeholders instead of using string concatenation, which improves readability and minimizes potential formatting issues.

                        "Field [{}] with size ({}) is less than max size ({}), will be padded with null values. " + "This may happen when JSON paths return different numbers of elements.",

hailin0 · 2025-04-02T04:08:13Z

Please add test cases

hailin0 · 2025-04-03T05:48:05Z

+}
+source{
+  Http {
+    url = "https://api-seller.ozon.ru/v2/finance/realization"


user mockserver mock api & response

https://github.com/apache/seatunnel/blob/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/github_json_to_assert.conf#L26
https://github.com/apache/seatunnel/blob/dev/seatunnel-e2e/seatunnel-connector-v2-e2e/connector-http-e2e/src/test/resources/mockserver-config.json#L430

Hisoka-X

We need add new option to change the behavior.

Hisoka-X

Could you add a unit test for https://github.com/apache/seatunnel/pull/9103/files#diff-37ec4d574545a62fd6623d950e8ba947c72116663ada3424cd673ffbe2a733daL214 ？

Hisoka-X · 2025-04-11T10:44:01Z

+        if (httpParameter.isJsonFiledMissedReturnNull()) {
+            int maxLength = 0;
+            for (List<?> result : results) {
+                maxLength = Math.max(maxLength, result.size());
+            }
+            for (int i = 0; i < results.size(); i++) {
+                List<String> result = results.get(i);
+                if (result.size() < maxLength) {
+                    log.warn(
+                            "Field [{}] with size ({}) is less than max size ({}), will be padded with null values. "
+                                    + "This may happen when JSON paths return different numbers of elements.",
+                            jsonPaths[i].getPath(),
+                            result.size(),
+                            maxLength);
+                    for (int j = result.size(); j < maxLength; j++) {
+                        result.add(null);
+                    }
+                }
+            }
+        } else {


This solution will cause the data to be out of order. For example:

[{ "key1":"value11", "key2":"value22" }, { "key2":"value33" "key3":"value44" }, { "key1":"value55" "key3":"value66" } ]

Json Fields:

json_field = { key1 = "$[*].key1" key2 = "$[*].key2" }

The expected results should be:

key1 key2

value11 value22

null value33

value55 null

But current logic of results is:

key1 key2

value11 value22

value55 value33

The current implementation just wants to add null at the end, but does not accurately locate which data is null.

Yes, it was indeed an issue and I didn't take into account the data correspondence, now I fixed it.

Hisoka-X

Also please check why Option.DEFAULT_PATH_LEAF_TO_NULL not worked. If it work well. The result size should always equals. https://github.com/apache/seatunnel/pull/9103/files#diff-37ec4d574545a62fd6623d950e8ba947c72116663ada3424cd673ffbe2a733daL64

Hisoka-X · 2025-04-14T02:58:53Z

 | common-options              |         | No       | -       | Source plugin common parameters, please refer to [Source Common Options](../source-common-options.md) for details                                                                 |
 | keep_params_as_form         |    Boolean     | No       | false       | Whether the params are submitted according to the form, used for compatibility with legacy behaviors. When true, the value of the params parameter is submitted through the form. |
 | keep_page_param_as_http_param         |    Boolean     | No       | false       | Whether to set the paging parameters to params. For compatibility with legacy behaviors.                                                                                                                                  |
+| json_filed_missed_return_null         |    Boolean     | No       | false       | When the json field is missing, set true return null else error.


Hisoka-X · 2025-04-14T03:00:34Z

+
+        Container.ExecResult execResult21 = container.executeJob("/http_to_console.conf");
+        Assertions.assertEquals(0, execResult21.getExitCode());


I perfer use unit test to check it rather than e2e. Unit test can check all return value more easier. Current e2e test case you added even not check result data.

Let's remove this e2e test case @ocean-zhc

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

+        <dependency>
+            <groupId>cn.hutool</groupId>
+            <artifactId>hutool-json</artifactId>
+            <version>${hutool-json.version}</version>
+            <scope>test</scope>


Please do not introduce new json dependency. Just use jackson enough.

Hisoka-X · 2025-04-15T02:58:52Z

+
+        Container.ExecResult execResult21 = container.executeJob("/http_to_console.conf");
+        Assertions.assertEquals(0, execResult21.getExitCode());


Let's remove this e2e test case @ocean-zhc

Hisoka-X · 2025-04-15T03:21:49Z

+        if (httpParameter.isJsonFiledMissedReturnNull()) {
+            // Extract the common parent path from all configured paths
+            String commonParentPath = extractCommonParentPath(jsonPaths);
+            if (commonParentPath == null) {
+                throw new HttpConnectorException(
+                        HttpConnectorErrorCode.FIELD_DATA_IS_INCONSISTENT,
+                        "Could not find common parent path in JsonPaths. All paths must share a common array parent.");
+            }
+
+            // Get all objects under the common parent path
+            List<Map<String, Object>> objects;
+            try {
+                objects = jsonReadContext.read(commonParentPath);
+            } catch (Exception e) {


The current implementation is a bit complicated.
In fact, the jsonpath can do this by itself. In jsonpath,

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

Line 64 in d2ac555

Option.SUPPRESS_EXCEPTIONS, Option.ALWAYS_RETURN_LIST, Option.DEFAULT_PATH_LEAF_TO_NULL

we already enabled DEFAULT_PATH_LEAF_TO_NULL,

according the comment, when some path leaf not existed, it should return null. That's what we want, so we should check why this configure not work first.

The current implementation is a bit complicated. In fact, the jsonpath can do this by itself. In jsonpath,

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

Line 64 in d2ac555

Option.SUPPRESS_EXCEPTIONS, Option.ALWAYS_RETURN_LIST, Option.DEFAULT_PATH_LEAF_TO_NULL

we already enabled DEFAULT_PATH_LEAF_TO_NULL,

according the comment, when some path leaf not existed, it should return null. That's what we want, so we should check why this configure not work first.

DEFAULT_PATH_LEAF_TO_NULL can't be handled for complex, this configuration is only effective for leaf nodes, and can't be handled for missing parent nodes, see details:

Get parent node according to user configuration, e.g.
key1_1 = "$.result.rows[].key1.key1_1"
key2_1 = "$.result.rows[].key2.key2_1"
will get result.rows[*] as the parent node, and the child node is missing to fill null

result：

unit test：
https://github.com/ocean-zhc/seatunnel/blob/dev/seatunnel-connectors-v2/connector-http/connector-http-base/src/test/java/org/apache/seatunnel/connectors/seatunnel/http/JsonFieldMissedReturnNullComplexTest.java

https://github.com/ocean-zhc/seatunnel/blob/dev/seatunnel-connectors-v2/connector-http/connector-http-base/src/test/java/org/apache/seatunnel/connectors/seatunnel/http/JsonFieldMissedReturnNullTest.java

Copilot

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

seatunnel-connectors-v2/connector-http/connector-http-base/pom.xml: Language not supported

Comments suppressed due to low confidence (1)

seatunnel-connectors-v2/connector-http/connector-http-base/src/test/java/org/apache/seatunnel/connectors/seatunnel/http/JsonFieldMissedReturnNullComplexTest.java:182

The test comment indicates collector.collect is expected to be called once for each JSON object, but the verification uses times(1). Please review the intended behavior and update the comment or verification to avoid confusion.

verify(collector, times(1)).collect(rowCaptor.capture());

Copilot · 2025-04-16T06:47:02Z

+        sourceReader.pollNext(collector);
+
+        // Verify collector.collect was called 3 times (once for each JSON object)
+        verify(collector, times(1)).collect(rowCaptor.capture());


The comment above this verification states that collector.collect should be called 3 times (once for each JSON object) while the code verifies a single call. Please clarify whether the collector should be invoked once or multiple times and adjust the comment or verification accordingly.

single call

Hisoka-X

If we have key:

key1 = $.result.key1
key1 = $.result2.key2.key2

and json

{
  "result": {
    "key1": "value1"
  }, 
  "result2": {
  }
}

Even we set new option to true, the error will still be throwed. We should fix this situation.

Hisoka-X · 2025-04-21T02:20:43Z

-| pageing.total_page_size       | Int     | No       | -           | This parameter is used to control the total number of pages                                                                                                                       |
-| pageing.batch_size            | Int     | No       | -           | The batch size returned per request is used to determine whether to continue when the total number of pages is unknown                                                            |
-| pageing.start_page_number     | Int     | No       | 1           | Specify the page number from which synchronization starts                                                                                                                         |
-| pageing.page_type             | String  | No       | PageNumber  | this parameter is used to specify the page type ,or PageNumber if not set, only support `PageNumber` and `Cursor`.                                  |


Please rebase on dev. You lose some change from #9109

Hisoka-X · 2025-04-21T02:21:10Z

 - 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
-The current supported pagination type are `PageNumber` and `Cursor`. 


Hisoka-X · 2025-05-06T07:16:29Z

+    static {
+        // Register all available processors in order of precedence
+        PROCESSORS.add(new ArrayJsonPathProcessor());
+        PROCESSORS.add(new BracketJsonPathProcessor());
+        PROCESSORS.add(new ObjectJsonPathProcessor());
+    }


If a jsonpath contains three features of $.store['book'][*].title, can it still be processed normally? Please add corresponding test cases

Hisoka-X · 2025-05-07T09:53:32Z

+     * @param allowNullFields Whether to allow missing fields
+     * @return List of data extracted from JSON
+     */
+    public static List<Map<String, String>> extractData(


Hisoka-X · 2025-05-07T09:54:01Z

@@ -467,8 +447,8 @@ private String getPartOfJson(String data) {
    }

    private List<List<String>> dataFlip(List<List<String>> results) {


useless now?

Hisoka-X · 2025-05-07T10:04:07Z

+    public static String extractCommonParentPath(JsonPath[] paths) {
+        return JsonPathProcessorFactory.getProcessor(paths).extractCommonParentPath(paths);
+    }
+
+    /**
+     * Gets a relative path from a parent path and a full path.
+     *
+     * @param parentPath The parent path
+     * @param fullPath The full path
+     * @return The relative path
+     */
+    public static String getRelativePath(String parentPath, String fullPath) {
+        return JsonPathProcessorFactory.getProcessor(fullPath)
+                .getRelativePath(parentPath, fullPath);
+    }


useless too?

Hisoka-X · 2025-05-07T10:24:23Z

+                List<?> list = (List<?>) value;
+                return !list.isEmpty() ? list.get(0).toString() : null;


I think return list string instead of return first element is a better choice.

Hisoka-X · 2025-05-07T10:38:27Z

+
+    /** {@inheritDoc} */
+    @Override
+    public String extractCommonParentPath(JsonPath[] paths) {


useless too?

But it only used in ArrayJsonPathProcessor. Same as getRelativePath, readObjectsFromPath.

Hisoka-X · 2025-05-07T10:41:04Z

+        Field httpClientField = HttpSourceReader.class.getDeclaredField("httpClient");
+        httpClientField.setAccessible(true);
+        httpClientField.set(sourceReader, httpClientProvider);


Let's use our reflection tool

seatunnel/seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/ReflectionUtils.java

Line 79 in 051d19c

public static void setField(Object object, String fieldName, Object value) {

Eliminates the use of reflection

Hisoka-X · 2025-05-12T12:26:34Z

+            }
+            if (value instanceof List) {
+                List<?> list = (List<?>) value;
+                return !list.isEmpty() ? list.toString() : null;


We should return json style list string. Not use list toString value.

+1, use JsonString replace String

Hisoka-X · 2025-05-26T12:43:18Z

+public class BracketJsonPathProcessor extends AbstractJsonPathProcessor {
+    /** {@inheritDoc} */
+    @Override
+    public List<List<String>> processJsonData(ReadContext jsonReadContext, JsonPath[] paths) {


Recheck the logic of BracketJsonPathProcessor and ObjectJsonPathProcessor. They are totally same. We can keep only one.

Yes, it is indeed redundant, and I have optimized the relevant code and implementation

ocean-zhc

yes

ocean-zhc

yes

Hisoka-X · 2025-08-25T08:12:06Z

+     * @param paths Array of JsonPath objects
+     * @return The appropriate JsonPathProcessor
+     */
+    public static JsonPathProcessor getProcessor(JsonPath[] paths) {


Hisoka-X · 2025-08-25T08:14:28Z

@@ -44,37 +44,38 @@ They can be downloaded via install-plugin.sh or from the Maven central repositor

 ## Source Options


please update zh docs too.

Hisoka-X

Please update

seatunnel/seatunnel-connectors-v2/connector-http/connector-http-base/src/main/java/org/apache/seatunnel/connectors/seatunnel/http/source/HttpSourceFactory.java

Line 55 in 4beb2b9

return OptionRule.builder()

LeonYoah · 2025-08-26T03:03:38Z

The jsonpath util class might be extracted for use by kafka and the file connector

github-actions Bot added connectors-v2 labels Apr 2, 2025

ocean-zhc mentioned this pull request Apr 2, 2025

解析httpjson的时候，两个字段数量不一致导入失败 #9093

Closed

3 tasks

hailin0 requested a review from Copilot April 2, 2025 04:07

Copilot AI reviewed Apr 2, 2025

View reviewed changes

github-actions Bot added the e2e label Apr 2, 2025

hailin0 reviewed Apr 3, 2025

View reviewed changes

Hisoka-X reviewed Apr 7, 2025

View reviewed changes

This was linked to issues Apr 10, 2025

http source，The field data is inconsistent #9091

Closed

解析httpjson的时候，两个字段数量不一致导入失败 #9093

Closed

github-actions Bot added the document label Apr 10, 2025

Hisoka-X reviewed Apr 11, 2025

View reviewed changes

Hisoka-X requested changes Apr 11, 2025

View reviewed changes

Hisoka-X reviewed Apr 11, 2025

View reviewed changes

Hisoka-X reviewed Apr 14, 2025

View reviewed changes

Hisoka-X reviewed Apr 15, 2025

View reviewed changes

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

Copilot AI reviewed Apr 16, 2025

View reviewed changes

Hisoka-X reviewed Apr 21, 2025

View reviewed changes

ocean-zhc closed this Apr 30, 2025

ocean-zhc force-pushed the dev branch from abb13bb to 8617014 Compare April 30, 2025 05:42

ocean-zhc reopened this Apr 30, 2025

github-actions Bot removed the e2e label Apr 30, 2025

Hisoka-X reviewed May 6, 2025

View reviewed changes

Hisoka-X reviewed May 7, 2025

View reviewed changes

ocean-zhc requested a review from Hisoka-X May 9, 2025 03:50

Hisoka-X reviewed May 12, 2025

View reviewed changes

Comment thread .../main/java/org/apache/seatunnel/connectors/seatunnel/http/util/JsonPathProcessorFactory.java

Hisoka-X reviewed May 12, 2025

View reviewed changes

github-actions Bot added http and removed api labels May 22, 2025

ocean-zhc added 2 commits May 22, 2025 14:04

Optimization Code

f4d96d6

spotless:apply

49984c8

github-actions Bot removed the e2e label May 22, 2025

ocean-zhc and others added 2 commits May 26, 2025 13:38

Merge branch 'apache:dev' into dev

715a492

Optimization Code

2062e4f

Hisoka-X reviewed May 26, 2025

View reviewed changes

ocean-zhc and others added 4 commits June 4, 2025 09:08

Merge branch 'apache:dev' into dev

c29d47f

Optimization Code

0ec0549

Merge branch 'apache:dev' into dev

c27d297

Merge branch 'apache:dev' into dev

ca6a6af

ocean-zhc requested a review from Hisoka-X June 6, 2025 01:47

ocean-zhc commented Jun 6, 2025

View reviewed changes

ocean-zhc and others added 5 commits June 9, 2025 14:36

Merge branch 'apache:dev' into dev

3797224

Merge branch 'apache:dev' into dev

d39e061

Optimization Code

7a2a1a4

Optimization Code

9ab305c

Optimization Code

c119438

Hisoka-X reviewed Aug 25, 2025

View reviewed changes

Add json_filed_missed_return_null option to Http configuration

0c0e2de

Hisoka-X approved these changes Aug 26, 2025

View reviewed changes

github-actions Bot added approved reviewed labels Aug 26, 2025

corgy-w approved these changes Aug 26, 2025

View reviewed changes

corgy-w merged commit c8ade09 into apache:dev Aug 26, 2025
5 checks passed

	\| json_filed_missed_return_null \| Boolean \| No \| false \| When the json field is missing, set true return null else error.
	\| json_filed_missed_return_null \| Boolean \| No \| false \| When the json field is missing, set true return null else error. \|


		Container.ExecResult execResult21 = container.executeJob("/http_to_console.conf");
		Assertions.assertEquals(0, execResult21.getExitCode());

		@@ -467,8 +447,8 @@ private String getPartOfJson(String data) {
		}

		private List<List<String>> dataFlip(List<List<String>> results) {

		List<?> list = (List<?>) value;
		return !list.isEmpty() ? list.get(0).toString() : null;

		@@ -44,37 +44,38 @@ They can be downloaded via install-plugin.sh or from the Maven central repositor

		## Source Options

Uh oh!

Conversation

ocean-zhc commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

ocean-zhc commented Apr 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

hailin0 commented Apr 2, 2025

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!

Hisoka-X left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hisoka-X left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

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

ocean-zhc commented Apr 2, 2025 •

edited

Loading

Hisoka-X left a comment •

edited

Loading

Hisoka-X Apr 11, 2025 •

edited

Loading

Hisoka-X left a comment •

edited

Loading

Hisoka-X May 12, 2025 •

edited

Loading

Hisoka-X May 26, 2025 •

edited

Loading