Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Automations not exclusive for project #2049

Merged
merged 3 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
)
;
}
Loading