Skip to content

Commit

Permalink
fix: Automations not exclusive for project (#2049)
Browse files Browse the repository at this point in the history
  • Loading branch information
JanCizmar authored Dec 28, 2023
1 parent 6c89099 commit 234ad6a
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.swagger.v3.oas.annotations.tags.Tag
import io.tolgee.component.reporting.BusinessEventPublisher
import io.tolgee.dtos.request.BusinessEventReportRequest
import io.tolgee.dtos.request.IdentifyRequest
import io.tolgee.exceptions.AuthenticationException
import io.tolgee.service.organization.OrganizationRoleService
import io.tolgee.service.security.SecurityService
import io.tolgee.util.Logging
Expand Down Expand Up @@ -33,6 +34,9 @@ class BusinessEventController(
eventData.organizationId?.let { organizationRoleService.checkUserCanView(it) }
businessEventPublisher.publish(eventData)
} catch (e: Throwable) {
if (e is AuthenticationException) {
return
}
logger.error("Error storing event", e)
Sentry.captureException(e)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import io.tolgee.fixtures.waitForNotThrowing
import io.tolgee.service.contentDelivery.ContentDeliveryConfigService
import io.tolgee.testing.annotations.ProjectJWTAuthTestMethod
import io.tolgee.testing.assert
import io.tolgee.util.addMinutes
import io.tolgee.util.addSeconds
import net.javacrumbs.jsonunit.assertj.assertThatJson
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
Expand Down Expand Up @@ -75,16 +75,13 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") {
@BeforeEach
fun before() {
Mockito.reset(restTemplate, webhookRestTemplate)
webhookInvocationCount = 0
}

@AfterEach
fun after() {
currentDateProvider.forcedDate = null
}

var webhookInvocationCount = 0

@Test
@ProjectJWTAuthTestMethod
fun `publishes to Content Delivery`() {
Expand Down Expand Up @@ -124,9 +121,9 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") {
this.projectSupplier = { testData.projectBuilder.self }
mockWebhookResponse(HttpStatus.OK)

modifyTranslationData()

verifyWebhookExecuted(testData)
verifyWebhookExecuted(testData) {
modifyTranslationData()
}
}

@Test
Expand All @@ -139,17 +136,23 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") {
this.projectSupplier = { testData.projectBuilder.self }
mockWebhookResponse(HttpStatus.BAD_REQUEST)

modifyTranslationData()

verifyWebhookExecuted(testData)
verifyWebhookExecuted(testData) {
modifyTranslationData()
}
webhookConfigService.get(testData.webhookConfig.self.id).firstFailed!!
.time.assert.isEqualTo(currentDateProvider.date.time)

mockWebhookResponse(HttpStatus.OK)

currentDateProvider.forcedDate = currentDateProvider.date.addMinutes(60)
modifyTranslationData()
verifyWebhookExecuted(testData)
verifyWebhookExecuted(testData) {
// webhooks are configured to be retried after 5 seconds
currentDateProvider.forcedDate = currentDateProvider.date.addSeconds(5)
}

verifyWebhookExecuted(testData) {
modifyTranslationData()
}

webhookConfigService.get(testData.webhookConfig.self.id).firstFailed.assert.isNull()
}

Expand All @@ -165,10 +168,11 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") {
)
}

