From d2da1cecd88567a150dad02fac228406102306b3 Mon Sep 17 00:00:00 2001 From: gnaegi <none@none> Date: Wed, 21 Jan 2015 12:16:40 +0100 Subject: [PATCH] OO-1162 fix NPE with subdirectories, refactor resolve-or-create methods to VFSManager --- .../filechooser/FileCreatorController.java | 33 +---- .../LinkFileCombiCalloutController.java | 18 +-- .../modules/bc/FileUploadController.java | 44 ++----- .../org/olat/core/util/vfs/VFSManager.java | 118 ++++++++++++++++++ 4 files changed, 132 insertions(+), 81 deletions(-) diff --git a/src/main/java/org/olat/core/commons/controllers/filechooser/FileCreatorController.java b/src/main/java/org/olat/core/commons/controllers/filechooser/FileCreatorController.java index f44fe12e872..e8f1f482263 100644 --- a/src/main/java/org/olat/core/commons/controllers/filechooser/FileCreatorController.java +++ b/src/main/java/org/olat/core/commons/controllers/filechooser/FileCreatorController.java @@ -39,6 +39,7 @@ import org.olat.core.util.Util; import org.olat.core.util.vfs.VFSContainer; import org.olat.core.util.vfs.VFSItem; import org.olat.core.util.vfs.VFSLeaf; +import org.olat.core.util.vfs.VFSManager; /** * Description: @@ -209,36 +210,12 @@ public class FileCreatorController extends FormBasicController { @Override protected void formOK(UserRequest ureq) { // 1: Get parent container for new file - VFSContainer parentContainer = baseContainer; String uploadRelPath = targetSubPath.getValue(); - if (StringHelper.containsNonWhitespace(uploadRelPath)){ - // Try to resolve given rel path from current container - VFSItem resolvedPath = baseContainer.resolve(uploadRelPath); - if (resolvedPath == null) { - // Does not yet exist - create subdir - String[] pathSegments = uploadRelPath.split("/"); - for (int i = 0; i < pathSegments.length; i++) { - String segment = pathSegments[i]; - if (StringHelper.containsNonWhitespace(segment)) { - VFSContainer newParentContainer = parentContainer.createChildContainer(segment.trim()); - if (newParentContainer == null) { - // Huh? don't know what to do, use last folder level that could be created - logError("Could not create container with name::" + segment + " in relPath::" + uploadRelPath, null); - break; - } else { - parentContainer = newParentContainer; - } - } - } - } else { - // Parent dir already exists, make sure this is really a container and not a file! - if (resolvedPath instanceof VFSContainer) { - parentContainer = (VFSContainer) resolvedPath; - } - // else: boy, don't know what to do. just use base container - } + VFSContainer parentContainer = VFSManager.resolveOrCreateContainerFromPath(baseContainer, uploadRelPath); + if (parentContainer == null) { + logError("Can not create target sub path::" + uploadRelPath + ", fall back to base container", null); + parentContainer = baseContainer; } - // else: no rel path - use base as default parent // 2: Create empty file in parent String fileName = fileNameElement.getValue(); diff --git a/src/main/java/org/olat/core/commons/controllers/filechooser/LinkFileCombiCalloutController.java b/src/main/java/org/olat/core/commons/controllers/filechooser/LinkFileCombiCalloutController.java index 8706feb8135..b30df51842e 100644 --- a/src/main/java/org/olat/core/commons/controllers/filechooser/LinkFileCombiCalloutController.java +++ b/src/main/java/org/olat/core/commons/controllers/filechooser/LinkFileCombiCalloutController.java @@ -233,23 +233,7 @@ public class LinkFileCombiCalloutController extends BasicController { private void doOpenWysiwygEditor(UserRequest ureq) { if(relFilPathIsProposal){ - file = (VFSLeaf) baseContainer.resolve(relFilePath); - if (file == null) { - // Expected: file does not exist, create it now. - String[] pathSegments = relFilePath.split("/"); - VFSContainer parent = baseContainer; - for (int i = 0; i < pathSegments.length; i++) { - String segment = pathSegments[i]; - if (StringHelper.containsNonWhitespace(segment)) { - if (i == pathSegments.length -1) { - // last one is leaf - file = parent.createChildLeaf(segment); - } else { - parent = parent.createChildContainer(segment); - } - } - } - } + file = VFSManager.resolveOrCreateLeafFromPath(baseContainer, relFilePath); } if (file == null) { // huh? no idea what happend, do nothing and log error diff --git a/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java b/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java index be54493f108..4952cad1f84 100644 --- a/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java +++ b/src/main/java/org/olat/core/commons/modules/bc/FileUploadController.java @@ -71,7 +71,6 @@ import org.olat.core.util.Formatter; import org.olat.core.util.StringHelper; import org.olat.core.util.WebappHelper; import org.olat.core.util.vfs.LocalImpl; -import org.olat.core.util.vfs.VFSConstants; import org.olat.core.util.vfs.VFSContainer; import org.olat.core.util.vfs.VFSItem; import org.olat.core.util.vfs.VFSLeaf; @@ -765,41 +764,14 @@ public class FileUploadController extends FormBasicController { */ public void setUploadRelPath(String uploadRelPath) { this.uploadRelPath = uploadRelPath; - // reset to current base container as default - uploadVFSContainer = currentContainer; - // resolve upload dir from rel upload path - if (StringHelper.containsNonWhitespace(uploadRelPath)) { - // try to resolve given rel path from current container - VFSItem uploadDir = currentContainer.resolve(uploadRelPath); - if (uploadDir != null) { - // make sure this is really a container and not a file! - if (uploadDir instanceof VFSContainer) { - uploadVFSContainer = (VFSContainer) uploadDir; - } else { - // fallback to current base - uploadVFSContainer = currentContainer; - } - } else { - // does not yet exist - create subdir - if (VFSConstants.YES.equals(currentContainer.canWrite())) { - String[] pathSegments = uploadRelPath.split("/"); - for (int i = 0; i < pathSegments.length; i++) { - String segment = pathSegments[i]; - if (StringHelper.containsNonWhitespace(segment)) { - VFSContainer newUploadContainer = uploadVFSContainer.createChildContainer(segment.trim()); - if (newUploadContainer == null) { - // Huh? don't know what to do, use last folder level that could be created - logError("Could not create container with name::" + segment + " in relPath::" + uploadRelPath, null); - break; - } else { - uploadVFSContainer = newUploadContainer; - } - } - } - } - } - } - // update the destination path in the GUI + // Set upload directory from path + uploadVFSContainer = VFSManager.resolveOrCreateContainerFromPath(currentContainer, uploadRelPath); + if (uploadVFSContainer == null) { + logError("Can not create upload rel path::" + uploadRelPath + ", fall back to current container", null); + uploadVFSContainer = currentContainer; + } + + // Update the destination path in the GUI if (showTargetPath) { String path = "/ " + currentContainer.getName() + (uploadRelPath == null ? "" : " / " + uploadRelPath); VFSContainer container = currentContainer.getParentContainer(); diff --git a/src/main/java/org/olat/core/util/vfs/VFSManager.java b/src/main/java/org/olat/core/util/vfs/VFSManager.java index e6b63fb33c9..daf7effc9be 100644 --- a/src/main/java/org/olat/core/util/vfs/VFSManager.java +++ b/src/main/java/org/olat/core/util/vfs/VFSManager.java @@ -42,6 +42,7 @@ import org.olat.core.logging.OLog; import org.olat.core.logging.Tracing; import org.olat.core.manager.BasicManager; import org.olat.core.util.FileUtils; +import org.olat.core.util.StringHelper; import org.olat.core.util.vfs.callbacks.VFSSecurityCallback; import org.olat.core.util.vfs.util.ContainerAndFile; @@ -183,6 +184,123 @@ public class VFSManager extends BasicManager { return null; } + /** + * Resolves a directory path in the base container or creates this + * directory. The method creates any missing directories. + * + * @param baseContainer + * The base directory. User must have write permissions on this + * container + * @param relContainerPath + * The path relative to the base container. Must start with a + * '/'. To separate sub directories use '/' + * @return The resolved or created container or NULL if a problem happened + */ + public static VFSContainer resolveOrCreateContainerFromPath(VFSContainer baseContainer, String relContainerPath) { + VFSContainer resultContainer = baseContainer; + if (!VFSConstants.YES.equals(baseContainer.canWrite())) { + log.error("Could not create relPath::" + relContainerPath + ", base container::" + getRealPath(baseContainer) + " not writable", null); + resultContainer = null; + } else if (StringHelper.containsNonWhitespace(relContainerPath)){ + // Try to resolve given rel path from current container + VFSItem resolvedPath = baseContainer.resolve(relContainerPath.trim()); + if (resolvedPath == null) { + // Does not yet exist - create subdir + String[] pathSegments = relContainerPath.split("/"); + for (int i = 0; i < pathSegments.length; i++) { + String segment = pathSegments[i].trim(); + if (StringHelper.containsNonWhitespace(segment)) { + resolvedPath = resultContainer.resolve(segment); + if (resolvedPath == null) { + resultContainer = resultContainer.createChildContainer(segment); + if (resultContainer == null) { + log.error("Could not create container with name::" + segment + " in relPath::" + relContainerPath + " in base container::" + getRealPath(baseContainer), null); + break; + } + } else { + if (resolvedPath instanceof VFSContainer) { + resultContainer = (VFSContainer) resolvedPath; + } else { + resultContainer = null; + log.error("Could not create container with name::" + segment + " in relPath::" + relContainerPath + ", a file with this name exists (but not a directory) in base container::" + getRealPath(baseContainer), null); + break; + } + } + } + } + } else { + // Parent dir already exists, make sure this is really a container and not a file! + if (resolvedPath instanceof VFSContainer) { + resultContainer = (VFSContainer) resolvedPath; + } else { + resultContainer = null; + log.error("Could not create relPath::" + relContainerPath + ", a file with this name exists (but not a directory) in base container::" + getRealPath(baseContainer), null); + } + + } + } + return resultContainer; + } + + /** + * Resolves a file path in the base container or creates this file under the + * given path. The method creates any missing directories. + * + * @param baseContainer + * The base directory. User must have write permissions on this + * container + * @param relFilePath + * The path relative to the base container. Must start with a + * '/'. To separate sub directories use '/' + * @return The resolved or created leaf or NULL if a problem happened + */ + public static VFSLeaf resolveOrCreateLeafFromPath(VFSContainer baseContainer, String relFilePath) { + if (StringHelper.containsNonWhitespace(relFilePath)) { + int lastSlash = relFilePath.lastIndexOf("/"); + String relDirPath = relFilePath; + String fileName = null; + if (lastSlash == -1) { + // relFilePath is the file name - no directories involved + relDirPath = null; + fileName = relFilePath; + } if (lastSlash == 0) { + // Remove start slash from file name + relDirPath = null; + fileName = relFilePath.substring(1, relFilePath.length()); + } else { + relDirPath = relFilePath.substring(0, lastSlash-1); + fileName = relFilePath.substring(lastSlash); + } + + // Create missing directories and set parent dir for later file creation + VFSContainer parent = baseContainer; + if (StringHelper.containsNonWhitespace(relDirPath)) { + parent = resolveOrCreateContainerFromPath(baseContainer, relDirPath); + } + // Now create file in that dir + if (StringHelper.containsNonWhitespace(fileName)) { + VFSLeaf leaf = null; + VFSItem resolvedFile = parent.resolve(fileName); + if (resolvedFile == null) { + leaf = parent.createChildLeaf(fileName); + if (leaf == null) { + log.error("Could not create leaf with relPath::" + relFilePath + " in base container::" + getRealPath(baseContainer), null); + } + } else { + if (resolvedFile instanceof VFSLeaf) { + leaf = (VFSLeaf) resolvedFile; + } else { + leaf = null; + log.error("Could not create relPath::" + relFilePath + ", a directory with this name exists (but not a file) in base container::" + getRealPath(baseContainer), null); + } + } + return leaf; + } + } + return null; + } + + /** * Get the security callback which affects this item. This searches up the path * of parents to see wether it can find any callback. If no callback -- GitLab