From c1f5244873fdadebd8036df373d867c2058b5f92 Mon Sep 17 00:00:00 2001 From: Mark Iantorno Date: Fri, 19 Jun 2020 09:28:36 -0400 Subject: [PATCH] Adding more verbose output for bad file path validator input (#249) * Adding more verbose output for bad file path validator input * Moving error output to i18n tooling. Adding tests to tooling, and addressing some issues with i18n implementation. * Apparently the intended behavior is to not tell the user if they are localizing a string that doesn't exist. --- .../org/hl7/fhir/utilities/i18n/I18nBase.java | 48 ++++++++++++++++--- .../fhir/utilities/i18n/I18nConstants.java | 1 + .../src/main/resources/Messages.properties | 1 + .../hl7/fhir/utilities/i18n/I18nBaseTest.java | 45 +++++++++++++++++ .../fhir/utilities/i18n/I18nTestClass.java | 5 ++ .../hl7/fhir/validation/ValidationEngine.java | 16 ++++--- 6 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nBaseTest.java create mode 100644 org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nTestClass.java diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nBase.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nBase.java index 7fd4fd710..5d7a5e162 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nBase.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nBase.java @@ -12,12 +12,11 @@ import java.util.ResourceBundle; */ public abstract class I18nBase { - private Locale locale; - private ResourceBundle i18Nmessages; + private ResourceBundle i18nMessages; public Locale getLocale() { - if (Objects.nonNull(locale)){ + if (Objects.nonNull(locale)) { return locale; } else { return Locale.US; @@ -29,19 +28,54 @@ public abstract class I18nBase { setValidationMessageLanguage(getLocale()); } + /** + * Verifies if a {@link ResourceBundle} has been loaded for the current {@link Locale}. If not, it triggers a load. + */ + private void checkResourceBundleIsLoaded() { + if (i18nMessages == null) { + setValidationMessageLanguage(getLocale()); + } + } + + /** + * Checks the loaded {@link ResourceBundle} to see if the passed in message exists with the current loaded {@link Locale}. + * If no {@link Locale} is currently loaded, it will load the {@link Locale} (default {@link Locale#US} is none is + * specified), and search. + * @param message The {@link String} message to search for within the current {@link Locale} + * @return {@link Boolean#TRUE} if the message exists within the loaded {@link Locale}. + */ + private boolean messageExistsForLocale(String message) { + checkResourceBundleIsLoaded(); + if (!i18nMessages.containsKey(message)) { + System.out.println("Attempting to localize message " + message + ", but no such equivalent message exists for" + + " the local " + getLocale()); + } + return i18nMessages.containsKey(message); + } + + /** + * Formats the given message, if needed, with the passed in message arguments. + * @param theMessage Base message to format. + * @param theMessageArguments Placeholder arguments, if needed. + * @return The formatted, internationalized, {@link String} + */ public String formatMessage(String theMessage, Object... theMessageArguments) { String message = theMessage; - if (Objects.nonNull(i18Nmessages) && i18Nmessages.containsKey(theMessage)) { + if (messageExistsForLocale(theMessage)) { if (Objects.nonNull(theMessageArguments) && theMessageArguments.length > 0) { - message = MessageFormat.format(i18Nmessages.getString(theMessage), theMessageArguments); + message = MessageFormat.format(i18nMessages.getString(theMessage), theMessageArguments); } else { - message = i18Nmessages.getString(theMessage); + message = i18nMessages.getString(theMessage); } } return message; } + /** + * Loads the corresponding {@link ResourceBundle} for the passed in {@link Locale}. + * @param locale {@link Locale} to load resources for. + */ public void setValidationMessageLanguage(Locale locale) { - i18Nmessages = ResourceBundle.getBundle("Messages", locale); + i18nMessages = ResourceBundle.getBundle("Messages", locale); } } \ No newline at end of file diff --git a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java index 9e3b37f1f..89c74884d 100644 --- a/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java +++ b/org.hl7.fhir.utilities/src/main/java/org/hl7/fhir/utilities/i18n/I18nConstants.java @@ -12,6 +12,7 @@ public class I18nConstants { public static final String ATTEMPT_TO_REPLACE_ELEMENT_NAME_FOR_A_NONCHOICE_TYPE = "Attempt_to_replace_element_name_for_a_nonchoice_type"; public static final String ATTEMPT_TO_USE_A_SNAPSHOT_ON_PROFILE__AS__BEFORE_IT_IS_GENERATED = "Attempt_to_use_a_snapshot_on_profile__as__before_it_is_generated"; public static final String ATTEMPT_TO_USE_TERMINOLOGY_SERVER_WHEN_NO_TERMINOLOGY_SERVER_IS_AVAILABLE = "Attempt_to_use_Terminology_server_when_no_Terminology_server_is_available"; + public static final String BAD_FILE_PATH_ERROR = "Bad_file_path_error"; public static final String BASE_PROFILE__HAS_NO_TYPE = "Base_profile__has_no_type"; public static final String BASE__DERIVED_PROFILES_HAVE_DIFFERENT_TYPES____VS___ = "Base__Derived_profiles_have_different_types____vs___"; public static final String BUNDLE_BUNDLE_ENTRY_CANONICAL = "Bundle_BUNDLE_Entry_Canonical"; diff --git a/org.hl7.fhir.utilities/src/main/resources/Messages.properties b/org.hl7.fhir.utilities/src/main/resources/Messages.properties index 87f8562ee..f46b3554c 100644 --- a/org.hl7.fhir.utilities/src/main/resources/Messages.properties +++ b/org.hl7.fhir.utilities/src/main/resources/Messages.properties @@ -1,4 +1,5 @@ #InstanceValidator +Bad_file_path_error = \n********************\n* The file name you passed in, "{0}", doesn't exist on the local filesystem.\n* Please verify that this is valid file location.\n********************\n\n Bundle_BUNDLE_Entry_Canonical = The canonical URL ({0}) cannot match the fullUrl ({1}) unless on the canonical server itself Bundle_BUNDLE_Entry_Document = The first entry in a document must be a composition Bundle_BUNDLE_Entry_IdUrlMismatch = Resource ID does not match the ID in the entry full URL ("{0}" vs "{1}") diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nBaseTest.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nBaseTest.java new file mode 100644 index 000000000..6a6669703 --- /dev/null +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nBaseTest.java @@ -0,0 +1,45 @@ +package org.hl7.fhir.utilities.i18n; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.text.MessageFormat; +import java.util.Locale; +import java.util.ResourceBundle; + +class I18nBaseTest { + + public static final String BAD_STRING_ARG = "THIS_DOES_NOT_EXIST"; + public static final String ARG_1 = "test arg"; + + @Test + @DisplayName("Test argument substitution with initializing Locale.") + void testFormatMessageWithInitLocale() { + I18nTestClass testClass = new I18nTestClass(); + ResourceBundle loadedBundle = ResourceBundle.getBundle("Messages", Locale.GERMAN); + testClass.setLocale(Locale.GERMAN); + String result = testClass.formatMessage(I18nConstants.BUNDLE_BUNDLE_MULTIPLEMATCHES, ARG_1); + MessageFormat form = new MessageFormat(loadedBundle.getString(I18nConstants.BUNDLE_BUNDLE_MULTIPLEMATCHES)); + Object[] testArgs = {ARG_1}; + Assertions.assertEquals(form.format(testArgs), result); + } + + @Test + @DisplayName("Test argument substitution without initializing Locale.") + void testFormatMessageWithoutInitLocale() { + I18nTestClass testClass = new I18nTestClass(); + ResourceBundle loadedBundle = ResourceBundle.getBundle("Messages", Locale.US); + String result = testClass.formatMessage(I18nConstants.BUNDLE_BUNDLE_MULTIPLEMATCHES, ARG_1); + MessageFormat form = new MessageFormat(loadedBundle.getString(I18nConstants.BUNDLE_BUNDLE_MULTIPLEMATCHES)); + Object[] testArgs = {ARG_1}; + Assertions.assertEquals(form.format(testArgs), result); + } + + @Test + @DisplayName("Assert no string modification is done when no match is found.") + void testFormatMessageForNonExistentMessage() { + I18nTestClass testClass = new I18nTestClass(); + Assertions.assertEquals(BAD_STRING_ARG, testClass.formatMessage(BAD_STRING_ARG, ARG_1)); + } +} \ No newline at end of file diff --git a/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nTestClass.java b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nTestClass.java new file mode 100644 index 000000000..48576491d --- /dev/null +++ b/org.hl7.fhir.utilities/src/test/java/org/hl7/fhir/utilities/i18n/I18nTestClass.java @@ -0,0 +1,5 @@ +package org.hl7.fhir.utilities.i18n; + +public class I18nTestClass extends I18nBase { + public I18nTestClass() {} +} diff --git a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java index e4bea56f7..1214f90ab 100644 --- a/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java +++ b/org.hl7.fhir.validation/src/main/java/org/hl7/fhir/validation/ValidationEngine.java @@ -31,6 +31,7 @@ import org.hl7.fhir.r5.terminologies.ConceptMapEngine; import org.hl7.fhir.r5.utils.*; import org.hl7.fhir.r5.utils.IResourceValidator.*; import org.hl7.fhir.r5.utils.StructureMapUtilities.ITransformerServices; +import org.hl7.fhir.utilities.i18n.I18nConstants; import org.hl7.fhir.validation.instance.InstanceValidator; import org.hl7.fhir.utilities.IniFile; import org.hl7.fhir.utilities.TextFile; @@ -158,8 +159,6 @@ POSSIBILITY OF SUCH DAMAGE. */ public class ValidationEngine implements IValidatorResourceFetcher { - - public class ScanOutputItem { private String ref; private ImplementationGuide ig; @@ -1064,16 +1063,19 @@ public class ValidationEngine implements IValidatorResourceFetcher { for (int i=0; i < files.length; i++) { refs.add(files[i].getPath()); } - } else { File file = new File(name); - - if (!file.exists()) + if (!file.exists()) { + if (System.console() != null) { + System.console().printf(context.formatMessage(I18nConstants.BAD_FILE_PATH_ERROR, name)); + } else { + System.out.println(context.formatMessage(I18nConstants.BAD_FILE_PATH_ERROR, name)); + } throw new IOException("File " + name + " does not exist"); - + } + if (file.isFile()) { refs.add(name); - } else { isBundle = true; for (int i=0; i < file.listFiles().length; i++) {