Fix DS-2593 : Withdrawn items are now given a "tombstone" in OAI-PMH. Also fix Context mgmt issues & authorization code.

dspace-oai/src/main/java/org/dspace/xoai/app/XOAI.java

@@ -167,10 +167,11 @@ public class XOAI {
         System.out
                 .println("Incremental import. Searching for documents modified after: "
                         + last.toString());
-
+        // Index both in_archive items AND withdrawn items. Withdrawn items will be flagged withdrawn
-        String sqlQuery = "SELECT item_id FROM item WHERE in_archive=TRUE AND discoverable=TRUE AND last_modified > ?";
+        // (in order to notify external OAI harvesters of their new status)
+        String sqlQuery = "SELECT item_id FROM item WHERE (in_archive=TRUE OR withdrawn=TRUE) AND discoverable=TRUE AND last_modified > ?";
         if(DatabaseManager.isOracle()){
-                sqlQuery = "SELECT item_id FROM item WHERE in_archive=1 AND discoverable=1 AND last_modified > ?";
+                sqlQuery = "SELECT item_id FROM item WHERE (in_archive=1 OR withdrawn=1) AND discoverable=1 AND last_modified > ?";
         }
 
         try {
@@ -187,10 +188,11 @@ public class XOAI {
     private int indexAll() throws DSpaceSolrIndexerException {
         System.out.println("Full import");
         try {
-
+            // Index both in_archive items AND withdrawn items. Withdrawn items will be flagged withdrawn
-            String sqlQuery = "SELECT item_id FROM item WHERE in_archive=TRUE AND discoverable=TRUE";
+            // (in order to notify external OAI harvesters of their new status)
+            String sqlQuery = "SELECT item_id FROM item WHERE (in_archive=TRUE OR withdrawn=TRUE) AND discoverable=TRUE";
             if(DatabaseManager.isOracle()){
-                sqlQuery = "SELECT item_id FROM item WHERE in_archive=1 AND discoverable=1";
+                sqlQuery = "SELECT item_id FROM item WHERE (in_archive=1 OR withdrawn=1) AND discoverable=1";
             }
 
             TableRowIterator iterator = DatabaseManager.query(context,
@@ -287,17 +289,14 @@ public class XOAI {
     }
 
     private boolean isPublic(Item item) {
+        boolean pub = false;
         try {
-            AuthorizeManager.authorizeAction(context, item, Constants.READ);
+            //Check if READ access allowed on this Item
-            for (Bundle b : item.getBundles())
+            pub = AuthorizeManager.authorizeActionBoolean(context, item, Constants.READ);
-                AuthorizeManager.authorizeAction(context, b, Constants.READ);
-            return true;
-        } catch (AuthorizeException ex) {
-            log.debug(ex.getMessage());
         } catch (SQLException ex) {
             log.error(ex.getMessage());
         }
-        return false;
+        return pub;
     }
 
 
@@ -355,6 +354,8 @@ public class XOAI {
         XOAICacheService cacheService = applicationContext.getBean(XOAICacheService.class);
         XOAIItemCacheService itemCacheService = applicationContext.getBean(XOAIItemCacheService.class);
 
+        Context ctx = null;
+
         try {
             CommandLineParser parser = new PosixParser();
             Options options = new Options();
@@ -394,7 +395,7 @@ public class XOAI {
                 String command = line.getArgs()[0];
 
                 if (COMMAND_IMPORT.equals(command)) {
-                    Context ctx = new Context();
+                    ctx = new Context();
                     XOAI indexer = new XOAI(ctx,
                             line.hasOption('o'),
                             line.hasOption('c'),
@@ -404,21 +405,17 @@ public class XOAI {
 
                     int imported = indexer.index();
                     if (imported > 0) cleanCache(itemCacheService, cacheService);
-
-                    ctx.abort();
                 } else if (COMMAND_CLEAN_CACHE.equals(command)) {
                     cleanCache(itemCacheService, cacheService);
                 } else if (COMMAND_COMPILE_ITEMS.equals(command)) {
 
-                    Context ctx = new Context();
+                    ctx = new Context();
                     XOAI indexer = new XOAI(ctx, line.hasOption('v'));
                     applicationContext.getAutowireCapableBeanFactory().autowireBean(indexer);
 
                     indexer.compile();
 
                     cleanCache(itemCacheService, cacheService);
-
-                    ctx.abort();
                 } else if (COMMAND_ERASE_COMPILED_ITEMS.equals(command)) {
                     cleanCompiledItems(itemCacheService);
                     cleanCache(itemCacheService, cacheService);
@@ -436,6 +433,12 @@ public class XOAI {
             }
             log.error(ex.getMessage(), ex);
         }
+        finally
+        {
+            // Abort our context, if still open
+            if(ctx!=null && ctx.isValid())
+                ctx.abort();
+        }
     }
 
     private static void cleanCompiledItems(XOAIItemCacheService itemCacheService) throws IOException {

dspace-oai/src/main/java/org/dspace/xoai/controller/DSpaceOAIDataProvider.java

@@ -145,7 +145,7 @@ public class DSpaceOAIDataProvider
     }
 
     private void closeContext(Context context) {
-        if (context != null)
+        if (context != null && context.isValid())
             context.abort();
     }
 

dspace-oai/src/main/java/org/dspace/xoai/filter/DSpaceAuthorizationFilter.java

@@ -49,29 +49,33 @@ public class DSpaceAuthorizationFilter extends DSpaceFilter
     @Override
     public boolean isShown(DSpaceItem item)
     {
+        boolean pub = false;
         try
         {
-            String handle = DSpaceItem.parseHandle(item.getIdentifier());
+            // For DSpace, if an Item is withdrawn, "isDeleted()" will be true.
-            if (handle == null) return false;
+            // In this scenario, we want a withdrawn item to be *shown* so that
-            Item dspaceItem = (Item) HandleManager.resolveToObject(context, handle);
+            // we can properly respond with a "deleted" status via OAI-PMH.
-            AuthorizeManager.authorizeAction(context, dspaceItem, Constants.READ);
+            // Don't worry, this does NOT make the metadata public for withdrawn items,
-            for (Bundle b : dspaceItem.getBundles())
+            // it merely provides an item "tombstone" via OAI-PMH.
-                AuthorizeManager.authorizeAction(context, b, Constants.READ);
+            if (item.isDeleted())
                 return true;
-        }
+
-        catch (AuthorizeException ex)
+            // If Handle or Item are not found, return false
-        {
+            String handle = DSpaceItem.parseHandle(item.getIdentifier());
-            log.error(ex.getMessage(), ex);
+            if (handle == null)
+                return false;
+            Item dspaceItem = (Item) HandleManager.resolveToObject(context, handle);
+            if (dspaceItem == null)
+                return false;
+
+            // Check if READ access allowed on Item
+            pub = AuthorizeManager.authorizeActionBoolean(context, dspaceItem, Constants.READ);
         }
         catch (SQLException ex)
         {
             log.error(ex.getMessage(), ex);
         }
-        catch (Exception ex)
+        return pub;
-        {
-            log.error(ex.getMessage(), ex);
-        }
-        return false;
     }
 
     @Override