Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for publishing breaks designated Dataset thumbnail, messes up Collection page #10820

Merged
merged 7 commits into from
Sep 3, 2024
4 changes: 4 additions & 0 deletions doc/release-notes/10819-publish-thumbnail-bug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
The initial release of the Dataverse v6.3 introduced a bug where publishing would break the dataset thumbnail, which in turn broke the rendering of the parent Collection ("dataverse") page. This problem was fixed in the PR 10820.

Additionally, the same PR made it possible to turn off the feature that automatically selects of one of the image datafiles to serve as the thumbnail of the parent dataset. An admin can turn it off by raising the feature flag `<jvm-options>-Ddataverse.feature.disable-dataset-thumbnail-autoselect=true</jvm-options>`. When the feature is disabled, a user can still manually pick a thumbnail image, or upload a dedicated thumbnail image.

5 changes: 5 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/Dataset.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import edu.harvard.iq.dataverse.license.License;
import edu.harvard.iq.dataverse.makedatacount.DatasetExternalCitations;
import edu.harvard.iq.dataverse.makedatacount.DatasetMetrics;
import edu.harvard.iq.dataverse.settings.FeatureFlags;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.sql.Timestamp;
Expand Down Expand Up @@ -206,6 +207,10 @@ public Dataset(boolean isHarvested) {
StorageUse storageUse = new StorageUse(this);
this.setStorageUse(storageUse);
}

if (FeatureFlags.DISABLE_DATASET_THUMBNAIL_AUTOSELECT.enabled()) {
this.setUseGenericThumbnail(true);
}
}

/**
Expand Down
140 changes: 71 additions & 69 deletions src/main/java/edu/harvard/iq/dataverse/DatasetVersionServiceBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import static edu.harvard.iq.dataverse.batch.jobs.importer.filesystem.FileRecordJobListener.SEP;
import edu.harvard.iq.dataverse.batch.util.LoggingUtil;
import edu.harvard.iq.dataverse.search.SolrSearchResult;
import edu.harvard.iq.dataverse.settings.FeatureFlags;
import edu.harvard.iq.dataverse.settings.SettingsServiceBean;
import edu.harvard.iq.dataverse.util.BundleUtil;
import edu.harvard.iq.dataverse.util.MarkupChecker;
Expand Down Expand Up @@ -807,100 +808,101 @@ public Long getThumbnailByVersionId(Long versionId) {
return null;
}

Long thumbnailFileId;

// First, let's see if there are thumbnails that have already been
// generated:
try {
thumbnailFileId = (Long) em.createNativeQuery("SELECT df.id "
+ "FROM datafile df, filemetadata fm, datasetversion dv, dvobject o "
+ "WHERE dv.id = " + versionId + " "
+ "AND df.id = o.id "
+ "AND fm.datasetversion_id = dv.id "
+ "AND fm.datafile_id = df.id "
+ "AND df.restricted = false "
+ "AND df.embargo_id is null "
+ "AND df.retention_id is null "
+ "AND o.previewImageAvailable = true "
+ "ORDER BY df.id LIMIT 1;").getSingleResult();
} catch (Exception ex) {
thumbnailFileId = null;
}

if (thumbnailFileId != null) {
logger.fine("DatasetVersionService,getThumbnailByVersionid(): found already generated thumbnail for version " + versionId + ": " + thumbnailFileId);
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
}

if (!systemConfig.isThumbnailGenerationDisabledForImages()) {
// OK, let's try and generate an image thumbnail!
long imageThumbnailSizeLimit = systemConfig.getThumbnailSizeLimitImage();
if (!FeatureFlags.DISABLE_DATASET_THUMBNAIL_AUTOSELECT.enabled()) {
Long thumbnailFileId;

// First, let's see if there are thumbnails that have already been
// generated:
try {
thumbnailFileId = (Long) em.createNativeQuery("SELECT df.id "
+ "FROM datafile df, filemetadata fm, datasetversion dv, dvobject o "
+ "WHERE dv.id = " + versionId + " "
+ "AND df.id = o.id "
+ "AND fm.datasetversion_id = dv.id "
+ "AND fm.datafile_id = df.id "
+ "AND o.previewimagefail = false "
+ "AND df.restricted = false "
+ "AND df.embargo_id is null "
+ "AND df.retention_id is null "
+ "AND df.contenttype LIKE 'image/%' "
+ "AND NOT df.contenttype = 'image/fits' "
+ "AND df.filesize < " + imageThumbnailSizeLimit + " "
+ "ORDER BY df.filesize ASC LIMIT 1;").getSingleResult();
+ "AND o.previewImageAvailable = true "
+ "ORDER BY df.id LIMIT 1;").getSingleResult();
} catch (Exception ex) {
thumbnailFileId = null;
}

if (thumbnailFileId != null) {
logger.fine("obtained file id: " + thumbnailFileId);
DataFile thumbnailFile = datafileService.find(thumbnailFileId);
if (thumbnailFile != null) {
if (datafileService.isThumbnailAvailable(thumbnailFile)) {
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
logger.fine("DatasetVersionService,getThumbnailByVersionid(): found already generated thumbnail for version " + versionId + ": " + thumbnailFileId);
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
}

if (!systemConfig.isThumbnailGenerationDisabledForImages()) {
// OK, let's try and generate an image thumbnail!
long imageThumbnailSizeLimit = systemConfig.getThumbnailSizeLimitImage();

try {
thumbnailFileId = (Long) em.createNativeQuery("SELECT df.id "
+ "FROM datafile df, filemetadata fm, datasetversion dv, dvobject o "
+ "WHERE dv.id = " + versionId + " "
+ "AND df.id = o.id "
+ "AND fm.datasetversion_id = dv.id "
+ "AND fm.datafile_id = df.id "
+ "AND o.previewimagefail = false "
+ "AND df.restricted = false "
+ "AND df.embargo_id is null "
+ "AND df.retention_id is null "
+ "AND df.contenttype LIKE 'image/%' "
+ "AND NOT df.contenttype = 'image/fits' "
+ "AND df.filesize < " + imageThumbnailSizeLimit + " "
+ "ORDER BY df.filesize ASC LIMIT 1;").getSingleResult();
} catch (Exception ex) {
thumbnailFileId = null;
}

if (thumbnailFileId != null) {
logger.fine("obtained file id: " + thumbnailFileId);
DataFile thumbnailFile = datafileService.find(thumbnailFileId);
if (thumbnailFile != null) {
if (datafileService.isThumbnailAvailable(thumbnailFile)) {
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
}
}
}
}
}

// And if that didn't work, try the same thing for PDFs:
if (!systemConfig.isThumbnailGenerationDisabledForPDF()) {
// OK, let's try and generate an image thumbnail!
long imageThumbnailSizeLimit = systemConfig.getThumbnailSizeLimitPDF();
try {
thumbnailFileId = (Long) em.createNativeQuery("SELECT df.id "
+ "FROM datafile df, filemetadata fm, datasetversion dv, dvobject o "
+ "WHERE dv.id = " + versionId + " "
+ "AND df.id = o.id "
+ "AND fm.datasetversion_id = dv.id "
+ "AND fm.datafile_id = df.id "
+ "AND o.previewimagefail = false "
+ "AND df.restricted = false "
+ "AND df.embargo_id is null "
+ "AND df.retention_id is null "
+ "AND df.contenttype = 'application/pdf' "
+ "AND df.filesize < " + imageThumbnailSizeLimit + " "
+ "ORDER BY df.filesize ASC LIMIT 1;").getSingleResult();
} catch (Exception ex) {
thumbnailFileId = null;
}
// And if that didn't work, try the same thing for PDFs:
if (!systemConfig.isThumbnailGenerationDisabledForPDF()) {
// OK, let's try and generate an image thumbnail!
long imageThumbnailSizeLimit = systemConfig.getThumbnailSizeLimitPDF();
try {
thumbnailFileId = (Long) em.createNativeQuery("SELECT df.id "
+ "FROM datafile df, filemetadata fm, datasetversion dv, dvobject o "
+ "WHERE dv.id = " + versionId + " "
+ "AND df.id = o.id "
+ "AND fm.datasetversion_id = dv.id "
+ "AND fm.datafile_id = df.id "
+ "AND o.previewimagefail = false "
+ "AND df.restricted = false "
+ "AND df.embargo_id is null "
+ "AND df.retention_id is null "
+ "AND df.contenttype = 'application/pdf' "
+ "AND df.filesize < " + imageThumbnailSizeLimit + " "
+ "ORDER BY df.filesize ASC LIMIT 1;").getSingleResult();
} catch (Exception ex) {
thumbnailFileId = null;
}

if (thumbnailFileId != null) {
DataFile thumbnailFile = datafileService.find(thumbnailFileId);
if (thumbnailFile != null) {
if (datafileService.isThumbnailAvailable(thumbnailFile)) {
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
if (thumbnailFileId != null) {
DataFile thumbnailFile = datafileService.find(thumbnailFileId);
if (thumbnailFile != null) {
if (datafileService.isThumbnailAvailable(thumbnailFile)) {
assignDatasetThumbnailByNativeQuery(versionId, thumbnailFileId);
return thumbnailFileId;
}
}
}
}
}

return null;
}

Expand Down
10 changes: 10 additions & 0 deletions src/main/java/edu/harvard/iq/dataverse/settings/FeatureFlags.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ public enum FeatureFlags {
* @since Dataverse 6.3
*/
DISABLE_RETURN_TO_AUTHOR_REASON("disable-return-to-author-reason"),
/**
* This flag disables the feature that automatically selects one of the
* DataFile thumbnails in the dataset/version as the dedicated thumbnail
* for the dataset.
*
* @apiNote Raise flag by setting
* "dataverse.feature.enable-dataset-thumbnail-autoselect"
* @since Dataverse 6.4
*/
DISABLE_DATASET_THUMBNAIL_AUTOSELECT("disable-dataset-thumbnail-autoselect"),
;

