Perform follow-up refactoring for ArisaMain.kt changes (#730)

src/main/kotlin/io/github/mojira/arisa/ArisaMain.kt

@@ -17,14 +17,15 @@ lateinit var jiraClient: JiraClient
 
 fun main() {
     val configService = ConfigService()
-    val webhookService = WebhookService(configService.config)
-    val connectionService = JiraConnectionService(configService.config)
-    val executionService = ExecutionService(configService.config, connectionService)
 
+    val webhookService = WebhookService(configService.config)
     webhookService.setLoggerWebhooks()
 
+    val connectionService = JiraConnectionService(configService.config)
     connectionService.connect()
 
+    val executionService = ExecutionService(configService.config, connectionService)
+
     while (true) {
         val secondsToSleep = executionService.runExecutionCycle()
         TimeUnit.SECONDS.sleep(secondsToSleep)

src/main/kotlin/io/github/mojira/arisa/ExecutionTimeframe.kt

@@ -13,14 +13,14 @@ class ExecutionTimeframe(
     private val openEnded: Boolean
 ) {
     companion object {
-        const val MAX_TIMEFRAME_DURATION_IN_MINUTES = 10L
+        // Visible for testing
+        internal const val MAX_TIMEFRAME_DURATION_IN_MINUTES = 10L
 
         /**
          * @return An [ExecutionTimeframe] beginning at [LastRun], either until right now,
          * or until [MAX_TIMEFRAME_DURATION_IN_MINUTES] after [LastRun].
          */
         fun getTimeframeFromLastRun(lastRun: LastRun): ExecutionTimeframe {
-            // Save time before run, so nothing happening during the run is missed
             val currentTime = Instant.now().truncatedTo(ChronoUnit.MILLIS)
             val endOfMaxTimeframe = lastRun.time.plus(MAX_TIMEFRAME_DURATION_IN_MINUTES, ChronoUnit.MINUTES)
 
@@ -39,14 +39,29 @@ class ExecutionTimeframe(
         }
     }
 
-    fun duration(): Duration = Duration.between(lastRunTime, currentRunTime).abs()
+    /**
+     * Creates a JQL query for freshly updated issues.
+     */
+    fun getFreshlyUpdatedJql() =
+        "updated > ${lastRunTime.toEpochMilli()}${capIfNotOpenEnded()}"
+
+    /**
+     * Creates a JQL query for issues which have been updated in the execution timeframe, shifted to the past
+     * by [offset].
+     */
+    fun getDelayedUpdatedJql(offset: Duration): String {
+        require(!offset.isNegative)
+        val checkStart = lastRunTime.minus(offset)
+        val checkEnd = currentRunTime.minus(offset)
+        return "updated > ${checkStart.toEpochMilli()} AND updated <= ${checkEnd.toEpochMilli()}"
+    }
 
     /**
      * Adds a cap to a JQL query if this time frame is not open.
      *
      * @return If open ended: empty string. Otherwise: ` AND updated <= [currentRunTime]`
      */
-    fun capIfNotOpenEnded(): String =
+    private fun capIfNotOpenEnded(): String =
         if (openEnded) "" else " AND updated <= ${ currentRunTime.toEpochMilli() }"
 
     override fun toString(): String {

src/main/kotlin/io/github/mojira/arisa/JiraConnectionService.kt

@@ -68,10 +68,7 @@ class JiraConnectionService(
         return if (secondsSinceLastSuccessfulConnection > MAX_SECONDS_SINCE_LAST_SUCCESSFUL_CONNECTION) {
             log.info("Trying to relog")
 
-            val exception = establishConnection() ?: run {
-                notifyOfSuccessfulConnection()
-                return@tryRelog RelogResult.SuccessfulRelog()
-            }
+            val exception = establishConnection() ?: return RelogResult.SuccessfulRelog()
 
             val relogResult = RelogResult.UnsucessfulRelog()
             log.error(

src/main/kotlin/io/github/mojira/arisa/registry/DelayedModuleRegistry.kt

@@ -4,19 +4,16 @@ import com.uchuhimo.konf.Config
 import io.github.mojira.arisa.ExecutionTimeframe
 import io.github.mojira.arisa.infrastructure.config.Arisa
 import io.github.mojira.arisa.modules.DuplicateMessageModule
-import java.time.temporal.ChronoUnit
+import java.time.Duration
 
 /**
  * This class is the registry for modules that get executed `commentDelayMinutes` after the ticket has been updated.
  */
 class DelayedModuleRegistry(config: Config) : ModuleRegistry(config) {
-    override fun getJql(timeframe: ExecutionTimeframe): String {
-        val checkStart = timeframe.lastRunTime
-            .minus(config[Arisa.Modules.DuplicateMessage.commentDelayMinutes], ChronoUnit.MINUTES)
-        val checkEnd = timeframe.currentRunTime
-            .minus(config[Arisa.Modules.DuplicateMessage.commentDelayMinutes], ChronoUnit.MINUTES)
+    private val delayOffset = Duration.ofMinutes(config[Arisa.Modules.DuplicateMessage.commentDelayMinutes])
 
-        return "updated > ${checkStart.toEpochMilli()} AND updated <= ${checkEnd.toEpochMilli()}"
+    override fun getJql(timeframe: ExecutionTimeframe): String {
+        return timeframe.getDelayedUpdatedJql(delayOffset)
     }
 
     init {

src/main/kotlin/io/github/mojira/arisa/registry/InstantModuleRegistry.kt

@@ -38,7 +38,7 @@ import io.github.mojira.arisa.modules.TransferVersionsModule
  */
 class InstantModuleRegistry(config: Config) : ModuleRegistry(config) {
     override fun getJql(timeframe: ExecutionTimeframe): String {
-        return "updated > ${ timeframe.lastRunTime.toEpochMilli() }${ timeframe.capIfNotOpenEnded() }"
+        return timeframe.getFreshlyUpdatedJql()
     }
 
     init {

src/main/kotlin/io/github/mojira/arisa/registry/LinkedModuleRegistry.kt

@@ -4,22 +4,19 @@ import com.uchuhimo.konf.Config
 import io.github.mojira.arisa.ExecutionTimeframe
 import io.github.mojira.arisa.infrastructure.config.Arisa
 import io.github.mojira.arisa.modules.UpdateLinkedModule
-import java.time.temporal.ChronoUnit
+import java.time.Duration
 
 /**
  * This class is the registry for the UpdateLinkedModule.
  * It only updates the linked field once if it's not set, and otherwise only updates it at most once per day.
  * This is done in order to avoid spam.
  */
 class LinkedModuleRegistry(config: Config) : ModuleRegistry(config) {
-    override fun getJql(timeframe: ExecutionTimeframe): String {
-        val freshlyUpdatedJql = "updated > ${ timeframe.lastRunTime.toEpochMilli() }${ timeframe.capIfNotOpenEnded() }"
+    private val delayOffset = Duration.ofHours(config[Arisa.Modules.UpdateLinked.updateIntervalHours])
 
-        val intervalEnd = timeframe.currentRunTime.minus(
-            config[Arisa.Modules.UpdateLinked.updateIntervalHours], ChronoUnit.HOURS
-        )
-        val intervalStart = intervalEnd.minus(timeframe.duration())
-        val delayedJql = "updated > ${ intervalStart.toEpochMilli() } AND updated <= ${ intervalEnd.toEpochMilli() }"
+    override fun getJql(timeframe: ExecutionTimeframe): String {
+        val freshlyUpdatedJql = timeframe.getFreshlyUpdatedJql()
+        val delayedJql = timeframe.getDelayedUpdatedJql(delayOffset)
 
         return "($freshlyUpdatedJql) OR ($delayedJql)"
     }

src/test/kotlin/io/github/mojira/arisa/ExecutionTimeframeTest.kt

@@ -2,8 +2,11 @@ package io.github.mojira.arisa
 
 import io.kotest.core.spec.style.StringSpec
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.string.shouldNotContain
 import java.time.Duration
 import java.time.Instant
+import java.time.LocalDateTime
+import java.time.ZoneOffset
 import java.time.temporal.ChronoUnit
 
 class ExecutionTimeframeTest : StringSpec({
@@ -19,20 +22,26 @@ class ExecutionTimeframeTest : StringSpec({
 
         val timeframe = ExecutionTimeframe.getTimeframeFromLastRun(lastRun)
 
-        val timeframeEnd = Instant.now().truncatedTo(ChronoUnit.MILLIS)
-
         timeframe.lastRunTime shouldBe lastRunTime
-        timeframe.currentRunTime.isAfter(timeframeEnd) shouldBe false
-        timeframe.capIfNotOpenEnded() shouldBe ""
-        timeframe.duration() shouldBe Duration.between(lastRunTime, timeframeEnd).abs()
+        timeframe.currentRunTime.isAfter(Instant.now()) shouldBe false
+        // Should not be capped
+        timeframe.getFreshlyUpdatedJql() shouldNotContain " AND updated <= "
+
+        val delayedStart = LocalDateTime.of(2021, 1, 1, 12, 0, 0)
+            .atZone(ZoneOffset.UTC)
+            .toInstant()
+        val delayedEnd = delayedStart.plus(Duration.between(timeframe.lastRunTime, timeframe.currentRunTime))
+        val offset = Duration.between(delayedStart, timeframe.lastRunTime)
+        // Shift timeframe to start at `offsetBaseInstant`
+        // Note: Cannot hardcode `delayedEnd` value in string because it depends on how fast
+        // `ExecutionTimeframe.getTimeframeFromLastRun` executes
+        timeframe.getDelayedUpdatedJql(offset) shouldBe "updated > 1609502400000 AND updated <= ${delayedEnd.toEpochMilli()}"
     }
 
     "getTimeframeFromLastRun should return the correct timeframe if last run was a while ago" {
-        val offsetInMinutes = 20L
-
-        val lastRunTime = Instant.now()
-            .minus(offsetInMinutes, ChronoUnit.MINUTES)
-            .truncatedTo(ChronoUnit.MILLIS)
+        val lastRunTime = LocalDateTime.of(2021, 1, 1, 12, 0, 0)
+            .atZone(ZoneOffset.UTC)
+            .toInstant()
 
         val timeframeEnd = lastRunTime
             .plus(ExecutionTimeframe.MAX_TIMEFRAME_DURATION_IN_MINUTES, ChronoUnit.MINUTES)
@@ -46,8 +55,8 @@ class ExecutionTimeframeTest : StringSpec({
         val timeframe = ExecutionTimeframe.getTimeframeFromLastRun(lastRun)
 
         timeframe.lastRunTime shouldBe lastRunTime
-        timeframe.currentRunTime.isAfter(timeframeEnd) shouldBe false
-        timeframe.capIfNotOpenEnded() shouldBe " AND updated <= ${ timeframeEnd.toEpochMilli() }"
-        timeframe.duration() shouldBe Duration.between(lastRunTime, timeframeEnd).abs()
+        timeframe.currentRunTime shouldBe timeframeEnd
+        timeframe.getFreshlyUpdatedJql() shouldBe "updated > 1609502400000 AND updated <= 1609503000000"
+        timeframe.getDelayedUpdatedJql(Duration.ofHours(1)) shouldBe "updated > 1609498800000 AND updated <= 1609499400000"
     }
 })