Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature][Transform] add JsonPath transform #5632

Merged
merged 26 commits into from
Nov 7, 2023
Merged

Conversation

FuYouJ
Copy link
Contributor

@FuYouJ FuYouJ commented Oct 15, 2023

related issue

#5633

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

FuYouJ and others added 5 commits October 15, 2023 19:57
[Transform][Jsonpath]add JsonPathTransform
[Transform][Jsonpath][Doc] add doc
[Transform][Jsonpath][Doc]update release
[Transform][Jsonpath]private variables may not use concurrent containers
pom.xml Outdated
@@ -128,6 +128,7 @@
<testcontainer.version>1.17.6</testcontainer.version>
<spotless.version>2.29.0</spotless.version>
<jsqlparser.version>4.5</jsqlparser.version>
<jsonpath.version>2.8.0</jsonpath.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There already exist 2.7.0, it's not a good idea to used multiple version json-path.

Copy link
Contributor Author

@FuYouJ FuYouJ Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not found a dependency for 2.7.0 yet.Could you point out where it is thank you

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<properties>
<httpclient.version>4.5.13</httpclient.version>
<httpcore.version>4.4.4</httpcore.version>
<guava-retrying.version>2.0.0</guava-retrying.version>
<json-path.version>2.7.0</json-path.version>
</properties>

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 17, 2023

@EricJoy2048 could you review this pr,thx

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 17, 2023

@EricJoy2048 could you review this pr,thx

jsonpath depend on

  • net.minidev:json-smart:2.4.7
  • net.minidev:accessors-smart:2.4.7
  • org.ow2.asm:asm:9.1

@ruanwenjun ruanwenjun added dependencies Pull requests that update a dependency file feature New feature labels Oct 18, 2023
Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add parse array and parse nest object test cases.

import static org.apache.seatunnel.transform.exception.JsonPathTransformErrorCode.JSON_PATH_COMPILE_ERROR;

@Slf4j
@AutoService(JsonPathTransform.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not Transform.class?

Comment on lines 22 to 26
private String path;

private String srcField;

private String destField;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final could be better.

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 22, 2023

@ruanwenjun jun✌🏻 please review

@ruanwenjun
Copy link
Member

CI failed, you need to check if the code has miss something.

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 23, 2023

CI failed, you need to check if the code has miss something.

well,I finally fixed the problem. now, ci is successed

Copy link
Member

@TyrantLucifer TyrantLucifer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add e2e test cases to parse array nest object

Comment on lines 52 to 53
private JsonPathTransformConfig config;
private SeaTunnelRowType seaTunnelRowType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final be better

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 29, 2023

@ruanwenjun @ruanwenjun @EricJoy2048
I refactored the code reference httpsource and fakesource to make it simpler and more powerful. Please review

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Oct 31, 2023

@ruanwenjun jun✌🏻 please review

@FuYouJ
Copy link
Contributor Author

FuYouJ commented Nov 4, 2023

@ruanwenjun @TyrantLucifer It's been a long time. I miss you so much(✺ω✺)

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @FuYouJ !

@Hisoka-X Hisoka-X dismissed stale reviews from TyrantLucifer and ruanwenjun November 7, 2023 04:12

already add.

@Hisoka-X Hisoka-X merged commit d908f0a into apache:dev Nov 7, 2023
8 checks passed
@FuYouJ
Copy link
Contributor Author

FuYouJ commented Nov 7, 2023

Thanks @FuYouJ !

You're welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants