From 93c5778a7f091b220db70ebf9c92f9fa3ea8d196 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Mon, 25 Mar 2019 10:59:30 -0700 Subject: [PATCH 1/2] Refactor WebDAV URL generation to make it easier to use in other code paths A few misc cleanups as well --- api/src/org/labkey/api/exp/api/ExpData.java | 8 +++ api/src/org/labkey/api/pipeline/PipeRoot.java | 2 + api/src/org/labkey/api/util/PageFlowUtil.java | 2 +- .../labkey/core/junit/JunitController.java | 2 +- .../labkey/experiment/api/ExpDataImpl.java | 56 ++++++++++++++++++- .../experiment/api/ExpDataTableImpl.java | 41 +------------- 6 files changed, 69 insertions(+), 42 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExpData.java b/api/src/org/labkey/api/exp/api/ExpData.java index c70bb7b79ed..7e5339f9fcb 100644 --- a/api/src/org/labkey/api/exp/api/ExpData.java +++ b/api/src/org/labkey/api/exp/api/ExpData.java @@ -16,12 +16,14 @@ package org.labkey.api.exp.api; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.exp.ExperimentDataHandler; import org.labkey.api.exp.ExperimentException; import org.labkey.api.exp.XarSource; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.security.User; +import org.labkey.api.util.URLHelper; import java.io.File; import java.net.URI; @@ -85,4 +87,10 @@ public interface ExpData extends ExpRunItem /** Override to signal that we never throw BatchValidationExceptions */ @Override void save(User user); + + enum PathType { full, serverRelative, folderRelative } + + /** If this file is under the file root for its parent container, return the WebDAV URL that can be used to interact with it */ + @Nullable + String getWebDavURL(@NotNull PathType type); } diff --git a/api/src/org/labkey/api/pipeline/PipeRoot.java b/api/src/org/labkey/api/pipeline/PipeRoot.java index e5d64db04e2..bf70ad1df9f 100644 --- a/api/src/org/labkey/api/pipeline/PipeRoot.java +++ b/api/src/org/labkey/api/pipeline/PipeRoot.java @@ -87,6 +87,8 @@ public interface PipeRoot extends SecurableResource /** @return relative path to the file from the root. null if the file isn't under the root. Does not include a leading slash */ String relativePath(File file); + + /** @return relative path to the file from the root. null if the path isn't under the root. Does not include a leading slash */ String relativePath(Path file); /** @return whether the file specified is a child of the pipeline root */ diff --git a/api/src/org/labkey/api/util/PageFlowUtil.java b/api/src/org/labkey/api/util/PageFlowUtil.java index 21df36cfd1a..c153852c8bd 100644 --- a/api/src/org/labkey/api/util/PageFlowUtil.java +++ b/api/src/org/labkey/api/util/PageFlowUtil.java @@ -1405,7 +1405,7 @@ public static String textLink(String text, URLHelper url, @Nullable String onCli return link(text).href(url).onClick(onClickScript).id(id).attributes(properties).build().toString(); } - public static String iconLink(String iconCls, String tooltip, String url, @Nullable String onClickScript, @Nullable String id, Map properties) + public static String iconLink(String iconCls, String tooltip, @Nullable String url, @Nullable String onClickScript, @Nullable String id, Map properties) { return new LinkBuilder().iconCls(iconCls).tooltip(tooltip).href(url).onClick(onClickScript).id(id).attributes(properties).build().toString(); } diff --git a/core/src/org/labkey/core/junit/JunitController.java b/core/src/org/labkey/core/junit/JunitController.java index 741c91fef08..71d26692e39 100644 --- a/core/src/org/labkey/core/junit/JunitController.java +++ b/core/src/org/labkey/core/junit/JunitController.java @@ -142,7 +142,7 @@ public ModelAndView getView(Object o, BindException errors) public NavTree appendNavTrail(NavTree root) { - return null; + return root.addChild("Unit and integration tests"); } } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataImpl.java index 39adc311e44..72f08a45cea 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataImpl.java @@ -16,7 +16,9 @@ package org.labkey.experiment.api; +import org.apache.commons.io.FilenameUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; @@ -71,6 +73,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.nio.file.Files; +import java.nio.file.InvalidPathException; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; @@ -88,8 +91,9 @@ public class ExpDataImpl extends AbstractRunItemImpl implements ExpData { - public static final SearchService.SearchCategory expDataCategory = new SearchService.SearchCategory("data", "ExpData"); + private static final Logger LOG = Logger.getLogger(ExpDataImpl.class); + public static final SearchService.SearchCategory expDataCategory = new SearchService.SearchCategory("data", "ExpData"); /** * Temporary mapping until experiment.xml contains the mime type @@ -526,6 +530,56 @@ public static ExpDataImpl fromDocumentId(String resourceIdentifier) return ExperimentServiceImpl.get().getExpData(rowId); } + @Nullable + public String getWebDavURL(@NotNull PathType type) + { + if (getFilePath() == null) + return null; + + if (getContainer() == null) + { + return null; + } + + PipeRoot root = PipelineService.get().getPipelineRootSetting(getContainer()); + if (root == null) + return null; + + try + { + java.nio.file.Path path = getFilePath(); + if (path == null) + { + return null; + } + + path = path.toAbsolutePath(); + + //currently only report if the file is under the container for this ExpData + if (root.isUnderRoot(path)) + { + String relPath = root.relativePath(path); + if (relPath == null) + return null; + + relPath = Path.parse(FilenameUtils.separatorsToUnix(relPath)).encode(); + switch (type) + { + case folderRelative: return relPath; + case serverRelative: return root.getWebdavURL() + relPath; + case full: return AppProps.getInstance().getBaseServerUrl() + root.getWebdavURL() + relPath; + default: + throw new IllegalArgumentException("Unexpected path type: " + type); + } + } + } + catch (InvalidPathException e) + { + LOG.error("Invalid path for expData: " + getRowId(), e); + } + return null; + } + public void index(SearchService.IndexTask task) { if (task == null) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataTableImpl.java index f79259a7b43..7f5f583bc65 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataTableImpl.java @@ -784,47 +784,10 @@ public WebDavUrlColumn (ColumnInfo colInfo, boolean relative) @Override public Object getJsonValue(ExpData data) { - if (data == null || data.getFile() == null) + if (data == null) return null; - Container c = data.getContainer(); - if (c == null) - { - return null; - } - - PipeRoot root = PipelineService.get().getPipelineRootSetting(c); - if (root == null) - return null; - - try - { - java.nio.file.Path path = data.getFilePath(); - if (path == null) - { - return null; - } - - path = path.toAbsolutePath(); - - //currently only report if the file is under the container for this ExpData - if (root.isUnderRoot(path)) - { - String relPath = root.relativePath(path); - if (relPath == null) - return null; - - relPath = Path.parse(FilenameUtils.separatorsToUnix(relPath)).encode(); - return _relative ? relPath : root.getWebdavURL() + relPath; - } - } - catch (InvalidPathException e) - { - _log.error("Invalid path for expData: " + data.getRowId(), e); - } - - //NOTE: should we try to see if this is under the site root and resolve across folders? - return null; + return data.getWebDavURL(_relative ? ExpData.PathType.folderRelative : ExpData.PathType.serverRelative); } @Override From 70d8a9442e1164a7791596e243f25389f72a5be9 Mon Sep 17 00:00:00 2001 From: labkey-jeckels Date: Fri, 29 Mar 2019 16:43:02 -0700 Subject: [PATCH 2/2] Code review updates --- .../src/org/labkey/experiment/api/ExpDataImpl.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataImpl.java index 72f08a45cea..a08f9a71ecd 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataImpl.java @@ -533,8 +533,11 @@ public static ExpDataImpl fromDocumentId(String resourceIdentifier) @Nullable public String getWebDavURL(@NotNull PathType type) { - if (getFilePath() == null) + java.nio.file.Path path = getFilePath(); + if (path == null) + { return null; + } if (getContainer() == null) { @@ -547,12 +550,6 @@ public String getWebDavURL(@NotNull PathType type) try { - java.nio.file.Path path = getFilePath(); - if (path == null) - { - return null; - } - path = path.toAbsolutePath(); //currently only report if the file is under the container for this ExpData