fix: add optomization to prevent proxying downloads for canfar deploy…

…ment
opencadc · Dec 3, 2024 · 05d289e · 05d289e
1 parent 514e403
commit 05d289e
Show file tree

Hide file tree

Showing 7 changed files with 158 additions and 47 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,9 @@
-# Storage User Interface
+# Storage User Interface 1.2.0
+
+## December 3, 2024 (version 1.2.0)
+* Support for TAR and ZIP downloads
+* Small optimizations and bug fixes
+  * For those deployments that support a direct auth connection (or only support public) with the backend VOSpace API, redirect the download, rather than proxying it. 
 
 ## January 30, 2024
 * Support for IAM Group querying

diff --git a/VERSION b/VERSION
@@ -1,6 +1,6 @@
 ## deployable containers have a semantic and build tag
 # semantic version tag: major.minor
 # build version tag: timestamp
-VERSION="1.2.0"
+VERSION="1.2.1"
 TAGS="${VERSION} ${VERSION}-$(date -u +"%Y%m%dT%H%M%S")"
 unset VERSION
diff --git a/src/main/java/net/canfar/storage/web/config/VOSpaceServiceConfig.java b/src/main/java/net/canfar/storage/web/config/VOSpaceServiceConfig.java
@@ -68,13 +68,12 @@
 package net.canfar.storage.web.config;
 
 import ca.nrc.cadc.util.StringUtil;
