1.) Adding null pointer guards against collections being null

2.) Improving logic of Browse and Search consumers because both API support the same method for both the update and add of items and internally manage decisions about the appropriateness of the item to be indexed.

git-svn-id: http://scm.dspace.org/svn/repo/trunk@2163 9c30dcfa-912a-0410-8fc2-9e0234be79fd

dspace-api/src/main/java/org/dspace/browse/BrowseConsumer.java

@@ -47,6 +47,7 @@ import java.util.Set;
 import java.util.HashSet;
 import java.util.Iterator;
 
+import org.dspace.content.Bundle;
 import org.dspace.content.Item;
 import org.dspace.content.DSpaceObject;
 import org.dspace.core.Context;
@@ -82,72 +83,60 @@ public class BrowseConsumer implements Consumer
     /** log4j logger */
     private static Logger log = Logger.getLogger(BrowseConsumer.class);
 
-    // items to be added to browse index
-    private Set toAdd = null;
-
     // items to be updated in browse index
     private Set toUpdate = null;
 
-
     public void initialize()
         throws Exception
     {
-        toAdd = new HashSet();
+       
-        toUpdate = new HashSet();
     }
 
     public void consume(Context ctx, Event event)
         throws Exception
     {
+        if(toUpdate == null)
+        {
+            toUpdate = new HashSet();
+        }
+        
         DSpaceObject subj = event.getSubject(ctx);
+        
+        int st = event.getSubjectType();
         int et = event.getEventType();
         
-        // If an Item is added or modified..
+        switch (st)
-        if (subj != null && subj.getType() == Constants.ITEM)
         {
-            if (et == Event.CREATE)
-                toAdd.add(subj);
-            else
-                toUpdate.add(subj);
 
+        // If an Item is created or modified..
+        case Constants.ITEM:
+            toUpdate.add(subj);
+            break;
         // track ADD and REMOVE from collections, that changes browse index.
-        } else if (subj != null && subj.getType() == Constants.COLLECTION &&
+        case Constants.COLLECTION:
-                   event.getObjectType() == Constants.ITEM &&
+            if (event.getObjectType() == Constants.ITEM
-                   (et == Event.ADD || et == Event.REMOVE))
+                    && (et == Event.ADD || et == Event.REMOVE))
             {
                 DSpaceObject obj = event.getObject(ctx);
                 if (obj != null)
                     toUpdate.add(obj);
             }
-        else if (subj != null)
+            break;
+        default:
             log.warn("consume() got unrecognized event: " + event.toString());
+
+        }
+        
     }
 
     public void end(Context ctx)
         throws Exception
     {
-        for (Iterator ai = toAdd.iterator(); ai.hasNext();)
-        {
-            Item i = (Item)ai.next();
-            // FIXME: there is an exception handling problem here
-            try
-            {
-            	// Update browse indices
-            	IndexBrowse ib = new IndexBrowse(ctx);
-            	ib.indexItem(i);
-            }
-            catch (BrowseException e)
-            {
-            	log.error("caught exception: ", e);
-            	throw new SQLException(e.getMessage());
-            }
         
-            toUpdate.remove(i);
+        if (toUpdate != null)
-            if (log.isDebugEnabled())
+        {
-                log.debug("Added browse indices for Item id="+String.valueOf(i.getID())+", hdl="+i.getHandle());
-        }
 
-        // don't update an item we've just added.
+            // Update/Add items
             for (Iterator ui = toUpdate.iterator(); ui.hasNext();)
             {
                 Item i = (Item) ui.next();
@@ -161,11 +150,13 @@ public class BrowseConsumer implements Consumer
                 catch (BrowseException e)
                 {
                     log.error("caught exception: ", e);
-            	throw new SQLException(e.getMessage());
+                    //throw new SQLException(e.getMessage());
                 }
 
                 if (log.isDebugEnabled())
-                log.debug("Updated browse indices for Item id="+String.valueOf(i.getID())+", hdl="+i.getHandle());
+                    log.debug("Updated browse indices for Item id="
+                            + String.valueOf(i.getID()) + ", hdl="
+                            + i.getHandle());
             }
 
             // NOTE: Removed items are necessarily handled inline (ugh).
@@ -173,14 +164,13 @@ public class BrowseConsumer implements Consumer
             // browse updates wrote to the DB, so we have to commit.
             ctx.getDBConnection().commit();
 
-        // clean out toAdd & toUpdate
+        }
-        toAdd.clear();
+        
-        toUpdate.clear();
+        // clean out toUpdate
+        toUpdate = null;
     }
     
     public void finish(Context ctx) {
     	
-    	toAdd = toUpdate = null;
-    	return;
     }
 }

