Ptt-official-app / Ptt-backend

PTT APP 的後端
BSD 3-Clause "New" or "Revised" License
208 stars 68 forks source link

增加 smtp provider 回傳 #198

Closed y2468101216 closed 3 years ago

y2468101216 commented 3 years ago

👏 解決掉的 issue / Resolved Issues

📝 相關的 issue / Related Issues

⛏ 變更內容 / Details of Changes

增加 smtp provider

codecov-commenter commented 3 years ago

Codecov Report

Merging #198 (7a28650) into development (0dc9641) will increase coverage by 1.43%. The diff coverage is 77.27%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #198      +/-   ##
===============================================
+ Coverage        45.20%   46.64%   +1.43%     
===============================================
  Files               25       27       +2     
  Lines             1294     1342      +48     
===============================================
+ Hits               585      626      +41     
- Misses             623      628       +5     
- Partials            86       88       +2     
Impacted Files Coverage Δ
internal/usecase/article.go 52.38% <ø> (+7.38%) :arrow_up:
internal/mail/mail.go 50.00% <55.55%> (ø)
internal/mail/smtp.go 90.00% <90.00%> (ø)
internal/usecase/mail.go 100.00% <100.00%> (ø)
internal/usecase/usecase.go 100.00% <100.00%> (ø)
internal/repository/user.go 1.85% <0.00%> (-0.11%) :arrow_down:
internal/repository/article.go 0.00% <0.00%> (ø)
internal/repository/repository.go 0.00% <0.00%> (ø)
internal/delivery/http/route_token.go 13.95% <0.00%> (ø)
internal/delivery/http/route_boards.go 56.57% <0.00%> (ø)
... and 5 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 0dc9641...7a28650. Read the comment docs.

y2468101216 commented 3 years ago

有幾個地方要討論的

  1. usecase.go 裡的 NewUsecase 在取得 mail provider 沒有地方可以回傳 error
  2. sendmail 不直接支援 tls sendmail ,必須用587 port 做跳轉
SivWatt commented 3 years ago
1. usecase.go 裡的 NewUsecase 在取得 mail provider 沒有地方可以回傳 error

以現況來說 parse url fail 會回一個error 如果這會讓後面的功能走不下去 NewUsecase 就應該也要回error 不然後面拿nil 的mail去Send也遲早會 panic

另外,newSMTPProvider() function 裡面現在看起來不會產生 error 我覺得這種情況就可以不用return error

2. sendmail 不直接支援 tls sendmail ,必須用587 port 做跳轉

Example 1 Example 2 這邊有兩個tls 的example 不過我沒有實際測過,類似的我只玩過FTP XD

y2468101216 commented 3 years ago
1. usecase.go 裡的 NewUsecase 在取得 mail provider 沒有地方可以回傳 error

以現況來說 parse url fail 會回一個error 如果這會讓後面的功能走不下去 NewUsecase 就應該也要回error 不然後面拿nil 的mail去Send也遲早會 panic

另外,newSMTPProvider() function 裡面現在看起來不會產生 error 我覺得這種情況就可以不用return error

發信功能不是必須,不過我覺得還是要給架設的人知道他哪裡設錯了,我目前還在考慮這個問題要怎處理 應該說這似乎牽扯到 usercase 是否要因為設定錯誤處理 err 問題,也許該另開 issue 討論。

2. sendmail 不直接支援 tls sendmail ,必須用587 port 做跳轉

Example 1 Example 2 這邊有兩個tls 的example 不過我沒有實際測過,類似的我只玩過FTP XD

這個問題其實是應該要改為是否要改為使用 tls,因為其實發信這件事情是 server to server,被中間人攻擊的可能性較低