-
Notifications
You must be signed in to change notification settings - Fork 492
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
standardize image url #10855
standardize image url #10855
Changes from 6 commits
695709c
c80a3df
b923814
18c9211
47a2233
48f9f72
d215221
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
Search API (/api/search) response will now include new image_url format for the Datafile and Dataverse logo. | ||
Note to release note writer: this supersedes the release note 10810-search-api-payload-extensions.md | ||
|
||
For Dataverse: | ||
|
||
- "image_url" (optional) | ||
|
||
```javascript | ||
"items": [ | ||
{ | ||
"name": "Darwin's Finches", | ||
... | ||
"image_url":"/api/access/dvCardImage/{identifier}" | ||
(etc, etc) | ||
``` | ||
|
||
For DataFile: | ||
|
||
- "image_url" (optional) | ||
|
||
```javascript | ||
"items": [ | ||
{ | ||
"name": "test.txt", | ||
... | ||
"image_url":"/api/access/datafile/{identifier}?imageThumb=true" | ||
(etc, etc) | ||
``` |
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -48,6 +48,17 @@ public class ThumbnailServiceWrapper implements java.io.Serializable { | |||||||||||||||
private Map<Long, DvObject> dvobjectViewMap = new HashMap<>(); | ||||||||||||||||
private Map<Long, Boolean> hasThumbMap = new HashMap<>(); | ||||||||||||||||
|
||||||||||||||||
public String getFileCardImageAsUrl(SolrSearchResult result) { | ||||||||||||||||
|
||||||||||||||||
if (!result.isHarvested() && result.getEntity() != null && (!((DataFile)result.getEntity()).isRestricted() | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably use the logic in
dataverse/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java Lines 5831 to 5836 in 9a1e494
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @qqmyers Wouldn't the Access call using the image_url go through the isAccessAuthorized() method anyways? So if we supply the url the user then needs to make the call to get the image which would fail is they don't have access. I don't want to duplicate the code so I would need to make it public and be able to call it from the wrapper to prevent sending the url. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and it would make sense to change the code to public or move the relevant parts to a util class to avoid duplicating code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @qqmyers I went with the second option of copying the code from DatasetPage |
||||||||||||||||
|| permissionsWrapper.hasDownloadFilePermission(result.getEntity())) | ||||||||||||||||
&& isThumbnailAvailable((DataFile) result.getEntity())) { | ||||||||||||||||
return SystemConfig.getDataverseSiteUrlStatic() + "/api/access/datafile/" + result.getEntity().getId() + "?imageThumb=true"; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
return null; | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
// it's the responsibility of the user - to make sure the search result | ||||||||||||||||
// passed to this method is of the Datafile type! | ||||||||||||||||
public String getFileCardImageAsBase64Url(SolrSearchResult result) { | ||||||||||||||||
|
@@ -208,7 +219,13 @@ public String getDatasetCardImageAsUrl(Dataset dataset, Long versionId, boolean | |||||||||||||||
public String getDataverseCardImageAsBase64Url(SolrSearchResult result) { | ||||||||||||||||
return dataverseService.getDataverseLogoThumbnailAsBase64ById(result.getEntityId()); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
|
||||||||||||||||
// it's the responsibility of the user - to make sure the search result | ||||||||||||||||
// passed to this method is of the Dataverse type! | ||||||||||||||||
public String getDataverseCardImageAsUrl(SolrSearchResult result) { | ||||||||||||||||
return dataverseService.getDataverseLogoThumbnailAsUrl(result.getEntityId()); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
public void resetObjectMaps() { | ||||||||||||||||
dvobjectThumbnailsMap = new HashMap<>(); | ||||||||||||||||
dvobjectViewMap = new HashMap<>(); | ||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the image_url is always returned, even though the URL returns a 404 for files and a 204 for collections, then it is not an optional field. Anyway, check my final review comment, where I suggest not adding the fields to the payload in these cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GPortas what if when no image is available the endpoint just returns
image_url: null
?In that way we will know that
image_url
could be either astring
(image exists) ornull
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed image_url if none exists