Last few fixes for the "performance improvements" release: fixed the …

…bug with file-level compute urls; got rid of duplicated code in the S3 driver; updated the version number in the docs.
cscn · Oct 6, 2017 · ceffe08 · ceffe08
1 parent 86113a2
commit ceffe08
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 42 deletions.
diff --git a/doc/sphinx-guides/source/conf.py b/doc/sphinx-guides/source/conf.py
@@ -64,9 +64,9 @@
 # built documents.
 #
 # The short X.Y version.
-version = '4.8'
+version = '4.8.1'
 # The full version, including alpha/beta/rc tags.
-release = '4.8'
+release = '4.8.1'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

diff --git a/doc/sphinx-guides/source/index.rst b/doc/sphinx-guides/source/index.rst
@@ -3,7 +3,7 @@
    You can adapt this file completely to your liking, but it should at least
    contain the root `toctree` directive.
 
-Dataverse 4.8 Guides
+Dataverse 4.8.1 Guides
 ======================
 
 These guides are for the most recent version of Dataverse. For the guides for **version 4.7.1** please go `here <http://guides.dataverse.org/en/4.7.1/>`_.

diff --git a/pom.xml b/pom.xml
@@ -4,7 +4,7 @@
 
     <groupId>edu.harvard.iq</groupId>
     <artifactId>dataverse</artifactId>
-    <version>4.8</version>
+    <version>4.8.1</version>
     <packaging>war</packaging>
 
     <name>dataverse</name>

diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java
@@ -510,19 +510,24 @@ public boolean showComputeButton() {
         return showComputeButtonForDataset;
     }
 
