[ISSUE #4099]Refactor re-used constant's usage from dedicated file #4257

Ruhshan · 2023-07-20T10:17:52Z

Motivation

Mentioned file in the issue has some constants that are being used in two different methods. Refactoring was needed to ensure consistency.

Modifications

Added the constants in separate file and used as import in the file of interest.

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? (it's a refactoring)
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

… file

Alonexc · 2023-07-21T01:33:39Z

...-mongodb/src/main/java/org/apache/eventmesh/storage/mongodb/utils/MongodbCloudEventUtil.java

 import io.cloudevents.SpecVersion;
 import io.cloudevents.core.builder.CloudEventBuilder;

+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.*;


checkstyle doesn't seem to support '. *' import.

Thanks for the feedback,

Updated in my last commit.

Pil0tXia · 2023-07-21T02:18:49Z

...ge-mongodb/src/main/java/org/apache/eventmesh/storage/mongodb/constant/MongodbConstants.java

+    public static final String DOC_KEY_VERSION = "version";
+    public static final String DOC_KEY_DATA = "data";
+    public static final String DOC_KEY_ID = "id";
+    public static final String DOC_KEY_SOURCE = "source";
+    public static final String DOC_KEY_TYPE = "type";
+    public static final String DOC_KEY_DATA_CONTENT_TYPE = "datacontenttype";
+    public static final String DOC_KEY_SUBJECT = "subject";


Suggested change

public static final String DOC_KEY_VERSION = "version";

public static final String DOC_KEY_DATA = "data";

public static final String DOC_KEY_ID = "id";

public static final String DOC_KEY_SOURCE = "source";

public static final String DOC_KEY_TYPE = "type";

public static final String DOC_KEY_DATA_CONTENT_TYPE = "datacontenttype";

public static final String DOC_KEY_SUBJECT = "subject";

public static final String CLOUD_EVENT_DOC_VERSION = "version";

public static final String CLOUD_EVENT_DOC_DATA = "data";

public static final String CLOUD_EVENT_DOC_ID = "id";

public static final String CLOUD_EVENT_DOC_SOURCE = "source";

public static final String CLOUD_EVENT_DOC_TYPE = "type";

public static final String CLOUD_EVENT_DOC_DATACONTENTTYPE = "datacontenttype";

public static final String CLOUD_EVENT_DOC_SUBJECT = "subject";

There is no need to add KEY_ in the middle of constants. Considering their usage in both convertToCloudEvent and convertToDocument methods, the suggested naming is more readable.

Thanks for the feedback. Implement in latest commit.

…util

Pil0tXia · 2023-07-21T14:49:14Z

...-mongodb/src/main/java/org/apache/eventmesh/storage/mongodb/utils/MongodbCloudEventUtil.java

+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_DATA;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_DATACONTENTTYPE;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_ID;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_SOURCE;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_SUBJECT;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_TYPE;
+import static org.apache.eventmesh.storage.mongodb.constant.MongodbConstants.CLOUD_EVENT_DOC_VERSION;


I suggest importing the MongodbConstants class only and using Constants.NAME to refer to the constant, which has the advantage of explicitness. Anyone reading the code can immediately understand that the constant NAME comes from the MongodbConstants class. This can reduce cognitive overhead and prevent confusion with constants of the same prefix in other classes.

Thanks for clarifying again. Will definitely use this pattern in future for appropriate scenarios.

And implemented your feedback in the last commit.

…o cloud event util

Pil0tXia

LGTM

codecov · 2023-07-22T15:14:24Z

Codecov Report

Merging #4257 (6f1add9) into master (4b994b0) will increase coverage by 0.10%.
The diff coverage is 70.00%.

❗ Current head 6f1add9 differs from pull request most recent head 3eb4d00. Consider uploading reports for the commit 3eb4d00 to get more accurate results

@@             Coverage Diff              @@
##             master    #4257      +/-   ##
============================================
+ Coverage     16.75%   16.86%   +0.10%     
- Complexity     1415     1426      +11     
============================================
  Files           592      593       +1     
  Lines         26029    26073      +44     
  Branches       2384     2401      +17     
============================================
+ Hits           4362     4398      +36     
- Misses        21230    21234       +4     
- Partials        437      441       +4

Impacted Files	Coverage Δ
...he/eventmesh/runtime/boot/EventMeshHTTPServer.java	`0.52% <0.00%> (ø)`
.../protocol/http/processor/CreateTopicProcessor.java	`0.00% <0.00%> (ø)`
.../protocol/http/processor/DeleteTopicProcessor.java	`0.00% <0.00%> (ø)`
...col/http/processor/QuerySubscriptionProcessor.java	`0.00% <0.00%> (ø)`
...ache/eventmesh/runtime/util/HttpResponseUtils.java	`23.07% <0.00%> (+1.64%)`	⬆️
...apache/eventmesh/runtime/util/ValueComparator.java	`25.00% <ø> (ø)`
...nt/tcp/impl/cloudevent/CloudEventTCPPubClient.java	`6.06% <ø> (ø)`
...nt/tcp/impl/cloudevent/CloudEventTCPSubClient.java	`5.88% <ø> (ø)`
...esh/storage/mongodb/constant/MongodbConstants.java	`0.00% <ø> (ø)`
...h/registry/nacos/service/NacosRegistryService.java	`49.29% <74.28%> (+5.54%)`	⬆️
... and 2 more

... and 8 files with indirect coverage changes

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

mxsm

LGTM

…ile (apache#4257) * [ISSUE apache#4099]: Refactor re-used constant's usage from dedicated file * [ISSUE apache#4099] Rename some constants for mongodb storage plugin * [ISSUE apache#4099] Refactor constant imports in mongodb cloud event util * [ISSUE apache#4099]: Replace contants imports to class import in mongo cloud event util

* feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * [ISSUE #4023]Comparison using reference equality instead of value equality.[Operation] (#4276) * [ISSUE #4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] (#4270) * [ISSUE #4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE #4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE #4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE #4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE #4099]Refactor re-used constant's usage from dedicated file (#4257) * [ISSUE #4099]: Refactor re-used constant's usage from dedicated file * [ISSUE #4099] Rename some constants for mongodb storage plugin * [ISSUE #4099] Refactor constant imports in mongodb cloud event util * [ISSUE #4099]: Replace contants imports to class import in mongo cloud event util * fix import order. * fix import order. * fix import order. * fix import order. --------- Co-authored-by: Fabian Bao <[email protected]> Co-authored-by: kyooosukedn <[email protected]> Co-authored-by: Sam V Jose <[email protected]> Co-authored-by: ruhshan <[email protected]>

…ile (apache#4257) * [ISSUE apache#4099]: Refactor re-used constant's usage from dedicated file * [ISSUE apache#4099] Rename some constants for mongodb storage plugin * [ISSUE apache#4099] Refactor constant imports in mongodb cloud event util * [ISSUE apache#4099]: Replace contants imports to class import in mongo cloud event util

* feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * feat: add redis connector. * [ISSUE apache#4023]Comparison using reference equality instead of value equality.[Operation] (apache#4276) * [ISSUE apache#4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] (apache#4270) * [ISSUE apache#4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE apache#4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE apache#4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE apache#4269] Used switch to replace the if-else. [MeshMessageProtocolAdaptor] * [ISSUE apache#4099]Refactor re-used constant's usage from dedicated file (apache#4257) * [ISSUE apache#4099]: Refactor re-used constant's usage from dedicated file * [ISSUE apache#4099] Rename some constants for mongodb storage plugin * [ISSUE apache#4099] Refactor constant imports in mongodb cloud event util * [ISSUE apache#4099]: Replace contants imports to class import in mongo cloud event util * fix import order. * fix import order. * fix import order. * fix import order. --------- Co-authored-by: Fabian Bao <[email protected]> Co-authored-by: kyooosukedn <[email protected]> Co-authored-by: Sam V Jose <[email protected]> Co-authored-by: ruhshan <[email protected]>

[ISSUE apache#4099]: Refactor re-used constant's usage from dedicated…

bfd33d0

… file

Alonexc reviewed Jul 21, 2023

View reviewed changes

Pil0tXia requested changes Jul 21, 2023

View reviewed changes

Ruhshan added 2 commits July 21, 2023 19:19

[ISSUE apache#4099] Rename some constants for mongodb storage plugin

8f18c78

[ISSUE apache#4099] Refactor constant imports in mongodb cloud event …

d18c8e9

…util

Ruhshan requested review from Alonexc and Pil0tXia July 21, 2023 13:58

Pil0tXia requested changes Jul 21, 2023

View reviewed changes

[ISSUE apache#4099]: Replace contants imports to class import in mong…

3eb4d00

…o cloud event util

Ruhshan requested a review from Pil0tXia July 21, 2023 18:22

Pil0tXia approved these changes Jul 22, 2023

View reviewed changes

mxsm approved these changes Jul 23, 2023

View reviewed changes

Alonexc approved these changes Jul 24, 2023

View reviewed changes

xwm1992 changed the title ~~[ISSUE #4099]: Refactor re-used constant's usage from dedicated file~~ [ISSUE #4099]Refactor re-used constant's usage from dedicated file Jul 25, 2023

xwm1992 approved these changes Jul 25, 2023

View reviewed changes

xwm1992 merged commit 984eacc into apache:master Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ISSUE #4099]Refactor re-used constant's usage from dedicated file #4257

[ISSUE #4099]Refactor re-used constant's usage from dedicated file #4257

Uh oh!

Ruhshan commented Jul 20, 2023

Uh oh!

Alonexc Jul 21, 2023

Uh oh!

Ruhshan Jul 21, 2023

Uh oh!

Pil0tXia Jul 21, 2023

Uh oh!

Ruhshan Jul 21, 2023

Uh oh!

Pil0tXia Jul 21, 2023

Uh oh!

Ruhshan Jul 21, 2023

Uh oh!

Pil0tXia left a comment

Uh oh!

codecov bot commented Jul 22, 2023 •

edited

Loading

Uh oh!

mxsm left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[ISSUE #4099]Refactor re-used constant's usage from dedicated file #4257

[ISSUE #4099]Refactor re-used constant's usage from dedicated file #4257

Uh oh!

Conversation

Ruhshan commented Jul 20, 2023

Motivation

Modifications

Documentation

Uh oh!

Alonexc Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Ruhshan Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Pil0tXia Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Ruhshan Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Pil0tXia Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Ruhshan Jul 21, 2023

Choose a reason for hiding this comment

Uh oh!

Pil0tXia left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

mxsm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Jul 22, 2023 •

edited

Loading