Improve logic for async RPA approval processing

Author: elmiomar

pom.xml

@@ -29,6 +29,8 @@
         <sonar.jacoco.reportPath>${coverage.reports.dir}/jacoco-ut.exec</sonar.jacoco.reportPath>
         <sonar.jacoco.itReportPath>${coverage.reports.dir}/jacoco-it.exec</sonar.jacoco.itReportPath>
         <sonar.host.url>http://oardev3.nist.gov:9000/sonarqube</sonar.host.url>
+        <surefireArgLine></surefireArgLine>
+        <failsafeArgLine></failsafeArgLine>
     </properties>
 
     <dependencyManagement>
@@ -279,15 +281,16 @@
                 <artifactId>maven-surefire-plugin</artifactId>
 
                 <configuration>
-                    <argLine>${surefireArgLine}</argLine>
-                    <argLine>-Djava.io.tmpdir=${basedir}/target/tmp</argLine>
+                    <!-- Mockito inline needs instrumentation; preload Byte Buddy instead of relying on JDK self-attach. -->
+                    <argLine>${surefireArgLine} -javaagent:${settings.localRepository}/net/bytebuddy/byte-buddy-agent/${byte-buddy.version}/byte-buddy-agent-${byte-buddy.version}.jar -Djava.io.tmpdir=${project.build.directory}</argLine>
                     <includes>
                         <include>**/*Test.java</include>
                     </includes>
                     <systemPropertyVariables>
                         <basedir>${project.basedir}</basedir>
                     </systemPropertyVariables>
-                    <forkCount>0</forkCount>
+                    <forkCount>1</forkCount>
+                    <reuseForks>true</reuseForks>
                     <systemPropertyVariables>
                         <conf.path>${user.home}/spm</conf.path>
                         <project.test.resourceDirectory>${project.basedir}/src/test/resources</project.test.resourceDirectory>

src/main/java/gov/nist/oar/distrib/cachemgr/CacheExpiryCheck.java

@@ -21,7 +21,7 @@ public CacheExpiryCheck(StorageInventoryDB inventoryDB) {
     /**
      * Checks if a cache object is expired and removes it from the cache if it is.
      * The method uses the {@code expires} metadata field to determine the expiration status.
-     * The expiration time is calculated based on the {@code LastModified} time plus the {@code expires} duration.
+     * The expiration time is stored as an absolute epoch millisecond value.
      * If the current time is past the calculated expiry time, the object is removed from the inventory database.
      *
      * @param co The cache object to check for expiration.
@@ -36,17 +36,11 @@ public void check(CacheObject co) throws IntegrityException, StorageVolumeExcept
         }
 
         if (co.hasMetadatum("expires")) {
-            long expiresDuration = co.getMetadatumLong("expires", -1L);
-            if (expiresDuration == -1L) {
+            long expiryTime = co.getMetadatumLong("expires", -1L);
+            if (expiryTime == -1L) {
                 throw new IntegrityException("Invalid 'expires' metadata value");
             }
 
-            long lastModified = co.getLastModified();
-            if (lastModified == -1L) {
-                throw new IntegrityException("CacheObject 'lastModified' time not available");
-            }
-
-            long expiryTime = lastModified + expiresDuration;
             long currentTime = Instant.now().toEpochMilli();
 
             // Check if the object is expired

src/main/java/gov/nist/oar/distrib/service/rpa/EmailInfoProvider.java

@@ -7,6 +7,7 @@
 
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 import java.util.AbstractMap;
 import java.util.List;
 import java.util.Locale;
@@ -21,7 +22,6 @@
 public class EmailInfoProvider {
 
     private static final String DATE_PATTERN = "EEEE, MM/dd/yyyy 'at' hh:mm a z";
-    private static final int EXPIRATION_DAYS = 14;
     private final RPAConfiguration rpaConfiguration;
 
     /**
@@ -208,7 +208,7 @@ private Map<String, String> getNamedPlaceholders(Record record, String downloadU
      */
     private String getExpirationDate() {
         DateTimeFormatter formatter = DateTimeFormatter.ofPattern(DATE_PATTERN, Locale.ENGLISH);
-        ZonedDateTime date = ZonedDateTime.now().plusDays(EXPIRATION_DAYS);
+        ZonedDateTime date = ZonedDateTime.now().plus(rpaConfiguration.getExpiresAfterMillis(), ChronoUnit.MILLIS);
         return formatter.format(date);
     }
 }

src/main/java/gov/nist/oar/distrib/service/rpa/HttpURLConnectionRPARequestHandlerService.java

@@ -713,6 +713,7 @@ public RecordUpdateResult updateRecord(String recordId, String status, String sm
 
         // Return result for async processing
         // Note: Email notifications and caching are handled asynchronously
+        record.getUserInfo().setApprovalStatus(approvalStatus);
         return new RecordUpdateResult(recordStatus, record, datasetId);
     }
 
@@ -755,6 +756,7 @@ private void handleApprovalPostProcessing(Record record, String datasetId)
 
             if (randomId == null) {
                 LOGGER.error("Caching process returned a null randomId for dataset: {}", datasetId);
+                patchFailureStatus(record);
                 this.recordResponseHandler.onFailure(record);
                 return;
             }
@@ -763,14 +765,23 @@ private void handleApprovalPostProcessing(Record record, String datasetId)
             String currentStatus = record.getUserInfo().getApprovalStatus();
             String finalStatus = updateApprovalStatusWithRandomId(currentStatus, randomId);
             patchRecordStatus(record.getId(), finalStatus);
+            record.getUserInfo().setApprovalStatus(finalStatus);
 
             // Send success email with download link
             this.recordResponseHandler.onRecordUpdateApproved(record, randomId);
             LOGGER.info("Approval post-processing completed for record: {}", record.getId());
 
         } catch (Exception e) {
             LOGGER.error("Approval post-processing failed for record {}: {}", record.getId(), e.getMessage(), e);
-            // Send failure notification to user
+            if (randomId != null) {
+                try {
+                    this.rpaDatasetCacher.uncache(randomId);
+                } catch (Exception uncacheException) {
+                    LOGGER.error("Failed to uncache dataset after approval failure (random ID {}): {}",
+                            randomId, uncacheException.getMessage(), uncacheException);
+                }
+            }
+            patchFailureStatus(record);
             this.recordResponseHandler.onFailure(record);
         }
     }
@@ -799,14 +810,25 @@ private void handleDeclinePostProcessing(Record record) {
         LOGGER.info("Decline post-processing completed for record: {}", record.getId());
     }
 
