From 5ae8418fba4acd9e918ceef6a50188d013610e86 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 3 Jan 2018 16:00:53 -0500 Subject: [PATCH 01/25] stub out modular explore #3657 #4249 --- .../files/root/external-tools/dataExplorer.json | 15 +++++++++++++++ .../harvard/iq/dataverse/FileDownloadHelper.java | 8 ++++++-- .../java/edu/harvard/iq/dataverse/FilePage.java | 10 ++++++++++ .../externaltools/ExternalToolHandler.java | 6 +++--- .../externaltools/ExternalToolServiceBean.java | 1 + .../webapp/file-download-button-fragment.xhtml | 10 +++++++++- 6 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json diff --git a/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json b/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json new file mode 100644 index 00000000000..aba681ba788 --- /dev/null +++ b/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json @@ -0,0 +1,15 @@ +{ + "displayName": "Data Explorer", + "description": "Explore tabular data.", + "toolUrl": "https://scholarsportal.github.io/Dataverse-Data-Explorer/", + "toolParameters": { + "queryParameters": [ + { + "fileid": "{fileId}" + }, + { + "siteUrl": "{siteUrl}" + } + ] + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java index 716d34e6310..b6e1caed20d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java @@ -275,5 +275,9 @@ public DataverseSession getSession() { public void setSession(DataverseSession session) { this.session = session; } - -} + + public boolean isExternalToolsAvailable(){ + return true; + } + + } diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index 853137daaef..9126c3827de 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -22,6 +22,7 @@ import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.export.spi.Exporter; import edu.harvard.iq.dataverse.externaltools.ExternalTool; +import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.FileUtil; @@ -65,6 +66,7 @@ public class FilePage implements java.io.Serializable { private List datasetVersionsForTab; private List fileMetadatasForTab; private List externalTools; + private List externalToolHandlers; @EJB DataFileServiceBean datafileService; @@ -162,6 +164,10 @@ public String init() { List allTools = externalToolService.findAll(); externalTools = externalToolService.findExternalToolsByFile(allTools, file); + externalToolHandlers = new ArrayList<>(); + for (ExternalTool externalTool : allTools) { + externalToolHandlers.add(new ExternalToolHandler(externalTool, file, null)); + } } else { @@ -770,5 +776,9 @@ public String getPublicDownloadUrl() { public List getExternalTools() { return externalTools; } + + public List getExternalToolHandlers() { + return externalToolHandlers; + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java index 9805fb6e9b1..c32b9524e36 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java @@ -3,6 +3,7 @@ import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.authorization.users.ApiToken; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean.ReservedWord; +import edu.harvard.iq.dataverse.util.SystemConfig; import java.io.StringReader; import java.util.ArrayList; import java.util.List; @@ -75,15 +76,14 @@ public String getQueryParametersForUrl() { return "?" + String.join("&", params); } - // TODO: Make this parser much smarter in the future. Tools like Data Explorer will want - // to use reserved words within a value like this: - // "uri": "{siteUrl}/api/access/datafile/{fileId}/metadata/ddi" private String getQueryParam(String key, String value) { ReservedWord reservedWord = ReservedWord.fromString(value); switch (reservedWord) { case FILE_ID: // getDataFile is never null because of the constructor return key + "=" + getDataFile().getId(); + case SITE_URL: + return key + "=" + SystemConfig.getDataverseSiteUrlStatic(); case API_TOKEN: String apiTokenString = null; ApiToken theApiToken = getApiToken(); diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java index 8fb9a205864..84d082b49d4 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java @@ -71,6 +71,7 @@ public enum ReservedWord { // various REST APIs. For example, "Variable substitutions will be made when a variable is named in {brackets}." // from https://swagger.io/specification/#fixed-fields-29 but that's for URLs. FILE_ID("fileId"), + SITE_URL("siteUrl"), API_TOKEN("apiToken"); private final String text; diff --git a/src/main/webapp/file-download-button-fragment.xhtml b/src/main/webapp/file-download-button-fragment.xhtml index 9d344fb0183..06bbfe05dfc 100644 --- a/src/main/webapp/file-download-button-fragment.xhtml +++ b/src/main/webapp/file-download-button-fragment.xhtml @@ -47,12 +47,20 @@ -
+ + +
- #{bundle['acceptTerms']} + + #{bundle['acceptTerms']} + +
\ No newline at end of file From 79161d05efe6f2b5d756d372248933bd023617c5 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Mon, 8 Jan 2018 15:29:31 -0500 Subject: [PATCH 11/25] mention external tools in Data Exploration Guide #3657 --- doc/sphinx-guides/source/user/data-exploration/index.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/sphinx-guides/source/user/data-exploration/index.rst b/doc/sphinx-guides/source/user/data-exploration/index.rst index 708f774bb46..e6af35387b9 100755 --- a/doc/sphinx-guides/source/user/data-exploration/index.rst +++ b/doc/sphinx-guides/source/user/data-exploration/index.rst @@ -6,6 +6,8 @@ Data Exploration Guide ======================================================= +Note that the installation of Dataverse you are using may have additional or different tools configured. Developers interested in creating tools should refer to the :doc:`/installation/external-tools` section of the Installation Guide. + Contents: .. toctree:: From f460d5ba441f50ab6df1acde065cf32894517e5f Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 10 Jan 2018 12:44:49 -0500 Subject: [PATCH 12/25] get TwoRavens working again (FIXME for external tools) #3657 Here we are differentiating between popups for TwoRavens ("explore") and popups for external tools ("externalTool"). --- .../edu/harvard/iq/dataverse/FileDownloadServiceBean.java | 1 + src/main/webapp/file-download-button-fragment.xhtml | 2 +- src/main/webapp/file-download-popup-fragment.xhtml | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java index f2cc37f6548..4a621a72fac 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java @@ -149,6 +149,7 @@ public void startFileDownload(GuestbookResponse guestbookResponse, FileMetadata public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalToolHandler externalToolHandler) { logger.info("explore button clicked... FIXME: Why do we get here from the file page but not the dataset page?"); + // FIXME: Why is externalToolHandler null from the dataset page (NullPointerException thrown) but not the file page? String toolUrl = externalToolHandler.getToolUrlWithQueryParams(); logger.info("Exploring with " + toolUrl); try { diff --git a/src/main/webapp/file-download-button-fragment.xhtml b/src/main/webapp/file-download-button-fragment.xhtml index 37b8c8e674d..be330aff9f4 100644 --- a/src/main/webapp/file-download-button-fragment.xhtml +++ b/src/main/webapp/file-download-button-fragment.xhtml @@ -65,7 +65,7 @@ #{tool.externalTool.displayName}
- #{bundle['acceptTerms']} - #{bundle['acceptTerms']} From ef87fdef6f02e882fce26aa54692ce20f5dcce41 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 10 Jan 2018 13:13:15 -0500 Subject: [PATCH 13/25] get external tool popup working on dataset page #3657 --- .../iq/dataverse/FileDownloadServiceBean.java | 8 +++++--- .../harvard/iq/dataverse/GuestbookResponse.java | 12 ++++++++++++ .../dataverse/GuestbookResponseServiceBean.java | 15 +++++++++++++++ .../webapp/file-download-button-fragment.xhtml | 2 +- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java index 4a621a72fac..76241d7cdf7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java @@ -148,10 +148,12 @@ public void startFileDownload(GuestbookResponse guestbookResponse, FileMetadata } public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalToolHandler externalToolHandler) { - logger.info("explore button clicked... FIXME: Why do we get here from the file page but not the dataset page?"); - // FIXME: Why is externalToolHandler null from the dataset page (NullPointerException thrown) but not the file page? + if (externalToolHandler == null) { + // Must be from the dataset page. + externalToolHandler = guestbookResponse.getExternalToolHandler(); + } String toolUrl = externalToolHandler.getToolUrlWithQueryParams(); - logger.info("Exploring with " + toolUrl); + logger.fine("Exploring with " + toolUrl); try { FacesContext.getCurrentInstance().getExternalContext().redirect(toolUrl); } catch (IOException ex) { diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java index a3790fd32ce..ca259934524 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java @@ -6,6 +6,7 @@ package edu.harvard.iq.dataverse; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; +import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import java.io.Serializable; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -81,6 +82,9 @@ public class GuestbookResponse implements Serializable { @Transient private boolean writeResponse = true; + @Transient + private ExternalToolHandler externalToolHandler; + public boolean isWriteResponse() { return writeResponse; } @@ -106,6 +110,14 @@ public void setFileFormat(String downloadFormat) { this.fileFormat = downloadFormat; } + public ExternalToolHandler getExternalToolHandler() { + return externalToolHandler; + } + + public void setExternalToolHandler(ExternalToolHandler externalToolHandler) { + this.externalToolHandler = externalToolHandler; + } + public GuestbookResponse(){ } diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java index 50fe6f9632f..da04a9a2148 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java @@ -7,6 +7,7 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.User; +import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import static edu.harvard.iq.dataverse.util.JsfHelper.JH; import java.io.IOException; import java.io.OutputStream; @@ -784,6 +785,20 @@ public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetad return in; } + public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetadata fm, String format, ExternalToolHandler externalToolHandler) { + if (in != null && fm.getDataFile() != null) { + in.setFileFormat(format); + in.setDataFile(fm.getDataFile()); + } + if (in != null && fm.getDatasetVersion() != null && fm.getDatasetVersion().isDraft() ) { + in.setWriteResponse(false); + } + if (in != null && externalToolHandler != null) { + in.setExternalToolHandler(externalToolHandler); + } + return in; + } + public Boolean validateGuestbookResponse(GuestbookResponse guestbookResponse, String type) { boolean valid = true; Dataset dataset = guestbookResponse.getDataset(); diff --git a/src/main/webapp/file-download-button-fragment.xhtml b/src/main/webapp/file-download-button-fragment.xhtml index be330aff9f4..eaeadf2cc56 100644 --- a/src/main/webapp/file-download-button-fragment.xhtml +++ b/src/main/webapp/file-download-button-fragment.xhtml @@ -65,7 +65,7 @@ #{tool.externalTool.displayName} Date: Thu, 11 Jan 2018 11:04:25 -0500 Subject: [PATCH 14/25] only show explore tools for tabular data #3657 We don't want to show explore tools for images and such until there is a use case, at which time we can add a column to the externaltool table. --- src/main/java/edu/harvard/iq/dataverse/DatasetPage.java | 6 ++++-- src/main/java/edu/harvard/iq/dataverse/FilePage.java | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index f0371ba6edf..b2f549a88bd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -4074,8 +4074,10 @@ public List getExternalToolHandlersForDataFile(Long fileId) externalToolHandlers = new ArrayList<>(); for (ExternalTool externalTool : allTools) { if (ExternalTool.Type.EXPLORE.equals(externalTool.getType())) { - // TODO: What about the API Token? Data Explorer doesn't need it but TwoRavens will if it's made into an external tool. - externalToolHandlers.add(new ExternalToolHandler(externalTool, dataFile, null)); + if (dataFile.isTabularData()) { + // TODO: What about the API Token? Data Explorer doesn't need it but TwoRavens will if it's made into an external tool. + externalToolHandlers.add(new ExternalToolHandler(externalTool, dataFile, null)); + } } } externalToolHandlersByFileId.put(fileId, externalToolHandlers); //add externalTools to map so we don't have to do the lifting again diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index f4551b61125..fc5d9c0f03f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -176,7 +176,9 @@ public String init() { externalToolHandlers = new ArrayList<>(); for (ExternalTool externalTool : allTools) { if (ExternalTool.Type.EXPLORE.equals(externalTool.getType())) { - externalToolHandlers.add(new ExternalToolHandler(externalTool, file, null)); + if (file.isTabularData()) { + externalToolHandlers.add(new ExternalToolHandler(externalTool, file, null)); + } } } From 9c1502d93a3af7bd8b19dcb52ed84ab617a3c928 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 11 Jan 2018 15:42:29 -0500 Subject: [PATCH 15/25] get validation working on external tools #3657 --- .../harvard/iq/dataverse/FileDownloadHelper.java | 16 +++++++++++++++- .../webapp/file-download-popup-fragment.xhtml | 4 ++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java index 1723ed9e2b1..8b6a907a791 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java @@ -8,6 +8,7 @@ import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.GuestUser; +import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import edu.harvard.iq.dataverse.util.BundleUtil; import static edu.harvard.iq.dataverse.util.JsfHelper.JH; import java.util.ArrayList; @@ -261,7 +262,20 @@ public String startExploreDownloadLink(GuestbookResponse guestbookResponse, File requestContext.execute("PF('downloadPopup').hide()"); return retVal; } - + + public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalToolHandler externalToolHandler) { + + RequestContext requestContext = RequestContext.getCurrentInstance(); + boolean valid = validateGuestbookResponse(guestbookResponse); + + if (!valid) { + return; + } + guestbookResponse.setDownloadtype("Explore"); + fileDownloadService.explore(guestbookResponse, fmd, externalToolHandler); + requestContext.execute("PF('downloadPopup').hide()"); + } + public String startWorldMapDownloadLink(GuestbookResponse guestbookResponse, FileMetadata fmd){ RequestContext requestContext = RequestContext.getCurrentInstance(); diff --git a/src/main/webapp/file-download-popup-fragment.xhtml b/src/main/webapp/file-download-popup-fragment.xhtml index fd5fe55c544..5b091f631bb 100644 --- a/src/main/webapp/file-download-popup-fragment.xhtml +++ b/src/main/webapp/file-download-popup-fragment.xhtml @@ -173,8 +173,8 @@ + action="#{fileDownloadHelper.explore(guestbookResponse, fileMetadata, tool)}" target="_blank" + update="guestbookUIFragment"> #{bundle['acceptTerms']} From 515c3515f9fa060d968be724ceeaa384d28cf0c0 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Jan 2018 10:29:08 -0500 Subject: [PATCH 16/25] move constants from ExternalToolHandler to ExternalTool #3657 --- .../iq/dataverse/externaltools/ExternalTool.java | 16 +++++++++++----- .../externaltools/ExternalToolHandler.java | 6 ------ .../externaltools/ExternalToolServiceBean.java | 10 +++++----- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java index 66f5b102f2d..53750df36c1 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java @@ -20,6 +20,12 @@ @Entity public class ExternalTool implements Serializable { + public static final String DISPLAY_NAME = "displayName"; + public static final String DESCRIPTION = "description"; + public static final String TYPE = "type"; + public static final String TOOL_URL = "toolUrl"; + public static final String TOOL_PARAMETERS = "toolParameters"; + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; @@ -152,11 +158,11 @@ public void setToolParameters(String toolParameters) { public JsonObjectBuilder toJson() { JsonObjectBuilder jab = Json.createObjectBuilder(); jab.add("id", getId()); - jab.add(ExternalToolHandler.DISPLAY_NAME, getDisplayName()); - jab.add(ExternalToolHandler.DESCRIPTION, getDescription()); - jab.add(ExternalToolHandler.TYPE, getType().text); - jab.add(ExternalToolHandler.TOOL_URL, getToolUrl()); - jab.add(ExternalToolHandler.TOOL_PARAMETERS, getToolParameters()); + jab.add(DISPLAY_NAME, getDisplayName()); + jab.add(DESCRIPTION, getDescription()); + jab.add(TYPE, getType().text); + jab.add(TOOL_URL, getToolUrl()); + jab.add(TOOL_PARAMETERS, getToolParameters()); return jab; } diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java index d1197b3ba22..8bc5036768b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java @@ -22,12 +22,6 @@ public class ExternalToolHandler { private static final Logger logger = Logger.getLogger(ExternalToolHandler.class.getCanonicalName()); - public static final String DISPLAY_NAME = "displayName"; - public static final String DESCRIPTION = "description"; - public static final String TYPE = "type"; - public static final String TOOL_URL = "toolUrl"; - public static final String TOOL_PARAMETERS = "toolParameters"; - private final ExternalTool externalTool; private final DataFile dataFile; diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java index ee18e851efa..ed0dfe8afd0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java @@ -1,11 +1,11 @@ package edu.harvard.iq.dataverse.externaltools; import edu.harvard.iq.dataverse.DataFile; -import static edu.harvard.iq.dataverse.externaltools.ExternalToolHandler.DESCRIPTION; -import static edu.harvard.iq.dataverse.externaltools.ExternalToolHandler.DISPLAY_NAME; -import static edu.harvard.iq.dataverse.externaltools.ExternalToolHandler.TOOL_PARAMETERS; -import static edu.harvard.iq.dataverse.externaltools.ExternalToolHandler.TOOL_URL; -import static edu.harvard.iq.dataverse.externaltools.ExternalToolHandler.TYPE; +import static edu.harvard.iq.dataverse.externaltools.ExternalTool.DESCRIPTION; +import static edu.harvard.iq.dataverse.externaltools.ExternalTool.DISPLAY_NAME; +import static edu.harvard.iq.dataverse.externaltools.ExternalTool.TOOL_PARAMETERS; +import static edu.harvard.iq.dataverse.externaltools.ExternalTool.TOOL_URL; +import static edu.harvard.iq.dataverse.externaltools.ExternalTool.TYPE; import java.io.StringReader; import java.util.ArrayList; import java.util.Arrays; From eded8ef81a807d9370b0d501a761e8305db7530d Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Jan 2018 10:38:57 -0500 Subject: [PATCH 17/25] simplify SQL update script (no UPDATE) #3657 We haven't shipped a release with the externaltool table yet so people shouldn't have any existing data. --- scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql b/scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql index b600fa74ea3..6700d30054c 100644 --- a/scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql +++ b/scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql @@ -1,3 +1,2 @@ ALTER TABLE externaltool ADD COLUMN type character varying(255); -UPDATE externaltool SET type = 'CONFIGURE'; ALTER TABLE externaltool ALTER COLUMN type SET NOT NULL; From 04be093ce3763440269aed7c1198e5dfc20e1f06 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Jan 2018 16:17:29 -0500 Subject: [PATCH 18/25] type as param, consolidate after switch from handler to tool #3657 --- .../edu/harvard/iq/dataverse/DatasetPage.java | 66 +++++++------------ .../iq/dataverse/FileDownloadHelper.java | 8 +-- .../iq/dataverse/FileDownloadServiceBean.java | 37 ++++++++--- .../edu/harvard/iq/dataverse/FilePage.java | 37 +++-------- .../iq/dataverse/GuestbookResponse.java | 12 ++-- .../GuestbookResponseServiceBean.java | 11 ++-- .../ExternalToolServiceBean.java | 15 +++++ .../file-configure-dropdown-fragment.xhtml | 2 +- .../file-download-button-fragment.xhtml | 14 ++-- src/main/webapp/file.xhtml | 6 +- src/main/webapp/filesFragment.xhtml | 6 +- 11 files changed, 106 insertions(+), 108 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index bbbf96d776f..807b90843b2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -89,7 +89,6 @@ import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean; import edu.harvard.iq.dataverse.export.SchemaDotOrgExporter; -import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import java.util.Collections; import javax.faces.event.AjaxBehaviorEvent; @@ -255,9 +254,10 @@ public enum DisplayMode { private Boolean hasRsyncScript = false; - List allTools = new ArrayList<>(); - Map> queriedFileTools = new HashMap<>(); - Map> externalToolHandlersByFileId = new HashMap<>(); + List configureTools = new ArrayList<>(); + List exploreTools = new ArrayList<>(); + Map> configureToolsByFileId = new HashMap<>(); + Map> exploreToolsByFileId = new HashMap<>(); public Boolean isHasRsyncScript() { return hasRsyncScript; @@ -1531,9 +1531,10 @@ private String init(boolean initFull) { BundleUtil.getStringFromBundle("file.rsyncUpload.inProgressMessage.details")); } } - - allTools = externalToolService.findAll(); - + + configureTools = externalToolService.findByType(ExternalTool.Type.CONFIGURE); + exploreTools = externalToolService.findByType(ExternalTool.Type.EXPLORE); + return null; } @@ -4044,47 +4045,28 @@ public List getDatasetSummaryFields() { return DatasetUtil.getDatasetSummaryFields(workingVersion, customFields); } - public List getExternalToolsForDataFile(Long fileId) { - List fileTools = queriedFileTools.get(fileId); - if(fileTools != null) { //if already queried before and added to list - return fileTools; + public List getConfigureToolsForDataFile(Long fileId) { + List cachedConfigTools = configureToolsByFileId.get(fileId); + if (cachedConfigTools != null) { //if already queried before and added to list + return cachedConfigTools; } - - DataFile dataFile = datafileService.find(fileId); - fileTools = externalToolService.findExternalToolsByFile(allTools, dataFile); - // TODO: Do we even need "configure" tools right now? Should we delete all this code? - List onlyConfigureTools = new ArrayList<>(); - for (ExternalTool fileTool : fileTools) { - if (ExternalTool.Type.CONFIGURE.equals(fileTool.getType())) { - onlyConfigureTools.add(fileTool); - } - } - fileTools = onlyConfigureTools; - - queriedFileTools.put(fileId, fileTools); //add externalTools to map so we don't have to do the lifting again - - return fileTools; + DataFile dataFile = datafileService.find(fileId); + cachedConfigTools = ExternalToolServiceBean.findExternalToolsByFile(configureTools, dataFile); + configureToolsByFileId.put(fileId, cachedConfigTools); //add to map so we don't have to do the lifting again + return cachedConfigTools; } - public List getExternalToolHandlersForDataFile(Long fileId) { - List externalToolHandlers = externalToolHandlersByFileId.get(fileId); - if (externalToolHandlers != null) { //if already queried before and added to list - return externalToolHandlers; + public List getExploreToolsForDataFile(Long fileId) { + List cachedExploreTools = exploreToolsByFileId.get(fileId); + if (cachedExploreTools != null) { //if already queried before and added to list + return cachedExploreTools; } DataFile dataFile = datafileService.find(fileId); - externalToolHandlers = new ArrayList<>(); - for (ExternalTool externalTool : allTools) { - if (ExternalTool.Type.EXPLORE.equals(externalTool.getType())) { - if (dataFile.isTabularData()) { - // TODO: What about the API Token? Data Explorer doesn't need it but TwoRavens will if it's made into an external tool. - externalToolHandlers.add(new ExternalToolHandler(externalTool, dataFile, null)); - } - } - } - externalToolHandlersByFileId.put(fileId, externalToolHandlers); //add externalTools to map so we don't have to do the lifting again - return externalToolHandlers; + cachedExploreTools = ExternalToolServiceBean.findExternalToolsByFile(exploreTools, dataFile); + exploreToolsByFileId.put(fileId, cachedExploreTools); //add to map so we don't have to do the lifting again + return cachedExploreTools; } - + Boolean thisLatestReleasedVersion = null; public boolean isThisLatestReleasedVersion() { diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java index 8b6a907a791..da538f709c2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java @@ -7,8 +7,7 @@ import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; -import edu.harvard.iq.dataverse.authorization.users.GuestUser; -import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; +import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.util.BundleUtil; import static edu.harvard.iq.dataverse.util.JsfHelper.JH; import java.util.ArrayList; @@ -263,7 +262,7 @@ public String startExploreDownloadLink(GuestbookResponse guestbookResponse, File return retVal; } - public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalToolHandler externalToolHandler) { + public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalTool externalTool) { RequestContext requestContext = RequestContext.getCurrentInstance(); boolean valid = validateGuestbookResponse(guestbookResponse); @@ -271,8 +270,9 @@ public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, Exter if (!valid) { return; } + // Rather that putting "Explore" in the database, we *could* put externalTool.getDisplayName() for "Data Explorer" or whatever. guestbookResponse.setDownloadtype("Explore"); - fileDownloadService.explore(guestbookResponse, fmd, externalToolHandler); + fileDownloadService.explore(guestbookResponse, fmd, externalTool); requestContext.execute("PF('downloadPopup').hide()"); } diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java index f8f50280dcc..6c1a6c9a62b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadServiceBean.java @@ -1,14 +1,16 @@ package edu.harvard.iq.dataverse; +import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.authorization.users.ApiToken; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; -import edu.harvard.iq.dataverse.dataaccess.SwiftAccessIO; +import edu.harvard.iq.dataverse.authorization.users.User; import edu.harvard.iq.dataverse.datasetutility.TwoRavensHelper; import edu.harvard.iq.dataverse.datasetutility.WorldMapPermissionHelper; -import edu.harvard.iq.dataverse.engine.command.Command; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.impl.CreateGuestbookResponseCommand; import edu.harvard.iq.dataverse.engine.command.impl.RequestAccessCommand; +import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import edu.harvard.iq.dataverse.util.FileUtil; import java.io.IOException; @@ -20,16 +22,13 @@ import java.util.logging.Logger; import javax.ejb.EJB; import javax.ejb.Stateless; -import javax.faces.context.ExternalContext; import javax.faces.context.FacesContext; -import javax.faces.event.ValueChangeEvent; import javax.inject.Inject; import javax.inject.Named; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; -import org.primefaces.context.RequestContext; /** * @@ -58,6 +57,8 @@ public class FileDownloadServiceBean implements java.io.Serializable { DataverseServiceBean dataverseService; @EJB UserNotificationServiceBean userNotificationService; + @EJB + AuthenticationServiceBean authService; @Inject DataverseSession session; @@ -149,11 +150,30 @@ public void startFileDownload(GuestbookResponse guestbookResponse, FileMetadata logger.fine("issued file download redirect for filemetadata "+fileMetadata.getId()+", datafile "+fileMetadata.getDataFile().getId()); } - public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalToolHandler externalToolHandler) { - if (externalToolHandler == null) { + public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, ExternalTool externalTool) { + if (externalTool == null) { // Must be from the dataset page. - externalToolHandler = guestbookResponse.getExternalToolHandler(); + logger.fine("null, must be on dataset page"); + externalTool = guestbookResponse.getExternalTool(); + } else { + logger.fine("non-null, must be on file page"); + } + ApiToken apiToken = null; + User user = session.getUser(); + if (user instanceof AuthenticatedUser) { + AuthenticatedUser authenticatedUser = (AuthenticatedUser) user; + apiToken = authService.findApiTokenByUser(authenticatedUser); + } + DataFile dataFile = null; + if (fmd != null) { + dataFile = fmd.getDataFile(); + } else { + if (guestbookResponse != null) { + dataFile = guestbookResponse.getDataFile(); + } } + // Note that the API token might be null. You can end up with "key=null" or whatever. + ExternalToolHandler externalToolHandler = new ExternalToolHandler(externalTool, dataFile, apiToken); String toolUrl = externalToolHandler.getToolUrlWithQueryParams(); logger.fine("Exploring with " + toolUrl); try { @@ -161,6 +181,7 @@ public void explore(GuestbookResponse guestbookResponse, FileMetadata fmd, Exter } catch (IOException ex) { logger.info("Problem exploring with " + toolUrl + " - " + ex); } + // fmd is null from the popup if (guestbookResponse != null && guestbookResponse.isWriteResponse() && ((fmd != null && fmd.getDataFile() != null) || guestbookResponse.getDataFile() != null)) { if (guestbookResponse.getDataFile() == null && fmd != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index 2452790bce0..07faeb8b526 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -22,7 +22,6 @@ import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.export.spi.Exporter; import edu.harvard.iq.dataverse.externaltools.ExternalTool; -import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.FileUtil; @@ -65,8 +64,8 @@ public class FilePage implements java.io.Serializable { private Dataset dataset; private List datasetVersionsForTab; private List fileMetadatasForTab; - private List externalTools; - private List externalToolHandlers; + private List configureTools; + private List exploreTools; @EJB DataFileServiceBean datafileService; @@ -163,27 +162,11 @@ public String init() { // this.getFileDownloadHelper().setGuestbookResponse(guestbookResponse); - List allTools = externalToolService.findAll(); - - externalTools = externalToolService.findExternalToolsByFile(allTools, file); - // TODO: Do we even need "configure" tools right now? Should we delete all this code? - List onlyConfigureTools = new ArrayList<>(); - for (ExternalTool externalTool : externalTools) { - if (ExternalTool.Type.CONFIGURE.equals(externalTool.getType())) { - onlyConfigureTools.add(externalTool); - } + if (file.isTabularData()) { + configureTools = externalToolService.findByType(ExternalTool.Type.CONFIGURE); + exploreTools = externalToolService.findByType(ExternalTool.Type.EXPLORE); } - externalTools = onlyConfigureTools; - externalToolHandlers = new ArrayList<>(); - for (ExternalTool externalTool : allTools) { - if (ExternalTool.Type.EXPLORE.equals(externalTool.getType())) { - if (file.isTabularData()) { - externalToolHandlers.add(new ExternalToolHandler(externalTool, file, null)); - } - } - } - } else { return permissionsWrapper.notFound(); @@ -788,12 +771,12 @@ public String getPublicDownloadUrl() { return FileUtil.getPublicDownloadUrl(systemConfig.getDataverseSiteUrl(), fileId); } - public List getExternalTools() { - return externalTools; + public List getConfigureTools() { + return configureTools; } - public List getExternalToolHandlers() { - return externalToolHandlers; + public List getExploreTools() { + return exploreTools; } - + } diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java index ca259934524..bc1a7d55fa7 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java @@ -6,7 +6,7 @@ package edu.harvard.iq.dataverse; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; -import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; +import edu.harvard.iq.dataverse.externaltools.ExternalTool; import java.io.Serializable; import java.text.SimpleDateFormat; import java.util.ArrayList; @@ -83,7 +83,7 @@ public class GuestbookResponse implements Serializable { private boolean writeResponse = true; @Transient - private ExternalToolHandler externalToolHandler; + private ExternalTool externalTool; public boolean isWriteResponse() { return writeResponse; @@ -110,12 +110,12 @@ public void setFileFormat(String downloadFormat) { this.fileFormat = downloadFormat; } - public ExternalToolHandler getExternalToolHandler() { - return externalToolHandler; + public ExternalTool getExternalTool() { + return externalTool; } - public void setExternalToolHandler(ExternalToolHandler externalToolHandler) { - this.externalToolHandler = externalToolHandler; + public void setExternalTool(ExternalTool externalTool) { + this.externalTool = externalTool; } public GuestbookResponse(){ diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java index ca69f27f5f1..8ce3f3473da 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java @@ -7,9 +7,8 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.User; -import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; +import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.util.BundleUtil; -import static edu.harvard.iq.dataverse.util.JsfHelper.JH; import java.io.IOException; import java.io.OutputStream; import java.text.SimpleDateFormat; @@ -25,8 +24,6 @@ import javax.ejb.TransactionAttribute; import javax.ejb.TransactionAttributeType; import javax.faces.application.FacesMessage; -import javax.faces.component.EditableValueHolder; -import javax.faces.component.UIComponent; import javax.faces.component.UIInput; import javax.faces.context.FacesContext; import javax.faces.model.SelectItem; @@ -786,7 +783,7 @@ public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetad return in; } - public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetadata fm, String format, ExternalToolHandler externalToolHandler) { + public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetadata fm, String format, ExternalTool externalTool) { if (in != null && fm.getDataFile() != null) { in.setFileFormat(format); in.setDataFile(fm.getDataFile()); @@ -794,8 +791,8 @@ public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetad if (in != null && fm.getDatasetVersion() != null && fm.getDatasetVersion().isDraft() ) { in.setWriteResponse(false); } - if (in != null && externalToolHandler != null) { - in.setExternalToolHandler(externalToolHandler); + if (in != null && externalTool != null) { + in.setExternalTool(externalTool); } return in; } diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java index ed0dfe8afd0..2d19c02adcb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java @@ -38,6 +38,21 @@ public List findAll() { return typedQuery.getResultList(); } + + /** + * @return A list of tools or an empty list. + */ + public List findByType(ExternalTool.Type type) { + List externalTools = new ArrayList<>(); + TypedQuery typedQuery = em.createQuery("SELECT OBJECT(o) FROM ExternalTool AS o WHERE o.type = :type", ExternalTool.class); + typedQuery.setParameter("type", type); + List toolsFromQuery = typedQuery.getResultList(); + if (toolsFromQuery != null) { + externalTools = toolsFromQuery; + } + return externalTools; + } + public ExternalTool findById(long id) { TypedQuery typedQuery = em.createQuery("SELECT OBJECT(o) FROM ExternalTool AS o WHERE o.id = :id", ExternalTool.class); typedQuery.setParameter("id", id); diff --git a/src/main/webapp/file-configure-dropdown-fragment.xhtml b/src/main/webapp/file-configure-dropdown-fragment.xhtml index 2ff00d17533..f2e11a4383c 100644 --- a/src/main/webapp/file-configure-dropdown-fragment.xhtml +++ b/src/main/webapp/file-configure-dropdown-fragment.xhtml @@ -24,7 +24,7 @@
  • #{bundle['file.map']}
    • - +
  • - -
    + +
    - + - + - + @@ -170,7 +170,7 @@ - +
    diff --git a/src/main/webapp/filesFragment.xhtml b/src/main/webapp/filesFragment.xhtml index b1769df01b9..a52f0b9f1ec 100644 --- a/src/main/webapp/filesFragment.xhtml +++ b/src/main/webapp/filesFragment.xhtml @@ -365,9 +365,9 @@ - + - + @@ -383,7 +383,7 @@ - +
    • From f9d11d92f2ee9f77f8f1c79ca6945b0f8fa5cb3e Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 12 Jan 2018 16:21:02 -0500 Subject: [PATCH 19/25] remove cruft #3657 --- .../java/edu/harvard/iq/dataverse/FileDownloadHelper.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java index da538f709c2..a3076a0289a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java +++ b/src/main/java/edu/harvard/iq/dataverse/FileDownloadHelper.java @@ -96,7 +96,6 @@ public void setPositionField(UIInput positionField) { private final Map fileDownloadPermissionMap = new HashMap<>(); // { FileMetadata.id : Boolean } -// private List externalToolHandlers; public void nameValueChangeListener(AjaxBehaviorEvent e) { String name= (String) ((UIOutput) e.getSource()).getValue(); @@ -478,5 +477,5 @@ public DataverseSession getSession() { public void setSession(DataverseSession session) { this.session = session; } - - } + +} From 05dd4bfe8942f435d7f9aaeb3ca965431ab3b38e Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Jan 2018 14:24:37 -0500 Subject: [PATCH 20/25] move ReservedWord enum to ExternalTool #3657 --- .../dataverse/externaltools/ExternalTool.java | 49 +++++++++++++++++ .../externaltools/ExternalToolHandler.java | 2 +- .../ExternalToolServiceBean.java | 53 +------------------ 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java index 53750df36c1..fee92b6c0b9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalTool.java @@ -166,4 +166,53 @@ public JsonObjectBuilder toJson() { return jab; } + public enum ReservedWord { + + // TODO: Research if a format like "{reservedWord}" is easily parse-able or if another format would be + // better. The choice of curly braces is somewhat arbitrary, but has been observed in documenation for + // various REST APIs. For example, "Variable substitutions will be made when a variable is named in {brackets}." + // from https://swagger.io/specification/#fixed-fields-29 but that's for URLs. + FILE_ID("fileId"), + SITE_URL("siteUrl"), + API_TOKEN("apiToken"); + + private final String text; + private final String START = "{"; + private final String END = "}"; + + private ReservedWord(final String text) { + this.text = START + text + END; + } + + /** + * This is a centralized method that enforces that only reserved words + * are allowed to be used by external tools. External tool authors + * cannot pass their own query parameters through Dataverse such as + * "mode=mode1". + * + * @throws IllegalArgumentException + */ + public static ReservedWord fromString(String text) throws IllegalArgumentException { + if (text != null) { + for (ReservedWord reservedWord : ReservedWord.values()) { + if (text.equals(reservedWord.text)) { + return reservedWord; + } + } + } + // TODO: Consider switching to a more informative message that enumerates the valid reserved words. + boolean moreInformativeMessage = false; + if (moreInformativeMessage) { + throw new IllegalArgumentException("Unknown reserved word: " + text + ". A reserved word must be one of these values: " + Arrays.asList(ReservedWord.values()) + "."); + } else { + throw new IllegalArgumentException("Unknown reserved word: " + text); + } + } + + @Override + public String toString() { + return text; + } + } + } diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java index 8bc5036768b..3cc7f5bf567 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java @@ -2,7 +2,7 @@ import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.authorization.users.ApiToken; -import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean.ReservedWord; +import edu.harvard.iq.dataverse.externaltools.ExternalTool.ReservedWord; import edu.harvard.iq.dataverse.util.SystemConfig; import java.io.StringReader; import java.util.ArrayList; diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java index 2d19c02adcb..35406a7f22b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolServiceBean.java @@ -1,6 +1,7 @@ package edu.harvard.iq.dataverse.externaltools; import edu.harvard.iq.dataverse.DataFile; +import edu.harvard.iq.dataverse.externaltools.ExternalTool.ReservedWord; import static edu.harvard.iq.dataverse.externaltools.ExternalTool.DESCRIPTION; import static edu.harvard.iq.dataverse.externaltools.ExternalTool.DISPLAY_NAME; import static edu.harvard.iq.dataverse.externaltools.ExternalTool.TOOL_PARAMETERS; @@ -8,7 +9,6 @@ import static edu.harvard.iq.dataverse.externaltools.ExternalTool.TYPE; import java.io.StringReader; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.logging.Logger; @@ -80,55 +80,6 @@ public ExternalTool save(ExternalTool externalTool) { return em.merge(externalTool); } - public enum ReservedWord { - - // TODO: Research if a format like "{reservedWord}" is easily parse-able or if another format would be - // better. The choice of curly braces is somewhat arbitrary, but has been observed in documenation for - // various REST APIs. For example, "Variable substitutions will be made when a variable is named in {brackets}." - // from https://swagger.io/specification/#fixed-fields-29 but that's for URLs. - FILE_ID("fileId"), - SITE_URL("siteUrl"), - API_TOKEN("apiToken"); - - private final String text; - private final String START = "{"; - private final String END = "}"; - - private ReservedWord(final String text) { - this.text = START + text + END; - } - - /** - * This is a centralized method that enforces that only reserved words - * are allowed to be used by external tools. External tool authors - * cannot pass their own query parameters through Dataverse such as - * "mode=mode1". - * - * @throws IllegalArgumentException - */ - public static ReservedWord fromString(String text) throws IllegalArgumentException { - if (text != null) { - for (ReservedWord reservedWord : ReservedWord.values()) { - if (text.equals(reservedWord.text)) { - return reservedWord; - } - } - } - // TODO: Consider switching to a more informative message that enumerates the valid reserved words. - boolean moreInformativeMessage = false; - if (moreInformativeMessage) { - throw new IllegalArgumentException("Unknown reserved word: " + text + ". A reserved word must be one of these values: " + Arrays.asList(ReservedWord.values()) + "."); - } else { - throw new IllegalArgumentException("Unknown reserved word: " + text); - } - } - - @Override - public String toString() { - return text; - } - } - /** * This method takes a list of tools and a file and returns which tools that file supports * The list of tools is passed in so it doesn't hit the database each time @@ -171,7 +122,7 @@ public static ExternalTool parseAddExternalToolManifest(String manifest) { } if (!allRequiredReservedWordsFound) { // Some day there might be more reserved words than just {fileId}. - throw new IllegalArgumentException("Required reserved word not found: " + ReservedWord.FILE_ID.text); + throw new IllegalArgumentException("Required reserved word not found: " + ReservedWord.FILE_ID.toString()); } String toolParameters = toolParametersObj.toString(); return new ExternalTool(displayName, description, type, toolUrl, toolParameters); From b502c21ed1ace3b42217b4db6829a4bd9f35b021 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Jan 2018 14:54:32 -0500 Subject: [PATCH 21/25] consolidate logic into getCachedToolsForDataFile #3657 --- .../edu/harvard/iq/dataverse/DatasetPage.java | 41 ++++++++++++------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 807b90843b2..e071e63dd3d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -4044,27 +4044,38 @@ public List getDatasetSummaryFields() { return DatasetUtil.getDatasetSummaryFields(workingVersion, customFields); } - + public List getConfigureToolsForDataFile(Long fileId) { - List cachedConfigTools = configureToolsByFileId.get(fileId); - if (cachedConfigTools != null) { //if already queried before and added to list - return cachedConfigTools; - } - DataFile dataFile = datafileService.find(fileId); - cachedConfigTools = ExternalToolServiceBean.findExternalToolsByFile(configureTools, dataFile); - configureToolsByFileId.put(fileId, cachedConfigTools); //add to map so we don't have to do the lifting again - return cachedConfigTools; + return getCachedToolsForDataFile(fileId, ExternalTool.Type.CONFIGURE); } public List getExploreToolsForDataFile(Long fileId) { - List cachedExploreTools = exploreToolsByFileId.get(fileId); - if (cachedExploreTools != null) { //if already queried before and added to list - return cachedExploreTools; + return getCachedToolsForDataFile(fileId, ExternalTool.Type.EXPLORE); + } + + public List getCachedToolsForDataFile(Long fileId, ExternalTool.Type type) { + Map> cachedToolsByFileId = new HashMap<>(); + List externalTools = new ArrayList<>(); + switch (type) { + case EXPLORE: + cachedToolsByFileId = exploreToolsByFileId; + externalTools = exploreTools; + break; + case CONFIGURE: + cachedToolsByFileId = configureToolsByFileId; + externalTools = configureTools; + break; + default: + break; + } + List cachedTools = cachedToolsByFileId.get(fileId); + if (cachedTools != null) { //if already queried before and added to list + return cachedTools; } DataFile dataFile = datafileService.find(fileId); - cachedExploreTools = ExternalToolServiceBean.findExternalToolsByFile(exploreTools, dataFile); - exploreToolsByFileId.put(fileId, cachedExploreTools); //add to map so we don't have to do the lifting again - return cachedExploreTools; + cachedTools = ExternalToolServiceBean.findExternalToolsByFile(externalTools, dataFile); + cachedToolsByFileId.put(fileId, cachedTools); //add to map so we don't have to do the lifting again + return cachedTools; } Boolean thisLatestReleasedVersion = null; From 6f48df74fee4a58397f185176c5d4b7f7338d831 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Jan 2018 15:16:57 -0500 Subject: [PATCH 22/25] for null API token, don't pass token string #3657 --- .../files/root/external-tools/dataExplorer.json | 3 +++ .../source/installation/external-tools.rst | 2 +- .../dataverse/externaltools/ExternalToolHandler.java | 12 ++++++++---- .../externaltools/ExternalToolHandlerTest.java | 2 +- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json b/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json index a8be19313f2..85e63950c7e 100644 --- a/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json +++ b/doc/sphinx-guides/source/_static/installation/files/root/external-tools/dataExplorer.json @@ -10,6 +10,9 @@ }, { "siteUrl": "{siteUrl}" + }, + { + "key": "{apiToken}" } ] } diff --git a/doc/sphinx-guides/source/installation/external-tools.rst b/doc/sphinx-guides/source/installation/external-tools.rst index 71ce87c4994..9aead122103 100644 --- a/doc/sphinx-guides/source/installation/external-tools.rst +++ b/doc/sphinx-guides/source/installation/external-tools.rst @@ -19,7 +19,7 @@ In the example above, a mix of required and optional reserved words appear that - ``{fileId}`` (required) - The Dataverse database ID of a file the external tool has been launched on. - ``{siteUrl}`` (optional) - The URL of the Dataverse installation that hosts the file with the fileId above. -- ``{apiToken}`` (optional) - The Dataverse API token of the user launching the external tool. +- ``{apiToken}`` (optional) - The Dataverse API token of the user launching the external tool, if available. Making an External Tool Available in Dataverse ---------------------------------------------- diff --git a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java index 3cc7f5bf567..25b94d3ae78 100644 --- a/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java +++ b/src/main/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandler.java @@ -65,7 +65,10 @@ public String getQueryParametersForUrl() { queryParams.getValuesAs(JsonObject.class).forEach((queryParam) -> { queryParam.keySet().forEach((key) -> { String value = queryParam.getString(key); - params.add(getQueryParam(key, value)); + String param = getQueryParam(key, value); + if (param != null && !param.isEmpty()) { + params.add(getQueryParam(key, value)); + } }); }); return "?" + String.join("&", params); @@ -84,12 +87,13 @@ private String getQueryParam(String key, String value) { ApiToken theApiToken = getApiToken(); if (theApiToken != null) { apiTokenString = theApiToken.getTokenString(); + return key + "=" + apiTokenString; } - return key + "=" + apiTokenString; + break; default: - // We should never reach here. - return null; + break; } + return null; } public String getToolUrlWithQueryParams() { diff --git a/src/test/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandlerTest.java b/src/test/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandlerTest.java index 496c1f2f4a4..02d198b05e8 100644 --- a/src/test/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandlerTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/externaltools/ExternalToolHandlerTest.java @@ -89,7 +89,7 @@ public void testGetToolUrlWithOptionalQueryParameters() { ExternalToolHandler externalToolHandler4 = new ExternalToolHandler(externalTool, dataFile, nullApiToken); String result4 = externalToolHandler4.getQueryParametersForUrl(); System.out.println("result4: " + result4); - assertEquals("?key1=42&key2=null", result4); + assertEquals("?key1=42", result4); // Two query parameters, attempt to use a reserved word that doesn't exist. externalTool.setToolParameters(Json.createObjectBuilder() From 332c3589c2114c8e4e41554e64bf0d013170573a Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Wed, 17 Jan 2018 15:40:12 -0500 Subject: [PATCH 23/25] add comments to null exploreTool workaround #3657 --- .../java/edu/harvard/iq/dataverse/GuestbookResponse.java | 5 +++++ .../harvard/iq/dataverse/GuestbookResponseServiceBean.java | 6 ++++++ src/main/webapp/file-download-button-fragment.xhtml | 1 + src/main/webapp/file-download-popup-fragment.xhtml | 1 + 4 files changed, 13 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java index bc1a7d55fa7..297836385d6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java @@ -82,6 +82,11 @@ public class GuestbookResponse implements Serializable { @Transient private boolean writeResponse = true; + /** + * This transient variable is a place to temporarily retrieve the + * ExternalTool object from the popup when the popup is required on the + * dataset page. + */ @Transient private ExternalTool externalTool; diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java index 8ce3f3473da..0d00bcc1e39 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java @@ -783,6 +783,12 @@ public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetad return in; } + /** + * This method was added because on the dataset page when a popup is + * required, ExternalTool is null in the poup itself. The + * modifyDatafileAndFormat method above was copied and a new argument for + * ExternalTool was added. + */ public GuestbookResponse modifyDatafileAndFormat(GuestbookResponse in, FileMetadata fm, String format, ExternalTool externalTool) { if (in != null && fm.getDataFile() != null) { in.setFileFormat(format); diff --git a/src/main/webapp/file-download-button-fragment.xhtml b/src/main/webapp/file-download-button-fragment.xhtml index 739fd06e2e0..98007b4b207 100644 --- a/src/main/webapp/file-download-button-fragment.xhtml +++ b/src/main/webapp/file-download-button-fragment.xhtml @@ -64,6 +64,7 @@ action="#{fileDownloadService.explore(guestbookResponse, fileMetadata, tool )}" target="_blank"> #{tool.displayName} + + From c6d62e66bf4cef8d0d45ea5f527909ec6af91dff Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Thu, 18 Jan 2018 14:42:12 -0500 Subject: [PATCH 24/25] 4.8.5 has been released, bump version in sql script #3657 --- .../{upgrade_v4.8.4_to_v4.9.sql => upgrade_v4.8.5_to_v4.9.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename scripts/database/upgrades/{upgrade_v4.8.4_to_v4.9.sql => upgrade_v4.8.5_to_v4.9.sql} (100%) diff --git a/scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql b/scripts/database/upgrades/upgrade_v4.8.5_to_v4.9.sql similarity index 100% rename from scripts/database/upgrades/upgrade_v4.8.4_to_v4.9.sql rename to scripts/database/upgrades/upgrade_v4.8.5_to_v4.9.sql From 5dee1268f21b632d5c38b59963e9c39c8a75a266 Mon Sep 17 00:00:00 2001 From: Philip Durbin Date: Fri, 19 Jan 2018 16:07:48 -0500 Subject: [PATCH 25/25] only show explore tools if file can be downloaded #3657 --- src/main/webapp/file-download-button-fragment.xhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/webapp/file-download-button-fragment.xhtml b/src/main/webapp/file-download-button-fragment.xhtml index 98007b4b207..be744091f89 100644 --- a/src/main/webapp/file-download-button-fragment.xhtml +++ b/src/main/webapp/file-download-button-fragment.xhtml @@ -49,7 +49,7 @@ -
    +