Changeset 34232


Ignore:
Timestamp:
2020-07-01T01:05:57+12:00 (4 years ago)
Author:
ak19
Message:

Bugfix. When using client-GLI noticed that creating or editing an existing metadataset (GEMS) caused an error. The problem was compounded by a deadlock situation in displaying the error message in a popup. That deadlock is not resolved here (see future commit for attempted fix of it). This commit resolves the root cause: which was that Configuration.site_name was suddenly set to null, thereby failing to upload the mds file to the remote GS3. The nulled site_name may however not be a problem that will only affect client-GLI, as the Preferences pane would not open and things froze in client-GLI because site_name was null, which I think can affect GLI too. Need to check this, albeit with the bug still intact, by using GEMS in GLI to create/edit an mds file, then going to Preferences (or for local GLI perhaps the active site_name should moreover be set to other than localsite first). The cause was that there was a single GEMS constructor, used both when GEMS is launched as a standalone app and when GEMS launched through GLI. However, in both cases, the GEMS constructor dangerously erased and replaced the static-looking Configuration object (and Dictionary) with a basic Configuration object, losing crucial information like site_name and who knows what else. The problem was duplicated in GEMS.java as this instantiated a MetadataSetManager object whose constructor did the same thing of erasing and replacing Configuration, where it may have been a copy-paste error from GEMS.java. Once the mysterious cause of this problem was finally tracked down, the solution was just to have additional constructors that assume Configuration and Dictionary exist (thus not overwriting them), to be called when GEMS is launched through GLI, and when MetadataSetManager is launched through GEMS as GEMS always ensures a Configuration object exists.