+    private void patchFailureStatus(Record record) {
+        String failureStatus = updateApprovalStatusPrefix(record.getUserInfo().getApprovalStatus(), "ApprovalFailed");
+        try {
+            patchRecordStatus(record.getId(), failureStatus);
+            record.getUserInfo().setApprovalStatus(failureStatus);
+        } catch (RequestProcessingException e) {
+            LOGGER.error("Failed to update approval failure status for record {}: {}",
+                    record.getId(), e.getMessage(), e);
+        }
+    }
+
     /**
      * Updates the approval status string to include the random ID.
      * Converts "ApprovalPending_timestamp_smeId" to "Approved_timestamp_smeId_randomId"
      */
     private String updateApprovalStatusWithRandomId(String currentStatus, String randomId) {
         // Replace "ApprovalPending" with "Approved" and append randomId
         if (currentStatus != null && currentStatus.startsWith("ApprovalPending_")) {
-            return currentStatus.replace("ApprovalPending_", "Approved_") + "_" + randomId;
+            return updateApprovalStatusPrefix(currentStatus, "Approved") + "_" + randomId;
         }
         // Fallback: if already "Approved_", just append randomId
         if (currentStatus != null && currentStatus.startsWith("Approved_") && !currentStatus.contains("_" + randomId)) {
@@ -815,6 +837,13 @@ private String updateApprovalStatusWithRandomId(String currentStatus, String ran
         return currentStatus;
     }
 
+    private String updateApprovalStatusPrefix(String currentStatus, String newPrefix) {
+        if (currentStatus != null && currentStatus.contains("_")) {
+            return newPrefix + currentStatus.substring(currentStatus.indexOf("_"));
+        }
+        return newPrefix + "_" + Instant.now().toString();
+    }
+
     /**
      * Sends a PATCH request to update the record status in Salesforce.
      */

src/main/java/gov/nist/oar/distrib/service/rpa/RecordResponseHandlerImpl.java

@@ -158,7 +158,12 @@ public void onRecordUpdateApproved(Record record, String randomId) throws Invali
      */
     @Override
     public void onRecordUpdateDeclined(Record record) throws InvalidRequestException, RequestProcessingException {
-        LOGGER.debug("User was declined by SME");
+        LOGGER.debug("User was declined by SME. Sending decline notification...");
+        if (this.emailSender.sendDeclinedEmailToEndUser(record)) {
+            LOGGER.debug("Decline notification email sent successfully (RecordID=" + record.getId() + ")");
+        } else {
+            throw new RequestProcessingException("Failed to send decline notification email to end user");
+        }
     }
 
     /**

src/main/java/gov/nist/oar/distrib/web/RPAAsyncExecutor.java

@@ -37,7 +37,8 @@ public void handleAfterRecordCreationAsync(RecordWrapper wrapper, UserInfoWrappe
         try {
             handler.handleAfterRecordCreation(wrapper, input, code);
         } catch (RequestProcessingException e) {
-            LOGGER.error("Async post-processing failed for record ID {}: {}", wrapper.getRecord().getId(),
+            String recordId = wrapper != null && wrapper.getRecord() != null ? wrapper.getRecord().getId() : "unknown";
+            LOGGER.error("Async post-processing failed for record ID {}: {}", recordId,
                     e.getMessage(), e);
         }
     }

src/main/java/gov/nist/oar/distrib/web/RPAConfiguration.java

@@ -61,7 +61,7 @@ public class RPAConfiguration {
     private Map<String, BlacklistConfig> blacklists = new HashMap<>();
 
     @JsonProperty("expiresAfterMillis")
-    long expiresAfterMillis = 0L;
+    long expiresAfterMillis = 1209600000L;
 
     @JsonProperty("supportEmail")
     private String supportEmail;

src/test/java/gov/nist/oar/distrib/cachemgr/CacheExpiryCheckTest.java

@@ -1,6 +1,5 @@
 package gov.nist.oar.distrib.cachemgr;
 
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -32,8 +31,8 @@ public void setUp() {
 
     /**
      * Test to verify that {@link CacheExpiryCheck} correctly identifies and processes an expired cache object.
-     * An object is considered expired based on the `expires` metadata, which defines the duration after which
-     * an object should be considered expired from the time of its last modification. This test ensures that an object
+     * An object is considered expired based on the `expires` metadata, which defines the absolute epoch
+     * millisecond time when the object should expire. This test ensures that an object
      * past its expiration is appropriately removed from the inventory database.
      *
      * @throws Exception to handle any exceptions thrown during the test execution
@@ -43,8 +42,7 @@ public void testExpiredObjectRemoval() throws Exception {
         // Setup an expired cache object
         cacheObject.volume = mockVolume;
         when(cacheObject.hasMetadatum("expires")).thenReturn(true);
-        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(1000L); // Expires in 1 second
-        when(cacheObject.getLastModified()).thenReturn(Instant.now().minusSeconds(10).toEpochMilli());
+        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(Instant.now().minusSeconds(1).toEpochMilli());
         when(cacheObject.volume.remove(cacheObject.name)).thenReturn(true);
 
         expiryCheck.check(cacheObject);
@@ -55,7 +53,7 @@ public void testExpiredObjectRemoval() throws Exception {
 
     /**
      * Test to ensure that {@link CacheExpiryCheck} does not flag a cache object as expired if the current time has not
-     * exceeded its `expires` duration since its last modification. This test verifies that no removal action is taken
+     * exceeded its absolute `expires` time. This test verifies that no removal action is taken
      * for such non-expired objects.
      *
      * @throws Exception to handle any exceptions thrown during the test execution
@@ -66,9 +64,7 @@ public void testNonExpiredObject() throws Exception {
         cacheObject.name = "nonExpiredObject";
         cacheObject.volname = "testVolume";
         when(cacheObject.hasMetadatum("expires")).thenReturn(true);
-        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(14 * 24 * 60 * 60 * 1000L); // 14 days in milliseconds
-        long lastModified = System.currentTimeMillis() - (7 * 24 * 60 * 60 * 1000L); // 7 days ago, within expiry period
-        when(cacheObject.getLastModified()).thenReturn(lastModified);
+        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(System.currentTimeMillis() + (7 * 24 * 60 * 60 * 1000L));
 
         // Perform the check
         expiryCheck.check(cacheObject);
@@ -106,7 +102,6 @@ public void testNonExpiredObject_NoRemoval() throws Exception {
         // Setup a non-expired cache object
         when(cacheObject.hasMetadatum("expires")).thenReturn(true);
         when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(System.currentTimeMillis() + 10000L); // Expires in the future
-        when(cacheObject.getLastModified()).thenReturn(System.currentTimeMillis());
 
         expiryCheck.check(cacheObject);
 
@@ -115,22 +110,20 @@ public void testNonExpiredObject_NoRemoval() throws Exception {
     }
 
     /**
-     * Tests that an {@link IntegrityException} is thrown when a cache object has the {@code expires} metadata
-     * but lacks a valid {@code lastModified} time.
+     * Tests that lastModified is not required because {@code expires} is stored as an absolute timestamp.
      *
      * @throws Exception to handle any exceptions thrown during the test execution
      */
     @Test
-    public void testObjectWithExpiresButNoLastModified_ThrowsException() throws Exception {
+    public void testObjectWithExpiresButNoLastModified_UsesAbsoluteExpires() throws Exception {
         cacheObject.name = "objectWithNoLastModified";
         cacheObject.volname = "testVolume";
         when(cacheObject.hasMetadatum("expires")).thenReturn(true);
-        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(1000L); // Expires in 1 second
-        when(cacheObject.getLastModified()).thenReturn(-1L); // Last modified not available
+        when(cacheObject.getMetadatumLong("expires", -1L)).thenReturn(System.currentTimeMillis() + 10000L);
 
-        assertThrows(IntegrityException.class, () -> {
-            expiryCheck.check(cacheObject);
-        });
+        expiryCheck.check(cacheObject);
+
+        verify(mockInventoryDB, never()).removeObject(anyString(), anyString());
     }
 
     /**