-
Notifications
You must be signed in to change notification settings - Fork 640
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
[ISSUE #4646] Add condition for transforming non-data values #4637
base: master
Are you sure you want to change the base?
Conversation
@pmupkin please fix the ci check error, and associate this pr with an issue first. |
eventmesh/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java Lines 43 to 61 in fdd8796
I didn't understand the logic here, could you explain it? The code in line 44 uses Jackson to convert JSON format strings into JsonNode, which seems to have nothing to do with JSON path expressions. |
Originally, the tools for serializing JSON (Jackson) and extracting JSON (JsonPath) were both placed within a single class. Looking at it now, the naming indeed can be confusing. It might be beneficial to reorganize them separately in the future. |
I don't think it's a big problem to put them both in JsonPathUtils, because some methods do use Jackson and JsonPath at the same time. 我觉得将它们都放到JsonPathUtils中问题不大,因为有些方法确实同时用到Jackson和JsonPath。
I don't think it's a confusing naming issue right now. More seriously, because the I don't know if I misunderstood anything, I look forward to your reply. 我觉得现在不是困惑的命名问题。 |
The
The |
eventmesh/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java Lines 74 to 83 in fdd8796
@pmupkin This is the logic of match . The code in line 82 , it retrieves the content specified in the first parameter JSON based on the second parameter, JSON path. The second parameter, JSON path, is obtained from variablesList property of JsonPathParser . You also said that this attribute stores JSON keys and values without a JSON path, does this mean that the match method is invalid?
这是 |
Regarding the code at line 82, for instance, if a value in the provided JSON format by the user is $.data.time, the JsonPath tool will be used to match corresponding values in CloudEvents. Could you provide an example illustrating the issue you mentioned? |
@pmupkin 我有些困惑。JSON格式不应该是像这样吗 |
Here's a fact: |
@pmupkin Addtionally, please pay attention to the first review comment above #4637 (comment). 您能否提供一个或多个完整的Transformer规则编写示例?然后我根据它们,再学习一遍您的PR #4365中有关Transformer的代码,以及PR #4622的代码。谢谢! |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #4637 +/- ##
============================================
+ Coverage 17.28% 17.40% +0.12%
- Complexity 1741 1762 +21
============================================
Files 792 797 +5
Lines 29697 29798 +101
Branches 2567 2579 +12
============================================
+ Hits 5132 5187 +55
- Misses 24090 24130 +40
- Partials 475 481 +6 ☔ View full report in Codecov by Sentry. |
It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback. If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4637 +/- ##
============================================
+ Coverage 17.28% 17.40% +0.12%
- Complexity 1741 1762 +21
============================================
Files 792 797 +5
Lines 29697 29798 +101
Branches 2567 2579 +12
============================================
+ Hits 5132 5187 +55
- Misses 24090 24130 +40
- Partials 475 481 +6 ☔ View full report in Codecov by Sentry. |
May I ask that is this PR ready for review? |
It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback. If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh. |
It has been 60 days since the last activity on this pull request. I am reaching out here to gently remind you that the Apache EventMesh community values every pull request, and please feel free to get in touch with the reviewers at any time. They are available to assist you in advancing the progress of your pull request and offering the latest feedback. If you encounter any challenges during development, seeking support within the community is encouraged. We sincerely appreciate your contributions to Apache EventMesh. |
Fixes #4646
From a design perspective, the transformer needs to avoid converting non-data fields that users may attempt to transform. The original code allowed users to transform fields in the EventMesh; restrictions have been added. If users utilize non-data fields, an exception will be thrown.