From 06637365bcc5279a4cf41d4bbef2245bd5ec01d2 Mon Sep 17 00:00:00 2001 From: Fabrizzio Araya <37148755+fabrizzio-dotCMS@users.noreply.github.com> Date: Mon, 29 Jul 2024 15:44:19 -0600 Subject: [PATCH] fix(CLI) Refs: #28593 (#29353) ### Proposed Changes 1. When the` files push ` command gets called from the global push, passing an existing folder but outside the files folder an error is thrown 2. This change makes the command default to the workspace root folder and continues processing when the scenario described in the first point occurs --- .../java/com/dotcms/common/AssetsUtils.java | 34 ++++++--------- .../com/dotcms/cli/common/FilesUtils.java | 16 +++++++ .../cli/command/files/FilesPushCommandIT.java | 43 +++++++++++++++++++ 3 files changed, 72 insertions(+), 21 deletions(-) diff --git a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/common/AssetsUtils.java b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/common/AssetsUtils.java index f4b5c4305f52..703a4d6615b6 100644 --- a/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/common/AssetsUtils.java +++ b/tools/dotcms-cli/api-data-model/src/main/java/com/dotcms/common/AssetsUtils.java @@ -1,8 +1,8 @@ package com.dotcms.common; import static com.dotcms.common.LocationUtils.encodePath; -import static com.dotcms.model.config.Workspace.FILES_NAMESPACE; import static com.dotcms.common.WorkspaceManager.resolvePath; +import static com.dotcms.model.config.Workspace.FILES_NAMESPACE; import com.dotcms.model.asset.AbstractAssetSync.PushType; import com.dotcms.model.asset.AssetSync; @@ -10,8 +10,6 @@ import com.dotcms.model.asset.FolderSync; import com.dotcms.model.asset.FolderView; import com.google.common.base.Strings; -import java.util.function.Function; -import java.util.Map; import java.io.File; import java.net.URI; import java.net.URISyntaxException; @@ -20,13 +18,19 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.Optional; +import java.util.function.Function; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class AssetsUtils { private static final String STATUS_LIVE = "live"; private static final String STATUS_WORKING = "working"; + private static final Logger logger = LoggerFactory.getLogger(AssetsUtils.class); + private AssetsUtils() { //Hide public constructor } @@ -194,25 +198,13 @@ public static List parseRootPaths(final File workspace, final File sourc throw new IllegalArgumentException("Source path cannot be outside of the workspace"); } + final Path filesPath = Path.of(workspace.getAbsolutePath(), FILES_NAMESPACE) + .toAbsolutePath().normalize(); // Check if we are inside the workspace but also inside the files folder - if (sourceCount > workspaceCount + 1) { - final Path files = Path.of(workspace.getAbsolutePath(), FILES_NAMESPACE) - .toAbsolutePath().normalize(); - if (!sourcePath.startsWith(files)) { - throw new IllegalArgumentException( - String.format( - "Invalid source path [%s]. Source path must be inside the files folder or " - + - "at the root of the workspace [%s] ", sourcePath, - workspacePath)); - } - } else if (sourceCount == workspaceCount + 1) { - final Path files = Path.of(FILES_NAMESPACE).toAbsolutePath().normalize(); - if (!sourcePath.startsWith(files)) { - throw new IllegalArgumentException( - "Invalid source path. Source path must be inside the files folder or " + - "at the root of the workspace"); - } + if (sourceCount > workspaceCount + 1 || (sourceCount == workspaceCount + 1 && !sourcePath.startsWith(filesPath))) { + logger.warn("Invalid source path provided for a files push {}. Source path must be inside the files folder. otherwise it will fall back to workspace. {}", sourcePath, workspacePath); + //if a source path is provided, but it is not inside the files folder but still is a valid folder then we will fall back to the workspace + return parseRootPaths(workspacePath, workspaceCount, workspaceCount); } return parseRootPaths(sourcePath, workspaceCount, sourceCount); diff --git a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/common/FilesUtils.java b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/common/FilesUtils.java index ee1ab2838a96..c97f6952d02d 100644 --- a/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/common/FilesUtils.java +++ b/tools/dotcms-cli/cli/src/main/java/com/dotcms/cli/common/FilesUtils.java @@ -1,6 +1,10 @@ package com.dotcms.cli.common; import com.dotcms.model.language.Language; +import java.io.IOException; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Arrays; import java.util.List; import java.util.Set; @@ -61,4 +65,16 @@ public static String cleanFileName(final String badFileName) { return cleanName.toString(); } + /** + * Checks if the specified directory is not empty. + * @param path the directory to check + * @return true if the directory is not empty, false otherwise + * @throws IOException if an I/O error occurs + */ + public static boolean isDirectoryNotEmpty(Path path) throws IOException { + try (DirectoryStream directoryStream = Files.newDirectoryStream(path)) { + return directoryStream.iterator().hasNext(); + } + } + } diff --git a/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/files/FilesPushCommandIT.java b/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/files/FilesPushCommandIT.java index a4f6f8c973ea..d0828474a778 100644 --- a/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/files/FilesPushCommandIT.java +++ b/tools/dotcms-cli/cli/src/test/java/com/dotcms/cli/command/files/FilesPushCommandIT.java @@ -1,5 +1,6 @@ package com.dotcms.cli.command.files; +import static com.dotcms.cli.common.FilesUtils.isDirectoryNotEmpty; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -22,6 +23,7 @@ import org.mockito.InjectMocks; import org.mockito.Mockito; import picocli.CommandLine; +import picocli.CommandLine.ExitCode; @QuarkusTest @TestProfile(DotCMSITProfile.class) @@ -363,4 +365,45 @@ void testPushRetryAttempts() throws IOException { deleteTempDirectory(tempFolder); } } + + /** + * Given Scenario: Pull down a workspace doesn't matter if it is empty, + * Call the push command with a path that doesn't match any files folder but exists within the workspace. + * Expected Result: When the command is called passing a folder that exists within the workspace but its outside files it should return OK cause we default to the workspace root + * If the command is called with a path that doesn't match any files folder in the workspace, it should return OK. + * + * @throws IOException If an I/O error occurs. + */ + @Test + void testPushNonFilesMatchingPath() throws IOException { + + // Create a temporal folder for the pull + var tempFolder = createTempFolder(); + final CommandLine commandLine = createCommand(); + final StringWriter writer = new StringWriter(); + try (PrintWriter out = new PrintWriter(writer)) { + //Pull down a workspace if empty + commandLine.setOut(out); + final String path = String.format("//%s", "default"); + int status = commandLine.execute(FilesCommand.NAME, FilesPull.NAME, path, "--workspace", + tempFolder.toString()); + Assertions.assertEquals(CommandLine.ExitCode.OK, status); + + status = commandLine.execute(FilesCommand.NAME, FilesPush.NAME, "--workspace", + tempFolder.toAbsolutePath().toString(), "content-types"); + Assertions.assertEquals(CommandLine.ExitCode.OK, status); + + Assertions.assertTrue(isDirectoryNotEmpty(tempFolder)); + + //But if called with a path that doesn't match any files folder in the workspace + status = commandLine.execute(FilesCommand.NAME, FilesPush.NAME, "--workspace", + tempFolder.toAbsolutePath().toString(), "non-existing-folder"); + Assertions.assertEquals(ExitCode.SOFTWARE, status); + + } finally { + deleteTempDirectory(tempFolder); + } + } + + }