final String flag;
Expand Down
2 changes: 1 addition & 1 deletion src/main/webapp/dataset.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
<meta property="og:title" content="#{DatasetPage.title}" />
<meta property="og:type" content="article" />
<meta property="og:url" content="#{DatasetPage.dataverseSiteUrl}/dataset.xhtml?persistentId=#{dataset.globalId}" />
<meta property="og:image" content="#{DatasetPage.dataset.getDatasetThumbnail(ImageThumbConverter.DEFAULT_PREVIEW_SIZE) == null ? DatasetPage.dataverseSiteUrl.concat(resource['images/dataverse-icon-1200.png']): DatasetPage.dataverseSiteUrl.concat('/api/datasets/:persistentId/thumbnail?persistentId=').concat(DatasetPage.dataset.getGlobalId().asString())}" />
<meta property="og:image" content="#{empty DatasetPage.thumbnailString ? DatasetPage.dataverseSiteUrl.concat(resource['images/dataverse-icon-1200.png']): DatasetPage.dataverseSiteUrl.concat('/api/datasets/:persistentId/thumbnail?persistentId=').concat(DatasetPage.dataset.getGlobalId().asString())}" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Part of #10687 was a different fix for this line - looks like it was asking for a size 0 preview. This fix here is fine - neither are actually asking for the same size that the api call returns (ImageThumbConverter.DEFAULT_CARDIMAGE_SIZE), so using a cached value makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, I completely missed all that - that you already had an issue and a PR for the bug in question. :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o:importConstants, huh - good to know.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yes, my logic was that if we have a 140px thumbnail available, generating the tiny one should not be a problem either.
I will add a test for if (size == 0) { ... return null; } to DatasetUtil.java though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's in the other PR I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I meant, that I was going to copy-and-paste these lines from #10687; done.
Same for the release note, added the recommendations to use the Clear apis, decided not to mention the direct database query.

<meta property="og:site_name" content="#{DatasetPage.publisher}" />
<meta property="og:description" content="#{(DatasetPage.description.length()>150 ? DatasetPage.description.substring(0,147).concat('...') : DatasetPage.description)}" />
<ui:repeat var="author" value="#{DatasetPage.datasetAuthors}">
Expand Down
Loading