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: issue #18 - Duplicate default strings.xml #19

Merged
merged 1 commit into from
Jun 21, 2024
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
28 changes: 15 additions & 13 deletions client/src/main/kotlin/phraseapp/internal/platforms/Platform.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package phraseapp.internal.platforms
import phraseapp.internal.xml.ArbPrinterScanner
import phraseapp.internal.xml.Visitor
import phraseapp.internal.xml.XmlPrinterScanner
import phraseapp.repositories.operations.DefaultType
import phraseapp.repositories.operations.LanguageType
import phraseapp.repositories.operations.LocaleType
import phraseapp.repositories.operations.ResFolderType
Expand Down Expand Up @@ -32,10 +31,11 @@ object Android : Platform() {
return defaultStringsFile
}

override fun getResPath(type: ResFolderType): String = when (type) {
is DefaultType -> "values"
is LanguageType -> "values-${type.language.lowercase()}"
is LocaleType -> "values-${type.language.lowercase()}-r${type.country.uppercase()}"
override fun getResPath(type: ResFolderType): String = when {
type.isDefault -> "values"
type is LanguageType -> "values-${type.language.lowercase()}"
type is LocaleType -> "values-${type.language.lowercase()}-r${type.country.uppercase()}"
else -> throw NotImplementedError()
}

override fun getStringsFilesExceptDefault(resFolder: String): List<File> {
Expand Down Expand Up @@ -63,10 +63,11 @@ object iOS : Platform() {
TODO("not implemented") //To change body of created functions use File | Settings | File Templates.
}

override fun getResPath(type: ResFolderType): String = when (type) {
is DefaultType -> "Base.lproj"
is LanguageType -> "${type.language.lowercase()}.lproj"
is LocaleType -> "${type.language.lowercase()}-${type.country.uppercase()}.lproj"
override fun getResPath(type: ResFolderType): String = when {
type.isDefault -> "Base.lproj"
type is LanguageType -> "${type.language.lowercase()}.lproj"
type is LocaleType -> "${type.language.lowercase()}-${type.country.uppercase()}.lproj"
else -> throw NotImplementedError()
}

override fun getStringsFilesExceptDefault(resFolder: String): List<File> {
Expand All @@ -90,14 +91,15 @@ object Flutter : Platform() {
override val format: String
get() = "xml"

override fun getFilename(type: ResFolderType): String = when (type) {
is DefaultType -> defaultStringsFile
is LanguageType -> {
override fun getFilename(type: ResFolderType): String = when {
type.isDefault -> defaultStringsFile
type is LanguageType -> {
"strings_${type.language.lowercase()}.arb"
}
is LocaleType -> {
type is LocaleType -> {
"strings_${type.language.lowercase()}_${type.country.uppercase()}.arb"
}
else -> throw NotImplementedError()
}

override fun getResPath(type: ResFolderType): String = "values"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ import java.io.File
interface FileOperation {
fun print(path: String, content: String)
fun delete(file: File)
fun copy(path: String, newPath: String)
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ class FileOperationImpl : FileOperation {
content.writeTo(path)
}

override fun copy(path: String, newPath: String){
val file = File(path)
val duplicateFile = File(newPath)
file.copyTo(duplicateFile, overwrite = true)
}

override fun delete(file: File) {
if (file.parentFile.listFiles()?.size == 1 && file.exists()) {
file.parentFile.deleteRecursively()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ data class LocaleContent(val content: String, val isDefault: Boolean)

interface PhraseAppNetworkDataSource {
suspend fun downloadAllLocales(
overrideDefaultFile: Boolean = DEFAULT_OVERRIDE_DEFAULT_FILE,
exceptions: Map<String, String> = DEFAULT_EXCEPTIONS,
placeHolder: Boolean = DEFAULT_PLACEHOLDER,
localeNameRegex: String = DEFAULT_REGEX,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package phraseapp.network

import kotlinx.coroutines.*
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.delay
import okhttp3.MediaType.Companion.toMediaTypeOrNull
import okhttp3.MultipartBody.Companion.FORM
import okhttp3.MultipartBody.Part
Expand All @@ -18,21 +21,19 @@ class PhraseAppNetworkDataSourceImpl(
private val token: String,
private val projectId: String,
private val fileFormat: String,
private val service: PhraseAppService
private val service: PhraseAppService,
) : PhraseAppNetworkDataSource {

override suspend fun downloadAllLocales(
overrideDefaultFile: Boolean,
exceptions: Map<String, String>,
placeHolder: Boolean,
localeNameRegex: String,
allowedLocaleCodes: List<String>
allowedLocaleCodes: List<String>,
): Map<String, LocaleContent> = coroutineScope {
val namePattern = Pattern.compile(localeNameRegex)
val locales = service.getLocales(token, projectId)
.filter {
(it.isDefault.not() or overrideDefaultFile) &&
(allowedLocaleCodes.isEmpty() || allowedLocaleCodes.contains(it.code))
(allowedLocaleCodes.isEmpty() || allowedLocaleCodes.contains(it.code))
}
val chunked = locales.chunked(CONCURRENT_LIMIT)
val localesResponse = iterative(chunked, THROTTLING_LIMIT, placeHolder)
Expand All @@ -50,15 +51,21 @@ class PhraseAppNetworkDataSourceImpl(
private suspend fun iterative(
list: List<List<Locale>>,
onePerMillis: Long,
placeHolder: Boolean
placeHolder: Boolean,
) = coroutineScope<List<LocaleResponse>> {
val target = arrayListOf<LocaleResponse>()
for (item in list) {
target.addAll(
item
.map { locale ->
async {
val file = service.download(token, projectId, locale.id, fileFormat, placeHolder)
val file = service.download(
token,
projectId,
locale.id,
fileFormat,
placeHolder
)
return@async LocaleResponse(locale, file.string())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CheckRepositoryImpl(
}

private suspend fun check(checkType: CheckType): List<CheckLocaleError> = coroutineScope {
val localesContent = phraseAppNetworkDataSource.downloadAllLocales(true, emptyMap(), true, localeRegex)
val localesContent = phraseAppNetworkDataSource.downloadAllLocales(emptyMap(), true, localeRegex)
val defaultContent = localesContent.values.first { it.isDefault }.content.parse(platform.format)
val targetsContent = localesContent.entries
.filter { it.value.isDefault.not() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import kotlinx.coroutines.coroutineScope
import phraseapp.internal.platforms.Platform
import phraseapp.internal.printers.FileOperation
import phraseapp.internal.xml.Resource
import phraseapp.network.*
import phraseapp.network.DEFAULT_ALLOWED_LOCALE_CODES
import phraseapp.network.DEFAULT_EXCEPTIONS
import phraseapp.network.DEFAULT_OVERRIDE_DEFAULT_FILE
import phraseapp.network.DEFAULT_PLACEHOLDER
import phraseapp.network.DEFAULT_REGEX
import phraseapp.network.PhraseAppNetworkDataSource
import phraseapp.parsers.xml.DEFAULT_IGNORE_COMMENTS
import phraseapp.repositories.operations.helpers.LocalHelper
import phraseapp.repositories.operations.helpers.PrinterHelper
Expand All @@ -14,7 +19,7 @@ class Downloader(
platform: Platform,
buildDir: String,
fileOperation: FileOperation,
private val network: PhraseAppNetworkDataSource
private val network: PhraseAppNetworkDataSource,
) {
private val localHelper = LocalHelper(platform)
private val reducerHelper = ReducerHelper(platform)
Expand All @@ -27,27 +32,34 @@ class Downloader(
placeholder: Boolean = DEFAULT_PLACEHOLDER,
localeNameRegex: String = DEFAULT_REGEX,
ignoreComments: Boolean = DEFAULT_IGNORE_COMMENTS,
allowedLocaleCodes: List<String> = DEFAULT_ALLOWED_LOCALE_CODES
allowedLocaleCodes: List<String> = DEFAULT_ALLOWED_LOCALE_CODES,
) = coroutineScope {
val strings = localHelper.getStringsFileByResFolder(resFolders)
val strings = localHelper.getStringsFileByResFolder(resFolders = resFolders)
val locales = network.downloadAllLocales(
overrideDefaultFile,
exceptions,
placeholder,
localeNameRegex,
allowedLocaleCodes
exceptions = exceptions,
placeHolder = placeholder,
localeNameRegex = localeNameRegex,
allowedLocaleCodes = allowedLocaleCodes
)
val resources = reducerHelper.reduceKeysForAllStringsFilesAndForAllLocales(strings, locales, ignoreComments)
printerHelper.printResources(resources)
printerHelper.printLocales(getTypes(resources))
val resources = reducerHelper.reduceKeysForAllStringsFilesAndForAllLocales(
strings = strings,
remoteStrings = locales,
ignoreComments = ignoreComments
)
printerHelper.printResources(
configurations = resources,
overrideDefaultFile = overrideDefaultFile
)
printerHelper.printLocales(types = getTypes(resources))

return@coroutineScope resources
}

private fun getTypes(configurations: Map<String, Map<ResFolderType, Resource>>): List<ResFolderType> =
configurations.entries.first().value.keys.toList()
}

sealed class ResFolderType
object DefaultType : ResFolderType()
class LanguageType(val language: String) : ResFolderType()
class LocaleType(val language: String, val country: String) : ResFolderType()
sealed class ResFolderType(open val language: String, val isDefault: Boolean)
class LanguageType(language: String, isDefault: Boolean) : ResFolderType(language, isDefault)
class LocaleType(language: String, val country: String, isDefault: Boolean) :
ResFolderType(language, isDefault)
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import phraseapp.internal.platforms.Platform
import phraseapp.internal.xml.PluralsTranslation
import phraseapp.internal.xml.StringTranslation
import phraseapp.internal.xml.StringsArrayTranslation
import phraseapp.repositories.operations.DefaultType
import phraseapp.repositories.operations.LanguageType
import phraseapp.repositories.operations.ResFolderType
import java.io.File

Expand Down Expand Up @@ -46,7 +46,7 @@ class LocalHelper(val platform: Platform) {
resFolder: String, filenames: List<String>
): List<ResourceTranslation> =
filenames
.map { getResFolderFile(resFolder, it, DefaultType) }
.map { getResFolderFile(resFolder, it) }
.map { it.readText().parse(it) }
.toList()

Expand All @@ -56,7 +56,7 @@ class LocalHelper(val platform: Platform) {
*/
private fun getStringsFile(resFolder: String, filenames: List<String>): ResourceTranslation {
val resources: List<ResourceTranslation> = filenames
.map { getResFolderFile(resFolder, it, DefaultType) }
.map { getResFolderFile(resFolder, it) }
.map { it.readText().parse(it) }
.toList()
return mergeResourceTranslations(resources)
Expand All @@ -66,8 +66,8 @@ class LocalHelper(val platform: Platform) {
* Build resource folder file from res folder path, filename and the type of the res folder (default or locale).
* @return File if exist or throw NoSuchFileException
*/
private fun getResFolderFile(resFolder: String, filename: String, type: ResFolderType): File {
val value = platform.getResPath(type)
private fun getResFolderFile(resFolder: String, filename: String): File {
val value = platform.getResPath(LanguageType("", true))
val file = File("${resFolder}${File.separator}${value}${File.separator}${filename}")
if (file.exists().not()) throw NoSuchFileException(file)
return file
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ import phraseapp.internal.printers.FileOperation
import phraseapp.internal.printers.FileOperationImpl
import phraseapp.internal.xml.ArbPrinterScanner
import phraseapp.internal.xml.Resource
import phraseapp.internal.xml.Visitor
import phraseapp.internal.xml.XmlPrinterScanner
import phraseapp.network.DEFAULT_OVERRIDE_DEFAULT_FILE
import phraseapp.repositories.operations.LanguageType
import phraseapp.repositories.operations.LocaleType
import phraseapp.repositories.operations.ResFolderType
import java.io.File

class PrinterHelper(
val platform: Platform,
val buildDir: String,
val fileOperation: FileOperation = FileOperationImpl()
val fileOperation: FileOperation = FileOperationImpl(),
) {
val tempStringFilePath: String = if (platform is Flutter) {
"$buildDir${File.separator}string.xml"
Expand Down Expand Up @@ -56,27 +57,93 @@ class PrinterHelper(
/**
* Print all resources in all paths.
*/
fun printResources(configurations: Map<String, Map<ResFolderType, Resource>>) {
fun printResources(
configurations: Map<String, Map<ResFolderType, Resource>>,
overrideDefaultFile: Boolean = DEFAULT_OVERRIDE_DEFAULT_FILE,
) {
configurations.forEach { configuration ->
configuration.value.forEach { resource ->
printResourceByType(configuration.key, resource.key, resource.value)
if ((!resource.key.isDefault || overrideDefaultFile)) {
printResourceByType(
resFolder = configuration.key,
type = resource.key,
resource = resource.value
)
}
copyDefaultFileIfNeeded(configuration, resource.key)
}
}
}


/**
* Copy default file strings.xml to value-DEFAULT_LANGUAGE/strings.xml
* if there is no variant, default file will be taken instead so we don't need to copy it.
*
* This method corrects this issue : https://github.com/Decathlon/gradle-plugin-phraseapp/issues/18
* When a default language (ex: "en") has some variants (ex: en-GB) which have a translation which is different between them,
* before this fix, there was no values-en file created, so, for an other variant (ex: en-IE) the translation will be pull from en-GB and not from "en".
* See google "resource resolution order" doc for more details ->
* https://developer.android.com/guide/topics/resources/multilingual-support?hl=fr#resource-resolution-examples
*
*
*/
private fun copyDefaultFileIfNeeded(
configuration: Map.Entry<String, Map<ResFolderType, Resource>>,
type: ResFolderType
) {
// Variant of default language ex: default = en, variant = en-IE or en-GB
val hasVariant =
configuration.value.filter { it.key.language.contains(type.language) && !it.key.isDefault }
.isNotEmpty()

if (type.isDefault && hasVariant) {
duplicateFile(
resFolder = configuration.key,
type = type
)
}
}

private fun duplicateFile(
resFolder: String,
type: ResFolderType,
) {
val defaultPath =
"$resFolder${File.separator}${platform.getResPath(type)}${File.separator}${
platform.getFilename(type)
}"

val duplicateFileType = LanguageType(type.language, false)
val duplicatePath =
"$resFolder${File.separator}${platform.getResPath(duplicateFileType)}${File.separator}${
platform.getFilename(duplicateFileType)
}"

fileOperation.copy(defaultPath, duplicatePath)
}

/**
* Build the path from res folder path and its type and print the resource at this target path.
*/
private fun printResourceByType(resFolder: String, type: ResFolderType, resource: Resource) {
val path =
"$resFolder${File.separator}${platform.getResPath(type)}${File.separator}${platform.getFilename(type)}"
"$resFolder${File.separator}${platform.getResPath(type)}${File.separator}${
platform.getFilename(
type
)
}"
printResource(path, resource)
}

/**
* Print resource on the target path.
*/
private fun printResource(targetPath: String, resource: Resource, forceXMLPrinter: Boolean = false) {
private fun printResource(
targetPath: String,
resource: Resource,
forceXMLPrinter: Boolean = false,
) {
val content = if (forceXMLPrinter) {
XmlPrinterScanner().start(resource)
} else {
Expand Down
Loading
Loading