Enable filtering of non-enabled i18n language files from being loaded…

[awoods]

… into triple store

Define name of available language listing file (found in Vitro-languages)
Update RDFFilesLoader method interfaces to include ServletContext

Part of resolution to: https://jira.lyrasis.org/browse/VIVO-1836

What does this pull request do?

This pull-request, in combination with the three other pull-requests noted below, does the following:

• Enable language Maven dependencies by default (they should not have to be turned off)
• Add logic that filters out language triples files from being loaded into the Vitro triple store
  • The logic is based on comparing the list of all available i18n languages with the languages that are enabled in the runtime.properties
  • If no languages are enabled (default case), then all languages except 'en_US' are omitted
  • The core of the pull-request is found in RDFFilesLoader.java

Related pull-requests

• Add language-specific modules into the core build artifact Vitro-languages#18
• Add language-specific modules into the core build artifact VIVO-languages#41
• Include language dependencies in build all of the time VIVO#168

How should this be tested?

To test the set of pull-requests,

1. Default case
   • build all four pull-requests
   • install VIVO like normal
   • observe that only U.S. English is enabled
2. Multi-language case
   • install VIVO with an updated runtime.properties that includes multiple languages
   • observe that VIVO has those language options in the i18n dropdown
3. Both cases above
   • observe the vivo.all.log for info messages detailing which RDF files are loaded and ignored

Interested parties

@VIVO-project/vivo-committers , @MichelHeon

[gneissone]

I'm having trouble understanding why we need to track the available locales in available-langs.properties. Can't we just load the files that have an enabled locale suffix in runtime.properties and ignore the others without tracking everything that's available? If the runtime.properties includes an invalid language VIVO throws a warning to the user already.

[gneissone]

Do you need to know the locale suffixes to make sure we don't unintentionally omit extra (non-18n) files? If so, couldn't we use the locale suffix list in Java's getAvailableLocales() method and remove all files with those suffixes minus enabled languages?

[brianjlowe]

I share that concern and made a similar comment on the other pull request: vivo-project/Vitro-languages#18 . I can appreciate now that there are practical problems with the kind of alternative I proposed due to the (silly, imo) lack of convention for naming properties files.

On the other hand, the problem with only loading files that terminate in an accepted locale string is that any given RDF directory can include files that are pure ontology axioms without language-specific literals, and we would not want to eliminate those. Off the top of my head, other possible approaches would be to (a) give all such non-language files a specific suffix forcing their inclusion regardless of language settings, or (b) arrange rdf files into top-level directories by language, including one top-level directory that is always loaded.

[awoods]

other possible approaches would be to (a) give all such non-language files a specific suffix forcing their inclusion regardless of language settings, or (b) arrange rdf files into top-level directories by language, including one top-level directory that is always loaded.

Either of these approaches seem feasible.

@gneissone , as @brianjlowe mentions, if you look in ${VIVO_HOME}/rdf you will see lots of non-i18n files that also need to be loaded.

[brianjlowe]

I also noticed the getAvailableLocales() method, and it's probably worth investigating. I think we would just have to make sure that it doesn't raise the risk of (or at least document the possibility of) unexpected behavior where a specific locale string may exist in VIVO that does not have built-in support at the server/Java VM level.

[awoods]

Grappling with this issue during development, I had an "ah-ha" moment when I realized the whole problem can go away with the "available-langs.properties"... which provides 100% confidence since we know exactly which languages are available in the repo itself.

[brianjlowe]

Yeah, but the idea of adding optional functionality by having to update one central registry always makes me nervous. It's like the problem of copying web.xml because you added a custom servlet and then if you forget to merge upstream changes you lose any new servlets anyone else adds and get frustrated when the app breaks in unpredictable ways at runtime. We have things like annotations now to allow you to add new functionality while avoiding that problem. Sure, we may wish that everyone add new language support collaboratively by carefully reading all the developer documentation and submitting pull requests to the central language repositories, but sooner or later someone won't, and they'll either get frustrated that their new language files aren't showing up because they missed that crucial step or they'll end up with a local override of available-langs and the next maintenance dev who comes on the project and performs an upgrade will wonder why the newly-advertised languages aren't showing up.

[awoods]

@brianjlowe : looking out for future robustness is a fair point. Renaming/Restructuring the VIVO-/Vitro-language files is reasonable... and now would be the time. I would suggest that such a change be done in a separate PR, but as a requirement before considering merging the sprint-i18n branches into master.

[brianjlowe]

@awoods Sounds like a reasonable approach. E.g., a "blocker" issue for the -i8n release?

[awoods]

New ticket: https://jira.lyrasis.org/browse/VIVO-1848

[gneissone]

@brianjlowe would a non-standard locale suffix even work? VIVO uses methods from the Locale library in various places that seem to rely on the suffix being valid, e.g. https://github.com/vivo-project/Vitro/blob/master/api/src/main/java/edu/cornell/mannlib/vitro/webapp/i18n/selection/LocaleSelectionDataGetter.java#L92. Out of the other proposals, I would strongly prefer that i18n files live in their own directory rather than giving all the other files a suffix to ensure inclusion.

[brianjlowe]

Yep, it does appear that we already depend on VIVO's locales being "available" to the JVM the application is running in. Those locale objects are set up in LocalSelectionSetup.java, which includes this curious section

Vitro/api/src/main/java/edu/cornell/mannlib/vitro/webapp/i18n/selection/LocaleSelectionSetup.java

Line 143 in 6d64d27

if (!"es_GO".equals(localeString) && // No complaint about bogus locale

if (!"es_GO".equals(localeString) && // No complaint about bogus locale
!LocaleUtils.isAvailableLocale(locale)) {
ssWarning("'" + locale + "' is not a recognized locale.");
}

so we are indeed already testing that each locale (except for "es-GO", which we let slide) is "available" to the JVM, evidently without problems so far. I don't know if a standard set of all known world locales is shipped with every JVM, or whether this depends on the configuration of the underlying OS. The docs for getAvailableLocales() say that only Locale.US is the minimum requirement. If there's a possibility that someone might need to do additional system-level configuration to enable support for a particular locale, we should probably document it.

I also agree that the directory-based layout would be much more elegant.

[gneissone]

Hah, es_GO was the placeholder spanish included with VIVO that was created by running everything through Google Translate. It appears somebody went to great lengths to ensure nobody mistook that as a proper translation.

[gneissone]

Tested and works as advertised. It's a vast improvement over the current deployment procedure. Will be looking for an incremental improvement on this method per our discussion on Vitro #160 .