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

[Vivo 1918] i18n: Reload firsttime files on start-up if changed #195

Merged
merged 21 commits into from
Jan 14, 2021

Conversation

dofeldsc
Copy link
Contributor

@dofeldsc dofeldsc commented Nov 3, 2020

JIRA Issue

What does this pull request do?

This PR adds the functionality requested in the JIRA-Ticket
On the firsttime Start-Up a backup of the firsttime files is saved to a new model.
On every Start-Up these backup will be compared to the current state of the firsttime files. If the files have changed, try to apply
these changes to the user model. So if you activate a new language or change stuff in some firsttime files, you will no longer need to do a firsttime Start-Up of your system, with a normal restart the changes will be applied.

How should this be tested?

You need a fresh system (firsttime Start-Up) - for that, you can delete the tdb* directories in vivo_home (if you do not want to set up a whole new system)
Start the system so the new backup models will be created. After this, change some firsttime files and restart the system.
The changes should now be available without a new firsttime Start-Up.

good examples:

  • vivo_home_dir/rdf/i18n/en_US/tbox/firsttime/vitroAnnotations_en_US.n3
  • vivo_home_dir/rdf/i18n/en_US/display/firsttime/menu_en_US.nt

Copy link
Member

@awoods awoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, @dofeldsc !
I still need to test the functionality, but here is the code review.

Model difOldNew = baseModel.difference(newModel);
Model difNewOld = newModel.difference(baseModel);

// remove special case for display, problem with quickView -triple and blank nodes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the problem with "display" models?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One triple was marked as a change on every startup
<https://some.othernamespace.edu/vivo/individual/portal1> <http://vitro.mannlib.cornell.edu/ns/vitro/0.7#rootTab> [] .
But i should not change.
So i ignore/remove this one in the diff. Maybe you have a better idea, probably isAnon would work here too?!


