Skip to content

Commit d7ca889

Browse files
jeffhwang-sqjeffhwang-sq
committed
add business hour logic
1 parent db4e569 commit d7ca889

File tree

9 files changed

+190
-192
lines changed

9 files changed

+190
-192
lines changed

‎service/src/main/kotlin/app/cash/backfila/service/deletion/DeprecationNotificationHelper.kt‎

Lines changed: 110 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@ import app.cash.backfila.service.listener.SlackHelper
44
import app.cash.backfila.service.persistence.BackfilaDb
55
import app.cash.backfila.service.persistence.BackfillRunQuery
66
import app.cash.backfila.service.persistence.BackfillState
7-
import app.cash.backfila.service.persistence.DbBackfillRun
8-
import app.cash.backfila.service.persistence.DbEventLog
7+
import app.cash.backfila.service.persistence.DbDeprecationReminder
98
import app.cash.backfila.service.persistence.DbRegisteredBackfill
10-
import app.cash.backfila.service.persistence.EventLogQuery
9+
import app.cash.backfila.service.persistence.DeprecationReminderQuery
1110
import app.cash.backfila.service.persistence.RegisteredBackfillQuery
1211
import java.time.Clock
12+
import java.time.DayOfWeek
1313
import java.time.Duration
1414
import java.time.Instant
15-
import java.time.temporal.ChronoUnit
15+
import java.time.ZoneOffset
1616
import javax.inject.Inject
1717
import javax.inject.Singleton
1818
import misk.hibernate.Query
@@ -27,21 +27,7 @@ class DeprecationNotificationHelper @Inject constructor(
2727
private val notificationProvider: DeprecationNotificationProvider,
2828
private val clock: Clock,
2929
) {
30-
// Data class to hold all the info needed for message generation
31-
data class NotificationContext(
32-
val backfill: DbRegisteredBackfill,
33-
val latestRun: DbBackfillRun?,
34-
val effectiveDeleteBy: Instant,
35-
val deleteByReason: DeleteByReason,
36-
)
37-
38-
// Enum to clearly indicate why a date was chosen
39-
enum class DeleteByReason {
40-
EXPLICIT_DELETE_BY,
41-
SUCCESSFUL_RUN,
42-
PAUSED_RUN,
43-
DEFAULT_CREATION,
44-
}
30+
private val daysInMonth = 30L
4531

4632
fun getRegisteredBackfillsForNotification(): List<DbRegisteredBackfill> {
4733
return transacter.transaction { session ->
@@ -51,7 +37,7 @@ class DeprecationNotificationHelper @Inject constructor(
5137
}
5238
}
5339

54-
fun evaluateRegisteredBackfill(registeredBackfill: DbRegisteredBackfill): NotificationDecision {
40+
fun notifyRegisteredBackfill(registeredBackfill: DbRegisteredBackfill): NotificationDecision? {
5541
return transacter.transaction { session ->
5642
val now = clock.instant()
5743
val config = notificationProvider.getNotificationConfig()
@@ -60,12 +46,26 @@ class DeprecationNotificationHelper @Inject constructor(
6046
val service = registeredBackfill.service // This should be loaded due to JPA relationship
6147
val lastRegisteredAt = service.last_registered_at
6248
if (lastRegisteredAt != null &&
63-
Duration.between(lastRegisteredAt, now) > config.registerRetention
49+
Duration.between(lastRegisteredAt, now) > Duration.ofDays(daysInMonth * 3)
6450
) {
65-
return@transaction NotificationDecision.NONE
51+
return@transaction null
52+
}
53+
54+
if (!isBusinessHours(registeredBackfill)) {
55+
return@transaction null
6656
}
6757

68-
val defaultDeleteBy = registeredBackfill.created_at.plus(config.defaultDeleteByDuration)
58+
val deleteByDates = mutableListOf<Pair<NotificationDecision, Instant>>()
59+
60+
// Explicit delete-by date
61+
registeredBackfill.delete_by?.let { deleteBy ->
62+
deleteByDates.add(NotificationDecision.EXPLICIT_DELETE_BY to deleteBy)
63+
}
64+
65+
// Default creation-based delete-by
66+
val defaultDeleteBy = registeredBackfill.created_at
67+
.plus(config.defaultDelayDays[NotificationDecision.DEFAULT_CREATION]!!)
68+
deleteByDates.add(NotificationDecision.DEFAULT_CREATION to defaultDeleteBy)
6969

7070
// Get the latest run and its status
7171
val latestRun = queryFactory.newQuery<BackfillRunQuery>()
@@ -77,166 +77,134 @@ class DeprecationNotificationHelper @Inject constructor(
7777
.list(session)
7878
.firstOrNull()
7979

80-
// Calculate delete_by date based on latest run
81-
val runBasedDeleteBy = latestRun?.let {
80+
// Add run-based delete_by dates to deleteByDates
81+
latestRun?.let {
8282
val runDate = it.created_at
8383
when (it.state) {
84-
BackfillState.COMPLETE -> runDate.plus(config.completeRunRetention)
85-
BackfillState.PAUSED -> runDate.plus(config.pausedRunRetention)
84+
BackfillState.COMPLETE -> deleteByDates.add(
85+
NotificationDecision.COMPLETE_RUN to runDate.plus(config.defaultDelayDays[NotificationDecision.COMPLETE_RUN]!!),
86+
)
87+
BackfillState.PAUSED -> deleteByDates.add(
88+
NotificationDecision.PAUSED_RUN to runDate.plus(config.defaultDelayDays[NotificationDecision.PAUSED_RUN]!!),
89+
)
8690
else -> null
8791
}
8892
}
8993

90-
// Determine the maximum date to use and why
91-
val (effectiveDeleteBy, deleteByReason) = when {
92-
registeredBackfill.delete_by != null &&
93-
(registeredBackfill.delete_by == listOfNotNull(registeredBackfill.delete_by, defaultDeleteBy, runBasedDeleteBy).maxOrNull()) ->
94-
registeredBackfill.delete_by!! to DeleteByReason.EXPLICIT_DELETE_BY
95-
96-
runBasedDeleteBy != null &&
97-
(runBasedDeleteBy == listOfNotNull(registeredBackfill.delete_by, defaultDeleteBy, runBasedDeleteBy).maxOrNull()) ->
98-
if (latestRun.state == BackfillState.COMPLETE) {
99-
runBasedDeleteBy to DeleteByReason.SUCCESSFUL_RUN
100-
} else {
101-
runBasedDeleteBy to DeleteByReason.PAUSED_RUN
102-
}
103-
104-
else -> defaultDeleteBy to DeleteByReason.DEFAULT_CREATION
105-
}
94+
// Find the latest delete-by date and its associated decision
95+
val (effectiveDecision, effectiveDeleteBy) = deleteByDates.maxByOrNull { it.second }
96+
?: return@transaction null
10697

10798
// Don't send notifications before the delete_by date
10899
val timeUntilDeletion = Duration.between(now, effectiveDeleteBy)
109100
if (!timeUntilDeletion.isNegative) {
110-
return@transaction NotificationDecision.NONE
101+
return@transaction null
111102
}
112103

113104
// We're past the delete_by date, determine notification frequency
114105
val timeSinceDeletion = Duration.between(effectiveDeleteBy, now)
106+
val notifications = config.notifications[effectiveDecision] ?: emptyList()
107+
108+
// Find the appropriate notification based on timeSinceDeletion
109+
val appropriateNotification = notifications
110+
.sortedBy { it.delay } // Sort by delay to get earliest first
111+
.findLast { notification ->
112+
// Find the last notification whose delay is less than or equal to timeSinceDeletion
113+
timeSinceDeletion >= notification.delay
114+
} ?: return@transaction null
115115

116116
// Get the last notification sent
117-
// First get all backfill run IDs for this registered backfill
118-
val backfillRunIds = queryFactory.newQuery<BackfillRunQuery>()
117+
val lastReminder = queryFactory.newQuery<DeprecationReminderQuery>()
119118
.registeredBackfillId(registeredBackfill.id)
119+
.orderByCreatedAtDesc()
120+
.apply { maxRows = 1 }
120121
.list(session)
121-
.map { it.id }
122-
123-
// Then if we have any runs, query event logs for notifications across all these runs
124-
val lastNotification = if (backfillRunIds.isNotEmpty()) {
125-
queryFactory.newQuery<EventLogQuery>()
126-
.backfillRunIdIn(backfillRunIds) // We'd need to add this method to EventLogQuery
127-
.type(DbEventLog.Type.NOTIFICATION)
128-
.orderByUpdatedAtDesc()
129-
.apply {
130-
maxRows = 1
131-
}
132-
.list(session)
133-
.firstOrNull()
134-
} else {
135-
null
136-
}
137-
138-
// First 3 months: monthly reminders
139-
if (timeSinceDeletion <= config.monthlyRemindersPhase) {
140-
val shouldSendMonthly = lastNotification?.let {
141-
Duration.between(it.created_at, now) >= Duration.ofDays(30)
142-
} ?: true
143-
144-
if (shouldSendMonthly) {
145-
val context = NotificationContext(
146-
backfill = registeredBackfill,
147-
latestRun = latestRun,
148-
effectiveDeleteBy = effectiveDeleteBy,
149-
deleteByReason = deleteByReason,
150-
)
122+
.firstOrNull()
151123

152-
currentNotificationContext = context
153-
return@transaction NotificationDecision.NOTIFY_EXPIRED
124+
// Check if we should send this notification
125+
val shouldSendNotification = when {
126+
lastReminder == null -> {
127+
// No previous reminder, should send
128+
true
129+
}
130+
appropriateNotification.repeated -> {
131+
// For repeated notifications, check if enough time has passed since last reminder
132+
Duration.between(lastReminder.created_at, now) >= appropriateNotification.delay
133+
}
134+
else -> {
135+
// For non-repeated notifications, check if this specific type hasn't been sent
136+
!(lastReminder.message_last_user == appropriateNotification.messageLastUser && !lastReminder.repeated)
154137
}
155138
}
156-
// After 3 months: weekly reminders
157-
else {
158-
val shouldSendWeekly = lastNotification?.let {
159-
Duration.between(it.created_at, now) >= Duration.ofDays(7)
160-
} ?: true
161-
162-
if (shouldSendWeekly) {
163-
val context = NotificationContext(
164-
backfill = registeredBackfill,
165-
latestRun = latestRun,
166-
effectiveDeleteBy = effectiveDeleteBy,
167-
deleteByReason = deleteByReason,
168-
)
169139

170-
currentNotificationContext = context
171-
return@transaction NotificationDecision.NOTIFY_EXPIRED
172-
}
140+
if (shouldSendNotification) {
141+
val message = generateNotificationMessage(appropriateNotification, registeredBackfill)
142+
sendNotification(message, registeredBackfill.service.slack_channel)
143+
// Log to deprecation reminder table
144+
val reminder = DbDeprecationReminder(
145+
registeredBackfill.id,
146+
appropriateNotification.messageLastUser, appropriateNotification.repeated,
147+
now,
148+
)
149+
session.save(reminder)
150+
return@transaction effectiveDecision
173151
}
174152

175-
NotificationDecision.NONE
153+
return@transaction null
176154
}
177155
}
178156

179157
// Separate message generation function that doesn't need DB access
180158
private fun generateNotificationMessage(
181-
decision: NotificationDecision,
182-
context: NotificationContext,
159+
notification: DeprecationMessage,
160+
registeredBackfill: DbRegisteredBackfill,
183161
): String {
184-
val now = clock.instant()
185-
val daysSinceDeletion = ChronoUnit.DAYS.between(context.effectiveDeleteBy, now)
186-
187-
val deleteByReasonMessage = when (context.deleteByReason) {
188-
DeleteByReason.EXPLICIT_DELETE_BY -> "explicitly set delete_by date"
189-
DeleteByReason.SUCCESSFUL_RUN -> "30 days after last successful run"
190-
DeleteByReason.PAUSED_RUN -> "90 days after last failed run"
191-
DeleteByReason.DEFAULT_CREATION -> "6 months after creation"
162+
// Use the configured message directly
163+
val baseMessage = notification.message
164+
165+
// Add metadata about the backfill
166+
val metadata = buildString {
167+
appendLine()
168+
appendLine("*Additional Information:*")
169+
appendLine("• Backfill: `${registeredBackfill.name}`")
170+
171+
// Add action items
172+
appendLine()
173+
appendLine("*Actions Required:*")
174+
appendLine("1. Review if this backfill is still needed")
175+
appendLine("2. Update the `@DeleteBy` annotation with a new date")
176+
appendLine("3. Or run the backfill again to extend its lifetime")
192177
}
193178

194-
return """
195-
|${decision.emoji} *Backfill Deletion Reminder*
196-
|Backfill `${context.backfill.name}` was due for deletion on ${context.effectiveDeleteBy}.
197-
|
198-
|• Days since deletion date: $daysSinceDeletion
199-
|• Deletion date determined by: $deleteByReasonMessage
200-
|${context.latestRun?.let { "• Last run status: ${it.state} on ${it.created_at}" } ?: "• No runs found"}
201-
|
202-
|To keep this backfill:
203-
|1. Review if this backfill is still needed
204-
|2. Update the `@DeleteBy` annotation with a new date
205-
|3. Or run the backfill again to extend its lifetime
206-
|
207-
|_This reminder will be sent monthly for 3 months, then weekly until action is taken._
208-
""".trimMargin()
179+
return baseMessage + metadata
209180
}
210181

211182
// Update send notification to use the new message generation
212183
fun sendNotification(
213-
decision: NotificationDecision,
214-
channel: String,
184+
message: String,
185+
channel: String?,
215186
) {
216-
val message = generateNotificationMessage(decision, currentNotificationContext!!)
217-
187+
if (channel == null) {
188+
// logger.warn { "No Slack channel specified for notification. Skipping." }
189+
return
190+
}
218191
// Send to Slack
219192
slackHelper.sendDeletionNotification(message, channel)
193+
}
220194

221-
// Record notification in event_logs for the latest run if it exists
222-
currentNotificationContext?.latestRun?.let { latestRun ->
223-
transacter.transaction { session ->
224-
session.save(
225-
DbEventLog(
226-
backfill_run_id = latestRun.id,
227-
partition_id = null,
228-
type = DbEventLog.Type.NOTIFICATION,
229-
message = "Deletion notification sent to $channel",
230-
extra_data = message,
231-
),
232-
)
233-
}
195+
private fun isBusinessHours(registeredBackfill: DbRegisteredBackfill): Boolean {
196+
val creationTime = registeredBackfill.created_at
197+
val currentTime = clock.instant()
198+
199+
// Avoid weekends in UTC
200+
if (currentTime.atZone(ZoneOffset.UTC).dayOfWeek !in listOf(DayOfWeek.SATURDAY, DayOfWeek.SUNDAY)) {
201+
return false
234202
}
235203

236-
// Clear the context after use
237-
currentNotificationContext = null
238-
}
204+
// Keep the same hour of day as when the backfill was created
205+
val creationHour = creationTime.atZone(ZoneOffset.UTC).hour
206+
val currentHour = currentTime.atZone(ZoneOffset.UTC).hour
239207

240-
// Add property to store context between evaluate and send
241-
private var currentNotificationContext: NotificationContext? = null
208+
return creationHour == currentHour
209+
}
242210
}

‎service/src/main/kotlin/app/cash/backfila/service/deletion/DeprecationNotificationProvider.kt‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,25 @@ package app.cash.backfila.service.deletion
22

33
import java.time.Duration
44

5-
interface DeprecationNotificationProvider {
6-
fun getNotificationConfig(): DeprecationNotificationConfig
5+
enum class NotificationDecision {
6+
EXPLICIT_DELETE_BY,
7+
COMPLETE_RUN,
8+
PAUSED_RUN,
9+
DEFAULT_CREATION,
710
}
811

9-
data class DeprecationNotificationConfig(
10-
val defaultDeleteByDuration: Duration,
11-
val completeRunRetention: Duration,
12-
val pausedRunRetention: Duration,
13-
val registerRetention: Duration,
14-
val monthlyRemindersPhase: Duration,
12+
data class DeprecationMessage(
13+
val delay: Duration,
14+
val message: String,
15+
val messageLastUser: Boolean = false,
16+
val repeated: Boolean = false,
1517
)
18+
19+
interface DeprecationMessageBuilder {
20+
val notifications: Map<NotificationDecision, List<DeprecationMessage>>
21+
val defaultDelayDays: Map<NotificationDecision, Duration>
22+
}
23+
24+
interface DeprecationNotificationProvider {
25+
fun getNotificationConfig(): DeprecationMessageBuilder
26+
}

0 commit comments

Comments
 (0)