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/#139] 구독시 디스코드 훅 / 신규 구독 안되는 문제 해결 #140

Merged
merged 15 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.few.api.repo.dao.subscription.command.UpdateDeletedAtInWorkbookSubscr
import com.few.api.repo.dao.subscription.query.SelectAllWorkbookSubscriptionStatusQueryNotConsiderDeletedAt
import com.few.api.repo.dao.subscription.record.WorkbookSubscriptionStatus
import com.few.api.repo.dao.subscription.query.CountWorkbookMappedArticlesQuery
import com.few.api.repo.dao.subscription.record.CountAllSubscriptionStatusRecord
import jooq.jooq_dsl.Tables.MAPPING_WORKBOOK_ARTICLE
import jooq.jooq_dsl.Tables.SUBSCRIPTION
import org.jooq.DSLContext
Expand Down Expand Up @@ -68,4 +69,15 @@ class SubscriptionDao(
.where(MAPPING_WORKBOOK_ARTICLE.WORKBOOK_ID.eq(query.workbookId))
.fetchOne(0, Int::class.java)
}

fun countAllSubscriptionStatus(): CountAllSubscriptionStatusRecord {
val total = dslContext.selectCount()
.from(SUBSCRIPTION)
.fetchOne(0, Int::class.java)!!
val active = dslContext.selectCount()
.from(SUBSCRIPTION)
.where(SUBSCRIPTION.DELETED_AT.isNull)
.fetchOne(0, Int::class.java)!!
return CountAllSubscriptionStatusRecord(total.toLong(), active.toLong())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.few.api.repo.dao.subscription.record

data class CountAllSubscriptionStatusRecord(
val totalSubscriptions: Long,
val activeSubscriptions: Long
)
24 changes: 24 additions & 0 deletions api/src/main/kotlin/com/few/api/client/config/ClientConfig.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.few.api.client.config

import org.springframework.beans.factory.annotation.Value
import org.springframework.boot.web.client.RestTemplateBuilder
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
import org.springframework.web.client.RestTemplate
import java.time.Duration

@Configuration
class ClientConfig {

@Bean
fun restTemplate(
restTemplateBuilder: RestTemplateBuilder,
Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

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

RestTemplate 쓰면 동기로 처리하는걸로 알고 있어요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

네 저도 알고 있지만, 지금 우리가 전부 비동기적으로 코드를 작성하고 있는게 아니기 때문에 익숙하지 않은 webclient 보다는 resttemplate을 우선 선택했어요!

@Value("\${client.timeout.connect}") connectTimeout: Int,
@Value("\${client.timeout.read}") readTimeout: Int
): RestTemplate {
return restTemplateBuilder
.setConnectTimeout(Duration.ofSeconds(connectTimeout.toLong()))
.setReadTimeout(Duration.ofSeconds(readTimeout.toLong()))
.build()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.few.api.client.config.properties

data class DiscordBodyProperty(
val content: String,
val embeds: List<Embed>
)

data class Embed(
val title: String,
val description: String
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.few.api.client.subscription

import com.few.api.client.config.properties.DiscordBodyProperty
import com.few.api.client.config.properties.Embed
import com.few.api.client.subscription.dto.WorkbookSubscriptionArgs
import org.springframework.beans.factory.annotation.Value
import org.springframework.http.HttpEntity
import org.springframework.http.HttpMethod
import org.springframework.stereotype.Service
import org.springframework.web.client.RestTemplate

@Service
class SubscriptionClient(
private val restTemplate: RestTemplate,
@Value("\${webhook.discord}") private val discordWebhook: String
) {
fun announceWorkbookSubscription(args: WorkbookSubscriptionArgs) {
args.let {
DiscordBodyProperty(
content = "🎉 신규 구독 알림 ",
Copy link
Member

Choose a reason for hiding this comment

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

content는 따로 파일로 빼는게 좋을듯

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아 그 validation 하는 것 처럼 말이죠?!

Copy link
Member

Choose a reason for hiding this comment

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

일단은 이렇게 가고 컨텐츠가 많아지거나 할 때 변경해도 좋을가 같아요

embeds = listOf(
Embed(
title = "Total Subscriptions",
description = it.totalSubscriptions.toString()
),
Embed(
title = "Active Subscriptions",
description = it.activeSubscriptions.toString()
),
Embed(
title = "Workbook Title",
description = it.workbookTitle
)
)
)
}.let { body ->
restTemplate.exchange(
discordWebhook,
HttpMethod.POST,
HttpEntity(body),
String::class.java
)
Copy link
Member

@hun-ca hun-ca Jul 1, 2024

Choose a reason for hiding this comment

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

여기서 블락됨. 근데 응답 코드는 안가져오나요? 응답 로깅까지만이라도 하면 좋을거 같은데

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

바디가 No-content 라서 로깅을 할 수는 없을 것 같아요
한다면 요청 성고했다 정도?

Copy link
Member

Choose a reason for hiding this comment

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

응답코드 로깅까지만 고고하시죠

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.few.api.client.subscription.dto

data class WorkbookSubscriptionArgs(
val totalSubscriptions: Long,
val activeSubscriptions: Long,
val workbookTitle: String
)
2 changes: 2 additions & 0 deletions api/src/main/kotlin/com/few/api/config/ApiConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import com.few.storage.image.config.ImageStorageConfig
import org.springframework.context.annotation.ComponentScan
import org.springframework.context.annotation.Configuration
import org.springframework.context.annotation.Import
import org.springframework.scheduling.annotation.EnableAsync
import org.springframework.web.servlet.config.annotation.EnableWebMvc

@Configuration
@ComponentScan(basePackages = [ApiConfig.BASE_PACKAGE])
@Import(ApiRepoConfig::class, BatchConfig::class, ImageStorageConfig::class, DocumentStorageConfig::class)
@EnableWebMvc
@EnableAsync
class ApiConfig {
companion object {
const val BASE_PACKAGE = "com.few.api"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.few.api.domain.subscription.event

import com.few.api.client.subscription.SubscriptionClient
import com.few.api.client.subscription.dto.WorkbookSubscriptionArgs
import com.few.api.domain.subscription.event.dto.WorkbookSubscriptionEvent
import com.few.api.domain.subscription.service.WorkbookService
import com.few.api.domain.subscription.service.dto.ReadWorkbookTitleDto
import com.few.api.repo.dao.subscription.SubscriptionDao
import org.springframework.context.event.EventListener
import org.springframework.scheduling.annotation.Async
import org.springframework.stereotype.Component

@Component
class WorkbookSubscriptionEventListener(
private val subscriptionDao: SubscriptionDao,
private val subscriptionClient: SubscriptionClient,
private val workbookService: WorkbookService
) {

@Async
@EventListener
fun handleWorkbookSubscriptionEvent(event: WorkbookSubscriptionEvent) {
val title = ReadWorkbookTitleDto(event.workbookId).let { dto ->
workbookService.readWorkbookTitle(dto)
?: throw RuntimeException("Workbook not found")
}
subscriptionDao.countAllSubscriptionStatus().let { record ->
WorkbookSubscriptionArgs(
totalSubscriptions = record.totalSubscriptions,
activeSubscriptions = record.activeSubscriptions,
workbookTitle = title
).let { args ->
subscriptionClient.announceWorkbookSubscription(args)
}
Comment on lines +20 to +34
Copy link
Member

Choose a reason for hiding this comment

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

@async를 썻기때문에 애플리케이션 전체적인 그림에선 비동기로 동작하겠지만, 결국 restTemplate를 쓰는 부분(실제 I/O 발생하는 부분)에선 블락됩니다. 근데 @async를 써야할 이유가 있을까요? 현재 이 프로세스가 그리 오래걸리는 작업은 아닐걸로 생각되어서 @async로 갈 필요가 있을까라는 생각입니다..

Copy link
Member

@hun-ca hun-ca Jul 1, 2024

Choose a reason for hiding this comment

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

추가로 @async 설정도 추가해주세요. @async로 동작할 쓰레드 풀 개수 등등 (was 쓰레드랑 별도)

Copy link
Member

Choose a reason for hiding this comment

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

아.. 그리고 “만약 디코 통신에서 에러 나면 우리 api도 실패로 처리할 것인가??” 가 중요할거 같은데 물론 저는 아니라고 생각해요. 그럼 try~catch를 활용해야 할거 같아요 이부분도 추가필요해보입니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

스크린샷 2024-07-01 오후 10 44 52 스크린샷 2024-07-01 오후 10 44 19 스크린샷 2024-07-01 오후 10 47 19

첫 번째가 구독 UC
두 번째가 @async가 있는 이벤트 리스너
세 번째가 @async가 없는 이벤트 리스너
에서 트랜잭션 적용을 확인한 것인데

우선 웹훅을 보내는 것은 응답과 관련없는 기능이어 이를 이벤트로 처리하여 이를 분리하는 것이 좋겠다고 생각했습니다.
이벤트 처리기에서는 간단한 정보를 조회하고 이를 디코로 보내는 역할을 할 것인데
사진에서 확인할 수 있는 것처럼 이를 @async 없이 동기적으로 처리한다면 UC의 트랜잭션이 전파되어 디스코드의 에러에도 UC가 실패로 여겨지게 됩니다.
하지만 이벤트로 분리하였는데 이를 try-catch로 감싼다면 차라리 이벤트가 아니라 메서드로 처리하는 것이 좋겠다는 생각이 듭니다.

@async를 통해 비동기로 처리할 때는 별도의 @transactional 전파 속성을 추가하지 않으면 UC의 트렌잭션이 전파되지 않아 3번째 댓글의 걱정은 하지 않아도 될 것 같습니다.

Copy link
Member

Choose a reason for hiding this comment

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

아 네네 결국 제가 단 3번째 댓글로 인해서 어싱크로 가신거군요 좋습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

추가로 @async 설정도 추가해주세요. @async로 동작할 쓰레드 풀 개수 등등 (was 쓰레드랑 별도)

요거는 어떻게 설정하는게 좋을까요?
설정해본적은 있는데 적절한 수치?가 어떻게 되는지는 모르겟어요..ㅠㅠ

	@Bean(REQUEST_ASYNC_EXECUTOR_BEAN_NAME)
	public Executor requestAsyncExecutor(LoggingTaskDecorator taskDecorator) {
		ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
		executor.setCorePoolSize(REQ_CORE_POOL_SIZE); // 풀 사이즈
		executor.setMaxPoolSize(REQ_MAX_POOL_SIZE); // 풀 맥스
		executor.setQueueCapacity(REQ_QUEUE_CAPACITY); // 큐 크기
		executor.setThreadNamePrefix(REQ_THREAD_NAME_PREFIX); // 스레드 프리픽스
		executor.setAwaitTerminationSeconds(AWAIT_TERMINATION_SECONDS); // 대기 초
		executor.setRejectedExecutionHandler(new ThreadPoolExecutor.CallerRunsPolicy());
		executor.setWaitForTasksToCompleteOnShutdown(true); 
		executor.setTaskDecorator(taskDecorator);
		return executor;
	}

이전에는 요정도 설정햇습니다.

Copy link
Member

Choose a reason for hiding this comment

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

block io를 수행할 쓰레드이기 때문에 너무 적으면 안되고 구독 자체가 엄청 자주 발생할것 같진 않으니 적절히 max 10으로 갔다가 부하테스트 하면서 조절하는걸로 하시죠

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#173 에서 따로 처리하겠습니다!

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.few.api.domain.subscription.event.dto

data class WorkbookSubscriptionEvent(
val workbookId: Long
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.few.api.domain.subscription.service

import com.few.api.domain.subscription.service.dto.ReadWorkbookTitleDto
import com.few.api.repo.dao.workbook.WorkbookDao
import com.few.api.repo.dao.workbook.query.SelectWorkBookRecordQuery
import org.springframework.stereotype.Service

@Service
class WorkbookService(
private val workbookDao: WorkbookDao
) {

fun readWorkbookTitle(dto: ReadWorkbookTitleDto): String? {
return SelectWorkBookRecordQuery(dto.workbookId).let { query ->
workbookDao.selectWorkBook(query)?.title
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.few.api.domain.subscription.service.dto

data class ReadWorkbookTitleDto(
val workbookId: Long
)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.few.api.domain.subscription.usecase

import com.few.api.domain.subscription.event.dto.WorkbookSubscriptionEvent
import com.few.api.domain.subscription.service.MemberService
import com.few.api.domain.subscription.service.dto.InsertMemberDto
import com.few.api.domain.subscription.service.dto.ReadMemberIdDto
Expand All @@ -9,13 +10,15 @@ import com.few.api.repo.dao.subscription.query.SelectAllWorkbookSubscriptionStat
import com.few.api.domain.subscription.usecase.`in`.SubscribeWorkbookUseCaseIn
import com.few.api.repo.dao.subscription.query.CountWorkbookMappedArticlesQuery
import com.few.data.common.code.MemberType
import org.springframework.context.ApplicationEventPublisher
import org.springframework.stereotype.Component
import org.springframework.transaction.annotation.Transactional

@Component
class SubscribeWorkbookUseCase(
private val subscriptionDao: SubscriptionDao,
private val memberService: MemberService
private val memberService: MemberService,
private val applicationEventPublisher: ApplicationEventPublisher
) {

// todo 이미 가입된 경우
Expand All @@ -28,28 +31,37 @@ class SubscribeWorkbookUseCase(
)

val subTargetWorkbookId = useCaseIn.workbookId
val command = InsertWorkbookSubscriptionCommand(
memberId = memberId,
workbookId = subTargetWorkbookId
)

/** 구독 히스토리가 있는지 확인 */
SelectAllWorkbookSubscriptionStatusQueryNotConsiderDeletedAt(memberId = memberId, workbookId = subTargetWorkbookId).let { query ->
subscriptionDao.selectAllWorkbookSubscriptionStatus(query).let { subscriptionStatusList ->
/** 이미 구독한 경우가 있는 경우 */
if (subscriptionStatusList.isNotEmpty()) {
subscriptionStatusList.stream().filter { status ->
status.id == query.workbookId
}.findAny().get().let { status ->
InsertWorkbookSubscriptionCommand(memberId = memberId, workbookId = subTargetWorkbookId).let { command ->
if (status.subHistory) {
CountWorkbookMappedArticlesQuery(subTargetWorkbookId).let { query ->
subscriptionDao.countWorkbookMappedArticles(query)
}?.let { lastDay ->
if (lastDay <= (status.day)) {
throw RuntimeException("이미 학습을 완료한 워크북입니다.")
}
subscriptionDao.reSubscribeWorkbookSubscription(command)
if (status.subHistory) {
CountWorkbookMappedArticlesQuery(subTargetWorkbookId).let { query ->
subscriptionDao.countWorkbookMappedArticles(query)
}?.let { lastDay ->
/** 이미 학습을 완료한 경우 */
if (lastDay <= (status.day)) {
throw RuntimeException("이미 학습을 완료한 워크북입니다.")
}
} else {
subscriptionDao.insertWorkbookSubscription(command)
/** 재구독 */
subscriptionDao.reSubscribeWorkbookSubscription(command)
}
}
}
} else {
/** 구독한 경우가 없는 경우 */
subscriptionDao.insertWorkbookSubscription(command)
}
applicationEventPublisher.publishEvent(WorkbookSubscriptionEvent(workbookId = subTargetWorkbookId))
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions api/src/main/resources/application-client-local.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
client:
timeout:
connect: 5000
read: 5000

webhook:
discord: ${WEBHOOK_DISCORD}
7 changes: 7 additions & 0 deletions api/src/main/resources/application-client-prd.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
client:
timeout:
connect: ${TIMEOUT_CONNECT:5000}
read: ${TIMEOUT_READ:5000}
Comment on lines +3 to +4
Copy link
Member

Choose a reason for hiding this comment

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

5000이 5초인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

옙!


webhook:
discord: ${WEBHOOK_DISCORD}
2 changes: 2 additions & 0 deletions api/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ spring:
profiles:
group:
local:
- client-local
- api-repo-local
- email-local
- storage-local
prd:
- client-prd
- api-repo-prd
- email-prd
- storage-prd
Expand Down
3 changes: 3 additions & 0 deletions resources/docker/docker-compose-api.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ services:
IMAGE_STORE_BUCKET_NAME: ${IMAGE_STORE_BUCKET_NAME}
DOCUMENT_STORE_BUCKET_NAME: ${DOCUMENT_STORE_BUCKET_NAME}
CDN_URL: ${CDN_URL}
WEBHOOK_DISCORD: ${WEBHOOK_DISCORD}
TIMEOUT_CONNECT: ${TIMEOUT_CONNECT}
TIMEOUT_READ: ${TIMEOUT_READ}
Loading