Skip to content

[BUG#2359][Alert] Mail Chinese name is too long garbled#3199

Merged
davidzollo merged 4 commits intoapache:devfrom
johnnygo920:dev
Jul 14, 2020
Merged

[BUG#2359][Alert] Mail Chinese name is too long garbled#3199
davidzollo merged 4 commits intoapache:devfrom
johnnygo920:dev

Conversation

@johnnygo920
Copy link
Copy Markdown
Contributor

@johnnygo920 johnnygo920 commented Jul 13, 2020

Brief change log

  • Set the system property of "mail.mime.splitlongparameters" is false in the class "org.apache.dolphinscheduler.alert.utils.MailUtils"

Verify this pull request

This pull request is already covered by existing tests.

  • Manually verified the change by testing locally.

local test method : org.apache.dolphinscheduler.alert.utils.MailUtilsTest#testSendMails
situation one: Chinese 100. result: success
image

situation two: Chinese 100 + English 100. result: success
image

other situation: Whether I set this property or not, and whether the name contains Chinese or not.when the length of attachment is too long, the mail will send failed. ( i have been test the situation that the length is 500, but i'm not sure the max length,because the exception is thrown by a native method)

@johnnygo920
Copy link
Copy Markdown
Contributor Author

johnnygo920 commented Jul 13, 2020

#2359

@davidzollo davidzollo linked an issue Jul 13, 2020 that may be closed by this pull request
[BUG]The attachment name in excel Chinese is too long, and the attachment name in the received email becomes a string of characters #2359
Closed
CalvinKirs
CalvinKirs approved these changes Jul 13, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@CalvinKirs CalvinKirs left a comment

Choose a reason for hiding this comment

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

LGTM.is it better to change the title to [BUG#2359][Alert] Mail Chinese name is too long garbled?

@johnnygo920 johnnygo920 changed the title Solve the problem of messy Chinese name in excel attachment(issue:2359) [BUG#2359][Alert] Mail Chinese name is too long garbled Jul 13, 2020
@yangyichao-mango
Copy link
Copy Markdown
Contributor

yangyichao-mango commented Jul 13, 2020

Hi,
Thx a lot for your contribution.
LGTM.

qiaozhanwei
qiaozhanwei reviewed Jul 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@qiaozhanwei qiaozhanwei left a comment

Choose a reason for hiding this comment

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

SonarCloud Code Analysis is not running . please retry commit . Thx

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 13, 2020

Codecov Report

Merging #3199 into dev will decrease coverage by 5.91%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                dev    #3199      +/-   ##
============================================
- Coverage     39.70%   33.79%   -5.92%     
+ Complexity     2723     2405     -318     
============================================
  Files           436      442       +6     
  Lines         20172    20605     +433     
  Branches       2456     2524      +68     
============================================
- Hits           8010     6964    -1046     
- Misses        11405    12980    +1575     
+ Partials        757      661      -96
Impacted Files Coverage Δ Complexity Δ
...apache/dolphinscheduler/alert/utils/MailUtils.java 44.64% <100.00%> (-28.09%) 8.00 <0.00> (-13.00)
...er/server/builder/TaskExecutionContextBuilder.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-10.00%)
...rver/master/processor/queue/TaskResponseEvent.java 0.00% <0.00%> (-100.00%) 0.00% <0.00%> (-23.00%)
...duler/server/entity/DataxTaskExecutionContext.java 0.00% <0.00%> (-95.00%) 0.00% <0.00%> (-13.00%)
...g/apache/dolphinscheduler/remote/command/Ping.java 0.00% <0.00%> (-87.50%) 0.00% <0.00%> (-2.00%)
...phinscheduler/server/log/TaskLogDiscriminator.java 0.00% <0.00%> (-87.50%) 0.00% <0.00%> (-7.00%)
.../dolphinscheduler/server/utils/FlinkArgsUtils.java 0.00% <0.00%> (-87.04%) 0.00% <0.00%> (-10.00%)
...rver/master/dispatch/context/ExecutionContext.java 0.00% <0.00%> (-86.67%) 0.00% <0.00%> (-7.00%)
...lphinscheduler/remote/future/ReleaseSemaphore.java 0.00% <0.00%> (-85.72%) 0.00% <0.00%> (-2.00%)
...nscheduler/server/entity/TaskExecutionContext.java 16.83% <0.00%> (-82.18%) 10.00% <0.00%> (-56.00%)
... and 108 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38e4853...a51ba37. Read the comment docs.

@johnnygo920
Copy link
Copy Markdown
Contributor Author

johnnygo920 commented Jul 14, 2020

SonarCloud Code Analysis is not running . please retry commit . Thx

ok,I have resubmitted it

liukunyuan
liukunyuan reviewed Jul 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@liukunyuan liukunyuan left a comment

Choose a reason for hiding this comment

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

ok

davidzollo
davidzollo approved these changes Jul 14, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@davidzollo davidzollo left a comment

Choose a reason for hiding this comment

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

+1
good job

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Jul 14, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@davidzollo davidzollo merged commit 1dfde72 into apache:dev Jul 14, 2020
tswstarplanet pushed a commit to tswstarplanet/incubator-dolphinscheduler that referenced this pull request Jul 15, 2020
@johnnygo920 @davidzollo @tswstarplanet
[BUG#2359][Alert] Mail Chinese name is too long garbled (apache#3199)
c9c47f8 
* 解决中文附件名过长时邮件附件中文乱码问题

* Solve the problem of messy Chinese name in excel attachment

* SonarCloud Code Analysis is not running . retry commit

* Update MailUtils.java

Co-authored-by: johnnygo920 <W@ngchen2020>
Co-authored-by: dailidong <dailidong66@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qiaozhanwei qiaozhanwei qiaozhanwei left review comments

@davidzollo davidzollo davidzollo approved these changes

@CalvinKirs CalvinKirs CalvinKirs approved these changes

@yangyichao-mango yangyichao-mango yangyichao-mango approved these changes

+1 more reviewer

@liukunyuan liukunyuan liukunyuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG]The attachment name in excel Chinese is too long, and the attachment name in the received email becomes a string of characters

7 participants

@johnnygo920 @yangyichao-mango @codecov-commenter @davidzollo @CalvinKirs @qiaozhanwei @liukunyuan