-
Notifications
You must be signed in to change notification settings - Fork 4
refactor: 우산 대여 코드 이벤트 분리 #502
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
base: dev
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package upbrella.be.config.event | ||
|
|
||
| import org.springframework.beans.factory.InitializingBean | ||
| import org.springframework.context.ApplicationContext | ||
| import org.springframework.context.annotation.Bean | ||
| import org.springframework.context.annotation.Configuration | ||
| import upbrella.be.util.event.Events | ||
|
|
||
| @Configuration | ||
| class EventsConfiguration( | ||
| private val applicationContext: ApplicationContext | ||
| ) { | ||
|
|
||
| @Bean | ||
| fun eventsInitializer(): InitializingBean { | ||
| return InitializingBean { Events.setPublisher(applicationContext) } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package upbrella.be.rent.event | ||
|
|
||
| import org.springframework.context.event.EventListener | ||
| import org.springframework.stereotype.Service | ||
| import upbrella.be.rent.entity.ConditionReport | ||
| import upbrella.be.rent.service.ConditionReportService | ||
| import upbrella.be.slack.SlackAlarmService | ||
| import upbrella.be.slack.dto.service.input.NotifyConditionReportInput | ||
| import upbrella.be.slack.dto.service.input.NotifyRentInput | ||
|
|
||
| @Service | ||
| class RentEventHandler( | ||
| private val slackAlarmService: SlackAlarmService, | ||
| private val conditionReportService: ConditionReportService, | ||
| ) { | ||
|
|
||
| @EventListener(UmbrellaRentedEvent::class) | ||
| fun handleUmbrellaRentedEvent(event: UmbrellaRentedEvent) { | ||
| slackAlarmService.notifyRent( | ||
| NotifyRentInput( | ||
| userId = event.userId, | ||
| userName = event.userName, | ||
| rentStoreName = event.rentStoreName | ||
| ) | ||
| ) | ||
|
|
||
| event.conditionReportContent | ||
| ?.takeIf { it.isNotBlank() } | ||
| ?.let { content -> | ||
| ConditionReport( | ||
| history = event.history, | ||
| content = content | ||
| ).also { conditionReport -> | ||
| conditionReportService.saveConditionReport(conditionReport) | ||
| slackAlarmService.notifyConditionReport( | ||
| NotifyConditionReportInput( | ||
| umbrellaId = event.umbrellaId, | ||
| rentStoreName = event.rentStoreName, | ||
| content = content | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package upbrella.be.rent.event | ||
|
|
||
| import upbrella.be.rent.entity.History | ||
| import upbrella.be.util.event.Event | ||
|
|
||
| class UmbrellaRentedEvent( | ||
| val userId: Long, | ||
| val userName: String, | ||
| val rentStoreName: String, | ||
| val conditionReportContent: String? = null, | ||
| val umbrellaId: Long, | ||
| val history: History, | ||
|
||
| ) : Event() | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,16 +7,18 @@ import upbrella.be.rent.dto.request.HistoryFilterRequest | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.dto.request.RentUmbrellaByUserRequest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.dto.request.ReturnUmbrellaByUserRequest | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.dto.response.* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.ConditionReport | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.History | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.ImprovementReport | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.exception.* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.event.UmbrellaRentedEvent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.exception.CannotBeRentedException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.exception.ExistingUmbrellaForRentException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.exception.NonExistingHistoryException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.exception.NonExistingUmbrellaForRentException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.repository.RentRepository | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.slack.SlackAlarmService | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.store.entity.StoreMeta | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.store.repository.StoreMetaReader | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.umbrella.entity.Umbrella | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.umbrella.exception.MissingUmbrellaException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.umbrella.exception.NonExistingBorrowedHistoryException | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.umbrella.service.UmbrellaService | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.user.dto.response.AllHistoryResponse | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -25,6 +27,7 @@ import upbrella.be.user.dto.response.SingleHistoryResponse | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.user.entity.User | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.user.repository.UserReader | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.user.service.BlackListService | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import upbrella.be.util.event.Events | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.LocalDateTime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.time.temporal.ChronoUnit | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,8 +38,6 @@ class RentService( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val slackAlarmService: SlackAlarmService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val improvementReportService: ImprovementReportService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val rentRepository: RentRepository, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val conditionReportService: ConditionReportService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val lockerService: LockerService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val blackListService: BlackListService, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private val userReader: UserReader, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -66,30 +67,25 @@ class RentService( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw ExistingUmbrellaForRentException("[ERROR] 해당 유저가 대여 중인 우산이 있습니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| val umbrella = umbrellaService.findUmbrellaById(rentUmbrellaByUserRequest.umbrellaId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (umbrella.storeMeta.id != rentUmbrellaByUserRequest.storeId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw UmbrellaStoreMissMatchException("[ERROR] 해당 우산은 해당 매장에 존재하지 않습니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (umbrella.missed) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw MissingUmbrellaException("[ERROR] 해당 우산은 분실되었습니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!umbrella.rentable) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw NotAvailableUmbrellaException("[ERROR] 해당 우산은 대여중입니다.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| umbrella.rentUmbrella() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| umbrella.rentUmbrella(rentUmbrellaByUserRequest.storeId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
-69
to
+71
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 검증 로직 umbrella 객체가 처리하도록 변경
Comment on lines
+70
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Concurrent rentals need locking/optimistic concurrency. umbrella.rentUmbrella only flips a boolean. Without locking, concurrent rentals can both pass checks and set rentable=false with only one History saved correctly, or worse, double-rent. Fetch the umbrella with a write lock or add @Version to Umbrella for optimistic locking. Options:
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| val rentalStore = storeMetaReader.findById(rentUmbrellaByUserRequest.storeId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| val history = rentRepository.save( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| History.ofCreatedByNewRent(umbrella, userToRent, rentalStore) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| slackAlarmService.notifyRent(userToRent, history) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rentUmbrellaByUserRequest.conditionReport | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ?.takeIf { it.isNotBlank() } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ?.let { content -> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ConditionReport(history = history, content = content).also { conditionReport -> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conditionReportService.saveConditionReport(conditionReport) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| slackAlarmService.notifyConditionReport(conditionReport) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // umbrellaRentedEvent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Events.raise( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| UmbrellaRentedEvent( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| userId = userToRent.id, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| userName = userToRent.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rentStoreName = rentalStore.name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| conditionReportContent = rentUmbrellaByUserRequest.conditionReport, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| umbrellaId = umbrella.id!!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| history = history, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainPublish notifications after commit; keep events side-effect-safe. Events.raise is invoked inside @transactional addRental. With the current @eventlistener, Slack calls run in-tx and can roll back the rental. Align with AFTER_COMMIT listeners as suggested in RentEventHandler. To verify current event publisher semantics and listeners: If Events is synchronous/in-tx, proceed with @TransactionalEventListener(phase = AFTER_COMMIT) in handlers (and/or provide a separate Events.raiseAfterCommit if you maintain an event bus wrapper). 🏁 Script executed: #!/bin/bash
# Inspect event infra and listeners
fd -a "Event*.kt" | sed 's|^|# |'
rg -n "object\\s+Events|class\\s+Events|ApplicationEventPublisher|raise\\(" -A3 -B2
rg -n "@TransactionalEventListener|TransactionPhase|@EventListener\\(" -A2 -B2Length of output: 3100 🏁 Script executed: #!/bin/bash
sed -n '1,200p' src/main/kotlin/upbrella/be/config/event/EventConfiguration.ktLength of output: 617 Switch Slack notification listener to AFTER_COMMIT The • File: src/main/kotlin/upbrella/be/rent/event/RentEventHandler.kt - @EventListener(UmbrellaRentedEvent::class)
- fun handleUmbrellaRentedEvent(event: UmbrellaRentedEvent) {
+ import org.springframework.transaction.event.TransactionalEventListener
+ import org.springframework.transaction.event.TransactionPhase
+
+ @TransactionalEventListener(
+ phase = TransactionPhase.AFTER_COMMIT,
+ classes = [UmbrellaRentedEvent::class]
+ )
+ fun handleUmbrellaRentedEvent(event: UmbrellaRentedEvent) {
slackAlarmService.notifyRent(
event.userId,
event.userName,
event.rentStoreName,
event.umbrellaId,
event.conditionReportContent,
event.history
)
}This ensures notifications are sent only once the rental transaction has successfully committed. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Transactional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,9 +5,10 @@ import org.springframework.http.HttpMethod.POST | |||||||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Service | ||||||||||||||||||||||||||||||||||||
| import org.springframework.web.client.RestTemplate | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.config.SlackBotConfig | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.ConditionReport | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.History | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.rent.entity.ImprovementReport | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.slack.dto.service.input.NotifyConditionReportInput | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.slack.dto.service.input.NotifyRentInput | ||||||||||||||||||||||||||||||||||||
| import upbrella.be.user.entity.User | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Service | ||||||||||||||||||||||||||||||||||||
|
|
@@ -16,12 +17,12 @@ class SlackAlarmService( | |||||||||||||||||||||||||||||||||||
| private val restTemplate: RestTemplate | ||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| fun notifyRent(user: User, history: History) { | ||||||||||||||||||||||||||||||||||||
| fun notifyRent(input: NotifyRentInput) { | ||||||||||||||||||||||||||||||||||||
| val message = buildString { | ||||||||||||||||||||||||||||||||||||
| append("*우산 대여 알림: 입금을 확인해주세요.*\n\n") | ||||||||||||||||||||||||||||||||||||
| append("사용자 ID : ${user.id}\n") | ||||||||||||||||||||||||||||||||||||
| append("예금주 이름 : ${user.name}\n") | ||||||||||||||||||||||||||||||||||||
| append("대여 지점 이름 : ${history.rentStoreMeta.name}\n") | ||||||||||||||||||||||||||||||||||||
| append("사용자 ID : ${input.userId}\n") | ||||||||||||||||||||||||||||||||||||
| append("예금주 이름 : ${input.userName}\n") | ||||||||||||||||||||||||||||||||||||
| append("대여 지점 이름 : ${input.rentStoreName}\n") | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| send(message) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
@@ -39,20 +40,12 @@ class SlackAlarmService( | |||||||||||||||||||||||||||||||||||
| send(message) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private fun send(message: String) { | ||||||||||||||||||||||||||||||||||||
| val request = mutableMapOf<String, Any>( | ||||||||||||||||||||||||||||||||||||
| "text" to message | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| val entity = HttpEntity(request) | ||||||||||||||||||||||||||||||||||||
| restTemplate.exchange(slackBotConfig.webHookUrl, POST, entity, String::class.java) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| fun notifyConditionReport(conditionReport: ConditionReport) { | ||||||||||||||||||||||||||||||||||||
| fun notifyConditionReport(input: NotifyConditionReportInput) { | ||||||||||||||||||||||||||||||||||||
| val message = buildString { | ||||||||||||||||||||||||||||||||||||
| append("*우산 상태 신고 접수*\n\n") | ||||||||||||||||||||||||||||||||||||
| append("우산 ID : ${conditionReport.history.umbrella.id}\n") | ||||||||||||||||||||||||||||||||||||
| append("대여 지점 이름 : ${conditionReport.history.rentStoreMeta.name}\n") | ||||||||||||||||||||||||||||||||||||
| append("신고 내용 : ${conditionReport.content}\n") | ||||||||||||||||||||||||||||||||||||
| append("우산 ID : ${input.umbrellaId}\n") | ||||||||||||||||||||||||||||||||||||
| append("대여 지점 이름 : ${input.rentStoreName}\n") | ||||||||||||||||||||||||||||||||||||
| append("신고 내용 : ${input.content}\n") | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| send(message) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
@@ -69,4 +62,12 @@ class SlackAlarmService( | |||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| send(message) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private fun send(message: String) { | ||||||||||||||||||||||||||||||||||||
| val request = mutableMapOf<String, Any>( | ||||||||||||||||||||||||||||||||||||
| "text" to message | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| val entity = HttpEntity(request) | ||||||||||||||||||||||||||||||||||||
| restTemplate.exchange(slackBotConfig.webHookUrl, POST, entity, String::class.java) | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+71
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Harden Slack sending: don't fail business Tx on webhook errors. Wrap the HTTP call and log failures so Slack outages don’t cascade into rental failures. This is especially important if listeners run in-tx. - private fun send(message: String) {
- val request = mutableMapOf<String, Any>(
- "text" to message
- )
- val entity = HttpEntity(request)
- restTemplate.exchange(slackBotConfig.webHookUrl, POST, entity, String::class.java)
- }
+ private fun send(message: String) {
+ val request = mutableMapOf<String, Any>("text" to message)
+ val entity = HttpEntity(request)
+ try {
+ restTemplate.exchange(slackBotConfig.webHookUrl, POST, entity, String::class.java)
+ } catch (ex: Exception) {
+ // Do not propagate; Slack outage should not break rentals.
+ logger.warn("Slack webhook send failed: ${ex.message}", ex)
+ }
+ }Additional code required outside the selected range:
📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package upbrella.be.slack.dto.service.input | ||
|
|
||
| data class NotifyRentInput( | ||
| val userId: Long, | ||
| val userName: String, | ||
| val rentStoreName: String, | ||
| ) | ||
|
|
||
| data class NotifyConditionReportInput( | ||
| val umbrellaId: Long, | ||
| val rentStoreName: String, | ||
| val content: String, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| package upbrella.be.umbrella.entity | ||
|
|
||
| import upbrella.be.rent.exception.NotAvailableUmbrellaException | ||
| import upbrella.be.rent.exception.UmbrellaStoreMissMatchException | ||
| import upbrella.be.store.entity.StoreMeta | ||
| import upbrella.be.umbrella.dto.request.UmbrellaCreateRequest | ||
| import upbrella.be.umbrella.dto.request.UmbrellaModifyRequest | ||
| import upbrella.be.umbrella.exception.MissingUmbrellaException | ||
| import java.time.LocalDateTime | ||
| import javax.persistence.* | ||
|
|
||
|
|
@@ -49,7 +52,16 @@ class Umbrella( | |
| this.missed = request.missed | ||
| } | ||
|
|
||
| fun rentUmbrella() { | ||
| fun rentUmbrella(storeIdForRent: Long) { | ||
| if (this.storeMeta.id != storeIdForRent) { | ||
| throw UmbrellaStoreMissMatchException("[ERROR] 해당 우산은 해당 매장에 존재하지 않습니다.") | ||
| } | ||
| if (this.missed) { | ||
| throw MissingUmbrellaException("[ERROR] 해당 우산은 분실되었습니다.") | ||
| } | ||
| if (!this.rentable) { | ||
| throw NotAvailableUmbrellaException("[ERROR] 해당 우산은 대여중입니다.") | ||
| } | ||
| this.rentable = false | ||
|
Comment on lines
+55
to
65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Block rentals of deleted umbrellas inside the entity (consistency with cannotBeRented). rentUmbrella currently checks store mismatch, missed, and rentable, but not deleted. This allows renting a logically deleted umbrella if rentUmbrella is called directly. Add a deleted check (or reuse cannotBeRented) to keep invariants consistent. Apply within this method: fun rentUmbrella(storeIdForRent: Long) {
if (this.storeMeta.id != storeIdForRent) {
throw UmbrellaStoreMissMatchException("[ERROR] 해당 우산은 해당 매장에 존재하지 않습니다.")
}
if (this.missed) {
throw MissingUmbrellaException("[ERROR] 해당 우산은 분실되었습니다.")
}
+ if (this.deleted) {
+ throw CannotBeRentedException("[ERROR] 해당 우산은 대여 불가능한 우산입니다.")
+ }
if (!this.rentable) {
throw NotAvailableUmbrellaException("[ERROR] 해당 우산은 대여중입니다.")
}
this.rentable = false
}Additional change required outside the selected range:
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package upbrella.be.util.event | ||
|
|
||
| abstract class Event( | ||
| val timestamp: Long = System.currentTimeMillis(), | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package upbrella.be.util.event | ||
|
|
||
| import org.springframework.context.ApplicationEventPublisher | ||
|
|
||
| object Events { | ||
| private var publisher: ApplicationEventPublisher? = null | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Harden the publisher and narrow the API surface
Apply: object Events {
- private var publisher: ApplicationEventPublisher? = null
+ @Volatile
+ private var publisher: ApplicationEventPublisher? = null
- fun raise(event: Any) {
+ fun raise(event: Event) {
publisher?.publishEvent(event)
?: throw IllegalStateException("ApplicationEventPublisher가 설정되지 않았습니다.")
}
}Also applies to: 12-15 🤖 Prompt for AI Agents |
||
|
|
||
| fun setPublisher(publisher: ApplicationEventPublisher) { | ||
| this.publisher = publisher | ||
| } | ||
|
|
||
| fun raise(event: Any) { | ||
| publisher?.publishEvent(event) | ||
| ?: throw IllegalStateException("ApplicationEventPublisher가 설정되지 않았습니다.") | ||
| } | ||
| } | ||
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.
🛠️ Refactor suggestion
Avoid notifying Slack inside the main transaction; fire AFTER_COMMIT.
Current @eventlistener runs synchronously within the publishing transaction. If Slack fails, it can roll back the rental; if the transaction rolls back, Slack can be sent for a failed rental. Split persistence and side-effects and use @TransactionalEventListener phases.
Apply this refactor:
Additional imports required:
🤖 Prompt for AI Agents