Skip to content

Commit

Permalink
## Description
Browse files Browse the repository at this point in the history
This PR introduces a cronPattern parameter to CronRunnableEntry and
CronEntryModule, enabling clients to specify a custom cron pattern
directly when registering a runnable, rather than relying solely on
annotations. This change maintains backward compatibility by setting
cronPattern to null by default, preserving existing behavior.

### Key Changes
- CronRunnableEntry Update: Added an optional `cronPattern` property
with a default value of `null`. This allows clients to specify a cron
pattern directly when creating the entry.
- CronEntryModule Update: Modified `CronEntryModule` to accept an
optional `cronPattern` parameter in its factory methods, enabling users
to provide a cron pattern when registering a runnable.
- `CronService::startUp` Logic: Enhanced startUp to prioritize the
cronPattern from `CronRunnableEntry`. If no pattern is specified, the
service falls back to the `@CronPattern` annotation on the runnable
class. This update improves flexibility, allowing clients to override
the annotation pattern with a custom pattern if needed.

### Compatibility
- Backward Compatibility: Existing clients that rely on annotations for
cron scheduling will continue to work without modification, as
cronPattern defaults to null.
- Enhanced Flexibility: Clients now have the option to specify a custom
cron pattern directly, without changing the underlying annotations.

### Use Cases
This enhancement is particularly valuable for cron jobs that need
flexible scheduling across different contexts. For instance, a database
cleanup job may require frequent execution for services with high
insertion rates, ensuring timely data management and optimal
performance. Conversely, for services with low insertion rates, the job
can be scheduled less frequently, conserving resources while still
meeting maintenance needs.

### Additional Notes
- Updated comments in `CronService::startUp` to clarify the precedence
between cronPattern in `CronRunnableEntry` and the @CronPattern
annotation.
- This change is additive and introduces no breaking changes for
existing clients.

GitOrigin-RevId: 838c362bd0c589486d3dee12dd24455c40d0940f
  • Loading branch information