-    private Boolean showComputeButtonForFile = null; 
+    private Map<Long, Boolean> showComputeButtonForFile = new HashMap<>();
     //this function applies to a single datafile
     public boolean showComputeButton(FileMetadata metadata) {
-        if (showComputeButtonForFile != null) {
-            return showComputeButtonForFile;
+        Long fileId = metadata.getDataFile().getId();
+
+        if (fileId == null) {
+            return false;
         }
 
-        if (isSwiftStorage(metadata) && (settingsService.getValueForKey(SettingsServiceBean.Key.ComputeBaseUrl) != null)) {
-            showComputeButtonForFile = true;
-        } else {
-            showComputeButtonForFile = false;
+        if (showComputeButtonForFile.containsKey(fileId)) {
+            return showComputeButtonForFile.get(fileId);
         }
-        return showComputeButtonForFile;
+
+        boolean result = isSwiftStorage(metadata) 
+                && settingsService.getValueForKey(SettingsServiceBean.Key.ComputeBaseUrl) != null;
+
+        showComputeButtonForFile.put(fileId, result);
+        return result;
     }
 
     public boolean canDownloadAllFiles(){
@@ -700,9 +705,8 @@ public boolean isThumbnailAvailable(FileMetadata fileMetadata) {
 
         // new and optimized logic: 
         // - check download permission here (should be cached - so it's free!)
-        // - only then ask the file service if the thumbnail is available/exists.
-        // the service itself no longer checks download permissions.  
-        // plus, cache the results!
+        // - only then check if the thumbnail is available/exists.
+        // then cache the results!
 
         Long dataFileId = fileMetadata.getDataFile().getId();
 

diff --git a/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java b/src/main/java/edu/harvard/iq/dataverse/dataaccess/S3AccessIO.java
@@ -78,6 +78,8 @@ public S3AccessIO(T dvObject, DataAccessRequest req) {
         }
     }
 
+    public static String S3_IDENTIFIER_PREFIX = "s3";
+
     private AWSCredentials awsCredentials = null;
     private AmazonS3 s3 = null;
     private String bucketName = System.getProperty("dataverse.files.s3-bucket-name");
@@ -118,12 +120,7 @@ public void open(DataAccessOption... options) throws IOException {
             }
 
             if (isReadAccess) {
-                if (storageIdentifier.startsWith("s3://")) {
-                    bucketName = storageIdentifier.substring(storageIdentifier.indexOf(":") + 3, storageIdentifier.lastIndexOf(":"));
-                    key += "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
-                } else {
-                    throw new IOException("IO driver mismatch: S3AccessIO called on a non-s3 stored object.");
-                }
+                key = getBaseKey();
                 S3Object s3object = s3.getObject(new GetObjectRequest(bucketName, key));
                 InputStream in = s3object.getObjectContent();
 
@@ -148,11 +145,11 @@ public void open(DataAccessOption... options) throws IOException {
                 }
 
             } else if (isWriteAccess) {
-                if (storageIdentifier.startsWith("s3://")) {
+                if (storageIdentifier.startsWith(S3_IDENTIFIER_PREFIX + "://")) {
                     key += "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
                 } else {
                     key += "/" + storageIdentifier;
-                    dvObject.setStorageIdentifier("s3://" + bucketName + ":" + storageIdentifier);
+                    dvObject.setStorageIdentifier(S3_IDENTIFIER_PREFIX + "://" + bucketName + ":" + storageIdentifier);
                 }
 
             }
@@ -167,7 +164,7 @@ public void open(DataAccessOption... options) throws IOException {
         } else if (dvObject instanceof Dataset) {
             Dataset dataset = this.getDataset();
             key = dataset.getAuthority() + "/" + dataset.getIdentifier();
-            dataset.setStorageIdentifier("s3://" + key);
+            dataset.setStorageIdentifier(S3_IDENTIFIER_PREFIX + "://" + key);
         } else if (dvObject instanceof Dataverse) {
             throw new IOException("Data Access: Invalid DvObject type : Dataverse");
         } else {
@@ -534,27 +531,31 @@ public InputStream getAuxFileAsInputStream(String auxItemTag) throws IOException
 
     private String getDestinationKey(String auxItemTag) throws IOException {
         if (dvObject instanceof DataFile) {
-            if (key == null) {
-                String baseKey = this.getDataFile().getOwner().getAuthority() + "/" + this.getDataFile().getOwner().getIdentifier();
-                String storageIdentifier = dvObject.getStorageIdentifier();
-
-                if (storageIdentifier == null || "".equals(storageIdentifier)) {
-                    throw new FileNotFoundException("Data Access: No local storage identifier defined for this datafile.");
-                }
-
-                if (storageIdentifier.startsWith("s3://")) {
-                    bucketName = storageIdentifier.substring(storageIdentifier.indexOf(":") + 3, storageIdentifier.lastIndexOf(":"));
-                    key = baseKey + "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
-                } else {
-                    throw new IOException("S3AccessIO: DataFile (storage identifier "+storageIdentifier+") does not appear to be an S3 object.");
-                }
-            }
-
-            return key + "." + auxItemTag;
+            return getBaseKey() + "." + auxItemTag;
         } else if (dvObject instanceof Dataset) {
-            return key + "/" + auxItemTag;
+            return getBaseKey() + "/" + auxItemTag;
         } else {
             throw new IOException("S3AccessIO: This operation is only supported for Datasets and DataFiles.");
         }
     }
+
+    private String getBaseKey() throws IOException {
+        if (key == null) {
+            String baseKey = this.getDataFile().getOwner().getAuthority() + "/" + this.getDataFile().getOwner().getIdentifier();
+            String storageIdentifier = dvObject.getStorageIdentifier();
+
+            if (storageIdentifier == null || "".equals(storageIdentifier)) {
+                throw new FileNotFoundException("Data Access: No local storage identifier defined for this datafile.");
+            }
+
+            if (storageIdentifier.startsWith(S3_IDENTIFIER_PREFIX + "://")) {
+                bucketName = storageIdentifier.substring((S3_IDENTIFIER_PREFIX + "://").length(), storageIdentifier.lastIndexOf(":"));
+                key = baseKey + "/" + storageIdentifier.substring(storageIdentifier.lastIndexOf(":") + 1);
+            } else {
+                throw new IOException("S3AccessIO: DataFile (storage identifier " + storageIdentifier + ") does not appear to be an S3 object.");
+            }
+        }
+
+        return key;
+    }
 }