-import net.canfar.storage.PathUtils;
-import org.opencadc.vospace.Node;
-import org.opencadc.vospace.VOSURI;
-
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.nio.file.Path;
+import net.canfar.storage.PathUtils;
+import org.opencadc.vospace.Node;
+import org.opencadc.vospace.VOSURI;
 
 public class VOSpaceServiceConfig {
     private final String name;
@@ -120,22 +119,26 @@ public URI getNodeResourceID() {
         return this.nodeResourceID;
     }
 
-    public final boolean supportsBatchDownloads() {
+    public boolean supportsBatchDownloads() {
         return this.features.supportsBatchDownloads;
     }
 
-    public final boolean supportsBatchUploads() {
+    public boolean supportsBatchUploads() {
         return this.features.supportsBatchUploads;
     }
 
-    public final boolean supportsExternalLinks() {
+    public boolean supportsExternalLinks() {
         return this.features.supportsExternalLinks;
     }
 
-    public final boolean supportsPaging() {
+    public boolean supportsPaging() {
         return this.features.supportsPaging;
     }
 
+    public boolean supportsDirectDownload() {
+        return this.features.supportsDirectDownload;
+    }
+
     public VOSURI toURI(final String path) throws URISyntaxException {
         return new VOSURI(new URI(this.nodeResourceID + path));
     }
@@ -145,27 +148,30 @@ public VOSURI toURI(final Node node) throws URISyntaxException {
         return toURI(path.toString());
     }
 
-
     /**
-     * Features that require flags to disable it, or is generally optional.  Some Cavern style VOSpace services do not
+     * Features that require flags to disable it, or is generally optional. Some Cavern style VOSpace services do not
      * support pagination (i.e. limit={number}&startURI={pageStartURI}), for example.
      */
     public static final class Features {
         private boolean supportsBatchDownloads = false;
         private boolean supportsBatchUploads = false;
         private boolean supportsExternalLinks = false;
         private boolean supportsPaging = false;
+        private boolean supportsDirectDownload = false;
 
-        public Features() {
+        public Features() {}
 
-        }
-
-        Features(boolean supportsBatchDownloads, boolean supportsBatchUploads, boolean supportsExternalLinks,
-                 boolean supportsPaging) {
+        Features(
+                boolean supportsBatchDownloads,
+                boolean supportsBatchUploads,
+                boolean supportsExternalLinks,
+                boolean supportsPaging,
+                boolean supportsDirectDownload) {
             this.supportsBatchDownloads = supportsBatchDownloads;
             this.supportsBatchUploads = supportsBatchUploads;
             this.supportsExternalLinks = supportsExternalLinks;
             this.supportsPaging = supportsPaging;
+            this.supportsDirectDownload = supportsBatchDownloads;
         }
 
         public void supportsBatchDownloads() {
@@ -183,5 +189,9 @@ public void supportsExternalLinks() {
         public void supportsPaging() {
             this.supportsPaging = true;
         }
+
+        public void supportsDirectDownload() {
+            this.supportsDirectDownload = true;
+        }
     }
 }
diff --git a/src/main/java/net/canfar/storage/web/config/VOSpaceServiceConfigManager.java b/src/main/java/net/canfar/storage/web/config/VOSpaceServiceConfigManager.java
@@ -73,10 +73,8 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.TreeMap;
-
 import org.apache.log4j.Logger;
 
-
 public class VOSpaceServiceConfigManager {
     private static final Logger LOGGER = Logger.getLogger(VOSpaceServiceConfigManager.class);
     private final TreeMap<String, VOSpaceServiceConfig> serviceConfigMap = new TreeMap<>();
@@ -94,13 +92,14 @@ public class VOSpaceServiceConfigManager {
     public static final String SERVICE_FEATURE_EXTERNAL_LINKS_PROPERTY_KEY_FORMAT =
             "%s%s.service.features.externalLinks";
     public static final String SERVICE_FEATURE_PAGING_PROPERTY_KEY_FORMAT = "%s%s.service.features.paging";
+    public static final String SERVICE_FEATURE_DIRECT_DOWNLOAD_PROPERTY_KEY_FORMAT =
+            "%s%s.service.features.directDownload";
 
     // Used to construct service-specific property keys (ie KEY_BASE + {svc name} + {rest of key}
     public static final String KEY_BASE = "org.opencadc.vosui.";
 
     private final StorageConfiguration applicationConfiguration;
 
-
     public VOSpaceServiceConfigManager(StorageConfiguration appConfig) {
         this.applicationConfiguration = appConfig;
         loadConfig();
@@ -120,33 +119,34 @@ public void loadConfig() {
 
     private void loadServices() {
         this.serviceList.forEach(storageServiceName -> {
-            final String servicePrefixKey =
-                    String.format(VOSpaceServiceConfigManager.SERVICE_NAME_RESOURCE_ID_PROPERTY_KEY_FORMAT,
-                                  VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+            final String servicePrefixKey = String.format(
+                    VOSpaceServiceConfigManager.SERVICE_NAME_RESOURCE_ID_PROPERTY_KEY_FORMAT,
+                    VOSpaceServiceConfigManager.KEY_BASE,
+                    storageServiceName);
 
             LOGGER.debug("adding vospace service to map: " + servicePrefixKey + ": " + storageServiceName);
 
             final String vospaceResourceIDStr = applicationConfiguration.lookup(servicePrefixKey, true);
             final URI vospaceResourceID = URI.create(vospaceResourceIDStr);
 
-            final String nodeResourcePrefixKey =
-                    String.format(VOSpaceServiceConfigManager.SERVICE_NODE_RESOURCE_ID_PROPERTY_KEY_FORMAT,
-                                  VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+            final String nodeResourcePrefixKey = String.format(
+                    VOSpaceServiceConfigManager.SERVICE_NODE_RESOURCE_ID_PROPERTY_KEY_FORMAT,
+                    VOSpaceServiceConfigManager.KEY_BASE,
+                    storageServiceName);
             final String nodeResourceIDStr = applicationConfiguration.lookup(nodeResourcePrefixKey, true);
             final URI nodeResourceID = URI.create(nodeResourceIDStr);
 
             LOGGER.debug("node resource id base: " + nodeResourceID);
 
-            final String userHomePrefixKey =
-                    String.format(VOSpaceServiceConfigManager.SERVICE_USER_HOME_PROPERTY_KEY_FORMAT,
-                                  VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+            final String userHomePrefixKey = String.format(
+                    VOSpaceServiceConfigManager.SERVICE_USER_HOME_PROPERTY_KEY_FORMAT,
+                    VOSpaceServiceConfigManager.KEY_BASE,
+                    storageServiceName);
             final String userHomeStr = applicationConfiguration.lookup(userHomePrefixKey, true);
 
             // At this point, the values have been validated
-            final VOSpaceServiceConfig voSpaceServiceConfig = new VOSpaceServiceConfig(storageServiceName,
-                                                                                       vospaceResourceID,
-                                                                                       nodeResourceID,
-                                                                                       getFeatures(storageServiceName));
+            final VOSpaceServiceConfig voSpaceServiceConfig = new VOSpaceServiceConfig(
+                    storageServiceName, vospaceResourceID, nodeResourceID, getFeatures(storageServiceName));
             voSpaceServiceConfig.homeDir = userHomeStr;
 
             this.serviceConfigMap.put(storageServiceName, voSpaceServiceConfig);
@@ -172,38 +172,52 @@ public List<String> getServiceList() {
     VOSpaceServiceConfig.Features getFeatures(final String storageServiceName) {
         final VOSpaceServiceConfig.Features features = new VOSpaceServiceConfig.Features();
 
-        final String supportsBatchDownloadProperty =
-                String.format(VOSpaceServiceConfigManager.SERVICE_FEATURE_BATCH_DOWNLOAD_PROPERTY_KEY_FORMAT,
-                              VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+        final String supportsBatchDownloadProperty = String.format(
+                VOSpaceServiceConfigManager.SERVICE_FEATURE_BATCH_DOWNLOAD_PROPERTY_KEY_FORMAT,
+                VOSpaceServiceConfigManager.KEY_BASE,
+                storageServiceName);
         final boolean supportsBatchDownload = applicationConfiguration.lookupFlag(supportsBatchDownloadProperty, true);
         if (supportsBatchDownload) {
             features.supportsBatchDownloads();
         }
 
-        final String supportsBatchUploadProperty =
-                String.format(VOSpaceServiceConfigManager.SERVICE_FEATURE_BATCH_UPLOAD_PROPERTY_KEY_FORMAT,
-                              VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+        final String supportsBatchUploadProperty = String.format(
+                VOSpaceServiceConfigManager.SERVICE_FEATURE_BATCH_UPLOAD_PROPERTY_KEY_FORMAT,
+                VOSpaceServiceConfigManager.KEY_BASE,
+                storageServiceName);
         final boolean supportsBatchUpload = applicationConfiguration.lookupFlag(supportsBatchUploadProperty, true);
         if (supportsBatchUpload) {
             features.supportsBatchUploads();
         }
 
-        final String supportsExternalLinksProperty =
-                String.format(VOSpaceServiceConfigManager.SERVICE_FEATURE_EXTERNAL_LINKS_PROPERTY_KEY_FORMAT,
-                              VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+        final String supportsExternalLinksProperty = String.format(
+                VOSpaceServiceConfigManager.SERVICE_FEATURE_EXTERNAL_LINKS_PROPERTY_KEY_FORMAT,
+                VOSpaceServiceConfigManager.KEY_BASE,
+                storageServiceName);
         final boolean supportsExternalLinks = applicationConfiguration.lookupFlag(supportsExternalLinksProperty, true);
         if (supportsExternalLinks) {
             features.supportsExternalLinks();
         }
 
-        final String supportsPagingProperty =
-                String.format(VOSpaceServiceConfigManager.SERVICE_FEATURE_PAGING_PROPERTY_KEY_FORMAT,
-                              VOSpaceServiceConfigManager.KEY_BASE, storageServiceName);
+        final String supportsPagingProperty = String.format(
+                VOSpaceServiceConfigManager.SERVICE_FEATURE_PAGING_PROPERTY_KEY_FORMAT,
+                VOSpaceServiceConfigManager.KEY_BASE,
+                storageServiceName);
         final boolean supportsPaging = applicationConfiguration.lookupFlag(supportsPagingProperty, true);
         if (supportsPaging) {
             features.supportsPaging();
         }
 
+        final String supportsDirectDownloadProperty = String.format(
+                VOSpaceServiceConfigManager.SERVICE_FEATURE_DIRECT_DOWNLOAD_PROPERTY_KEY_FORMAT,
+                VOSpaceServiceConfigManager.KEY_BASE,
+                storageServiceName);
+        final boolean supportsDirectDownload =
+                applicationConfiguration.lookupFlag(supportsDirectDownloadProperty, false);
+        if (supportsDirectDownload) {
+            features.supportsDirectDownload();
+        }
+
         return features;
     }
 }
diff --git a/src/main/java/org/opencadc/storage/StorageAction.java b/src/main/java/org/opencadc/storage/StorageAction.java
@@ -103,7 +103,7 @@ protected URL lookupEndpoint(final URI serviceURI, final URI capabilityStandardU
         return new RegistryClient().getServiceURL(serviceURI, capabilityStandardURI, authMethod);
     }
 
-    protected final VOSpaceServiceConfig getCurrentService() {
+    protected VOSpaceServiceConfig getCurrentService() {
         final String providedServiceName = this.syncInput.getParameter("service");
         final String serviceName = StringUtil.hasText(providedServiceName)
                 ? providedServiceName

diff --git a/src/main/java/org/opencadc/storage/pkg/PostAction.java b/src/main/java/org/opencadc/storage/pkg/PostAction.java
@@ -21,6 +21,7 @@
 import java.util.NoSuchElementException;
 import java.util.stream.Collectors;
 import javax.security.auth.Subject;
+import javax.servlet.http.HttpServletResponse;
 import net.canfar.storage.web.config.StorageConfiguration;
 import net.canfar.storage.web.config.VOSpaceServiceConfig;
 import net.canfar.storage.web.config.VOSpaceServiceConfigManager;
@@ -64,11 +65,29 @@ public void doAction() throws Exception {
         if (LOGGER.isDebugEnabled()) {
             targetList.forEach(target -> LOGGER.debug("Sending target " + target));
         }
-
         final Subject currentSubject = getCurrentSubject(
                 lookupEndpoint(getCurrentService().getResourceID(), Standards.VOSPACE_NODES_20, AuthMethod.TOKEN));
         final URL transferRunURL = getTransferRunURL(currentSubject, getTransferRunPayload(targetList, responseFormat));
 
+        processDownload(transferRunURL, currentSubject);
+    }
+
+    void processDownload(final URL transferRunURL, final Subject currentSubject) throws Exception {
+        if (this.getCurrentService().supportsDirectDownload()) {
+            LOGGER.debug("Direct download supported, redirecting to " + transferRunURL);
+            redirect(transferRunURL);
+        } else {
+            LOGGER.debug("Direct download not supported, proxying from " + transferRunURL);
+            proxyDownload(transferRunURL, currentSubject);
+        }
+    }
+
+    void redirect(final URL redirectURL) {
+        this.syncOutput.setCode(HttpServletResponse.SC_MOVED_TEMPORARILY);
+        this.syncOutput.addHeader("location", redirectURL.toExternalForm());
+    }
+
+    void proxyDownload(final URL transferRunURL, final Subject currentSubject) throws Exception {
         final HttpGet httpGet = new HttpGet(transferRunURL, true);
         Subject.doAs(currentSubject, (PrivilegedExceptionAction<Void>) () -> {
             httpGet.prepare();

diff --git a/src/test/java/org/opencadc/storage/pkg/PostActionTest.java b/src/test/java/org/opencadc/storage/pkg/PostActionTest.java
@@ -3,7 +3,10 @@
 import ca.nrc.cadc.rest.SyncInput;
 import ca.nrc.cadc.rest.SyncOutput;
 import java.net.URI;
+import java.net.URL;
 import java.util.Arrays;
+import javax.security.auth.Subject;
+import net.canfar.storage.web.config.VOSpaceServiceConfig;
 import org.junit.Assert;
 import org.junit.Test;
 import org.mockito.Mockito;
@@ -74,4 +77,64 @@ public void testGetTargetURIs() {
         Assert.assertArrayEquals("Wrong URIs.", expectedURIs, actualURIs);
         Mockito.verify(mockSyncInput, Mockito.times(1)).getParameters("uri");
     }
+
+    @Test
+    public void testDirectDownload() throws Exception {
+        final VOSpaceServiceConfig mockService = Mockito.mock(VOSpaceServiceConfig.class);
+
+        Mockito.when(mockService.supportsDirectDownload()).thenReturn(true);
+        final boolean[] expectedCalls = new boolean[] {false};
+
+        final PostAction postAction = new PostAction(null, null, null, null) {
+            @Override
+            protected VOSpaceServiceConfig getCurrentService() {
+                return mockService;
+            }
+
+            @Override
+            void proxyDownload(URL transferRunURL, Subject currentSubject) {
+                Assert.fail("Should not be called");
+            }
+
+            @Override
+            void redirect(URL redirectURL) {
+                expectedCalls[0] = true;
+            }
+        };
+
+        postAction.processDownload(null, null);
+        Assert.assertTrue("Should call redirect.", expectedCalls[0]);
+
+        Mockito.verify(mockService, Mockito.times(1)).supportsDirectDownload();
+    }
+
+    @Test
+    public void testProxyDownload() throws Exception {
+        final VOSpaceServiceConfig mockService = Mockito.mock(VOSpaceServiceConfig.class);
+
+        Mockito.when(mockService.supportsDirectDownload()).thenReturn(false);
+        final boolean[] expectedCalls = new boolean[] {false};
+
+        final PostAction postAction = new PostAction(null, null, null, null) {
+            @Override
+            protected VOSpaceServiceConfig getCurrentService() {
+                return mockService;
+            }
+
+            @Override
+            void proxyDownload(URL transferRunURL, Subject currentSubject) throws Exception {
+                expectedCalls[0] = true;
+            }
+
+            @Override
+            void redirect(URL redirectURL) {
+                Assert.fail("Should not be called");
+            }
+        };
+
+        postAction.processDownload(null, null);
+        Assert.assertTrue("Should call proxyDownload.", expectedCalls[0]);
+
+        Mockito.verify(mockService, Mockito.times(1)).supportsDirectDownload();
+    }
 }