lcabancla authored and svc-squareup-copybara committed Nov 12, 2024
1 parent 7e28375 commit ea14d10
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
5 changes: 3 additions & 2 deletions misk-cron/api/misk-cron.api
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
public final class misk/cron/CronEntryModule : misk/inject/KAbstractModule {
public static final field Companion Lmisk/cron/CronEntryModule$Companion;
public synthetic fun <init> (Lkotlin/reflect/KClass;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Lkotlin/reflect/KClass;Lmisk/cron/CronPattern;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
}

public final class misk/cron/CronEntryModule$Companion {
public final fun create (Lkotlin/reflect/KClass;)Lmisk/cron/CronEntryModule;
public final fun create (Lkotlin/reflect/KClass;Lmisk/cron/CronPattern;)Lmisk/cron/CronEntryModule;
public static synthetic fun create$default (Lmisk/cron/CronEntryModule$Companion;Lkotlin/reflect/KClass;Lmisk/cron/CronPattern;ILjava/lang/Object;)Lmisk/cron/CronEntryModule;
}

public final class misk/cron/CronManager {
Expand Down
13 changes: 7 additions & 6 deletions misk-cron/src/main/kotlin/misk/cron/CronEntryModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,24 @@ package misk.cron
import misk.inject.KAbstractModule
import kotlin.reflect.KClass

internal class CronRunnableEntry(val runnableClass: KClass<out Runnable>)
internal class CronRunnableEntry @JvmOverloads constructor(val runnableClass: KClass<out Runnable>, val cronPattern: CronPattern? = null)

class CronEntryModule<A : Runnable> private constructor(
private val runnableClass: KClass<A>
private val runnableClass: KClass<A>,
private val cronPattern: CronPattern? = null
) : KAbstractModule() {
override fun configure() {
multibind<CronRunnableEntry>().toInstance(CronRunnableEntry(runnableClass))
multibind<CronRunnableEntry>().toInstance(CronRunnableEntry(runnableClass, cronPattern))
}
companion object {
inline fun <reified A : Runnable> create(): CronEntryModule<A> = create(A::class)
inline fun <reified A : Runnable> create(cronPattern: CronPattern? = null): CronEntryModule<A> = create(A::class, cronPattern)

/**
* Registers a cron runnable.
* @param cronRunnableClass: The cron runnable to register.
*/
fun <A : Runnable> create(cronRunnableClass: KClass<A>): CronEntryModule<A> {
return CronEntryModule(cronRunnableClass)
fun <A : Runnable> create(cronRunnableClass: KClass<A>, cronPattern: CronPattern? = null): CronEntryModule<A> {
return CronEntryModule(cronRunnableClass, cronPattern)
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion misk-cron/src/main/kotlin/misk/cron/CronService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@ internal class CronService @Inject constructor(
cronRunnableEntries.forEach { cronRunnable ->
val name = cronRunnable.runnableClass.qualifiedName!!
val annotations = cronRunnable.runnableClass.annotations
val cronPattern = annotations.find { it is CronPattern } as? CronPattern
val annotationPattern = annotations.find { it is CronPattern } as? CronPattern
// Use the cron pattern specified in CronRunnableEntry if available.
// If it's null, fall back to the pattern from the @CronPattern annotation on the class.
// This allows the annotation pattern to serve as a default, while specifying
// a cron pattern in CronRunnableEntry will override it if provided.
val cronPattern = cronRunnable.cronPattern
?: annotationPattern
?: throw IllegalArgumentException("Expected $name to have @CronPattern specified")

val runnable = injector.getProvider(cronRunnable.runnableClass.java).get()
Expand Down
18 changes: 18 additions & 0 deletions misk-cron/src/test/kotlin/misk/cron/CronTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class CronTest {
install(CronEntryModule.create<MinuteCron>())
install(CronEntryModule.create<HourCron>())
install(CronEntryModule.create<ThrowsExceptionCron>())
// Override ConfigurableHourCron's schedule to every minute.
install(CronEntryModule.create<ConfigurableHourCron>(cronPattern = CronPattern("* * * * *")))
}
}

Expand All @@ -41,6 +43,7 @@ class CronTest {
@Inject private lateinit var minuteCron: MinuteCron
@Inject private lateinit var hourCron: HourCron
@Inject private lateinit var throwsExceptionCron: ThrowsExceptionCron
@Inject private lateinit var configurableHourCron: ConfigurableHourCron

private lateinit var lastRun: Instant

Expand All @@ -59,15 +62,20 @@ class CronTest {
assertThat(minuteCron.counter).isEqualTo(0)
assertThat(hourCron.counter).isEqualTo(0)
assertThat(throwsExceptionCron.counter).isEqualTo(0)
assertThat(configurableHourCron.counter).isEqualTo(0)

// Advance one hour in one minute intervals.
repeat(60) {
clock.add(Duration.ofMinutes(1))
runCrons()
}

assertThat(minuteCron.counter).isEqualTo(60)
assertThat(throwsExceptionCron.counter).isEqualTo(4)
assertThat(hourCron.counter).isEqualTo(1)
// configurableHourCron has @CronPattern configured to hourly, but is overridden
// to every minute by CronEntryModule::create().
assertThat(configurableHourCron.counter).isEqualTo(60)
}

@Test
Expand Down Expand Up @@ -114,6 +122,16 @@ class HourCron @Inject constructor() : Runnable {
}
}

@Singleton
@CronPattern("0 * * * *") // Default cron schedule of hourly.
class ConfigurableHourCron @Inject constructor() : Runnable {
var counter = 0

override fun run() {
counter++
}
}

@Singleton
@CronPattern("1/15 * * * *")
class ThrowsExceptionCron @Inject constructor() : Runnable {
Expand Down

0 comments on commit ea14d10

Please sign in to comment.