Skip to content

Conversation

shendongsd
Copy link
Contributor

[#2863] fix the problem of potential NPE in ACL plain and revise codeStyle

@coveralls
Copy link

coveralls commented May 14, 2021

Coverage Status

Coverage increased (+1.7%) to 54.099% when pulling e320775 on shendongsd:develop into be6eaf8 on apache:develop.

private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);

private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

Do you check your style correct, just like here.?#2899

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you check your style correct, just like here.?#2899

yes, i checked

JSONObject.class);
if (plainAclConfData == null || plainAclConfData.isEmpty()) {
throw new AclException(String.format("%s file is not data", fileHome + File.separator + fileName));
throw new AclException(String.format("%s file is not data", fileHome + File.separator + fileName));
Copy link
Member

Choose a reason for hiding this comment

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

the middle of file and is has two blanks, right?

Map.class);

if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
throw new AclException(String.format("%s file is not data", fileHome + File.separator + fileName));
Copy link
Member

Choose a reason for hiding this comment

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

what is file is not data, is it right sematic?

@codecov-commenter
Copy link

codecov-commenter commented May 20, 2021

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 47.86%. Comparing base (be6eaf8) to head (e320775).
Report is 7192 commits behind head on develop.

Files with missing lines Patch % Lines
...che/rocketmq/acl/plain/PlainPermissionManager.java 50.00% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #2893      +/-   ##
=============================================
+ Coverage      46.67%   47.86%   +1.19%     
- Complexity      4418     4538     +120     
=============================================
  Files            552      552              
  Lines          36500    36524      +24     
  Branches        4828     4834       +6     
=============================================
+ Hits           17035    17481     +446     
+ Misses         17344    16817     -527     
- Partials        2121     2226     +105     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


Map<String, Object> newAccountsMap = null;
if (existedAccountMap == null) {
if (existedAccoutMap == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

accout -> account

Map.class);

if (aclAccessConfigMap == null || aclAccessConfigMap.isEmpty()) {
throw new AclException(String.format("%s file not found or empty", fileHome + File.separator + fileName));
Copy link
Member

Choose a reason for hiding this comment

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

"the xxx file is not found or empty " seems better.

@duhenglucky duhenglucky merged commit d21f4d3 into apache:develop May 23, 2021
@zongtanghu zongtanghu added this to the 4.9.0 milestone Jun 4, 2021
GenerousMan pushed a commit to GenerousMan/rocketmq that referenced this pull request Aug 12, 2022
fix the problem of potential NPE in ACL plain
pulllock pushed a commit to pulllock/rocketmq that referenced this pull request Oct 19, 2023
fix the problem of potential NPE in ACL plain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants