Refactor Resource interface (#5422)

* Path.getPathPart() FileUtil.appendName() * NPE * Path.Part may contain any character * NPE * remove unnecessary toString() * MockServlet null check
LabKey · May 2, 2024 · 5a4ec2f · 5a4ec2f
1 parent 2c84204
commit 5a4ec2f
Show file tree

Hide file tree

Showing 40 changed files with 350 additions and 147 deletions.
diff --git a/api/src/org/labkey/api/files/FileContentService.java b/api/src/org/labkey/api/files/FileContentService.java
@@ -33,8 +33,8 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URI;
 import java.nio.file.Path;
+import java.net.URI;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;

diff --git a/api/src/org/labkey/api/module/ModuleHtmlViewDefinition.java b/api/src/org/labkey/api/module/ModuleHtmlViewDefinition.java
@@ -27,6 +27,7 @@
 import org.labkey.api.util.HtmlString;
 import org.labkey.api.util.HtmlStringBuilder;
 import org.labkey.api.util.MinorConfigurationException;
+import org.labkey.api.util.Path;
 import org.labkey.api.util.StringUtilsLabKey;
 import org.labkey.api.util.XmlBeansUtil;
 import org.labkey.api.view.WebPartView;
@@ -92,7 +93,7 @@ public ModuleHtmlViewDefinition(Resource r)
         Resource parent = r.parent();
         if (parent != null)
         {
-            Resource metadataResource = parent.find(_name + VIEW_METADATA_EXTENSION);
+            Resource metadataResource = parent.find(Path.toPathPart(_name + VIEW_METADATA_EXTENSION));
             if (metadataResource != null)
                 parseMetadata(metadataResource);
         }

diff --git a/api/src/org/labkey/api/module/ModuleResourceCache.java b/api/src/org/labkey/api/module/ModuleResourceCache.java
@@ -317,7 +317,7 @@ public Resource parent()
         }
 
         @Override
