[ISSUE #4474] Uniformly manage duplicate static variables #4495

VishalMCF · 2023-10-19T11:53:47Z

Motivation

There were duplicate final static constants that could have been created in a single common file and could be reused anywhere in the project.

Modifications

The common file which is suitable for hosting the static constants was found to be inside the package -> package org.apache.eventmesh.common. The file is the Constants.java file. There I added the following String constants ->
{ HTTP = "HTTP", TCP = "TCP", GRPC = "GRPC"}
Other than this there is one more constant in the eventmesh-common.Constants file called ( CLOUD_EVENTS_PROTOCOL_NAME = "cloudevents" ) which I have added wherever applicable.
and changed the import statements accordingly.

Documentation

Does this pull request introduce a new feature? no
If yes, how is the feature documented? not applicable
If a feature is not applicable for documentation, explain why? PR deals with only refactoring

…entmesh.common module

VishalMCF · 2023-10-19T11:58:50Z

@mxsm I ran the following command -> ./gradlew build -x test and it worked fine.

But for a few modules, there were test-case failures. I can name them here as a reference. Is there any specific reason for that? Because I have made only small changes and if the compilation was successful then my changes should not have any impact on the test cases. Please suggest accordingly. I will look further into more detail if my changes are causing the issue.

eventmesh-common -> IPUtilsTest
evemtmesh-runtime
eventmesh-sdks
eventmesh-storage-plugin
eventmesh-trace-plugin

Pil0tXia · 2023-10-19T12:34:46Z

@VishalMCF You may checkout master branch, run the gradlew command again and try to reproduce it.

Pil0tXia · 2023-10-19T14:43:48Z

...ks/eventmesh-sdk-java/src/main/java/org/apache/eventmesh/client/tcp/common/MessageUtils.java

            Preconditions.checkNotNull(cloudEvent.getDataContentType(), "DateContentType cannot be null");
-            msg.getHeader().putProperty(Constants.PROTOCOL_TYPE, EventMeshCommon.CLOUD_EVENTS_PROTOCOL_NAME);
+            msg.getHeader().putProperty(Constants.PROTOCOL_TYPE, CLOUD_EVENTS_PROTOCOL_NAME);
            msg.getHeader().putProperty(Constants.PROTOCOL_VERSION, cloudEvent.getSpecVersion().toString());


Coding Style differs here. Please use static import only when this class uses single constant class.

codecov · 2023-10-19T15:02:59Z

Codecov Report

Merging #4495 (7a5fd63) into master (f629730) will increase coverage by 0.01%.
The diff coverage is 0.00%.

❗ Current head 7a5fd63 differs from pull request most recent head b39ef9f. Consider uploading reports for the commit b39ef9f to get more accurate results

@@             Coverage Diff              @@
##             master    #4495      +/-   ##
============================================
+ Coverage     15.46%   15.47%   +0.01%     
  Complexity     1452     1452              
============================================
  Files           691      691              
  Lines         28101    28106       +5     
  Branches       2624     2626       +2     
============================================
+ Hits           4345     4349       +4     
- Misses        23309    23312       +3     
+ Partials        447      445       -2

Files	Coverage Δ
...in/java/org/apache/eventmesh/common/Constants.java	`75.00% <ø> (ø)`
...entmesh/common/utils/ConfigurationContextUtil.java	`70.00% <ø> (ø)`
...ics/prometheus/metrics/PrometheusGrpcExporter.java	`0.00% <ø> (ø)`
...ics/prometheus/metrics/PrometheusHttpExporter.java	`0.00% <ø> (ø)`
...rics/prometheus/metrics/PrometheusTcpExporter.java	`0.00% <ø> (ø)`
.../prometheus/utils/PrometheusExporterConstants.java	`0.00% <ø> (ø)`
...apache/eventmesh/runtime/boot/EventMeshServer.java	`0.00% <ø> (ø)`
...apache/eventmesh/client/http/ProtocolConstant.java	`0.00% <ø> (ø)`
...e/eventmesh/client/tcp/common/EventMeshCommon.java	`0.00% <ø> (ø)`
...e/eventmesh/common/config/CommonConfiguration.java	`83.33% <0.00%> (ø)`
... and 9 more

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pandaapo · 2023-10-19T15:16:03Z

Have all duplicate variables been optimized? If not, it is recommended that you create a new issue as a sub issue of issue #4474 and link this PR to the new issue.

Pil0tXia

I don't think this PR has done all it can do. @VishalMCF Please optimize it further when you have time~

Pil0tXia · 2023-10-20T02:00:56Z

...eventmesh-sdk-java/src/main/java/org/apache/eventmesh/client/tcp/common/EventMeshCommon.java


-    // protocol type
-    public static final String CLOUD_EVENTS_PROTOCOL_NAME = "cloudevents";
    public static final String EM_MESSAGE_PROTOCOL_NAME = "eventmeshmessage";
    public static final String OPEN_MESSAGE_PROTOCOL_NAME = "openmessage";
 }


