Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mail): support the use of markdown in the email body #726

Merged
merged 16 commits into from
Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ dependencies {
implementation("com.amazonaws:aws-java-sdk-ses:1.11.880")
implementation("com.amazonaws:aws-java-sdk-sqs:1.12.+")
implementation("io.github.microutils:kotlin-logging-jvm:3.0.0")
implementation("org.jetbrains:markdown:0.5.0")
compileOnly("io.jsonwebtoken:jjwt-api:0.11.2")
runtimeOnly("io.jsonwebtoken:jjwt-impl:0.11.2")
runtimeOnly("io.jsonwebtoken:jjwt-jackson:0.11.2")
Expand Down
5 changes: 5 additions & 0 deletions src/main/kotlin/apply/application/mail/MailData.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package apply.application.mail

import apply.domain.mail.MailHistory
import apply.utils.MarkdownProcessor
import org.springframework.core.io.ByteArrayResource
import java.time.LocalDateTime
import javax.validation.constraints.NotEmpty
Expand Down Expand Up @@ -38,4 +39,8 @@ data class MailData(
mailHistory.sentTime,
id = mailHistory.id
)

fun getHtmlBody(): String {
return MarkdownProcessor.generateToHtml(body)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 로직이 복잡하지 않고 위임만 하면 MailService에서 직접 호출할 수 있을 것 같은데, 어떻게 생각하세요?

Copy link
Author

Choose a reason for hiding this comment

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

DTO에 Markdown 의존성이 생기는 것에 공감하여 이동하였습니다.

}
22 changes: 13 additions & 9 deletions src/main/kotlin/apply/application/mail/MailService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,7 @@ class MailService(

@Async
fun sendMailsByBcc(request: MailData, files: Map<String, ByteArrayResource>) {
val context = Context().apply {
setVariables(
mapOf(
"content" to request.body,
"url" to applicationProperties.url
)
)
}
val body = templateEngine.process("mail/common", context)
val body = createCommonMailBodyFrom(request)
val recipients = request.recipients + mailProperties.username

// TODO: 성공과 실패를 분리하여 히스토리 관리
Expand All @@ -119,4 +111,16 @@ class MailService(
)
)
}

fun createCommonMailBodyFrom(mailData: MailData): String {
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 보다 간결하고 명확한 함수 이름을 제안해 봐요!

Suggested change
fun createCommonMailBodyFrom(mailData: MailData): String {
fun String.toMarkdownBody(): String {

Copy link
Author

Choose a reason for hiding this comment

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

리턴 되는 문자열이 html이라 toHtmlBody로 변경했어요.

Copy link
Contributor

Choose a reason for hiding this comment

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

r: MailService 외에 사용하는 곳이 없어 보이네요. public이 아닌 private으로 바꾸면 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

미리보기 기능을 만들 때 필요해서 public으로 두었습니다.

val context = Context().apply {
setVariables(
mapOf(
"content" to mailData.getHtmlBody(),
"url" to applicationProperties.url
)
)
}
return templateEngine.process("mail/common", context)
}
}
24 changes: 24 additions & 0 deletions src/main/kotlin/apply/infra/mail/FakeMailSender.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package apply.infra.mail

import apply.application.mail.MailSender
import mu.KotlinLogging
import org.springframework.core.io.ByteArrayResource

private val logger = KotlinLogging.logger { }

class FakeMailSender : MailSender {
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 가짜 객체는 테스트 디렉터리로 이동해야 하지 않을까요?

override fun send(toAddress: String, subject: String, body: String) {
logger.info("send mail: to - {}, subject - {}", toAddress, subject)
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 코틀린 스타일로 작성해 볼까요?

logger.debug("hello {}", heavyWorld(13)) // 항상 연산
logger.debug { "hello ${heavyWorld(33)}" } // debug 레벨 로그 남길 때만 연산

logger.info("body: {}", body)
}

override fun sendBcc(
toAddresses: List<String>,
subject: String,
body: String,
attachments: Map<String, ByteArrayResource>
) {
logger.info("send mail: to - {}, subject - {}", toAddresses, subject)
logger.info("body: {}", body)
}
}
14 changes: 14 additions & 0 deletions src/main/kotlin/apply/utils/MarkdownProcessor.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package apply.utils
Copy link
Contributor

Choose a reason for hiding this comment

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

r: apply.utils 대신 support로 옮기는 것은 어떨까요? support는 이 프로젝트에만 필요한 것이 아니라 일반적으로 사용할 수 있는 것들의 모음입니다.

Copy link
Author

Choose a reason for hiding this comment

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

마크다운과 관련된 기능은 support로 옮기고, 기존 기능에 대한 변경을 위해 이슈 생성해뒀습니다.
#729


import org.intellij.markdown.flavours.commonmark.CommonMarkFlavourDescriptor
import org.intellij.markdown.html.HtmlGenerator
import org.intellij.markdown.parser.MarkdownParser

object MarkdownProcessor {
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 객체로 만든 이유가 있을까요? 유틸리티 함수는 가급적 최상위 함수로 만드는 것이 가장 좋으며, 함수 이름을 직접 호출하기 어려운 경우(e.g. 함수 이름이 바로 기억나지 않아 힌트를 주고 싶을 때) 객체로 한 번 더 감싸는 것이 좋습니다.

private val flavour = CommonMarkFlavourDescriptor()
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 타입 선언이 빠졌습니다. 지역 변수를 제외한 모든 곳에 타입을 선언해 주세요.

Suggested change
private val flavour = CommonMarkFlavourDescriptor()
private val flavour: MarkdownFlavourDescriptor = CommonMarkFlavourDescriptor()


fun generateToHtml(source: String): String {
Copy link
Contributor

Choose a reason for hiding this comment

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

a: text 또는 markdownText와 같이 사용 중인 API에서 지정한 매개 변수 이름을 따르는 것은 어떨까요?

val parsedTree = MarkdownParser(flavour).buildMarkdownTreeFromString(source)
return HtmlGenerator(source, parsedTree, flavour).generateHtml()
}
}
2 changes: 1 addition & 1 deletion src/main/resources/templates/mail/common.html
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
padding-right: 8px;
"
>
<span th:utext="${content}"></span>
[(${content})]
Copy link
Contributor

Choose a reason for hiding this comment

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

a: 이 문법은 무슨 뜻인가요?

Copy link
Author

Choose a reason for hiding this comment

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

content가 html string이여서 text inlining 을 적용했습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

첨부하신 문서는 Thymeleaf 2.1 버전이라서 찾을 수 없네요! 3.x의 기능인 것 같은데, 스프링 부트는 오랫동안 3.x를 사용해 왔더라고요.
https://www.thymeleaf.org/doc/tutorials/3.1/usingthymeleaf.html#expression-inlining

</td>
</tr>
</table>
Expand Down
30 changes: 30 additions & 0 deletions src/test/kotlin/apply/application/mail/MailServiceTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package apply.application.mail

import apply.config.TestMailConfig
import apply.createMailData
import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.matchers.string.shouldContain
import org.springframework.test.context.ContextConfiguration
import support.test.IntegrationTest

@IntegrationTest
@ContextConfiguration(classes = [TestMailConfig::class])
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 자주 변경될 수 있는 애너테이션은 상단으로 옮기는 것이 좋습니다.

class MailServiceTest(
private val mailService: MailService
) : BehaviorSpec({

Given("MailData를 사용해서 메일을 보내는 경우") {
val body = "안녕하세요. 우아한테크코스입니다.\n" + "\n" +
"1주 차 미션은 개발 환경을 세팅하고, GitHub에 과제를 제출하는 등 미션 외에도 추가로 익혀야 하는 부분들이 있어 가벼운 미션으로 준비했어요."
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 수많은 이스케이프가 필요한 문자열의 경우 3중 따옴표 문자열을 사용하면 더 깔끔하게 표현할 수 있습니다.
https://kotlinlang.org/docs/strings.html#multiline-strings

val mailData = createMailData(body = body)

When("common 메일 본문에 적용하면") {
val commonMailBody = mailService.createCommonMailBodyFrom(mailData)

Then("본문이 포함된 HTML로 작성된다.") {
println(commonMailBody)
Copy link
Contributor

Choose a reason for hiding this comment

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

r: 콘솔로 출력하는 대신 테스트 코드를 더 보완하면 어떨까요?

commonMailBody shouldContain "<body><p>안녕하세요. 우아한테크코스입니다."
}
}
}
})
16 changes: 16 additions & 0 deletions src/test/kotlin/apply/config/TestMailConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package apply.config

import apply.application.mail.MailSender
import apply.infra.mail.FakeMailSender
import org.springframework.boot.test.context.TestConfiguration
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Primary

@TestConfiguration
class TestMailConfig {
@Primary
@Bean
fun testMailSender(): MailSender {
return FakeMailSender()
}
}
29 changes: 29 additions & 0 deletions src/test/kotlin/apply/utils/MarkdownProcessorTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package apply.utils

import io.kotest.core.spec.style.StringSpec
import io.kotest.matchers.string.shouldContain

class MarkdownProcessorTest : StringSpec({
"마크다운 문법으로 작성된 내용을 HTML로 변환할 수 있다" {
val src = "안녕하세요. 우아한테크코스입니다.\n" +
"3주 차 미션에서는 2주 차에서 학습한 것에 2가지 목표를 추가했어요.\n" +
"1. 클래스(객체)를 분리하는 연습\n" +
"2. 도메인 로직에 대한 단위 테스트를 작성하는 연습\n" +
"## 프리코스 진행방식\n" +
"### 미션 제출 방법\n" +
"- 미션 구현을 완료한 후 GitHub을 통해 제출해야 한다.\n" +
"- GitHub을 활용한 제출 방법은 " +
"[문서](https://github.com/woowacourse/woowacourse-docs/tree/master/precourse)를 참고해 제출한다.\n" +
"- GitHub에 미션을 제출한 후 " +
"[우아한테크코스 지원 플랫폼](https://apply.techcourse.co.kr/)에서 프리코스 과제를 제출한다.\n" +
"- 지원 플랫폼에서 과제를 제출할 때 미션을 진행하며 경험한 내용에 대한 소감문을 작성한다."

val html = MarkdownProcessor.generateToHtml(src)

println(html)
html shouldContain "<ul>"
html shouldContain "<li>클래스(객체)를 분리하는 연습</li>"
html shouldContain "<h3>"
html shouldContain "<a href"
}
})