Ticket #895 (closed enhancement: fixed)

Opened 3 years ago

Last modified 7 months ago

user comments for greenstone 3

Reported by: kjdon Owned by: ak19
Priority: moderate Milestone: 3.09 Release
Component: Greenstone3 Runtime Severity: enhancement
Keywords: Cc:

Description

Has this been done yet? if not, add it to gs3.

Change History

Changed 9 months ago by ak19

  • owner changed from nobody to ak19

Changesets for adding User Comments feature to GS3. So far tested on Linux.

http://trac.greenstone.org/changeset/31537

http://trac.greenstone.org/changeset/31540 to http://trac.greenstone.org/changeset/31547

http://trac.greenstone.org/changeset/31555 to http://trac.greenstone.org/changeset/31559

Still to do: test GS3 User comments feature on Windows, and remove the debug stmts that have now merely been commented out.

Changed 8 months ago by ak19

Windows testing revealed a problem with adding user comments and document editing: there was blocking in the java code launching perl at GS2PerlConstructor.runPerlCommand().

Wrote SafeProcess?.java based on the combination of GLI's Input- and OutputStreamGobblers? and how FormatConversionDialog? used them, and used the new SafeProcess? class in GS2PerlConstructor.runPerlCommand().

Commits:

http://trac.greenstone.org/changeset/31568, http://trac.greenstone.org/changeset/31569

http://trac.greenstone.org/changeset/31572 to http://trac.greenstone.org/changeset/31574

http://trac.greenstone.org/changeset/31578,

http://trac.greenstone.org/changeset/31579 which contains a correction (and undoing accidental commit: http://trac.greenstone.org/changeset/31580 )

Windows testing appeared successful: user comments and doc editing worked. No more blocking.

Then brought SafeProcess? over to GLI to first replace the extgernal Input- and OutputStreamGobbler? classes, and to make GLI's FormatConversionDialog?, GS3ServerThread (and Gatherer's use of it) and DownloadPane? go through SafeProcess? when running processes.

Commits: http://trac.greenstone.org/changeset/31581 to http://trac.greenstone.org/changeset/31587

which also required bringing the GS3 version of SafeProcess? up to speed. The differences between the GLI and GS3 version are in package name and usage of logger by GS3. Need to retest the GS3 changes with doc editing and user comments on Windows.

Changed 8 months ago by kjdon

  • milestone changed from Greenstone 3 catch-up to 3.09 Release

Changed 7 months ago by ak19

Other Java classes of GLI and GS3 src code were shifted from using Java's Process class directly to using our new SafeProcess? class.

The changes in GLI were to classes

GLI

wharariki:[265]/Scratch/ak19/gs3-svn-15Nov2016/gli>fgrep -rl "Process" src

+ src/org/greenstone/gatherer/util/SafeProcess.java

+ src/org/greenstone/gatherer/gui/FormatConversionDialog.java

+ src/org/greenstone/gatherer/gui/DownloadPane.java

+ src/org/greenstone/gatherer/util/GS3ServerThread.java

+ 2 src/org/greenstone/gatherer/Gatherer.java [2 MORE but don't want to mess with those]

+ 1 src/org/greenstone/gatherer/download/ServerInfoDialog.java

+ 2 src/org/greenstone/gatherer/greenstone/Classifiers.java

+ 2 src/org/greenstone/gatherer/greenstone/Plugins.java

+ 1 src/org/greenstone/gatherer/collection/ScriptOptions.java

+ !! src/org/greenstone/gatherer/util/ExternalProgram.java && file/FileAssociationManager.java

After updating GS3 src code to using SafeProcess?, returned to

VERY HARD:

+ 1 src/org/greenstone/gatherer/shell/GShell.java

VERY, VERY HARD:

+ 2 src/org/greenstone/gatherer/download/DownloadJob.java

GS3 src code:

fgrep -r "Process" . | grep -v ".svn" | grep -v "~" | less

+ 2 ./java/org/greenstone/admin/guiext/Command.java

X ./java/org/greenstone/gsdl3/util/Processing.java

+ MANY OCCURRENCES ./java/org/greenstone/gsdl3/service/MapRetrieve.java

+ Uses Processing 2 times: ./java/org/greenstone/gsdl3/util/GDBMWrapper.java

+ ./java/org/greenstone/util/BrowserLauncher.java

+ ./java/org/greenstone/util/RunTarget.java

In the GLI and GS3 src files modified to use SafeProcess? above, only the modifications to GS3's Command.java, MapRetreive?.java and GDMBWrapper.java are untested. All others have been tested, at least on Linux. Many have also been tested on Windows.

Tested GLI's collection building (uses GShell.java's modifications) and DownloadJob?.java on all 3 OS. Both have a Cancel button that should cancel any processes and subprocesses launched. Before the modifications, subprocesses would still be running on Windows and during (full-)import on the Mac when cancel was pressed. But now have added code to SafeProcess? to issue OS specific system calls to terminate the process and its subprocesses successfully on Windows and Mac too.

Also tested GS3 user comments and the online GS3's doc editor's meta editing followed by rebuilding. All worked successfully on Linux, Windows and Mac with the modifications to use SafeProcess?.

The modifications to GLI started with GS3ServerThread.java and other *easily* changed classes in http://trac.greenstone.org/changeset/31582 (beginning of GLI modification) until http://trac.greenstone.org/changeset/31665 (GS3 src code modification).

The version of GS3 and GLI Java code before the GS3 src was modified is at http://trac.greenstone.org/browser/main/tags/GS3-src-SafeProcess

Next, GLI's GShell.java and any associated files were modified along with SafeProcess?, to support cancelling.

Then, around 22 May, GLI's DownloadJob?.java and any associated class files were modified along with SafeProcess? to ensure subprocesses were safely terminated on Windows when the download action was cancelled OR when building (GShell.java) was cancelled. The Mac modifications to SafeProcess? to ensure subprocesses were terminated was finished on 26 May 2017.

The tagged version of the GS3 and GLI Java code for when all the changes related to using SafeProcess? were made to GS3 and GLI code is at http://trac.greenstone.org/browser/main/tags/UsingSafeProcess

Changed 7 months ago by ak19

  • status changed from new to closed
  • resolution set to fixed

More changesets

http://trac.greenstone.org/changeset/31711 to http://trac.greenstone.org/changeset/31718

These changesets involved:

- a README on SafeProcess?

- the remaining 2 instances of using Runtime.exec/Java Process in Gatherer have been ported to using SafeProcess?. These instances were of the inner classes Gatherer.ExternalApplication? and Gatherer.BrowserApplication?, both of which needed SafeProcess? to have the cancel functionality before they could be ported to use SafeProcess?. Tested the modified inner classes on Linux.

When tested on Mac, I found that with or without these new changes, GLI does not wait to exit if external applications are still running. On Linux (and the way GLI is coded), GLI's GUI may disappear, but the overall GLI process still waits for external processes launched via GLI to have exited before GLI exits. This doesn't happen on Mac with either the original or the modified code. Worth investigating. But at least the modified code has not broken something that used to work.

Note: See TracTickets for help on using tickets.