cloudevents removed but eventmeshmessage and openmessage remaining here.

Pil0tXia · 2023-10-20T02:03:07Z

...sdks/eventmesh-sdk-java/src/main/java/org/apache/eventmesh/client/http/ProtocolConstant.java

 public final class ProtocolConstant {

-    public static final String CE_PROTOCOL = "cloudevents";
    public static final String EM_MESSAGE_PROTOCOL = "eventmeshmessage";
    public static final String OP_MESSAGE_PROTOCOL = "openmessage";
    public static final String PROTOCOL_DESC = "http";


Another cloudevents removed but eventmeshmessage and openmessage remaining here. The protocols are torn apart and still duplicate.

Pil0tXia · 2023-10-20T02:04:28Z

...ks/eventmesh-sdk-java/src/main/java/org/apache/eventmesh/client/tcp/common/MessageUtils.java

 package org.apache.eventmesh.client.tcp.common;

+import static org.apache.eventmesh.common.Constants.CLOUD_EVENTS_PROTOCOL_NAME;
+
 import org.apache.eventmesh.common.Constants;
 import org.apache.eventmesh.common.protocol.SubscriptionItem;


Static imports should not be used here, because Constants class has already been imported.

…he#4495) * 4474 -> moved common constants to appache-common * shifted common static variables to a common Constants.java file in eventmesh.common module * removed unused constant from the file. * refactored constants related to HTTP, GRPC, TCP in the relevant files

VishalMCF added 4 commits October 19, 2023 13:39

4474 -> moved common constants to appache-common

7a4c891

shifted common static variables to a common Constants.java file in ev…

0fbbef6

…entmesh.common module

removed unused constant from the file.

d067801

refactored constants related to HTTP, GRPC, TCP in the relevant files

b39ef9f

pandaapo changed the title ~~ISSUE #4474 -> moved common constants to eventmesh-common.Constants file.~~ [ISSUE #4474] Uniformly manage duplicate static variables Oct 19, 2023

Pil0tXia requested changes Oct 19, 2023

View reviewed changes

pandaapo approved these changes Oct 19, 2023

View reviewed changes

xwm1992 approved these changes Oct 20, 2023

View reviewed changes

xwm1992 merged commit d2531b2 into apache:master Oct 20, 2023

mxsm mentioned this pull request Oct 20, 2023

[Enhancement]Uniformly manage duplicate static variables #4474

Closed

3 tasks

Pil0tXia reviewed Oct 20, 2023

View reviewed changes

Pil0tXia mentioned this pull request Oct 20, 2023

[ISSUE #4494] RESTful API framework for EventMeshAdmin #4498

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ISSUE #4474] Uniformly manage duplicate static variables #4495

[ISSUE #4474] Uniformly manage duplicate static variables #4495

Uh oh!

VishalMCF commented Oct 19, 2023 •

edited by mxsm

Loading

Uh oh!

VishalMCF commented Oct 19, 2023

Uh oh!

Pil0tXia commented Oct 19, 2023 •

edited

Loading

Uh oh!

Pil0tXia Oct 19, 2023

Uh oh!

codecov bot commented Oct 19, 2023 •

edited

Loading

Uh oh!

pandaapo commented Oct 19, 2023

Uh oh!

Pil0tXia left a comment •

edited

Loading

Uh oh!

Pil0tXia Oct 20, 2023

Uh oh!

Pil0tXia Oct 20, 2023

Uh oh!

Pil0tXia Oct 20, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[ISSUE #4474] Uniformly manage duplicate static variables #4495

[ISSUE #4474] Uniformly manage duplicate static variables #4495

Uh oh!

Conversation

VishalMCF commented Oct 19, 2023 • edited by mxsm Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Documentation

Uh oh!

VishalMCF commented Oct 19, 2023

Uh oh!

Pil0tXia commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pil0tXia Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pandaapo commented Oct 19, 2023

Uh oh!

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

Pil0tXia Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

Pil0tXia Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

Pil0tXia Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

VishalMCF commented Oct 19, 2023 •

edited by mxsm

Loading

Pil0tXia commented Oct 19, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading

Pil0tXia left a comment •

edited

Loading