Location:
main/trunk/gli/src/org/greenstone/gatherer
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • main/trunk/gli/src/org/greenstone/gatherer/gems/GEMS.java

    r32276 r34232  
    135135       
    136136    }
    137 
    138 
    139     /** Constructor.
     137   
     138    /** Constructor - original. Called when GEMS is launched as a standalone application.
     139     * Sets up a very basic static-looking Configuration object, and Dictionary.
     140     * Do not call if GEMS is launched through GLI, as this constructor will erase and replace
     141     * GLI's carefully configured Configuration and Dictionary causing hard to track down errors.
     142     * So don't call this constructor if standalone = false.
    140143     */
    141144    public GEMS(String gsdl_path, String gsdl3_path, String metadata_path, boolean standalone, boolean new_set)
    142145    {
    143146    self = this;
    144     JarTools.initialise(this);       
    145     screen_size = Configuration.screen_size;
    146 
     147    JarTools.initialise(this);
    147148    // Load GLI config file: set up the Configuration so we can set up the Dictionary language
    148149    // Determine the GLI user directory path
    … …  
    154155            gli_user_directory_path += ".gli" + File.separator;
    155156        }   
     157
     158    // Since GEMS launched standalone, need to set up Configuration and Dictionary
    156159    new Configuration(gli_user_directory_path, gsdl_path, gsdl3_path,
    157160              null /*gsdl3_src_path*/, null /*site_name*/, null /*fedora_info*/); // null for parameters unknown to GEMS
    158        
     161
    159162    // Read Dictionary in the locale specified in the config.xml
    160163    new Dictionary(Configuration.getLocale("general.locale", true), Configuration.getFont("general.font", true));
    161    
    162         msm = new MetadataSetManager(gsdl_path,gsdl3_path);       
     164    init(metadata_path, standalone, new_set);
     165    }
     166
     167    /** Constructor - overloaded to use existing Configuration and Dictionary
     168     * This Constructor is used when GEMS is launched through GLI.
     169     * It's crucial for GLI to launch GEMS with this constructor as the other one will erase and
     170     * replace GLI's carefully set-up Configuration and Dictionary, resulting in subtle but major
     171     * flow-on errors particularly because of the alternate's cutdown version of Configuration.
     172     */
     173    public GEMS(String metadata_path, boolean new_set)
     174    {   
     175    self = this;
     176    JarTools.initialise(this);
     177
     178    // When GEMS is launched through GLI (so not standalone)
     179    // a Configuration and Dictionary is already set up
     180
     181    init(metadata_path, false /*standalone*/, new_set);
     182    }
     183
     184    private void init(String metadata_path, boolean standalone, boolean new_set) {
     185    screen_size = Configuration.screen_size;
     186   
     187    //msm = new MetadataSetManager(Configuration.gsdl_path,Configuration.gsdl3_path);
     188             // dangerously side-effecting version of MetadataSetManager constructor that
     189        // erases Configuration all over again (as the original GEMS constructor also did)
     190        // and is noticeable when remote GS3 server cryptically fails because
     191        // Configuration.site_name is suddenly null due to these constructors.
     192        // Creating/adding a new metadata set also prevents GLI > Preferences from opening
     193        // because Configuration.site_name is nulled as a consequence of launching GEMS through GLI
     194
     195    // Instead, call the MetadataSetManager() constructor as it reuses existing Configuration,
     196    // whether this is a basic Configuration created by standalone GEMS
     197    // or a carefully and highly configured Configuration object created by GLI launching GEMS.
     198    msm = new MetadataSetManager();
     199   
    163200        stand_alone = standalone;
    164201    listeners = new ArrayList();
    … …  
    219256        new_prompt.addMetadataSetListener((MetadataSetListener)attribute_table);
    220257        new_prompt.addMetadataSetListener((MetadataSetListener)language_dependent_attribute_table);
    221  
    222 
     258   
     259   
    223260        // load the initial metadataset
    224261        if (metadata_path !=null && !metadata_path.equals("")){
    … …  
    320357    getContentPane().add(card_pane,BorderLayout.CENTER);
    321358    setLocation((screen_size.width - SIZE.width) / 2, (screen_size.height - SIZE.height) / 2);
    322     if (stand_alone)
     359   
     360    if (stand_alone) {
    323361        setVisible(true);
     362    }
    324363    }
    325364

  • main/trunk/gli/src/org/greenstone/gatherer/gems/MetadataSetManager.java

    r14974 r34232  
    7070    private String current_language = GEMSConstants.DEFAULT_LANGUAGE;
    7171
    72     /** Constructor. */
     72    /** Constructor.
     73     * This constructor can be dangerous if launched by a Greenstone application
     74     * that already set up the static looking Configuration object, because
     75     * this constructor will subtly erase and replace any previously existing
     76     * carefully configured Configuration object.
     77     * I think this constructor would only be useful if MetadataSetManager is ever launched
     78     * as a standalone GS3 application.
     79     * If instantiating a MetadataSetManager through GEMS, use the MetadataSetManager() constructor,
     80     * including if GEMS is launched through GLI.
     81     * Also use MetadataSetManager() if creating a MetadataSetManager object through GLI.
     82     */
    7383    public MetadataSetManager(String gsdl_path, String gsdl3_path) {
    7484        // Determine the GLI user directory path
    … …  
    8797        languageList = new ArrayList();
    8898    }
    89 
     99   
     100    /** Constructor - overloaded to use any existing Configuration already set up.
     101     * This Constructor is used by GEMS when GEMS is launched through GLI.
     102     */
     103    public MetadataSetManager() {           
     104    current_language = Configuration.getLanguage();
     105
     106        languageList = new ArrayList();
     107    }
     108   
    90109    public ArrayList getLanguageList(){
    91110        //already loaded

  • main/trunk/gli/src/org/greenstone/gatherer/gui/MetadataSetDialog.java

    r18370 r34232  
    138138        String metadata_path = metadata_set.getMetadataSetFile().toString();
    139139        if (gems == null){
    140         gems = new GEMS(Configuration.gsdl_path,Configuration.gsdl3_path,"", false, false);
     140       
     141        //gems = new GEMS(Configuration.gsdl_path,Configuration.gsdl3_path,"", false, false, site_name); // dangerous: this version of GEMS constructor only for if GEMS not launched through GLI but standalone
     142        // refer to comments in GEMS.java for GEMS constructors
     143        gems = new GEMS("", false); // right way to create GEMS app when launched by GLI
    141144        gems.addGEMSListener(this);
    142145        }
    … …  
    270273            public void actionPerformed(ActionEvent event) {
    271274            if (gems == null) {
    272                 gems = new GEMS(Configuration.gsdl_path, Configuration.gsdl3_path, "", false, false);
     275                //gems = new GEMS(Configuration.gsdl_path, Configuration.gsdl3_path, "", false, false, site_name); // dangerous: this version of GEMS constructor only for if GEMS not launched through GLI but standalone
     276                // refer to comments in GEMS.java for GEMS constructors
     277                gems = new GEMS("", false); // right way to create GEMS app when launched by GLI
    273278            }
    274279            gems.newMetadataSet();
Note: See TracChangeset for help on using the changeset viewer.