API tests now exercise dataset thumbnails in GUI #3559

IQSS · Feb 28, 2017 · a56829b · a56829b
1 parent 591c0c2
commit a56829b
Show file tree

Hide file tree

Showing 11 changed files with 145 additions and 106 deletions.
diff --git a/scripts/search/data/binary/trees.png b/scripts/search/data/binary/trees.png
diff --git a/src/main/java/Bundle.properties b/src/main/java/Bundle.properties
@@ -1490,7 +1490,6 @@ dataset.thumbnailsAndWidget.thumbnailImage.title=The logo or image file you wish
 dataset.thumbnailsAndWidget.thumbnailImage.tip=Supported image types are JPEG, TIFF, or PNG and should be no larger than 500 KB. The maximum display size for an image file as a dataset thumbnail is 48 pixels wide by 48 pixels high.
 dataset.thumbnailsAndWidget.thumbnailImage.upload=Upload Image
 dataset.thumbnailsAndWidget.thumbnailImage.upload.invalidMsg=The image could not be uploaded. Please try again with a jpeg, tiff, or png file.
-dataset.thumbnailsAndWidget.thumbnailImage.nonDatasetFile=[not a dataset file...]
 dataset.thumbnailsAndWidget.success=Dataset thumbnail updated.
 
 # file.xhtml

diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetWidgetsPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetWidgetsPage.java
@@ -126,7 +126,7 @@ public void handleImageFileUpload(FileUploadEvent event) {
         String base64image = null;
         try {
             base64image = FileUtil.rescaleImage(file);
-            datasetThumbnail = new DatasetThumbnail("staged", base64image, datasetFileThumbnailToSwitchTo);
+            datasetThumbnail = new DatasetThumbnail(base64image, datasetFileThumbnailToSwitchTo);
         } catch (IOException ex) {
             Logger.getLogger(DatasetWidgetsPage.class.getName()).log(Level.SEVERE, null, ex);
         }
@@ -151,7 +151,7 @@ public void setDataFileAsThumbnail() {
         logger.fine("setDataFileAsThumbnail clicked");
         if (datasetFileThumbnailToSwitchTo != null) {
             String base64image = ImageThumbConverter.getImageThumbAsBase64(datasetFileThumbnailToSwitchTo, ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE);
-            datasetThumbnail = new DatasetThumbnail("staged", base64image, datasetFileThumbnailToSwitchTo);
+            datasetThumbnail = new DatasetThumbnail(base64image, datasetFileThumbnailToSwitchTo);
             userWantsToRemoveThumbnail = false;
         }
     }

diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java b/src/main/java/edu/harvard/iq/dataverse/api/Datasets.java
@@ -582,7 +582,7 @@ public Response getDatasetThumbnailCandidates(@PathParam("id") String idSupplied
             JsonArrayBuilder data = Json.createArrayBuilder();
             boolean considerDatasetLogoAsCandidate = true;
             for (DatasetThumbnail datasetThumbnail : DatasetUtil.getThumbnailCandidates(dataset, considerDatasetLogoAsCandidate)) {
-                data.add(datasetThumbnail.getFilename());
+                data.add(datasetThumbnail.getBase64image());
             }
             return ok(data);
         } catch (WrappedResponse ex) {
@@ -602,11 +602,9 @@ public Response getDatasetThumbnail(@PathParam("id") String idSupplied) {
             Dataset dataset = findDatasetOrDie(idSupplied);
             JsonObjectBuilder data = Json.createObjectBuilder();
             DatasetThumbnail datasetThumbnail = dataset.getDatasetThumbnail(datasetVersionService, fileService);
+            data.add("isUseGenericThumbnail", dataset.isUseGenericThumbnail());
             if (datasetThumbnail != null) {
-                data.add("datasetThumbnail", datasetThumbnail.getFilename());
-                data.add("isUseGenericThumbnail", dataset.isUseGenericThumbnail());
-            } else {
-                data.add("isUseGenericThumbnail", dataset.isUseGenericThumbnail());
+                data.add("datasetThumbnailBase64image", datasetThumbnail.getBase64image());
             }
             return ok(data);
         } catch (WrappedResponse ex) {
@@ -627,7 +625,7 @@ public Response setDataFileAsThumbnail(@PathParam("id") String idSupplied, @Path
                 return error(Response.Status.NOT_FOUND, "Could not find file based on id supplied: " + dataFileIdSupplied + ".");
             }
             Dataset datasetThatMayHaveChanged = datasetService.setDataFileAsThumbnail(dataset, datasetFileThumbnailToSwitchTo);
-            return ok("Thumbnail set to " + datasetThatMayHaveChanged.getDatasetThumbnail(datasetVersionService, fileService).getFilename());
+            return ok("Thumbnail set to " + datasetThatMayHaveChanged.getDatasetThumbnail(datasetVersionService, fileService).getBase64image());
         } catch (WrappedResponse ex) {
             return error(Response.Status.NOT_FOUND, "Could not find dataset based on id supplied: " + idSupplied + ".");
         }
@@ -654,7 +652,7 @@ public Response uploadDatasetLogo(@PathParam("id") String idSupplied, @FormDataP
             }
             Dataset datasetThatMayHaveChanged = datasetService.writeDatasetLogoToStagingArea(dataset, file);
             datasetThatMayHaveChanged = datasetService.moveDatasetLogoFromStagingToFinal(dataset);
-            return ok("Thumbnail is now " + datasetThatMayHaveChanged.getDatasetThumbnail(datasetVersionService, fileService).getFilename());
+            return ok("Thumbnail is now " + datasetThatMayHaveChanged.getDatasetThumbnail(datasetVersionService, fileService).getBase64image());
         } catch (WrappedResponse ex) {
             return error(Response.Status.NOT_FOUND, "Could not find dataset based on id supplied: " + idSupplied + ".");
         }

diff --git a/src/main/java/edu/harvard/iq/dataverse/dataset/DatasetThumbnail.java b/src/main/java/edu/harvard/iq/dataverse/dataset/DatasetThumbnail.java
@@ -4,20 +4,14 @@
 
 public class DatasetThumbnail {
 
-    private final String filename;
     private final String base64image;
     private final DataFile dataFile;
 
-    public DatasetThumbnail(String filename, String base64image, DataFile dataFile) {
-        this.filename = filename;
+    public DatasetThumbnail(String base64image, DataFile dataFile) {
         this.base64image = base64image;
         this.dataFile = dataFile;
     }
 
-    public String getFilename() {
-        return filename;
-    }
-
     public String getBase64image() {
         return base64image;
     }

diff --git a/src/main/java/edu/harvard/iq/dataverse/dataset/DatasetUtil.java b/src/main/java/edu/harvard/iq/dataverse/dataset/DatasetUtil.java
@@ -6,7 +6,6 @@
 import edu.harvard.iq.dataverse.DatasetVersionServiceBean;
 import edu.harvard.iq.dataverse.FileMetadata;
 import edu.harvard.iq.dataverse.dataaccess.ImageThumbConverter;
-import edu.harvard.iq.dataverse.util.BundleUtil;
 import edu.harvard.iq.dataverse.util.FileUtil;
 import java.io.File;
 import java.io.IOException;
@@ -23,7 +22,6 @@ public class DatasetUtil {
     private static final Logger logger = Logger.getLogger(DatasetUtil.class.getCanonicalName());
     public static String datasetLogoFilenameFinal = "dataset_logo_final";
     public static String datasetLogoFilenameStaging = "dataset_logo_staging";
-    public static String datasetLogoNameInGUI = BundleUtil.getStringFromBundle("dataset.thumbnailsAndWidget.thumbnailImage.nonDatasetFile");
 
     public static List<DatasetThumbnail> getThumbnailCandidates(Dataset dataset, boolean considerDatasetLogoAsCandidate) {
         List<DatasetThumbnail> thumbnails = new ArrayList<>();
@@ -37,7 +35,7 @@ public static List<DatasetThumbnail> getThumbnailCandidates(Dataset dataset, boo
                 String imageSourceBase64 = null;
                 try {
                     imageSourceBase64 = FileUtil.rescaleImage(file);
-                    DatasetThumbnail datasetThumbnail = new DatasetThumbnail(DatasetUtil.datasetLogoNameInGUI, imageSourceBase64, null);
+                    DatasetThumbnail datasetThumbnail = new DatasetThumbnail(imageSourceBase64, null);
                     thumbnails.add(datasetThumbnail);
                 } catch (IOException ex) {
                     logger.info("Unable to rescale image: " + ex);
@@ -48,7 +46,7 @@ public static List<DatasetThumbnail> getThumbnailCandidates(Dataset dataset, boo
             DataFile dataFile = fileMetadata.getDataFile();
             if (dataFile != null && dataFile.isImage()) {
                 String imageSourceBase64 = ImageThumbConverter.getImageThumbAsBase64(dataFile, ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE);
-                DatasetThumbnail datasetThumbnail = new DatasetThumbnail(fileMetadata.getLabel(), imageSourceBase64, dataFile);
+                DatasetThumbnail datasetThumbnail = new DatasetThumbnail(imageSourceBase64, dataFile);
                 thumbnails.add(datasetThumbnail);
             }
         }
@@ -66,7 +64,7 @@ public static DatasetThumbnail getThumbnail(Dataset dataset, DatasetVersionServi
             String imageSourceBase64 = null;
             try {
                 imageSourceBase64 = FileUtil.rescaleImage(file);
-                DatasetThumbnail datasetThumbnail = new DatasetThumbnail(DatasetUtil.datasetLogoNameInGUI, imageSourceBase64, null);
+                DatasetThumbnail datasetThumbnail = new DatasetThumbnail(imageSourceBase64, null);
                 logger.info(title + " will get thumbnail from dataset logo.");
                 return datasetThumbnail;
             } catch (IOException ex) {
@@ -79,32 +77,26 @@ public static DatasetThumbnail getThumbnail(Dataset dataset, DatasetVersionServi
                 logger.info(title + " does not have a thumbnail file set but the search card might have one");
                 DatasetThumbnail thumbnailThatMightBeOnSearchCard = findThumbnailThatMightBeShowingOnTheSearchCards(dataset, datasetVersionService, dataFileService);
                 if (thumbnailThatMightBeOnSearchCard != null) {
-                    logger.info(title + " does not have a thumbnail file set but a thumbnail was found as a search card thumbnail. Returning " + thumbnailThatMightBeOnSearchCard.getFilename());
+                    logger.info(title + " does not have a thumbnail file set but a thumbnail was found as a search card thumbnail. Returning " + thumbnailThatMightBeOnSearchCard.getBase64image());
                     return thumbnailThatMightBeOnSearchCard;
                 } else {
                     logger.info(title + " does not have a thumbnail file set but and couldn't find one in use on the search card.");
                     // returning null because dataFile.equals(thumbnailFile) will never match since thumbnailFile is null and there's no point in interating through the files
                     return null;
                 }
             }
-            for (FileMetadata fileMetadata : dataset.getLatestVersion().getFileMetadatas()) {
-                DataFile dataFile = fileMetadata.getDataFile();
-                logger.info(title + " has datafile id of " + dataFile.getId() + " and " + thumbnailFile + " id is " + thumbnailFile.getId() + ".");
-                if (dataFile.equals(thumbnailFile)) {
-                    String imageSourceBase64 = ImageThumbConverter.getImageThumbAsBase64(dataFile, ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE);
-                    String filename = fileMetadata.getLabel();
-                    logger.fine("dataset.getThumbnailFile() is equal to " + filename);
-                    DatasetThumbnail datasetThumbnail = new DatasetThumbnail(filename, imageSourceBase64, dataFile);
-                    logger.info(title + " will get thumbnail from dataset file " + filename);
-                    return datasetThumbnail;
-                }
-            }
-            logger.info(title + " will get the default dataset icon.");
-            return null;
+            String imageSourceBase64 = ImageThumbConverter.getImageThumbAsBase64(thumbnailFile, ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE);
+            DatasetThumbnail datasetThumbnail = new DatasetThumbnail(imageSourceBase64, thumbnailFile);
+            logger.info(title + " will get thumbnail from dataset file " + datasetThumbnail.getBase64image());
+            return datasetThumbnail;
         }
     }
 
     public static DatasetThumbnail findThumbnailThatMightBeShowingOnTheSearchCards(Dataset dataset, DatasetVersionServiceBean datasetVersionService, DataFileServiceBean dataFileService) {
+        boolean disableThisMethodToSeeIfWeCanDeleteIt = false;
+        if (disableThisMethodToSeeIfWeCanDeleteIt) {
+            return null;
+        }
         if (dataset == null) {
             logger.info("Dataset is null so returning null.");
             return null;
@@ -121,7 +113,7 @@ public static DatasetThumbnail findThumbnailThatMightBeShowingOnTheSearchCards(D
                 String randomlySelectedThumbnail = ImageThumbConverter.getImageThumbAsBase64(
                         thumbnailImageFile,
                         ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE);
-                DatasetThumbnail datasetThumbnail = new DatasetThumbnail("randomFromDataFile" + thumbnailImageFile.getId(), randomlySelectedThumbnail, thumbnailImageFile);
+                DatasetThumbnail datasetThumbnail = new DatasetThumbnail(randomlySelectedThumbnail, thumbnailImageFile);
                 return datasetThumbnail;
             }
         }

diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SearchIncludeFragment.java b/src/main/java/edu/harvard/iq/dataverse/search/SearchIncludeFragment.java
@@ -1075,29 +1075,22 @@ public void setDisplayCardValues() {
                 result.setImageUrl(getDataverseCardImageUrl(result));
                 valueSet = true;
             } else if (result.getType().equals("datasets") /*&& result.getEntity() instanceof Dataset*/) {
-                String imageFoundBygetDatasetCardImageUrl = getDatasetCardImageUrl(result);
-                result.setImageUrl(imageFoundBygetDatasetCardImageUrl);
                 /**
-                 * @todo: remove this "true". Still working on centralizing the
-                 * business logic of the GUI and the Search API. Investigate
-                 * more how getDatasetCardImageUrl above works and what the
-                 * implications are of no longer calling it. Can it be moved to
-                 * SearchServiceBean or some other central bean?
+                 * @todo Someday we should probably revert this setImageUrl to
+                 * the original meaning "image_url" to address this issue:
+                 * `image_url` from Search API results no longer yields a
+                 * downloadable image -
+                 * https://github.com/IQSS/dataverse/issues/3616
+                 *
+                 * Note that for datasets, search-include-fragment.xhtml now
+                 * uses the new getDatasetThumbnailBase64image rather than the
+                 * old getImageUrl. This means that we might be able to comment
+                 * out the `result.setImageUrl(getDatasetCardImageUrl` line
+                 * below and all the code it calls.
                  */
-                if (true) {
-                    DatasetThumbnail datasetThumbnail = result.getDatasetThumbnail();
-                    if (datasetThumbnail != null) {
-                        result.setImageUrl(datasetThumbnail.getBase64image());
-                    } else {
-                        if (imageFoundBygetDatasetCardImageUrl != null) {
-                            logger.info("dataset file thumbnail found by imageFoundBygetDatasetCardImageUrl method, using it because dataset.getDatasetThumbnail() was null");
-                            result.setImageUrl(imageFoundBygetDatasetCardImageUrl);
-                        } else {
-                            logger.info("No dataset thumbnail should be shown.");
-                            result.setImageUrl(null);
-                        }
-                    }
-                }
+                result.setImageUrl(getDatasetCardImageUrl(result));
+                result.setDatasetThumbnailBase64image(result.getDatasetThumbnailBase64image());
+
                 valueSet = true;
                 if (result.isHarvested()) {
                     if (harvestedDatasetIds == null) {
@@ -1123,6 +1116,9 @@ public void setDisplayCardValues() {
                 if (result.getImageUrl() != null) {
                     result.setDisplayImage(true);
                 }
+                if (result.getDatasetThumbnailBase64image() != null) {
+                    result.setDisplayImage(true);
+                }
             } else {
                 logger.warning("Index result / entity mismatch (id:resultType) - " + result.getId() + ":" + result.getType());
             }

diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SearchServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/search/SearchServiceBean.java
@@ -507,7 +507,7 @@ public SolrQueryResponse search(DataverseRequest dataverseRequest, Dataverse dat
                 if (dataset != null) {
                     DatasetThumbnail datasetThumbnail = dataset.getDatasetThumbnail(datasetVersionService, dataFileService);
                     if (datasetThumbnail != null) {
-                        solrSearchResult.setThumbnailFilename(datasetThumbnail.getFilename());
+                        solrSearchResult.setDatasetThumbnailBase64image(datasetThumbnail.getBase64image());
                         solrSearchResult.setDatasetThumbnail(datasetThumbnail);
                     }
                 }

diff --git a/src/main/java/edu/harvard/iq/dataverse/search/SolrSearchResult.java b/src/main/java/edu/harvard/iq/dataverse/search/SolrSearchResult.java
@@ -17,8 +17,6 @@
 import javax.json.JsonObject;
 import javax.json.JsonObjectBuilder;
 import static edu.harvard.iq.dataverse.util.json.NullSafeJsonBuilder.jsonObjectBuilder;
-import java.math.BigDecimal;
-import javax.json.JsonValue;
 
 public class SolrSearchResult {
 
@@ -33,13 +31,17 @@ public class SolrSearchResult {
     private String persistentUrl;
     private String downloadUrl;
     private String apiUrl;
-    private String imageUrl;
     /**
-     * @todo consider removing thumbnailFilename since it's available in
-     * DatasetThumbnail.
+     * This is called "imageUrl" because it used to really be a URL. While
+     * performance improvements were being made in the 4.2 timeframe, we started
+     * putting base64 representations of images in this String instead, which
+     * broke the Search API and probably things built on top of it such as
+     * MyData. See "`image_url` from Search API results no longer yields a
+     * downloadable image" at https://github.com/IQSS/dataverse/issues/3616
      */
-    private String thumbnailFilename;
+    private String imageUrl;
     private DatasetThumbnail datasetThumbnail;
+    private String datasetThumbnailBase64image;
     private boolean displayImage;
     private String query;
     private String name;
@@ -545,7 +547,9 @@ public JsonObjectBuilder json(boolean showRelevance, boolean showEntityIds, bool
             }
         }
         if (showThumbnails) {
-            nullSafeJsonBuilder.add("thumbnailFilename", getThumbnailFilename());
+            if (datasetThumbnailBase64image != null) {
+                nullSafeJsonBuilder.add("datasetThumbnailBase64image", datasetThumbnailBase64image);
+            }
         }
         // NullSafeJsonBuilder is awesome but can't build null safe arrays. :(
         if (!datasetAuthors.isEmpty()) {
@@ -646,14 +650,6 @@ public void setImageUrl(String imageUrl) {
         this.imageUrl = imageUrl;
     }
 
-    public String getThumbnailFilename() {
-        return thumbnailFilename;
-    }
-
-    public void setThumbnailFilename(String thumbnailFilename) {
-        this.thumbnailFilename = thumbnailFilename;
-    }
-
     public DatasetThumbnail getDatasetThumbnail() {
         return datasetThumbnail;
     }
@@ -662,6 +658,14 @@ public void setDatasetThumbnail(DatasetThumbnail datasetThumbnail) {
         this.datasetThumbnail = datasetThumbnail;
     }
 
+    public String getDatasetThumbnailBase64image() {
+        return datasetThumbnailBase64image;
+    }
+
+    public void setDatasetThumbnailBase64image(String datasetThumbnailBase64image) {
+        this.datasetThumbnailBase64image = datasetThumbnailBase64image;
+    }
+
     public boolean isDisplayImage() {
         return displayImage;
     }

diff --git a/src/main/webapp/search-include-fragment.xhtml b/src/main/webapp/search-include-fragment.xhtml
@@ -504,7 +504,8 @@
                         </div>
                         <div class="card-preview-icon-block text-center">
                             <a href="#{!SearchIncludeFragment.rootDv and !result.isInTree ? result.datasetUrl : widgetWrapper.wrapURL(result.datasetUrl)}" target="#{(!SearchIncludeFragment.rootDv and !result.isInTree and widgetWrapper.widgetView) or result.harvested ? '_blank' : ''}">
-                                <h:graphicImage value="#{result.imageUrl}" rendered="#{result.displayImage}"/>
+                                <!--FIXME: Double check permissions... are people able to see an image from a restricted file when they shouldn't? What about harvested datasets?-->
+                                <h:graphicImage value="#{result.datasetThumbnailBase64image}" rendered="#{result.displayImage}"/>
                                 <span class="icon-dataset text-info" jsf:rendered="#{!result.displayImage}"/>
                             </a>
                         </div>