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

[DPMBE-73] 인터렉션 init, 비지니스 로직을 구현한다 #111

Merged
merged 14 commits into from
Jun 29, 2023

Conversation

kdomo
Copy link
Member

@kdomo kdomo commented Jun 29, 2023

개요

작업사항

  • 인터렉션 비지니스 로직을 구현하였습니다.
  1. PromiseUserRegisterEvent를 받아 인터렉션을 init한다.
  2. 인터렉션 요청받으면 인터렉션 히스토리에 내역을 생성함과 동시에 이벤트(InteractionHistoryRegisterEvent)를 발행하고 202를 응답한다
  3. 이벤트 핸들러에서 Async로 인터렉션에 increment해준다. (이 때 동시성 문제 발행 가능성에 따라 redis 락 사용)
  • 해당 유저가 해당 약속에 있는지 체크하는 AOP 생성 (CheckUserParticipation어노테이션 사용)

변경로직

  • 내용을 적어주세요.

@kdomo kdomo added the function 특정 기능 추가 label Jun 29, 2023
@kdomo kdomo self-assigned this Jun 29, 2023
Copy link
Member

@ImNM ImNM left a comment

Choose a reason for hiding this comment

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

Comment on lines 21 to 31

@RedissonLock(
lockName = "인터렉션",
identifier = "userId",
)
@Transactional
@CheckUserParticipation
fun increment(promiseId: Long, userId: Long, interactionType: InteractionType) {
val interaction = interactionAdapter.queryInteraction(promiseId, userId, interactionType)
interaction.increment()
}
Copy link
Member

Choose a reason for hiding this comment

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

RedissonLock
분산락 안에
트랜잭션이 이미 존재해서
빼셔두 됩니당
트랜잭셔널 어노테이션

Comment on lines +18 to +24

@CheckUserParticipation
@Transactional(readOnly = true)
fun queryAllByInteractionType(userId: Long, promiseId: Long, interactionType: InteractionType): List<InteractionHistory> {
return interactionHistoryAdapter.queryAllByInteractionType(userId, promiseId, interactionType)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

검증 도메인 레이어 까지 내린거 좋은것 같아요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

앞에서도 하는데 뒤에서도 체크를 하는건 왜 좋아보이시나요 @ImNM 님?
그럼 사실 검증이 2번 들어가는 거잖아요? 왜 그럴까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

@CheckUserParticipation를 사용하는 저 부분 제외하고는 usecase에서 사용하는데 저부분을 domain으로 내린게 좋다고 하신거같아요!

Comment on lines 40 to 52
val arg = args[i]
if (arg is Long) {
return arg
} else if (arg is String) {
try {
return arg.toLong()
} catch (e: NumberFormatException) {
throw UserIdConversionException.EXCEPTION
}
} else {
UserIdParameterNotFoundException.EXCEPTION
}
}
Copy link
Member

Choose a reason for hiding this comment

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

when 으로 좀더 이쁘게 바꿔볼수 있을것같음!!

Comment on lines 9 to 20
val count: Long,
val interactionType: InteractionType,
) {
companion object {
fun from(it: InteractionHistory) {
TODO("Not yet implemented")
}
// fun from(interaction: Interaction): InteractionDetailDto {
// return InteractionDetailDto(interaction.promiseId, interaction.userId, interaction.interactionType, interaction.count)
// }
}
}
Copy link
Member

Choose a reason for hiding this comment

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

요곤 아직 구현이 안된건가요?

Comment on lines 39 to 55

@GetMapping("/users/me/promises/{promiseId}/interactions")
@Operation(summary = "자신의 인터렉션 정보를 가져옵니다.")
fun getMyInteraction(
@PathVariable promiseId: Long,
): InteractionResponse {
return interactionReadUseCase.findMyInteraction(promiseId)
}

@GetMapping("/users/me/promises/{promiseId}/interactions/{interactionType}")
@Operation(summary = "자신의 인터렉션 상세 정보를 가져옵니다.")
fun getMyInteractionDetail(
@PathVariable promiseId: Long,
@PathVariable interactionType: InteractionType,
): InteractionDetailResponse {
return interactionReadDetailUseCase.findMyInteractionDetail(promiseId, interactionType)
}
Copy link
Member

Choose a reason for hiding this comment

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


매핑이
/promises/{promiseId}/interactions/{interactionType}
로 해도 충분할것 같고
다른 유저아이디로 조회할게 있다 하면
/promises/{promiseId}/user/{userId}/interactions/{interactionType}
이렇게 하는게 뭔가 더 직관적인것같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은 의견인것같슴니다!

Copy link
Collaborator

@BlackBean99 BlackBean99 left a comment

Choose a reason for hiding this comment

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

LGTM

val interactionReadUseCase: InteractionReadUseCase,
val interactionReadDetailUseCase: InteractionReadDetailUseCase,
) {
@PostMapping("/promises/{promiseId}/interactions/{interactionType}/target/{targetUserId}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

target 이 userId 와 매칭 되는 맥락이 부족한 것 같아요.
users/{user-id} ? 같은 느낌

Copy link
Member Author

Choose a reason for hiding this comment

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

음 뭔가 직관적인게 있을까요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

리뷰 남겨주시면 확인후 다음 PR에서 반영해보겠습니다

var img: String,
var promiseId: Long,

var count: Long,
Copy link
Collaborator

Choose a reason for hiding this comment

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

count 를 그렇게 많이 하려나??? Integer로 선언하는건 어떤가요? 버튼 클릭 수가 그렇게 많지는 않을 것 같아서요

Copy link
Member Author

@kdomo kdomo Jun 29, 2023

Choose a reason for hiding this comment

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

약간 entity에 id를 생각했을때처럼 Long이 좋을 것 같아서 Long으로 선언했는데 Integer로 변경할까요?!

Comment on lines +18 to +24

@CheckUserParticipation
@Transactional(readOnly = true)
fun queryAllByInteractionType(userId: Long, promiseId: Long, interactionType: InteractionType): List<InteractionHistory> {
return interactionHistoryAdapter.queryAllByInteractionType(userId, promiseId, interactionType)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

앞에서도 하는데 뒤에서도 체크를 하는건 왜 좋아보이시나요 @ImNM 님?
그럼 사실 검증이 2번 들어가는 거잖아요? 왜 그럴까요?

@sonarcloud
Copy link

sonarcloud bot commented Jun 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

32.1% 32.1% Coverage
0.0% 0.0% Duplication

@kdomo kdomo merged commit 4d9a93a into develop Jun 29, 2023
3 checks passed
@kdomo kdomo deleted the feature/DPMBE-73 branch June 29, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
function 특정 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DPMBE-73] 인터렉션 init, 비지니스 로직을 구현한다
3 participants