Changeset 28382 for main/trunk/greenstone3/src/java/org/greenstone/gsdl3/action/QueryAction.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/QueryAction.java (modified) (20 diffs)

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

@@ -11 +11 @@
 import org.greenstone.gsdl3.util.GSXSLT;
 import org.greenstone.gsdl3.util.UserContext;
+import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
@@ -26 +27 @@
     public Node process(Node message_node)
     {
-
         Element message = this.converter.nodeToElement(message_node);
-
+        Document doc = message.getOwnerDocument();
+        
         // get the request - assume there is only one
         Element request = (Element) GSXML.getChildByTagName(message, GSXML.REQUEST_ELEM);
 
         // create the return message
-        Element result = this.doc.createElement(GSXML.MESSAGE_ELEM);
+        Element result = doc.createElement(GSXML.MESSAGE_ELEM);
         Element response = basicQuery(request);
-        result.appendChild(this.doc.importNode(response, true));
+        result.appendChild(doc.importNode(response, true));
         return result;
     }
@@ -47 +48 @@
     {
         // the result
-        Element page_response = this.doc.createElement(GSXML.RESPONSE_ELEM);
+        Document doc = request.getOwnerDocument();
+        Element page_response = doc.createElement(GSXML.RESPONSE_ELEM);
 
         // extract the params from the cgi-request, and check that we have a coll specified
@@ -75 +77 @@
         format_elem.setAttribute(GSXML.TYPE_ATT, "search");
         // for now just add to the response
-        page_response.appendChild(this.doc.importNode(format_elem, true));
+        page_response.appendChild(doc.importNode(format_elem, true));
 
         if (request_type.indexOf("d") != -1)
         {
             // we have been asked for the service description
-            Element mr_info_message = this.doc.createElement(GSXML.MESSAGE_ELEM);
-            Element mr_info_request = GSXML.createBasicRequest(this.doc, GSXML.REQUEST_TYPE_DESCRIBE, to, userContext);
+            Element mr_info_message = doc.createElement(GSXML.MESSAGE_ELEM);
+            Element mr_info_request = GSXML.createBasicRequest(doc, GSXML.REQUEST_TYPE_DESCRIBE, to, userContext);
             mr_info_message.appendChild(mr_info_request);
 
@@ -90 +92 @@
             Element service_response = (Element) GSXML.getChildByTagName(mr_info_response, GSXML.RESPONSE_ELEM);
 
-            Element service_description = (Element) this.doc.importNode(GSXML.getChildByTagName(service_response, GSXML.SERVICE_ELEM), true);
+            Element service_description = (Element) doc.importNode(GSXML.getChildByTagName(service_response, GSXML.SERVICE_ELEM), true);
             page_response.appendChild(service_description);
         }
@@ -114 +116 @@
 
         // create the query request
-        Element mr_query_message = this.doc.createElement(GSXML.MESSAGE_ELEM);
-        Element mr_query_request = GSXML.createBasicRequest(this.doc, GSXML.REQUEST_TYPE_PROCESS, to, userContext);
+        Element mr_query_message = doc.createElement(GSXML.MESSAGE_ELEM);
+        Element mr_query_request = GSXML.createBasicRequest(doc, GSXML.REQUEST_TYPE_PROCESS, to, userContext);
         mr_query_message.appendChild(mr_query_request);
 
-        Element query_param_list = this.doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
-        GSXML.addParametersToList(this.doc, query_param_list, service_params);
+        Element query_param_list = doc.createElement(GSXML.PARAM_ELEM + GSXML.LIST_MODIFIER);
+        GSXML.addParametersToList(doc, query_param_list, service_params);
         mr_query_request.appendChild(query_param_list);
 
@@ -144 +146 @@
         else
         { // add it into the page response
-            page_response.appendChild(this.doc.importNode(query_result_metadata_list, true));
+            page_response.appendChild(doc.importNode(query_result_metadata_list, true));
         }
 
@@ -154 +156 @@
         else
         { // add it into the page response
-            page_response.appendChild(this.doc.importNode(query_term_info_list, true));
+            page_response.appendChild(doc.importNode(query_term_info_list, true));
         }
 
@@ -164 +166 @@
         else
         { // add it into the page response
-            page_response.appendChild(this.doc.importNode(facet_list, true));
+            page_response.appendChild(doc.importNode(facet_list, true));
         }
 
@@ -173 +175 @@
         {
             // add in a dummy doc node list - used by the display. need to think about this
-            page_response.appendChild(this.doc.createElement(GSXML.DOC_NODE_ELEM + GSXML.LIST_MODIFIER));
+            page_response.appendChild(doc.createElement(GSXML.DOC_NODE_ELEM + GSXML.LIST_MODIFIER));
             //append site metadata
             addSiteMetadata(page_response, userContext);
@@ -186 +188 @@
             logger.error("have already found metadata!");
             // append the doc list to the result
-            page_response.appendChild(this.doc.importNode(document_list, true));
+            page_response.appendChild(doc.importNode(document_list, true));
             //append site metadata
             addSiteMetadata(page_response, userContext);
@@ -203 +205 @@
 
         // paging of the results is done here - we filter the list to remove unwanted entries before retrieving metadata
-        Element filtered_doc_list = filterDocList(params, service_params, document_list);
+        Element filtered_doc_list = filterDocList(doc, params, service_params, document_list);
 
         // do the metadata request on the filtered list
-        Element mr_metadata_message = this.doc.createElement(GSXML.MESSAGE_ELEM);
+        Element mr_metadata_message = doc.createElement(GSXML.MESSAGE_ELEM);
         to = "DocumentMetadataRetrieve";
         if (collection != null)
@@ -212 +214 @@
             to = GSPath.prependLink(to, collection);
         }
-        Element mr_metadata_request = GSXML.createBasicRequest(this.doc, GSXML.REQUEST_TYPE_PROCESS, to, userContext);
+        Element mr_metadata_request = GSXML.createBasicRequest(doc, GSXML.REQUEST_TYPE_PROCESS, to, userContext);
         mr_metadata_message.appendChild(mr_metadata_request);
 
@@ -228 +230 @@
         }
 
-        Element dm_param_list = createMetadataParamList(metadata_names);
+        Element dm_param_list = createMetadataParamList(doc,metadata_names);
 
         mr_metadata_request.appendChild(dm_param_list);
@@ -246 +248 @@
         if (query_result_document_list != null)
         {
-            page_response.appendChild(this.doc.importNode(query_result_document_list, true));
+            page_response.appendChild(doc.importNode(query_result_document_list, true));
         }
 
@@ -257 +259 @@
 
     /** this filters out some of the doc results for result paging */
-    protected Element filterDocList(HashMap<String, Serializable> params, HashMap service_params, Element orig_doc_list)
+    protected Element filterDocList(Document doc, HashMap<String, Serializable> params, HashMap service_params, Element orig_doc_list)
     {
 
@@ -265 +267 @@
         {
             // the service is doing the paging, so we want to display all of the returned docs(???)
-            //    return (Element)this.doc.importNode(orig_doc_list, true);
+            //    return (Element)doc.importNode(orig_doc_list, true);
             // try hitsPerPage in the globle param
             hits_pp = (String) params.get("hitsPerPage");
@@ -285 +287 @@
         if (hits == -1)
         { // all
-            return (Element) this.doc.importNode(orig_doc_list, true);
+            return (Element) doc.importNode(orig_doc_list, true);
         }
         NodeList result_docs = orig_doc_list.getElementsByTagName(GSXML.DOC_NODE_ELEM);
@@ -293 +295 @@
         {
             // too few docs to do paging
-            return (Element) this.doc.importNode(orig_doc_list, true);
+            return (Element) doc.importNode(orig_doc_list, true);
         }
 
         // now we need our own doc list
-        Element result_list = this.doc.createElement(GSXML.DOC_NODE_ELEM + GSXML.LIST_MODIFIER);
+        Element result_list = doc.createElement(GSXML.DOC_NODE_ELEM + GSXML.LIST_MODIFIER);
 
         String start_p = (String) service_params.get("startPage");
@@ -335 +337 @@
         for (int i = start_from; i <= end_at; i++)
         {
-            result_list.appendChild(this.doc.importNode(result_docs.item(i), true));
+            result_list.appendChild(doc.importNode(result_docs.item(i), true));
         }