private fun verifyWebhookExecuted(testData: WebhooksTestData) {
val newExpectedInvocationsCount = ++webhookInvocationCount
private fun verifyWebhookExecuted(testData: WebhooksTestData, webhookTriggeringCallback: () -> Unit) {
val invocations = getWebhookRestTemplateInvocationCount()
webhookTriggeringCallback()
waitForNotThrowing {
Mockito.mockingDetails(webhookRestTemplate).invocations.count().assert.isEqualTo(newExpectedInvocationsCount)
getWebhookRestTemplateInvocationCount().assert.isEqualTo(invocations + 1)
val callArguments = Mockito.mockingDetails(webhookRestTemplate).invocations.last().arguments
callArguments[0].assert
.isEqualTo(testData.webhookConfig.self.url)
Expand All @@ -188,6 +192,8 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") {
}
}

private fun getWebhookRestTemplateInvocationCount() = Mockito.mockingDetails(webhookRestTemplate).invocations.count()

private fun verifyWebhookSignature(httpEntity: HttpEntity<String>, secret: String) {
val signature = httpEntity.headers["Tolgee-Signature"]
signature.assert.isNotNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,7 @@ abstract class AbstractBatchJobsGeneralTest : AbstractSpringTest(), Logging {
val thirdExecution = executions[job1.id]!![1]
val fourthExecution = executions[job2.id]!![1]

util.waitForQueueSize(4)
batchJobChunkExecutionQueue.clear()
util.waitForQueueSize(0)
batchJobConcurrentLauncher.pause = false
util.waitAndClearQueue(4)

batchJobChunkExecutionQueue.addToQueue(listOf(firstExecution))
util.waitForExecutionSuccess(firstExecution)
Expand Down Expand Up @@ -234,6 +231,29 @@ abstract class AbstractBatchJobsGeneralTest : AbstractSpringTest(), Logging {
util.assertJobUnlocked()
}

@Test
fun `doesn't lock non-exclusive job`() {
clearForcedDate()
batchJobConcurrentLauncher.pause = true

val job1 = util.runChunkedJob(20)
val job2 = util.runNonExclusiveJob()

val executions = util.getExecutions(listOf(job1.id, job2.id))

val firstExecution = executions[job1.id]!!.first()
val secondExecution = executions[job2.id]!!.first()

util.waitAndClearQueue(3)

batchJobChunkExecutionQueue.addToQueue(listOf(firstExecution))
util.waitForExecutionSuccess(firstExecution)
util.verifyJobLocked(job1)

batchJobChunkExecutionQueue.addToQueue(listOf(secondExecution))
util.waitForExecutionSuccess(secondExecution)
}

/**
* the chunk processing status is stored in the database in the same transaction
* so when it fails on some management processing issue, we need to handle this
Expand Down
22 changes: 20 additions & 2 deletions backend/app/src/test/kotlin/io/tolgee/batch/BatchJobTestUtil.kt
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,21 @@ class BatchJobTestUtil(
}

fun runDebouncedJob(): BatchJob {
return runAutomationJob(Duration.ofSeconds(10))
}

fun runNonExclusiveJob(): BatchJob {
return runAutomationJob(null)
}

private fun runAutomationJob(duration: Duration?): BatchJob {
return executeInNewTransaction(transactionManager) {
batchJobService.startJob(
request = AutomationBjRequest(1, 1, 1),
project = testData.projectBuilder.self,
author = testData.user,
type = BatchJobType.AUTOMATION,
debounceDuration = Duration.ofSeconds(10)
debounceDuration = duration
)
}
}
Expand All @@ -348,6 +356,13 @@ class BatchJobTestUtil(
}
}

fun waitAndClearQueue(waitForQueueSize: Int) {
this.waitForQueueSize(waitForQueueSize)
batchJobChunkExecutionQueue.clear()
this.waitForQueueSize(0)
batchJobConcurrentLauncher.pause = false
}

fun waitForExecutionSuccess(execution: BatchJobChunkExecution) {
waitFor(pollTime = 1000) {
batchJobService.getExecution(execution.id).status == BatchJobChunkExecutionStatus.SUCCESS
Expand All @@ -361,7 +376,7 @@ class BatchJobTestUtil(
fun verifiedTriedToLockJob(jobId: Long) {
waitForNotThrowing {
verify(batchJobProjectLockingManager, atLeast(1))
.canRunBatchJobOfExecution(ArgumentMatchers.eq(jobId))
.canLockJobForProject(ArgumentMatchers.eq(jobId))
}
}

Expand Down Expand Up @@ -423,4 +438,7 @@ class BatchJobTestUtil(

private val batchJobService: BatchJobService
get() = applicationContext.getBean(BatchJobService::class.java)

private val batchJobConcurrentLauncher: BatchJobConcurrentLauncher
get() = applicationContext.getBean(BatchJobConcurrentLauncher::class.java)
}
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class BatchJobConcurrentLauncher(
/**
* Only single job can run in project at the same time
*/
if (!batchJobProjectLockingManager.canRunBatchJobOfExecution(executionItem.jobId)) {
if (!batchJobProjectLockingManager.canLockJobForProject(executionItem.jobId)) {
logger.debug(
"⚠️ Cannot run execution ${executionItem.chunkExecutionId}. " +
"Other job from the project is currently running, skipping"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ class BatchJobProjectLockingManager(
}
}

fun canRunBatchJobOfExecution(batchJobId: Long): Boolean {
fun canLockJobForProject(batchJobId: Long): Boolean {
val jobDto = batchJobService.getJobDto(batchJobId)
if (!jobDto.type.exclusive) {
return true
}
return tryLockJobForProject(jobDto)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ enum class BatchJobType(
val maxRetries: Int,
val processor: KClass<out ChunkProcessor<*, *, *>>,
val defaultRetryWaitTimeInMs: Int = 2000,
val exclusive: Boolean = true,
) {
PRE_TRANSLATE_BT_TM(
activityType = ActivityType.BATCH_PRE_TRANSLATE_BY_TM,
Expand Down Expand Up @@ -78,6 +79,7 @@ enum class BatchJobType(
activityType = ActivityType.AUTOMATION,
maxRetries = 3,
processor = AutomationChunkProcessor::class,
exclusive = false
)
;
}

0 comments on commit 234ad6a

Please sign in to comment.