improved logging; lint fixes; corrected typos

nwoodward · nwoodward · commit 0982f65214c6 · 2026-06-12T14:37:56.000-05:00
diff --git a/src/main/java/org/dspace/ctask/replicate/BagItReplicateConsumer.java b/src/main/java/org/dspace/ctask/replicate/BagItReplicateConsumer.java
@@ -62,7 +62,6 @@
  * @author richardrodgers
  */
 public class BagItReplicateConsumer implements Consumer {
-
     private Logger log = LogManager.getLogger();
 
     private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
@@ -175,10 +174,9 @@ public void consume(Context ctx, Event event) throws Exception {
             case MODIFY_METADATA: // MODIFY_METADATA = just modify an object's metadata
                 // If subject of event is null, this means the object was likely deleted
                 if (event.getSubject(ctx) == null) {
-                    log.warn(event.getEventTypeAsString() + " event, could not get object for "
-                            + event.getSubjectTypeAsString() + " id="
-                            + String.valueOf(event.getSubjectID())
-                            + ", perhaps it has been deleted.");
+                    log.warn("{} event, could not get object for {} id={}, perhaps it has been deleted.",
+                        event.getEventTypeAsString(), event.getSubjectTypeAsString(),
+                        String.valueOf(event.getSubjectID()));
                     break;
                 }
 
@@ -243,7 +241,7 @@ public void end(Context ctx) throws Exception {
             taskPMap.clear();
         }
 
-        // if there any uncommitted deletions, record them now
+        // if there are any uncommitted deletions, record them now
         if (delObjId != null) {
             if (delTasks != null) {
                 entrySet.add(new TaskQueueEntry(name, stamp, delTasks, delObjId));
@@ -362,7 +360,7 @@ private void processDelete(Context context) throws IOException {
                     String catID = repMan.deletionCatalogId(delObjId, null);
                     File packDir = repMan.stage(context, deleteGroupName, catID);
                     File archive = packer.pack(packDir);
-                    // System.out.println("delcat about to transfer");
+                    // System.out.println("deleted catalog about to transfer");
                     repMan.transferObject(deleteGroupName, archive);
                 }
             } catch (AuthorizeException | SQLException e) {
@@ -394,7 +392,7 @@ private boolean loadIdFilter(String filterName) {
                 }
                 return true;
             } catch (IOException ioE) {
-                // log.error("Unable to read filter file '" + filterName + "'");
+                // log.error("Unable to read filter file '{}'", filterName);
                 idFilter = null;
             } finally {
                 if (reader != null) {
diff --git a/src/main/java/org/dspace/ctask/replicate/CompareWithAIP.java b/src/main/java/org/dspace/ctask/replicate/CompareWithAIP.java
@@ -14,6 +14,8 @@
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.Collection;
 import org.dspace.content.Community;
@@ -51,6 +53,8 @@
  */
 @Suspendable(invoked = Curator.Invoked.INTERACTIVE)
 public class CompareWithAIP extends AbstractCurationTask {
+    private final Logger log = LogManager.getLogger();
+
     private String archFmt;
     private int status = Curator.CURATE_UNSET;
     private String result = null;
@@ -88,7 +92,10 @@ public int perform(DSpaceObject dso) throws IOException {
                 File archive = packer.pack(packDir);
                 String chkSum = Utils.checksum(archive, "MD5");
                 // remove local archive file -- it's no longer needed
-                archive.delete();
+                //boolean successful = archive.delete();
+                //if (!successful) {
+                //    log.warn("Failed to delete archive {}", archive.getAbsolutePath());
+                //}
 
                 // compare with replica
                 String repChkSum = repMan.objectAttribute(storeGroupName, objId, "checksum");
@@ -133,25 +140,25 @@ private void auditExtent(Context context, ReplicaManager repMan, DSpaceObject ds
 
         // If container is a Collection, make sure all Items have AIPs in remote storage
         if (Constants.COLLECTION == type) {
-            Collection coll = (Collection)dso;
-            Iterator<Item> iter = itemService.findByCollection(context, coll);
+            Collection collection = (Collection)dso;
+            Iterator<Item> iter = itemService.findByCollection(context, collection);
             while (iter.hasNext()) {
                 checkReplica(context, repMan, iter.next());
             }
         } else if (Constants.COMMUNITY == type) {
-            // If Community, make sure all Sub-Communities/Collections have AIPs in remote storage
-            Community comm = (Community)dso;
-            for (Community subcomm : comm.getSubcommunities()) {
-                checkReplica(context, repMan, subcomm);
+            // If it is a Community, make sure all Sub-Communities/Collections have AIPs in remote storage
+            Community community = (Community)dso;
+            for (Community subCommunity : community.getSubcommunities()) {
+                checkReplica(context, repMan, subCommunity);
             }
-            for (Collection coll : comm.getCollections()) {
-                checkReplica(context, repMan, coll);
+            for (Collection collection : community.getCollections()) {
+                checkReplica(context, repMan, collection);
             }
         } else if (Constants.SITE == type) {
-            // If Site, check to see all Top-Level Communities have an AIP in remote storage
-            List<Community> topComm = communityService.findAllTop(context);
-            for (Community comm : topComm) {
-                checkReplica(context, repMan, comm);
+            // If it is a Site, check to see all Top-Level Communities have an AIP in remote storage
+            List<Community> topCommunities = communityService.findAllTop(context);
+            for (Community topCommunity : topCommunities) {
+                checkReplica(context, repMan, topCommunity);
             }
         }
     }
diff --git a/src/main/java/org/dspace/ctask/replicate/FetchAIP.java b/src/main/java/org/dspace/ctask/replicate/FetchAIP.java
@@ -63,7 +63,6 @@ public int perform(DSpaceObject dso) throws IOException {
         }
     }
 
-
     /**
      * Perform the 'Fetch AIP' task
      * @param ctx DSpace Context
diff --git a/src/main/java/org/dspace/ctask/replicate/FilteredFileTaskQueue.java b/src/main/java/org/dspace/ctask/replicate/FilteredFileTaskQueue.java
@@ -41,8 +41,6 @@
  * @author Tim Donohue
  */
 public class FilteredFileTaskQueue extends FileTaskQueue {
-    private static Logger log = LogManager.getLogger();
-
     /**
      * Returns the set of UNIQUE task entries from the named queue. Any duplicate
      * task entries in the queue are ignored. The operation locks
diff --git a/src/main/java/org/dspace/ctask/replicate/METSReplicateConsumer.java b/src/main/java/org/dspace/ctask/replicate/METSReplicateConsumer.java
@@ -82,7 +82,6 @@
  * @author richardrodgers
  */
 public class METSReplicateConsumer implements Consumer {
-
     private Logger log = LogManager.getLogger();
 
     private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
@@ -255,10 +254,9 @@ public void consume(Context ctx, Event event) throws Exception {
                 case MODIFY_METADATA: // MODIFY_METADATA = just modify an object's metadata
                     // If subject of event is null, this means the object was likely deleted
                     if (event.getSubject(ctx) == null) {
-                        log.warn(event.getEventTypeAsString() + " event, could not get object for "
-                                + event.getSubjectTypeAsString() + " id="
-                                + String.valueOf(event.getSubjectID())
-                                + ", perhaps it has been deleted.");
+                        log.warn("{} event, could not get object for {} id={}, perhaps it has been deleted.",
+                            event.getEventTypeAsString(), event.getSubjectTypeAsString(),
+                            String.valueOf(event.getSubjectID()));
                         break;
                     }
 
@@ -329,7 +327,7 @@ public void end(Context ctx) throws Exception {
             taskPMap.clear();
         }
 
-        // if there any uncommitted deletions, record them now
+        // if there are any uncommitted deletions, record them now
         if (delObjId != null) {
             if (delTasks != null) {
                 entrySet.add(new TaskQueueEntry(name, stamp, delTasks, delObjId));
@@ -566,7 +564,7 @@ private void parseTasks(String propName) {
                 }
             } else {
                 // Otherwise (if the task ends in "+p"),
-                //  it should be added to the list of tasks to perform immediately
+                // it should be added to the list of tasks to perform immediately
                 String sTask = task.substring(0, task.lastIndexOf("+p"));
                 if ("add".equals(propName)) {
                     if (addPTasks == null) {
diff --git a/src/main/java/org/dspace/ctask/replicate/METSRestoreFromAIP.java b/src/main/java/org/dspace/ctask/replicate/METSRestoreFromAIP.java
@@ -93,7 +93,10 @@ public int perform(Context ctx, String id) throws IOException {
                 // remove the deletion catalog (as the object is now restored)
                 repMan.removeObject(deleteGroupName, catId);
                 // remove from local cache as well
-                catArchive.delete();
+                boolean successful = catArchive.delete();
+                if (!successful) {
+                    log.warn("Unable to delete object: {}", catArchive);
+                }
             }
 
             result = getSuccessMsg(id, pkgParams);
@@ -146,12 +149,15 @@ private void restoreObject(Context context, ReplicaManager repMan, File archive,
 
             // Remove the locally cached archive file - it is no longer needed.
             if (archive.exists()) {
-                archive.delete();
+                boolean successful = archive.delete();
+                if (!successful) {
+                    log.warn("Unable to delete archived file: {}", archive);
+                }
             }
 
             // check if recursiveMode is enabled (restore/replace multiple objects)
             if (pkgParams.recursiveModeEnabled()) {
-                // See if this package refered to child packages,
+                // See if this package referred to child packages,
                 // if so, we want to also replace those child objects
                 List<String> childPkgRefs = packer.getChildPackageRefs();
                 if (childPkgRefs != null && !childPkgRefs.isEmpty()) {
diff --git a/src/main/java/org/dspace/ctask/replicate/MoveToTrashSingleAIP.java b/src/main/java/org/dspace/ctask/replicate/MoveToTrashSingleAIP.java
@@ -37,14 +37,13 @@
  */
 @Distributive
 public class MoveToTrashSingleAIP extends AbstractCurationTask {
+    private static Logger log = LogManager.getLogger();
+
     // Source and destination group where AIP will be moved to
     private String srcGroupName;
     private String destGroupName;
-
     private String archFmt;
 
-    private static Logger log = LogManager.getLogger();
-
     @Override
     public void init(Curator curator, String taskId) throws IOException {
         super.init(curator, taskId);
diff --git a/src/main/java/org/dspace/ctask/replicate/TransmitAIP.java b/src/main/java/org/dspace/ctask/replicate/TransmitAIP.java
@@ -12,6 +12,8 @@
 import java.io.IOException;
 import java.sql.SQLException;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.DSpaceObject;
 import org.dspace.core.Context;
@@ -42,12 +44,15 @@
  */
 @Suspendable(invoked = Curator.Invoked.INTERACTIVE)
 public class TransmitAIP extends AbstractCurationTask {
+    private static final Logger log = LogManager.getLogger();
+
     // Group where all AIPs will be stored
     private String storeGroupName;
 
     @Override
     public void init(Curator curator, String taskId) throws IOException {
         super.init(curator, taskId);
+        log.info("Init TransmitAIP");
         storeGroupName = configurationService.getProperty("replicate.group.aip.name");
     }
 
diff --git a/src/main/java/org/dspace/ctask/replicate/checkm/TransmitManifest.java b/src/main/java/org/dspace/ctask/replicate/checkm/TransmitManifest.java
@@ -102,7 +102,9 @@ public int perform(DSpaceObject dso) throws IOException {
                 manFile = siteManifest(context, repMan, (Site)dso);
             }
 
-            repMan.transferObject(manifestGroupName, manFile);
+            if (manFile != null) {
+                repMan.transferObject(manifestGroupName, manFile);
+            }
         } catch (SQLException sqlE) {
             throw new IOException(sqlE);
         }
@@ -219,7 +221,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti
          // Manifests stored as text files
         String filename = repMan.storageId(context, coll.getHandle(), MANIFEST_EXTENSION);
 
-        log.debug("Creating manifest for: " + coll.getHandle());
+        log.debug("Creating manifest for collection: {}", coll.getHandle());
 
         //Create Collection manifest
         File manFile = repMan.stage(context, manifestGroupName, filename);
@@ -241,7 +243,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti
         }
 
         writer.close();
-        report("Created manifest for: " + coll.getHandle());
+        report("Created manifest for collection: " + coll.getHandle());
         return manFile;
     }
 
@@ -258,7 +260,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti
     private File itemManifest(Context context, ReplicaManager repMan, Item item) throws IOException, SQLException {
         String filename = repMan.storageId(context, item.getHandle(), MANIFEST_EXTENSION);
 
-        log.debug("Creating manifest for: " + item.getHandle());
+        log.debug("Creating manifest for item: {}", item.getHandle());
 
         //Create Item manifest
         File manFile = repMan.stage(context, manifestGroupName, filename);
@@ -319,7 +321,7 @@ private File itemManifest(Context context, ReplicaManager repMan, Item item) thr
         }
 
         writer.close();
-        report("Created manifest for: " + item.getHandle());
+        report("Created manifest for item: " + item.getHandle());
         return manFile;
     }
 
diff --git a/src/main/java/org/dspace/ctask/replicate/store/LocalObjectStore.java b/src/main/java/org/dspace/ctask/replicate/store/LocalObjectStore.java
@@ -9,7 +9,11 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.dspace.ctask.replicate.ObjectStore;
 import org.dspace.curate.Utils;
 import org.dspace.services.ConfigurationService;
@@ -30,7 +34,9 @@
  * @author richardrodgers
  */
 public class LocalObjectStore implements ObjectStore {
-    private ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
+    private final Logger log = LogManager.getLogger(LocalObjectStore.class);
+
+    private final ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
 
     // where replicas are kept
     protected String storeDir = null;
@@ -43,9 +49,13 @@ public LocalObjectStore() {
     public void init() throws IOException {
         storeDir = configurationService.getProperty("replicate.store.dir");
 
-        File storeFile = new File(storeDir);
-        if (! storeFile.exists()) {
-            storeFile.mkdirs();
+        Path storePath = Path.of(storeDir);
+        try {
+            Files.createDirectories(storePath);
+            log.info("Successfully created replicate store directory: {}", storePath);
+        } catch (IOException e) {
+            log.warn("Failed to create replicate.store.dir: {}", e.getMessage());
+            throw e;
         }
     }
 
@@ -76,7 +86,10 @@ public long removeObject(String group, String id) {
         File remFile = new File(storeDir + File.separator + group, id);
         if (remFile.exists()) {
             size = remFile.length();
-            remFile.delete();
+            boolean successful = remFile.delete();
+            if (!successful) {
+                log.warn("Cannot delete object: '{}'.", remFile);
+            }
         }
         return size;
     }
diff --git a/src/main/java/org/dspace/ctask/replicate/store/MountableObjectStore.java b/src/main/java/org/dspace/ctask/replicate/store/MountableObjectStore.java
@@ -39,7 +39,10 @@ public long transferObject(String group, File file) throws IOException {
 
         File archFile = new File(storeDir + File.separator + group, file.getName());
         if (archFile.exists()) {
-            archFile.delete();
+            boolean successful = archFile.delete();
+            if (!successful) {
+                System.out.println("Cannot delete archive file: " +file.getAbsolutePath());
+            }
         }
 
         Utils.copy(file, archFile);
diff --git a/src/main/java/org/dspace/pack/mets/METSPacker.java b/src/main/java/org/dspace/pack/mets/METSPacker.java

Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,6 @@ public int perform(DSpaceObject dso) throws IOException {`
`63`	`63`	`}`
`64`	`64`	`}`
`65`	`65`
`66`		`-`
`67`	`66`	`/**`
`68`	`67`	`* Perform the 'Fetch AIP' task`
`69`	`68`	`* @param ctx DSpace Context`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,9 @@ public int perform(DSpaceObject dso) throws IOException {`
`102`	`102`	`manFile = siteManifest(context, repMan, (Site)dso);`
`103`	`103`	`}`
`104`	`104`
`105`		`- repMan.transferObject(manifestGroupName, manFile);`
	`105`	`+ if (manFile != null) {`
	`106`	`+ repMan.transferObject(manifestGroupName, manFile);`
	`107`	`+ }`
`106`	`108`	`} catch (SQLException sqlE) {`
`107`	`109`	`throw new IOException(sqlE);`
`108`	`110`	`}`
`@@ -219,7 +221,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti`
`219`	`221`	`// Manifests stored as text files`
`220`	`222`	`String filename = repMan.storageId(context, coll.getHandle(), MANIFEST_EXTENSION);`
`221`	`223`
`222`		`- log.debug("Creating manifest for: " + coll.getHandle());`
	`224`	`+ log.debug("Creating manifest for collection: {}", coll.getHandle());`
`223`	`225`
`224`	`226`	`//Create Collection manifest`
`225`	`227`	`File manFile = repMan.stage(context, manifestGroupName, filename);`
`@@ -241,7 +243,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti`
`241`	`243`	`}`
`242`	`244`
`243`	`245`	`writer.close();`
`244`		`- report("Created manifest for: " + coll.getHandle());`
	`246`	`+ report("Created manifest for collection: " + coll.getHandle());`
`245`	`247`	`return manFile;`
`246`	`248`	`}`
`247`	`249`
`@@ -258,7 +260,7 @@ private File collectionManifest(Context context, ReplicaManager repMan, Collecti`
`258`	`260`	`private File itemManifest(Context context, ReplicaManager repMan, Item item) throws IOException, SQLException {`
`259`	`261`	`String filename = repMan.storageId(context, item.getHandle(), MANIFEST_EXTENSION);`
`260`	`262`
`261`		`- log.debug("Creating manifest for: " + item.getHandle());`
	`263`	`+ log.debug("Creating manifest for item: {}", item.getHandle());`
`262`	`264`
`263`	`265`	`//Create Item manifest`
`264`	`266`	`File manFile = repMan.stage(context, manifestGroupName, filename);`
`@@ -319,7 +321,7 @@ private File itemManifest(Context context, ReplicaManager repMan, Item item) thr`
`319`	`321`	`}`
`320`	`322`
`321`	`323`	`writer.close();`
`322`		`- report("Created manifest for: " + item.getHandle());`
	`324`	`+ report("Created manifest for item: " + item.getHandle());`
`323`	`325`	`return manFile;`
`324`	`326`	`}`
`325`	`327`