-
Notifications
You must be signed in to change notification settings - Fork 1
[DPMBE-73] 인터렉션 init, 비지니스 로직을 구현한다 #111
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
분산락 관련해서는
제가 테스트 하게 편하게 만들어놓은 유틸있어요
동시성 테스트 한번 해보셔유!
레퍼런스
|
||
@RedissonLock( | ||
lockName = "인터렉션", | ||
identifier = "userId", | ||
) | ||
@Transactional | ||
@CheckUserParticipation | ||
fun increment(promiseId: Long, userId: Long, interactionType: InteractionType) { | ||
val interaction = interactionAdapter.queryInteraction(promiseId, userId, interactionType) | ||
interaction.increment() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RedissonLock
분산락 안에
트랜잭션이 이미 존재해서
빼셔두 됩니당
트랜잭셔널 어노테이션
|
||
@CheckUserParticipation | ||
@Transactional(readOnly = true) | ||
fun queryAllByInteractionType(userId: Long, promiseId: Long, interactionType: InteractionType): List<InteractionHistory> { | ||
return interactionHistoryAdapter.queryAllByInteractionType(userId, promiseId, interactionType) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
검증 도메인 레이어 까지 내린거 좋은것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앞에서도 하는데 뒤에서도 체크를 하는건 왜 좋아보이시나요 @ImNM 님?
그럼 사실 검증이 2번 들어가는 거잖아요? 왜 그럴까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CheckUserParticipation
를 사용하는 저 부분 제외하고는 usecase에서 사용하는데 저부분을 domain으로 내린게 좋다고 하신거같아요!
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 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when 으로 좀더 이쁘게 바꿔볼수 있을것같음!!
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) | ||
// } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요곤 아직 구현이 안된건가요?
|
||
@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) | ||
} |
There was a problem hiding this comment.
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}
이렇게 하는게 뭔가 더 직관적인것같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견인것같슴니다!
There was a problem hiding this 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}") |
There was a problem hiding this comment.
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} ? 같은 느낌
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음 뭔가 직관적인게 있을까요?!
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
count 를 그렇게 많이 하려나??? Integer로 선언하는건 어떤가요? 버튼 클릭 수가 그렇게 많지는 않을 것 같아서요
There was a problem hiding this comment.
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로 변경할까요?!
|
||
@CheckUserParticipation | ||
@Transactional(readOnly = true) | ||
fun queryAllByInteractionType(userId: Long, promiseId: Long, interactionType: InteractionType): List<InteractionHistory> { | ||
return interactionHistoryAdapter.queryAllByInteractionType(userId, promiseId, interactionType) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앞에서도 하는데 뒤에서도 체크를 하는건 왜 좋아보이시나요 @ImNM 님?
그럼 사실 검증이 2번 들어가는 거잖아요? 왜 그럴까요?
Kudos, SonarCloud Quality Gate passed! |
개요
작업사항
CheckUserParticipation
어노테이션 사용)변경로직