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: Import improvements #2044

Merged
merged 13 commits into from
Dec 31, 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 @@ -4,6 +4,7 @@

package io.tolgee.api.v2.controllers

import com.fasterxml.jackson.databind.ObjectMapper
import io.swagger.v3.oas.annotations.Operation
import io.swagger.v3.oas.annotations.Parameter
import io.swagger.v3.oas.annotations.tags.Tag
Expand Down Expand Up @@ -38,7 +39,10 @@ import io.tolgee.security.authorization.RequiresProjectPermissions
import io.tolgee.service.LanguageService
import io.tolgee.service.dataImport.ForceMode
import io.tolgee.service.dataImport.ImportService
import io.tolgee.service.dataImport.status.ImportApplicationStatus
import io.tolgee.service.dataImport.status.ImportApplicationStatusItem
import io.tolgee.service.key.NamespaceService
import io.tolgee.util.StreamingResponseBodyProvider
import jakarta.servlet.http.HttpServletRequest
import org.springdoc.core.annotations.ParameterObject
import org.springframework.data.domain.PageRequest
Expand All @@ -49,6 +53,7 @@ import org.springframework.hateoas.CollectionModel
import org.springframework.hateoas.PagedModel
import org.springframework.hateoas.server.mvc.RepresentationModelAssemblerSupport
import org.springframework.http.MediaType
import org.springframework.http.ResponseEntity
import org.springframework.web.bind.annotation.CrossOrigin
import org.springframework.web.bind.annotation.DeleteMapping
import org.springframework.web.bind.annotation.GetMapping
Expand All @@ -61,6 +66,7 @@ import org.springframework.web.bind.annotation.RequestParam
import org.springframework.web.bind.annotation.RequestPart
import org.springframework.web.bind.annotation.RestController
import org.springframework.web.multipart.MultipartFile
import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody

@Suppress("MVCPathVariableInspection")
@RestController
Expand All @@ -85,6 +91,8 @@ class V2ImportController(
private val languageService: LanguageService,
private val namespaceService: NamespaceService,
private val importFileIssueModelAssembler: ImportFileIssueModelAssembler,
private val streamingResponseBodyProvider: StreamingResponseBodyProvider,
private val objectMapper: ObjectMapper,
) {
@PostMapping("", consumes = [MediaType.MULTIPART_FORM_DATA_VALUE])
@Operation(description = "Prepares provided files to import.", summary = "Add files")
Expand All @@ -94,7 +102,7 @@ class V2ImportController(
@RequestPart("files") files: Array<MultipartFile>,
@ParameterObject params: ImportAddFilesParams,
): ImportAddFilesResultModel {
val fileDtos = files.map { ImportFileDto(it.originalFilename ?: "", it.inputStream) }
val fileDtos = files.map { ImportFileDto(it.originalFilename ?: "", it.inputStream.readAllBytes()) }
val errors =
importService.addFiles(
files = fileDtos,
Expand All @@ -116,7 +124,7 @@ class V2ImportController(
}

@PutMapping("/apply")
@Operation(description = "Imports the data prepared in previous step", summary = "Apply")
@Operation(description = "Imports the data prepared in previous step")
@RequestActivity(ActivityType.IMPORT)
@RequiresProjectPermissions([Scope.TRANSLATIONS_VIEW])
@AllowApiAccess
Expand All @@ -129,6 +137,27 @@ class V2ImportController(
this.importService.import(projectId, authenticationFacade.authenticatedUser.id, forceMode)
}

@PutMapping("/apply-streaming", produces = [MediaType.APPLICATION_NDJSON_VALUE])
@Operation(description = "Imports the data prepared in previous step. Streams current status.")
@RequestActivity(ActivityType.IMPORT)
@RequiresProjectPermissions([Scope.TRANSLATIONS_VIEW])
@AllowApiAccess
fun applyImportStreaming(
@Parameter(description = "Whether override or keep all translations with unresolved conflicts")
@RequestParam(defaultValue = "NO_FORCE")
forceMode: ForceMode,
): ResponseEntity<StreamingResponseBody> {
val projectId = projectHolder.project.id

return streamingResponseBodyProvider.streamNdJson { write ->
val writeStatus = { status: ImportApplicationStatus ->
write(ImportApplicationStatusItem(status))
}

this.importService.import(projectId, authenticationFacade.authenticatedUser.id, forceMode, writeStatus)
}
}

@GetMapping("/result")
@Operation(description = "Returns the result of preparation.", summary = "Get result")
@RequiresProjectPermissions([Scope.TRANSLATIONS_VIEW])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import io.tolgee.service.LanguageService
import io.tolgee.service.key.KeyService
import io.tolgee.service.security.SecurityService
import io.tolgee.service.translation.TranslationMemoryService
import io.tolgee.util.disableAccelBuffering
import jakarta.validation.Valid
import org.springdoc.core.annotations.ParameterObject
import org.springframework.data.domain.Pageable
Expand Down Expand Up @@ -49,7 +50,7 @@ class TranslationSuggestionController(
) {
@PostMapping("/machine-translations")
@Operation(summary = "Suggests machine translations from enabled services")
@RequiresProjectPermissions([ Scope.TRANSLATIONS_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_EDIT])
@AllowApiAccess
fun suggestMachineTranslations(
@RequestBody @Valid
Expand All @@ -65,15 +66,13 @@ class TranslationSuggestionController(
"If an error occurs when any of the services is used," +
" the error information is returned as a part of the result item, while the response has 200 status code.",
)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_EDIT])
@AllowApiAccess
fun suggestMachineTranslationsStreaming(
@RequestBody @Valid
dto: SuggestRequestDto,
): ResponseEntity<StreamingResponseBody> {
return ResponseEntity.ok().headers {
it.add("X-Accel-Buffering", "no")
}.body(
return ResponseEntity.ok().disableAccelBuffering().body(
machineTranslationSuggestionFacade.suggestStreaming(dto),
)
}
Expand All @@ -84,7 +83,7 @@ class TranslationSuggestionController(
"Suggests machine translations from translation memory." +
"\n\nThe result is always sorted by similarity, so sorting is not supported.",
)
@RequiresProjectPermissions([ Scope.TRANSLATIONS_EDIT ])
@RequiresProjectPermissions([Scope.TRANSLATIONS_EDIT])
@AllowApiAccess
fun suggestTranslationMemory(
@RequestBody @Valid
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import io.tolgee.component.fileStorage.LocalFileStorage
import io.tolgee.component.fileStorage.S3ClientProvider
import io.tolgee.component.fileStorage.S3FileStorage
import io.tolgee.configuration.tolgee.TolgeeProperties
import io.tolgee.util.InMemoryFileStorage
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration

Expand All @@ -20,6 +21,9 @@ class FileStorageConfiguration(

@Bean
fun fileStorage(): FileStorage {
if (properties.internal.useInMemoryFileStorage) {
return InMemoryFileStorage()
}
if (s3config.enabled) {
val amazonS3 = S3ClientProvider(s3config).provide()
val bucketName = properties.fileStorage.s3.bucketName ?: throw RuntimeException("Bucket name is not set")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ class V2ExportControllerTest : ProjectAuthControllerTest("/v2/projects/") {
retryingOnCommonIssues {
initBaseData()
try {
executeInNewTransaction {
}
performExport()
performExport()
waitForNotThrowing(pollTime = 50, timeout = 3000) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.springframework.boot.test.context.SpringBootTest
import java.io.File
import java.time.Duration
import java.util.*

Expand Down Expand Up @@ -70,18 +69,16 @@ class SecuredV2ImageUploadControllerTest : AbstractV2ImageUploadControllerTest()
performAuthGet("/uploaded-images/${image.filename}.png?token=$token")
.andIsOk.andReturn().response.contentAsByteArray

val file = File(tolgeeProperties.fileStorage.fsDataPath + "/uploadedImages/" + image.filename + ".png")
assertThat(storedImage).isEqualTo(file.readBytes())
assertThat(storedImage).isEqualTo(fileStorage.readFile("uploadedImages/" + image.filename + ".png"))
}

@Test
@ProjectJWTAuthTestMethod
fun upload() {
performStoreImage().andPrettyPrint.andIsCreated.andAssertThatJson {
node("filename").isString.satisfies {
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/uploadedImages/" + it + ".png")
assertThat(file).exists()
assertThat(file.readBytes().size).isCloseTo(5538, Offset.offset(500))
val path = "uploadedImages/" + it + ".png"
assertThat(fileStorage.readFile(path).size).isCloseTo(5538, Offset.offset(500))
}
node("requestFilename").isString.satisfies {
val parts = it.split("?token=")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import io.tolgee.fixtures.andIsCreated
import io.tolgee.fixtures.andIsForbidden
import io.tolgee.fixtures.andIsOk
import io.tolgee.fixtures.andPrettyPrint
import io.tolgee.testing.assert
import io.tolgee.testing.assertions.Assertions.assertThat
import org.assertj.core.data.Offset
import org.junit.jupiter.api.AfterAll
Expand All @@ -22,7 +23,6 @@ import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.mock.mockito.MockBean
import org.springframework.mock.web.MockMultipartFile
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
import java.io.File
import java.util.stream.Collectors

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
Expand Down Expand Up @@ -50,9 +50,9 @@ class V2ImageUploadControllerTest : AbstractV2ImageUploadControllerTest() {
performStoreImage().andPrettyPrint.andIsCreated.andAssertThatJson {
node("fileUrl").isString.startsWith("http://").endsWith(".png")
node("requestFilename").isString.satisfies {
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/uploadedImages/" + it)
assertThat(file).exists()
assertThat(file.readBytes().size).isCloseTo(5538, Offset.offset(500))
val file = fileStorage.fileExists("uploadedImages/" + it).assert.isTrue()
fileStorage.readFile("uploadedImages/" + it)
.size.assert.isCloseTo(5538, Offset.offset(500))
}
}
}
Expand All @@ -78,15 +78,16 @@ class V2ImageUploadControllerTest : AbstractV2ImageUploadControllerTest() {
@Test
fun `returns file`() {
val image = imageUploadService.store(screenshotFile, userAccount!!, null)

val file = File("""${tolgeeProperties.fileStorage.fsDataPath}/uploadedImages/${image.filenameWithExtension}""")
val result =
performAuthGet("/uploaded-images/${image.filenameWithExtension}").andIsOk
.andExpect(
header().string("Cache-Control", "max-age=365, must-revalidate, no-transform"),
)
.andReturn()
assertThat(result.response.contentAsByteArray).isEqualTo(file.readBytes())
assertThat(result.response.contentAsByteArray)
.isEqualTo(
fileStorage.readFile("uploadedImages/${image.filenameWithExtension}"),
)
}

@Test
Expand All @@ -100,19 +101,15 @@ class V2ImageUploadControllerTest : AbstractV2ImageUploadControllerTest() {
val idsToDelete = list.take(10).map { it.id }.joinToString(",")

list.asSequence().take(10).forEach {
assertThat(
File("""${tolgeeProperties.fileStorage.fsDataPath}/uploadedImages/${it.filenameWithExtension}"""),
).exists()
fileStorage.fileExists("uploadedImages/${it.filenameWithExtension}").assert.isTrue()
}

performAuthDelete("/v2/image-upload/$idsToDelete", null).andIsOk
val rest = imageUploadService.find(list.map { it.id }.toSet())
assertThat(rest).isEqualTo(list.stream().skip(10).collect(Collectors.toList()))

list.asSequence().take(10).forEach {
assertThat(
File("""${tolgeeProperties.fileStorage.fsDataPath}/uploadedImages/${it.filenameWithExtension}"""),
).doesNotExist()
fileStorage.fileExists("uploadedImages/${it.filenameWithExtension}").assert.isFalse()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import io.tolgee.model.dataImport.issues.issueTypes.FileIssueType
import io.tolgee.testing.AuthorizedControllerTest
import io.tolgee.testing.assert
import io.tolgee.testing.assertions.Assertions.assertThat
import io.tolgee.util.InMemoryFileStorage
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Test
import org.springframework.beans.factory.annotation.Value
Expand Down Expand Up @@ -67,16 +68,40 @@ class V2ImportControllerAddFilesTest : AuthorizedControllerTest() {
@AfterEach
fun resetProps() {
tolgeeProperties.maxTranslationTextLength = 10000
(fileStorage as InMemoryFileStorage).clear()
tolgeeProperties.import.storeFilesForDebugging = false
}

@Test
fun `it parses zip file and saves issues`() {
fun `it parses zip file stores it for debugging and saves issues`() {
tolgeeProperties.import.storeFilesForDebugging = true
val base = dbPopulator.createBase(generateUniqueString())
commitTransaction()

performImport(projectId = base.project.id, mapOf(Pair("zipOfUnknown.zip", zipOfUnknown))).andAssertThatJson {
val fileName = "zipOfUnknown.zip"
performImport(projectId = base.project.id, mapOf(Pair(fileName, zipOfUnknown))).andAssertThatJson {
node("errors[2].code").isEqualTo("cannot_parse_file")
}

doesStoredFileExists(fileName, base.project.id).assert.isTrue()
}

@Test
fun `doesn't store file for when disabled`() {
val base = dbPopulator.createBase(generateUniqueString())
commitTransaction()

val fileName = "zipOfUnknown.zip"
performImport(projectId = base.project.id, mapOf(Pair(fileName, zipOfUnknown)))
doesStoredFileExists(fileName, base.project.id).assert.isFalse()
}

fun doesStoredFileExists(
fileName: String,
projectId: Long,
): Boolean {
val import = importService.find(projectId, userAccount!!.id)
return fileStorage.fileExists("importFiles/${import!!.id}/$fileName")
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import io.tolgee.fixtures.andPrettyPrint
import io.tolgee.testing.annotations.ProjectJWTAuthTestMethod
import io.tolgee.testing.assert
import io.tolgee.testing.assertions.Assertions.assertThat
import io.tolgee.util.InMemoryFileStorage
import org.junit.jupiter.api.AfterAll
import org.junit.jupiter.api.BeforeAll
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.springframework.mock.web.MockMultipartFile
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import java.io.File

@TestInstance(TestInstance.Lifecycle.PER_CLASS)
class KeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest() {
Expand All @@ -34,6 +34,7 @@ class KeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest() {
@AfterAll
fun after() {
tolgeeProperties.fileStorageUrl = initialScreenshotUrl
(fileStorage as InMemoryFileStorage).clear()
}

@Test
Expand Down Expand Up @@ -63,8 +64,7 @@ class KeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest() {
val screenshots = screenshotService.findAll(key = key)
assertThat(screenshots).hasSize(1)
node("filename").isEqualTo(screenshots[0].filename)
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/screenshots/" + screenshots[0].filename)
assertThat(file).exists()
fileStorage.fileExists("screenshots/" + screenshots[0].filename).assert.isTrue()
val reference = screenshots[0].keyScreenshotReferences[0]
reference.originalText.assert.isEqualTo(text)
reference.positions!![0].x.assert.isEqualTo(200)
Expand Down Expand Up @@ -116,8 +116,7 @@ class KeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest() {
performProjectAuthGet("keys/${key.id}/screenshots").andIsOk.andPrettyPrint.andAssertThatJson {
node("_embedded.screenshots").isArray.hasSize(2)
node("_embedded.screenshots[0].filename").isString.satisfies {
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/screenshots/" + it)
assertThat(file.exists()).isTrue()
fileStorage.fileExists("screenshots/" + it).assert.isTrue()
}
}

Expand Down Expand Up @@ -151,15 +150,14 @@ class KeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest() {
val key = keyService.create(project, CreateKeyDto("test"))
screenshotService.store(screenshotFile, key, null)
}
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/screenshots/" + screenshot.filename)
val result =
performAuthGet("/screenshots/${screenshot.filename}").andIsOk
.andExpect(
header().string("Cache-Control", "max-age=365, must-revalidate, no-transform"),
)
.andReturn()
performAuthGet("/screenshots/${screenshot.thumbnailFilename}").andIsOk
assertThat(result.response.contentAsByteArray).isEqualTo(file.readBytes())
assertThat(result.response.contentAsByteArray).isEqualTo(fileStorage.readFile("screenshots/" + screenshot.filename))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.TestInstance
import org.springframework.boot.test.context.SpringBootTest
import java.io.File
import java.time.Duration
import java.util.*

Expand Down Expand Up @@ -103,9 +102,8 @@ class SecuredKeyScreenshotControllerTest : AbstractV2ScreenshotControllerTest()
executeInNewTransaction {
val screenshots = screenshotService.findAll(key = key)
assertThat(screenshots).hasSize(1)
val file = File(tolgeeProperties.fileStorage.fsDataPath + "/screenshots/" + screenshots[0].filename)
assertThat(file).exists()
assertThat(file.readBytes().size).isCloseTo(1070, Offset.offset(500))
val bytes = fileStorage.readFile("screenshots/" + screenshots[0].filename)
assertThat(bytes.size).isCloseTo(1070, Offset.offset(500))
node("filename").isString.startsWith(screenshots[0].filename).satisfies {
val parts = it.split("?token=")
val auth = jwtService.validateTicket(parts[1], JwtService.TicketType.IMG_ACCESS)
Expand Down
Loading
Loading