Changeset 28382 for main/trunk/greenstone3/src/java/org/greenstone/gsdl3/action/Action.java

Timestamp:

2013-10-10T17:21:30+13:00 (11 years ago)

Author:

davidb

Message:

Elimination of the 'this.doc' field from the Action baseclass and the subclasses that rely on it. For Greenstone3 purposes it is unsafe to create this object in the constructor to the action and then store it for other methods to access. This is because the Greenstone 3 (and in particular calls to 'process' operate in a multi-threaded context, that is managed by the Servlet server (e.g. Tomcat by default). Calls to DOM methods are not guaranteed to be thread safe, this became apparent when we started looking in to an exception that was being thrown, and centred around use of the DOM method 'item(i)'. The change this commit makes is to remove 'this.doc' being stored as a field. A document is now created in the top level of a call to 'process()' and when a DOM reference is needed in a subsequent method an Element variable (typically passed in as a parameter to the method) is used (through 'Document doc = element.getOwnerDocument()') to gain access to the DOM

File:

• main/trunk/greenstone3/src/java/org/greenstone/gsdl3/action/Action.java (modified) (7 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/action/Action.java

@@ -24 +24 @@
     /** the system set up variables */
     protected HashMap<String, Object> config_params = null;
-    /** container Document to create XML Nodes */
-    protected Document doc = null;
+    
+    
     /** a converter class to parse XML and create Docs */
     protected XMLConverter converter = null;
@@ -39 +39 @@
     {
         this.converter = new XMLConverter();
-        this.doc = this.converter.newDOM();
     }
 
@@ -193 +192 @@
     }
 
-    protected Element createMetadataParamList(HashSet<String> metadata_names)
-    {
-        Element param_list = this.doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
+    protected Element createMetadataParamList(Document doc, HashSet<String> metadata_names)
+    {
+        Element param_list = doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
 
         Element param = null;
@@ -202 +201 @@
         {
             String name = i.next();
-            param = this.doc.createElement(GSXML.PARAM_ELEM);
+            param = doc.createElement(GSXML.PARAM_ELEM);
             param_list.appendChild(param);
             param.setAttribute(GSXML.NAME_ATT, "metadata");
@@ -240 +239 @@
     protected void addSiteMetadata(Element element, UserContext userContext)
     {
+        Document doc = element.getOwnerDocument();
+        
         //ADD SITE METADATA
-        Element metadata_request = GSXML.createBasicRequest(this.doc, GSXML.REQUEST_TYPE_DESCRIBE, "", userContext);
+        Element metadata_request = GSXML.createBasicRequest(doc, GSXML.REQUEST_TYPE_DESCRIBE, "", userContext);
         //create a hashmap of params
         HashMap subset_params = new HashMap(1);
         subset_params.put(GSXML.SUBSET_PARAM, GSXML.METADATA_ELEM + GSXML.LIST_MODIFIER);
         //create the element to put the params in
-        Element param_list = this.doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
+        Element param_list = doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
         //put them in
-        GSXML.addParametersToList(this.doc, param_list, subset_params);
+        GSXML.addParametersToList(doc, param_list, subset_params);
         metadata_request.appendChild(param_list);
         //create the message
-        Element metadata_message = this.doc.createElement(GSXML.MESSAGE_ELEM);
+        Element metadata_message = doc.createElement(GSXML.MESSAGE_ELEM);
         metadata_message.appendChild(metadata_request);
         //get response
@@ -263 +264 @@
     protected void addInterfaceOptions(Element elem)
     {
-        Element documentOptionList = this.doc.createElement("interfaceOptions");
+        Document doc = elem.getOwnerDocument();
+        
+        Element documentOptionList = doc.createElement("interfaceOptions");
         for (Object key : this.config_params.keySet())
         {
-            Element option = this.doc.createElement("option");
+            Element option = doc.createElement("option");
             option.setAttribute(GSXML.NAME_ATT, (String) key);
             option.setAttribute(GSXML.VALUE_ATT, this.config_params.get(key).toString());
@@ -276 +279 @@
     protected Element getFormatInfo(String to, UserContext userContext)
     {
-        Element mr_format_message = this.doc.createElement(GSXML.MESSAGE_ELEM);
-        Element mr_format_request = GSXML.createBasicRequest(this.doc, GSXML.REQUEST_TYPE_FORMAT, to, userContext);
+        // Eclipse call hierarchy shows the element returned from this method is 
+        // subsequently used in a 'importNode'.  For this reason it is safe here
+        // for call up our own document DOM.  
+        //
+        // If this pattern changes for any reason, then the DOM will need to be
+        // passed in as a parameter
+        
+        Document doc = this.converter.newDOM();
+        
+        Element mr_format_message = doc.createElement(GSXML.MESSAGE_ELEM);
+        Element mr_format_request = GSXML.createBasicRequest(doc, GSXML.REQUEST_TYPE_FORMAT, to, userContext);
         mr_format_message.appendChild(mr_format_request);