Skip to content

Commit

Permalink
fix: Import performance (#2025)
Browse files Browse the repository at this point in the history
Co-authored-by: Geert Zondervan <zondervan@serviceplanet.nl>
  • Loading branch information
JanCizmar and Geert Zondervan authored Dec 18, 2023
1 parent 86ad4bd commit 5525cb3
Show file tree
Hide file tree
Showing 47 changed files with 741 additions and 198 deletions.
9 changes: 9 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ jobs:
path: |
./**/build/reports/**/*
- name: Test Report
uses: dorny/test-reporter@v1
if: always()
with:
name: Backend Tests
path: "**/build/test-results/**/TEST-*.xml"
reporter: java-junit


e2e:
needs: [frontend-build, backend-build, e2e-install-deps]
runs-on: ubuntu-latest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ class TranslationCommentController(
@UseDefaultPermissions
@AllowApiAccess
fun get(@PathVariable translationId: Long, @PathVariable commentId: Long): TranslationCommentModel {
val comment = translationCommentService.get(commentId)
val comment = translationCommentService.getWithAuthorFetched(commentId)
comment.checkFromProject()
return translationCommentModelAssembler.toModel(comment)
}
Expand All @@ -106,7 +106,7 @@ class TranslationCommentController(
@UseDefaultPermissions // Security: Permission check done inside; users should be able to edit their comments
@AllowApiAccess
fun update(@PathVariable commentId: Long, @RequestBody @Valid dto: TranslationCommentDto): TranslationCommentModel {
val comment = translationCommentService.get(commentId)
val comment = translationCommentService.getWithAuthorFetched(commentId)
if (comment.author.id != authenticationFacade.authenticatedUser.id) {
throw BadRequestException(io.tolgee.constants.Message.CAN_EDIT_ONLY_OWN_COMMENT)
}
Expand All @@ -123,7 +123,7 @@ class TranslationCommentController(
@PathVariable commentId: Long,
@PathVariable state: TranslationCommentState
): TranslationCommentModel {
val comment = translationCommentService.get(commentId)
val comment = translationCommentService.getWithAuthorFetched(commentId)
comment.checkFromProject()
translationCommentService.setState(comment, state)
return translationCommentModelAssembler.toModel(comment)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ When null, resulting file will be a flat key-value object.
@PutMapping("")
@Operation(summary = "Sets translations for existing key")
@RequestActivity(ActivityType.SET_TRANSLATIONS)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_EDIT])
@AllowApiAccess
fun setTranslations(@RequestBody @Valid dto: SetTranslationsWithKeyDto): SetTranslationsResponseModel {
val key = keyService.get(projectHolder.project.id, dto.key, dto.namespace)
Expand All @@ -182,8 +182,7 @@ When null, resulting file will be a flat key-value object.

val translations = dto.languagesToReturn
?.let { languagesToReturn ->
key.translations
.filter { languagesToReturn.contains(it.language.tag) }
translationService.findForKeyByLanguages(key, languagesToReturn)
.associateBy { it.language.tag }
}
?: modifiedTranslations
Expand All @@ -194,7 +193,7 @@ When null, resulting file will be a flat key-value object.
@PostMapping("")
@Operation(summary = "Sets translations for existing or not existing key.")
@RequestActivity(ActivityType.SET_TRANSLATIONS)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_EDIT])
@AllowApiAccess
fun createOrUpdateTranslations(@RequestBody @Valid dto: SetTranslationsWithKeyDto): SetTranslationsResponseModel {
val key = keyService.find(projectHolder.projectEntity.id, dto.key, dto.namespace)?.also {
Expand All @@ -211,7 +210,7 @@ When null, resulting file will be a flat key-value object.
@PutMapping("/{translationId}/set-state/{state}")
@Operation(summary = "Sets translation state")
@RequestActivity(ActivityType.SET_TRANSLATION_STATE)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_STATE_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_STATE_EDIT])
@AllowApiAccess
fun setTranslationState(
@PathVariable translationId: Long,
Expand Down Expand Up @@ -266,7 +265,7 @@ When null, resulting file will be a flat key-value object.

@GetMapping(value = ["select-all"])
@Operation(summary = "Get select all keys")
@RequiresProjectPermissions([ Scope.KEYS_VIEW ])
@RequiresProjectPermissions([Scope.KEYS_VIEW])
@AllowApiAccess
fun getSelectAllKeyIds(
@ParameterObject @ModelAttribute("translationFilters") params: TranslationFilters,
Expand All @@ -289,7 +288,7 @@ When null, resulting file will be a flat key-value object.
@PutMapping(value = ["/{translationId:[0-9]+}/dismiss-auto-translated-state"])
@Operation(summary = """Removes "auto translated" indication""")
@RequestActivity(ActivityType.DISMISS_AUTO_TRANSLATED_STATE)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_STATE_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_STATE_EDIT])
@AllowApiAccess
fun dismissAutoTranslatedState(
@PathVariable translationId: Long
Expand All @@ -304,7 +303,7 @@ When null, resulting file will be a flat key-value object.
@PutMapping(value = ["/{translationId:[0-9]+}/set-outdated-flag/{state}"])
@Operation(summary = """Set's "outdated" indication""")
@RequestActivity(ActivityType.SET_OUTDATED_FLAG)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_STATE_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_STATE_EDIT])
@AllowApiAccess
fun setOutdated(
@PathVariable translationId: Long,
Expand All @@ -322,7 +321,7 @@ When null, resulting file will be a flat key-value object.
Sorting is not supported for supported. It is automatically sorted from newest to oldest."""
)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_VIEW ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_VIEW])
@AllowApiAccess
fun getTranslationHistory(
@PathVariable translationId: Long,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package io.tolgee.hateoas.translations.comments

import io.swagger.v3.oas.annotations.media.Schema
import io.tolgee.hateoas.user_account.UserAccountModel
import io.tolgee.hateoas.user_account.SimpleUserAccountModel
import io.tolgee.model.enums.TranslationCommentState
import org.springframework.hateoas.RepresentationModel
import org.springframework.hateoas.server.core.Relation
Expand All @@ -20,7 +20,7 @@ open class TranslationCommentModel(
val state: TranslationCommentState,

@Schema(description = "User who created the comment")
val author: UserAccountModel,
val author: SimpleUserAccountModel,

@Schema(description = "Date when it was created")
val createdAt: Date,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package io.tolgee.hateoas.translations.comments

import io.tolgee.api.v2.controllers.translation.TranslationCommentController
import io.tolgee.hateoas.user_account.UserAccountModelAssembler
import io.tolgee.hateoas.user_account.SimpleUserAccountModelAssembler
import io.tolgee.model.translation.TranslationComment
import org.springframework.hateoas.server.mvc.RepresentationModelAssemblerSupport
import org.springframework.stereotype.Component
import java.util.*

@Component
class TranslationCommentModelAssembler(
private val userAccountModelAssembler: UserAccountModelAssembler
private val simpleUserAccountModelAssembler: SimpleUserAccountModelAssembler
) : RepresentationModelAssemblerSupport<TranslationComment, TranslationCommentModel>(
TranslationCommentController::class.java, TranslationCommentModel::class.java
) {
Expand All @@ -18,7 +18,7 @@ class TranslationCommentModelAssembler(
id = entity.id,
text = entity.text,
state = entity.state,
author = entity.author.let { userAccountModelAssembler.toModel(it) },
author = entity.author.let { simpleUserAccountModelAssembler.toModel(it) },
createdAt = entity.createdAt ?: Date(),
updatedAt = entity.updatedAt ?: Date()
)
Expand Down
2 changes: 0 additions & 2 deletions backend/app/src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ spring:
pathmatch:
matching-strategy: ant_path_matcher
jpa:
# open-in-view: false
properties:
hibernate:
order_by:
Expand All @@ -25,7 +24,6 @@ spring:
enhancer:
enableLazyInitialization: true
enableDirtyTracking: true
# open-in-view: false
batch:
job:
enabled: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,37 +69,38 @@ class V2LanguageControllerTest : ProjectAuthControllerTest("/v2/projects/") {

@Test
fun deleteLanguage() {
val base = dbPopulator.createBase(generateUniqueString())
val project = base.project
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }
performDelete(project.id, deutsch.id).andExpect(MockMvcResultMatchers.status().isOk)
executeInNewTransaction {
val base = dbPopulator.createBase(generateUniqueString())
val project = base.project
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }
performDelete(project.id, deutsch.id).andExpect(MockMvcResultMatchers.status().isOk)
Assertions.assertThat(languageService.findById(deutsch.id)).isEmpty
}
}

@Test
@ProjectApiKeyAuthTestMethod(scopes = [Scope.LANGUAGES_EDIT])
fun `deletes language with API key`() {
val base = dbPopulator.createBase(generateUniqueString())
this.userAccount = base.userAccount
this.projectSupplier = { base.project }
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }

performProjectAuthDelete("languages/${deutsch.id}", null)
.andExpect(MockMvcResultMatchers.status().isOk)
executeInNewTransaction {
val base = dbPopulator.createBase(generateUniqueString())
this.userAccount = base.userAccount
this.projectSupplier = { base.project }
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }
performProjectAuthDelete("languages/${deutsch.id}", null)
.andExpect(MockMvcResultMatchers.status().isOk)
Assertions.assertThat(languageService.findById(deutsch.id)).isEmpty
}
}

@Test
@ProjectApiKeyAuthTestMethod(scopes = [Scope.TRANSLATIONS_VIEW])
fun `does not delete language with API key (permissions)`() {
val base = dbPopulator.createBase(generateUniqueString())
this.userAccount = base.userAccount
this.projectSupplier = { base.project }
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }
executeInNewTransaction {
val base = dbPopulator.createBase(generateUniqueString())
this.userAccount = base.userAccount
this.projectSupplier = { base.project }
val deutsch = project.findLanguageOptional("de").orElseThrow { NotFoundException() }
performProjectAuthDelete("languages/${deutsch.id}", null).andIsForbidden
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ class SecuredKeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest()
@Test
@ProjectJWTAuthTestMethod
fun uploadScreenshot() {
executeInNewTransaction {
val key = keyService.create(project, CreateKeyDto("test"))
val key = keyService.create(project, CreateKeyDto("test"))

performStoreScreenshot(project, key).andIsCreated.andAssertThatJson {
performStoreScreenshot(project, key).andIsCreated.andAssertThatJson {
executeInNewTransaction {
val screenshots = screenshotService.findAll(key = key)
assertThat(screenshots).hasSize(1)
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/screenshots/" + screenshots[0].filename)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import io.tolgee.AbstractSpringTest
import io.tolgee.development.testDataBuilder.data.dataImport.ImportNamespacesTestData
import io.tolgee.development.testDataBuilder.data.dataImport.ImportTestData
import io.tolgee.dtos.cacheable.UserAccountDto
import io.tolgee.fixtures.waitForNotThrowing
import io.tolgee.model.dataImport.Import
import io.tolgee.model.dataImport.ImportTranslation
import io.tolgee.security.authentication.TolgeeAuthentication
import io.tolgee.security.authentication.TolgeeAuthenticationDetails
import io.tolgee.testing.assert
Expand Down Expand Up @@ -37,11 +40,13 @@ class ImportServiceTest : AbstractSpringTest() {
importService.selectExistingLanguage(importFrench, french)
assertThat(importFrench.existingLanguage).isEqualTo(french)
val translations = importService.findTranslations(importTestData.importFrench.id)
assertThat(translations[0].conflict).isNotNull
assertThat(translations[1].conflict).isNull()
assertThat(translations.getByKey("what a key").conflict).isNotNull
assertThat(translations.getByKey("what a beautiful key").conflict).isNull()
}
}

fun Collection<ImportTranslation>.getByKey(key: String) = this.find { it.key.name == key } ?: error("Key not found")

@Test
fun `deletes import language`() {
val testData = executeInNewTransaction {
Expand All @@ -63,7 +68,30 @@ class ImportServiceTest : AbstractSpringTest() {
}

@Test
fun `deletes import`() {
fun `hard deletes import`() {
val testData = ImportTestData()
executeInNewTransaction {
testData.addFileIssues()
testData.addKeyMetadata()
testDataService.saveTestData(testData.root)
}

executeInNewTransaction {
val import = importService.get(testData.import.id)
importService.hardDeleteImport(import)
}
}

private fun checkImportHardDeleted(id: Long) {
executeInNewTransaction {
entityManager.createQuery("from Import i where i.id = :id", Import::class.java)
.setParameter("id", id)
.resultList.firstOrNull().assert.isNull()
}
}

@Test
fun `soft deletes import`() {
val testData = ImportTestData()
executeInNewTransaction {
testData.addFileIssues()
Expand All @@ -79,6 +107,10 @@ class ImportServiceTest : AbstractSpringTest() {
executeInNewTransaction {
assertThat(importService.find(testData.import.project.id, testData.import.author.id)).isNull()
}

waitForNotThrowing(pollTime = 200, timeout = 10000) {
checkImportHardDeleted(testData.import.id)
}
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion backend/data/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ dependencies {
/**
* DB
*/
runtimeOnly 'org.postgresql:postgresql'
implementation 'org.postgresql:postgresql'
implementation "org.hibernate:hibernate-jpamodelgen:$hibernateVersion"
kapt "org.hibernate:hibernate-jpamodelgen:$hibernateVersion"

Expand Down
21 changes: 21 additions & 0 deletions backend/data/src/main/kotlin/io/tolgee/activity/ActivityHolder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package io.tolgee.activity
import io.tolgee.activity.data.ActivityType
import io.tolgee.activity.iterceptor.InterceptedEventsManager
import io.tolgee.model.EntityWithId
import io.tolgee.model.activity.ActivityDescribingEntity
import io.tolgee.model.activity.ActivityModifiedEntity
import io.tolgee.model.activity.ActivityRevision
import jakarta.annotation.PreDestroy
import org.springframework.context.ApplicationContext
import kotlin.reflect.KClass

Expand Down Expand Up @@ -44,6 +46,25 @@ open class ActivityHolder(val applicationContext: ApplicationContext) {
this.applicationContext.getBean(InterceptedEventsManager::class.java).initActivityHolder()
field = value
}

var destroyer: (() -> Unit)? = null

open val describingRelationCache: MutableMap<Pair<Long, String>, ActivityDescribingEntity> by lazy {
activityRevision.describingRelations.associateBy { it.entityId to it.entityClass }.toMutableMap()
}

fun getDescribingRelationFromCache(
entityId: Long,
entityClass: String,
provider: () -> ActivityDescribingEntity
): ActivityDescribingEntity {
return describingRelationCache.getOrPut(entityId to entityClass, provider)
}

@PreDestroy
fun destroy() {
destroyer?.invoke()
}
}

typealias ModifiedEntitiesType = MutableMap<KClass<out EntityWithId>, MutableMap<Long, ActivityModifiedEntity>>
Loading

0 comments on commit 5525cb3

Please sign in to comment.