From be06b6887176e9bb11ad48626433be42406eb634 Mon Sep 17 00:00:00 2001
From: j2blake <j2blake@440791e1-c2bf-4c1e-ad7c-caf3f8b0ebe8>
Date: Mon, 10 Oct 2011 19:09:10 +0000
Subject: [PATCH] NIHVIVO-3209 Simplify logic in IndexBuilder; don't expose
 intermediate results as instance variables, where they could be subject to
 concurrency issues.

---
 .../webapp/search/indexing/IndexBuilder.java  | 76 ++++++++-----------
 1 file changed, 32 insertions(+), 44 deletions(-)

diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/indexing/IndexBuilder.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/indexing/IndexBuilder.java
index a96539916..dd1607f76 100644
--- a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/indexing/IndexBuilder.java
+++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/indexing/IndexBuilder.java
@@ -58,17 +58,6 @@ public class IndexBuilder extends Thread {
      * updated in the search index when a statement changes.  */     
     protected List<StatementToURIsToUpdate> stmtToURIsToIndexFunctions;    
     
-    /**
-     * updatedUris will only be accessed from the IndexBuilder thread
-     * so it doesn't need to be synchronized.
-     */
-    private List<String> updatedUris = null;
-    /**
-     * deletedUris will only be accessed from the IndexBuilder thread
-     * so it doesn't need to be synchronized.
-     */
-    private List<String> deletedUris = null;    
-    
     /**
      * Indicates that a full index re-build has been requested.
      */
@@ -101,8 +90,8 @@ public class IndexBuilder extends Thread {
         super("IndexBuilder");
         
         this.indexer = indexer;
-        this.wdf = wdf;            
                 
+        this.wdf = wdf;            
         if( stmtToURIsToIndexFunctions != null )
             this.stmtToURIsToIndexFunctions = stmtToURIsToIndexFunctions;
         else
@@ -248,32 +237,29 @@ public class IndexBuilder extends Thread {
     }
     
 	/**
-	 * Sets updatedUris and deletedUris lists from the changedStmtQueue.
-	 * updatedUris and deletedUris will only be accessed from the IndexBuilder thread
-	 * so they don't need to be synchronized.
+	 * Take the URIs that we got from the changedStmtQueue, and create the lists
+	 * of updated URIs and deleted URIs.
 	 */
-	private void makeAddAndDeleteLists( Collection<String> uris){	    				
-	    IndividualDao indDao = wdf.getIndividualDao();
-	    
-		/* clear updateInds and deletedUris.  This is the only method that should set these. */
-		this.updatedUris = new LinkedList<String>();
-		this.deletedUris = new LinkedList<String>();
-						
-    	for( String uri: uris){
-    		if( uri != null ){
-    			try{
-    			    Individual ind = indDao.getIndividualByURI(uri);    			    
-	    			if( ind != null)
-	    				this.updatedUris.add(uri);
-	    			else{
-	    				log.debug("found delete in changed uris");
-	    				this.deletedUris.add(uri);
-	    			}
-    			} catch(QueryParseException ex){
-    				log.error("could not get Individual "+ uri,ex);
-    			}
-    		}
-    	}    		    	            	
+	private UriLists makeAddAndDeleteLists(Collection<String> uris) {
+		IndividualDao indDao = wdf.getIndividualDao();
+
+		UriLists uriLists = new UriLists();
+		for (String uri : uris) {
+			if (uri != null) {
+				try {
+					Individual ind = indDao.getIndividualByURI(uri);
+					if (ind != null) {
+						uriLists.updatedUris.add(uri);
+					} else {
+						log.debug("found delete in changed uris");
+						uriLists.deletedUris.add(uri);
+					}
+				} catch (QueryParseException ex) {
+					log.error("could not get Individual " + uri, ex);
+				}
+			}
+		}
+		return uriLists;
 	}	
 
 	/**
@@ -298,13 +284,10 @@ public class IndexBuilder extends Thread {
     protected void updatedIndex() {
         log.debug("Starting updateIndex()");       
                      
-        makeAddAndDeleteLists( statementsToUris(getAndEmptyChangedStatements()) );
+        UriLists uriLists = makeAddAndDeleteLists( statementsToUris(getAndEmptyChangedStatements()) );
         
-        this.numberOfThreads = Math.max( MAX_UPDATE_THREADS, updatedUris.size() / 20); 
-        doBuild( updatedUris.iterator(), deletedUris );
-        
-        this.updatedUris = null;
-        this.deletedUris = null;
+        this.numberOfThreads = Math.max( MAX_UPDATE_THREADS, uriLists.updatedUris.size() / 20); 
+        doBuild( uriLists.updatedUris.iterator(), uriLists.deletedUris );
         
         log.debug("Ending updateIndex()");
     }
@@ -453,6 +436,11 @@ public class IndexBuilder extends Thread {
         synchronized( changedStmtQueue ){
             return reindexRequested || ! changedStmtQueue.isEmpty() ;
         }
-    }        
+    }
+    
+    private static class UriLists {
+        private final List<String> updatedUris = new ArrayList<String>();
+        private final List<String> deletedUris = new ArrayList<String>();
+    }
 }