dspace-api/src/main/java/org/dspace/search/SearchConsumer.java

@@ -66,10 +66,7 @@ public class SearchConsumer implements Consumer
     /** log4j logger */
     private static Logger log = Logger.getLogger(SearchConsumer.class);
 
-    // collect Items, Collections, Communities newly created.
+    // collect Items, Collections, Communities that need indexing
-    private Set objectsCreated = null;
-
-    // collect Items, Collections, Communities that need reindexing
     private Set objectsToUpdate = null;
 
     // handles to delete since IDs are not useful by now.
@@ -93,9 +90,8 @@ public class SearchConsumer implements Consumer
     public void consume(Context ctx, Event event) throws Exception
     {
 
-        if (objectsCreated == null)
+        if (objectsToUpdate == null)
         {
-            objectsCreated = new HashSet();
             objectsToUpdate = new HashSet();
             handlesToDelete = new HashSet();
         }
@@ -144,18 +140,10 @@ public class SearchConsumer implements Consumer
         switch (et)
         {
         case Event.CREATE:
-            if (dso == null)
-                log.warn("CREATE event, could not get object for "
-                        + event.getSubjectTypeAsString() + " id="
-                        + String.valueOf(event.getSubjectID())
-                        + ", perhaps it has been deleted.");
-            else
-                objectsCreated.add(dso);
-            break;
         case Event.MODIFY:
         case Event.MODIFY_METADATA:
             if (dso == null)
-                log.warn("MODIFY event, could not get object for "
+                log.warn(event.getEventTypeAsString() + " event, could not get object for "
                         + event.getSubjectTypeAsString() + " id="
                         + String.valueOf(event.getSubjectID())
                         + ", perhaps it has been deleted.");
@@ -186,37 +174,9 @@ public class SearchConsumer implements Consumer
      */
     public void end(Context ctx) throws Exception
     {
-        // add new created items to index, unless they were deleted.
+        
-        for (Iterator ii = objectsCreated.iterator(); ii.hasNext();)
+        if(objectsToUpdate != null && handlesToDelete != null)
         {
-            DSpaceObject ic = (DSpaceObject) ii.next();
-            if (ic.getType() != Constants.ITEM || ((Item) ic).isArchived())
-            {
-                // if handle is NOT in list of deleted objects, index it:
-                String hdl = ic.getHandle();
-                if (hdl != null && !handlesToDelete.contains(hdl))
-                {
-                    try
-                    {
-                        DSIndexer.indexContent(ctx, ic);
-                        if (log.isDebugEnabled())
-                            log.debug("Indexed NEW "
-                                    + Constants.typeText[ic.getType()]
-                                    + ", id=" + String.valueOf(ic.getID())
-                                    + ", handle=" + hdl);
-                    }
-                    catch (Exception e)
-                    {
-                        log.error("Failed while indexing new object: ", e);
-                        objectsCreated = null;
-                        objectsToUpdate = null;
-                        handlesToDelete = null;
-                    }
-                }
-            }
-            // remove it from modified list since we just indexed it.
-            objectsToUpdate.remove(ic);
-        }
          
             // update the changed Items not deleted because they were on create list
             for (Iterator ii = objectsToUpdate.iterator(); ii.hasNext();)
@@ -230,19 +190,16 @@ public class SearchConsumer implements Consumer
                     {
                         try
                         {
-                        DSIndexer.reIndexContent(ctx, iu);
+                            DSIndexer.indexContent(ctx, iu);
                             if (log.isDebugEnabled())
-                            log.debug("RE-Indexed "
+                                log.debug("Indexed "
                                         + Constants.typeText[iu.getType()]
                                         + ", id=" + String.valueOf(iu.getID())
                                         + ", handle=" + hdl);
                         }
                         catch (Exception e)
                         {
-                        log.error("Failed while RE-indexing object: ", e);
+                            log.error("Failed while indexing object: ", e);
-                        objectsCreated = null;
-                        objectsToUpdate = null;
-                        handlesToDelete = null;
                         }
                     }
                 }
@@ -260,15 +217,13 @@ public class SearchConsumer implements Consumer
                 catch (Exception e)
                 {
                     log.error("Failed while UN-indexing object: " + hdl, e);
-                objectsCreated = new HashSet();
+                }
-                objectsToUpdate = new HashSet();
+
-                handlesToDelete = new HashSet();
             }
 
         }
         
         // "free" the resources
-        objectsCreated = null;
         objectsToUpdate = null;
         handlesToDelete = null;
     }