From 305176a40daa29b76b14317b06d092605e1e0100 Mon Sep 17 00:00:00 2001 From: Jan Cizmar Date: Wed, 27 Dec 2023 16:25:23 +0100 Subject: [PATCH 1/3] fix: Automations not exclusive for project --- .../batch/AbstractBatchJobsGeneralTest.kt | 28 ++++++++++++++++--- .../io/tolgee/batch/BatchJobTestUtil.kt | 22 +++++++++++++-- .../batch/BatchJobConcurrentLauncher.kt | 2 +- .../batch/BatchJobProjectLockingManager.kt | 5 +++- .../io/tolgee/batch/data/BatchJobType.kt | 2 ++ 5 files changed, 51 insertions(+), 8 deletions(-) diff --git a/backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt b/backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt index c246277e0d..bbfccec892 100644 --- a/backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt +++ b/backend/app/src/test/kotlin/io/tolgee/batch/AbstractBatchJobsGeneralTest.kt @@ -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) @@ -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 diff --git a/backend/app/src/test/kotlin/io/tolgee/batch/BatchJobTestUtil.kt b/backend/app/src/test/kotlin/io/tolgee/batch/BatchJobTestUtil.kt index e02b62289c..cfa6d901b0 100644 --- a/backend/app/src/test/kotlin/io/tolgee/batch/BatchJobTestUtil.kt +++ b/backend/app/src/test/kotlin/io/tolgee/batch/BatchJobTestUtil.kt @@ -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 ) } } @@ -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 @@ -361,7 +376,7 @@ class BatchJobTestUtil( fun verifiedTriedToLockJob(jobId: Long) { waitForNotThrowing { verify(batchJobProjectLockingManager, atLeast(1)) - .canRunBatchJobOfExecution(ArgumentMatchers.eq(jobId)) + .canLockJobForProject(ArgumentMatchers.eq(jobId)) } } @@ -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) } diff --git a/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobConcurrentLauncher.kt b/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobConcurrentLauncher.kt index 46693be253..b6cb79524d 100644 --- a/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobConcurrentLauncher.kt +++ b/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobConcurrentLauncher.kt @@ -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" diff --git a/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobProjectLockingManager.kt b/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobProjectLockingManager.kt index 2c1fb969d6..c9d6f0be0c 100644 --- a/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobProjectLockingManager.kt +++ b/backend/data/src/main/kotlin/io/tolgee/batch/BatchJobProjectLockingManager.kt @@ -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) } diff --git a/backend/data/src/main/kotlin/io/tolgee/batch/data/BatchJobType.kt b/backend/data/src/main/kotlin/io/tolgee/batch/data/BatchJobType.kt index 79babbd818..d00a5453b4 100644 --- a/backend/data/src/main/kotlin/io/tolgee/batch/data/BatchJobType.kt +++ b/backend/data/src/main/kotlin/io/tolgee/batch/data/BatchJobType.kt @@ -23,6 +23,7 @@ enum class BatchJobType( val maxRetries: Int, val processor: KClass>, val defaultRetryWaitTimeInMs: Int = 2000, + val exclusive: Boolean = true, ) { PRE_TRANSLATE_BT_TM( activityType = ActivityType.BATCH_PRE_TRANSLATE_BY_TM, @@ -78,6 +79,7 @@ enum class BatchJobType( activityType = ActivityType.AUTOMATION, maxRetries = 3, processor = AutomationChunkProcessor::class, + exclusive = false ) ; } From eaffdb9edc79cd5489cf37c5b282bb5b17f238ba Mon Sep 17 00:00:00 2001 From: Jan Cizmar Date: Thu, 28 Dec 2023 11:03:02 +0100 Subject: [PATCH 2/3] chore: Fix test, sentry error logging --- .../v2/controllers/BusinessEventController.kt | 4 +++ .../automation/AutomationIntegrationTest.kt | 29 ++++++++++--------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt b/backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt index 78d8703cff..2d8c61c5dc 100644 --- a/backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt +++ b/backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/BusinessEventController.kt @@ -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 @@ -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) } diff --git a/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt b/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt index f59a6f253a..5451eafa31 100644 --- a/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt +++ b/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt @@ -75,7 +75,6 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { @BeforeEach fun before() { Mockito.reset(restTemplate, webhookRestTemplate) - webhookInvocationCount = 0 } @AfterEach @@ -83,8 +82,6 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { currentDateProvider.forcedDate = null } - var webhookInvocationCount = 0 - @Test @ProjectJWTAuthTestMethod fun `publishes to Content Delivery`() { @@ -124,9 +121,9 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { this.projectSupplier = { testData.projectBuilder.self } mockWebhookResponse(HttpStatus.OK) - modifyTranslationData() - - verifyWebhookExecuted(testData) + verifyWebhookExecuted(testData) { + modifyTranslationData() + } } @Test @@ -139,17 +136,20 @@ 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) { + modifyTranslationData() + } + webhookConfigService.get(testData.webhookConfig.self.id).firstFailed.assert.isNull() } @@ -165,10 +165,11 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { ) } - private fun verifyWebhookExecuted(testData: WebhooksTestData) { - val newExpectedInvocationsCount = ++webhookInvocationCount + private fun verifyWebhookExecuted(testData: WebhooksTestData, webhookTriggeringCallback: () -> Unit) { + val invocations = Mockito.mockingDetails(webhookRestTemplate).invocations.count() + webhookTriggeringCallback() waitForNotThrowing { - Mockito.mockingDetails(webhookRestTemplate).invocations.count().assert.isEqualTo(newExpectedInvocationsCount) + Mockito.mockingDetails(webhookRestTemplate).invocations.count().assert.isEqualTo(invocations + 1) val callArguments = Mockito.mockingDetails(webhookRestTemplate).invocations.last().arguments callArguments[0].assert .isEqualTo(testData.webhookConfig.self.url) From 37448dbb356487b7942d848656561b8981fe37b5 Mon Sep 17 00:00:00 2001 From: Jan Cizmar Date: Thu, 28 Dec 2023 12:11:09 +0100 Subject: [PATCH 3/3] chore: Fix test --- .../tolgee/automation/AutomationIntegrationTest.kt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt b/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt index 5451eafa31..944b002564 100644 --- a/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt +++ b/backend/app/src/test/kotlin/io/tolgee/automation/AutomationIntegrationTest.kt @@ -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 @@ -144,7 +144,10 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { mockWebhookResponse(HttpStatus.OK) - currentDateProvider.forcedDate = currentDateProvider.date.addMinutes(60) + verifyWebhookExecuted(testData) { + // webhooks are configured to be retried after 5 seconds + currentDateProvider.forcedDate = currentDateProvider.date.addSeconds(5) + } verifyWebhookExecuted(testData) { modifyTranslationData() @@ -166,10 +169,10 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { } private fun verifyWebhookExecuted(testData: WebhooksTestData, webhookTriggeringCallback: () -> Unit) { - val invocations = Mockito.mockingDetails(webhookRestTemplate).invocations.count() + val invocations = getWebhookRestTemplateInvocationCount() webhookTriggeringCallback() waitForNotThrowing { - Mockito.mockingDetails(webhookRestTemplate).invocations.count().assert.isEqualTo(invocations + 1) + getWebhookRestTemplateInvocationCount().assert.isEqualTo(invocations + 1) val callArguments = Mockito.mockingDetails(webhookRestTemplate).invocations.last().arguments callArguments[0].assert .isEqualTo(testData.webhookConfig.self.url) @@ -189,6 +192,8 @@ class AutomationIntegrationTest : ProjectAuthControllerTest("/v2/projects/") { } } + private fun getWebhookRestTemplateInvocationCount() = Mockito.mockingDetails(webhookRestTemplate).invocations.count() + private fun verifyWebhookSignature(httpEntity: HttpEntity, secret: String) { val signature = httpEntity.headers["Tolgee-Signature"] signature.assert.isNotNull