-        public Resource find(String name)
+        public Resource find(Path.Part name)
         {
             ensureListener();
 

diff --git a/api/src/org/labkey/api/module/ModuleResourceResolver.java b/api/src/org/labkey/api/module/ModuleResourceResolver.java
@@ -145,8 +145,8 @@ public Resource lookup(Path path)
         Resource r = _root;
         for (int i=0 ; i<path.size() ; i++)
         {
-            String p = path.get(i);
-            if (null == p || filter(p))
+            Path.Part p = path.getPart(i);
+            if (null == p || filter(p.toString()))
                 return null;
             r = r.find(p);
             if (null == r)

diff --git a/api/src/org/labkey/api/module/ResourceRootProvider.java b/api/src/org/labkey/api/module/ResourceRootProvider.java
@@ -104,7 +104,7 @@ static ResourceRootProvider getAssayProviders(Path path)
     {
         return (topRoot, roots) ->
         {
-            Resource assayRoot = topRoot.find(AssayService.ASSAY_DIR_NAME);
+            Resource assayRoot = topRoot.find(Path.toPathPart(AssayService.ASSAY_DIR_NAME));
 
             if (null != assayRoot && assayRoot.isCollection())
             {

diff --git a/api/src/org/labkey/api/query/QueryService.java b/api/src/org/labkey/api/query/QueryService.java
@@ -17,6 +17,7 @@
 package org.labkey.api.query;
 
 import org.apache.commons.collections4.SetValuedMap;
+import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.json.JSONObject;
@@ -547,7 +548,7 @@ public DependencyObject(@NotNull DependencyType type, @NotNull Container c, @Not
             this.schemaKey = key;
             this.name = name;
             this.url = url;
-            this.key = new Path(container.getId(), type.name(), schemaKey.encode(), name).toString().toLowerCase();
+            this.key = new Path(container.getId(), type.name(), schemaKey.toString().toLowerCase(), name.toLowerCase()).encode();
         }
 
         @Override

diff --git a/api/src/org/labkey/api/reports/report/ModuleReportResource.java b/api/src/org/labkey/api/reports/report/ModuleReportResource.java
@@ -21,6 +21,7 @@
 import org.labkey.api.resource.Resource;
 import org.labkey.api.util.DateUtil;
 import org.labkey.api.util.PageFlowUtil;
+import org.labkey.api.util.Path;
 import org.labkey.query.xml.ReportDescriptorType;
 
 import java.io.IOException;
@@ -45,7 +46,7 @@ public ModuleReportResource(ReportDescriptor reportDescriptor, Resource sourceFi
         _reportDescriptor = reportDescriptor;
         _sourceFile = sourceFile;
         Resource dir = sourceFile.parent();
-        _metaDataFile = dir.find(_reportDescriptor.getReportName() + ScriptReportDescriptor.REPORT_METADATA_EXTENSION);
+        _metaDataFile = dir.find(Path.toPathPart(_reportDescriptor.getReportName() + ScriptReportDescriptor.REPORT_METADATA_EXTENSION));
     }
 
     public void loadScript()

diff --git a/api/src/org/labkey/api/reports/report/view/ReportUtil.java b/api/src/org/labkey/api/reports/report/view/ReportUtil.java
@@ -71,6 +71,7 @@
 import org.labkey.api.study.StudyService;
 import org.labkey.api.thumbnail.ThumbnailService.ImageType;
 import org.labkey.api.util.PageFlowUtil;
+import org.labkey.api.util.Path;
 import org.labkey.api.util.ThumbnailUtil;
 import org.labkey.api.util.URLHelper;
 import org.labkey.api.util.UniqueID;
@@ -188,7 +189,7 @@ public static Resource getModuleImageFile(Report r, String fileTypePrefix)
         Resource parent = descriptor.getSourceFile().parent();
         if (null == parent)
             return null;
-        Resource attachmentDir = parent.find(ReportUtil.getSerializedName(r.getDescriptor()));
+        Resource attachmentDir = parent.find(Path.toPathPart(ReportUtil.getSerializedName(r.getDescriptor())));
         if (null == attachmentDir)
             return null;
         String imageFileName = null;
@@ -204,7 +205,7 @@ public static Resource getModuleImageFile(Report r, String fileTypePrefix)
         if (null == imageFileName)
             return null;
 
-        return attachmentDir.find(imageFileName);
+        return attachmentDir.find(Path.toPathPart(imageFileName));
     }
 
     // Consider: combine these two methods... need standard way to get static thumbnail and icon url

diff --git a/api/src/org/labkey/api/resource/AbstractResource.java b/api/src/org/labkey/api/resource/AbstractResource.java
@@ -97,7 +97,7 @@ public boolean isCollectionType()
     }
 
     @Override
-    public Resource find(String name)
+    public Resource find(Path.Part name)
     {
         return null;
     }

diff --git a/api/src/org/labkey/api/resource/AbstractResourceCollection.java b/api/src/org/labkey/api/resource/AbstractResourceCollection.java
@@ -76,7 +76,7 @@ public Collection<? extends Resource> list()
         Collection<Resource> list = new ArrayList<>(names.size());
         for (String name : names)
         {
-            Resource r = find(name);
+            Resource r = find(Path.toPathPart(name));
             if (r != null)
                 list.add(r);
         }

diff --git a/api/src/org/labkey/api/resource/DirectoryResource.java b/api/src/org/labkey/api/resource/DirectoryResource.java
@@ -136,9 +136,9 @@ public boolean isCollection()
     }
 
     @Override
-    public Resource find(String name)
+    public Resource find(Path.Part name)
     {
-        return getChildren().get(name);
+        return getChildren().get(name.toString());
     }
 
     @Override

diff --git a/api/src/org/labkey/api/resource/Resource.java b/api/src/org/labkey/api/resource/Resource.java
@@ -41,19 +41,20 @@ public interface Resource
     // should really be 'isResource()'
     boolean isCollection();
 
-    Resource find(String name);
+    Resource find(Path.Part name);
 
     /**
-     * Traverse a relative folder/collection path from this resource, invoking find(String) on each part
+     * Traverse a relative folder/collection path from this resource, invoking find(PathPart) on each part
      * @param path The path to traverse
      * @return The collection resource at the requested location or null if path is invalid
      */
     default Resource find(Path path)
     {
         Resource r = this;
 
-        for (String part : path)
+        for (int i=0 ; i<path.size() ; i++)
         {
+            Path.Part part = path.getPart(i);
             r = r.find(part);
 
             if (null == r || !r.isCollection())

diff --git a/api/src/org/labkey/api/resource/ResourceWrapper.java b/api/src/org/labkey/api/resource/ResourceWrapper.java
@@ -70,7 +70,7 @@ public boolean isCollection()
     }
 
     @Override
-    public Resource find(String name)
+    public Resource find(Path.Part name)
     {
         return _resource.find(name);
     }

diff --git a/api/src/org/labkey/api/util/FileUtil.java b/api/src/org/labkey/api/util/FileUtil.java
@@ -626,6 +626,7 @@ public static URI createUri(String str, boolean isEncoded)
         }
     }
 
+
     @NotNull
     public static String getFileName(Path fullPath)
     {
@@ -641,6 +642,48 @@ public static String getFileName(Path fullPath)
         }
     }
 
+
+    /* Only returns a child path */
+    public static File appendPath(File dir, org.labkey.api.util.Path path)
+    {
+        path = path.normalize();
+        if (path.size() > 0 && "..".equals(path.get(0)))
+            throw new IllegalArgumentException(path.toString());
+        var ret = new File(dir, path.toString());
+        if (!URIUtil.isDescendant(dir.toURI(), ret.toURI()))
+            throw new IllegalArgumentException(path.toString());
+        return ret;
+    }
+
+
+    /* Only returns an immediate child */
+    public static File appendName(File dir, org.labkey.api.util.Path.Part part)
+    {
+        return appendName(dir, part.toString());
+    }
+
+
+    /* Only returns an immediate child */
+    public static File appendName(File dir, String... parts)
+    {
+        File ret = dir;
+
+        for (String name : parts)
+        {
+            if (StringUtils.contains(name, '/'))
+                throw new IllegalArgumentException(name);
+            if (StringUtils.contains(name, File.separatorChar))
+                throw new IllegalArgumentException(name);
+            if (".".equals(name) || "..".equals(name))
+                throw new IllegalArgumentException(name);
+            ret = new File(dir, name);
+        }
+        if (!URIUtil.isDescendant(dir.toURI(), ret.toURI()))
+            throw new IllegalArgumentException(StringUtils.join(parts,File.separatorChar));
+        return ret;
+    }
+
+
     public static String decodeSpaces(@NotNull String str)
     {
         return str.replace("%20", " ");