eventmesh
                                
                                
                                
                                    eventmesh copied to clipboard
                            
                            
                            
                        [ISSUE #4646] Add condition for transforming non-data values
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.
@pmupkin please fix the ci check error, and associate this pr with an issue first. https://eventmesh.apache.org/community/contribute/contribute this doc will help you fix the ci check error.
https://github.com/apache/eventmesh/blob/fdd87969e34e6e831a420aeb9cfbc6776ecaeec7/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java#L43-L61 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.
https://github.com/apache/eventmesh/blob/fdd87969e34e6e831a420aeb9cfbc6776ecaeec7/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java#L43-L61
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.
@pmupkin
Originally, the tools for serializing JSON (Jackson) and extracting JSON (JsonPath) were both placed within a single class.
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。
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 confusing naming issue right now. JsonPathUtils#parseStrict() cannot handle Json path expressions, it only converts JSON strings into JsonNode, so if (valueText.startsWith("$") && !valueText.startsWith("$.data"))  you add in this PR seems unnecessary.
More seriously, because the JsonPathUtils.parseStrict() function converts JSON strings to JsonNode, the data stored in the JsonPathParser's variablesList property does not have any JSON path. It makes JsonPathParser#match() method ineffective as it cannot retrieve any JSON paths from variablesList property, which are exactly what it needs.
I don't know if I misunderstood anything, I look forward to your reply.
我觉得现在不是困惑的命名问题。JsonPathUtils#parseStrict()就处理不了Json路径表达式,它只是将json字符串转成JsonNode,所以您在该PR中加上if (valueText.startsWith("$") && !valueText.startsWith("$.data")) 似乎是没必要的。
更严重的是,因为JsonPathUtils.parseStrict()功能是将json字符串转成JsonNode,所以JsonPathParser的variablesList属性保存的数据中就没有json路径。这样JsonPathParser#match()方法就失效了,因为该方法从variablesList属性拿不到任何json路径,但这恰恰是它需要的。
我不知道我有没有哪里理解错了,期待您的回复。
@pmupkin
Originally, the tools for serializing JSON (Jackson) and extracting JSON (JsonPath) were both placed within a single class.
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。
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 confusing naming issue right now.
JsonPathUtils#parseStrict()cannot handle Json path expressions, it only converts JSON strings into JsonNode, soif (valueText.startsWith("$") && !valueText.startsWith("$.data"))you add in this PR seems unnecessary.More seriously, because the
JsonPathUtils.parseStrict()function converts JSON strings to JsonNode, the data stored in theJsonPathParser'svariablesListproperty does not have any JSON path. It makesJsonPathParser#match()method ineffective as it cannot retrieve any JSON paths fromvariablesListproperty, which are exactly what it needs.I don't know if I misunderstood anything, I look forward to your reply.
我觉得现在不是困惑的命名问题。
JsonPathUtils#parseStrict()就处理不了Json路径表达式,它只是将json字符串转成JsonNode,所以您在该PR中加上if (valueText.startsWith("$") && !valueText.startsWith("$.data"))似乎是没必要的。 更严重的是,因为JsonPathUtils.parseStrict()功能是将json字符串转成JsonNode,所以JsonPathParser的variablesList属性保存的数据中就没有json路径。这样JsonPathParser#match()方法就失效了,因为该方法从variablesList属性拿不到任何json路径,但这恰恰是它需要的。 我不知道我有没有哪里理解错了,期待您的回复。
The variablesList holds pairs of data from a JSON. Put simply, the initialization of JsonPathParser serves the following purposes:
- Checking strict adherence to JSON format.
 - Serializing JSON for easy access to its keys and values.
 - Storing the keys and values in 
variablesList. Ultimately, what gets stored invariablesListare the string representations of JSON keys and values. 
The match function, on the other hand, is used to parse CloudEvents messages using JSONPath.
The
matchfunction, on the other hand, is used to parse CloudEvents messages using JSONPath.
https://github.com/apache/eventmesh/blob/fdd87969e34e6e831a420aeb9cfbc6776ecaeec7/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java#L74-L83
@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?
这是match的逻辑,在第82行代码,是根据第二个参数即JSON path,获取第一个参数JSON中指定的内容。第二个参数JSON path正是从JsonPathParser的variablesList属性中获取。您也说该属性保存的是JSON键和值,没有JSON path,所以是不是意味着match方法失效了?
The
matchfunction, on the other hand, is used to parse CloudEvents messages using JSONPath.https://github.com/apache/eventmesh/blob/fdd87969e34e6e831a420aeb9cfbc6776ecaeec7/eventmesh-transformer/src/main/java/org/apache/eventmesh/transformer/JsonPathParser.java#L74-L83
@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 fromvariablesListproperty ofJsonPathParser. You also said that this attribute stores JSON keys and values without a JSON path, does this mean that thematchmethod is invalid? 这是match的逻辑,在第82行代码,是根据第二个参数即JSON path,获取第一个参数JSON中指定的内容。第二个参数JSON path正是从JsonPathParser的variablesList属性中获取。您也说该属性保存的是JSON键和值,没有JSON path,所以是不是意味着match方法失效了?
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?
if a value in the provided JSON format by the user is $.data.time, ... Could you provide an example illustrating the issue you mentioned?
@pmupkin
I'm a bit confused. Shouldn't the JSON format be like the following: {"a": "1", "b": {"c": "2"}, "d": []}? How is $.data.time in JSON format? Isn't it a JSON Path expression? Do you mean there is a value like $.data.time in JSON?
我有些困惑。JSON格式不应该是像这样吗{"a": "1", "b": {"c": "2"}, "d": []}?$.data.time这怎么是JSON格式呢,这不是JSON Path表达式吗?难道您的意思是在JSON中存在$.data.time这样的value?
$.data.time
if a value in the provided JSON format by the user is $.data.time, ... Could you provide an example illustrating the issue you mentioned?
@pmupkin I'm a bit confused. Shouldn't the JSON format be like the following:
{"a": "1", "b": {"c": "2"}, "d": []}? How is$.data.timein JSON format? Isn't it a JSON Path expression? Do you mean there is a value like$.data.timein JSON?我有些困惑。JSON格式不应该是像这样吗
{"a": "1", "b": {"c": "2"}, "d": []}?$.data.time这怎么是JSON格式呢,这不是JSON Path表达式吗?难道您的意思是在JSON中存在$.data.time这样的value?
Here's a fact: $.data.time is stored as a string value,like "{"time":"$.data.time"}" and ultimately, the $.data.time extracted can be used for JsonPath matching. That's my understanding.
Here's a fact:
$.data.timeis stored as a string value,like "{"time":"$.data.time"}" and ultimately, the $.data.time extracted can be used for JsonPath matching. That's my understanding.
@pmupkin Could you provide one or more complete examples of Transformer rule writing? Then based on them, I will study again the code related to Transformer in your PR #4365 and the code in PR #4622. Thank you.
Addtionally, please pay attention to the first review comment above https://github.com/apache/eventmesh/pull/4637#issuecomment-1851215780.
您能否提供一个或多个完整的Transformer规则编写示例?然后我根据它们,再学习一遍您的PR #4365中有关Transformer的代码,以及PR #4622的代码。谢谢! 另外,请您注意一下上面第一条review意见https://github.com/apache/eventmesh/pull/4637#issuecomment-1851215780 。
Codecov Report
Attention: 36 lines in your changes are missing coverage. Please review.
Comparison is base (
24873dd) 17.28% compared to head (f9cd804) 17.40%. Report is 16 commits behind head on master.
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     
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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 Report
Attention: Patch coverage is 39.49447% with 766 lines in your changes missing coverage. Please review.
Project coverage is 17.40%. Comparing base (
1600c6b) to head (f9cd804). Report is 4691 commits behind head on master.
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4637      +/-   ##
============================================
+ Coverage     16.93%   17.40%   +0.47%     
- Complexity     1413     1762     +349     
============================================
  Files           589      797     +208     
  Lines         25786    29798    +4012     
  Branches       2397     2579     +182     
============================================
+ Hits           4367     5187     +820     
- Misses        20983    24130    +3147     
- Partials        436      481      +45     
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
 
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.
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.
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.