diff --git a/.azure/i18n-coverage-table/generate-i18n-coverage-table.py b/.azure/i18n-coverage-table/generate-i18n-coverage-table.py new file mode 100644 index 000000000..b857e0534 --- /dev/null +++ b/.azure/i18n-coverage-table/generate-i18n-coverage-table.py @@ -0,0 +1,34 @@ +import numpy as np +import pandas as pd +import matplotlib.pyplot as plt + +#define figure and axes +fig, ax = plt.subplots(1,1) + +#hide the axes +fig.patch.set_visible(False) +ax.axis('off') +ax.axis('tight') + +#read data +df = pd.read_csv('i18n-coverage.csv') +#create table +table = ax.table(cellText=df.values, colLabels=df.columns, loc='center') + +table.scale(1, 4) +table.auto_set_font_size(False) +table.set_fontsize(14) + +fig.tight_layout() +fig.set_figheight(2) +fig.set_figwidth(4) + + +ax.set_title('Internationalization Phrase Coverage by Locale') + +fig = plt.gcf() + +plt.savefig('i18n-coverage-table.png', + bbox_inches='tight', + dpi=150 + ) \ No newline at end of file diff --git a/.github/workflows/bidi-checker.yml b/.github/workflows/bidi-checker.yml index 39cba1e06..622549dbf 100644 --- a/.github/workflows/bidi-checker.yml +++ b/.github/workflows/bidi-checker.yml @@ -25,6 +25,6 @@ jobs: id: bidi_check uses: HL7/bidi-checker-action@v1.9 env: - IGNORE: dummy-package.tgz$ + IGNORE: i18n-coverage-table\.png$|dummy-package.tgz$ - name: Get the output time run: echo "The time was ${{ steps.bidi_check.outputs.time }}" diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 32292b345..7b06c6ab5 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,21 +1,7 @@ ## Validator Changes -* fix NPE loading resources -* Don't enforce ids on elements when processing CDA -* Send supplements to tx server -* fix bug processing code bindings when value sets are complex (multiple filters) -* fix spelling of heirarchy -* Look up CodeSystem from terminology server -* Don't use tx-registry when manual terminology server is set +* no changes ## Other code changes -* More work on WHO language support ($1592) -* allow validation message to have count -* render versions in profile links when necessary -* rework OID handling for better OID -> CodeSystem resolution -* fix up vsac importer for changes to client -* don't send xhtml for tx operations -* FHIRPath: Backport the defineVariable code to the R4 and R4B fhirpath implementations -* FHIRPath: Remove the alias/aliasAs custom functions (use standard defineVariable now) -* Bump lombok (#1603) +* no changes \ No newline at end of file diff --git a/i18n-coverage-table.png b/i18n-coverage-table.png new file mode 100644 index 000000000..4a9be37c1 Binary files /dev/null and b/i18n-coverage-table.png differ diff --git a/i18n-coverage.csv b/i18n-coverage.csv new file mode 100644 index 000000000..4a7f617ac --- /dev/null +++ b/i18n-coverage.csv @@ -0,0 +1,5 @@ +Locale,Coverage #,Coverage % +de,869,70% +es,740,59% +ja,935,75% +nl,873,70% diff --git a/master-branch-i18n-coverage-pipeline.yml b/master-branch-i18n-coverage-pipeline.yml index 69581378a..b5763a921 100644 --- a/master-branch-i18n-coverage-pipeline.yml +++ b/master-branch-i18n-coverage-pipeline.yml @@ -1,12 +1,17 @@ -# We only want to trigger a test build on PRs to the main branch. -trigger: none +# This pipeline runs the internationalization coverage test and then uses a +# python script to generate a table from the results for viewing in the +# README.md file +pr: none -pr: +trigger: - master variables: + # Normally this test outputs to console. This variable appears as env param + # I18N_COVERAGE_FILE, which tells the test to write the output to a file + # instead. - name: i18n.coverage.file - value: i18n-coverage.csv + value: ../i18n-coverage.csv - group: PGP_VAR_GROUP - group: SONATYPE_VAR_GROUP - group: GIT_VAR_GROUP @@ -39,6 +44,8 @@ jobs: jdkVersionOption: '1.11' jdkArchitectureOption: 'x64' goals: 'install' + displayName: 'Build utilities module' + - task: Maven@3 inputs: mavenPomFile: 'pom.xml' @@ -48,9 +55,27 @@ jobs: jdkVersionOption: '1.11' jdkArchitectureOption: 'x64' goals: 'surefire:test' + displayName: 'Run i18n coverage test to generate csv' - task: PythonScript@0 inputs: scriptSource: 'filePath' scriptPath: .azure/i18n-coverage-table/generate-i18n-coverage-table.py arguments: + displayName: 'Make png table from coverage test csv' + + # Verify png file generation + - bash: | + ls -l ./i18n-coverage-table.png + + - bash: | + git fetch + git checkout master + git status + git add ./i18n-coverage.csv + git add ./i18n-coverage-table.png + git commit . -m "Updating i18n-coverage csv and png table ***NO_CI***" + + git push https://$(GIT_PAT)@github.com/hapifhir/org.hl7.fhir.core.git + + displayName: 'Push updated csv and plot to git.' diff --git a/org.hl7.fhir.convertors/pom.xml b/org.hl7.fhir.convertors/pom.xml index 32478e37c..3006d62cd 100644 --- a/org.hl7.fhir.convertors/pom.xml +++ b/org.hl7.fhir.convertors/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.dstu2/pom.xml b/org.hl7.fhir.dstu2/pom.xml index b8ea599f1..9d56ef28c 100644 --- a/org.hl7.fhir.dstu2/pom.xml +++ b/org.hl7.fhir.dstu2/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.dstu2016may/pom.xml b/org.hl7.fhir.dstu2016may/pom.xml index 13534d4ba..3df699a78 100644 --- a/org.hl7.fhir.dstu2016may/pom.xml +++ b/org.hl7.fhir.dstu2016may/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.dstu3/pom.xml b/org.hl7.fhir.dstu3/pom.xml index 64578180d..6ce87f0a6 100644 --- a/org.hl7.fhir.dstu3/pom.xml +++ b/org.hl7.fhir.dstu3/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.r4/pom.xml b/org.hl7.fhir.r4/pom.xml index 0969876e7..6f1e87866 100644 --- a/org.hl7.fhir.r4/pom.xml +++ b/org.hl7.fhir.r4/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.r4b/pom.xml b/org.hl7.fhir.r4b/pom.xml index 9745dbfb7..0bbd7e99f 100644 --- a/org.hl7.fhir.r4b/pom.xml +++ b/org.hl7.fhir.r4b/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.r5/pom.xml b/org.hl7.fhir.r5/pom.xml index 4c574bce5..9b8e4ff1d 100644 --- a/org.hl7.fhir.r5/pom.xml +++ b/org.hl7.fhir.r5/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.report/pom.xml b/org.hl7.fhir.report/pom.xml index 5fe5e420c..ffe00f636 100644 --- a/org.hl7.fhir.report/pom.xml +++ b/org.hl7.fhir.report/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.utilities/pom.xml b/org.hl7.fhir.utilities/pom.xml index 89bc969a9..161f4d075 100644 --- a/org.hl7.fhir.utilities/pom.xml +++ b/org.hl7.fhir.utilities/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nCoverageTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nCoverageTest.java index 4de63253d..6bd082f72 100644 --- a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nCoverageTest.java +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nCoverageTest.java @@ -3,13 +3,12 @@ package org.hl7.fhir.utilities.i18n; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.PrintStream; import java.lang.reflect.Field; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Locale; -import java.util.Map; -import java.util.ResourceBundle; -import java.util.Set; +import java.util.*; import javax.annotation.Nonnull; @@ -17,7 +16,12 @@ import org.junit.jupiter.api.Test; public class I18nCoverageTest { - + private static class I18nCoverage { + final Set englishKeys = new HashSet<>(); + final Set englishPluralKeys = new HashSet<>(); + final HashMap foundKeys = new HashMap<>(); + final HashMap foundPluralKeys = new HashMap<>(); + } final Set locales = Set.of( Locale.ENGLISH, @@ -27,8 +31,141 @@ public class I18nCoverageTest { Locale.forLanguageTag("ja") ); + final Locale sourceLocale = Locale.ENGLISH; + @Test - public void testCoverage() throws IllegalAccessException { + public void testPhraseCoverage() throws IOException { + + I18nCoverage messages = getI18nCoverage("Messages"); + I18nCoverage renderingPhrases = getI18nCoverage("rendering-phrases"); + + PrintStream out = getCSVOutputStream(); + printPhraseCoverageCSV(out, List.of(messages, renderingPhrases)); + } + + private I18nCoverage getI18nCoverage(String messageFilePrefix) throws IOException { + I18nCoverage i18nCoverage = new I18nCoverage(); + + Properties englishMessages = new Properties(); + englishMessages.load(I18nTestClass.class.getClassLoader().getResourceAsStream(messageFilePrefix + ".properties")); + + I18nTestClass englishTestClass = getI18nTestClass(Locale.ENGLISH); + Set englishPluralSuffixes = englishTestClass.getPluralSuffixes(); + + for (Object objectKey : englishMessages.keySet()) { + String key = (String) objectKey; + if (isPluralKey(key, englishPluralSuffixes)) { + final String pluralKeyRoot = getPluralKeyRoot(key, englishPluralSuffixes); + i18nCoverage.englishPluralKeys.add(pluralKeyRoot); + } else { + i18nCoverage.englishKeys.add(key); + } + } + + for (Locale locale : locales) { + if (!locale.equals(sourceLocale)) { + Properties translatedMessages = new Properties(); + translatedMessages.load(I18nTestClass.class.getClassLoader().getResourceAsStream(messageFilePrefix + "_" + locale.toString() + ".properties")); + I18nTestClass translatedTestClass = getI18nTestClass(sourceLocale); + Set translatedPluralSuffixes = translatedTestClass.getPluralSuffixes(); + + Set translatedPluralKeys = new HashSet<>(); + Set translatedKeys = new HashSet<>(); + + for (Object objectKey : translatedMessages.keySet()) { + String key = (String) objectKey; + Object value = translatedMessages.get(objectKey); + if ( + value instanceof String && + !((String) value).trim().isEmpty()) { + if (isPluralKey(key, translatedPluralSuffixes)) { + final String pluralKeyRoot = getPluralKeyRoot(key, englishPluralSuffixes); + translatedPluralKeys.add(pluralKeyRoot); + } else { + translatedKeys.add(key); + } + } + } + + Set intersectionKeys = new HashSet<>(i18nCoverage.englishKeys); + intersectionKeys.retainAll(translatedKeys); + Set intersectionPluralKeys = new HashSet<>(i18nCoverage.englishPluralKeys); + intersectionPluralKeys.retainAll(translatedPluralKeys); + + Set missingKeys = new HashSet<>(i18nCoverage.englishKeys); + Set missingPluralKeys = new HashSet<>(i18nCoverage.englishPluralKeys); + + missingKeys.removeAll(translatedKeys); + missingPluralKeys.removeAll(translatedPluralKeys); + + i18nCoverage.foundKeys.put(locale, intersectionKeys.size()); + i18nCoverage.foundPluralKeys.put(locale, intersectionPluralKeys.size()); + + for (String missingKey : missingKeys) { + System.err.println("Missing key for locale " + locale + ": " + missingKey); + } + for (String missingPluralKey : missingPluralKeys) { + System.err.println("Missing plural key for locale " + locale + ": " + missingPluralKey); + } + } + } + return i18nCoverage; + } + + private static PrintStream getCSVOutputStream() throws FileNotFoundException { + String outputFile = System.getenv("I18N_COVERAGE_FILE"); + + return outputFile == null + ? System.out + : new PrintStream(new File(outputFile)); + } + + private void printPhraseCoverageCSV(PrintStream out, List i18nCoverageList) { + out.println("Locale,Coverage #,Coverage %"); + + List sortedLocales = new ArrayList<>(locales); + sortedLocales.sort(Comparator.comparing(Locale::toString)); + + for (Locale locale : sortedLocales) { + if (!locale.equals(sourceLocale)) { + int count = 0; + int total = 0; + for (I18nCoverage i18nCoverage : i18nCoverageList) { + int singleCount = i18nCoverage.foundKeys.get(locale); + int pluralCount = i18nCoverage.foundPluralKeys.get(locale); + count += singleCount + pluralCount; + total += i18nCoverage.englishKeys.size() + i18nCoverage.englishPluralKeys.size(); + } + out.println(locale + "," + count + "," + getPercent(count, total)); + } + } + } + + private static String getPercent(int numerator, int denominator) { + return (int) (((double) numerator / denominator) * 100) + "%"; + } + + private String getPluralKeyRoot(String key, Set pluralKeys) { + for (String pluralKey : pluralKeys) { + final String suffix = I18nBase.KEY_DELIMITER + pluralKey; + if (key.endsWith(suffix)) { + return key.substring(0, key.lastIndexOf(suffix)); + } + } + throw new IllegalArgumentException(key + " does not terminate with a plural suffix. Available: " + pluralKeys); + } + + private boolean isPluralKey(String key, Set pluralKeys) { + for (String pluralKey : pluralKeys) { + if (key.endsWith(I18nBase.KEY_DELIMITER + pluralKey)) { + return true; + } + } + return false; + } + + @Test + public void testConstantsCoverage() throws IllegalAccessException { Field[] fields = I18nConstants.class.getDeclaredFields(); Map testClassMap = new HashMap<>(); @@ -40,16 +177,15 @@ public class I18nCoverageTest { Set messages = new HashSet<>(); for (Field field : fields) { - String message = (String)field.get(new String()); + String message = (String) field.get(new String()); messages.add(message); if (field.getType() == String.class) { - Map isSingularPhrase = new HashMap<>(); + Map isSingularPhrase = new HashMap<>(); Map isPluralPhrase = new HashMap<>(); for (Locale locale : locales) { I18nBase base = testClassMap.get(locale); - isSingularPhrase.put(locale, base.messageKeyExistsForLocale(message)); isPluralPhrase.put(locale, existsAsPluralPhrase(base, message)); } @@ -65,7 +201,7 @@ public class I18nCoverageTest { boolean mapsToConstant = messages.contains(message); boolean mapsToPluralPhrase = mapsToPluralPhrase(messages, message, testClassMap.get(locale)); if (!(mapsToConstant || mapsToPluralPhrase)) { - System.err.println("Message " + message + " in " + locale.getLanguage() + " properties resource does not have a matching entry in " + I18nConstants.class.getName() ); + System.err.println("Message " + message + " in " + locale.getLanguage() + " properties resource does not have a matching entry in " + I18nConstants.class.getName()); } } } @@ -81,14 +217,14 @@ public class I18nCoverageTest { } private void assertPhraseTypeAgreement(Field field, - Map isSingularPhrase, - Map isPluralPhrase) { + Map isSingularPhrase, + Map isPluralPhrase) { boolean existsAsSingular = isSingularPhrase.values().stream().anyMatch(value -> value == true); boolean existsAsPlural = isPluralPhrase.values().stream().anyMatch(value -> value == true); assertTrue( //The phrase might not exist (existsAsPlural == false && existsAsSingular == false) - // But if it does exist, it must consistently be of singular or plural + // But if it does exist, it must consistently be of singular or plural || existsAsPlural ^ existsAsSingular, "Constant " + field.getName() + " has inconsistent plural properties in I18n property definitions: " + pluralPropertySummary(isSingularPhrase, isPluralPhrase)); } @@ -103,7 +239,8 @@ public class I18nCoverageTest { if (!existsInSomeLanguage) { System.err.println("Constant " + field.getName() + " does not exist in any I18n property definition"); return; - }; + } + ; if (existsAsSingular) { logMissingPhrases(field, isSingularPhrase, "singular"); } @@ -120,14 +257,15 @@ public class I18nCoverageTest { } } - private String pluralPropertySummary( Map isSingularPhrase, - Map isPluralPhrase) { + private String pluralPropertySummary(Map isSingularPhrase, + Map isPluralPhrase) { StringBuilder stringBuilder = new StringBuilder(); for (Locale locale : locales) { stringBuilder.append("locale: " + locale.getDisplayName() + " singular:" + isSingularPhrase.get(locale) + " plural: " + isPluralPhrase.get(locale) + ";"); } return stringBuilder.toString(); } + @Nonnull private static I18nTestClass getI18nTestClass(Locale locale) { I18nTestClass testClass = new I18nTestClass(); diff --git a/org.hl7.fhir.validation.cli/pom.xml b/org.hl7.fhir.validation.cli/pom.xml index cfacf9c10..6a3c55d66 100644 --- a/org.hl7.fhir.validation.cli/pom.xml +++ b/org.hl7.fhir.validation.cli/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/org.hl7.fhir.validation/pom.xml b/org.hl7.fhir.validation/pom.xml index a0d3bb090..9ca1191b6 100644 --- a/org.hl7.fhir.validation/pom.xml +++ b/org.hl7.fhir.validation/pom.xml @@ -5,7 +5,7 @@ ca.uhn.hapi.fhir org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT ../pom.xml diff --git a/pom.xml b/pom.xml index 49507dad7..f414bd97e 100644 --- a/pom.xml +++ b/pom.xml @@ -14,7 +14,7 @@ HAPI FHIR --> org.hl7.fhir.core - 6.3.6 + 6.3.7-SNAPSHOT pom