srm: Unpin files when aborting prepareToGet

Motivation:

The bring-online request handling has logic to abort a pin when
the request is aborted. The prepareToGet code is laking this logic.

Modification:

The primary difference is to call a catch-all unpin by ID and SURL
if the request fails before the pin has completed. This is a fairly
expensive call, but is only invoked in case of failures.

Also restructured the code slightly to make the two request implementations
more alike.

Result:

No lingering pins.

Target: trunk
Require-notes: no
Require-book: no
Acked-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Patch: https://rb.dcache.org/r/8557/

modules/srm-server/src/main/java/org/dcache/srm/request/BringOnlineFileRequest.java

@@ -153,18 +153,18 @@ public BringOnlineFileRequest(
     String pinId
     ) {
         super(id,
-        nextJobId,
-        creationTime,
-        lifetime,
-        stateId,
-        errorMessage,
-        scheduelerId,
-        schedulerTimeStamp,
-        numberOfRetries,
-        lastStateTransitionTime,
-        jobHistoryArray,
-        requestId,
-        statusCodeString);
+              nextJobId,
+              creationTime,
+              lifetime,
+              stateId,
+              errorMessage,
+              scheduelerId,
+              schedulerTimeStamp,
+              numberOfRetries,
+              lastStateTransitionTime,
+              jobHistoryArray,
+              requestId,
+              statusCodeString);
 
         this.surl = URI.create(SURL);
 
@@ -405,12 +405,10 @@ protected void stateChanged(State oldState) {
                 AbstractStorageElement storage = getStorage();
                 if (fileId != null && pinId != null) {
                     logger.info("State changed to final state, unpinning fileId = {} pinId = {}.", fileId, pinId);
-                    final CheckedFuture<String, ? extends SRMException> future =
-                            storage.unPinFile(user, fileId, pinId);
+                    CheckedFuture<String, ? extends SRMException> future = storage.unPinFile(user, fileId, pinId);
                     future.addListener(() -> {
                         try {
-                            String pinId1 = future.checkedGet();
-                            logger.debug("File unpinned (pinId={}).", pinId1);
+                            logger.debug("File unpinned (pinId={}).", future.checkedGet());
                         } catch (SRMException e) {
                             logger.error("Unpinning failed: {}", e.getMessage());
                         }
@@ -421,7 +419,6 @@ protected void stateChanged(State oldState) {
                 }
             } catch (SRMInternalErrorException | SRMInvalidRequestException ire) {
                 logger.error(ire.toString());
-                return;
             }
             break;
         }

modules/srm-server/src/main/java/org/dcache/srm/request/GetFileRequest.java

@@ -87,6 +87,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import org.dcache.srm.SRM;
 import org.dcache.srm.SRMException;
 import org.dcache.srm.SRMFileBusyException;
+import org.dcache.srm.SRMInternalErrorException;
 import org.dcache.srm.SRMInvalidRequestException;
 import org.dcache.srm.SRMUser;
 import org.dcache.srm.scheduler.IllegalStateTransition;
@@ -409,35 +410,39 @@ public void pinFile(ContainerRequest request)
     @Override
     protected void stateChanged(State oldState) {
         State state = getState();
-        logger.debug("State changed from " + oldState + " to " + getState());
-        if(state == State.READY) {
+        logger.debug("State changed from {} to {}", oldState, getState());
+        switch (state) {
+        case READY:
             try {
                 getContainerRequest().resetRetryDeltaTime();
             } catch (SRMInvalidRequestException ire) {
                 logger.error(ire.toString());
             }
-        }
+            break;
 
-        if(state.isFinal()) {
-            if(getFileId() != null && getPinId() != null) {
-                logger.info("state changed to final state, unpinning fileId= "+ getFileId()+" pinId = "+getPinId());
-                SRMUser user;
-                try {
-                    user = getUser();
-                } catch (SRMInvalidRequestException ire) {
-                    logger.error(ire.toString()) ;
-                    return;
+        case DONE:
+        case FAILED:
+        case CANCELED:
+            AbstractStorageElement storage = getStorage();
+            try {
+                SRMUser user = getUser();
+                String fileId = getFileId();
+                String pinId = getPinId();
+                if (fileId != null && pinId != null) {
+                    logger.info("State changed to final state, unpinning fileId = {} pinId = {}.", fileId, pinId);
+                    CheckedFuture<String, ? extends SRMException> future = storage.unPinFile(user, fileId, pinId);
+                    future.addListener(() -> {
+                        try {
+                            logger.debug("Unpinned (pinId={}).", future.checkedGet());
+                        } catch (SRMException e) {
+                            logger.error("Unpinning failed: {}", e.getMessage());
+                        }
+                    }, MoreExecutors.directExecutor());
+                } else {
+                    BringOnlineFileRequest.unpinBySURLandRequestToken(storage, user, String.valueOf(getRequestId()), getSurl());
                 }
-                final CheckedFuture<String, ? extends SRMException> future =
-                        getStorage().unPinFile(user, getFileId(), getPinId());
-                future.addListener(() -> {
-                    try {
-                        String pinId1 = future.checkedGet();
-                        logger.debug("Unpinned (pinId={}).", pinId1);
-                    } catch (SRMException e) {
-                        logger.error("Unpinning failed: {}", e.getMessage());
-                    }
-                }, MoreExecutors.directExecutor());
+            } catch (SRMInternalErrorException | SRMInvalidRequestException e) {
+                logger.error(e.toString()) ;
             }
         }