Skip to content

Commit

Permalink
Merge pull request #10509 from IQSS/graceful-failure-mode-for-TIFF-im…
Browse files Browse the repository at this point in the history
…ages-failure

catch failures with tiff file thumbnail generation
  • Loading branch information
landreev authored Apr 30, 2024
2 parents 77c7102 + 1efdf9c commit e615050
Show file tree
Hide file tree
Showing 14 changed files with 261 additions and 96 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,7 @@ public boolean isThumbnailAvailable (DataFile file) {
return true;
}
file.setPreviewImageFail(true);
file.setPreviewImageAvailable(false);
this.save(file);
return false;
}
Expand Down
19 changes: 17 additions & 2 deletions src/main/java/edu/harvard/iq/dataverse/DatasetServiceBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -860,18 +860,33 @@ public Dataset setDatasetFileAsThumbnail(Dataset dataset, DataFile datasetFileTh
logger.fine("In setDatasetFileAsThumbnail but dataset is null! Returning null.");
return null;
}
// Just in case the previously designated thumbnail for the dataset was
// a "custom" kind, i.e. an uploaded "dataset_logo" file, the following method
// will try to delete it, and all the associated caches here (because there
// are no other uses for the file). This method is apparently called in all
// cases, without trying to check if the dataset was in fact using a custom
// logo; probably under the assumption that it can't hurt.
DatasetUtil.deleteDatasetLogo(dataset);
dataset.setThumbnailFile(datasetFileThumbnailToSwitchTo);
dataset.setUseGenericThumbnail(false);
return merge(dataset);
}

public Dataset removeDatasetThumbnail(Dataset dataset) {
public Dataset clearDatasetLevelThumbnail(Dataset dataset) {
if (dataset == null) {
logger.fine("In removeDatasetThumbnail but dataset is null! Returning null.");
logger.fine("In clearDatasetLevelThumbnail but dataset is null! Returning null.");
return null;
}

// Just in case the thumbnail that was designated for the dataset was
// a "custom logo" kind, i.e. an uploaded "dataset_logo" file, the following method
// will try to delete it, and all the associated caches here (because there
// are no other uses for the file). This method is apparently called in all
// cases, without trying to check if the dataset was in fact using a custom
// logo; probably under the assumption that it can't hurt.
DatasetUtil.deleteDatasetLogo(dataset);

// Clear any designated thumbnails for the dataset:
dataset.setThumbnailFile(null);
dataset.setUseGenericThumbnail(true);
return merge(dataset);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public String getDatasetCardImageAsUrl(Dataset dataset, Long versionId, boolean
StorageIO<DvObject> storageIO = null;
try {
storageIO = DataAccess.getStorageIO(dataset);
if (storageIO.isAuxObjectCached(DatasetUtil.datasetLogoFilenameFinal)) {
if (storageIO != null && storageIO.isAuxObjectCached(DatasetUtil.datasetLogoFilenameFinal)) {
// If not, return null/use the default, otherwise pass the logo URL
hasDatasetLogo = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public void writeTo(DownloadInstance di, Class<?> clazz, Type type, Annotation[]
storageIO = ImageThumbConverter.getImageThumbnailAsInputStream(storageIO, ImageThumbConverter.DEFAULT_THUMBNAIL_SIZE);
} else {
try {
int size = new Integer(di.getConversionParamValue());
int size = Integer.parseInt(di.getConversionParamValue());
if (size > 0) {
storageIO = ImageThumbConverter.getImageThumbnailAsInputStream(storageIO, size);
}
Expand All @@ -294,8 +294,10 @@ public void writeTo(DownloadInstance di, Class<?> clazz, Type type, Annotation[]
// and, since we now have tabular data files that can
// have thumbnail previews... obviously, we don't want to
// add the variable header to the image stream!
storageIO.setNoVarHeader(Boolean.TRUE);
storageIO.setVarHeader(null);
if (storageIO != null) { // ImageThumbConverter returns null if thumbnail conversion fails
storageIO.setNoVarHeader(Boolean.TRUE);
storageIO.setVarHeader(null);
}
}
} else if (dataFile.isTabularData()) {
logger.fine("request for tabular data download;");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ public static <T extends DvObject> StorageIO<T> createNewStorageIO(T dvObject, S
logger.warning("Could not find storage driver for: " + storageTag);
throw new IOException("createDataAccessObject: Unsupported storage method " + storageDriverId);
}
if (storageIO == null) {
logger.warning("Could not find storage driver for: " + storageTag);
throw new IOException("createDataAccessObject: Unsupported storage method " + storageDriverId);
}
// Note: All storageIO classes must assure that dvObject instances' storageIdentifiers are prepended with
// the <driverId>:// + any additional storageIO type information required (e.g. the bucketname for s3/swift)
// This currently happens when the storageIO is opened for write access
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@
import javax.imageio.stream.ImageOutputStream;

import edu.harvard.iq.dataverse.DataFile;
import edu.harvard.iq.dataverse.DataFileServiceBean;
import edu.harvard.iq.dataverse.util.FileUtil;
import edu.harvard.iq.dataverse.util.SystemConfig;
import java.io.ByteArrayOutputStream;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.OutputStream;
import java.nio.channels.Channel;
import java.nio.channels.Channels;
import java.nio.channels.FileChannel;
import java.nio.channels.ReadableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.logging.Level;
import java.util.logging.Logger;

import jakarta.enterprise.inject.spi.CDI;
import org.apache.commons.io.IOUtils;
//import org.primefaces.util.Base64;
import java.util.Base64;
Expand Down Expand Up @@ -110,6 +110,12 @@ private static boolean isThumbnailAvailable(StorageIO<DataFile> storageIO, int s
return false;
}

// check if thumbnail generation failed:
if (file.isPreviewImageFail()) {
logger.fine("Thumbnail failed to be generated for "+ file.getId());
return false;
}

if (isThumbnailCached(storageIO, size)) {
logger.fine("Found cached thumbnail for " + file.getId());
return true;
Expand All @@ -119,22 +125,23 @@ private static boolean isThumbnailAvailable(StorageIO<DataFile> storageIO, int s
}

private static boolean generateThumbnail(DataFile file, StorageIO<DataFile> storageIO, int size) {
logger.log(Level.FINE, (file.isPreviewImageFail() ? "Not trying" : "Trying") + " to generate thumbnail, file id: " + file.getId());
logger.fine((file.isPreviewImageFail() ? "Not trying" : "Trying") + " to generate thumbnail, file id: " + file.getId());
boolean thumbnailGenerated = false;
// Don't try to generate if there have been failures:
if (!file.isPreviewImageFail()) {
boolean thumbnailGenerated = false;
if (file.getContentType().substring(0, 6).equalsIgnoreCase("image/")) {
thumbnailGenerated = generateImageThumbnail(storageIO, size);
} else if (file.getContentType().equalsIgnoreCase("application/pdf")) {
thumbnailGenerated = generatePDFThumbnail(storageIO, size);
}
if (!thumbnailGenerated) {
file.setPreviewImageFail(true);
file.setPreviewImageAvailable(false);
logger.fine("No thumbnail generated for " + file.getId());
}
return thumbnailGenerated;
}

return false;
return thumbnailGenerated;
}

// Note that this method works on ALL file types for which thumbnail
Expand Down Expand Up @@ -165,15 +172,30 @@ public static InputStreamIO getImageThumbnailAsInputStream(StorageIO<DataFile> s
return null;
}
int cachedThumbnailSize = (int) storageIO.getAuxObjectSize(THUMBNAIL_SUFFIX + size);
InputStreamIO inputStreamIO = cachedThumbnailSize > 0 ? new InputStreamIO(cachedThumbnailInputStream, cachedThumbnailSize) : null;

InputStreamIO inputStreamIO = new InputStreamIO(cachedThumbnailInputStream, cachedThumbnailSize);

inputStreamIO.setMimeType(THUMBNAIL_MIME_TYPE);
if (inputStreamIO != null) {
inputStreamIO.setMimeType(THUMBNAIL_MIME_TYPE);

String fileName = storageIO.getFileName();
if (fileName != null) {
fileName = fileName.replaceAll("\\.[^\\.]*$", THUMBNAIL_FILE_EXTENSION);
inputStreamIO.setFileName(fileName);
String fileName = storageIO.getFileName();
if (fileName != null) {
fileName = fileName.replaceAll("\\.[^\\.]*$", THUMBNAIL_FILE_EXTENSION);
inputStreamIO.setFileName(fileName);
}
} else {
if (storageIO.getDataFile() != null && cachedThumbnailSize == 0) {
// We found an older 0 length thumbnail. Newer image uploads will not have this issue.
// Once cleaned up, this thumbnail will no longer have this issue
logger.warning("Cleaning up zero sized thumbnail ID: "+ storageIO.getDataFile().getId());
storageIO.getDataFile().setPreviewImageFail(true);
storageIO.getDataFile().setPreviewImageAvailable(false);
DataFileServiceBean datafileService = CDI.current().select(DataFileServiceBean.class).get();
datafileService.save(storageIO.getDataFile());

// Now that we have marked this File as a thumbnail failure,
// no reason not to try and delete this 0-size cache here:
storageIO.deleteAuxObject(THUMBNAIL_SUFFIX + size);
}
}
return inputStreamIO;
} catch (Exception ioex) {
Expand Down Expand Up @@ -307,6 +329,7 @@ private static boolean generateImageThumbnail(StorageIO<DataFile> storageIO, int
private static boolean generateImageThumbnailFromInputStream(StorageIO<DataFile> storageIO, int size, InputStream inputStream) {

BufferedImage fullSizeImage;
boolean thumbnailGenerated = false;

try {
logger.fine("attempting to read the image file with ImageIO.read(InputStream), " + storageIO.getDataFile().getStorageIdentifier());
Expand Down Expand Up @@ -359,34 +382,35 @@ private static boolean generateImageThumbnailFromInputStream(StorageIO<DataFile>
try {

rescaleImage(fullSizeImage, width, height, size, outputStream);
/*
// while we are at it, let's make sure other size thumbnails are
// generated too:
for (int s : (new int[]{DEFAULT_PREVIEW_SIZE, DEFAULT_THUMBNAIL_SIZE, DEFAULT_CARDIMAGE_SIZE})) {
if (size != s && !thumbnailFileExists(fileLocation, s)) {
rescaleImage(fullSizeImage, width, height, s, fileLocation);
}
}
*/

if (tempFileRequired) {
storageIO.savePathAsAux(Paths.get(tempFile.getAbsolutePath()), THUMBNAIL_SUFFIX + size);
}
thumbnailGenerated = true;

} catch (Exception ioex) {
logger.warning("Failed to rescale and/or save the image: " + ioex.getMessage());
return false;
thumbnailGenerated = false;
}
finally {
if(tempFileRequired) {
try {
tempFile.delete();
}
catch (Exception e) {}
} else if (!thumbnailGenerated) {
// if it was a local file - let's make sure we are not leaving
// behind a half-baked, broken image - such as a 0-size file -
// if this was a failure.
try {
storageIO.deleteAuxObject(THUMBNAIL_SUFFIX + size);
} catch (IOException ioex) {
logger.fine("Failed attempt to delete the result of a failed thumbnail rescaling; this is most likely ok - for ex., because it was never created in the first place.");
}
}
}

return true;
return thumbnailGenerated;

}

Expand Down Expand Up @@ -544,12 +568,10 @@ private static String getImageAsBase64FromInputStream(InputStream inputStream) {
public static String getImageAsBase64FromFile(File imageFile) {
InputStream imageInputStream = null;
try {

int imageSize = (int) imageFile.length();

imageInputStream = new FileInputStream(imageFile);

return getImageAsBase64FromInputStream(imageInputStream); //, imageSize);
if (imageFile.length() > 0) {
imageInputStream = new FileInputStream(imageFile);
return getImageAsBase64FromInputStream(imageInputStream);
}
} catch (IOException ex) {
// too bad - but not fatal
logger.warning("getImageAsBase64FromFile: Failed to read data from thumbnail file");
Expand Down Expand Up @@ -609,16 +631,12 @@ public static String generateImageThumbnailFromFile(String fileLocation, int siz

logger.fine("image dimensions: " + width + "x" + height);

thumbFileLocation = rescaleImage(fullSizeImage, width, height, size, fileLocation);
return rescaleImage(fullSizeImage, width, height, size, fileLocation);

if (thumbFileLocation != null) {
return thumbFileLocation;
}
} catch (Exception e) {
logger.warning("Failed to read in an image from " + fileLocation + ": " + e.getMessage());
}
return null;

}

/*
Expand Down Expand Up @@ -657,10 +675,14 @@ public static String rescaleImage(BufferedImage fullSizeImage, int width, int he
try {
rescaleImage(fullSizeImage, width, height, size, outputFileStream);
} catch (Exception ioex) {
logger.warning("caught Exceptiopn trying to create rescaled image " + outputLocation);
return null;
logger.warning("caught Exception trying to create rescaled image " + outputLocation);
outputLocation = null;
} finally {
IOUtils.closeQuietly(outputFileStream);
// delete the file if the rescaleImage failed
if (outputLocation == null) {
outputFile.delete();
}
}

return outputLocation;
Expand Down Expand Up @@ -716,13 +738,19 @@ private static void rescaleImage(BufferedImage fullSizeImage, int width, int hei
if (iter.hasNext()) {
writer = (ImageWriter) iter.next();
} else {
throw new IOException("Failed to locatie ImageWriter plugin for image type PNG");
throw new IOException("Failed to locate ImageWriter plugin for image type PNG");
}

BufferedImage lowRes = new BufferedImage(thumbWidth, thumbHeight, BufferedImage.TYPE_INT_ARGB);
Graphics2D g2 = lowRes.createGraphics();
g2.drawImage(thumbImage, 0, 0, null);
g2.dispose();
BufferedImage lowRes = null;
try {
lowRes = new BufferedImage(thumbWidth, thumbHeight, BufferedImage.TYPE_INT_ARGB);
Graphics2D g2 = lowRes.createGraphics();
g2.drawImage(thumbImage, 0, 0, null);
g2.dispose();
} catch (Exception ex) {
logger.warning("Failed to create LoRes Image: " + ex.getMessage());
throw new IOException("Caught exception trying to generate thumbnail: " + ex.getMessage());
}

try (ImageOutputStream ios = ImageIO.createImageOutputStream(outputStream);) {

Expand Down Expand Up @@ -838,6 +866,7 @@ public static String generatePDFThumbnailFromFile(String fileLocation, int size)

// generate the thumbnail for the requested size, *using the already scaled-down
// 400x400 png version, above*:
// (the "exists()" check below appears to be unnecessary - we've already checked early on - ?)
if (!((new File(thumbFileLocation)).exists())) {
thumbFileLocation = runImageMagick(imageMagickExec, previewFileLocation, thumbFileLocation, size, "png");
}
Expand Down
Loading

0 comments on commit e615050

Please sign in to comment.