Skip to content

Commit 317fc13

Browse files
ja-openaija-openai
committed
Ad hoc relaxation of the integrity checker for plural strings
Only the CLDR 'other' form must have the exact same number of placeholders. For other forms, we allow one placeholder to be removed. Note that placeholders are stored in a set, so duplicate placeholders will count as one. This only target printflike check, so mainly targeting Android, and some gettext. This is pretty adhoc anyway
1 parent f2077ba commit 317fc13

File tree

4 files changed

+158
-22
lines changed

4 files changed

+158
-22
lines changed

‎webapp/src/main/java/com/box/l10n/mojito/service/assetintegritychecker/integritychecker/PluralIntegrityCheckerRelaxer.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package com.box.l10n.mojito.service.assetintegritychecker.integritychecker;
2+
3+
import java.util.Set;
4+
import org.springframework.stereotype.Component;
5+
6+
@Component
7+
public class PluralIntegrityCheckerRelaxer {
8+
9+
/**
10+
* This is very ad hoc!
11+
*
12+
* <p>There are cases where the plural form doesn’t retain the number. Right now, those are
13+
* wrongly marked as rejected. Ideally, we should clearly identify which placeholder might be
14+
* missing from the string, but that’s another level of complexity. Also, use the class name to
15+
* target certain integrity checks to keep this hack constrained.
16+
*/
17+
public boolean shouldRelaxIntegrityCheck(
18+
String source, String target, String pluralForm, TextUnitIntegrityChecker textUnitChecker) {
19+
20+
boolean shouldRelax = false;
21+
22+
if (pluralForm != null && !"other".equals(pluralForm)) {
23+
if (textUnitChecker instanceof PrintfLikeIntegrityChecker
24+
|| textUnitChecker instanceof PrintfLikeIgnorePercentageAfterBracketsIntegrityChecker
25+
|| textUnitChecker instanceof PrintfLikeVariableTypeIntegrityChecker
26+
|| textUnitChecker instanceof SimplePrintfLikeIntegrityChecker) {
27+
28+
RegexIntegrityChecker regexIntegrityChecker = (RegexIntegrityChecker) textUnitChecker;
29+
30+
Set<String> sourcePlaceholders = regexIntegrityChecker.getPlaceholders(source);
31+
Set<String> targetPlaceholders = regexIntegrityChecker.getPlaceholders(target);
32+
33+
if (sourcePlaceholders.size() - targetPlaceholders.size() <= 1) {
34+
shouldRelax = true;
35+
}
36+
}
37+
}
38+
39+
return shouldRelax;
40+
}
41+
}