if(subject.isAnon() || object.isAnon())
{
removeStatement.add(stmt);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are statements with anonymous subjects or objects being removed for further consideration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a problem with this statement
<http://vitro.mannlib.cornell.edu/ontologies/display/1.1#ProfilePageType> <http://www.w3.org/2002/07/owl#equivalentClass> [ a <http://www.w3.org/2002/07/owl#Class> ; <http://www.w3.org/2002/07/owl#oneOf> [ <http://www.w3.org/1999/02/22-rdf-syntax-ns#rest> [ <http://www.w3.org/1999/02/22-rdf-syntax-ns#rest> () ] ]
It was marked as a change on every startup.
And Brians suggestion was to ignore any statements where either the subject or the object is a blank node (.isAnon())

dofeldsc and others added 2 commits November 11, 2020 11:43
…RDFService (vivo-project#194)

- No functional change in this update

Resolves: https://jira.lyrasis.org/browse/VIVO-1931

Co-authored-by: Andrew Woods <awoods@duraspace.org>
@gneissone
Copy link
Member

Just taking a look for the first time here, so forgive me if I'm out of the loop.
My understanding is, each time upon startup, the contents of the firsttime files are saved into a new graph, which includes the suffix 'Firsttime.'

Taking a look at the 'Available Jena models' page in the UI, I see the new models, but it's not obvious what their purpose is or how they are organized. Backups of the configuration models, like displayMetadata, are shown on the main store models page while the live graph is on a separate page.

Screen Shot 2020-11-18 at 10 49 07 AM

I am also noticing that these backup triples appear in query results, which is confusing.
Screen Shot 2020-11-18 at 10 54 35 AM

If we change a value for these triples in the UI (e.g. a class group) the value is updated in the primary graph, but not the backup, as expected. Is there any possibility of conflicts here? Or does the application effectively ignore the backups during operation?

Some suggestions after the first pass review:

  • Rename the backup graphs so their purpose is more obvious - perhaps include the word 'backup' in the name.
  • Update the manage jena models page to separate the backup models from the functional ones (or exclude them).
  • Confirm backup models are only used during the startup process and don't conflict with the 'live' triples anywhere.

@awoods
Copy link
Member

awoods commented Nov 18, 2020

Thanks, @gneissone .
Regarding your suggestions,

Rename the backup graphs so their purpose is more obvious - perhaps include the word 'backup' in the name.

This seems like something that could reasonable be changed in this PR.

Update the manage jena models page to separate the backup models from the functional ones (or exclude them).

This may be best for a follow-on ticket.

Copy link
Member

@awoods awoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR appears to work; however, given the number of unrelated bugs in the i18n:siteAdmin functionality, it is a big challenging to test. For example, when using multiple languages, most of the "page management" features fail with errors.

The tests that I was able to successfully perform were:

  • edit page-title text in UI: it remains after restart - Good
  • edit page-title text in firsttime files: it is reloaded and used on restart - Good
  • change menu order of page both in UI and in firsttime files: the UI order takes precedence and is used - Good

@awoods
Copy link
Member

awoods commented Nov 21, 2020

I was able to locally patch (see: gist) VIVO-i18n in order to perform the following test for this pull-request:

  • Build and install this pull-request, while leaving Tomcat stopped
  • Delete any existing triplestores (tdbContentModels/ and tdbModels/)
  • Start Tomcat
  • Log in as site admin
  • Navigate to: "Site Admin" -> "Page management"
  • Click the pencil icon next to the "Capability Map" row
  • Change "Menu Item Name" to "Capability Map-UI1"
  • Verify tabs shows new menu item name
  • In firsttime file: $VIVO_HOME/rdf/i18n/en_US/display/firsttime/menu_en_US.nt
    • Change two "linkText" entries:
      • "Events" to "Events-file1"
      • "Capability Map" to "Capability Map-file1"
  • Restart Tomcat
  • Verify that the tabs show:
    • "Events-file1" and
    • "Capability Map-UI1" -- :( I am seeing "Capability Map-file1"

If changes happen via the UI and the firsttime file, the UI should take precedence.

@dofeldsc
Copy link
Contributor Author

dofeldsc commented Nov 30, 2020

I am also noticing that these backup triples appear in query results, which is confusing.

That really shouldn't be the case, what's the best way to prevent it?

If we change a value for these triples in the UI (e.g. a class group) the value is updated in the primary graph, but not the backup, as expected. Is there any possibility of conflicts here? Or does the application effectively ignore the backups during operation?

They should be ignored, I only use them here to check and update them when changes are made to the firsttime files (after a restart). Or are new models "automatically" accessed somewhere?

  • Rename the backup graphs so their purpose is more obvious - perhaps include the word 'backup' in the name.

I added the word 'backup' at the end of the model name.

@dofeldsc
Copy link
Contributor Author

  • "Capability Map-UI1" -- :( I am seeing "Capability Map-file1"
    If changes happen via the UI and the firsttime file, the UI should take precedence.

Right, I can confirm the error :( I'm trying to find a solution for this problem. In the algorithm described at the beginning (see JIRA-ticket) this would not have happened but new language files would not be loaded neither when something in the model was changed in the UI. Maybe for each change compare subject and predicate and only import them if they have not changed between firsttime-backup and UI-model? This would be my first approach

@awoods
Copy link
Member

awoods commented Dec 10, 2020

Maybe for each change compare subject and predicate and only import them if they have not changed between firsttime-backup and UI-model?

That sounds right.

@dofeldsc
Copy link
Contributor Author

I have fixed the bug, so changes in the UI should no longer be overwritten by the changes from the files.
But I'm not sure how to configure the backup-models so that they can't be queried via the SPARQL-API.

As an example, if i now query something like this over the SPARQL-API UI in Vivo

SELECT ?title
WHERE
{
<http://vitro.mannlib.cornell.edu/ontologies/display/1.1#CapabilityMap> <http://vitro.mannlib.cornell.edu/ontologies/display/1.1#title> ?title
}

I will get the value set in the firsttime-files. Before the backup models you did not get a result. But on the actual page everything is displayed correctly.

Copy link
Member

@awoods awoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updates work as advertised. It is sometimes difficult to test because of the issue you noted in the JIRA:

The second probably has nothing to do with this ticket. Can someone confirm that with two available languages after a firsttime startup, the page
"Site Admin -> Page management -> Click the pencil icon next to the 'Capability Map' row" or any other page on this site, is not fully translated when the language is changed? I see this behavior also in the i18n-sprint branch.

..and because a patch is required, (gist).

One minor comment remaining on this PR.

@awoods
Copy link
Member

awoods commented Jan 11, 2021

@gneissone : if you can verify the updates with your tests, we can get this merged.

@gneissone gneissone self-requested a review January 13, 2021 23:49
Copy link
Member

@gneissone gneissone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the test procedure described by Andrew and things are working as expected. I also tried editing class groups and had success there as well. Weirdness with the behavior in a multi-language environment I believe is unrelated to this PR.

@gneissone gneissone merged commit 45f752b into vivo-project:sprint-i18n Jan 14, 2021
roflinn added a commit that referenced this pull request Feb 10, 2021
* Layer uqam updates onto master (minus trailing whitespace)

* Update RDFServiceFactorySingle.java

* Sprint i18n whitespace (#143)

* Removed extraneous whitespace
   - modified:   api/src/main/java/edu/cornell/mannlib/vitro/webapp/edit/n3editing/VTwo/BaseEditElementVTwo.java

* move RootUserPolicy.java from VIVO to Vitro repo.

* Remove blank line

* Fixed indentations

* Update whitespace for: TemplateProcessingHelper.java

* Update whitespace for: RDFServiceModel.java

* Removed extraneous whitespaces.

* Tagging UQAM Comments with following tags

-Add-Feature
-Optimization
-Linguistic-Management
-Bug-Correction

* Resolve compilation failures in Vitro tests

* Remove whitespace changes from SelectListGeneratorVTwo.java

* Add null check on field values of RDF Form (#158)

Resolves: https://jira.lyrasis.org/browse/VIVO-1800

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Fix incorrect tool-tip in language selection dropdown (#156)

Resolves: https://jira.lyrasis.org/browse/VIVO-1783

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Sprint i18n/lang filtering model (#159)

* Add language-aware graph filtering instead of layering on additional RDFService

Resolves: https://jira.lyrasis.org/browse/VIVO-1771

* Remove hardcoded link label (#161)

Resolves: https://jira.lyrasis.org/browse/VIVO-1779

* Enable filtering of non-enabled i18n language files from being loaded into triple store (#160)

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

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Add i18n version of Edit page title (#157)

Related to: https://jira.lyrasis.org/browse/VIVO-1779

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Changing ontology extensions for n3 & assigning a base IRI to each (#163)

ontology

Co-authored-by: michelheon <heon@videotorn.ca>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

https://jira.lyrasis.org/browse/VIVO-1862

* Update sprint-i18n with master branch (#166)

* Make data property richtext editor option selectable from UI

* [VIVO-1755] - Better error handling when reasoner disabled (#137)

* Better error handling when reasoner disabled

* Change reasoner error log message to debug

* Extend reasoner status error handling

* Improve reasoner error log message and extend to JenaAdminActions

* Bump up log level for admin action to 'warn' and edit admin panel error message

* Change Model writer lang from "N3-PP" to "N3" (#149)

Resolves: https://jira.lyrasis.org/browse/VIVO-1761

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [VIVO-1851] - Add 'remove' option to named graph management page (#162)

* Add 'remove' option to named graph management page

Resolves: https://jira.lyrasis.org/browse/VIVO-1851

* [VIVO-1872] - Add download option to SPARQL Query page (#164)

* Add download option to SPARQL Query page

* Set SPARQL query results content type even if downloading

* Address pull request comments

Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [VIVO-1839] - Use language name for selector instead of flags (#165)

* Use language name for selector instead of flags
* Add country name to language selector
* Increase min-width of language dropdown
* Update data tree comment in languageSelector.ftl
* Min-width for language dropdown
* Language dropdown css adjustment
* Capitalize locale label
* Indent on languageSelector.ftl

Resolves: https://jira.lyrasis.org/browse/VIVO-1839

* Adds internationalization to the admin/sparql-query page (#167)

* Adds internationalization to the admin/sparql-query page

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1873

* Add i18n for 'save query result'

Related to: https://jira.lyrasis.org/browse/VIVO-1873

* code review

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* removed redundat files which are now available via Vitro- and VIVO-languages

* Making UQAM-optimization aware of https://

* Add parent reference to installer/pom.xml (#174)

Resolves: https://jira.lyrasis.org/browse/VIVO-1903

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Revert non-function RDF changes (#177)

Related to: https://jira.lyrasis.org/browse/VIVO-1905

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Ensure 'other' in Individual->Teaching->advisees->type is translated

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1881

* Re-add logic from commit 7420957

* [VIVO-1812] use i18n translation for validation messages (#179)

* replace constants with i18n text in BasicValidationVTwo
* use constants for i18n template key

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1812

* added js_string at i18n strings to handle quotes properly, ticket vivo-1842 (#180)

Partial resolution for: https://jira.lyrasis.org/browse/VIVO-1842

* [VIVO-1870] simplify and concat translation for browse by vclass (#182)

* simplify and concat translation for browse by vclass

Partial resolution of: https://jira.lyrasis.org/browse/VIVO-1870

* [VIVO-1900] i18n: added empty check for getCountry, fixing bug with spanish label (es) (#181)

* added empty check for getCountry, fixing bug with spanish label (es), ticket vivo-1900

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1900

* [VIVO 1870] update i18n key to JavaScript mapping (#183)

* update menupage-script i18n key to JavaScript mapping

Follow-on to resolution of: https://jira.lyrasis.org/browse/VIVO-1870

* [VIVO-1837] get i18n bundle from WebappDaoFactoryConfig preferred locales (#178)

* apply i18n text to VClassDaoJena.getLabelForClass
* expose i18n bundle through webapp dao factory interface
* use whole messages for i18n
* if request available use request bundle else use context bundle
* clear cache when context theme directory changes
* make getOverridingLocale from context private
* select locale based on preferred locale from webapp dao factory
* use first preferred local even when no selectable locales available on context
* build default preferred locales from default preferred languages

Resolves: https://jira.lyrasis.org/browse/VIVO-1837

* Ensure that "available langs" include base langs (#185)

Resolves: https://jira.lyrasis.org/browse/VIVO-1922

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Ensure I18nStub is initialized before pertinent tests (#186)

Resolves: https://jira.lyrasis.org/browse/VIVO-1923

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* add additional null checks to ensure context is ready for theme change (#187)

* [VIVO-1915] i18n - Adding a label with language tag through the "new" manageLabelsForindividualAddForm.ftl does not take the setting for language tag (#184)

* Fixed bug when addng a label with language tag through managLabelsForindividualAddForm, re ticket VIVO-1915

* removed the language select for managing multi language labels, ticket vivo-1915

* readded the fix for language selection because of the fr-CA version of the manageLabelsForIndividualftl, ticket vivo-1915

* removed earlier fix, removed comments, modified ManageLabelGenrator, ticket vivo-1915

Partial resolution to: https://jira.lyrasis.org/browse/VIVO-1915

* - replaced hard coded "or"s with i18n().or

* [VIVO-1925] i18n: Editing labels results in new label (#188)

* fixed bug: Editing language labels results in new label, ticket vivo-1925

Resolves: https://jira.lyrasis.org/browse/VIVO-1925

* Enable lang selection without need for 'available-langs.properties' (#169)

* Enable lang selection without need for 'available-langs.properties'
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Reduce number of times QuerySolutions are looped in LanguageFilteringRDFService (#194)

- No functional change in this update

Resolves: https://jira.lyrasis.org/browse/VIVO-1931

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Prevent 'ProcessRdfForm.parseN3ToRDF' from using null linguisticContext (#197)

Resolves: https://jira.lyrasis.org/browse/VIVO-1944

Co-authored-by: Andrew Woods <awoods@duraspace.org>

* [Vivo 1918] i18n: Reload firsttime files on start-up if changed (#195)

Co-authored-by: Andrew Woods <awoods@lyrasis.org>

* [VIVO-1936] i18n: updated the language comment in runtime.properties

* ftl function to capitalize group name affording override

* Further i18n for BasicValidationVTwo.java

* Fixed DeletePropertyController.java - getting localname of property properly (#200)

* Fixed DeletePropertyController.java - getting localname of property properly

Resolves: https://jira.lyrasis.org/browse/VIVO-1816

* Removed hardcoded time units by properties calls in dateTimeWithPrecision.ftl

* Use i18n values for date time form (#204)

Resolves: https://jira.lyrasis.org/browse/VIVO-1953

* VIVO-1929: patch authorizing create individual form (#206)

* patch authorizing create individual form

Resolves: https://jira.lyrasis.org/browse/VIVO-1929

* Issue/vivo 1947 (#205)

* Make data property richtext editor option selectable from UI
* Better error handling when reasoner disabled
* Change reasoner error log message to debug
* Extend reasoner status error handling
* Improve reasoner error log message and extend to JenaAdminActions
* Bump up log level for admin action to 'warn' and edit admin panel error message
* Change Model writer lang from "N3-PP" to "N3" (#149)
* Prevent ontology editor and N3 editing from deleting property values in languages other than the one associated with the editing request. [https://jira.lyrasis.org/browse/VIVO-1947]

Resolves: https://jira.lyrasis.org/browse/VIVO-1947

Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: Andrew Woods <awoods@lyrasis.org>
Co-authored-by: Andrew Woods <awoods@duraspace.org>

* Remove comments

Co-authored-by: Andrew Woods <awoods@duraspace.org>
Co-authored-by: Nicolas D <46490666+nicalico@users.noreply.github.com>
Co-authored-by: VIVO UQAM <62542918+UQAM-VIVO@users.noreply.github.com>
Co-authored-by: Matthias Luehr <luehr@hs-mittweida.de>
Co-authored-by: michelheonuqam <heon.michel@uqam.ca>
Co-authored-by: Brian Lowe <brianjlowe@gmail.com>
Co-authored-by: j-dornbusch <joachim.dornbusch@ehess.fr>
Co-authored-by: Michel Heon <heon@videotron.ca>
Co-authored-by: gneissone <mbgross@wustl.edu>
Co-authored-by: Ralph O'Flinn <roflinn@users.noreply.github.com>
Co-authored-by: dofeldsc <dofeldsc@uos.de>
Co-authored-by: root <root@vivo-development.hs-mittweida.de>
Co-authored-by: gneissone <mbgross@unavco.org>
Co-authored-by: William Welling <wwelling@library.tamu.edu>
Co-authored-by: Kampe <Benjamin.Kampe@tib.eu>
Co-authored-by: Gross, Benjamin <benjamin.gross@clarivate.com>
Co-authored-by: matthiasluehr <60263380+matthiasluehr@users.noreply.github.com>
Co-authored-by: nicolasdickner <dickner.nicolas@uqam.ca>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants