Merge pull request #11466 from IQSS/9620-better-file-bundle-name

Prettier names for the zipped multi-file bundles

Author: ofahimIQSS

doc/release-notes/9620-better-file-bundle-name.md

@@ -0,0 +1,2 @@
+The Data Access APIs that generate multi-file zipped bundles will offer file name suggestions based on the persistent identifiers (for example, `doi-10.70122-fk2-xxyyzz.zip`), instead of the fixed `dataverse_files.zip` as in prior versions.
+See the Data Access API guide for more info. 

doc/sphinx-guides/source/api/dataaccess.rst

@@ -21,7 +21,7 @@ There are a number of reasons why not all of the files can be downloaded:
 - Some of the files are restricted and your API token doesn't have access (you will still get the unrestricted files).
 - The Dataverse installation has limited how large the zip bundle can be.
 
-In the curl example below, the flags ``-O`` and ``J`` are used. When there are no errors, this has the effect of saving the file as "dataverse_files.zip" (just like the web interface). The flags force errors to be downloaded as a file.
+In the curl example below, the flags ``-O`` and ``-J`` are used. When there are no errors, this has the effect of saving the file under the name suggested by Dataverse (which as of v6.7 will be based on the persistent identifier of the dataset and the latest version number, for example ``doi-10.70122-fk2-n2xgbj_1.1.zip``; in prior versions the file name was ``dataverse_files.zip`` in all cases). This mirrors the way the files are saved when downloaded in a browser. The flags also force error messages to be downloaded as a file.
 
 Please note that in addition to the files from dataset, an additional file call "MANIFEST.TXT" will be included in the zipped bundle. It has additional information about the files.
 
@@ -70,6 +70,8 @@ A curl example using a DOI (with version):
 
   curl -O -J -H "X-Dataverse-key:$API_TOKEN" $SERVER_URL/api/access/dataset/:persistentId/versions/$VERSION?persistentId=$PERSISTENT_ID
 
+Similarly to the API above, this will save the downloaded bundle under the name based on the persistent identifier and the version number, for example, ``doi-10.70122-fk2-n2xgbj_1.1.zip`` or ``doi-10.70122-fk2-n2xgbj_draft.zip``.
+
 The fully expanded example above (without environment variables) looks like this:
 
 .. code-block:: bash
@@ -173,7 +175,7 @@ Multiple File ("bundle") download
 
 Alternate Form: POST to ``/api/access/datafiles`` with a ``fileIds`` input field containing the same comma separated list of file ids. This is most useful when your list of files surpasses the allowed URL length (varies but can be ~2000 characters).  
 
-Returns the files listed, zipped. 
+Returns the files listed, zipped. As of v6.7 the name of the zipped bundle will be based on the persistent identifier of the parent dataset, for example, ``doi-10.70122-fk2-xxyyzz.zip``; in prior versions the file name was ``dataverse_files.zip`` in all cases).
 
 .. note:: If the request can only be completed partially - if only *some* of the requested files can be served (because of the permissions and/or size restrictions), the file MANIFEST.TXT included in the zipped bundle will have entries specifying the reasons the missing files could not be downloaded. IN THE FUTURE the API will return a 207 status code to indicate that the result was a partial success. (As of writing this - v.4.11 - this hasn't been implemented yet)
 

src/main/java/edu/harvard/iq/dataverse/api/Access.java

@@ -169,6 +169,7 @@ public class Access extends AbstractApiBean {
     @Inject
     DataverseFeaturedItemServiceBean dataverseFeaturedItemServiceBean;
 
+    private static final String DEFAULT_BUNDLE_NAME = "dataverse_files.zip";
     //@EJB
 
     // TODO: 
@@ -643,7 +644,7 @@ public DownloadInstance downloadAuxiliaryFile(@Context ContainerRequestContext c
     public Response postDownloadDatafiles(@Context ContainerRequestContext crc, String fileIds, @QueryParam("gbrecs") boolean gbrecs, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) throws WebApplicationException {
 
 
-        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response);
+        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
     }
 
     @GET
@@ -664,7 +665,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
                     // We don't want downloads from Draft versions to be counted, 
                     // so we are setting the gbrecs (aka "do not write guestbook response") 
                     // variable accordingly:
-                    return downloadDatafiles(getRequestUser(crc), fileIds, true, uriInfo, headers, response);
+                    return downloadDatafiles(getRequestUser(crc), fileIds, true, uriInfo, headers, response, "draft");
                 }
             }
 
@@ -685,7 +686,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
             }
 
             String fileIds = getFileIdsAsCommaSeparated(latest.getFileMetadatas());
-            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response);
+            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, latest.getFriendlyVersionNumber());
         } catch (WrappedResponse wr) {
             return wr.getResponse();
         }
@@ -735,7 +736,7 @@ public Command<DatasetVersion> handleLatestPublished() {
             if (dsv.isDraft()) {
                 gbrecs = true;
             }
-            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response);
+            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, dsv.getFriendlyVersionNumber().toLowerCase());
         } catch (WrappedResponse wr) {
             return wr.getResponse();
         }
@@ -749,6 +750,24 @@ private static String getFileIdsAsCommaSeparated(List<FileMetadata> fileMetadata
         }
         return String.join(",", ids);
     }
+    
+    private String generateMultiFileBundleName(Dataset dataset, String versionTag) {
+        String bundleName = DEFAULT_BUNDLE_NAME;
+        
+        if (dataset != null && dataset.getGlobalId() != null) {
+            String protocol = dataset.getProtocol();
+            String authority = dataset.getAuthority().toLowerCase();
+            String identifier = dataset.getIdentifier().replace('/', '-').toLowerCase();
+                
+            if (versionTag != null) {
+                bundleName = protocol + "-" + authority + "-" + identifier + "_" + versionTag + ".zip"; 
+            } else {
+                bundleName = protocol + "-" + authority + "-" + identifier + ".zip";
+            }
+        }
+        
+        return bundleName;
+    }
 
     /*
      * API method for downloading zipped bundles of multiple files:
@@ -758,10 +777,10 @@ private static String getFileIdsAsCommaSeparated(List<FileMetadata> fileMetadata
     @Path("datafiles/{fileIds}")
     @Produces({"application/zip"})
     public Response datafiles(@Context ContainerRequestContext crc, @PathParam("fileIds") String fileIds, @QueryParam("gbrecs") boolean gbrecs, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) throws WebApplicationException {
-        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response);
+        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
     }
 
-    private Response downloadDatafiles(User user, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
+    private Response downloadDatafiles(User user, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response, String versionTag) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
         final long zipDownloadSizeLimit = systemConfig.getZipDownloadLimit();
 
         logger.fine("setting zip download size limit to " + zipDownloadSizeLimit + " bytes.");
@@ -852,8 +871,9 @@ public void write(OutputStream os) throws IOException,
                                         // to produce some output.
                                         zipper = new DataFileZipper(os);
                                         zipper.setFileManifest(fileManifest);
-                                        response.setHeader("Content-disposition", "attachment; filename=\"dataverse_files.zip\"");
-                                        response.setHeader("Content-Type", "application/zip; name=\"dataverse_files.zip\"");
+                                        String bundleName = generateMultiFileBundleName(file.getOwner(), versionTag);
+                                        response.setHeader("Content-disposition", "attachment; filename=\"" + bundleName + "\"");
+                                        response.setHeader("Content-Type", "application/zip; name=\"" + bundleName + "\"");
                                     }
 
                                     long size = 0L;
@@ -960,8 +980,8 @@ public InputStream tempPreview(@PathParam("fileSystemId") String fileSystemId, @
 
     }*/
 
-    
-    
+        
+
     // TODO: Rather than only supporting looking up files by their database IDs, consider supporting persistent identifiers.
     @Path("fileCardImage/{fileId}")
     @GET

src/test/java/edu/harvard/iq/dataverse/api/AccessIT.java

@@ -21,6 +21,8 @@
 import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
 import java.util.HashMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.hamcrest.collection.IsMapContaining;
 
@@ -43,7 +45,8 @@ public class AccessIT {
     public static String apiToken;
     public static String dataverseAlias;
     public static Integer datasetId;
-    
+    public static String persistentId; 
+
     public static Integer basicFileId;
     public static Integer tabFile1Id;
     public static Integer tabFile2Id;
@@ -78,7 +81,6 @@ public class AccessIT {
     private static String testFileFromZipUploadWithFoldersChecksum1 = "8f326944be21361ad8219bc3269bc9eb";
     private static String testFileFromZipUploadWithFoldersChecksum2 = "0fe4efd85229bad6e587fd3f1a6c8e05";
     private static String testFileFromZipUploadWithFoldersChecksum3 = "00433ccb20111f9d40f0e5ab6fa8396f";
-
 
     @BeforeAll
     public static void setUp() throws InterruptedException {
@@ -101,6 +103,7 @@ public static void setUp() throws InterruptedException {
         Response createDatasetResponse = UtilIT.createDatasetViaNativeApi(dataverseAlias, pathToJsonFile, apiToken);
         createDatasetResponse.prettyPrint();
         datasetId = JsonPath.from(createDatasetResponse.body().asString()).getInt("data.id");
+        persistentId = JsonPath.from(createDatasetResponse.body().asString()).getString("data.persistentId");
 
         Response allowAccessRequests = UtilIT.allowAccessRequests(datasetId.toString(), true, apiToken);
         allowAccessRequests.prettyPrint();
@@ -285,7 +288,27 @@ public void testDownloadMultipleFiles_NonLoggedInOpen() throws IOException {
         assertThat(files2, IsMapContaining.hasKey(tabFile2NameConvert));
 
         System.out.println("origSize: " + origSizeAnon + " | convertSize: " + convertSizeAnon);
-        assertThat(origSizeAnon, is(not(convertSizeAnon)));  
+        assertThat(origSizeAnon, is(not(convertSizeAnon))); 
+        
+        // Finally, verify that the multi-file bundle produced by the API  
+        // is properly named (as of v6.7 this should be a pretty name based on 
+        // the persistent Id of the dataset). 
+        
+        String contentDispositionHeader = anonDownloadConverted.getHeader("Content-disposition");
+        System.out.println("Response header: "+contentDispositionHeader);
+        
+        Pattern regexPattern = Pattern.compile("attachment; filename=\"([a-z0-9\\.-]*\\.zip)\"");
+        Matcher regexMatcher = regexPattern.matcher(contentDispositionHeader);
+        boolean regexMatch = regexMatcher.find();
+        assertTrue(regexMatch);
+        
+        String expectedPrettyName = persistentId.replaceAll("[:/]", "-").toLowerCase() + ".zip";
+        System.out.println("expected \"pretty\" file name of the zipped multi-file bundle: " + expectedPrettyName);
+        
+        String fileBundleName = regexMatcher.group(1);
+        System.out.println("file name found in the header: "+fileBundleName);
+        
+        assertEquals(fileBundleName, expectedPrettyName);
     }
 
     @Test
@@ -476,7 +499,7 @@ public void testRequestAccess() throws InterruptedException {
         basicFileName = "004.txt";
         String basicPathToFile = "scripts/search/data/replace_test/" + basicFileName;
         Response basicAddResponse = UtilIT.uploadFileViaNative(datasetIdNew.toString(), basicPathToFile, apiToken);
-        basicFileId = JsonPath.from(basicAddResponse.body().asString()).getInt("data.files[0].dataFile.id");
+        Integer basicFileIdNew = JsonPath.from(basicAddResponse.body().asString()).getInt("data.files[0].dataFile.id");
 
         String tabFile3NameRestrictedNew = "stata13-auto-withstrls.dta";
         String tab3PathToFile = "scripts/search/data/tabular/" + tabFile3NameRestrictedNew;
@@ -534,7 +557,7 @@ public void testRequestAccess() throws InterruptedException {
         assertEquals(400, requestFileAccessResponse.getStatusCode());
 
         //if you make a request of a public file you should also get a command exception
-        requestFileAccessResponse = UtilIT.requestFileAccess(basicFileId.toString(), apiTokenRando);
+        requestFileAccessResponse = UtilIT.requestFileAccess(basicFileIdNew.toString(), apiTokenRando);
         assertEquals(400, requestFileAccessResponse.getStatusCode());