‎webapp/src/main/java/com/box/l10n/mojito/service/tm/TMTextUnitIntegrityCheckService.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import com.box.l10n.mojito.service.asset.AssetRepository;
88
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.IntegrityCheckException;
99
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.IntegrityCheckerFactory;
10+
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.PluralIntegrityCheckerRelaxer;
1011
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.TextUnitIntegrityChecker;
1112
import java.util.Set;
1213
import org.slf4j.Logger;
@@ -27,6 +28,8 @@ public class TMTextUnitIntegrityCheckService {
2728

2829
@Autowired TMTextUnitRepository tmTextUnitRepository;
2930

31+
@Autowired PluralIntegrityCheckerRelaxer pluralIntegrityCheckerRelaxer;
32+
3033
/**
3134
* Checks the integrity of the content given the {@link com.box.l10n.mojito.entity.TMTextUnit#id}
3235
*
@@ -46,7 +49,21 @@ public void checkTMTextUnitIntegrity(Long tmTextUnitId, String contentToCheck)
4649
logger.debug("No designated checker for this asset. Nothing to do");
4750
} else {
4851
for (TextUnitIntegrityChecker textUnitChecker : textUnitCheckers) {
49-
textUnitChecker.check(tmTextUnit.getContent(), contentToCheck);
52+
try {
53+
textUnitChecker.check(tmTextUnit.getContent(), contentToCheck);
54+
} catch (IntegrityCheckException e) {
55+
if (pluralIntegrityCheckerRelaxer.shouldRelaxIntegrityCheck(
56+
tmTextUnit.getContent(),
57+
contentToCheck,
58+
tmTextUnit.getPluralForm().getName(),
59+
textUnitChecker)) {
60+
logger.debug(
61+
"Relaxing the check for plural string with form: {}",
62+
tmTextUnit.getPluralForm().getName());
63+
} else {
64+
throw e;
65+
}
66+
}
5067
}
5168
}
5269
}

‎webapp/src/main/java/com/box/l10n/mojito/service/tm/importer/TextUnitBatchImporterService.java‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.box.l10n.mojito.service.asset.ImportTextUnitJobInput;
2323
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.IntegrityCheckException;
2424
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.IntegrityCheckerFactory;
25+
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.PluralIntegrityCheckerRelaxer;
2526
import com.box.l10n.mojito.service.assetintegritychecker.integritychecker.TextUnitIntegrityChecker;
2627
import com.box.l10n.mojito.service.locale.LocaleService;
2728
import com.box.l10n.mojito.service.pollableTask.PollableFuture;
@@ -96,6 +97,8 @@ public class TextUnitBatchImporterService {
9697

9798
@Autowired MeterRegistry meterRegistry;
9899

100+
@Autowired PluralIntegrityCheckerRelaxer pluralIntegrityCheckerRelaxer;
101+
99102
@Value("${l10n.textUnitBatchImporterService.quartz.schedulerName:" + DEFAULT_SCHEDULER_NAME + "}")
100103
String schedulerName;
101104

@@ -386,30 +389,41 @@ void applyIntegrityChecks(
386389
currentTextUnit.getSource(),
387390
textUnitForBatchImport.getContent(),
388391
ice);
389-
textUnitForBatchImport.setIncludedInLocalizedFile(false);
390-
textUnitForBatchImport.setStatus(Status.TRANSLATION_NEEDED);
391-
392-
if (hasSameTarget) {
393-
switch (integrityChecksType) {
394-
case UNLESS_TAGGED_USE_INTEGRITY_CHECKER_STATUS:
395-
if (!Strings.nullToEmpty(currentTextUnit.getTargetComment())
396-
.toLowerCase()
397-
.contains(FALSE_POSITIVE_TAG_FOR_STATUS)) {
392+
393+
if (pluralIntegrityCheckerRelaxer.shouldRelaxIntegrityCheck(
394+
currentTextUnit.getSource(),
395+
textUnitForBatchImport.getContent(),
396+
textUnitForBatchImport.getCurrentTextUnit().getPluralForm(),
397+
textUnitChecker)) {
398+
logger.debug(
399+
"Relaxing the check for plural string with form: {}",
400+
textUnitForBatchImport.getCurrentTextUnit().getPluralForm());
401+
} else {
402+
textUnitForBatchImport.setIncludedInLocalizedFile(false);
403+
textUnitForBatchImport.setStatus(Status.TRANSLATION_NEEDED);
404+
405+
if (hasSameTarget) {
406+
switch (integrityChecksType) {
407+
case UNLESS_TAGGED_USE_INTEGRITY_CHECKER_STATUS:
408+
if (!Strings.nullToEmpty(currentTextUnit.getTargetComment())
409+
.toLowerCase()
410+
.contains(FALSE_POSITIVE_TAG_FOR_STATUS)) {
411+
break;
412+
}
413+
case KEEP_STATUS_IF_SAME_TARGET:
414+
case KEEP_STATUS_IF_REJECTED_AND_SAME_TARGET:
415+
textUnitForBatchImport.setIncludedInLocalizedFile(
416+
currentTextUnit.isIncludedInLocalizedFile());
417+
textUnitForBatchImport.setStatus(currentTextUnit.getStatus());
398418
break;
399-
}
400-
case KEEP_STATUS_IF_SAME_TARGET:
401-
case KEEP_STATUS_IF_REJECTED_AND_SAME_TARGET:
402-
textUnitForBatchImport.setIncludedInLocalizedFile(
403-
currentTextUnit.isIncludedInLocalizedFile());
404-
textUnitForBatchImport.setStatus(currentTextUnit.getStatus());
405-
break;
419+
}
406420
}
407-
}
408421

409-
TMTextUnitVariantComment tmTextUnitVariantComment = new TMTextUnitVariantComment();
410-
tmTextUnitVariantComment.setSeverity(Severity.ERROR);
411-
tmTextUnitVariantComment.setContent(ice.getMessage());
412-
textUnitForBatchImport.getTmTextUnitVariantComments().add(tmTextUnitVariantComment);
422+
TMTextUnitVariantComment tmTextUnitVariantComment = new TMTextUnitVariantComment();
423+
tmTextUnitVariantComment.setSeverity(Severity.ERROR);
424+
tmTextUnitVariantComment.setContent(ice.getMessage());
425+
textUnitForBatchImport.getTmTextUnitVariantComments().add(tmTextUnitVariantComment);
426+
}
413427
}
414428
}
415429
}

‎webapp/src/test/java/com/box/l10n/mojito/service/assetintegritychecker/integritychecker/PluralIntegrityCheckerRelaxerTest.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package com.box.l10n.mojito.service.assetintegritychecker.integritychecker;
2+
3+
import static org.junit.jupiter.api.Assertions.assertFalse;
4+
import static org.junit.jupiter.api.Assertions.assertTrue;
5+
6+
import org.junit.Test;
7+
8+
public class PluralIntegrityCheckerRelaxerTest {
9+
10+
@Test
11+
public void testShouldRelaxIntegrityCheck_whenPluralFormIsOther() {
12+
PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer();
13+
boolean result =
14+
relaxer.shouldRelaxIntegrityCheck(
15+
"source %d", "target", "other", new PrintfLikeIntegrityChecker());
16+
17+
assertFalse(result, "Integrity check should not be relaxed for plural form 'other'.");
18+
}
19+
20+
@Test
21+
public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByOne_One() {
22+
PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer();
23+
boolean result =
24+
relaxer.shouldRelaxIntegrityCheck(
25+
"source %d", "target", "one", new PrintfLikeIntegrityChecker());
26+
27+
assertTrue(
28+
result, "Integrity check should be relaxed when placeholders differ by one for 'one' form");
29+
}
30+
31+
@Test
32+
public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByOne_One_DuplicatePlaceholder() {
33+
PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer();
34+
boolean result =
35+
relaxer.shouldRelaxIntegrityCheck(
36+
"source %d %d", "target", "one", new PrintfLikeIntegrityChecker());
37+
38+
assertTrue(
39+
result,
40+
"Integrity check should be relaxed when placeholders differ by one for 'one' form (as placeholders are stored in a set, ignoring duplicates)");
41+
}
42+
43+
@Test
44+
public void testShouldRelaxIntegrityCheck_whenPlaceholdersDifferByTwo_One() {
45+
PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer();
46+
boolean result =
47+
relaxer.shouldRelaxIntegrityCheck(
48+
"source %1d %2d", "target", "one", new PrintfLikeIntegrityChecker());
49+
50+
assertFalse(
51+
result,
52+
"Integrity check should not be relaxed when placeholders differ by two for 'one' form");
53+
}
54+
55+
@Test
56+
public void testShouldRelaxIntegrityCheck_withUnsupportedChecker() {
57+
PluralIntegrityCheckerRelaxer relaxer = new PluralIntegrityCheckerRelaxer();
58+
59+
boolean result =
60+
relaxer.shouldRelaxIntegrityCheck("source %d", "target", "one", new URLIntegrityChecker());
61+
62+
assertFalse(result, "Integrity check should not be relaxed for unsupported checker types.");
63+
}
64+
}

0 commit comments

Comments
 (0)